Improve git tag semver resolution

util/git/client.go

@@ -631,14 +631,9 @@ func (m *nativeGitClient) lsRemote(revision string) (string, error) {
 		revision = "HEAD"
 	}
 
-	// Check if the revision is a valid semver constraint before attempting to resolve it
-	if constraint, err := semver.NewConstraint(revision); err == nil {
-		semverSha := m.resolveSemverRevision(constraint, refs)
-		if semverSha != "" {
-			return semverSha, nil
-		}
-	} else {
-		log.Debugf("Revision '%s' is not a valid semver constraint, skipping semver resolution.", revision)
+	semverSha := m.resolveSemverRevision(revision, refs)
+	if semverSha != "" {
+		return semverSha, nil
 	}
 
 	// refToHash keeps a maps of remote refs to their hash
@@ -684,18 +679,31 @@ func (m *nativeGitClient) lsRemote(revision string) (string, error) {
 
 	// If we get here, revision string had non hexadecimal characters (indicating its a branch, tag,
 	// or symbolic ref) and we were unable to resolve it to a commit SHA.
-	return "", fmt.Errorf("Unable to resolve '%s' to a commit SHA", revision)
+	return "", fmt.Errorf("unable to resolve '%s' to a commit SHA", revision)
 }
 
 // resolveSemverRevision is a part of the lsRemote method workflow.
-// When the user configure correctly the Git repository revision and the revision is a valid semver constraint
-// only the for loop in this function will run, otherwise the lsRemote loop will try to resolve the revision.
+// When the user correctly configures the Git repository revision, and that revision is a valid semver constraint, we
+// use this logic path rather than the standard lsRemote revision resolution loop.
 // Some examples to illustrate the actual behavior, if:
-// * The revision is "v0.1.*"/"0.1.*" or "v0.1.2"/"0.1.2" and there's a tag matching that constraint only this function loop will run;
-// * The revision is "v0.1.*"/"0.1.*" or "0.1.2"/"0.1.2" and there is no tag matching that constraint this function loop and lsRemote loop will run for backward compatibility;
-// * The revision is "custom-tag" only the lsRemote loop will run because that revision is an invalid semver;
-// * The revision is "master-branch" only the lsRemote loop will run because that revision is an invalid semver;
-func (m *nativeGitClient) resolveSemverRevision(constraint *semver.Constraints, refs []*plumbing.Reference) string {
+// * The revision is "v0.1.2"/"0.1.2", then this is not a constraint, it's a pinned version - so we fall back to the standard tag matching in the lsRemote loop.
+// * The revision is "v0.1.*"/"0.1.*", and there's a tag matching that constraint, then we find the latest matching version and return its commit hash.
+// * The revision is "v0.1.*"/"0.1.*", and there is *no* tag matching that constraint, then we fall back to the standard tag matching in the lsRemote loop.
+// * The revision is "custom-tag", only the lsRemote loop will run - because that revision is an invalid semver;
+// * The revision is "master-branch", only the lsRemote loop will run because that revision is an invalid semver;
+func (m *nativeGitClient) resolveSemverRevision(revision string, refs []*plumbing.Reference) string {
+	if _, err := semver.NewVersion(revision); err == nil {
+		// If the revision is a valid version, then we know it isn't a constraint; it's just a pin.
+		// In which case, we should use standard tag resolution mechanisms.
+		return ""
+	}
+
+	constraint, err := semver.NewConstraint(revision)
+	if err != nil {
+		log.Debugf("Revision '%s' is not a valid semver constraint, skipping semver resolution.", revision)
+		return ""
+	}
+
 	maxVersion := semver.New(0, 0, 0, "", "")
 	maxVersionHash := plumbing.ZeroHash
 	for _, ref := range refs {
@@ -723,6 +731,7 @@ func (m *nativeGitClient) resolveSemverRevision(constraint *semver.Constraints,
 		return ""
 	}
 
+	log.Infof("Semver constraint '%s' resolved to tag '%s', at reference '%s'", revision, maxVersion.Original(), maxVersionHash.String())
 	return maxVersionHash.String()
 }