make-derivation: Don't add host-suffix to fixed-output derivations names

[dtzWill]

Not only does the suffix unnecessarily reduce sharing, but it also breaks
unpacker setup hooks (e.g. that of unzip) which identify interesting tarballs
using the file extension.

Motivation for this change

Fixes #32986.

(another from #30882!)

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Fits CONTRIBUTING.md.

---

[Ericson2314]

Ah so this part isn't actually true. Say we are building compiler-rt; it run-time depends on nothing, but cares about the host platform as much as any other platform.

In that thread, I instead meant that separately we should do a check like:

let
  fixedOutputDrv = args ? outputHash;
  noNonNativeDeps = builtins.length (depsBuildTarget ++ depsBuildTargetPropagated
                                  ++ depsHostHost ++ depsHostHostPropagated
                                  ++ buildInputs ++ propagatedBuildInputs
                                  ++ depsTargetTarget ++ depsTargetTargetPropagated) == 0;
in
assert fixedOutputDrv -> noNonNativeDeps;

[Ericson2314]

Technically, even that isn't always necessarily true. Yes, any of those deps mean we now need host- or target-platform-specific hashes, but for pre-built stuff (bootstrapping, proprietary, etc) we do just that. So I suppose pre-built stuff could have such deps. But unless all transitive deps were also fixed-output, it would be a total pain to keep the hash correct.

In practice, for the platform-specific deps we do use, run-time deps are hooked up in a downstream deriving, e.g. with substituteAll or makeWrapper, so I don't there's a practical reason not to have that assertion. But it would be fine to leave it for a separate PR if you like, as it might require fixing miscategorized deps / is a separate issue from this one.

[Ericson2314]

Oh also with this change, we can remove hacks in https://github.com/NixOS/nixpkgs/blob/master/pkgs/top-level/splice.nix#L34-L40 and knock off that todo.

Sorry the conversation wasn't the clearest thing to follow, but thanks for tackling this!

[Ericson2314]

Lastly, not sure weather its worth it until we go back to staging mass rebuilds, but I've been trying to linearizing the history of my PRs under https://github.com/obsidiansystems/nixpkgs/tree/ericson2314-cross-master, for easy diffing and bisecting if anything goes wrong. I just rebased this on top of that.

I fixed my own issues

[Ericson2314]

phobosUnittests is a very odd thing I'm not sure what to do about.

[dtzWill]

@Ericson2314 sounds like a test that should be disabled on cross! Right?

[Ericson2314]

@dtzWill well I disabled fixed output derivations with run-time dependencies across the board. But those test drvs' deps shouldn't be nativeBuildInputs because they're run-time dependencies....of the thing which isn't actually being built because we just run the tests and through everything away.
They're fixed output derivations which are more or less just being used for their side affects.

[Ericson2314]

I suppose while we decide whether or not to reflex the restriction, we can still clean up the deps that unequivocally need it #33681 .

[vcunat]

I disabled fixed output derivations with run-time dependencies across the board.

This confuses me. I don't think nix allows fixed-output derivations to retain any rutime references. EDIT: ah, eval-time check for buildInputs, right...

[Ericson2314]

@vcunat what do you think now? Switched back to keeping hash with run-time deps, and just making a warning.

[vcunat]

Changes in laziness of derivation attributes #33057 conflicted this, and it makes me wonder why you chose to do this warning so strictly. I think it's just another way of broken-ness and would better be evaluated in the same "cases" (i.e. with the same "laziness"). Maybe it would even be worth to move it to ./check-meta.nix, even though it's not about meta – but we haven't checked anything else so far, so perhaps just the name doesn't fit.

[dtzWill]

(I'm more or less disowning this PR, looks like @Ericson2314 is leading the way, thank you! 👍)

[Ericson2314]

I believe this about the hardening flags, but it's quite unclear so I just removed it.

[Ericson2314]

I just skipped the warning for now, avoiding what @vcunat was concerned about entirely.

[edolstra]

What is the impact of this change on evaluation performance?

(Asking because there has been a pretty awful slowdown in Nixpkgs evaluation speed in the last year, and I suspect it's due to all the stdenv changes.)

[Ericson2314]

@edolstra I did think about that here. There should be no effect on native, since the && will short circuit. [On cross it's unclear, with the length check per derivation, but the splicing nasty fetcher filtering removed.]

[Ericson2314]

@edolstra this is the && I'm referring to.