Add a scons pnacl finalize step for pnacl_generate_pexe tests.

SConstruct

Index: SConstruct
diff --git a/SConstruct b/SConstruct
index 55b483b47c6efd03ddac58a1badca3270dbe8049..bbf395f8bb9d608ffa47eb01383b819993e7d141 100755
--- a/SConstruct
+++ b/SConstruct
@@ -352,6 +352,9 @@ def SetUpArgumentBits(env):
   BitFromArgument(env, 'arm_hard_float', default=False,
                   desc='Build for hard float ARM ABI')
 
+  BitFromArgument(env, 'skip_nonstable_bitcode', default=False,
+                  desc='Skip tests involving non-stable bitcode')
+
   #########################################################################
   # EXPERIMENTAL
   # This is for generating a testing library for use within private test
@@ -1574,7 +1577,6 @@ def GetTranslatedNexe(env, pexe):
 
 pre_base_env.AddMethod(GetTranslatedNexe)
 
-

[Mark Seaborn, 2013/03/28 01:31:34]

Nit: Can you leave 2 empty lines between top-level defs please?  That's the
usual Python style.

[jvoung - send to chromium..., 2013/03/28 15:50:29]

Done.

 def ShouldTranslateToNexe(env, pexe):
   """ Determine when we need to translate a Pexe to a Nexe.
   """
@@ -1598,6 +1600,42 @@ def ShouldTranslateToNexe(env, pexe):
 
 pre_base_env.AddMethod(ShouldTranslateToNexe)
 
+def GetFinalizedPexe(env, pexe):
+  """ Prep and finalize the ABI for a given pexe.
+  """
+  pexe_name = pexe.abspath
+  final_name = pexe_name[:pexe_name.index('.pexe')] + '.final.pexe'

[Mark Seaborn, 2013/03/28 01:31:34]

You should assert that pexe_name ends with ".pexe" so that this doesn't mangle
the name if it doesn't.

Could we use a slightly different naming convention?  e.g. ".final-pexe", so
that globbing for "*.pexe" and "*.final-pexe" match disjoint sets of files (for
when copying around sets of these manually).

I'd suggest reversing the naming actually: how about ".nonfinal-pexe" or
".unstripped-pexe" and ".pexe"?  We'd want to use a briefer name for the file
that's intended to be stable and published.

+  # Make sure the pexe doesn't get removed by the fake builders when
+  # built_elsewhere=1
+  env.Precious(pexe)
+  node = env.Command(target=final_name, source=[pexe_name],
+                     action=[Action('${PNACLFINALIZECOM}',
+                                    '${PNACLFINALIZECOMSTR}')])
+  assert len(node) == 1, node
+  return node[0]
+
+pre_base_env.AddMethod(GetFinalizedPexe)
+
+# Bit to be set by individual test/nacl.scons files that need to opt-out.

[Mark Seaborn, 2013/03/28 01:31:34]

Nit: "opt out", no hyphen :-)

[jvoung - send to chromium..., 2013/03/28 15:50:29]

Done.

+DeclareBit('nonstable_bitcode', 'Tests use non-stable bitcode features', False)
+
+def ShouldFinalizePexe(env, pexe):
+  """ Determine when we need to prep a pexe to finalize it for ABI stability.
+  """
+  if not env.Bit('pnacl_generate_pexe') or env.Bit('nonstable_bitcode'):
+    return False
+
+  # There is no bitcode for trusted code.
+  if env['NACL_BUILD_FAMILY'] == 'TRUSTED':
+    return False
+
+  # Unlike ShouldTranslateToNexe, we always prep during the build step,
+  # since there is no prep tool that can run on triggered bots such as the
+  # ARM HW bots.
+  return True
+
+pre_base_env.AddMethod(ShouldFinalizePexe)
+
 
 def CommandTestFileDumpCheck(env,
                              name,
@@ -1614,12 +1652,12 @@ def CommandTestFileDumpCheck(env,
   # ARM objdump though... a TODO(jvoung) for when there is time.
   if env.Bit('built_elsewhere'):
     return []
+  if env.ShouldFinalizePexe(target):
+    target = env.GetFinalizedPexe(target)

[Mark Seaborn, 2013/03/28 01:31:34]

GetFinalizedPexe() is always called after ShouldFinalizePexe(), so why not merge
the two functions?

(The same goes for GetTranslatedNexe() but I'm not asking you to refactor that
now. :-) )

Is there a reason not to have GetTranslatedNexe() call ShouldFinalizePexe() for
you?  There are a few other calls to GetTranslatedNexe() in nacl.scons files --
should these be calling ShouldFinalizePexe()?

[jvoung - send to chromium..., 2013/03/28 15:50:29]

Yes, I think we can merge these.
I'd kept them separate because of the ShouldTranslateToNexe() condition is
different, so I didn't want to accidentally leave out a call to
GetFinalizedPexe, just because ShouldTranslateToNexe() is false.

We probably do want to have the finalized pexe for those other nacl.scons files
too, for consistency, so it might make sense to fuse all the Should* with the
Get*, and have one Get...() that does both.

With this file renaming, I'm also a bit worried now about nacl integration tests
making an URL for the non-finalized pexe in the autogenerated NMF files... but
that can probably be fixed in a later CL.

   if env.ShouldTranslateToNexe(target):
-    target_obj = env.GetTranslatedNexe(target)
-  else:
-    target_obj = target
+    target = env.GetTranslatedNexe(target)
   return env.CommandTestFileCheck(name,
-                                  ['${OBJDUMP}', objdump_flags, target_obj],
+                                  ['${OBJDUMP}', objdump_flags, target],
                                   check_file)
 
 pre_base_env.AddMethod(CommandTestFileDumpCheck)
@@ -1660,6 +1698,11 @@ def CommandSelLdrTestNacl(env, name, nexe,
       env['TRUSTED_ENV'].Bit('windows')):
     return []
 
+  if env.ShouldFinalizePexe(nexe):
+    # The nexe is actually a pexe and we need to prep it for
+    # ABI stability first.
+    nexe = env.GetFinalizedPexe(nexe)
+
   if env.ShouldTranslateToNexe(nexe):
     # The nexe is actually a pexe.  Translate it before we run it.
     nexe = env.GetTranslatedNexe(nexe)