Opened 6 weeks ago

Closed 4 days ago

#26251 closed task (fixed)

Adapt macOS snowflake compilation to new toolchain

Reported by: gk Owned by: tbb-team, sukhbir
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-rbm, TorBrowserTeam201807R
Cc: dcf, arlolra, sukhbir Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

We need to adapt our macOS toolchain due to new Firefox requirements when switching to ESR 60 (the new one will use a different cctools, clang 3.9.1 and the SDK 10.11). This means the snowflake and related projects need to get updated, too.

I'll disable building and shipping snowflake until this bug is fixed (alas, it's not a top prio right now for us) in order to get macOS nightlies based on ESR 60 going as fast as possible.

Child Tickets

Change History (13)

comment:1 Changed 6 weeks ago by gk

bug_26195 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/log/?h=bug_26195) has three commits that build the new toolchain and could be a good start for this bug. I am not done preparing the patches for all the projects yet, but I can build the whole mac bundle with it (modulo the snowflake parts) if I did not miss some project (I did not attempt to build the whole bundle straight yet).

comment:2 Changed 5 weeks ago by gk

Cc: sukhbir added

comment:3 Changed 4 weeks ago by sukhbir

Owner: changed from tbb-team to tbb-team, sukhbir
Status: newassigned

comment:4 Changed 4 weeks ago by sukhbir

Status: assignedneeds_review

Please review:

https://github.com/azadi/tor-browser-build-1/tree/bug-26251

I have not properly patched build/config/mac/BUILD.gn in webrtc-mac.patch as you may notice and just added the -Wno-unknown-warning-option manually. I can do that in rev2 or before finalizing the patch, but I wanted to put it out for review before I deal with gclient.

I tested it on macOS and by modifying torrc (thanks to arlolra for the tip) as PT selection is missing pending #26039.

https://github.com/keroserene/snowflake/blob/master/client/torrc

Tor NOTICE: Bootstrapped 5%: Connecting to directory server 
Tor NOTICE: Bootstrapped 10%: Finishing handshake with directory server 
Tor NOTICE: Learned fingerprint <> for bridge 0.0.3.0:1 (with transport 'snowflake'). 
Tor NOTICE: Bootstrapped 15%: Establishing an encrypted directory connection 
Tor NOTICE: Bootstrapped 20%: Asking for networkstatus consensus 
Tor NOTICE: new bridge descriptor <>
Tor NOTICE: Bootstrapped 25%: Loading networkstatus consensus 
Tor NOTICE: I learned some more directory information, but not enough to build a circuit: We have no usable consensus. 
Tor NOTICE: Bootstrapped 40%: Loading authority key certs 
Tor NOTICE: Bootstrapped 45%: Asking for relay descriptors 
...
Tor NOTICE: Bootstrapped 50%: Loading relay descriptors 
Tor NOTICE: Bootstrapped 57%: Loading relay descriptors 
Tor NOTICE: Bootstrapped 66%: Loading relay descriptors 
Tor NOTICE: Bootstrapped 73%: Loading relay descriptors 
Tor NOTICE: Bootstrapped 80%: Connecting to the Tor network 
Tor NOTICE: Bootstrapped 90%: Establishing a Tor circuit 
Tor NOTICE: Tor has successfully opened a circuit. Looks like client functionality is working. 
Last edited 4 weeks ago by sukhbir (previous) (diff)

comment:5 Changed 4 weeks ago by gk

Before I forget it: we should make sure snowflake is working on macOS < 10.11. IIRC there was a note somewhere in the snowflake build patch indicating that the SDK version used is at the same time the minimum macOS version supported. But that's not the case for us: we use the 10.11 SDK but want to have Tor Browser running on, say, 10.9 as well.

comment:6 in reply to:  5 Changed 4 weeks ago by sukhbir

Replying to gk:

Before I forget it: we should make sure snowflake is working on macOS < 10.11. IIRC there was a note somewhere in the snowflake build patch indicating that the SDK version used is at the same time the minimum macOS version supported. But that's not the case for us: we use the 10.11 SDK but want to have Tor Browser running on, say, 10.9 as well.

