173262 - GRE enable the Mozilla client..

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Chak Nanga]

We need to  GRE enable the Mozilla client on all the platforms. Currently the
only "client" which uses the GRE functionality is the MfcEmbed test harness on
the Windows platform.

All the core XPCOM related changes for the GRE are already on the current
Mozilla trunk.

More info. on the GRE at : http://www.mozilla.org/projects/embedding/MRE.html

Comment 1

[Chak Nanga]

The latest attachment in http://bugzilla.mozilla.org/show_bug.cgi?id=129582
shows the changes that were required to GRE enable MfcEmbed.
[Please rename references to MRE into GRE in that patch]

Comment 2

[Sean Su]

mine now.

Comment 3

[Sean Su]

Attachment: patch v1.0

This patch is not final, but it seems to work (for me).  Chak, when you test
this patch, make sure to add GRE's path to mozilla.exe's (or netscp.exe's) App
Paths in the windows registry.

The following still needs to be done for this patch:
* add env var check that overrides the GRE path look up for developers who have
everthing in their /bin dir.

* update the installer to set up the appropriate environment vars for both GRE
and Mozilla.

* update the installer packaging build process to build both GRE and Mozilla
products.

we need to also have at least one or more of GRE libs versioned at build time. 
Having to rely on the version in the windows registry is not accurate enough as
there are many builds between each release.  bug 180383 has been filed.

Comment 4

[Chak Nanga]

I'm going to apply this patch and try with my build...thanks

Sean : Meanwhile, can you please take a look at your compreg.dat(under the
dist\bin\components dir) to make sure your build infact is picking up components
etc. from the GRE dir and not from the bin dir? {sorry, for being paranoid here!)

If it's using the GRE then you should see entries such as the following in the
compreg.dat:

abs:c:\mozilla_src\mozilla\dist\gre\components\accessibility.dll,1037343341479
abs:c:\mozilla_src\mozilla\dist\gre\components\appshell.dll,1037343350091
abs:c:\mozilla_src\mozilla\dist\gre\components\caps.dll,1037342522922
abs:c:\mozilla_src\mozilla\dist\gre\components\chrome.dll,1037342531244
abs:c:\mozilla_src\mozilla\dist\gre\components\cookie.dll,1037343406492
abs:c:\mozilla_src\mozilla\dist\gre\components\docshell.dll,1037343113792

If it's not using the GRE then you should see something like:
rel:accessibility.dll,1037343341479
rel:appcomps.dll,1037343383039
rel:appshell.dll,1037343350091
rel:autoconfig.dll,1037203242984
rel:caps.dll,1037342522922
rel:chrome.dll,1037342531244

Comment 5

[Sean Su]

yep... here's part of what is in compreg.dat:

 abs:C:\Program Files\mozilla.org\GRE\1.3a\components\necko.dll,1037304180000
 abs:C:\Program Files\mozilla.org\GRE\1.3a\components\necko2.dll,1037304180000
 abs:C:\Program Files\mozilla.org\GRE\1.3a\components\oji.dll,1037304300000

and in xpti.dat:

 [Directories,3]
 0,T:\trees\seamonkey\mozilla\dist\bin\components
 1,C:\Program Files\mozilla.org\GRE\1.3a\components
 2,T:\trees\seamonkey\mozilla\dist\bin\plugins

Comment 6

[Sean Su]

Attachment: patch v1.1

I've mainly changed the way the code finds the path to GRE.  instead of using
the windows registries to locate the correct version, it will use the loaded
xpcom.dll's path from now on in all cases (where the cases are xpcom.dll in the
current dir and xpcom.dll in a common GRE dir).

Checking this patch in will not affect current builds where GRE is in the /bin
dir.  When we switch to having GRE outside of the /bin dir, this patch will
still work.

The installer will be in control of which GRE mozilla will use by setting the
appropriate AppPaths registry key.

This also means that once GRE has been moved to be run outside of /bin, users
with multiple copies of GRE and/or mozilla will encounter issues with
mozilla.exe locating and running with a compatible version of GRE.  Normally it
will be the last copy of mozilla installed that its GRE will be picked up.

We don't need to worry about that problem in this particular bug.  I'll file
another bug to address the multiple copies of GRE issue.

