Activity
Today
dmitshur reviewed +2 on golang.org/x/build/internal/task: upgrade dependencies after vscode-go insider minor43m
Thanks.
Optionally, might be more readable to use a named variable:
```suggestion
if minor := release.Patch == 0; !minor || !isVSCodeGoInsiderVersion(release, "") {
```
I see other similar code d…dmitshur reviewed +2 on golang.org/x/vscode-go/tools: run go fix1h
Thanks.dmitshur reviewed +2 on golang.org/x/vscode-go/go.mod: update dependencies following the v0.57.0 release2h
Thanks.
Noting that this zero version that was here previously made the replacement directive highly load-bearing; the new version makes it more optional.dmitshur reviewed +2 on golang.org/x/vscode-go/...: upgrade go directive to at least 1.25.02h
Thanks.
In general, be mindful that going from 1.21 to 1.25 will include a fair number of changes, including the scope of variables in for loops (https://go.dev/doc/go1.22#language) and others at ht…dmitshur reviewed +2 on golang.org/x/build/internal/task: add vscode-go to the go directive update2h
Thanks.
Not seeing this change.
Also, what does the 'X' in `TestSelectGoDirectiveXReposLive` refer to? Previously the 'X' in `TestSelectGolangOrgXReposLive` was a part of "golang.org/x". Now that i…dmitshur commented on golang.org/x/build/internal/task: upgrade dependencies after vscode-go insider minor4h
(1 comment)
I see, so dropping -u and switching to `work` at once indeed doesn't work - `go get work` ends up _not_ doing `go get directdependency@upgrade` at all. Consider keeping `all` and only dr…dmitshur commented on net/url: quadratic complexity in resolvePath when processing backtrack path elements [1.26 backport]4h
CC @golang/release.dmitshur commented on net/url: quadratic complexity in resolvePath when processing backtrack path elements [1.25 backport]4h
CC @golang/release.dmitshur commented on golang.org/x/build/internal/task: add vscode-go to the go directive update7h
(7 comments)
https://go.dev/doc/contribute#commit_messages suggests "Write in complete sentences with correct punctuation, just like for your comments in Go." Perhaps:
```suggestion
This will help …Yesterday
dmitshur reopened an issue net/url: quadratic complexity in resolvePath when processing backtrack path elements18h
Reopening and moving to Go 1.27 milestone to track [cherry-picking](https://groups.google.com/g/golang-dev/c/jMWTCiV0aSk/m/Qo6ozI0dBQAJ) [CL 807700](https://go.dev/cl/807700) onto release-branch.go1.…dmitshur commented on go/types: avoid infinitely recursive instantiation through method signatures18h
@mrkfrmn This issue is in Go1.18 milestone, and it's also frozen/locked due to age. Maybe it's better to file a new one and reference this old issue? (Otherwise we should move it to a future mileston…dmitshur fixed an issue encoding/xml: (*Decoder).DecodeElement bypasses recursion-depth guard18h
Closed by merging [CL 807680](https://go.dev/cl/807680) (commit d5eeaa7de337f01173036096619cba09e31bde1a) to release-branch.go1.27.dmitshur reviewed +2 on encoding/xml: [release-branch.go1.27] fix depth processing in (*Decoder).unmarshal20h
(1 comment)
Since that issue was reopened and updated to track the Go1.27 fix, this could say "Fixes" instead of "Updates" so the issue will get automatically closed when this lands. It's also fine …dmitshur commented on encoding/xml: [release-branch.go1.26] fix depth processing in (*Decoder).unmarshal20h
Retrying trybots since "stream error: stream ID 11; INTERNAL_ERROR" may not happen a second time.dmitshur commented on golang.org/x/build/internal/task: upgrade dependencies after vscode-go insider minor21h
(1 comment)
I think you might want to consider `go get work` instead of `go get -u all` here.
The -u flag means to update not only the direct dependencies of the specified packages, but also their …dmitshur reopened an issue encoding/xml: (*Decoder).DecodeElement bypasses recursion-depth guard1d
dmitshur reopened an issue encoding/xml: (*Decoder).DecodeElement bypasses recursion-depth guard1d
Reopening and moving to Go 1.27 milestone to track [cherry-picking](https://groups.google.com/g/golang-dev/c/jMWTCiV0aSk/m/Qo6ozI0dBQAJ) [CL 803320](https://go.dev/cl/803320) onto release-branch.go1.…dmitshur commented on encoding/xml: (*Decoder).DecodeElement bypasses recursion-depth guard1d
Reopening and moving to Go 1.27 milestone to track [cherry-picking](https://groups.google.com/g/golang-dev/c/jMWTCiV0aSk/m/Qo6ozI0dBQAJ) [CL 803320](https://go.dev/cl/803320) onto release-branch.go1.…dmitshur commented on encoding/xml: (*Decoder).DecodeElement bypasses recursion-depth guard [1.25 backport]1d
CC @golang/release.dmitshur commented on encoding/xml: (*Decoder).DecodeElement bypasses recursion-depth guard [1.26 backport]1d
CC @golang/release.dmitshur reopened an issue crypto/tls: escape hatch for FIPS 140-3 mode Extended Master Secret enforcement [freeze exception]1d
Reopening to track [cherry-picking](https://groups.google.com/g/golang-dev/c/jMWTCiV0aSk/m/Qo6ozI0dBQAJ) [CL 804062](https://go.dev/cl/804062) onto release-branch.go1.27.dmitshur reopened an issue crypto/tls: escape hatch for FIPS 140-3 mode Extended Master Secret enforcement [freeze exception]1d
Reopening to track [cherry-picking](https://groups.google.com/g/golang-dev/c/jMWTCiV0aSk/m/Qo6ozI0dBQAJ) [CL 804062](https://go.dev/cl/804062) onto release-branch.go1.27.This Week
dmitshur reviewed +2 on golang.org/x/build/main.star: remove sizespecializedmalloc builders1d
Thanks.dmitshur closed an issue runtime: js/wasm: "found bad pointer in Go heap" — link-layout-constant value recorded as a pointer in the write-barrier buffer [1.27 backport]2d
Updated milestone of #80472 to Go 1.27; marking this as a duplicate.dmitshur commented on cmd/go: toolchain escape via PATH in covermode when GOTOOLCHAIN auto-switches2d
This may be the same issue as #75031, which is fixed by now. @zccold, does this still reproduce for you with go1.27rc1+, or go1.26.3 or 1.25.10?dmitshur commented on crypto/tls: handshake messages can bypass non-advancing record limit in TLS 1.32d
@nicholashusin Please be aware that without a milestone set, the release-blocker label doesn't block any particular Go release.dmitshur commented on encoding/json/v2: remove detection of cyclic data structures2d
@dsnet Please be aware that without a milestone set, the release-blocker label doesn't block any particular Go release.dmitshur commented on crypto/tls: escape hatch for FIPS 140-3 mode Extended Master Secret enforcement [freeze exception]2d
@hardikmodha Both the freeze exception and backports to Go 1.26 and 1.25 are actively being considered. See https://github.com/golang/go/issues/80516#issuecomment-5071611439.
@gopherbot Please open…dmitshur commented on cmd/go: "run" and "test" behave differently3d
It's documented at https://pkg.go.dev/cmd/go@go1.27rc2#hdr-Test_packages that:
> As part of building a test binary, go test runs go vet on the package and its test source files to identify significa…Last Week
dmitshur commented on make.bash: use toolchain switching to acquire bootstrap1w
(1 comment)
Please consider adding “Fixes #75143.” here. Or “For” if you think there’s more left to do after this.Earlier
dmitshur commented on build: LUCI Python update needed for openbsd ppc64 builder?1w
As far as I can tell, the swarming bot code hasn't been updated to be compatible with Python 3.14 yet, and you're among the first to notice and report this. I see that `pkgutil.get_loader` call is st…dmitshur starred github.com/filippo-claude/goapi1w
dmitshur commented on cmd/go: inferred GOAMD64 value in go env should be applied to build1w
It’s documented at https://pkg.go.dev/cmd/go#hdr-Build_constraints - search for “amd64.v3” as the exact name of the build tag, not “GOamd64.v3”; the GO prefix is a part of the environment v…dmitshur commented on cmd/go: check that insecure scheme is permitted by GOINSECURE1w
(1 comment)
I removed it to improve readability of this code. The blank line doesn’t seem helpful. Without the blank line, it’s clear that the value of security is computed to be one of web.Secu…dmitshur commented on doc: write release notes for Go 1.271w
@stefanb It was noted in https://go.dev/doc/go1.25#json_v2, back when the encoding/json/v2 package started its experimental phase, that its design would evolve. I imagine users who have been experime…dmitshur reviewed +2 on cmd/dist: remove unused omitVariant field1w
Thanks.dmitshur fixed an issue golang.org/x/pkgsite/internal/godoc/dochtml/internal/render: TestFormatDocHTML failures1w
Fixed by [CL 801520](https://go.dev/cl/801520).dmitshur commented on golang.org/x/pkgsite/internal/godoc/dochtml/internal/render: TestFormatDocHTML failures1w
Fixed by [CL 801520](https://go.dev/cl/801520).dmitshur reviewed +2 on golang.org/x/playground/...: run go fix ./...1w
Thanks.
I think some of the fixes by default leave code in a way that its readability can be improved when considering the context. I left some minor suggestions.
The variable name "before" is very…dmitshur reviewed +2 on golang.org/x/playground/sandbox: instrument container wait time (queue delay)1w
Thanks.
```suggestion
func getContainer(ctx context.Context) (_ *Container, err error) {
start := time.Now()
```dmitshur commented on golang.org/x/playground/...: turn down go2goplay appengine instance1w
(4 comments)
https://go.dev/doc/contribute#commit_messages suggest to "Write in complete sentences with correct punctuation, just like for your comments in Go." It's probably fine not to even mentio…dmitshur reviewed +2 on go.dev/wiki/Watchflakes: add example for matching all subtests1w
Thanks.
Because of the mandatory '/', this will exclude failures in the top-level TestServeIndexHtml test itself. Maybe it's more common to want to match the test and all its subtests too? If so, co…dmitshur reviewed +2 on Revert "cmd/compile: optimize abi.Type.TFlag loads"1w
Thanks.
I'll also add an optional x_build-gotip-linux-amd64 trybot here so we can see what it reports.
https://go.dev/wiki/CommitMessage#reverts suggests including the CL number.
```suggestion
Thi…dmitshur commented on cmd/compile: "hasUncommon: methods not computed" ICE as of CL 7012991w
It's not a problem for x/build itself, since we generally use stable Go releases there, so no worries there. It's good that it was able to uncover this.
Depending on how much other code this problem…dmitshur opened an issue cmd/compile: "hasUncommon: methods not computed" ICE as of CL 7012991w
At tip, starting with [CL 701299](https://go.dev/cl/701299), building the [golang.org/x/build/internal/relui](https://golang.org/x/build/internal/relui) package has started to fail with:
```
FAIL go…dmitshur commented on html/template: potential XSS caused by wrong JavaScript regexp context tracking after top-level `{`2w
Which release is this a blocker for? (This issue currently has no Milestone, making the release-blocker label a no-op.)dmitshur reviewed +2 on Revert "cmd/compile: on AMD64 use leave instruction for go compiled functions"2w
Thanks.dmitshur reviewed +2 on all: update vendored dependencies2w
Thanks.dmitshur commented on all: update vendored dependencies2w
(1 comment)
I'd expect this version to get maintained, but you may have not picked up the absolute latest updatestd. It's fine since it'll happen in a future run too.dmitshur commented on all: update vendored dependencies2w
The bootstrap on the builders should be go1.26.0 as of CL 801200. There's also been a small updatestd update recently, so consider rebasing and regenerating. I also left a suggestion about git-genera…dmitshur commented on golang.org/x/pkgsite/internal/godoc/dochtml/internal/render: fix tests2w
(1 comment)
```suggestion
For golang/go#76975.
Change-Id: Ibc508c39eb4785515d98b4eaf4fce299831b4924
```
So that this CL shows up in the timeline of that proposal.dmitshur commented on cmd/go/internal/telemetrystats: drop go/platform/target/{goos,goarch}:*2w
Letting this land for Go 1.28.
Thanks.dmitshur commented on reflect: [release-branch.go1.27] extract potentially embedded reflect types before use2w
Trybot failure looks like #76901, retrying it.dmitshur reopened an issue reflect: memory unsafety due to interface shadowing in reflect2w
Reopening for [CL 801220](https://go.dev/cl/801220) submission.dmitshur reopened an issue reflect: memory unsafety due to interface shadowing in reflect2w
Reopening for [CL 801220](https://go.dev/cl/801220) submission.dmitshur reopened an issue cmd/go: doc -http fails with "doc: no such file or directory" on first run with clean GOCACHE2w
The fix landed after the tree opened for Go 1.28 development, so reopening to track this being cherry picked to release-branch.go1.27.dmitshur reopened an issue cmd/go: doc -http fails with "doc: no such file or directory" on first run with clean GOCACHE2w
The fix landed after the tree opened for Go 1.28 development, so reopening to track this being cherry picked to release-branch.go1.27.dmitshur reviewed +2 on golang.org/x/website/internal/web: render markdown tables with scrollbars2w
Thanks.
This seems like a reasonable approach. I'll note that since we're dealing with HTML and Markdown and CSS and differently sized devices here, I also anticipate there's some chance of this lea…dmitshur commented on golang.org/x/build/main.star: use Go 1.26.0 as bootstrap on tip (Go 1.28 to be)2w
(1 comment)
This change is ready for review.
Tested via a shadow bucket build at https://ci.chromium.org/b/8676172686530536289. It's based on the real build https://ci.chromium.org/b/86761811769047…dmitshur commented on golang.org/x/website/cmd/golangorg: add <head> tag before </head>2w
Thanks.dmitshur opened a change golang.org/x/build/main.star: use Go 1.26.0 as bootstrap on tip (Go 1.28 to be)2w
For golang/go#77341.dmitshur reviewed +2 on cmd/go: [release-branch.go1.27] do not pass -mthreads to C compiler on Windows2w
Thanks.dmitshur commented on golang.org/x/build/cmd/updatestd: maintain cmd/go/internal/doc selection of pkgsite version2w
Thanks.
Thanks for suggesting astutil.Apply - though I knew about the existence of that package, I hadn't used it in a long while and didn't think to look there when implementing this. I did conside…dmitshur commented on internal/goversion: increment Version at start of cycle2w
This is already done for the Go 1.28 cycle via [CL 800721](https://go.dev/cl/800721) (CC @matloob). Moving to the next milestone.dmitshur closed an issue cmd/distpack: exclude the .jj directory [1.27 backport]2w
@nsrip-dd Please note https://go.dev/s/release#july--january-week-3-work-on-the-next-release-begins:
> During this period, fixes intended for the current release need to be [cherry-picked onto the r…dmitshur commented on : [release-branch.go1.27] Merge commit 'master^{/internal/goversion}~' into release-work2w
https://go.googlesource.com/go/+diff/refs/changes/60/800760/1..a5f29a67d7a26ce6d4d7c2fbae12d93a4ea8b24e matches my expectation.
Suggestion:
```
[release-branch.go1.27] all: merge master (a5f29a67) …dmitshur commented on cmd/distpack: exclude the .jj directory2w
(1 comment)
This '.git/**' entry has corresponding checks in src/cmd/distpack/test.go. Maybe it's worth adding analogous ones for similar reasons.dmitshur reviewed +1 on src/cmd/go/internal/doc: always return cached executable2w
(3 comments)
In the main Go repo, the "src" path element is normally left out, so that the path is closer to the actual import path of the relevant Go package.
```suggestion
cmd/go/internal/doc: al…dmitshur reviewed +2 on go/build/constraint: match "go1." only as a tag prefix in GoVersion2w
Thanks again.dmitshur reviewed +2 on go/build/constraint: match "go1." only as a tag prefix in GoVersion2w
(1 comment)
This parent commit is a bit old, before the fix for #80212 landed, so that particular test will fail. Rebasing would resolve that, otherwise we can bypass if that's the only failing test.dmitshur commented on cmd/vet: improve how stdversion analyzer refers to methods2w
(1 comment)
I checked locally that this works with a generic method (that @mpratt@google.com originally reported this with), and it does work as expected:
```
$ go vet -stdversion
main.go:7:19: ran…dmitshur reviewed +2 on go/build/constraint: match "go1." only as a tag prefix in GoVersion2w
Thanks.
I understand this test case, in contrast to the one above, captures the case where it's not a valid version but it does have a valid version as a substring. It seems like a good idea to add …dmitshur opened a change cmd/vet: improve how stdversion check refers to methods2w
Pull in the fix implemented in CL 800620.
Fixes #80363.
[git-generate]
updatestd -goroot=$(pwd) -branch=internal-branch.go1.27-vendordmitshur commented on github.com/go-gl/gl: Error about -mthreads option, when trying to use clang on Windows2w
Also see golang/go#80290.dmitshur commented on doc: write release notes for Go 1.272w
We removed the release-blocker label since there doesn't seem to be anything actionable left that would block the release. This issue will stay open to track any further refinement and improvement le…dmitshur opened a change golang.org/x/tools/go/analysis/passes/stdversion: [internal-branch.go1.27-vendor] correctly format symbol names2w
The existing formatting completely forgot to include the
type name T in the name of a field or method pkg.T.name.
Since this cannot be derived from a field *types.Var, we
change TooNewStdSymbols to …dmitshur commented on lib/time: update tzdata before release2w
Thanks. 2026c landed in time for Go 1.27.0. It's likely the last tzdata release to be included, so moving this to the next milestone. (If another tzdata release happens with some time before 1.27.0, …dmitshur reviewed +2 on std: address ptrtoerror analyzer diagnostics2w
Thanks.
Do you have an opinion on whether an assertion like this should go into test code (i.e., in a _test.go file) or the package implementation code? I have a slight preference for placing these …dmitshur reviewed +2 on golang.org/x/tools/go/analysis/passes/stdversion: correctly format symbol names2w
dmitshur commented on cmd/vet: pull in CL 7994412w
(1 comment)
Fix applied.dmitshur reviewed +2 on golang.org/x/tools/go/analysis/passes/stdversion: correctly format symbol names2w
Thanks. There are some gopls tests that aren't passing yet.
Is you preference to include this in 1.27's vet output too, or do you think this might be too risky or disruptive and better to leave to G…dmitshur commented on cmd/fix: pull in CL 7994412w
(1 comment)
```suggestion
cmd/vet: pull in CL 799441
```
Will apply before submitting (not while trybots are running, to avoid interrupting).dmitshur opened a change cmd/fix: pull in CL 7994412w
Fixes #80360.
[git-generate]
updatestd -goroot=$(pwd) -branch=internal-branch.go1.27-vendordmitshur commented on golang.org/x/tools/go/analysis/passes/stdversion: stdversion check refers to "rand.(*Rand).N" method as "rand.N"2w
It looks like this isn't a regression related to generic methods, rather it's how this check displays methods in general:
<img width="568" height="293" alt="Image" src="https://github.com/user-attac…dmitshur commented on cmd/vet: stdversion check doesn't report uses of Go 1.27 APIs in pre-1.27 modules2w
@prattmic Thanks. Factored that out into #80363 since it might be more involved, and updated this to be about the more general problem of cmd/vet not being aware of all new-to-1.27 symbols.dmitshur opened an issue golang.org/x/tools/go/analysis/passes/stdversion: stdversion check refers to "rand.(*Rand).N" method as "rand.N"2w
Consider:
```shell
$ cat <(echo -- go.mod --) go.mod <(echo -- main.go --) main.go
-- go.mod --
module test
go 1.25.0
-- main.go --
package main
import ("fmt"; "math/rand/v2")
func main() {
r…dmitshur opened a change golang.org/x/tools/internal/stdlib: [internal-branch.go1.27-vendor] update stdlib index for Go 1.27 Release Candidate 22w
For golang/go#38706.
For golang/go#80360.
[git-generate]
go run golang.org/dl/go1.27rc2@latest download
go run golang.org/dl/go1.27rc2@latest generate ./internal/stdlibdmitshur opened a change golang.org/x/tools/internal/stdlib: [internal-branch.go1.27-vendor] update stdlib index for Go 1.27 Release Candidate 12w
For golang/go#38706.
For golang/go#80360.
[git-generate]
go run golang.org/dl/go1.27rc1@latest download
go run golang.org/dl/go1.27rc1@latest generate ./internal/stdlibdmitshur opened an issue cmd/vet: stdversion check doesn't report uses of Go 1.27 APIs in pre-1.27 modules2w
Consider:
```shell
$ cat <(echo -- go.mod --) go.mod <(echo -- main.go --) main.go
-- go.mod --
module test
go 1.25.0
-- main.go --
package main
import ("fmt"; "math/rand/v2")
func main() {
r…dmitshur commented on cmd/go: don't run cgo compiler version probe under -toolexec2w
(1 comment)
I'm okay with your decision on this. Thanks for considering it.dmitshur commented on golang.org/x/build/cmd/relui: consider target versions of security fixes in security comms2w
(1 comment)
Sounds good. Given we have agreement, we can move in that direction. It can happen independently of this change though, so added it as a TODO comment for now.dmitshur reviewed +2 on golang.org/x/playground/sandbox: prevent startContainer from hanging on timeout2w
Thanks.
Note that this non-nil error is also being returned to the caller to be handled. Adding a log line here will cause it to be double-logged, see line 185.
The new log line includes scanner.Te…dmitshur reviewed +2 on golang.org/x/playground/sandbox: parameterize image tags and update docker install2w
Thanks.
Does it help for this to use $TAG too?
```suggestion
docker run --name=sandbox_dev --rm --network=sandnet -ti -p 127.0.0.1:8080:80/tcp -v /var/run/docker.sock:/var/run/docker.sock golang/p…dmitshur reviewed +2 on golang.org/x/build/cmd/relui: add govulncheck-action repo tagging workflow2w
dmitshur reviewed +2 on golang.org/x/build/cmd/relui: add govulncheck-action repo tagging workflow3w
Thanks. One more comment in Workflow.Run, otherwise I don't see anything more from my side, and I think you should feel free to submit this and/or try it out. As with any new workflow, trying it out …dmitshur reviewed +2 on all: use path for go.mod module path3w
Thanks.
```suggestion
module crypto/internal/fips140/sha3/_asm
```
Unrelated to this CL, but I wonder what is the motivation for this go.mod? Usually they're used to track tools used for code gener…dmitshur commented on golang.org/x/build: try a -racegocommand builder3w
Both the -racegocommand and -racegocommand-noopt builders have been running for some time, and so far they haven't uncovered interesting races in the go command. Issue #79667 turned out to be a probl…dmitshur reviewed +1 on cmd/go: don't run cgo compiler version probe under -toolexec3w
(1 comment)
A question related to this. As far as I see, TOOLEXEC_CDRIVER_LOG is only used to by this test in one way: to point it to a cdriver.log file and then later on check under cgo that it doe…dmitshur opened an issue os: TestRootMultiReadFile fails on netbsd/arm64 after CL 7978803w
See https://ci.chromium.org/ui/p/golang/builders/ci/gotip-netbsd-arm64/b8676812685865790769/overview for example. (A different test fails on Plan 9, see #80319.)
CC @golang/netbsd.dmitshur closed an issue golang.org/x/build/cmd/coordinator: mind combined memory usage when concurrently processing large payloads3w
We got by with increasing memory available to coordinator. By now it no longer does builds itself, only displays an old-style dashboard [build.golang.org](https://build.golang.org) pointing to [LUCI …