Skip to content

Commit

Permalink
Merge pull request #344 from saschagrunert/perfsprint
Browse files Browse the repository at this point in the history
Enable and fix `perfsprint` linter
  • Loading branch information
k8s-ci-robot committed Apr 29, 2024
2 parents d28042e + 456ed88 commit 7102281
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 34 deletions.
1 change: 1 addition & 0 deletions .golangci.yml
Expand Up @@ -64,6 +64,7 @@ linters:
- noctx
- nolintlint
- nosprintfhostport
- perfsprint
- prealloc
- predeclared
- promlinter
Expand Down
19 changes: 10 additions & 9 deletions git/git.go
Expand Up @@ -28,6 +28,7 @@ import (
"path/filepath"
"regexp"
"sort"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -115,35 +116,35 @@ func setVerbose(
// 4 shows all paths as they are processed.
// 5 and above show detailed debugging information.
// The default value is 2.
"GIT_MERGE_VERBOSITY": fmt.Sprint(merge),
"GIT_MERGE_VERBOSITY": strconv.FormatUint(uint64(merge), 10),

// Git uses the curl library to do network operations over HTTP, so
// GIT_CURL_VERBOSE tells Git to emit all the messages generated by
// that library. This is similar to doing curl -v on the command line.
"GIT_CURL_VERBOSE": fmt.Sprint(curl),
"GIT_CURL_VERBOSE": strconv.FormatUint(uint64(curl), 10),

// Controls general traces, which don’t fit into any specific category.
// This includes the expansion of aliases, and delegation to other
// sub-programs.
"GIT_TRACE": fmt.Sprint(trace),
"GIT_TRACE": strconv.FormatUint(uint64(trace), 10),

// Controls tracing of packfile access. The first field is the packfile
// being accessed, the second is the offset within that file.
"GIT_TRACE_PACK_ACCESS": fmt.Sprint(tracePackAccess),
"GIT_TRACE_PACK_ACCESS": strconv.FormatUint(uint64(tracePackAccess), 10),

// Enables packet-level tracing for network operations.
"GIT_TRACE_PACKET": fmt.Sprint(tracePacket),
"GIT_TRACE_PACKET": strconv.FormatUint(uint64(tracePacket), 10),

// Controls logging of performance data. The output shows how long each
// particular git invocation takes.
"GIT_TRACE_PERFORMANCE": fmt.Sprint(tracePerformance),
"GIT_TRACE_PERFORMANCE": strconv.FormatUint(uint64(tracePerformance), 10),

// Shows information about what Git is discovering about the repository
// and environment it’s interacting with.
"GIT_TRACE_SETUP": fmt.Sprint(traceSetup),
"GIT_TRACE_SETUP": strconv.FormatUint(uint64(traceSetup), 10),

// Debugging fetching/cloning of shallow repositories.
"GIT_TRACE_SHALLOW": fmt.Sprint(traceShallow),
"GIT_TRACE_SHALLOW": strconv.FormatUint(uint64(traceShallow), 10),
} {
if err := os.Setenv(key, value); err != nil {
return fmt.Errorf("unable to set %s=%s: %w", key, value, err)
Expand Down Expand Up @@ -742,7 +743,7 @@ func (r *Repo) latestNonPatchFinalVersions() ([]semver.Version, error) {
return nil
})
if len(latestVersions) == 0 {
return nil, fmt.Errorf("unable to find latest non patch release")
return nil, errors.New("unable to find latest non patch release")
}
return latestVersions, nil
}
Expand Down
39 changes: 22 additions & 17 deletions git/git_test.go
Expand Up @@ -42,6 +42,11 @@ var testAuthor = &object.Signature{
When: time.Now(),
}

const (
errDesc = "opening test repo in "
origin = "origin"
)

func newSUT() (*git.Repo, *gitfakes.FakeWorktree) {
repoMock := &gitfakes.FakeRepository{}
worktreeMock := &gitfakes.FakeWorktree{}
Expand Down Expand Up @@ -108,10 +113,10 @@ func TestGetUserName(t *testing.T) {

// Call git to configure the user's name:
_, err = exec.Command("git", "config", "user.name", fakeUserName).Output()
require.Nil(t, err, fmt.Sprintf("configuring fake user email in %s", repoPath))
require.Nil(t, err, "configuring fake user email in "+repoPath)

testRepo, err := git.OpenRepo(repoPath)
require.Nil(t, err, fmt.Sprintf("opening test repo in %s", repoPath))
require.Nil(t, err, errDesc+repoPath)
defer testRepo.Cleanup() //nolint: errcheck

actual, err := git.GetUserName()
Expand All @@ -132,10 +137,10 @@ func TestGetUserEmail(t *testing.T) {

// Call git to configure the user's name:
_, err = exec.Command("git", "config", "user.email", fakeUserEmail).Output()
require.Nil(t, err, fmt.Sprintf("configuring fake user email in %s", repoPath))
require.Nil(t, err, "configuring fake user email in "+repoPath)

testRepo, err := git.OpenRepo(repoPath)
require.Nil(t, err, fmt.Sprintf("opening test repo in %s", repoPath))
require.Nil(t, err, errDesc+repoPath)
defer testRepo.Cleanup() //nolint: errcheck

// Do the actual call
Expand Down Expand Up @@ -206,7 +211,7 @@ func TestGetRepoURLSuccess(t *testing.T) {
func TestRemotify(t *testing.T) {
testcases := []struct{ provided, expected string }{
{provided: git.DefaultBranch, expected: git.DefaultRemote + "/" + git.DefaultBranch},
{provided: "origin/ref", expected: "origin/ref"},
{provided: origin + "/ref", expected: origin + "/ref"},
{provided: "base/another_ref", expected: "base/another_ref"},
}

Expand Down Expand Up @@ -346,18 +351,18 @@ func TestHasBranch(t *testing.T) {
require.Nil(t, err, "writing test file")

err = command.NewWithWorkDir(repoPath, "git", "add", testfile).RunSuccess()
require.Nil(t, err, fmt.Sprintf("adding test file in %s", repoPath))
require.Nil(t, err, "adding test file in "+repoPath)

err = command.NewWithWorkDir(repoPath, "git", "commit", "-m", "adding test file").RunSuccess()
require.Nil(t, err, "creating first commit")

// Call git to configure the user's name:
err = command.NewWithWorkDir(repoPath, "git", "branch", testBranchName).RunSuccess()
require.Nil(t, err, fmt.Sprintf("configuring test branch in %s", repoPath))
require.Nil(t, err, "configuring test branch in "+repoPath)

// Now, open the repo and test to see if branches are there
testRepo, err := git.OpenRepo(repoPath)
require.Nil(t, err, fmt.Sprintf("opening test repo in %s", repoPath))
require.Nil(t, err, errDesc+repoPath)
defer testRepo.Cleanup() //nolint: errcheck

actual, err := testRepo.HasBranch(testBranchName)
Expand Down Expand Up @@ -434,7 +439,7 @@ func TestShowLastCommit(t *testing.T) {
// Create an untracked file
require.Nil(t, os.WriteFile(filepath.Join(testRepo.Dir(), testFile), []byte("Hello SIG Release"), 0o644))
require.Nil(t, testRepo.Add(testFile))
require.Nil(t, testRepo.Commit(fmt.Sprintf("Commit test file at %s", timeNow)))
require.Nil(t, testRepo.Commit("Commit test file at "+timeNow))

// Now get the log message back and check if it contains the time
lastLog, err := testRepo.ShowLastCommit()
Expand Down Expand Up @@ -487,12 +492,12 @@ func TestFetchRemote(t *testing.T) {
require.Nil(t, err)

// Now, call fetch
newContent, err := testRepo.FetchRemote("origin")
newContent, err := testRepo.FetchRemote(origin)
require.Nil(t, err, "Calling fetch to get a test tag")
require.True(t, newContent)

// Fetching again should provide no updates
newContent, err = testRepo.FetchRemote("origin")
newContent, err = testRepo.FetchRemote(origin)
require.Nil(t, err, "Calling fetch to get a test tag again")
require.False(t, newContent)

Expand Down Expand Up @@ -535,7 +540,7 @@ func TestRebase(t *testing.T) {
defer testRepo.Cleanup() //nolint: errcheck

// Test 1. Rebase should not fail if both repos are in sync
require.Nil(t, testRepo.Rebase(fmt.Sprintf("origin/%s", branchName)), "cloning synchronizaed repos")
require.Nil(t, testRepo.Rebase(origin+"/"+branchName), "cloning synchronizaed repos")

// Test 2. Rebase should not fail with pulling changes in the remote
require.Nil(t, os.WriteFile(filepath.Join(rawRepoDir, testFile), []byte("Hello SIG Release"), 0o644))
Expand All @@ -548,20 +553,20 @@ func TestRebase(t *testing.T) {
require.Nil(t, err)

// Pull the changes to the test repo
newContent, err := testRepo.FetchRemote("origin")
newContent, err := testRepo.FetchRemote(origin)
require.Nil(t, err)
require.True(t, newContent)

// Do the Rebase
require.Nil(t, testRepo.Rebase(fmt.Sprintf("origin/%s", branchName)), "rebasing changes from origin")
require.Nil(t, testRepo.Rebase(origin+"/"+branchName), "rebasing changes from origin")

// Verify we got the commit
lastLog, err := testRepo.ShowLastCommit()
require.Nil(t, err)
require.True(t, strings.Contains(lastLog, "Test2-Commit"))

// Test 3: Rebase must on an invalid branch
require.NotNil(t, testRepo.Rebase("origin/invalidBranch"), "rebasing to invalid branch")
require.NotNil(t, testRepo.Rebase(origin+"/invalidBranch"), "rebasing to invalid branch")

// Test 4: Rebase must fail on merge conflicts
require.Nil(t, os.WriteFile(filepath.Join(rawRepoDir, testFile), []byte("Hello again SIG Release"), 0o644))
Expand All @@ -579,11 +584,11 @@ func TestRebase(t *testing.T) {
require.Nil(t, testRepo.Commit("Adding file to cause conflict"))

// Now, fetch and rebase
newContent, err = testRepo.FetchRemote("origin")
newContent, err = testRepo.FetchRemote(origin)
require.Nil(t, err)
require.True(t, newContent)

err = testRepo.Rebase(fmt.Sprintf("origin/%s", branchName))
err = testRepo.Rebase(origin + "/" + branchName)
require.NotNil(t, err, "testing for merge conflicts")
}

Expand Down
6 changes: 3 additions & 3 deletions github/internal/retry_test.go
Expand Up @@ -17,7 +17,7 @@ limitations under the License.
package internal_test

import (
"fmt"
"errors"
"io"
"os"
"testing"
Expand Down Expand Up @@ -54,7 +54,7 @@ func TestGithubRetryer(t *testing.T) {
"when error is a random error, don't retry": {
maxTries: 1,
sleeper: nilSleeper,
errs: []error{fmt.Errorf("some randm error")},
errs: []error{errors.New("some randm error")},
expectedResults: []bool{false},
},
"when the error is a github rate limit error, retry": {
Expand Down Expand Up @@ -86,7 +86,7 @@ func TestGithubRetryer(t *testing.T) {
"when hitting the secondary rate limit, sleep for random": {
maxTries: 1,
sleeper: nilSleeper,
errs: []error{fmt.Errorf("You have exceeded a secondary rate limit. Please wait a few minutes")},
errs: []error{errors.New("You have exceeded a secondary rate limit. Please wait a few minutes")},
expectedResults: []bool{true},
},
"when the error is a github abuse rate limit error but max tries have been reached, don't retry": {
Expand Down
15 changes: 10 additions & 5 deletions sign/sign.go
Expand Up @@ -53,6 +53,11 @@ type Signer struct {
signedObjs *ttlcache.Cache[string, *SignedObject] // key: imageRef, value: signed object
}

const (
sigExt = ".sig"
certExt = ".cert"
)

// New returns a new Signer instance.
func New(options *Options) *Signer {
if options == nil {
Expand Down Expand Up @@ -251,10 +256,10 @@ func (s *Signer) SignFile(path string) (*SignedObject, error) {
}

if s.options.OutputCertificatePath == "" {
s.options.OutputCertificatePath = fmt.Sprintf("%s.cert", path)
s.options.OutputCertificatePath = path + certExt
}
if s.options.OutputSignaturePath == "" {
s.options.OutputSignaturePath = fmt.Sprintf("%s.sig", path)
s.options.OutputSignaturePath = path + sigExt
}

fileSHA, err := hash.SHA256ForFile(path)
Expand Down Expand Up @@ -460,10 +465,10 @@ func (s *Signer) VerifyFile(path string, ignoreTLog bool) (*SignedObject, error)
}

if s.options.OutputCertificatePath == "" {
s.options.OutputCertificatePath = fmt.Sprintf("%s.cert", path)
s.options.OutputCertificatePath = path + certExt
}
if s.options.OutputSignaturePath == "" {
s.options.OutputSignaturePath = fmt.Sprintf("%s.sig", path)
s.options.OutputSignaturePath = path + sigExt
}

certOpts := cliOpts.CertVerifyOptions{
Expand Down Expand Up @@ -704,7 +709,7 @@ func (s *Signer) transportForRepo(ctx context.Context, repo name.Repository) (ht
}

func repoDigestToSig(repo name.Repository, digest string) string {
return repo.Name() + ":" + strings.Replace(digest, ":", "-", 1) + ".sig"
return repo.Name() + ":" + strings.Replace(digest, ":", "-", 1) + sigExt
}

// IsFileSigned takes an path reference and return true if there is a signature
Expand Down

0 comments on commit 7102281

Please sign in to comment.