medvednikov

/

vqPublic
0 commits21 issue12 pull requests0 contributorsDiscussionsProjectsCI

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

Openguweigangwants to mergepr/27281intomaster· last May 28
3 files changed+119-1
vlib/v/builder/builder.v+28-1

@@ -846,15 +846,25 @@ fn (b &Builder) candidate_belongs_to_foreign_project(candidate_path string, impo

846846 if candidate_vmod_matches_import(abs_candidate, mod) {

847847 return false

848848 }

849+ candidate_lookup_path := comparable_path(candidate_path)

849850 for lookup in b.pref.lookup_path {

850851 abs_lookup := comparable_real_path(lookup)

851852 if path_is_at_or_inside(abs_candidate, abs_lookup) {

852853 return false

853854 }

855+ // Also check without resolving symlinks, so that a module installed

856+ // as a symlink inside a lookup path (e.g. `.vmodules/pkg -> /real/pkg`)

857+ // is still recognized as belonging to the project.

858+ lookup_path := comparable_path(lookup)

859+ if path_is_at_or_inside(candidate_lookup_path, lookup_path) {

860+ return false

861+ }

854862 }

855863 return true

856864}

857865

866+// path_belongs_to_lookup_path returns true when the given path is at or

867+// inside any of the configured lookup paths.

