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: raise when virtual library / implementations are in the same directory #6553

Merged
merged 2 commits into from
Nov 24, 2022

Conversation

anmonteiro
Copy link
Collaborator

This is a follow-up to #6550

@@ -217,7 +217,7 @@ let modules t ~for_ = modules_and_obj_dir t ~for_ |> fst

let find_origin (t : t) name = Module_name.Map.find t.modules.rev_map name

let virtual_modules lookup_vlib vlib =
let virtual_modules ~lookup_vlib vlib =
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is a drive-by refactoring that slipped through. Happy to revert.

@anmonteiro
Copy link
Collaborator Author

Addressed both review comments.

@anmonteiro
Copy link
Collaborator Author

Does this need a changelog entry?

davesnx and others added 2 commits November 24, 2022 00:16
add a test for a virtual library and its implementation defined in the
same directory.

currently this results in an ugly error crash

Signed-off-by: David Sancho Moreno <dsnxmoreno@gmail.com>
introduce proper error message when virtual library / implementations
are in the same directory

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
@rgrinberg
Copy link
Member

It's just an error message so it's fine to not include it.

@rgrinberg rgrinberg merged commit 1583e10 into ocaml:main Nov 24, 2022
@rgrinberg rgrinberg added this to the 3.7.0 milestone Nov 24, 2022
@anmonteiro anmonteiro deleted the anmonteiro/bug-virtual-libs branch November 24, 2022 05:28
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.

None yet

3 participants