274820 - enable all.sh to succeed on systems without gmake

Status: RESOLVED WONTFIX

(NSS :: Test, enhancement, P3)

Description

[Jason Reid]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040113
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040113

Change some of the variable assignments in all.sh from foo is assigned to
foo is assigned if not already having a value e.g. OBJDIR=`(cd $COMMON; gmake
objdir_name)` to OBJDIR=${OBJDIR:-`(cd $COMMON; gmake objdir_name)`}.

Changing OBJDIR, OS_ARCH, DLL_PREFIX, and DLL_SUFFIX to the second format
would allow cross platform testing e.g. making sure the Solaris 8 binaries work
correctly on Solaris 10 or x86 binaries work on a native AMD64 system.

I estimate four lines would be changed.

Reproducible: Always

Comment 1

[Jason Reid]

The change would actually be in mozilla/security/nss/tests/common/init.sh.

Comment 2

[Jason Reid]

Proposed code changes:
mace[svbld]:/tmp> diff init.sh update.sh
153,156c153,156
<       OBJDIR=${OBJDIR:-`(cd $COMMON; gmake objdir_name)`}
<       OS_ARCH=${OS_ARCH:-`(cd $COMMON; gmake os_arch)`}
<       DLL_PREFIX=${DLL_PREFIX:-`(cd $COMMON; gmake dll_prefix)`}
<       DLL_SUFFIX=${DLL_SUFFIX:-`(cd $COMMON; gmake dll_suffix)`}
---
>     OBJDIR=`(cd $COMMON; gmake objdir_name)`
>     OS_ARCH=`(cd $COMMON; gmake os_arch)`
>     DLL_PREFIX=`(cd $COMMON; gmake dll_prefix)`
>     DLL_SUFFIX=`(cd $COMMON; gmake dll_suffix)`

Comment 3

[Jason Reid]

Attachment: proposed patch

Comment 4

[Julien Pierre]

Comment on attachment 191775 [details] [diff] [review]
proposed patch

I believe this patch will allow you to do what you need, but some improvements
can be made.

1. I think you will always want the DLL_PREFIX, DLL_SUFFIX, and OS_ARCH to be
the same as on the original build. Unless you can think of situations where
that's not true, you can remove the 3 lines that allow their override.

2. You only need to be able to override OBJDIR, in order to test binaries on a
different OS revision than the machines they were built on.

You can change OBJDIR by just overriding OBJDIR completely as you did. But in
this case, the caller is responsible for completely reconstructing OBJDIR - and
figuring out DBG vs OPT, 64 vs regular, etc. This is a waste, as most other
parameters that affect OBJDIR can already be controlled by environment
variables (eg. BUILD_OPT and USE_64), and the logic is already in coreconf .

If the only thing that's needed (as I believe, please correct me if I'm wrong)
is to change the OS revision to match the one of the build machine, you could
simply override OS_RELEASE and pass that to the gmake objdir_name command - eg.
"gmake objdir_name OS_RELEASE=5.8". You would probably want to have the caller
set OS_RELEASE, have your script check if it's set, and if so, pass it in to
gmake.

Comment 5

[Jason Reid]

I wanted avoid calling gmake entirely. I do not want to have
to depend on working with the build environment to run
the tests. Removing the gmake dependency makes the job
of running tests on new machines easier. Not all systems 
ship with gmake installed.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 191775 [details] [diff] [review]
proposed patch

Julien, please re-review this patch in light of 
the objective (first articulated after your 
previous review) of obviating gmake on test systems.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Slavo, in your opinion, how desirable is this proposed change?

Comment 8

[Slavomir Katuscak]

Nelson, I see it more like nice to have feature, but I'm not sure if anyone will ever use it. We have gmake installed on all testing machines and if someone wants to run these tests on machine without gmake, he will most probably install it after first error message that gmake was not found.

Comment 9

[Julien Pierre]

Comment on attachment 191775 [details] [diff] [review]
proposed patch

I agree with Slavo, we don't need to fix this. And I think anyone running the tests externally would want to have gmake as well. It's not like it's hard to get.
It's probably easier to get it than to set the 4 variables here. I suggest WONTFIX.

Comment 10

[Slavomir Katuscak]

Nelson, do you agree with WONTFIX ?

Comment 11

[Nelson Bolyard (seldom reads bugmail)]

yes