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

Jump to bottom

runtime: Go version 1.10 will introduce problems when comparing versions #23553

Closed
jhinrichsen opened this issue Jan 25, 2018 · 7 comments
Closed

runtime: Go version 1.10 will introduce problems when comparing versions #23553

jhinrichsen opened this issue Jan 25, 2018 · 7 comments
Labels
FrozenDueToAge

Comments

@jhinrichsen
Copy link

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

1.10beta2

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

independent of OS/ CPU

What did you do?

Run goconvey (https://github.com/smartystreets/goconvey) on my current go project

If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

What did you expect to see?

No warning (same as Go 1.9)

What did you see instead?

2018/01/25 17:05:47 goconvey.go:210: Go version is less that 1.2 (go1.10beta2), please upgrade to the latest stable version to enable coverage reporting.

Agreed that the method to compare versions is suboptimal:

	version := runtime.Version() // 'go1.2....'
	major, minor := version[2], version[4]
	version_1_2 := major >= byte('1') && minor >= byte('2')
	if !version_1_2 {
		log.Printf(pleaseUpgradeGoVersion, version)
		return false
	}
	return true

but i did not find a way to extract this information from runtime's func Version() string, other than importing a full blown lib such as hashicorp/go-version.

I am aware of #11972 (Open), #21207 (Closed), and #21209 (Closed).
@bradfitz
Copy link
Contributor

Have you filed a bug against goconvey?

@ianlancetaylor
Copy link
Contributor

What action do you suggest we take?

@rasky
Copy link
Member

rasky commented Jan 25, 2018

There are many possibilities to provide a better API to decrease the burden on developers and thus the likeliness of errors. I don't want to bike-shed but off the top my mind a few options are:

  • This problem is basically already solved at compile-time through tags. We could add an API to do a runtime check on compile-time tags runtime.HasTag("go1.2")
  • Implement a version comparison API. runtime.CompareVersion("1.2") or runtime.VersionAtLeast("1.2").
  • Return version number as tuple of integers. This solves only half of the problem, but probably better than nothing: runtime.VersionTuple() (int,int,int).

One of the problems is that versions strings are always (probably inherently) underspecified so it's hard to write future-proof code. I submitted a recent CL that was parsing a Darwin version string in the runtime, and since I have no clue how Apple might decide to evolve that string, the code is really fragile.

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Jan 25, 2018 •
edited

You can check compile-time tags at run time today by looking at Default.ReleaseTags in the go/build package.

A string based version comparison API doesn't make sense to me. How will people know what to pass?

The problem with VersionTuple is understanding how to handle 1.10beta1, 1.10beta2, 1.10rc1, 1.10.1, 1.10.2, etc.

Perhaps the problem is doing a run time version check in the first place. The code above from goconvey could be implemented reliably using build tags.

@rasky
Copy link
Member

rasky commented Jan 25, 2018 •
edited

I can't see Default in the index of the documentation of go/build. It looks like the documentation is at the bottom of type Context, I wonder if it's a godoc syntax error.

build.Default.ReleaseTags sounds fine to me, but it is basically hidden for people that don't know what to search. We should probably point to it from the documentation of runtime.Version()

@andybons
Copy link
Member

The design discussion around runtime.Version() and any new functionality added to better detect/compare versions should be in #11972, no?

@ianlancetaylor
Copy link
Contributor

SGTM. Closing as dup. Thanks.

@mikioh mikioh changed the title Go version 1.10 will introduce problems when comparing versions runtime: Go version 1.10 will introduce problems when comparing versions Jan 26, 2018
@golang golang locked and limited conversation to collaborators Jan 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@bradfitz @andybons @rasky @jhinrichsen @ianlancetaylor @gopherbot