Skip to content

Commit 4a0682c

Browse files
findleyrgopherbot
authored andcommitted
internal/configgen: add special handling for toolchain program versions
Main module version information is not stamped in toolchain binaries, and therefore we do not have a program version for toolchain programs. Rather than add special handling for toolchain programs in many places, to handle the missing version, instead enforce the convention that for toolchain programs, program version == go version. Specifically: - Add internal/telemetry.{IsToolchainProgram,ProgramInfo}, to share the logic for interrogating program information. - Update ProgramInfo to reuse the go version as program version for toolchain programs. - Update config generation to reuse the go version list as program version list, for toolchain programs. - Use the go/version package rather than copying it, now that it is available. This means that the config generator requires go1.22, which should be fine as it is only run by developers or RelUI. - Update config generation and validation to use go/version package rather than semver package for working with toolchain program versions. - Move chartconfig.Validate to configgen.ValidateChartConfig, since validation now requires the go/version package and requires go1.22 (chartconfig is linked by more packages). For golang/go#67244 Change-Id: I226b18d37e1ad973002b19afa8afdb8cf97b358c Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/585198 Auto-Submit: Robert Findley <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 609bac3 commit 4a0682c

File tree

13 files changed

+227
-305
lines changed

13 files changed

+227
-305
lines changed

internal/configgen/gover.go

Lines changed: 0 additions & 166 deletions
This file was deleted.

internal/configgen/main.go

Lines changed: 51 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
//go:generate go run . -w
66

7-
//go:build go1.21
7+
//go:build go1.22
88

