Closed Bug 304239 Opened 14 years ago Closed 14 years ago

Fix issues in NSPR's real_install makefile target

Categories

(NSPR :: NSPR, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: wtc, Assigned: wtc)

References

Details

Attachments

(4 files, 1 obsolete file)

I found three issues while playing with NSPR's real_install
makefile target.

1. For NSPR 4.6, "nspr-config --version" reports "4.6.0".  I
think "4.6" is more desirable.

2. real_install installs the private headers under the "md"
subdirectory.  This is wrong.

3. Now that the "install" makefile target is obsolete, we
should remove it and rename "real_install" as "install".
Will dropping the 0 patch level break any script (e.g.,
mozilla's --with-system-nspr configure option)?
Attachment #192310 - Flags: superreview?(cls)
Attachment #192310 - Flags: review?(caillon)
Comment on attachment 192311 [details] [diff] [review]
Don't install the headers in nsprpub/pr/include/md, except for prcpucfg.h

If real_install shouldn't install the md files, neitehr should the export
target.  Those two targets should be in sync regarding what gets installed.  

However, in both cases, I think we need to install the md files to support
cross-compiling.  When building the host version of xpidl, at least, we use
those files directly instead of using prcpucfg.h, which is only set up for the
target.  

http://lxr.mozilla.org/seamonkey/search?string=HOST_NSPR_MDCPUCFG
Attachment #192311 - Flags: review?(cls) → review-
Comment on attachment 192310 [details] [diff] [review]
Report NSPR 4.6's version as "4.6" instead of "4.6.0"

I don't like this kind of special casing.  And the checks in nspr.m4 & values
in mozilla-nspr.pc would need to be updated as well.

This change shouldn't break Mozilla's system-nspr checks because we don't
really do strict version checking,  It could break someone else's check that
does though.
Attachment #192310 - Flags: superreview?(cls) → superreview-
Attachment #192310 - Flags: review?(caillon)
cls, I am so glad I asked you to review the patch. I didn't
realize that's how we compile the host's xpidl when we cross
compile mozilla.  So is prcpucfg.h always the target's .cfg
file?

In this new patch, I continue to export and install *.cfg
from nsprpub/pr/include/md, but I don't export and install
the *.h files because they are private headers.

I found that the real_install target installs $(HEADERS),
which I believe is a mistake.  It should install $(CONFIGS).
Attachment #192311 - Attachment is obsolete: true
Attachment #192318 - Flags: review?(cls)
cls:

I have a question about what we need from NSPR
for the host platform when cross-compiling Mozilla.
Do you just need the headers or do you also need
the libraries?  I found that xpidl is not linked
with any NSPR library, so I suspect you only need
the NSPR headers for the host platform when
cross-compiling Mozilla.

I'm also wondering if cross-compiling Mozilla with a
pre-built NSPR really works. It seems that we can do this
easily if the pre-built NSPR is for the target platform
and the host platform only needs the headers.  Otherwise,
we will need to provide two sets of libraries: one for the
host and one for the target.

If my understanding is correct, then we can't cross-compile
Mozilla with the --with-system-nspr option, because by
definition system NSPR is for the host platform.  It seems
that we need to use --with-nspr=arg to point to a pre-built
NSPR for the target platform.

Finally, I'm wondering if we can make xpidl not use
NSPR headers.  I suspect xpidl only needs some typedefs
from prtypes.h (most likely the fixed size integer
types such as PRInt32 and PRInt64).

In any case, I don't mind installing the *.cfg files
as long as they can really be used to cross-compile
Mozilla.
Comment on attachment 192320 [details] [diff] [review]
Remove the obsolete install makefile target and rename real_install as install

We'll need to update mozilla/Makefile.in as well when this lands.
Attachment #192320 - Flags: review?(cls) → review+
*** Bug 290724 has been marked as a duplicate of this bug. ***
Attachment #202203 - Flags: review?(wtchang)
Comment on attachment 202203 [details] [diff] [review]
Update mozilla uses of real_install target

r=wtc.
Attachment #202203 - Flags: review?(wtchang) → review+
Attachment #192318 - Flags: review?(cls) → review+
> I have a question about what we need from NSPR
> for the host platform when cross-compiling Mozilla.
> Do you just need the headers or do you also need
> the libraries?  I found that xpidl is not linked
> with any NSPR library, so I suspect you only need
> the NSPR headers for the host platform when
> cross-compiling Mozilla.

Correct.  We only use the NSPR headers when building host versions of xpidl & the mar tools.

> I'm also wondering if cross-compiling Mozilla with a
> pre-built NSPR really works. It seems that we can do this
> easily if the pre-built NSPR is for the target platform
> and the host platform only needs the headers.  Otherwise,
> we will need to provide two sets of libraries: one for the
> host and one for the target.

Since we do not link against the NSPR libraries when building the host versions of certain build tools, we do not need to build NSPR twice.

> If my understanding is correct, then we can't cross-compile
> Mozilla with the --with-system-nspr option, because by
> definition system NSPR is for the host platform.  It seems
> that we need to use --with-nspr=arg to point to a pre-built
> NSPR for the target platform.

A system NSPR install really means just means a prebuilt NSPR installation. 
--with-system-nspr is meant to be used when building the target libraries and it can be used for cross-compiling as well.
  
> Finally, I'm wondering if we can make xpidl not use
> NSPR headers.  I suspect xpidl only needs some typedefs
> from prtypes.h (most likely the fixed size integer
> types such as PRInt32 and PRInt64).

That appears to be the case.  JS & libmar also use the NSPR types and have header dependencies (at least when cross-compiling).  I'm not sure how practical it would be to remove those dependencies.
The patches have been checked into the NSPR client branch & the NSPR & mozilla trunks.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
I verified that all the reviewed patches were checked in
on the necessary trunks and branches correctly.  I also
verified that the 'install' target works.  Thanks a lot
for your help, cls.
Status: RESOLVED → VERIFIED
Whiteboard: [4.7]
Whiteboard: [4.7]
Target Milestone: --- → 4.7
You need to log in before you can comment on or make changes to this bug.