From bb22144d4f7be843a55f3aad884e42f65c1d2183 Mon Sep 17 00:00:00 2001 From: justinsb Date: Sat, 29 Jul 2023 16:37:07 -0400 Subject: [PATCH] lint: fix remaining lint issues. These fix the issues identified by the k/k inspired linter configuration that we will be adding: ``` pkg/pid1/pid1.go:72:14: ST1005: error strings should not end with punctuation or newlines (stylecheck) return 0, fmt.Errorf("unhandled exit status: 0x%x\n", status) ^ pkg/pid1/pid1.go:86:21: ST1005: error strings should not end with punctuation or newlines (stylecheck) return false, 0, fmt.Errorf("wait4(): %w\n", err) ^ main.go:480:34: Error return value of `pflag.CommandLine.MarkDeprecated` is not checked (errcheck) pflag.CommandLine.MarkDeprecated("branch", "use --ref instead") ^ main.go:483:34: Error return value of `pflag.CommandLine.MarkDeprecated` is not checked (errcheck) pflag.CommandLine.MarkDeprecated("change-permissions", "use --group-write instead") ^ main.go:486:34: Error return value of `pflag.CommandLine.MarkDeprecated` is not checked (errcheck) pflag.CommandLine.MarkDeprecated("dest", "use --link instead") ^ main.go:1897:16: Error return value of `io.WriteString` is not checked (errcheck) io.WriteString(h, s) ^ main.go:555:2: ifElseChain: rewrite if-else to switch statement (gocritic) if *flDeprecatedBranch != "" && (*flDeprecatedRev == "" || *flDeprecatedRev == "HEAD") { ^ ``` --- main.go | 41 ++++++++++++++++++++++++++++------------- pkg/pid1/pid1.go | 4 ++-- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/main.go b/main.go index af2a4a9..5e62b98 100644 --- a/main.go +++ b/main.go @@ -477,31 +477,31 @@ func main() { // Obsolete flags, kept for compat. flDeprecatedBranch := pflag.String("branch", envString("", "GIT_SYNC_BRANCH"), "DEPRECATED: use --ref instead") - pflag.CommandLine.MarkDeprecated("branch", "use --ref instead") + mustMarkDeprecated("branch", "use --ref instead") flDeprecatedChmod := pflag.Int("change-permissions", envInt(0, "GIT_SYNC_PERMISSIONS"), "DEPRECATED: use --group-write instead") - pflag.CommandLine.MarkDeprecated("change-permissions", "use --group-write instead") + mustMarkDeprecated("change-permissions", "use --group-write instead") flDeprecatedDest := pflag.String("dest", envString("", "GIT_SYNC_DEST"), "DEPRECATED: use --link instead") - pflag.CommandLine.MarkDeprecated("dest", "use --link instead") + mustMarkDeprecated("dest", "use --link instead") flDeprecatedMaxSyncFailures := pflag.Int("max-sync-failures", envInt(0, "GIT_SYNC_MAX_SYNC_FAILURES"), "DEPRECATED: use --max-failures instead") - pflag.CommandLine.MarkDeprecated("max-sync-failures", "use --max-failures instead") + mustMarkDeprecated("max-sync-failures", "use --max-failures instead") flDeprecatedRev := pflag.String("rev", envString("", "GIT_SYNC_REV"), "DEPRECATED: use --ref instead") - pflag.CommandLine.MarkDeprecated("rev", "use --ref instead") + mustMarkDeprecated("rev", "use --ref instead") flDeprecatedSyncHookCommand := pflag.String("sync-hook-command", envString("", "GIT_SYNC_HOOK_COMMAND"), "DEPRECATED: use --exechook-command instead") - pflag.CommandLine.MarkDeprecated("sync-hook-command", "use --exechook-command instead") + mustMarkDeprecated("sync-hook-command", "use --exechook-command instead") flDeprecatedTimeout := pflag.Int("timeout", envInt(0, "GIT_SYNC_TIMEOUT"), "DEPRECATED: use --sync-timeout instead") - pflag.CommandLine.MarkDeprecated("timeout", "use --sync-timeout instead") + mustMarkDeprecated("timeout", "use --sync-timeout instead") flDeprecatedV := pflag.Int("v", -1, "DEPRECATED: use -v or --verbose instead") - pflag.CommandLine.MarkDeprecated("v", "use -v or --verbose instead") + mustMarkDeprecated("v", "use -v or --verbose instead") flDeprecatedWait := pflag.Float64("wait", envFloat(0, "GIT_SYNC_WAIT"), "DEPRECATED: use --period instead") - pflag.CommandLine.MarkDeprecated("wait", "use --period instead") + mustMarkDeprecated("wait", "use --period instead") // // Parse and verify flags. Errors here are fatal. @@ -552,17 +552,19 @@ func main() { handleConfigError(log, true, "ERROR: --repo must be specified") } - if *flDeprecatedBranch != "" && (*flDeprecatedRev == "" || *flDeprecatedRev == "HEAD") { + switch { + case *flDeprecatedBranch != "" && (*flDeprecatedRev == "" || *flDeprecatedRev == "HEAD"): // Back-compat log.V(0).Info("setting --ref from deprecated --branch") *flRef = *flDeprecatedBranch - } else if *flDeprecatedRev != "" { + case *flDeprecatedRev != "": // Back-compat log.V(0).Info("setting --ref from deprecated --rev") *flRef = *flDeprecatedRev - } else if *flDeprecatedBranch != "" && *flDeprecatedRev != "" { + case *flDeprecatedBranch != "" && *flDeprecatedRev != "": handleConfigError(log, true, "ERROR: can't set --ref from deprecated --branch and --rev") } + if *flRef == "" { handleConfigError(log, true, "ERROR: --ref must be specified") } @@ -1052,6 +1054,16 @@ func main() { } } +// mustMarkDeprecated is a helper around pflag.CommandLine.MarkDeprecated. +// It panics if there is an error (as these indicate a coding issue). +// This makes it easier to keep the linters happy. +func mustMarkDeprecated(name string, usageMessage string) { + err := pflag.CommandLine.MarkDeprecated(name, usageMessage) + if err != nil { + panic(fmt.Sprintf("error marking flag %q as deprecated: %v", name, err)) + } +} + // 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 @@ -1894,7 +1906,10 @@ func (git *repoSync) isShallow(ctx context.Context) (bool, error) { func md5sum(s string) string { h := md5.New() - io.WriteString(h, s) + if _, err := io.WriteString(h, s); err != nil { + // Documented as never failing, so panic + panic(fmt.Sprintf("md5 WriteString failed: %v", err)) + } return fmt.Sprintf("%x", h.Sum(nil)) } diff --git a/pkg/pid1/pid1.go b/pkg/pid1/pid1.go index 0a218d1..c84633d 100644 --- a/pkg/pid1/pid1.go +++ b/pkg/pid1/pid1.go @@ -69,7 +69,7 @@ func runInit(firstborn int) (int, error) { if status.Exited() { return status.ExitStatus(), nil } - return 0, fmt.Errorf("unhandled exit status: 0x%x\n", status) + return 0, fmt.Errorf("unhandled exit status: 0x%x", status) } } return 0, fmt.Errorf("signal handler terminated unexpectedly") @@ -83,7 +83,7 @@ func sigchld(firstborn int) (bool, syscall.WaitStatus, error) { var status syscall.WaitStatus pid, err := syscall.Wait4(-1, &status, syscall.WNOHANG, nil) if err != nil { - return false, 0, fmt.Errorf("wait4(): %w\n", err) + return false, 0, fmt.Errorf("wait4(): %w", err) } if pid == firstborn {