This is an offshoot to bug 391958. We sometimes need to force files for updates while running in "fast mode." We should add this functionality to make_incremental_updates.py. The forced files list currently gets improperly set to "$VAR1 => []" in the patchlist file. https://bugzilla.mozilla.org/show_bug.cgi?id=391958#c23

This change gets rid of the VAR rubbish and replaces it with a space separated list of files to force. Index: patcher2.pl =================================================================== RCS file: /cvsroot/mozilla/tools/patcher/patcher2.pl,v retrieving revision 1.32 diff -U6 -r1.32 patcher2.pl --- patcher2.pl 29 Apr 2008 14:01:33 -0000 1.32 +++ patcher2.pl 23 May 2008 12:52:28 -0000 @@ -535,13 +535,13 @@ if ($useFastPatcher) { $partial_pathname =~ m/^(.*)\/[^\/]*$/g; print PARTIAL_PATCHLIST_FILE getcwd() . '/' . $from_path . ',' . getcwd() . '/' . $to_path . ',' . getcwd() . '/' . $partial_pathname . ',' . - Data::Dumper::Dumper($forcedUpdateList); + "@{$forcedUpdateList}\n"; } else { my $start_time = time(); PrintProgress(total => $total, current => $i, string => "$u/$p/$l"); make_incremental_updates.py would still need to be taught to use it's forced_updates var.

This patch adds "force" support to make_incremental_updates.py and to the patcher "fast" mode which calls this script. "force" lets us specify that a file is not to be diffed in the partial MAR, the whole file should be ADDed instead (this is normally used to iron out inconsistencies caused by bugs, without having to force everyone over to a full MAR). Quick rundown of the changes here: In patcher, write the filenames to be forced into the patchlist file, using pipe as separator (because the patchlist file already uses comma-separated values). Using this from patcher involves adding multiple "force" lines to the current release block, one filename per line. In make_incremental_updates.py, do an explicit ADD instead of diffing for any files that are common in both the "from" and "to" MARs. Files that only exist in the "to" MAR will already get an ADD because there's nothing to diff. Files that only exist in the "from" MAR are not applicable, I think, because it's not clear what to force... to "to" MAR must have the file that is to be forced.

Comment on attachment 328836 [details] [diff] [review] [checked in] add "force" support to make_incremental_updates.py and the patcher patchlist writer I'm scared to make update-related changes without benjamin looking over it, but the patcher hooks are hairy so maybe bhearsum could ok this too?

Just FYI Helmer pinged me on IM - we lost the unit tests for the python code (never checked it) but are trying to retrieve from backups...

Comment on attachment 328836 [details] [diff] [review] [checked in] add "force" support to make_incremental_updates.py and the patcher patchlist writer I think this will be OK. I'm not too familiar with this code though, so take it fwiw :). Have you done any testing yet? I don't want to check this in until we know this doesn't break things in any obvious way.

(In reply to comment #5) > (From update of attachment 328836 [details] [diff] [review]) > I think this will be OK. I'm not too familiar with this code though, so take it > fwiw :). Have you done any testing yet? I don't want to check this in until we > know this doesn't break things in any obvious way. Yes I tested the latest run in the moz19 patcher.cfg, with and without forced files. Pretty sure the way I did it, if forced_list is empty, then nothing will happen ("if filename in forced_list" will always be false), and the usual case will happen. For paranoia's sake it'd probably be worth doing a full run of the same release before and after this change and comparing them; I can do that if you like.

Would also be worth double-checking that there's no OS-specific weirdness (I'm looking at you OS X), eg by checking the update manifest and file locations in a generated mar for Mac.

(In reply to comment #7) > Would also be worth double-checking that there's no OS-specific weirdness (I'm > looking at you OS X), eg by checking the update manifest and file locations in > a generated mar for Mac. You do need to use force with the explicit path, so yes it would be a different path for OS X, but so would windows (.exe and .dll extensions, for example). You need a force line for every potential file that the script might run across; the patcher implementation isn't OS-specific and I didn't try to change this. I did my initial testing on OS X fwiw :) But I'll do a full run sometime this week, before and after this patch. It would probably be informative to do a full run with patcher "fast mode" off, to see if the way patcher does it is any different.. I believe that it should not be but haven't tested.

(In reply to comment #8) > I did my initial testing on OS X fwiw :) But I'll do a full run sometime this > week, before and after this patch. It would probably be informative to do a > full run with patcher "fast mode" off, to see if the way patcher does it is any > different.. I believe that it should not be but haven't tested. I lied, haven't had a chance to set up the test yet. I will do it on a Linux box though (I have CentOS 5.2 handy, maybe I will grab the ref platform instead if that's up to date?). Also, we should be landing unit tests for the make_incremental_updates.py script soon, I'd like to make sure I didn't break those and also integrate this feature into the tests if applicable.. I'll comment here once I've investigated.

(In reply to comment #9) > I lied, haven't had a chance to set up the test yet. I will do it on a Linux > box though (I have CentOS 5.2 handy, maybe I will grab the ref platform instead > if that's up to date?). The downloadable ref platform is probably a little stale by now. http://stage.mozilla.org/pub/mozilla.org/mozilla/VMs/CentOS5-ReferencePlatform.zip is dated in mid-January.

(In reply to comment #10) > (In reply to comment #9) > > I lied, haven't had a chance to set up the test yet. I will do it on a Linux > > box though (I have CentOS 5.2 handy, maybe I will grab the ref platform instead > > if that's up to date?). > > The downloadable ref platform is probably a little stale by now. > http://stage.mozilla.org/pub/mozilla.org/mozilla/VMs/CentOS5-ReferencePlatform.zip > is dated in mid-January. > It's missing a few tools (hg, nagios, etc), but the important stuff is the same.

just fyi - force file support should be working just fine if "fast mode" is disabled, so if this does not land in time for bug 449474 I don't think it completely blocks.. I think it should, but I still haven't started any testing yet :) Hopefully will have time this weekend.

I'm going to do some testing on this over the weekend to try to get it into the tree asap.

My testing was for the case of forcing a single file (modules/distribution.js used by Fx3.0.x partner builds); it confirms that the patch here is working well. I generated partial updates for Fx3.0.1 -> 3.0.2build4 using the original patcher2 method (the slow mode Rob refers to in comment #12), and the fastmode with the patch applied. Then I unpacked the partial update from each method and compared them with diff; there were only differences in the update.manifest file. Partly this was entries appearing in a different order (but still before all the remove instructions), and partly bug 454115 (patch-if vs patch on Mac). This is harmless and both methods have add "modules/distribution.js" So I think we're good to go with landing this, so I'll do that and set up UPDATE_PACKAGING_R5.

Comment on attachment 328836 [details] [diff] [review] [checked in] add "force" support to make_incremental_updates.py and the patcher patchlist writer Checking in patcher/patcher2.pl; /cvsroot/mozilla/tools/patcher/patcher2.pl,v <-- patcher2.pl new revision: 1.34; previous revision: 1.33 done Checking in update-packaging/make_incremental_updates.py; /cvsroot/mozilla/tools/update-packaging/make_incremental_updates.py,v <-- make_incremental_updates.py new revision: 1.7; previous revision: 1.6 done

UPDATE_PACKAGING_R5 created, picking up the change her and bug 444266. Thanks for doing all the work here Rob.

Also landed the make_incremental_updates.py part of this in releases/mozilla-1.9.1 and mozilla-central, and updated UPDATE_PACKAGING_R7. http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f7a9328d3fb6 http://hg.mozilla.org/mozilla-central/rev/fb32f6e1859c Note that the tag change picks up everything since _R6 was created in the hg repositories, and that's well out of sync with the same tag in CVS (and also with each other in hg). I think that's probably going to be OK, because mbsdiff and mar include NSPR headers but don't actually link against it, so the exposure to bustage is small.

Moving closed Future bugs into Release Engineering in preparation for removing the Future component.