All users were logged out of Bugzilla on October 13th, 2018

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

VERIFIED FIXED in Q3 11 - Serrano

Status

VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: brbaker, Assigned: brbaker)

Tracking

unspecified
Q3 11 - Serrano
Bug Flags:
flashplayer-qrb +
flashplayer-triage +

Details

(Whiteboard: buildbot has-patch)

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

8 years ago
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
Flags: flashplayer-triage+
Flags: flashplayer-qrb?
(Assignee)

Comment 1

8 years ago
Created attachment 462121 [details] [diff] [review]
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'
Attachment #462121 - Flags: review?(cpeyer)
(Assignee)

Updated

8 years ago
Assignee: nobody → brbaker
Whiteboard: buildbot → buildbot has-patch

Comment 2

8 years ago
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.
Attachment #462121 - Flags: review?(cpeyer) → review+
(Assignee)

Comment 3

8 years ago
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
(Assignee)

Updated

8 years ago
Status: NEW → ASSIGNED

Updated

8 years ago
Target Milestone: --- → flash10.2

Updated

8 years ago
Flags: flashplayer-qrb? → flashplayer-qrb+
(Assignee)

Comment 4

8 years ago
Created attachment 476814 [details] [diff] [review]
v2. verify shell features
Attachment #462121 - Attachment is obsolete: true
Attachment #476814 - Flags: review?(dschaffe)
(Assignee)

Comment 5

8 years ago
(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

8 years ago
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.
Attachment #476814 - Flags: review?(dschaffe) → review+
(Assignee)

Comment 7

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

patch pushed as 5261:ffef04f1ea93
(Assignee)

Comment 8

8 years ago
Created attachment 478776 [details] [diff] [review]
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.
Attachment #478776 - Flags: review?(jsudduth)

Comment 9

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

Patch looks good.
Attachment #478776 - Flags: review?(jsudduth) → review+
(Assignee)

Comment 10

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

patch pushed as 5316:4e4a474941f3
(Assignee)

Comment 11

8 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
Status: RESOLVED → VERIFIED
(Assignee)

Comment 12

8 years ago
Created attachment 511035 [details] [diff] [review]
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.
Attachment #511035 - Flags: review?(dschaffe)

Updated

8 years ago
Attachment #511035 - Flags: review?(dschaffe) → review+

Comment 13

8 years ago
changeset: 5909:38509a51b668
user:      Brent Baker <brbaker@adobe.com>
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
You need to log in before you can comment on or make changes to this bug.