TBR: Use unique keys and not project names to determine equality.
Jiri used to assume that project names were unique. Once we implement
importing remote manifests, that assumption becomes hard to enforce.
This CL adds a Key() method to projects which returns a new ProjectKey
type. The key is currently a combination of the project's name and
remote, although this is easy to change in the future.
The Projects map is now a map[ProjectKey]Project.
There is still a lot of code that tries to lookup projects by their
name. For instance:
* tools and hooks in the manifest both refer to their projects by name
* some command-line tools take a list of project names as arguments
To support these cases, I added two methods to Projects map:
* FindName, which returns all projects matching the name, and
* FindUnique, which returns a single project matching the name, or
an error if none or many are found.
NOTE: This is *almost* the same CL as
https://vanadium-review.googlesource.com/#/c/18205/. The only
difference is that we now call writeMetadata() inside
nullOperation.Run(). We need to do this to make sure the remotes in the
metadata file are always in sync with the remotes in the manifest,
otherwise the project keys will differ.
MultiPart: 1/2
Change-Id: I334ffed799e8900b01865da2211b2de4cd04e9bf
diff --git a/contrib.go b/contrib.go
index 066850a..5c4897b 100644
--- a/contrib.go
+++ b/contrib.go
@@ -126,7 +126,7 @@
}
func runContributors(jirix *jiri.X, args []string) error {
- projects, err := project.LocalProjects(jirix, project.FastScan)
+ localProjects, 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 name, _ := range projects {
- projectNames[name] = struct{}{}
+ for _, p := range localProjects {
+ projectNames[p.Name] = struct{}{}
}
}
@@ -145,41 +145,43 @@
}
contributors := map[string]*contributor{}
for name, _ := range projectNames {
- project, ok := projects[name]
- if !ok {
+ projects := localProjects.Find(name)
+ if len(projects) == 0 {
continue
}
- if err := jirix.Run().Chdir(project.Path); err != nil {
- return err
- }
- switch project.Protocol {
- case "git":
- lines, err := listCommitters(jirix)
- if err != nil {
+ for _, project := range projects {
+ if err := jirix.Run().Chdir(project.Path); 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)
- }
- count, err := strconv.Atoi(strings.TrimSpace(matches[1]))
+ switch project.Protocol {
+ case "git":
+ lines, err := listCommitters(jirix)
if err != nil {
- return fmt.Errorf("Atoi(%v) failed: %v", strings.TrimSpace(matches[1]), err)
+ return 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
+ 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
+ }
}
}
}
diff --git a/project.go b/project.go
index f59871a..fa2e77b 100644
--- a/project.go
+++ b/project.go
@@ -7,7 +7,6 @@
import (
"encoding/json"
"fmt"
- "path"
"path/filepath"
"sort"
"strings"
@@ -62,13 +61,14 @@
if err != nil {
return err
}
- projects := map[string]project.Project{}
+ var projects project.Projects
if len(args) > 0 {
for _, arg := range args {
- if p, ok := localProjects[arg]; ok {
- projects[p.Name] = p
+ p, err := localProjects.FindUnique(arg)
+ if err != nil {
+ fmt.Fprintf(jirix.Stderr(), "Error finding local project %q: %v.\n", p.Name, err)
} else {
- fmt.Fprintf(jirix.Stderr(), "Local project %q not found.\n", p.Name)
+ projects[p.Key()] = p
}
}
} else {
@@ -94,21 +94,21 @@
if err != nil {
return err
}
- names := []string{}
- for name := range states {
- names = append(names, name)
+ var keys project.ProjectKeys
+ for key := range states {
+ keys = append(keys, key)
}
- sort.Strings(names)
+ sort.Sort(keys)
- for _, name := range names {
- state := states[name]
+ for _, key := range keys {
+ state := states[key]
if noPristineFlag {
pristine := len(state.Branches) == 1 && state.CurrentBranch == "master" && !state.HasUncommitted && !state.HasUntracked
if pristine {
continue
}
}
- fmt.Fprintf(jirix.Stdout(), "project=%q path=%q\n", path.Base(name), state.Project.Path)
+ fmt.Fprintf(jirix.Stdout(), "project-key=%q path=%q\n", key, state.Project.Path)
if branchesFlag {
for _, branch := range state.Branches {
s := " "
@@ -144,20 +144,20 @@
if err != nil {
return err
}
- names := []string{}
- for name := range states {
- names = append(names, name)
+ var keys project.ProjectKeys
+ for key := range states {
+ keys = append(keys, key)
}
- sort.Strings(names)
+ sort.Sort(keys)
- // Get the name of the current project.
- currentProjectName, err := project.CurrentProjectName(jirix)
+ // Get the key of the current project.
+ currentProjectKey, err := project.CurrentProjectKey(jirix)
if err != nil {
return err
}
var statuses []string
- for _, name := range names {
- state := states[name]
+ for _, key := range keys {
+ state := states[key]
status := ""
if checkDirtyFlag {
if state.HasUncommitted {
@@ -168,8 +168,8 @@
}
}
short := state.CurrentBranch + status
- long := filepath.Base(name) + ":" + short
- if name == currentProjectName {
+ long := filepath.Base(states[key].Project.Name) + ":" + short
+ if key == currentProjectKey {
if showNameFlag {
statuses = append([]string{long}, statuses...)
} else {
diff --git a/project/.api b/project/.api
index 7558eaa..82b6d30 100644
--- a/project/.api
+++ b/project/.api
@@ -5,18 +5,24 @@
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 CurrentProjectName(*jiri.X) (string, error)
+pkg project, func CurrentProjectKey(*jiri.X) (ProjectKey, 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[string]*ProjectState, error)
+pkg project, func GetProjectStates(*jiri.X, bool) (map[ProjectKey]*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{}) (map[string]Project, error)
+pkg project, func ParseNames(*jiri.X, []string, map[string]struct{}) (Projects, 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
@@ -62,13 +68,15 @@
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[string]Project
+pkg project, type Projects map[ProjectKey]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 51ed88b..ce0e523 100644
--- a/project/paths.go
+++ b/project/paths.go
@@ -31,10 +31,12 @@
if !ok {
return "", fmt.Errorf("tool %q not found in the manifest", toolName)
}
- projectName := tool.Project
- project, ok := projects[projectName]
- if !ok {
- return "", fmt.Errorf("project %q not found in the manifest", projectName)
+ // 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
}
return filepath.Join(project.Path, tool.Data), nil
}
@@ -61,8 +63,6 @@
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 2364c52..f6f2d5f 100644
--- a/project/project.go
+++ b/project/project.go
@@ -106,8 +106,15 @@
Name string `xml:"name,attr"`
}
-// Projects maps project names to their detailed description.
-type Projects map[string]Project
+// 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] }
// Project represents a jiri project.
type Project struct {
@@ -136,6 +143,47 @@
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
@@ -271,10 +319,10 @@
return nil
}
-// 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) {
+// 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) {
topLevel, err := jirix.Git().TopLevel()
if err != nil {
return "", nil
@@ -290,7 +338,7 @@
if err := xml.Unmarshal(bytes, &project); err != nil {
return "", fmt.Errorf("Unmarshal() failed: %v", err)
}
- return project.Name, nil
+ return project.Key(), nil
}
return "", nil
}
@@ -618,9 +666,9 @@
workspaceSet := map[string]bool{}
for _, tool := range tools {
toolPkgs = append(toolPkgs, tool.Package)
- toolProject, ok := projects[tool.Project]
- if !ok {
- return fmt.Errorf("project not found for tool %v", tool.Name)
+ toolProject, err := projects.FindUnique(tool.Project)
+ if err != nil {
+ return err
}
// Identify the Go workspace the tool is in. To this end we use a
// heuristic that identifies the maximal suffix of the project path
@@ -687,11 +735,11 @@
if tool.Package == "" {
continue
}
- project, ok := localProjects[tool.Project]
- if !ok {
- fmt.Errorf("unknown project %v for tool %v", tool.Project, tool.Name)
+ project, err := localProjects.FindUnique(tool.Project)
+ if err != nil {
+ return err
}
- toolProjects[tool.Project] = project
+ toolProjects[project.Key()] = project
toolsToBuild[tool.Name] = tool
toolNames = append(toolNames, tool.Name)
}
@@ -828,10 +876,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.Name]; ok {
- return fmt.Errorf("name conflict: both %v and %v contain the project %v", p.Path, project.Path, project.Name)
+ 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())
}
- projects[project.Name] = project
+ projects[project.Key()] = project
}
// Recurse into all the sub directories.
@@ -990,7 +1038,7 @@
return UnsupportedProtocolErr(project.Protocol)
}
}
- return ApplyToLocalMaster(jirix, Projects{project.Name: project}, fn)
+ return ApplyToLocalMaster(jirix, Projects{project.Key(): project}, fn)
}
// loadManifest loads the given manifest, processing all of its
@@ -1021,10 +1069,13 @@
}
// 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.Name)
+ delete(projects, project.Key())
continue
}
// Replace the relative path with an absolute one.
@@ -1047,7 +1098,7 @@
if project.RemoteBranch == "" {
project.RemoteBranch = "master"
}
- projects[project.Name] = project
+ projects[project.Key()] = project
}
// Process all tools.
for _, tool := range m.Tools {
@@ -1075,10 +1126,9 @@
delete(hooks, hook.Name)
continue
}
- project, found := projects[hook.Project]
- if !found {
- return fmt.Errorf("hook %v specified project %v which was not found",
- hook.Name, hook.Project)
+ 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)
}
// Replace project-relative path with absolute path.
hook.Path = filepath.Join(project.Path, hook.Path)
@@ -1658,16 +1708,16 @@
// projects.
func computeOperations(localProjects, remoteProjects Projects, gc bool) (operations, error) {
result := operations{}
- allProjects := map[string]struct{}{}
- for name, _ := range localProjects {
- allProjects[name] = struct{}{}
+ allProjects := map[ProjectKey]struct{}{}
+ for _, p := range localProjects {
+ allProjects[p.Key()] = struct{}{}
}
- for name, _ := range remoteProjects {
- allProjects[name] = struct{}{}
+ for _, p := range remoteProjects {
+ allProjects[p.Key()] = struct{}{}
}
- for name, _ := range allProjects {
- if localProject, ok := localProjects[name]; ok {
- if remoteProject, ok := remoteProjects[name]; ok {
+ for key, _ := range allProjects {
+ if localProject, ok := localProjects[key]; ok {
+ if remoteProject, ok := remoteProjects[key]; ok {
if localProject.Path != remoteProject.Path {
// moveOperation also does an update, so we don't need to
// check the revision here.
@@ -1698,14 +1748,14 @@
source: localProject.Path,
}, gc})
}
- } else if remoteProject, ok := remoteProjects[name]; ok {
+ } else if remoteProject, ok := remoteProjects[key]; ok {
result = append(result, createOperation{commonOperation{
destination: remoteProject.Path,
project: remoteProject,
source: "",
}})
} else {
- return nil, fmt.Errorf("project %v does not exist", name)
+ return nil, fmt.Errorf("project with key %v does not exist", key)
}
}
sort.Sort(result)
@@ -1714,23 +1764,25 @@
// 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{}) (map[string]Project, error) {
- projects, _, err := ReadManifest(jirix)
+func ParseNames(jirix *jiri.X, args []string, defaultProjects map[string]struct{}) (Projects, error) {
+ manifestProjects, _, err := ReadManifest(jirix)
if err != nil {
return nil, err
}
- result := map[string]Project{}
+ result := Projects{}
if len(args) == 0 {
// Use the default set of projects.
args = set.String.ToSlice(defaultProjects)
}
for _, name := range args {
- if project, ok := projects[name]; ok {
- result[name] = project
- } else {
+ projects := manifestProjects.Find(name)
+ if len(projects) == 0 {
// Issue a warning if the target project does not exist in the
// project manifest.
- fmt.Fprintf(jirix.Stderr(), "WARNING: project %q does not exist in the project manifest and will be skipped\n", name)
+ fmt.Fprintf(jirix.Stderr(), "project %q does not exist in the project manifest", name)
+ }
+ for _, project := range projects {
+ result[project.Key()] = project
}
}
return result, nil
diff --git a/project/project_test.go b/project/project_test.go
index 9230999..37d4046 100644
--- a/project/project_test.go
+++ b/project/project_test.go
@@ -168,7 +168,7 @@
commitManifest(t, jirix, &manifest, manifestDir)
}
-func deleteProject(t *testing.T, jirix *jiri.X, manifestDir, name string) {
+func deleteProject(t *testing.T, jirix *jiri.X, manifestDir, remote string) {
manifestFile := filepath.Join(manifestDir, "v2", "default")
data, err := ioutil.ReadFile(manifestFile)
if err != nil {
@@ -178,7 +178,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: name})
+ manifest.Projects = append(manifest.Projects, project.Project{Exclude: true, Name: remote, Remote: remote})
commitManifest(t, jirix, &manifest, manifestDir)
}
diff --git a/project/state.go b/project/state.go
index 3b19dd7..98c92d4 100644
--- a/project/state.go
+++ b/project/state.go
@@ -70,18 +70,18 @@
ch <- nil
}
-func GetProjectStates(jirix *jiri.X, checkDirty bool) (map[string]*ProjectState, error) {
+func GetProjectStates(jirix *jiri.X, checkDirty bool) (map[ProjectKey]*ProjectState, error) {
projects, err := LocalProjects(jirix, FastScan)
if err != nil {
return nil, err
}
- states := make(map[string]*ProjectState, len(projects))
+ states := make(map[ProjectKey]*ProjectState, len(projects))
sem := make(chan error, len(projects))
- for name, project := range projects {
+ for key, project := range projects {
state := &ProjectState{
Project: project,
}
- states[name] = state
+ states[key] = 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 16da995..eef6c0c 100644
--- a/snapshot.go
+++ b/snapshot.go
@@ -117,7 +117,7 @@
Protocol: "git",
Revision: "HEAD",
}
- if err := project.ApplyToLocalMaster(jirix, project.Projects{p.Name: p}, createFn); err != nil {
+ if err := project.ApplyToLocalMaster(jirix, project.Projects{p.Key(): p}, createFn); err != nil {
return err
}
return nil
