builder, util: fix symlinked vmodules import and temp dir v.mod pollution#27281
builder, util: fix symlinked vmodules import and temp dir v.mod pollution#27281guweigang wants to merge 1 commit into
Conversation
![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: 21bb290123
ℹ️ 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".
guweigang
force-pushed
the
fix-vmodules-symlink-import
branch
from
21bb290 to
5324b83
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5324b83122
ℹ️ 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".
guweigang
force-pushed
the
fix-vmodules-symlink-import
branch
from
5324b83 to
48fbd88
Compare
|
I am stuck due to |
…tion builder: also check candidate paths without resolving symlinks, so modules installed as symlinks inside `.vmodules` are recognized as belonging to the project. util: reject v.mod files found in or above the system temp directory when the relative path contains uppercase letters (e.g. ULID-based test session dirs), as they are likely unrelated to the project. Also respect `.v.mod.stop` and `.git` as project boundaries when walking up to find the project root. Add regression test for symlinked vmodules import.
guweigang
force-pushed
the
fix-vmodules-symlink-import
branch
from
48fbd88 to
6eeb8d9
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6eeb8d9dba
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| temp_dir := os.real_path(os.temp_dir()) | ||
| is_in_temp := vmod_folder == temp_dir || vmod_folder.starts_with(temp_dir + os.path_separator) | ||
| || temp_dir.starts_with(vmod_folder + os.path_separator) |
There was a problem hiding this comment.
When the fallback scan passes parent into this helper, that path is sliced from the original source path and is not resolved. On macOS os.temp_dir() returns /tmp but os.real_path(os.temp_dir()) is typically /private/tmp; compiling a temp-session path spelled as /tmp/... therefore makes this comparison return false and still accepts the unrelated /tmp/.../v.mod, so the uppercase temp-session module prefix leak remains. Normalize vmod_folder here, or compare against both temp-dir spellings, before deciding it is outside temp.
Useful? React with 👍 / 👎.
4 participants
Summary
Fix two module resolution issues:
Symlinked vmodules import: Modules installed as symlinks inside
.vmodules(e.g..vmodules/pkg -> /real/pkg) were incorrectly classified as foreign projects, causing import failures.Temp dir v.mod pollution: When tests run in temp directories (e.g.
/tmp/v_501/tsession_<ULID>/...), the module resolution walk could pick up an unrelatedv.modfrom a parent temp directory, causing the module name to include the full temp path with uppercase ULID characters. This triggered falsecannot contain uppercase letterschecker errors.Changes
vlib/v/builder/builder.vcomparable_path(normalizes without resolving symlinks) alongside existingcomparable_real_pathcandidate_belongs_to_foreign_projectnow checks both resolved and unresolved paths, so symlinked modules inside lookup paths are recognized as belonging to the projectvlib/v/util/module.vis_unrelated_vmod_in_temp_dirhelper to deduplicate temp directory detection logicproject_root_vmod_foldernow respects.v.mod.stopand.gitas project boundariesmod_path_to_full_namerejectsv.modfiles in or above the system temp directory when the relative path contains uppercase letters (indicating temp directory names like ULIDs)vlib/v/tests/vmodules_package_compile_regression_test.vtest_vmodules_symlinked_package_import_compilesregression testTests
All originally failing tests now pass:
hyphenated_module_path_testlocal_module_submodules_testprojects_that_should_compile_testrun_project_folders_testcoutput_testvmodules_package_compile_regression_test(including new symlink test)compiler_errors_test(1573/1573)module_*/import_*tests (7/7)