Skip to content
New issue

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

#

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

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # 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