Activity
Last Week
dmitshur reviewed +2 on cmd: update x/tools to 59ff18c1d
Thanks.
(nit) Since this is the main repo, the "golang/go" prefix isn't needed and could be left out. Other than a few extra characters, including it is harmless though.dmitshur commented on golang.org/x/mod/x/mod: apply go fix and go vet1d
Thanks.
It can sometimes be handy to do pure algorithmic changes in patch set 1, and the manual followups in patch set 2, and leave a comment about that. That way the end result code being checked i…dmitshur opened a change golang.org/x/mod/sumdb/note: delete chop1d
CL 720581 replaced almost all instances of it, this gets the last one.
[git-generate]
cd sumdb/note
rf '
ex { import "strings"; chop -> strings.Cut }
rm chop
'dmitshur reviewed +2 on std: fix printf("%q", int) mistakes1d
Thanks.dmitshur reviewed +2 on golang.org/x/mod/x/mod: apply go fix and go vet1d
Thanks. This generally looks good, but I left some suggestions to avoid leaving some of the code in a state where it's doing something in a more verbose/complicated way, making it harder to read. If …dmitshur commented on go/version: use "custom" as an example of a version suffix1d
Thanks.dmitshur opened a change golang.org/x/telemetry/internal/telemetry: update test for valid version with GOEXPERIMENT1d
Add a test case for GOEXPERIMENT toolchain versions after CL 719701.
For golang/go#75953.dmitshur commented on go/version: does not support the `X:` suffix from `go env GOVERSION`1d
@qmuntal On the point about telemetry, I think that is 1) working as intended and 2) there's no change being introduced here.
In the general case, a toolchain built with an experiment may differ in …dmitshur opened a change go/version: use "custom" as an example of a version suffix1d
The suffix in a non-standard toolchain version can be any string. Show
more of a middle ground example of a non-standard version suffix,
aligning it with the example used at https://go.dev/doc/toolch…dmitshur commented on go/version: does not support the `X:` suffix from `go env GOVERSION`1d
The suffix after the dash of a non-standard toolchain can be any string, and at https://go.dev/doc/toolchain#name the example shown uses "custom" rather than "bigcorp". Perhaps it'd be more clear if …dmitshur opened a change cmd/internal/objabi: remove -V=goexperiment internal special case1d
This special case was added in CL 310171 for test/run.go use, as the
comment still says, but run.go (cmd/internal/testdir/testdir_test.go
by now) stopped using this in CL 310732. There don't seem to …dmitshur reviewed +2 on cmd/go: add test checking version with experiment is valid1d
Thanks.
I suspect that it will be possible to cause this test to fail by doing something like:
```
cat "bad" > VERSION
cd src && ./make.bash
```
That is, since it's possible to set a custom versio…dmitshur commented on cmd/link/internal/ld: make runtime.buildVersion with experiments valid1d
(1 comment)
If you want, consider modifying this test case to also check that version.IsValid(runtime.Version()) isn't unexpectedly false.
It's probably not going to catch every instance of regress…dmitshur reviewed +2 on cmd/link/internal/ld: make runtime.buildVersion with experiments valid1d
Thanks.
That works, and has the advantage of being less likely to run into a false positive match.dmitshur commented on runtime/secret: implement new secret package2d
A new builder is typically appropriate when the goal is to run all of the Go tests under a different mode. For example, if the GOEXPERIMENT modifies runtime behavior.
If the change of behavior here …dmitshur reviewed +2 on golang.org/x/tools/internal/analysis/analyzerutil: fix FileUsesGoVersion3d
Thanks.
You added a Auto-Submit+1 vote to patch set 2, which had no effect because patch set 3 was latest, and that vote doesn't transfer between patch sets. Re-adding it to PS3 which I understand w…dmitshur reviewed +2 on golang.org/x/text/...: eliminate vet diagnostics3d
Thanks.
Yes, that seems like a false positive in that vet check, especially since the package documentation at https://pkg.go.dev/testing#hdr-Benchmarks shows examples of for b.Loop() loops, and non…dmitshur reviewed +2 on golang.org/x/tools/internal/analysis/analyzerutil: fix FileUsesGoVersion3d
Thanks.
(nit) What's the motivation to include an underscore prefix in the result bool name?
I don't quite see how this comment is connected to the code, `stdlib.IsBootstrapPackage(pkgpath)` as I i…dmitshur reviewed +2 on golang.org/x/build/main.star: invert greenteagc and sizespecializedmalloc builders3d
Thanks.dmitshur commented on golang.org/x/tools/go/analysis/passes/modernize: respect bootstrap toolchain version4d
(1 comment)
@adonovan@google.com Would it make sense to add a special case here to not apply the lower BootstrapVersion to package tests?
The packages used during bootstrap do need to build and wor…dmitshur reviewed +2 on std: go fix -any std cmd4d
Thanks.
None of these tests were sensitive to whether interface{} or any is used.
This variable name becomes more indirect than it was before, but it seems fine.dmitshur commented on golang.org/x/website/internal/dl: unpredictably applied HTTP transport compression for .tar.gz resources results in negligible bandwidth savings while confusing contemporary versions of Firefox4d
@luismferreira You should confirm that it's not your environment forcing the go1.25.0 toolchain to [be selected](https://go.dev/doc/toolchain#select). Try re-running `go version` with `GODEBUG=toolch…dmitshur reviewed +2 on golang.org/x/text/...: eliminate vet diagnostics4d
Thanks.
Would it be better to resolve the unused result by modernizing this benchmark with the `testing.B.Loop` API?
```
func BenchmarkString(b *testing.B) {
for b.Loop() {
USD.String()
}
}
```…dmitshur reviewed +2 on golang.org/x/perf/...: eliminate vet diagnostics4d
Thanks.
I'll note that compared to other things vet reports, it's not clear to me that report is in this context is very useful. The parse.SyntaxError type is defined in an internal package relative…dmitshur reviewed +2 on golang.org/x/image/...: eliminate vet diagnostics4d
Thanks.
I see. Well, if we're going to rely on being able to generate these files, the generator program probably needs to be added to this repo where it can be modified more easily.
But this packa…dmitshur commented on golang.org/x/image/...: eliminate vet diagnostics5d
(1 comment)
These two files are generated. Won't the change be undone by next 'go generate' invocation? Or is the generator updated already?dmitshur reviewed +2 on golang.org/x/build/...: eliminate vet diagnostics5d
Thanks.Earlier
dmitshur commented on golang.org/x/website: sha256sum for go1.25.4.linux-amd64.tar.gz download does not match1w
9fa5ffeda4170de60f67f3aa0f824e426421ba724c21e133c1e35d6159ca1bec is the right sha256 checksum for the go1.25.4.linux-amd64.tar.gz file. It can be reproduced with distpack by building Go 1.25.4 from s…dmitshur closed a change path/filepath: [release-branch.go1.24] return cleaned path from Rel1w
Thanks for preparing the CL. Closing since the decision in #75817 is not to backport.dmitshur closed a change path/filepath: [release-branch.go1.25] return cleaned path from Rel1w
Thanks for preparing the CL. Closing since the decision in #75818 is not to backport.dmitshur commented on path/filepath: [release-branch.go1.25] return cleaned path from Rel1w
(1 comment)
No, this was just a temporary hold to make it clear that this CL was waiting on a decision to happen in the backport issue #75818. If the backport issue were approved, the hold would be …dmitshur commented on internal/syscall/windows: [release-branch.go1.25] fix ReOpenFile sentinel error value1w
In this case since Go 1.25.4 is released by now, yes, it is better to create a new backport issue to track this CL. The issue can say it's a follow-up fix for issue #75989 and provide a brief rationa…dmitshur starred github.com/u-root/cpu1w
dmitshur commented on os: ReadDir on regular file returns non nil slice on Windows1w
@stevenh I agree that it shouldn't be needed to skip tests, but I think it's possible to update them not to rely on undocumented behavior, while still checking the property you're looking to test. Se…dmitshur commented on github.com/stevenh/tracktools/chore: update modules1w
I left a few suggestions on how the test cases can be updated to not run into issue golang/go#75157. It should work without needing to skip those tests.
Consider this modified test case:
```diff
…dmitshur commented on internal/syscall/windows: [release-branch.go1.25] fix ReOpenFile sentinel error value1w
Please see https://go.dev/wiki/MinorReleases.
All backport CLs need an accompanying cherry pick issue with a rationale, which we'll need to review before the CL can land. I see the tip CL didn't hav…dmitshur starred github.com/makeworld-the-better-one/dither/v22w
dmitshur commented on cmd/go: behaviour of "go doc" in module mode is not documented3w
CC @golang/command-line.dmitshur commented on golang.org/x/build: add LUCI linux-mips64le builder3w
That hostname should work well, thanks. Here is the corresponding certificate: [linux-mips64le-cip-united-1761320497.cert.txt](https://github.com/user-attachments/files/23127740/linux-mips64le-cip-un…dmitshur commented on golang.org/x/build: add LUCI linux-mipsle builder3w
@Rongronggg9 Thank you for offering that builder to test the linux/mipsle port. Its hardware should work as long as it provides useful signal. Does all.bash pass on it with GOOS=linux GOARCH=mipsle a…dmitshur opened an issue cmd/dist: test -json emits some non-JSON lines for packages without tests3w
When the -json flag is used with `go tool dist test` (added in #37486), it's expected to only write JSON lines.
There seems to be a path where some non-JSON is written to stdout too, which is a bug:…dmitshur closed an issue cmd/compile: any(func(){})==any(func(){}) does not panic but should [1.24 backport]3w
dmitshur fixed an issue golang.org/x/build/internal/relui: TestRelease/major failures3w
Fixed in [CL 711920](https://go.dev/cl/711920).dmitshur closed an issue riscv64: can't build on FreeBSD with cgo [1.24 backport]3w
Closing for now because the freebsd/riscv64 port is [marked broken](https://cs.opensource.google/go/go/+/master:src/cmd/dist/build.go;l=1830;drc=6f4c63ba63fdec6e4a42e2e19ac71937973dedef) at tip and […dmitshur commented on path/filepath: Rel returns non-Clean'ed path. [1.24 backport]3w
Asked for a more detailed backport rationale in https://github.com/golang/go/issues/75763#issuecomment-3433007301.dmitshur commented on path/filepath: Rel returns non-Clean'ed path. [1.25 backport]3w
Asked for a more detailed backport rationale in https://github.com/golang/go/issues/75763#issuecomment-3433007301.dmitshur commented on path/filepath: Rel returns non-Clean'ed path3w
@kolyshkin What's the reason to suggest backporting this issue? We'd need a bit more information to review the backport request.
Our backporting criteria is documented at https://go.dev/wiki/MinorRe…dmitshur commented on builders: linux-mipsle/linux-mips64le builders are missing3w
@MingcongBai Yes, please do.
In general, you could reuse an existing open issue for a given GOOS/GOARCH pair and move on to step 2 at https://go.dev/wiki/DashboardBuilders#how-to-set-up-a-builder (g…dmitshur commented on builders: linux-mipsle/linux-mips64le builders are missing3w
Also see tracking issues #67304 and #67306.dmitshur opened an issue cmd/compile/internal/importer: data race in TestIssue69912 as of CL 6959774w
This is being reported by race builders across various ports at tip:
```
=== RUN TestIssue69912
==================
WARNING: DATA RACE
Write at 0x00c0001aa098 by goroutine 1118:
cmd/compile/inter…dmitshur commented on golang.org/x/pkgsite: still no update for > 2h and counting after a new tag is available on proxy and requested4w
CC @golang/pkgsite.dmitshur commented on cmd/go: `go tool` doesn't list nm, objdump and pprof4w
CC @golang/command-line.dmitshur commented on cmd/pprof: incorrect CPU profile graph4w
CC @golang/runtime, @cherrymui.dmitshur commented on cmd/compile/internal/inline/inlheur: dumpscores test failure4w
Thanks for the report. Does the failure happen reproducibly or intermittently for you?
It seems not to happen on the linux/amd64 builder ([here](https://ci.chromium.org/ui/p/golang/builders/ci/gotip…dmitshur commented on cmd/go: work edit -json doesn't include the toolchain4w
CC @golang/command-line.dmitshur commented on compress/flate: size optimization of final deflate block (btype 0 -> btype 1)4w
CC @dsnet, @mdempsky.dmitshur commented on net/http/httputil: ReverseProxy stuck forever when handling request with expect: 100-continue header and upstream connection creation fails4w
CC @neild, @thatnealpatel.dmitshur commented on cmd/go: env should report GOPACKAGESDRIVER4w
CC @golang/command-line.dmitshur commented on os: on windows RemoveAll removing directories containing read-only files errors with `unlinkat ... Access is denied`4w
CC @golang/windows, @griesemer, @neild.dmitshur commented on path/filepath: improve `Rel`'s documentation4w
CC @robpike, @rsc.dmitshur commented on github.com/actions/setup-go: Fall back to downloading from go.dev/dl instead of storage.googleapis.com/golang4w
Thanks for doing that!
From my perspective, changing download URLs from `https://storage.googleapis.com/golang/{file}` to `https://go.dev/dl/{file}` is the right thing to do, so I'll leave further…dmitshur commented on github.com/actions/setup-go: Fall back to downloading from go.dev/dl instead of storage.googleapis.com/golang4w
Consider using https://go.dev/dl/ itself and following the redirects. It's likely that https://dl.google.com/go/ will remain current for a long time, but if it ends up changing in the future, this wi…dmitshur commented on github.com/actions/setup-go: storage.googleapis.com/golang no longer allows anonymous access causing 403 on Golang download fallback4w
The official Go downloads page (https://go.dev/dl/) contains the most up-to-date URLs for individual Go release files. They follow the pattern `https://go.dev/dl/{file name}`, so for example the go1.…dmitshur commented on github.com/microsoft/azure-pipelines-tasks/[BUG]: gotooling task cannot download go distribution4w
The official Go downloads page (https://go.dev/dl/) contains the most up-to-date URLs for individual Go release files. They follow the pattern `https://go.dev/dl/{file name}`, so for example the go1.…dmitshur commented on golang.org/x/build: add Debian Trixie builders4w
The linux/arm64 trixie builder is up (with a known issue) and seems to be running into some problem with its C toolchain. Some of the packages fail with "running gcc failed [...] cannot find ‘ld’…dmitshur pushed to
main in dmitri.shuralyov.com/service/change6modmitshur 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.dmitshur merged a change dmitri.shuralyov.com/scratch: add AUTHORS.md file1y
dmitshur reviewed +2 on dmitri.shuralyov.com/scratch: add AUTHORS.md file1y
Thanks! If it's ready to be submitted from your side, I can do that now.dmitshur pushed to
main in dmitri.shuralyov.com/html/belt1ydmitshur commented on dmitri.shuralyov.com/scratch: main: add package quine1y
They are. Although Go can make them too easy via `go:embed`.
See some initial comments.
I suggest either adding a quite that can be licensed via the existing BSD-3-Clause LICENSE at the top level o…dmitshur reviewed +2 on dmitri.shuralyov.com/scratch: add AUTHORS.md file1y
Thanks! Congrats on your first change. :D
See one inline comment that's unresolved (yellow border). You can leave a draft comment like "Done." and mark the 'Resolved' checkbox, then when you upload …dmitshur pushed to
main in dmitri.shuralyov.com/go/generated2ydmitshur pushed to
main in dmitri.shuralyov.com/gpu/mtl2ydmitshur pushed to
main in dmitri.shuralyov.com/gpu/mtl2ydmitshur commented on dmitri.shuralyov.com/idiomatic-go: Use defer if its overhead is relatively negligible, or it buys meaningful readability3y
I agree with the points made above. This entry tried to say "use this language feature when it's appropriate; don't use it when it's not", which can be said for any language feature. I'll close this …dmitshur pushed to
master in dmitri.shuralyov.com/service/change3ydmitshur closed an issue dmitri.shuralyov.com/go/generated: update implementation for 2020 spec changes3y
dmitshur merged a change dmitri.shuralyov.com/go/generated: update implementation for 2020 spec changes3y
dmitshur commented on dmitri.shuralyov.com/go/generated: update implementation for 2020 spec changes3y
Go fuzzing support is out of beta and no longer uses this build constraint. It should be replaced with `go1.18` by now.
Done in PS 3.
I'm following the Go style for commit messages, which chooses…dmitshur commented on dmitri.shuralyov.com/go/generated: update implementation for 2020 spec changes3y
Shortened 'golang.org' to 'go.dev' here, and on line 24.dmitshur pushed to
master in dmitri.shuralyov.com/website/gido4ydmitshur opened an issue dmitri.shuralyov.com/go/generated: update implementation for 2020 spec changes4y
dmitshur pushed to
master in dmitri.shuralyov.com/gpu/mtl4ydmitshur pushed to
master in dmitri.shuralyov.com/website/gido5ydmitshur pushed to
master in dmitri.shuralyov.com/website/gido5ydmitshur pushed to
master in dmitri.shuralyov.com/html/belt5ydmitshur closed an issue github.com/shurcooL/issuesapp: Improve use of http.Error.5y
dmitshur commented on github.com/shurcooL/issuesapp: Improve use of http.Error.5y
Resolved a while ago in [commit `431bb22d`](https://github.com/shurcooL/issuesapp/commit/431bb22d86ad0a95db0780e27ab6221b49238464).dmitshur commented on dmitri.shuralyov.com/blog: Replacing WordPress with GitHub Issues-like blog comments5y
I've decided to publish this post on the [blog front page](https://dmitri.shuralyov.com/blog) now, so re-opening.dmitshur reopened an issue dmitri.shuralyov.com/blog: Replacing WordPress with GitHub Issues-like blog comments5y
dmitshur opened an issue dmitri.shuralyov.com/blog: A half-hour to learn Rust, except it's Go instead of Rust5y
dmitshur pushed to
master in dmitri.shuralyov.com/website/gido5y