sourcehut: fix build

[winterqt]

Description of changes

Fixes #198478.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• 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/)
• 22.11 Release Notes (or backporting 22.05 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
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[tjni]

Thank you so much for tracking this down. It makes a lot of sense once I see the change, but it's quite subtle.

[tjni]

I'm really impressed by the mental leap you took to realize this was the problem.

Have a few suggestions coming from someone who didn't figure this out, which could be more representative of a future reader of this derivation. Nothing mandatory, just opinion.

1. What do you think of calling this coreSrhtNativeBuildInputs? This emphasizes that the source of the inputs is the core.sr.ht repo. Or, sharedSrhtNativeBuildInputs, which speaks to the purpose of the core.sr.ht repo as a container for "code shared among all sr.ht projects" (c.f. the README).
2. Moving this comment to the definition of nativeBuildInputs a few lines above can be better. I feel that, aside from explanation, the main impact of the comment is to discourage depending on nativeBuildInputs directly, which makes sense when next to the property itself. It could offer a warning to not use it because [insert your explanation of non-idempotent hooks being added more than once].

[winterqt]

1. What do you think of calling this coreSrhtNativeBuildInputs? This emphasizes that the source of the inputs is the core.sr.ht repo. Or, sharedSrhtNativeBuildInputs, which speaks to the purpose of the core.sr.ht repo as a container for "code shared among all sr.ht projects" (c.f. the README).

I like the sound of sharedNativeBuildInputs (which in usage would be srht.sharedNativeBuildInputs), thanks!

2. Moving this comment to the definition of nativeBuildInputs a few lines above can be better. I feel that, aside from explanation, the main impact of the comment is to discourage depending on nativeBuildInputs directly, which makes sense when next to the property itself. It could offer a warning to not use it because [insert your explanation of non-idempotent hooks being added more than once].

I've moved the comment and reworded it a bit -- let me know what you think. (Is the passthru fine where it is, though? They're so far apart, so not sure.)

[tjni]

I think it's fine.

[bjornfor]

Commit (and PR) message typo: sourehut -> sourcehut

[winterqt]

Commit (and PR) message typo: sourehut -> sourcehut

Fixed, thanks.

[tjni]

LGTM

[winterqt]

Made lines on the explanation comment more even.

[winterqt]

(Realized I should probably get @ju1m's input as well before merging.)

[winterqt]

I ran these locally, but just to show it here...
@ofborg test sourcehut

[ju1m]

@winterqt, many thanks for that fix. I don't know the context in which this manual passing around of dependencies was introduced to begin with, but I think using propagatedNativeBuildInputs is cleaner. Please see #198720 for a proposal

[winterqt]

Closing in favor of #198720.