Skip to content

builder, util: fix symlinked vmodules import and temp dir v.mod pollution#27281

Open
guweigang wants to merge 1 commit into
vlang:masterfrom
guweigang:fix-vmodules-symlink-import
Open

builder, util: fix symlinked vmodules import and temp dir v.mod pollution#27281
guweigang wants to merge 1 commit into
vlang:masterfrom
guweigang:fix-vmodules-symlink-import

Conversation

@guweigang
Copy link
Copy Markdown
Contributor

Summary

Fix two module resolution issues:

  1. Symlinked vmodules import: Modules installed as symlinks inside .vmodules (e.g. .vmodules/pkg -> /real/pkg) were incorrectly classified as foreign projects, causing import failures.

  2. 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 unrelated v.mod from a parent temp directory, causing the module name to include the full temp path with uppercase ULID characters. This triggered false cannot contain uppercase letters checker errors.

Changes

vlib/v/builder/builder.v

  • Added comparable_path (normalizes without resolving symlinks) alongside existing comparable_real_path
  • candidate_belongs_to_foreign_project now checks both resolved and unresolved paths, so symlinked modules inside lookup paths are recognized as belonging to the project

vlib/v/util/module.v

  • Extracted is_unrelated_vmod_in_temp_dir helper to deduplicate temp directory detection logic
  • project_root_vmod_folder now respects .v.mod.stop and .git as project boundaries
  • mod_path_to_full_name rejects v.mod files 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.v

  • Added test_vmodules_symlinked_package_import_compiles regression test

Tests

All originally failing tests now pass:

  • hyphenated_module_path_test
  • local_module_submodules_test
  • projects_that_should_compile_test
  • run_project_folders_test
  • coutput_test
  • vmodules_package_compile_regression_test (including new symlink test)
  • compiler_errors_test (1573/1573)
  • All module_* / import_* tests (7/7)

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 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".

Comment thread vlib/v/util/module.v Outdated
@guweigang guweigang force-pushed the fix-vmodules-symlink-import branch from 21bb290 to 5324b83 Compare May 28, 2026 12:20
@medvednikov
Copy link
Copy Markdown
Member

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 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".

Comment thread vlib/v/util/module.v Outdated
…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 guweigang force-pushed the fix-vmodules-symlink-import branch from 5324b83 to 48fbd88 Compare May 31, 2026 10:17
@einar-hjortdal
Copy link
Copy Markdown
Contributor

einar-hjortdal commented May 31, 2026

I am stuck due to builder error: cannot import module "einar_hjortdal.luuid" (not found) errors (symlinked modules in .vmodules), if this fixes them I really need it 🙏

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.

3 participants