Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#20023 closed task (fixed)

Upgrade Go to 1.7.3

Reported by: dcf Owned by: dcf
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-gitian TorBrowserTeam201611R
Cc: brade, mcs, gk Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Blog post: https://blog.golang.org/go1.7 (2016-08-15).

We are on 1.6.3 now.

Past tickets: #18333 1.6.2, #19703 1.6.3

Child Tickets

Attachments (4)

Change History (20)

comment:1 Changed 3 years ago by dcf

Summary: Upgrade Go to 1.7Upgrade Go to 1.7.1

comment:2 Changed 3 years ago by dcf

Go 1.7.1 is out now, released 2016-09-07: https://golang.org/doc/devel/release.html#go1.7.minor

I'm testing a build of the attached patches now. If they work, I'll mark this ticket needs_review.

According to comment:26:ticket:20250, we're going to need to use Go 1.7 or later even for the stable builds, for the sake of compatibility with macOS Sierra. There's no release of the 1.4 series or the 1.6 series that has the necessary fix (see GH#16352 and GH#17234). That's why the maint-6.0 patch is bigger than the master patch, because it also includes upgrades that were intentionally left out of maint-6.0 in #19703.

#20290 (upgrade meek to 0.24) should be merged before this ticket, since that version of meek fixes a problem that only arises when it is built with Go 1.6 or later (#20030).

comment:3 Changed 3 years ago by mcs

Cc: brade mcs added

comment:4 Changed 3 years ago by gk

Cc: gk added

comment:5 in reply to:  2 Changed 3 years ago by dcf

Replying to dcf:

I'm testing a build of the attached patches now. If they work, I'll mark this ticket needs_review.

The build failed for me, during the building of go1.7.1 in the mac pluggable-transports descriptor, in the make.bash command. I haven't figured out what went wrong yet. The exit status was 2. The last few lines of output were

cmd/compile/internal/x390x
cmd/compile/internal/x86
cmd/yacc
cmd/compile

comment:6 Changed 3 years ago by dcf

This is going to be a little more work than I expected. Some cgo components have changed in 1.7 in ways that affect the build.

The build error in comment:5 was caused by this:

# crypto/x509
crypto/x509/root_cgo_darwin.go: In function 'FetchPEMRoots':
crypto/x509/root_cgo_darwin.go:97: error: 'for' loop initial declaration used outside C99 mode
crypto/x509/root_cgo_darwin.go:106: error: 'for' loop initial declaration used outside C99 mode

MacPorts has a ticket with the same error: https://trac.macports.org/ticket/52506
I think it's caused by the old gcc we use in the mac descriptor.

If I add -std=gnu99 to CFLAGS, the build gets a little farther, but then fails at:

crypto/x509/root_cgo_darwin.go: In function 'FetchPEMRoots':
crypto/x509/root_cgo_darwin.go:114: warning: implicit declaration of function 'SecCertificateCopyNormalizedSubjectContent'
crypto/x509/root_cgo_darwin.go:114: warning: initialization makes pointer from integer without a cast
crypto/x509/root_cgo_darwin.go:119: warning: implicit declaration of function 'SecCertificateCopyNormalizedIssuerContent'
crypto/x509/root_cgo_darwin.go:119: warning: initialization makes pointer from integer without a cast
...
Undefined symbols for architecture i386:
  "_SecCertificateCopyNormalizedSubjectContent", referenced from:
      _FetchPEMRoots in root_cgo_darwin.cgo2.o
  "_SecCertificateCopyNormalizedIssuerContent", referenced from:
      _FetchPEMRoots in root_cgo_darwin.cgo2.o
ld: symbol(s) not found for architecture i386

MacPort has a ticket for this error too: https://trac.macports.org/ticket/52036

Both of these errors are caused by the same piece of new C code in 1.7: https://golang.org/cl/20351

The Apple docs say that SecCertificateCopyNormalizedIssuerContent requires SDK 1.7 or later. My plan is to next try building with the 10.7 SDK rather than 10.6.

Potentially related golang tickets:

https://github.com/golang/go/issues/14514
https://github.com/golang/go/issues/16473
https://github.com/golang/go/issues/16508

comment:7 in reply to:  6 Changed 3 years ago by gk

Replying to dcf:

This is going to be a little more work than I expected. Some cgo components have changed in 1.7 in ways that affect the build.

The build error in comment:5 was caused by this:

# crypto/x509
crypto/x509/root_cgo_darwin.go: In function 'FetchPEMRoots':
crypto/x509/root_cgo_darwin.go:97: error: 'for' loop initial declaration used outside C99 mode
crypto/x509/root_cgo_darwin.go:106: error: 'for' loop initial declaration used outside C99 mode

MacPorts has a ticket with the same error: https://trac.macports.org/ticket/52506
I think it's caused by the old gcc we use in the mac descriptor.

FWIW: if it's easier than fixing the old toolchain related issues then it might be a good time to switch to the newer one we use for the Firefox part. I am doing that for Tor in #20184 for unrelated reasons.

comment:8 Changed 3 years ago by dcf

Summary: Upgrade Go to 1.7.1Upgrade Go to 1.7.3

I got it building using the 10.7 SDK. But I also had to patch one of the source files (root_cgo_darwin.go, the source of the problematic new code) in order to remove what appears to be contradictory CFLAGS.

#cgo CFLAGS: -mmacosx-version-min=10.6 -D__MAC_OS_X_VERSION_MAX_ALLOWED=1060

These flags seems to say that the 10.6 SDK is required. But the SecCertificateCopyNormalizedSubjectContent and SecCertificateCopyNormalizedIssuerContent functions are first available in the 10.7 SDK. Trying to compile with these flags says

crypto/x509/root_cgo_darwin.go:114: error: 'SecCertificateCopyNormalizedSubjectContent' is unavailable (declared at /System/Library/Frameworks/Security.framework/Headers/SecCertificate.h:460)
crypto/x509/root_cgo_darwin.go:119: error: 'SecCertificateCopyNormalizedIssuerContent' is unavailable (declared at /System/Library/Frameworks/Security.framework/Headers/SecCertificate.h:443)

even though the functions are present in the SDK, presumably because the build thinks it's on 10.6 because of the CFLAGS. In the 10.7 SDK, these two functions are annotated with __OSX_AVAILABLE_STARTING(__MAC_10_7, __IPHONE_NA), which is the source of the "unavailable" message.

The CFLAGS seem like an error to me. I'm planning to file an issue in go and ask the developers' perspective.

Simply deleting -D__MAC_OS_X_VERSION_MAX_ALLOWED=1060 allows the build to finish, so that's what I'm going to go ahead with. The other alternative, I suppose, is to start using the 10.8 SDK. (Although I don't see how it could compile even under 10.8, unless the availability annotations are removed in the 10.8 SDK.)

Changed 3 years ago by dcf

Changed 3 years ago by dcf

comment:9 Changed 3 years ago by dcf

Keywords: TorBrowserTeam201611R added
Status: newneeds_review

Here are patches for review. On the maint-6.0 branch, you will need to apply both:

On the master branch, you only need:

For the SDK version issue in comment:8, I opened an upstream issue: https://github.com/golang/go/issues/17732. Maybe they can shed some light on whether the problematic CFLAGS are intended. In the patches above, my workaround is just to patch one file:

sed -i -e '/^#cgo CFLAGS:/s/-D__MAC_OS_X_VERSION_MAX_ALLOWED=1060//' crypto/x509/root_cgo_darwin.go

comment:10 Changed 3 years ago by gk

OKay, I applied the patch to master and hardened-builds (commits 824043e9b60c322c4c95b91478acadd0412a2e6d and a10116149af502d483bea0a347a880eb2c7faf42). As a note: removing the $FLAGS is probably problematic for FTE but that is not built currently due to #18495. Thus, we can adjust those flags if needed once that bug is sorted out.

This should be available in the next alpha release. I am still torn about taking this into stable, though. Especially as other PTs are potentially affected by any Go related issues that could come up.

comment:11 Changed 3 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

I put the fix into maint-6.0 as well with commits 3dee56db1330daa8df7e4d0c90d88bff17b80fec and 4a0bf1ed2578e3e742e00f95385f914a241904fd.

comment:12 Changed 3 years ago by gk

Resolution: fixed
Status: closedreopened

At least the stable build is busted now (I expect the alpha builds as well):

+ cd go/src
+ ./make.bash
##### Building Go bootstrap tool.
cmd/dist

##### Building Go toolchain using /home/debian/build/go1.4.
bootstrap/internal/sys
bootstrap/asm/internal/flags
bootstrap/internal/bio
bootstrap/compile/internal/big
bootstrap/internal/gcprog
bootstrap/internal/obj
bootstrap/internal/obj/arm
bootstrap/internal/obj/arm64
bootstrap/internal/obj/mips
bootstrap/internal/obj/ppc64
bootstrap/internal/obj/s390x
bootstrap/internal/obj/x86
bootstrap/asm/internal/lex
bootstrap/link/internal/ld
bootstrap/asm/internal/arch
bootstrap/compile/internal/ssa
bootstrap/asm/internal/asm
bootstrap/asm
bootstrap/link/internal/amd64
bootstrap/link/internal/arm
bootstrap/link/internal/arm64
bootstrap/link/internal/mips64
bootstrap/link/internal/ppc64
bootstrap/link/internal/s390x
bootstrap/link/internal/x86
bootstrap/link
bootstrap/compile/internal/gc
go build bootstrap/compile/internal/gc: /home/debian/build/go1.4/pkg/tool/linux_386/8g: signal: segmentation fault
go tool dist: FAILED: /home/debian/build/go1.4/bin/go install -gcflags=-l -v bootstrap/...: exit status 1

I am going to back that out from maint-6.0, alas, to get the 6.0.6 build going.

Last edited 3 years ago by gk (previous) (diff)

comment:13 Changed 3 years ago by boklm

Did you get this error in the Linux, Windows or OS X part?

In my case, the build of the pluggable transports finished for linux32 and linux64 without error (I stopped the build after that to restart it on the new tag).

comment:14 in reply to:  12 ; Changed 3 years ago by dcf

Replying to gk:

At least the stable build is busted now (I expect the alpha builds as well):

That's surprising. Sorry about that. I'm going to start a build now from tbb-6.0.6-build3 and try to reproduce.

I'm continually getting this error from make prep when trying to build lately. I'm working around it by setting VERIFY_TAGS=0.

gpg: keybox '/tmp/tmp.fWwPYe385T/pubring.kbx' created
gpg: Signature made Thu 11 Feb 2016 01:51:24 AM PST
gpg:                using RSA key 733018FA13A3DE49
gpg: Can't check signature: No public key
tbb-windows-installer: verification of tag v0.3.1 against /home/david/branches/tor-browser-bundle/gitian/gpg/tbb-windows-installer.gpg failed!

comment:15 in reply to:  14 Changed 3 years ago by gk

Resolution: fixed
Status: reopenedclosed

Replying to dcf:

Replying to gk:

At least the stable build is busted now (I expect the alpha builds as well):

That's surprising. Sorry about that. I'm going to start a build now from tbb-6.0.6-build3 and try to reproduce.

I'm continually getting this error from make prep when trying to build lately. I'm working around it by setting VERIFY_TAGS=0.

gpg: keybox '/tmp/tmp.fWwPYe385T/pubring.kbx' created
gpg: Signature made Thu 11 Feb 2016 01:51:24 AM PST
gpg:                using RSA key 733018FA13A3DE49
gpg: Can't check signature: No public key
tbb-windows-installer: verification of tag v0.3.1 against /home/david/branches/tor-browser-bundle/gitian/gpg/tbb-windows-installer.gpg failed!

That is surprising, too. :) Could you open a a ticket, mention what OS you are using and Cc boklm?

Sorry, for the noise actually. It seems waiting a couple of hours "fixed" the problem. Not sure what happened. I made sure to have enough disk space and no qemu process was running before I started the build. It is working for me now and I have reverted the reverted Go related commits for 6.0.6.

comment:16 in reply to:  14 Changed 3 years ago by dcf

Replying to dcf:

Replying to gk:

At least the stable build is busted now (I expect the alpha builds as well):

That's surprising. Sorry about that. I'm going to start a build now from tbb-6.0.6-build3 and try to reproduce.

FWIW, my fresh build of 6.0.6-build3 succeeded for all platforms.

Note: See TracTickets for help on using tickets.