Opened 6 weeks ago

Closed 6 weeks ago

Last modified 4 weeks ago

#22832 closed defect (fixed)

libwebrtc and snowflake are not being built reproducibly (unless you build in the same month)

Reported by: gk Owned by: dcf
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Major Keywords: tbb-gitian TorBrowserTeam201707R
Cc: boklm, dcf, arlolra Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

I already ran into this problem when building 7.0a4 but did not have time to investigate it back then: I did only a partial rebuild (not touching snowflake as this was not necessary) but boklm did a full build. The result was a different snowflake-client.

Now this happened with the build for 7.5a2 again. The diff is:

--- /dev/fd/63	2017-07-06 09:57:34.341983842 +0200
+++ /dev/fd/62	2017-07-06 09:57:34.341983842 +0200
@@ -750821,7 +750821,7 @@
 00b74e40: 6e6e 656c 3d25 642c 2025 6c75 2900 7468  nnel=%d, %lu).th
 00b74e50: 6973 203d 3d20 675f 746f 705f 6d61 6e61  is == g_top_mana
 00b74e60: 6765 7200 4368 6563 6b20 6661 696c 6564  ger.Check failed
-00b74e70: 3a20 6675 6e63 2e20 004a 756e 2030 3420  : func. .Jun 04 
+00b74e70: 3a20 6675 6e63 2e20 004a 756c 2030 3220  : func. .Jul 02 
 00b74e80: 3230 3137 2030 353a 3030 3a30 3000 4368  2017 05:00:00.Ch
 00b74e90: 6563 6b20 6661 696c 6564 3a20 6269 6e64  eck failed: bind
 00b74ea0: 5f73 7461 7465 5f2e 2000 2c3c 2a00 4368  _state_. .,<*.Ch

That seems to be a bit weird given the actual build date but boklm had the idea it could be the first Sunday of the month/week getting embedded.

And, indeed, as boklm pointed out /build/write_build_date_header.py seems to do such a thing.

I can trace the problem back to build_time.o in the libwebrtc .a file.

Child Tickets

Change History (8)

comment:1 Changed 6 weeks ago by gk

boklm pointed out that there is an override_build_date argument in base/BUILD.gn that might be definable in GN_ARGS.

comment:2 Changed 6 weeks ago by dcf

Owner: changed from tbb-team to dcf
Status: newassigned

comment:3 Changed 6 weeks ago by dcf

I searched for the date strings in a build I had lying around from #19001/#22831 (Snowflake for mac), and they weren't there. I think the difference is this commit which reduces the scope of .o files that get included into libwebrtc-magic.a. I discovered, while working on the mac descriptor, that we could reduce the number of .o files (because some of them are unit tests or build helpers). I did it for size reasons, but as a side effect it seems that it also excludes build_time.o. The commit I linked was just to make the linux descriptor uniform with the new mac descriptor.

Let me know if that commit works for you (feel free to edit as necessary). If we want to set override_build_date as well (even though it seems unnecessary it might be a good idea for future insurance), this seems to be the expected format (taken from build/write_build_date_header.py). I haven't tested it, but I'm starting a build to do so now.

GN_ARGS+=" override_build_date=\"$(date --utc --date="$REFERENCE_DATETIME" +"%b %d %Y %H:%M:%S")\""

comment:4 in reply to:  3 Changed 6 weeks ago by dcf

Replying to dcf:

Let me know if that commit works for you (feel free to edit as necessary). If we want to set override_build_date as well (even though it seems unnecessary it might be a good idea for future insurance), this seems to be the expected format (taken from build/write_build_date_header.py). I haven't tested it, but I'm starting a build to do so now.

GN_ARGS+=" override_build_date=\"$(date --utc --date="$REFERENCE_DATETIME" +"%b %d %Y %H:%M:%S")\""

I just tested, and setting override_build_date doesn't work. It results in this message in build.log:

+ /home/debian/build/webrtc/src/out_bootstrap/gn gen out/Release '--args= target_os="linux" target_cpu="x64" override_build_date="Jan 01 2000 00:00:00" is_debug=false treat_warnings_as_errors=false is_component_build=false is_clang=false use_sysroot=false gold_path="/home/debian/install/binutils/bin" use_ozone=true use_gconf=false rtc_include_opus=false rtc_include_ilbc=false rtc_include_internal_audio_device=false rtc_include_pulse_audio=false'
ERROR at the command-line "--args":1:57: Build argument has no effect.
 target_os="linux" target_cpu="x64" override_build_date="Jan 01 2000 00:00:00" is_debug=false treat_warnings_as_errors=false is_component_build=false is_clang=false use_sysroot=false gold_path="/home/debian/install/binutils/bin" use_ozone=true use_gconf=false rtc_include_opus=false rtc_include_ilbc=false rtc_include_internal_audio_device=false rtc_include_pulse_audio=false
                                                        ^---------------------
The variable "override_build_date" was set as a build argument
but never appeared in a declare_args() block in any buildfile.

To view all possible args, run "gn args --list <builddir>"
Done. Made 418 targets from 113 files in 438ms

Applying the patch from comment:3 still works to fix the embedded date issue.

What's going on is that base/build_time.o is only being built as a side effect of bootstrap/bootstrap.py, which builds gn, a build helper program. Formerly, we were linking more .o files into libwebrtc-magic.a than we needed, including the base/build_time.o that was part of gn, not part of webrtc. The reason I noticed this while working on the mac descriptor, was that because of cross compiling, some .o files (including those belonging to gn) were compiled for the host architecture (i.e. linux) and some (those belonging to webrtc) were compiled for the target architecture (i.e. mac). Linking them all together into one archive produced a non-working .a file, which is when I realized that we were including too many .o files.

comment:5 Changed 6 weeks ago by gk

I am fine with just not including build_time.o leaving override_build_date untouched. However, I like to see at least a comment pointing to this ticket in the linux and mac descriptor changes so that we don't forget that not including some .o files is partly due to the problem in this bug. dcf: could you make the necessary changes in a new branch for #22831 and I just cherry-pick the commit that fixes this issue?

comment:6 Changed 6 weeks ago by dcf

Keywords: TorBrowserTeam201707R added
Status: assignedneeds_review

I made a new snowflake-mac-5 branch and this commit is the one you want.

Including those extra .o files was always a bug. We just didn't notice until now, because they happened to be of a compatible architecture to the actual webrtc .o files. Not including the inappropriate .o files is also the reason why it's no longer necessary to have -not -name '*.main.o' -not -name 'dump_syms_regtest.o' in the find command.

comment:7 Changed 6 weeks ago by gk

Resolution: fixed
Status: needs_reviewclosed

Thanks. Cherry-picked to master as commit 781da76f120f849416c772cef45a9a9a103b4eef.

comment:8 Changed 4 weeks ago by boklm

In tor-browser-build.git, this is done in commit dac7649db2b8567c8ace185dcdac56edd098d55b.

Note: See TracTickets for help on using tickets.