I have installed all previous 2.1* milestone builds and 2.0.12 in en-US and de as available (all Linux), updated them either via auto-update or manually using the complete.mar to 2.1b2 and diffed a sorted output of |find .| of plain 2.1b2 installs against those updated builds. This showed not just that the partial update for 2.1b2 has problems (will file another bug for that) but also that complete MAR works fine in terms of builds, but a few files are left in the system that we still should clean up with removed-files. I ignored left-over empty directories as updater can't currently remove them (bug is IIRC on file), and there is an open bug on even Firefox 4 not correctly removing old locale files (so let's leave that part to a different bug as well). Everything else is something we should add. I'm attaching my plain update log that includes the relevant diff parts.

I just performed the same matrix on windows.

report from manual 2.0.12,2.1b1 updates to 2.1b2 (I couldn't get the autoupdates to work...)

For Linux and Windows.

Comment on attachment 512390 [details] [diff] [review] Updated removed-files.in for findings from 2.1b2 update checks. (v1) I'll give it a real review later, but one-off comments. [Hold on new attachment until my real review though] > #endif >+modules/HUDService.jsm > modules/JSON.jsm > modules/dbViewWrapper.js > modules/mailViewManager.js > modules/quickSearchManager.js > modules/searchSpec.js >+modules/autoconfigUtils.jsm > #ifdef MOZ_STATIC_JS > @DLL_PREFIX@mozjs@DLL_SUFFIX@ > #endif please sort this section (e.g. put autoconfigUtils.jsm above dbViewWrapper.js) >@@ -498,6 +519,9 @@ xpicleanup@BIN_SUFFIX@ > components/interfaces.manifest > components/mail.xpt > components/components.manifest >+ #ifdef XP_UNIX >+ components/nsFilePicker.js >+ #endif > defaults/profile/bookmarks.html > defaults/profile/mimeTypes.rdf > defaults/profile/chrome This was already listed above.

Comment on attachment 512390 [details] [diff] [review] Updated removed-files.in for findings from 2.1b2 update checks. (v1) I also want to take a look at this patch. And thanks for writing it up!

Comment on attachment 512390 [details] [diff] [review] Updated removed-files.in for findings from 2.1b2 update checks. (v1) >+chrome/en-US.manifest No, this should be chrome/@AB_CD@.manifest - but we need to see that replacing this variable actually works even in the locale repack scenario. >+#ifdef XP_UNIX >+components/nsFilePicker.js >+#endif That one's already in the file, but with an ifdef that isn't defined in this scope. Either change to your ifdef or copy the definition from package-manifest.in over or (which I think would be best) migrate this to be added to the defines in the Makefile instead. > modules/dbViewWrapper.js > modules/mailViewManager.js > modules/quickSearchManager.js > modules/searchSpec.js >+modules/autoconfigUtils.jsm Sort this to be before dbViewWrapper, please. >+ #ifdef XP_UNIX >+ components/nsFilePicker.js >+ #endif No need to add this twice. Also, please add the entries from the 2.0.12 update test in attachment 512050 [details] - at least those inside Contents/MacOS/ (of course using an #ifdef XP_MACOSX). Not sure if we can do anything about those outside the MacOS/ dir there, but a look into the Firefox removed-files.in might show something up there. feedback- for addressing those comments. Many thanks for looking into this, though, the packaging stuff is tedious work, I know... (We really should figure out the locales stuff, but I think that needs a look into repack logic and that's not for the faint of the heart, and I think FF has the same "problems" there, so I still hop to see a solution from them and us just following. Probably should all go into its own bug, though.)

(In reply to comment #6) > Comment on attachment 512390 [details] [diff] [review] > Updated removed-files.in for findings from 2.1b2 update checks. (v1) > >+chrome/en-US.manifest > No, this should be chrome/@AB_CD@.manifest - but we need to see that replacing > this variable actually works even in the locale repack scenario. Actually I am fairly sure this WONT work in Firefox; since I recall reading a bug there that mentioned this was wrong, I'll plan to lookup that bug.

(In reply to comment #7) > Actually I am fairly sure this WONT work in Firefox; since I recall reading a > bug there that mentioned this was wrong, I'll plan to lookup that bug. True. This still doesn't make using @AB_CD@ here wrong, as for the default case it resolves fine to en-US - and then we'll only need to make the repack case working, but will already have the correct thing in here.

Comment on attachment 513343 [details] [diff] [review] Updated removed-files.in for findings from 2.1b2 update checks. (v2) > #ifdef XP_MACOSX [...] >+ chrome/classic.manifest etc. No need to list those again that are already listed for all platforms and added globally in this patch! This includes >+ extensions/langpack-pl@chatzilla.mozilla.org/chrome.manifest >+ extensions/langpack-pl@chatzilla.mozilla.org/chrome/chatzilla.jar >+ extensions/langpack-pl@chatzilla.mozilla.org/chrome/chatzilla.manifest >+ extensions/langpack-pl@chatzilla.mozilla.org/install.js >+ extensions/langpack-pl@chatzilla.mozilla.org/install.rdf >+ extensions/langpack-pl@venkman.mozilla.org/chrome.manifest >+ extensions/langpack-pl@venkman.mozilla.org/chrome/venkman.jar >+ extensions/langpack-pl@venkman.mozilla.org/chrome/venkman.manifest >+ extensions/langpack-pl@venkman.mozilla.org/install.js >+ extensions/langpack-pl@venkman.mozilla.org/install.rdf which fall under the whole @AB_CD@ "problem" though ("pl" is the AB_CD value for Polish). >+#ifdef XP_UNIX >+#ifndef XP_MACOSX >+#define UNIX_NOT_MAC >+#endif >+#endif >+ Please put that at the start of the file, just like it's done for package-manifest.in as well.

Comment on attachment 513343 [details] [diff] [review] Updated removed-files.in for findings from 2.1b2 update checks. (v2) I suspect I'll have nits for a v4, but KaiRo's comments warrant another go at this before I stick my nose too deep.

Comment on attachment 513930 [details] [diff] [review] Update removed-files.in for findings from 2.1b2 update checks. Switch the def at top from UNIX_NOT_MAC to UNIX_BUT_NOT_MAC please >+#ifdef XP_UNIX >+components/nsFilePicker.js >+#endif use UNIX_BUT_NOT_MAC here please. >+extensions/inspector@mozilla.org/components/components.list > #ifdef UNIX_BUT_NOT_MAC ^^^ We missed this tidbit before [was never defined] (not an issue from your patch) > #ifdef XP_MACOSX >- ../Plug-Ins/PrintPDE.plugin/Contents/Info.plist Keep all these (../ beginning) lines at the start of this ifdef please. >+ components/mozldap.xpt >+ components/nsAboutAbout.js This [nsAboutAbout.js] (and others) that are universally removed (elsewhere) that you are adding, please don't re-add them here. >+ plugins/DefaultPlugin.plugin/Contents/Resources/English.lproj I'm less certain on these plugins/ changes, but I would feel 100% confident here if we just copied what Firefox has at http://mxr.mozilla.org/mozilla-central/source/browser/installer/removed-files.in?mark=1212-1234#1212 to this spot. while we are at it, lets add |updater.app/Contents/MacOS/updater.ini| to the mac ifdef here, (Firefox has it). I'm not sure if we need it, but I do know it won't hurt. r- based on the above. I think we're almost there!

Comment on attachment 516584 [details] [diff] [review] Update removed-files.in for findings from 2.1b2 update checks. (v3) This is very close, but... >+ ../Plug-Ins/PrintPDE.plugin/Contents/Resources/English.lproj >+ ../Plug-Ins/PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib >+ ../Resources/seamonkey-bin.rsrc >+ ../Plug-Ins/PrintPDE.plugin/Contents/Info.plist >+ ../Plug-Ins/PrintPDE.plugin/Contents/MacOS/PrintPDE >+ ../Plug-Ins/PrintPDE.plugin/Contents/Resources/English.lproj/Localizable.strings >+ ../Plug-Ins/PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib/classes.nib >+ ../Plug-Ins/PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib/info.nib >+ ../Plug-Ins/PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib/objects.xib Could you please order those the first two (which are directories, so make them end in /) actually go before the entries inside those dirs? >-plugins/DefaultPlugin.plugin/ >+../plugins/DefaultPlugin.plugin/ No, this was correct. > plugins/DefaultPlugin.plugin/Contents/ > plugins/DefaultPlugin.plugin/Contents/Info.plist > plugins/DefaultPlugin.plugin/Contents/MacOS/ And those are duplicated above and here - no chance to have them in a single section only? The rest looks good to me from a fast glance!

Comment on attachment 518053 [details] [diff] [review] Update removed-files.in for findings from 2.1b2 update checks. (v4) Mostly good. (Note I tried out reviewboard for once, so I could get more context easily; the URL for this is http://reviews.visophyte.org/r/518053/ note the first two reviews posted there by me were failures on my part to understand that system. -- The line #'s below are referencing an already patched file) on file: suite/installer/removed-files.in line 146 > #ifdef XP_UNIX > components/nsFilePicker.js > #endif Use UNIX_BUT_NOT_MAC here please on file: suite/installer/removed-files.in line 257 > ../Resources/seamonkey-bin.rsrc Sort this alphabetically to below the ../Plug-Ins please on file: suite/installer/removed-files.in line 275 > plugins/DefaultPlugin.plugin/Contents/Resources/English.lproj/InfoPlist.strings > plugins/DefaultPlugin.plugin/Contents/Resources/plugin.png You missed two lines here from the Firefox section I identified: http://mxr.mozilla.org/mozilla-central/source/browser/installer/removed-files.in?mark=1217-1220,1217,1218#1212 on file: suite/installer/removed-files.in line 300 > plugins/DefaultPlugin.plugin/ > plugins/DefaultPlugin.plugin/Contents/ > plugins/DefaultPlugin.plugin/Contents/Info.plist > plugins/DefaultPlugin.plugin/Contents/MacOS/ > plugins/DefaultPlugin.plugin/Contents/MacOS/DefaultPlugin > plugins/DefaultPlugin.plugin/Contents/Resources/ > plugins/DefaultPlugin.plugin/Contents/Resources/English.lproj/ > plugins/DefaultPlugin.plugin/Contents/Resources/English.lproj/InfoPlist.strings Please also remove these since they are above.

Comment on attachment 518289 [details] [diff] [review] Updated removed-files.in for findings from 2.1b2 update checks. (v5) Perfect, thank you.

Pushed as http://hg.mozilla.org/comm-central/rev/c74e6965bd50 - thanks a lot for working on this one!