Activity
Today
dmitshur commented on runtime: native SEH unwinding faults in mcall on windows/amd64 since Go 1.2510m
CC @golang/runtime, @golang/windows.dmitshur commented on golang.org/x/pkgsite: package removal request for github.com/sebastienrousseau/scout, scout-mcp, scout-reporting, scout-action10m
CC @golang/pkgsite.dmitshur commented on encoding/xml: Marshal and EncodeToken emit ill-formed element/attribute names with nil error11m
CC @rsc.dmitshur commented on go/types: types.Error.go116end end position derived from placeholder Ident.Name is garbage12m
CC @griesemer, @mrkfrmn.dmitshur commented on go/types: Interface.Method and ExplicitMethod are not ordered by Id, contra docs13m
CC @griesemer, @mrkfrmn.dmitshur commented on net/http: FileServer issues incorrect redirect to regular file given multiple trailing slashes14m
CC @neild, @nicholashusin.dmitshur closed an issue : Add minia2a — x402 marketplace for agent monetization via API16m
dmitshur commented on golang.org/x/build/main.star: add an experimental n4d go repo postsubmit builder1h
(2 comments)
> If we can use 12 for the default builders I think that would be better.
We're already using 12 for linux/arm64 (CL 622318), but still 11 for linux/amd64. Changing the version comes w…dmitshur commented on golang.org/x/build: evaluate switching default builders to a newer architecture1h
CC @golang/release.dmitshur reviewed +2 on golang.org/x/build/internal/relui: make poll-wait for submit less spammy1h
Thanks.
(nit) Given a not very self-descriptive type like `chan string`, 'ch' is not a very descriptive variable name for it. Since it's being used to log notable state changes, consider naming it s…dmitshur commented on security: fix CVE-2026-568612h
Removing release-blocker for clarity. Since this issue was moved to the Unreleased milestone, that label has no effect. It can be re-added if this needs to block a specific release.dmitshur commented on net/http: Transport reuses cached unencrypted HTTP/2 connection for https:// request2h
Removing release-blocker for clarity. Since this issue was moved to the Unreleased milestone, that label has no effect. It can be re-added if this needs to block a specific release.dmitshur commented on cmd/go: go vet -fix does not apply fixes in std packages, and -tags turns unsafeptr back on2h
CC @adonovan, @matloob.dmitshur reviewed +2 on golang.org/x/build/internal/relui: transition WIP CLs to Ready3h
Thanks.
https://go.dev/doc/comment suggests "Every exported (capitalized) name should have a doc comment."
I wonder if it could work out slightly better to do the mark ready step as soon as the con…dmitshur reviewed +2 on golang.org/x/build/gerrit: fix incorrect return schema for SetCommitMessage4h
Thanks.
It's kinda surprising to see that given it's [not documented](https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#set-message) in the Gerrit API REST docs, and [this](…dmitshur reviewed +2 on golang.org/x/crypto/acme: prefer test log helpers in test contexts4h
Thanks.dmitshur reviewed +2 on golang.org/x/build/cmd/watchflakes: don't require credentials when debugging one build or script5h
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: do not fail on cherry-pick merge conflicts6h
Thanks.
What is the effect and motivation of having a sleep here and on line 961 (above)? They're in the same goroutine, so as far as I can tell they have no effect inside synctest.Test. If they hav…dmitshur reviewed +2 on golang.org/x/build/internal/task: add observability to MoveAndRebase6h
Thanks.
By this line, ci has been assigned to movedCI. Will its Branch field be the new branch by then? If so, perhaps something like:
```suggestion
prevBranch := ci.Branch
ci = &movedCI
ctx.…Last Week
dmitshur commented on golang.org/x/build/relmeta: add Symbols to SecurityPatch3d
Another tool to consider is cmp.Diff, since its output on failure can be easier to read.dmitshur reviewed +2 on golang.org/x/build/relmeta: add Symbols to SecurityPatch3d
Thanks.
I'll note that it seems a bit unexpected that DeploymentMap would have this effect on Symbols, where an empty map causes p.Symbols to be used as is, whereas a non-empty map causes some filte…dmitshur reviewed +2 on golang.org/x/build/relmeta: add escape hatch for manual deployment3d
dmitshur commented on image/draw: speed up finding the closest palette color3d
The net/http.TestTransportMaxConnsPerHostDialCancellation/h1 failure is an unrelated flaky test (tracked in issue #69182), and everything else passed.dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27.md: add two late release notes3d
Thanks.dmitshur fixed an issue golang.org/x/build: migrate low-capacity (reverse) builders to LUCI4d
This migration is done, closing out this umbrella issue. Any other builder-specific work can be tracked in individual issues.dmitshur opened an issue golang.org/x/build: add darwin/arm64 builders with macOS 27 Golden Gate4d
This is a tracking issue for adding darwin/arm64 builders with macOS 27 Golden Gate. (There's no need for amd64 ones since macOS 27 drops support for amd64.) Issue #76798 was for previous year's vers…dmitshur commented on golang.org/x/vgo/Automated: Migrate go/vgo from gsutil to gcloud storage5d
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 deployment5d
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 …dmitshur reviewed +2 on golang.org/x/build/internal/relui: factor upstreaming private-track CLs tasks into methods5d
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 privXMilestoneParameter5d
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 pointer5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: remove indirections where possible5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: lift CL conversion in ReleaseMilestone5d
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 creation5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: lift await for disclosed security patches5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: lift public branch head guards5d
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 patches5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/task: lift linting of security patches5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: lift API for applying security riders5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: dilineate between allowed riders5d
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 atomically5d
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 patches5d
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.go5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: remove redundant no-milestone-approval gate5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: add runMinorReleaseToFailure helper5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: fold publicize test fixtures into helper5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/workflowtest: fold helpers for workflow Start/Run5d
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 helper5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: refactor testing to use synctest5d
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 fixtures5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/workflowtest: fold remaining test fixtures5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/task: remove TCP from test fakes5d
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 everywhere5d
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 listener5d
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 TestAwaitFunc5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal: honor errors returned by workflow5d
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 fixtures5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/workflowtest: add package and fold in abstractions5d
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 programatically5d
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 manifest5d
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 SecurityPatch5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/internal/relui: update GitHub issues post-disclosure5d
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 blockers5d
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 formatting5d
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.yaml6d
dmitshur closed an issue net/http: Request.TLS is not properly populated for HTTP/2 requests [1.26 backport]1w
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]1w
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 builder1w
(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…Earlier
dmitshur reviewed +2 on golang.org/x/perf/benchstat: add godoc links from legacy benchstat to the new one1w
Thanks.dmitshur reviewed +2 on golang.org/x/build/...: migrate ssh package to github.com/tailscale/gliderssh1w
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/gcm1w
@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…dmitshur commented on cmd/go: don't write to GOROOT in TestBoringInternalLink2w
This change is ready for review.dmitshur opened a change cmd/go: don't write to GOROOT in TestBoringInternalLink2w
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 LUCI2w
This change is ready for review.dmitshur opened a change test/fixedbugs: don't write to GOROOT in some tests2w
Use a temporary directory instead. GOROOT may not be writable.dmitshur opened a change cmd/dist: update Linux builder detection for LUCI2w
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 stages2w
Thanks.dmitshur reviewed +2 on golang.org/x/build/main.star: email pkgsite maintainers on Docker postsubmit failures2w
Thanks.dmitshur reviewed +2 on golang.org/x/build/main.star: enable go test artifacts on the x/pkgsite Docker builders2w
Thanks.dmitshur reviewed +1 on golang.org/x/pkgsite/integration_test.go: run the container CI stages from go test2w
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)2w
(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)2w
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 LUCI2w
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 proposals3w
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