ruby: re-enable yjit, and fix, on cross#461684
Conversation
|
I can fold back the |
nim65s
left a comment
There was a problem hiding this comment.
Great, thanks !
Diff LGTM, and I can confirm this fix the issue that got me to deactivate this, so I'm happy for the revert
| @@ -181,6 +174,13 @@ let | |||
| ]; | |||
| propagatedBuildInputs = op jemallocSupport jemalloc; | |||
|
|
|||
| env = lib.optionalAttrs (stdenv.hostPlatform != stdenv.buildPlatform && yjitSupport) { | |||
There was a problem hiding this comment.
i think this is cause of mass rebuild, can it be split ? if not, this should go to staging
There was a problem hiding this comment.
Not in this case, it's the removal of cargo from the inputs that changes the ruby derivation.
The env input attribute in mkDerivation is special-cased and when it's an empty set, it has no effect.
nix-repl> stdenv.mkDerivation { name = "foo"; }
«derivation /nix/store/w0ps5bzjcc5x3xcv8b327jfzmd5k5nsp-foo.drv»
nix-repl> stdenv.mkDerivation { name = "foo"; env = lib.optionalAttrs false { x = "x"; }; }
«derivation /nix/store/w0ps5bzjcc5x3xcv8b327jfzmd5k5nsp-foo.drv»
nix-repl> stdenv.mkDerivation { name = "foo"; env = lib.optionalAttrs true { x = "x"; }; }
«derivation /nix/store/216b6l77hb1n2s6n6n7asfdj4b49qcg4-foo.drv»
I'll split that (cargo) change out if ruby cross-compiles fine without that change.
This reverts commit 4fce4c9.
2db1dcf to
26ddea6
Compare
Otherwise it will produce `yjit.a` with the default target for the `rustc` compiler, which in turn turns out to be the build platform? It could be configured using the `RUST` environment variable, and since it's used without quoting it would expand the arguments added to it. Using `RUST = "rust --target ..."` I think would be undesirable for cross-compiling (and native), since it will save those arguments in `lib/ruby/*/*/rbconfig.rb`. Though that might be fine too. I guess. Note that 3.2 breaks differently. I haven't investigated it, since it's not the default anymore, and will eventually be dropped. Signed-off-by: Samuel Dionne-Riel <samuel@dionne-riel.com> (cherry picked from commit 75d6216)
|
(Previous force-push is a simple rebase on top of current |
|
With the following force-push'd changes: And it still cross-compiles. |
26ddea6 to
c286dcb
Compare
This PR fixes yjit support, after re-enabling it (see: #449938).
This was authored sometimes last year when working on fixing Mobile NixOS cross-compilation issues with Ruby. I never got around to sending the fix.
Currently verified with:
For the record, before this change:
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.