Activity
Yesterday
dmitshur reviewed +2 on golang.org/x/build/internal/task: reconcile public CLs before cherry-picking9h
Thanks.
Most of my concern was that the earlier implementation added complexity and made it harder to reason about the correctness of the behavior, with unclear long-term benefit. Now that in.Securi…dmitshur reviewed +2 on golang.org/x/build/main.star: replace linux-amd64_docker vscode-go builder with longtest9h
Thanks.dmitshur commented on golang.org/x/build/main.star: enable go test artifacts on main go repo builders10h
Patch set 2 already rebased.dmitshur reviewed +2 on golang.org/x/build/main.star: enable go test artifacts on main go repo builders10h
Thanks.
This likely needs to rebased and regenerated since a number of CLs have landed on luci-config branch since Oct 16.
Thanks, marking this as resolved.dmitshur commented on golang.org/x/build: recent Linux kernel builder10h
https://crrev.com/i/8198148 landed some time ago, and as far as I can tell it seemed to work okay? At least I didn't see too many false positive signals from it, but not sure how many true positives …dmitshur commented on cmd/compile: staticcheck still crashes with generic methods on Go 1.27.2 [1.27 backport]10h
Tentatively added release-blocker given the discussion in #82105 about wanting this fix to be made available sooner, and this seems close ([CL 847627](https://go.dev/cl/847627) was submitted on Oct 1…dmitshur commented on golang.org/x/net/html: quadratic complexity when coalescing text nodes10h
> I will re-assign this to the Go1.28 milestone.
@thatnealpatel This package isn't vendored in the main Go repo, and there doesn't seem to be anything tied to the Go 1.28 release here. Okay to move …dmitshur closed an issue encoding/json/v2: possibly clarify retention prohibitions in documentation [1.26 backport]13h
dmitshur reviewed +2 on cmd/dist: skip sync:cpu10 in compile-only mode13h
Thanks.dmitshur reviewed +2 on cmd/dist: simplify calculation of maxbg for tests13h
Thanks.
I agree it should be okay to drop the NumCPU check on top of GOMAXPROCS, and use GOMAXPROCS directly; it seems like it'd be a no-op in practice. There are a few builders that set that env va…dmitshur reviewed +2 on golang.org/x/build/env: add podman to trixie Dockerfile and add forky Dockerfile13h
Thanks. Minor optional comments.
The sid builder was added as a somewhat of an advisory builder, intended to give signal about problems that might start happening in future Linux versions in advance…dmitshur commented on golang.org/x/build/cmd/gopherbot: extract auto-submit for x/build into dedicated deployment14h
(1 comment)
That part was missed, thanks for catching. Should be done in PS 4, PTAL.dmitshur commented on golang.org/x/build/cmd/gopherbot: extract auto-submit for x/build into dedicated deployment15h
(1 comment)
> So overall each task will be running only once.
To be more precise, noting that this CL splits the auto-submit task into a separate deployment loop only for x/build for now (where we'…dmitshur commented on golang.org/x/build/cmd/gopherbot: extract auto-submit for x/build into dedicated deployment15h
(1 comment)
One instance will be running all tasks but auto-submit, and the second instance will be running auto-submit task only. So overall each task will be running only once. And auto-submit sho…dmitshur reviewed +2 on golang.org/x/build/main.star: add linux-amd64-longtest builder for vscode-go15h
Thanks.
They do show up on the main LUCI post-submit dashboard (https://ci.chromium.org/ui/p/golang, linked at https://go.dev/wiki/LUCI), if you scroll down to 'x/vscode-go (gotip)', but if it's exp…dmitshur commented on golang.org/x/build/cmd/gopherbot: extract auto-submit for x/build into dedicated deployment15h
This change is ready for review.dmitshur reviewed +2 on golang.org/x/build/main.star: bump up timeouts for some platforms15h
Thanks.
https://ci.chromium.org/ui/p/golang/builders/ci/gotip-darwin-amd64_13 and https://ci.chromium.org/ui/p/golang/builders/ci/gotip-darwin-amd64_14 seem to also be timing out fairly consistently…dmitshur reviewed +2 on golang.org/x/build/main.star: add a linux-amd64-screentest builder for pkgsite16h
dmitshur reviewed +2 on cmd/dist: clear GOWASM when using bootstrap toolchain16h
Thanks.
This section seems pretty similar in intent. Could clearing (and restoring) GOWASM be inserted into it, instead of a separate section somewhat far apart? Or is something about GOWASM differe…dmitshur opened an issue golang.org/x/build: try out CL submission via queue in golang.org/x/build Gerrit repo16h
We're building a prototype of CL submission via queue, initially limited in scope to cover the golang.org/x/build repository, and this is a tracking issue for that. See issue #12482 for motivation an…This Week
dmitshur commented on cmd/go: fix TestScript failures on AIX (aix/ppc64)1d
That one particular failure in cmd/cgo/internal/testcarchive.TestPreemption seems to be due to a pre-existing flaky test, not related to this change.dmitshur reviewed +2 on golang.org/x/build/main.star: allow network on vscode-go and pkgsite linux-amd64 builders1d
(1 comment)
This means the builders won't catch accidental uses of internet in short tests in those 2 repos, which seems unfortunate.
Can longtest builders be used instead? x/pkgsite already has on…dmitshur reviewed +2 on golang.org/x/build/main.star: add a known issue for the linux-amd64 vscode builders1d
dmitshur reviewed +2 on cmd/api: always run api checks when run locally1d
Thanks.
If this CL is a part of a larger whole, it'd be helpful to include at least one issue reference. In the future, someone may run into a need (or desire) to change how the cmd/api test works o…dmitshur commented on golang.org/x/build/main.star: add a linux-amd64-screentest builder for pkgsite1d
(1 comment)
If your plan is to eventually remove this 2x test timeout scaling, it seems like even more motivation not to apply it to builders that didn't already have it. But if it's going to be rem…dmitshur commented on cmd/api: always run api check test on common development platforms1d
(1 comment)
SGTM.dmitshur reviewed +2 on cmd/go: slice ~10 minutes off darwin race builders by adjusting GORACE=atexit_sleep_ms1d
Thanks.
You can give Michael some time to look too, if you prefer.
That issue was de-dup'ed with another. Since this section needs changing for the lines below, consider also:
```suggestion
Fixes …dmitshur reviewed +2 on cmd/internal/script: report timed out tests with better error than 'exit status 2'1d
Thanks.
These 3 lines seem incomplete (missing issue references, or can be deleted).dmitshur commented on cmd/dist: only check fips140 builds on the snapshots1d
(1 comment)
I see, thanks.dmitshur commented on cmd/api: always run api check test on common development platforms1d
(1 comment)
When running on a developer machine (i.e., os.Getenv("GO_BUILDER_NAME") == ""), this check was previously running on typical dev machines but it was still possible to run it explicitly o…dmitshur commented on cmd/dist: only check fips140 builds on the snapshots1d
(1 comment)
Is there a good enough reason for this import to deserve a standalone group? It doesn't seem all that special compared to other std lib imports, so it should be cleaner and simpler to ad…dmitshur reviewed +2 on cmd/relnote: always run relnote check1d
Thanks.dmitshur reviewed +2 on golang.org/x/build/main.star: add a linux-amd64-screentest builder for pkgsite1d
Thanks.
The change below (line 1882) checks for docker suffix or screentest in run_mods, but here it's more general. That means the 2x timeout scaling starts to apply to seemingly unrelated builders…dmitshur reviewed +2 on golang.org/x/build/main.star: add a linux-amd64 non-docker vscode-go builder1d
Thanks.dmitshur reviewed +2 on golang.org/x/build/cmd/xb: update to be able to push with podman1d
Thanks.
helper's stdout seems to be left sent to DevNull. Is it that its content isn't expected to be relevant even on error?dmitshur reviewed +2 on cmd/internal/fuzztest: limit GOMAXPROCS for 386 subtests1d
Thanks.
Is it worth adding a check for when `runtime.GOMAXPROCS(0) > 2`, so that if GOMAXPROCS is already or 2 lower it doesn't get raised? Or is it better to potentially raise it too (and update ph…dmitshur commented on cmd/go/internal/modfetch: build with a Go 1.26 bootstrap on Plan 91d
(1 comment)
Suggestion to make it more clear this change is fixing a build error, rather than causing something building to happen with a Go 1.26 bootstrap.
```suggestion
cmd/go/internal/modfetch: …Last Week
dmitshur reviewed +2 on golang.org/x/build/main.star: add a goamd64v4 builder4d
Thanks.dmitshur reviewed +2 on golang.org/x/build/main.star: add noavx512 builders running on n2d-standard-16 machines4d
Thanks.dmitshur reviewed +2 on cmd/dist: drop sharding for testdir tests4d
Thanks.
This variant name is the same as the package name, which doesn't convey much information about how this particular test variant differs from the nominal "go test cmd/internal/testdir" invoca…dmitshur commented on cmd/dist: run testdir tests in initial go test run4d
(1 comment)
(Sorry, I meant to post this comment in CL 853365 but accidentally posted it the wrong CL. Feel free to apply it anywhere, or even better all this "drop unused sharding" functionality fe…dmitshur commented on cmd/dist: run testdir tests in initial go test run4d
(1 comment)
Remove these since you're removing the only reason these still exist?dmitshur commented on cmd/dist: run testdir tests in initial go test run4d
(1 comment)
Oh, I see how I misread it - this continues to skip this test on mobile platforms; the reason this special registerTest call is still left behind here is for Wasm targets. That's unfortu…dmitshur commented on cmd/dist: run testdir tests in initial go test run4d
(2 comments)
Consider upstreaming this skip into cmd/internal/testdir itself by adding a `//go:build !race` constraint (similar to [here](https://cs.opensource.google/go/go/+/master:src/sync/pool_te…dmitshur reviewed +2 on cmd/dist: consolidate maxbg logic4d
dmitshur commented on cmd/dist: drop sharding for testdir tests4d
(5 comments)
This CL is a bit hard to review because it removes partial support for sharding cmd/internal/testdir and leaves some unneeded parts of the mechanism behind. It'd be easier to review the…dmitshur reviewed +2 on cmd/dist: consolidate maxbg logic4d
LGTM % remove accidental binary. Thanks.dmitshur commented on cmd/dist: consolidate maxbg logic4d
(1 comment)
Please do not check in an unneeded 4.6 MB binary file. Also added a Hold+1 for that.dmitshur reviewed +2 on golang.org/x/build/env: install podman on Debian 12 images4d
Thanks.dmitshur reviewed +2 on golang.org/x/build/env: allow using podman instead of docker to build images4d
Thanks.
Have you also considered migrating to using Cloud Build to build images? That should be more predictable and requires less local tools installed; we already switched to using Cloud Build for…dmitshur reviewed +2 on golang.org/x/build/main.star: remove freebsd 15.0 builder from known issue list4d
Thanks.dmitshur reviewed +2 on golang.org/x/build/main.star: remove avx512 builders4d
Thanks.dmitshur reviewed +2 on golang.org/x/build/Revert "main.star: wait for capacity for docker instances"4d
Thanks.
```suggestion
This reverts CL 852985.
```
(As suggested at https://go.dev/wiki/CommitMessage#reverts.)dmitshur commented on golang.org/x/build/main.star: wait for capacity for docker instances5d
(1 comment)
Also, expiration_timeout for linux-amd64_docker can probably stay at 6 hours (for the same reason it's 6 hrs for all other builders on GCE). But it's not a big deal either way in the sho…dmitshur reviewed +2 on golang.org/x/build/main.star: wait for capacity for docker instances5d
Thanks.
To comment to this part of the commit message:
> (I'm not sure why we don't always wait for capacity in case the builder isn't available).
This part of the comment motivates why.
If we fi…dmitshur commented on golang.org/x/build: upgrade freebsd builders to 14.5 and 15.15d
CC @golang/freebsd, @golang/release.dmitshur commented on golang.org/x/build: add OpenBSD 8.0 builder5d
CC @golang/openbsd, @golang/release.dmitshur fixed an issue golang.org/x/build/internal/installer/darwinpkg: unclear purpose and effect of "Fixing permissions" step during postinstall1w
The "Fixing permissions" step removal in Go 1.27 didn't seem to uncover anything unexpected, so closing this as done.
<details><summary>Confirmation of removal details</summary>
```shell
$ pkgutil …dmitshur fixed an issue golang.org/x/build/internal/installer/darwinpkg: unclear purpose and effect of "Fixing permissions" step during postinstall1w
The "Fixing permissions" step removal in Go 1.27 didn't seem to uncover anything unexpected, so closing this as done.
<details><summary>Confirmation of removal details</summary>
```shell
$ pkgutil …dmitshur commented on golang.org/x/build: add GOAMD64=v3 perf builders1w
There were GOAMD64=v3 perf builders added above, and they're running. It's also added to the perf dashboard.
@matloob Should this be closed as done or was there something specific still left to do h…dmitshur pushed to
master in github.com/mdempsky/unconvert1wdmitshur merged a change github.com/mdempsky/unconvert/...: update go and dependency versions- #701w
dmitshur reviewed +2 on github.com/mdempsky/unconvert/...: update go and dependency versions- #701w
Thanks.
Note that in the general case, it's not necessary to include the -u flag in the "go get" invocation in order to update dependencies. The -u flag means to update indirect dependencies (depend…dmitshur fixed an issue golang.org/x/build: linux-amd64-race is taking about 5 minutes too long for pre-submit purposes1w
dmitshur commented on golang.org/x/build: linux-amd64-race is taking about 5 minutes too long for pre-submit purposes1w
linux-amd64-race trybots are now consistently averaging 5-6 minute completion time (with around 7 minutes in the worst case) after @matloob's work in #82017. That's well within 15 minutes. Calling th…Earlier
dmitshur commented on cmd/compile: ICE "Int64Val(uint64(9223372036854775808))" when ranging over an unsigned constant ≥ 2^63 in a clear loop (`for i := range uint64(1<<63) { s[i] = 0 }`)1w
CC @golang/compiler.dmitshur opened an issue golang.org/x/build: frequent builder infra failures, including linux/amd64 and linux/arm64, on 2026-10-151w
We're aware of an issue most visible on the [Go tip post-submit dashboard](https://ci.chromium.org/p/golang/g/go-gotip/console), where many of the linux/amd64 and linux/arm64 builds are frequently ru…dmitshur commented on golang.org/x/build/cmd/golangbuild: add support for test artifacts in LUCI1w
The latest status here is that functionality is enabled in x/pkgsite, for docker builder. What's left is enabling it for the remaining Go repositories and probably posting something in golang-dev so …dmitshur starred github.com/qmuntal/gobuilder1w
dmitshur commented on cmd/link/internal/ppc64: genpltstub panics instead of reporting a link error when the caller symbol has no data2w
CC @golang/ppc64.dmitshur commented on dmitri.shuralyov.com/website/gido: Support proposals1mo
Proposals are already supported, in the sense they're listed as issues at https://goissues.org/ with a "Proposal" label.
See [prefixtitle.ParseIssue](https://pkg.go.dev/dmitri.shuralyov.com/go/prefi…dmitshur pushed to
main in dmitri.shuralyov.com/service/change1ydmitshur commented on dmitri.shuralyov.com/blog: Trying out the Oculus Rift CV1 for the first time2y
July 4, 2024. Today I got a chance to try out the Apple Vision Pro. :)dmitshur pushed to
main in dmitri.shuralyov.com/scratch2ydmitshur created package in dmitri.shuralyov.com/scratch/quine2y
dmitshur merged a change dmitri.shuralyov.com/scratch: quine: add package quine2y
dmitshur reviewed +2 on dmitri.shuralyov.com/scratch: quine: add package quine2y
LGTM. Thanks, and congrats on your first new Go package here!
Done in PS 2.
Patch set 2 seems to resolve this.dmitshur merged a change dmitri.shuralyov.com/scratch: add AUTHORS.md file2y
dmitshur reviewed +2 on dmitri.shuralyov.com/scratch: add AUTHORS.md file2y
Thanks! If it's ready to be submitted from your side, I can do that now.dmitshur pushed to
main in dmitri.shuralyov.com/html/belt2ydmitshur commented on dmitri.shuralyov.com/scratch: main: add package quine2y
They are. Although Go can make them too easy via `go:embed`.
See some initial comments.
I suggest either adding a quite that can be licensed via the existing BSD-3-Clause LICENSE at the top level o…dmitshur reviewed +2 on dmitri.shuralyov.com/scratch: add AUTHORS.md file2y
Thanks! Congrats on your first change. :D
See one inline comment that's unresolved (yellow border). You can leave a draft comment like "Done." and mark the 'Resolved' checkbox, then when you upload …dmitshur pushed to
main in dmitri.shuralyov.com/go/generated3ydmitshur pushed to
main in dmitri.shuralyov.com/gpu/mtl3ydmitshur pushed to
main in dmitri.shuralyov.com/gpu/mtl3ydmitshur commented on dmitri.shuralyov.com/idiomatic-go: Use defer if its overhead is relatively negligible, or it buys meaningful readability4y
I agree with the points made above. This entry tried to say "use this language feature when it's appropriate; don't use it when it's not", which can be said for any language feature. I'll close this …dmitshur pushed to
master in dmitri.shuralyov.com/service/change4ydmitshur closed an issue dmitri.shuralyov.com/go/generated: update implementation for 2020 spec changes4y
dmitshur merged a change dmitri.shuralyov.com/go/generated: update implementation for 2020 spec changes4y
dmitshur commented on dmitri.shuralyov.com/go/generated: update implementation for 2020 spec changes4y
Go fuzzing support is out of beta and no longer uses this build constraint. It should be replaced with `go1.18` by now.
Done in PS 3.
I'm following the Go style for commit messages, which chooses…dmitshur commented on dmitri.shuralyov.com/go/generated: update implementation for 2020 spec changes4y
Shortened 'golang.org' to 'go.dev' here, and on line 24.dmitshur pushed to
master in dmitri.shuralyov.com/website/gido5ydmitshur opened an issue dmitri.shuralyov.com/go/generated: update implementation for 2020 spec changes5y
dmitshur pushed to
master in dmitri.shuralyov.com/gpu/mtl5ydmitshur pushed to
master in dmitri.shuralyov.com/website/gido6ydmitshur pushed to
master in dmitri.shuralyov.com/website/gido6ydmitshur pushed to
master in dmitri.shuralyov.com/html/belt6ydmitshur closed an issue github.com/shurcooL/issuesapp: Improve use of http.Error.6y