Good point and I think dcf or arlolra can comment better, but two things I did not change for this reason:

print("machine_os_build=\"10.7\"")

and

GN_ARGS+=" target_os=\"mac\" target_cpu=\"x64\" mac_deployment_target=\"10.7\""

But I tested it on 10.13.4 High Sierra so I am not sure.

EDIT: Version on which I tested.

Last edited 4 weeks ago by sukhbir (previous) (diff)

comment:7 in reply to:  4 Changed 4 weeks ago by gk

Status: needs_reviewneeds_revision

Replying to sukhbir:

Please review:

https://github.com/azadi/tor-browser-build-1/tree/bug-26251

I have not properly patched build/config/mac/BUILD.gn in webrtc-mac.patch as you may notice and just added the -Wno-unknown-warning-option manually. I can do that in rev2 or before finalizing the patch, but I wanted to put it out for review before I deal with gclient.

Does not look unreasonable -mlinker-version=136 and probably other flags are not needed anymore. You could compare the macOS mozconfig file used on the maint-7.5 and on the master branch for the diff. Then I am looking forward to the full patch with gclient etc.

comment:8 Changed 3 weeks ago by sukhbir

Status: needs_revisionneeds_review

For review:

https://github.com/azadi/tor-browser-build-1/tree/bug-26251-rev1

I removed -mlinker-version=136 but the other stuff seems to be relevant.

$ git diff tor/maint-7.5...tor/master  -- projects/firefox/mozconfig-osx-x86_64

-FLAGS="-target x86_64-apple-darwin10 -mlinker-version=136 -B $CROSS_CCTOOLS_PATH/bin -isysroot $CROSS_SYSROOT $HARDENING_FLAGS"
+FLAGS="-target x86_64-apple-darwin11 -B $CROSS_CCTOOLS_PATH/bin -isysroot $CROSS_SYSROOT $HARDENING_FLAGS"

-export TOOLCHAIN_PREFIX=$CROSS_CCTOOLS_PATH/bin/x86_64-apple-darwin10-
-#TODO: bug 1184202 - would be nice if these could be detected with TOOLCHAIN_PREFIX automatically
-export AR=${TOOLCHAIN_PREFIX}ar
-export RANLIB=${TOOLCHAIN_PREFIX}ranlib
-export STRIP=${TOOLCHAIN_PREFIX}strip
-export OTOOL=${TOOLCHAIN_PREFIX}otool
+export BINDGEN_CFLAGS="$FLAGS"
+export TOOLCHAIN_PREFIX=$CROSS_CCTOOLS_PATH/bin/x86_64-apple-darwin11-

I tested with the same setup as in #comment:4. As for as gclient, I didn't need to do that since I did an in-place of the patch. (We will end up with the same thing since I replaced the linker line with the clang flag, so I didn't bother.)

Last edited 3 weeks ago by sukhbir (previous) (diff)

comment:9 Changed 3 weeks ago by sukhbir

dcf: This commit makes a change to your patch. Can you please go through it once and see if the change is fine with you before it's merged? Since I modified the patch in-place, I don't want it to attribute to you without your permission.

comment:10 in reply to:  9 Changed 3 weeks ago by dcf

Replying to sukhbir:

dcf: This commit makes a change to your patch. Can you please go through it once and see if the change is fine with you before it's merged? Since I modified the patch in-place, I don't want it to attribute to you without your permission.

I'll take a look but generally I don't worry about the attribution--set it to yourself or to me, it doesn't matter much. Thanks for checking.

comment:11 Changed 3 weeks ago by gk

Keywords: TorBrowserTeam201806R added

comment:12 Changed 12 days ago by gk

Keywords: TorBrowserTeam201807R added; TorBrowserTeam201806R removed

Moving reviews to July.

comment:13 Changed 4 days ago by boklm

Resolution: fixed
Status: needs_reviewclosed

This looks good to me. I merged this to master with commit 82a0bd2d85fe53b61460a3e3b833f10a0bb01519.

Note: See TracTickets for help on using tickets.