88283 - LDFLAGS not used for nsinstall.c on IRIX

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P3)

Description

[John Mark Vandenberg]

SGI uses ROOT's and TOOLROOT's to build products against, and hence the need 
for all the build to use CFLAGS, LDFLAGS, etc (or equivalent should these be 
reserved).

Comment 1

[John Mark Vandenberg]

Attachment: nsprpub/config/Makefile.in : Patch to use @LDFLAGS@

Comment 2

[cls]

Should use $(LDFLAGS) not @LDFLAGS@. 

Comment 3

[John Mark Vandenberg]

Attachment: nsprpub/config/Makefile.in : Patch to use $(LDFLAGS)

Comment 4

[cls]

r=cls

Comment 5

[Wan-Teh Chang]

This patch should not be checked in without further
investigation.

John, unless I missed something, this is a style issue
because I can build NSPR on IRIX without this patch.

The philosophy of NSPR's build system is that simplicity
has priority over flexibility.  Some people disagree with
this philosophy, but they either have excellent memory or
only build on one or two platforms regularly.

If you think NSPR should use some compiler or linker
flags on IRIX, we should add them to our makefile so that
everyone building on IRIX can benefit from your knowledge.

On the other hand, the makefile in question builds two
tools ('nsinstall' and 'now') that are only used during the
build, so we may be wasting our time discussing something
that won't affect the end products (NSPR headers and .so's)
anyway.

I would resolve this bug WONTFIX.

Comment 6

[John Mark Vandenberg]

Yes I can build NSPR without this patch, but not when there is no compiler 
installed on the system.  In this case I use a 'toolroot' which is version 
controlled set of tools in some hard to reach path.  In order to do this, I 
must add -L paths within the 'toolroot' to crt1.o, and also -I paths to system 
headers.  By doing this I can start working towards supporting mozilla on more 
versions of IRIX than 6.5.latest which is generally what my build machine is.

As these paths change as the toolset compiled against changes, I can not add 
magic flags to the Makefiles to fix this problem.  Nor would these flags assist 
anyone else.  However to use a generic LDFLAGS could benifit others to 
integrate NSPR into there own build system.

This is why I initially used @LDFLAGS@, which means there are no variation 
after configure.  $(LDFLAGS) seems less precise.

To conclude, I am happy with WONTFIX, as I could build now and nsinstall 
separately, and still achieve the same result, however as the rest of NSPR 
seems to honour LDFLAGS, it seemed logical that these utilities did as well.

Comment 7

[cls]

Wan-Teh, this is one of those situations where the build system is making an
assumption about the location of things on the system and does not properly
handle  any variation of the system.  These changes are harmless for the default
build.  The user will have to explicitly set LDFLAGS when running configure to
use these changes.  I don't see any inherent harm in checking in the patch.

Comment 8

[Wan-Teh Chang]

John Vandenberg wrote:
>
> ... however as the rest of NSPR seems to honour LDFLAGS,
> it seemed logical that these utilities did as well.

It is actually the opposite.  I don't know of a place in
NSPR where we use LDFLAGS.  Instead, we use variables like
XLDOPTS (nsprpub/config/Makefile.in) and LDOPTS
(nsprpub/pr/tests/Makefile.in).

The only reference to LDFLAGS is a rule in
nsprpub/config/rules.mk to build $(PROGRAM) from $(OBJS),
but that rule is not used.  Since I don't know what
LDFLAGS means in NSPR, I can't check in your patch
yet.  I want to define what LDFLAGS means in NSPR first.
Then, we will (one platform at a time) replace XLDOPTS
and LDOPTS by LDFLAGS, and your patch would be a first
step towards that.

John, Chris, does this sound like a reasonable plan?

Comment 9

[John Mark Vandenberg]

There is no hurry with this bug as this should not affect others.  I am quite 
happy to use XLDOPTS rather than LDFLAGS ... or any other environment variable 
for that matter ... I just need to be able to supply variable information to 
the compiler/linker.

Comment 10

[cls]

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

Comment 11

[hacker formerly known as [e-mail]]

Looking at this again, it appears as though we do use $(LDFLAGS) in NSPR. 
rules.mk shows it being used in both the $(PROGRAM) & $(SHARED_LIBRARY) rules.
{X,}LDOPTS appear to only be used in localize Makefile rules.  I still see
nothing wrong with this patch as is.

Comment 12

[hacker formerly known as [e-mail]]

Over to NSPR.

Comment 13

[hacker formerly known as [e-mail]]

->seawood.

wtc: any further thoughts on this one?

Comment 14

[hacker formerly known as [e-mail]]

Attachment: Add $(LDFLAGS) to custom nsinstall compile line.

Comment 15

[Wan-Teh Chang]

Comment on attachment 108994 [details] [diff] [review]
Add $(LDFLAGS) to custom nsinstall compile line.

r=wtc.

Comment 16

[hacker formerly known as [e-mail]]

Patch has been checked into the NSPR trunk & the client branch.