Opened 4 years ago

Closed 4 years ago

#13776 closed defect (fixed)

Generation of incremental mar files is not reproducible

Reported by: gk Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Keywords: tbb-gitian, TorBrowserTeam201412, TorBrowserTeam201412R
Cc: boklm, mikeperry, mcs, brade Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Preparing the release for 4.5-alpha-1 it turns our that incremental mar files are not reproducible. Specifically, only the updatev2.manifest and updatev3.manifest files differ. Attached is the updatev2.manifest-diff using https://people.torproject.org/~mikeperry/builds/4.5-alpha-1/tor-browser-win32-4.0-4.5-alpha-1_ar.incremental.mar and https://people.torproject.org/~gk/misc/tor-browser-win32-4.0-4.5-alpha-1_ar.incremental.mar

Child Tickets

Attachments (3)

updatev2diff (18.1 KB) - added by gk 4 years ago.
updatev2.manifest diff
updatev2manifest.diff (14.4 KB) - added by boklm 4 years ago.
diff of uncompressed updatev2.manifest
402_inc_mar_diff (7.8 KB) - added by gk 4 years ago.
Diff between linux32 4.0-4.0.2 ar incremental mar files

Download all attachments as: .zip

Change History (15)

Changed 4 years ago by gk

Attachment: updatev2diff added

updatev2.manifest diff

Changed 4 years ago by boklm

Attachment: updatev2manifest.diff added

diff of uncompressed updatev2.manifest

comment:1 Changed 4 years ago by boklm

The updatev2.manifest and updatev3.manifest are compressed with bzip2. I attached a diff of the uncompressed updatev2.manifest files.

It looks like that both files contain the same lines, but some are not in the same order.

comment:2 Changed 4 years ago by mcs

The root cause of the problem appears to be a locale difference that causes case-insensitive sort for the mikeperry build (there may also be differences in how punctuation characters are handled). Almost certainly the correct fix is to set LC_ALL=C somewhere.

In the short run, maybe just try setting it before running the make command, e.g.,

LC_ALL=C make incrementals

The problem does not occur for full MARs because those are generating inside a VM.

A real fix would require adding LC_ALL=C inside the incrementals target within gitian/Makefile. Or we could modify Mozilla's scripts to set the locale (tools/update-packaging/make_incremental_update.sh and, for safety's sake, make_full_update.sh).

comment:3 Changed 4 years ago by mcs

mikeperry or gk: Did you confirm the locale theory? Kathy and I included

export LC_ALL=C

in the new gen-update-info.sh script that we wrote for #13379; since update_responses will be run from that script, hopefully this will fix the incremental MAR reproducibility problem.

comment:4 Changed 4 years ago by gk

Keywords: MikePerry201411R added

I got

58862712abef6c3fcffc8cd532bdc402af461d97a2f86a67c3ce34a4e02b0b33  4.5-alpha-1/sha256sums.incrementals.txt

after removing the incremental mar files, doing a export LC_ALL=C, make incrementals and make hash-alpha. Mike?

comment:5 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201411R added; MikePerry201411R removed

comment:6 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201412R added; TorBrowserTeam201411R removed

comment:7 Changed 4 years ago by gk

Keywords: TorBrowserTeam201412 added; TorBrowserTeam201412R removed
Status: newneeds_revision

Setting LC_ALL to C solves the difference of the manifest files. But the resulting incremental .mar files are still different for some reason. The only differences I see on the attachment are a4 -> b0.
I have not checked what these correspond to yet.

Changed 4 years ago by gk

Attachment: 402_inc_mar_diff added

Diff between linux32 4.0-4.0.2 ar incremental mar files

comment:8 in reply to:  7 ; Changed 4 years ago by mcs

Summary: Generation of incremental mar files is not reproducbileGeneration of incremental mar files is not reproducible

Replying to gk:

Setting LC_ALL to C solves the difference of the manifest files. But the resulting incremental .mar files are still different for some reason. The only differences I see on the attachment are a4 -> b0.

It looks all of the differences are in the bytes that immediately precede a file name within the MAR header. Looking at the code in modules/libmar/src/mar_create.c, that byte is a flags field that is derived from the file mode bits, like this:

st.st_mode & 0777

The difference is really 1A4 vs. 1B0 (0644 vs. 0660). So we probably just need to set the umask before running the make_incremental_update.sh script. Or we could avoid these kinds of issues by generating the incremental MARs inside a VM.

comment:9 in reply to:  8 Changed 4 years ago by gk

Replying to mcs:

The difference is really 1A4 vs. 1B0 (0644 vs. 0660). So we probably just need to set the umask before running the make_incremental_update.sh script. Or we could avoid these kinds of issues by generating the incremental MARs inside a VM.

Let's try setting umask directly before generating incremental MARs. Doing that inside a VM, too, might be a bit cumbersome given our current build process (there need to be e.g. builds from the alpha series copied around in order to generate the incremental MAR files for the stable series and vice versa etc.).

comment:10 Changed 4 years ago by mcs

Keywords: TorBrowserTeam201412R added
Status: needs_revisionneeds_review

Kathy and I created a fix that should avoid problems due to locale and umask differences. To avoid problems if someone invokes the update_responses program outside of a make target, we decided that setting those things should be done inside that script. Please review:

https://gitweb.torproject.org/user/brade/tor-browser-bundle.git/commit/?h=bug13776-01&id=93c7688e2eb2ef6cbef1a9e911586bd1a2e55ff7

comment:12 Changed 4 years ago by mcs

Resolution: fixed
Status: needs_reviewclosed

Thanks for the review. Merged into master as 5130f4ded3705e13f2d20fe49a502ec11e8d7cf2.

I am optimistically resolving this bug as fixed.

Note: See TracTickets for help on using tickets.