Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: accepts std/go/token as valid package? #31769

Closed
bradfitz opened this issue Apr 30, 2019 · 4 comments
Closed

cmd/compile: accepts std/go/token as valid package? #31769

bradfitz opened this issue Apr 30, 2019 · 4 comments
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bradfitz
Copy link
Contributor

goimports inserted a "std/go/token" package for me, which kinda works in some places (go test) but fails in others (trybots of

diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 857f0d5928..e5c9f7e37b 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -35,6 +35,7 @@ import (
        "os"
        "reflect"
        "runtime"
+       "std/go/token"
        "strconv"
        "strings"
        "sync"
@@ -5220,3 +5221,45 @@ func TestTransportRequestWriteRoundTrip(t *testing.T) {
                })
        }
 }
+
+func TestTransportClone(t *testing.T) {
+       tr := &Transport{
+               Proxy:                  func(*Request) (*url.URL, error) { panic("") },
+               DialContext:            func(ctx context.Context, network, addr string) (net.Conn, error) { panic("") },
+               Dial:                   func(network, addr string) (net.Conn, error) { panic("") },
+               DialTLS:                func(network, addr string) (net.Conn, error) { panic("") },
+               TLSClientConfig:        ne)w(tls.Config),
+               TLSHandshakeTimeout:    time.Second,
+               DisableKeepAlives:      true,
+               DisableCompression:     true,
+               MaxIdleConns:           1,
+               MaxIdleConnsPerHost:    1,
+               MaxConnsPerHost:        1,
+               IdleConnTimeout:        time.Second,
+               ResponseHeaderTimeout:  time.Second,
+               ExpectContinueTimeout:  time.Second,
+               ProxyConnectHeader:     Header{},
+               MaxResponseHeaderBytes: 1,
+               ForceAttemptHTTP2:      true,
+               TLSNextProto:           map[string]func(authority string, c *tls.Conn) RoundTripper{},
+       }
+       tr2 := tr.Clone()
+       rv := reflect.ValueOf(tr2).Elem()
+       rt := rv.Type()
+       for i := 0; i < rt.NumField(); i++ {
+               sf := rt.Field(i)
+               if !token.IsExported(sf.Name) {
+                       continue
+               }
+               if rv.Field(i).IsZero() {
+                       t.Errorf("cloned field t2.%s is zero", sf.Name)
+               }
+       }
+
+       // But test that a nil TLSNextProto is kept nil:
+       tr = new(Transport)
+       tr2 = tr.Clone()
+       if tr2.TLSNextProto != nil {
+               t.Errorf("Transport.TLSNextProto unexpected non-nil")
+       }
+}
bradfitz@go:~/go/src$ cd net/http/
bradfitz@go:~/go/src/net/http$ go test -short
PASS
ok      net/http        4.134s
@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 30, 2019
@bradfitz bradfitz added this to the Go1.13 milestone Apr 30, 2019
@dmitshur

This comment has been minimized.

@bradfitz
Copy link
Contributor Author

bradfitz commented May 6, 2019

Note that it only works under $GOROOT/src, so it's not a release blocker. It only affects the standard library.

@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label May 6, 2019
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 6, 2019
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@odeke-em
Copy link
Member

odeke-em commented Oct 17, 2019

Hey @bradfitz, am currently on Go tip 943df4f and when I try to make this reproduction I get

With GO111MODULES=on

$ go test -short
# net/http
package net/http_test
	imports std/go/token: cannot find package "." in:
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/std/go/token
FAIL	net/http [setup failed]

With GO111MODULES=off

$ GO111MODULE=off go test -short
# net/http
package net/http_test
	imports std/go/token: cannot find package "std/go/token" in any of:
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/vendor/std/go/token (vendor tree)
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/vendor/std/go/token
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/std/go/token (from $GOROOT)
	/Users/emmanuelodeke/go/src/std/go/token (from $GOPATH)
FAIL	net/http [setup failed]

Could you please try again and see if this is still an issue?

@bradfitz
Copy link
Contributor Author

Great, sounds like it's fixed then. Thanks for trying!

@golang golang locked and limited conversation to collaborators Oct 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants