tools.vpm: fix install --once for external modules, add already ins…

…talled info, add test (#19702)

cmd/tools/vpm/install_test.v

@@ -1,9 +1,12 @@
 import os
 import v.vmod
 
-// Running tests appends a tsession path to VTMP, which is automatically cleaned up after the test.
-// The following will result in e.g. `$VTMP/tsession_7fe8e93bd740_1612958707536/test-vmodules/`.
-const test_path = os.join_path(os.vtmp_dir(), 'test-vmodules')
+const (
+	v         = os.quoted_path(@VEXE)
+	// Running tests appends a tsession path to VTMP, which is automatically cleaned up after the test.
+	// The following will result in e.g. `$VTMP/tsession_7fe8e93bd740_1612958707536/test-vmodules/`.
+	test_path = os.join_path(os.vtmp_dir(), 'test-vmodules')
+)
 
 fn testsuite_begin() {
 	os.setenv('VMODULES', test_path, true)
@@ -13,21 +16,8 @@ fn testsuite_end() {
 	os.rmdir_all(test_path) or {}
 }
 
-fn test_install_from_git_url() {
-	res := os.execute(@VEXE + ' install https://github.com/vlang/markdown')
-	assert res.exit_code == 0, res.output
-	mod := vmod.from_file(os.join_path(test_path, 'markdown', 'v.mod')) or {
-		assert false, err.str()
-		return
-	}
-	assert mod.name == 'markdown'
-	assert mod.dependencies == []string{}
-	assert res.output.contains('Installing module "markdown" from "https://github.com/vlang/markdown')
-	assert res.output.contains('Relocating module from "vlang/markdown" to "markdown"')
-}
-
 fn test_install_from_vpm_ident() {
-	res := os.execute(@VEXE + ' install nedpals.args')
+	res := os.execute('${v} install nedpals.args')
 	assert res.exit_code == 0, res.output
 	mod := vmod.from_file(os.join_path(test_path, 'nedpals', 'args', 'v.mod')) or {
 		assert false, err.str()
@@ -38,7 +28,7 @@ fn test_install_from_vpm_ident() {
 }
 
 fn test_install_from_vpm_short_ident() {
-	res := os.execute(@VEXE + ' install pcre')
+	res := os.execute('${v} install pcre')
 	assert res.exit_code == 0, res.output
 	mod := vmod.from_file(os.join_path(test_path, 'pcre', 'v.mod')) or {
 		assert false, err.str()
@@ -48,28 +38,78 @@ fn test_install_from_vpm_short_ident() {
 	assert mod.description == 'A simple regex library for V.'
 }
 
-fn test_install_already_existant() {
-	// Skip on windows for now due permission errors with rmdir.
+fn test_install_from_git_url() {
+	res := os.execute('${v} install https://github.com/vlang/markdown')
+	assert res.exit_code == 0, res.output
+	assert res.output.contains('Installing module "markdown" from "https://github.com/vlang/markdown')
+	assert res.output.contains('Relocating module from "vlang/markdown" to "markdown"')
+	mod := vmod.from_file(os.join_path(test_path, 'markdown', 'v.mod')) or {
+		assert false, err.str()
+		return
+	}
+	assert mod.name == 'markdown'
+	assert mod.dependencies == []string{}
+}
+
+fn test_install_already_existent() {
+	// FIXME: Skip this for now on Windows, as `rmdir_all` results in permission
+	// errors when vpm tries to remove existing modules.
 	$if windows {
 		return
 	}
-	mod_url := 'https://github.com/vlang/markdown'
-	mut res := os.execute(@VEXE + ' install ${mod_url}')
-	assert res.exit_code == 0, res.output
-	res = os.execute(@VEXE + ' install ${mod_url}')
+	mut res := os.execute('${v} install https://github.com/vlang/markdown')
 	assert res.exit_code == 0, res.output
+	assert res.output.contains('already exists')
 	mod := vmod.from_file(os.join_path(test_path, 'markdown', 'v.mod')) or {
 		assert false, err.str()
 		return
 	}
 	assert mod.name == 'markdown'
 	assert mod.dependencies == []string{}
-	assert res.output.contains('already exists')
+}
+
+fn test_install_once() {
+	// Start with a clean test path.
+	$if windows {
+		// FIXME: Workaround for failing `rmdir` commands on Windows.
+		os.system('rd /s /q ${test_path}')
+	} $else {
+		os.rmdir_all(test_path) or {}
+	}
+	os.mkdir_all(test_path) or {}
+
+	// Install markdown module.
+	mut res := os.execute('${v} install markdown')
+	assert res.exit_code == 0, res.output
+	// Keep track of the last modified state of the v.mod file of the installed markdown module.
+	md_last_modified := os.file_last_mod_unix(os.join_path(test_path, 'markdown', 'v.mod'))
+
+	install_cmd := '${@VEXE} install https://github.com/vlang/markdown https://github.com/vlang/pcre --once -v'
+	// Try installing two modules, one of which is already installed.
+	res = os.execute(install_cmd)
+	assert res.exit_code == 0, res.output
+	assert res.output.contains("Already installed modules: ['markdown']")
+	mod := vmod.from_file(os.join_path(test_path, 'pcre', 'v.mod')) or {
+		assert false, err.str()
+		return
+	}
+	assert mod.name == 'pcre'
+	assert mod.description == 'A simple regex library for V.'
+	// Ensure the before installed markdown module wasn't modified.
+	assert md_last_modified == os.file_last_mod_unix(os.join_path(test_path, 'markdown',
+		'v.mod'))
+
+	// Try installing two modules that are both already installed.
+	res = os.execute(install_cmd)
+	assert res.exit_code == 0, res.output
+	assert res.output.contains('All modules are already installed.')
+	assert md_last_modified == os.file_last_mod_unix(os.join_path(test_path, 'markdown',
+		'v.mod'))
 }
 
 fn test_missing_repo_name_in_url() {
 	incomplete_url := 'https://github.com/vlang'
-	res := os.execute(@VEXE + ' install ${incomplete_url}')
+	res := os.execute('${v} install ${incomplete_url}')
 	assert res.exit_code == 1
 	assert res.output.trim_space() == 'Errors while retrieving module name for: "${incomplete_url}"'
 }

cmd/tools/vpm/vpm.v

@@ -65,50 +65,21 @@ fn main() {
 		help.print_and_exit('vpm', exit_code: 5)
 	}
 	vpm_command := params[0]
-	mut module_names := params[1..].clone()
+	mut requested_modules := params[1..].clone()
 	ensure_vmodules_dir_exist()
-	// println('module names: ') println(module_names)
+	// println('module names: ') println(requested_modules)
 	match vpm_command {
 		'help' {
 			help.print_and_exit('vpm')
 		}
 		'search' {
-			vpm_search(module_names)
+			vpm_search(requested_modules)
 		}
 		'install' {
-			if module_names.len == 0 && os.exists('./v.mod') {
-				println('Detected v.mod file inside the project directory. Using it...')
-				manifest := vmod.from_file('./v.mod') or { panic(err) }
-				module_names = manifest.dependencies.clone()
-			}
-
-			if '--once' in options {
-				module_names = vpm_once_filter(module_names)
-
-				if module_names.len == 0 {
-					return
-				}
-			}
-
-			external_module_names := module_names.filter(it.starts_with('https://'))
-			vpm_module_names := module_names.filter(it !in external_module_names)
-
-			if vpm_module_names.len > 0 {
-				vpm_install(vpm_module_names, Source.vpm)
-			}
-
-			if external_module_names.len > 0 {
-				mut external_source := Source.git
-
-				if '--hg' in options {
-					external_source = Source.hg
-				}
-
-				vpm_install(external_module_names, external_source)
-			}
+			vpm_install_(requested_modules, options)
 		}
 		'update' {
-			vpm_update(module_names)
+			vpm_update(requested_modules)
 		}
 		'upgrade' {
 			vpm_upgrade()
@@ -120,10 +91,10 @@ fn main() {
 			vpm_list()
 		}
 		'remove' {
-			vpm_remove(module_names)
+			vpm_remove(requested_modules)
 		}
 		'show' {
-			vpm_show(module_names)
+			vpm_show(requested_modules)
 		}
 		else {
 			eprintln('Error: you tried to run "v ${vpm_command}"')
@@ -136,6 +107,85 @@ fn main() {
 	}
 }
 
+fn vpm_install_(requested_modules []string, opts []string) {
+	if settings.is_help {
+		help.print_and_exit('vpm')
+	}
+
+	modules := if requested_modules.len == 0 {
+		// Run `v install` in a directory of another V module without passing modules as arguments
+		// to install its dependencies.
+		if os.exists('./v.mod') {
+			println('Detected v.mod file inside the project directory. Using it...')
+			manifest := vmod.from_file('./v.mod') or { panic(err) }
+			if manifest.dependencies.len == 0 {
+				println('Nothing to install.')
+				exit(0)
+			}
+			manifest.dependencies.clone()
+		} else {
+			eprintln('Specify a module for installation.')
+			help.print_and_exit('vpm')
+			return
+		}
+	} else {
+		requested_modules.clone()
+	}
+
+	mut external_modules := modules.filter(it.starts_with('https://'))
+	mut vpm_modules := modules.filter(it !in external_modules)
+	installed_modules := get_installed_modules()
+
+	if installed_modules.len > 0 && '--once' in opts {
+		mut already_installed := []string{}
+		if external_modules.len > 0 {
+			mut i_deleted := []int{}
+			for i, raw_url in external_modules {
+				url := urllib.parse(raw_url) or {
+					eprintln('Errors while parsing module url "${raw_url}" : ${err}')
+					continue
+				}
+				mod_name := url.path.all_after_last('/')
+				if mod_name in installed_modules {
+					already_installed << mod_name
+					i_deleted << i
+				}
+			}
+			for i in i_deleted.reverse() {
+				external_modules.delete(i)
+			}
+		}
+		if vpm_modules.len > 0 {
+			mut i_deleted := []int{}
+			for i, mod_name in vpm_modules {
+				if mod_name in installed_modules {
+					already_installed << mod_name
+					i_deleted << i
+					continue
+				}
+			}
+			for i in i_deleted.reverse() {
+				vpm_modules.delete(i)
+			}
+		}
+		if already_installed.len > 0 {
+			verbose_println('Already installed modules: ${already_installed}')
+			if already_installed.len == modules.len {
+				verbose_println('All modules are already installed.')
+				exit(0)
+			}
+		}
+	}
+
+	if vpm_modules.len > 0 {
+		vpm_install(vpm_modules, Source.vpm)
+	}
+	if external_modules.len > 0 {
+		source := if '--hg' in opts { Source.hg } else { Source.git }
+		vpm_install(external_modules, source)
+	}
+}
+
 fn vpm_search(keywords []string) {
 	search_keys := keywords.map(it.replace('_', '-'))
 	if settings.is_help {
@@ -349,17 +399,6 @@ fn vpm_install_from_vcs(modules []string, vcs_key string) {
 	}
 }
 
-fn vpm_once_filter(module_names []string) []string {
-	installed_modules := get_installed_modules()
-	mut toinstall := []string{}
-	for mn in module_names {
-		if mn !in installed_modules {
-			toinstall << mn
-		}
-	}
-	return toinstall
-}
-
 fn vpm_install(module_names []string, source Source) {
 	if settings.is_help {
 		help.print_and_exit('install')