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

x/build/maintner/maintnerd/maintapi/version: support parsing all Go version tags, remove allocations #40558

Open
dmitshur opened this issue Aug 3, 2020 · 1 comment
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Aug 3, 2020

The current Go tag parser supports only final releases, not betas and not RCs. It would become more useful if it supported all release types. There are some places in x/build where we've wanted to use it, but couldn't do so because of this limitation.

Historically, we've worked around not having a parser by using string manipulation to analyze and modify Go versions, but that can be error prone and won't detect all problems a complete parser would. It's hard to compare or manipulate versions (e.g., get a previous minor release).

Examples
if strings.HasPrefix(goVer, "go1.13") || strings.HasPrefix(goVer, "go1.14") {
	// ...
}
func nextVersion(version string) (string, error) {
	parts := strings.Split(version, ".")
	n, err := strconv.Atoi(parts[len(parts)-1])
	if err != nil {
		return "", err
	}
	parts[len(parts)-1] = strconv.Itoa(n + 1)
	return strings.Join(parts, "."), nil
}
if strings.Contains(release, "beta") {
	// ...
} else if strings.Contains(release, "rc") {
	// ...
} else if strings.Count(w.Version, ".") == 1 {
	// ...
} else if strings.Count(w.Version, ".") == 2 {
	// ...
}

Additionally, the ParseTag function currently makes allocations, which can be avoided. That would be nice to make it less expensive to use in various loops in the build infrastructure.

CL 245277 implements this.

In the future, if more places in x/build start using this, we may consider moving it to a shorter import path. It doesn't have to happen right away.

@dmitshur dmitshur added Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done. labels Aug 3, 2020
@dmitshur dmitshur added this to the Unreleased milestone Aug 3, 2020
@dmitshur dmitshur self-assigned this Aug 3, 2020
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/246597 mentions this issue: cmd/release: start using FreeBSD 11.2 builders for Go 1.15 RC 2+

gopherbot pushed a commit to golang/build that referenced this issue Aug 4, 2020
Add the ability to have builds apply only to select Go versions
according to a module-query-like but fictional syntax. We don't
need to support all arbitrary queries right away, so start with
a smaller set of queries, and let this inform future work.

Fixes golang/go#40563.
Updates golang/go#40558.

Change-Id: I8f7afa90bfe1f91190cee34af51c012216bba455
Reviewed-on: https://go-review.googlesource.com/c/build/+/246597
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

2 participants