go: use internal version string (#649)

**Public-Facing Changes**

None.

**Description**
With context from golang/go#29228, the result of runtime/debug.BuildInfo.Main.Version is not well defined. Here we use an internally-defined Version as our library version in all contexts. We also add a test when using a go library release tag `go/mcap/v1.2.3` that the Version string is correct.

This PR also changes the behaviour of `Writer` to only append the existing library version if it's different from the current version. This removes the awkward behaviour of `mcap filter` where the resulting mcap Library would be `mcap go #(devel); mcap go #(devel); mcap go #(devel)...`.

Fixes #591

.github/workflows/ci.yml

@@ -323,6 +323,11 @@ jobs:
         run: go install github.com/golangci/golangci-lint/cmd/[email protected]
       - run: make lint
       - run: make test
+      - name: Check library version
+        if: |
+          !github.event.pull_request.head.repo.fork &&
+          startsWith(github.ref, 'refs/tags/go/mcap/v')
+        run: make -C cli/mcap build && ./check_tag.sh cli/mcap/bin/mcap
 
   go-release-cli:
     permissions:

go/check_tag.sh

@@ -0,0 +1,25 @@
+#!/usr/bin/env bash
+# Used in go CI on tagged workflows.
+# Checks that the current commit is tagged with the correct MCAP library version.
+set -eo pipefail
+
+if [ $# -ne 1 ]; then
+    echo "Usage: $0 <path-to-mcap-binary>"
+    exit 1
+fi
+
+expected_tag="go/mcap/$($1 version --library)"
+read -ra all_tags <<< "$(git tag --points-at HEAD)"
+found="false"
+for tag in "${all_tags[@]}"; do
+    if [ "$tag" = "$expected_tag" ]; then
+        found="true"
+    fi
+done
+
+if [ "$found" != "true" ]; then
+    echo "failed: expected tag $expected_tag in found tags: [${all_tags[*]}]"
+    exit 1
+else
+    echo "success"
+fi

go/cli/mcap/cmd/filter_test.go

@@ -11,6 +11,7 @@ import (
 )
 
 func writeFilterTestInput(t *testing.T, w io.Writer) {
+	mcap.Version = "1"
 	writer, err := mcap.NewWriter(w, &mcap.WriterOptions{
 		Chunked:   true,
 		ChunkSize: 10,

go/cli/mcap/cmd/version.go

@@ -3,22 +3,27 @@ package cmd
 import (
 	"fmt"
 
+	"github.com/foxglove/mcap/go/mcap"
 	"github.com/spf13/cobra"
 )
 
 var Version string
+var printLibraryVersion bool
 
 // versionCmd represents the version command
-func versionCmd(version string) *cobra.Command {
-	return &cobra.Command{
-		Use:   "version",
-		Short: "Output version information",
-		Run: func(cmd *cobra.Command, args []string) {
+var versionCmd = &cobra.Command{
+	Use:   "version",
+	Short: "Output version information",
+	Run: func(cmd *cobra.Command, args []string) {
+		if printLibraryVersion {
+			fmt.Println(mcap.Version)
+		} else {
 			fmt.Println(Version)
-		},
-	}
+		}
+	},
 }
 
 func init() {
-	rootCmd.AddCommand(versionCmd(Version))
+	versionCmd.PersistentFlags().BoolVarP(&printLibraryVersion, "library", "l", false, "print MCAP library version instead of CLI version")
+	rootCmd.AddCommand(versionCmd)
 }

go/mcap/mcap.go

@@ -4,7 +4,6 @@ import (
 	"errors"
 	"fmt"
 	"math"
-	"runtime/debug"
 )
 
 // Magic is the magic number for an MCAP file.
@@ -29,14 +28,6 @@ func (c CompressionFormat) String() string {
 	return string(c)
 }
 
-func Version() string {
-	info, ok := debug.ReadBuildInfo()
-	if !ok {
-		return "unknown"
-	}
-	return info.Main.Version
-}
-
 const (
 	OpReserved        OpCode = 0x00
 	OpHeader          OpCode = 0x01

go/mcap/version.go

@@ -0,0 +1,4 @@
+package mcap
+
+// Version of the MCAP library.
+var Version = "v0.1.0"

go/mcap/writer.go

@@ -52,8 +52,8 @@ type Writer struct {
 func (w *Writer) WriteHeader(header *Header) error {
 	var library string
 	if !w.opts.OverrideLibrary {
-		library = fmt.Sprintf("mcap go #%s", Version())
-		if header.Library != "" {
+		library = fmt.Sprintf("mcap go %s", Version)
+		if header.Library != "" && header.Library != library {
 			library += "; " + header.Library
 		}
 	} else {

go/mcap/writer_test.go

@@ -10,13 +10,16 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+const libraryString = "libfoo v0"
+
 func TestMCAPReadWrite(t *testing.T) {
 	t.Run("test header", func(t *testing.T) {
 		buf := &bytes.Buffer{}
-		w, err := NewWriter(buf, &WriterOptions{Compression: CompressionZSTD})
+		w, err := NewWriter(buf, &WriterOptions{Compression: CompressionZSTD, OverrideLibrary: true})
 		assert.Nil(t, err)
 		err = w.WriteHeader(&Header{
 			Profile: "ros1",
+			Library: libraryString,
 		})
 		assert.Nil(t, err)
 		lexer, err := NewLexer(buf)
@@ -30,7 +33,7 @@ func TestMCAPReadWrite(t *testing.T) {
 		assert.Equal(t, "ros1", profile)
 		library, _, err := readPrefixedString(record, offset)
 		assert.Nil(t, err)
-		assert.Equal(t, "mcap go #", library)
+		assert.Equal(t, "libfoo v0", library)
 		assert.Equal(t, TokenHeader, tokenType)
 	})
 	t.Run("zero-valued schema IDs permitted", func(t *testing.T) {
@@ -70,14 +73,16 @@ func TestOutputDeterminism(t *testing.T) {
 	for i := 0; i < 10; i++ {
 		buf := &bytes.Buffer{}
 		w, err := NewWriter(buf, &WriterOptions{
-			Chunked:     true,
-			Compression: CompressionZSTD,
-			IncludeCRC:  true,
-			ChunkSize:   1024,
+			Chunked:         true,
+			Compression:     CompressionZSTD,
+			IncludeCRC:      true,
+			ChunkSize:       1024,
+			OverrideLibrary: true,
 		})
 		assert.Nil(t, err)
 		assert.Nil(t, w.WriteHeader(&Header{
 			Profile: "ros1",
+			Library: libraryString,
 		}))
 		assert.Nil(t, w.WriteSchema(&Schema{
 			ID:       1,
@@ -141,13 +146,15 @@ func TestChunkedReadWrite(t *testing.T) {
 		t.Run(fmt.Sprintf("chunked file with %s", compression), func(t *testing.T) {
 			buf := &bytes.Buffer{}
 			w, err := NewWriter(buf, &WriterOptions{
-				Chunked:     true,
-				Compression: compression,
-				IncludeCRC:  true,
+				Chunked:         true,
+				Compression:     compression,
+				IncludeCRC:      true,
+				OverrideLibrary: true,
 			})
 			assert.Nil(t, err)
 			assert.Nil(t, w.WriteHeader(&Header{
 				Profile: "ros1",
+				Library: libraryString,
 			}))
 			assert.Nil(t, w.WriteSchema(&Schema{
 				ID:       1,
@@ -218,13 +225,15 @@ func TestChunkBoundaryIndexing(t *testing.T) {
 	// Each chunk in the index should reflect the time of the corresponding
 	// message.
 	w, err := NewWriter(buf, &WriterOptions{
-		Chunked:     true,
-		ChunkSize:   20,
-		Compression: CompressionZSTD,
+		Chunked:         true,
+		ChunkSize:       20,
+		Compression:     CompressionZSTD,
+		OverrideLibrary: true,
 	})
 	assert.Nil(t, err)
 	err = w.WriteHeader(&Header{
 		Profile: "ros1",
+		Library: libraryString,
 	})
 	assert.Nil(t, err)
 	assert.Nil(t, w.WriteSchema(&Schema{
@@ -266,13 +275,15 @@ func TestChunkBoundaryIndexing(t *testing.T) {
 func TestIndexStructures(t *testing.T) {
 	buf := &bytes.Buffer{}
 	w, err := NewWriter(buf, &WriterOptions{
-		Chunked:     true,
-		ChunkSize:   1024,
-		Compression: CompressionZSTD,
+		Chunked:         true,
+		ChunkSize:       1024,
+		Compression:     CompressionZSTD,
+		OverrideLibrary: true,
 	})
 	assert.Nil(t, err)
 	err = w.WriteHeader(&Header{
 		Profile: "ros1",
+		Library: libraryString,
 	})
 	assert.Nil(t, err)
 	assert.Nil(t, w.WriteSchema(&Schema{
@@ -339,13 +350,15 @@ func TestIndexStructures(t *testing.T) {
 func TestStatistics(t *testing.T) {
 	buf := &bytes.Buffer{}
 	w, err := NewWriter(buf, &WriterOptions{
-		Chunked:     true,
-		ChunkSize:   1024,
-		Compression: CompressionZSTD,
+		Chunked:         true,
+		ChunkSize:       1024,
+		Compression:     CompressionZSTD,
+		OverrideLibrary: true,
 	})
 	assert.Nil(t, err)
 	assert.Nil(t, w.WriteHeader(&Header{
 		Profile: "ros1",
+		Library: libraryString,
 	}))
 	assert.Nil(t, w.WriteSchema(&Schema{
 		ID:       1,
@@ -386,10 +399,11 @@ func TestStatistics(t *testing.T) {
 
 func TestUnchunkedReadWrite(t *testing.T) {
 	buf := &bytes.Buffer{}
-	w, err := NewWriter(buf, &WriterOptions{})
+	w, err := NewWriter(buf, &WriterOptions{OverrideLibrary: true})
 	assert.Nil(t, err)
 	err = w.WriteHeader(&Header{
 		Profile: "ros1",
+		Library: libraryString,
 	})
 	assert.Nil(t, err)
 	err = w.WriteSchema(&Schema{
@@ -465,6 +479,43 @@ func TestUnchunkedReadWrite(t *testing.T) {
 	}
 }
 
+func TestLibraryString(t *testing.T) {
+	thisLibraryString := fmt.Sprintf("mcap go %s", Version)
+	cases := []struct {
+		input  string
+		output string
+	}{
+		{"", thisLibraryString},
+		{thisLibraryString, thisLibraryString},
+		{"some-library", fmt.Sprintf("%s; some-library", thisLibraryString)},
+	}
+	for _, c := range cases {
+		t.Run("library string is automatically filled", func(t *testing.T) {
+			buf := &bytes.Buffer{}
+			w, err := NewWriter(buf, &WriterOptions{})
+			assert.Nil(t, err)
+			err = w.WriteHeader(&Header{
+				Profile: "ros1",
+				Library: c.input,
+			})
+			assert.Nil(t, err)
+			w.Close()
+			lexer, err := NewLexer(buf)
+			assert.Nil(t, err)
+			tokenType, record, err := lexer.Next(nil)
+			assert.Nil(t, err)
+			assert.Equal(t, tokenType, TokenHeader)
+			offset := 0
+			profile, offset, err := readPrefixedString(record, offset)
+			assert.Nil(t, err)
+			assert.Equal(t, "ros1", profile)
+			library, _, err := readPrefixedString(record, offset)
+			assert.Nil(t, err)
+			assert.Equal(t, library, c.output)
+		})
+	}
+}
+
 func TestMakePrefixedMap(t *testing.T) {
 	t.Run("output is deterministic", func(t *testing.T) {
 		bytes := makePrefixedMap(map[string]string{