TBR: Revert "TBR: Use unique keys and not project names to determine equality."

This reverts commit 0a86786e85d769080d964905072df01fd09acd30.

Change-Id: I84bd0ccca74e1160e7b883a5dd6616bb03a48f40
diff --git a/contrib.go b/contrib.go
index 5c4897b..066850a 100644
--- a/contrib.go
+++ b/contrib.go
@@ -126,7 +126,7 @@
 }
 
 func runContributors(jirix *jiri.X, args []string) error {
-	localProjects, err := project.LocalProjects(jirix, project.FastScan)
+	projects, err := project.LocalProjects(jirix, project.FastScan)
 	if err != nil {
 		return err
 	}
@@ -134,8 +134,8 @@
 	if len(args) != 0 {
 		projectNames = set.String.FromSlice(args)
 	} else {
-		for _, p := range localProjects {
-			projectNames[p.Name] = struct{}{}
+		for name, _ := range projects {
+			projectNames[name] = struct{}{}
 		}
 	}
 
@@ -145,43 +145,41 @@
 	}
 	contributors := map[string]*contributor{}
 	for name, _ := range projectNames {
-		projects := localProjects.Find(name)
-		if len(projects) == 0 {
+		project, ok := projects[name]
+		if !ok {
 			continue
 		}
-		for _, project := range projects {
-			if err := jirix.Run().Chdir(project.Path); err != nil {
+		if err := jirix.Run().Chdir(project.Path); err != nil {
+			return err
+		}
+		switch project.Protocol {
+		case "git":
+			lines, err := listCommitters(jirix)
+			if err != nil {
 				return err
 			}
-			switch project.Protocol {
-			case "git":
-				lines, err := listCommitters(jirix)
-				if err != nil {
-					return err
+			for _, line := range lines {
+				matches := contributorRE.FindStringSubmatch(line)
+				if got, want := len(matches), 4; got != want {
+					return fmt.Errorf("unexpected length of %v: got %v, want %v", matches, got, want)
 				}
-				for _, line := range lines {
-					matches := contributorRE.FindStringSubmatch(line)
-					if got, want := len(matches), 4; got != want {
-						return fmt.Errorf("unexpected length of %v: got %v, want %v", matches, got, want)
-					}
-					count, err := strconv.Atoi(strings.TrimSpace(matches[1]))
-					if err != nil {
-						return fmt.Errorf("Atoi(%v) failed: %v", strings.TrimSpace(matches[1]), err)
-					}
-					c := &contributor{
-						count: count,
-						email: strings.TrimSpace(matches[3]),
-						name:  strings.TrimSpace(matches[2]),
-					}
-					if c.email == "jenkins.veyron@gmail.com" || c.email == "jenkins.veyron.rw@gmail.com" {
-						continue
-					}
-					c.email, c.name = canonicalize(aliases, c.email, c.name)
-					if existing, ok := contributors[c.name]; ok {
-						existing.count += c.count
-					} else {
-						contributors[c.name] = c
-					}
+				count, err := strconv.Atoi(strings.TrimSpace(matches[1]))
+				if err != nil {
+					return fmt.Errorf("Atoi(%v) failed: %v", strings.TrimSpace(matches[1]), err)
+				}
+				c := &contributor{
+					count: count,
+					email: strings.TrimSpace(matches[3]),
+					name:  strings.TrimSpace(matches[2]),
+				}
+				if c.email == "jenkins.veyron@gmail.com" || c.email == "jenkins.veyron.rw@gmail.com" {
+					continue
+				}
+				c.email, c.name = canonicalize(aliases, c.email, c.name)
+				if existing, ok := contributors[c.name]; ok {
+					existing.count += c.count
+				} else {
+					contributors[c.name] = c
 				}
 			}
 		}
diff --git a/project.go b/project.go
index fa2e77b..f59871a 100644
--- a/project.go
+++ b/project.go
@@ -7,6 +7,7 @@
 import (
 	"encoding/json"
 	"fmt"
+	"path"
 	"path/filepath"
 	"sort"
 	"strings"
@@ -61,14 +62,13 @@
 	if err != nil {
 		return err
 	}
-	var projects project.Projects
+	projects := map[string]project.Project{}
 	if len(args) > 0 {
 		for _, arg := range args {
-			p, err := localProjects.FindUnique(arg)
-			if err != nil {
-				fmt.Fprintf(jirix.Stderr(), "Error finding local project %q: %v.\n", p.Name, err)
+			if p, ok := localProjects[arg]; ok {
+				projects[p.Name] = p
 			} else {
-				projects[p.Key()] = p
+				fmt.Fprintf(jirix.Stderr(), "Local project %q not found.\n", p.Name)
 			}
 		}
 	} else {
@@ -94,21 +94,21 @@
 	if err != nil {
 		return err
 	}
-	var keys project.ProjectKeys
-	for key := range states {
-		keys = append(keys, key)
+	names := []string{}
+	for name := range states {
+		names = append(names, name)
 	}
-	sort.Sort(keys)
+	sort.Strings(names)
 
-	for _, key := range keys {
-		state := states[key]
+	for _, name := range names {
+		state := states[name]
 		if noPristineFlag {
 			pristine := len(state.Branches) == 1 && state.CurrentBranch == "master" && !state.HasUncommitted && !state.HasUntracked
 			if pristine {
 				continue
 			}
 		}
-		fmt.Fprintf(jirix.Stdout(), "project-key=%q path=%q\n", key, state.Project.Path)
+		fmt.Fprintf(jirix.Stdout(), "project=%q path=%q\n", path.Base(name), state.Project.Path)
 		if branchesFlag {
 			for _, branch := range state.Branches {
 				s := "  "
@@ -144,20 +144,20 @@
 	if err != nil {
 		return err
 	}
-	var keys project.ProjectKeys
-	for key := range states {
-		keys = append(keys, key)
+	names := []string{}
+	for name := range states {
+		names = append(names, name)
 	}
-	sort.Sort(keys)
+	sort.Strings(names)
 
-	// Get the key of the current project.
-	currentProjectKey, err := project.CurrentProjectKey(jirix)
+	// Get the name of the current project.
+	currentProjectName, err := project.CurrentProjectName(jirix)
 	if err != nil {
 		return err
 	}
 	var statuses []string
-	for _, key := range keys {
-		state := states[key]
+	for _, name := range names {
+		state := states[name]
 		status := ""
 		if checkDirtyFlag {
 			if state.HasUncommitted {
@@ -168,8 +168,8 @@
 			}
 		}
 		short := state.CurrentBranch + status
-		long := filepath.Base(states[key].Project.Name) + ":" + short
-		if key == currentProjectKey {
+		long := filepath.Base(name) + ":" + short
+		if name == currentProjectName {
 			if showNameFlag {
 				statuses = append([]string{long}, statuses...)
 			} else {
diff --git a/project/.api b/project/.api
index 82b6d30..7558eaa 100644
--- a/project/.api
+++ b/project/.api
@@ -5,24 +5,18 @@
 pkg project, func CleanupProjects(*jiri.X, Projects, bool) error
 pkg project, func CreateSnapshot(*jiri.X, string) error
 pkg project, func CurrentManifest(*jiri.X) (*Manifest, error)
-pkg project, func CurrentProjectKey(*jiri.X) (ProjectKey, error)
+pkg project, func CurrentProjectName(*jiri.X) (string, error)
 pkg project, func DataDirPath(*jiri.X, string) (string, error)
 pkg project, func GerritHost(*jiri.X) (string, error)
-pkg project, func GetProjectStates(*jiri.X, bool) (map[ProjectKey]*ProjectState, error)
+pkg project, func GetProjectStates(*jiri.X, bool) (map[string]*ProjectState, error)
 pkg project, func GitHost(*jiri.X) (string, error)
 pkg project, func InstallTools(*jiri.X, string) error
 pkg project, func LocalProjects(*jiri.X, ScanMode) (Projects, error)
-pkg project, func ParseNames(*jiri.X, []string, map[string]struct{}) (Projects, error)
+pkg project, func ParseNames(*jiri.X, []string, map[string]struct{}) (map[string]Project, error)
 pkg project, func PollProjects(*jiri.X, map[string]struct{}) (Update, error)
 pkg project, func ReadManifest(*jiri.X) (Projects, Tools, error)
 pkg project, func TransitionBinDir(*jiri.X) error
 pkg project, func UpdateUniverse(*jiri.X, bool) error
-pkg project, method (Project) Key() ProjectKey
-pkg project, method (ProjectKeys) Len() int
-pkg project, method (ProjectKeys) Less(int, int) bool
-pkg project, method (ProjectKeys) Swap(int, int)
-pkg project, method (Projects) Find(string) Projects
-pkg project, method (Projects) FindUnique(string) (Project, error)
 pkg project, method (UnsupportedProtocolErr) Error() string
 pkg project, type BranchState struct
 pkg project, type BranchState struct, HasGerritMessage bool
@@ -68,15 +62,13 @@
 pkg project, type Project struct, Remote string
 pkg project, type Project struct, RemoteBranch string
 pkg project, type Project struct, Revision string
-pkg project, type ProjectKey string
-pkg project, type ProjectKeys []ProjectKey
 pkg project, type ProjectState struct
 pkg project, type ProjectState struct, Branches []BranchState
 pkg project, type ProjectState struct, CurrentBranch string
 pkg project, type ProjectState struct, HasUncommitted bool
 pkg project, type ProjectState struct, HasUntracked bool
 pkg project, type ProjectState struct, Project Project
-pkg project, type Projects map[ProjectKey]Project
+pkg project, type Projects map[string]Project
 pkg project, type ScanMode bool
 pkg project, type Tool struct
 pkg project, type Tool struct, Data string
diff --git a/project/paths.go b/project/paths.go
index ce0e523..51ed88b 100644
--- a/project/paths.go
+++ b/project/paths.go
@@ -31,12 +31,10 @@
 	if !ok {
 		return "", fmt.Errorf("tool %q not found in the manifest", toolName)
 	}
-	// TODO(nlacasse): Tools refer to their project by name, but project name
-	// might not be unique.  We really should stop telling telling tools what their
-	// projects are.
-	project, err := projects.FindUnique(tool.Project)
-	if err != nil {
-		return "", err
+	projectName := tool.Project
+	project, ok := projects[projectName]
+	if !ok {
+		return "", fmt.Errorf("project %q not found in the manifest", projectName)
 	}
 	return filepath.Join(project.Path, tool.Data), nil
 }
@@ -63,6 +61,8 @@
 	return getHost(jirix, "git")
 }
 
+// toAbs returns the given path rooted in JIRI_ROOT, if it is not already an
+// absolute path.
 func toAbs(jirix *jiri.X, path string) string {
 	if filepath.IsAbs(path) {
 		return path
diff --git a/project/project.go b/project/project.go
index b1948db..1feca1c 100644
--- a/project/project.go
+++ b/project/project.go
@@ -106,15 +106,8 @@
 	Name string `xml:"name,attr"`
 }
 
-// ProjectKey is a unique string for a project.
-type ProjectKey string
-
-// ProjectKeys is a slice of ProjectKeys implementing the Sort interface.
-type ProjectKeys []ProjectKey
-
-func (pks ProjectKeys) Len() int           { return len(pks) }
-func (pks ProjectKeys) Less(i, j int) bool { return string(pks[i]) < string(pks[j]) }
-func (pks ProjectKeys) Swap(i, j int)      { pks[i], pks[j] = pks[j], pks[i] }
+// Projects maps project names to their detailed description.
+type Projects map[string]Project
 
 // Project represents a jiri project.
 type Project struct {
@@ -143,47 +136,6 @@
 	Revision string `xml:"revision,attr"`
 }
 
-// projectKeySeparator is a reserved string used in ProjectKeys.  It cannot
-// occur in Project names or remotes.
-const projectKeySeparator = "="
-
-// Key returns a unique ProjectKey for the project.
-func (p Project) Key() ProjectKey {
-	return ProjectKey(p.Name + projectKeySeparator + p.Remote)
-}
-
-// Projects maps ProjectKeys to Projects.
-type Projects map[ProjectKey]Project
-
-// Find returns all projects in Projects with the given key or name.
-func (ps Projects) Find(name string) Projects {
-	projects := Projects{}
-	for _, p := range ps {
-		if name == p.Name {
-			projects[p.Key()] = p
-		}
-	}
-	return projects
-}
-
-// FindUnique returns the project in Projects with the given key or
-// name, and returns an error if none or multiple matching projects are found.
-func (ps Projects) FindUnique(name string) (Project, error) {
-	var p Project
-	projects := ps.Find(name)
-	if len(projects) == 0 {
-		return p, fmt.Errorf("no projects found with name %q", name)
-	}
-	if len(projects) > 1 {
-		return p, fmt.Errorf("multiple projects found with name %q", name)
-	}
-	// Return the only project in projects.
-	for _, project := range projects {
-		p = project
-	}
-	return p, nil
-}
-
 // Tools maps jiri tool names, to their detailed description.
 type Tools map[string]Tool
 
@@ -319,10 +271,10 @@
 	return nil
 }
 
-// CurrentProjectKey gets the key of the current project from the current
-// directory by reading the jiri project metadata located in a directory at the
-// root of the current repository.
-func CurrentProjectKey(jirix *jiri.X) (ProjectKey, error) {
+// CurrentProjectName gets the name of the current project from the
+// current directory by reading the jiri project metadata located in a
+// directory at the root of the current repository.
+func CurrentProjectName(jirix *jiri.X) (string, error) {
 	topLevel, err := jirix.Git().TopLevel()
 	if err != nil {
 		return "", nil
@@ -338,7 +290,7 @@
 		if err := xml.Unmarshal(bytes, &project); err != nil {
 			return "", fmt.Errorf("Unmarshal() failed: %v", err)
 		}
-		return project.Key(), nil
+		return project.Name, nil
 	}
 	return "", nil
 }
@@ -666,9 +618,9 @@
 	workspaceSet := map[string]bool{}
 	for _, tool := range tools {
 		toolPkgs = append(toolPkgs, tool.Package)
-		toolProject, err := projects.FindUnique(tool.Project)
-		if err != nil {
-			return err
+		toolProject, ok := projects[tool.Project]
+		if !ok {
+			return fmt.Errorf("project not found for tool %v", tool.Name)
 		}
 		// Identify the Go workspace the tool is in. To this end we use a
 		// heuristic that identifies the maximal suffix of the project path
@@ -735,11 +687,11 @@
 		if tool.Package == "" {
 			continue
 		}
-		project, err := localProjects.FindUnique(tool.Project)
-		if err != nil {
-			return err
+		project, ok := localProjects[tool.Project]
+		if !ok {
+			fmt.Errorf("unknown project %v for tool %v", tool.Project, tool.Name)
 		}
-		toolProjects[project.Key()] = project
+		toolProjects[tool.Project] = project
 		toolsToBuild[tool.Name] = tool
 		toolNames = append(toolNames, tool.Name)
 	}
@@ -876,10 +828,10 @@
 		if absPath != project.Path {
 			return fmt.Errorf("project %v has path %v but was found in %v", project.Name, project.Path, absPath)
 		}
-		if p, ok := projects[project.Key()]; ok {
-			return fmt.Errorf("name conflict: both %v and %v contain project with key %v", p.Path, project.Path, project.Key())
+		if p, ok := projects[project.Name]; ok {
+			return fmt.Errorf("name conflict: both %v and %v contain the project %v", p.Path, project.Path, project.Name)
 		}
-		projects[project.Key()] = project
+		projects[project.Name] = project
 	}
 
 	// Recurse into all the sub directories.
@@ -1038,7 +990,7 @@
 			return UnsupportedProtocolErr(project.Protocol)
 		}
 	}
-	return ApplyToLocalMaster(jirix, Projects{project.Key(): project}, fn)
+	return ApplyToLocalMaster(jirix, Projects{project.Name: project}, fn)
 }
 
 // loadManifest loads the given manifest, processing all of its
@@ -1069,13 +1021,10 @@
 	}
 	// Process all projects.
 	for _, project := range m.Projects {
-		if strings.Contains(project.Name, projectKeySeparator) {
-			return fmt.Errorf("project name cannot contain %q: %q", projectKeySeparator, project.Name)
-		}
 		if project.Exclude {
 			// Exclude the project in case it was
 			// previously included.
-			delete(projects, project.Key())
+			delete(projects, project.Name)
 			continue
 		}
 		// Replace the relative path with an absolute one.
@@ -1098,7 +1047,7 @@
 		if project.RemoteBranch == "" {
 			project.RemoteBranch = "master"
 		}
-		projects[project.Key()] = project
+		projects[project.Name] = project
 	}
 	// Process all tools.
 	for _, tool := range m.Tools {
@@ -1126,9 +1075,10 @@
 			delete(hooks, hook.Name)
 			continue
 		}
-		project, err := projects.FindUnique(hook.Project)
-		if err != nil {
-			return fmt.Errorf("error while finding project %q for hook %q: %v", hook.Project, hook.Name, err)
+		project, found := projects[hook.Project]
+		if !found {
+			return fmt.Errorf("hook %v specified project %v which was not found",
+				hook.Name, hook.Project)
 		}
 		// Replace project-relative path with absolute path.
 		hook.Path = filepath.Join(project.Path, hook.Path)
@@ -1615,9 +1565,6 @@
 }
 
 func (op nullOperation) Run(jirix *jiri.X, manifest *Manifest) error {
-	if err := writeMetadata(jirix, op.project, op.project.Path); err != nil {
-		return err
-	}
 	return addProjectToManifest(jirix, manifest, op.project)
 }
 
@@ -1680,16 +1627,16 @@
 // projects.
 func computeOperations(localProjects, remoteProjects Projects, gc bool) (operations, error) {
 	result := operations{}
-	allProjects := map[ProjectKey]struct{}{}
-	for _, p := range localProjects {
-		allProjects[p.Key()] = struct{}{}
+	allProjects := map[string]struct{}{}
+	for name, _ := range localProjects {
+		allProjects[name] = struct{}{}
 	}
-	for _, p := range remoteProjects {
-		allProjects[p.Key()] = struct{}{}
+	for name, _ := range remoteProjects {
+		allProjects[name] = struct{}{}
 	}
-	for key, _ := range allProjects {
-		if localProject, ok := localProjects[key]; ok {
-			if remoteProject, ok := remoteProjects[key]; ok {
+	for name, _ := range allProjects {
+		if localProject, ok := localProjects[name]; ok {
+			if remoteProject, ok := remoteProjects[name]; ok {
 				if localProject.Path != remoteProject.Path {
 					// moveOperation also does an update, so we don't need to
 					// check the revision here.
@@ -1720,14 +1667,14 @@
 					source:      localProject.Path,
 				}, gc})
 			}
-		} else if remoteProject, ok := remoteProjects[key]; ok {
+		} else if remoteProject, ok := remoteProjects[name]; ok {
 			result = append(result, createOperation{commonOperation{
 				destination: remoteProject.Path,
 				project:     remoteProject,
 				source:      "",
 			}})
 		} else {
-			return nil, fmt.Errorf("project with key %v does not exist", key)
+			return nil, fmt.Errorf("project %v does not exist", name)
 		}
 	}
 	sort.Sort(result)
@@ -1736,25 +1683,23 @@
 
 // ParseNames identifies the set of projects that a jiri command should
 // be applied to.
-func ParseNames(jirix *jiri.X, args []string, defaultProjects map[string]struct{}) (Projects, error) {
-	manifestProjects, _, err := ReadManifest(jirix)
+func ParseNames(jirix *jiri.X, args []string, defaultProjects map[string]struct{}) (map[string]Project, error) {
+	projects, _, err := ReadManifest(jirix)
 	if err != nil {
 		return nil, err
 	}
-	result := Projects{}
+	result := map[string]Project{}
 	if len(args) == 0 {
 		// Use the default set of projects.
 		args = set.String.ToSlice(defaultProjects)
 	}
 	for _, name := range args {
-		projects := manifestProjects.Find(name)
-		if len(projects) == 0 {
+		if project, ok := projects[name]; ok {
+			result[name] = project
+		} else {
 			// Issue a warning if the target project does not exist in the
 			// project manifest.
-			fmt.Fprintf(jirix.Stderr(), "project %q does not exist in the project manifest", name)
-		}
-		for _, project := range projects {
-			result[project.Key()] = project
+			fmt.Fprintf(jirix.Stderr(), "WARNING: project %q does not exist in the project manifest and will be skipped\n", name)
 		}
 	}
 	return result, nil
diff --git a/project/project_test.go b/project/project_test.go
index 8392501..7c248a9 100644
--- a/project/project_test.go
+++ b/project/project_test.go
@@ -154,7 +154,7 @@
 	}
 }
 
-func deleteProject(t *testing.T, jirix *jiri.X, manifestDir, remote string) {
+func deleteProject(t *testing.T, jirix *jiri.X, manifestDir, name string) {
 	manifestFile := filepath.Join(manifestDir, "v2", "default")
 	data, err := ioutil.ReadFile(manifestFile)
 	if err != nil {
@@ -164,7 +164,7 @@
 	if err := xml.Unmarshal(data, &manifest); err != nil {
 		t.Fatalf("Unmarshal() failed: %v\n%v", err, data)
 	}
-	manifest.Projects = append(manifest.Projects, project.Project{Exclude: true, Name: remote, Remote: remote})
+	manifest.Projects = append(manifest.Projects, project.Project{Exclude: true, Name: name})
 	commitManifest(t, jirix, &manifest, manifestDir)
 }
 
diff --git a/project/state.go b/project/state.go
index 98c92d4..3b19dd7 100644
--- a/project/state.go
+++ b/project/state.go
@@ -70,18 +70,18 @@
 	ch <- nil
 }
 
-func GetProjectStates(jirix *jiri.X, checkDirty bool) (map[ProjectKey]*ProjectState, error) {
+func GetProjectStates(jirix *jiri.X, checkDirty bool) (map[string]*ProjectState, error) {
 	projects, err := LocalProjects(jirix, FastScan)
 	if err != nil {
 		return nil, err
 	}
-	states := make(map[ProjectKey]*ProjectState, len(projects))
+	states := make(map[string]*ProjectState, len(projects))
 	sem := make(chan error, len(projects))
-	for key, project := range projects {
+	for name, project := range projects {
 		state := &ProjectState{
 			Project: project,
 		}
-		states[key] = state
+		states[name] = state
 		// jirix is not threadsafe, so we make a clone for each goroutine.
 		go setProjectState(jirix.Clone(tool.ContextOpts{}), state, checkDirty, sem)
 	}
diff --git a/snapshot.go b/snapshot.go
index 55f0868..21c0032 100644
--- a/snapshot.go
+++ b/snapshot.go
@@ -116,7 +116,7 @@
 		Protocol: "git",
 		Revision: "HEAD",
 	}
-	if err := project.ApplyToLocalMaster(jirix, project.Projects{p.Key(): p}, createFn); err != nil {
+	if err := project.ApplyToLocalMaster(jirix, project.Projects{p.Name: p}, createFn); err != nil {
 		return err
 	}
 	return nil