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.

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

Binary Versioning #1370

Open
0pcom opened this issue Dec 14, 2024 · 7 comments
Open

Binary Versioning #1370

0pcom opened this issue Dec 14, 2024 · 7 comments
Labels
approved This feature request will be implemented enhancement A new feature request
Milestone

Comments

@0pcom
Copy link

0pcom commented Dec 14, 2024

Describe the feature

I notice that the core binary does not have the version compiled into it.

This begs the question: when did I build this binary and is this the latest version? There is no way to tell.

So I propose a simple method which I've recently devised to compile the version into the binary.

Relevant code

it's possible to get the informaton regarding the latest commits to a branch of this repo (for example main) with go list ; example:

$ go list -m -json github.com/cogentcore/core@main

{
	"Path": "github.com/cogentcore/core",
	"Version": "v0.3.8-0.20241213221857-7f699071cef2",
	"Query": "main",
	"Time": "2024-12-13T22:18:57Z",
	"GoMod": "/home/user/go/pkg/mod/cache/download/github.com/cogentcore/core/@v/v0.3.8-0.20241213221857-7f699071cef2.mod",
	"GoVersion": "1.22",
	"Origin": {
		"VCS": "git",
		"URL": "https://github.com/cogentcore/core",
		"Hash": "7f699071cef2f7365f35c8b470e4066a8ce3b0cc"
	}
}

The above should be executed in a subshell to set a variable at compile time via -ldflags= -X ... - in a library to be created. This can be, for instance pkg/buildinfo or, I might even suggest putting a file like core.go in the root of this repository where that can be set.

So, for instance, instead of:

go install cogentcore.org/cmd/core@main

one might run

go install -ldflags=" -X github.com/cogentcore/core.buildInfo=$(go list -m -json github.com/cogentcore/core@main)" cogentcore.org/cmd/core@main

and there should perhaps be some init function to parse the version and commit out of the json set in that variable at compile time.

then cmd/core would just import "github.com/cogentcore/core" or "cogentcore.org/core" and then add a -v --version flag to the command line interface to print the version. And printing the version in the --help menu by default never hurts either. And it will still work without setting the version, just default to unknown version if the variable was not set on compilation.

...

I may submit this as a PR - if it would help. I thought of this for another project for which the versioning is more critical a concern ; so i'll be implementing it there already.

@0pcom 0pcom added the enhancement A new feature request label Dec 14, 2024
@kkoreilly
Copy link
Member

Thank you for filing this issue. We have wanted to do something like this for a while, and in fact we have this logic already for building apps with the core tool: https://github.com/cogentcore/core/blob/7f69907/cmd/core/config/config.go#L256. If you right click in a Cogent Core app built with the core tool and then click About, you will see both the version of the app and the version of Cogent Core being used.

However, we have not yet implemented such a version storage system for the core tool itself, since as you noted, doing so using the same approach would require a much longer install command that injects the version. Although that might be fine for a package manager that automatically executes that command, I would much rather avoid users having to deal with that verbosity.

That being said, I saw in the Go 1.24 draft release notes that the main module's version will now be available at runtime, which would allow us to directly get the version without having to deal with any linker flag injection. However, Go 1.24 does not come out until February, and we are unlikely to update immediately, especially given that we are hoping to use gopherjs at some point, which is several versions behind (see #974). Regardless, once we do update to 1.24, we will definitely replace our existing linker flag usage with that functionality, and we will update to use it in the core tool itself to resolve this issue.

In the meantime, I tried adding a fmt.Println(debug.ReadBuildInfo()) call to the start of an app, and I did get this at the end:

build   vcs=git
build   vcs.revision=55e6ea8edc4efa83958bc52a6c54600bf7dac0e9
build   vcs.time=2024-12-15T22:56:39Z
build   vcs.modified=true

However, I am not sure if that information will be there if the core tool is installed using go install from outside the module, so I need to test that by actually implementing it and then testing it using a real install from a pushed branch. If it does work, that would be great. Otherwise, we can try to figure out some temporary solution until we update to 1.24. I will get back to you soon.

@kkoreilly
Copy link
Member

kkoreilly commented Dec 16, 2024

Unfortunately, it does not appear to have those vcs debug info entries when installing the core tool from outside the module using go install, so that is not an effective solution to this issue. If you have any ideas on how to proceed while still keeping the install command simple, that would be great.

Regardless, I filed #1376 as a draft, so you can look at that and experiment with it if you want. It doesn't hurt to have this logic implemented in cli for when you are building from inside the repository, and as a backup for when you build an app without the core tool.

Note that the use case for building an app is more sound for this, since you do typically do that from inside a repository, but the formatting is worse than that of the core tool linker flags, so we will only use that as a backup for now with the core tool linker flags still the main implementation.

@kkoreilly kkoreilly reopened this Dec 16, 2024
@kkoreilly
Copy link
Member

We did not mean to close this issue. We merged #1376 as it is helpful in some cases and doesn't hurt as mentioned above. However, it does not provide a full solution to the issue of getting the version of the core tool when installing it from outside the git repository.

