-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
openvswitch: fix cross-compilation #68895
Conversation
@@ -5298,7 +5298,7 @@ in | |||
opentracing-cpp = callPackage ../development/libraries/opentracing-cpp { }; | |||
|
|||
openvswitch = callPackage ../os-specific/linux/openvswitch { | |||
openssl = openssl_1_0_2; | |||
openssl = __splicedPackages.openssl_1_0_2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @Ericson2314
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is __splicedPackages
targetPackages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or in other words, what does this change fixes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
openssl_1_0_2
is just a derivation, whereas __splicedPackages.openssl_1_0_2
is augmented with attributes referring to that derivation for other (host, target)
pairs. For example, __splicedPackages.openssl_1_0_2.nativeDrv
refers to buildPackages.openssl_1_0_2
.
See this comment for more detail: #68967 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
openvswitch/default.nix
refers to openssl
in both .nativeBuildInputs
and .buildInputs
. In the case of cross-compilation, mkDerivation
accesses spliced attributes for elements of these lists. So, the openssl
argument to openvswitch/default.nix
must be spliced.
callPackage
automatically uses spliced packages. However, the packages in scope in top-level/all-packages.nix
are not spliced. So, to pass in a spliced openssl_1_0_2
, we must explicitly refer to __splicedPackages.openssl_1_0_2
.
This may not be the best solution to cross-compiling this particular package, but similar situations appear throughout Nixpkgs. I think it's worth considering how we should be handling them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. @Ericson2314 seems to prefer buildPackages
though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with them. This case is a bit more complicated because openvswitch
really takes two distinct openssl
derivations as parameters, one for the build and one to link against, which are different in the case of cross compilation. They suggest what seems like the most sensible solution right now: #68900 (comment). That is, to explicitly allow the overriding of both derivations separately.
Thank you for your contributions.
|
(In response to the automatic marking of this PR as stale) I'm still interested in how cross compilations issues like this one, brought about as a side effect of splicing, are best addressed. See also #68900 |
Sorry I missed this for the better part of a year 😱! It's a clever fix, but I am wary of using splicing explicitly like this as it's such an ugly mechanism with so many problems. Can you rewrite this with an explicit |
Do you have a good solution on how to deal with overrides in all-packages.nix or should we make all overrides there explicit? |
Yes, I think it is just better to make them explicit for now. #58327 did not work to well as it caused a bunch of infinite recursion. This will be more verbose, but ultimately easier to debug I think, and we can point to the the verbosity after the fact as evidence if someone is trying to overhaul |
I marked this as stale due to inactivity. → More info |
openvswitch
depends on both build and hostopenssl
. Both must be overridden inall-packages.nix
. I propose the following:However,
__splicedPackages
is not used in this way anywhere else in nixpkgs.A discussion of this sort of case can be found here: #49526 (comment).
Motivation for this change
Fixes cross-compilation for
openvswitch
.Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @