Simplifications, tests improvements, etc

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,38 +331,23 @@ 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 <hashWidthInRelease>
-// characters of the commit hashes in the SRC_<N> env vars.
+// Otherwise it is constructed by combining a shortened hash of the
+// commit from the the SRC_<N> env vars.
 // If the env vars are unset, an empty string is returned.
 func getRpmReleaseMacro(pkgSpec *manifest.Package, errPrefix util.ErrPrefix) (
 	string, error) {
 	if pkgSpec.RpmReleaseMacro != "" {
 		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_<N> 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 {

impl/create_srpm_for_git.go → 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)
 }

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()

impl/create_srpm_for_others.go → 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)

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)
 }