423511 - Test failure because make prints CWD

Status: RESOLVED FIXED

(NSS :: Test, defect)

Description

[Kai Engert (:KaiE:)]

Today, when I try to build the most recent NSS snapshot as an RPM package, the suite fails (we run the test suite as part of the build).

The first error message was
  all.sh: too many parameters.

I learned that OBJDIR had a multi-line-value similar to the following:
  entering directory ...
  name of objdir
  leaving directory ...

This is because we have statements like
  OBJDIR=`(cd $COMMON; $MAKE objdir_name)`

For whatever reason, make gets executed with option "-w" or "--print-directory" enabled. I don't know where this gets set, maybe some environment variable set by something in the recursive make system.

I did not try to find out exactly which part sets that flag. It seems to be related to the RPM build, because outside of the RPM build I don't get this failure.

Anyway, I propose, NSS should disable printing of the directories when NSS is trying to capture the output of a make command.

I'm attaching a patch that was sufficient in my scenario.

Comment 1

[Kai Engert (:KaiE:)]

Attachment: Patch v1

Comment 2

[Kai Engert (:KaiE:)]

Comment on attachment 310058 [details] [diff] [review]
Patch v1

Not sure who should review this.

Comment 3

[Kai Engert (:KaiE:)]

Maybe you'll tell me that this patch isn't appropriate, because the option --no-print-directory is not available on all platforms?

Comment 4

[Robert Relyea]

Looks like we already depend on the flag, for precisely the same reasons as kai needs.

bobs-laptop(96) cvs diff  -u -r 1.4 -r 1.3 rules.mk
Index: rules.mk
===================================================================
RCS file: /cvsroot/mozilla/security/coreconf/rules.mk,v
retrieving revision 1.4
retrieving revision 1.3
diff -u -r1.4 -r1.3
--- rules.mk    28 Dec 2000 02:22:26 -0000      1.4
+++ rules.mk    27 Dec 2000 00:42:46 -0000      1.3
@@ -303,9 +303,6 @@
        -$(PLCYPATCH) $(PLCYPATCH_ARGS) $@
 endif
 
-get_objs:
-       @echo $(OBJS)
-
 $(LIBRARY): $(OBJS)
        @$(MAKE_OBJDIR)
        rm -f $@
@@ -317,10 +314,6 @@
        wlib +$(SHARED_LIBRARY)
 endif
 
-ifdef SHARED_LIBRARY_LIBS
-SUB_SHLOBJS = $(foreach dir,$(SHARED_LIBRARY_DIRS),$(addprefix $(dir)/,$(shell $(MAKE) -C $(dir) --no-print-directory get_objs)))
-endif
-
 $(SHARED_LIBRARY): $(OBJS)
        @$(MAKE_OBJDIR)
        rm -f $@
@@ -351,7 +344,7 @@
        $(LINK_DLL) -MAP $(DLLBASE) $(OBJS) $(EXTRA_LIBS) $(EXTRA_SHARED_LIBS) $(OS_LIBS) $(LD_LIBS)
 endif
 else
-       $(MKSHLIB) -o $@ $(OBJS) $(SUB_SHLOBJS) $(LD_LIBS) $(EXTRA_LIBS) $(EXTRA_SHARED_LIBS)
+       $(MKSHLIB) -o $@ $(OBJS) $(LD_LIBS) $(EXTRA_LIBS) $(EXTRA_SHARED_LIBS)
        chmod +x $@
 endif
 endif

Comment 5

[Robert Relyea]

Some additional observations:

1) this problem will occur for anyone who tries to run all.sh out of a makefile.
2) The commands in question are already explicitly calling gmake rather than make.
3) For reviews... this patch looks like a gendiff patch (used extensively in rpm packaging). I'm sure kai would be willing to apply it to an CVS diff, since such a patch would be needed to check in anyway. The read the existing patch, ignore the .prex suffix.

bob

Comment 6

[Robert Relyea]

arg in comment 4, the patch is 'backwards' - is v 1.4 and + is v1.3

Comment 7

[Kai Engert (:KaiE:)]

Attachment: Patch v1 - real cvs

Bob suggested I attach a patch which has been created using cvs diff (instead of a local diff), so here it is.

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Do all the flavors of make and gmake on the systems supported by NSS 
also support (or at least ignore) the --no-print-directory option?

Comment 9

[Robert Relyea]

If they didn't, our builds would break (rules.mk uses --no-print-directory).

bob

Comment 10

[Wan-Teh Chang]

Comment on attachment 310075 [details] [diff] [review]
Patch v1 - real cvs

Thanks for the patch.  In tests/common/init.sh, you should make
--no-print-directory part of the MAKE variable.  I don't think you
need to patch the other two files because they're not used by
all.sh.  If you still want to patch them, I suggest you define
MAKE=gmake --no-print-directory and use $(MAKE) instead
of gmake in those two files.

Comment 11

[Robert Relyea]

Comment on attachment 310075 [details] [diff] [review]
Patch v1 - real cvs

given that gmake is already hard coded, I think this patch is fine, but I would definately entertain a chain more in line with wtc's suggestion.

bob

Comment 12

[Kai Engert (:KaiE:)]

Attachment: Patch v2

Maybe it was obvious for you ... But it took me a couple of attempts and test runs to figure out the combination of use of brackets and required quoting to get it work.

This patch does it for me.

Comment 13

[Wan-Teh Chang]

Comment on attachment 323990 [details] [diff] [review]
Patch v2

Kai, please remove mozilla/security/nss/tests/header
and mozilla/security/nss/tests/nsspath from your patch
unless you're sure that you're using these two files.

In mozilla/security/nss/tests/common/init.sh, please
remove the curly braces around MAKE.  Just write
$MAKE.  This will make your patch a one-line change.

Comment 14

[Kai Engert (:KaiE:)]

Attachment: Patch v3

Comment 15

[Wan-Teh Chang]

Comment on attachment 324620 [details] [diff] [review]
Patch v3

r=wtc.

>     MAKE=gmake
>     $MAKE -v >/dev/null 2>&1 || MAKE=make
>     $MAKE -v >/dev/null 2>&1 || { echo "You are missing make."; exit 5; }
>+    MAKE="${MAKE} --no-print-directory"

For a consistent style, I'd prefer that you do without
the curly braces {} here.

Comment 16

[Kai Engert (:KaiE:)]

Attachment: Patch v4

I'll check in this patch

Comment 17

[Kai Engert (:KaiE:)]

fixed

Checking in init.sh;
/cvsroot/mozilla/security/nss/tests/common/init.sh,v  <--  init.sh
new revision: 1.64; previous revision: 1.63