Activity
Yesterday
dmitshur commented on github.com/andygrunwald/go-gerrit: Projects.SetConfig endpoint fails with 400 Bad Request due to ConfigInput.MaxObjectSizeLimit zero value not being omitted19h
@andygrunwald It should be possible to infer the type from Gerrit's code. If [ProjectInput.java](https://cs.opensource.google/gerrit/gerrit/gerrit/+/master:java/com/google/gerrit/extensions/api/proje…dmitshur reviewed +2 on golang.org/x/build/gerrit: add Gerrit-related release tasks20h
(10 comments)
The #put-edit-file docs explicitly document 409 status code when "the change edit is a no-op".
It doesn't seem this applies to #delete-edit-file, at least based on its doc…dmitshur commented on golang.org/x/build/gerrit: add Gerrit-related release tasks22h
(6 comments)
Is a new field needed, given there's already a *http.Response?
I expect e.Res.Request.URL (https://pkg.go.dev/net/http#Response.Request) can be used in HTTPError.Error instead.
Si…dmitshur reviewed +2 on golang.org/x/build/kubernetes/gke: skip tests unless specifically requested22h
Thanks!
Updates golang/go#28543.
(I know it's somewhat orthogonal, but still nice to see in the timeline of that issue.)
If you move the flag to be right under this comment, I think that'l…dmitshur reviewed +2 on golang.org/x/build/internal/task: add milestone tasks23h
(5 comments)
I suspected this was an intentional desire to show up in a future scan for "does anyone use goto in non-generated Go code". 🙈
It seems the throwaway nit stole the highlight …dmitshur commented on golang.org/x/build/internal/task: add milestone tasks1d
(1 comment)
FWIW I was gonna delete the suggestion to use 'Fetch' since it doesn't fit well, but 'Find' or 'Lookup' might be better. Get's fine too.dmitshur reviewed +2 on golang.org/x/build/internal/task: add milestone tasks1d
+2, I think this can be submitted after unresolved threads are resolved.
Since there may be changes that you'd want me to take another look before submitting, please feel feel to indicate so and…dmitshur reviewed +2 on golang.org/x/build/cmd/fetchlogs: fix a typo'd comparison from CL 4089371d
dmitshur commented on golang.org/x/build/cmd/fetchlogs: refresh maintner data if a GoRevision appears to be missing1d
(1 comment)
!=
(Sorry, spotted after submission.)dmitshur reviewed +2 on golang.org/x/build/cmd/fetchlogs: refresh maintner data if a GoRevision appears to be missing1d
(1 comment)
I guess when we're using log.Fatal, we don't care the mutex happens not get unlocked. :)dmitshur reviewed +1 on golang.org/x/build/cmd/greplogs: use a built-in list of broken builders instead of rebuilding it1d
(A quick +1 look with a trivial comment, leaving +2ing to the selected reviewer.)
This part looks obsolete and can probably be removed (or updated, if you prefer).dmitshur commented on golang.org/x/build/buildlet: fix Exec to return ErrTimeout on timeout1d
(1 comment)
Ah, yeah, it only needs to keep the connection open continuously, writing data continuously is optional. This way is shorter code and should be closer to what happens when remote command…dmitshur commented on golang.org/x/build/buildlet: fix Exec to return ErrTimeout on timeout1d
(1 comment)
I believe it's still necessary to prevent a busy loop (the /exec handler needs to be writing data continuously). If you can suggest a simpler way to achieve that, it can be used.dmitshur reviewed +1 on golang.org/x/pkgsite/internal/godoc/dochtml/internal/render: remove redundant accounting code1d
This looks good to me (+1) but I think a x/pkgsite owner can review this further or comment if this shouldn't happen (e.g., because of future changes planned to this package). Jonathan, maybe you…dmitshur commented on golang.org/x/build/buildlet: fix Exec to return ErrTimeout on timeout1d
(1 comment)
This was initially an "external" test: it just tested what happens after arbitrary timeouts in hope of catching a problem (without relying on implementation details of Exec too m…dmitshur commented on golang.org/x/build/cmd/gopherbot: label proxy.golang.org issues1d
(1 comment)
When viewing https://proxy.golang.org in a browser, under "Status: Launched" heading it is mentioned:
> These services are ready for production use. Please file issues if you…dmitshur commented on github.com/andygrunwald/go-gerrit: Aboud ConfigInput field MaxObjectSizeLimit problem1d
Thanks for reporting.
Since the field is optional, `MaxObjectSizeLimitInfo` is definitely not right as it'll always be included despite the `omitempty` tag, while `*MaxObjectSizeLimitInfo` would a…This Week
dmitshur merged a change doc/go1.19: fix strconv.Quote pkg link1d
dmitshur reviewed +2 on Revert "reflect: deprecate (Slice|String)Header"2d
dmitshur reviewed +2 on golang.org/x/build/dashboard: allow multiple known-issues for builders2d
Sounds good, thanks. Having the flexibility to select multiple known issues will allow people to pick the path that works better.
This might add an unnecessary empty line to the end of the message (…dmitshur commented on golang.org/x/build/dashboard: allow multiple known-issues for builders2d
Have you already considered but dismissed the idea of making the single known issue an umbrella issue, and link to childred builder issues from there? I'm asking because there's a bit more ov…dmitshur commented on golang.org/x/net/go.mod: update golang.org/x/sys to latest version2d
Sure. I see from logs there's now a GOPROXY set in the environment, and the 'i/o timeout' builder problem while downloading modules is resolved. Thank you!dmitshur commented on golang.org/x/net/go.mod: update golang.org/x/sys to latest version2d
CL 408434 had the effect that GOPROXY is no longer being set by coordinator. The latest slowbot failure shows it's still the value being used, perhaps because another value isn't provided in …dmitshur commented on golang.org/x/net/go.mod: update golang.org/x/sys to latest version2d
Thanks for working on it. Trying again with that change deployed.dmitshur commented on golang.org/x/build/cmd/releasebot: create version CL via Gerrit API instead of filesystem+git+codereview2d
I explored this possibility in September 2021 and wrote a short internal doc to report my findings. The summary was that this is feasible to do, but I found it was not worth doing in cmd/releasebot, …dmitshur commented on golang.org/x/build/gerrit: add Gerrit-related release tasks2d
(4 comments)
Design looks fine to me, not seeing concerns.
for creating a tag.
Also s/TagInfo/TagInput/ at the start of the sentence.
Illustrative value needs updating.
Note that setting these v…dmitshur commented on reflect: undeprecate StringHeader, SliceHeader3d
CC @ianlancetaylor.dmitshur reviewed +2 on doc/go1.19: fix strconv.Quote pkg link3d
Thanks.
Optionally, mention the tracking issue here:
```
For #51400.
```dmitshur commented on golang.org/x/build/internal/task: add milestone tasks3d
Initial pass, leaving two high level comments and suggestions. Depending on your reply, will adjust second pass review accordingly.
First, I think there's value in making code in the task packa…dmitshur reviewed +2 on doc/go1.19: fix reflect pkg link and "a" section3d
Thank you.dmitshur reviewed +2 on golang.org/x/build/dashboard: add privateGoProxy setting for linux-loong64-3a50003d
dmitshur commented on golang.org/x/build/dashboard: add privateGoProxy setting for linux-loong64-3a50003d
(1 comment)
s/goprox/proxy/dmitshur reviewed +2 on golang.org/x/build/dashboard: add privateGoProxy setting for linux-loong64-3a50003d
Thanks.
Consider adding here:
For golang/go#46229.
Since this change helps make progress on that issue.dmitshur commented on doc/go1.19: fix reflect pkg link and "a" section3d
(2 comments)
If you'd like, remove "reflect." here as motivated in https://go-review.googlesource.com/c/go/+/401434/comments/1f7db341_ee8d4e06.
For consistency, please keep the trailing…dmitshur reviewed +2 on golang.org/x/build/dashboard: update ExpectNum for linux-loong64-3a50003d
(1 comment)
Sure. If you'd like to more people to be listed as the builder owners (so we can reach them as needed), that is welcome and you can always take care of that in a follow up change. Th…dmitshur reviewed +1 on golang.org/x/build/cmd/relnote: change base URL to go.dev3d
(1 comment)
If you care, this is an instance of the longer host in a comment.dmitshur commented on doc/go1.19: update Go 1.19 relese note TODOs using relnote3d
(Moving myself to CC since this has been reviewed.)dmitshur reviewed +1 on golang.org/x/build/cmd/gomote: implements GRPC puttar command4d
(+1 review since this already has +2.)
Style note: when a function call that returns an error is inside an if statement like here and there's already an 'err' in parent scope, it can eit…dmitshur opened a change golang.org/x/build/devapp: propagate updateReviewsData error from ForeachProjectUnsorted4d
CL 407394 added a detailed error in case of a rare, unexpected problem.
Propagate it in one more place so we benefit from the new information
if this happens again.
Updates golang/go#47695.
Updates …dmitshur reviewed +2 on golang.org/x/build/dashboard: update ExpectNum for linux-loong64-3a50004d
Thanks.
If tests fail because gh panics, it means an addPerson entry with "@chenguoqi-cc" hasn't yet been added to the x/build/internal/gophers package.dmitshur reviewed +2 on golang.org/x/build/maintner: return an explicit error from (*GerritProject).GitCommit4d
(1 comment)
I also noticed this is the first instance of a non-nil error being returned in the p.ForeachOpenCL iterator. It should either be logged and and return nil to keep going (to skip a bad/un…dmitshur closed an issue cmd/go: workspace problems5d
dmitshur commented on cmd/go: workspace problems5d
Modules inside subdirectories indeed need to be tagged separately, as described at https://go.dev/ref/mod#module-path and https://go.dev/ref/mod#vcs-version. Since I understand there's no more issue …dmitshur commented on github.com/go-gl/gl/bad access: nil dereference GenBuffers()6d
Does the [`gl41core-cube`](https://github.com/go-gl/example/tree/master/gl41core-cube) example program run okay for you?
If so, it has [similar code](https://github.com/go-gl/example/blob/d71b0d9f82…Last Week
dmitshur reopened an issue cmd/cgo: with zig as CC/CXX, Go linker does not put libc onto the linker line, causing undefined symbol errors1w
dmitshur commented on runtime: "fatal: systemstack called from unexpected goroutine" on android/amd641w
I've mailed [CL 407615](https://go.dev/cl/407615) that makes android-amd64-emu a post-submit builder only (in the main repo) while investigation of this issue is underway. If submitted, this issue ca…dmitshur commented on cmd/compile: improve fuzzer [freeze exception]1w
Thanks for letting us know about this. I believe we're in agreement that this is okay. Approved.dmitshur pushed to
master in github.com/shurcooL/githubv41w
dmitshur pushed to
master in github.com/shurcooL/graphql1wdmitshur commented on runtime: TestGcSys is still flaky1w
@mknyszek The test `TestGcSys` is still [checked in](https://cs.opensource.google/go/go/+/master:src/runtime/gc_test.go;l=23-33;drc=f01c20bf2ba889e5c9e3565175cc4276f9c11516) on tip, but with a skip t…dmitshur commented on misc/cgo/testsanitizers: occasionally hangs indefinitely1w
Thanks Ian.
For the record, my upcoming coordinator change was going to include a 5 minute timeout on the x/build side via [`DistTestsExecTimeout`](https://pkg.go.dev/golang.org/x/build/dashboard#…dmitshur commented on golang.org/x/build/buildlet: add context support1w
I suspect [CL 207908](https://go.dev/cl/207908) and [CL 208157](https://golang.org/cl/208157) made a lot of progress on this, and we're in a state where most buildlet methods that should have a conte…dmitshur commented on golang.org/x/build/cmd/coordinator: transparent TryBot retries mask stalls that should be fixed1w
I ran into this most recently while sending a trivial fix in https://go.dev/cl/406897, where a linux-amd64-longtest SlowBot request took almost 2 hrs instead of the usual 10-15 min, and while waiting…dmitshur closed an issue golang.org/x/build/cmd/coordinator: some linux-amd64 trybot builds getting stuck at the end1w
dmitshur commented on golang.org/x/build/cmd/coordinator: some linux-amd64 trybot builds getting stuck at the end1w
I've observed a few more instances of this with `testsanitizers`. I think the root cause here is the same as issue #42699: some test is stalling, and eventually gets retried and succeeds.
I've fil…dmitshur opened an issue misc/cgo/testsanitizers: occasionally hangs indefinitely1w
(This is mostly a continuation of #33249, with some new information.)
There's evidence of a problem where this test can sometimes hang indefinitely, though in some environments there is an externa…dmitshur closed an issue golang.org/x/build/cmd/coordinator: missing timeouts during build (make.bash phase? overall?)1w
dmitshur commented on golang.org/x/build/cmd/coordinator: missing timeouts during build (make.bash phase? overall?)1w
We generally have a global timeout for VM/pod-based builders (see #52929), though not sure about if reverse builders have something equivalent or not. I'll close this in favor of #35364, a more recen…dmitshur commented on net: deadlock in TestDialParallelSpuriousConnection on darwin-arm*1w
Running the test 100'000 times on a darwin/amd64 machine did not reproduce the problem.
Running the same on a darwin/arm64 machine seems to reproduce after a while:
```
$ go test -count=100000…dmitshur commented on golang.org/x/build/cmd/gopherbot: add Gerrit mock/integration tests1w
`git-codereview` implements a fake Gerrit server (type `gerritServer`) and a git helper (type `gitTest`) for its tests, which might be relevant here (spotted via #50576).dmitshur commented on golang.org/x/build/cmd/gopherbot: add Gerrit mock/integration tests1w
Since it's not already mentioned, I'll add that one of the few ways to test `gopherbot` changes now is via its `-dry-run` mode. This helps, but it's not a complete and ideal solution.
If we mock i…dmitshur commented on net: TestDialCancel is not compatible with new macOS ARM64 builders1w
My understanding of the state of this issue is that it can in Backlog milestone without a release-blocker label. That is, the known-failing test is skipped (via [CL 402181](https://go.dev/cl/402181))…dmitshur commented on all: update standard-library dependencies at the start and end of each development cycle1w
[CL 406355](https://go.dev/cl/406355) and [CL 406356](https://go.dev/cl/406356) updated golang.org/x dependencies.
[CL 406359](https://go.dev/cl/406359) updated github.com/google/pprof.
(Other indi…dmitshur commented on github.com/google/pprof/...: update dependencies1w
Thanks!dmitshur deleted branch in github.com/dmitshur/pprof1w
update-depdmitshur pushed to
update-dep in github.com/dmitshur/pprof1w
dmitshur opened a change github.com/google/pprof/...: update dependencies1w
There are newer versions of dependencies available. Start using them.
I reviewed the change locally by comparing the diff of `go mod vendor` before and after. There were expected changes in x/sys/…dmitshur pushed to
update-dep in github.com/dmitshur/pprof1wdmitshur created branch in github.com/dmitshur/pprof1w
update-depdmitshur deleted branch in github.com/dmitshur/pprof1w
update-depdmitshur pushed to
update-dep in github.com/dmitshur/pprof1wdmitshur created branch in github.com/dmitshur/pprof1w
update-depdmitshur forked in github.com/google/pprof1w
dmitshur commented on golang.org/x/build: windows-amd64-longtest builder not keeping up with commits1w
[CL 406216](https://go.dev/cl/406216) increased the timeout to 2 hours, and longtest builds are completing on first try or so. Compare [before](https://user-images.githubusercontent.com/1924134/16840…dmitshur opened an issue golang.org/x/build/cmd/coordinator: decide what to do about a global build time limit1w
The comment of [`CleanUpOldVMs`](https://cs.opensource.google/go/x/build/+/master:internal/coordinator/pool/gce.go;l=648-661;drc=3ab5e7e87a80893ee88aba0e9de7e86cde9f5a0a) (and `CleanUpOldPodsLoop`) i…dmitshur pushed to
master in github.com/go-gl/glfw1w
dmitshur merged a change github.com/go-gl/glfw: Fix typos in documentation1w
dmitshur reviewed +2 on github.com/go-gl/glfw: Fix typos in documentation1w
LGTM. Thanks.dmitshur pushed to
master in github.com/go-gl/glfw1w
dmitshur merged a change github.com/go-gl/glfw/v3.3/glfw: minor code cleanups1w
dmitshur commented on github.com/go-gl/glfw/v3.3/glfw: minor code cleanups1w
Thanks.dmitshur commented on github.com/go-gl/glfw: Fix typos in documentation1w
If you'd like, lines 174, 229, and 231 can also benefit from a similar fix.Earlier
dmitshur commented on github.com/go-gl/glfw/v3.3/glfw: port `GLFW_CONTEXT_NO_ERROR`1w
Yep, looks like it hasn't been added. Please feel free to send a PR if you have a need for it.dmitshur pushed to
master in github.com/go-gl/glfw1wdmitshur merged a change github.com/go-gl/glfw: Add a go.mod file for Go modules1w
dmitshur reviewed +2 on github.com/go-gl/glfw: Add a go.mod file for Go modules1w
I think this is okay to add. Thanks.dmitshur commented on github.com/go-gl/glfw: Fix _GNU_SOURCE defined twice warning1w
Yeah, if we stop defining _GNU_SOURCE and that causes a warning in GLFW C codebase due to _GNU_SOURCE not being defined, that might be something that is better suited to be addressed upstream.
Are…dmitshur commented on github.com/go-gl/glfw/v3.3/glfw: update to GLFW 3.3.71w
Thanks!dmitshur pushed to
master in github.com/go-gl/glfw1wdmitshur merged a change github.com/go-gl/glfw/v3.3/glfw: update to GLFW 3.3.71w
dmitshur commented on wasm_exec.js: out-of-sync with runtime.walltime changes2w
To clarify, are you running into this when using a .wasm module built with Go 1.18 together with the wasm_exec.js file from Go 1.18? Are you able to reproduce it with a simple hello-world program, or…dmitshur closed an issue golang.org/x/build/cmd/gomote: show periodic buildlet acquisition status updates to stderr2w
dmitshur commented on golang.org/x/build/cmd/gomote: show periodic buildlet acquisition status updates to stderr2w
There are definitely regular updates being printed now which provide pretty good information, so I think this issue is less actionable now. We can consider this to be resolved by [CL 207841](https://…dmitshur commented on cmd/compile: 1.18 produces different type descriptor for identical struct in different packages [1.18 backport]2w
In issue #52856 it was determined that the current 1.18 behavior is correct, so no fix needs to be backported to 1.18.dmitshur commented on github.com/go-gl/glfw/v3.3/glfw: update to GLFW 3.3.72w
To me, the warning that `_GNU_SOURCE` is defined twice seems more harmless. If you're not able to find a solution for `ppoll` problem easily, I'd suggest let's revert 692d67bc69fe52c59d22b0fcad6adff6…dmitshur commented on reflect: different behavior in versions 1.17.10 and 1.18.12w
A possibly minified repro (with `struct { int }`): https://go.dev/play/p/F_PIyGVxd0E.dmitshur commented on build: announce end of support for old Windows releases2w
Adding a data point observed in Git for Windows release notes [upcoming breaking changes](https://github.com/git-for-windows/build-extra/blob/76d6b0286c275eb445af2f4ab2fae41fe302341c/ReleaseNotes.md#…dmitshur commented on syscall: Faccessat checks wrong group2w
Fixed for Go 1.19 in [CL 399539](https://go.dev/go/399539). (This didn't get closed because its commit message had "For" rather than "Fixes".)dmitshur commented on golang.org/x/website: `Internal Server Error` from `go-get` requests to golang.org2w
There doesn't appear to be anything abnormal for yesterday at https://console.cloud.google.com/appengine?project=golang-org nor https://console.cloud.google.com/monitoring/uptime?project=golang-org (…