On Mac OS the precomplete file will need to be moved to Contents/Resources

removed-files change had snuck into the first patch

IIRC we put precomplete at the top of the .app to force OSX to refresh something after an update (icon, or version in the Get Info maybe). Do we have to accept a regression as part of the packaging changes ?

Already handled. There is code to perform essentially a touch to the app bundle after a successful update. I had hope that this one-off code could one day be removed with the addition of the precomplete file that I added as part of Firefox 5 since that does the same thing but with it moving the code that performs the touch will suffice. http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/updater.cpp#2315

I need to modify the test mar files before I can change this properly but this suffices for now.

Comment on attachment 8480244 [details] [diff] [review] Mar packaging code and precomplete generation (wip) Review of attachment 8480244 [details] [diff] [review]: ----------------------------------------------------------------- ::: tools/update-packaging/make_incremental_updates.py @@ +335,5 @@ > to_dir_path = os.path.abspath(to_dir_path) > # Create a hashtable of the from and to directories > from_dir_hash,from_file_set,from_dir_set = patch_info.build_marfile_entry_hash(from_dir_path) > to_dir_hash,to_file_set,to_dir_set = patch_info.build_marfile_entry_hash(to_dir_path) > + # Create a list of the forced updates Nit, trailing whitespace. @@ +348,5 @@ > + forced_list.append("Contents/Resources/precomplete") > + else: > + forced_list.append("Contents\Resources\precomplete") > + else: > + forced_list.append("precomplete") Alternatively something like: for location in ('precomplete', 'Contents/Resources/precomplete', 'Contents\Resources\precomplete'): if location in to_file_set: forced_list.append(location) break else: raise Exception, "missing precomplete file in: "+to_dir_path

(In reply to Nick Thomas [:nthomas] from comment #6) >... > @@ +348,5 @@ > > + forced_list.append("Contents/Resources/precomplete") > > + else: > > + forced_list.append("Contents\Resources\precomplete") > > + else: > > + forced_list.append("precomplete") > > Alternatively something like: > for location in ('precomplete', 'Contents/Resources/precomplete', > 'Contents\Resources\precomplete'): > if location in to_file_set: > forced_list.append(location) > break > else: > raise Exception, "missing precomplete file in: "+to_dir_path That will raise an exception except when the first item is found so I went with what I originally had.

The test changes to support v2 signing are massive and will be done in bug bug1058182.

I reverted one change so the tests continue to pass on Mac without creating the new mar files. I'll create those as soon as I am able.

This way the precomplete file is always listed in the precomplete file to lessen the chance that the precomplete file will be different than the one present when generating the bundle's signature so the signature won't be invalid.

stray tab snuck in

Comment on attachment 8489186 [details] [diff] [review] Create precomplete before enumerating files r+, assuming the updater has read and closed precomplete before trying to remove it.

It is

Brian, can I get an r+ for the following change to the original updater.cpp patch which is now possible with the updated mar files in bug 1058182. diff --git a/toolkit/mozapps/update/updater/updater.cpp b/toolkit/mozapps/update/updater/updater.cpp --- a/toolkit/mozapps/update/updater/updater.cpp +++ b/toolkit/mozapps/update/updater/updater.cpp @@ -3646,27 +3619,27 @@ GetManifestContents(const NS_tchar *mani } int AddPreCompleteActions(ActionList *list) { if (sIsOSUpdate) { return OK; } +#ifdef XP_MACOSX + NS_tchar *rb = GetManifestContents(NS_T("Contents/Resources/precomplete")); +#else NS_tchar *rb = GetManifestContents(NS_T("precomplete")); +#endif if (rb == nullptr) { - //XXX Temporary since requiring this location on Mac OS X will require making new test mar files - rb = GetManifestContents(NS_T("Contents/Resources/precomplete")); - if (rb == nullptr) { - LOG(("AddPreCompleteActions: error getting contents of precomplete " \ - "manifest")); - // Applications aren't required to have a precomplete manifest. The mar - // generation scripts enforce the presence of a precomplete manifest. - return OK; - } + LOG(("AddPreCompleteActions: error getting contents of precomplete " \ + "manifest")); + // Applications aren't required to have a precomplete manifest. The mar + // generation scripts enforce the presence of a precomplete manifest. + return OK; } int rv; NS_tchar *line; while((line = mstrtok(kNL, &rb)) != 0) { // skip comments if (*line == NS_T('#')) continue;

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #15) > Created attachment 8489879 [details] [diff] [review] > Combined patch > > Brian, can I get an r+ for the following change to the original updater.cpp > patch which is now possible with the updated mar files in bug 1058182. > > > diff --git a/toolkit/mozapps/update/updater/updater.cpp > b/toolkit/mozapps/update/updater/updater.cpp > --- a/toolkit/mozapps/update/updater/updater.cpp > +++ b/toolkit/mozapps/update/updater/updater.cpp > @@ -3646,27 +3619,27 @@ GetManifestContents(const NS_tchar *mani > } > > int AddPreCompleteActions(ActionList *list) > { > if (sIsOSUpdate) { > return OK; > } > > +#ifdef XP_MACOSX > + NS_tchar *rb = > GetManifestContents(NS_T("Contents/Resources/precomplete")); > +#else > NS_tchar *rb = GetManifestContents(NS_T("precomplete")); > +#endif > if (rb == nullptr) { > - //XXX Temporary since requiring this location on Mac OS X will require > making new test mar files > - rb = GetManifestContents(NS_T("Contents/Resources/precomplete")); > - if (rb == nullptr) { > - LOG(("AddPreCompleteActions: error getting contents of precomplete " \ > - "manifest")); > - // Applications aren't required to have a precomplete manifest. The > mar > - // generation scripts enforce the presence of a precomplete manifest. > - return OK; > - } > + LOG(("AddPreCompleteActions: error getting contents of precomplete " \ > + "manifest")); > + // Applications aren't required to have a precomplete manifest. The mar > + // generation scripts enforce the presence of a precomplete manifest. > + return OK; > } > > int rv; > NS_tchar *line; > while((line = mstrtok(kNL, &rb)) != 0) { > // skip comments > if (*line == NS_T('#')) > continue; yep :)

Pushed to fx-team https://hg.mozilla.org/integration/fx-team/rev/d3025e55dfc3

Landed on aurora in the Mac V2 signing combined patch in bug 1047584