583751 - Verify that compiled shells have the expected features enabled/disabled in the build system

Status: VERIFIED FIXED

(Tamarin Graveyard :: Build Config, defect)

Description

[Brent Baker]

The build system compiles the shell in many different ways with specific features enable or disabled, however the requested features are not confirmed to be enabled.

The compile-generic script should attempt to verify that a feature is enabled or disabled.

Example:
There is a build step in the deep phase that is supposed to compile the shell without a JIT. The compilation and acceptance has been passing but there was a change to the x-platform configure script that removed handling of the switch used to disable the JIT and this was never caught. The shell was being compiled and run with the JIT enabled, NOT what was supposed to be tested.

After compiling the shell, the script should confirm either the existence or absence of a feature via the -Dversion switch that is available on the shell.


build/buildbot/all/compile-generic.sh

--features="+<feature> -<feature>"
   +<feature> will ensure that this feature is enabled in the shell
   -<feature> will ensure that this feature is not enabled in the shell
   If either + or - features do not match, the script will exit with a non-zero exitcode, causing a failure in the build system

--features="+AVMFEATURE_JIT -AVMFEATURE_WORDCODE_INTERP" , this would ensure that the shell has the JIT enabled and that the wordcode interpreter is not enabled

Comment 1

[Brent Baker]

Attachment: option to verify shell features

In the build system calls to the compile-generic would become
something like this:

../all/compile-generic.sh 4960 '--enable-shell --disable-jit --target=x86_64-darwin --mac-sdk=105' 'avmshell_nojit' 'false' '+AVMSYSTEM_64BIT -AVMFEATURE_JIT'

This call will ensure that the shell is compiled with the
AVMSYSTEM_64BIT feature enabled and AVMSYSTEM_JIT not enabled, aka not
defined

../all/compile-generic.sh 4960 '--enable-shell' 'avmshell' 'false' '+AVMSYSTEM_32BIT'
This will validate that the build is 32bit 'AVMSYSTEM_32BIT'

Comment 2

[Chris Peyer]

Comment on attachment 462121 [details] [diff] [review]
option to verify shell features

Looks good - I only tested the feature code locally, but everything worked as expected.

Minor nit: Fix the indenting after the if blocks so that everything is nested correctly.

Comment 3

[Brent Baker]

As a follow on patch I was thinking of basically just verifying the basics for now in the build system:

- 32bit vs 64bit
- debugger enabled
- wordcode, nojit
- PPC vs IA32 of the mac PPC/x86 builds that happen on same machine

We can add additional feature/syste/tweak check points at any time

Comment 4

[Brent Baker]

Attachment: v2. verify shell features

Comment 5

[Brent Baker]

(In reply to comment #4)
> Created attachment 476814 [details] [diff] [review]
> v2. verify shell features

Same as previous patch with the following tweaks:

Make sure that the shell can be run on the host machine prior to confirming features. It is possible that the shell was cross compiled on a platform that is not able to run the shell. 

Prior to exiting if a feature check failed, revert the avmplusVersion.h and call endSilent so that logs will be posted if running in the sandbox.

Comment 6

[Dan Schaffer]

Comment on attachment 476814 [details] [diff] [review]
v2. verify shell features

patch look good.  I am always a little wary in bash of not using "" around expressions in ==.  Like in $feat == ${i:1} instead of  "$feat" == "${i:1}" in case an empty value is somehow introduced it will cause an argument mismatch.  I don't think it applies to this situation.

Comment 7

[Brent Baker]

Comment on attachment 476814 [details] [diff] [review]
v2. verify shell features

patch pushed as 5261:ffef04f1ea93

Comment 8

[Brent Baker]

Attachment: verify features in buildbot

This is what is currently being verified: 

Release:
+AVMSYSTEM_32BIT +AVMSYSTEM_IA32
+AVMSYSTEM_64BIT +AVMSYSTEM_AMD64
+AVMSYSTEM_32BIT +AVMSYSTEM_PPC
+AVMSYSTEM_32BIT +AVMSYSTEM_SPARC

Release-Wordcode:
<release> +AVMFEATURE_WORDCODE_INTERP

Release-Debugger:
<release> +AVMFEATURE_DEBUGGER

Debug:
<release>

Debug-Debugger
<release> +AVMFEATURE_DEBUGGER


Unable to verify PPC64 builds since rosetta does not emulate G5 processors and we are compiling on intel
http://en.wikipedia.org/wiki/Rosetta_%28software%29

Deep builds:
AIR: <release-debugger> +AVMFEATURE_OVERRIDE_GLOBAL_NEW +AVMFEATURE_USE_SYSTEM_MALLOC

I will apply the same phase one changes to the sandbox config if this is approved.

Comment 9

[James Sudduth]

Comment on attachment 478776 [details] [diff] [review]
verify features in buildbot

Patch looks good.

Comment 10

[Brent Baker]

Comment on attachment 478776 [details] [diff] [review]
verify features in buildbot

patch pushed as 5316:4e4a474941f3

Comment 11

[Brent Baker]

This is a sample from the build log showing verification of features, if a feature check fails it will fail the build step:

Make sure that AVMSYSTEM_32BIT is enabled
---> PASS

Make sure that AVMSYSTEM_IA32 is enabled
---> PASS

Make sure that AVMFEATURE_WORDCODE_INTERP is enabled
---> PASS

Comment 12

[Brent Baker]

Attachment: Remove shell binary if feature check fails

If the binary does NOT match what we expected, then do not leave the binary on the machine. This makes sure that other steps that either check that binaries exist or just run the binary will NOT pass. We have just determined that the binary is bogus so do not keep it.

Comment 13

[Tamarin Bot]

changeset: 5909:38509a51b668
user:      Brent Baker <[e-mail]>
summary:   Bug 583751: do not keep generated binaries on build machines if the AVMFEATURE check fails (r=dschaffe)

http://hg.mozilla.org/tamarin-redux/rev/38509a51b668