Activity
Yesterday
dmitshur commented on golang.org/x/tools/...: set gotypesalias=1 when using >=1.23 toolchain14h
(1 comment)
There should be a blank line left in cases like this, since you don’t want the copyright comment to become a package comment.dmitshur reviewed +2 on golang.org/x/build/main.star: update wazero version to 1.8.115h
(1 comment)
I'll answer anyway since I too was curious and looked into it before. :) It's a global version epoch, bumped when the process for packaging changes, so that each individual package doesn…This Week
dmitshur reviewed +2 on golang.org/x/build/main.star: update wazero version to 1.8.11d
Thanks.
I kicked off a shadow build that you can take a look at to see that everything's still passing with the new version, at least for the main Go repo:
```
$ led get-build -real-build 873451722…dmitshur closed an issue golang.org/x/build/dashboard: add OpenBSD 7.3 and 7.4 builders (for 386, amd64)1d
Marking this as "not planned" so the issue outcome is more visible at a high level.dmitshur commented on golang.org/x/build/env/openbsd-amd64: update build image script to OpenBSD 7.61d
Thanks.
What's the effect of '%' here?
(minor) Now that the go.mod defines the minimum Go language version (and thus toolchain), there's less need to maintain the go version via the filename, and m…dmitshur commented on golang.org/x/website/_content/js: generate TOC items before adding anchor links1d
Thanks.dmitshur opened a change golang.org/x/website/_content/blog: don't load godocs.js (and other scripts) more than once1d
By now, the /js/godocs.js and /js/playground.js scripts are already
loaded on all go.dev pages because they're included at the bottom of
the root site.tmpl template. The layout template for blog page…dmitshur commented on golang.org/x/exp/apidiff: avoid comparing instantiated aliases2d
(1 comment)
Note that this line won't automatically close [issue 67051](https://go.dev/issue/67051) as is because it'll be interpreted as https://github.com/golang/exp/issues/67051 which is 404.
Si…dmitshur commented on golang.org/x/website/_content/js: generate TOC items before adding anchor links2d
(1 comment)
Done in PS 2, PTAL.dmitshur commented on golang.org/x/website/_content/js: generate TOC items before adding anchor links2d
(1 comment)
@hyangah@gmail.com If there isn't a good reason for setAnchors to stay in site.js, this CL can just move it there, after generateTOC, instead of using window.initFuncs to achieve that. B…dmitshur commented on golang.org/x/website: code and other elements no longer display in TOC2d
[CL 618635](https://go.dev/cl/618635) changes the order to be:
```
setAnchors is being scheduled
generateTOC is being scheduled
generateTOC is starting
setAnchors is starting
```
<img widt…dmitshur opened a change golang.org/x/website/_content/js: generate TOC items before adding anchor links2d
Use the existing window.initFuncs mechanism to arrange for setAnchors
to run after generateTOC, instead of before. That way generateTOC can
go back to using the full heading text as it did before CL …dmitshur commented on golang.org/x/website: code and other elements no longer display in TOC.2d
[CL 601055](https://go.dev/cl/601055) made the change to start using `node.firstChild.textContent` as the TOC item text, instead of `$(node).text()`. It looks like it was trying not to include the '…dmitshur commented on golang.org/x/website/go.mod: update google.golang.org/protobuf2d
(1 comment)
FWIW these all are in older Go toolchains, not related to code inside x/website. I don't see them under go1.23.2.dmitshur reviewed +2 on golang.org/x/website/go.mod: update google.golang.org/protobuf2d
Thanks.dmitshur commented on golang.org/x/website/_content/doc/faq: avoid markdown in section titles2d
(1 comment)
This approach does fix the issue, but perhaps it would be better to adjust the TOC generation code so that it's possible to continue to use code blocks in headings. That is, it seems thi…dmitshur starred honnef.co/go/jello3d
dmitshur starred honnef.co/go/curve3d
dmitshur commented on github.com/go-gl/glfw/PlatformError: Cocoa: Failed to retrieve display name4d
Thanks for the report. GLFW v3.2 is an older version. Does this also happen with v3.3?Last Week
dmitshur commented on golang.org/x/tools/internal/aliases: add a function to conditionally enable aliases6d
Commit-Queue+2 permission was granted only to members of the team working on the LUCI migration when it began (CL 468955), and by now it's in an in-between state - neither removed from members of tha…dmitshur commented on golang.org/x/build/internal/task: surface script logs to relui even on script failure6d
(4 comments)
CloneAsSubDir is a hard-to-use-correctly API that's only needed here in one place. It's hard to use because it requires the caller to keep track of what subdirectory the Git was cloned …dmitshur reviewed +2 on golang.org/x/build/internal/task: replace reviewers parameter with release coordinators6d
Thanks.dmitshur commented on golang.org/x/build: add LUCI linux-s390x builder6d
Thanks. CC @mknyszek.dmitshur commented on all: audit x/ repos for gotypesalias settings1w
Gaby didn't include it in https://github.com/golang/go/issues/69772#issuecomment-2392352550, but issue #67791 is also related here. If there are tests in x/ repos that would fail when switching to th…dmitshur commented on golang.org/x/tools/...: set gotypesalias=0 explicitly1w
(1 comment)
@taking@google.com Note that inserting blank lines between the [package comment](https://go.dev/doc/comment#package) and the `package` clause is problematic. For example, compare documen…dmitshur commented on golang.org/x/build/internal/task: dynamically find nested modules to tidy during tagging1w
This change is ready for review.dmitshur commented on golang.org/x/build/internal/gitfs: add a generated copy of x/website/internal/gitfs1w
This change is ready for review.dmitshur commented on golang.org/x/build/internal/task: update tagging for x/oscar/internal/devtools nested module1w
This change is ready for review.dmitshur opened a change golang.org/x/build/internal/task: dynamically find nested modules to tidy during tagging1w
Fetch the git tree where go.mod is being updated, walk over its content
to find nested modules to tidy. This way even if new nested modules are
added, they'll be handled without needing additional at…dmitshur commented on golang.org/x/build: OOM on windows-arm64 builder1w
Are the OOMs in x/tools ongoing? If it was just that burst around September 20-22, I added some more detail in https://github.com/golang/go/issues/66962#issuecomment-2391753139 that's likely relevant.dmitshur commented on golang.org/x/build: gotip-windows-arm64 builders stops working1w
Recording here that there was another instance of the builders disconnecting and needing to be restarted around September 20-23:
<img width="37" alt="image" src="https://github.com/user-attachment…dmitshur reviewed +2 on golang.org/x/build/main.star: add linux-arm64-clang15 builder type1w
Thanks.dmitshur reviewed +2 on golang.org/x/build/main.star: make freebsd-amd64 high capacity and enable subrepo builders1w
Thanks.
Looks like formatting needs to be fixed.
Since we’re in x/build here, this can be written in the usual way as golang/go#61095. Not a big deal of course.dmitshur reviewed +2 on golang.org/x/build/maintner: use more recent protobuf module1w
Thanks.dmitshur commented on make.bash: hint how to install the bootstrap Go1w
Something related that's been on my mind: if we have a directory with a go.mod file with the following content:
```
module minimumbootstrap
go 1.22.6
```
Then it becomes trivial for a user with a …dmitshur commented on make.bash: hint how to install the bootstrap Go1w
(2 comments)
Yeah, I understand this message is only printed when there's no "go" binary in PATH, and none of $HOME/go1.22.6, $HOME/sdk/go1.22.6, nor $HOME/go1.4 exist. So saying "if you have Go ins…dmitshur fixed an issue golang.org/x/website: update mentions of GOARCH=wasm files in misc/wasm to lib/wasm where needed1w
All 3 CLs mentioned above are submitted, so I think this is done. Feel free to comment if we missed something.dmitshur reviewed +2 on golang.org/x/build/env/freebsd-amd64: port to LUCI and upgrade to 14.11w
Thanks.
Asking to understand why this is 'go1.21.0' - it is because a binary with that name is expected to be definitely Go 1.21.0 and can upgrade itself to the minimum specified in go.mod, whereas …dmitshur commented on golang.org/x/website/wasi: update wasm runtime file location1w
Thanks.dmitshur commented on golang.org/x/website/wasi: update wasm runtime file location1w
Let's consider proceeding here analogously to https://go-review.googlesource.com/c/wiki/+/604119/comments/27f10acb_3d4f05ce.dmitshur commented on go.dev/wiki/WebAssembly: update support file location1w
Thanks.dmitshur reviewed +2 on golang.org/x/build/...: add support for lib/wasm folder1w
dmitshur commented on go.dev/wiki/WebAssembly: update support file location1w
Some time has passed since #68024 was resolved. Since there is a note pointing to the old path for Go 1.23 and earlier, should we proceed with submitting this now so that documentation is also accura…dmitshur commented on cmd/go: breaking change in 1.23rc2 with version constraints in GOPATH mode1w
@timothy-king You're running into friction issue #25574 – there was already a backport request in https://github.com/golang/go/issues/68658#issuecomment-2313617202. As a workaround, you can create …dmitshur commented on golang.org/x/build/...: add support for lib/wasm folder1w
Thanks.
#68024 is now resolved.dmitshur commented on golang.org/x/website: update mentions of GOARCH=wasm files in misc/wasm to lib/wasm where needed1w
Added release-blocker for now so we can track this better. (It's a documentation only issue for a secondary port, so it wouldn't block the release in practice.)dmitshur commented on golang.org/x/website/internal/history: document Go 1.23.2 and 1.22.8 releases1w
In the failed runs, it said "LUCI CV no longer needs this Tryjob".
This happened because patch set 2 was uploaded without an explicit CQ+1 vote, and the CQ+1 vote from patch set 1 was removed as out…dmitshur reviewed +2 on golang.org/x/build/internal/task: add workflow to build internal security release branches1w
(1 comment)
(minor) Returning `ci` here made sense, in that the change was created successfully and `ci.ChangeID` is its ID - the only thing that failed was setting a CQ+1 vote on it.dmitshur reviewed +2 on golang.org/x/build/internal/gomote: specify workdir to buildlet as an absolute path1w
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/gomote: specify workdir to buildlet as an absolute path1w
Thanks.dmitshur reviewed +2 on go.dev/wiki/WebAssembly: fix reference to `go install`1w
Thanks.
Looks like https://go.googlesource.com/wiki/+/c775517b3fee297ad3321f53dbd5c39a92f30f1b%5E%21 updated it from GOPATH mode era to module mode era, but partially.dmitshur reviewed +2 on README.vendor: add note about GOROOT, recommend fresh go1w
dmitshur reviewed +2 on README.vendor: add note about GOROOT, recommend fresh go1w
Thanks.
I think we can say "Otherwise, the results are undefined."
(In general, the go command supports working on standard library packages in its own GOROOT. Working outside of its own GOROOT mea…dmitshur closed an issue github: block merge button on github x/ repos1w
dmitshur commented on github: block merge button on github x/ repos1w
This is issue #59048. Issue #39055 is also related.dmitshur reviewed +2 on golang.org/x/mobile/asset: do not use 'assets' directory for iOS1w
Leaving a Commit-Queue+1 vote starts LUCI trybots.
It's documented at https://go.dev/wiki/LUCI#trybots.dmitshur reviewed +2 on golang.org/x/mobile/asset: do not use 'asset' directory for iOS1w
Thanks.
```
asset: do not use 'assets' directory for iOS
```
'assets'
Does this build under GOOS=ios? I expect a build error due to filepath package being unused.dmitshur deleted branch in rsc.io/github1w
masterdmitshur opened a change rsc.io/github/issue: various fixes1w
As reported by vet and staticcheck.dmitshur created branch in rsc.io/github1w
various-fixesdmitshur forked in rsc.io/github1w
Earlier
dmitshur fixed an issue runtime: unrecognized failures1w
The most recent failures here since this was reopened are all due to a core file being left behind, and the pattern isn't narrow enough to match anything else specific. So closing this in favor of #6…dmitshur closed an issue runtime: TestStackSwitchCallback failures1w
Merged into #69457.dmitshur commented on golang.org/x/vulndb/internal/symbols: TestPatchedSymbols failures1w
This can be reproduced locally by running the test in the x/vulndb module located in the module cache, rather than a git repository checkout. For example:
```
$ cd $(mktemp -d)
$ go mod init exa…dmitshur fixed an issue build: build failure on gotip-linux-amd64-newinliner1w
Based on issue #69457, the watchflakes change described in https://github.com/golang/go/issues/65977#issuecomment-1969407349 above is done. It sounds like there's nothing more to do here, and we can …dmitshur closed an issue runtime:mayMoreStackMove: unrecognized failures1w
Merged into #69457.dmitshur closed an issue build: build failure on x_tools-go1.23-windows-arm641w
Merged into #69664.dmitshur closed an issue make.bat: unrecognized failures1w
Merged into #69664.dmitshur closed an issue cmd/internal/testdir: Test/fixedbugs/issue20749.go failures1w
Merged into #69664.dmitshur commented on golang.org/x/build: OOM on windows-arm64 builder1w
Issue #33986 is relevant.
Issue #66962 may also be relevant.
dmitshur fixed an issue build: build failure on x_sys-gotip-linux-amd64-misccompile1w
Fixed in [CL 610940](https://go.dev/cl/610940).dmitshur commented on github.com/go-gl/glfw: GLFW v3.41w
Build constraints are documented at https://pkg.go.dev/cmd/go#hdr-Build_constraints.dmitshur commented on github.com/go-gl/glfw: GLFW v3.41w
Checking in on the status. @Geo25rey, would you like to keep pushing this forward? If so, there are some unresolved review comments that need to be addressed, otherwise this seems close. I've been us…dmitshur commented on golang.org/x/build/build/env: add rsync to some gomote images1w
(1 comment)
(And just `env:` as the prefix is enough, no need to include repo name in front. Not a big deal either way.)dmitshur reviewed +2 on golang.org/x/build/build/env: add rsync to some gomote images1w
Thanks.
(nit) Since this affects builders overall, not only gonotes:
```suggestion
build/env: add rsync to builder images for gomote
```
Here and below, please add a line explaining the motivation…dmitshur reviewed +2 on golang.org/x/build/internal/gomote: align golangbuild's and buildlet's work directory1w
Thanks.dmitshur fixed an issue cmd/go: TestScript/list_pkgconfig_error failures [consistent failure]1w
[CL 616336](https://go.dev/cl/616336) is submitted, nothing more to do here.dmitshur commented on golang.org/x/build/internal/task: replace reviewers parameter with release coordinators1w
(1 comment)
If it's okay to leave the issue unassigned if no coordinators are entered, then you can reuse `reviewersParam`, right? The only difference between `reviewersParam` and `releaseCoordinato…dmitshur reviewed +2 on golang.org/x/build/internal/gomote: extend gomote max timeout1w
Thanks.
has now?
Is it your intention to bump the timeout only when using golangbuild, and keep 24 hr when golangbuild isn't used?dmitshur commented on golang.org/x/build/cmd/coordinator/internal/lucipoll: update for new JSONPB field naming1w
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/gophers: change github user name1w
Thanks.
You could add this, to avoid future instances of meaningless multiple markers. There aren't any now.
```suggestion
if p.Bot {
panic(fmt.Errorf("*bot marker is set multiple times for …dmitshur commented on golang.org/x/build/cmd/watchflakes: fails to run after a build output property field gitilesCommit → gitiles_commit rename1w
The production instance isn't healthy yet:
```
$ kubectl get pods | grep watchflakes
watchflakes-deployment-7dd687d899-8rf75 0/1 CrashLoopBackOff 1876 (3m53s ago) 17d
```
@dr2chase …dmitshur commented on golang.org/x/build/cmd/golangbuild: potential send on closed channel during cleanup1w
Another instance reported at https://ci.chromium.org/b/8735636356859002161/infra, this time on windows/amd64. So this likely applies to all platforms rather than linux/ppc64le specifically.dmitshur commented on golang.org/x/build/internal/task: add release coordinator parameter2w
(1 comment)
Note that one of the fields of a Person is their GitHub username. See [here](https://cs.opensource.google/go/x/build/+/master:internal/gophers/gophers.go;l=18;drc=cf2ab8746449c0af3c48c37…dmitshur fixed an issue golang.org/x/build/cmd/golangbuild: also move or delete untested nested modules for run-tests-outside-repository check2w
Fixed in [crrev.com/c/5873674](https://crrev.com/c/5873674).
As a simple demonstration, [CL 615681](https://go.dev/cl/615681) contains a test that intentionally reaches across module boundary and …dmitshur opened a change golang.org/x/build/cmd/coordinator/internal/lucipoll: update for new JSONPB field naming2w
As of crrev.com/c/5875121, golangbuild switched to the new default
behavior of using 'proto' field naming (such as "gitiles_commit")
instead of using protobuf default names (such as "gitilesCommit").…dmitshur opened a change golang.org/x/mod/zip: add TestVoid2w
DO NOT REVIEW, DO NOT SUBMIT: This is a test.dmitshur reviewed +1 on golang.org/x/build/internal/task: create release tag and github release in vscode release2w
dmitshur reviewed +2 on golang.org/x/build/cmd/gomote: account for no-network builders2w
Hopefully changes/additions of behavior-changing properties like no_network are fairly rare, but it's still good to try to make this scale better. But I think this is pragmatic now.
Thanks.
My read…dmitshur commented on golang.org/x/build/cmd/watchflakes: look for both gitilesCommit and gitiles_commit2w
Moving myself to CC since this is already reviewed. Thanks.dmitshur commented on golang.org/x/build/cmd/watchflakes: fails to run after a build output property field gitilesCommit → gitiles_commit rename2w
One way to test it would be to run it locally; it should no longer run into the "repo mismatch" error.
You'll also know it works when you [deploy watchflakes with it](https://go.googlesource.com/bui…dmitshur reviewed +2 on golang.org/x/build/go.mod: update github.com/jackc/pgx/v4 to fix GHSA-mrww-27vc-gghv2w
Thanks.
Does 'go mod tidy' need to be run?dmitshur reviewed +2 on golang.org/x/build/internal/iapclient: handle authentication failures2w
Thanks.
Note that 'omitempty' option has effect when marshaling, no effect when unmarshaling.
In general it's slightly cleaner to disregard the `tokenRespErr` value if unmarshaling into it fails, b…dmitshur commented on golang.org/x/build/cmd/gomote: don't duplicate logic present in golangbuild in the repro subcommand2w
CC @golang/release.dmitshur reviewed +2 on cmd/dist: enforce the lowest bootstrap version2w
Thanks.
Done in PS 7.dmitshur opened an issue golang.org/x/build/cmd/watchflakes: fails to run after a build output property field gitilesCommit → gitiles_commit rename2w
Watchflakes is currently in a continuous crashloop:
```
$ kubectl logs -p watchflakes-deployment-7dd687d899-8rf75
watchflakes: 2024/09/24 12:12:43 ListBuilders
watchflakes: 2024/09/24 12:12:4…dmitshur reviewed +1 on golang.org/x/build/internal/task: add steps to build and publish package extension2w
(1 comment)
Thanks for clarifying this.dmitshur reviewed +2 on cmd/dist: enforce the lowest bootstrap version2w
Thanks.
It turns out that it might be more idiomatic to use 3-way Compare results by comparing with 0, instead of -1/1, so let's update this to be consistent with that practice:
```suggestion
if v…dmitshur reviewed +2 on golang.org/x/build/internal/task: add workflow to build internal security release branches2w
Thanks. Minor comments.
(minor) This looks like a sign that the `Version` field in `SecurityReleaseCoalesceTask` should be a pointer, so that dereferencing here isn't needed.
Probably better to use…dmitshur commented on golang.org/x/build/internal/task: add steps to build and publish package extension2w
(2 comments)
Being able to skip duplicate testing when you have assurance that it's already been done previously is great.
I don't entirely understand this change. If the constant string "s" is alw…