From 08296a48b012a6a5256cb2ad855c5bd1872473b7 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 7 Jul 2023 09:57:43 -0700 Subject: [PATCH] Make relative-path submodules work, via origin The "origin" remote is implicitly used as the basis for relative-paths in submodules. It's very subtly documented, and I have no idea if there are other places where it is used. It seems git really expects it to exist, so let's just do that. --- main.go | 43 +++++++++++++++++++++++++++++++++---------- pkg/cmd/cmd.go | 9 +++++---- test_e2e.sh | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 14 deletions(-) diff --git a/main.go b/main.go index b8e47d3..a97bf2b 100644 --- a/main.go +++ b/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 } diff --git a/pkg/cmd/cmd.go b/pkg/cmd/cmd.go index 861199a..c0894cf 100644 --- a/pkg/cmd/cmd.go +++ b/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) diff --git a/test_e2e.sh b/test_e2e.sh index 94340a0..ba86b11 100755 --- a/test_e2e.sh +++ b/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 ##############################################