180294 - OpenVMS build -> GNV: NSS Build Changes

Status: RESOLVED FIXED

(NSS :: Build, defect, P1)

Description

[Colin Blake]

This bug report will track the NSS build changes associated with the "move OpenVMS
build to GNV" work (meta-bug 180288).

Comment 1

[Colin Blake]

Attachment: NSS Build changes

Comment 2

[Colin Blake]

This is blocking my work on Mozilla 1.3. Setting severity field according.

Comment 3

[Wan-Teh Chang]

Comment on attachment 106356 [details] [diff] [review]
NSS Build changes

>Index: security/coreconf/OpenVMS.mk
>===================================================================
>RCS file: /cvsroot/mozilla/security/coreconf/OpenVMS.mk,v
>retrieving revision 1.5
>diff -u -r1.5 OpenVMS.mk
>--- security/coreconf/OpenVMS.mk	15 Feb 2002 22:53:12 -0000	1.5
>+++ security/coreconf/OpenVMS.mk	15 Nov 2002 12:10:33 -0000
>@@ -21,24 +21,10 @@
> 
> include $(CORE_DEPTH)/coreconf/UNIX.mk
> 
>-ifdef INTERNAL_TOOLS
>-CC			= c89
>-CCC			= cxx
>-OPTIMIZER		= -O
>-else
>-CC			= ccc
>-CCC			= ccc
>-endif

It seems like you are relying on GNU make's default value for
CC (cc).  (NSS does not have C++ files so the CCC value in
security/coreconf is ignored by NSS.)

If so, I think it is a good idea to still set CC and CCC
explicitly in this file.

>-RANLIB			= /bin/true
>+RANLIB			= /gnu/bin/true

You can also use the 'echo' command, if it exists.

>Index: security/coreconf/rules.mk
>===================================================================
>RCS file: /cvsroot/mozilla/security/coreconf/rules.mk,v
>retrieving revision 1.41
>diff -u -r1.41 rules.mk
>--- security/coreconf/rules.mk	5 Sep 2002 01:21:35 -0000	1.41
>+++ security/coreconf/rules.mk	15 Nov 2002 12:10:33 -0000
>@@ -71,7 +71,7 @@
> 
> import::
> 	@echo "== import.pl =="
>-	@perl -I$(CORE_DEPTH)/coreconf $(CORE_DEPTH)/coreconf/import.pl \
>+	@$(PERL) -I$(CORE_DEPTH)/coreconf $(CORE_DEPTH)/coreconf/import.pl \
> 		"RELEASE_TREE=$(RELEASE_TREE)"   \
> 		"IMPORTS=$(IMPORTS)"             \
> 		"VERSION=$(VERSION)" \

There is an open bug (bug 82268) on this using the PERL variable.
I can't make this change (and the other similar changes in this file)
right now.

>Index: security/coreconf/nsinstall/nsinstall.c

I think your changes to this file can be replaced by undefining the
HAVE_FCHMOD macro.  Look for BEOS in this file as an example.

Comment 4

[Colin Blake]

Wan-Teh,

My comments are prefixed with CRB:

>Index: security/coreconf/OpenVMS.mk
>===================================================================
>RCS file: /cvsroot/mozilla/security/coreconf/OpenVMS.mk,v
>retrieving revision 1.5
>diff -u -r1.5 OpenVMS.mk
>--- security/coreconf/OpenVMS.mk	15 Feb 2002 22:53:12 -0000	1.5
>+++ security/coreconf/OpenVMS.mk	15 Nov 2002 12:10:33 -0000
>@@ -21,24 +21,10 @@
> 
> include $(CORE_DEPTH)/coreconf/UNIX.mk
> 
>-ifdef INTERNAL_TOOLS
>-CC			= c89
>-CCC			= cxx
>-OPTIMIZER		= -O
>-else
>-CC			= ccc
>-CCC			= ccc
>-endif

It seems like you are relying on GNU make's default value for
CC (cc).  (NSS does not have C++ files so the CCC value in
security/coreconf is ignored by NSS.)

If so, I think it is a good idea to still set CC and CCC
explicitly in this file.

CRB: Yes, I'm relying on defaults. You really think that I
CRB: should explicitly define defaults?

>-RANLIB			= /bin/true
>+RANLIB			= /gnu/bin/true

You can also use the 'echo' command, if it exists.

CRB: OK, will do. What about the "noise" in the output?

