tools.vpm: simplify, use more stdlib functionality part2, add more tests (#19692)

Author: ttytm

cmd/tools/vpm/install_test.v

@@ -5,24 +5,30 @@ import v.vmod
 // The following will result in e.g. `$VTMP/tsession_7fe8e93bd740_1612958707536/test-vmodules/`.
 const test_path = os.join_path(os.vtmp_dir(), 'test-vmodules')
 
-fn prep_test_path() {
+fn testsuite_begin() {
 	os.setenv('VMODULES', test_path, true)
 }
 
+fn testsuite_end() {
+	os.rmdir_all(test_path) or {}
+}
+
 fn test_install_from_git_url() {
-	prep_test_path()
 	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() {
-	prep_test_path()
 	res := os.execute(@VEXE + ' 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()
 		return
@@ -32,12 +38,38 @@ fn test_install_from_vpm_ident() {
 }
 
 fn test_install_from_vpm_short_ident() {
-	prep_test_path()
 	res := os.execute(@VEXE + ' 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()
 		return
 	}
 	assert mod.name == 'pcre'
 	assert mod.description == 'A simple regex library for V.'
 }
+
+fn test_install_already_existant() {
+	// Skip on windows for now due permission errors with rmdir.
+	$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}')
+	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('already exists')
+}
+
+fn test_missing_repo_name_in_url() {
+	incomplete_url := 'https://github.com/vlang'
+	res := os.execute(@VEXE + ' 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

@@ -258,48 +258,45 @@ fn ensure_vcs_is_installed(vcs string) bool {
 	return res
 }
 
-fn vpm_install_from_vcs(module_names []string, vcs_key string) {
+fn vpm_install_from_vcs(modules []string, vcs_key string) {
 	mut errors := 0
-	for n in module_names {
-		url := n.trim_space()
-
-		first_cut_pos := url.last_index('/') or {
+	for raw_url in modules {
+		url := urllib.parse(raw_url) or {
 			errors++
-			eprintln('Errors while retrieving name for module "${url}" :')
-			eprintln(err)
+			eprintln('Errors while parsing module url "${raw_url}" : ${err}')
 			continue
 		}
 
-		mod_name := url.substr(first_cut_pos + 1, url.len)
-
-		second_cut_pos := url.substr(0, first_cut_pos).last_index('/') or {
+		// Module identifier based on URL.
+		// E.g.: `https://github.com/owner/awesome-v-project` -> `owner/awesome_v_project`
+		mut ident := url.path#[1..].replace('-', '_')
+		owner, repo_name := ident.split_once('/') or {
 			errors++
-			eprintln('Errors while retrieving name for module "${url}" :')
-			eprintln(err)
+			eprintln('Errors while retrieving module name for: "${url}"')
 			continue
 		}
+		mut final_module_path := os.real_path(os.join_path(settings.vmodules_path, owner.to_lower(),
+			repo_name.to_lower()))
 
-		repo_name := url.substr(second_cut_pos + 1, first_cut_pos)
-		mut name := os.join_path(repo_name, mod_name)
-		mod_name_as_path := name.replace('-', '_').to_lower()
-		mut final_module_path := os.real_path(os.join_path(settings.vmodules_path, mod_name_as_path))
 		if os.exists(final_module_path) {
-			vpm_update([name.replace('-', '_')])
+			vpm_update([ident])
 			continue
 		}
+
 		if !ensure_vcs_is_installed(vcs_key) {
 			errors++
 			eprintln('VPM needs `${vcs_key}` to be installed.')
 			continue
 		}
-		println('Installing module "${name}" from "${url}" to "${final_module_path}" ...')
+
+		println('Installing module "${repo_name}" from "${url}" to "${final_module_path}" ...')
 		vcs_install_cmd := '${vcs_key} ${supported_vcs_install_cmds[vcs_key]}'
 		cmd := '${vcs_install_cmd} "${url}" "${final_module_path}"'
 		verbose_println('      command: ${cmd}')
 		cmdres := os.execute(cmd)
 		if cmdres.exit_code != 0 {
 			errors++
-			eprintln('Failed installing module "${name}" to "${final_module_path}" .')
+			eprintln('Failed installing module "${repo_name}" to "${final_module_path}" .')
 			print_failed_cmd(cmd, cmdres)
 			continue
 		}
@@ -311,7 +308,7 @@ fn vpm_install_from_vcs(module_names []string, vcs_key string) {
 			}
 			minfo := mod_name_info(manifest.name)
 			if final_module_path != minfo.final_module_path {
-				println('Relocating module from "${name}" to "${manifest.name}" ( "${minfo.final_module_path}" ) ...')
+				println('Relocating module from "${ident}" to "${manifest.name}" ("${minfo.final_module_path}") ...')
 				if os.exists(minfo.final_module_path) {
 					eprintln('Warning module "${minfo.final_module_path}" already exists!')
 					eprintln('Removing module "${minfo.final_module_path}" ...')
@@ -324,7 +321,7 @@ fn vpm_install_from_vcs(module_names []string, vcs_key string) {
 				}
 				os.mv(final_module_path, minfo.final_module_path) or {
 					errors++
-					eprintln('Errors while relocating module "${name}" :')
+					eprintln('Errors while relocating module "${repo_name}" :')
 					eprintln(err)
 					os.rmdir_all(final_module_path) or {
 						errors++
@@ -334,7 +331,7 @@ fn vpm_install_from_vcs(module_names []string, vcs_key string) {
 					}
 					continue
 				}
-				println('Module "${name}" relocated to "${manifest.name}" successfully.')
+				println('Module "${repo_name}" relocated to "${manifest.name}" successfully.')
 				publisher_dir := final_module_path.all_before_last(os.path_separator)
 				if os.is_dir_empty(publisher_dir) {
 					os.rmdir(publisher_dir) or {
@@ -345,9 +342,9 @@ fn vpm_install_from_vcs(module_names []string, vcs_key string) {
 				}
 				final_module_path = minfo.final_module_path
 			}
-			name = manifest.name
+			ident = manifest.name
 		}
-		resolve_dependencies(name, final_module_path, module_names)
+		resolve_dependencies(ident, final_module_path, modules)
 	}
 	if errors > 0 {
 		exit(1)