Opened 3 years ago

Closed 3 years ago

#19528 closed defect (fixed)

The build id stays the same with every Firefox update

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

Description (last modified by gk)

#19400 was among other things a result of our hard-coded REFERENCE_DATETIME that gets used as the build id. We should change that with every Firefox release.

Child Tickets

Change History (42)

comment:1 Changed 3 years ago by gk

Some useful discussion we already had in #11506:

<boklm>

I think there are different solutions:

    we can use the time of the commit from tor-browser-bundle.git. The drawback is that we can't do partial rebuilds anymore, as any commit will change the timestamp used everywhere
    we can use the time of the last commit on the gitian descriptor file. However, the descriptor files are not always changed between two releases
    we can use the time of the last commit from one of the remotes specified in the descriptor (tor-browser.git for the firefox descriptor)
    we can manually set a value for REFERENCE_DATETIME in gitian/versions and update it while preparing a new release 

<gk>

Partial rebuilds are worth to have. But I actually don't see how any of the four possible solutions you brought up solves this bug while keeping this option. I mean, even if we set REFERENCE_DATETIME in gitian/versions the timestamps would be different (e.g. those embedded in binaries that don't get rebuilt).

<boklm>

If we set REFERENCE_DATETIME in gitian/versions, we can update its value when preparing a new release, but keep the same value for all the intermediate builds we do for one release. This does not allow partial rebuilds between two releases, but allows it for intermediate builds for one release.

comment:2 Changed 3 years ago by gk

What about just updating REFERENCE_DATETIME in the Firefox descriptor if a new Firefox version is used or using a separate one (like REFERENCE_DATETIME_FIREFOX) to begin with just for the Firefox part? That way we would solve both problems like #19400 and we could retain partial builds as we have them now: no need to rebuild all the artifacts if the Firefox version changes or if we only rebundle or are doing a buildN.

comment:3 Changed 3 years ago by gk

We could use the output of e.g. cat browser/config/version.txt to construct something.

comment:4 in reply to:  2 ; Changed 3 years ago by boklm

Replying to gk:

What about just updating REFERENCE_DATETIME in the Firefox descriptor if a new Firefox version is used or using a separate one (like REFERENCE_DATETIME_FIREFOX) to begin with just for the Firefox part? That way we would solve both problems like #19400 and we could retain partial builds as we have them now: no need to rebuild all the artifacts if the Firefox version changes or if we only rebundle or are doing a buildN.

This looks like a good idea.

Replying to gk:

We could use the output of e.g. cat browser/config/version.txt to construct something.

Yes, we could probably convert the version number to an increasing number. However, if we release a Tor Browser update based on the same firefox version, then the build id will not change.

comment:5 in reply to:  4 Changed 3 years ago by gk

Replying to boklm:

Replying to gk:

We could use the output of e.g. cat browser/config/version.txt to construct something.

Yes, we could probably convert the version number to an increasing number. However, if we release a Tor Browser update based on the same firefox version, then the build id will not change.

That's perfectly fine as the idea is to prevent things like #19400 which only happen with Firefox updates.

comment:6 Changed 3 years ago by boklm

In the branch bug_19528-v1 in my user repo, we are setting MOZ_BUILD_DATE (which is used as the build id) based on the Tor Browser version:
https://gitweb.torproject.org/user/boklm/tor-browser-bundle.git/commit/?h=bug_19528-v1

I have not yet tried doing a build using this branch. I can also change it to use the version from browser/config/version.txt rather than the Tor Browser version if that's better.

Here is the MOZ_BUILD_DATE for some values of TORBROWSER_VERSION:

$ TORBROWSER_VERSION=7.0 ./build-helpers/get-moz-build-date 
export MOZ_BUILD_DATE=20160208010101
$ TORBROWSER_VERSION=7.0.1 ./build-helpers/get-moz-build-date 
export MOZ_BUILD_DATE=20160208010201
$ TORBROWSER_VERSION=7.0.2 ./build-helpers/get-moz-build-date 
export MOZ_BUILD_DATE=20160208010301
$ TORBROWSER_VERSION=7.0.2.1 ./build-helpers/get-moz-build-date 
export MOZ_BUILD_DATE=20160208010302
$ TORBROWSER_VERSION=7.0a1 ./build-helpers/get-moz-build-date 
export MOZ_BUILD_DATE=20160108010201

comment:7 in reply to:  6 ; Changed 3 years ago by gk

Replying to boklm:

I have not yet tried doing a build using this branch. I can also change it to use the version from browser/config/version.txt rather than the Tor Browser version if that's better.

