diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index c5e5099..65f13bb 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -1094,30 +1094,35 @@ func (git *repoSync) CleanupWorkTree(ctx context.Context, gitRoot, worktree stri func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, hash string) (bool, string, error) { git.log.V(0).Info("syncing git", "rev", git.rev, "hash", hash) - args := []string{"fetch", "-f", "--tags"} - if git.depth != 0 { - args = append(args, "--depth", strconv.Itoa(git.depth)) - } - fetch := "HEAD" - if git.branch != "" { - fetch = git.branch - } - args = append(args, git.repo, fetch) - - // Update from the remote. - if _, err := git.run.Run(ctx, git.root, nil, git.cmd, args...); err != nil { - return false, "", err - } - - // With shallow fetches, it's possible to race with the upstream repo and - // end up NOT fetching the hash we wanted. If we can't resolve that hash - // to a commit we can just end early and leave it for the next sync period. - // This probably SHOULD be an error, but it can be a problem for startup - // and first-sync-must-succeed semantics. If/when we fix that this can be - // fixed. + // If we don't have this hash, we need to fetch it. if _, err := git.ResolveRef(ctx, hash); err != nil { - git.log.Error(err, "can't resolve commit, will retry", "rev", git.rev, "hash", hash) - return false, "", nil + git.log.V(2).Info("can't resolve commit, will try fetch", "rev", git.rev, "hash", hash) + + args := []string{"fetch", "-f", "--tags"} + if git.depth != 0 { + args = append(args, "--depth", strconv.Itoa(git.depth)) + } + fetch := "HEAD" + if git.branch != "" { + fetch = git.branch + } + args = append(args, git.repo, fetch) + + // Update from the remote. + if _, err := git.run.Run(ctx, git.root, nil, git.cmd, args...); err != nil { + return false, "", err + } + + // With shallow fetches, it's possible to race with the upstream repo and + // end up NOT fetching the hash we wanted. If we can't resolve that hash + // to a commit we can just end early and leave it for the next sync period. + // This probably SHOULD be an error, but it can be a problem for startup + // and first-sync-must-succeed semantics. If/when we fix that this can be + // fixed. + if _, err := git.ResolveRef(ctx, hash); err != nil { + git.log.Error(err, "can't resolve commit, will retry", "rev", git.rev, "hash", hash) + return false, "", nil + } } // Make a worktree for this exact git hash. @@ -1373,6 +1378,16 @@ func (git *repoSync) LocalHashForRev(ctx context.Context, rev string) (string, e if err != nil { return "", err } + result := strings.Trim(string(output), "\n") + if result == rev { + // It appears to be a SHA, so we need to verify that we have it. We + // don't care what cat-file says, just whether it succeeds or fails. + _, err := git.run.Run(ctx, git.root, nil, git.cmd, "cat-file", "-t", rev) + if err != nil { + // Indicate that we do not have a local hash for this hash. + return "", err + } + } return strings.Trim(string(output), "\n"), nil } @@ -1494,16 +1509,20 @@ func stringContainsOneOf(s string, matches ...string) bool { return false } -// GetRevs returns the local and upstream hashes for rev. +// GetRevs returns the current HEAD and upstream hash for rev. Normally the +// current HEAD is a previous or current version of git.rev, but if the app was +// started with one rev and then restarted with a different one, HEAD could be +// anything. func (git *repoSync) GetRevs(ctx context.Context) (string, string, error) { - // Ask git what the exact hash is for rev. - local, err := git.LocalHashForRev(ctx, git.rev) + // Find the currently synced HEAD. + local, err := git.LocalHashForRev(ctx, "HEAD") if err != nil { return "", "", err } // Build a ref string, depending on whether the user asked to track HEAD or - // a tag. + // a tag. We can't really tell if it is a SHA yet, so we will catch that + // case later. ref := "HEAD" if git.rev != "HEAD" { ref = "refs/tags/" + git.rev @@ -1517,6 +1536,20 @@ func (git *repoSync) GetRevs(ctx context.Context) (string, string, error) { return "", "", err } + // If we couldn't find a remote hash, it might have been a SHA literal. + if remote == "" { + // If git thinks it tastes like a SHA, we just return that and if it + // is wrong, we will fail later. + output, err := git.run.Run(ctx, git.root, nil, git.cmd, "rev-parse", git.rev) + if err != nil { + return "", "", err + } + result := strings.Trim(string(output), "\n") + if result == git.rev { + remote = git.rev + } + } + return local, remote, nil } diff --git a/test_e2e.sh b/test_e2e.sh index 9f774af..83bad51 100755 --- a/test_e2e.sh +++ b/test_e2e.sh @@ -853,6 +853,90 @@ function e2e::sync_sha_once() { assert_file_eq "$ROOT"/link/file "$FUNCNAME" } +############################################## +# Test rev-sync on a different rev we already have +############################################## +function e2e::sync_sha_once_sync_different_sha_known() { + # All revs will be known because we check out the branch + echo "$FUNCNAME 1" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME 1" + REV1=$(git -C "$REPO" rev-list -n1 HEAD) + echo "$FUNCNAME 2" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME 2" + REV2=$(git -C "$REPO" rev-list -n1 HEAD) + echo "$FUNCNAME 3" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME 3" + + # Sync REV1 + GIT_SYNC \ + --one-time \ + --repo="file://$REPO" \ + --branch="$MAIN_BRANCH" \ + --rev="$REV1" \ + --root="$ROOT" \ + --link="link" \ + >> "$1" 2>&1 + assert_link_exists "$ROOT"/link + assert_file_exists "$ROOT"/link/file + assert_file_eq "$ROOT"/link/file "$FUNCNAME 1" + + # Sync REV2 + GIT_SYNC \ + --one-time \ + --repo="file://$REPO" \ + --branch="$MAIN_BRANCH" \ + --rev="$REV2" \ + --root="$ROOT" \ + --link="link" \ + >> "$1" 2>&1 + assert_link_exists "$ROOT"/link + assert_file_exists "$ROOT"/link/file + assert_file_eq "$ROOT"/link/file "$FUNCNAME 2" +} + +############################################## +# Test rev-sync on a different rev we do not have +############################################## +function e2e::sync_sha_once_sync_different_sha_unknown() { + echo "$FUNCNAME 1" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME 1" + REV1=$(git -C "$REPO" rev-list -n1 HEAD) + + # Sync REV1 + GIT_SYNC \ + --one-time \ + --repo="file://$REPO" \ + --branch="$MAIN_BRANCH" \ + --rev="$REV1" \ + --root="$ROOT" \ + --link="link" \ + >> "$1" 2>&1 + assert_link_exists "$ROOT"/link + assert_file_exists "$ROOT"/link/file + assert_file_eq "$ROOT"/link/file "$FUNCNAME 1" + + # The locally synced repo does not know this new SHA. + echo "$FUNCNAME 2" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME 2" + REV2=$(git -C "$REPO" rev-list -n1 HEAD) + # Make sure the SHA is not at HEAD, to prevent things that only work in + # that case. + echo "$FUNCNAME 3" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME 3" + + # Sync REV2 + GIT_SYNC \ + --one-time \ + --repo="file://$REPO" \ + --branch="$MAIN_BRANCH" \ + --rev="$REV2" \ + --root="$ROOT" \ + --link="link" \ + >> "$1" 2>&1 + assert_link_exists "$ROOT"/link + assert_file_exists "$ROOT"/link/file + assert_file_eq "$ROOT"/link/file "$FUNCNAME 2" +} ############################################## # Test syncing after a crash ##############################################