Bug #15041

Firefox gets uselessly downloaded at ISO build time when Debian security has a newer Firefox than our Tor Browser

Added by anonym 2017-12-11 19:38:39 . Updated 2018-08-16 13:00:03 .

Status:
Rejected
Priority:
Low
Assignee:
Category:
Build system
Target version:
Start date:
2017-12-11
Due date:
% Done:

0%

Feature Branch:
Type of work:
Research
Blueprint:

Starter:
Affected tool:
Browser
Deliverable for:

Description

At the moment, we install a Tor Browser 7.0.10 based on Firefox 52.5.0, so we generate a fake iceweasel (but should probably be firefox-esr theses days) with that version to satisfy the various Debian-packages extensions we install. However, now Tor Browser 7.0.11 based on Firefox 52.5.2 is out, but we skipped it, so we still generate iceweasel 52.5.0, which gets upgraded to (the real) 52.5.2 since that was uploaded to stable-security yesterday. In other words, if we have to release an emergency release before we import a new Tor Browser, we’ll ship a vanilla Firefox.

A possible solution is to look what the firefox-esr version is at build time, and just add a +fake. This should guarantee that this problem can not reoccur.

Details: config/chroot_local-hooks/10-tbb


Subtasks


History

#1 Updated by anonym 2017-12-11 19:39:19

#2 Updated by anonym 2017-12-11 19:43:20

  • Assignee set to intrigeri

#3 Updated by anonym 2017-12-11 19:55:19

  • Priority changed from High to Normal

Whoops. So far I had “only” looked at what the .build-manifest, .packages and .buildlog files said was installed, but when actually booting the ISO they’re not. And then, after looking a bit closer in the .buildlog, I now see that the upgrade of iceweasel happens after live-build’s “P: Begin copying chroot”, so this is just a waste of bandwidth and disk space, so it should still be fixed but clearly not at that prio.

#4 Updated by intrigeri 2017-12-12 10:46:20

> Whoops. So far I had “only” looked at what the .build-manifest, .packages and .buildlog files said was installed, but when actually booting the ISO they’re not. And then, after looking a bit closer in the .buildlog, I now see that the upgrade of iceweasel happens after live-build’s “P: Begin copying chroot”, so this is just a waste of bandwidth and disk space, so it should still be fixed but clearly not at that prio.

Right. Also “release an emergency release before we import a new Tor Browser” is quite unlikely (generally Tor Browser is updated before Debian security, and the vast majority of our emergency releases are precisely meant to upgrade Tor Browser). So my understanding is that this ticket is about a build performance optimization that’s only relevant in rather rare cases i.e. during the time when our major branches include a Tor Browser that’s based on an older Firefox version than the one in Debian security, and I don’t think it qualifies for Foundations Team work, and regardless I’m sure we could do higher impact work in the same time. I’ll update the ticket metadata accordingly.

