Merge pull request #765 from thockin/v4_fix_relative_submodules

Make relative-path submodules work, via origin

main.go

@@ -1208,6 +1208,8 @@ func (git *repoSync) RunWithStdin(ctx context.Context, cwd absPath, stdin string
 // not, it will (re)initialize it.  After running this function, callers can
 // assume the repo is valid, though maybe empty.
 func (git *repoSync) initRepo(ctx context.Context) error {
+	needGitInit := false
+
 	// Check out the git root, and see if it is already usable.
 	_, err := os.Stat(git.root.String())
 	switch {
@@ -1218,6 +1220,7 @@ func (git *repoSync) initRepo(ctx context.Context) error {
 		if err := os.MkdirAll(git.root.String(), defaultDirMode); err != nil {
 			return err
 		}
+		needGitInit = true
 	case err != nil:
 		return err
 	default:
@@ -1225,7 +1228,6 @@ func (git *repoSync) initRepo(ctx context.Context) error {
 		git.log.V(3).Info("repo directory exists", "path", git.root)
 		if git.sanityCheckRepo(ctx) {
 			git.log.V(4).Info("repo directory is valid", "path", git.root)
-			return nil
 		} else {
 			// Maybe a previous run crashed?  Git won't use this dir.  We remove
 			// the contents rather than the dir itself, because a common use-case
@@ -1235,17 +1237,38 @@ func (git *repoSync) initRepo(ctx context.Context) error {
 			if err := removeDirContents(git.root, git.log); err != nil {
 				return fmt.Errorf("can't wipe unusable root directory: %w", err)
 			}
+			needGitInit = true
 		}
 	}
 
-	// Running `git init` in an existing repo is safe (according to git docs).
-	git.log.V(0).Info("initializing repo directory", "path", git.root)
-	if _, _, err := git.Run(ctx, git.root, "init", "-b", "git-sync"); err != nil {
-		return err
+	if needGitInit {
+		// Running `git init` in an existing repo is safe (according to git docs).
+		git.log.V(0).Info("initializing repo directory", "path", git.root)
+		if _, _, err := git.Run(ctx, git.root, "init", "-b", "git-sync"); err != nil {
+			return err
+		}
+		if !git.sanityCheckRepo(ctx) {
+			return fmt.Errorf("can't initialize git repo directory")
+		}
 	}
-	if !git.sanityCheckRepo(ctx) {
-		return fmt.Errorf("can't initialize git repo directory")
+
+	// The "origin" remote has special meaning, like in relative-path
+	// submodules.
+	if stdout, stderr, err := git.Run(ctx, git.root, "remote", "get-url", "origin"); err != nil {
+		if !strings.Contains(stderr, "No such remote") {
+			return err
+		}
+		// It doesn't exist - make it.
+		if _, _, err := git.Run(ctx, git.root, "remote", "add", "origin", git.repo); err != nil {
+			return err
+		}
+	} else if strings.TrimSpace(stdout) != git.repo {
+		// It exists, but is wrong.
+		if _, _, err := git.Run(ctx, git.root, "remote", "set-url", "origin", git.repo); err != nil {
+			return err
+		}
 	}
+
 	return nil
 }
 
@@ -1643,14 +1666,14 @@ func lastNonEmptyLine(text string) string {
 // is not a hash or is not known to this repo, even if it appears to be a hash,
 // this will return false.
 func (git *repoSync) IsKnownHash(ctx context.Context, ref string) (bool, error) {
-	output, _, err := git.Run(ctx, git.root, "rev-parse", ref+"^{commit}")
+	stdout, stderr, err := git.Run(ctx, git.root, "rev-parse", ref+"^{commit}")
 	if err != nil {
-		if strings.Contains(err.Error(), "unknown revision") {
+		if strings.Contains(stderr, "unknown revision") {
 			return false, nil
 		}
 		return false, err
 	}
-	line := lastNonEmptyLine(output)
+	line := lastNonEmptyLine(stdout)
 	return strings.HasPrefix(line, ref), nil
 }
 

pkg/cmd/cmd.go

@@ -45,13 +45,14 @@ func NewRunner(log logintf) Runner {
 	return Runner{log: log}
 }
 
-// Run runs given command
+// Run runs the given command, returning the stdout, stderr, and any error.
 func (r Runner) Run(ctx context.Context, cwd string, env []string, command string, args ...string) (string, string, error) {
 	// call depth = 2 to erase the runWithStdin frame and this one
 	return runWithStdin(ctx, r.log.WithCallDepth(2), cwd, env, "", command, args...)
 }
 
-// RunWithStdin runs given command with standard input
+// RunWithStdin runs the given command with standard input, returning the stdout,
+// stderr, and any error.
 func (r Runner) RunWithStdin(ctx context.Context, cwd string, env []string, stdin, command string, args ...string) (string, string, error) {
 	// call depth = 2 to erase the runWithStdin frame and this one
 	return runWithStdin(ctx, r.log.WithCallDepth(2), cwd, env, stdin, command, args...)
@@ -80,10 +81,10 @@ func runWithStdin(ctx context.Context, log logintf, cwd string, env []string, st
 	stdout := strings.TrimSpace(outbuf.String())
 	stderr := strings.TrimSpace(errbuf.String())
 	if ctx.Err() == context.DeadlineExceeded {
-		return "", "", fmt.Errorf("Run(%s): %w: { stdout: %q, stderr: %q }", cmdStr, ctx.Err(), stdout, stderr)
+		return stdout, stderr, fmt.Errorf("Run(%s): %w: { stdout: %q, stderr: %q }", cmdStr, ctx.Err(), stdout, stderr)
 	}
 	if err != nil {
-		return "", "", fmt.Errorf("Run(%s): %w: { stdout: %q, stderr: %q }", cmdStr, err, stdout, stderr)
+		return stdout, stderr, fmt.Errorf("Run(%s): %w: { stdout: %q, stderr: %q }", cmdStr, err, stdout, stderr)
 	}
 	log.V(6).Info("command result", "stdout", stdout, "stderr", stderr, "time", wallTime)
 

test_e2e.sh

@@ -2527,6 +2527,42 @@ function e2e::submodule_sync_shallow() {
     rm -rf $NESTED_SUBMODULE
 }
 
+##############################################
+# Test submodule sync with a relative path
+##############################################
+function e2e::submodule_sync_relative() {
+    # Init submodule repo
+    SUBMODULE_REPO_NAME="sub"
+    SUBMODULE="$WORK/$SUBMODULE_REPO_NAME"
+    mkdir "$SUBMODULE"
+
+    git -C "$SUBMODULE" init -q -b "$MAIN_BRANCH"
+    echo "submodule" > "$SUBMODULE/submodule"
+    git -C "$SUBMODULE" add submodule
+    git -C "$SUBMODULE" commit -aqm "init submodule file"
+
+    # Add submodule
+    REL="$(realpath --relative-to "$REPO" "$WORK/$SUBMODULE_REPO_NAME")"
+    echo $REL
+    git -C "$REPO" -c protocol.file.allow=always submodule add -q "${REL}"
+    git -C "$REPO" commit -aqm "add submodule"
+
+    GIT_SYNC \
+        --period=100ms \
+        --repo="file://$REPO" \
+        --root="$ROOT" \
+        --link="link" \
+        &
+    wait_for_sync "${MAXWAIT}"
+    assert_link_exists "$ROOT/link"
+    assert_file_exists "$ROOT/link/file"
+    assert_file_exists "$ROOT/link/$SUBMODULE_REPO_NAME/submodule"
+    assert_file_eq "$ROOT/link/$SUBMODULE_REPO_NAME/submodule" "submodule"
+    assert_metric_eq "${METRIC_GOOD_SYNC_COUNT}" 1
+
+    rm -rf $SUBMODULE
+}
+
 ##############################################
 # Test SSH
 ##############################################