Yes, that would be better because currently this breaks building Tor Browser using already built artifacts in case we don't need to rebuild the browser part: Users building, say 7.0.2, from scratch would get a different result than we which just redid the bundle part and used, say 7.0.1, otherwise (maybe 7.0.2 just needed a newer Torbutton). So, we need something that changes only if a newer Firefox is built to construct MOZ_BUILD_DATE.

comment:8 in reply to:  7 ; Changed 3 years ago by boklm

Replying to gk:

Replying to boklm:

I have not yet tried doing a build using this branch. I can also change it to use the version from browser/config/version.txt rather than the Tor Browser version if that's better.

Yes, that would be better because currently this breaks building Tor Browser using already built artifacts in case we don't need to rebuild the browser part: Users building, say 7.0.2, from scratch would get a different result than we which just redid the bundle part and used, say 7.0.1, otherwise (maybe 7.0.2 just needed a newer Torbutton). So, we need something that changes only if a newer Firefox is built to construct MOZ_BUILD_DATE.

However, we are already using the Tor Browser version as a firefox configure argument --with-tor-browser-version=${TORBROWSER_VERSION} which is then used in a few places in the updater, so we already can't change the Tor Browser version without rebuilding the firefox part.

comment:9 Changed 3 years ago by gk

Keywords: tbb-gitian added

comment:10 in reply to:  8 Changed 3 years ago by gk

Replying to boklm:

Replying to gk:

Replying to boklm:

I have not yet tried doing a build using this branch. I can also change it to use the version from browser/config/version.txt rather than the Tor Browser version if that's better.

Yes, that would be better because currently this breaks building Tor Browser using already built artifacts in case we don't need to rebuild the browser part: Users building, say 7.0.2, from scratch would get a different result than we which just redid the bundle part and used, say 7.0.1, otherwise (maybe 7.0.2 just needed a newer Torbutton). So, we need something that changes only if a newer Firefox is built to construct MOZ_BUILD_DATE.

However, we are already using the Tor Browser version as a firefox configure argument --with-tor-browser-version=${TORBROWSER_VERSION} which is then used in a few places in the updater, so we already can't change the Tor Browser version without rebuilding the firefox part.

Yes, that's true. But I think the right fix is not to depend on a non-Firefox version number and I still have hope to get #18325 solved some day. :) Thus, if we can avoid it I'd be happy to not create more dependencies on TORBROWSER_VERSION in the Firefox descriptors.

comment:11 Changed 3 years ago by boklm

Ah indeed. I created branch bug_19528-v2 which is using the firefox version:
https://gitweb.torproject.org/user/boklm/tor-browser-bundle.git/commit/?h=bug_19528-v2

I did not try a build yet with this branch.

comment:12 Changed 3 years ago by boklm

Keywords: TorBrowserTeam201607R added
Status: newneeds_review

I tried an alpha build with branch bug_19528-v2 and it produced a Tor Browser with the expected build id: 20160201030101 (the same as the output from ./build-helpers/get-moz-build-date 45.2.0).

comment:13 Changed 3 years ago by mcs

A couple of quick comments:

  • Can we automate the generation of the year portion? Leaving it as 2016 forever will be a strange thing to do (or if we plan to manually update it there is a good chance we will forget to do so). One approach is to use the same method we use for COPYRIGHT_YEAR in gitian/descriptors/mac/gitian-firefox.yml.
  • We should carry the build id through to update_responses so that the update manifests include the correct build ID.

comment:14 Changed 3 years ago by gk

Keywords: TorBrowserTeam201607 added; TorBrowserTeam201607R removed
Status: needs_reviewneeds_revision

comment:15 Changed 3 years ago by gk

Keywords: TorBrowserTeam201608 added; TorBrowserTeam201607 removed

Moving items to August 2016.

comment:16 in reply to:  13 ; Changed 3 years ago by boklm

Replying to mcs:

A couple of quick comments:

  • Can we automate the generation of the year portion? Leaving it as 2016 forever will be a strange thing to do (or if we plan to manually update it there is a good chance we will forget to do so). One approach is to use the same method we use for COPYRIGHT_YEAR in gitian/descriptors/mac/gitian-firefox.yml.

An other part of the script that we need to manually update is the esr branch number that we substract so that it is less than 30. To make sure we have an always increasing build ID when the version increase, I think we should update the esr number we substract and the year at the same time. So I think it is better to have the year manually updated too. I updated the script with a comment and separate variables for the year and esr branch to make updating that easier.

