Activity
Today
dmitshur commented on golang.org/x/vgo/Automated: Migrate go/vgo from gsutil to gcloud storage3h
Thanks for preparing this change. As noted in the [README](https://go.googlesource.com/vgo#obsolete), this repository is an archive, preserved for historical interest only. It's not actively used nor…dmitshur reviewed +2 on golang.org/x/build/relmeta: add escape hatch for manual deployment3h
Thanks.
I see a lot of uses of DeploymentMap in this CL, but I'm having a hard time finding any one place that gives it a clear description. Given it's a map of A to B, what do A and B represent? I …Yesterday
dmitshur reviewed +2 on golang.org/x/build/internal/relui: factor upstreaming private-track CLs tasks into methods22h
Thanks.
Noting that this whole "Confirm PRIVATE-track security CLs" step was meant to be temporarily guard against human release coordinators accidentally pasting in the wrong ref until the metadata…dmitshur reviewed +2 on golang.org/x/build/internal/relui: split off privXMilestoneParameter22h
Yep, relui parameter definitions are basically free so it's good to specialize them to be more precise when applicable (such as here). Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: return milestones by pointer22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: remove indirections where possible22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: lift CL conversion in ReleaseMilestone22h
Thanks.
This is a general comment. I see a pattern across various CLs to convert some method-based tasks into function-based tasks and provide dependencies to them as parameters (wf.Const since they…dmitshur reviewed +2 on golang.org/x/build/internal/relui: lift checkpoint branch creation22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: lift await for disclosed security patches22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: lift public branch head guards22h
I wished (for purposes of readability of the CL stack) this was a part of CL 826666 rather than happening later, but maybe these couldn't be moved earlier until some of the refactors in between.
Tha…dmitshur reviewed +2 on golang.org/x/build/internal/relui: lift submission of security patches22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/task: lift linting of security patches22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: lift API for applying security riders22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: dilineate between allowed riders22h
This is a bit unfortunate because it sets a slightly confusing precedent, and one would expect this to be something that's easier to do programmatically than to expect humans to do it. But I think it…dmitshur reviewed +2 on golang.org/x/build/internal/task: disclose x-repo patches atomically22h
Thanks.
The '%' is needed before the first [push option](https://gerrit-review.googlesource.com/Documentation/user-upload.html#push_options), and ',' separates multiple options. So I think as writte…dmitshur reviewed +2 on golang.org/x/build/internal/task: lift atomic disclosure of security patches22h
Thanks.
I think fairly modern versions of `git` support fetching a revision directly via a `--revision` flag, so it might be viable to completely remove the need for callers to specify a ref. But th…dmitshur reviewed +2 on golang.org/x/build/internal/task: wire 'Wait to disclose' correct in privx.go22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: remove redundant no-milestone-approval gate22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: add runMinorReleaseToFailure helper22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: fold publicize test fixtures into helper22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/workflowtest: fold helpers for workflow Start/Run22h
Thanks.
Please document that a nil `listener` means to use a basic verbose listener, so that callers don't need to read implementation details of this helper to be able to rely on it.dmitshur reviewed +2 on golang.org/x/build/internal/relui: fold security setup into a test helper22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: refactor testing to use synctest22h
Very nice to see this. Thanks.
(I guess we can't do this for any tests that depend on a real database dependency, but those are a small subset; most release-related tests just use the workflow packa…dmitshur commented on golang.org/x/build/internal/workflowtest: fold remaining test fixtures22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/workflowtest: fold remaining test fixtures22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/task: remove TCP from test fakes22h
Thanks.
Note that the canonical source of this package is in x/website; this is an x/build copy.
The API change below is fine to make, but please also send it to x/website to keep them in sync and …dmitshur reviewed +2 on golang.org/x/build/internal/relui: refactor to use http.Client everywhere22h
Thanks.
While looking at this line (that this CL happens to touch), it's noticeable that this too can eventually be simplified by taking advantage of either `http.FileServerFS` or `http.Dir`.
Pleas…dmitshur reviewed +2 on golang.org/x/build/internal/task: switch FakeGerrit to use in-memory listener22h
Thanks.
We're deploying relui [with Go 1.27](https://cs.opensource.google/go/x/build/+/master:cmd/relui/Dockerfile;l=5;drc=de126992084793ed2efd31a4babdf01684b03c10), and if it's really worthwhile to…dmitshur reviewed +2 on golang.org/x/build/internal/relui: use synctest in TestAwaitFunc22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal: honor errors returned by workflow22h
Thanks.
I'm not sure how easy it'll be for a person seeing this to tell what "handler" refers to. Maybe mention OnStall more explicitly.dmitshur reviewed +2 on golang.org/x/build/internal/relui: fold remaining test fixtures22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/workflowtest: add package and fold in abstractions22h
This seems fine. I expect there will still be some instances where it's easier to create a dedicated testing helper, so not absolutely everything has to be de-duplicated and made general purpose, but…dmitshur reviewed +2 on golang.org/x/build/...: apply security patch riders programatically22h
Thanks.
Nice to see this.
As a side note, this is the first time I'm seeing the term "riders". The closest I knew of was git commit message footers/trailers, but I guess "riders" refer to the "Fixe…dmitshur reviewed +2 on golang.org/x/build/internal/relui: build backport issue manifest22h
Thanks.
Should this return a non-nil error if the issue doesn't exist? Similarly to GetIssue above.
```
if !ok {
return nil, fmt.Errorf("the issue %v does not exist", number)
}
```dmitshur reviewed +2 on golang.org/x/build/relmeta: remove VulnReport field from SecurityPatch22h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: update GitHub issues post-disclosure22h
Thanks.
It might help to note which type is the input to this template. For example, that'd make it easier to get to the definition of "{{.ID}}" and look up its [meaning](https://pkg.go.dev/golang.o…dmitshur reviewed +2 on golang.org/x/build/internal/relui: enforce 1:1 map of security patches to release blockers22h
Thanks.
Is it viable (and helpful) to have CheckSecurityIssues also check that when rm is nil, there are exactly 0 security issues?dmitshur reviewed +2 on golang.org/x/build/internal/task: fix x-repo announce formatting22h
Thanks.
Note that it can generally be useful to document fields for templates, and maybe include example values. Compare with above and below.
Not having any docs means that readers can only derive…dmitshur closed an issue simd: midway panics with incorrect comments.yaml1d
This Week
dmitshur closed an issue net/http: Request.TLS is not properly populated for HTTP/2 requests [1.26 backport]2d
This bug doesn't exist in Go 1.26. This backport issue was opened only because "1.26" was mentioned; closing.dmitshur closed an issue net/http: Request.TLS is not properly populated for HTTP/2 requests [1.26 backport]2d
This bug doesn't exist in Go 1.26. This backport issue was opened only because "1.26" was mentioned; closing.dmitshur commented on golang.org/x/build/main.star: add an experimental n4d go repo postsubmit builder2d
(4 comments)
For golang/go#nnn? It'd be good to have some issue to track the state of this experimental builder.
See my other inline comment; just pointing out here that this is where you can see t…Last Week
dmitshur reviewed +2 on golang.org/x/perf/benchstat: add godoc links from legacy benchstat to the new one4d
Thanks.dmitshur reviewed +2 on golang.org/x/build/...: migrate ssh package to github.com/tailscale/gliderssh5d
Thanks.
https://go.dev/blog/go1.13-errors#whether-to-wrap suggests:
> Wrap an error to expose it to callers. Do not wrap an error when doing so would expose implementation details.
If it's not nec…dmitshur commented on crypto/internal/boring: avoid pulling in .../fips140/aes/gcm6d
@filippo@golang.org Note that 5 crypto/rand:gofips140 tests without -short flag [seem to fail](https://ci.chromium.org/b/8669790779333004049/test-results) as of this CL.dmitshur opened a change cmd/dist: revert "update Linux builder detection for LUCI"1w
This reverts CL 834945 (commit 936cd9c3e8585b3bb1ef0ba83488bfe78c9bf61c).
Reason for revert: Breaks TestAllDependencies test on longtest builders.
That test will also need to be updated to not write…dmitshur opened an issue golang.org/x/build/cmd/watchflakes: report broken bot beyond first occurrence1w
Issue #68790 added functionality to watchflakes to watch and report broken bots. A known limitation is that it's willing to report a given unique builder at most once, but it neither reopens¹ a prev…dmitshur commented on golang.org/x/build: bot linux-arm-aws--dev-3e01 reported as broken1w
There was another instance of 20 consecutive failures (due to the make.bash problem between [CL 830064](https://go.dev/cl/830064) and [CL 831204](https://go.dev/cl/831204)) placing the bot into quara…dmitshur reviewed +2 on cmd/dist: add a --debug-trace flag to dist bootstrap1w
Thanks.
Thanks for finding the context of where it was added.
Since it has no current users, and the newer functionality seems to superseded it, I think this CL is a good prompt to eventually drop …dmitshur commented on golang.org/x/review/git-codereview: skip tests that need gofmt when it is not in PATH1w
(1 comment)
What do you think of naming this `mustHaveGofmt`, similarly to the pattern used in the [internal/testenv](https://go.dev/pkg/internal/testenv) package and [`mustHaveWorktree`](https://cs…Earlier
dmitshur commented on cmd/go: don't write to GOROOT in TestBoringInternalLink1w
This change is ready for review.dmitshur opened a change cmd/go: don't write to GOROOT in TestBoringInternalLink1w
Set -o=/dev/null when building a test command so that GOROOT isn't
written to. GOROOT may not be writable.dmitshur commented on cmd/dist: update Linux builder detection for LUCI1w
This change is ready for review.dmitshur opened a change test/fixedbugs: don't write to GOROOT in some tests1w
Use a temporary directory instead. GOROOT may not be writable.dmitshur opened a change cmd/dist: update Linux builder detection for LUCI1w
LUCI builder names encode the release branch in front,
such as "gotip-linux-amd64" or "go1.27-linux-amd64".dmitshur reviewed +2 on golang.org/x/pkgsite/integration_test.go: set CI=true for the npm stages1w
Thanks.dmitshur reviewed +2 on golang.org/x/build/main.star: email pkgsite maintainers on Docker postsubmit failures1w
Thanks.dmitshur reviewed +2 on golang.org/x/build/main.star: enable go test artifacts on the x/pkgsite Docker builders1w
Thanks.dmitshur reviewed +1 on golang.org/x/pkgsite/integration_test.go: run the container CI stages from go test1w
Thanks. This looks good to me, but I'll give Jonathan a chance to also look.dmitshur reviewed +2 on golang.org/x/website/website/tour: display notfound.html on bad tour page (404)1w
(1 comment)
Makes sense and sounds good, thanks!dmitshur reviewed +2 on golang.org/x/website/website/tour: display notfound.html on bad tour page (404)1w
Thanks.
```suggestion
website/tour: display notfound.html on bad tour page (404)
```
It might be slightly easier to reason about the logic if this variable is always defined, and its value is set t…dmitshur commented on golang.org/x/build/cmd/golangbuild: add support for test artifacts in LUCI1w
CC @ethanalee-work.dmitshur commented on golang.org/x/tools/go/analysis/passes/modernize: errorsastype rewrites errors.As target interface that does not implement error [1.26 backport]2w
This cherry pick has already been approved in #81389.dmitshur commented on golang.org/x/tools/go/analysis/passes/modernize: errorsastype rewrites errors.As target interface that does not implement error [1.27 backport]2w
@madelinekalil Does this bug apply to 1.26 and 1.27, or 1.27 only? Asking given our policy to support [both Go releases equally](https://go.dev/issue/34536#issuecomment-572188389).dmitshur closed an issue os: File sporadic waits upon non-blocking raw connect2w
dmitshur commented on dmitri.shuralyov.com/website/gido: Support proposals2w
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 commented on build: build failure on x_pkgsite-gotip-linux-amd64-misccompile2w
Yes, it it failed while fetching dependencies because a GET request to the module mirror returned 502 Bad Gateway.dmitshur commented on golang.org/x/website: tour: add maintained Chinese, French, German, and Korean translations2w
Is https://github.com/shuijingwan/go-tour-i18n/issues the right issue tracker where reports related to the translated tours should be filed? If so, please update the "submit feedback" button in the t…dmitshur closed an issue golang.org/x/build: Microsoft store release2w
dmitshur closed an issue encoding/json: Marshal now honors MarshalText for string-kind map keys [1.26 backport]3w
dmitshur commented on testing: TestArtifactDirectoryPaths fails in GOPATH mode (no module context)3w
It may be helpful to hear back on the question above, but I'll note that testing standard library packages with GO111MODULE=off is documented as not something that's expected to work - see `go help …dmitshur opened an issue cmd/go: go test std should give a clearer error with GO111MODULE=off3w
### Go version
go version go1.27.1 darwin/arm64
### What did you do?
Prior to filing this issue, I forgot that it was already decided and documented that testing the Go standard library with GO111…dmitshur opened an issue golang.org/x/build/internal/gcsfs: handle explicit GCS folders3w
When working with GCS, one can end up with folders that are either implicit (aka "simulated"?) or explicit (aka "zero-byte object whose name ends with a trailing / delimiter"). Currently the gcsfs.Ne…dmitshur commented on golang.org/x/build/cmd/relui: unexplained instance of a workflow stall3w
In addition to the above, perhaps we could enable the race detector and see if that uncovers something we're not seeing locally? [CL 827824](https://go.dev/cl/827824).dmitshur commented on access: qiulaidongfeng approvers3w
CC @golang/release.Bisected the failure to [CL 814681](https://go.dev/cl/814681), which updated cloud.google.com/go/storage from v1.31.0 to v1.59.2. Bisected that further to v1.50.0 as the last version that works while…dmitshur opened an issue golang.org/x/build/internal/gcsfs: walking a filesystem stopped working, produces errors like "doesn't exist: googleapi: Error 404: No such object: golang-release-staging/relui-scratch/build-outputs/script-build-8525853265259187891, notFound"3w
On August 27, we noticed a case where `buildToOutputs` panicked during a relui workflow execution. The panic revealed a missing error check, which [CL 823324](https://go.dev/cl/823324) has since fixe…dmitshur opened an issue golang.org/x/build/cmd/relui: unexplained instance of a workflow stall4w
During one of the workflow runs for Go minor releases on August 11, 2026, we ran into an unexplained relui behavior:
> [...] something up with relui - some of its tasks have appeared to stall, not m…dmitshur starred github.com/go-git/go-git/v64w
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…