[ops] Modify jiri project move behavior
Previously, `jiri` would attempt to move a directory atomically with
os.Rename(), falling back to exec'ing `mv` directly if the move crossed
a mount point.
This behavior does not handle a move of a directory to a child of
itself, in either the primary or fallback path. Instead we opt to route
all moves of directories thru a temporary swap directory in `.jiri_root`
to ensure we do not fall into any child path pitfalls and to avoid the
mount point complexities if we were to swap thru tmp.
Bug: b/195346750
Change-Id: Id99e3127c942f08e4da93d6a5ecf21189b3c8de5
Reviewed-on: https://fuchsia-review.googlesource.com/c/jiri/+/574151
Reviewed-by: Petr Hosek <phosek@google.com>
Commit-Queue: Nathan Mulcahey <nmulcahey@google.com>
diff --git a/project/operations.go b/project/operations.go
index e6ecabd..8ff9427 100644
--- a/project/operations.go
+++ b/project/operations.go
@@ -6,6 +6,7 @@
import (
"fmt"
+ "hash/fnv"
"io"
"io/ioutil"
"os"
@@ -371,11 +372,7 @@
}
// If it was nested project it might have been moved with its parent project
if op.source != op.destination {
- path, perm := filepath.Dir(op.destination), os.FileMode(0755)
- if err := os.MkdirAll(path, perm); err != nil {
- return fmtError(err)
- }
- if err := osutil.Rename(op.source, op.destination); err != nil {
+ if err := renameDir(jirix, op.source, op.destination); err != nil {
return fmtError(err)
}
}
@@ -930,3 +927,41 @@
}
return nil
}
+
+func renameDir(jirix *jiri.X, src, dst string) error {
+ // Parent directory permissions
+ perm := os.FileMode(0755)
+ swapDir := jirix.SwapDir()
+
+ // Hash src path as swap dir name
+ h := fnv.New32a()
+ h.Write([]byte(src))
+ tmp := filepath.Join(swapDir, fmt.Sprintf("%d", h.Sum32()))
+
+ // Ensure .jiri_root/swap exists
+ if err := os.MkdirAll(swapDir, perm); err != nil {
+ return err
+ }
+
+ // Move src -> tmp
+ if err := osutil.Rename(src, tmp); err != nil {
+ return err
+ }
+
+ // Ensure the dst's parent exists, it may have
+ // been within src
+ parentDir := filepath.Dir(dst)
+ if err := os.MkdirAll(parentDir, perm); err != nil {
+ return err
+ }
+
+ // Move tmp -> dst
+ if err := osutil.Rename(tmp, dst); err != nil {
+ if err := osutil.Rename(tmp, src); err != nil {
+ jirix.Logger.Errorf("Could not move %s to %s, original contents are in %s. Please complete the move manually", src, dst, tmp)
+ }
+ return err
+ }
+
+ return nil
+}
\ No newline at end of file
diff --git a/project/project_test.go b/project/project_test.go
index 08b17e4..bb439c4 100644
--- a/project/project_test.go
+++ b/project/project_test.go
@@ -1275,6 +1275,43 @@
checkReadme(t, fake.X, localProjects[1], "initial readme")
}
+// TestUpdateUniverseMovedProjectSelf checks that UpdateUniverse can move a
+// project to a path currently nested within itself.
+func TestUpdateUniverseMovedProjectSelf(t *testing.T) {
+ localProjects, fake, cleanup := setupUniverse(t)
+ defer cleanup()
+ if err := fake.UpdateUniverse(false); err != nil {
+ t.Fatal(err)
+ }
+
+ // Update the local path at which project 1 is located.
+ m, err := fake.ReadRemoteManifest()
+ if err != nil {
+ t.Fatal(err)
+ }
+ oldProjectPath := localProjects[1].Path
+ localProjects[1].Path = filepath.Join(oldProjectPath, "new-project-path")
+ projects := []project.Project{}
+ for _, p := range m.Projects {
+ if p.Name == localProjects[1].Name {
+ p.Path = localProjects[1].Path
+ }
+ projects = append(projects, p)
+ }
+ m.Projects = projects
+ if err := fake.WriteRemoteManifest(m); err != nil {
+ t.Fatal(err)
+ }
+ // Check that UpdateUniverse() moves the local copy of the project 1.
+ if err := fake.UpdateUniverse(false); err != nil {
+ t.Fatal(err)
+ }
+ if err := dirExists(localProjects[2].Path); err != nil {
+ t.Fatalf("expected project %q at path %q to exist but it did not", localProjects[1].Name, localProjects[1].Path)
+ }
+ checkReadme(t, fake.X, localProjects[1], "initial readme")
+}
+
// TestUpdateUniverseChangeRemote checks that UpdateUniverse can change remote
// of a project.
func TestUpdateUniverseChangeRemote(t *testing.T) {
diff --git a/x.go b/x.go
index fcd3a4f..0362e2f 100644
--- a/x.go
+++ b/x.go
@@ -458,6 +458,16 @@
return filepath.Join(x.RootMetaDir(), "scripts")
}
+// SwapDir returns the path to the swap directory. This is used
+// to stage moves of direcories that may be moving into subdirectories
+// of themselves or to handle unnesting of repositories.
+//
+// We use a directory in `.jiri_root` to prevent the complexities of an
+// arbitrary $TMPDIR which can cross mountpoints/filesystems.
+func (x *X) SwapDir() string {
+ return filepath.Join(x.RootMetaDir(), "swap")
+}
+
// UpdateHistoryDir returns the path to the update history directory.
func (x *X) UpdateHistoryDir() string {
return filepath.Join(x.RootMetaDir(), "update_history")
