Activity
Today
dmitshur commented on sync/atomic: TestNilDeref flaky failure on windows-386 with runtime fatal error1h
@gopherbot Please consider this issue for backport. It appears to be a case of stack corruption during fault handling, and we're now seeing this failure on both 1.23 and 1.22 release branches (e.g., …dmitshur reviewed +2 on golang.org/x/vuln/cmd/govulncheck: add missing test data2h
Thanks.
This does fix test failures on 1.23 branch (see [here](https://ci.chromium.org/b/8730719768254959025)), but there are still other test failures with tip (see [here](https://ci.chromium.org/b…dmitshur reviewed +2 on doc/godebug: document we removed x509sha13h
Thanks.dmitshur commented on crypto/x509: remove x509sha1 GODEBUG3h
(1 comment)
I tried to look if we've done this before, and I didn't spot any instances, so this might be the first.
Not sure, but it seems that it would make the page more readable to update it. In…dmitshur closed an issue sync/atomic: TestNilDeref failures3h
It's happening on the Go 1.23 release branch now.
Closing in favor of #70288 again. I relaxed the pattern there and removed this issue from Test Flakes project, so watchflakes sends its reports th…dmitshur commented on os/signal: TestSignalTrace is flaky on openbsd/arm643h
CC @golang/openbsd.dmitshur commented on golang.org/x/text/message: catalog.Var between two substitutions3h
CC @mpvl.dmitshur commented on net/http/cgi: should accept "INCLUDED" as protocol for server side includes4h
Thanks for the report. Note that the net/http/cgi package comment includes "This package is intended primarily for compatibility with existing systems." but it sounds like this issue might affect exi…dmitshur fixed an issue syscall: SyscallN always escapes the variadic argument [1.22 backport]4h
Closed by merging [CL 630215](https://go.dev/cl/630215) (commit 6f05fa7a4f632fee7fadada8d31cf8755c709610) to release-branch.go1.22.dmitshur fixed an issue syscall: SyscallN always escapes the variadic argument [1.23 backport]4h
Closed by merging [CL 630196](https://go.dev/cl/630196) (commit 847cb6f9ca43da48cb10e98808a74a40b41242fa) to release-branch.go1.23.dmitshur commented on golang.org/x/vuln/internal/sarif: use empty arrays instead of nils4h
Note that the x/vuln longtest builders started to fail with Go 1.23 and 1.22 seemingly as of this CL, see [here](https://ci.chromium.org/ui/p/golang/builders/ci/x_vuln-go1.23-linux-amd64-longtest/b87…dmitshur commented on cmd/compile: let MADD/MSUB combination happen more often on arm644h
TestUint64Modulo in golang.org/x/exp/rand has started to [fail](https://ci.chromium.org/ui/p/golang/builders/ci/x_exp-gotip-linux-arm64/b8730733624381859905/test-results?q=ExactID%3Agolang.org%2Fx%2F…dmitshur opened an issue github.com/golang/protobuf/google.golang.org/protobuf/types/known/structpb: TestToStruct fails at Go tip after the runtime map change in Go CL 6277164h
The TestToStruct test in `google.golang.org/protobuf/types/known/structpb` package has started to fail at Go tip, as of [go.dev/cl/627716](https://go.dev/cl/627716) (CC @randall77):
```
=== RUN …dmitshur commented on crypto/ed25519: move implementation to crypto/internal/fips/ed255194h
Note that TestTypeAlias in golang.org/x/crypto/ed25519 [fails](https://ci.chromium.org/ui/p/golang/builders/ci/x_crypto-gotip-linux-amd64/b8730799065470690641/test-results?q=ExactID%3Agolang.org%2Fx%…dmitshur reviewed +2 on syscall: [release-branch.go1.22] mark SyscallN as noescape4h
(1 comment)
Yes, both [go1.22-windows-amd64-longtest](https://ci.chromium.org/ui/p/golang/builders/try/go1.22-windows-amd64-longtest/b8730729501094353009/overview) and [go1.22-windows-amd64-race](ht…dmitshur commented on syscall: [release-branch.go1.22] mark SyscallN as noescape5h
(1 comment)
Oh I see you mentioned this in a Gerrit comment.dmitshur reviewed +2 on syscall: [release-branch.go1.22] mark SyscallN as noescape5h
Thanks.
Not a big deal, but it might help to mention that this includes CL 630136 too.
Please either s/gotip/go1.22/g on this line (since this branch needs Go 1.22 builders) or remove it.dmitshur commented on cmd/internal/obj/ppc64: support for decimal floating point instructions6h
(1 comment)
No need to change in this CL in particular as it's minor, but noting for future CLs: it's common to use a blank line to separate the git footers (Change-Id, etc.) from the commit message…dmitshur commented on cmd/dist: vet non-test packages in short mode6h
Thanks. Some comments/questions.
I understand you've considered the possibility of keeping this on builders, and dropping this only when GO_BUILDER_NAME == "", but decided to apply this change to bu…dmitshur commented on golang.org/x/tools/refactor/eg: Using `./...` only matches one file even though there are many more8h
CC @golang/tools-team.dmitshur commented on cmd/compile: some codes in DSE rely on the order of values8h
CC @golang/compiler.dmitshur commented on os: File sporadic waits upon non-blocking raw connect8h
CC @rsc, @ianlancetaylor, @griesemer.Yesterday
dmitshur commented on cmd/internal/testdir: print stderr if go list fails while gathering stdlib import config19h
Thanks.dmitshur commented on cmd/internal/testdir: update errors when filepaths include 'C:\'19h
Thanks.dmitshur commented on runtime: avoid deadlock in synctest changegstatus when copying stacks21h
Those longtest failures (on PS 1) look like #70429, #70430, #70431 due to the parent commit a65f1a46 being after CL 608335 and before CL 629475. It passed on the rebased PS 2.dmitshur commented on crypto/x509: remove x509sha1 GODEBUG21h
(1 comment)
Perhaps it could work well to update this, now that it's known what the future release where it's removed will be.dmitshur fixed an issue time: TestLoadFixed failures1d
Should be fixed on release branches too, now. Closing optimistically.dmitshur commented on runtime: deprecate SetFinalizer1d
(1 comment)
FWIW, the documentation for this release note linking support is in doc/README.md, and this short form indeed should work as listed on [line 43](https://cs.opensource.google/go/go/+/mast…dmitshur commented on runtime: deprecate SetFinalizer1d
Noting here that https://go.dev/wiki/Deprecated is the place where it is documented that:
> If function F1 is being replaced by function F2 and the first release in which F2 is available is Go 1.N…dmitshur reviewed +2 on crypto/internal/fips/nistec: remove P-256 from generate.go1d
Thanks.dmitshur commented on crypto/sha3: new package1d
(1 comment)
The "Minor changes to the library" section is intended for minor changes to existing packages. A new package normally deserves its own standalone section under "Standard library". See ht…dmitshur commented on internal/synctest: new package for testing concurrent code1d
Note that runtime:mayMoreStackMove.TestSynctest is [failing](https://ci.chromium.org/ui/p/golang/builders/ci/gotip-linux-amd64-longtest/b8730812608564042305/test-results?q=ExactID%3Aruntime%3AmayMore…dmitshur commented on crypto/internal/nistec: use Booth multiplication in purego P-2561d
(1 comment)
Given this is still here, will the p256.go file not be overwritten next time `go generate` is done?dmitshur commented on golang.org/x/build/internal/workflow: run at most one expansion at a time1d
Thanks.dmitshur commented on github.com/go-gl/glfw: GLFW 3.3 missing functions like window attention1d
Some of the GLFW functions are methods in the Go bindings. For example, `void glfwRequestWindowAttention(GLFWwindow * window)` is [Window.RequestAttention](https://pkg.go.dev/github.com/go-gl/glfw/v3…dmitshur commented on cmd/go: using both -v and -json with go test has unintended effects1d
This is documented at https://pkg.go.dev/cmd/test2json as:
> The test should be invoked with -test.v=test2json. Using only -test.v (or -test.v=true) is permissible but produces lower fidelity resu…dmitshur reviewed +2 on time: [release-branch.go1.22] accept "+01" in TestLoadFixed on OpenBSD1d
dmitshur reviewed +2 on [release-branch.go1.23]time: accept "+01" in TestLoadFixed on OpenBSD1d
dmitshur reviewed +2 on golang.org/x/build/main.star: remove non-tip ASAN and MSAN builders1d
Thanks.
Perhaps:
```suggestion
enabled = define_for_go_starting_at("go1.24", presubmit = False),
```
```suggestion
enabled = define_for_go_starting_at("go1.24", presubmit = False),…dmitshur commented on cmd/compile/internal/ssa: make _gen an actual submodule and skip it in TestStdlib1d
(4 comments)
FWIW, x/tools still contains astutil, it just stops being vendored because `go mod vendor` doesn't consider imports in this _gen directory due to the leading underscore.
Also here.
Go…dmitshur commented on golang.org/x/build/cmd/gopherbot: add matthew@go.dev1d
Thanks!This Week
dmitshur reviewed +2 on doc/godebug: document that unrecognized GODEBUG entries are ignored1d
Thanks.dmitshur reviewed +2 on doc/godebug: fix tipo1d
dmitshur opened an issue golang.org/x/build/cmd/golangbuild: update result_adapter to handle Go 1.24's go test -json output1d
This is a continunuation of https://github.com/golang/go/issues/70402#issuecomment-2484442242, tracking the change on the LUCI side.
CC @golang/release, @mknyszek.dmitshur commented on build: build failure on darwin builders since CL 536399 [consistent failure]1d
Updated plan, which takes into account https://pkg.go.dev/cmd/go@master#hdr-Build__json_encoding and https://github.com/golang/go/issues/70402#issuecomment-2484043547, is to determine that a given JS…dmitshur commented on os: correctly handle errno==0 in (*Process).blockUntilWaitable1d
(1 comment)
Is it expected that https://go.dev/s/style#indent-error-flow doesn't apply to errors of type `syscall.Errno`, explaining why this isn't written as `if errno != 0 { return errno }; return…dmitshur reviewed +2 on cmd/go: re-enable build JSON from go test -json, now with GODEBUG2d
Thanks. I sent CL 629375 to use this new GODEBUG setting in LUCI, so this can land before the result_adapter/ResultDB changes are done and propagated.
Since this GODEBUG is intended to be available …dmitshur opened a change golang.org/x/build/main.star: set gotestjsonbuildtext=1 globally at tip2d
Two reasons: to buy more time to work on the result_adapter/ResultDB
side (beyond just the bare minimum behavior of not regressing beyond
what we had before), and to test that using a GODEBUG for thi…dmitshur commented on os/user: skip tests that create users when running on dev machines2d
(2 comments)
Optionally, there's an existing higher-level helper in testenv package that can be used:
```suggestion
if testenv.Builder() == "" {
```
This message seems to be inverted compared to …dmitshur commented on build: build failure on darwin builders since CL 536399 [consistent failure]2d
Updating this with some details that I'm seeing so far.
The `Test result for "" is invalid: test_id: unspecified` error happens because the system that stores individual test results requires each…dmitshur reviewed +2 on golang.org/x/website/_content: update GitHub links2d
Thanks.
No problem, thanks.dmitshur commented on internal/runtime/maps: eliminate a load from the hot path3d
Note that internal/runtime/maps.TestMapPut [fails](https://ci.chromium.org/b/8730991533000725377) on the `GOEXPERIMENT=noswissmap` builder.dmitshur fixed an issue golang.org/x/tools/go/analysis/unitchecker: TestVetStdlib failures3d
Fixed in [CL 628555](https://go.dev/cl/628555).dmitshur commented on golang.org/x/perf/benchseries: do not crash if no denominator is present3d
@murp@ibm.com What's the reason to close this? Is issue #66685 fixed via another means, or no longer relevant?dmitshur commented on golang.org/x/website/_content: update GitHub links3d
(7 comments)
See inline comment about this. I think it's fine to do when the original URL clearly points to its new location, but we probably shouldn't do it when there are many potential alternativ…dmitshur commented on crypto/internal/fips/check: remove unreachable return3d
Thanks.Last Week
dmitshur commented on make.rc: correct test for undefined GOROOT_BOOTSTRAP6d
Thanks.dmitshur commented on golang.org/x/website/internal/screentest: get tests to pass1w
(1 comment)
Good to see the [deployment](https://go.googlesource.com/website#deploying) with the [go122 runtime](https://cs.opensource.google/go/x/website/+/master:cmd/golangorg/app.yaml;l=5;drc=e78…dmitshur closed an issue sync/atomic: TestNilDeref failures1w
dmitshur commented on golang.org/x/lint/README: remove Contributions section1w
There are no trybots defined for this repo; bypassing.dmitshur reviewed +2 on golang.org/x/build/perf: add linux/arm64 dropdown option to dashboard1w
Thanks, and congrats!dmitshur reviewed +2 on time: regenerate zoneinfo_abbrs_windows.go1w
Thanks.
Consider adding 'For #58113.' here.dmitshur reviewed +2 on image/color/palette: add godoc link to generator program1w
Thanks.dmitshur reviewed +2 on golang.org/x/lint/README: remove Contributions section1w
Thanks.dmitshur reviewed +2 on golang.org/x/website/_content/blog: add 15years post1w
(1 comment)
I expect a test should suggest using a relative link here and elsewhere, so it works in more contexts.Earlier
dmitshur reviewed +2 on golang.org/x/build/main.star: add experimental PGO performance builder1w
Thanks.
Nice to see SECOND_GO is handy.
(nit) Consider adding a blank line after line 1862, otherwise the "# Bump the timeout ..." comment sounds as if it applies to the entire block, while really …dmitshur commented on golang.org/x/mobile/go.mod: update golang.org/x dependencies1w
(1 comment)
Done in I42a24c462acf7340410614fa3b0dd80608efcdaa. Rebased this CL on top.dmitshur commented on golang.org/x/exp/go.mod: update golang.org/x dependencies1w
(1 comment)
Done in I0a31a56df624f6052249122c66153b91bb321a37. Rebased this CL on top.dmitshur commented on golang.org/x/mobile/go.mod: update golang.org/x dependencies1w
Indeed. I left an inline comment about that.
In a modern version of the go command, running go mod tidy ignores requirements that come from files with the "ignore" build constraint. This should prob…dmitshur commented on golang.org/x/exp/go.mod: update golang.org/x dependencies1w
(1 comment)
In a modern version of the go command, running `go mod tidy` in a module without a go directive will cause it to be added using the go command's own version.
Leaving out the go directiv…dmitshur reviewed +2 on golang.org/x/build/main.star: add linux-arm64 performance builders1w
Thanks.
Asking to understand the difference: perf_vs_gopls_0_11 exists only for the 16 vCPU amd64 builder, not 88 one, but here it's being added for both. Is the asymmetry intended? (SG if so.)dmitshur commented on golang.org/x/build/cmd/relui: add a "failed to schedule workflows" section1w
(Whoops, I wrote a draft reply but didn't send it earlier.)
i and j are two indexes from slices.IndexFunc, whose results are immediately handled by a switch. Giving then longer names makes the switc…dmitshur commented on golang.org/x/telemetry/go.mod: update golang.org/x dependencies1w
(Marking this thread unresolved so this CL isn't accidentally submitted before Hana has a chance to see this thread.)dmitshur commented on golang.org/x/telemetry/go.mod: update golang.org/x dependencies1w
@hyangah@gmail.com, this CL is automatically generated as part of the monthly "tag golang.org/x repos" workflow. In addition to tagging, it also updates golang.org/x dependencies.
I know you tempora…dmitshur commented on golang.org/x/build/cmd/relui: add a "failed to schedule workflows" section1w
Thanks.dmitshur opened a change golang.org/x/build/internal/workflow: fix sub-workflow prefix for tasks added via expansion1w
Tasks added by expansions, in contrast to all other tasks, didn't handle
the sub-workflow prefix. A while back I looked into it briefly, and saw
that shallowClone didn't clone that one field, so figu…dmitshur commented on golang.org/x/build/cmd/relui: sometimes impossible to retry/approve tasks in Go minor release workflow, appearing as an "unknown task" error1w
I considered how we can fix this. One path is to modify the workflow so that it can't run two expansions in parallel. This is tricky, and would make the workflow code less readable - it currently cal…dmitshur commented on golang.org/x/build/cmd/relui: sometimes impossible to retry/approve tasks in Go minor release workflow, appearing as an "unknown task" error1w
I was able to successfully reproduce this "unknown task" error condition locally with a simplified dry-run release workflow:
This is done.dmitshur fixed an issue build: adopt Go 1.22 as bootstrap toolchain for Go 1.241w
This is done.dmitshur opened an issue cmd/compile: linux/mips fails to make.bash with "internal compiler error: panic: runtime error: index out of range [0] with length 0"1w
From https://build.golang.org/log/5e58860b238976b022020c1cb9b3537a20adc863:
```
Building Go cmd/dist using /tmp/gobuilder-mips64/go1.4/go. (go1.22.6 linux/mips64)
Building Go toolchain1 using /t…dmitshur commented on golang.org/x/build: add LUCI linux-mips64 builder1w
Noting here that if a LUCI builder isn't added to provide test coverage for this port, it risks becoming broken without anyone noticing. There's signal provided by the legacy builders, but it's not v…dmitshur commented on runtime: severe performance drop for cgo calls in go1.22.5 [1.23 backport]2w
We moved the NextMinor label to the parent tracking issue #68587, removing here.dmitshur reviewed +2 on golang.org/x/build/main.star: adjust LUCI load at openbsd-ppc64 builder2w
dmitshur reviewed +2 on golang.org/x/build/x/build: adjust LUCI load at openbsd-ppc64 builder2w
Thanks. Just minor comments for the commit message.
The repo name is only needed for the issue tracker. Please replace this prefix to be 'main.star: adjust ...'.
We probably want to keep the issue …dmitshur commented on golang.org/x/build/README: mention the git repo2w
(1 comment)
FYI it wasn't applied. (It makes no significant difference in this particular case, that's why I made this suggestion optional, but letting you know in case it helps you avoid this happe…dmitshur reviewed +2 on golang.org/x/build/README: mention the git repo2w
Thanks.
Optionally, we could go further and use the same wording here as in all other repos:
```suggestion
This repository uses Gerrit for code changes. To learn how to submit changes to
this repos…dmitshur commented on all: update golang.org/x/sys [generated]2w
runtime/race.TestRace failure is pre-existing, #70164, bypassing.dmitshur commented on golang.org/x/website/internal/history: document Go 1.23.3 and 1.22.9 releases2w
Minor comment, but I'll let Carlos review. Thanks.
```suggestion
Components: []template.HTML{"the linker", "cgo", "the runtime"},
```dmitshur commented on golang.org/x/build: gotip-windows-arm64 builders stops working occasionally2w
Again, both windows-arm64 builders have gone offline as of around Nov 2-5, with first OOMs on Oct 29 ([here](https://github.com/golang/go/issues/69664#issuecomment-2445132269)):
<img width="147" a…dmitshur commented on all: update golang.org/x/sys [generated]2w
Thanks. I've updated this CL to pull in the tip of x/sys now, which includes the revert.dmitshur reviewed +2 on golang.org/x/build/main.star: replace -swissmap builder with -noswissmap2w
Thanks.dmitshur commented on golang.org/x/build/internal/task: move NewTaskRequest.Properties into TaskSlices field2w
Thanks.dmitshur commented on syscall: SyscallN always escapes the variadic argument2w
@qmuntal I'm a bit confused by the specifics in your question (maybe you got the wrong CL number mentioned? that one pulls in many x/sys CLs all at once), but in general, if there is a serious issue …dmitshur commented on time: TestLoadFixed failures2w
[CL 625197](https://go.dev/cl/625197) is an implementation of https://github.com/golang/go/issues/69840#issuecomment-2448571828.dmitshur opened a change time: accept "+01" in TestLoadFixed on OpenBSD2w
This stops the test from failing with a known failure mode, and
creates time to look into what the next steps should be, if any.
For #69840.dmitshur commented on golang.org/x/build/internal/relui: serve 404 when workflow doesn't exist, fix DB tests2w
Thanks.dmitshur commented on golang.org/x/build/...: regenerate .pb.go files, unify generation process2w
There indeed turned out to be a small¹ dragon that lurked for some years, but I wouldn't have caught it ahead of trying this out. :)
¹ It's quite literally one byte long, probably couldn't be any …