But there is still a good chance we will forget to update this. Maybe we can raise an error if the year used in the script and the year of the Tor Browser commit are different, so we don't forget to update it?

  • We should carry the build id through to update_responses so that the update manifests include the correct build ID.

I pushed a new branch bug_19528-v3 doing that:
https://gitweb.torproject.org/user/boklm/tor-browser-bundle.git/commit/?h=bug_19528-v3

In the bundle step we extract the build id from the application.ini file and put it in a buildid.txt file. The update_responses script then uses the value from that file as the build id.

comment:17 Changed 3 years ago by boklm

Keywords: TorBrowserTeam201608R added; TorBrowserTeam201608 removed
Status: needs_revisionneeds_review

comment:18 in reply to:  16 Changed 3 years ago by mcs

Replying to boklm:

...
But there is still a good chance we will forget to update this. Maybe we can raise an error if the year used in the script and the year of the Tor Browser commit are different, so we don't forget to update it?

This seems like a good idea, but if gk and you think it is sufficient to document this as part of our build procedures then just adding a mention there seems OK to me.

  • We should carry the build id through to update_responses so that the update manifests include the correct build ID.

I pushed a new branch bug_19528-v3 doing that:
https://gitweb.torproject.org/user/boklm/tor-browser-bundle.git/commit/?h=bug_19528-v3

r=brade, r=mcs
We did not run a test build, but the changes look good.

comment:19 Changed 3 years ago by gk

Status: needs_reviewneeds_information

So, hrm. I am not so happy with keeping an extra buildid.txt file around. Can't we extract this info once from one of the full MAR files just before we are creating the update response files?

I am not sure I understand the need for a hard-coded oldest supported ESR version? Could you elaborate?

I am interested in getting rid of hard-coding the year as well but have not looked at how we could do that yet.

comment:20 in reply to:  19 ; Changed 3 years ago by boklm

Replying to gk:

So, hrm. I am not so happy with keeping an extra buildid.txt file around. Can't we extract this info once from one of the full MAR files just before we are creating the update response files?

Yes, I think we can do that if we want to avoid adding a buildid.txt file.

I am not sure I understand the need for a hard-coded oldest supported ESR version? Could you elaborate?

I am interested in getting rid of hard-coding the year as well but have not looked at how we could do that yet.

When MOZ_BUILD_DATE is not set, the buildid is the current date. With the changes from this ticket we are converting the firefox version to a number that looks like a date, that we set in MOZ_BUILD_DATE. However the ESR branch (currently 45) is used as the month in the buildid number, but 45 is not a valid month so we need to subtract something if we want it to look like a valid date.

If we don't care about MOZ_BUILD_DATE containing an invalid date, we could stop adding a year and subtracting something to the ESR version. But I don't know if some part of the build would break if MOZ_BUILD_DATE is not a valid date, or if it can work with any number.

comment:21 in reply to:  20 ; Changed 3 years ago by gk

Replying to boklm:

Replying to gk:

I am not sure I understand the need for a hard-coded oldest supported ESR version? Could you elaborate?

I am interested in getting rid of hard-coding the year as well but have not looked at how we could do that yet.

When MOZ_BUILD_DATE is not set, the buildid is the current date. With the changes from this ticket we are converting the firefox version to a number that looks like a date, that we set in MOZ_BUILD_DATE. However the ESR branch (currently 45) is used as the month in the buildid number, but 45 is not a valid month so we need to subtract something if we want it to look like a valid date.

If we don't care about MOZ_BUILD_DATE containing an invalid date, we could stop adding a year and subtracting something to the ESR version. But I don't know if some part of the build would break if MOZ_BUILD_DATE is not a valid date, or if it can work with any number.

