Activity
Yesterday
dmitshur opened a change net/http: [release-branch.go1.14] handle body rewind in HTTP/2 connection loss better3h
In certain cases the HTTP/2 stack needs to resend a request.
It obtains a fresh body to send by calling req.GetBody.
This call was missing from the path where the HTTP/2
round tripper returns ErrSkip…dmitshur commented on net/http: Re-connect with upgraded HTTP2 connection fails to send Request.body [1.13 backport]3h
@rsc This backport has been approved for 1.14 and 1.13. It applies cleanly to 1.14, but not 1.13. Since you authored the original CL, would you like to resolve the conflict and send a [cherry-pick CL…dmitshur commented on doc/go1.14: document that Unmarshal supports map keys with string underlying types4h
(3 comments)
Daniel, how does patch set 2 look to you?
s/supports/support/
s/types/type/dmitshur merged a change reflect: [release-branch.go1.13] zero stack slots before writing to them with write barriers4h
dmitshur merged a change reflect: [release-branch.go1.14] zero stack slots before writing to them with write barriers4h
dmitshur commented on database/sql: [release-branch.go1.14] backport 3 Tx rollback related CLs4h
(2 comments)
Thanks for working on this!
> Oddly git-codereview complained about gofmt for some runtime files and the only
way to get this change up was to submit to its complaint and then make thi…dmitshur commented on golang.org/x/crypto/acme: Retry on LetsEncrypt rate limit5h
/cc @FiloSottile @x1ddos per [owners](https://dev.golang.org/owners).dmitshur commented on golang.org/x/sys/windows/svc/example: does not start5h
/cc @alexbrainman @bradfitz per [owners](https://dev.golang.org/owners).dmitshur commented on golang.org/x/pkgsite: run locally to view documentation of a pending CL5h
@jayconrod `x/tools/cmd/godoc` has support for module mode (see https://groups.google.com/g/golang-dev/c/uxtj_w7wpnY/m/jR3ui47eBQAJ), what's the reason to want to use `pkgsite` instead of continuing …dmitshur commented on golang.org/x/pkgsite: run locally to view documentation of a pending CL5h
/cc @julieqiu per [owners](https://dev.golang.org/owners).dmitshur commented on golang.org/x/pkgsite: in cmd/frontend's --proxy_url argument, exclude localhost from https requirement5h
/cc @julieqiu per [owners](https://dev.golang.org/owners).dmitshur commented on golang.org/x/pkgsite: handle queries with https:// and http:// scheme(strip schemes from searches)5h
/cc @julieqiu per [owners](https://dev.golang.org/owners).dmitshur commented on golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow: fixing shadowed errs results in code more likely to cause errors5h
/cc @matloob per [owners](https://dev.golang.org/owners).dmitshur commented on golang.org/x/net/html: doesnt support attributes without values ie <script async "foo.js">5h
Aside from visual noise and extra bytes, does the added `=""` cause any issues when the HTML is parsed?
/cc @mikioh @bradfitz @ianlancetaylor per [owners](https://dev.golang.org/owners).dmitshur commented on net/http: add http.ListenAndServe to ServeMux.Handle example5h
Thanks for reporting. I agree the example, as displayed at https://golang.org/pkg/net/http/#example_ServeMux_Handle, can feel like it's missing some code to start the server.
However, the example …dmitshur commented on cmd/vet: structtag field repeats json warning with valid override6h
/cc @alandonovan (per [owners](https://dev.golang.org/owners)) @ianlancetaylor @matloob.dmitshur opened a change net/http: [release-branch.go1.13] fix panic with If-None-Match value in http.ServeContent6h
Updates #39817.
Fixes #39925.dmitshur commented on net/http: [release-branch.go1.14] fix panic with If-None-Match value in http.ServeContent6h
(1 comment)
Done.dmitshur commented on net/http: panic on misformed If-None-Match Header with http.ServeContent [1.13 backport]6h
Backport approved for Go 1.14 (#39920) and 1.13 (this issue) as this is a serious issue with no workaround.dmitshur commented on net/http: panic on misformed If-None-Match Header with http.ServeContent [1.14 backport]6h
Backport approved for Go 1.14 (this issue) and 1.13 (#39925) as this is a serious issue with no workaround.dmitshur commented on cmd/go: [release-branch.go1.14] fix parallel chatty tests on solaris-amd64 builder7h
(1 comment)
TRY=solaris-amd64-oraclereldmitshur opened a change testing: [release-branch.go1.14] capture testname on --- PASS and --- FAIL lines7h
This fixes an issue raised at https://github.com/golang/go/issues/38458#issuecomment-635617670
in which --- PASS and --- FAIL lines would not trigger --- CONT lines
of other tests.
Updates #38458.
F…dmitshur merged a change cmd/compile: [release-branch.go1.13] remove check that Zero's arg has the correct base type8h
dmitshur merged a change cmd/link: [release-branch.go1.14] detect trampoline of deferreturn call8h
dmitshur commented on cmd/compile: internal compile error when using sync.Pool: mismatched zero/store sizes [1.13 backport]9h
Backport approved for Go 1.14 (#39849) and 1.13 (this issue) because it's a small fix to a serious problem without a workaround.dmitshur commented on cmd/compile: internal compile error when using sync.Pool: mismatched zero/store sizes [1.14 backport]9h
Backport approved for Go 1.14 (this issue) and 1.13 (#39848) because it's a small fix to a serious problem without a workaround.dmitshur commented on cmd/link: [release-branch.go1.14] detect trampoline of deferreturn call9h
(1 comment)
TRY=linux-ppc64,linux-ppc64ledmitshur commented on cmd/link: [release-branch.go1.14] detect trampoline of deferreturn call9h
(1 comment)
I'll rebase and submit after trybots pass.dmitshur commented on cmd/test2json: tests that panic are marked as passing11h
/cc @jayconrod @bcmills @matloobdmitshur commented on image/jpeg: "bad RST marker" error when decoding11h
/cc @nigeltao @robpike per [owners](https://dev.golang.org/owners).dmitshur commented on : missing function body - x/tools/go/ssa/interp/testdata/src/os/os.go:3:611h
Can you please provide a more complete reproduce case? Where did you add `if err := Config.validate(p); err != nil { ... }`? What does `Config` refer to?dmitshur commented on golang.org/x/build: Updated email in gophers.go11h
(1 comment)
Since this change was sent as a GitHub PR, https://golang.org/wiki/CommitMessage#github-pull-requests is relevant.dmitshur commented on golang.org/x/build: Updated email in gophers.go11h
(3 comments)
The subject should be something like:
internal/gophers: add new email for Roberto Clapis
Notably, package path prefix, colon, space, lower case letter, verb that completes "This c…dmitshur commented on cmd/link: [release-branch.go1.14] detect trampoline of deferreturn call12h
(1 comment)
Cherry, I have a question about this 1.14 backport CL. Its parent is currently "[release-branch.go1.14] cmd/link: fix GC data reading from shared library (attempt 2)", which means trybot…dmitshur commented on cmd/link: incorrect GC bitmap when global's type is in another shared object [1.13 backport]12h
@cherrymui Yes, 1.13 will become unsupported once 1.15 is released, which should happen next month. However, both 1.14 and 1.13 are supported equally until then (see #34536 for more details), so if t…dmitshur commented on cmd/compile: s390x floating point <-> integer conversions clobbering the condition code [1.14 backport]14h
@mundaym This backport doesn't apply cleanly. Would you like to work on creating the backport CL as described at https://golang.org/wiki/MinorReleases#making-cherry-pick-cls and see if the merge conf…dmitshur opened a change cmd/compile: [release-branch.go1.14] mark s390x int <-> float conversions as clobbering flags14h
These conversion instructions set the condition code and so should
be marked as clobbering flags.
Updates #39651.
Fixes #39690.dmitshur commented on cmd/compile: s390x floating point <-> integer conversions clobbering the condition code [1.13 backport]14h
Approved for 1.14 (#39690) and 1.13 (this issue) as it's a serious problem without a workaround.dmitshur commented on cmd/compile: s390x floating point <-> integer conversions clobbering the condition code [1.14 backport]14h
Approved for 1.14 (this issue) and 1.13 (#39689) as it's a serious issue without a workaround.dmitshur commented on golang.org/x/build: some Gerrit CLs are getting into a state where they can't be updated14h
The relevant code for this error message in Gerrit source code is at https://gerrit.googlesource.com/gerrit/+/refs/tags/v3.2.2/java/com/google/gerrit/server/notedb/OpenRepo.java#181.dmitshur commented on golang.org/x/build/cmd/gerritbot: incorrectly attempts to close a cherry-pick CL15h
Issue #25020 (/cc @ALTree) might be relevant here. If GerritBot indeed doesn't support branches other than `master`, then the fix for this issue will be very simple: it should not consider a Gerrit C…dmitshur opened an issue golang.org/x/build/cmd/gerritbot: incorrectly attempts to close a cherry-pick CL15h
Consider a Gerrit CL that was imported from a GitHub PR, and merged (or closed without merging). For example, see PR #39821.
It's possible that a cherry-pick of that imported Gerrit CL will be mad…dmitshur opened an issue golang.org/x/build/cmd/gopherbot: "close stale WaitingForInfo" task failing on non-existing issue15h
From `gopherbot` logs:
```
2020/07/10 13:33:30 close stale WaitingForInfo: POST https://api.github.com/repos/golang/go/issues/39453/comments: 422 Validation Failed [{Resource:IssueComment Field:d…dmitshur commented on cmd/link: incorrect GC bitmap when global's type is in another shared object [1.13 backport]16h
@cherrymui You've sent a 1.14 backport CL for #39955. Are you able to send one for 1.13 (this issue) as well?dmitshur commented on cmd/link: incorrect GC bitmap when global's type is in another shared object [1.13 backport]16h
Backport approved for Go 1.14 (#39955) and 1.13 (this issue) based on the rationale in https://github.com/golang/go/issues/39955#issuecomment-652647856.dmitshur commented on cmd/link: incorrect GC bitmap when global's type is in another shared object [1.14 backport]16h
Backport approved for Go 1.14 and 1.13 based on that rationale.dmitshur commented on runtime: missing deferreturn on linux/ppc64le [1.14 backport]16h
@mkumatag There will be a security release on Tuesday, July 14, as recently [pre-announced](https://groups.google.com/d/msg/golang-announce/f2c5bqrGH_g/GnH6k56AAAAJ). It will include only the securit…dmitshur commented on runtime: missing deferreturn on linux/ppc64le [1.14 backport]16h
Approving as it is a serious problem without a workaround. It is a 1.14-only regression, so a fix for 1.13 isn't needed.dmitshur opened an issue golang.org/x/build: some Gerrit CLs are getting into a state where they can't be updated17h
From `gopherbot` logs, there are a number of CLs where a reviewer fails to be added due to the error "Change nnn may not exceed 5000 updates. It may still be abandoned or submitted. To continue worki…dmitshur commented on golang.org/x/tools/godoc: allow change of GOOS and GOARCH through environment17h
Thanks for sending a change.
Per https://golang.org/doc/contribute.html#design, please open an issue and fill in the template, then add a "Fixes golang/go#nnn." line here.
I'd like to understan…dmitshur reopened a change golang.org/x/tools/godoc: allow change of GOOS and GOARCH through environment17h
dmitshur closed a change golang.org/x/tools/godoc: allow change of GOOS and GOARCH through environment17h
dmitshur merged a change all: [dev.boringcrypto] merge master into dev.boringcrypto18h
This Week
dmitshur opened a change all: [dev.boringcrypto] merge master into dev.boringcrypto1d
dmitshur reviewed +2 on golang.org/x/build/cmd/gopherbot: fix missing import1d
(1 comment)
Thanks for following up!dmitshur merged a change all: [dev.boringcrypto] merge master into dev.boringcrypto1d
dmitshur commented on all: [dev.boringcrypto] merge master into dev.boringcrypto1d
(1 comment)
> For now, let's just bundle all crypto together in the branch, because that's effectively what crypto/internal/boring does.
Done, PTAL.
The final result diff turned out smaller and mo…dmitshur merged a change encoding/json: [release-branch.go1.14] revert "avoid work when unquoting strings, take 2"1d
dmitshur commented on encoding/json: incorrect object key unmarshaling when using custom TextUnmarshaler as Key with string values [1.14 backport]1d
Approved as this 1.14-only regression is a serious issue without a workaround.dmitshur commented on golang.org/x/mobile/cmd/gomobile: test failed w/ "Apple Development" certificate1d
I've run into this too when investigating iOS support recently, and temporarily changed to "Apple Development" locally. I haven't followed up on that yet, but just wanted to add a data point.dmitshur commented on all: [dev.boringcrypto] merge master into dev.boringcrypto1d
(1 comment)
> Removing encoding/asn1 is easy, removing math/big is not.
Got it. Removing encoding/asn1 but keeping math/big would still be an improvement (albeit a smaller one), as it would allow t…dmitshur opened an issue crypto/internal/boring: consider splitting up into smaller packages based on math/big requirement [dev.boringcrypto]1d
As discussed in [golang.org/cl/241442](https://go-review.googlesource.com/c/go/+/241442/1#message-bb399544c64ab073e5adf50eef6a961dc798ab5e), this is a tracking issue to look into breaking `crypto/int…dmitshur reviewed +2 on doc/go1.15: add line for testing streaming change1d
(4 comments)
Maybe also add the related issues:
Updates #38458.
Updates #24929.
Thanks!
(This note.)
This is an .html file and Markdown isn't supported here, so please convert this to HT…dmitshur commented on cmd/go: include GOMODCACHE in documented list of environment variables1d
There's a CL out to fix this (that just needs review), I think we don't want to release Go 1.15 without this, so marking release-blocker so this isn't missed by accident.dmitshur commented on : WebAssembly not executing/compiling due to incorrect response MIME type. Expected 'application/wasm'.1d
> Running the mentioned go playground program prints "application/wasm", so there it is working correctly.
What about if you run the same program on your computer, what does it print then?dmitshur commented on all: [dev.boringcrypto] merge master into dev.boringcrypto2d
(1 comment)
Yes, if encoding/asn1 and math/big dependencies are removed from crypto/internal/boring, it would make fitting it in much easier! All other packages it imports are much lower level.
Tho…dmitshur commented on all: [dev.boringcrypto] merge master into dev.boringcrypto2d
(1 comment)
> I don't think the distinction between which crypto packages import cgo and which ones don't is meaningful in dev.boringcrypto.
Thanks for this information.
However, even if we drop t…dmitshur opened a change all: [dev.boringcrypto] merge master into dev.boringcrypto2d
dmitshur commented on cmd/go: include GOMODCACHE in 'go help environment'2d
(2 comments)
An optional comment about a small detail.
Compared to how many other env vars are currently described here, this sentence seems a bit out of place and may be prone to being misinterpre…dmitshur commented on golang.org/x/build/maintner/maintnerd/maintapi: TRY= comments (for SlowBots) without a preceding blank line are silently ignored3d
Removed the fixed pitfall from the SlowBots wiki page in commit [`2b70f5ce`](https://github.com/golang/go/wiki/SlowBots/_compare/c6e42666f7905fcaef067d5495004c47d1426ae7...2b70f5ce3d2354f29339716db0c…dmitshur edited the wiki3d
dmitshur merged a change golang.org/x/build/maintner/maintnerd/maintapi: look for TRY= in inline comments3d
dmitshur commented on golang.org/x/build/maintner/maintnerd/maintapi: look for TRY= in inline comments3d
(1 comment)
This has been tested and it's working as expected so far; I'm not spotting any issues.
After leaving https://go-review.googlesource.com/c/go/+/241274/3#message-b8608a9934c725916edcf6dfe…dmitshur commented on golang.org/x/build/maintner/maintnerd/maintapi: look for TRY= in inline comments3d
(2 comments)
Yep, thanks.
Done.dmitshur commented on crypto/x509/internal/macos: rename package to lowercase3d
(1 comment)
TRY=macos, iosdmitshur commented on golang.org/x/build/maintner/maintnerd/maintapi: look for TRY= in inline comments3d
(3 comments)
Added a comment.
Done.
Done.dmitshur commented on golang.org/x/tools/internal/lsp/regtest: regression tests for on-disk file changes failing on darwin builders3d
> are there any known issues with that builder?
I can't think of anything relevant off the top of my head, and the "notes" field at https://farmer.golang.org/builders is empty. In general, the darwi…dmitshur commented on golang.org/x/build/maintner/maintnerd/maintapi: look for TRY= in inline comments3d
(3 comments)
I agree. If there's ever a better way, I'd be happy to switch to it. That said, I can say I've been associating inline comments with the top-level message just by the equating the time …dmitshur opened a change golang.org/x/build/maintner/maintnerd/maintapi: look for TRY= in inline comments3d
Fixes golang/go#40106.
Fixes golang/go#38747.dmitshur commented on golang.org/x/build: Gerrit new patchset comments break SlowBots3d
I've looked into this and know what needs to be done to restore service:
- update maintapi to extract TRY= comments from inline comments (where they now end up)
- [likely] add [List Change Comm…dmitshur commented on : [release-branch.go1.13] empty change3d
(2 comments)
TRY=linux-amd64-longtest
(Unresolved comment.)
Reply.dmitshur commented on : [release-branch.go1.13] empty change3d
(1 comment)
TRY=linux-amd64-longtest
(Resolved comment.)dmitshur reviewed +1 on crypto/x509/internal/macos: rename package to lowercase3d
(1 comment)
Tests in build_test.go are primarily testing that the go/build package itself is working correctly.
I suggest moving this to the bottom of deps_test.go, that seems like a better place t…dmitshur commented on crypto/x509/internal/macos: rename package to lowercase3d
(1 comment)
I realize that this was likely copied directly from TestDependencies, but consider reusing testenv.HasSrc() (see https://pkg.go.dev/internal/testenv#HasSrc) to achieve the same effect an…dmitshur commented on golang.org/x/tools: freebsd-amd64-race builder started to fail often after CL 2397543d
The problem with "unable to add local env to initialize request" doesn't look related to the race in #39865, but maybe I'm misunderstanding it.
If you think all of the recent failures on `freebsd-am…dmitshur opened an issue golang.org/x/tools/internal/lsp/regtest: regression tests for on-disk file changes failing on darwin builders3d
The test added in [CL 238917](https://golang.org/cl/238917) are not passing on darwin builders:
The RELNOTES comment in CL 207877 includes a partial note, maybe it's worth copying it here:
> The linker defaults to internal linking mode for PIE on linux/amd64 and linux/arm64, which…dmitshur commented on misc/wasm: calling fmt.Println in different goroutines causes a crash3d
The fix for #38574 was included in go1.15beta1. @albrow, can you please see if the issue is fixed in that version?dmitshur commented on misc/wasm: Calling fmt.Println in different goroutines causes a crash4d
Thanks for reporting.
/cc @neelance @cherrymuidmitshur commented on cmd/go: go test -json splits log records4d
/cc @matloob @bcmills @jayconroddmitshur commented on golang.org/x/tools/present: unexpected eof on slide header only4d
/cc @rsc per [owners](https://dev.golang.org/owners).dmitshur commented on golang.org/x/tools/go/packages: test timeout on freebsd-amd64-race builder4d
I believe this has been fixed by the builder change in [CL 227859](https://golang.org/cl/227859). There have not been timeout errors in the `x/tools/go/packages` package lately. Closing.
This is a…dmitshur closed an issue golang.org/x/tools/go/packages: test timeout on freebsd-amd64-race builder4d
dmitshur opened an issue golang.org/x/build: freebsd-amd64-race builder started to fail often on golang.org/x/tools repo4d
The `freebsd-amd64-race` builder was largely passing on x/tools before June 25, and started to often since then:
| Before | After |
|--------|------|
. It's mentioned in the release notes ([CL 230537](https://golang.org/cl/230537)). Should it also be included in the list at https://tip.gol…dmitshur commented on golang.org/x/tools: frequent "out of memory" errors on windows-amd64-race builder4d
I've looked at the last 2 weeks of results (this is after the builder change in [CL 235419](https://golang.org/cl/235419)), and I'm not spotting OOM-related failures. There are frequent failures on `…dmitshur commented on golang.org/x/tools/internal/lsp/source: race detected during execution of TestSource4d
I was able to reproduce locally on darwin/amd64 at golang/tools@95bc2bdf7e31d255ef5ac04e85be0e11883c7cf2 with:
```
$ go version
go version go1.14.4 darwin/amd64
$ go test -count=10 -race golang…dmitshur opened an issue golang.org/x/tools/internal/lsp/source: race detected during execution of TestSource4d
Spotted via `linux-amd64-race` and `windows-amd64-race` builders at https://build.golang.org/?repo=golang.org%2fx%2ftools:
```
--- FAIL: TestSource (99.21s)
--- FAIL: TestSource/GOPATH (56.5…