tailscale / go

Tailscale's rolling temporary fork of Go with emphasis on performance and memory/binary size reduction. We try to upstream everything and keep this repo's diff small.
https://github.com/tailscale/go/commits
BSD 3-Clause "New" or "Revised" License
66 stars 6 forks source link

Update go1.23 #95

Closed patrickod closed 2 months ago

patrickod commented 3 months ago

This PR will be imported into Gerrit with the title and first comment (this text) used to generate the subject and body of the Gerrit change.

Please ensure you adhere to every item in this list.

More info can be found at https://github.com/golang/go/wiki/CommitMessage

bradfitz commented 3 months ago

Thanks! I'll review this this evening (outside of GitHub)

awly commented 3 months ago

git diff go1.23.0...origin/update-go1.23 and git diff go1.22.5..origin/tailscale.go1.22 look a bit different, maybe @bradfitz upstreamed some of our internal changes?

bradfitz commented 3 months ago

Per https://github.com/tailscale/go/issues labels, I only upstreamed one of them.

bradfitz commented 3 months ago

Historically we haven't merged one Go release branch into another release branch, but I guess that works. What we did previously was start with a merge commit between our old branch (but not taking any of its contents; just as a git commit parent to keep history) and the Go upstream, and then cherry-pick our handful of changes atop. I think we were doing this because it made git merges for patch releases less painful later? But I forget.

In any case, this style is also fine.

But what are the changes to encoding/gob in two files I see in git diff go/release-branch.go1.23 update-go1.23?

bradfitz@book1pro go % git diff update-go1.23 go/release-branch.go1.23 -- src/encoding/gob
diff --git a/src/encoding/gob/encode.go b/src/encoding/gob/encode.go
index c83071c717..5f4d2539fa 100644
--- a/src/encoding/gob/encode.go
+++ b/src/encoding/gob/encode.go
@@ -601,7 +601,7 @@ func compileEnc(ut *userTypeInfo, building map[*typeInfo]bool) *encEngine {
        if ut.externalEnc == 0 && srt.Kind() == reflect.Struct {
                for fieldNum, wireFieldNum := 0, 0; fieldNum < srt.NumField(); fieldNum++ {
                        f := srt.Field(fieldNum)
-                       if !isSent(srt, &f) {
+                       if !isSent(&f) {
                                continue
                        }
                        op, indir := encOpFor(f.Type, seen, building)
diff --git a/src/encoding/gob/type.go b/src/encoding/gob/type.go
index 13e41728a7..c3ac1dbd61 100644
--- a/src/encoding/gob/type.go
+++ b/src/encoding/gob/type.go
@@ -538,7 +538,7 @@ func newTypeObject(name string, ut *userTypeInfo, rt reflect.Type) (gobType, err
                idToTypeSlice[st.id()] = st
                for i := 0; i < t.NumField(); i++ {
                        f := t.Field(i)
-                       if !isSent(t, &f) {
+                       if !isSent(&f) {
                                continue
                        }
                        typ := userType(f.Type).base
@@ -576,7 +576,7 @@ func isExported(name string) bool {
 // isSent reports whether this struct field is to be transmitted.
 // It will be transmitted only if it is exported and not a chan or func field
 // or pointer to chan or func.
-func isSent(struct_ reflect.Type, field *reflect.StructField) bool {
+func isSent(field *reflect.StructField) bool {
        if !isExported(field.Name) {
                return false
        }
awly commented 2 months ago

Historically we haven't merged one Go release branch into another release branch, but I guess that works. What we did previously was start with a merge commit between our old branch (but not taking any of its contents; just as a git commit parent to keep history) and the Go upstream, and then cherry-pick our handful of changes atop. I think we were doing this because it made git merges for patch releases less painful later? But I forget.

@bradfitz do you want to take a stab and documenting how to do this in https://github.com/tailscale/corp/blob/main/engdoc/updating-toolchain.md? I don't fully understand all the steps you describe.