-
Notifications
You must be signed in to change notification settings - Fork 26
Use copy_files instead of symlink for dune to be happy #950
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
Conversation
Signed-off-by: Sacha Ayoun <[email protected]>
| preCheck = | ||
| if doCheck then '' | ||
| ln -sf ${charon}/tests-llbc charon-ml/tests/test-outputs | ||
| '' else | ||
| ""; |
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.
Something like this is needed because in the nix env the test outputs are otherwise not accessible
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.
You cuold rm -r test-outputs first, that should work
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.
Hmm, not sure what that means
Why does it need to be available for nix? Isn't dune taking care of all of this?
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.
In nix we build the rust side and ocaml sides separately. So when we get to building the ocaml side, the llbc files emitted by running the rust tests are not there. We must put them there ourselves, which is what this does.
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.
Anyway I can do the nix bits, but I'm not sure I want two copies to happen on the ocaml side
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.
Aaaaah, got it
Closing this anyway until it works one the dune side
Signed-off-by: Sacha Ayoun <[email protected]>
| (lib.hasPrefix (toString ../charon-ml) path) | ||
| || (lib.hasPrefix (toString ../dune-project) path); | ||
| || (lib.hasPrefix (toString ../dune-project) path) | ||
| || (lib.hasPrefix (toString ../charon/tests/ui) path); |
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.
That's not needed: ocaml only needs .llbc files, none of which are part of the git repo.
|
This still doesn't allow me to use dune package management so closing this for now until it's useful |
I want to use the dune "package management" (which is kind of a replacement for using opam separately). So far, it fails, and it seems to be because of this issue, where dune has issues dealing with symlinks.
This PR updates charon to avoid requiring a symlink for the tests, and uses dune's
copy_filesinstead