#5 Updated by intrigeri 2017-12-12 10:46:48

  • blocked by deleted (Feature #13244: Core work 2017Q4: Foundations Team)

#6 Updated by intrigeri 2017-12-12 10:48:10

  • Subject changed from Our fake iceweasel package can be older than to Firefox gets uselessly downloaded at ISO build time when Debian security has a newer Firefox than our Tor Browser
  • Category set to Build system
  • Assignee deleted (intrigeri)
  • Target version deleted (Tails_3.5)

#7 Updated by anonym 2017-12-13 16:45:51

  • Assignee set to intrigeri
  • QA Check set to Info Needed

intrigeri wrote:
> Right. Also “release an emergency release before we import a new Tor Browser” is quite unlikely (generally Tor Browser is updated before Debian security, and the vast majority of our emergency releases are precisely meant to upgrade Tor Browser).

Agreed.

> So my understanding is that this ticket is about a build performance optimization that’s only relevant in rather rare cases i.e. during the time when our major branches include a Tor Browser that’s based on an older Firefox version than the one in Debian security, and I don’t think it qualifies for Foundations Team work, and regardless I’m sure we could do higher impact work in the same time. I’ll update the ticket metadata accordingly.

IMHO this is not a “build performance optimization”, but fixing a bug that causes unintentional work that luckily has no effect on the end product. I agree that premature optimization isn’t core work, but this isn’t as clear to me. I of course get the “we could do higher impact work” argument, and was about to accept it, but then I came up with another reason for solving this:

First, note that the .packages file lists the upgraded iceweasel version, so apparently packages installed after the chroot is copied end up there. This gave me some serious headache, so I think we should add a nice sanity check toward the end of chroot_local-hooks that checks that there are no upgradable packages — and that is why we need this ticket fixed too (otherwise, next time this happens we FTBFS). What do you think?

#8 Updated by intrigeri 2017-12-13 19:33:16

  • Assignee changed from intrigeri to anonym

> IMHO this is not a “build performance optimization”, but fixing a bug that causes unintentional work that luckily has no effect on the end product.

I see what you mean. Now, I care much more about bugs that directly affect our users (I’m not suggesting you don’t). Frankly, I’d rather see us think about why we broke uBlock in 3.3, and how we could prevent it, than on the problem at hand here.

> First, note that the .packages file lists the upgraded iceweasel version, so apparently packages installed after the chroot is copied end up there. This gave me some serious headache, so I think we should add a nice sanity check toward the end of chroot_local-hooks that checks that there are no upgradable packages — and that is why we need this ticket fixed too (otherwise, next time this happens we FTBFS). What do you think?

OK, if this problem caused headaches perhaps that’s a good reason to fix it! You’re suggesting there’s a bug in how we’re relying on live-build to generate our .packages file (this bug can affect other stuff we do in the chroot, by the way, not just Firefox). If we fixed it, then no more headaches, right? I’d rather see this bug fixed than a “nice sanity check” added to ensure we’re not putting ourselves in a situation that will trigger the bug (especially if then we’re going to workaround such situations with things like the +fake idea). I’m not convinced this sanity check is easier to implement than the actual bugfix, i.e. generate our .packages file at a time when it actually describes what we’re going to ship in the chroot.

This other bug seems to warrant a dedicated ticket, because if we fix it in a nice way, likely Bug #15041 goes back to being purely a build performance matter.

> A possible solution is to look what the firefox-esr version is at build time, and just add a +fake

I’m not a fan, this feels like a kludge with hard to control side-effects. It means we’re going to claim we’re shipping a version of Firefox potentially different from the one we’re actually shipping. Right now it does not matter, but as soon as Debian starts shipping the next ESR while we’re stuck on the old one for a few months, and possibly updated extensions drop support for the ESR52, then this may seriously matter, and then this fix may cause new problems. As we’ve noticed recently with uBlock, we’re not very good at ensuring we don’t break add-ons, so putting ourselves in an even more risky place on this front feels wrong. I’d rather instruct APT to not install Firefox, e.g. maybe something like this would prevent the upgrade:

Package: firefox-esr
[…]
Pin-Priority: -10

… without having to cheat on versions? If something like this can work, I think it would much better express our intent.

Conclusion: if you think there are good enough solutions with a reasonable cost/benefit to the two problems you’re describing here, then go ahead, add this to the Foundations Team’s plate and fix it.

#9 Updated by intrigeri 2018-05-24 13:05:42

  • Assignee deleted (anonym)
  • Priority changed from Normal to Low
  • QA Check deleted (Info Needed)
  • Type of work changed from Code to Research

intrigeri wrote:
> Conclusion: if you think there are good enough solutions with a reasonable cost/benefit to the two problems you’re describing here, then go ahead, add this to the Foundations Team’s plate and fix it.

This did not happen so I’m turning this into a low priority research ticket.

Let’s close this if there’s no progress by the next ticket triaging session.

#10 Updated by Anonymous 2018-08-16 13:00:03

  • Status changed from Confirmed to Rejected

No progress since 3 months → closing.