858868fn (b &Builder) path_belongs_to_lookup_path(path string) bool {

859869 abs_path := comparable_real_path(path)

860870 for lookup in b.pref.lookup_path {

@@ -866,8 +876,25 @@ fn (b &Builder) path_belongs_to_lookup_path(path string) bool {

866876 return false

867877}

868878

879+// comparable_real_path normalizes a path for comparison, resolving symlinks

880+// via os.real_path. Use when both sides of a comparison should refer to the

881+// same physical location on disk.

869882fn comparable_real_path(path string) string {

870- mut normalized := os.real_path(path).replace('\\', '/')

883+ return comparable_path_from(os.real_path(path))

884+}

885+

886+// comparable_path normalizes a path for comparison without resolving symlinks.

887+// Use when the original (logical) path matters, e.g. for symlinked modules

888+// inside `.vmodules` that should match their lookup path as-is.

889+fn comparable_path(path string) string {

890+ return comparable_path_from(os.abs_path(path))

891+}

892+

893+// comparable_path_from normalizes a path string for consistent comparison:

894+// converts backslashes to forward slashes, collapses duplicate separators,

895+// and strips trailing slashes.

896+fn comparable_path_from(path string) string {

897+ mut normalized := path.replace('\\', '/')

871898 for normalized.contains('//') {

872899 normalized = normalized.replace('//', '/')

873900 }

vlib/v/tests/vmodules_package_compile_regression_test.v+47-0

@@ -30,6 +30,32 @@ fn issue_20147_write_file(path string, contents string) {

3030 os.write_file(path, contents) or { panic(err) }

3131}

3232

33+fn vmodules_symlink_workspace() string {

34+ return os.join_path(os.vtmp_dir(), 'vmodules_symlink_import_compile')

35+}

36+

37+fn vmodules_symlink_write_project() ! {

38+ workspace := vmodules_symlink_workspace()

39+ vmodules_dir := os.join_path(workspace, '.vmodules')

40+ real_module_root := os.join_path(workspace, 'real_vmarkdown')

41+ link_module_root := os.join_path(vmodules_dir, 'vmarkdown')

42+ app_root := os.join_path(workspace, 'app')

43+ os.rmdir_all(workspace) or {}

44+ os.mkdir_all(vmodules_dir)!

45+ os.mkdir_all(real_module_root)!

46+ os.mkdir_all(app_root)!

47+ os.symlink(real_module_root, link_module_root)!

48+ module_contents :=

49+ ['module vmarkdown', '', "pub fn ok() string { return 'ok' }"].join_lines() + '\n'

50+ app_vmod_contents := ['Module {', "\tname: 'app'", '}'].join_lines() + '\n'

51+ app_contents :=

52+ ['module main', '', 'import vmarkdown', '', 'fn main() {', '\tprintln(vmarkdown.ok())', '}'].join_lines() +

53+ '\n'

54+ issue_20147_write_file(os.join_path(real_module_root, 'vmarkdown.v'), module_contents)

55+ issue_20147_write_file(os.join_path(app_root, 'v.mod'), app_vmod_contents)

56+ issue_20147_write_file(os.join_path(app_root, 'main.v'), app_contents)

57+}

58+

3359fn issue_20147_write_project() {

3460 basepath := issue_20147_module_root()

3561 vmod_contents := ['Module {', "\tname: 'msgpack'", '}'].join_lines() + '\n'

@@ -75,3 +101,24 @@ fn test_issue_20147_vmodules_package_tests_compile() {

75101 res := os.execute('${os.quoted_path(issue_20147_vexe)} test .')

76102 assert res.exit_code == 0, res.output

77103}

104+

105+fn test_vmodules_symlinked_package_import_compiles() {

106+ vmodules_symlink_write_project() or {

107+ $if windows {

108+ eprintln('skipping symlink import regression test: ${err}')

109+ return

110+ } $else {

111+ panic(err)

112+ }

113+ }

114+ old_vmodules, had_vmodules := issue_20147_env_snapshot('VMODULES')

115+ os.setenv('VMODULES', os.join_path(vmodules_symlink_workspace(), '.vmodules'), true)

116+ defer {

117+ issue_20147_restore_env('VMODULES', old_vmodules, had_vmodules)

118+ os.rmdir_all(vmodules_symlink_workspace()) or {}

119+ }

120+ main_file := os.join_path(vmodules_symlink_workspace(), 'app', 'main.v')

121+ res := os.execute('${os.quoted_path(issue_20147_vexe)} run ${os.quoted_path(main_file)}')

122+ assert res.exit_code == 0, res.output

123+ assert res.output.trim_space() == 'ok'

124+}

vlib/v/util/module.v+44-0

@@ -187,6 +187,13 @@ fn mod_path_to_full_name(pref_ &pref.Preferences, mod string, path string) !stri

187187 // after `'v.mod' in ls` can be removed once a proper solution is added

188188 if 'v.mod' in ls

189189 && (try_path_parts.len > i && try_path_parts[i] != 'v' && 'vlib' !in ls) {

190+ // Reject v.mod files in or above the system temp

191+ // directory when the path contains uppercase

192+ // letters (e.g. ULID-based test session dirs),

193+ // as they are likely unrelated to the project.

194+ if j < i && is_unrelated_vmod_in_temp_dir(parent, try_path_parts[j..i]) {

195+ continue

196+ }

190197 last_v_mod = j

191198 break

192199 }

@@ -238,6 +245,8 @@ fn module_name_has_empty_part(name string) bool {

238245// enclosing v.mod for the current compilation (`pref_.path`). Module-name

239246// qualification uses this as the boundary so a nested v.mod inside the

240247// project does not silently rename its sub-modules.

248+// It also respects `.v.mod.stop` and `.git` as project boundaries to

249+// prevent walking past the current project's root.

241250fn project_root_vmod_folder(pref_ &pref.Preferences) string {

242251 if pref_.path == '' {

243252 return ''

@@ -252,10 +261,30 @@ fn project_root_vmod_folder(pref_ &pref.Preferences) string {

252261 return ''

253262 }

254263 mut cfolder := os.real_path(start)

264+ start_folder := cfolder

255265 for {

256266 if os.is_file(os.join_path(cfolder, 'v.mod')) {

267+ // Reject v.mod files in or above the system temp directory

268+ // when the path contains uppercase letters (e.g. ULID-based

269+ // test session dirs), as they are likely unrelated.

270+ if cfolder != start_folder {

271+ rel := start_folder.all_after(cfolder + os.path_separator)

272+ if is_unrelated_vmod_in_temp_dir(cfolder, rel.split(os.path_separator)) {

273+ return ''

274+ }

275+ }

257276 return cfolder

258277 }

278+ // `.v.mod.stop` and `.git` mark project boundaries; stop walking

279+ // up to avoid picking up a v.mod from an unrelated parent project

280+ // (e.g. the V compiler repo when compiling tests in a temp dir).

281+ // These markers are NOT v.mod roots — they only stop the search.

282+ // `.git` can be a directory (normal repos) or a file (worktrees,

283+ // submodules), so use os.exists instead of os.is_dir.

284+ if os.is_file(os.join_path(cfolder, '.v.mod.stop'))

285+ || os.exists(os.join_path(cfolder, '.git')) {

286+ return ''

287+ }

259288 parent := os.dir(cfolder)

260289 if parent == cfolder || parent == '' {

261290 return ''

@@ -265,6 +294,21 @@ fn project_root_vmod_folder(pref_ &pref.Preferences) string {

265294 return ''

266295}

267296

297+// is_unrelated_vmod_in_temp_dir returns true when `vmod_folder` is in or

298+// above the system temp directory and `rel_parts` (the path segments between

299+// the v.mod and the source file) contain uppercase letters. This pattern

300+// indicates the v.mod belongs to an unrelated project that happens to live

301+// in a shared temp location, not to the current compilation.

302+fn is_unrelated_vmod_in_temp_dir(vmod_folder string, rel_parts []string) bool {

303+ temp_dir := os.real_path(os.temp_dir())

304+ is_in_temp := vmod_folder == temp_dir || vmod_folder.starts_with(temp_dir + os.path_separator)

305+ || temp_dir.starts_with(vmod_folder + os.path_separator)

306+ if !is_in_temp {

307+ return false

308+ }

309+ return rel_parts.filter(it.len > 0).any(contains_capital(it))

310+}

311+

268312// normalize_base_url_mod_name strips the `base_url` prefix from `mod_full_name`

269313// when the module lives in a folder configured via v.mod's `base_url`. Without

270314// this, a module rooted at `<pkg>/source/feature` would be named `pkg.source.feature`