diff --git a/impl/common.go b/impl/common.go index b6d391f..880f65d 100644 --- a/impl/common.go +++ b/impl/common.go @@ -299,14 +299,12 @@ func loadGpgKeys() error { } func combineSrcEnv( - onlyHash bool, - hashMaxWidth uint, + useHash bool, sep string, - maxSources int, errPrefix util.ErrPrefix) (string, error) { envPrefix := viper.GetString("SrcEnvPrefix") var releaseFields []string - for i := 0; i < maxSources; i++ { + for i := 0; ; i++ { envVar := envPrefix + strconv.Itoa(i) srcI := os.Getenv(envVar) if srcI == "" { @@ -320,13 +318,8 @@ func combineSrcEnv( } var field string - if onlyHash { - fullHash := srcIComps[1] - if hashMaxWidth != 0 { - field = fullHash[:hashMaxWidth] - } else { - field = fullHash - } + if useHash { + field = srcIComps[1][:7] // first 7 chars of the hash } else { field = srcI } @@ -338,8 +331,8 @@ func combineSrcEnv( // getRpmReleaseMacro returns the release rpm macro to be defined // for building srpms and rpms. // If the value is hardcoded in the manifest, we use that. -// Otherwise it is constructed by combining the first -// characters of the commit hashes in the SRC_ env vars. +// Otherwise it is constructed by combining a shortened hash of the +// commit from the the SRC_ env vars. // If the env vars are unset, an empty string is returned. func getRpmReleaseMacro(pkgSpec *manifest.Package, errPrefix util.ErrPrefix) ( string, error) { @@ -347,29 +340,14 @@ func getRpmReleaseMacro(pkgSpec *manifest.Package, errPrefix util.ErrPrefix) ( return pkgSpec.RpmReleaseMacro, nil } - const hashWidthInRelease uint = 7 - const maxSources int = 10 - const sep string = "_" - - return combineSrcEnv( - true, hashWidthInRelease, - sep, - maxSources, - errPrefix) + return combineSrcEnv(true, "_", errPrefix) } // getEextSignature returns a signature of the Source and BuildRequires // It is derived by combining the SRC_ environment variables. func getEextSignature(errPrefix util.ErrPrefix) ( string, error) { - const sep string = "," - const maxSources int = 10 - - return combineSrcEnv( - false, 0, - sep, - maxSources, - errPrefix) + return combineSrcEnv(false, ",", errPrefix) } func setup() error { diff --git a/impl/create_srpm_for_git.go b/impl/create_srpm_from_git.go similarity index 76% rename from impl/create_srpm_for_git.go rename to impl/create_srpm_from_git.go index 367b755..3be8310 100644 --- a/impl/create_srpm_for_git.go +++ b/impl/create_srpm_from_git.go @@ -4,10 +4,8 @@ package impl import ( - "bytes" "fmt" "os" - "os/exec" "path/filepath" "strings" @@ -22,62 +20,6 @@ type gitSpec struct { ClonedDir string } -type GitRevisionType int - -const ( - UNDEFINED GitRevisionType = 0 - COMMIT GitRevisionType = 1 - TAG GitRevisionType = 2 -) - -func (spec *gitSpec) typeOfGitRevisionFromRemote() (GitRevisionType, error) { - srcURL := spec.SrcUrl - revision := spec.Revision - cmd := exec.Command("git", "ls-remote", srcURL, revision) - var out bytes.Buffer - cmd.Stdout = &out - if err := cmd.Run(); err != nil { - return UNDEFINED, fmt.Errorf("git ls-remote of repo %s failed: %s", srcURL, err) - } - if len(out.String()) == 0 { - return COMMIT, nil - } else if strings.Contains(out.String(), revision) { - return TAG, nil - } else { - return UNDEFINED, fmt.Errorf("revision %s not found in repo %s, please provide valid commit/tag", revision, srcURL) - } -} - -func (spec *gitSpec) typeOfGitRevision() (GitRevisionType, error) { - repoPath := spec.ClonedDir - revision := spec.Revision - if err := util.CheckPath(repoPath, true, false); err != nil { - return spec.typeOfGitRevisionFromRemote() - } - - cmd := exec.Command("git", "show", revision) - cmd.Dir = repoPath - var out bytes.Buffer - cmd.Stdout = &out - if err := cmd.Run(); err != nil { - return UNDEFINED, fmt.Errorf("git show of repo %s failed %s", repoPath, err) - } - - tagCmd := exec.Command("git", "show-ref", "--tags", revision) - tagCmd.Dir = repoPath - tagCmd.Stdout = &out - if err := tagCmd.Run(); err == nil { - return TAG, nil - } - - topLine := strings.SplitAfterN(out.String(), "\n", 2)[0] - if strings.Contains(topLine, revision) { - return COMMIT, nil - } - - return UNDEFINED, fmt.Errorf("revision %s not found in repo %s, provide valid commit/tag", revision, repoPath) -} - func getRpmNameFromSpecFile(repo, pkg string, isPkgSubdirInRepo bool) (string, error) { pkgSpecDirInRepo := getPkgSpecDirInRepo(repo, pkg, isPkgSubdirInRepo) specFiles, _ := filepath.Glob(filepath.Join(pkgSpecDirInRepo, "*.spec")) @@ -279,25 +221,15 @@ func verifyGitSignature(pubKeyPath string, gitSpec gitSpec, errPrefix util.ErrPr errPrefix, err, pubKeyPath) } - var verifyRepoCmd []string + clonedDir := gitSpec.ClonedDir revision := gitSpec.Revision - revisionType, err := gitSpec.typeOfGitRevision() - if err != nil { - return fmt.Errorf("%sinvalid revision %s provided, provide either a COMMIT or TAG %s", errPrefix, revision, err) - } - if revisionType == COMMIT { - verifyRepoCmd = []string{"verify-commit", "-v", revision} - } else if revisionType == TAG { - verifyRepoCmd = []string{"verify-tag", "-v", revision} - } else { - return fmt.Errorf("%sinvalid revision %s provided, provide either a COMMIT or TAG", errPrefix, revision) + if err := util.RunSystemCmdInDir(clonedDir, "git", "show-ref", "--quiet", "--tags"); err == nil { + // the provided ref is a tag + return util.RunSystemCmdInDir(clonedDir, "git", "verify-tag", "-v", revision) } - - clonedDir := gitSpec.ClonedDir - err = util.RunSystemCmdInDir(clonedDir, "git", verifyRepoCmd...) - if err != nil { - return fmt.Errorf("%serror during verifying git repo at %s: %s", errPrefix, clonedDir, err) + if err := util.RunSystemCmdInDir(clonedDir, "git", "cat-file", "-e", revision); err == nil { + // found an object with that hash + return util.RunSystemCmdInDir(clonedDir, "git", "verify-commit", "-v", revision) } - - return nil + return fmt.Errorf("%sinvalid revision %s provided, provide either a COMMIT or TAG: %s", errPrefix, revision, err) } diff --git a/impl/create_srpm_from_git_test.go b/impl/create_srpm_from_git_test.go index 13b28d8..5d86818 100644 --- a/impl/create_srpm_from_git_test.go +++ b/impl/create_srpm_from_git_test.go @@ -152,38 +152,6 @@ func populateTestDataForGitSignature(cloneDir string) []*TestDataType { return populateTestData(cloneDir, revisionList, expectedList) } -func TestRevisionType(t *testing.T) { - cloneDir, err := cloneGitDir() - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(cloneDir) - - // To reuse populateTestData(), we convert the obtained GitRevisionType to string - resolveGitRevisionTypeToString := []string{"UNDEFINED", "COMMIT", "TAG"} - testDataList := populateTestDataForRevision(cloneDir) - for _, data := range testDataList { - gitSpec := data.gitSpec - expectedType := data.expectedValue - - typeLocalRepo, err := gitSpec.typeOfGitRevision() - if err != nil { - t.Fatal(err) - } - - // Test requires call to remote git repo. - // Enable when we use remote repo for test. - /*typeRemoteRepo, err := gitSpec.typeOfGitRevisionFromRemote() - if err != nil { - t.Fatal(err) - }*/ - - require.Equal(t, expectedType, resolveGitRevisionTypeToString[typeLocalRepo]) - //require.Equal(t, expectedType, resolveGitRevisionTypeToString[typeRemoteRepo]) - } - t.Log("Test typeOfGitRevision PASSED") -} - func TestRpmNameFromSpecFile(t *testing.T) { viper.Set("SrcDir", "testData/") defer viper.Reset() diff --git a/impl/create_srpm_for_others.go b/impl/create_srpm_from_others.go similarity index 92% rename from impl/create_srpm_for_others.go rename to impl/create_srpm_from_others.go index 8f4dfd1..1f3aefa 100644 --- a/impl/create_srpm_for_others.go +++ b/impl/create_srpm_from_others.go @@ -103,18 +103,17 @@ func verifyRpmSignature(rpmPath string, errPrefix util.ErrPrefix) error { return nil } -// checkValidSignature verifies that tarball anf signature -// correspond to same package -func checkValidSignature(tarballPath, tarballSigPath string) ( - bool, bool) { +// isSigfileApplicable checks if the tarball and sigfile paths correspond to the same package. +// Returns two bools: if the sigfile is applicable and if decompression is required. +func isSigfileApplicable(tarballPath, tarballSigPath string) (bool, bool) { lastDotIndex := strings.LastIndex(tarballSigPath, ".") if lastDotIndex == -1 || !strings.HasPrefix( tarballPath, tarballSigPath[:lastDotIndex]) { return false, false } - decompress := strings.Count(tarballPath[lastDotIndex:], ".") - dcmprsnReqd := (decompress > 0) - return true, dcmprsnReqd + suffixCount := strings.Count(tarballPath[lastDotIndex:], ".") + decompressionRequired := suffixCount > 0 + return true, decompressionRequired } // uncompressTarball decompresses the compression one layer at a time @@ -134,7 +133,7 @@ func uncompressTarball(tarballPath string, downloadDir string) (string, error) { // that matches with the sign file. func matchTarballSignCmprsn(tarballPath string, tarballSigPath string, downloadDir string, errPrefix util.ErrPrefix) (string, error) { - ok, dcmprsnReqd := checkValidSignature(tarballPath, tarballSigPath) + ok, dcmprsnReqd := isSigfileApplicable(tarballPath, tarballSigPath) if !ok { return "", fmt.Errorf("%sError while matching tarball and signature", errPrefix) diff --git a/impl/create_srpm_from_others_test.go b/impl/create_srpm_from_others_test.go index 92981df..9ce0109 100644 --- a/impl/create_srpm_from_others_test.go +++ b/impl/create_srpm_from_others_test.go @@ -8,37 +8,39 @@ import ( "github.com/stretchr/testify/require" ) -func testTarballSig(t *testing.T, folder string) { - curPath, _ := os.Getwd() - workingDir := filepath.Join(curPath, "testData/tarballSig", folder) - tarballPath := map[string]string{ - "checkTarball": filepath.Join(workingDir, "linux.10.4.1.tar.gz"), - "matchTarball": filepath.Join(workingDir, "libpcap-1.10.4.tar.gz"), +func TestIsSigfileApplicable(t *testing.T) { + testCases := []struct { + tarballPath string + signaturePath string + out1 bool + out2 bool + }{ + {"foo.tar.gz", "foo.tar.gz.sig", true, false}, + {"foo.tar.gz", "foo.tar.sig", true, true}, + {"foobar.tar.gz", "signature", false, false}, + {"foo.tar.gz", "bar.tar.gz.sig", false, false}, } - tarballSigPath := filepath.Join(workingDir, "libpcap-1.10.4.tar.gz.sig") - - switch folder { - case "checkTarball": - ok, _ := checkValidSignature(tarballPath[folder], tarballSigPath) - require.Equal(t, false, ok) - case "matchTarball": - intermediateTarball, err := matchTarballSignCmprsn( - tarballPath[folder], - tarballSigPath, - workingDir, - "TestmatchTarballSignature : ", - ) - os.Remove(intermediateTarball) - require.Equal(t, nil, err) + for _, tc := range testCases { + res1, res2 := isSigfileApplicable(tc.tarballPath, tc.signaturePath) + if res1 != tc.out1 || res2 != tc.out2 { + t.Errorf("isSigfileApplicable for (%s, %s) -> (%t, %t); expected (%t, %t)", + tc.tarballPath, tc.signaturePath, res1, res2, tc.out1, tc.out2) + } } } -func TestCheckTarballSignature(t *testing.T) { - t.Log("Test tarball Signatue Check") - testTarballSig(t, "checkTarball") -} - func TestMatchTarballSignature(t *testing.T) { - t.Log("Test tarball Signatue Match") - testTarballSig(t, "matchTarball") + t.Log("Test tarball Signature Match") + curPath, _ := os.Getwd() + workingDir := filepath.Join(curPath, "testData/tarballSig/checkTarball") + tarballSigPath := filepath.Join(workingDir, "libpcap-1.10.4.tar.gz.sig") + tarballPath := filepath.Join(workingDir, "libpcap-1.10.4.tar.gz") + intermediateTarball, err := matchTarballSignCmprsn( + tarballPath, + tarballSigPath, + workingDir, + "TestmatchTarballSignature : ", + ) + os.Remove(intermediateTarball) + require.NoError(t, err) }