Activity
Today
dmitshur commented on cmd/go: start a test case for an insecure scheme check1h
Thanks for reviewing. I'll defer landing this CL until CL 770060 is reviewed too, so removing this from my attention set for now.dmitshur commented on cmd/go/internal/doc: move buildPkgsite version constant to package scope1h
Thanks.dmitshur commented on cmd/dist: redefine goTest.timeout to raise timeout only, not lower it1h
Thanks. I'll let this and the rest of the stack land for Go 1.28 (via wait-release).dmitshur commented on internal/godebugs: tests exceed package test timeout of 6 minutes5h
It is possible to skip except on longtest builder, but we probably don’t want to do that because all.bash doesn’t run in longtest mode by default, so it means forgetting to regenerate that packag…dmitshur commented on golang.org/x/website/_content/doc/go1.27: minor edits6h
Doesn't need to be in this CL, but maybe are you also planning to say something about improvements to GOEXPERIMENT=simd in Go 1.27.dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27: minor edits7h
Thanks.
It's probably a no-op for rendered HTML, but it's breaking highlighting in Gerrit.
```suggestion
<!-- https://go.dev/issue/77626, https://go.dev/issue/78888 -->
```
The letter 'v' here …dmitshur commented on golang.org/x/net/html: `html.Parse` Changes Attribute Order Starting in v0.55.09h
I also found this while looking into the trybot failure in [CL 789800](https://go-review.googlesource.com/c/pkgsite/+/789800/comments/05bb9ce4_a90a56ca). Some testdata there happened to depend on the…dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27: remove unnecessary TODOs10h
Thanks.Yesterday
dmitshur commented on golang.org/x/pkgsite/go.mod: update golang.org/x dependencies1d
Git bisect finds that the relevant x/net commit is CL 781702. Its commit message includes:
> Instead of [... slow thing ...], sort the attributes and [... now a fast thing is viable ...]
So it seem…dmitshur opened a change cmd/dist: drop 3 minute (or 9 for cmd/go) custom timeout for std cmd1d
The reason we have these particular custom timeouts in cmd/dist today
is because CL 120762 increased cmd/go timeout from 6 minutes to 9,
and before that CL 84235 increased cmd/go timeout from 3 minut…dmitshur commented on cmd/dist: drop 3 minute timeout for a few packages that get close1d
Thanks.dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27: break go command notes to subsections1d
Thanks.
nit: I think we spell it as one word without a space more commonly (e.g., see two instances in this file, and https://go.dev/doc/toolchain). (I realize this text was pre-existing, so this is…dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27: document crypto/tls GODEBUG removal1d
Thanks.dmitshur opened a change cmd/dist: drop 3 minute timeout for a few packages that get close1d
This change drops the constant timeout from two packages where it's
been observed to be unhelpful. Applying this more widely is left to
subsequent changes.
For #79970.dmitshur opened an issue cmd/dist: remove remaining cases of unhelpful overlapping constant timeouts1d
As motivated in the commit message of [CL 504495](https://go.dev/cl/504495):
> Since go test has a default top-level timeout, individual tests don't
need to impose their own timeouts too. It's possi…dmitshur commented on golang.org/x/pkgsite/go.mod: update golang.org/x dependencies1d
There've been a number of x/net/html changes in x/net@v0.56.0 (CC @roland@golang.org), test failures are likely related to that.
The diff seems to just be the position of the aria-label attribute, a…dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27: remove TODO for security GODEBUG1d
Thanks.dmitshur fixed an issue golang.org/x/tools/go/packages: TestConfigEnvDoesNotInheritProcessEnv fails with "cannot find GOROOT directory: 'go' binary is trimmed and GOROOT is not set"1d
This failure mode should be fixed by [CL 789500](https://go.dev/cl/789500). Closing, and watchflakes can reopen if it keeps happening.dmitshur fixed an issue golang.org/x/tools/go/packages: TestConfigEnvDoesNotInheritProcessEnv fails with "cannot find GOROOT directory: 'go' binary is trimmed and GOROOT is not set"1d
This failure mode should be fixed by [CL 789500](https://go.dev/cl/789500). Closing, and watchflakes can reopen if it keeps happening.dmitshur opened a change compress/flate: move previous implementation behind GODEBUG slowerflate=go1261d
DO NOT REVIEW
DO NOT SUBMIT
The compress/flate package documentation for NewWriter and NewWriterDict
notes:
> Note that the exact bytes written to w are not covered by the Go 1
> compatibility prom…dmitshur commented on golang.org/x/tools/go/packages: propagate PATH in TestConfigEnvDoesNotInheritProcessEnv1d
Thanks.dmitshur starred github.com/sourcegear/diffmerge1d
This Week
dmitshur commented on golang.org/x/tools/go/packages: propagate PATH in TestConfigEnvDoesNotInheritProcessEnv1d
This change is ready for review.dmitshur opened a change golang.org/x/tools/go/packages: propagate PATH in TestConfigEnvDoesNotInheritProcessEnv1d
The packages.Load call needs to be able to find a driver such as the
go command, either via the GOPACKAGESDRIVER environment variable, or
failing that, by looking in PATH.dmitshur commented on cmd/dist: test encoding/json/... with GOEXPERIMENT=nojsonv22d
Thanks.dmitshur opened a change cmd/dist: test encoding/json/... with GOEXPERIMENT=nojsonv22d
The intent of this custom test is to cover what isn't already tested
as part of the normal 'go test std cmd' sequence that all.bash runs.
It started providing duplicate signal once jsonv2 became the …dmitshur opened a change golang.org/x/build/main.star: add a linux-amd64-racegocommand builder at tip2d
This is like linux-amd64-racegocommand-noopt, but without the noopt
modifier.
The previous linux-amd64-racegocommand-noopt builder hasn't found races
in the go command itself so far, but it ran into…@oakad It is a valid alternative approach to put more emphasis on GOROOT and rely on relative paths to find the go command. Migrating to that would be a large change and would need a proposal, so bet…dmitshur commented on golang.org/x/tools/go/packages: TestConfigEnvDoesNotInheritProcessEnv failures on OpenBSD builders2d
The fix for the test is probably to propagate the PATH environment variable.
Issue #62114 is also related here. If it's viable to modify packages.Load itself to return an error if the PATH it's prov…dmitshur opened an issue cmd/go: occasional failures on gotip-linux-amd64-racegocommand-noopt builder during `go install -race cmd/go` step2d
The gotip-linux-amd64-racegocommand-noopt builder has had a couple failures while running `go install -race cmd/go` ([complete environment details](https://logs.chromium.org/logs/golang/buildbucket/c…dmitshur commented on api: audit for Go 1.272d
The approved freeze exception for proposal #24673 (CC @rolandshoemaker) adds a new field in crypto/tls:
- `pkg crypto/tls, type ConnectionState struct, LocalCertificate [][]uint8`
- #24673
Edited…dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27: document go test -json output annotation2d
Thanks.
At a high level, we want to avoid making the release notes seem like a canonical or only home for feature documentation; rather they aim to say what's new/changed and point to the actual und…dmitshur commented on golang.org/x/website/doc: add release notes for encoding/json/v2 in Go 1.273d
(1 comment)
Consider either changing this to 'Fixes' or manually closing afterwards. If there's something left to do, it's better to file individual issues. Otherwise that proposal could use a comme…dmitshur opened a change cmd/fix: pull in CL 7870203d
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.20260604184323-2111063761b3
go mod tidy
go mod v…dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27: document "go doc" changes3d
Thanks.dmitshur reviewed +1 on golang.org/x/build/internal/gophers: add alex to gophers3d
Indeed. :)dmitshur commented on golang.org/x/review/git-codereview: give actionable error when Gerrit cookie is missing3d
(1 comment)
Instead of directly pointing to a Gerrit URL here, let's point to the relevant section of the Go contribution guide:
https://go.dev/doc/contribute#config_git_auth
That way if some of t…dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27: document linker change4d
Thanks.dmitshur starred github.com/mauri870/gomodmerge4d
dmitshur closed an issue golang.org/x/text/unicode/bidi: DefaultDirection(LeftToRight) is silently ignored [1.25 backport]4d
No backport rationale provided here nor in https://github.com/golang/go/issues/71809#issuecomment-4461422460.dmitshur closed an issue golang.org/x/text/unicode/bidi: DefaultDirection(LeftToRight) is silently ignored [1.26 backport]4d
No backport rationale provided here nor in https://github.com/golang/go/issues/71809#issuecomment-4461422460.dmitshur closed an issue golang.org/x/text/unicode/bidi: DefaultDirection(LeftToRight) is silently ignored [1.26 backport]4d
No backport rationale provided here nor in https://github.com/golang/go/issues/71809#issuecomment-4461422460.dmitshur commented on runtime: version parsing fails4d
@bradfitz Was [CL 781800](https://go.dev/cl/781800) enough to fix this for Go1.27, or is this issue open because there's more to do? (Fixing x/sys can probably be tracked another way, since it's in a…Last Week
dmitshur commented on golang.org/x/website/solutions: update urfave/cli link to v3 GitHub URL6d
@Jah-yee Thanks for sending this change. Please see information posted in https://github.com/golang/website/pull/359#issuecomment-4317356043. I left review comments at https://go.dev/cl/770646.dmitshur commented on golang.org/x/website/solutions: update urfave/cli link to v3 GitHub URL6d
Thanks for updating this entry. See inline comments for a few suggestions.
Moved the 'Fixes' line to the bottom of the commit message. Done.
Maybe it's worth it to include "v3" here too, since this…dmitshur commented on encoding/json: consider implications of using TextAppender in V1 backwards-compatibility mode1w
That mentioned package has a comment in its AppendText method implementation:
```
// NOTE(anderseknert): this DOES allocate still, and while that's unfortunate,
// we'll be better off dealing with t…dmitshur reviewed +1 on cmd/go/internal/doc: clear packageCache before building pkgsite1w
This seems reasonable to me, but I defer to Michael for +2 review. I left minor suggestions in inline comments. Thanks.
It'd be good to document what this exported function does, and maybe mention t…dmitshur reviewed +2 on cmd/go/internal/doc: update pkgsite version1w
Thanks.dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27: remove unnecessary TODOs1w
Thanks.dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27: remove TODOs for subrepo changes1w
Thanks.dmitshur opened a change golang.org/x/build/cmd/updatestd: maintain cmd/go/internal/doc selection of pkgsite version1w
Make it easier to update the version included as a constant inside
the cmd/go/internal/doc command by having updatestd take it on too.
For golang/go#36905.dmitshur opened a change cmd/go/internal/doc: move buildPkgsite version constant to package scope1w
This will make it easier to have its value maintained as part of the
x/build/cmd/updatestd command used to help with the go.dev/issue/36905
dependency maintenance process.
For golang/go#36905.dmitshur commented on golang.org/x/pkgsite/cmd/internal/doc: update dependencies1w
(1 comment)
This update can count as the "soon after the release freeze starts" update that we normally do as part of issue #36905. We're still before Go 1.27 RC 1.
We may want to consider starting…dmitshur reviewed +2 on golang.org/x/pkgsite/cmd/internal/doc: update dependencies1w
Thanks.dmitshur reviewed +2 on golang.org/x/build/cmd/relnote: added markdown summary info from issue and CLs for each TODO1w
(3 comments)
```suggestion
provenance: fmt.Sprintf("RELNOTE comment in [CL %d](/cl/%[1]d)", cl.Number),
```
```suggestion
provenance: fmt.Sprintf("[CL %d](/cl/%[1]d)", cl.Number),
```
The p…dmitshur commented on golang.org/x/tools/go/analysis/passes/stdversion: language vs toolchain version1w
I'm not entirely sure where the best documentation for per-file language version is, so I'll refer to https://go.dev/doc/toolchain#config as the best I could find so far. It says:
> The go line for …dmitshur commented on golang.org/x/build/main.star: remove openbsd-arm64 from KNOWN_ISSUE_BUILDER_TYPES1w
Thanks. This might need to be rebased and regenerated if luciconfig.TestValidate fails.
Some time has passed and results are visible:
https://ci.chromium.org/ui/p/golang/g/port-openbsd-arm64/builde…dmitshur commented on cmd/go: `-help` and `go version` are broken when go.mod contains non-standard version1w
As far as I can tell, the original reproducer works with go1.21rc2 (as the local toolchain), and not with go1.21rc3. Plausibly fixed by [CL 509097](https://go.dev/cl/509097). Setting the milestone to…dmitshur closed an issue github.com/shurcooL/vfsgen: Test issue1w
dmitshur commented on crypto/tls: benchmark needs no entropy, old suites1w
Thanks. This has two +2s and is needed to fix longtest-race builders, so letting this submit sooner. If there's more to do beyond what CL 779984 takes on, it's worth filing an issue (it can be releas…dmitshur commented on cmd/link: TestELFHeadersSorted/exe failures1w
CC @golang/riscv64.dmitshur commented on golang.org/x/tools/gopls: crash in runtime trace writer1w
From issue triage, this may be explainable by memory corruption.dmitshur commented on runtime:cpu4: TestCgoCallbackGC failures1w
From release triage, this is probably consistent with a timeout at the package-level.dmitshur commented on runtime: TestFakeTime failures1w
From release triage, this also could be a timeout at the package-level.dmitshur closed an issue runtime: TestSecretCgo failures1w
dmitshur commented on runtime:cpu1: TestSecretCgo failures1w
From release triage, these could be timeouts at the package-level.dmitshur reopened an issue cmd/fix: modernizer for direct references to embedded fields in struct literals1w
dmitshur commented on golang.org/x/build/internal/installer/darwinpkg: remove "Fixing permissions" step1w
Thanks.dmitshur reviewed +2 on cmd/go: [release-branch.go1.25] use fsys.ReadDir for IsStandardPackage1w
Thanks.dmitshur reviewed +2 on cmd/go: [release-branch.go1.26] use fsys.ReadDir for IsStandardPackage1w
Thanks.dmitshur reopened an issue golang.org/x/tools/gopls: modernizer for direct references to embedded fields in struct literals1w
I understand the intent of https://github.com/golang/go/issues/77965#event-26295992291 is to make sure this change makes its way into cmd/fix, so this still needs to be updated using the process desc…dmitshur commented on cmd/fix: modernizer for direct references to embedded fields in struct literals1w
I understand the intent of https://github.com/golang/go/issues/77965#event-26295992291 is to make sure this change makes its way into cmd/fix in time for Go 1.27 RC 1, so this still needs to be updat…dmitshur commented on encoding/json/v2: new API for encoding/json1w
This is currently blocking Go 1.27 release candidate 1. I understand all that's left to close this is adding release notes.dmitshur fixed an issue simd: support WASM and limited machine-independent SIMD under a GOEXPERIMENT [freeze exception]1w
The originally requested freeze exception was completed, closing.dmitshur commented on simd: support WASM and limited machine-independent SIMD under a GOEXPERIMENT [freeze exception]1w
The originally requested freeze exception was completed, closing.dmitshur commented on golang.org/x/build/...: use 'an error satisfying errors.Is/As(err, target)' phrasing1w
(1 comment)
This change is ready for review.
Thanks.dmitshur commented on golang.org/x/vuln/cmd/govulncheck: fix exit code propagation for wrapped scan errors1w
Thanks for working on a fix. I may not be the best reviewer for this since I'm not familiar with x/vuln and conventions or constraints involved here. I left some questions, but if there's someone bet…dmitshur commented on golang.org/x/build/...: use 'an error satisfying errors.Is/As(err, target)' phrasing1w
Friendly ping since it's been a while.dmitshur reviewed +2 on golang.org/x/website/_content/doc: remove uneeded TODOs1w
Thanks.dmitshur reviewed +2 on golang.org/x/website/internal/history: document Go 1.26.4 and 1.25.11 releases1w
Thanks. I'm not seeing any issues.dmitshur reviewed +2 on golang.org/x/website/_content/doc: add backwards compatibility note for ppc64/linux1w
Thanks.dmitshur reviewed +2 on golang.org/x/website/_content/doc: remove unneeded release notes TODOs1w
Thanks.dmitshur reviewed +1 on golang.org/x/website/internal/history: document Go 1.26.4 and 1.25.11 releases1w
+1 so David can review more closely.dmitshur commented on golang.org/x/build/main.star: add a linux-amd64-racegocommand-noopt builder at tip1w
Thanks.dmitshur opened an issue encoding/json/v2: keep v2 and jsontext packages buildable under GOEXPERIMENT=nojsonv21w
In Go 1.26 and older, GOEXPERIMENT=jsonv2 had the effect of "make still-experimental encoding/json/v2, encoding/json/jsontext packages available, and also modify the encoding/json implementation to b…dmitshur reviewed +2 on golang.org/x/website/_content/doc: add backwards compatibility note for ppc64/linux1w
Thanks.
Yes, it's very helpful. You can include "For golang/go#78779." in the commit message because that's the tracking issue for doing this. Thanks.
https://go.dev/wiki/Spelling suggests avoiding…dmitshur commented on golang.org/x/mobile/internal/sdkpath: accept minor Android platform versions1w
There are some notes relevant to this in [this golang-dev thread](https://groups.google.com/g/golang-dev/c/4Li4Ovd_ehE).dmitshur commented on golang.org/x/build: try a -racegocommand builder1w
Sent [crrev.com/c/7892066](https://crrev.com/c/7892066) and [go.dev/cl/785920](https://go.dev/cl/785920).dmitshur opened a change golang.org/x/build/main.star: add a linux-amd64-racegocommand-noopt builder at tip1w
Add a post-submit-only builder that runs at tip with a new racegocommand
mod, one which will reinstall the go command with race mode enabled.
Also try combining it with a noopt mod, since the existin…dmitshur opened an issue golang.org/x/build: try a -racegocommand builder1w
Between #77053, #79667, #79680, there may be a remaining data race in the `cmd/go/internal/load.PackagesAndErrors` implementation. Maybe a builder similar to [-racecompile](https://cs.opensource.goog…dmitshur commented on net/http/httptest: synctest support1w
@neild [CL 769521](https://go.dev/cl/769521) is submitted and there's a [release note entry](https://tip.golang.org/doc/go1.27#nethttphttptestpkgnethttphttptest). Is there anything more to do for Go …Earlier
dmitshur commented on internal/buildcfg: [dev.simd] turn SIMD goexperiment back on1w
(1 comment)
Sounds good, thanks.dmitshur reviewed +2 on golang.org/x/build/...: fix some comments to improve readability1w
Thanks.dmitshur commented on internal/buildcfg: [dev.simd] turn SIMD goexperiment back on2w
(1 comment)
The cmd/api:check.TestCheck failure here is due to CL 784980.
You can consider either starting to add the (admittedly large) API files to a doc/next/nnn.txt file, as that would make the…dmitshur commented on : policy for removing GODEBUG flags2w
The proposal process to define a policy for removing GODEBUG flags has considered many nuances and reached the likely accept state. I did a quick grep to see if GOEXPERIMENT came up here, and it hasn…dmitshur opened an issue internal/goexperiment: delete Flags.GenericMethods field in time for Go 1.27.02w
The discussions that I recall seeing, as well as the commit messages of [CL 772441](https://go.dev/cl/772441), [CL 779221](https://go.dev/cl/779221) and [CL 779301](https://go.dev/cl/779301), and htt…dmitshur commented on golang.org/x/website/_content/doc: document new `go mod tidy` behavior2w
(1 comment)
Ah, thanks. No need, either having the blank lines or not having them is fine.dmitshur commented on api: promote next to go1.272w
For posterity, that link should've been #79737.dmitshur commented on golang.org/x/website/_content/doc: document new `go mod tidy` behavior2w
(2 comments)
(a minor suggestion for the future)
For a release notes CL, the [motivation](https://go.dev/wiki/CommitMessage#non-normative-references) is mostly clear, so a blank commit message body…dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27.md: tweak release notes2w
Thanks.
"new ValueEnd field" implies the field is newly added in Go 1.27, but it was actually added back in Go 1.26:
https://go.dev/doc/go1.26#goastpkggoast:~:text=The%20new%20BasicLit.ValueEnd%20f…