Activity
Today
dmitshur reviewed +2 on golang.org/x/build/cmd/gomoteserver: add Kubernetes health checks10m
Thanks.
This makes sense to me. My one minor comment is about possibly simplifying the probe configuration a bit, but I have no objections to any of it if it helps.
From briefly re-reading about ho…dmitshur closed a change golang.org/x/build: Expose more ReviewInfo fields in the Go Gerrit client1h
This PR is being closed because [go.dev/cl/234660](https://go.dev/cl/234660) was closed.
The x/build/gerrit [package comment](https://pkg.go.dev/golang.org/x/build/gerrit#pkg-overview) is now more…dmitshur commented on os: notify testlog from (*File).Chdir1h
This should be covered at https://go.dev/wiki/MinorReleases. In particular, see this paragraph:
> When the child issue is labeled CherryPickApproved, the original author of the change fixing that is…dmitshur opened a change golang.org/x/website/go.mod: pull in "relnote: include file name for stdlib minor changes"1h
Pull in x/build CL 783780 so that its effect on the /doc/go1.27 page
applies sooner.Yesterday
dmitshur opened a change golang.org/x/build/internal/relui: only run TestRelease/major on 1.26 and newer13h
A fixup for CL 768223 to unbreak x/build tests with Go 1.25.dmitshur reviewed +2 on golang.org/x/exp/typeparams: deprecate and inline aliased functions13h
> as far as I know that is not considered an "API change" and so no proposal needed
I prepared CL 783580 but became less sure about that. After all, a similar argument may apply - adding a go:fix in…dmitshur commented on golang.org/x/exp/typeparams: update termlist, typeterm source files13h
I suspect it may work to make GO111MODULE=on work by taking advantage of go:generate directives being defined to execute in a predictable order, and doing something like:
```
//go:generate go get go…dmitshur commented on testing: allow examples to return an error13h
(Someone may extend that argument to say that it's odd to accept an error return from example and test functions, but not from net/http [handlers](https://pkg.go.dev/net/http#Handler).)dmitshur commented on cmd/go: check that insecure scheme is permitted by GOINSECURE14h
This change is ready for review.dmitshur commented on cmd/go: start a test case for an insecure scheme check14h
(1 comment)
This change is ready for review.
(nit) This should maintain a trailing newline. A self-reminder to remember to push a patch set with a fix.dmitshur opened a change cmd/go: start a test case for an insecure scheme check15h
This is before the fix. The test case passes, but it's exhibiting the
buggy behavior. The next CL implements a fix and updates the test case
to require intended behavior.
The test case defines a vcs…dmitshur commented on cmd/go: concurrent map writes in `go/build.(*Context).Import`16h
@prattmic Do you happen to have a link to the build handy? I tried looking over the last few https://ci.chromium.org/ui/p/golang/builders/luci.golang.ci/gotip-linux-amd64-noopt and didn't spot this f…dmitshur reviewed +1 on os/signal: [release-branch.go1.26] make NotifyContext Cause match context.Canceled16h
dmitshur reviewed +2 on go/types: add simd/.../sgutil to the TestStdlib ignores16h
Thanks.
I left a comment at https://go.dev/issue/79676#issuecomment-4557824015 on a similar issue. It seems to apply to issue #79677. The test is passing at the tip commit of the main branch. But ma…dmitshur commented on golang.org/x/build: add LUCI openbsd-arm64 builder17h
@4a6f656c Should we take the next steps towards removing the known issue for this builder, so that it starts to shows up at https://ci.chromium.org/p/golang/g/go-gotip/console instead of only appeari…dmitshur opened a change cmd/fix: [release-branch.go1.26] pull in CL 78396019h
Generated with updatestd, though it's equivalent to manually running:
cd src/cmd
go get golang.org/x/tools@internal-branch.go1.26-vendor # v0.39.1-0.20260527181557-0f52e3809b35
go mod tidy
go mod v…dmitshur fixed an issue cmd/compile/internal/types2: TestStdlib failures19h
dmitshur fixed an issue cmd/compile/internal/types2: TestStdlib failures19h
The failures seem to be commits on the dev.simd branch, prior to it being merged into the main branch. The merge conflict that was resolved at https://go-review.googlesource.com/c/go/+/782083/4/src/g…dmitshur commented on cmd/compile/internal/types2: TestStdlib failures19h
The failures seem to be commits on the dev.simd branch, prior to it being merged into the main branch. The merge conflict that was resolved at https://go-review.googlesource.com/c/go/+/782083/4/src/g…dmitshur commented on cmd/compile/internal/types2: TestStdlib failures20h
The build log is large, but "could not import _gen/sgutil (cannot find package "_gen/sgutil" in any of: ..." seems to be the problem inside.dmitshur closed an issue runtime: `SIGSEGV` with a c-shared library and called from C#20h
dmitshur opened a change golang.org/x/tools/go/analysis/passes/modernize: [internal-branch.go1.26-vendor] slicescontains: check for side effects20h
We should avoid suggesting a fix if the needle or predicate may
have side effects. Otherwise, the program behavior may change.
For example, in converting from a loop to slices.ContainsFunc,
the needl…dmitshur commented on os: make tests pass even without Administrator account22h
(2 comments)
Are there still tests that fail when running 'go test os' on Windows without an administrator account after this change? If not, this can become Fixes, otherwise Updates is fine.
The n…dmitshur reviewed +2 on golang.org/x/build/relnote: include file name for stdlib minor changes22h
Copied votes on follow-up patch sets have been updated:
* Code-Review+2 has been copied to patch set 2 (copy condition: "is:ANY").
(3 comments)
For golang/go#64169.
Though I guess by now that issu…This Week
dmitshur commented on golang.org/x/exp/typeparams: deprecate and inline aliased functions1d
I think it's a good idea to add //go:fix inline directive, and as far as I know that is not considered an "API change" and so no proposal needed. But deprecating symbols does need a proposal. Up to y…dmitshur commented on golang.org/x/exp/typeparams: simplify now that minimum required Go is 1.25.01d
Thanks.dmitshur commented on golang.org/x/exp/typeparams: consider String methods optional1d
Thanks.
This was a minimal change to get the test working at tip. It was only afterwards that I realized the entire 1.17 codepath and the test are made obsolete by the go.mod file's go 1.25.0 direct…dmitshur reviewed +2 on doc: document removal of asynctimerchan GODEBUG setting (update release notes)1d
Thanks.dmitshur commented on doc: document removal of gotypesalias GODEBUG setting (update release notes)1d
runtime.TestGoroutineLeakProfile/Istio18454 failure is not caused by this documentation-only change.dmitshur commented on golang.org/x/exp/typeparams: deprecate and delete1d
I initially left a 👍 reaction, but have since removed it. I originally thought this package's scope was only to cover the transition period. The commit message of [CL 383375](https://go.dev/cl/38…dmitshur commented on go/types: add String methods1d
Sent CL 783322.dmitshur opened a change golang.org/x/exp/typeparams: consider String methods optional1d
TestAPIConsistency correctly reports at Go tip that String methods
on the TypeList, TypeParamList, and Instance types are inconsistent.
They were added in Go 1.27 as part of proposal go.dev/issue/792…dmitshur reviewed +1 on doc: document generic methods (update release notes)1d
dmitshur reviewed +2 on cmd/go/internal/work: add signpost to x/tools vet std test1d
Thanks.dmitshur reviewed +2 on golang.org/x/tools/go/analysis/unitchecker: disable unreachable in vet_std_test.go1d
Thanks.dmitshur fixed an issue golang.org/x/telemetry/config: counters for target GOOS/GOARCH1d
Thanks.
[CL 775981](https://go.dev/cl/775981) added a local counter for the target port GOOS/GOARCH, and [CL 776000](https://go.dev/cl/776000) enabled its uploading for Go 1.27+. We should start se…dmitshur commented on golang.org/x/telemetry/config: counters for target GOOS/GOARCH1d
Thanks.
[CL 775981](https://go.dev/cl/775981) added a local counter for the target port GOOS/GOARCH, and [CL 776000](https://go.dev/cl/776000) enabled its uploading for Go 1.27+. We should start se…dmitshur commented on golang.org/x/telemetry/internal/chartconfig: add go/platform/target/port:*-* counter1d
Thanks.
I take it you mean across multiple lines? Sure, done in PS 3.dmitshur opened a change golang.org/x/build/main.star: include longtest builders in pre-submit in x/vuln1d
The longtest builders in this repository are fast (3 min), and this
should help catch problems that tests miss when -short flag is used.dmitshur commented on cmd/compile: update compiler's PGO profile1d
This should be safe to do before or after Go 1.27 RC 1, probably better no later than RC 2. Added that label for visibility.dmitshur commented on all: update standard-library dependencies at the start and end of each development cycle1d
The two CLs above did the "at the end of dev cycle" update for Go 1.27. Moving to the next.dmitshur fixed an issue runtime/coverage: TestCoverageApis failures1d
Closing since this hasn't happened in a while. [CL 588235](https://go.dev/cl/588235) may have helped (or will help for next time it flakes.) Watchflakes will reopen or file a new issue as needed.dmitshur commented on runtime/coverage: TestCoverageApis failures1d
Closing since this hasn't happened in a while. [CL 588235](https://go.dev/cl/588235) may have helped (or will help for next time it flakes.) Watchflakes will reopen or file a new issue as needed.dmitshur commented on cmd/pprof: update vendored github.com/google/pprof1d
internal/coverage/cfile.TestCoverageApis is a flaky test tracked in issue #74632. It also passed in the parent CL. Bypassing.dmitshur commented on all: update vendored dependencies1d
It also passed in the next CL. Bypassing.dmitshur closed an issue internal/coverage/cfile: TestCoverageApis/emitToFailingWriter failures1d
dmitshur closed an issue internal/coverage/cfile: TestCoverageApis/emitToNonexistentDir failures1d
dmitshur closed an issue internal/coverage/cfile: TestCoverageApis/emitToNilWriter failures1d
dmitshur closed an issue internal/coverage/cfile: TestCoverageApis/emitToDir failures1d
dmitshur closed an issue internal/coverage/cfile: TestCoverageApis/emitWithCounterClear failures1d
dmitshur commented on all: update vendored dependencies1d
TestFileAssociatedWithExternalIOCP looks like a pre-existing flaky test, tracked in issue #73726.dmitshur commented on runtime: remove asynctimerchan GODEBUG setting1d
@gri@golang.org Since you are the author of this change, and self-review in this repository is not allowed, it would be better to leave a +1 vote with your @google.com account rather than +2. This al…dmitshur opened a change cmd/pprof: update vendored github.com/google/pprof1d
Pull in the latest published version of github.com/google/pprof
as part of the continuous process of keeping Go's dependencies
up to date.
For #36905.
[git-generate]
cd src/cmd
go get github.com/go…dmitshur commented on golang.org/x/build/internal/releasetargets: update for each upcoming major Go release1d
[CL 767441](https://go.dev/cl/767441) generated a preliminary list for Go 1.27. It's the same list as for Go 1.26 except the macOS minimum version became 13, and that's noted at https://tip.golang.or…dmitshur commented on all: announce end of support for old Linux versions1d
There haven't been any accepted proposals related to old versions of Linux during the Go 1.27 development cycle, so moving to the next milestone.dmitshur commented on all: announce end of support for old Windows releases1d
There haven't been any accepted proposals related to old versions of Windows during the Go 1.27 development cycle, so moving to the next milestone.dmitshur commented on all: announce end of support for old macOS releases1d
[CL 739380](https://go.dev/cl/739380) added an entry to the Go 1.27 release notes. Moving to the next milestone.dmitshur reviewed +2 on cmd/go: return non-zero when go tool is interrupted2d
Thanks.
@olivier.mengue@gmail.com See comments Russ and I left on issue #13440 about why it's difficult to do better.
@sean@liao.dev This seems like a bigger question and discussion. `go run` could…dmitshur commented on os: running "go test os -short" command fails3d
@alexbrainman Your understanding above about longtest builders is right. Only builders with the "longtest" runtime modifier run `go test` without `-short` flag. All other builders run `go test -short…dmitshur commented on os: make tests pass even without Administrator account3d
(3 comments)
Consider changing this to:
```
// hasLink reports whether the test filesystem ...
func hasLink ...
```
Based on https://go.dev/s/style#mixed-caps.
Since the first two return values f…dmitshur closed a change golang.org/x/mobile: Create SECURITY.md3d
Last Week
dmitshur closed a change golang.org/x/mobile: Create SECURITY.md4d
dmitshur starred github.com/AlekSi/xfail4d
dmitshur reviewed +2 on cmd/go: return non-zero when go tool is interrupted4d
Thanks.
This is pretty minor so either one will do. `go 1.24.0` is fine because this go.mod file uses `tool` directives and those were introduced in Go 1.24, so it can't be lower than that. `go 1.26…dmitshur commented on golang.org/x/build/main.star: limit scope of slow openbsd-riscv64 host to go, net, sys4d
Thanks.dmitshur commented on golang.org/x/build/cmd/gopherbot: apply the FixPending label to issues with pending cls5d
(3 comments)
Have you tried running this task locally with -dry-run flag? Does the output match your expectations?
How long does it take to run? There's quite a bit of regexp matching. Some of the …dmitshur commented on golang.org/x/build/cmd/gopherbot: track Gerrit "Fixes" CLs by applying the "FixPending" label5d
This sounds like a useful automatic behavior. It would help make existing CLs (or lack thereof) easier to see, and it would reduce some of the toil involved in [issue gardening](https://go.dev/wiki/G…dmitshur reviewed +2 on golang.org/x/build/internal/task: remove leading newline in fake binary def5d
Thanks.dmitshur reviewed +2 on golang.org/x/build/cmd/watchflakes: label Automation when opening a new issue5d
Thanks.dmitshur commented on golang.org/x/build/cmd/watchflakes: label open issues5d
I agree that -author:gopherbot is probably a reasonable way to filter out such issues. Still, since the Automation label exists and its description matches, watchflakes might as well use it.
Note th…dmitshur commented on cmd/go: return non-zero when go tool is interrupted5d
Thanks for finding where exactly in the tool layer the problem was. I left some minor suggestions.
I'm a bit surprised that this exported function isn't documented. https://go.dev/doc/comment sugges…dmitshur commented on golang.org/x/build: extend main Go repo's "every vet check is forcibly made high-confidence" behavior to golang.org/x modules5d
Yes, we can also implement it that way.dmitshur commented on golang.org/x/build: go vet std cmd5d
My recollection of the current behavior from issue #74011 is that:
- in the main Go repository, `go test` has a special case to run almost all vet checks that `go vet` would run, not only the usual …dmitshur commented on golang.org/x/build: extend main Go repo's "every vet check is forcibly made high-confidence" behavior to golang.org/x modules5d
It seems that the recent #79622 is another instance of friction caused by skew, though a slightly different case compared to the more common vendoring case described here.
Based on discussion above,…dmitshur commented on golang.org/x/build/devapp/owners: add primary owners for x/review5d
Thanks.
I interpret no comment from Austin as no objection, but otherwise I can follow up. Submitting. (Even if our code owner tracking system is imperfect, this should make it slightly more useful …dmitshur commented on go/types: add String methods5d
Note that the TestAPIConsistency test in golang.org/x/exp/typeparams starts to fail at tip as of this change:
```
=== RUN TestAPIConsistency
typeparams_test.go:46: "*Instance.String": got func…dmitshur reviewed +2 on .github: update PULL_REQUEST_TEMPLATE to match the latest instruction5d
Thanks for doing this. A few minor suggestions.
Let's remove 'under' to keep this in sync with the other template, and s/./,/ too.
```suggestion
+ Keep the title as short as possible, ideally 72 …dmitshur reviewed +2 on golang.org/x/dot-github/.github: add default PR template5d
Thanks.dmitshur reviewed +2 on golang.org/x/vulndb/...: bump x/net to 0.55.05d
Thanks.dmitshur reviewed +2 on cmd/go/testdata/script: remove get_404_meta net dependency5d
Thanks.
One of the https://pkg.go.dev/net/http#ResponseWriter behaviors is that:
> If WriteHeader is not called explicitly, the first call to Write will trigger an implicit WriteHeader(http.StatusO…dmitshur commented on golang.org/x/build/internal/task: don't consider misc-compile builder signal when tagging5d
Thanks.dmitshur opened a change golang.org/x/build/internal/task: don't consider misc-compile builder signal when tagging5d
Misc-compile builders are primarily intended for pre-submit, to help
quickly catch cases when a non-first class port (one that may not have
a builder in the default pre-submit builder set) starts to …dmitshur opened a change golang.org/x/build/main.star: remove known issue for x_net-gotip-linux-amd64-misccompile5d
It was fixed in CL 781920.
For golang/go#79605.
For golang/go#79607.dmitshur commented on golang.org/x/dot-github/.github: add default PR template5d
(1 comment)
Since this sentence talks about "repo other than golang/go", it might be more helpful to show an example of that. Maybe vscode-go is a reasonable example of a Go repo with its own dedica…dmitshur reviewed +2 on golang.org/x/dot-github/.github: add default PR template5d
Thanks.
Is the reason this file is placed in a .github/PULL_REQUEST_TEMPLATE directory so that if we want later on to add another template, the diff will be smaller? Right now it's a single template…dmitshur reviewed +2 on syscall: add export linknames for socketcall on S390X5d
Thanks.dmitshur reviewed +2 on syscall: add export linknames for socketcall on S390X5d
Thanks.
It's true that these are accessed via assembly in x/sys/unix (and in x/net/internal/socket) for linux/386. However, on linux/390x, they don't seem to be accessed in x/sys/unix at all, only i…dmitshur reviewed +1 on golang.org/x/vulndb/data/reports: add 1 first-party report5d
(1 comment)
The fix for the PUBLIC-track issue #78916 (CVE-2026-39824), CL 770080, was submitted on April 23rd. It got picked up when the monthly tagging workflow ran on May 8th and tagged x/sys at …dmitshur opened an issue golang.org/x/build/internal/task: TagXReposTasks.findMissingBuilders should consider individual ports within -misccompile builders6d
The tagging workflow's findMissingBuilders method already has logic to block on failures within first-class ports. A -misccompile builder may fail due to any number of port(s) running into a problem,…dmitshur opened an issue syscall, x/net/bpf, x/net/icmp, x/net/internal/socket, x/net/ipv{4,6}: package tests fail to build on linux/s390x at tip with "invalid reference to syscall.socketcall"6d
Based on https://ci.chromium.org/p/golang/g/x-net-gotip-by-go/console?limit=200, it seems that Go tip (1.27 to be) starting with [CL 761481](https://go.dev/cl/761481) runs into a build error in x/net…dmitshur reopened an issue runtime: `SIGSEGV` with a c-shared library and called from C#6d
Would it be worthwhile to try making the 32K → 16K change @ianlancetaylor suggested in https://github.com/golang/go/issues/78883#issuecomment-4310496625? It's hard to track this issue since it's cu…dmitshur fixed an issue cmd/link/internal/ld: TestAbstractOriginSanity failures1w
The original failure from Jan 8 was likely due to #77515,
and today's failures were handled by [CL 780760](https://go.dev/cl/780760)
(issue #79547 tracks the problem and removing the skip; CC @prattm…dmitshur commented on cmd/go: clean should honor the -o flag as a build parameter so it can remove the final built artifacts1w
@matloob Is this intended to be a [proposal](https://go.dev/s/proposal)? It's in the Proposal milestone, but doesn't have a "proposal: " prefix in the title nor Proposal label. I fixed that assuming …dmitshur commented on cmd/dist: all.bash does not fail when run.bash fails1w
As a baseline, the `go test` command does return a non-zero exit code on Ctrl+C (SIGINT):
```shell
src $ go test all
ok archive/tar (cached)
ok archive/zip (cached)
ok bufio (cached)
^C
src $ …dmitshur opened an issue cmd/go: sending an interrupt signal (SIGINT) to a go tool invocation of a tool causes it to return with a zero exit code1w
Here's a small reproducer for `go tool` itself, found while looking into #79535:
```shell
$ cd $(mktemp -d)
$ go mod init test && go get go@1.26.0
$ mkdir -p cmd/importantwork && echo 'package main;…dmitshur commented on cmd/compile: Bug in rewrite rules for AMD64 causes SHL instruction overflow and miscompilation [1.25 backport]1w
The Critical label tracks upstream issues; it was applied there (https://github.com/golang/go/issues/79182#event-25767278742) so removing it here.dmitshur commented on cmd/compile: Bug in rewrite rules for AMD64 causes SHL instruction overflow and miscompilation [1.26 backport]1w
The Critical label tracks upstream issues; it was applied there (https://github.com/golang/go/issues/79182#event-25767278742) so removing it here.Earlier
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 time1y
July 4, 2024. Today I got a chance to try out the Apple Vision Pro. :)