>Index: security/coreconf/rules.mk
>===================================================================
>RCS file: /cvsroot/mozilla/security/coreconf/rules.mk,v
>retrieving revision 1.41
>diff -u -r1.41 rules.mk
>--- security/coreconf/rules.mk	5 Sep 2002 01:21:35 -0000	1.41
>+++ security/coreconf/rules.mk	15 Nov 2002 12:10:33 -0000
>@@ -71,7 +71,7 @@
> 
> import::
> 	@echo "== import.pl =="
>-	@perl -I$(CORE_DEPTH)/coreconf $(CORE_DEPTH)/coreconf/import.pl \
>+	@$(PERL) -I$(CORE_DEPTH)/coreconf $(CORE_DEPTH)/coreconf/import.pl \
> 		"RELEASE_TREE=$(RELEASE_TREE)"   \
> 		"IMPORTS=$(IMPORTS)"             \
> 		"VERSION=$(VERSION)" \

There is an open bug (bug 82268) on this using the PERL variable.
I can't make this change (and the other similar changes in this file)
right now.

CRB: When will you be able to make this fix?

>Index: security/coreconf/nsinstall/nsinstall.c

I think your changes to this file can be replaced by undefining the
HAVE_FCHMOD macro.  Look for BEOS in this file as an example.

CRB: Same comments as in bug 180293. There are two problems that I am
CRB: fixing. First, I need to do the chmod AFTER the file is closed,
CRB: otherwise it fails because the file is open. Second, I need to do
CRB: the utime last because a chmod on OpenVMS will update the file's
CRB: timestamps.

Comment 5

[Colin Blake]

Wan-Teh,

If you could respond to my comments on your initial reviews, I'll update and
repost the patch.

Comment 6

[Wan-Teh Chang]

Colin:

1. Yes, please explicitly define CC and CCC, even if
their values are the same as the default.  This is for
documentation purposes.

2. I was just worried that /gnu/bin/true looks like a
third-party tool that one needs to install in order to
build Mozilla.  If /gnu/bin/true is always installed on
OpenVMS, you don't need to define RANLIB as 'echo'.  By
the way, why can't you use /bin/true?

Yes, the "noise" in the output is a little annoying.

3. I am planning to fix bug 82268 (replace hardcoded 'perl'
by $(PERL)) in NSS 3.8 (March 2003 timeframe).  It has a
low priority though so I may not get around to it.

4. In nsinstall.c, I suggest we do things in this order
to avoid the VMS ifdef's.

    ftruncate
#ifdef HAVE_FCHMOD
    fchmod
#endif
    fchown
    close
    /*
     * On OpenVMS chmod can't be done while the file
     * is open.
     */
#ifndef HAVE_FCHMOD
    chmod
#endif
    utime

Note that your patch (attachment 106356 [details] [diff] [review]) does the
fchown on tofd after tofd is closed, which is wrong.

We should ask Brendan Eich to review the nsinstall.c
changes when they are ready.

Comment 7

[Colin Blake]

> 1. Yes, please explicitly define CC and CCC, even if
> their values are the same as the default.  This is for
> documentation purposes.

OK.

> 2. I was just worried that /gnu/bin/true looks like a
> third-party tool that one needs to install in order to
> build Mozilla.  If /gnu/bin/true is always installed on
> OpenVMS, you don't need to define RANLIB as 'echo'.  By
> the way, why can't you use /bin/true?

/gnu/bin/true is part of the GNV distribution on OpenVMS. So its always there.

Why isn't it /bin/true? Because /bin isn't where the true image resides. In
GNV's bash environment on OpenVMS, /bin points to the location where all the
standard OpenVMS "binaries" live (the OpenVMS binaries which ship with the O/S),
and /gnu/bin points to the location where all the GNV images (such as bash and
all the utilities) reside.

> Yes, the "noise" in the output is a little annoying.

For this reason I'd like to stick with /gnu/bin/true, now that I've hopefully
satisfied your concern about the validity of it.

> 3. I am planning to fix bug 82268 (replace hardcoded 'perl'
> by $(PERL)) in NSS 3.8 (March 2003 timeframe).  It has a
> low priority though so I may not get around to it.

And in the mean time, what do I do?

Doesn't the fact that the OpenVMS build is busted without this fix increase its
priority at all :-)

> 4. In nsinstall.c, I suggest we do things in this order
> to avoid the VMS ifdef's.

As with NSPR. Will do.

