Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Homebrew version of mcap records a strange library string #591

Closed
jhurliman opened this issue Sep 20, 2022 · 6 comments · Fixed by #649
Closed

Homebrew version of mcap records a strange library string #591

jhurliman opened this issue Sep 20, 2022 · 6 comments · Fixed by #649
Assignees
@james-rms
Labels
bug Something isn't working cli

Comments

@jhurliman
Copy link
Contributor

Description

After installing mcap with Homebrew, running mcap filter on a file followed by mcap info on the resulting file shows the following library string:

library: mcap go #(devel); mcap go #(devel)

The same string is repeated twice with a ; separator, and the version is reported as "devel" even though this is the official 0.0.19 release distributed via Homebrew.

  • Version: 0.0.19
  • Platform: MacOS (M1)
@jhurliman jhurliman added bug Something isn't working cli labels Sep 20, 2022
@jtbandes
Copy link
Member

FWIW, homebrew manually passes VERSION=v... when building: https://github.com/Homebrew/homebrew-core/blob/d313c0378c0392bf14848879943d24dcfafb4325/Formula/mcap.rb#L32

Not sure how this would end up being #(devel).

@jtbandes
Copy link
Member

mcap/go/mcap/writer.go

Line 55 in d96232f

library = fmt.Sprintf("mcap go #%s", Version())

@jtbandes
Copy link
Member

It looks like the version is pulled from here with git describe --tags:

mcap/go/cli/mcap/Makefile

Line 23 in 2079142

version: $(eval VERSION=$(shell sed 's_releases/mcap-cli/__g' <<< $$(git describe --tags)))

This could be using the version number from some homebrew repo and not the mcap repo.

@jhurliman
Copy link
Contributor Author

This appears to be fixed in v0.0.20

@jtbandes
Copy link
Member

I'm still seeing that mcap go #(devel); gets prepended to whatever the library string was before mcap filter.

@jtbandes jtbandes reopened this Sep 29, 2022
@james-rms james-rms self-assigned this Oct 10, 2022
@james-rms
Copy link
Collaborator

(devel) comes from here: golang/go#29228

@james-rms james-rms mentioned this issue Oct 11, 2022
Merged
james-rms added a commit that referenced this issue Oct 13, 2022
@james-rms
go: use internal version string (#649)
7425b07 
**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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@james-rms james-rms

Labels
bug Something isn't working cli
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

go: use internal version string foxglove/mcap
3 participants
@jtbandes @jhurliman @james-rms