Activity
Yesterday
dmitshur commented on golang.org/x/build: no SlowBot for "linux-amd64-nocgo"5h
I've looked over relevant code and wasn't able to spot a problem that would explain this. The linux-amd64-nocgo builder should be available as a SlowBot, like all other builders. It's possible I miss…dmitshur opened an issue golang.org/x/build/dashboard: add nocgo slowbot alias to linux-amd64-nocgo builder5h
We can add a "nocgo" SlowBot alias, mapped to the linux-amd64-nocgo builder, so it's easier/shorter to test a CL with Cgo disabled. See #42084 for context. It should be a small CL.
CC @golang/rele…dmitshur commented on : x/website:5h
What is the URL of the page where you're seeing this? Can you please include a screenshot?dmitshur commented on golang.org/x/tools/godoc/vfs: files bound as root are treated as files5h
(1 comment)
I'm not sure if you saw it, but I left a note about this on the issue. See https://golang.org/issue/33495#issuecomment-536235982.dmitshur commented on golang.org/x/tools/godoc/vfs: include dirs needed to reach mount points in Stat5h
(2 comments)
Thanks. I'll try to take a look, but I don't think I'll able to get to it soon, as there is more pressing release-related work.
Jay's review helped me identify some issues with the cur…dmitshur commented on src: [release-branch.go1.14] update vendor, regenerate h2_bundle.go9h
(2 comments)
In order to fix issue #41386, this should be the only CL included. I'm not sure why the other CLs are appearing here. Please see my other comment.
When I run this command, I get a diff…dmitshur commented on golang.org/x/build/cmd/updatestd: investigate (at an appropriate time) a package-first update strategy, instead of module-first12h
Here is a strategy I am currently imagining if such a hypothetical flag is supported:
1. Determine an import path pattern that matches packages contained in the current module. For the `cmd` module,…dmitshur commented on golang.org/x/build/cmd/updatestd: investigate (at an appropriate time) a package-first update strategy, instead of module-first15h
I've spent some time thinking about this suggestion, and I understand it a lot better now. I also agree that a package-centric approach, if it is made robust, can be better and cleaner. However, I di…dmitshur commented on go/doc: Synopsis can return a sentence spanning multiple paragraphs and code blocks16h
I ran an experiment on the standard library and some modules that were recently published (by using the [module index](https://index.golang.org)). The results seem favorable, and there are quite a fe…dmitshur starred in github.com/8byt/gox18h
This Week
dmitshur reviewed +2 on cmd/go/internal/renameio: include ios in the darwin test-flake mitigation1d
(2 comments)
Since we’ve confirmed this is true, maybe remove the “I believe that” prefix.
Indeed. Thanks.dmitshur commented on cmd/go/internal/renameio: include ios in the darwin test-flake mitigation1d
(2 comments)
If you're referring to the flakiness workaround in robustio_flaky.go, I'm not seeing ios included there. Would you like to change that?
I read this as not matching iOS since there's no…dmitshur commented on golang.org/x/build/env/android-amdn64-emu: upgrade NDK to version 21.1.63524621d
(1 comment)
Sorry I missed this CL earlier. I should be able to review it soon.
There's a merge conflict, are you able to rebase and resolve it?dmitshur commented on cmd/go/internal/renameio: TestConcurrentReadsAndWrites flaky on ios-arm64-corellium builder1d
Also CC @eliasnaur. Any idea of this darwin kernel bug can be worked around on the side of the builder?dmitshur reviewed +1 on golang.org/x/pkgsite/doc: add information about running the frontend with -local1d
(1 comment)
Should a similar change be made here, or is it fine as is?dmitshur reviewed +2 on Revert "net/http: test that ParseMultipartForm returns an error for int overflow"1d
dmitshur commented on golang.org/x/build/dashboard: openbsd-mips64-jsing inconsistently greyed out1d
The feature of graying out a builder with a known issue has been implemented in x/build/app/appengine but only for the main section. It should be grayed out in the golang.org/x repository sections as…dmitshur commented on golang.org/x/build/release: Go installer appends insecure path 'C:\Go\bin' to Windows system PATH1d
CC @golang/security, @toothrot, @cagedmantis.dmitshur commented on golang.org/x/build: improve trybot aliases for ppc641d
Sound good, thanks for doing that.
I've remembered that we have a test that ensures each GOOS/GOARCH term has *some* slowbot alias. But given how similar `ppc64{,le}` are, I think it's fine for us t…dmitshur commented on golang.org/x/build: improve trybot aliases for ppc641d
> I frequently see changes being made to code related to PPC64 which only use TRY=ppc64 for testing those changes. [...] Could we change the ppc64 alias in build/dashboard/builders.go to run the linu…dmitshur commented on golang.org/x/build: add noopt trybot1d
I'm not working on this at this time, so unassigning.
CC @toothrot, @cagedmantis.dmitshur pushed to
master in github.com/shurcooL/gtdo2d
dmitshur opened a change cmd/go: add /v2 to another require example2d
This is the same change as in CL 144917, but applied to a nearby line.
For #28374.Last Week
dmitshur opened an issue cmd/go/internal/imports: ScanDir ignores .go files that start with _ but not .3d
From https://golang.org/cmd/go/:
> Files whose names begin with "_" (including "_test.go") or "." are ignored.
[`ScanDir`](https://github.com/golang/go/blob/fc981654c763c2c1d72df2a6e35ba3dfc78d…dmitshur opened an issue cmd/objdump: TestDisasmExtld fails with "no runtime.symtab symbol found" on AIX4d
On the aix-ppc64 builder:
```
--- FAIL: TestDisasmExtld (2.79s)
objdump_test.go:157: Running [/ramdisk8GB/workdir-host-aix-ppc64-osuosl/go/bin/go build -o /ramdisk8GB/workdir-host-aix-ppc64-…dmitshur reviewed +2 on net/http: try to deflake TestTransportDiscardsUnneededConns4d
dmitshur commented on golang.org/x/build/dashboard: add linux-riscv64-jsing builder (reverse)4d
(1 comment)
Thanks Joel.
I'll redeploy at the nearest opportunity.dmitshur commented on golang.org/x/tools/go/analysis/passes/buildtag: buildtag check triggers on multiline comment that mentions "+build" at start of line4d
Now that x/build test failure is fixed, I've retitled to be about the buildtag check.
As far as I can tell, this is working as intended and this issue should be closed. But I'll let @rsc or @matlo…dmitshur merged a change golang.org/x/build/cmd/release: adjust comment to not begin with "// +build"4d
dmitshur commented on golang.org/x/build/cmd/release: adjust comment to not begin with "// +build"4d
(1 comment)
I'll submit as this fixes tests; we can follow up on the issue if there's more to do.dmitshur commented on golang.org/x/build/dashboard: add openbsd-arm64-jsing builder (reverse)4d
(1 comment)
Filed #42019 and sent CL 263117 for the cmd/release failure.dmitshur opened a change golang.org/x/build/cmd/release: adjust comment to not begin with "// +build"4d
For golang/go#42019.dmitshur opened an issue golang.org/x/build/cmd/release: // +build comment after package clause causes a vet error on tip4d
Likely related to [CL 261958](https://golang.org/cl/261958), `gotip test golang.org/x/build/cmd/release` fails:
```
$ gotip test
# golang.org/x/build/cmd/release
./releaselet.go:609:1: +buil…dmitshur commented on net/rpc: Client.Go allocates a buffered channel with large capacity (10) instead of small (1) when done is nil4d
Thanks for filling in the template.
Please be aware that package `net/rpc` is frozen and not accepting new features.
I don't believe there's an issue here. Capacity 1 is enough for when the fre…dmitshur commented on golang.org/x/website/content/static: widen download button to avoid wrapping4d
(2 comments)
The "Fixes" line got removed in patch set 3:
Fixes golang/go#41856.
Looks good, thanks.dmitshur commented on golang.org/x/build/cmd/coordinator: improve error message when build fails due to a module not being tidy (and causing failed requests to sum.golang.org)4d
> it may suffice to set `GOFLAGS=-mod=readonly` in the builder environment
That has become the default in Go 1.16 (#40728), so the error message there looks like this:
```
... in golang.org/x/…dmitshur commented on cmd/go: rewrite paths for overlaid files using -trimpath5d
(1 comment)
The TestScript/build_overlay test is failing on the windows-amd64-longtest builder: https://build.golang.org/log/24c67afcd3eb8fefe65b729035d0fbfaf7a5a1fd.
Opened #42008 for tracking.dmitshur opened an issue cmd/go: TestScript/build_overlay is failing on windows-amd64-longtest builder5d
The `TestScript/build_overlay` test is failing on the `windows-amd64-longtest` builder as of [CL 257198](https://golang.org/cl/257198):
• https://build.golang.org/log/24c67afcd3eb8fefe65b729035d…dmitshur commented on golang.org/x/build/cmd/gopherbot: "congratulations" message was posted a second time5d
I think I see the problem. There wasn't a duplicate message sent to Gerrit. Instead, the "gobot@golang.org" Gerrit account name changed from "Gobot Gobot" to "Go Bot", which caused the Gerrit → Git…dmitshur opened an issue golang.org/x/build/cmd/gopherbot: "congratulations" message was posted a second time5d
GopherBot (along with some other services) was restarted around 12 hours ago and when it came back up, the `congratulateNewContributors` task ended up posting a repeat "congratulations" message on so…dmitshur commented on testing: Provide ability to inspect output of Example tests6d
> I do not have a way to access(programmatically) the output of the test.
Have you considered the `-json` flag of `go test`? For example:
```
{"Time":"2020-10-14T20:22:16.736222-04:00","Action…dmitshur commented on net/http: strange TLS1.3 behavior on test site, "tls: error decoding message"6d
CC @FiloSottile.dmitshur reviewed +2 on golang.org/dl/dl: add go1.15.3 and go1.14.106d
dmitshur commented on golang.org/x/website: Update add-a-test.html6d
(1 comment)
The ./content/static package needs to be regenerated after a change to the static content. This is caught by tests:
$ go test ./...
--- FAIL: TestStaticIsUpToDate (0.21s)
…dmitshur commented on golang.org/x/website: update /doc/install/source page after GO386 changes6d
Reopening because the x/website CL was reverted in [CL 261557](https://golang.org/cl/261557). x/website needs to be modified when Go 1.16 is released. I'll move this to Go 1.17 milestone with early-i…dmitshur reopened an issue golang.org/x/website: update /doc/install/source page after GO386 changes6d
dmitshur reviewed +2 on golang.org/x/website/Revert "content/static/doc: update install docs for 387->softfloat transition"6d
(1 comment)
Thanks.
I also filed #41979 about considering an automated mechanism to prevent accidental early submission of CLs with a wait-release hashtag.dmitshur opened an issue golang.org/x/build: prevent submission of CL with an outstanding wait-* hashtag6d
There is a convention of using a wait-* hashtag to indicate a CL is waiting on some specific item before progress can be made. This is described in issue #10658, which may need to be resolved more be…dmitshur reviewed +2 on golang.org/x/website/internal/history: document Go 1.15.3 and Go 1.14.106d
(1 comment)
If this is here because of CL 259858 only, I think it should be left out since that change was a fix for a flaky test (motivated by release testing needs). There is no change in behavior…dmitshur reviewed +2 on : [release-branch.go1.15] go1.15.36d
(1 comment)
LGTM. I've checked the build dashboard and the contents of release-branch.go1.15.dmitshur reviewed +2 on : [release-branch.go1.14] go1.14.106d
(1 comment)
LGTM. I've checked the build dashboard and the contents of release-branch.go1.14.dmitshur commented on golang.org/x/build/cmd/gopherbot: CC triaged issues to owners6d
(1 comment)
I posted a comment in https://go-review.googlesource.com/c/build/+/171238/10#message-752470128a36b70063527ced7c78712ed8650376 which would've been better to post here instead.dmitshur commented on golang.org/x/build/cmd/gopherbot: assign reviewers based on commit message prefixes6d
(1 comment)
(I recall Bryan was okay with other people picking this up from an earlier discussion.)
I've recently done some work locally to rebase this, resolve merge conflicts, and do some testing…dmitshur commented on cmd/compile: 1.15 heap allocations regression when calling Write on os.Stdout6d
Given that we've backported a fix for this issue to Go 1.15, I believe we should not release Go 1.16 without fixing this (or re-visiting this in some way), so marking as a release-blocker.dmitshur commented on src: [release-branch.go1.15] update vendor, regenerate h2_bundle.go6d
(1 comment)
I haven't had a chance to verify this yet, but if that is the reason, I think we may need to deal with this similarly to how we deal with an inconsistency of version selection between th…dmitshur commented on src: [release-branch.go1.15] update vendor, regenerate h2_bundle.go6d
(1 comment)
Do you know why this change appears here?
I suspect that it's because of #41375. If so, that is unfortunate and we should think about whether we want to include it.
The backport reques…dmitshur reviewed +1 on io: add a new ReadSeekCloser interface1w
dmitshur commented on golang.org/x/build/dashboard: add openbsd-arm64-jsing builder (reverse)1w
(1 comment)
This can probably be "Fixes" (unless you expect more will need to be done, then keeping "Updates" is fine).dmitshur opened an issue golang.org/x/build: linux-riscv64-unleashed builder is missing1w
From https://farmer.golang.org/:
```
host-linux-riscv64-unleashed: 0/0 (1 missing)
```
@bradfitz Are you able to get this builder up and running again? Does it just need to be restarted, or i…dmitshur reviewed +2 on golang.org/x/build/dashboard: add linux-riscv64-jsing builder (reverse)1w
(2 comments)
Add:
For golang/go#27532.
Thank you. Sorry that it took a while to get to this.dmitshur reviewed +2 on golang.org/x/build/dashboard: add openbsd-mips64-jsing builder (reverse)1w
(2 comments)
Thank you. One minor comment, otherwise LGTM.
I understand this builder is expected to fail initially because #40995 hasn't been resolved yet.
Please set KnownIssue to 40995 here so t…dmitshur commented on golang.org/x/build/dashboard: add openbsd-arm64-jsing builder (reverse)1w
(3 comments)
You're right, I forgot about that when making this suggestion. That's a good reason to leave it as is. Thanks.
This just needs to be rebased and it has a +2. I think it can be submitte…dmitshur commented on golang.org/x/build/dashboard: add openbsd-arm64-jsing builder (reverse)1w
(1 comment)
Thanks for the ping Joel. This CL (and 2 more) have been on my list and I plan to get to it today.dmitshur commented on net/http: deflake TestServerEmptyBodyRace_h1, or at least try1w
(1 comment)
Thanks, I'm taking that into account. :) In this case, it was quick for me to try and get a little more signal, so I ended up choosing to do it.dmitshur reviewed +2 on net/http: deflake TestServerEmptyBodyRace_h1, or at least try1w
(3 comments)
Since this change (and the flake problem) affects both TestServerEmptyBodyRace_h{1,2}, maybe remove the _h1 suffix.
Seems to work well in my brief testing.
It may be helpful to add an…dmitshur commented on cmd/link: collect2: error: ld returned 1 exit status1w
From your report, the error happens when testing the `github.com/getupio-undistro/undistro/client/cluster` package, but that package doesn't appear to be available publicly.
Is it possible to redu…dmitshur commented on access: Approver Access ("approvers")1w
Thank you for making the request @mengzhuo, we will consider it.dmitshur commented on cmd/go: module proxy protocol, extending the information endpoint1w
CC @bcmills, @jayconrod, @matloob per [owners](https://dev.golang.org/owners).dmitshur commented on net/rpc/client: too big capacity of Call's buffered Done channel1w
If this is a question, please see https://golang.org/wiki/Questions.
If this is a bug, please fill in the questions in the issue template.dmitshur commented on cmd/gofmt: unexpected indentation in chained calls1w
CC @griesemer.dmitshur commented on golang.org/x/crypto/ssh: KeyboardInteractiveChallenge error message is incorrect1w
Yes, filing an issue and discussing it first is very helpful, thanks.
CC @FiloSottile per [owners](https://dev.golang.org/owners).dmitshur commented on golang.org/x/net/ipv6: ControlMessage considers HopLimit==0 as missing1w
CC @mikioh, @ianlancetaylor per [owners](https://dev.golang.org/owners).dmitshur commented on crypto/x509/pkix: Name.String() rfc2253 AttributeTypes 1w
CC @FiloSottile, @agl, @rsc per [owners](https://dev.golang.org/owners).dmitshur commented on cmd/link: collect2: error: ld returned 1 exit status1w
Please provide more information about how to reproduce the error. "go test ./..." is not enough information. What repository did you run that in?dmitshur commented on cmd/compile: [signal SIGBUS: bus error] on Apple Silicon DTK (amd64 emulation)1w
Thanks for the report.
Given it has happened just once, in an old unsupported version, and the error doesn't make much sense, we can probably close this issue for now. It can be re-opened if it ha…dmitshur commented on golang.org/x/mobile: output diagnostic messages when skipping variables and methods1w
CC @hyangah, @hajimehoshi.dmitshur commented on golang.org/x/review/git-codereview: make 'git codereview mail' run 'git push' using --no-follow-tags1w
CC @rsc, @kevinburke.dmitshur commented on golang.org/x/net/ipv4: PacketConn.MulticastInterface is implemented incorrectly on Linux1w
CC @mikioh, @ianlancetaylor per [owners](https://dev.golang.org/owners).dmitshur commented on golang.org/x/tools/go/pointer: internal panic in pointer analysis1w
CC @golang/tools-team per [owners](https://dev.golang.org/owners).dmitshur commented on golang.org/x/tools/cmd/present2md: conversion strips comments and .caption commands1w
CC @rsc per [owners](https://dev.golang.org/owners).dmitshur commented on golang.org/x/text/internal/colltab: numeric.go should not skip "0" when is followed by a non-number1w
CC @mpvl per [owners](https://dev.golang.org/owners).dmitshur commented on net/http/httputil: ReverseProxy flush immediately additional case1w
This was a TODO comment added 2 years ago that was only reported 2 weeks ago. That suggests to me it's not a very serious issue, and fine to leave for 1.16.dmitshur reviewed +2 on syscall: switch go:generate directives back to mksyscall_windows.go1w
(4 comments)
s/meta-package/meta-pattern/
I think this is a good solution given the constraints today.
Some minor comments.
What does "pkgIsStd" short for? "Package is std"? Did you mean to say "…dmitshur commented on cmd/go: go generate sets more variables than its documentation specifies1w
> it doesn't say anything about starting from an empty environment
I agree. This issue isn't expecting an empty environment plus the 6 documented variables. It's expecting the caller's environment p…dmitshur commented on syscall: switch go:generate directives back to mksyscall_windows.go1w
(1 comment)
I think a trivial workaround could be to use a relative path instead of absolute path here.dmitshur commented on syscall: switch go:generate directives back to mksyscall_windows.go1w
(1 comment)
This reads nice, but as far as I can tell it relies on questionable cmd/go behavior. I was surprised it worked even when the GOROOT environment variable is unset (quite common).
I filed…dmitshur commented on cmd/go: go generate sets more variables than its documentation specifies1w
NeedsDecision is to decide whether to fix the implementation or the documentation.dmitshur opened an issue cmd/go: go generate sets more variables than its documentation specifies1w
`go generate` [documentation](https://golang.org/cmd/go/#hdr-Generate_Go_files_by_processing_source) currently says:
> Go generate sets several variables when it runs the generator:
>
> ```
> $…dmitshur merged a change golang.org/x/build/dashboard: remove known issue for GO386=softfloat builder1w
dmitshur reviewed +2 on net/http: deflake TestTransportPersistConnLeak on macOS1w
dmitshur commented on net/http: deflake TestTransportPersistConnLeak on macOS1w
(1 comment)
My second comment was sent too quickly. This loop is reading gotReqCh, not didReqCh. So I think my original comment applies.dmitshur commented on net/http: deflake TestTransportPersistConnLeak on macOS1w
(1 comment)
Oh, I misread when the send to didReqCh happens. There will be all 25 of them. So there isn't a problem.
However, the failed channel has become largely unused (there's never a value sen…dmitshur commented on net/http: deflake TestTransportPersistConnLeak on macOS1w
(1 comment)
If a client fetch error condition happens, there will be fewer than 25 sends to gotReqCh, and 0 sends to failed channel since line 1705 is deleted. That means execution won't get past th…dmitshur closed an issue cmd/cgo: jmethodID/jfieldID is not mapped to uintptr if building with the Android NDK [1.15 backport]1w
dmitshur commented on cmd/cgo: jmethodID/jfieldID is not mapped to uintptr if building with the Android NDK [1.15 backport]1w
Closed by merging 76a2c87a2c4e78f40f0f70bda3da93c773630179 to release-branch.go1.15.dmitshur merged a change cmd/compile: [release-branch.go1.15] export notinheap annotation to object file1w
dmitshur commented on cmd/compile: [release-branch.go1.15] export notinheap annotation to object file1w
(3 comments)
This was resolved in patch set 7.
Makes sense, thanks.
Answered above.dmitshur commented on golang.org/x/net/http2: connection-level flow control not returned if stream errors, causes server hang [1.14 backport]1w
Reopening for updating vendored and bundled copies in the standard library.