Skip to content
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

rust-overlay: restore SYSROOT detection for Darwin #309

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ggreif
Copy link

@ggreif ggreif commented Apr 25, 2023

f6fe850 broke Darwin (fixes #304).
This PR:

  • removes rm before cp (and uses --remove-destination instead)
  • copies librustc_driver-*.dylib too

Takes some cues from #306 (and supersedes it).

@ggreif ggreif changed the title Restore SYSROOT detection for Darwin rust-overlay: restore SYSROOT detection for Darwin Apr 25, 2023
@ggreif ggreif marked this pull request as draft April 25, 2023 23:25
ggreif added a commit to dfinity/motoko that referenced this pull request Apr 26, 2023
and a few deps too
needs patched toolset from mozilla (PRs submitted:
mozilla/nixpkgs-mozilla#309)
@ggreif ggreif marked this pull request as ready for review April 26, 2023 06:51
mergify bot pushed a commit to dfinity/motoko that referenced this pull request May 24, 2023
and a few deps too:
- `wasi-libc`

needs patched toolset from mozilla (PRs submitted: mozilla/nixpkgs-mozilla#309)

TODO:
- [x] `motoko-rts-test` fails in `stream flushing` with unaligned memory access — see #3981
@AliSajid
Copy link

AliSajid commented Jun 2, 2023

Bump for attention.

@nbp
Copy link
Collaborator

nbp commented Jun 26, 2023

Bump for attention.

Unfortunately, this does not work this way …
You can always ping me on https://matrix.to/#/#mozilla:nixos.org .

rust-overlay.nix Outdated
Comment on lines 301 to 305
# Here we copy the librustc_driver-*.{so,dylib} to our derivation.
# The SYSROOT is determined based on the path of this library.
if ls $out/lib/librustc_driver-*.so &> /dev/null; then
RUSTC_DRIVER_PATH=$(realpath -e $out/lib/librustc_driver-*.so)
rm $out/lib/librustc_driver-*.so
cp $RUSTC_DRIVER_PATH $out/lib/
fi
shopt -s nullglob
RUSTC_DRIVER_PATH=$(realpath -e $out/lib/librustc_driver-*.{so,dylib})
cp --remove-destination $RUSTC_DRIVER_PATH $out/lib/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this file be copied before the loop which is using conditionally checking whether this file exists before using patchelf?

@ggreif
Copy link
Author

ggreif commented Jul 28, 2023

Looks like #304 has fixed this, so closing for now.

@ggreif
Copy link
Author

ggreif commented Jul 31, 2023

Unfortunately .dylibs are not considered in #304 yet, so reopening.

@ggreif ggreif reopened this Jul 31, 2023
Copy link
Contributor

@Artturin Artturin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's stdenv.hostPlatform.extensions.sharedLibrary so no need for duplication

…ed copying

this is review feedback, thanks!
@ggreif
Copy link
Author

ggreif commented Aug 1, 2023

@AliSajid I am using this branch as pull/309/head and it works for both Linux and Darwin in our setup.

@ggreif
Copy link
Author

ggreif commented Aug 11, 2023

@Artturin @nbp This is now a very mild delta, any chance to review?

Copy link
Contributor

@Artturin Artturin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't use this overlay nor can I merge but the code looks ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Building rustChannelOf({...}).rust-src from rust-overlay.nix errors.
4 participants