Merge 895aec35a2 into 2896aed9f4

when building from a version-tagged commit with a clean tree, omit the
redundant git commit hash from the libp2p user agent string, saving
bytes in HTTP requests and libp2p identify. `ipfs version --commit`
still reports the full commit hash.

before (tagged release): kubo/0.37.0/6898472
after  (tagged release): kubo/0.37.0

non-tagged and dirty builds are unaffected.

cmd/ipfs/Rules.mk

@@ -12,7 +12,7 @@ PATH := $(realpath $(d)):$(PATH)
 # DEPS_OO_$(d) += merkledag/pb/merkledag.pb.go namesys/pb/namesys.pb.go
 # DEPS_OO_$(d) += pin/internal/pb/header.pb.go unixfs/pb/unixfs.pb.go
 
-$(d)_flags =-ldflags="-X "github.com/ipfs/kubo".CurrentCommit=$(git-hash)"
+$(d)_flags =-ldflags="-X "github.com/ipfs/kubo".CurrentCommit=$(git-hash) -X "github.com/ipfs/kubo".taggedRelease=$(git-tag)"
 
 $(IPFS_BIN_$(d)): GOFLAGS += $(cmd/ipfs_flags)
 

mk/git.mk

@@ -2,3 +2,11 @@
 # If that fails (e.g., we're building a docker image and have an empty objects
 # directory), assume the source isn't dirty and build anyways.
 git-hash:=$(shell git describe --always --match=NeVeRmAtCh --dirty 2>/dev/null || git rev-parse --short HEAD 2>/dev/null)
+
+# Detect if HEAD is a clean, tagged release. Used to omit redundant commit
+# hash from the libp2p user agent (the version number suffices).
+ifeq ($(findstring dirty,$(git-hash)),)
+  git-tag:=$(shell git tag --points-at HEAD 2>/dev/null | grep '^v' | head -1)
+else
+  git-tag:=
+endif

test/sharness/t0026-id.sh

@@ -16,12 +16,12 @@ test_id_compute_agent() {
     else
         AGENT_COMMIT="${AGENT_COMMIT##$AGENT_VERSION-}"
     fi
-    AGENT_VERSION="kubo/$AGENT_VERSION/$AGENT_COMMIT"
-    if test -n "$AGENT_SUFFIX"; then
+    AGENT_VERSION="kubo/$AGENT_VERSION"
     if test -n "$AGENT_COMMIT"; then
-        AGENT_VERSION="$AGENT_VERSION/"
+        AGENT_VERSION="$AGENT_VERSION/$AGENT_COMMIT"
     fi
-      AGENT_VERSION="$AGENT_VERSION$AGENT_SUFFIX"
+    if test -n "$AGENT_SUFFIX"; then
+        AGENT_VERSION="$AGENT_VERSION/$AGENT_SUFFIX"
     fi
     echo "$AGENT_VERSION"
 }

version.go

@@ -10,8 +10,14 @@ import (
 // CurrentCommit is the current git commit, this is set as a ldflag in the Makefile.
 var CurrentCommit string
 
+// taggedRelease is set via ldflag when building from a version-tagged commit
+// with a clean tree. When set, the commit hash is omitted from the libp2p
+// identify agent version and the HTTP user agent, since the version number
+// already identifies the exact source.
+var taggedRelease string
+
 // CurrentVersionNumber is the current application's version literal.
-const CurrentVersionNumber = "0.40.0-rc1"
+const CurrentVersionNumber = "0.40.0-rc2"
 
 const ApiVersion = "/kubo/" + CurrentVersionNumber + "/" //nolint
 
@@ -19,15 +25,20 @@ const ApiVersion = "/kubo/" + CurrentVersionNumber + "/" //nolint
 const RepoVersion = 18
 
 // GetUserAgentVersion is the libp2p user agent used by go-ipfs.
-//
-// Note: This will end in `/` when no commit is available. This is expected.
 func GetUserAgentVersion() string {
-	userAgent := "kubo/" + CurrentVersionNumber + "/" + CurrentCommit
-	if userAgentSuffix != "" {
-		if CurrentCommit != "" {
-			userAgent += "/"
+	// For tagged release builds with a clean tree, the commit hash is
+	// redundant since the version number identifies the exact source.
+	commit := CurrentCommit
+	if taggedRelease != "" {
+		commit = ""
 	}
-		userAgent += userAgentSuffix
+
+	userAgent := "kubo/" + CurrentVersionNumber
+	if commit != "" {
+		userAgent += "/" + commit
+	}
+	if userAgentSuffix != "" {
+		userAgent += "/" + userAgentSuffix
 	}
 	return cmdutils.CleanAndTrim(userAgent)
 }

version_test.go

@@ -0,0 +1,79 @@
+package ipfs
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestGetUserAgentVersion verifies the user agent string used in libp2p
+// identify and HTTP requests. Tagged release builds (where the commit matches
+// the tag) skip the commit hash from the agent version, since the version
+// number already identifies the exact source.
+func TestGetUserAgentVersion(t *testing.T) {
+	origCommit := CurrentCommit
+	origTagged := taggedRelease
+	origSuffix := userAgentSuffix
+	t.Cleanup(func() {
+		CurrentCommit = origCommit
+		taggedRelease = origTagged
+		userAgentSuffix = origSuffix
+	})
+
+	tests := []struct {
+		name     string
+		commit   string
+		tagged   string
+		suffix   string
+		expected string
+	}{
+		// dev builds without ldflags
+		{
+			name:     "no commit, no suffix",
+			expected: "kubo/" + CurrentVersionNumber,
+		},
+		// dev builds with commit set via ldflags
+		{
+			name:     "with commit",
+			commit:   "abc1234",
+			expected: "kubo/" + CurrentVersionNumber + "/abc1234",
+		},
+		{
+			name:     "with suffix, no commit",
+			suffix:   "test-suffix",
+			expected: "kubo/" + CurrentVersionNumber + "/test-suffix",
+		},
+		{
+			name:     "with commit and suffix",
+			commit:   "abc1234",
+			suffix:   "test-suffix",
+			expected: "kubo/" + CurrentVersionNumber + "/abc1234/test-suffix",
+		},
+		// tagged release builds: commit is redundant because the version
+		// number already maps to an exact git tag, so it is omitted to
+		// save bytes in identify and HTTP user-agent headers.
+		{
+			name:     "tagged release ignores commit",
+			commit:   "abc1234",
+			tagged:   "1",
+			expected: "kubo/" + CurrentVersionNumber,
+		},
+		{
+			name:     "tagged release with suffix ignores commit",
+			commit:   "abc1234",
+			tagged:   "1",
+			suffix:   "test-suffix",
+			expected: "kubo/" + CurrentVersionNumber + "/test-suffix",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			CurrentCommit = tt.commit
+			taggedRelease = tt.tagged
+			SetUserAgentSuffix(tt.suffix)
+
+			assert.Equal(t, tt.expected, GetUserAgentVersion())
+		})
+	}
+}