Activity
Yesterday
dmitshur commented on cmd/go: add information about GODEBUGs to go help gopath12h
(1 comment)
The issue and this part of the commit message say 'GO111MODULE=auto', but the text being added says 'GO111MODULE=off'. As I understand, standard library tests work with the default moder…dmitshur reviewed +2 on cmd/go: add information about GODEBUGs to go help gopath12h
Thanks.
One too many closing double quotes here.dmitshur commented on golang.org/x/build/cmd/watchflakes: stop applying NeedsInvestigation label12h
(2 comments)
This part of the postNew's documentation seems out of date. Same on line 415.
No need to change it now; just noting that since this ends up being the only place where the constant is n…dmitshur reviewed +2 on golang.org/x/build/cmd/watchflakes: stop applying NeedsInvestigation label12h
Thanks.
I suggest filing some tracking issue for this change, and adding a 'Fixes golang/go#nnn.' line here.
This change will add to the incoming volume of issues that end up in the https://go.dev/…dmitshur reviewed +2 on golang.org/x/build/main.star: remove linux-amd64-spectre known issue12h
Thanks. Note that I rebased and regenerated this change in PS 2.dmitshur commented on cmd/dist: add GO_TEST_ASMFLAGS to set -asmflags during dist test12h
(1 comment)
gogcflags has "all=" implicitly prepended; removed here in PS 4.dmitshur commented on golang.org/x/build/main.star: set {gc,asm}flags via GO_{GC,TEST_ASM}FLAGS in spectre runmod12h
(1 comment)
I missed that GO_GCFLAGS differs from setting -gcflags directly in that its value gets an implicit "all=" prefix [already prepended](https://cs.opensource.google/go/go/+/master:src/cmd/d…dmitshur reviewed +2 on cmd/go/testdata/vcstest: explicitly set invalid-version's branch to main14h
Glad to see the fix ended up this small. Thanks.
It's probably fine to backport then, if this is all it takes to to unblock the builder on 1.26+1.25 release branches too. I'll check if that's the ca…dmitshur commented on cmd/dist: add GO_TEST_ASMFLAGS to set -asmflags during dist test15h
(1 comment)
Probably cleaner to add a field to tester instead of a global. It was already not being given any value if cmdtest wasn't run, but seems nicer to make it a clear build error if accidenta…dmitshur commented on cmd/dist: add GO_TEST_ASMFLAGS to set -asmflags during dist test15h
(1 comment)
This struct field and associated code is very easy to add anew when/if we have a need to handle it differently per test, but that's not the case so far, so I'll avoid adding it and makin…dmitshur opened a change golang.org/x/build/main.star: make linux-amd64-longtest-git2.24.0 available in presubmit16h
Being able to optionally request it in presubmit should be helpful when
developing fixes for go.dev/issue/78323.
For golang/go#26653.dmitshur commented on cmd/go: support for older versions of git broken again17h
The new [*-git2.24.0](https://ci.chromium.org/ui/p/golang/g/port-linux-amd64/builders?q=-git2.24.0) builder isn't passing all tests yet, but it's very close to it. Filed a followup issue #78323 for i…dmitshur commented on golang.org/x/build: add a builder with oldest git supported by cmd/go to catch regressions17h
[crrev.com/c/7694655](https://crrev.com/c/7694655) and [go.dev/cl/758381](https://go.dev/cl/758381) added a builder that tests with git 2.24.0, the latest git minimum version based on https://github.…dmitshur opened an issue cmd/go: TestScript/mod_invalid_version and ./internal/vcweb/vcstest.TestScripts/git/invalid-version.txt fail with git 2.24.017h
The builder with the minimum supported git version is being added in issue #26653, and so far it seems all but these two tests are passing with it:
- [cmd/go.TestScript/mod_invalid_version](https://…dmitshur reviewed +2 on golang.org/x/build/main.star: add mapsplitgroup builder17h
Thanks.dmitshur commented on test/fixedbugs: [release-branch.go1.26] remove issue46234 test timeout21h
net/http.TestOptions/h1 is a pre-existing flaky test tracked in issue #69002. Not related to this change, and everything else passed, so applying TryBot-Bypass.dmitshur commented on golang.org/x/benchmarks/sweet/common: invoke go env with -json flag21h
Thanks.dmitshur opened a change golang.org/x/benchmarks/sweet/common: invoke go env with -json flag21h
The go command supports the -json flag to make it convenient to parse
its output programmatically. Since human-readable output isn't needed
here, switch to using the -json flag.dmitshur opened a change test/fixedbugs: [release-branch.go1.25] remove issue46234 test timeout21h
issue46234 is flaky on slow machines due to the possibility of running
into the constant timeout. Delete it in favor of timeouts applied at a
higher level.
Note that that test timeout already got in…dmitshur opened a change test/fixedbugs: [release-branch.go1.26] remove issue46234 test timeout21h
issue46234 is flaky on slow machines due to the possibility of running
into the constant timeout. Delete it in favor of timeouts applied at a
higher level.
Note that that test timeout already got in…dmitshur commented on test: issue46234.go failures with "command exceeded time limit"21h
@gopherbot Please consider this for backport. This is a test-only fix that removes a constant timeout for one subtest (in favor of timeouts applied at a higher level), and is needed to get the darwin…This Week
dmitshur commented on golang.org/x/build/main.star: add linux-amd64-longtest-git2.24.0 builder1d
Thanks.
I was too (in [this thread](https://go-review.googlesource.com/c/build/+/758381/comment/fbb41a80_6bf219c7/)). :) It seems something like "min" might be a good fit for builders where the mini…dmitshur commented on internal/cpu: support windows/arm641d
As for where the GOROOT/src/go/build/deps_test.go dependency policy should be discussed: this issue is fine. You can find reasonable examples of where it last changed via git history (e.g., [here](ht…dmitshur commented on golang.org/x/build/main.star: add linux-amd64-longtest-git2.24.0 builder1d
(1 comment)
Done. We already use dots in some builder name versions, like "freebsd-amd64_15.0", so done the same here since git2.24.0 is easier to read than git2240.dmitshur commented on golang.org/x/build/main.star: add linux-amd64-longtest-mingit builder1d
(1 comment)
Hmm, maybe it'll be simpler in the long run to give this a specific version. That way when this eventually becomes a newer version, it can be added as a separate runmod with a separate k…dmitshur opened a change golang.org/x/build/main.star: add linux-amd64-longtest-mingit builder1d
For golang/go#26653.dmitshur commented on golang.org/x/image/tiff: cap buffer growth to prevent OOM from malicious IFD offset1d
Thanks.
No, since this is ready from your side and all submit requirements are met, it just needs someone with submit access to either press the 'Submit' button directly or to add an Auto-Submit+1 v…dmitshur commented on golang.org/x/benchmarks/cmd/bench: compute distribution size1d
(1 comment)
There's no date in `go env -json GOVERSION`; see related [comment](https://go-review.googlesource.com/c/benchmarks/+/758120/comment/aa9f0f2d_c7bb460d/).dmitshur reviewed +2 on golang.org/x/benchmarks/cmd/bench: compute distribution size1d
Thanks.
This seems like it might run into an off-by-one-character problem, unless I missed something. go/version doesn't tolerate extraneous characters (e.g., https://go.dev/play/p/eGgX4tlSAXs), so …dmitshur commented on builtin: [release-branch.go1.26] incorporate all feedback into doc string for new1d
(1 comment)
Fix applied.dmitshur commented on testing: within a B.Loop loop, assigning function result to _ allows body to be optimized away (1.26 regression) [1.26 backport]1d
@JunyangShao This backport issue is approved, and it needs a backport CL to be created as the next step here. Since you authored the original CL, mind doing that? Note that there is a merge conflict …dmitshur commented on golang.org/x/tools/go/analysis/passes/inline: [internal-branch.go1.26-vendor] fix panic in inlineAlias with instantiated generic alias1d
CL 758300 vendors this change on main Go repo's release-branch.go1.26 branch.dmitshur opened a change cmd/fix: [release-branch.go1.26] pull in CL 7559801d
Fixes #78191.
[git-generate]
cd src/cmd
go get golang.org/x/tools@internal-branch.go1.26-vendor # v0.39.1-0.20260323181443-4f499ecaa91d
go mod tidy
go mod vendordmitshur commented on crypto/x509: quadratic work in policy validation with large PolicyMappings1d
CC @rolandshoemaker.dmitshur commented on crypto/x509: excessive chain-building work in Verify with many candidate parents1d
CC @rolandshoemaker.dmitshur commented on builtin: [release-branch.go1.26] incorporate all feedback into doc string for new1d
(1 comment)
```suggestion
Fixes #77586.
```
Will apply after trybots finish running to avoid interrupting them.dmitshur opened a change builtin: [release-branch.go1.26] incorporate all feedback into doc string for new1d
The previously submitted CL didn't include some of the feedback
that was provided on the CL. Specifically, it didn't mention the
behavior of `new` for untyped constant arguments.
For #77584.
Fixes #…dmitshur opened a change builtin: [release-branch.go1.26] update new function comment1d
The function comment for new does not reflect the recent change in behaviour since Go 1.26.
For #77584.
For #77586.dmitshur commented on golang.org/x/build: add LUCI linux-riscv64-rva22u64 builder1d
@mengzhuo Thanks, here's the corresponding certificate: [linux-riscv64-rva22u64-mengzhuo-1774283578.cert.txt](https://github.com/user-attachments/files/26188379/linux-riscv64-rva22u64-mengzhuo-177428…dmitshur reviewed +2 on golang.org/x/benchmarks/cmd/bench: compute distribution size1d
Thanks.
It made sense to use sweet's packages when this was a benchmark in sweet, but now that it's not, provided it works out well to use https://pkg.go.dev/os#CopyFS and runtime.GOOS/GOARCH direct…Last Week
dmitshur reviewed +2 on Revert "runtime: disable test for 78081"4d
Thanks.dmitshur commented on runtime: disable test for 780814d
Sounds good, thanks.dmitshur commented on spec: predeclared types are named, not defined types4d
TryBots did pass on patch set 1, and patch set 2 was a rebase. The result failed only because the rebase interrupted the trybots that started running. So, adding a bypass.dmitshur reviewed +2 on runtime: disable test for 780814d
Since trybots are passing at CL 757343, could it work okay to add TryBot-Bypass to CL 757342 and let them both submit, and then you don't need this CL? Or is the goal of this CL to avoid having a fai…dmitshur reviewed +2 on golang.org/x/benchmarks/x/benchmarks: add a sweet benchmark for distribution size4d
I'm not very familiar with the APIs and conventions of x/benchmarks; @mpratt@google.com or @drchase@google.com might have more useful review comments or suggestions.
That said, this seems reasonable…dmitshur fixed an issue cmd/internal/moddeps: TestAllDependencies failures during migration to pkgbits.V34d
dmitshur fixed an issue cmd/internal/moddeps: TestAllDependencies failures during migration to pkgbits.V34d
dmitshur commented on all: update to x/tools@5d7afbc08aec4d
Thanks.dmitshur reopened an issue internal/cpu: support windows/arm644d
Reopening this issue because [CL 740880](https://go.dev/cl/740880) was rolled back ([CL 757482](https://go.dev/cl/757482)).
Adding an import of golang.org/x/sys/windows inside the golang.org/x/sys/c…dmitshur reopened an issue internal/cpu: support windows/arm644d
Reopening this issue because [CL 740880](https://go.dev/cl/740880) was rolled back ([CL 757482](https://go.dev/cl/757482)).
Adding an import of golang.org/x/sys/windows inside the golang.org/x/sys/c…dmitshur commented on golang.org/x/sys/cpu: use IsProcessorFeaturePresent to calculate ARM64 on windows4d
This change unfortunately needed to be rolled back in CL 757482. I'll reopen #76791 for applying it again or other followup.dmitshur commented on all: update to x/tools@5d7afbc08aec4d
(1 comment)
Done in PS 3.dmitshur commented on golang.org/x/sys/cpu: roll back "use IsProcessorFeaturePresent to calculate ARM64 on windows"4d
Thanks. I'll reopen #76791 afterwards for the follow up.
To be clear, the discussion that happens may conclude that we should permit x/sys/cpu to import x/sys/windows, and then this exact same CL ca…dmitshur commented on go/build: latest version of x/sys/cpu is incompatible with dependency policy in the standard library4d
CC @golang/runtime, @golang/windows.dmitshur commented on all: update to x/tools@5d7afbc08aec4d
(2 comments)
It does seem that we may be very close to removing all remaining uses of bundle here, but at this moment it's still used in [3 places](https://cs.opensource.google/search?q=%22%2F%2Fgo:…dmitshur opened a change golang.org/x/sys/cpu: roll back "use IsProcessorFeaturePresent to calculate ARM64 on windows"4d
Roll back CL 740880 (commit fc646e489fd944b6f77d327ab77f1a4bab81d5ad)
because it adds a golang.org/x/sys/windows import to the cpu package.
The cpu package is vendored into the main Go tree and the p…dmitshur opened an issue go/build: latest version of x/sys/cpu is incompatible with dependency policy in the standard library4d
Currently at tip, the only package from x/sys vendored into the std module of the main Go repo is [x/sys/cpu](https://cs.opensource.google/go/go/+/master:src/vendor/modules.txt;l=19-20;drc=ca5ffe0092…dmitshur commented on cmd/internal/moddeps: TestAllDependencies/std(thorough) failures4d
[CL 757520](https://go.dev/cl/757520) will fix this.dmitshur opened a change all: update to x/tools@5d7afbc08aec4d
Pull in CL 757060 to get x/tools/cmd/bundle working again.
For #36905.
For #9859.
[git-generate]
cd src/cmd
go get golang.org/x/tools@v0.43.1-0.20260319213245-5d7afbc08aec # CL 757060
go mod tidy
…dmitshur opened a change golang.org/x/build/main.star: add x/website presubmit builder for all of GOROOT/doc4d
The GOROOT/doc directory contains a few HTML and Markdown files like
godebug.md, go_spec, asm.html and such. CL 746740 updates TestAll in
x/website to test the files in GOROOT/doc directory too. Upda…dmitshur fixed an issue golang.org/x/playground: error communicating with remote server on 2026-03-204d
That particular version of the 1.26 playground backend got into an unhealthy state for reasons that aren't yet understood. Since this is the first occurrence of this specific failure mode for the pla…dmitshur commented on golang.org/x/playground: error communicating with remote server on 2026-03-204d
That particular version of the 1.26 playground backend got into an unhealthy state for reasons that aren't yet understood. Since this is the first occurrence of this specific failure mode for the pla…dmitshur commented on golang.org/x/website/cmd/golangorg: catch instances of bad escaping due to template use4d
(1 comment)
This change is ready for review.
That CL landed so tip builders should pass now. I expect the 1.26/1.25 builders to still run into this until CL 757420 and CL 757421 land, but otherwise…dmitshur opened a change doc: [release-branch.go1.25] remove template use in godebug.md4d
This file used to be implicitly a template prior to CL 733500, but now
it's no longer a template. The only template use here can be trivially
expressed with pure Markdown syntax, so do that. (The alt…dmitshur opened a change doc: [release-branch.go1.26] remove template use in godebug.md4d
This file used to be implicitly a template prior to CL 733500, but now
it's no longer a template. The only template use here can be trivially
expressed with pure Markdown syntax, so do that. (The alt…dmitshur commented on doc: go.dev/doc/godebug page contains "{{raw" due to misconfigured template use4d
The fix is visible at https://tip.golang.org/doc/godebug now, but not yet at https://go.dev/doc/godebug. We still need to backport it, since go.dev serves content from a stable release branch. This i…dmitshur commented on golang.org/x/playground: error communicating with remote server on 2026-03-204d
The immediate problem should be resolved. Thanks @dolmen for confirming.
I'll keep this issue open for some time since we're still looking more into what happened and what we can do to reduce chance…dmitshur commented on golang.org/x/playground: Error communicating with remote server4d
Thanks for reporting this. We're working on resolving this, and will update this issue once this is fixed.
As the original issue noted, "Go 1.25 and gotip backends are working at the moment.", so as…dmitshur commented on golang.org/x/crypto/ssh: respect signer's algorithm preference in pickSignatureAlgorithm5d
(1 comment)
Yes, fixing a bug doesn't need a proposal. It sounds like you should either update #746020 to describe the bug instead of the original proposal, or if the original proposal should still …dmitshur reviewed +2 on test/convert5: [release-branch.go1.26] skip mipsle5d
By now the builder is passing at tip, 1.25 release branch, x/ repos (all but x/tools), leaving 1.26 with only this test failure: https://ci.chromium.org/ui/p/golang/g/port-linux-mipsle/builders. Back…dmitshur commented on runtime: segmentation violation when debug and using race detector [1.26 backport]5d
Perhaps it could help to also include the test-only follow up in [CL 756780](https://go.dev/cl/756780).dmitshur commented on golang.org/x/build/maintner: add TrackGitHub method variant taking an oauth2.TokenSource5d
(1 comment)
Makes sense.
> seems like it!
To an extent. :) Issue #37603 is still applicable today. For instance, it's taken me a while to find time to review CL 726080 partly because managing risk…dmitshur commented on golang.org/x/build/maintner: migrate to bartventer/httpcache5d
(3 comments)
Will this work as is, or does this package also need to import `github.com/bartventer/httpcache/store/memcache` for side-effect or registering that store?
Reviewed the new dependency b…dmitshur reviewed +1 on debug/buildinfo: remove macho/elf DataStart fallback code5d
This generally looks good to me, leaving +1 so that Cherry can also look.
A side note: if you'd like, URLs like this can be shortened by using Go website's newer domain, such as go.dev/cl/173343. (T…dmitshur commented on golang.org/x/build: publish Go source code releases to the Checksum Database5d
Thanks for filing this feature request.
As background context, the golang.org/toolchain modules exist primarily to serve the internal needs of the cmd/go command and its https://go.dev/doc/toolchain…dmitshur reviewed +2 on cmd/go: make all script tests parallel5d
Thanks.
I understand the new state is that testsuite no longer supports testdata at all, since it doesn't support serial test execution at all anymore. The old phrasing in this Fatal message seems a…dmitshur commented on cmd/distpack: add a test on distribution size5d
Thanks for working on this. I left some high level comments to get your feedback; that said, please let me know if you'd find it useful to consider them out of scope for this issue/CL, and instead ai…dmitshur commented on doc: remove template use in godebug.md6d
Thanks.dmitshur commented on doc: fix 2 links in godebug.md6d
Thanks.
Done.dmitshur commented on doc: document the gotypesalias removal6d
(1 comment)
I think the intent here was to use markdown link syntax, which is `[text](url)`. [a][b] shows up as is, visible at https://tip.golang.org/doc/godebug#go-127. Sent CL 756580.dmitshur opened a change doc: fix 2 links in godebug.md6d
For #76349.dmitshur opened a change doc: remove template use in godebug.md6d
This file used to be implicitly a template prior to CL 733500, but now
it's no longer a template. The only template use here can be trivially
expressed with pure Markdown syntax, so do that. (The alt…dmitshur opened an issue doc: go.dev/doc/godebug page contains "{{raw" due to misconfigured template use6d
The https://go.dev/doc/godebug page has some unintended template use showing up:
<img width="481" height="37" alt="Image" src="https://github.com/user-attachments/assets/a052c838-6947-44bd-870f-9a5a…dmitshur opened a change golang.org/x/website/cmd/golangorg: support absolute paths in testTree6d
The use of strings.TrimPrefix on filepath.ToSlash output works on all
OSes when testTree is given a relative path like "../../_content",
but it doesn't work on Windows if testTree is given an absolut…dmitshur commented on golang.org/x/build/maintner: add TrackGitHub method variant taking an oauth2.TokenSource6d
(1 comment)
Can the potential latency be dealt with by wrapping with something like [oauth2.ReuseTokenSource](https://pkg.go.dev/golang.org/x/oauth2#ReuseTokenSource)?dmitshur reviewed +2 on golang.org/x/build/maintner: add TrackGitHub method variant taking an oauth2.TokenSource6d
Thanks.
I think we could start using `tok.SetAuthHeader(req)` here now that there's an `oauth2.Token`, if you want. But this should be equivalent at least for today's needs, so either way is fine. (…dmitshur commented on runtime: Windows crash with Go 1.26.0, 1.26.1 [1.26 backport]6d
Moving Critical label to [the upstream issue](https://github.com/golang/go/issues/77975#event-23683617381).dmitshur closed an issue golang.org/x/net/http2: TestTransportFailureErrorForHTTP1Response/with_enough_frame_size_to_start_reading failures1w
Merged into #72981.dmitshur commented on cmd/go/internal/verylongtest: check MustHaveGoBuild in TestGoBuildUmask1w
misccompile builder failure looks like #78081. It also passed in a [previous attempt](https://ci.chromium.org/b/8687137151622998929) on the same patch set. Importantly, noopt ran and passed. Bypassin…dmitshur commented on cmd/go/internal/test: enable stdversion by default1w
(1 comment)
@sean@liao.dev This seems like a change that should be in the release notes, right? If you don't mind, please add a note for it here. Thanks.dmitshur commented on golang.org/x/build/{influx,perf}: build Docker images via Cloud Build instead of local Docker1w
Thanks.dmitshur commented on golang.org/x/build/influx: move influx command to x/build/cmd/influx1w
Thanks.dmitshur commented on golang.org/x/build/perf: move perf command to x/build/cmd/perf1w
Thanks.dmitshur commented on : make `go test -buildmode=c-archive/c-shared` useful1w
@ethanmontgomery148-hub Please see https://go.dev/wiki/NoPlusOne. You've also left a lot of comments on other Go issues recently. Please be aware of the [Go Code of Conduct](https://go.dev/conduct), …Earlier
dmitshur pushed to
main in dmitri.shuralyov.com/service/change10modmitshur 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. :)dmitshur pushed to
main in dmitri.shuralyov.com/scratch1ydmitshur created package in dmitri.shuralyov.com/scratch/quine1y
dmitshur merged a change dmitri.shuralyov.com/scratch: quine: add package quine1y
dmitshur reviewed +2 on dmitri.shuralyov.com/scratch: quine: add package quine1y
LGTM. Thanks, and congrats on your first new Go package here!
Done in PS 2.
Patch set 2 seems to resolve this.