Skip to content

Commit 29eda89

Browse files
ttytmttytm
authored
tools.vpm: fix installing of modules with conflicting names, extend tests (#19961)
1 parent fb93828 commit 29eda89

File tree

5 files changed

+160
-143
lines changed

5 files changed

+160
-143
lines changed

‎cmd/tools/vpm/common.v

Lines changed: 80 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ mut:
2020
is_installed bool
2121
is_external bool
2222
vcs ?VCS
23+
manifest vmod.Manifest
2324
}
2425

2526
struct ModuleVpmInfo {
@@ -43,7 +44,10 @@ struct ErrorOptions {
4344
verbose bool // is used to only output the error message if the verbose setting is enabled.
4445
}
4546

46-
const home_dir = os.home_dir()
47+
const (
48+
vexe = os.quoted_path(os.getenv('VEXE'))
49+
home_dir = os.home_dir()
50+
)
4751

4852
fn parse_query(query []string) ([]Module, []Module) {
4953
mut vpm_modules, mut external_modules := []Module{}, []Module{}
@@ -61,26 +65,30 @@ fn parse_query(query []string) ([]Module, []Module) {
6165
false
6266
}
6367
mut mod := if is_http || ident.starts_with('https://') {
64-
// External module.
68+
// External module. The idenifier is an URL.
6569
publisher, name := get_ident_from_url(ident) or {
6670
vpm_error(err.msg())
6771
errors++
6872
continue
6973
}
70-
// Resolve path, verify existence of manifest.
71-
base := if is_http { publisher } else { '' }
72-
install_path := normalize_mod_path(os.real_path(os.join_path(settings.vmodules_path,
73-
base, name)))
74-
if is_git_setting && !has_vmod(ident, install_path) {
75-
vpm_error('failed to find `v.mod` for `${ident}`.')
74+
// Fetch manifest.
75+
manifest := fetch_manifest(name, ident, version, is_git_setting) or {
76+
vpm_error('failed to find `v.mod` for `${ident}${at_version(version)}`.',
77+
details: err.msg()
78+
)
7679
errors++
7780
continue
7881
}
82+
// Resolve path.
83+
base := if is_http { publisher } else { '' }
84+
install_path := normalize_mod_path(os.real_path(os.join_path(settings.vmodules_path,
85+
base, manifest.name)))
7986
Module{
80-
name: name
87+
name: manifest.name
8188
url: ident
8289
install_path: install_path
8390
is_external: true
91+
manifest: manifest
8492
}
8593
} else {
8694
// VPM registered module.
@@ -112,11 +120,9 @@ fn parse_query(query []string) ([]Module, []Module) {
112120
errors++
113121
continue
114122
}
115-
// Resolve path, verify existence of manifest.
116-
ident_as_path := info.name.replace('.', os.path_separator)
117-
install_path := normalize_mod_path(os.real_path(os.join_path(settings.vmodules_path,
118-
ident_as_path)))
119-
if is_git_module && !has_vmod(info.url, install_path) {
123+
// Fetch manifest.
124+
manifest := fetch_manifest(info.name, info.url, version, is_git_module) or {
125+
// Add link with issue template requesting to add a manifest.
120126
mut details := ''
121127
if resp := http.head('${info.url}/issues/new') {
122128
if resp.status_code == 200 {
@@ -125,34 +131,23 @@ fn parse_query(query []string) ([]Module, []Module) {
125131
}
126132
}
127133
vpm_warn('`${info.name}` is missing a manifest file.', details: details)
134+
vpm_log(@FILE_LINE, @FN, 'vpm manifest detection error: ${err}')
135+
vmod.Manifest{}
128136
}
137+
// Resolve path.
138+
ident_as_path := info.name.replace('.', os.path_separator)
139+
install_path := normalize_mod_path(os.real_path(os.join_path(settings.vmodules_path,
140+
ident_as_path)))
129141
Module{
130142
name: info.name
131143
url: info.url
132144
vcs: vcs
133145
install_path: install_path
146+
manifest: manifest
134147
}
135148
}
136149
mod.version = version
137-
mod.install_path_fmted = fmt_mod_path(mod.install_path)
138-
if refs := os.execute_opt('git ls-remote --refs ${mod.install_path}') {
139-
mod.is_installed = true
140-
// In case the head just temporarily matches a tag, make sure that there
141-
// really is a version installation before adding it as `installed_version`.
142-
// NOTE: can be refined for branch installations. E.g., for `sdl`.
143-
if refs.output.contains('refs/tags/') {
144-
tag := refs.output.all_after_last('refs/tags/').trim_space()
145-
head := if refs.output.contains('refs/heads/') {
146-
refs.output.all_after_last('refs/heads/').trim_space()
147-
} else {
148-
tag
149-
}
150-
vpm_log(@FILE_LINE, @FN, 'head: ${head}, tag: ${tag}')
151-
if tag == head {
152-
mod.installed_version = tag
153-
}
154-
}
155-
}
150+
mod.get_installed()
156151
if mod.is_external {
157152
external_modules << mod
158153
} else {
@@ -171,23 +166,59 @@ fn parse_query(query []string) ([]Module, []Module) {
171166
return vpm_modules, external_modules
172167
}
173168

174-
fn has_vmod(url string, install_path string) bool {
175-
if install_path != '' && os.exists((os.join_path(install_path, 'v.mod'))) {
176-
// Skip fetching the repo when the module is already installed and has a `v.mod`.
177-
return true
178-
}
179-
head_branch := os.execute_opt('git ls-remote --symref ${url} HEAD') or {
180-
vpm_error('failed to find git HEAD for `${url}`.', details: err.msg())
181-
return false
182-
}.output.all_after_last('/').all_before(' ').all_before('\t')
169+
// TODO: add unit test
170+
fn (mut m Module) get_installed() {
171+
refs := os.execute_opt('git ls-remote --refs ${m.install_path}') or { return }
172+
m.is_installed = true
173+
// In case the head just temporarily matches a tag, make sure that there
174+
// really is a version installation before adding it as `installed_version`.
175+
// NOTE: can be refined for branch installations. E.g., for `sdl`.
176+
if refs.output.contains('refs/tags/') {
177+
tag := refs.output.all_after_last('refs/tags/').all_before('\n').trim_space()
178+
head := if refs.output.contains('refs/heads/') {
179+
refs.output.all_after_last('refs/heads/').all_before('\n').trim_space()
180+
} else {
181+
tag
182+
}
183+
vpm_log(@FILE_LINE, @FN, 'head: ${head}, tag: ${tag}')
184+
if tag == head {
185+
m.installed_version = tag
186+
}
187+
}
188+
}
189+
190+
fn fetch_manifest(name string, url string, version string, is_git bool) !vmod.Manifest {
191+
if !is_git {
192+
// TODO: fetch manifest for mercurial repositories
193+
return vmod.Manifest{
194+
name: name
195+
}
196+
}
197+
v := if version != '' {
198+
version
199+
} else {
200+
head_branch := os.execute_opt('git ls-remote --symref ${url} HEAD') or {
201+
return error('failed to find git HEAD. ${err}')
202+
}
203+
head_branch.output.all_after_last('/').all_before(' ').all_before('\t')
204+
}
183205
url_ := if url.ends_with('.git') { url.replace('.git', '') } else { url }
184-
manifest_url := '${url_}/blob/${head_branch}/v.mod'
185-
vpm_log(@FILE_LINE, @FN, 'manifest_url: ${manifest_url}')
186-
has_vmod := http.head(manifest_url) or {
187-
vpm_error('failed to retrieve module data for `${url}`.')
188-
return false
189-
}.status_code == 200
190-
return has_vmod
206+
// Scan both URLS. E.g.:
207+
// https://github.com/publisher/module/raw/v0.7.0/v.mod
208+
// https://gitlab.com/publisher/module/-/raw/main/v.mod
209+
raw_paths := ['raw/', '/-/raw/']
210+
for i, raw_p in raw_paths {
211+
manifest_url := '${url_}/${raw_p}/${v}/v.mod'
212+
vpm_log(@FILE_LINE, @FN, 'manifest_url ${i}: ${manifest_url}')
213+
raw_manifest_resp := http.get(manifest_url) or { continue }
214+
if raw_manifest_resp.status_code != 200 {
215+
return error('unsuccessful response status `${raw_manifest_resp.status_code}`.')
216+
}
217+
return vmod.decode(raw_manifest_resp.body) or {
218+
return error('failed to decode manifest `${raw_manifest_resp.body}`. ${err}')
219+
}
220+
}
221+
return error('failed to retrieve manifest.')
191222
}
192223

193224
fn get_mod_date_info(mut pp pool.PoolProcessor, idx int, wid int) &ModuleDateInfo {

‎cmd/tools/vpm/install.v

Lines changed: 3 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ fn vpm_install_from_vpm(modules []Module) {
9292
mut errors := 0
9393
for m in modules {
9494
vpm_log(@FILE_LINE, @FN, 'module: ${m}')
95-
last_errors := errors
9695
match m.install() {
9796
.installed {}
9897
.failed {
@@ -107,9 +106,7 @@ fn vpm_install_from_vpm(modules []Module) {
107106
vpm_error('failed to increment the download count for `${m.name}`', details: err.msg())
108107
errors++
109108
}
110-
if last_errors == errors {
111-
println('Installed `${m.name}`.')
112-
}
109+
println('Installed `${m.name}`.')
113110
resolve_dependencies(get_manifest(m.install_path), idents)
114111
}
115112
if errors > 0 {
@@ -123,7 +120,6 @@ fn vpm_install_from_vcs(modules []Module) {
123120
mut errors := 0
124121
for m in modules {
125122
vpm_log(@FILE_LINE, @FN, 'module: ${m}')
126-
last_errors := errors
127123
match m.install() {
128124
.installed {}
129125
.failed {
@@ -134,56 +130,8 @@ fn vpm_install_from_vcs(modules []Module) {
134130
continue
135131
}
136132
}
137-
manifest := get_manifest(m.install_path) or { continue }
138-
final_path := os.join_path(m.install_path.all_before_last(os.path_separator),
139-
normalize_mod_path(manifest.name))
140-
if m.install_path != final_path {
141-
verbose_println('Relocating `${m.name} (${m.install_path_fmted})` to `${manifest.name} (${final_path})`...')
142-
if os.exists(final_path) {
143-
println('Target directory for `${m.name} (${final_path})` already exists.')
144-
input := os.input('Replace it with the module directory? [Y/n]: ')
145-
match input.to_lower() {
146-
'', 'y' {
147-
m.remove() or {
148-
vpm_error('failed to remove `${final_path}`.', details: err.msg())
149-
errors++
150-
continue
151-
}
152-
}
153-
else {
154-
verbose_println('Skipping `${m.name}`.')
155-
continue
156-
}
157-
}
158-
}
159-
// When the module should be relocated into a subdirectory we need to make sure
160-
// it exists to not run into permission errors.
161-
if m.install_path.count(os.path_separator) < final_path.count(os.path_separator)
162-
&& !os.exists(final_path) {
163-
os.mkdir_all(final_path) or {
164-
vpm_error('failed to create directory for `${manifest.name}`.',
165-
details: err.msg()
166-
)
167-
errors++
168-
continue
169-
}
170-
}
171-
os.mv(m.install_path, final_path) or {
172-
errors++
173-
vpm_error('failed to relocate module `${m.name}`.', details: err.msg())
174-
os.rmdir_all(m.install_path) or {
175-
vpm_error('failed to remove `${m.install_path}`.', details: err.msg())
176-
errors++
177-
continue
178-
}
179-
continue
180-
}
181-
verbose_println('Relocated `${m.name}` to `${manifest.name}`.')
182-
}
183-
if last_errors == errors {
184-
println('Installed `${m.name}`.')
185-
}
186-
resolve_dependencies(manifest, urls)
133+
println('Installed `${m.name}`.')
134+
resolve_dependencies(m.manifest, urls)
187135
}
188136
if errors > 0 {
189137
exit(1)

0 commit comments

Comments
 (0)