From f8ed96a2e04e3054fddfdb2e3ff48a8ca846d632 Mon Sep 17 00:00:00 2001
From: Turiiya <34311583+ttytm@users.noreply.github.com>
Date: Tue, 21 Nov 2023 03:14:45 +0100
Subject: [PATCH] tools.vpm: validate VCS during parsing (#19943)

---
 cmd/tools/vpm/common.v   | 53 +++++++++++++++++++++++++++++++---------
 cmd/tools/vpm/install.v  | 34 +++++---------------------
 cmd/tools/vpm/settings.v |  6 +++--
 cmd/tools/vpm/vpm.v      | 17 +++++++------
 4 files changed, 62 insertions(+), 48 deletions(-)

diff --git a/cmd/tools/vpm/common.v b/cmd/tools/vpm/common.v
index 71337148e1e77c..fde0b72d7050ad 100644
--- a/cmd/tools/vpm/common.v
+++ b/cmd/tools/vpm/common.v
@@ -11,17 +11,15 @@ import log
 
 struct Module {
 mut:
-	// Fields determined by the url or the info received from the VPM API.
-	name string
-	url  string
-	vcs  string
-	// Fields based on preference / environment.
+	name               string
+	url                string
 	version            string // specifies the requested version.
 	install_path       string
 	install_path_fmted string
+	installed_version  string
 	is_installed       bool
 	is_external        bool
-	installed_version  string
+	vcs                ?VCS
 }
 
 struct ModuleVpmInfo {
@@ -50,6 +48,7 @@ const home_dir = os.home_dir()
 fn parse_query(query []string) ([]Module, []Module) {
 	mut vpm_modules, mut external_modules := []Module{}, []Module{}
 	mut errors := 0
+	is_git_setting := settings.vcs.cmd == 'git'
 	for m in query {
 		ident, version := m.rsplit_once('@') or { m, '' }
 		println('Scanning `${ident}`...')
@@ -62,15 +61,17 @@ fn parse_query(query []string) ([]Module, []Module) {
 			false
 		}
 		mut mod := if is_http || ident.starts_with('https://') {
+			// External module.
 			publisher, name := get_ident_from_url(ident) or {
 				vpm_error(err.msg())
 				errors++
 				continue
 			}
+			// Resolve path, verify existence of manifest.
 			base := if is_http { publisher } else { '' }
 			install_path := normalize_mod_path(os.real_path(os.join_path(settings.vmodules_path,
 				base, name)))
-			if !has_vmod(ident, install_path) {
+			if is_git_setting && !has_vmod(ident, install_path) {
 				vpm_error('failed to find `v.mod` for `${ident}`.')
 				errors++
 				continue
@@ -79,19 +80,43 @@ fn parse_query(query []string) ([]Module, []Module) {
 				name: name
 				url: ident
 				install_path: install_path
-				install_path_fmted: fmt_mod_path(install_path)
 				is_external: true
 			}
 		} else {
+			// VPM registered module.
 			info := get_mod_vpm_info(ident) or {
 				vpm_error('failed to retrieve metadata for `${ident}`.', details: err.msg())
 				errors++
 				continue
 			}
+			// Verify VCS.
+			mut is_git_module := true
+			vcs := if info.vcs != '' {
+				info_vcs := supported_vcs[info.vcs] or {
+					vpm_error('skipping `${info.name}`, since it uses an unsupported version control system `${info.vcs}`.')
+					errors++
+					continue
+				}
+				is_git_module = info.vcs == 'git'
+				if !is_git_module && version != '' {
+					vpm_error('skipping `${info.name}`, version installs are currently only supported for projects using `git`.')
+					errors++
+					continue
+				}
+				info_vcs
+			} else {
+				supported_vcs['git']
+			}
+			vcs.is_executable() or {
+				vpm_error(err.msg())
+				errors++
+				continue
+			}
+			// Resolve path, verify existence of manifest.
 			ident_as_path := info.name.replace('.', os.path_separator)
 			install_path := normalize_mod_path(os.real_path(os.join_path(settings.vmodules_path,
 				ident_as_path)))
-			if !has_vmod(info.url, install_path) {
+			if is_git_module && !has_vmod(info.url, install_path) {
 				mut details := ''
 				if resp := http.head('${info.url}/issues/new') {
 					if resp.status_code == 200 {
@@ -104,12 +129,12 @@ fn parse_query(query []string) ([]Module, []Module) {
 			Module{
 				name: info.name
 				url: info.url
-				vcs: info.vcs
+				vcs: vcs
 				install_path: install_path
-				install_path_fmted: fmt_mod_path(install_path)
 			}
 		}
 		mod.version = version
+		mod.install_path_fmted = fmt_mod_path(mod.install_path)
 		if refs := os.execute_opt('git ls-remote --refs ${mod.install_path}') {
 			mod.is_installed = true
 			// In case the head just temporarily matches a tag, make sure that there
@@ -137,6 +162,12 @@ fn parse_query(query []string) ([]Module, []Module) {
 	if errors > 0 && errors == query.len {
 		exit(1)
 	}
+	if external_modules.len > 0 {
+		settings.vcs.is_executable() or {
+			vpm_error(err.msg())
+			exit(1)
+		}
+	}
 	return vpm_modules, external_modules
 }
 
diff --git a/cmd/tools/vpm/install.v b/cmd/tools/vpm/install.v
index 062852598873ba..dce978a01546ae 100644
--- a/cmd/tools/vpm/install.v
+++ b/cmd/tools/vpm/install.v
@@ -93,21 +93,7 @@ fn vpm_install_from_vpm(modules []Module) {
 	for m in modules {
 		vpm_log(@FILE_LINE, @FN, 'module: ${m}')
 		last_errors := errors
-		vcs := if m.vcs != '' {
-			supported_vcs[m.vcs] or {
-				vpm_error('skipping `${m.name}`, since it uses an unsupported version control system `${m.vcs}`.')
-				errors++
-				continue
-			}
-		} else {
-			supported_vcs['git']
-		}
-		vcs.is_executable() or {
-			vpm_error(err.msg())
-			errors++
-			continue
-		}
-		match m.install(vcs) {
+		match m.install() {
 			.installed {}
 			.failed {
 				errors++
@@ -133,17 +119,12 @@ fn vpm_install_from_vpm(modules []Module) {
 
 fn vpm_install_from_vcs(modules []Module) {
 	vpm_log(@FILE_LINE, @FN, 'modules: ${modules}')
-	vcs := supported_vcs[settings.vcs]
-	vcs.is_executable() or {
-		vpm_error(err.msg())
-		exit(1)
-	}
 	urls := modules.map(it.url)
 	mut errors := 0
 	for m in modules {
 		vpm_log(@FILE_LINE, @FN, 'module: ${m}')
 		last_errors := errors
-		match m.install(vcs) {
+		match m.install() {
 			.installed {}
 			.failed {
 				errors++
@@ -209,7 +190,7 @@ fn vpm_install_from_vcs(modules []Module) {
 	}
 }
 
-fn (m Module) install(vcs &VCS) InstallResult {
+fn (m Module) install() InstallResult {
 	if m.is_installed {
 		// Case: installed, but not an explicit version. Update instead of continuing the installation.
 		if m.version == '' && m.installed_version == '' {
@@ -231,12 +212,9 @@ fn (m Module) install(vcs &VCS) InstallResult {
 			return .skipped
 		}
 	}
-	install_arg := if m.version != '' {
-		'${vcs.args.install} --single-branch -b ${m.version}'
-	} else {
-		vcs.args.install
-	}
-	cmd := '${vcs.cmd} ${install_arg} "${m.url}" "${m.install_path}"'
+	vcs := m.vcs or { settings.vcs }
+	version_opt := if m.version != '' { ' ${vcs.args.version} ${m.version}' } else { '' }
+	cmd := '${vcs.cmd} ${vcs.args.install}${version_opt} "${m.url}" "${m.install_path}"'
 	vpm_log(@FILE_LINE, @FN, 'command: ${cmd}')
 	println('Installing `${m.name}`...')
 	verbose_println('  cloning from `${m.url}` to `${m.install_path_fmted}`')
diff --git a/cmd/tools/vpm/settings.v b/cmd/tools/vpm/settings.v
index db82e875d25763..7cd4e0954121c1 100644
--- a/cmd/tools/vpm/settings.v
+++ b/cmd/tools/vpm/settings.v
@@ -11,9 +11,11 @@ mut:
 	is_verbose            bool
 	is_force              bool
 	server_urls           []string
-	vcs                   string
 	vmodules_path         string
 	no_dl_count_increment bool
+	// git is used by default. URL installations can specify `--hg`. For already installed modules
+	// and VPM modules that specify a different VCS in their `v.mod`, the VCS is validated separately.
+	vcs VCS
 }
 
 fn init_settings() VpmSettings {
@@ -29,8 +31,8 @@ fn init_settings() VpmSettings {
 		is_once: '--once' in opts
 		is_verbose: '-v' in opts || '--verbose' in opts
 		is_force: '-f' in opts || '--force' in opts
-		vcs: if '--hg' in opts { 'hg' } else { 'git' }
 		server_urls: cmdline.options(args, '--server-urls')
+		vcs: supported_vcs[if '--hg' in opts { 'hg' } else { 'git' }]
 		vmodules_path: os.vmodules_dir()
 		no_dl_count_increment: os.getenv('CI') != '' || (no_inc_env != '' && no_inc_env != '0')
 	}
diff --git a/cmd/tools/vpm/vpm.v b/cmd/tools/vpm/vpm.v
index 8dfa8d6468fa89..0a8a76af522fad 100644
--- a/cmd/tools/vpm/vpm.v
+++ b/cmd/tools/vpm/vpm.v
@@ -10,13 +10,14 @@ import v.help
 import v.vmod
 
 struct VCS {
-	dir  string
-	cmd  string
+	dir  string        @[required]
+	cmd  string        @[required]
 	args struct {
-		install  string
-		path     string // the flag used to specify a path. E.g., used to explicitly work on a path during multithreaded updating.
-		update   string
-		outdated []string
+		install  string   @[required]
+		version  string   @[required] // flag to specify a version, added to install.
+		path     string   @[required] // flag to specify a path. E.g., used to explicitly work on a path during multithreaded updating.
+		update   string   @[required]
+		outdated []string @[required]
 	}
 }
 
@@ -33,7 +34,8 @@ const (
 			cmd: 'git'
 			args: struct {
 				install: 'clone --depth=1 --recursive --shallow-submodules'
-				update: 'pull --recurse-submodules' // pulling with `--depth=1` leads to conflicts, when the upstream is more than 1 commit newer
+				version: '--single-branch -b'
+				update: 'pull --recurse-submodules' // pulling with `--depth=1` leads to conflicts when the upstream has more than 1 new commits.
 				path: '-C'
 				outdated: ['fetch', 'rev-parse @', 'rev-parse @{u}']
 			}
@@ -43,6 +45,7 @@ const (
 			cmd: 'hg'
 			args: struct {
 				install: 'clone'
+				version: '' // not supported yet.
 				update: 'pull --update'
 				path: '-R'
 				outdated: ['incoming']