Merge pull request #288 from vim-volt/avoid-dup-entries-lockjson

Avoid dup entries lock.json

lockjson/lockjson.go

@@ -342,7 +342,7 @@ func (plist ProfileList) RemoveAllReposPath(reposPath pathutil.ReposPath) error
 	removed := false
 	for i := range plist {
 		for j := 0; j < len(plist[i].ReposPath); {
-			if plist[i].ReposPath[j] == reposPath {
+			if plist[i].ReposPath[j].Equals(reposPath) {
 				plist[i].ReposPath = append(
 					plist[i].ReposPath[:j],
 					plist[i].ReposPath[j+1:]...,
@@ -361,26 +361,25 @@ func (plist ProfileList) RemoveAllReposPath(reposPath pathutil.ReposPath) error
 
 // Contains returns true if reposList contains reposPath.
 func (reposList ReposList) Contains(reposPath pathutil.ReposPath) bool {
-	_, err := reposList.FindByPath(reposPath)
-	return err == nil
+	return reposList.FindByPath(reposPath) != nil
 }
 
 // FindByPath finds reposPath from reposList.
 // Non-nil pointer is returned if found.
 // nil pointer is returned if not found.
-func (reposList ReposList) FindByPath(reposPath pathutil.ReposPath) (*Repos, error) {
+func (reposList ReposList) FindByPath(reposPath pathutil.ReposPath) *Repos {
 	for i := range reposList {
-		if reposList[i].Path == reposPath {
-			return &reposList[i], nil
+		if reposList[i].Path.Equals(reposPath) {
+			return &reposList[i]
 		}
 	}
-	return nil, errors.New("repos '" + reposPath.String() + "' does not exist")
+	return nil
 }
 
 // RemoveAllReposPath removes all reposPath from all repos path list.
 func (reposList *ReposList) RemoveAllReposPath(reposPath pathutil.ReposPath) error {
 	for i := range *reposList {
-		if (*reposList)[i].Path == reposPath {
+		if (*reposList)[i].Path.Equals(reposPath) {
 			*reposList = append((*reposList)[:i], (*reposList)[i+1:]...)
 			return nil
 		}
@@ -398,7 +397,7 @@ func (reposPathList profReposPath) Contains(reposPath pathutil.ReposPath) bool {
 // idx == -1 is returned if not found.
 func (reposPathList profReposPath) IndexOf(reposPath pathutil.ReposPath) int {
 	for i := range reposPathList {
-		if reposPathList[i] == reposPath {
+		if reposPathList[i].Equals(reposPath) {
 			return i
 		}
 	}
@@ -409,9 +408,9 @@ func (reposPathList profReposPath) IndexOf(reposPath pathutil.ReposPath) int {
 func (lockJSON *LockJSON) GetReposListByProfile(profile *Profile) (ReposList, error) {
 	reposList := make(ReposList, 0, len(profile.ReposPath))
 	for _, reposPath := range profile.ReposPath {
-		repos, err := lockJSON.Repos.FindByPath(reposPath)
-		if err != nil {
-			return nil, err
+		repos := lockJSON.Repos.FindByPath(reposPath)
+		if repos == nil {
+			return nil, errors.New("repos '" + reposPath.String() + "' does not exist")
 		}
 		reposList = append(reposList, *repos)
 	}

pathutil/pathutil.go

@@ -40,63 +40,33 @@ func NormalizeRepos(rawReposPath string) (ReposPath, error) {
 	if disallowSlash && m[5] == "/" {
 		return "", errors.New("invalid format of repository: " + rawReposPath)
 	}
+	m[2] = strings.ToLower(m[2]) // ignore hostname's case
 	hostUserName := m[2:5]
 	return ReposPath(strings.Join(hostUserName, "/")), nil
 }
 
 // ReposPath is string of "{site}/{user}/{repos}"
 type ReposPath string
 
-// ReposPathList is []ReposPath
-type ReposPathList []ReposPath
-
-func (path *ReposPath) String() string {
-	return string(*path)
+func (path ReposPath) String() string {
+	return string(path)
 }
 
-// Strings returns []string values.
-func (list ReposPathList) Strings() []string {
-	result := make([]string, 0, len(list))
-	for i := range list {
-		result = append(result, string(list[i]))
+// Equals returns true if path and p2 are the same.
+func (path ReposPath) Equals(p2 ReposPath) bool {
+	s1 := string(path)
+	s2 := string(p2)
+	if path.ignoreCase() {
+		s1 = strings.ToLower(s1)
+		s2 = strings.ToLower(s2)
 	}
-	return result
+	return s1 == s2
 }
 
-// NormalizeLocalRepos normalizes name into ReposPath.
-// If name does not contain "/", it is ReposPath("localhost/local/" + name).
-// Otherwise same as NormalizeRepos(name).
-func NormalizeLocalRepos(name string) (ReposPath, error) {
-	if !strings.Contains(name, "/") {
-		return ReposPath("localhost/local/" + name), nil
-	}
-	return NormalizeRepos(name)
-}
-
-// HomeDir detects HOME path.
-// If HOME environment variable is not set,
-// use USERPROFILE environment variable instead.
-func HomeDir() string {
-	home := os.Getenv("HOME")
-	if home != "" {
-		return home
-	}
-
-	home = os.Getenv("USERPROFILE") // windows
-	if home != "" {
-		return home
-	}
-
-	panic("Couldn't look up HOME")
-}
-
-// VoltPath returns fullpath of "$HOME/volt".
-func VoltPath() string {
-	path := os.Getenv("VOLTPATH")
-	if path != "" {
-		return path
-	}
-	return filepath.Join(HomeDir(), "volt")
+// ignoreCase returns true if this site ignores case of repository URL.
+// Most sites ignore cases of URLs. Add exceptions here if found.
+func (path ReposPath) ignoreCase() bool {
+	return true
 }
 
 // FullPath returns fullpath of ReposPath.
@@ -124,6 +94,34 @@ func (path ReposPath) Plugconf() string {
 	return filepath.Join(paths...)
 }
 
+// ReposPathList is []ReposPath
+type ReposPathList []ReposPath
+
+// Strings returns []string values.
+func (list ReposPathList) Strings() []string {
+	result := make([]string, 0, len(list))
+	for i := range list {
+		result = append(result, string(list[i]))
+	}
+	return result
+}
+
+// Contains returns true if list contains reposPath.
+func (list ReposPathList) Contains(reposPath ReposPath) bool {
+	return list.Find(reposPath).String() != ""
+}
+
+// Find returns an non-empty element (path.String() != "")
+// if list contains reposPath.
+func (list ReposPathList) Find(reposPath ReposPath) ReposPath {
+	for i := range list {
+		if list[i].Equals(reposPath) {
+			return list[i]
+		}
+	}
+	return ReposPath("")
+}
+
 // ProfileVimrc is the basename of profile vimrc.
 const ProfileVimrc = "vimrc.vim"
 
@@ -159,6 +157,32 @@ func DecodeReposPath(name string) ReposPath {
 	return ReposPath(unpacker2.Replace(unpacker1.Replace(name)))
 }
 
+// HomeDir detects HOME path.
+// If HOME environment variable is not set,
+// use USERPROFILE environment variable instead.
+func HomeDir() string {
+	home := os.Getenv("HOME")
+	if home != "" {
+		return home
+	}
+
+	home = os.Getenv("USERPROFILE") // windows
+	if home != "" {
+		return home
+	}
+
+	panic("Couldn't look up HOME")
+}
+
+// VoltPath returns fullpath of "$HOME/volt".
+func VoltPath() string {
+	path := os.Getenv("VOLTPATH")
+	if path != "" {
+		return path
+	}
+	return filepath.Join(HomeDir(), "volt")
+}
+
 // LockJSON returns fullpath of "$HOME/volt/lock.json".
 func LockJSON() string {
 	return filepath.Join(VoltPath(), "lock.json")

subcmd/buildinfo/buildinfo.go

@@ -87,7 +87,7 @@ func (buildInfo *BuildInfo) validate() error {
 func (reposList *ReposList) FindByReposPath(reposPath pathutil.ReposPath) *Repos {
 	for i := range *reposList {
 		repos := &(*reposList)[i]
-		if repos.Path == reposPath {
+		if repos.Path.Equals(reposPath) {
 			return repos
 		}
 	}
@@ -97,7 +97,7 @@ func (reposList *ReposList) FindByReposPath(reposPath pathutil.ReposPath) *Repos
 func (reposList *ReposList) RemoveByReposPath(reposPath pathutil.ReposPath) {
 	for i := range *reposList {
 		repos := &(*reposList)[i]
-		if repos.Path == reposPath {
+		if repos.Path.Equals(reposPath) {
 			*reposList = append((*reposList)[:i], (*reposList)[i+1:]...)
 			break
 		}

subcmd/get.go

@@ -181,6 +181,11 @@ func (cmd *getCmd) getReposPathList(args []string, lockJSON *lockjson.LockJSON)
 			if err != nil {
 				return nil, err
 			}
+			// Get the existing entries if already have it
+			// (e.g. github.com/tyru/CaW.vim -> github.com/tyru/caw.vim)
+			if r := lockJSON.Repos.FindByPath(reposPath); r != nil {
+				reposPath = r.Path
+			}
 			reposPathList = append(reposPathList, reposPath)
 		}
 	}
@@ -213,10 +218,7 @@ func (cmd *getCmd) doGet(reposPathList []pathutil.ReposPath, lockJSON *lockjson.
 	getCount := 0
 	// Invoke installing / upgrading tasks
 	for _, reposPath := range reposPathList {
-		repos, err := lockJSON.Repos.FindByPath(reposPath)
-		if err != nil {
-			repos = nil
-		}
+		repos := lockJSON.Repos.FindByPath(reposPath)
 		if repos == nil || repos.Type == lockjson.ReposGitType {
 			go cmd.getParallel(reposPath, repos, cfg, done)
 			getCount++
@@ -333,8 +335,8 @@ func (cmd *getCmd) getParallel(reposPath pathutil.ReposPath, repos *lockjson.Rep
 func (cmd *getCmd) installPlugin(reposPath pathutil.ReposPath, repos *lockjson.Repos, cfg *config.Config, done chan<- getParallelResult) {
 	// true:upgrade, false:install
 	fullReposPath := reposPath.FullPath()
-	doUpgrade := cmd.upgrade && pathutil.Exists(fullReposPath)
 	doInstall := !pathutil.Exists(fullReposPath)
+	doUpgrade := cmd.upgrade && !doInstall
 
 	var fromHash string
 	var err error
@@ -566,10 +568,7 @@ func (cmd *getCmd) downloadPlugconf(reposPath pathutil.ReposPath) error {
 // * Add repos to 'repos' if not found
 // * Add repos to 'profiles[]/repos_path' if not found
 func (*getCmd) updateReposVersion(lockJSON *lockjson.LockJSON, reposPath pathutil.ReposPath, reposType lockjson.ReposType, version string, profile *lockjson.Profile) bool {
-	repos, err := lockJSON.Repos.FindByPath(reposPath)
-	if err != nil {
-		repos = nil
-	}
+	repos := lockJSON.Repos.FindByPath(reposPath)
 
 	added := false
 

subcmd/get_test.go

@@ -495,6 +495,32 @@ func TestErrVoltGetNotFound(t *testing.T) {
 	}
 }
 
+// [error] Specify plugin which already has (A, B, K)
+func TestErrVoltGetDupRepos(t *testing.T) {
+	// =============== setup =============== //
+
+	testutil.SetUpEnv(t)
+
+	// =============== run =============== //
+
+	out, err := testutil.RunVolt("get", "github.com/tyru/caw.vim")
+	// (A, B)
+	testutil.SuccessExit(t, out, err)
+	reposPath := pathutil.ReposPath("github.com/tyru/caw.vim")
+
+	for _, path := range []string{"github.com/tyru/caw.vim", "github.com/tyru/CaW.vim"} {
+		out, err = testutil.RunVolt("get", path)
+		// (A, B)
+		testutil.SuccessExit(t, out, err)
+
+		// (K)
+		msg := fmt.Sprintf(fmtAlreadyExists, reposPath)
+		if !bytes.Contains(out, []byte(msg)) {
+			t.Errorf("Output does not contain %q\n%s", msg, string(out))
+		}
+	}
+}
+
 func testReposPathWereAdded(t *testing.T, reposPath pathutil.ReposPath) {
 	t.Helper()
 	lockJSON, err := lockjson.Read()
@@ -593,9 +619,8 @@ func gitCommitOne(reposPath pathutil.ReposPath) (prev plumbing.Hash, current plu
 	head, err := r.Head()
 	if err != nil {
 		return
-	} else {
-		prev = head.Hash()
 	}
+	prev = head.Hash()
 	w, err := r.Worktree()
 	if err != nil {
 		return
@@ -620,15 +645,13 @@ func gitResetHard(reposPath pathutil.ReposPath, ref string) (current plumbing.Ha
 	head, err := r.Head()
 	if err != nil {
 		return
-	} else {
-		next = head.Hash()
 	}
+	next = head.Hash()
 	rev, err := r.ResolveRevision(plumbing.Revision(ref))
 	if err != nil {
 		return
-	} else {
-		current = *rev
 	}
+	current = *rev
 	err = w.Reset(&git.ResetOptions{
 		Commit: current,
 		Mode:   git.HardReset,

subcmd/profile.go

@@ -538,9 +538,8 @@ func (cmd *profileCmd) parseAddArgs(lockJSON *lockjson.LockJSON, subCmd string,
 
 	// Validate if all repositories exist in repos[]
 	for i := range reposPathList {
-		_, err := lockJSON.Repos.FindByPath(reposPathList[i])
-		if err != nil {
-			return "", nil, err
+		if !lockJSON.Repos.Contains(reposPathList[i]) {
+			return "", nil, errors.New("repos '" + reposPathList[i].String() + "' does not exist")
 		}
 	}
 

subcmd/rm.go

@@ -123,6 +123,17 @@ func (cmd *rmCmd) doRemove(reposPathList []pathutil.ReposPath) error {
 	}
 	defer transaction.Remove()
 
+	// Get the existing entries if already have it
+	// (e.g. github.com/tyru/CaW.vim -> github.com/tyru/caw.vim)
+	for i := range reposPathList {
+		if r := lockJSON.Repos.FindByPath(reposPathList[i]); r != nil {
+			reposPathList[i] = r.Path
+		}
+		fmt.Printf("%+v\n", reposPathList[i])
+		fmt.Printf("  fullpath:%+v\n", reposPathList[i].FullPath())
+		fmt.Printf("  plugconf:%+v\n", reposPathList[i].Plugconf())
+	}
+
 	// Check if specified plugins are depended by some plugins
 	for _, reposPath := range reposPathList {
 		rdeps, err := plugconf.RdepsOf(reposPath, lockJSON.Repos)