One option for getting this to work is to make core setup re-install the core tool with the appropriate linker flags. We could also add a core update or core self-update command that updates the core tool to the latest or specified version, also with the correct linker flags. Package manager scripts would directly pass the linker flags instead.

If you are interested in that solution or have any other ideas, please let me know. Once we decide on a solution, you could submit a PR if you want, or I could implement it.

@0pcom
Copy link
Author

0pcom commented Dec 17, 2024

Thanks for pointing that out about the draft release notes for go v1.24; I will need to keep that in mind.

I generally have an aversion to update logic in binaries ; simply because I've seen this implemented in a very wrong way in the past. And if you are going to execute some go install command on the system when a certain core subcommand is run ; if it's short enough, then I think it might be better to just document it such that people can copy / paste it.

For your purposes, since versioning isn't critical, perhaps waiting until the next go version would be better.

For my purposes of versioning package-based installations, it should simply be possible to parse the output of go list to determine the version for the package. Then if I have it installed via the package, at least, I will know the version.

I think my buildinfo solution is simple enough ; and here is the basic implementation with parsing the output of go list already included.

https://github.com/skycoin/skywire/blob/develop/pkg/skywire-utilities/pkg/buildinfo/buildinfo.go

basically just set the version on compile - the formula for which is included in the comments there - and then add a flag to print the version and if desired print it in the help menu by default; ex.:

https://github.com/skycoin/skywire/blob/develop/cmd/skycoin-skywire/commands/root.go#L86

on the verbosity - it really could be worse.
If you set the URL as an env you can shorten it substantially.

_core="github.com/cogentcore/core" go install -ldflags=" -X ${_core}.buildInfo=$(go list -m -json ${_core}@main)" ${_core}@main

But the most important point is that versioning like this is totally optional. people can still install it with just go install without versioning it, and the version will just print as unknown because they didn't version it.

@kkoreilly
Copy link
Member

Okay, it seems reasonable to add that functionality as an optional feature that people can specify if they need. It is fine to wait for full support until Go 1.24, as getting the version is not that critical except in certain use cases, where you can use this optional feature.

If you are interested in implementing this, a PR would be great, or I could implement it if you prefer. I think this should be added into package cli (ex: cli.Version or similar), as this is generically useful functionality and it is easier to manipulate the help string there. You can see where I added the debug.ReadBuildInfo version logic in #1376 (https://github.com/cogentcore/core/blob/e7d7703/cli/usage.go#L53) and add the logic that uses cli.Version there if you want.

Also, if possible, it would be better to use go list -m -f "{{.Version}}" cogentcore.org/core@main so that we don't have to unnecessarily parse the JSON inside of the app. Another question is whether you need this information in a separate version command and/or flag, or just in the help one.

@0pcom
Copy link
Author

0pcom commented Dec 22, 2024

I attempted to implement this but it's proving a bit challenging to test it, I think because the variable I'm setting with the version only exists on my fork..

$ go run -ldflags="-X 'github.com/cogentcore/core/cli.version=$(go list -mod=mod -m -f "{{.Version}}" github.com/0pcom/core@version)'" github.com/0pcom/core/cmd/core@version --help
go: github.com/0pcom/core/cmd/core@version: version constraints conflict:
	github.com/0pcom/[email protected]: parsing go.mod:
	module declares its path as: cogentcore.org/core
	        but was required as: github.com/0pcom/core

I dare not change the go.mod for testing purposes if this would become a PR ; Here's a slimmed down version of the earlier implementation

package cli

const unknown = "unknown"

var version = unknown

// set build info from outside the source:
// $ go build -ldflags="-X 'github.com/cogentcore/core/cli.version=$(go list -mod=mod -m -f "{{.Version}}" github.com/cogentcore/core@main)'" cogentcore.org/cmd/core@main

// Version returns version
func Version() string {
	return version
}

and the changes to cli/usage

	if version == unknown {
		if bi, ok := debug.ReadBuildInfo(); ok {
			revision, time := "dev", "unknown"
			for _, set := range bi.Settings {
				if set.Key == "vcs.revision" {
					revision = set.Value
				}
				if set.Key == "vcs.time" {
					time = set.Value
				}
			}
			b.WriteString(logx.TitleColor("Version: ") + fmt.Sprintf("%s (%s)\n\n", revision, time))
		}
	} else {
		// default to manually set version if there is one
		b.WriteString(logx.TitleColor("Version: ") + fmt.Sprintf("%s\n", version)
	}

It's really not critical, for me, that this be implemented. More just a suggestion of an improvement.

The next go release is not far away, which should basically enable automatically versioning this ; so do with it as you will.

@kkoreilly
Copy link
Member

Thank you for the update. That looks like the right logic, so you are welcome to file that as a PR if you are interested, and if I look it over and it seems good, we could merge it and then test it on main to overcome the module issues (it wouldn't break anything, so it should be fine). On the other hand, if you don't need this functionality, I would also be perfectly fine just waiting for Go 1.24; your choice based on how much you need this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved This feature request will be implemented enhancement A new feature request
Projects
None yet
Development

No branches or pull requests

2 participants