555512 - Indent removed-files.in for readability

Status: RESOLVED FIXED

(Firefox :: Installer, enhancement)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

See https://bugzilla.mozilla.org/show_bug.cgi?id=554903#c5 for the motivation.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Allow whitespace in removed-files.in

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.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Reorganize removed-files.in

Indent and alphabetize.

Comment 3

[Robert Strong (they/them - no direct email)]

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.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Reorganize removed-files.in

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: V2.1

Comment 6

[Robert Strong (they/them - no direct email)]

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.

Comment 7

[Robert Strong (they/them - no direct email)]

btw: I verified the output on Windows already

Comment 8

[Robert Strong (they/them - no direct email)]

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

[Robert Strong (they/them - no direct email)]

(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

[Robert Strong (they/them - no direct email)]

I'll push this to try so the before / after removed-files can be compared

Comment 11

[Robert Strong (they/them - no direct email)]

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

[Robert Strong (they/them - no direct email)]

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

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: V2.2

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+.

Comment 14

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Builds are at https://build.mozilla.org/tryserver-builds/[e-mail]-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

[Robert Strong (they/them - no direct email)]

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 [e-mail] 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/[e-mail]/components/BrandRes.dll
  extensions/[e-mail]/components/fullsoft.dll
  extensions/[e-mail]/components/master.ini
  extensions/[e-mail]/components/talkback-l10n.ini
  extensions/[e-mail]/components/talkback.cnt
  extensions/[e-mail]/components/talkback.exe
  extensions/[e-mail]/components/talkback.hlp
  extensions/[e-mail]/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/[e-mail]/components/talkback/master.ini
    extensions/[e-mail]/components/talkback/talkback.dylib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Info.plist
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/MacOS/Talkback
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/pbdevelopment.plist
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/PkgInfo
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/delete.tiff
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/disable.tiff
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/enable.tiff
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/ArchivingSettings.nib/classes.nib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/ArchivingSettings.nib/info.nib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/ArchivingSettings.nib/objects.nib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/InfoPlist.strings
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/IntroWizard.nib/classes.nib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/IntroWizard.nib/info.nib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/IntroWizard.nib/objects.nib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/Localizable.strings
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/MainMenu.nib/classes.nib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/MainMenu.nib/info.nib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/MainMenu.nib/objects.nib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/ProxySettings.nib/classes.nib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/ProxySettings.nib/info.nib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/ProxySettings.nib/objects.nib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/SendingSettings.nib/classes.nib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/SendingSettings.nib/info.nib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/English.lproj/SendingSettings.nib/objects.nib
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/KeyInfoKeys.plist
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/KeyInfoSections.plist
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/send.tiff
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/sort_ascending.tiff
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/sort_descending.tiff
    extensions/[e-mail]/components/talkback/Talkback.app/Contents/Resources/Talkback.icns
  #else
    extensions/[e-mail]/components/talkback/talkback
    extensions/[e-mail]/components/talkback/XTalkback.ad
    extensions/[e-mail]/components/talkback/master.ini
    extensions/[e-mail]/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.

Comment 16

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

The weirdness is because I forgot to push the first patch as well :-P

Comment 17

[Robert Strong (they/them - no direct email)]

Comment on attachment 442992 [details] [diff] [review]
V2.2

Doh! Thanks for the explanation and r=me with autocomplete.xpt comment fixed.

Comment 18

[Robert Strong (they/them - no direct email)]

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.

Comment 19

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Pushed with the autocomplete fix.

http://hg.mozilla.org/mozilla-central/rev/189582773525
http://hg.mozilla.org/mozilla-central/rev/89fdae318636