Skip to content

Commit

Permalink
go: use internal version string (#649)
Browse files Browse the repository at this point in the history
**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
  • Loading branch information
james-rms committed Oct 13, 2022
1 parent b187669 commit 7425b07
Show file tree
Hide file tree
Showing 8 changed files with 120 additions and 38 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/ci.yml
Expand Up @@ -323,6 +323,11 @@ jobs:
run: go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.49.0
- 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:
Expand Down
25 changes: 25 additions & 0 deletions 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
1 change: 1 addition & 0 deletions go/cli/mcap/cmd/filter_test.go
Expand Up @@ -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,
Expand Down
21 changes: 13 additions & 8 deletions go/cli/mcap/cmd/version.go
Expand Up @@ -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)
}
9 changes: 0 additions & 9 deletions go/mcap/mcap.go
Expand Up @@ -4,7 +4,6 @@ import (
"errors"
"fmt"
"math"
"runtime/debug"
)

// Magic is the magic number for an MCAP file.
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions go/mcap/version.go
@@ -0,0 +1,4 @@
package mcap

// Version of the MCAP library.
var Version = "v0.1.0"
4 changes: 2 additions & 2 deletions go/mcap/writer.go
Expand Up @@ -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 {
Expand Down
89 changes: 70 additions & 19 deletions go/mcap/writer_test.go
Expand Up @@ -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)
Expand All @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down

0 comments on commit 7425b07

Please sign in to comment.