Opened 12 months ago

Closed 12 months ago

Last modified 10 months ago

#28002 closed defect (fixed)

The precomplete file in en-US Windows installer is incorrect

Reported by: boklm Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-rbm, TorBrowserTeam201810R, tbb-backported
Cc: mcs, brade Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In projects/tor-browser/build, we regenerate the precomplete file for the en-US bundle, in the $PKG_DIR/Browser directory:

cp -a ${TB_STAGE_DIR} $distdir/$PKG_DIR

[% IF c("var/windows") %]
  TBDIR="$distdir/$PKG_DIR/Tor Browser/Browser"
[% ELSIF c("var/osx") %]
  TBDIR="$distdir/$PKG_DIR/Tor Browser.app"
[% ELSE %]
  TBDIR="$distdir/$PKG_DIR/Browser"
[% END %]

pushd "$TBDIR[% IF c("var/osx") %]/Contents/Resources/[% END %]"
rm -f precomplete
python $MARTOOLS/createprecomplete.py
popd

However we generate the installer from the ${TB_STAGE_DIR} directory, which does not have the updated precomplete file:

[% ELSIF c("var/windows") %]
  find "${TB_STAGE_DIR}" -exec [% c("var/touch") %] {} \;
  pushd "${TB_STAGE_DIR}"
  makensis torbrowser.nsi
  # Working around NSIS braindamage
  mv torbrowser-install.exe torbrowser-install-tmp.exe
  python $rootdir/pe_checksum_fix.py
  mv torbrowser-install-tmp2.exe torbrowser-install.exe
  rm torbrowser-install-tmp.exe
  mv torbrowser-install.exe $OUTDIR/torbrowser-install[% IF c("var/windows-x86_64") %]-win64[% END %]-[% c("var/torbrowser_version") %]_${PKG_LOCALE}.exe
  popd
[% END %]
rm -rf $distdir/${PKG_DIR}

The mar files are generated using the $TBDIR directory, so they are using the correct precomplete file. The installer for the other locales are also generated in the correct directory, so the wrong precomplete file is only included in the en-US installer.

Child Tickets

Change History (7)

comment:1 Changed 12 months ago by boklm

Keywords: TorBrowserTeam201810R added; TorBrowserTeam201810 removed
Status: newneeds_review

comment:3 Changed 12 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Looks good to me. Applying to master (3fad7f22441cd2e106a2d5bd8ba7e4623fac409a). boklm, what's the impact of this bug? It seems neither partial nor full updates are failing?

comment:4 in reply to:  3 ; Changed 12 months ago by boklm

Cc: mcs brade added

Replying to gk:

Looks good to me. Applying to master (3fad7f22441cd2e106a2d5bd8ba7e4623fac409a). boklm, what's the impact of this bug? It seems neither partial nor full updates are failing?

I was expecting the first partial update to fail, as the precomplete file installed is different from the one included in the mar file used to generate the incremental, so patching that file would fail. But if I understand the check_for_forced_update function correctly in tools/update-packaging/make_incremental_update.sh, the precomplete file is always updated as a full update instead of a patch. So this bug should have no impact.

comment:5 in reply to:  4 Changed 12 months ago by mcs

Replying to boklm:

I was expecting the first partial update to fail, as the precomplete file installed is different from the one included in the mar file used to generate the incremental, so patching that file would fail. But if I understand the check_for_forced_update function correctly in tools/update-packaging/make_incremental_update.sh, the precomplete file is always updated as a full update instead of a patch. So this bug should have no impact.

That is correct. There might be a small impact during full updates since the contents of the precomplete file are used to remove old files, e.g., if a file is removed from the browser package between version 8.0.2 and 8.0.3 and a full update is used to make the update, precomplete is used to remove all of the old files. This avoids old, outdated files from remaining after an update. That said, I am not sure what was missing from the precomplete file we used to ship (before this ticket was fixed).

comment:6 in reply to:  4 ; Changed 10 months ago by boklm

Replying to boklm:

So this bug should have no impact.

Actually it has no impact when all bundles are generated sequentially. With the changes from #27218 however, we generate the en-US bundles and all the other locales in parallel, which means that we modify the content of ${TB_STAGE_DIR} while this directory can potentially be copied to start the generation of an other locale.

comment:7 in reply to:  6 Changed 10 months ago by gk

Keywords: tbb-backported added

Replying to boklm:

Replying to boklm:

So this bug should have no impact.

Actually it has no impact when all bundles are generated sequentially. With the changes from #27218 however, we generate the en-US bundles and all the other locales in parallel, which means that we modify the content of ${TB_STAGE_DIR} while this directory can potentially be copied to start the generation of an other locale.

Backported for maint-8.0 (commit 5c41fd90b0badbb79e37441bfd7a2a19ff42d0ca).

Note: See TracTickets for help on using tickets.