Closed
Bug 555512
Opened 15 years ago
Closed 15 years ago
Indent removed-files.in for readability
Categories
(Firefox :: Installer, enhancement)
Firefox
Installer
Tracking
()
RESOLVED
FIXED
Firefox 3.7a5
People
(Reporter: khuey, Assigned: khuey)
References
Details
Attachments
(2 files, 3 obsolete files)
(deleted),
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
See https://bugzilla.mozilla.org/show_bug.cgi?id=554903#c5 for the motivation.
Assignee | ||
Comment 1•15 years ago
|
||
Allows whitespace at the beginning of line in removed-files.in. This strips the whitespace at preprocessing time to avoid needing to change all of the removed-files consumers.
Attachment #435440 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 2•15 years ago
|
||
Indent and alphabetize.
Attachment #435441 -
Flags: review?(robert.bugzilla)
Updated•15 years ago
|
Attachment #435440 -
Flags: review?(robert.bugzilla) → review+
Comment 3•15 years ago
|
||
Comment on attachment 435441 [details] [diff] [review]
Reorganize removed-files.in
Since problems with this file are fairly catastrophic could you attach an updated patch? I should be able to get to it quickly now so it won't bitrot.
Attachment #435441 -
Flags: review?(robert.bugzilla) → review-
Assignee | ||
Comment 4•15 years ago
|
||
Attachment #435441 -
Attachment is obsolete: true
Attachment #442810 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 5•15 years ago
|
||
Attachment #442810 -
Attachment is obsolete: true
Attachment #442823 -
Flags: review?(robert.bugzilla)
Attachment #442810 -
Flags: review?(robert.bugzilla)
Comment 6•15 years ago
|
||
Comment on attachment 442823 [details] [diff] [review]
V2.1
>+#ifdef MOZ_ENABLE_LIBXUL
>+ @DLL_PREFIX@xpcom_core@DLL_SUFFIX@
>+ components/@DLL_PREFIX@jar50@DLL_SUFFIX@
>+ #ifdef XP_WIN
>+ components/jsd3250.dll
>+ components/nsPostUpdateWin.js
please move to the #ifdef XP_WIN without the #ifdef MOZ_ENABLE_LIBXUL
>+ components/xpinstal.dll
>+ #else
>+ components/@DLL_PREFIX@jsd@DLL_SUFFIX@
>+ components/@DLL_PREFIX@xpinstall@DLL_SUFFIX@
>+ #endif
>+#else
>+ #ifdef XP_MACOSX
>+ XUL
>+ #else
>+ @DLL_PREFIX@xul@DLL_SUFFIX@
>+ #endif
>+#endif
>+#ifndef MOZ_UPDATER
>+ components/nsUpdateService.js
>+ components/nsUpdateServiceStub.js
>+#endif
I'd also prefer it if the #ifdef PLATFORM sections were before all other #ifdef's
> #ifndef XP_MACOSX
>-components/autocomplete.xpt
>+ components/autocomplete.xpt
> #endif
I believe this should be removed here and below and put in the all platforms section
>...
>+#ifdef XP_UNIX
>+ #ifndef XP_MACOSX
>+ chrome/icons/default/default.xpm
>+ components/libimgicon.so
>+ dictionaries/PL.aff
>+ dictionaries/PL.dic
>+ icons/mozicon16.xpm
>+ icons/mozicon50.xpm
>+ libjemalloc.so
>+ readme.txt
>+ #endif
>+#endif
>+#ifndef XP_WIN
>+ res/fonts/mathfontSymbol.properties
>+#endif
please move to the #ifdef XP_WIN below
Can you run these two patches through the try server and compare the removed-files before and after on Linux and Mac OS X? If not, I should be able to. I want to be 100% sure nothing is added / missing when this lands since that can cause havoc.
Attachment #442823 -
Flags: review?(robert.bugzilla) → review-
Comment 7•15 years ago
|
||
btw: I verified the output on Windows already
Comment 8•15 years ago
|
||
Comment on attachment 442823 [details] [diff] [review]
V2.1
>diff --git a/browser/installer/removed-files.in b/browser/installer/removed-files.in
>--- a/browser/installer/removed-files.in
>+++ b/browser/installer/removed-files.in
>@@ -1,94 +1,829 @@
>+# Removed-files.in is processed at build time to create a list of files that
>+# should be removed by the installer. These files are in alphabetical order,
>+# except that files removed only on certain platforms are after all of the
>+# regular files and obsolete Talkback and Inspector files are at the very end.
s/should be removed by the installer/should be removed during an application update and by the Windows installer/
Comment 9•15 years ago
|
||
(In reply to comment #6)
> (From update of attachment 442823 [details] [diff] [review])
> >+#ifdef MOZ_ENABLE_LIBXUL
> >+ @DLL_PREFIX@xpcom_core@DLL_SUFFIX@
> >+ components/@DLL_PREFIX@jar50@DLL_SUFFIX@
> >+ #ifdef XP_WIN
> >+ components/jsd3250.dll
> >+ components/nsPostUpdateWin.js
> please move to the #ifdef XP_WIN without the #ifdef MOZ_ENABLE_LIBXUL
>
> >+ components/xpinstal.dll
> >+ #else
> >+ components/@DLL_PREFIX@jsd@DLL_SUFFIX@
> >+ components/@DLL_PREFIX@xpinstall@DLL_SUFFIX@
> >+ #endif
> >+#else
> >+ #ifdef XP_MACOSX
> >+ XUL
> >+ #else
> >+ @DLL_PREFIX@xul@DLL_SUFFIX@
> >+ #endif
> >+#endif
> >+#ifndef MOZ_UPDATER
> >+ components/nsUpdateService.js
> >+ components/nsUpdateServiceStub.js
> >+#endif
> I'd also prefer it if the #ifdef PLATFORM sections were before all other
> #ifdef's
>
> > #ifndef XP_MACOSX
> >-components/autocomplete.xpt
> >+ components/autocomplete.xpt
> > #endif
> I believe this should be removed here and below and put in the all platforms
> section
>
> >...
> >+#ifdef XP_UNIX
> >+ #ifndef XP_MACOSX
> >+ chrome/icons/default/default.xpm
> >+ components/libimgicon.so
> >+ dictionaries/PL.aff
> >+ dictionaries/PL.dic
> >+ icons/mozicon16.xpm
> >+ icons/mozicon50.xpm
> >+ libjemalloc.so
> >+ readme.txt
> >+ #endif
> >+#endif
> >+#ifndef XP_WIN
> >+ res/fonts/mathfontSymbol.properties
> >+#endif
> please move to the #ifdef XP_WIN below
forget that one... misread it as ifdef when it is actually ifndef
Comment 10•15 years ago
|
||
I'll push this to try so the before / after removed-files can be compared
Comment 11•15 years ago
|
||
I've verified that the before and after remove-files for Linux and Windows are the same... just need to verify Mac OS X now. Looking good
Comment 12•15 years ago
|
||
Mac OS X is good to go as well. Please resubmit with the requested changes and I'll plus it after a quick look over to verify the ordering of the different sections. The EM rewrite backout changed removed-files.in so the patch will need to be updated for that as well. Let me know if you would like me to check this in... I don't recall whether you have commit access. Cheers and thanks
Assignee | ||
Comment 13•15 years ago
|
||
Updated as asked. I've pushed this to tryserver again so we can make that verification. I can push this to m-c as well once it gets r+.
Attachment #442823 -
Attachment is obsolete: true
Attachment #442992 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 14•15 years ago
|
||
Builds are at https://build.mozilla.org/tryserver-builds/me@kylehuey.com-try-0aaab16bb888/. Looks good on Windows and Linux, I don't know of any way to open a DMG image on Windows to verify the Mac version though.
Comment 15•15 years ago
|
||
Comment on attachment 442992 [details] [diff] [review]
V2.2
>diff --git a/browser/installer/removed-files.in b/browser/installer/removed-files.in
>--- a/browser/installer/removed-files.in
>+++ b/browser/installer/removed-files.in
>@@ -1,96 +1,835 @@
>...
>+xpicleanup@BIN_SUFFIX@
>+#ifdef XP_MACOSX
>+ ../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
>+ ../Resources/firefox-bin.rsrc
>+ components/accessibility.xpt
>+ components/alerts.xpt
>+ components/appshell.xpt
>+ components/appstartup.xpt
>+ components/autocomplete.xpt
You added components/autocomplete.xpt in the non ifdef section and removed it in #ifndef XP_MACOSX but forgot to remove the it in #ifdef XP_MACOSX
dmg's can be opened by 7-zip.
I took a look at the try server builds (the only ones with me@kylehuey.com in the name dated today) and something appears to be busted on at least Mac OS X and Windows... I didn't check Linux.
Windows removed-files from try server
xpicleanup.exe
components/brwsrcmp.dll
components/jsd3250.dll
components/nsPostUpdateWin.js
js3250.dll
#ifdef MOZ_MEMORY
Microsoft.VC80.CRT.manifest
msvcm80.dll
msvcp80.dll
msvcr80.dll
#else
mozcrt19.dll
#endif
xpcom_core.dll
components/jar50.dll
#ifdef XP_WIN
components/xpinstal.dll
#else
components/jsd.dll
components/xpinstall.dll
#endif
extensions/talkback@mozilla.org/components/BrandRes.dll
extensions/talkback@mozilla.org/components/fullsoft.dll
extensions/talkback@mozilla.org/components/master.ini
extensions/talkback@mozilla.org/components/talkback-l10n.ini
extensions/talkback@mozilla.org/components/talkback.cnt
extensions/talkback@mozilla.org/components/talkback.exe
extensions/talkback@mozilla.org/components/talkback.hlp
extensions/talkback@mozilla.org/InstallDisabled
components/BrandRes.dll
Mac OS X removed-files from try server
xpicleanup
../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
<snip>
updater.app/Contents/MacOS/updater.ini
#ifndef XP_MACOSX
chrome/icons/default/default.xpm
components/libimgicon.so
dictionaries/PL.aff
dictionaries/PL.dic
icons/mozicon16.xpm
icons/mozicon50.xpm
libjemalloc.so
readme.txt
#endif
res/fonts/mathfontSymbol.properties
libxpcom_core.dylib
components/libjar50.dylib
#ifdef XP_WIN
components/xpinstal.dll
#else
components/libjsd.dylib
components/libxpinstall.dylib
#endif
#ifdef XP_MACOSX
extensions/talkback@mozilla.org/components/talkback/master.ini
extensions/talkback@mozilla.org/components/talkback/talkback.dylib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Info.plist
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/MacOS/Talkback
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/pbdevelopment.plist
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/PkgInfo
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/delete.tiff
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/disable.tiff
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/enable.tiff
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/ArchivingSettings.nib/classes.nib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/ArchivingSettings.nib/info.nib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/ArchivingSettings.nib/objects.nib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/InfoPlist.strings
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/IntroWizard.nib/classes.nib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/IntroWizard.nib/info.nib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/IntroWizard.nib/objects.nib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/Localizable.strings
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/MainMenu.nib/classes.nib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/MainMenu.nib/info.nib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/MainMenu.nib/objects.nib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/ProxySettings.nib/classes.nib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/ProxySettings.nib/info.nib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/ProxySettings.nib/objects.nib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/SendingSettings.nib/classes.nib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/SendingSettings.nib/info.nib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/English.lproj/SendingSettings.nib/objects.nib
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/KeyInfoKeys.plist
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/KeyInfoSections.plist
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/send.tiff
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/sort_ascending.tiff
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/sort_descending.tiff
extensions/talkback@mozilla.org/components/talkback/Talkback.app/Contents/Resources/Talkback.icns
#else
extensions/talkback@mozilla.org/components/talkback/talkback
extensions/talkback@mozilla.org/components/talkback/XTalkback.ad
extensions/talkback@mozilla.org/components/talkback/master.ini
extensions/talkback@mozilla.org/components/talkback/talkback.so
#endif
components/BrandRes.dll
My own try server builds to verify with slight modifications to the removed-files patch didn't exhibit this problem.
Attachment #442992 -
Flags: review?(robert.bugzilla) → review-
Assignee | ||
Comment 16•15 years ago
|
||
The weirdness is because I forgot to push the first patch as well :-P
Comment 17•15 years ago
|
||
Comment on attachment 442992 [details] [diff] [review]
V2.2
Doh! Thanks for the explanation and r=me with autocomplete.xpt comment fixed.
Attachment #442992 -
Flags: review- → review+
Comment 18•15 years ago
|
||
Mossop, just a heads up regarding changes to removed-files.in for when you merge to addonsmgr branch or land the EM rewrite bugs again on m-c.
Assignee | ||
Comment 19•15 years ago
|
||
Pushed with the autocomplete fix.
http://hg.mozilla.org/mozilla-central/rev/189582773525
http://hg.mozilla.org/mozilla-central/rev/89fdae318636
Assignee | ||
Updated•15 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a5
Updated•15 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•