jiri: Be consistent about absolute/relative paths.
The new policy is that Projects in memory should have absolute paths,
but files that we write (manifests and metadata) should use relative
paths.
This policy applies to all Project properties that are paths. Currently
this includes Path, RunHook, and GitHooks properties.
To help enforce this policy, there are two new methods on Project:
relativizePaths and absolutizePaths. relativizePaths is called when
writing a project to file, and absolutizePaths is called when reading a
project.
Note that unlike ProjectFromFile, the ManifestFromFile function does not
convert project paths to absolute paths. This is because when loading a
manifest through a remote import, its common to specify a root directory
different from the jiri root. The usual way to load a manifest is
though LoadManifest, which does convert project paths to absolute paths,
and uses the correct root directory.
This CL also removes the "root" property from operations, and we stop
passing "root" around during computeOperations. This was always the
empty string, so it never mattered.
Change-Id: I48b03ff7730b1e86b90717330c632ef9d02e5e10
diff --git a/project/project.go b/project/project.go
index 41f5ccb..dc972c7 100644
--- a/project/project.go
+++ b/project/project.go
@@ -66,6 +66,12 @@
// ManifestFromFile returns a manifest parsed from the contents of filename,
// with defaults filled in.
+//
+// Note that unlike ProjectFromFile, ManifestFromFile does not convert project
+// paths to absolute paths because it's possible to load a manifest with a
+// specific root directory different from jirix.Root. The usual way to load a
+// manifest is through LoadManifest, which does absolutize the paths, and uses
+// the correct root directory.
func ManifestFromFile(jirix *jiri.X, filename string) (*Manifest, error) {
data, err := jirix.NewSeq().ReadFile(filename)
if err != nil {
@@ -140,9 +146,16 @@
Done()
}
-// ToFile writes the manifest m to a file with the given filename, with defaults
-// unfilled.
+// ToFile writes the manifest m to a file with the given filename, with
+// defaults unfilled and all project paths relative to the jiri root.
func (m *Manifest) ToFile(jirix *jiri.X, filename string) error {
+ // Replace absolute paths with relative paths to make it possible to move
+ // the $JIRI_ROOT directory locally.
+ for _, project := range m.Projects {
+ if err := project.relativizePaths(jirix.Root); err != nil {
+ return err
+ }
+ }
data, err := m.ToBytes()
if err != nil {
return err
@@ -388,7 +401,7 @@
)
// ProjectFromFile returns a project parsed from the contents of filename,
-// with defaults filled in.
+// with defaults filled in and all paths absolute.
func ProjectFromFile(jirix *jiri.X, filename string) (*Project, error) {
data, err := jirix.NewSeq().ReadFile(filename)
if err != nil {
@@ -412,15 +425,21 @@
if err := p.fillDefaults(); err != nil {
return nil, err
}
+ p.absolutizePaths(jirix.Root)
return p, nil
}
// ToFile writes the project p to a file with the given filename, with defaults
-// unfilled.
+// unfilled and all paths relative to the jiri root.
func (p Project) ToFile(jirix *jiri.X, filename string) error {
if err := p.unfillDefaults(); err != nil {
return err
}
+ // Replace absolute paths with relative paths to make it possible to move
+ // the $JIRI_ROOT directory locally.
+ if err := p.relativizePaths(jirix.Root); err != nil {
+ return err
+ }
data, err := xml.Marshal(p)
if err != nil {
return fmt.Errorf("project xml.Marshal failed: %v", err)
@@ -433,6 +452,45 @@
return safeWriteFile(jirix, filename, data)
}
+// absolutizePaths makes all relative paths absolute by prepending basepath.
+func (p *Project) absolutizePaths(basepath string) {
+ if p.Path != "" && !filepath.IsAbs(p.Path) {
+ p.Path = filepath.Join(basepath, p.Path)
+ }
+ if p.GitHooks != "" && !filepath.IsAbs(p.GitHooks) {
+ p.GitHooks = filepath.Join(basepath, p.GitHooks)
+ }
+ if p.RunHook != "" && !filepath.IsAbs(p.RunHook) {
+ p.RunHook = filepath.Join(basepath, p.RunHook)
+ }
+}
+
+// relativizePaths makes all absolute paths relative to basepath.
+func (p *Project) relativizePaths(basepath string) error {
+ if filepath.IsAbs(p.Path) {
+ relPath, err := filepath.Rel(basepath, p.Path)
+ if err != nil {
+ return err
+ }
+ p.Path = relPath
+ }
+ if filepath.IsAbs(p.GitHooks) {
+ relGitHooks, err := filepath.Rel(basepath, p.GitHooks)
+ if err != nil {
+ return err
+ }
+ p.GitHooks = relGitHooks
+ }
+ if filepath.IsAbs(p.RunHook) {
+ relRunHook, err := filepath.Rel(basepath, p.RunHook)
+ if err != nil {
+ return err
+ }
+ p.RunHook = relRunHook
+ }
+ return nil
+}
+
// Key returns the unique ProjectKey for the project.
func (p Project) Key() ProjectKey {
return MakeProjectKey(p.Name, p.Remote)
@@ -619,11 +677,6 @@
return err
}
for _, project := range localProjects {
- relPath, err := filepath.Rel(jirix.Root, project.Path)
- if err != nil {
- return err
- }
- project.Path = relPath
manifest.Projects = append(manifest.Projects, project)
}
@@ -801,7 +854,7 @@
// Compute difference between local and remote.
update := Update{}
- ops := computeOperations(localProjects, remoteProjects, false, "")
+ ops := computeOperations(localProjects, remoteProjects, false)
s := jirix.NewSeq()
for _, op := range ops {
name := op.Project().Name
@@ -1283,7 +1336,6 @@
if err != nil {
return Project{}, err
}
- project.Path = filepath.Join(jirix.Root, project.Path)
return *project, nil
}
@@ -1646,7 +1698,8 @@
}
// Collect projects.
for _, project := range m.Projects {
- project.Path = filepath.Join(jirix.Root, root, project.Path)
+ // Make paths absolute by prepending JIRI_ROOT/<root>.
+ project.absolutizePaths(filepath.Join(jirix.Root, root))
key := project.Key()
if dup, ok := ld.Projects[key]; ok && dup != project {
// TODO(toddw): Tell the user the other conflicting file.
@@ -1782,7 +1835,7 @@
defer jirix.TimerPop()
getRemoteHeadRevisions(jirix, remoteProjects)
- ops := computeOperations(localProjects, remoteProjects, gc, "")
+ ops := computeOperations(localProjects, remoteProjects, gc)
updates := newFsUpdates()
for _, op := range ops {
if err := op.Test(jirix, updates); err != nil {
@@ -1817,9 +1870,7 @@
}
s := jirix.NewSeq()
s.Verbose(true).Output([]string{fmt.Sprintf("running hook for project %q", op.Project().Name)})
- rootDir := filepath.Join(jirix.Root, op.Root())
- hook := filepath.Join(rootDir, op.Project().RunHook)
- if err := s.Dir(op.Project().Path).Capture(os.Stdout, os.Stderr).Last(hook, op.Kind()); err != nil {
+ if err := s.Dir(op.Project().Path).Capture(os.Stdout, os.Stderr).Last(op.Project().RunHook, op.Kind()); err != nil {
// TODO(nlacasse): Should we delete projectDir or perform some
// other cleanup in the event of a hook failure?
return fmt.Errorf("error running hook for project %q: %v", op.Project().Name, err)
@@ -1855,7 +1906,6 @@
// Apply git hooks, overwriting any existing hooks. Jiri is in control of
// writing all hooks.
gitHooksDstDir := filepath.Join(op.Project().Path, ".git", "hooks")
- gitHooksSrcDir := filepath.Join(jirix.Root, op.Root(), op.Project().GitHooks)
// Copy the specified GitHooks directory into the project's git
// hook directory. We walk the file system, creating directories
// and copying files as we encounter them.
@@ -1863,7 +1913,7 @@
if err != nil {
return err
}
- relPath, err := filepath.Rel(gitHooksSrcDir, path)
+ relPath, err := filepath.Rel(op.Project().GitHooks, path)
if err != nil {
return err
}
@@ -1878,7 +1928,7 @@
// The file *must* be executable to be picked up by git.
return s.WriteFile(dst, src, 0755).Done()
}
- if err := filepath.Walk(gitHooksSrcDir, copyFn); err != nil {
+ if err := filepath.Walk(op.Project().GitHooks, copyFn); err != nil {
return err
}
}
@@ -1900,14 +1950,6 @@
Chdir(metadataDir).Done(); err != nil {
return err
}
-
- // Replace absolute project paths with relative paths to make it
- // possible to move the $JIRI_ROOT directory locally.
- relPath, err := filepath.Rel(jirix.Root, project.Path)
- if err != nil {
- return err
- }
- project.Path = relPath
metadataFile := filepath.Join(metadataDir, jiri.ProjectMetaFile)
return project.ToFile(jirix, metadataFile)
}
@@ -1942,8 +1984,6 @@
Project() Project
// Kind returns the kind of operation.
Kind() string
- // Root returns the operation's root directory, relative to JIRI_ROOT.
- Root() string
// Run executes the operation.
Run(jirix *jiri.X) error
// String returns a string representation of the operation.
@@ -1962,18 +2002,12 @@
destination string
// source is the current project path.
source string
- // root is the directory inside JIRI_ROOT where this operation will run.
- root string
}
func (op commonOperation) Project() Project {
return op.project
}
-func (op commonOperation) Root() string {
- return op.root
-}
-
// createOperation represents the creation of a project.
type createOperation struct {
commonOperation
@@ -2247,7 +2281,7 @@
// system and manifest file respectively) and outputs a collection of
// operations that describe the actions needed to update the target
// projects.
-func computeOperations(localProjects, remoteProjects Projects, gc bool, root string) operations {
+func computeOperations(localProjects, remoteProjects Projects, gc bool) operations {
result := operations{}
allProjects := map[ProjectKey]bool{}
for _, p := range localProjects {
@@ -2264,27 +2298,25 @@
if project, ok := remoteProjects[key]; ok {
remote = &project
}
- result = append(result, computeOp(local, remote, gc, root))
+ result = append(result, computeOp(local, remote, gc))
}
sort.Sort(result)
return result
}
-func computeOp(local, remote *Project, gc bool, root string) operation {
+func computeOp(local, remote *Project, gc bool) operation {
switch {
case local == nil && remote != nil:
return createOperation{commonOperation{
destination: remote.Path,
project: *remote,
source: "",
- root: root,
}}
case local != nil && remote == nil:
return deleteOperation{commonOperation{
destination: "",
project: *local,
source: local.Path,
- root: root,
}, gc}
case local != nil && remote != nil:
switch {
@@ -2295,21 +2327,18 @@
destination: remote.Path,
project: *remote,
source: local.Path,
- root: root,
}}
case local.Revision != remote.Revision:
return updateOperation{commonOperation{
destination: remote.Path,
project: *remote,
source: local.Path,
- root: root,
}}
default:
return nullOperation{commonOperation{
destination: remote.Path,
project: *remote,
source: local.Path,
- root: root,
}}
}
default:
diff --git a/project/project_test.go b/project/project_test.go
index c61710e..fb12f3e 100644
--- a/project/project_test.go
+++ b/project/project_test.go
@@ -1012,7 +1012,7 @@
// Default fields are dropped when marshaled, and added when unmarshaled.
project.Project{
Name: "project1",
- Path: "path1",
+ Path: filepath.Join(jirix.Root, "path1"),
Protocol: "git",
Remote: "remote1",
RemoteBranch: "master",
@@ -1024,13 +1024,15 @@
{
project.Project{
Name: "project2",
- Path: "path2",
+ Path: filepath.Join(jirix.Root, "path2"),
+ GitHooks: filepath.Join(jirix.Root, "git-hooks"),
+ RunHook: filepath.Join(jirix.Root, "run-hook"),
Protocol: "git",
Remote: "remote2",
RemoteBranch: "branch2",
Revision: "rev2",
},
- `<project name="project2" path="path2" remote="remote2" remotebranch="branch2" revision="rev2"/>
+ `<project name="project2" path="path2" remote="remote2" remotebranch="branch2" revision="rev2" githooks="git-hooks" runhook="run-hook"/>
`,
},
}
@@ -1069,7 +1071,7 @@
`<Project name="project" path="path" remote="remote"/>`,
project.Project{
Name: "project",
- Path: "path",
+ Path: filepath.Join(jirix.Root, "path"),
Protocol: "git",
Remote: "remote",
RemoteBranch: "master",
@@ -1081,7 +1083,7 @@
`<Project name="project" path="path" remote="remote"></Project>`,
project.Project{
Name: "project",
- Path: "path",
+ Path: filepath.Join(jirix.Root, "path"),
Protocol: "git",
Remote: "remote",
RemoteBranch: "master",
@@ -1093,7 +1095,7 @@
`<Project this_attribute_should_be_ignored="junk" name="project" path="path" remote="remote" remotebranch="branch" revision="rev"></Project>`,
project.Project{
Name: "project",
- Path: "path",
+ Path: filepath.Join(jirix.Root, "path"),
Protocol: "git",
Remote: "remote",
RemoteBranch: "branch",
