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.

Sign up for GitHub

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

Jump to bottom

net: remove dependency on unicode #30440

Closed
bradfitz opened this issue Feb 27, 2019 · 7 comments
Closed

net: remove dependency on unicode #30440

bradfitz opened this issue Feb 27, 2019 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.13

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Feb 27, 2019 •
edited

An unfortunate dependency on unicode has crept into the net package via context -> reflect, introduced mostly in mid 2017 in 538b3a5.

Make context depend on reflectlite instead and fix deps_test.go to enforce that net can't depend on unicode.
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 27, 2019
@bradfitz bradfitz added this to the Go1.13 milestone Feb 27, 2019
@fraenkel
Copy link
Contributor

How much of the net sub-packages should depend on reflectlite? net/rpc and net/http are doable.

@bradfitz
Copy link
Contributor Author

net/http already depends on the world. That's not a goal. And net/rpc is also pretty high level.

@gopherbot
Copy link

Change https://golang.org/cl/164239 mentions this issue: net: remove dependency on unicode

gopherbot pushed a commit that referenced this issue Mar 25, 2019
@fraenkel @bradfitz
context: remove dependency on reflect
88adc33 
Make context depend on reflectlite instead of reflect in effort to
eventually make net no longer depend on unicode tables.

With this CL we're down to just:

    net -> context -> fmt -> unicode tables

The next CL can remove context -> fmt.

Updates #30440

Change-Id: I7f5df15f975d9dc862c59aa8477c1cfd6ff4967e
Reviewed-on: https://go-review.googlesource.com/c/go/+/164239
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/169080 mentions this issue: context: don't depend on fmt

@gopherbot
Copy link

Change https://golang.org/cl/169137 mentions this issue: sort, internal/reflectlite: flesh out reflectlite enough for use by sort

gopherbot pushed a commit that referenced this issue Mar 27, 2019
@bradfitz
context: don't depend on fmt
724a86f 
So the net package doesn't indirectly depend on unicode tables.

But we're still not quite there, because a new test added in this CL
reveals that we still have a path to unicode via:

deps_test.go:570:
  TODO(issue 30440): policy violation: net => sort => reflect => unicode

Updates #30440

Change-Id: I710c2061dfbaa8e866c92e6c824bd8df35784165
Reviewed-on: https://go-review.googlesource.com/c/go/+/169080
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@bradfitz
Copy link
Contributor Author

With the CLs above, we're not all the way back to Go 1.4 sizes, but much of the way:

bradfitz@go:~$ cat net.go
package main

import "net"

func main() {
        c, err := net.Dial("tcp", "google.com:80")
        if err != nil {
                panic(err)
        }
        c.Close()
}

bradfitz@go:~$ ls -l net1*
-rwxr-xr-x 1 bradfitz bradfitz 1977408 Mar 27 04:50 net1.4
-rwxr-xr-x 1 bradfitz bradfitz 2726908 Mar 27 04:51 net1.9
-rwxr-xr-x 1 bradfitz bradfitz 2805218 Mar 27 04:50 net1.11
-rwxr-xr-x 1 bradfitz bradfitz 2286223 Mar 27 04:51 net1.13-pre

@dmitshur dmitshur mentioned this issue Apr 23, 2019
Closed
@gopherbot
Copy link

Change https://golang.org/cl/174525 mentions this issue: sort: remove confusing source file

gopherbot pushed a commit that referenced this issue May 2, 2019
@rsc @bradfitz
sort: simplify bootstrap
0a338f7 
We compile package sort as part of the compiler bootstrap,
to make sure the compiler uses a consistent sort algorithm
no matter what version of Go it is compiled against.
(This matters for elements that compare "equal" but are distinguishable.)

Package sort was compiled in such a way as to disallow
sort.Slice entirely during bootstrap (at least with some compilers),
while cmd/internal/obj was compiled in such a way as to
make obj.SortSlice available to all compilers, precisely because
sort.Slice was not. This is all highly confusing.
Simplify by making sort.Slice available all the time.

Followup to CL 169137 and #30440
(and also CL 40114 and CL 73951).

Change-Id: I127f4e02d6c71392805d256c3a90ef7c51f9ba0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/174525
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Apr 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
3 participants
@bradfitz @fraenkel @gopherbot