1102033 - package-manifest.in needs to be updated after the cleanup in bug 1096494

Status: RESOLVED FIXED

(SeaMonkey :: Installer, defect)

Description

[Stephen A Pohl [:spohl]]

+++ This bug was initially created as a clone of Bug #1096494 +++

Unfortunately, I didn't realize that the patch in bug 1096494 would bust comm-central on OSX. I'm assuming this also affects SeaMonkey. All that seems to be necessary is for package-manifest.in to use @RESPATH@ instead of @BINPATH@ for the resources that are located under Contents/Resources. The patch in bug 1096494 should be a good illustration how this was done for Firefox.

I'd usually write this patch myself, but I'm currently sidetracked with getting a Firefox hotfix out the door. I'll still write the patch if nobody else gets to this first.

Comment 1

[Edmund Wong (:ewong)]

Attachment: port patch (v1)

Comment 2

[Justin Wood (:Callek)]

Comment on attachment 8525808 [details] [diff] [review]
port patch (v1)

will be a few days for me (min), but if spohl or jcranmer want to give this a review before me, it can land.

Comment 3

[Stephen A Pohl [:spohl]]

Comment on attachment 8525808 [details] [diff] [review]
port patch (v1)

Review of attachment 8525808 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for jumping on this so quickly! Some feedback below. I'd like to do another quick sanity check of your new patch once you had a chance to address the feedback.

::: suite/installer/package-manifest.in
@@ +37,5 @@
>  #ifdef XP_MACOSX
>  ; Mac bundle stuff
>  @APPNAME@/Contents/Info.plist
>  @APPNAME@/Contents/PkgInfo
> +@RESPATH@/Contents/Resources/seamonkey.icns

This should be @RESPATH@/seamonkey.icns

@@ +38,5 @@
>  ; Mac bundle stuff
>  @APPNAME@/Contents/Info.plist
>  @APPNAME@/Contents/PkgInfo
> +@RESPATH@/Contents/Resources/seamonkey.icns
> +@RESPATH@/Contents/Resources/@AB@.lproj/*

This should be @RESPATH@/@AB@.lproj/*

@@ +57,2 @@
>  #ifdef XP_WIN32
> +@RESPATH@/uninstall/helper.exe

Although @BINPATH@ is equivalent to @RESPATH@ on Windows right now, we might as well keep this at @BINPATH@ since it's compiled code. I just noticed that this same mistake slipped into Desktop Firefox and I'll write a followup patch to bug 1096494.

@@ +81,5 @@
>  @BINPATH@/@DLL_PREFIX@plds4@DLL_SUFFIX@
>  #endif
>  #endif
>  #ifdef XP_MACOSX
>  @APPNAME@/Contents/MacOS/XUL

This can be changed to @BINPATH@/XUL

@@ +86,5 @@
>  #else
>  @BINPATH@/@DLL_PREFIX@xul@DLL_SUFFIX@
>  #endif
>  #ifdef XP_MACOSX
> +@RESPATH@/@MOZ_CHILD_PROCESS_NAME@.app/

This should use @BINPATH@.

@@ +87,5 @@
>  @BINPATH@/@DLL_PREFIX@xul@DLL_SUFFIX@
>  #endif
>  #ifdef XP_MACOSX
> +@RESPATH@/@MOZ_CHILD_PROCESS_NAME@.app/
> +@RESPATH@/@DLL_PREFIX@plugin_child_interpose@DLL_SUFFIX@

This should use @BINPATH@.

@@ -734,5 @@
> -@BINPATH@/components/nsLDAPProtocolHandler.js
> -#ifdef XP_WIN32
> -@BINPATH@/nsldap32v60.dll
> -@BINPATH@/nsldappr32v60.dll
> -@BINPATH@/nsldif32v60.dll

Did you mean for the XP_WIN32 section to be removed here? The #else section doesn't seem to match the filenames in the XP_WIN32 section...

@@ -749,5 @@
>  ; [Updater]
>  ;
>  #ifdef MOZ_UPDATER
> -#ifdef XP_MACOSX
> -@APPNAME@/Contents/MacOS/updater.app/

This should be changed to @BINPATH@/updater.app/ rather than removed completely.

Comment 4

[Edmund Wong (:ewong)]

(In reply to Stephen Pohl [:spohl] from comment #3)

> @@ -734,5 @@
> > -@BINPATH@/components/nsLDAPProtocolHandler.js
> > -#ifdef XP_WIN32
> > -@BINPATH@/nsldap32v60.dll
> > -@BINPATH@/nsldappr32v60.dll
> > -@BINPATH@/nsldif32v60.dll
> 
> Did you mean for the XP_WIN32 section to be removed here? The #else section
> doesn't seem to match the filenames in the XP_WIN32 section...

Tbh, I wasn't really sure if I was right.  I've re-added it back.

Comment 5

[Edmund Wong (:ewong)]

Attachment: patch (v2)

Comment 6

[Stephen A Pohl [:spohl]]

Comment on attachment 8526425 [details] [diff] [review]
patch (v2)

Review of attachment 8526425 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great! I realized that I missed a few more instances in your previous patch that should have been changed from @BINPATH@ to @RESPATH@. The dynamic libraries that are in the "components" subdirectory are actually under @RESPATH@ because the entire "components" directory was moved from Contents/MacOS to Contents/Resources. In the future, we would like to no longer have any dynamic libraries under Contents/Resources or its subdirectories, but this doesn't need to be handled in this bug.

r=spohl with the three corrections below addressed. Thanks again!

::: suite/installer/package-manifest.in
@@ +155,2 @@
>  #ifdef MOZ_UPDATER
>  @BINPATH@/update-settings.ini

This should be @RESPATH@/update-settings.ini

@@ +196,2 @@
>  #ifdef MOZ_ENABLE_DBUS
>  @BINPATH@/components/@DLL_PREFIX@dbusservice@DLL_SUFFIX@

This should be @RESPATH@/components/@DLL_PREFIX@dbusservice@DLL_SUFFIX@

@@ +276,2 @@
>  #ifdef MOZ_ENABLE_GNOME_COMPONENT
>  @BINPATH@/components/@DLL_PREFIX@mozgnome@DLL_SUFFIX@

This should be @RESPATH@/components/@DLL_PREFIX@mozgnome@DLL_SUFFIX@

Comment 7

[Edmund Wong (:ewong)]

Attachment: patch (v3)

Updated patch to fix the 3 places.  Forwarding r+.

Comment 8

[Edmund Wong (:ewong)]

Comment on attachment 8526585 [details] [diff] [review]
patch (v3)

Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/27a199642aaf