Activity
Today
dmitshur commented on golang.org/x/build/main.star: enable linux-amd64-typesalias presubmit for Go 1.23 and tip1h
Confirmed that the new builders are passing everywhere in post-submit, so now turning on in pre-submit too. Please do let me know if you spot anything unexpected.
Thanks.dmitshur commented on golang.org/x/tools/go/callgraph/vta: TestVTACallGraph fails with GODEBUG=gotypesalias=11h
Thanks for fixing this. I used the fact this was recently broken to confirm the x_tools-gotip-linux-amd64-typesalias builder being added for #68798 is working as expected, and it's indeed passing at …dmitshur commented on os: Windows-style absolute paths are not recognized correctly in wasm environments2h
Bugs are first fixed on the main branch, where development for the upcoming major release (Go 1.24) is happening. If appropriate, fixes may be backported to supported minor versions. See https://go.d…dmitshur commented on syscall: fix path expansion on non-unix platforms2h
I think it's okay not to worry about that in the context of this particular change. It's documented at https://go.dev/wiki/WebAssembly that mixing .wasm files and wasm_exec.js across major Go version…dmitshur commented on golang.org/x/build/main.star: enable linux-amd64-typesalias presubmit for Go 1.23 and tip3h
Thanks. I'll wait for these builders to run in post-submit and check that it's passing before submitting this.
It's documented [here](https://chromium.googlesource.com/infra/luci/luci-go/+/HEAD/luci…dmitshur commented on os/user: speed up Current on Windows3h
Note that there are 7 failing tests on windows/arm64 as of this change. See https://ci.chromium.org/b/8740108915777961249.dmitshur commented on syscall: fix path expansion on non-unix platforms4h
(1 comment)
Is there an existing issue for this? If not, it would help to make one and replace this with "Fixes #nnn." See https://go.dev/doc/contribute#design.
I understand this problem needs to b…dmitshur reviewed +2 on LICENSE: update per Google Legal7h
Thanks.dmitshur commented on golang.org/x/build/main.star: enable linux-amd64-typesalias postsubmit for Go 1.23 and tip8h
(1 comment)
Sounds good. I expect the plan for that would be to add a builder with a different run_mod, and configure it accordingly for the needs of the type parameterized aliases work. The new bui…dmitshur commented on os: [release-branch.go1.23] fix Chtimes test flakes9h
(1 comment)
Please update this to:
```
For #68687
For #68663
Fixes #68812
```
We'll also need to backport this to 1.22 (#68811) since the same problem exists there. And this is probably fine to le…dmitshur commented on os: TestChtimes failures9h
@gopherbot Please backport to Go 1.23 and 1.22. This is a test-only fix that makes a test more resilient against other programs possibly reading files in the background. It's also needed to fix test …dmitshur commented on cmd/go: bootstrapped vet tool for 1.23 lacks GODEBUG=gotypesalias=110h
I believe the latter is intended, see [here](https://cs.opensource.google/go/go/+/master:src/cmd/dist/README;l=12-13;drc=09c886d3936f5e1d34d7e467990622b9a1e0f858) and [here](https://cs.opensource.goo…Yesterday
dmitshur commented on golang.org/x/website/_content/doc/go1.23: os: document pidfd22h
(1 comment)
@kolyshkin@gmail.com Are you planning to make additional wording changes based on this suggestion, or would you like this CL to be submitted as is? (Asking because I'm not sure if you sa…dmitshur commented on cmd/vet: broken with GODEBUG=gotypesalias=123h
Thanks. For this to be fixed in go1.23.1, it'll still need to be fixed at tip first and then backported, so moving this to Go1.24 milestone and re-adding release-blocker. Please use the usual process…dmitshur opened a change golang.org/x/build/main.star: enable linux-amd64-typesalias presubmit for Go 1.23 and tip23h
This is separated out in a separate CL for a more gradual rollout.
Fixes golang/go#68798.dmitshur commented on golang.org/x/tools: add 1.23 and tip builders with GODEBUG=gotypesalias=11d
These builders were recently updated in [CL 601177](https://go.dev/cl/601177) to stop existing after 1.22 based on the idea that 1.23 and later already default to GODEBUG=gotypesalias=1. I'll refine …dmitshur commented on golang.org/x/build: add LUCI aix-ppc64 builder1d
> The cpu and cipd_platform values look suspect.
Indeed. The CIPD binaries built for GOOS=aix GOARCH=ppc64 are using "aix-ppc64" (https://chrome-infra-packages.appspot.com/p/infra/experimental/gol…dmitshur commented on os: TestChtimesOmit failures1d
> If someone who maintains LUCI runners can take a look at what was changed in environment used by `gotip-linux-arm`, I'd be grateful.
I commented on this in https://github.com/golang/go/issues/68…dmitshur commented on os: TestChtimes failures1d
The linux-arm builder had a change in its environment on 2024-07-29, which coincidences with the timing of this test becoming flaky on that builder. The host VM started running additional security so…dmitshur commented on golang.org/x/exp/go.mod: update golang.org/x dependencies1d
Adding bypass since the only failing test (TestAPIConsistency) is failing in the same way as at tip. We need to fix it, but submitting this in the meantime.This Week
dmitshur reviewed +2 on golang.org/x/build/cmd/relui: add instructions to enable github client in local relui1d
Thanks.
(nit) s/Github/GitHub/
It's also fine to write "github" when it needs to be an unexported Go identifier or so, but the capitalized version has a capital 'H' too.dmitshur commented on api: audit for Go 1.232d
For posterity: `type QUICConfig struct, EnableStoreSessionEvent bool` field has since been renamed to `EnableSessionEvents` after a deliberate decision in #68124.dmitshur commented on golang.org/x/website/_content/blog: move go12 to go1.22d
Thanks.dmitshur commented on lib/time: update tzdata before release2d
We're about a week away from the planned go1.23.0 release, and 2024a is still the latest available tzdata version. At this point we'll likely keep it, so moving this to the next milestone. We can rev…dmitshur commented on golang.org/x/build/internal/task: add pre-release tag to release branch's head2d
(1 comment)
Right.dmitshur opened a change golang.org/x/website/_content/blog: move go12 to go1.22d
All blog posts for major Go releases include a dot, other than the one
for Go 1.2. Fix the inconsistency and add a redirect from the old page.dmitshur commented on golang.org/x/website/_content/doc/go1.23: os: document pidfd2d
(1 comment)
Thanks. Is #65319 the right issue number? It’s not mentioned by CL 570036 nor CL 570681.dmitshur commented on golang.org/x/build/cmd/watchflakes: should report broken or missing builders3d
There's a recent case where watchflakes doesn't seem to be reporting a consistent failure. Filed #68753 for it.dmitshur opened an issue golang.org/x/build/cmd/watchflakes: should report x/ repo breakage caused by Go repo change3d
A part of issue #58819 was was making watchflakes report, rather than skip, cases of consistent test failures.
golang.org/x/exp/typeparams.TestAPIConsistency is currently broken (e.g., https://ci.…dmitshur reviewed +2 on golang.org/x/build/internal/task: add pre-release tag to release branch's head3d
This generally looks good. +2 for after inline comments are resolved. Feel free to also wait for Rob to look too before submitting. If there are significant changes, let me know and I can review agai…dmitshur reviewed +2 on golang.org/x/build/internal/task: verify gopls installation and run smoke test3d
Thanks.
(minor) Since `dependencyCommit` is now being passed as a value, no need to separately pass it via `wf.After`. It would be a no-op.
```suggestion
_ = wf.Action1(wd, "verify installing late…dmitshur closed an issue github.com/go-gl/glfw/Vulkan: Loader not found3d
dmitshur commented on golang.org/x/website/internal/history: catch empty non-nil fix summaries in TestReleases3d
Thanks.dmitshur commented on golang.org/x/website/internal/history: document Go 1.22.6 and 1.21.13 releases3d
(1 comment)
Good catch. The template would otherwise say "includes security fixes" without elaborating further, which is not useful (and not true for this release). Sent CL 603495 to improve the tes…dmitshur opened a change golang.org/x/website/internal/history: catch empty non-nil fix summaries in TestReleases3d
When there are no security fixes (or no bug fixes) in a given release,
it's represented with a nil *FixSummary. Since it also may reasonably
be represented with an empty FixSummary, add a test to enf…dmitshur commented on go/version: treat prerelease patch as valid version3d
Thanks for preparing the change. Adding wait-issue while the issue is still in NeedsInvestigation and not NeedsFix state (see https://go.dev/wiki/HandlingIssues#issue-states).dmitshur commented on go/version: IsValid reports false on inputs like "go1.8.5rc5", "go1.9.2rc2"3d
At this time it's not clear to me that the scope of this package intends to consider a number of one-off old (pre-go1.21.0) releases that followed the "goA.B.CrcN" pattern as valid. The package comme…dmitshur closed an issue golang.org/x/build: add LUCI openbsd-riscv64 builder3d
No problem, thanks for the update and for working on this. If the situation changes in the future, please feel free to file another new-builder issue. The openbsd-riscv64-mengzhuo-1701367184 certific…Last Week
dmitshur commented on golang.org/x/build: add LUCI openbsd-riscv64 builder6d
We can consider that if it is needed. The relevant parts within LUCI configuration are the [enabled](https://cs.opensource.google/go/x/build/+/luci-config:main.star;l=1748-1811;drc=6e15c992638ed13979…dmitshur commented on golang.org/x/build: add LUCI openbsd-riscv64 builder6d
@mengzhuo Friendly ping on this. Are you planning to continue to work on this builder, or should this be closed in favor of issue #67105?
By now the [CIPD dependencies](https://chrome-infra-packag…dmitshur starred github.com/AllenDang/giu1w
dmitshur starred github.com/jetsetilly/ivycel1w
dmitshur commented on github.com/go-gl/glfw/Vulkan: Loader not found1w
Since you're running into this problem in the context of that project, I suggest you report an issue in that project's issue tracker, and reference this issue if needed.dmitshur commented on github.com/go-gl/glfw/Vulkan: Loader not found1w
It sounds like Vulkan via MoltenVK on darwin/arm64 doesn’t work. It might not be supported.
Do you know if this is supported in the upstream GLFW C library? If it is, we should work on adding that…dmitshur commented on crypto/internal/cryptotest: add tests for the cipher.Stream interface1w
(1 comment)
I think I spotted the answer:
> It's meant to be eventually exposed in x/crypto for third-party
implementations to use and vendored back.dmitshur commented on crypto/internal/cryptotest: add tests for the cipher.Stream interface1w
(1 comment)
@filippo@golang.org Proposal #25309 talks about a new cryptotest package in x/crypto repo, but this is an internal package in the main Go repo. Can you briefly clarify why that is; are t…dmitshur reviewed +2 on golang.org/x/build/cmd/watchflakes: ignore builders with known issue1w
Thanks.dmitshur reviewed +2 on golang.org/x/build/main.star: add linux-amd64-longtest-swissmap builder1w
Thanks.
Nice approach!
Yeah, 'includable_only: true' is what you're looking for to disable pre-submit. The way to parse it is that "this builder runs in pre-submit only if included via Cq-Include-T…dmitshur commented on golang.org/x/build: Add linux-amd64-longtest-swissmap builder1w
(1 comment)
(nit) Lower-case s/Add/add/ per https://go.dev/doc/contribute#first_line.
Optionally maybe also add a "main.star: " suffix like most other CLs.dmitshur commented on golang.org/x/build: Add linux-amd64-longtest-swissmap builder1w
(2 comments)
Since this is intended to be a temporary builder, consider adding a comment that'll make it easier to remove it when it's no longer useful. Something like:
// This can be deleted after…dmitshur reviewed +2 on golang.org/x/build/cmd/relui: add instructions to enable cloud build in local relui1w
Thanks for documenting this.
Replied internally about what's available to us on the Go team.
For anyone else, we can't make a meaningful suggestion of what GCP project can be used, so I think it's f…dmitshur reviewed +2 on golang.org/x/build/internal/task: update x/tools dependency and create a CL for review1w
Nice work on this step. I'm sure there'll be minor things you'll want to come back and tweak later on, but this sets up a good general structure. Thanks.
Is `-u` included here intentionally because …dmitshur reviewed +2 on golang.org/x/build/internal/task: update codereview.cfg and create a CL for review1w
Thanks.
Yep, it's completely fine to keep it since it's expected to be used soon.
Since the task package has functionality for Go toolchain releases that also involves release branches, please incl…dmitshur commented on golang.org/x/oauth2/endpoints: add GitLab DeviceAuthURL1w
I'm not the right reviewer for this repository. Hopefully @codyoss@google.com or @shinfan@google.com will have a chance to look.dmitshur commented on golang.org/x/build/main.star: unset known issue for openbsd/ppc64 builder type1w
Thanks again!dmitshur reviewed +2 on golang.org/x/build/cmd/watchflakes: post new issues to GitHub at the end1w
(1 comment)
Thanks, that makes it very clear. :)dmitshur commented on golang.org/x/build: add LUCI openbsd-arm64 builder1w
@4a6f656c The builder seems to be doing well. It was fully passing at tip and on release-branch.go1.23, and passing short of 2-3 tests on older release branches. In case you're not aware, the bot has…dmitshur commented on golang.org/x/build: add LUCI openbsd-ppc64 builder1w
Following up here as some time has passed and the builder appears to have been doing well [recently](https://ci.chromium.org/ui/p/golang/g/port-openbsd-ppc64/builders). @n2vi Okay to submit [CL 59681…dmitshur reviewed +2 on golang.org/x/build/cmd/watchflakes: post new issues to GitHub at the end1w
Thanks.
(minor) You could consider also changing the function name not to begin with the report verb anymore, to make it easier at the call sites to tell it doesn't post. Perhaps `prepareNew` or so.dmitshur commented on golang.org/x/build/main.star: mark windows-arm64 host as slower1w
Thanks.dmitshur closed an issue golang.org/x/build: add a Windows ARM builder 1w
This issue was tracking adding this builder to the coordinator, but that's replaced with LUCI now. Closing this in favor of #67308.dmitshur closed an issue golang.org/x/build: windows-arm64-10 builder occasionally crashes during sequential all.bash test execution1w
We don't have the windows-arm64-10 builder anymore; the current [windows-arm64](https://ci.chromium.org/ui/p/golang/builders/luci.golang.ci/gotip-windows-arm64) LUCI builder runs Windows 11, and does…dmitshur opened a change golang.org/x/build/main.star: mark windows-arm64 host as slower1w
As of a recent builder change related to its security, this host has
started to take longer to run tests and runs into timeouts. Examples:
- https://ci.chromium.org/b/8740874888365769313
- https://c…dmitshur reviewed +2 on golang.org/x/build/cmd/gomote: add a gomote login command1w
Thanks.
(minor) Maybe `tokenFile` can work better, since it doesn't seem there's more configuration stored in that file other than the token.
This new function is called in exactly one place and th…dmitshur commented on golang.org/x/build/internal/task: update codereview.cfg and create a CL for review1w
This is good progress. Please see some inline comments.
Yes, this looks good. `wf.After(branchCreated)` will cause the "updating branch's codereview.cfg" task to run only after "creating new branch …dmitshur closed an issue cmd/compile: internal compiler error: 'foo.toStrings': value v2 still has 1 uses1w
Closing as duplicate of #65808.dmitshur commented on cmd/go: change go test to FAIL if `-run` is specified but no tests match1w
@navytux That's a good point. In the original proposal here, there was only the -run flag specified, without -bench, so it likely shouldn't apply if something else, like benchmarks, does match and ru…dmitshur reviewed +2 on golang.org/x/website/tour: add a Spanish translation1w
(1 comment)
Thanks. The 'Resolved' box in this thread needed to checked to mark it as resolved. That's done now.dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.23: fix a typo and some formatting1w
Thanks.dmitshur reviewed +2 on golang.org/x/website/tour: add a Spanish translation1w
dmitshur reviewed +2 on golang.org/x/website/tour: spanish translation added1w
Thanks. See inline comment to keep the list sorted.
This list is currently sorted by the language name in English. Please move the new entry here, between Polish and Thai, to keep it sorted.dmitshur reviewed +2 on golang.org/x/build/cmd/watchflakes: set RPC deadline1w
Thanks.dmitshur commented on golang.org/x/build/cmd/gomote: add a "# " prefix for all logs1w
(1 comment)
`usage` should probably write to `flag.CommandLine.Output()` directly (not via log package), otherwise it now gets spurious '# ' prefixes:
```
$ gomote -help …dmitshur commented on build: adopt Go 1.22 as bootstrap toolchain for Go 1.241w
There's an order we'll need to follow for this.
First, the builders need to be configured to start using Go 1.22 as the bootstrap. I'll be working on this part.
After that's done, cmd/dist and …dmitshur commented on golang.org/x/build/internal/task: create a new branch for minor release1w
(2 comments)
No problem, thanks.
Noting for the future: at some point when you're ready to try this workflow in prod, we'll need to grant relui permissions to create branches and for any other step…dmitshur reviewed +2 on golang.org/x/build/internal/task: create a new branch for minor release1w
Thanks.
Please change this to be a complete sentence as discussed in https://go-review.googlesource.com/c/build/+/601655/comment/77c9440b_467eca76/.dmitshur commented on : build failure >= 1.18 on windows arm321w
I sent [CL 601777](https://go.dev/cl/601777) to mark this port broken per https://go.dev/wiki/PortingPolicy#broken-ports. CC @golang/windows.dmitshur commented on golang.org/x/build/internal/task: create a new branch for minor release1w
(6 comments)
Please replace 'gerrit/gerrit' with 'gerrit'.
You can use the short URL form for Go issues, go.dev/issue/nnn. That is:
A local relui screenshot is at https://go.dev/issue/57643#issuec…dmitshur opened a change cmd/dist: mark windows/arm (32-bit ARM) broken1w
The port is reportedly broken, and there isn't a builder testing it.
For #68552.
For #67308.dmitshur reviewed +2 on golang.org/x/build/cmd/gomote: emit repro test commands with CWD set to package directory1w
Thanks. Glad there's a simple way to handle this need!
(minor) The new code cuts a prefix, then immediately re-joins it. So if you want, this could be simplified to:
```
if strings.HasPrefix(rest, …dmitshur starred github.com/jmigpin/editor1w
dmitshur reviewed +2 on golang.org/x/build/cmd/gomote: remove duplicate "# " prefix to log messages1w
Thanks. Nice use of switching to the log package.
One more missed here. It should be caught with `sed -i 's/log.Print("# /log.Print("/g' *.go`.dmitshur commented on golang.org/x/build/cmd/gomote: use log package consistently1w
(1 comment)
Certainly not for this CL, but in general: since there aren't many backticks involved in this usage text, consider using an inline multi-line raw string literal to improve readability an…dmitshur reviewed +2 on golang.org/x/build/cmd/gomote: use log package consistently1w
Thanks.
What’s the reason Fprintln (with ln at the end) is replaced with log.Print instead of with log.Println? Is it because none of the Fprintln calls pass more than argument, so their behavior …dmitshur reviewed +2 on golang.org/x/build/cmd/gomote: move log flag setup to main1w
Thanks.dmitshur commented on golang.org/x/crypto/x509roots/fallback: tag versions to reduce version churn from x/crypto commits1w
The x509roots/fallback module updates are done via a [relui workflow](https://cs.opensource.google/go/x/build/+/master:internal/task/x509bundle.go;drc=0166ca569d279a3a154a863db9e87e81ca9c8082). It cu…dmitshur reviewed +2 on golang.org/x/build/internal/task: add definition for releasing go pls1w
Thanks.
(nit) Delete the space.
For consistency with Rob's earlier comment:
```suggestion
releaseGoplsTasks := task.ReleaseGoplsTasks{
```
```suggestion
return false, fmt.Errorf("failed to get…dmitshur reviewed +1 on golang.org/x/build/cmd/gomote: add repro subcommand1w
I looked only briefly since Carlos already reviewed it. Thanks.
(minor) Is it useful to control log flags per sub-command? I've seen it more commonly done once at the top of main for the entire comm…dmitshur commented on runtime/secret: implement new secret package1w
(1 comment)
This file should be named `api/next/21865.txt` for now; it'll be merged along with others into api/go1.24.txt at the start of the Go 1.24 release freeze.dmitshur commented on go/version: incorrect version comparison result, go1 is bigger than go1.8.5rc51w
The reason go1.8.5rc5 sorts as oldest is because the go/version package doesn't consider it valid; go1 isn't very relevant as you could pick any other valid version to compare with. [Compare](https:/…dmitshur commented on golang.org/x/build/main.star: add a linux/amd64 x/website trybot for release note CLs1w
Thanks.dmitshur commented on golang.org/x/build/main.star: add a linux/amd64 x/website trybot for release note CLs1w
(1 comment)
Hmm, I'm not sure this will add much value to run on release branches. I'll make it gotip only to start out with. Done in PS 2.dmitshur opened a change golang.org/x/build/main.star: add a linux/amd64 x/website trybot for release note CLs1w
Copying the test to the main Go repository requires vendoring x/net/html
as a test-only dependency, which works (prototype in go.dev/cl/600816)
but adds size and requires a copy of the test to be mai…dmitshur opened an issue golang.org/x/build: reuse x/website's TestAll for catching problems in release note fragments in the main Go repository1w
x/website has an [excellent test](https://cs.opensource.google/go/x/website/+/master:cmd/golangorg/server_test.go;l=89-258;drc=b52edba3f82120d0f819beafbfda191a0a747241) for issue #37047 that catches …dmitshur commented on golang.org/x/build/internal/relui: require group membership for workflows1w
(3 comments)
This import isn't being used. (Detected via a build error.)
This return was needed previously, but it's less useful now. :)
Seeing this IsMemberOfAny call fail with 403:
```
2024/07/…dmitshur closed an issue golang.org/x/build: gotip-freebsd-riscv64 builder is in quarantined state1w
Thanks!Earlier
dmitshur pushed to
main in github.com/shurcooL/githubv41w
dmitshur commented on cmd/compile: allow parameterized type aliases to omit redundant type parameters1w
CC @golang/compiler.dmitshur reviewed +2 on golang.org/x/build/cmd/watchflakes: coalesce a package-level failure to a test failure in the same package1w
Thanks.
This seems like an improvement to me. Sounds good to update the few existing issues, thanks.dmitshur reviewed +2 on golang.org/x/build/cmd/watchflakes: add -build flag for analyze a single build1w
Thanks. Just minor comments.
Maybe while you're here:
```suggestion
if b.GetBuilder().GetBuilder() != bName { // coherence check
```
(I realize the code is just moving, so this is optional.)
If …