I think we should care about getting a valid date out of it (at least it can't hurt). What I do not understand is the need for hard-coding the oldest supported ESR version. Why can't we extract a version from the parameter the script get passed anyway (in fact the major ESR version does already get extracted)? It seems to me there always will be times where the alphas and the stable series share build ids which does not seem to be bad to me. So, what is your motivation for hard-coding that number?

comment:22 Changed 3 years ago by gk

Keywords: TorBrowserTeam201608 added; TorBrowserTeam201608R removed
Status: needs_informationneeds_revision

Getting the current year with the method mcs mentioned for COPYRIGHT_YEAR seems reasonable to me.

comment:23 in reply to:  21 ; Changed 3 years ago by boklm

Replying to gk:

I think we should care about getting a valid date out of it (at least it can't hurt). What I do not understand is the need for hard-coding the oldest supported ESR version. Why can't we extract a version from the parameter the script get passed anyway (in fact the major ESR version does already get extracted)? It seems to me there always will be times where the alphas and the stable series share build ids which does not seem to be bad to me. So, what is your motivation for hard-coding that number?

Maybe the "oldest support ESR version" name is confusing. It is just a number that we subtract to the major ESR version to make it less than 30 (I was wrong in my previous comment, the major ESR version is not used as the month but as day of the month). We don't want this number to be based on the current version, because for instance when we release a version based on ESR 52, we still want to continue subtracting 45 so that the buildid for ESR 52 releases is bigger than for ESR 45 releases.

For ESR 45 releases, the buildid will start with 20160101 or 20170101 (for the releases made in 2017). The ESR 52 releases made in 2017 will have a buildid starting with 20170108. In 2018 we will change $oldest_esr to 52 and ESR 52 releases will have a buildid starting with 20180101, ESR 59 releases a buildid starting with 20180108.

comment:24 in reply to:  23 Changed 3 years ago by gk

Replying to boklm:

Replying to gk:

I think we should care about getting a valid date out of it (at least it can't hurt). What I do not understand is the need for hard-coding the oldest supported ESR version. Why can't we extract a version from the parameter the script get passed anyway (in fact the major ESR version does already get extracted)? It seems to me there always will be times where the alphas and the stable series share build ids which does not seem to be bad to me. So, what is your motivation for hard-coding that number?

Maybe the "oldest support ESR version" name is confusing. It is just a number that we subtract to the major ESR version to make it less than 30 (I was wrong in my previous comment, the major ESR version is not used as the month but as day of the month). We don't want this number to be based on the current version, because for instance when we release a version based on ESR 52, we still want to continue subtracting 45 so that the buildid for ESR 52 releases is bigger than for ESR 45 releases.

For ESR 45 releases, the buildid will start with 20160101 or 20170101 (for the releases made in 2017). The ESR 52 releases made in 2017 will have a buildid starting with 20170108. In 2018 we will change $oldest_esr to 52 and ESR 52 releases will have a buildid starting with 20180101, ESR 59 releases a buildid starting with 20180108.

I see. So, bumping the hard-coded value is year-bound, right? More precisely: every two years starting with 2018 we increase the hard-coded value by 7, correct? If that's the case then we can hard-code the current year and then we compare the year extracted (like done with COPYRIGHT_YEAR) with the one we hard-coded: if the extracted one is the hard-coded one + 2 we add to the hard-coded "old-ESR" value 7 if + 4 then we add +14 etc. If incrementing is not year bound (maybe it should not because ESR release are not happening exactly every 12 month) I guess we could construct something similar taking ESR versions into account (like every two major ESR versions we can add 7 to our hard-coded value.

comment:25 Changed 3 years ago by boklm

This sounds like a good idea. I will try to do something like that.

comment:26 Changed 3 years ago by boklm

Keywords: TorBrowserTeam201608R added; TorBrowserTeam201608 removed
Status: needs_revisionneeds_review

I pushed a new branch bug_19528-v4:
https://gitweb.torproject.org/user/boklm/tor-browser-bundle.git/commit/?h=bug_19528-v4

In this version we use the date of the git commit to set the year.

To set the day in the buildid, we take the difference between the ESR version and 45, and divide it by 5. We could divide by 7 as there usually is an ESR version every 7 versions, but this only works if the difference between 2 ESR versions is 7, while dividing by 5 works even if even if the difference between 2 ESR versions is not exactly 7 (as long as it is 5 or more). We will need to change this around ESR 180 when we will reach day 28, or before if we start using non-ESR versions.

comment:27 Changed 3 years ago by boklm

A possible issue is that the buildid can be accessed with navigator.buildID, which can maybe help for fingerprinting:
https://developer.mozilla.org/en-US/docs/Web/API/Navigator/buildID

comment:28 in reply to:  27 Changed 3 years ago by mcs

Replying to boklm:

A possible issue is that the buildid can be accessed with navigator.buildID, which can maybe help for fingerprinting:
https://developer.mozilla.org/en-US/docs/Web/API/Navigator/buildID

We could set avoid that problem bhy setting general.buildID.override to a constant value such as 20000101000000 (see https://bugzilla.mozilla.org/show_bug.cgi?id=417994).

Version 0, edited 3 years ago by mcs (next)

comment:29 Changed 3 years ago by gk

Keywords: TorBrowserTeam201609R added; TorBrowserTeam201608R removed

Moving review tickets to September

comment:30 Changed 3 years ago by gk

Keywords: TorBrowserTeam201609 added; TorBrowserTeam201609R removed
Status: needs_reviewneeds_revision

Looks better, thanks. Two things remain:

1) Could we get rid of buildid.txt as well (see comment:19 and comment:20)
2) Could you provide a patch for 000-tor-browser.js, too, setting general.buildID.override accordingly?

comment:31 in reply to:  30 ; Changed 3 years ago by boklm

Replying to gk:

Looks better, thanks. Two things remain:

1) Could we get rid of buildid.txt as well (see comment:19 and comment:20)

Ah, I forgot about this change, I'm going to post a new version of the patch, extracting the buildid from one of the mar files instead of using a buildid.txt file.

While doing that, I noticed that we could also do the same for the platformVersion (currently defined manually in the update_responses/config.yml file) and use the version defined in application.ini from one of the mar files.

comment:32 in reply to:  31 Changed 3 years ago by gk

Replying to boklm:

Replying to gk:

Looks better, thanks. Two things remain:

1) Could we get rid of buildid.txt as well (see comment:19 and comment:20)

