treewide: remove multiArch=false and wrong usage of extraPkgs

[SuperSamus]

Description of changes

Cleanup after the work of #240860.
Motivation: prevent new packages to copy from outdated examples.
Didn't test.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/3593

[SuperSandro2000]

fyi @Atemu

[Atemu]

This appears to change eval on many of the touched packages, what's the difference?

[SuperSamus]

This appears to change eval on many of the touched packages, what's the difference?

Sorry for the late response, but I don't know.

[Atemu]

Please investigate that, I do not feel comfortable merging this otherwise.

[SuperSamus]

Alright, I found the cause.
Before, some packages were listed twice:

• In multiPkgs (because they were there by default)
• In extraPkgs (because of extraPkgs = pkgs: (appimageTools.defaultFhsEnvArgs.multiPkgs pkgs))

By removing the latter, these packages are listed only once.

Example with Zettlr (I took the pkgs from the derivations and prettified the JSON).

• After PR:
  h3py958xa6rmhv10q9xizlx5y5qlhpqf-zettlr-3.0.2-usr-target.drv.json
• Before PR:
  xjl0fnnwy3ad1czrnk6slci2kf4brp7n-zettlr-3.0.2-usr-target.drv.json

By using a text comparison tool (or a duplicate counter), you can see that "After PR" only removes duplicates.

I only tested Zettlr, but I suppose it's the same for the others.

[Atemu]

Could you squash this into one commit? It's a tree-wide change, so that's one logical step.

[pbsds]

Result of nixpkgs-review pr 279260 run on x86_64-linux 1

13 packages built:

• Sylk
• bloomrpc
• chrysalis
• flexoptix-app
• joplin-desktop
• marktext
• mycrypto
• notable
• plexamp
• polypane
• station
• tusk
• zettlr

[Atemu]

There's a conflict now. Could you also add a commit message that explains the background of the change? The contributor's manual has a section on that. It should also say something to the effect of treewide: remove multiArch=false from buildFHSEnv usages.

Other than that, this LGTM.

[SuperSamus]

So, I just noticed that there are other packages that do extraPkgs = pkgs: appimageTools.defaultFhsEnvArgs.multiPkgs pkgs;, which I didn't notice before because they didn't set multiArch = false; (the only thing I searched for).
I guess I should make a separate commit for that. Or an entirely different PR.

What do you say?

[SuperSamus]

35 packages that either do extraPkgs = pkgs: appimageTools.defaultFhsEnvArgs.multiPkgs pkgs; or accidentally use the top-level input.
With the latter I mean:

{ appimageTools, pandoc }:
appimageTools.wrapType2 {
  # Haa, the joys of being bamboozled by the `with;` keyword!
  # I can say that I've had experience with that, I'm definitively not still angry about the time I wasted for #248898.
  extraPkgs = pkgs: with pkgs; [ pandoc ];
}

Maybe fixing this should be a separate PR.

[Atemu]

I guess I should make a separate commit for that. Or an entirely different PR.

Certainly a separate commit.

It fits the theme of outdated pattern/convention cleanup surrounding buildFHSEnv of this PR and is related, so I wouldn't mind having it in here.

accidentally use the top-level input.

Oh man, time for NixOS/rfcs#110...

[SuperSamus]

So, there are 5 packages that use wrapAppImage instead of wrapType2.
Documentation doesn't mention the existence of the former at all.
Also, while in wrapType2 extraPkgs defaults to [ ], the same doesn't apply to wrapAppImage (which has no default).
It seems to me that wrapAppImage is only meant to be called by wrapType2 and nothing else.

Should I convert all the usages of wrapAppImage to wrapType2?
Or just put extraPkgs ? pkgs: [ ] in wrapAppImage?

[SuperSamus]

I'm waiting for comments before proceeding further.

[pbsds]

extraPkgs ? pkgs: [ ] strikes me as a sane default

[Atemu]

I think we're looking at remnants of previous iterations of this interface where there was support for multiple versions of the appimage format. These days, everything just uses wrapType2 IIRC.

Appimage support in general needs some cleanup.

This LGTM.

[Atemu]

Thank you!