> Note that your patch (attachment 106356 [details] [diff] [review]) does the
> fchown on tofd after tofd is closed, which is wrong.

Correct. Oversight on my part. The change will fix this problem though.

> We should ask Brendan Eich to review the nsinstall.c
> changes when they are ready.

Will do.

Comment 8

[Colin Blake]

Attachment: Revised patch

New patch containing all discussed changes.

Comment 9

[Colin Blake]

Attachment: New nsinstall.c

In line with Brendan's comments in bug 180293, I'm submitting a revised patch
for nsinstall.c. This one puts the non-VMS code back to what it was, and only
changes the order for the VMS code path.

Comment 10

[Colin Blake]

Comment on attachment 108775 [details] [diff] [review]
New nsinstall.c

Forget this patch. The similar patch in bug 180293 is still getting some
action.

Comment 11

[Colin Blake]

Attachment: New patch for nsinstall.c

Only change the order of things for OpenVMS. As per bug 180293.

Comment 12

[Wan-Teh Chang]

Comment on attachment 108802 [details] [diff] [review]
New patch for nsinstall.c

r=wtc.

Comment 13

[Wan-Teh Chang]

Comment on attachment 108754 [details] [diff] [review]
Revised patch

r=wtc.

Comment 14

[Colin Blake]

Wan-Teh, what's the next step here? Who super-reviews NSS stuff? I don't see NSS
mentioned on the superreview page. Should I ask Brendan since he's listed as a
catch-all?

Comment 15

[Wan-Teh Chang]

Colin: superreview is not required for NSS patches.

Comment 16

[Wan-Teh Chang]

Attachment: Combined patch

1. Combined attachment 108754 [details] [diff] [review] and 108802 into one patch.

2. Changed CXX to CCC.	NSS uses the make variable CCC as
the C++ compiler.

3. Added -DVMS back.

4. Other minor editing.

Colin, please review this patch.  Thanks.

Comment 17

[Colin Blake]

That's fine by me. Will you be able to check this one in once we get approval,
as I don't have a patch utility?

Comment 18

[Wan-Teh Chang]

The latest patch has been checked into the trunk,
NSS_3_7_BRANCH, and NSS_CLIENT_TAG of NSS.

Comment 19

[Asa Dotzler [:asa]]

Comment on attachment 108754 [details] [diff] [review]
Revised patch

1.3a is complete. not taking further patches.

Comment 20

[Asa Dotzler [:asa]]

Comment on attachment 108802 [details] [diff] [review]
New patch for nsinstall.c

1.3a is complete. not taking further patches.

Comment 21

[Colin Blake]

Attachment: One more fix

I'm missing one change. Not sure how this one slipped through the cracks.
Anyway, what hoops do I need to jump through to get this one in?

Comment 22

[Colin Blake]

Reopening since one fix is still outstanding.

Comment 23

[Wan-Teh Chang]

Comment on attachment 111822 [details] [diff] [review]
One more fix

Setting OS_CFLAGS on the gmake command line overrides
the setting of OS_CFLAGS in security/coreconf/OpenVMS.mk,
which is very confusing.

Ideally, you should modify security/coreconf/OpenVMS.mk
so that it sets OS_CFLAGS to the value you want.  Can
you do that?

If you really need to pass extra flags to the C compiler,
I think coreconf's XCFLAGS variable is intended for this
purpose.  There are some (outdated) comments in
security/coreconf/OpenVMS.mk about XCFLAGS that would need
to be removed.

Comment 24

[Colin Blake]

What I really want is for the OS_CFLAGS which is defined in the parent make to
be be in effect. Maybe I just need to remove OS_CFLAGS and OS_CXXFLAGS from
security/coreconf/OpenVMS.mk???

Comment 25

[Wan-Teh Chang]

Why don't you define OS_CFLAGS appropriately in
security/coreconf/OpenVMS.mk?  NSS can be built
as a stand-alone library, without a "parent make".

Comment 26

[Colin Blake]

This comes back to the same discussion we had before. I want the flags to come
in via CFLAGS since some of them are build environment dependant. It doesn't
make sense to have them hardcoded.

Comment 27

[Wan-Teh Chang]

You can pass such flags to coreconf in XCFLAGS.
Remember to delete the old comments and setting
of XCFLAGS in security/coreconf/OpenVMS.mk.

Comment 28

[Colin Blake]

