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

fix: symlink .impl.d files for virtual library modules too #10289

Conversation

anmonteiro
Copy link
Collaborator

  • when setting up rules for implementation files in a virtual library, their artifacts are symlinked to the object directory for the concrete implementation too
  • this change adds rules to symlink the ocamldep resulting files too, as they may be needed in the object directory
  • specifically, this fixes the failing test in fix(melange): track immediate .cmj deps as dependencies of JS rules #10286 as will be demonstrated once I cherry-pick this commit in that PR.

@anmonteiro anmonteiro requested a review from rgrinberg March 20, 2024 02:58
@anmonteiro anmonteiro force-pushed the anmonteiro/copy-ocamldep-files-for-vlib-rules branch 2 times, most recently from ef06fca to 412ad73 Compare March 20, 2024 03:09
@rgrinberg
Copy link
Member

You say that they may be needed, but more concretely this is needed for melange only? I don't think this is needed anywhere else because I just merge the dependency graphs for vlibs and impls everywhere else and use that.

@anmonteiro anmonteiro force-pushed the anmonteiro/copy-ocamldep-files-for-vlib-rules branch from 412ad73 to 7531778 Compare March 20, 2024 21:36
@anmonteiro
Copy link
Collaborator Author

@rgrinberg thanks. I made it conditional on melange.

@rgrinberg
Copy link
Member

Okay, then I'd just include this as a commit in #10286. If it doesn't fix anything on its own, a separate PR isn't really needed.

@anmonteiro
Copy link
Collaborator Author

Sounds good, will incorporate it there.

@anmonteiro anmonteiro closed this Mar 20, 2024
@anmonteiro anmonteiro deleted the anmonteiro/copy-ocamldep-files-for-vlib-rules branch March 21, 2024 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants