304239 - Fix issues in NSPR's real_install makefile target

Status: VERIFIED FIXED

(NSPR :: NSPR, defect)

Description

[Wan-Teh Chang]

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

Comment 1

[Wan-Teh Chang]

Attachment: Report NSPR 4.6's version as "4.6" instead of "4.6.0"

Will dropping the 0 patch level break any script (e.g.,
mozilla's --with-system-nspr configure option)?

Comment 2

[Wan-Teh Chang]

Attachment: Don't install the headers in nsprpub/pr/include/md, except for prcpucfg.h

Comment 3

[cls]

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

Comment 4

[cls]

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.

Comment 5

[Wan-Teh Chang]

Attachment: Only export and install the *.cfg files from nsprpub/pr/include/md (for cross-compilation support)

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

Comment 6

[Wan-Teh Chang]

Attachment: Remove the obsolete install makefile target and rename real_install as install

Comment 7

[Wan-Teh Chang]

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 8

[cls]

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.

Comment 9

[cls]

*** Bug 290724 has been marked as a duplicate of this bug. ***

Comment 10

[cls]

Attachment: Update mozilla uses of real_install target

Comment 11

[Wan-Teh Chang]

Comment on attachment 202203 [details] [diff] [review]
Update mozilla uses of real_install target

r=wtc.

Comment 12

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

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.

Comment 13

[cls]

The patches have been checked into the NSPR client branch & the NSPR & mozilla trunks.

Comment 14

[Wan-Teh Chang]

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.