Ah, I forgot about this change, I'm going to post a new version of the patch, extracting the buildid from one of the mar files instead of using a buildid.txt file.

While doing that, I noticed that we could also do the same for the platformVersion (currently defined manually in the update_responses/config.yml file) and use the version defined in application.ini from one of the mar files.

Yes. But this could be done in a new bug as well if you think this is cleaner or whatever.

comment:33 Changed 3 years ago by boklm

I pushed a new branch bug_19528-v5 which does not use a buildid.txt file anymore:
https://gitweb.torproject.org/user/boklm/tor-browser-bundle.git/commit/?h=bug_19528-v5

The buildid is extracted from the application.ini file from the first mar file. The platformVersion is not extracted yet, I will do it in a separate bug.

comment:34 Changed 3 years ago by gk

Keywords: TorBrowserTeam201609R added; TorBrowserTeam201609 removed
Status: needs_revisionneeds_information

One question regarding:

+            my $appfile = "$tmpdir/application.ini" if -f "$tmpdir/application.ini";
+            $appfile = "$tmpdir/Contents/Resources/application.ini"
+                                if -f "$tmpdir/application.ini";

I think I understand the first line but I have a hard time doing so wrt the second and third: why is appfile getting $tmpdir/Contents/Resources/application.ini assigned if $tmpdir/application.ini exists? It already got assigned $tmpdir/application.ini in that case.

comment:35 Changed 3 years ago by boklm

Status: needs_informationneeds_review

Ah indeed, this was a copy past error. appfile should be getting $tmpdir/Contents/Resources/application.ini assigned if $tmpdir/Contents/Resources/application.ini exists.

I pushed a new branch bug_19528-v6 to correct this:
https://gitweb.torproject.org/user/boklm/tor-browser-bundle.git/commit/?h=bug_19528-v6

comment:36 Changed 3 years ago by gk

Description: modified (diff)

comment:37 Changed 3 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

Thanks. This got applied to master with commit ba7f0fcf914e889f78c0562a46e6026462f10ac8.

Last edited 3 years ago by gk (previous) (diff)

comment:38 Changed 3 years ago by gk

FWIW: hardened-builds has this, too, with commit ac62d42011e18a5cf124d3265b3b62a931c17e9a.

comment:39 in reply to:  30 Changed 3 years ago by boklm

Replying to gk:

2) Could you provide a patch for 000-tor-browser.js, too, setting general.buildID.override accordingly?

After looking at 000-tor-browser.js, we are already setting general.buildID.override there, so no new patch is needed.

comment:40 Changed 3 years ago by boklm

Resolution: fixed
Status: closedreopened

The MOZ_BUILD_DATE variable is not set correctly:

+++ cat browser/config/version.txt
++ /home/ubuntu/build/get-moz-build-date 45.3.0
fatal: Not a git repository (or any of the parent directories): .git
Use of uninitialized value $year in multiplication (*) at /home/ubuntu/build/get-moz-build-date line 15.
+ eval export MOZ_BUILD_DATE=201040101

The reason is that we are running get-moz-build-date, which uses the current git commit to get the year, after removing the .git directory.

comment:41 Changed 3 years ago by boklm

Status: reopenedneeds_review

comment:42 Changed 3 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

Looks good to me. Applied to master (commit 02ff59eaeb1bea0ce00ee0223da9eab0ff69834d) and hardened-builds (commit b79f5d274b2093758af6cf9a2076224c40295b1d).

Note: See TracTickets for help on using tickets.