Comment 7

[Chak Nanga]

Sean...come to think of it using App Paths to pick up the correct version of the
xpcom.dll will not work when we have multiple versions of Mozilla(using
different  versions of GRE) on the same machine.

Cc:ing Juraj and Curt since they've been looking at using AppPaths for MfcEmbed.

Comment 8

[[e-mail] (away - not reading bugmail)]

Chak, Sean,

I was looking into what it would take to modify library loaders, since the App
Paths approach wouldn't support multiple GRE app installations depending on
different GRE versions.

I believe we might have touched upon this idea, although I'm not sure how
realistic it is.

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dllproc/base/loadlibraryex.asp
http://lxr.mozilla.org/seamonkey/source/xpcom/glue/standalone/nsXPCOMGlue.cpp#69
http://lxr.mozilla.org/seamonkey/source/nsprpub/pr/src/linking/prlink.c#536
http://lxr.mozilla.org/seamonkey/source/nsprpub/pr/src/linking/prlink.c#511
http://lxr.mozilla.org/seamonkey/source/nsprpub/pr/src/linking/prlink.c#618

Comment 9

[Doug Turner (:dougt)]

the system loads xpcom.dll.  your proposal to modify glue will not work since
most of the application directly links to xpcom for symbols not provided by glue.

Comment 10

[Chak Nanga]

We can get around the dll location issue without having to do App Paths etc:

* mozilla.exe should not directly link against xpcom libs
* Create a new utility dll which exports a bunch of functions such as
InitGecko(), UnInitGecko() etc. which are just wrappers around NS_InitXpCom()
etc. So, this dll directly links against the xpcom libs.
* When the app loads it modifies the app's PATH env var (via the
SetEnvironmentVariable() API) to include the GRE's path read from the registry
* The app then loads (via LoadLibrary) the intermediate utility dll(created
above). Once this library is loaded it successfully pulls in the correct
xpcom.dll, nspr.dll etc. from the correct location [based on the previously set
PATH]

Comment 11

[Doug Turner (:dougt)]

chak, do you realize that having mozilla.exe not directly link against xpcom
libs amounts to making mozilla.exe an embedding app.  If you going to sign up
for this work, don't hack it with a new library; instead fix up mozilla.exe to
be a proper embedding application.

Comment 12

[Chak Nanga]

Hi Doug...that was just a suggestion on how we could get around the issue of
locating/loading the right xpcom.dll when we have multiple GRE versions on a
machine.

Ok...I'm out of suggestions on how to accomplish this without having to make
mozilla.exe a true embedding app. Doug, any suggestions on how we can accomplish
this without going the embedding API usage route...thanks

Comment 13

[Sean Su]

actually chak, I was thinking that the following would be easier:

* instead of having nsAppRunner be compiled into mozilla.exe, have it be
compiled as nsAppRunner.dll (or something more mnemonic).
* create another main()/WinMain() in a new file to be compiled as mozilla.exe
* have this new mozilla.exe call LoadLibraryEx() on nsAppRunner.dll.

This will give us control over which copy/version of xpcom.dll to load while at
the same time not having to change the way we link against xpcom.dll.

Comment 14

[Doug Turner (:dougt)]

How about just putting version into the file name.  I don't really like it all
that much, but it is alot less hacky than what has bee proposed.

Comment 15

[Doug Turner (:dougt)]

Comment on attachment 106960 [details] [diff] [review]
patch v1.1

I would rather you create a function NS_CreateEmbedFileLocProvider which is XP,
then #ifdef inside of that.

Don't use nsCRT.

Why do you need these:
+#include "nsAppDirectoryServiceDefs.h"
+#include "nsDirectoryServiceDefs.h"

and this:
+#include <shlobj.h>

Move these comments above the variable declaration.  Also use the correct
variable name in the comment.:
+public:
+			 // productDirName is the name (not path) of the dir
+			 // in which the application registry and profiles
live.

Use NS_ENSURE_ARG_POINTER:

