Repository navigation
builder: skip foreign v.mod project dirs when resolving imports (fix #27151) - #27231
Conversation
…27151) When V is invoked from inside another project (e.g. coreutils) that contains a sibling directory whose name matches a vlib module (like `src/sync/`), `find_module_path` would pick that user directory before trying vlib, and parsing then failed with "bad module definition" because the user's files declare `module main`. Skip candidate directories whose enclosing `v.mod` belongs to a different project than the importer, unless the candidate sits inside a configured `lookup_path` entry (vlib / vmodules).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2a4dfc70b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for lookup in b.pref.lookup_path { | ||
| abs_lookup := os.real_path(lookup) | ||
| if abs_candidate == abs_lookup | ||
| || abs_candidate.starts_with(abs_lookup + os.path_separator) { | ||
| return false |
There was a problem hiding this comment.
modules/ deps as foreign
candidate_belongs_to_foreign_project exempts only pref.lookup_path, but module resolution also searches non-lookup roots like <project>/modules (added by set_module_lookup_paths in vlib/v/builder/compile.v). When a dependency in modules/<name>/ has its own v.mod (common for vendored VPM packages), get_by_folder reports that nested v.mod root, which differs from the importer root, so this code marks it as foreign and skips it. That regresses valid imports that previously resolved from the project's modules/ directory.
Useful? React with 👍 / 👎.
Compare the candidate path against the importer's `v.mod` tree directly instead of comparing the nearest enclosing `v.mod` folders. A vendored dependency at `<project>/modules/<name>/` carries its own `v.mod`, so the previous check classified it as a "foreign project" and skipped it, breaking imports from `<project>`'s own code. Added a regression test alongside the #27151 test.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0381e5d420
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| importer_vmod_folder := if vmod_file_location.vmod_file.len != 0 { | ||
| vmod_file_location.vmod_folder | ||
| } else { |
There was a problem hiding this comment.
find_module_path now sets importer_vmod_folder to the nearest v.mod folder, so for files inside a vendored package (e.g. <project>/modules/foo/...) the anchor becomes <project>/modules/foo. In that context, imports of sibling vendored dependencies (<project>/modules/bar) are treated as foreign by candidate_belongs_to_foreign_project (they are outside foo and not in pref.lookup_path), so transitive vendored imports can fail with module ... not found. This is a regression introduced by the new foreign-project filter for projects that vendor multiple dependent modules under the same top-level modules/ directory.
Useful? React with 👍 / 👎.
Vendored sibling dependencies under <project>/modules/ each carry their own v.mod. Using the nearest enclosing v.mod as the importer's anchor made the foreign-project check treat sibling vendored modules as foreign and refuse to resolve them. Walk up to the outermost enclosing v.mod instead, so the importer is anchored to the top-level project root. Adds a regression test where modules/foo (own v.mod) imports modules/bar (own v.mod).
Summary
vlang/coreutils) that contains a sibling directory whose name matches a vlib module (likesrc/sync/),find_module_pathwould pick that user directory before trying vlib, and parsing then failed withbad module definition: ... imports module "sync" but ...src/sync/sync.v is defined as module+ "main" + `.Builder.candidate_belongs_to_foreign_projectand use it in both lookup loops offind_module_pathto skip candidates whose enclosingv.modbelongs to a different project than the importer, unless the candidate sits inside a configuredlookup_pathentry (vlib / vmodules).vlib/v/builder/builder_test.vthat constructs a fake user project with asrc/sync/ofmodule mainfiles and-checks a file that importsv.ast(which transitively importssyncviashared).Test plan
v -d trace_same_results -stats src/uptime/uptime_nix_test.vinside a freshvlang/coreutilsclone.bad module definitionerror for that command../v selfsucceeds and a second self-host pass succeeds.test_vlib_import_does_not_match_user_project_dirtest invlib/v/builder/builder_test.vfails on master and passes with this change.vlib/v/builder/base_url_test.vstill passes; thevlib/v/checker/tests/modules/checker fixtures still resolve their imports.