refactor: move logic to version.go

cmd/ipfs/daemon.go

@@ -414,10 +414,7 @@ func daemonFunc(req *cmds.Request, re cmds.ResponseEmitter, env cmds.Environment
 
 	agentVersionSuffixString, _ := req.Options[agentVersionSuffix].(string)
 	if agentVersionSuffixString != "" {
-		// Although advertised as a suffix for the `AgentVersion` this applies
-		// to the `UserAgent` internally which will be reflected in the
-		// `AgentVersion` with the `ipfs id` command.
-		ncfg.UserAgentSuffix = agentVersionSuffixString
+		version.SetUserAgentSuffix(agentVersionSuffixString)
 	}
 
 	node, err := core.NewNode(req.Context, ncfg)

core/builder.go

@@ -38,12 +38,6 @@ func NewNode(ctx context.Context, cfg *BuildCfg) (*IpfsNode, error) {
 		ctx: ctx,
 	}
 
-	// FIXME: Consider moving this to an `fx.Option` if we end up storing
-	//  the suffix inside the IpfsNode structure.
-	if cfg.UserAgentSuffix != "" {
-		n.UserAgentSuffix = cfg.UserAgentSuffix
-	}
-
 	app := fx.New(
 		node.IPFS(ctx, cfg),
 

core/commands/id.go

@@ -223,9 +223,6 @@ func printSelf(keyEnc ke.KeyEncoder, node *core.IpfsNode) (interface{}, error) {
 		sort.Strings(info.Protocols)
 	}
 	info.ProtocolVersion = identify.LibP2PVersion
-	// FIXME: Does this apply to remote requests as well? It would likely
-	//  seem so as UserAgent() is what libp2p uses to configure itself and
-	//  it's what's likely advertised over the wire. Confirm this.
-	info.AgentVersion = version.UserAgent(node.UserAgentSuffix)
+	info.AgentVersion = version.GetUserAgentVersion()
 	return info, nil
 }

core/core.go

@@ -61,9 +61,6 @@ type IpfsNode struct {
 	// Self
 	Identity peer.ID // the local node's identity
 
-	// FIXME: Check if this is the right place for it.
-	UserAgentSuffix string
-
 	Repo repo.Repo
 
 	// Local node

core/corehttp/gateway.go

@@ -104,7 +104,7 @@ func VersionOption() ServeOption {
 	return func(n *core.IpfsNode, _ net.Listener, mux *http.ServeMux) (*http.ServeMux, error) {
 		mux.HandleFunc("/version", func(w http.ResponseWriter, r *http.Request) {
 			fmt.Fprintf(w, "Commit: %s\n", version.CurrentCommit)
-			fmt.Fprintf(w, "Client Version: %s\n", version.UserAgent(n.UserAgentSuffix))
+			fmt.Fprintf(w, "Client Version: %s\n", version.GetUserAgentVersion())
 			fmt.Fprintf(w, "Protocol Version: %s\n", id.LibP2PVersion)
 		})
 		return mux, nil

core/corehttp/gateway_test.go

@@ -732,7 +732,7 @@ func TestVersion(t *testing.T) {
 		t.Fatalf("response doesn't contain commit:\n%s", s)
 	}
 
-	if !strings.Contains(s, "Client Version: "+version.UserAgent("")) {
+	if !strings.Contains(s, "Client Version: "+version.GetUserAgentVersion()) {
 		t.Fatalf("response doesn't contain client version:\n%s", s)
 	}
 

core/node/builder.go

@@ -40,8 +40,6 @@ type BuildCfg struct {
 	Routing libp2p.RoutingOption
 	Host    libp2p.HostOption
 	Repo    repo.Repo
-
-	UserAgentSuffix string
 }
 
 func (cfg *BuildCfg) getOpt(key string) bool {

core/node/libp2p/libp2p.go

@@ -25,16 +25,7 @@ type Libp2pOpts struct {
 }
 
 // Misc options
-var UserAgent = simpleOpt(libp2p.UserAgent(version.UserAgent("")))
-
-// FIXME: Check after first review pass if this is the correct structuring.
-//  (This currently has an import cycle due to the `core` dependency.)
-//func UserAgent(node *core.IpfsNode) func() (opts Libp2pOpts, err error) {
-//	return func() (opts Libp2pOpts, err error) {
-//		opts.Opts = append(opts.Opts, libp2p.UserAgent(version.UserAgent(node.UserAgentSuffix)))
-//		return
-//	}
-//}
+var UserAgent = simpleOpt(libp2p.UserAgent(version.GetUserAgentVersion()))
 
 func ConnectionManager(low, high int, grace time.Duration) func() (opts Libp2pOpts, err error) {
 	return func() (opts Libp2pOpts, err error) {

test/sharness/t0026-id.sh

@@ -16,8 +16,9 @@ test_id_compute_agent() {
     else
         AGENT_COMMIT="${AGENT_COMMIT##$AGENT_VERSION-}"
     fi
-    AGENT_VERSION="go-ipfs/$AGENT_VERSION/$AGENT_COMMIT$AGENT_SUFFIX"
+    AGENT_VERSION="go-ipfs/$AGENT_VERSION/$AGENT_COMMIT"
     test -z "$AGENT_SUFFIX" || AGENT_VERSION="$AGENT_VERSION/$AGENT_SUFFIX"
+    echo "$AGENT_VERSION"
 }
 
 test_expect_success "checking AgentVersion" '

version.go

@@ -8,18 +8,21 @@ const CurrentVersionNumber = "0.11.0-dev"
 
 const ApiVersion = "/go-ipfs/" + CurrentVersionNumber + "/"
 
-// UserAgent is the libp2p user agent used by go-ipfs.
+// GetUserAgentVersion is the libp2p user agent used by go-ipfs.
 //
 // Note: This will end in `/` when no commit is available. This is expected.
-func UserAgent(suffix string) string {
+func GetUserAgentVersion() string {
 	userAgent := "go-ipfs/" + CurrentVersionNumber + "/" + CurrentCommit
-	if suffix != "" {
-		userAgent += suffix + "/"
-		// FIXME: An alternative to passing the suffix as string would be to have a
-		//  static `var UserAgentSuffix string` here and modify it during `ipfs daemon`
-		//  startup. The drawback is that ideally this variable should only be allowed
-		//  to be modified there so we instead make it part of the node structure and
-		// explicitly pass it here. Check if this is the correct alternative.
+	if userAgentSuffix != "" {
+		if CurrentCommit != "" {
+			userAgent += "/"
+		}
+		userAgent += userAgentSuffix
 	}
 	return userAgent
 }
+
+var userAgentSuffix string
+func SetUserAgentSuffix(suffix string) {
+	userAgentSuffix = suffix
+}