golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.15k stars 17.56k forks source link

x/build/cmd/gopherbot: released things get marked Unreleased #39213

Open rsc opened 4 years ago

rsc commented 4 years ago

Any issue in an x/ package that is vendored into the release should not be milestoned Unreleased by Gopherbot.

Issue #32441 was left alone for a long time in part because it was in the Unreleased milestone. And it appears to have ended up in the Unreleased milestone because Gopherbot put it there. And Gopherbot appears to have put it there because the title said "x/net/http". My assumption is that Gopherbot has a rule that issues with titles beginning with "x/" (or maybe a specific list of repos) get placed in the Unreleased milestone. This rule is too aggressive: x/net/http is included in releases, so it is wrong to mark its issues Unreleased.

In addition to fixing Gopherbot, we may want to investigate potentially mis-milestoned issues. A quick scan turns up the following issues in release-vendored packages which are in the Unreleased milestone but perhaps should not be:

24870 x/net/dns/dnsmessage: cannot parse mDNS SRV records

24288 x/net/dns: invalid assumptions about domain names and character strings

38064 x/net/http2/h2c: POST from Java 11 client code causing error in h2cUpgrade

37089 x/net/http2/h2c: http BaseContext/ConnContext methods are not used

26682 x/net/http2/h2c: support closure of all connections and graceful shutdown

29187 x/net/http2/hpack: Regression in dynamic table size updates

20574 x/net/http2/hpack: Write customer-header into HPACK byName Map rather than byNameValue

30407 x/net/http2: Expose isBadCipher

13434 x/net/http2: TestServer_RejectsLargeFrames fails with "An existing connection was forcibly closed by the remote host" on windows

18273 x/net/http2: TestTransportFlowControl takes 31 seconds on OpenBSD

25793 x/net/http2: Transport ignores net/http.Transport.Proxy once connected

18341 x/net/http2: Transport should handle 421

17265 x/net/http2: access to maxConcurrentStreams

17776 x/net/http2: add a MarkComplete method to ClientConnPool

35860 x/net/http2: cannot create 'MaxConcurrentStreams' streams with a single ClientConn

37505 x/net/http2: data race in onSameConn test helper

15998 x/net/http2: data race on DebugGoroutines

16029 x/net/http2: deal with half-closed remote in Transport, flaky TestTransportResPattern_c0h1d0t0, TestTransportResPattern_c1h2d0t0

17775 x/net/http2: expose CloseIfIdle and ClientConnPoolIdleCloser

32388 x/net/http2: pool deadlock

16581 x/net/http2: rename and export Transport's t1 field

31170 x/net/http2: sendWindowUpdate may send invalid window size increment

25117 x/net/http2: steady performance degeneration seen with HTTP/2 over recent releases

15592 x/net/http2: toggle HPACK dynamic table indexing for header

29125 x/net/http: PROTOCOL_ERROR with HTTP2

19603 x/net/idna: support label separators other than ASCII dot

33508 x/sys/cpu: add support for ARM

30239 x/sys/cpu: report core information and speed

33963 x/sys/cpu: respect CPU feature overrides specified in GODEBUG

9055 x/text/unicode/norm: LastBoundary is inconsistent

andybons commented 4 years ago

Yep. The logic for this is https://github.com/golang/build/blob/master/cmd/gopherbot/gopherbot.go#L835

Edit: we explicitly leave out vendored subrepos: https://github.com/golang/build/blob/master/cmd/gopherbot/gopherbot.go#L851 but the above issues don’t meet the initial requirements to be excluded, so they can bucketed in “Unreleased” then never moved out.

@dmitshur @cagedmantis @toothrot

dmitshur commented 4 years ago

Issue #32441 was unfortunate in that it both had a "x/" prefix, yet it didn't match a real package "x/net/http2" that exists and is vendored.

One way to make that task of adding "Unreleased" milestone less likely to be applied on the wrong issues, without also causing more work for humans doing the issue triage, is if we start (automatically) tracking directories that exist in golang.org/x repos and packages that are vendored in the past 2 releases. (For an example of this being done, see the source code behind the https://goissues.org/-/packages page.)

Then it becomes possible to make it only add the "Unreleased" milestone if a the issue prefix matches a real package that exists, and that package isn't vendored.

This would prevent the milestone from being incorrectly added when an issue mistitled like "x/net/http" (instead of "net/http"). But it wouldn't help if it's mistitled like "x/image/draw" (instead of "image/draw") because golang.org/x/image/draw is a package that exists.

dmitshur commented 3 years ago

I have long term plans to work on this, but it won't happen in the Go 1.17 timeline, so moving this issue to Unreleased milestone which is appropriate.