+nsresult
+NS_CreateWinEmbedFileLocProvider( nsIDirectoryServiceProvider **aResult ) {
+    if ( aResult ) {

Flip this around so that there isn't an else:

+	 winEmbedFileLocProvider *provider = new
winEmbedFileLocProvider("MozEmbed");
+	 if ( provider ) {
+	     *aResult = provider;
+	     NS_ADDREF( *aResult );
+	 } else {
+	     return NS_ERROR_OUT_OF_MEMORY;
+	 }

This strncpy is going to copy more then needed:

+    strncpy(mProductDirName, productDirName, sizeof(mProductDirName) - 1);

Just make mProductDirName a CString and be done with it.

Use only one space between <nsILocalFile> and localFile:
+	nsCOMPtr<nsILocalFile>	localFile;

nit: you only use these once, get rid of the #defines:
+#define COMPONENTS_DIR_NAME	NS_LITERAL_CSTRING("components")
+#define XPCOM_DLL_FILE 	NS_LITERAL_CSTRING("xpcom.dll")


nit:
+// If an embedding application is written to use an GRE it determines

use _a_ GRE



+// the compatible GRE's location by looking in the Windows registry

Windows's Registry. 

+// In this case GetGreDirectory(

In this case,

There are other mistakes in that comment block.  Lets get them cleaned up.


you don't need to assign   here:

+    nsresult rv = NS_ERROR_FAILURE;

You should put a big comment above 
winEmbedFileLocProvider::GetGreDirectory stating that XPCOM has already been
loaded by the OS.

Comment 16

[timeless]

-the Windows Registry- is correct, eg

Manipulating the Windows Registry with WFC
Manipulating the Windows Registry with WFC. Posted: November 16, 1999,
Download. ... currently being used. The Windows Registry. The registry is ...
msdn.microsoft.com/vjsharp/productinfo/visualj/
technical/tutorial/wfcregistry/default.asp

GetGreDirectory bothers me. I don't think it needs to be a helper function.

+  // it intends to be "run against" that GRE
Dougt's right, that paragraph needs a lot of work. Sentences should end with
periods....

don't use tabs (follow the modeline).
don't compare rv's to 0.

Comment 17

[Chak Nanga]

Besides the naming issues and commenting style being pointed out in the patch...

The logic which GetGreDirectory() uses to determine the location of a compatible
GRE will not work when we have multiple versions of Mozilla (potentially) using
multiple installed versions of GREs on the machine.

The current patch uses a two pronged approach to find the GRE location:
1. The Mozilla installer sets the App Paths registry key during installation
2. When Mozilla.exe is run it picks up xpcom.dll based on the location specified
in App Paths. GetGreDirectory() uses the dll location to determine the GRE location.

I'm not sure this is the right approach to be following.

Instead, the app should be looking in the registry for a version of the GRE it
is compatible with and use it when queried via the FileLocationProvider.

Comment 18

[Sean Su]

Chak, this patch does not address the issue of multiple GREs installed on the
system.  That will be a seperate patch which I am working on as well.

This fix is still needed to register the appropriate components from the loaded GRE.

Comment 19

[Sean Su]

Attachment: patch v1.2

>I would rather you create a function NS_CreateEmbedFileLocProvider which is
XP,
>then #ifdef inside of that.
done

>Don't use nsCRT.
done.

>Why do you need these:
>+#include "nsAppDirectoryServiceDefs.h"
>+#include "nsDirectoryServiceDefs.h"
>
>and this:
>+#include <shlobj.h>
nsDirectoryServiceDefs.h is required for NS_GRE_DIR and NS_GRE_COMPONENT_DIR.
The others are not needed, but were left over from the first patch.  They have
been removed.

>Move these comments above the variable declaration.  Also use the correct
>variable name in the comment.:
>+public:
>+			 // productDirName is the name (not path) of the dir
>+			 // in which the application registry and profiles
>live.
done.

>Use NS_ENSURE_ARG_POINTER:
>
>+nsresult
>+NS_CreateWinEmbedFileLocProvider( nsIDirectoryServiceProvider **aResult ) {
>+    if ( aResult ) {
>
>Flip this around so that there isn't an else:
>
>+	 winEmbedFileLocProvider *provider = new
>winEmbedFileLocProvider("MozEmbed");
>+	 if ( provider ) {
>+	     *aResult = provider;
>+	     NS_ADDREF( *aResult );
>+	 } else {
>+	     return NS_ERROR_OUT_OF_MEMORY;
>+	 }
done.

>This strncpy is going to copy more then needed:
>
>+    strncpy(mProductDirName, productDirName, sizeof(mProductDirName) - 1);
>
>Just make mProductDirName a CString and be done with it.
done.

>Use only one space between <nsILocalFile> and localFile:
>+	nsCOMPtr<nsILocalFile>	localFile;
done.

>nit: you only use these once, get rid of the #defines:
>+#define COMPONENTS_DIR_NAME	NS_LITERAL_CSTRING("components")
>+#define XPCOM_DLL_FILE	NS_LITERAL_CSTRING("xpcom.dll")
done.

>nit:
>+// If an embedding application is written to use an GRE it determines
>
>use _a_ GRE
>
>+// the compatible GRE's location by looking in the Windows registry
>
>Windows's Registry. 
>
>+// In this case GetGreDirectory(
>
>In this case,
>
>There are other mistakes in that comment block.  Lets get them cleaned up.
done.

>you don't need to assign   here:
>
>+    nsresult rv = NS_ERROR_FAILURE;
done.

>You should put a big comment above 
>winEmbedFileLocProvider::GetGreDirectory stating that XPCOM has already been
>loaded by the OS.
done.

Comment 20

[Doug Turner (:dougt)]

I would really like to see *one* directory service provider for all platforms
and just #ifdef the platform stuff in GetFile. 

Comment 21

[Sean Su]

Attachment: patch v1.3

there's only one directory service provider for all platforms now.

Comment 22

[timeless]

Comment on attachment 108255 [details] [diff] [review]
patch v1.3

new files should be MPL/... tri licensed, they should not have (c) 1998. and
you should get a template from the website, i think the one you want is
http://www.mozilla.org/MPL/boilerplate-1.1/mpl-tri-license-c

don't use tabs!

>+  // Note that by returning a valid localFile's for NS_GRE_DIR and
>+  // NS_GRE_COMPONENT_DIR, your app is indicating to XPCOM that 
>+  // it found a GRE version with which it's compatible with and 
drop the second with
>+  // it intends to be "run against" that GRE.
why not 'intends to run using that GRE' ?
>+  //
>+  // Please see http://www.mozilla.org/projects/embedding/MRE.html
>+  // for more info. on GRE.
drop the first .
>+  //---------------------------------------------------------------
>+  if (nsCRT::strcmp(prop, NS_GRE_DIR) == 0)
drop nsCRT::

>+  else if (nsCRT::strcmp(prop, NS_GRE_COMPONENT_DIR) == 0)
drop nsCRT::

>+	if (localFile && NS_SUCCEEDED(rv))
>+		return localFile->QueryInterface(NS_GET_IID(nsIFile), (void**)_retval);
>+		
>+	return rv;
i'd prefer:
if (!localFile || NS_FAILED(rv))
  return rv;

return localFile->QueryInterface(NS_GET_IID(nsIFile), (void**)_retval);


>+// Create and return the location of the GRE the application is 
Create and return the location that the GRE application is 
>+// currently using, if any, via the |aLocalFile| param.
>+//
>+// The location of GRE at this point is the same directory as where
'as where' isn't great
>+// xpcom.dll is at.  We just need to query for the path of where
>+// xpcom.dll is at.
don't end sentences with prepositions [at].
>+//
>+// ** Xpcom is already loaded by the time we get here **
i don't think we write XPCOM that way.

>+NS_METHOD greEmbedFileLocProvider::GetGreDirectory(nsILocalFile **aLocalFile)
>+{
>+  NS_ENSURE_ARG_POINTER(aLocalFile);
>+  nsresult rv = NS_ERROR_FAILURE;
>+  nsCOMPtr<nsILocalFile> xpcomFile;
>+  nsCOMPtr<nsIFile> xpcomParent;
>+  char buffer[ _MAX_PATH ];
>+  HMODULE xpcomDllHandle = NULL;
>+  nsCOMPtr<nsILocalFile> tempLocal;
>+
>+#ifdef XP_PC
>+  xpcomDllHandle = ::GetModuleHandle("xpcom.dll");
>+  rv = ::GetModuleFileName( xpcomDllHandle, buffer, sizeof buffer );
err, GetModuleFileName doesn't return a nsresult, please use some other
variable.
mkaply doesn't like XP_PC. Use XP_WIN32.
>+  if(rv == 0)
after the change, this:
>+    return NS_ERROR_FAILURE;
can become return rv;
>+
>+  rv = NS_NewNativeLocalFile(nsDependentCString(buffer), TRUE, getter_AddRefs(xpcomFile));
>+  if (NS_FAILED(rv))
>+    return(rv);
>+
>+  xpcomFile->GetParent(getter_AddRefs(xpcomParent));
>+  nsCAutoString target;
>+
>+  xpcomParent->GetNativeTarget(target);
>+  rv = NS_NewNativeLocalFile(target, TRUE, getter_AddRefs(tempLocal));
>+  if(NS_SUCCEEDED(rv) && tempLocal)
again, early returns are preferred.
>+  {
>+    *aLocalFile = tempLocal;
>+    NS_ADDREF(*aLocalFile);
>+    rv = NS_OK;
>+  }
>+#endif
>+  return rv;
>+}

>Index: nsGREServiceProvider.h
>+#ifndef nsGREServiceProvider_h_
>+#define nsGREServiceProvider_h_
there's no wrapping to protect non windows clients from this windows stuff:
>+#include <windows.h>
>+#include <shellapi.h>
>+#include <ddeml.h>

Comment 23

[Sean Su]

Attachment: patch v1.4

Got pulled off to work on something else last week.  I'm back on this bug
again.

this patch has (almost all) the fixes timeless pointed out needed to be done. 
comments have been updated, error checks changed, etc...

I didn't agree with some minor ones, but the bulk of it has been addressed.

Comment 24

[Doug Turner (:dougt)]

save us the trouble and tell us which problems timeless pointed out you felt
where  too minor to fix?  

Comment 25

[Sean Su]

right, that would help wouldn't it? :)

didn't make all the changes to the grammar in the comments he suggested, and
also didn't change to call "return rv;" in the following code:

 +  if(!GetModuleFileName(xpcomDllHandle, buffer, sizeof buffer))
 +    return NS_ERROR_FAILURE;

because someone else could add code to change the value of rv above this
statement, which then could invalidate rv for this purpose.

oh and xpcom.dll is implicitly loaded the by time GetGreDirectory() is called. 
that is another problem altogether that I'm trying to solve seperately.

Comment 26

[timeless]

Comment on attachment 108974 [details] [diff] [review]
patch v1.4

let's see if i can avoid contradicting myself :)

+  rv = NS_InitXPCOM2(nsnull, nsnull, provider);
who owns provider? it looks like you do (dougt please correct me if i'm wrong)
in which case you're leaking it.

+ * Portions created by the Initial Developer are Copyright (C) 2001
The year is still wrong (multiple times), please fix it. It's 2002 right now,
and you should hurry, if it takes too long you might have to change it again
;-)

Change Xpcom to xpcom

+  HMODULE xpcomDllHandle = NULL;
is win32 specific, please guard it.

imo greEmbedFileLocProvider::GetGreDirectory
should return NS_ERROR_NOT_AVAILABLE or NS_ERROR_NOT_IMPLEMENTED for other
platforms...

+  xpcomFile->GetParent(getter_AddRefs(xpcomParent));
+  nsCAutoString target;
+
+  xpcomParent->GetNativeTarget(target);
if getParent fails, you crash...

+  rv = NS_NewNativeLocalFile(target, TRUE, getter_AddRefs(tempLocal));
wrong bool type:

192 NS_NewNativeLocalFile(const nsACString &path, 
193		       -> PRBool followLinks, 
194			  nsILocalFile* *result);

I'm pretty sure the rest is fine, but you should at least have dougt look
through this once.

Comment 27

[Sean Su]

Attachment: patch v1.5

timeless, good catch on the leak and potential crashers.

this new patch addresses all of timeless' comments.  changed the year to 2003
too.

Comment 28

[Daniel Veditz [:dveditz]]

Please change the year to 2002, that's when this is being created and you've
posted it here to this bug as proof of that. Lying about a date in the future is
just as bad as lying about it in the past, possibly worse because it might look
like we're trying to cheat and get more years of protection.

Comment 29

[Doug Turner (:dougt)]

do not check patch 1.5 in.  I am working on a much better patch to solve this
problem.

Comment 30

[Sean Su]

over to dougt

Comment 31

[Doug Turner (:dougt)]

Attachment: patch v2.0

Comment 32

[Darin Fisher]

Comment on attachment 111656 [details] [diff] [review]
patch v2.0

>Index: nsNativeAppSupportWin.cpp

>+#ifdef DEBUG_dougt
>+    *aResult = PR_TRUE;
>+    return NS_OK;
>+#endif

do we want this enabled for other users?

>Index: nsSigHandlers.cpp

>-    PR_GetCurrentThread();

> ah_crap_handler(int signum)
> {
>-  PR_GetCurrentThread();

seems wierd that these would have ever been there.  are you sure
there isn't some side-effect as a result of these calls??


>Index: nsStackFrameUnix.cpp

is this stack walking code new, or are you just moving it around?

Comment 33

[Chak Nanga]

Comment on attachment 111656 [details] [diff] [review]
patch v2.0

Guessing the nsStackFrameUnix.* files(included in this patch) are for a
different bug?

Comment 34

[Darin Fisher]

dougt said he was going to backout the PR_GetCurrentThread changes just in case
they do matter.  he also says that he and seawood agreed to use makefile fu to
copy the stack walker code at build time instead of adding a duplicate copy of
the code in xpfe/bootstrap.

Comment 35

[Chak Nanga]

Comment on attachment 111656 [details] [diff] [review]
patch v2.0

r=chak [assuming the changes doug/darin say in #34 gets made]

Comment 36

[Chak Nanga]

Comment on attachment 111656 [details] [diff] [review]
patch v2.0

r=chak [assuming the changes doug/darin say in #34 get made]

Comment 37

[Doug Turner (:dougt)]

Attachment: patch v2.1

Comment 38

[Doug Turner (:dougt)]

Attachment: patch v2.0

Comment 39

[Darin Fisher]

Comment on attachment 111663 [details] [diff] [review]
patch v2.0

sr=darin

Comment 40

[Chris McAfee]

Comment on attachment 111663 [details] [diff] [review]
patch v2.0

patch v2.0 has some lossage on line 275, looks like some part of the patch for
nsNativeAppSupportWin.cpp got pieced together wrong.  Also, lose the ^M's.  If
I can get another patch here I can test this.

Comment 41

[Doug Turner (:dougt)]

cls had a problem apply the patch, but I didn't.  I also don't think ccarlen had
a problem.  

Comment 42

[Chak Nanga]

Comment on attachment 111663 [details] [diff] [review]
patch v2.0

r=chak

Comment 43

[Doug Turner (:dougt)]

dispite the attachment names, the last patch is the one that we are going with...

Comment 44

[Doug Turner (:dougt)]

ignore my comment.  i didn't have any coffee this morning.  

Comment 45

[Doug Turner (:dougt)]

Checking in Makefile.in;
/cvsroot/mozilla/xpfe/bootstrap/Makefile.in,v  <--  Makefile.in
new revision: 1.213; previous revision: 1.212
done
Checking in nsAppRunner.cpp;
/cvsroot/mozilla/xpfe/bootstrap/nsAppRunner.cpp,v  <--  nsAppRunner.cpp
new revision: 1.381; previous revision: 1.380
done
Checking in nsNativeAppSupportOS2.cpp;
/cvsroot/mozilla/xpfe/bootstrap/nsNativeAppSupportOS2.cpp,v  <-- 
nsNativeAppSupportOS2.cpp
new revision: 1.37; previous revision: 1.36
done
Checking in nsNativeAppSupportWin.cpp;
/cvsroot/mozilla/xpfe/bootstrap/nsNativeAppSupportWin.cpp,v  <-- 
nsNativeAppSupportWin.cpp
new revision: 1.88; previous revision: 1.87
done
Checking in nsSigHandlers.cpp;
/cvsroot/mozilla/xpfe/bootstrap/nsSigHandlers.cpp,v  <--  nsSigHandlers.cpp
new revision: 1.29; previous revision: 1.28
done

Marking as fixed.  

Comment 46

[Nathan Kurz]

I apologize if this is the wrong place to report this, but the checking of this
 patch (IMHO) breaks the build.  Well, my build at least:

 make[3]: Entering directory `/usr/local/src/phoenix/mozilla/toolkit/xre'
 nsSigHandlers.cpp
 c++ -o nsSigHandlers.o -c -DOSTYPE=\"Linux2.4\" -DOSARCH=\"Linux\" -DOJI   
 -I../../xpfe/bootstrap -I../../dist/include/appcomps 
 -I../../dist/include/appshell -I../../dist/include/chrome 
 -I../../dist/include/docshell -I../../dist/include/dom 
 -I../../dist/include/embed_base -I../../dist/include/embedcomponents 
 -I../../dist/include/gfx -I../../dist/include/intl -I../../dist/include/locale 
 -I../../dist/include/necko -I../../dist/include/pref 
 -I../../dist/include/profile -I../../dist/include/string 
 -I../../dist/include/uconv -I../../dist/include/uriloader 
 -I../../dist/include/webbrwsr -I../../dist/include/widget 
 -I../../dist/include/windowwatcher -I../../dist/include/xpcom 
 -I../../dist/include/xpconnect -I../../dist/include/xpinstall 
 -I../../dist/include/xremoteservice -I../../dist/include/xulapp 
 -I../../dist/include -I/usr/local/src/phoenix/mozilla/dist/include/nspr      
 -I/usr/X11R6/include   -fPIC  -I/usr/X11R6/include -fno-rtti -fno-exceptions 
 -Wall -Wconversion -Wpointer-arith -Wcast-align -Woverloaded-virtual -Wsynth 
 -Wno-ctor-dtor-privacy -pedantic -Wno-long-long -fshort-wchar -pthread -pipe  
 -DDEBUG -D_DEBUG -DDEBUG_root -DTRACING -g -fno-inline -I/usr/include/gtk-1.2 
 -I/usr/include/glib-1.2 -I/usr/lib/glib/include -I/usr/X11R6/include  
 -I/usr/X11R6/include -DMOZILLA_CLIENT -include ../../mozilla-config.h 
 -Wp,-MD,.deps/nsSigHandlers.pp nsSigHandlers.cpp

 nsSigHandlers.cpp:117:30: nsStackFrameUnix.h: No such file or directory
 nsSigHandlers.cpp: In function `void ah_crap_handler(int)':
 nsSigHandlers.cpp:138: `DumpStackToFile' undeclared (first use this function)
 nsSigHandlers.cpp:138: (Each undeclared identifier is reported only once for 
    each function it appears in.)
 make[3]: *** [nsSigHandlers.o] Error 1
 make[3]: Leaving directory `/usr/local/src/phoenix/mozilla/toolkit/xre'

It's possible that this is only a local problem (I just upgraded to gcc 3.2.1),
but someone should take a look at this as soon as possible. 

Patch follows.

Comment 47

[Nathan Kurz]

Attachment: Stop the Breakage: patch to Makefile.in

It could just be me, but my builds are broken by the checkin for this bug. 
Here's a patch that allowed me to compile again.  Can't say if its the right
patch.

Comment 48

[Christian :Biesinger (don't email me, ping me on IRC)]

reopening bug, given that this broke a build, appparently (comment 46)

Comment 49

[Doug Turner (:dougt)]

did you try make export in xpfe/bootstrap?

do not reopen this bug unless we back out these patch, rather open new bugs.

Comment 50

[Robert Kaiser]

This checkin has caused bug 189839 (-remote doesn't return the correct return
values), see there for details.

Comment 51

[Nathan Kurz]

DougT writes:
> did you try make export in xpfe/bootstrap?

Yes, I tried doing that.  Nothing changed until I patched Makefile.in to add
nsStackFrameUnix.h to the list of exports ;).  Hence the patch. I apologize for
not filing a new bug.  I wasn't sure if nsStackFrameUnix.h was supposed to be
exported.  I will file a bug now.  Could you fix it in the meantime? :)