99
// Package configgen generates the upload config file stored in the config.json
1010
// file of golang.org/x/telemetry/config based on the chartconfig stored in
@@ -16,6 +16,7 @@ import (
1616
"encoding/json"
1717
"flag"
1818
"fmt"
19+
"go/version"
1920
"log"
2021
"os"
2122
"os/exec"
@@ -154,7 +155,7 @@ func generate(gcfgs []chartconfig.ChartConfig, padding padding) (*telemetry.Uplo
154155
}
155156

156157
for i, r := range gcfgs {
157-
if err := chartconfig.Validate(r); err != nil {
158+
if err := ValidateChartConfig(r); err != nil {
158159
// TODO(rfindley): this is a poor way to identify the faulty record. We
159160
// should probably store position information in the ChartConfig.
160161
return nil, fmt.Errorf("chart config #%d (%q): %v", i, r.Title, err)
@@ -189,27 +190,52 @@ func generate(gcfgs []chartconfig.ChartConfig, padding padding) (*telemetry.Uplo
189190

190191
for _, p := range programs {
191192
minVersion := minVersions[p.Name]
192-
versions, err := listProxyVersions(p.Name)
193-
if err != nil {
194-
return nil, fmt.Errorf("listing versions for %q: %v", p.Name, err)
195-
}
196-
// Filter proxy versions in place.
197-
i := 0
198-
for _, v := range versions {
199-
if !semver.IsValid(v) {
200-
// In order to perform semver comparison below, we must have valid
201-
// versions. This should always be the case for the proxy.
202-
// Trust, but verify.
203-
return nil, fmt.Errorf("invalid semver %q returned from proxy for %q", v, p.Name)
193+
194+
// Collect eligible program versions. If p is a toolchain tool (cmd/go,
195+
// cmd/compile, etc), these come out of the Go versions queried above.
196+
// Otherwise, they come from the proxy.
197+
//
198+
// In both of these cases, the versions should be valid, but we verify
199+
// anyway as otherwise the version comparison is meaningless.
200+
// (and in fact, there is an invalid go1.9.2rc2 version in the proxy)
201+
if telemetry.IsToolchainProgram(p.Name) {
202+
// Note: no need to pad versions for toolchain programs, since the
203+
// toolchain is released infrequently.
204+
// (and in any case, version padding only works for semantic versions)
205+
for _, v := range ucfg.GoVersion {
206+
if !version.IsValid(v) {
207+
// The proxy toolchain versions list go1.9.2rc2, which is invalid.
208+
// Skip it. Note: this also filters out "devel", but that is added
209+
// back below.
210+
continue
211+
}
212+
213+
if minVersion == "" || version.Compare(minVersion, v) <= 0 {
214+
p.Versions = append(p.Versions, v)
215+
}
216+
}
217+
} else {
218+
versions, err := listProxyVersions(p.Name)
219+
if err != nil {
220+
return nil, fmt.Errorf("listing versions for %q: %v", p.Name, err)
204221
}
205-
if minVersion == "" || semver.Compare(minVersion, v) <= 0 {
206-
versions[i] = v
207-
i++
222+
// Filter proxy versions in place.
223+
i := 0
224+
for _, v := range versions {
225+
if !semver.IsValid(v) {
226+
return nil, fmt.Errorf("invalid semver %q returned from proxy for %q", v, p.Name)
227+
}
228+
if minVersion == "" || semver.Compare(minVersion, v) <= 0 {
229+
versions[i] = v
230+
i++
231+
}
208232
}
233+
p.Versions = padVersions(versions[:i], prereleasesForProgram(p.Name), padding)
209234
}
210-
p.Versions = padVersions(versions[:i], prereleasesForProgram(p.Name), padding)
211-
// TODO(hakim): allow to collect counters from gopls@devel. go.dev/issues/62271
212-
if p.Name == "golang.org/x/tools/gopls" {
235+
// Allow collecting counters for devel versions of gopls and toolchain
236+
// programs.
237+
// TODO(golang/go#62271): revert once no longer needed
238+
if p.Name == "golang.org/x/tools/gopls" || telemetry.IsToolchainProgram(p.Name) {
213239
p.Versions = append(p.Versions, "devel") // added at the end.
214240
}
215241
ucfg.Programs = append(ucfg.Programs, p)
@@ -360,7 +386,7 @@ type byGoVersion []string
360386
func (vs byGoVersion) Len() int { return len(vs) }
361387
func (vs byGoVersion) Swap(i, j int) { vs[i], vs[j] = vs[j], vs[i] }
362388
func (vs byGoVersion) Less(i, j int) bool {
363-
cmp := Compare(vs[i], vs[j])
389+
cmp := version.Compare(vs[i], vs[j])
364390
if cmp != 0 {
365391
return cmp < 0
366392
}
@@ -445,7 +471,7 @@ func padVersions(versions []string, prereleasePatterns []string, padding padding
445471
// This assumes that the program in question doesn't patch older releases
446472
// (as is the case with gopls). If that assumption ever changes, we may need
447473
// to apply padding to older versions as well.
448-
versionsToPad := []version{parsedLatest}
474+
versionsToPad := []semversion{parsedLatest}
449475

450476
var maj, min, patch int
451477
for _, toPad := range versionsToPad {
@@ -501,16 +527,16 @@ func padVersions(versions []string, prereleasePatterns []string, padding padding
501527
}
502528

503529
// version is a parsed semantic version.
504-
type version struct {
530+
type semversion struct {
505531
major, minor, patch int
506532
pre string
507533
}
508534

509535
// parseSemver attempts to parse semver components out of the provided semver
510536
// v. If v is not valid semver in canonical form, parseSemver returns _, _, _,
511537
// _, false.
512-
func parseSemver(v string) (_ version, ok bool) {
513-
var parsed version
538+
func parseSemver(v string) (_ semversion, ok bool) {
539+
var parsed semversion
514540
v, parsed.pre, _ = strings.Cut(v, "-")
515541
if _, err := fmt.Sscanf(v, "v%d.%d.%d", &parsed.major, &parsed.minor, &parsed.patch); err == nil {
516542
ok = true

internal/configgen/main_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build go1.21
5+
//go:build go1.22
66

77
package main
88

internal/configgen/syslist.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build go1.21
5+
//go:build go1.22
66

77
package main
88

internal/chartconfig/validate.go renamed to internal/configgen/validate.go

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,23 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
package chartconfig
5+
//go:build go1.22
6+
7+
package main
68

79
import (
810
"errors"
911
"fmt"
12+
"go/version"
1013

1114
"golang.org/x/mod/semver"
15+
"golang.org/x/telemetry/internal/chartconfig"
16+
"golang.org/x/telemetry/internal/telemetry"
1217
)
1318

14-
// Validate checks that a ChartConfig is complete and coherent, returning an
15-
// error describing all problems encountered, or nil.
16-
func Validate(cfg ChartConfig) error {
19+
// ValidateChartConfig checks that a ChartConfig is complete and coherent,
20+
// returning an error describing all problems encountered, or nil.
21+
func ValidateChartConfig(cfg chartconfig.ChartConfig) error {
1722
var errs []error
1823
reportf := func(format string, args ...any) {
1924
errs = append(errs, fmt.Errorf(format, args...))
@@ -39,8 +44,12 @@ func Validate(cfg ChartConfig) error {
3944
if cfg.Depth != 0 && cfg.Type != "stack" {
4045
reportf("depth can only be set for \"stack\" chart types")
4146
}
42-
if cfg.Version != "" && !semver.IsValid(cfg.Version) {
43-
reportf("%q is not valid semver", cfg.Version)
47+
valid := semver.IsValid
48+
if telemetry.IsToolchainProgram(cfg.Program) {
49+
valid = version.IsValid
50+
}
51+
if cfg.Version != "" && !valid(cfg.Version) {
52+
reportf("%q is not a valid version (must be a go version or semver)", cfg.Version)
4453
}
4554
return errors.Join(errs...)
4655
}

0 commit comments

Comments
 (0)