Closed Bug 1577634 Opened 5 months ago Closed 5 months ago

devedition 70.0b2 release-update-verify "differences found after update"

Categories

(Release Engineering :: Release Automation: Updates, defect)

defect
Not set

Tracking

(firefox-esr60 unaffected, firefox-esr68 unaffected, firefox69 unaffected, firefox70 fixed, firefox71 fixed)

RESOLVED FIXED
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox69 --- unaffected
firefox70 --- fixed
firefox71 --- fixed

People

(Reporter: mhentges, Assigned: nthomas)

References

Details

Attachments

(1 file)

For the devedition 70.0b2 release, there are release-update-verify tasks failing (example).

[task 2019-08-29T21:02:20.524Z] TEST-UNEXPECTED-FAIL: differences found after update
[task 2019-08-29T21:02:20.524Z] TEST-UNEXPECTED-FAIL: [68.0 de complete] check_updates returned failure for Linux_x86_64-gcc3 downloads/firefox-68.0b7.tar.bz2 vs. downloads/firefox-70.0b2.tar.bz2: 1

A comment was added to defaults/prefs/channel-prefs.js that doesn't seem to be in the update pack. IIUC, that seems to be the cause of this issue

As discussed with Aki on IRC, it sounds like there's some options for solving this in the longer term:

  1. In the update-verify task, don't raise an error if the difference is just in comments.
    • Note: what about changes in the middle of multiline comments? Do we parse JS files?
    • Note: what about changes in comments in a multitude of different file types? Do we add parsing support for each one?
  2. When the update pack was created, the patch for channel-prefs.js wasn't included. Why was that? Include that "patch-filtering logic" into update-verify

For posterity, the log snippet looks like:

[task 2019-08-29T20:43:45.377Z] Difference found in defaults/pref/channel-prefs.js
[task 2019-08-29T20:43:45.377Z] defaults/pref/channel-prefs.js still differs after transforms, residual diff:
[task 2019-08-29T20:43:45.377Z] ---
[task 2019-08-29T20:43:45.377Z] +++
[task 2019-08-29T20:43:45.377Z] @@ -2,4 +2,8 @@
[task 2019-08-29T20:43:45.377Z]   * License, v. 2.0. If a copy of the MPL was not distributed with this
[task 2019-08-29T20:43:45.377Z]   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
[task 2019-08-29T20:43:45.377Z] 
[task 2019-08-29T20:43:45.377Z] +// This pref is in its own file for complex reasons. See the comment in
[task 2019-08-29T20:43:45.377Z] +// browser/app/Makefile.in, bug 756325, and bug 1431342 for details. Do not add
[task 2019-08-29T20:43:45.377Z] +// other prefs to this file.
[task 2019-08-29T20:43:45.377Z] +
[task 2019-08-29T20:43:45.377Z]  pref("app.update.channel", "aurora");
[task 2019-08-29T20:43:45.377Z] TEST-UNEXPECTED-FAIL: differences found after update
[task 2019-08-29T20:43:45.377Z] TEST-UNEXPECTED-FAIL: [57.0 ru complete] check_updates returned failure for Linux_x86_64-gcc3 downloads/firefox-57.0b2.tar.bz2 vs. downloads/firefox-70.0b2.tar.bz2: 1

(In reply to Mitchell Hentges [:mhentges] from comment #2)

As discussed with Aki on IRC, it sounds like there's some options for solving this in the longer term:

  1. In the update-verify task, don't raise an error if the difference is just in comments.
    • Note: what about changes in the middle of multiline comments? Do we parse JS files?
    • Note: what about changes in comments in a multitude of different file types? Do we add parsing support for each one?

Turns out we already support making transformations to cope with channel-prefs.js differences, see compare-directories.py. We can likely extend this to squash the new differences, and get back to green update-verify jobs.

  1. When the update pack was created, the patch for channel-prefs.js wasn't included. Why was that? Include that "patch-filtering logic" into update-verify

channel-prefs.js is the main place that sets what update channel any given install is on, and there are a couple of reasons that we don't modify it when applying an update. The main one is that the file will have different content for different releases, and we publish release RC builds on the beta channel. A user who installs beta has pref("app.update.channel", "beta"); in channel-prefs.js, while a regular/end-user would havepref("app.update.channel", "release");. We don't want to move beta users to release when we give them the release RC build [1] . So we use a special update instruction, add-if-not, which will add channel-prefs.js in the (unexpected) case of it being missing, but otherwise respect what is there already. This also helps for QA testing because they can modify the channel easily, and it won't be reset if multi-update scenarios from older releases.

[1] Why do we do that ? We want to make sure no issues have snuck into the RC since the last beta, and because we used to have a problem where any given compilation could tickle a bug in some BIOS code, and we didn't know until we shipped it to users.

See Also: → 1576546

Update verify noticed the new comments in channel-prefs.js from bug 1576546, and thought they were significant. This change suppresses that by ignoring all lines starting with '//'. Since we can't match on the significant new line but not on the existing one we swallow a rat and comment the new whitespace.

Attachment #9089268 - Flags: review+

https://hg.mozilla.org/releases/mozilla-beta/rev/04d54c74a7444cb8a3e90663b0e43b6593cf9c39

We'll land on autoland when rail/someone can r+ in phabricator.

Pushed by nthomas@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b64636e18945
suppress warnings about comment changes in channel-prefs.js, r=bhearsum
Assignee: nobody → nthomas
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
See Also: → 1588649
You need to log in before you can comment on or make changes to this bug.