Activity
Yesterday
dmitshur reviewed +2 on golang.org/x/build/internal/task: ping fsnotify@v1.9.0 in gopls release process7h
Thanks.dmitshur commented on golang.org/x/build/internal/task: remove -compat=1.19 since minimum version is 1.267h
(2 comments)
Consider doing the same change here. If you prefer to leave it for a future CL instead, that's fine too.
Not for this CL, but noting that another potential future simplification here i…dmitshur reviewed +2 on golang.org/x/build/internal/task: remove -compat=1.19 since minimum version is 1.267h
Thanks.dmitshur commented on golang.org/x/pkgsite/deploy: bump Go version to 1.26.48h
See an inline comment on go.mod. The rest of this change makes sense to me. Thanks for doing this.
I realize that the last time the deploy files in this repository were updated, the go directive was…dmitshur commented on golang.org/x/build: update gopls version requirements as part of the Go toolchain release workflow8h
I think the sections "Why not bump on each minor release?" at [go.dev/design/69095-x-repo-continuous-go#rationale](https://go.dev/design/69095-x-repo-continuous-go#rationale) is moderately relevant. …dmitshur commented on golang.org/x/build: automate maintenance of pinned Go toolchain/runtime versions in Dockerfile and app.yaml files8h
In addition to the 4 x/ repos already mentioned in the original issue, here are some more that also have Dockerfile and/or .yaml files that would benefit from automatic maintenance:
x/vulndb: https:…dmitshur opened a change golang.org/x/website/_content/doc/go1.27: note flate compression speed and exact bytes9h
The improvement to compression speed seems notable enough to be
worth mentioning. (Maybe it can be quantified succinctly enough
for release notes, or otherwise we can point to the issue.)
Even thoug…dmitshur commented on crypto/tls: only set LocalCertificate when a certificate is presented10h
@sin99xx Thanks for sending this change. As noted in https://github.com/golang/go/pull/79968#issuecomment-4684498141, it was successfully imported in Gerrit, and by now there are some review comments…dmitshur commented on golang.org/x/tools/go/packages: TestAdHocOverlays/GO111MODULE=auto failures10h
The failure above happened because package-level timeouts were reached. This was a darwin-arm64 race builder, which normally takes around 10 minutes to run on x/tools (https://ci.chromium.org/ui/p/go…dmitshur commented on crypto/mldsa: Verify accepts uninitialized PublicKey with empty signature10h
Thanks for trying out the package ahead of the release and reporting this finding. CC @golang/security.
Tentatively marking as a release blocker since this package is new to Go 1.27.dmitshur commented on crypto/tls: provide a way to access local certificate used to set up a connection [freeze exception]10h
Issue #79967 reported a problem with the new code. @rolandshoemaker Is it preferable to fix forward and proceed with it for 1.27, instead of reverting and trying again for 1.28? Given it seems like a…dmitshur commented on crypto/tls: LocalCertificate is populated on resumed TLS <=1.2 server connections11h
CC @rolandshoemaker, @golang/security.dmitshur commented on crypto/tls: TLS 1.3 post-handshake alerts not surfaced to the client11h
CC @golang/security.dmitshur commented on cmd/gofmt: comment alignment chunking behaviour different on MacOS vs Linux11h
Thanks for reporting.
It's expected that exact gofmt formatting nuances will change over time, across major Go releases, so a check like `go fmt ./...` needs to be arranged to run with a single cons…dmitshur commented on time: TestAfterQueuing/impl=func failures11h
The test contains a comment:
```Go
// This test flakes out on some systems,
// so we'll try it a few times before declaring it a failure.
const attempts = 5
```
So it seems this failure is expected…dmitshur commented on os/root_test: TestRootConsistencyStat/Lstat failures on XFS11h
Thanks for the report. Please note that Go 1.24 is no longer supported per https://go.dev/doc/devel/release#policy. Does this still happen with Go 1.26/1.25 or at tip?
CC @neild.dmitshur commented on golang.org/x/oauth2/fix: change the snake case endpoint to kebab-case11h
@codyoss@google.com I suspect you intended to start the current trybots via Commit-Queue+1 (see https://go.dev/wiki/GerritAccess#running-trybots-may-start-trybots and https://go.dev/wiki/LUCI#trybots…dmitshur reviewed +2 on golang.org/x/pkgsite/internal/fetch: inherit active tool tags in build context12h
Thanks.
If I understand right, the plan for x/pkgsite deploys will generally be to use a stable release of Go (similar to CL 733341), or a release candidate of the soon-to-become-latest-stable relea…dmitshur commented on golang.org/x/website/_content: fix some broken Markdown links12h
Thanks.dmitshur commented on golang.org/x/tools/go/analysis/passes/modernize: errorsastype rewrites errors.As target interface that does not implement error13h
CC @adonovan, @madelinekalil.dmitshur commented on cmd/cgo: panic in loadDWARF on out-of-bounds __cgo__ symbol index13h
CC @golang/compiler.dmitshur commented on golang.org/x/net/http3: missing header frame size limit allows OOM13h
CC @neild, @nicholashusin.dmitshur commented on net/http: optimize Date header generation to reduce per-request formatting overhead13h
CC @neild, @nicholashusin.dmitshur closed a change golang.org/x/crypto/openpgp/packet: reject too-short decrypted session key in EncryptedKey.Decrypt14h
Thanks for sending this change. Closing because this package is frozen and deprecated. In particular it is documented to be unmaintained except for security fixes. For security issues, please follow …dmitshur closed a change golang.org/x/crypto/openpgp/elgamal: fix index out of range panic in Decrypt when ciphertext is zero14h
Closing, see https://go.dev/issue/79841#issuecomment-4721777542.dmitshur commented on golang.org/x/crypto/openpgp/elgamal: fix index out of range panic in Decrypt when ciphertext is zero14h
I don't think there's anything that to be done here given the package is deprecated and frozen. There isn't anyone we can add to be its reviewer. This isn't the only security issue, as noted in the d…dmitshur closed an issue golang.org/x/crypto/openpgp/elgamal: Decrypt panics on zero ciphertext (DoS)14h
[golang.org/x/crypto/openpgp/elgamal](https://golang.org/x/crypto/openpgp/elgamal) is deprecated too. Its deprecation message notes:
> Deprecated: this package was only provided to support ElGamal e…dmitshur commented on cmd/compile: fix typo in comment14h
(1 comment)
The go/types and cmd/compile/internal/types2 packages are generally kept in sync. Please also apply the same change here:
```suggestion
// not permitted. Checker.funcInst must infer mi…dmitshur commented on cmd/doc: go doc synctest.Wait shows wrong package16h
This problem reproduces in older versions like Go 1.25.11 too, and the fix is localized, so it seems safe for it to happen by RC 1 or RC 2.dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27: minor wording edits17h
Thanks.dmitshur reviewed +2 on cmd/cgo: fix TestFortran on Windows17h
Thanks.
Asking to confirm my understanding.
As far as I see CL 787800 didn't make any Windows-specific changes. libExt was "so" on Windows both before and after that change. Is it case that it work…This Week
dmitshur commented on all: re-run stringer1d
Agreed. Thanks.dmitshur reviewed +2 on go.dev/wiki: Update minimum requirements for linux/ppc64 target1d
Thanks.dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27: add release notes for the new LocalCertificate field1d
Thanks.dmitshur commented on cmd/compile: add align128 compiler marker for 128-bit struct alignment1d
Since we're in release freeze for Go 1.27 and this seems like a new feature, should it have wait-release hashtag added and wait for when the tree reopens for Go 1.28 development?dmitshur commented on crypto/tls: ConnectionState isn't properly initialized until read or write from *tls.Conn1d
CC @golang/security.dmitshur commented on golang.org/x/vuln: add vulnerability-aware fix and dry-run modes to govulncheck1d
CC @golang/vulndb.dmitshur commented on net/http: Client forwards caller-set Proxy-Authorization to a direct origin on a same-host redirect that crosses a proxy boundary1d
CC @neild, @nicholashusin.dmitshur commented on access: join golang/riscv64 maintainer team1d
CC @golang/riscv64. Input from current members of the golang/riscv64 maintainer team would be helpful.dmitshur commented on golang.org/x/website/cmd/golangorg: add <head> tag before </head>1d
(1 comment)
Yeah, thanks for catching that. Normally <aaa/> would be self-closing syntax, but it seems that's not valid for a non-void element like head and it gets treated as a start tag. Fixed …dmitshur reviewed +2 on golang.org/x/perf/...: fix some comments to improve readability1d
dmitshur commented on golang.org/x/website/cmd/golangorg: add support for module form of Go distributions1d
(1 comment)
Thanks. Sent CL 790820.dmitshur opened a change golang.org/x/website/cmd/golangorg: add <head> tag before </head>1d
The HTML spec is permissive enough that missed <head> tags do not
generally cause problems, but fix up a few places for consistency.Last Week
dmitshur reviewed +2 on golang.org/x/text/...: fix some comments3d
Thanks.dmitshur commented on : policy for removing GODEBUG flags3d
This proposal has a release-blocker label which applies to the issue's milestone, and it's currently in Backlog. Moved to Go 1.27 since I think that's what was intended, though perhaps it doesn't nee…dmitshur commented on golang.org/x/website/_content/doc: update contribution guide to prefer gerrit over github3d
(1 comment)
Whoops, I forgot that CL 342610 and its attempted replacements haven't actually landed yet and this file is still written in HTML. :( Sent you CL 790480 to fix the broken link.dmitshur opened a change golang.org/x/website/_content: fix some broken Markdown links3d
I noticed "[GerritBot](/wiki/GerritBot)" rendered as text rather than a
link at go.dev/doc/contribute. That happened because that file is still
written in HTML rather than Markdown. Fix it and some m…dmitshur reviewed +2 on golang.org/x/mod/...: fix some comments to improve readability3d
Thanks.dmitshur commented on golang.org/x/text/...: fix some comments3d
Thanks. I left a few minor comments, otherwise this looks good.
Since this returns a boolean, and the boolean represents whether something is or isn't the case, I suggest using "reports whether" as …dmitshur closed a change golang.org/x/text/...: update to unicode 16.0.03d
Thanks for working on this. x/text was updated to Unicode 17 in CL 737404, so closing. If there's some aspect of this CL other than updating the Unicode version is still applicable, feel free to reop…dmitshur closed a change golang.org/x/text/...: update to unicode 16.0.03d
Thanks for working on this. x/text was updated to Unicode 17 in CL 737404, so closing. If there's some aspect of this CL other than updating the Unicode version is still applicable, feel free to reop…dmitshur closed a change golang.org/x/text/...: drop pre go1.16 versions (and unicode tables)3d
Thanks for making this change. These old files (and more) are dropped via CL 738580, so closing.dmitshur reviewed +2 on golang.org/x/playground/...: fix some comments to improve readability3d
Thanks.dmitshur reviewed +2 on golang.org/x/perf/...: fix some comments to improve readability3d
Thanks.dmitshur commented on cmd/go: start a test case for an insecure scheme check4d
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 scope4d
Thanks.dmitshur commented on cmd/dist: redefine goTest.timeout to raise timeout only, not lower it4d
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 minutes4d
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 edits4d
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 edits4d
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.04d
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 TODOs4d
Thanks.dmitshur commented on golang.org/x/pkgsite/go.mod: update golang.org/x dependencies5d
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 cmd5d
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 close5d
Thanks.dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27: break go command notes to subsections5d
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 removal5d
Thanks.dmitshur opened a change cmd/dist: drop 3 minute timeout for a few packages that get close5d
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 timeouts5d
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 dependencies5d
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 GODEBUG5d
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"5d
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"5d
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=go1265d
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 TestConfigEnvDoesNotInheritProcessEnv5d
Thanks.dmitshur starred github.com/sourcegear/diffmerge6d
dmitshur commented on golang.org/x/tools/go/packages: propagate PATH in TestConfigEnvDoesNotInheritProcessEnv6d
This change is ready for review.dmitshur opened a change golang.org/x/tools/go/packages: propagate PATH in TestConfigEnvDoesNotInheritProcessEnv6d
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=nojsonv26d
Thanks.dmitshur opened a change cmd/dist: test encoding/json/... with GOEXPERIMENT=nojsonv26d
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 tip6d
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 builders6d
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` step6d
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.276d
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 annotation6d
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.271w
(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 7870201w
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" changes1w
Thanks.dmitshur reviewed +1 on golang.org/x/build/internal/gophers: add alex to gophers1w
Indeed. :)dmitshur commented on golang.org/x/review/git-codereview: give actionable error when Gerrit cookie is missing1w
(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 change1w
Thanks.dmitshur closed an issue golang.org/x/text/unicode/bidi: DefaultDirection(LeftToRight) is silently ignored [1.25 backport]1w
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]1w
No backport rationale provided here nor in https://github.com/golang/go/issues/71809#issuecomment-4461422460.dmitshur commented on runtime: version parsing fails1w
@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…Earlier
dmitshur commented on golang.org/x/website/solutions: update urfave/cli link to v3 GitHub URL1w
@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 URL1w
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.