Activity
Today
dmitshur commented on golang.org/x/build/maintner/maintnerd: /logs endpoint fails to serve 304 successfully during long poll, ends up serving 502 instead3h
A few test queries with the /debug304 endpoint (from [CL 415254](https://go.dev/cl/415254)) show that waiting under 30 seconds results in 304 being returned, but 30 or more seconds results in 502 aft…dmitshur commented on golang.org/x/build/maintner: don't try to download negative bytes4h
(1 comment)
This came from initial code here, but since both of us prefer using the named consts, improved (in Patch Set 3 of parent CL). Thanks.dmitshur commented on golang.org/x/build/maintner/maintnerd/gcslog: adjust time before 3044h
Thanks for reviewing. I'll test it out before submission, so adding Hold for now.dmitshur closed an issue api: audit for Go 1.194h
dmitshur commented on api: audit for Go 1.194h
[CL 414635](https://go.dev/cl/414635) is in. Thank you, closing as done.
(I've also checked off the last 3 boxes so it's clear we didn't accidentally miss them. They corresponded to the 3 more fie…dmitshur commented on syscall: NewCallback triggers data race on Windows when used from different goroutine [1.18 backport]5h
The rationale to backport and safety of fix here seems very similar to #53590, so it can possibly be handled in a similar way.dmitshur opened a change golang.org/x/build/maintner/maintnerd/gcslog: adjust time before 3045h
Practice shows 55 seconds isn't working well by now. See if adjusting
the timeout slightly is all that's needed to make the problem go away.
For golang/go#53569.dmitshur reviewed +1 on golang.org/x/tools/gopls/doc: regenerate documentation8h
Thanks for fixing it quickly.dmitshur merged a change cmd/dist: [release-branch.go1.17] test cgo internal linking on darwin-arm6410h
dmitshur merged a change cmd/dist: [release-branch.go1.17] skip internal linking tests on arm6410h
dmitshur commented on misc/cgo/test: failure with gcc 10 [1.17 backport]10h
It does appear the failure isn't consistent, for some reason, but it happened again on the latest commit on the release branch (see [here](https://build.golang.org/log/5527be461db9e9c7576175b95366340…dmitshur opened a change cmd/dist: [release-branch.go1.17] test cgo internal linking on darwin-arm6410h
CL 383554 disables testing cgo internal linking on all ARM64 but
Windows, because it doesn't work with newer GCC. But
- darwin-arm64 works, and it does not use GCC
- we don't support cgo internal lin…dmitshur opened a change cmd/dist: [release-branch.go1.17] skip internal linking tests on arm6410h
The previous workaround for issue #39466 only disabled this test for
Linux. However, the issue manifests for all arm64 systems with gcc 9.4
and above. The new netbsd-arm64 builder uses NetBSD-current…dmitshur reviewed +2 on golang.org/x/build/cmd/gomote: use dirEntry for directory list parsing11h
Thanks, nice catch.
Since ssToDirEntries isn't doing anything other than converting every entry from string to buildlet.DirEntry, if you'd like, you have the option to simplify it by inlining the co…dmitshur reviewed +2 on golang.org/x/build/dashboard: drop known issue for linux-riscv64-unmatched12h
Thanks.Yesterday
dmitshur reviewed +2 on cmd/go: [release-branch.go1.17] pass --no-decorate when listing git tags for a commit1d
dmitshur reviewed +2 on cmd/go: [release-branch.go1.18] pass --no-decorate when listing git tags for a commit1d
dmitshur reviewed +2 on cmd/go: pass --no-decorate when listing git tags for a commit1d
(1 comment)
Minor: I think this refers to ~/.gitconfig, but it's clear from the context of line 26.dmitshur commented on meta: request output of `go list -m all` in the issue template1d
This seems reasonable, @bcmills want to send a CL for this? We can try it and adjust based on experience.dmitshur commented on golang.org/x/build/cmd/coordinator: join paths using the host's path separator1d
Thanks. Please see inline comments.
(Sorry about the long delay. I was finally able to work through my recent attention set backlog and am able to get to reviewing this CL now, if it's still helpful…This Week
dmitshur commented on net: really skip Windows TestLookupPTR test if we say we are skipping it2d
(1 comment)
Maybe out of scope of this CL, but pointing out in case it's helpful: this test can benefit from a similar improvement by using t.Skipf.dmitshur commented on golang.org/x/build/cmd/relui: join communication tasks in main release workflows2d
Thanks for reviewing.
The reason a minor release kind isn't listed in this switch is because it's inside createSingle, which is only used for final, RC and betas (see lines 608-616 below).
The mino…dmitshur commented on golang.org/x/build: build failures due to `410 Gone` responses from `symbolic-datum-552` GOPROXY2d
Re 1, I think we don't have a direct fallback because, if `GO_DISABLE_OUTBOUND_NETWORK` is working (it might not be, see #51444), then any fallback direct connections would be refused by the firewall…dmitshur commented on golang.org/x/build/internal/workflow: make Slice method variadic2d
Thanks.dmitshur commented on golang.org/x/build/cmd/relui: join communication tasks in main release workflows2d
(2 comments)
Done in PS 2.
Done in PS 2.dmitshur commented on golang.org/x/build/maintner: negative byte downloads2d
The downloading code was just missing the case for where the client had more bytes than server (due to server restarting with some uncommitted transactions).dmitshur commented on golang.org/x/build/maintner: test failures with "416 Requested Range Not Satisfiable" from maintner.golang.org2d
The 502 Bad Gateway from /logs is another (non-serious) issue, filed #53569 for it.
I was going to send a small change to add a log line, to learn what range value causes the server to respond wit…dmitshur opened a change golang.org/x/build/maintner: don't try to download negative bytes2d
If the maintnerd server restarts with some uncommitted transactions,
clients may end up having more downloaded bytes than the server, after
the server restarts. We're invariably heading towards an Er…dmitshur opened an issue golang.org/x/build/maintner/maintnerd: /logs endpoint fails to serve 304 successfully during long poll, ends up serving 502 instead2d
The /logs endpoint supports long polling via `waitsizenot` parameter. To avoid being flagged as a timeout, it tries to serve a 304 in just under a minute (55 seconds specifically):
```Go
// Retur…dmitshur starred github.com/soypat/gopherlings3d
dmitshur pushed to
master in dmitri.shuralyov.com/service/change3ddmitshur starred github.com/josharian/tstruct3d
Last Week
dmitshur commented on runtime: mark string comparison hooks as no split4d
(1 comment)
Thanks, I'll mark this thread as resolved.dmitshur reviewed +1 on cmd/go: prepend builtin prolog when checking for preamble errors4d
dmitshur commented on golang.org/x/build/maintner/godata: getGoData helper should handle failure without failing test, otherwise that causes occasional false positive test failures due to HTTP/2 internal errors while syncing segments4d
I initially wanted to update this issue with the latest status, and then saw that it'd be quick to send a trivial CL to close it. All that was needed was a trivial change to the following flaky-test-…dmitshur opened a change golang.org/x/build/maintner: retry network operations that may fail in getNewSegments4d
On the internet, servers generally don't go out of their way to ensure
graceful stream shutdowns, preferring to instead rely on the clients to
receive a network error and decide to repeat the same re…dmitshur commented on all: update vendored golang.org/x/crypto4d
TRY=longtest
It doesn’t make a difference in this case, but note in general we don’t need to include the -u flag when using go get in module mode. Using the selected dependency versions of the m…dmitshur closed an issue golang.org/x/build/maintner: don't sync GitHub issues when disabled4d
dmitshur commented on golang.org/x/build/maintner: don't sync GitHub issues when disabled4d
Issue #35299 is the same, I'll close one of them as a duplicate.dmitshur commented on runtime: mark string comparison hooks as no split5d
(1 comment)
Thanks. It needs to be added to the PR description to show up here, please see https://go.dev/wiki/CommitMessage#github-pull-requests for details.dmitshur commented on cmd/go: in module root, go list {module-root}/... behaves differently than go list ./..., yet go mod tidy removes go.sum entry needed for former5d
I think both the x/exp specifics and coordinator testing strategy (#51455) are in an okay shape by now, and not in scope of the problem described in the initial report here. Since it has the Thinking…dmitshur commented on runtime: mark string comparison hooks as no split5d
TRY=longtest, linux-amd64-race
Consider adding "For #53190." and possibly also mentioning this is a followup to CL 410034, since that information can be useful to future readers.dmitshur commented on golang.org/x/build/maintner/godata: getGoData helper should handle failure without failing test, otherwise that causes occasional false positive test failures due to HTTP/2 internal errors while syncing segments5d
Based on progress of investigation in #51323, the conclusion is that other than the error message not making it easy to see that the error is coming from a remote server for a possibly unavoidable re…dmitshur commented on golang.org/x/build/cmd/coordinator: improve header consistency5d
(1 comment)
Indeed. I think a better way to handle that would be to define a single top-level layout header with the top-level HTML and common header, and have individual pages provide "content" via…dmitshur opened a change golang.org/x/build/cmd/coordinator: improve header consistency5d
I forgot there was another copy of the template in status.go.
Move at least coordinator's 2 copies of the header into one
common place; we can do more later.
Also update the perf header to be consis…dmitshur reviewed +2 on golang.org/x/build/cmd/coordinator: fix misnamed span5d
(1 comment)
Minor: If the 's' wasn't included intentionally, given the method is called runBenchmarkTests, you can remove it.dmitshur reviewed +1 on cmd/go/internal/list: update help info with Deprecated field5d
(1 comment)
That sounds like an intentional change then, thanks.dmitshur commented on golang.org/x/build/cmd/relui: join communication tasks in main release workflows5d
This is generally ready (other than a small TODO), making it available for reading sooner while I make progress on #51150.dmitshur opened a change golang.org/x/build/internal/workflow: make Slice method variadic5d
Simplify API usage when the caller has multiple values they wish
to make a slice from, allowing:
Slice([]workflow.Value{v1, v2})
To be written as:
Slice(v1, v2)
For cases where the caller has a…dmitshur opened a change golang.org/x/build/cmd/relui: join communication tasks in main release workflows5d
The next step after joining the announcement mail and tweet tasks
is to append them to the end of the main existing release workflows.
This removes the need for the human release coordinator to manu…dmitshur commented on golang.org/x/build/cmd/relui: merge communication tasks with the main release workflow5d
This enables communication parameters to be entered at the start of workflow, and no more need to manually specify versions (those are computed during the release workflows, and available for reuse):…dmitshur opened an issue golang.org/x/build/cmd/relui: merge communication tasks with the main release workflow5d
Now that both announcement mail and tweet tasks are ready (and, as of #47406, joined together), the next step is to append them to the end of the main release workflows that build and publish artifac…dmitshur commented on golang.org/x/crypto/openpgp: a couple of packages do not invoke .Close and hence leak io.Closer values in some return paths5d
Thanks for reporting. Please note #44226.dmitshur commented on golang.org/x/crypto/openpgp/*: properly invoke .Close on errors5d
I'm not a great reviewer for these packages (also note they're marked as "Deprecated: this package is unmaintained except for security fixes"). I left one general suggestion in an inline comment.
Th…dmitshur reviewed +1 on cmd/go/internal/list: update help info with Deprecated field5d
dmitshur reviewed +2 on golang.org/x/website/_content/ref: specify redirect for /ref and /ref/ using /ref/index.md5d
Thanks.
(Two trivial nits, I marked them as resolved because I think it's fine not to do anything about them.)
Nit: There's a trailing '/' after 'c' in the final URL.
Nit: No newline at end of rig…dmitshur commented on github.com/glfw/glfw: Warning about "_GNU_SOURCE" redefined1w
I agree this looks like a duplicate of #2076, and should be discussed in go-gl/glfw#351.
We've just pushed a fix that should resolve that issue, @alexballas please feel free to try again with the …dmitshur commented on github.com/go-gl/glfw: fix _GNU_SOURCE redefined warning1w
Thanks.dmitshur pushed to
master in github.com/go-gl/glfw1w
dmitshur closed an issue github.com/go-gl/glfw/v3.3/glfw: _GNU_SOURCE is defined twice, emits a warning1w
dmitshur merged a change github.com/go-gl/glfw: fix _GNU_SOURCE redefined warning1w
dmitshur commented on github.com/go-gl/glfw: fix _GNU_SOURCE redefined warning1w
@cornelk I think what I wrote in https://github.com/go-gl/glfw/issues/351#issuecomment-1126811178 applies. If the problem can be reproduced with the GLFW C source code (independently of the Go bindin…dmitshur commented on golang.org/x/build: flesh out READMEs1w
> [...] it’s useful to have it automatically update, but the author should also be able to update the contents outside the block.
That was implemented in [CL 300229](https://go.dev/cl/300229); I…dmitshur closed an issue golang.org/x/build: flesh out READMEs1w
dmitshur commented on cmd/trace: document event trace viewer [freeze exception]1w
Since as https://go.dev/s/release says, "During the freeze, only bug fixes and doc updates are accepted.", this seems okay, and I don't think a freeze exception is needed.
This particular document…dmitshur commented on golang.org/x/website: wrong link to go1.19 sync/atomic package1w
Thanks for reporting. [CL 413235](https://go.dev/cl/413235) will fix this.dmitshur commented on golang.org/x/build/cmd/buildlet: add the ability to download an unzipped tarball from a URL 1w
This may also help with #51057 as I understand.Earlier
dmitshur starred github.com/open-goal/jak-project1w
dmitshur commented on golang.org/x/build/cmd/relui: use announcement link from e-mail task in tweet body1w
@heschi I plan to take this next now that work on #47405 is almost done, which I think is okay with you.dmitshur closed an issue golang.org/x/build: vendor gopkg.in/inf.v01w
dmitshur commented on golang.org/x/build: vendor gopkg.in/inf.v01w
Resolved by now via the module mode and the [module mirror](https://proxy.golang.org/), that dependency should be available even when gopkg.in is down.dmitshur commented on golang.org/x/build: record and collect compiler benchmarks1w
@mdempsky This issue might be okay as a duplicate to close given we now have #48803 and #49207. What do you think? (CC @prattmic)dmitshur starred github.com/nigeltao/fontscripts1w
dmitshur commented on golang.org/x/build/cmd/relui: draft minor release announcement e-mail1w
Thanks for offering help here, but this ended up being intertwined with #47405 and will be resolved as part of that.dmitshur commented on golang.org/x/build: decide status of Android TryBots1w
> Is it going to be similar to how iOS living today? (i.e. unless `try=ios`, the builder will only be tested after a CL is submitted?)
Yes, this is correct. "Removing the builder from the TryBot s…dmitshur commented on golang.org/x/build: decide status of Android TryBots2w
I think in the current state it makes sense for the `android-amd64-emu` builder to be post-submit only.
If I understand correctly, @cherrymui may support that too, based on [this comment](https://…dmitshur commented on golang.org/x/build: decide status of Android TryBots2w
As a small correction, only the `android-amd64-emu` builder is configured as a TryBot. The `android-386-emu` one is a post-submit only builder for all repos.dmitshur commented on github.com/gopherjs/gopherjs.github.io/Question: (How) Does the GopherJS playground handle includes?2w
Also see #26.
Its "Implementation" section discussed two approaches, though more relevant to the GOPATH mode. The Go module mode was added 3 years later and creates new possibilities to fetch depe…dmitshur closed an issue golang.org/x/build/cmd/releasebot: build releases regularly, automatically2w
dmitshur commented on golang.org/x/build/cmd/releasebot: build releases regularly, automatically2w
I think we can probably close this in favor of #40279, since the scope of that issue covers everything mentioned here, and it'll be in `cmd/relui` rather than `cmd/releasebot`.dmitshur commented on cmd/compile: generic interface conversion failed, reporting types from different scopes.2w
Since this issue is closed as a duplicate of #52026, and that issue is also closed, it's going to be harder to investigate here. If you're seeing a problem that looks related, please file a new issue…dmitshur starred github.com/opentomb/TRosettaStone32w
dmitshur pushed to
master in github.com/shurcooL/go-vcs2w
dmitshur commented on : build dashboard triage log2w
Notes:
From a quick random sampling of a few links and seeing the same x/exp test failure, I am guessing a large portion of the failures are all coming from one bad commit in x/exp. Even though th…dmitshur commented on : build dashboard triage log2w
Giving it a shot to at least collect new findings for this week.
<details><summary>Initial setup</summary>
```
$ go install golang.org/x/build/cmd/fetchlogs@latest
$ go install golang.org/x/b…dmitshur commented on cmd/go: module index [freeze exception]3w
With #53290 resolved, should this be closed too, or is there more to do?dmitshur opened an issue golang.org/x/build/internal/task: update postTweet to Twitter API v2 when possible3w
The tweet task currently uses Twitter API v1.1. We can't fully update to Twitter API v2 because the media endpoints aren't available there yet. This is the tracking issue for updating when it's possi…dmitshur commented on golang.org/x/website: https://go.dev/ref/ produces weird directory listing3w
`https://go.dev/ref` (no trailing slash) redirects to https://go.dev/doc/#references. I suspect this issue can be resolved by making `https://go.dev/ref/` do the same.dmitshur commented on go/doc: find a better home for Go documentation features like section headings3w
I expect [CL 410394](https://go.dev/cl/410394) (CC @rsc) will resolve this issue.dmitshur pushed to
master in github.com/shurcooL/graphql3w
dmitshur merged a change github.com/shurcooL/graphql: Add directive support3w
dmitshur reviewed +2 on github.com/shurcooL/graphql: Add directive support3w
dmitshur pushed to
master in github.com/david-bain/graphql3wdmitshur commented on github.com/shurcooL/graphql: Add directive support3w
Thanks, this change makes sense.
I'll refactor it slightly and add a test before merging.dmitshur deleted tag in github.com/shurcooL/go-goon3w
experiment-with-bypass_altdmitshur deleted branch in github.com/shurcooL-deprecated/goon3w
masterdmitshur created branch in github.com/shurcooL-deprecated/goon3w
maindmitshur deleted branch in github.com/shurcooL/go-goon3w
masterdmitshur created branch in github.com/shurcooL/go-goon3w
maindmitshur pushed to
master in github.com/shurcooL/go-goon3w