Activity
Today
dmitshur commented on runtime: severe performance drop for cgo calls in go1.22.5 [1.23 backport]6h
We moved the NextMinor label to the parent tracking issue #68587, removing here.Yesterday
dmitshur reviewed +2 on golang.org/x/build/main.star: adjust LUCI load at openbsd-ppc64 builder19h
dmitshur reviewed +2 on golang.org/x/build/x/build: adjust LUCI load at openbsd-ppc64 builder20h
Thanks. Just minor comments for the commit message.
The repo name is only needed for the issue tracker. Please replace this prefix to be 'main.star: adjust ...'.
We probably want to keep the issue …dmitshur commented on golang.org/x/build/README: mention the git repo1d
(1 comment)
FYI it wasn't applied. (It makes no significant difference in this particular case, that's why I made this suggestion optional, but letting you know in case it helps you avoid this happe…dmitshur reviewed +2 on golang.org/x/build/README: mention the git repo1d
Thanks.
Optionally, we could go further and use the same wording here as in all other repos:
```suggestion
This repository uses Gerrit for code changes. To learn how to submit changes to
this repos…dmitshur commented on all: update golang.org/x/sys [generated]1d
runtime/race.TestRace failure is pre-existing, #70164, bypassing.dmitshur commented on golang.org/x/website/internal/history: document Go 1.23.3 and 1.22.9 releases1d
Minor comment, but I'll let Carlos review. Thanks.
```suggestion
Components: []template.HTML{"the linker", "cgo", "the runtime"},
```dmitshur commented on golang.org/x/build: gotip-windows-arm64 builders stops working occasionally1d
Again, both windows-arm64 builders have gone offline as of around Nov 2-5, with first OOMs on Oct 29 ([here](https://github.com/golang/go/issues/69664#issuecomment-2445132269)):
<img width="147" a…dmitshur commented on all: update golang.org/x/sys [generated]1d
Thanks. I've updated this CL to pull in the tip of x/sys now, which includes the revert.dmitshur reviewed +2 on golang.org/x/build/main.star: replace -swissmap builder with -noswissmap1d
Thanks.dmitshur commented on golang.org/x/build/internal/task: move NewTaskRequest.Properties into TaskSlices field1d
Thanks.dmitshur commented on syscall: SyscallN always escapes the variadic argument1d
@qmuntal I'm a bit confused by the specifics in your question (maybe you got the wrong CL number mentioned? that one pulls in many x/sys CLs all at once), but in general, if there is a serious issue …This Week
dmitshur commented on time: TestLoadFixed failures2d
[CL 625197](https://go.dev/cl/625197) is an implementation of https://github.com/golang/go/issues/69840#issuecomment-2448571828.dmitshur opened a change time: accept "+01" in TestLoadFixed on OpenBSD2d
This stops the test from failing with a known failure mode, and
creates time to look into what the next steps should be, if any.
For #69840.dmitshur commented on golang.org/x/build/internal/relui: serve 404 when workflow doesn't exist, fix DB tests2d
Thanks.dmitshur commented on golang.org/x/build/...: regenerate .pb.go files, unify generation process2d
There indeed turned out to be a small¹ dragon that lurked for some years, but I wouldn't have caught it ahead of trying this out. :)
¹ It's quite literally one byte long, probably couldn't be any …dmitshur opened a change golang.org/x/build/cmd/coordinator: embed UnimplementedCoordinatorServer by value2d
This type is expected to be embedded by value in order to have the
intended effect. Caught by a newer version of generated code which
panics at startup when an unimplemented server type is embedded a…dmitshur reviewed +2 on image/jpeg: initialize dct_test constants at compile time2d
I see, thanks. I suggest adding a brief sentence noting this in the commit message so it's easier to find (e.g., something like "Having these more accurate values isn't necessary for tests to pass, b…dmitshur reviewed +2 on golang.org/x/build/luci-config: remove known issue for netbsd-arm2d
Thanks, and congratulations on reaching this point!dmitshur commented on github.com/go-gl/example: update dependencies and embed texture file2d
Yes, embed is available starting with 1.16. Updating to 1.22.0 is a part of "modernizing" the example.
There are several reasons not to try to stick with Go 1.16. By now that's an old, unsupport…dmitshur reviewed +2 on golang.org/x/build/cmd/buildlet: set a safe default for shell2d
Thanks.
Let's also bump the version so we can tell this apart from the previous one.
```suggestion
// 29: fall back to /bin/sh when SHELL is unset
const buildletVersion = 29
```dmitshur reviewed +2 on golang.org/x/build/dashboard: set netbsd-arm{,64} builders to 0 expected2d
It's fine not to update `ExpectNum` (the corresponding entries in the [migration.BuildersPortedToLUCI](https://pkg.go.dev/golang.org/x/build/internal/migration#BuildersPortedToLUCI) map imply this), …dmitshur reviewed +2 on golang.org/x/build/internal/migration: mark netbsd-arm-bsiegert as migrated to LUCI2d
Yes. build.golang.org don't show LUCI builder results for builders that have a non-zero known issue set. So the known issue #63698 on [this line](https://cs.opensource.google/go/x/build/+/luci-config…dmitshur pushed to
embed-texture in github.com/go-gl/example3d
dmitshur opened a change github.com/go-gl/example: update dependencies and embed texture file3d
Modernize examples slightly.dmitshur created branch in github.com/go-gl/example3d
embed-texturedmitshur created tag in github.com/go-gl/mathgl3d
v1.2.0dmitshur commented on github.com/go-gl/example: `go run` fails3d
Thanks for reporting. The example program should use go:embed and embed the assets into the binary to fix this.dmitshur reviewed +2 on image/jpeg: initialize dct_test constants at compile time3d
Thanks. I left a minor comment inline, and I have a high level question below to better understand the motivation for this change.
This change improves these constants in a _test.go file, so the mos…dmitshur reviewed +2 on image/jpeg: add more theHuffmanSpec comments3d
Thanks.Last Week
dmitshur closed a change golang.org/x/vgo/go/internal/modconv: conversion preserve replacements from vendor.json4d
This CL was already closed in 2019, and I'm not sure why this CL was reopened in 2022. Closing again since as noted at https://go.googlesource.com/vgo#obsolete, this repository is obsolete. Thanks.dmitshur commented on golang.org/x/vgo/README: explain that repository is obsolete4d
This repository doesn't have builders [defined](https://cs.opensource.google/go/x/build/+/luci-config:main.star;l=715-747;drc=35b81aad68643121979874862a0c069da432142b), so Commit-Queue+1 won't do any…dmitshur commented on golang.org/x/build: can't ssh to x_tools-gotip-netbsd-arm64 gomote4d
Thanks for reporting. Also CC @golang/netbsd.
[ssh.go#L52](https://cs.opensource.google/go/x/build/+/master:cmd/buildlet/ssh.go;l=52;drc=d74f3705ac9431ccc427d35d624edb251801cbc5) calls `shell()` t…dmitshur reviewed +2 on cmd/dist: update UnixOS comments4d
Thanks.dmitshur reviewed +2 on golang.org/x/sys/README: don't recommend go get4d
Thanks.dmitshur reviewed +2 on golang.org/x/term/README: don't recommend go get4d
Thanks.dmitshur reviewed +2 on golang.org/x/time/README: don't recommend go get4d
Thanks.dmitshur reviewed +2 on golang.org/x/text/README: don't recommend go get4d
Thanks.dmitshur reviewed +2 on golang.org/x/vgo/README: explain that repository is obsolete4d
Thanks.dmitshur reviewed +2 on golang.org/x/review/README: don't mention GOPATH4d
Thanks.dmitshur commented on crypto/md5: improve ARM64 MD5 performance by optimizing ROUND3 function5d
I'm not the right reviewer for changes to crypto/md5.dmitshur opened a change golang.org/x/build/internal/task: move NewTaskRequest.Properties into TaskSlices field5d
The top-level Properties field is deprecated¹ in favor of using
TaskSlices[0].Properties, so start using the new approach, as
already done in the ./internal/gomote package. The same applies
to the E…dmitshur commented on all: update golang.org/x/sys to pull in CL 614082 [generated]5d
Changes in x/sys cause more allocations on Windows, as reported by failing tests. For those more familiar with x/sys and Windows - is that expected, or a problem that needs to be fixed in x/sys? Than…dmitshur commented on golang.org/x/build: add MSAN and ASAN builders5d
Thanks for adding the builders and fixing tests at tip. The builders are currently failing on release branches 1.23 and 1.22: (CC @cagedmantis)
https://ci.chromium.org/p/golang/g/go-go1.23/console…dmitshur commented on syscall/mksyscall_windows.go: use @master when running golang.org/x/sys/windows/mkwinsyscall command5d
(1 comment)
I tried the same sequence of steps and I see Syscall6 → SyscallN changes in `internal/syscall/windows/zsyscall_windows.go`. See CL 623821 that's entirely generated by [git-generate](ht…dmitshur opened a change all: update golang.org/x/sys to pull in CL 614082 [generated]5d
A part of the keeping Go's vendored dependencies and generated code
up to date.
For #36905.
[git-generate]
cd src
go get golang.org/x/sys@v0.26.1-0.20241021134239-adbb8bbcaf61
go mod tidy
go mod ve…dmitshur commented on syscall/mksyscall_windows.go: use @master when running golang.org/x/sys/windows/mkwinsyscall command6d
(1 comment)
@iant@golang.org My understanding is that's the _current_ behavior (i.e., prior to this CL) - src/syscall/mksyscall_windows.go runs `golang.org/x/sys/windows/mkwinsyscall` using the vers…dmitshur commented on syscall/mksyscall_windows.go: use @master when running golang.org/x/sys/windows/mkwinsyscall command6d
(1 comment)
It's currently possible to use any commit, it just requires updating the vendored x/sys version too. That is:
```
go get golang.org/x/sys@desiredcommit
go mod tidy
go mod vendor
go gene…dmitshur commented on golang.org/x/build/internal/task: replace reviewers parameter with release coordinators6d
(1 comment)
It turned out this is the first time we ran into a workflow parameter change affecting existing scheduled workflows, and relui doesn't have good enough support for that yet (#70145). Sen…dmitshur opened a change golang.org/x/build/cmd/relui: add a "failed to schedule workflows" section6d
Make it possible to see any scheduled workflows that couldn't be
resumed due to their parameters changing, including their original
parameter values, and the original cron schedule (if any).
This al…dmitshur opened an issue golang.org/x/build/cmd/relui: shows a misleading prompt that says "immediately run" when scheduling a workflow (i.e., it doesn't run immediately)6d
When creating a workflow to be executed, relui always shows a prompt that asks:
"This will create and immediately run this workflow. Ready to proceed?"
The "immediately run" part is true whene…dmitshur commented on golang.org/x/build/cmd/relui: no support to re-create scheduled workflows after input parameters change6d
I'll send a change that makes it possible to re-create the workflows and clean up the old ones without needing database access. It's pretty minimal, but should be fine since we don't need to do this …dmitshur opened an issue golang.org/x/build/cmd/relui: no support to re-create scheduled workflows after input parameters change6d
Relui is designed to handle workflows that are added or modified over time. One area where it doesn't handle this well are scheduled workflows. If one of their input parameters is changed, they'll fa…dmitshur opened a change golang.org/x/build/main.star: add netbsd-arm to SLOW_HOSTS6d
As mentioned in https://go.dev/issue/63698#issuecomment-2450448780,
this might be a good starting point. Can be adjusted later as needed.
For golang/go#63698.dmitshur reviewed +2 on golang.org/x/build/cmd/watchflakes: do not exit in the case of a mismatch6d
Thanks. (Bypass since PS 2 differs from PS 1 only in comment space, and PS 1 passed.)
Yeah, it's a balance of a higher-visibility hard stop when encountering new unexpected cases vs. low-visibility …dmitshur commented on golang.org/x/build: add LUCI netbsd-arm builder6d
https://ci.chromium.org/b/8732718088961275521 was the first successful build - congrats on reaching this milestone!
The build took over 2 hrs to complete. Given netbsd-arm64 takes around 25 min an…dmitshur reviewed +2 on golang.org/x/build/internal/gophers: remove mdempsky's google address as primary6d
Thanks.dmitshur commented on all: announce end of support for old Windows releases1w
There are no changes here for the Go 1.24 milestone. Moving to the next.dmitshur commented on all: announce end of support for old Linux versions1w
[CL 622015](https://go.dev/cl/622015) added an announcement to [Go 1.24 release notes](https://tip.golang.org/doc/go1.24#linux). CC @cagedmantis. Moving to the next milestone, unless there's more I m…dmitshur commented on golang.org/x/build/cmd/watchflakes: currently in a CrashLoopBackOff state1w
The input of the LUCI [build 8733044805650708721](https://ci.chromium.org/b/8733044805650708721) is `https://go.googlesource.com/tools/+/889abb17e125bb0f5d8de61bb80ef15fbe2a130d`, which is indeed a r…dmitshur commented on syscall/mksyscall_windows.go: use @master when running golang.org/x/sys/windows/mkwinsyscall command1w
(1 comment)
What's the motivation to make this change? Previously, it would produce stable results when running multiple times. Using a moving target means running it again may produce different res…dmitshur commented on golang.org/x/build/main.star: use v7l suffix for netbsd-arm builder1w
Thanks.dmitshur opened a change golang.org/x/build/main.star: use v7l suffix for netbsd-arm builder1w
It was updated to target GOARM=7 because it can't work with GOARM=6.
For golang/go#63698.Earlier
dmitshur closed an issue net/http: short writes with FileServer on macos [1.22 backport]1w
dmitshur reviewed +2 on golang.org/x/build/main.star: add linux-amd64-asan-clang15 and linux-amd64-msan-clang151w
Thanks.dmitshur opened a change golang.org/x/sys/unix: gofmt after CL 6102961w
Also remove a few unhelpful blank lines.dmitshur commented on golang.org/x/sys/unix: update z/OS implementation of fcntl and mmap1w
TryBots passed on PS 2, and the only thing that changed since then is the base commit and commit message, so bypassing.dmitshur commented on golang.org/x/build/cmd/upload: delete unused buckets from bucketProject map1w
Thanks.dmitshur commented on golang.org/x/website/_content/blog/slices-intro: change to English in array1w
I agree with Ian's comment. For any natural language we pick, there will be some that don't know it. This is a blog post from 2011, let's leave it as is.dmitshur opened a change golang.org/x/build/cmd/upload: delete unused buckets from bucketProject map1w
The upload command doesn't need to track these anymore since they're
no longer used.dmitshur commented on golang.org/x/build/internal/task: read nested module's own go.mod files1w
Thanks.
Sounds good. In that case there's less need to save ReadFile's output in `goMod`. Done.dmitshur commented on golang.org/x/build/internal/gitfs: add a generated copy of x/website/internal/gitfs1w
Thanks.dmitshur commented on golang.org/x/build/internal/task: dynamically find nested modules to tidy during tagging1w
(2 comments)
No worries and thanks for the heads up. I did also try locally running TagXReposTasks.UpdateGoMod pointed to the real Gerrit server.
Done.dmitshur reviewed +2 on golang.org/x/build/main.star: upgrade linux-arm64 builders to Debian 12 (bookworm)1w
(1 comment)
Great. Thanks!dmitshur reviewed +2 on golang.org/x/build/env/freebsd-amd64: exit daemon when bootstrapswarm has a zero exit code1w
Thanks.
I hadn't noticed until now that the recent freebsd/amd64 LUCI builder conversion was different from the previous openbsd/amd64 one (bootstrapswarm is setup within `env/openbsd-amd64/make.bas…dmitshur reviewed +2 on golang.org/x/build/env/linux-arm64-bookworm: add image1w
Thanks.
(minor) This syntax (without a '=') is discouraged and might cause increasing friction as mentioned in CL 621956, so consider migrating to the current syntax while you're here:
```suggestio…dmitshur reviewed +2 on go.dev/wiki/AI: Added section for building LLM powered applications.2w
Thanks.
(nit) There are some suggestions for commit message style at https://go.dev/wiki/CommitMessage that can be applied here:
```suggestion
AI: add section for building LLM-powered applications
…dmitshur reviewed +2 on doc/next: document the minimum Linux kernel version for 1.242w
(1 comment)
https://go.dev/issue/67001#issuecomment-2389063345 noted that the release blocker was tracking adding a note to the release notes. Is this 'Updates', not 'Fixes', because there's more to…dmitshur reviewed +2 on doc/next: document the minimum Linux kernel version for 1.242w
Thanks.
Not sure it makes a big difference, but you could prefix this with a link to the pre-announcement if desired - something like:
```
As [announced](go1.23#linux) in the Go 1.23 release notes,…dmitshur commented on golang.org/x/website: go.dev/doc/install links to the darwin/amd64 installer for Apple Silicon users2w
As the title of this issue says, when it was originally filed, the "Download" button at https://go.dev/doc/install was quite misleading. [CL 480955](https://go.dev/cl/480955) made it point to the htt…dmitshur reviewed +2 on runtime/debug: minor cleanups after CL 3841542w
Thanks.dmitshur reviewed +2 on runtime/debug: document ParseBuildInfo and (*BuildInfo).String2w
Thanks.
This documentation is circular ("Parse parses what String outputs, and String outputs what Parse parses"), but it seems that's as much as can be said without committing to document this form…dmitshur reviewed +2 on golang.org/x/build/env/openbsd-amd64: update build image script to OpenBSD 7.62w
Thanks.dmitshur opened an issue runtime/debug: ParseBuildInfo is undocumented2w
[`debug.ParseBuildInfo`](https://pkg.go.dev/runtime/debug#ParseBuildInfo) is has no documentation. (https://go.dev/s/style#doc-comments and https://go.dev/doc/comment say that every exported identifi…dmitshur commented on golang.org/x/text/internal/texttest: remove Run and Bench helpers2w
Thanks.dmitshur commented on golang.org/x/text/...: normalize subtest names to NFC2w
Thanks.
Test names need to be unique to serve their role well, and it's unfortunate the testing package doesn't currently help catch instances of multiple different tests that have the same name aft…dmitshur opened a change golang.org/x/text/internal/texttest: remove Run and Bench helpers [generated]2w
Versions of Go whose testing package doesn't yet support subtests and
sub-benchmarks are no longer supported. Drop an intermediate layer of
compatibility from the ./internal/texttest package.
[git-g…dmitshur opened a change golang.org/x/text/cases: normalize TestWordBreaks subtest names to NFC2w
LUCI builders upload test results to ResultDB, which permits printable
Unicode characters in test names, provided they're written in NFC form.dmitshur commented on github.com/andygrunwald/go-gerrit/Change coming ahead: Renaming `master` branch to `main` 💥2w
Sounds great, I’m in agreement. This change will be transparent for users of the Go package, so only those who have a pre-existing git checkout for development purposes will need to migrate. FWIW, …dmitshur commented on internal/runtime/atomic: add Xchg8 for arm2w
freebsd-amd64-12_3 passed, and linux-arm-aws can't run because it doesn't exist.
Requesting a legacy linux-arm SlowBot won't work, that builder has been deleted in favor of the [new LUCI one](https:…dmitshur commented on access: loong64 team for @sophie-zhao 2w
Thanks. An invite has been sent, it will need to be accepted by @sophie-zhao.dmitshur commented on golang.org/x/website/_content/blog: don't load godocs.js (and other scripts) more than once2w
Thanks.dmitshur commented on net/http/pprof: Index doesn't include Symbol3w
CC @cherrymui.dmitshur reviewed +2 on golang.org/x/mobile/cmd/gomobile: use Output instead of CombinedOutput at envClang3w
Thanks.
[exec.Cmd.Output](https://pkg.go.dev/os/exec#Cmd.Output) documentation says:
> If c.Stderr was nil, Output populates ExitError.Stderr.
If there was something printed to stderr, it'd be wor…dmitshur fixed an issue golang.org/x/build/cmd/golangbuild: potential send on closed channel during cleanup3w
dmitshur commented on golang.org/x/build/cmd/golangbuild: potential send on closed channel during cleanup3w
Thanks @riannucci! The aforementioned CL is submitted and released by now, so closing as fixed.dmitshur commented on crypto/rand: TestAllocations fails on plan9 since CL 6024983w
CC @golang/plan9, @FiloSottile.dmitshur commented on proxy.golang.org: Intermittent TLS/Network errors with Google's Module Proxy3w
CC @golang/tools-team.dmitshur reviewed +2 on golang.org/x/build/...: execute gofmt3w
Thanks.dmitshur commented on golang.org/x/tools/...: set gotypesalias=1 when using >=1.23 toolchain3w
(1 comment)
There should be a blank line left in cases like this, since you don’t want the copyright comment to become a package comment.