But if I just remove the setting of OS_CFLAGS and OS_CXXFLAGS from
security/coreconf/OpenVMS.mk, won't it then inherit the OS_CFLAGS from the
parent make? And if its a standalone make, will it "see" the CFLAGS and use them?

Comment 29

[Wan-Teh Chang]

NSS won't inherit the OS_CFLAGS from the parent make
(security/manager) unless OS_CFLAGS is passed on the
make command line.  If OS_CFLAGS is passed on the make
command line, it overrides the value of OS_CFLAGS set
by security/coreconf/OpenVMS.mk.

In a standalone make, NSS will not "see" the CFLAGS
from the parent make or in the environment.  The NSS
make variable XCFLAGS is intended for this purpose.
So you can set the environment variable XCFLAGS (to
the same value as CFLAGS) and NSS will see it.
Alternatively, you can pass it on the make command
line:

DEFAULT_GMAKE_FLAGS += XCFLAGS="<the value you want>"

Comment 30

[Colin Blake]

Attachment: Try again

CFLAGS is used by other Mozilla components, so I'd like to stay away from that
otherwise other things may break.

It really seems like passing OS_CFLAGS down via DEFAULT_GMAKE_FLAGS is the
simplest way, since the flags are environment dependant and already defined in
OS_CFLAGS by Mozilla. If NSS is ever built outside of Mozilla, OS_CFLAGS will
need to be defined to whatever makes sense in that environment, and that can be
done on the make command line.

I've removed the definition of OS_CFLAGS from OpenVMS.mk to avoid any
confusion.

Comment 31

[Colin Blake]

Two months later... And idea of an ETA for this one? I just tried to build
Mozilla 1.4 and got bit because I forget to manually apply the patch in comment 30.

Comment 32

[Wan-Teh Chang]

Colin,

You can do this in security/manager/Makefile.in:

+ifeq ($(OS_ARCH),OpenVMS)
+# OS_CFLAGS needs to be passed on down.
+DEFAULT_GMAKE_FLAGS += XCFLAGS="$(OS_CFLAGS)"
+endif

Note that the XCFLAGS here is not the XCFLAGS used by Mozilla
but rather the XCFLAGS used by NSS, so it will not break the
other Mozilla components that use XCFLAGS.

However, if you insist, I will check in your patch.  I disagree
with the way you want to build NSS.  The design philosophy of
NSS's coreconf build system is that one should not need to set
any environment variables and all the appropriate compiler and
linker flags should be specified in NSS's makefiles.

But I know practically nothing about OpenVMS and you are probably
the only one building Mozilla on OpenVMS, so I'll yield to your
expert opinions.

Comment 33

[Colin Blake]

I will try it with the XCFLAGS and let you know if it works that way.

Comment 34

[Colin Blake]

I just rebuilt using XCFLAGS and everything appears fine. Let's go with that.
Thanks Wan-Teh.

Comment 35

[Wan-Teh Chang]

Colin, could you attach your current patch to mozilla/security?  Thanks.

Comment 36

[Colin Blake]

Attachment: Using XCFLAGS

Comment 37

[Wan-Teh Chang]

Comment on attachment 118943 [details] [diff] [review]
Using XCFLAGS

r=wtc.

cls, could you review this patch?  OpenVMS has some compiler
flags that are specific to the build machine and need to be
specified in the environment.  These flags need to be passed
from Mozilla's build system to NSS's coreconf build system.
This happens in security/manager/Makefile.in.  NSS uses the
make variable "XCFLAGS" to mean "extra C flags specified on
the make command line".

Comment 38

[Wan-Teh Chang]

Kai, the last patch in this NSS bug is a patch for a PSM
makefile (security/manager/Makefile.in).  I've reviewed it
and think it is fine.

Comment 39

[Wan-Teh Chang]

Comment on attachment 118943 [details] [diff] [review]
Using XCFLAGS

Requesting mozilla 1.4alpha approval.  This is a simple
makefile change ifdef's with OpenVMS.  It does not affect
other platforms.

Comment 40

[Asa Dotzler [:asa]]

Comment on attachment 118943 [details] [diff] [review]
Using XCFLAGS

a=asa (on behalf of drivers) for checkin to 1.4a

Comment 41

[Wan-Teh Chang]

The last patch (attachment 118943 [details] [diff] [review], for PSM) has been checked in
for mozilla 1.4a.

The NSS changes were checked in on 2002-12-10 (see comment 18).