Activity
Today
dmitshur reviewed +1 on src/cmd/go/internal/doc: always return cached executable1h
(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 GoVersion2h
Thanks again.dmitshur reviewed +2 on go/build/constraint: match "go1." only as a tag prefix in GoVersion2h
(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 methods3h
(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 GoVersion3h
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 methods3h
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 Windows4h
Also see golang/go#80290.dmitshur commented on doc: write release notes for Go 1.274h
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 names5h
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 release5h
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 diagnostics6h
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 …Yesterday
dmitshur reviewed +2 on golang.org/x/tools/go/analysis/passes/stdversion: correctly format symbol names1d
Last Week
dmitshur commented on cmd/vet: pull in CL 7994414d
(1 comment)
Fix applied.dmitshur reviewed +2 on golang.org/x/tools/go/analysis/passes/stdversion: correctly format symbol names4d
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 7994414d
(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 7994414d
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"4d
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 modules4d
@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"4d
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 24d
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 14d
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 modules4d
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 -toolexec4d
(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 comms4d
(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 timeout4d
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 install4d
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 workflow5d
dmitshur reviewed +2 on golang.org/x/build/cmd/relui: add govulncheck-action repo tagging workflow5d
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 path5d
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 builder5d
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 -toolexec5d
(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 7978805d
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 payloads5d
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 …dmitshur commented on os: TestRootConsistencyRemoveAll fails on Plan 9 after CL 7978805d
CC @golang/plan9.dmitshur commented on golang.org/x/build/relnote: ignore comment-only markdown files5d
Thanks for implementing this.
I left some review comments. In general, this looks like a good start, and when the comments are resolved we should be able to submit this.
Two suggestions here.
One,…dmitshur reviewed +2 on cmd/go: split test_vet script test for improved parallelism5d
Thanks.dmitshur reviewed +2 on cmd/go: split build_overlay script test to improve parallelism5d
Thanks.dmitshur reviewed +2 on golang.org/x/playground/sandbox: replace deprecated Konlet with systemd service in cloud-init5d
Thanks. I'm not familiar enough with the lower level details involved in this, but it makes sense to migrate off a deprecated mechanism.dmitshur closed a change golang.org/x/website/_content/doc/tutorial/generics.md: fix typo6d
This change is applied via https://go.dev/cl/782240.dmitshur reviewed +2 on golang.org/x/build/main.star: add c4d (AMD Turin) builders6d
Thanks.
Since this CL is a part of what's needed, you might want to keep the issue open for a bit until the VMs are up and everything works as you expect.
Unless I missed it, you'll still need to d…dmitshur commented on golang.org/x/build/internal/task: add support for okay-after-rc2 label6d
Thanks.
Yeah, I considered that too but stopped at just rc2 for now to match the labels that exist. For rc3 hopefully we'll really resolve all release blockers or remove the release-blocking label. …dmitshur closed a change golang.org/x/build/cmd/coordinator: lower the memory allocation6d
Replaced with CL 798540.dmitshur reviewed +2 on golang.org/x/build/cmd/coordinator: update resource allocations6d
Thanks.
I did something similar in CL 732263 earlier. It's still open because I was deciding whether to lower it a bit further before submitting. By now all legacy builds are stopped, so let's go wi…dmitshur commented on cmd/fix: pull in CL 7916606d
Thanks.dmitshur reviewed +2 on lib/time: update to 2026c/2026c6d
Thanks.dmitshur opened a change cmd/fix: pull in CL 7916606d
Generated with updatestd, though it's equivalent to manually running:
cd src/cmd
go get golang.org/x/tools@internal-branch.go1.27-vendor # v0.45.1-0.20260708165844-7044391f8d9f
go mod tidy
go mod v…dmitshur reviewed +2 on golang.org/x/build/internal/coordinator/remote: avoid a potential deadlock6d
Thanks.dmitshur fixed an issue golang.org/x/tools: tests on internal-branch.go1.27-vendor branch aren't passing with go1.27rc26d
Fixed by the aforementioned CLs.dmitshur reviewed +2 on golang.org/x/build/cmd/coordinator: remove gomote functionality from the coordinator6d
dmitshur reviewed +1 on golang.org/x/tools/go/analysis/passes/printf: reject %d of pointer or chan6d
Thanks. This looks okay to me so +1, though I expect there may be a better reviewer for go/analysis/passes packages. I understand this will be waiting on the proposal to be accepted anyway.
It took …dmitshur commented on golang.org/x/tools/go/analysis/passes/modernize: [internal-branch.go1.27-vendor] preserve symbols in reflecttypefor1w
Filed #80296 for this problem and mailed CLs. I'll rebase this on top, that should fix the trybots here.dmitshur commented on golang.org/x/tools/go/analysis/passes/printf: [internal-branch.go1.27-vendor] adjust printf analyzer for CL 7843811w
This change gets the [golang.org/x/tools/go/analysis/passes/printf.Test](https://ci.chromium.org/ui/p/golang/builders/try/x_tools-go1.27-linux-amd64-longtest/b8676896302437009505/test-results?q=Ex…dmitshur commented on golang.org/x/tools/go/ssa: [internal-branch.go1.27-vendor] reject generic methods from RuntimeTypes1w
This change gets the [golang.org/x/tools/go/ssa.TestStdlib](https://ci.chromium.org/ui/p/golang/builders/try/x_tools-go1.27-linux-amd64-longtest/b8676896325792242385/test-results?q=ExactID%3Agolan…dmitshur opened a change golang.org/x/tools/gopls/internal/test/marker: [internal-branch.go1.27-vendor] update hover test1w
CL 784060 updates Types.Info.Instantiations to preserve receivers
for generic methods, which breaks these tests. This change updates
a test to allow that CL to merge.
For golang/go#79657.
Fixes gola…dmitshur opened an issue golang.org/x/tools: tests on internal-branch.go1.27-vendor branch aren't passing with go1.27rc21w
The `internal-branch.go1.27-vendor` branch in x/tools was created quite early after the start of the Go 1.27 release freeze, as opposed to closer to when the Go 1.27 release freeze will be ending. So…dmitshur commented on golang.org/x/website/_content/doc: update PATH docs and restructure manage-install.html1w
Thanks.dmitshur reviewed +2 on os: properly handle trailing slashes in paths in Root1w
Thanks.dmitshur fixed an issue security: fix CVE-2026-39822 [1.25 backport]1w
Closed by merging [CL 797660](https://go.dev/cl/797660) (commit c94048f5638bbcaa22102bade5e9774e0d485315) to release-branch.go1.25.dmitshur fixed an issue os: Root escape via symlink plus trailing slash (CVE-2026-39822) [1.25 backport]1w
Closed by merging [CL 797660](https://go.dev/cl/797660) (commit c94048f5638bbcaa22102bade5e9774e0d485315) to release-branch.go1.25.dmitshur fixed an issue security: fix CVE-2026-39822 [1.26 backport]1w
Closed by merging [CL 797661](https://go.dev/cl/797661) (commit f9ef7f55988f03afeb3b8354367d0fa8d053683d) to release-branch.go1.26.dmitshur fixed an issue os: Root escape via symlink plus trailing slash (CVE-2026-39822) [1.26 backport]1w
Closed by merging [CL 797661](https://go.dev/cl/797661) (commit f9ef7f55988f03afeb3b8354367d0fa8d053683d) to release-branch.go1.26.dmitshur commented on os: [release-branch.go1.26] properly handle trailing slashes in paths in Root1w
(1 comment)
Since this is a release-branch.go1.26 backport CL, it would've been optimal if the commit message had:
```suggestion
For #79005.
Fixes #79027.
```
Then the Go1.26.5 backport issue #790…dmitshur fixed an issue golang.org/x/build: bot openbsd-riscv64-jsing reported as broken1w
It seems to be back online and running. Thanks @4a6f656c.dmitshur fixed an issue golang.org/x/build: bot openbsd-riscv64-jsing reported as broken1w
It seems to be back online and running. Thanks @4a6f656c.dmitshur reviewed +1 on golang.org/x/website/internal/history: document Go 1.26.5 and 1.25.12 releases1w
dmitshur commented on golang.org/x/pkgsite: support generic methods and show "Migrating to v2" in encoding/json1w
> I investigated this issue and found that updating "pkgsiteCmdInternalDocVersion" to "v0.0.0-20260630164626-55ca6a10dee5" fixes the generic method rendering problem. I verified that "go doc -http" n…dmitshur reviewed +2 on cmd/go: move part of gotoolchain_net to verylongtest1w
Thanks.
I take it that it's guaranteed that calling t.TempDir() first, and t.Cleanup second will cause the cleanup to run first, TempDir deletion second, right?
Is it intentional that removeAll not…dmitshur opened an issue cmd/go: doc -http fails with "doc: no such file or directory" on first run with clean GOCACHE1w
### Go version
go version go1.27rc1 darwin/arm64
### Output of `go env` in your module/workspace:
(`go env -changed` is empty.)
### What did you do?
```
$ export GOCACHE=$(mktemp -d)
$ go…dmitshur reviewed +2 on golang.org/x/build/cmd/coordinator: remove gomote functionality from the coordinator1w
Thanks.
Optional suggestion. Since the swarming gomotes are all that's left in the golang.org/x/build/internal/gomote package, consider appending helper.go to the bottom of swarming.go, and renaming…dmitshur reviewed +2 on golang.org/x/pkgsite/internal/fetch: inherit active tool tags in build context1w
Thanks.
This test case appears to be hard to read because of how it combines the terms "other", "jsonv2", and "archive/tar" that seemingly have little in common. As a result it's unclear what it's t…dmitshur opened a change golang.org/x/build/internal/task: add support for okay-after-rc2 label1w
We've created an okay-after-rc2 label, which works like okay-after-rc1
but for RC 2 instead of RC 1. Make the 'Check blocking issues' task more
useful by having it ask only about release blockers tha…dmitshur reviewed +2 on golang.org/x/build/main.star: remove openbsd-arm64 from KNOWN_ISSUE_BUILDER_TYPES1w
Thanks again.dmitshur commented on golang.org/x/build/cmd/relui: consider target versions of security fixes in security comms1w
(1 comment)
@neal@golang.org Do you think we should keep this version syntax as is, or is it worth considering making it match the https://go.dev/doc/toolchain#name? I understand that VulnDB needs t…dmitshur commented on golang.org/x/build/main.star: remove openbsd-arm64 from KNOWN_ISSUE_BUILDER_TYPES1w
Done in patch set 4.dmitshur commented on runtime: sizespecializedmalloc experiment1w
@matloob Thanks for marking this as done.
The release note currently says "[GOEXPERIMENT=nosizespecializedmalloc] is expected to be removed in Go 1.28."; is there an issue to track doing the work of…dmitshur commented on golang.org/x/website/solutions: update urfave/cli link to v3 GitHub URL1w
> 👋 Hi! Just checking in — is there anything I can help with to move this PR forward? Happy to address any feedback! 🙏
See https://github.com/golang/website/pull/359#issuecomment-463983711…dmitshur reviewed +2 on cmd/go: move mod_insecure_issue63845 and mod_proxy_https to verylongtest1w
dmitshur reviewed +1 on test: repair codegen/simd mask instructions test1w
dmitshur closed an issue cmd/internal/testdir:0_10: Test/codegen/simd.go failures [consistent failure]1w
Earlier
dmitshur reviewed +2 on go.dev/wiki/DashboardBuilders: remove documentation for legacy builders1w
dmitshur opened a change go.dev/wiki/DashboardBuilders: remove documentation for legacy builders1w
The legacy builders have been fully stopped and don't work anymore.
The documentation for them is obsolete and misleading. Remove it.
For golang/go#67761.dmitshur commented on runtime: implement stackcheck for riscv641w
The legacy slowbot infrastructure is unsupported and doesn't work. I added their LUCI counterparts as advisory slowbots to this CL:
- [gotip-linux-riscv64](https://ci.chromium.org/ui/p/golang/builde…dmitshur commented on golang.org/x/build: gotip-openbsd-riscv64 builder is missing1w
There is a [openbsd-riscv64-jsing](https://chromium-swarm.appspot.com/bot?id=openbsd-riscv64-jsing) builder now, but it's showing up as offline as of 24 hrs ago. CC @4a6f656c.dmitshur reviewed +2 on cmd/go/internal/work: cache coverage output1w
I think I understand this at a high level and it's mostly a refactor. However, I'm not familiar with go command's action caching and coverage instrumentation, so my review isn't going to be very thor…dmitshur reviewed +2 on cmd/go: remove net script test condition from main go command suite1w
Thanks.
🎉dmitshur reviewed +2 on cmd/go: move mod_sumdb_golang network tests to verylongtest1w
Thanks.
Consider passing in the *testing.T here and call t.Fatal instead. Also below.
`l` is never closed, even after the test completes. Is the reason for that because the test executable is deeme…dmitshur reviewed +2 on cmd/go: move part of gotoolchain_net to verylongtest1w
Thanks.
This copy of scripttest is internal to the cmd module, so it makes sense to modify it to suit the needs of all of its internal users. I agree it seems calling removeAll to clean up after the…dmitshur reviewed +2 on cmd/go: move mod_insecure_issue63845 and mod_proxy_https to verylongtest1w
Thanks.
This seems like the only use of fixReadme. Should the flag be removed because it no longer has any effect, or be fixed to work?dmitshur reviewed +2 on cmd/go/testdata/script: update mod_prefer_compatible to not use net1w
Thanks.dmitshur reviewed +2 on cmd/go/testdata/script: remove net condition from always skipped tests1w
Thanks.
I think this is saying that the test is being skipped because it has a 'skip' verb. It's fine, though a bit repetitive.
[go.dev/issue/54043](https://go.dev/issue/54043) is closed, and it's …dmitshur reviewed +2 on cmd/go/testdata/script: don't use network in mod_gonoproxy test1w
Thanks.dmitshur reviewed +2 on cmd/go/testdata/script: remove net from mod_download_private_vcs1w
Thanks.
😄dmitshur fixed an issue golang.org/x/crypto: upgrade x/net dependency1w
As noted in the comment above, closing as fixed by [CL 788222](https://go.dev/cl/788222) which is included in x/crypto@v0.53.0. (CC @golang/security.)
> although the problem is the scan engine
It s…dmitshur reviewed +2 on cmd/go/testdata/script: don't use net in mod_pseudo_cache1w
Thanks.dmitshur reviewed +2 on cmd/go/testdata/script: don't use net in mod_sumdb_file_path test1w
Thanks.dmitshur reviewed +2 on cmd/go/testdata/script: stop using network from mod_git_export_subst1w
Thanks.dmitshur reviewed +2 on cmd/go/testdata/script: remove net from gotoolchain_version test1w
Thanks.dmitshur reviewed +2 on cmd/go/testdata/script: remove net check in get_iussue53955 test1w
Thanks.
(minor typo)
```suggestion
cmd/go/testdata/script: remove net check in get_issue53955 test
```dmitshur reviewed +2 on cmd/go/testdata/script: don't use network in govcs script test1w
Thanks.
I left a suggestion to consider including at least one "For #nnn." line. That suggestion is for all CLs in this stack, but I won't repeat it everywhere.
Whenever there's a large stack of CL…dmitshur reviewed +2 on cmd/go: switch toolchain if necessary in applyUpgrades1w
Thanks. Just a minor comment/question.
I don't know what's the motivation to defer base.ExitIfErrors() here given it's already being called on line 1495, and there aren't any early returns before th…