Activity
Today
dmitshur commented on golang.org/x/oauth2/internal: include clientID in auth style cache key40m
(1 comment)
And clientID? Or maybe this comment is no longer needed, now that the map key type is the named authStyleCacheKey type with field names and not just a `string`.dmitshur commented on internal/fuzz: enable fuzz testing on OpenBSD 1h
It would be good to mention this in Go 1.25 release notes (#71661), otherwise users may not know about it. See [doc/README.md](https://cs.opensource.google/go/go/+/master:doc/README.md).dmitshur commented on runtime: add comparison tracing for libFuzzer on loong641h
(1 comment)
Is it intentional that `|| openbsd` is being removed here? It's not called out in the commit message and seems to be undoing a recent change in CL 499335 that enabled fuzz testing suppor…dmitshur reviewed +2 on golang.org/x/build/main.star: add the darwin-arm64-longtest and race builder type1h
(2 comments)
If you do change it, this needs updating too.
To add some data that we have available via telemetry: https://telemetry.go.dev/#charts%3acmd%2fgo%3ago%2fplatform%2fhost%2fdarwin%2fmajor…dmitshur reviewed +2 on golang.org/x/build/main.star: add the darwin-arm64-longtest and race builder type1h
Thanks.
I wonder if it would work slightly better to default to macOS 15 for darwin/arm64. It's the latest macOS version available, and it's likely more representative of typical darwin/arm64 users.…dmitshur commented on golang.org/x/oauth2/google: findDefaultCredentials fallback to ComputeTokenSource should be predicated on availability on service account5h
@seankhliao Can you please clarify what you mean with that comment? Why is this issue closed as not planned?dmitshur commented on golang.org/x/build: add darwin-arm64 longtest and race builders5h
CC @JunyangShao, @prattmic.Yesterday
dmitshur fixed an issue build: build failure on x/crypto GOARCH=loong64 with Go 1.23/1.24 [consistent failure]1d
These build failures are fixed via revert CLs above. Closing.This Week
dmitshur commented on net/http: upon http redirect, copy Request.GetBody in new request2d
Adding @dneil@google.com who is a better reviewer for the net/http package.dmitshur reviewed +2 on golang.org/x/build/perf: require IAP authentication for search access2d
Thanks.
before?
It might be worth documenting (via a doc comment and here and the App.IAP field) that isAuthenticated opts to report true when IAP is disabled ("deemed authenticated"), rather than …Last Week
dmitshur starred github.com/apple/pkl-go-examples5d
dmitshur starred github.com/apple/eidas5d
dmitshur commented on encoding/json: add json/v2 with GOEXPERIMENT=jsonv2 guard6d
(1 comment)
Is it WAI that both of these differently-named cases have an identical implementation? (I'd expect it could be written as `case "StringEscaped", "StringUnicode":` if it were intentional,…dmitshur fixed an issue golang.org/x/build: gotip-linux-amd64-misccompile builder fails with no space left on device6d
We haven't seen this happen for a while, right?
I believe this was a combination of some short-lived builder VMs inadvertently staying around for much longer than intended, and some cache directorie…dmitshur fixed an issue golang.org/x/build: set a maintenance window for the kubernetes instance6d
It seems to be generally working well. Let's call this done; we can reopen if more is needed.dmitshur commented on golang.org/x/build: set a maintenance window for the kubernetes instance6d
It seems to be generally working well. Let's call this done; we can reopen if more is needed.dmitshur reviewed +2 on runtime: don't read /dev/random on Plan 96d
Ok, thanks for confirming. Please do:
1. edit the commit message to include `Cq-Include-Trybots: luci.golang.try:gotip-plan9-arm` right after Change-Id (see [example](https://go-review.googlesource.…dmitshur commented on golang.org/x/crypto/acme: return error from pre-authorization when unsupported1w
This seems ready, so submitting.dmitshur reviewed +2 on runtime: don't read /dev/random on Plan 91w
@0intro@gmail.com Did you see this comment? Is it intentional that you're trying to run the plan9-arm builder in old infra instead of in LUCI, despite it not being present there for a long time?
Is …dmitshur commented on golang.org/x/build/internal/migration: stop running linux-…-{bullseye,bookworm} builders1w
Thanks.dmitshur commented on golang.org/x/build/cmd/bootstrapswarm: allow hostname flag to be left empty on GCE1w
Thanks.dmitshur commented on golang.org/x/build/env/linux-x86-sid: add a Python 3 dependency1w
Thanks.
> pipes — Interface to shell pipelines
>
> Deprecated since version 3.11, removed in version 3.13.
> Applications should use the subprocess module instead.
([source](https://docs.python.o…dmitshur commented on cmd/compile: infinite loop in the inliner1w
We'll need to create a backport candidate issue for the Go1.24.3 milestone; moving this to Go1.25.dmitshur commented on golang.org/x/build: add LUCI linux-s390x builder1w
@srinivas-pokala I believe the change in [crrev.com/c/6439429](https://crrev.com/c/6439429) is rolled out and you should be able to try starting up the builder again.dmitshur fixed an issue golang.org/x/build/internal/relui: TestRelease/major failures [consistent failure]1w
Fixed by [CL 664495](https://go.dev/cl/664495).dmitshur fixed an issue golang.org/x/build/env: host-linux-amd64-sid (Container Image: linux-x86-sid:latest)1w
Closed by merging [CL 665856](https://go.dev/cl/665856) (commit golang/build@48b27723f5bd8d92de3f0fff371a73debc7d8feb) to `luci-config`.dmitshur commented on golang.org/x/build/main.star: finish upgrading freebsd-amd64 to FreeBSD 14.21w
(1 comment)
Updated to keep a few 14.1 builders behind a bit longer. No need to block 14.2 from otherwise moving forward.dmitshur commented on golang.org/x/build/main.star: finish upgrading freebsd-amd64 to FreeBSD 14.21w
Copied Votes:
* Code-Review+1, Code-Review+2 (copy condition\*: "**is:ANY**")
Outdated Votes:
* LUCI-TryBot-Result+1 (copy condition: "changekind:NO_CODE_CHANGE")
\* The label has `labelCopyEnforce…dmitshur commented on golang.org/x/build/main.star: remove linux-{386,amd64}_debiansid known issues1w
Thanks.dmitshur commented on golang.org/x/build/env/linux-x86-sid: add a Python 3 dependency1w
Outdated Votes:
* Code-Review+1, Code-Review+2 (forced copy restriction\*: "**changeis:review-enforced_gerrit** AND NOT changekind:TRIVIAL_REBASE_WITH_MESSAGE_UPDATE AND NOT (**uploaderin:trusted-use…dmitshur reviewed +2 on golang.org/x/build/internal: remove duplicate package import1w
Thanks.
I'll note that in general it's not a problem to import a package under different identifiers, if done intentionally. For example, sometimes package "path" is useful to import as is, but also…dmitshur reviewed +2 on golang.org/x/build/main.star: remove darwin-arm64_14 and darwin-arm64_15 known issues1w
Thanks. Congrats!
Just noting that at some point, as darwin/arm64 continues to become more representative of darwin than darwin/amd64, we'll want to move these over (or alternatively add parallel ar…dmitshur reviewed +2 on golang.org/x/build/cmd/makemac: refresh amd64 images1w
Thanks. It's great that this behavior is improved. It can be useful in situations like in https://go.dev/issue/73124#issuecomment-2786030348.dmitshur commented on crypto/rand: add and update examples1w
Thanks.dmitshur commented on crypto/rand: add and update examples1w
Previously all sentences in this file ended with a period. This change adds 7 new comments: some of them end with a period, some do not, and there's not clear pattern (e.g., ExampleInt and ExampleTex…dmitshur opened a change golang.org/x/build/internal/relui: keep parent PATH, add skip if no python31w
Improve the ability of tests to run in more environments. In the general
case, it's not viable to assume that replacing the parent environment's
PATH with '/bin:/usr/bin' will work, it can cause some…dmitshur closed an issue golang.org/x/build: consider adding a linux builder with a newer kernel1w
The new issue #73405 (CC @prattmic) is a continuation of this, folding this into that.dmitshur closed an issue golang.org/x/build/env/linux: add bleeding edge kernel builder1w
The new issue #73405 (CC @prattmic) is a continuation of this, folding this into that.dmitshur commented on golang.org/x/build: recent Linux kernel builder1w
We can try moving the sid builder (#61112) to cos-dev (up from cos-113-lts). If that ends up being too new and too noisy to be useful, we can move down to something less bleeding edge like cos-beta o…dmitshur commented on math/big: optimize addMulVVW function for loong641w
(1 comment)
This reverts a change made recently in CL 664895 of replacing addMulVVW with addMulVVWW.
If that's intended, it's probably worth explaining why in the commit message.dmitshur reviewed +2 on golang.org/x/build/cmd/makemac: add darwin-arm64-14 and darwin-arm64-151w
Thanks.dmitshur commented on golang.org/x/build/main.star: add linux-386_debiansid1w
Thanks.dmitshur opened a change golang.org/x/build/main.star: add linux-386_debiansid1w
Move the linux-386-sid builder as it existed in the old infra to LUCI.
For golang/go#61112.dmitshur commented on golang.org/x/benchmarks/sweet: don't build tsgo on go1.231w
(1 comment)
Oh, actually not strictly necessary: https://ci.chromium.org/b/8717510946024848017 passed. (It's because it's on a release branch rather than the main branch, so the language version is …dmitshur commented on golang.org/x/build/main.star: add linux-amd64_debiansid1w
Copied Votes:
* Code-Review+1, Code-Review+2 (copy condition\*: "**is:ANY**")
Outdated Votes:
* LUCI-TryBot-Result+1 (copy condition: "changekind:NO_CODE_CHANGE")
\* The label has `labelCopyEnforce…dmitshur commented on golang.org/x/build/main.star: add linux-amd64_debiansid1w
Thanks.
Ended up taking this out so I can see their results sooner; I can always put it back if needed.dmitshur closed an issue build: build failure on gotip-linux-amd64-misccompile [consistent failure]1w
dmitshur fixed an issue math/big: wrong parameter names for mulAddVWW in loong64 assembly [consistent failure]1w
Fixed by [CL 665717](https://go.dev/cl/665717).dmitshur closed a change math/big: use the correct parameter name in the implementation of mulAddVWW on loong641w
The same change is applied via CL 665717, so closing. Thanks.dmitshur reviewed +2 on math/big: fix loong64 assembly for vet1w
Thanks.dmitshur commented on os: handle trailing slashes in os.RemoveDir on Windows1w
Bypassing the linux/loong64 compilation error found by misccompile builder, it's a pre-existing issue and not related to this CL. Everything else (including x_review-gotip-windows-amd64-longtest) pa…dmitshur commented on os: handle trailing slashes in os.RemoveDir on Windows1w
(1 comment)
FYI this will actually work now that CL 665017 landed, so please take advantage of this ability in future CLs whenever it's helpful. Thanks.dmitshur commented on golang.org/x/build/main.star: support blocking x/ repo SlowBots in the main Go repo1w
Thanks.dmitshur opened a change golang.org/x/build/internal/migration: stop running linux-{386,amd64}-sid builders1w
These builders have been migrated to LUCI.
For golang/go#61112.
For golang/go#67761.dmitshur commented on golang.org/x/build/cmd/buildlet/stage0: update Dockerfile to use Go 1.241w
Thanks.dmitshur commented on golang.org/x/build/internal/relui: only run TestRelease/major on 1.24 and newer1w
Thanks.dmitshur commented on golang.org/x/benchmarks/sweet: don't build tsgo on go1.231w
(1 comment)
I suspect it'll be necessary to apply the change made in CL 264938 to golangbuild for this logic to be able to work on LUCI builders (in addition to go1.23 devel toolchains compiled loca…dmitshur commented on golang.org/x/build/cmd/golangbuild: switch to generating a valid (per go/version.IsValid) version string1w
Yes, agreed, we should apply this to cmd/dist too. Retitled to capture that too.
As for why golangbuild (and coordinator, previously) generates a VERSION file, it does it for a few related reasons:
…dmitshur commented on golang.org/x/review: `~/go/bin/git-codereview change` fails with `git-codereview: not found`1w
It is intended for git-codereview to be available in PATH—see https://go.dev/doc/contribute#git-codereview_install. If you prefer not to add $HOME/go/bin to $PATH, it should still work to place the…dmitshur commented on golang.org/x/benchmarks/sweet: don't build tsgo on go1.231w
(1 comment)
A reminder that this x/benchmarks CL needs a 'golang/go' prefix here, it'll save you from needing to manually close those issues.dmitshur opened an issue golang.org/x/build/cmd/golangbuild: switch to generating a valid (per go/version.IsValid) version string1w
golangbuild currently generates VERSION files with strings like this, a behavior that the Go build system had for a while:
```
devel <commit>
devel <change>/<patchset>
```
That satisfies the proper…dmitshur commented on go/version: doesn't handle devel toolchain versions1w
The ability to be able to tell that a given Go version isn't a released version can be useful, and quite a bit of code relies on the presence of the "devel" substring as you mentioned. Instead of rem…Earlier
dmitshur commented on golang.org/x/website: automatic URL fragments for sub-sections of 'Minor changes to the library' are overly repetitive1w
I don't think it's viable to rely on manually specifying IDs as in that CL. People will forget to do it in some cases, and changing anchors afterwards on recent release notes might break links.dmitshur commented on runtime: don't read /dev/random on Plan 91w
The plan9-arm builder is [available in LUCI](https://ci.chromium.org/ui/p/golang/builders/luci.golang.ci/gotip-plan9-arm). Did you mean to trigger that one?
I've scheduled a run via the Choose Tryjo…dmitshur commented on runtime: reduce contention in (*lfstack).pop1w
There weren't any specific legacy trybots requested, to stopping this empty request.dmitshur commented on golang.org/x/build/main.star: support blocking x/ repo SlowBots in the main Go repo1w
Note for reviewer: patch set 1 makes the refactor but preserves previous behavior. Patch set 2 makes a small change to get the desired end result. So you might find it helpful to look at the PS1..PS2…dmitshur opened a change golang.org/x/build/main.star: support blocking x/ repo SlowBots in the main Go repo1w
It's possible for a CL that lands in the main Go repo to contain a bug
that is caught in post-submit by tests in one of the golang.org/x repos.
(See a recent example at go.dev/issue/73317.)
It's con…dmitshur reviewed +2 on golang.org/x/tools/go/analysis/passes/structtag: ignore findings for "encoding/..."1w
Thanks.dmitshur fixed an issue golang.org/x/build: migrate low-capacity (reverse) builders to LUCI1w
This umbrella issue is similar to the #60440 umbrella issue, we can merge them together. Closing this one. Thanks everyone.dmitshur opened a change golang.org/x/build/main.star: add linux-amd64_debiansid1w
Move the linux-amd64-sid builder as it existed in the old infra to LUCI.
For golang/go#61112.dmitshur opened a change golang.org/x/build/cmd/buildlet/stage0: update Dockerfile to use Go 1.241w
The x/build module has 1.23.0 as its minimum, and 1.19 doesn't support
toolchain upgrades.
For golang/go#61112.dmitshur reopened an issue golang.org/x/build/env: host-linux-amd64-sid (Container Image: linux-x86-sid:latest)1w
The build script was updated and a new LUCI-capable image was indeed built, though it's missing python3 (needed for the swarming bot). But we still need to add it to luci-config. Reopening for that.dmitshur commented on os: handle trailing slashes in os.RemoveDir on Windows1w
(1 comment)
Oh, I see now this actually runs into the "Failed to compute tryjob requirement" error when making it required this way.
I think I see the problem and we should be able to fix it so tha…dmitshur commented on golang.org/x/crypto/ssh: add integration tests against a reference implementation1w
(1 comment)
Notably, TestRunCommandSuccess was previously not running on js, wasip1.dmitshur commented on golang.org/x/crypto/ssh: add integration tests against a reference implementation1w
Note that on GOARCH=wasm builders, x/crypto/ssh/test.TestRunCommandSuccess/RunCommandSuccess is failing as seen on the [x/crypto (gotip)](https://ci.chromium.org/p/golang/g/x-crypto-gotip/console) vi…dmitshur reviewed +2 on golang.org/x/crypto/acme: return error from pre-authorization when unsupported1w
Thanks.dmitshur fixed an issue golang.org/x/build/cmd/racebuild: Error running create: failed to create buildlet on gotip-linux-loong641w
Yes, there is some similarity to #67468, but it's not the same issue. Over there it is a problem on the side of Gerrit, where for some reason triggering optional trybots doesn't use your primary Gerr…dmitshur reviewed +2 on golang.org/x/build/cmd/coordinator/internal/legacydash: use built-in max/min to simplify the code1w
Thanks.dmitshur commented on golang.org/x/review/git-codereview: do not write hooks if -n is given1w
There is a known regression affecting os.RemoveAll on Windows at tip, see issue #73317. It has a fix out and will be resolved soon, but we can bypass it since it's unrelated.dmitshur reviewed +2 on golang.org/x/website/doc: clarify Linux install instructions regarding permissions1w
Thanks.dmitshur commented on os: handle trailing slashes in os.RemoveDir on Windows1w
(1 comment)
I want to highlight that it's quite easy to add a required passing x/review trybot, since that might be helpful in the general case.
```suggestion
Change-Id: I21977b94bb08ff1e563de6f5f1…dmitshur reviewed +2 on golang.org/x/review/git-codereview: do not write hooks if -n is given1w
Thanks.
In some edge cases, this will produce slightly different results with -n flag compared to what it'd really do without -n flag, because this relies on os.Remove above changing the result of o…dmitshur commented on golang.org/x/review/git-codereview: do not write hooks if -n is given2w
(1 comment)
I think there’s a 'Publish Edit' button you still need to press for this to have effect.dmitshur commented on os: avoid symlink races in RemoveAll on Windows2w
Filed #73317 for it.dmitshur opened an issue golang.org/x/review/git-codereview: TestHooks and 5 more related tests are failing as of "os: avoid symlink races in RemoveAll" on Windows2w
As seen on the [x/review (gotip) by go commit](https://ci.chromium.org/p/golang/g/x-review-gotip-by-go/console) view, a number of tests have begun to fail starting with [CL 661575](https://go.dev/cl/…dmitshur commented on golang.org/x/review/git-codereview: do not write hooks if -n is given2w
(2 comments)
A small adjustment here to match the style suggested at https://go.dev/doc/contribute#ref_issues:
```suggestion
Fixes golang/go#73314
```
As noted in https://go.dev/doc/comment, Go co…dmitshur opened a change golang.org/x/build/internal/relui: only run TestRelease/major on 1.24 and newer2w
A fixup for CL 664216 to unbreak x/build tests with Go 1.23.dmitshur reviewed +2 on runtime: don't read /dev/random on Plan 92w
I'm not closely familiar with this, but two Plan 9 maintainers prefer to make this Plan 9-only change and it's moving back to previous behavior, so it seems okay to me. Thanks.
The commit message of…dmitshur commented on golang.org/x/review/git-codereview: `change -n` still writes the hooks without announcing2w
Thanks for reporting.
This should be a matter of adding a check for `*noRun` inside `installHook`, before it starts to do `os.Mkdir` and `os.WriteFile`.dmitshur commented on golang.org/x/crypto/ssh: add mlkem768x25519-sha256 Key Exchange algorithm2w
(1 comment)
In August when Go 1.25.0 is released, the go directive in go.mod will be bumped to `go 1.24.0`, so checking for the pre-release version will stop being needed.
It's fine as is, but you …dmitshur commented on golang.org/x/website: clarify Linux installation instructions regarding root/sudo permissions2w
Your suggested alternative sounds good. Please go ahead and send the change.
One minor suggestion, since that is a complete sentence inside the parentheses, let's also move the period to be inside.dmitshur commented on cmd/compile: internal compiler error: assertion failed with type aliases2w
CC @golang/compiler.dmitshur commented on crypto/x509: redundant UnknownExtKeyUsage check in verify.go2w
> So, considering this block of code: if cert has no ExtKeyUsage, but some UnknownExtKeyUsage, we also go through the remaining logic.
It looks like the current code results in such a cert contribut…dmitshur commented on crypto/x509: Go parsed a GeneralName with an incorrect tag2w
CC @golang/security.dmitshur commented on crypto/x509: Go accepts the IDP extension with DER encoding as an empty sequence2w
CC @golang/security.dmitshur commented on golang.org/x/crypto/acme: return error from pre-authorization when unsupported2w
(1 comment)
Does 'go test' pass locally without this changing too?dmitshur commented on net: deduplicate sendfile files2w
(1 comment)
This file is renamed and slightly modified, rather than being an entirely new file. It's probably not necessary to change the year in such cases, as mentioned in the last paragraph of ht…dmitshur commented on golang.org/x/build/cmd/racebuild: Error running create: failed to create buildlet on gotip-linux-loong642w
Does [gomote creation](https://go.dev/wiki/Gomote) work standalone from racebuild, or does it run into the same error? When authenticating, which of the two emails are you using, the @gmail.com one?
…dmitshur commented on github.com/OldUnreal/UnrealTournamentPatches: [469e-RC4] Mac Metal - gamma too high / looks washed out2w
The v469e-rc8 release notes mention something that appears to be relevant to this issue:
> Frucore will now use a 10 bpc frame buffer by default. This should make shadows look noticeably better
I t…dmitshur commented on net/http: initialize Value with File length in cloneMultipartForm2w
Only the commit message (and base commit) changed in PS 5, no need to run trybots again on it.