Activity
Today
dmitshur commented on cmd/link: skip TestOldLink if the old linker does not exist2h
(1 comment)
While reviewing this CL, I (re)discovered there is an API to get the directory where tools are installed: https://golang.org/pkg/go/build/#ToolDir.
However, to use it with os.Stat, you'…dmitshur commented on cmd/link: skip TestOldLink if the old linker does not exist2h
(1 comment)
We forgot the fully qualified issue reference in CL 242643, so the issue didn't get closed yet.
If you'd like, change this to Fixes as there's nothing more to do there after this is sub…dmitshur commented on golang.org/x/build/cmd/release: leave oldlink binary out of RC1 (if the beta doesn't uncover serious linker issues)3h
Thanks!
This is how the test fails when `oldlink` binary isn't present:
```
$ go test -v -run=TestOldLink -count=1 cmd/link
=== RUN TestOldLink
link_test.go:558: [/tmp/tmp.AG3oE0Ne/go/…dmitshur commented on golang.org/x/build/cmd/release: leave oldlink binary out of RC1 (if the beta doesn't uncover serious linker issues)4h
> A test may fail if the old linker is not present. We could change the test to skip if the binary does not exist.
@cherrymui Which test were you referring to?
I can try to find anything by tak…dmitshur commented on golang.org/x/build/cmd/release: remove oldlink binary from release artifacts4h
(2 comments)
The comment is optional. It'll be possible to figure out when it's safe to delete this even without it.
(Taking into account that we support two past versions of Go, of course.)
It do…dmitshur reviewed +2 on golang.org/x/build/cmd/release: remove oldlink binary from release artifacts4h
(1 comment)
I expect https://pkg.go.dev/cmd/oldlink will be removed in some future Go release. Do you know if there's already an issue tracking that?
If not, maybe make a new one. Then we can add a…dmitshur opened an issue golang.org/x/build/cmd/releasebot: improve -dry-run -mode=prepare to support advance testing for more release types7h
Right now, it's possible to use `releasebot -dry-run -mode=prepare` to run a release locally that doesn't produce any public artifacts or changes state, but only for the beta release type (e.g., `go1…dmitshur closed an issue net: remove dependency on fmt, unicode via golang.org/x/net/dns/dnsmessage9h
dmitshur commented on net: remove dependency on fmt, unicode via golang.org/x/net/dns/dnsmessage9h
I've confirmed the package `net` does not import `fmt` nor `unicode`, not even indirectly. It was resolved via [CL 241257](https://golang.org/cl/241257), and [CL 241078](https://golang.org/cl/241078)…dmitshur commented on runtime: pageAlloc.searchAddr may point to unmapped memory in discontiguous heaps, violating its invariant [1.14 backport]10h
@mknyszek Can you please add a short rationale about why the backport might be needed? (Per [MinorReleases](https://golang.org/wiki/MinorReleases).) Thanks.dmitshur commented on doc: document/reassert that last two releases are supported equally10h
@ianlancetaylor Issue #37097 was about a problem on the Go-Release-Cycle page.
I spotted a problematic statement in the MinorReleases page; filed #40206 about it.dmitshur opened an issue wiki: MinorReleases has an out of date statement on backporting policy10h
The https://golang.org/wiki/MinorReleases page currently still says:
> "Use a more recent stable version" is a valid workaround, so very few fixes will be backported to both previous issues.
Th…dmitshur commented on database/sql: context cancellation allows statements to execute after rollback11h
Based on https://github.com/golang/go/issues/34775#issuecomment-628115836:
> this issue has existed since Go1.12 and unfortunately because it wasn't a regression but rather a long standing bug
…dmitshur opened an issue database/sql: context cancellation allows statements to execute after rollback [1.13 backport]11h
@dmitshur requested issue #34775 to be considered for backport to the next 1.13 minor release.
The original issue exists in 1.13 too, so per backport policy, it must be considered for backporting …Yesterday
dmitshur closed an issue : missing function body - x/tools/go/ssa/interp/testdata/src/os/os.go:3:61d
dmitshur commented on : missing function body - x/tools/go/ssa/interp/testdata/src/os/os.go:3:61d
Thanks for clarifying, that's very helpful. In the original issue report, I didn't realize you were using an IDE and could not understand where the error was coming from.
> I wish I was already a …dmitshur merged a change net/http: [release-branch.go1.13] fix panic with If-None-Match value in http.ServeContent1d
dmitshur merged a change net/http: [release-branch.go1.14] fix panic with If-None-Match value in http.ServeContent1d
dmitshur commented on all: modify VERSION file1d
This was a test for #30037. It's no longer needed.dmitshur closed a change all: modify VERSION file1d
dmitshur closed an issue golang.org/x/build/cmd/gerritbot: some pull requests are neither imported nor closed1d
dmitshur commented on golang.org/x/build/cmd/gerritbot: some pull requests are neither imported nor closed1d
The current behavior is that GopherBot will import PRs into Gerrit (see https://github.com/golang/go/pull/40190 and https://go-review.googlesource.com/c/go/+/242279 for a recent example). However, it…dmitshur opened a change all: modify VERSION file1d
DO NOT REVIEW
DO NOT SUBMIT
This is a test.dmitshur created branch in github.com/dmitshur/go1d
modified-rb112dmitshur forked1d
dmitshur merged a change golang.org/x/website/internal/history: document upcoming Go 1.14.5 and Go 1.13.13 security releases1d
dmitshur opened a change golang.org/x/website/internal/history: document upcoming Go 1.14.5 and Go 1.13.13 security releases1d
They have been pre-announced¹.
¹ https://groups.google.com/d/msg/golang-announce/f2c5bqrGH_g/GnH6k56AAAAJdmitshur commented on golang.org/x/tour: Fix bump standard for error handling #9391d
@georettica Does the email you signed the CLA with match the one used as the git author email (can be seen in second line [here](https://github.com/georgettica/tour/commit/18cb8b1c03a4085fd45f71d3ec7…dmitshur merged a change golang.org/x/build/internal/gophers: add new email for Roberto Clapis1d
dmitshur reviewed +2 on golang.org/x/build/internal/gophers: add new email for Roberto Clapis1d
dmitshur commented on golang.org/x/pkgsite: some code examples are not displayed (but work at godoc.org)1d
One lead to investigate is that the exported `DB` type is defined as:
```
type DB struct {
*baseDB
ctx context.Context
}
```
Notably, `baseDB` is an embedded field. The `DB` type gets many of its…dmitshur commented on golang.org/x/pkgsite: some code examples are not displayed (but work at godoc.org)1d
Thanks for reporting.
I tested this package at commit https://github.com/go-pg/pg/commit/6fe164e2d690b7e22a4787f5d40d5dd93fc2e5e3 with pkg.go.dev, godoc.org, and x/tools/cmd/godoc, and can confirm…Last Week
dmitshur pushed to
master in dmitri.shuralyov.com/website/gido2ddmitshur commented on : missing function body - x/tools/go/ssa/interp/testdata/src/os/os.go:3:63d
I would help if you provided a higher level overview of the problem, not the exact specifics. Can you please answer these these questions:
1. Is there a snippet we can run to reproduce the problem…dmitshur pushed to
master in github.com/shurcooL/events3d
dmitshur pushed to
master in github.com/shurcooL/git-branches3ddmitshur commented on net/http: [release-branch.go1.14] fix panic with If-None-Match value in http.ServeContent3d
(1 comment)
Thanks Emmanuel. Only release managers can submit CLs to the release branches¹, so there's no expectation the CL owner to submit this. Now that both this and and the 1.13 CL are reviewe…dmitshur commented on database/sql: [release-branch.go1.14] backport 5 Tx rollback related CLs3d
(4 comments)
This statement seems out of date.
This list includes 6 CLs, the subject and list below includes only 5.
It’s hard to see that this note applies to the entire back port CL and is not…dmitshur opened a change net/http: [release-branch.go1.14] handle body rewind in HTTP/2 connection loss better3d
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]3d
@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 types3d
(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 barriers3d
dmitshur merged a change reflect: [release-branch.go1.14] zero stack slots before writing to them with write barriers3d
dmitshur commented on database/sql: [release-branch.go1.14] backport 3 Tx rollback related CLs4d
(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 limit4d
/cc @FiloSottile @x1ddos per [owners](https://dev.golang.org/owners).dmitshur commented on golang.org/x/sys/windows/svc/example: does not start4d
/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 CL4d
@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 CL4d
/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 requirement4d
/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)4d
/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 errors4d
/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">4d
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 example4d
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 override4d
/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.ServeContent4d
Updates #39817.
Fixes #39925.dmitshur commented on net/http: [release-branch.go1.14] fix panic with If-None-Match value in http.ServeContent4d
(1 comment)
Done.dmitshur commented on net/http: panic on misformed If-None-Match Header with http.ServeContent [1.13 backport]4d
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]4d
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 builder4d
(1 comment)
TRY=solaris-amd64-oraclereldmitshur opened a change testing: [release-branch.go1.14] capture testname on --- PASS and --- FAIL lines4d
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 type4d
dmitshur merged a change cmd/link: [release-branch.go1.14] detect trampoline of deferreturn call4d
dmitshur commented on cmd/compile: internal compile error when using sync.Pool: mismatched zero/store sizes [1.13 backport]4d
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]4d
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 call4d
(1 comment)
TRY=linux-ppc64,linux-ppc64ledmitshur commented on cmd/link: [release-branch.go1.14] detect trampoline of deferreturn call4d
(1 comment)
I'll rebase and submit after trybots pass.dmitshur commented on cmd/test2json: tests that panic are marked as passing4d
/cc @jayconrod @bcmills @matloobdmitshur commented on image/jpeg: "bad RST marker" error when decoding4d
/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:64d
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.go4d
(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.go4d
(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 call4d
(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]4d
@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]4d
@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 flags4d
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]4d
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]4d
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 updated4d
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 CL4d
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 CL4d
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 issue4d
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]4d
@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]4d
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]4d
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]4d
@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]4d
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 updated4d
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 environment4d
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 environment4d
dmitshur closed a change golang.org/x/tools/godoc: allow change of GOOS and GOARCH through environment4d
dmitshur merged a change all: [dev.boringcrypto] merge master into dev.boringcrypto4d
dmitshur opened a change all: [dev.boringcrypto] merge master into dev.boringcrypto5d
dmitshur reviewed +2 on golang.org/x/build/cmd/gopherbot: fix missing import5d
(1 comment)
Thanks for following up!dmitshur merged a change all: [dev.boringcrypto] merge master into dev.boringcrypto5d
dmitshur commented on all: [dev.boringcrypto] merge master into dev.boringcrypto5d
(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"5d
dmitshur commented on encoding/json: incorrect object key unmarshaling when using custom TextUnmarshaler as Key with string values [1.14 backport]5d
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" certificate5d
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.boringcrypto5d
(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]5d
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…