From 53319d006d32b8bd561d70af67ed2270bebe8a2f Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Thu, 14 Mar 2019 12:38:39 +0100 Subject: [PATCH] Desambiguate revisions and files when invoking `git` User Git repositories may use the same name for: * A file and a branch used by Flux * A file and the tag used by Flux for syncing For instance: ``` ts=2019-03-13T19:36:40.432679542Z caller=loop.go:90 component=sync-loop err="fatal: ambiguous argument 'staging': both revision and filename" ``` (See https://weave-community.slack.com/archives/C4U5ATZ9S/p1552505808477700?thread_ts=1552504490.476100&cid=C4U5ATZ9S ) This change removes that ambiguity by appending `--` at the right places when invoking `git`. In a few cases we were only appending it when adding files at the end, which wasn't correct. --- git/gittest/repo.go | 6 ++++++ git/operations.go | 25 +++++++++---------------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/git/gittest/repo.go b/git/gittest/repo.go index 0338f7d98..525b7333c 100644 --- a/git/gittest/repo.go +++ b/git/gittest/repo.go @@ -75,6 +75,10 @@ func Workloads() (res []flux.ResourceID) { // CheckoutWithConfig makes a standard repo, clones it, and returns // the clone, the original repo, and a cleanup function. func CheckoutWithConfig(t *testing.T, config git.Config) (*git.Checkout, *git.Repo, func()) { + // Add files to the repo with the same name as the git branch and the sync tag. + // This is to make sure that git commands don't have ambiguity problems between revisions and files. + testfiles.Files[config.Branch] = "Filename doctored to create a conflict with the git branch name" + testfiles.Files[config.SyncTag] = "Filename doctored to create a conflict with the git sync tag" repo, cleanup := Repo(t) if err := repo.Ready(context.Background()); err != nil { cleanup() @@ -89,6 +93,8 @@ func CheckoutWithConfig(t *testing.T, config git.Config) (*git.Checkout, *git.Re return co, repo, func() { co.Clean() cleanup() + delete(testfiles.Files, config.Branch) + delete(testfiles.Files, config.SyncTag) } } diff --git a/git/operations.go b/git/operations.go index cb9484459..c3f7daacf 100644 --- a/git/operations.go +++ b/git/operations.go @@ -65,7 +65,7 @@ func mirror(ctx context.Context, workingDir, repoURL string) (path string, err e } func checkout(ctx context.Context, workingDir, ref string) error { - args := []string{"checkout", ref} + args := []string{"checkout", ref, "--"} return execGitCmd(ctx, args, gitCmdConfig{dir: workingDir}) } @@ -95,6 +95,7 @@ func commit(ctx context.Context, workingDir string, commitAction CommitAction) e if commitAction.SigningKey != "" { args = append(args, fmt.Sprintf("--gpg-sign=%s", commitAction.SigningKey)) } + args = append(args, "--") if err := execGitCmd(ctx, args, gitCmdConfig{dir: workingDir, env: env}); err != nil { return errors.Wrap(err, "git commit") } @@ -121,9 +122,9 @@ func fetch(ctx context.Context, workingDir, upstream string, refspec ...string) } func refExists(ctx context.Context, workingDir, ref string) (bool, error) { - args := []string{"rev-list", ref} + args := []string{"rev-list", ref, "--"} if err := execGitCmd(ctx, args, gitCmdConfig{dir: workingDir}); err != nil { - if strings.Contains(err.Error(), "unknown revision") { + if strings.Contains(err.Error(), "bad revision") { return false, nil } return false, err @@ -188,28 +189,19 @@ func noteRevList(ctx context.Context, workingDir, notesRef string) (map[string]s // Get the commit hash for a reference func refRevision(ctx context.Context, workingDir, ref string) (string, error) { out := &bytes.Buffer{} - args := []string{"rev-list", "--max-count", "1", ref} + args := []string{"rev-list", "--max-count", "1", ref, "--"} if err := execGitCmd(ctx, args, gitCmdConfig{dir: workingDir, out: out}); err != nil { return "", err } return strings.TrimSpace(out.String()), nil } -func revlist(ctx context.Context, workingDir, ref string) ([]string, error) { - out := &bytes.Buffer{} - args := []string{"rev-list", ref} - if err := execGitCmd(ctx, args, gitCmdConfig{dir: workingDir, out: out}); err != nil { - return nil, err - } - return splitList(out.String()), nil -} - // Return the revisions and one-line log commit messages func onelinelog(ctx context.Context, workingDir, refspec string, subdirs []string) ([]Commit, error) { out := &bytes.Buffer{} args := []string{"log", "--pretty=format:%GK|%H|%s", refspec} + args = append(args, "--") if len(subdirs) > 0 { - args = append(args, "--") args = append(args, subdirs...) } @@ -273,8 +265,8 @@ func changed(ctx context.Context, workingDir, ref string, subPaths []string) ([] // the working dir_; i.e, we do not report on things that no // longer appear. args := []string{"diff", "--name-only", "--diff-filter=ACMRT", ref} + args = append(args, "--") if len(subPaths) > 0 { - args = append(args, "--") args = append(args, subPaths...) } @@ -292,6 +284,7 @@ func execGitCmd(ctx context.Context, args []string, config gitCmdConfig) error { } println() } + c := exec.CommandContext(ctx, "git", args...) if config.dir != "" { @@ -337,8 +330,8 @@ func env() []string { func check(ctx context.Context, workingDir string, subdirs []string) bool { // `--quiet` means "exit with 1 if there are changes" args := []string{"diff", "--quiet"} + args = append(args, "--") if len(subdirs) > 0 { - args = append(args, "--") args = append(args, subdirs...) } return execGitCmd(ctx, args, gitCmdConfig{dir: workingDir}) != nil