Activity
Last Week
dmitshur commented on golang.org/x/build: add LUCI openbsd-riscv64 builder1d
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 builder1d
@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/giu1d
dmitshur starred github.com/jetsetilly/ivycel1d
dmitshur commented on github.com/go-gl/glfw/Vulkan: Loader not found1d
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 found1d
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 interface1d
(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 interface1d
(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 issue2d
Thanks.dmitshur reviewed +2 on golang.org/x/build/main.star: add linux-amd64-longtest-swissmap builder2d
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 builder2d
(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 builder2d
(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 relui2d
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 review2d
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 review2d
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 DeviceAuthURL2d
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 type2d
Thanks again!dmitshur reviewed +2 on golang.org/x/build/cmd/watchflakes: post new issues to GitHub at the end2d
(1 comment)
Thanks, that makes it very clear. :)dmitshur commented on golang.org/x/build: add LUCI openbsd-arm64 builder2d
@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 builder2d
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 end2d
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 slower3d
Thanks.dmitshur closed an issue golang.org/x/build: add a Windows ARM builder 3d
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 execution3d
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 slower3d
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 command3d
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 review3d
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 uses3d
Closing as duplicate of #65808.dmitshur commented on cmd/go: change go test to FAIL if `-run` is specified but no tests match3d
@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 translation3d
(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 formatting3d
Thanks.dmitshur reviewed +2 on golang.org/x/website/tour: add a Spanish translation4d
dmitshur reviewed +2 on golang.org/x/website/tour: spanish translation added4d
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 deadline4d
Thanks.dmitshur commented on golang.org/x/build/cmd/gomote: add a "# " prefix for all logs4d
(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.244d
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 release4d
(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 release4d
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 arm324d
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 release4d
(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) broken4d
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 directory4d
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/editor4d
dmitshur reviewed +2 on golang.org/x/build/cmd/gomote: remove duplicate "# " prefix to log messages4d
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 consistently4d
(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 consistently4d
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 main4d
Thanks.dmitshur commented on golang.org/x/crypto/x509roots/fallback: tag versions to reduce version churn from x/crypto commits5d
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 pls5d
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 subcommand5d
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 package5d
(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.5rc55d
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 CLs5d
Thanks.dmitshur commented on golang.org/x/build/main.star: add a linux/amd64 x/website trybot for release note CLs5d
(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 CLs5d
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 repository5d
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 workflows5d
(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 state5d
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 …dmitshur commented on golang.org/x/tools/x/tools: update go.mod to 1.22 and gopls to go1.231w
That configuration lives on the `luci-config` branch of x/build, in `GO_BRANCHES` and `EXTRA_GO_BRANCHES`:
https://cs.opensource.google/go/x/build/+/luci-config:main.star;l=536-565;drc=5953629ed4eeb…dmitshur opened a change doc/next: introduce a problem (to see if it can be caught)1w
DO NOT REVIEW
DO NOT SUBMIT
Testing out an idea for checking links for problems (such as using
an absolute link instead of relative, non-canonical URLs, bad HTML,
and so on).dmitshur commented on crypto/tls: expose extensions presented by client to GetCertificate1w
(1 comment)
Please move this file to a directory called 'next' and rename to 32936.txt.
See https://cs.opensource.google/go/go/+/master:api/README;l=17-23;drc=600225f83f7846b1129af94253f5172a359c308…dmitshur reopened an issue golang.org/x/build: gotip-freebsd-riscv64 builder is in quarantined state1w
The bot is currently missing, last seen 11h 54m ago. CC @mengzhuo.dmitshur commented on golang.org/x/perf/cmd/benchstat: generate partial comparisons if some of the tests timeout on a particular Go version 1w
CC @golang/runtime.dmitshur commented on golang.org/x/build/dashboard: stop running remaining ported builders1w
Thanks.dmitshur opened a change golang.org/x/build/dashboard: stop running remaining ported builders1w
The gradual shutdown has gone smoothly and can be applied
to a couple more remaining builders that have been ported.
Fixes golang/go#67761.dmitshur reviewed +2 on runtime: remove darwin-amd64-10_14 builder check1w
Thanks.dmitshur opened a change net: unskip TestUDPZeroBytePayload on Darwin1w
From information we have available in the issue, this test was flaky
on OS X 10.10 but not newer. By now macOS 11 is the minimum required
version, and 1000 local runs of the test passed, so try to un…dmitshur commented on golang.org/x/build/internal/task: add definition for releasing go pls1w
Thanks, this is a great start! Left some inline comments, I think this should be ready once they're addressed.
This CL is a part of a larger work, and it helps to have an issue associated with it fo…dmitshur commented on golang.org/x/crypto/ssh: add support for unpadded RSA signatures1w
(Marking thread unresolved for visibility.)dmitshur commented on net/http: match os.File.Readdir behavior in DotFileHiding1w
This looks ready. Submit?dmitshur commented on golang.org/x/build/main.star: mark darwin-amd64 host as slower1w
(1 comment)
Thanks for clarifying. Added a brief note here too.dmitshur commented on golang.org/x/build/main.star: mark darwin-amd64 host as slower1w
(1 comment)
The fact that timeout scaling is multiplicative is mentioned as part of make_run_mod ("Compose timeout scaling factors by multiplying them", around line 700). Did you already see that bu…dmitshur closed an issue cmd/compile: panics with generic type alias and based-on constraint [1.23 backport]1w
@cuonglm Please note the second paragraph of https://groups.google.com/g/golang-dev/c/or4BmCu3LIE/m/a1Hv1yoxBAAJ, copied here:
> Any CLs that land after [CL 600176](https://go.dev/cl/600176) ("int…dmitshur commented on mime: incorrect mime-type for .mjs files on windows (text/plain instead of application/javascript)1w
CC @neild.dmitshur opened a change golang.org/x/build/main.star: mark darwin-amd64 host as slower1w
We're aware of an issue where the current darwin-amd64 hosts are slower
than we expect, primarily in the context of the darwin-amd64-longtest
builders. Various cmd/go tests often end up failing with …dmitshur commented on golang.org/x/build/cmd/coordinator: stop using internal module proxy1w
Thanks.dmitshur commented on golang.org/x/build/cmd/gopherbot: fix repository reference in "close luci-config issues"1w
Thanks.dmitshur commented on golang.org/x/build/main.star: re-enable misccompile builders everywhere but x/oscar1w
Thanks.dmitshur commented on golang.org/x/build/main.star: remove linux-amd64-typesalias for Go 1.23 onwards1w
Thanks.dmitshur opened a change golang.org/x/build/main.star: remove linux-amd64-typesalias for Go 1.23 onwards1w
Setting gotypesalias=1 is a no-op in Go 1.23 onwards, so constrain
the builder to exist only for 1.22 and older.dmitshur commented on golang.org/x/build/main.star: re-enable misccompile builders everywhere but x/oscar1w
(1 comment)
This change is ready for review.
To help a bit with review, PS 1 of this change was a pure revert, so you might want to look at the diff of PS 1 and PS 2 to see only x/oscar misccompile…dmitshur opened a change golang.org/x/build/main.star: re-enable misccompile builders everywhere but x/oscar1w
CL 597335 intended to disable the misccompile builder for x/oscar repo.
It did so successfully, stopping the misccompile builder from running
both in pre-submit (by default) and in post-submit in tha…dmitshur reviewed +2 on cmd/compile/internal/types: fix typo in comment1w
dmitshur reviewed +2 on cmd/compile/internal/types: fix typo in comment1w
Thanks.
Done in PS 3.
Done in PS 4.dmitshur commented on cmd/compile: emit tail call wrappers when possible1w
@khr@golang.org You +2ed this but this was previously waiting for the tree to reopen. Submit now?dmitshur opened a change golang.org/x/build/cmd/coordinator: stop using internal module proxy1w
We want to migrate from Global DNS for VMs to Zonal DNS, which is more
resilient. The only known use of Global DNS in the x/build GCP project
is the internal module proxy.
This was previously needed…dmitshur closed an issue golang.org/x/build/cmd/watchflakes: shouldn't reopen frozen issues1w
I agree, the auto-archival of closed issues without recent activity likely removes the main source of this happening. Let's not do anything for now unless we find out this is still a problem somehow.…dmitshur merged a change golang.org/x/build/cloudfns/{sendwikidiff,wikiwebhook}: delete1w
dmitshur commented on golang.org/x/build/cloudfns/{sendwikidiff,wikiwebhook}: delete1w
Thanks.dmitshur commented on golang.org/x/build/cloudfns/wikiwebhook: wiki updates reporter does not report deleted pages1w
Fortunately Gerrit supports setting up [notifications](https://go-review.googlesource.com/settings/#Notifications) for the wiki repo for all changes, including those that involve deleting pages:
!…dmitshur commented on x/build/cloudfns/{sendwikidiff,wikiwebhook}: remove1w
For posterity and in case we need to restore this or setup something similar in the future:
- deleted a [webhook](https://github.com/golang/go/settings/hooks) sending "Wiki" events only to `https:…dmitshur opened a change golang.org/x/build/cloudfns/{sendwikidiff,wikiwebhook}: delete1w
The need for this setup has been made obsolete via wiki move to Gerrit,
proposal golang/go#61940.
Fixes golang/go#64730.
Fixes golang/go#55263.
Fixes golang/go#33187 (obsolete).dmitshur reviewed +2 on golang.org/x/website/_content: add anchor links to article headers1w
Thanks.
It would be nice to finish CL 342610, but perhaps it's a bit unrealistic to forbid HTML from being used at all.
FWIW, since Markdown does get rendered to HTML in [golang.org/x/website/inter…dmitshur commented on runtime/cgocall: severe performance drop for cgo calls in go1.22.51w
CC @golang/runtime.dmitshur opened a change golang.org/x/build/cmd/gopherbot: fix repository reference in "close luci-config issues"1w
I anticipated that the commit link would need to be absolute, since
commits to the luci-config branch get mirrored to a different GitHub
repo than where the issue is being closed, but accidentally po…dmitshur commented on golang.org/x/build/main.star: unpin golangbuild version1w
Thanks.