From c32a042213dcb2a6836bddbfcc24fff8a8999968 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 31 Aug 2022 14:01:10 -0700 Subject: [PATCH] Make --error-file allow abs paths This also enforces th previously unenforced "must not start with a period" rule. --- README.md | 7 +++--- cmd/git-sync/main.go | 32 ++++++++++++++++++++---- cmd/git-sync/main_test.go | 27 ++++++++++++++++++++ test_e2e.sh | 52 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 110 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 26f9695..400048e 100644 --- a/README.md +++ b/README.md @@ -146,9 +146,10 @@ OPTIONS full history of the repo. --error-file , $GIT_SYNC_ERROR_FILE - The name of an optional file (under --root) into which errors will - be written. This must be a filename, not a path, and may not start - with a period. + The path to an optional file into which errors will be written. + This may be an absolute path or a relative path, in which case it + is relative to --root. If it is relative to --root, the first path + element may not start with a period. --exechook-backoff , $GIT_SYNC_EXECHOOK_BACKOFF The time to wait before retrying a failed --exechook-command. If diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index 19ec306..e4eb436 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -71,7 +71,7 @@ var flRoot = pflag.String("root", envString("GIT_SYNC_ROOT", ""), var flLink = pflag.String("link", envString("GIT_SYNC_LINK", ""), "the path (absolute or relative to --root) at which to create a symlink to the directory holding the checked-out files (defaults to the leaf dir of --repo)") var flErrorFile = pflag.String("error-file", envString("GIT_SYNC_ERROR_FILE", ""), - "an optional file into which errors will be written under --root (defaults to disabled)") + "the path (absolute or relative to --root) to an optional file into which errors will be written (defaults to disabled)") var flPeriod = pflag.Duration("period", envDuration("GIT_SYNC_PERIOD", 10*time.Second), "how long to wait between syncs, must be >= 10ms; --wait overrides this") var flSyncTimeout = pflag.Duration("sync-timeout", envDuration("GIT_SYNC_SYNC_TIMEOUT", 120*time.Second), @@ -326,7 +326,14 @@ func main() { pflag.Parse() // Needs to happen very early for errors to be written to a file. - log := logging.New(*flRoot, *flErrorFile, *flVerbose) + log := func() *logging.Logger { + if strings.HasPrefix(*flErrorFile, ".") { + fmt.Fprintf(os.Stderr, "ERROR: --error-file may not start with '.'") + os.Exit(1) + } + dir, file := filepath.Split(makeAbsPath(*flErrorFile, *flRoot)) + return logging.New(dir, file, *flVerbose) + }() cmdRunner := cmd.NewRunner(log) if *flVersion { @@ -754,6 +761,20 @@ func main() { } } +// makeAbsPath makes an absolute path from a path which might be absolute +// or relative. If the path is already absolute, it will be used. If it is +// not absolute, it will be joined with the provided root. If the path is +// empty, the result will be empty. +func makeAbsPath(path, root string) string { + if path == "" { + return "" + } + if filepath.IsAbs(path) { + return path + } + return filepath.Join(root, path) +} + const redactedString = "REDACTED" func redactURL(urlstr string) string { @@ -1844,9 +1865,10 @@ OPTIONS full history of the repo. --error-file , $GIT_SYNC_ERROR_FILE - The name of an optional file (under --root) into which errors will - be written. This must be a filename, not a path, and may not start - with a period. + The path to an optional file into which errors will be written. + This may be an absolute path or a relative path, in which case it + is relative to --root. If it is relative to --root, the first path + element may not start with a period. --exechook-backoff , $GIT_SYNC_EXECHOOK_BACKOFF The time to wait before retrying a failed --exechook-command. If diff --git a/cmd/git-sync/main_test.go b/cmd/git-sync/main_test.go index 9c191ec..9fb31f0 100644 --- a/cmd/git-sync/main_test.go +++ b/cmd/git-sync/main_test.go @@ -148,6 +148,33 @@ func TestEnvDuration(t *testing.T) { } } +func TestMakeAbsPath(t *testing.T) { + cases := []struct { + path string + root string + exp string + }{{ + path: "", root: "", exp: "", + }, { + path: "", root: "/root", exp: "", + }, { + path: "path", root: "/root", exp: "/root/path", + }, { + path: "p/a/t/h", root: "/root", exp: "/root/p/a/t/h", + }, { + path: "/path", root: "/root", exp: "/path", + }, { + path: "/p/a/t/h", root: "/root", exp: "/p/a/t/h", + }} + + for _, tc := range cases { + res := makeAbsPath(tc.path, tc.root) + if res != tc.exp { + t.Errorf("expected: %q, got: %q", tc.exp, res) + } + } +} + func TestManualHasNoTabs(t *testing.T) { if strings.Contains(manual, "\t") { t.Fatal("the manual text contains a tab") diff --git a/test_e2e.sh b/test_e2e.sh index 9606c42..17f0adc 100755 --- a/test_e2e.sh +++ b/test_e2e.sh @@ -2149,6 +2149,58 @@ function e2e::export_error() { assert_file_absent "$ROOT"/error.json } +############################################## +# Test export-error with an absolute path +############################################## +function e2e::export_error_abs_path() { + echo "$FUNCNAME" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME" + + ( + set +o errexit + GIT_SYNC \ + --repo="file://$REPO" \ + --branch=does-not-exit \ + --root="$ROOT" \ + --link="link" \ + --error-file="$ROOT/dir/error.json" \ + >> "$1" 2>&1 + RET=$? + if [[ "$RET" != 1 ]]; then + fail "expected exit code 1, got $RET" + fi + assert_file_absent "$ROOT"/link + assert_file_absent "$ROOT"/link/file + assert_file_contains "$ROOT"/dir/error.json "Remote branch does-not-exit not found in upstream origin" + ) +} + +############################################## +# Test export-error with invalid path +############################################## +function e2e::export_error_invalid_file() { + echo "$FUNCNAME" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME" + + ( + set +o errexit + GIT_SYNC \ + --repo="file://$REPO" \ + --branch="$MAIN_BRANCH" \ + --root="$ROOT" \ + --link="link" \ + --error-file=".error.json" \ + >> "$1" 2>&1 + RET=$? + if [[ "$RET" != 1 ]]; then + fail "expected exit code 1, got $RET" + fi + assert_file_absent "$ROOT"/link + assert_file_absent "$ROOT"/link/file + assert_file_absent "$ROOT"/.error.json + ) +} + ############################################## # Test github HTTPS # TODO: it would be better if we set up a local HTTPS server