-
Notifications
You must be signed in to change notification settings - Fork 462
Implement --target-exec #13112
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
Implement --target-exec #13112
Conversation
7bc90ff to
ecdfa3c
Compare
|
I did more testing and |
03ef4ee to
30ba98e
Compare
|
Q: I haven't had a good look at this yet, but why don't you just modify how the |
That's what it's doing indeed, see: https://github.com/ocaml/dune/pull/13112/files#diff-bb90136ac4be87c54face4d7dd2a55b65ce978a4497d19d53eb51350bb613c50R744 However, in cross-compilation context, there is still a need to ensure that some The pattern follows the existing |
30ba98e to
a587cb8
Compare
|
Is I think it's ok to accept this, but we need to communicate to users that this is experimental and unstable and that if we find a better way to do this, it is likely that the old way will be removed. |
|
@rgrinberg happy to add it to Can you expand on the unstable? I do think that the way |
|
@rgrinberg Just pushed support for |
cde07b3 to
96debe1
Compare
96debe1 to
3225167
Compare
3225167 to
b034cfb
Compare
|
@Alizter @rgrinberg I'm not sure why the Nix tests are failing. Is that something I should take care of? |
|
Those aren't nix-specific but failures in the test-suite. Could you promote the changes in |
b034cfb to
f3ab07f
Compare
|
Let me know if you need assistance, I can push the test changes here for you. |
69fb215 to
3082b9f
Compare
Signed-off-by: Romain Beauxis <romain.beauxis@gmail.com>
Signed-off-by: Romain Beauxis <romain.beauxis@gmail.com>
Signed-off-by: Romain Beauxis <romain.beauxis@gmail.com>
Signed-off-by: Romain Beauxis <romain.beauxis@gmail.com>
Signed-off-by: Romain Beauxis <romain.beauxis@gmail.com>
Signed-off-by: Romain Beauxis <romain.beauxis@gmail.com>
3082b9f to
1cd9189
Compare
|
@Alizter should be good now! |
rgrinberg
left a 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.
The name runexec doesn't seem particularly great to me. I might change it to run_host before we release this.
Anyway, the PR is good to go. Just needs to be bumped to 3.22 as 3.21 is already being released.
I have no strong opinion on the naming but I wanted to point out that But I agree with you that Should we do |
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
|
I agree that we need a consistent treatment for all of this. This is why I'm not particularly keen on making all of this stable. |
This PR implements runing a target binary from the host following the proposal at: #13052
--target-exec <toolchain>=<wrapper>dune-workspace:DUNE_TARGET_EXECandDUNE_CROSS_TARGETto specify--target-exec(resp.-x) through environment variablesrunexecaction that bypasses the target exec wrapper when needed.This is intended for cross-compilation only at the moment so the wrapper is only added in those cases.
Implementation details
Most of the implementation complexity is at the wrapper substitution level and mostly because it changes the relationship between program and dependency but also because it ties up the program and arguments together.
To help with that, the signature of
map_exeis changed to include arguments and return dependency, program and arguments. The rest of the downstream code is adapted accordingly while trying to keep the APIs as similar as possible.Example
Here's a PR with a complex build system that was lifted to do fully transparent cross-build: savonet/ocaml-posix#24
All you have to do it run:
and it cross-builds for windows using the exact same build setup as for native compilation.
Future work
The long-term goal is to lift this to
opamand allow transparent cross-building using the same dune-based native package.Something like:
And use the above environment variables to do a cross-build.
This should allow cross-building opam repository such as https://github.com/ocaml-cross/opam-cross-windows/ to drop all crossbuild enabled packages and directly use the native opam packages.