android: Require exe unittests to have a foo_unittest_stripped target.

build/android/pylib/gtest/test_package_exe.py

Index: build/android/pylib/gtest/test_package_exe.py
diff --git a/build/android/pylib/gtest/test_package_exe.py b/build/android/pylib/gtest/test_package_exe.py
index 5da86ba4c2061e309220ad352e6aff3dd566e4d3..579b070ef8c85b25c40a70e330fbf98d749e3999 100644
--- a/build/android/pylib/gtest/test_package_exe.py
+++ b/build/android/pylib/gtest/test_package_exe.py
@@ -122,26 +122,19 @@ class TestPackageExecutable(TestPackage):
     if self.tool.NeedsDebugInfo():
       target_name = self.suite_path
     else:
-      target_name = self.suite_path + '_' + adb.GetDevice() + '_stripped'
-      should_strip = True
-      if os.path.isfile(target_name):
-        logging.info('Found target file %s' % target_name)
-        target_mtime = os.stat(target_name).st_mtime
-        source_mtime = os.stat(self.suite_path).st_mtime
-        if target_mtime > source_mtime:
-          logging.info('Target mtime (%d) is newer than source (%d), assuming '
-                       'no change.' % (target_mtime, source_mtime))
-          should_strip = False
-
-      if should_strip:
-        logging.info('Did not find up-to-date stripped binary. Generating a '
-                     'new one (%s).' % target_name)
-        # Whenever we generate a stripped binary, copy to the symbols dir. If we
-        # aren't stripping a new binary, assume it's there.
-        if not os.path.exists(self._symbols_dir):
-          os.makedirs(self._symbols_dir)
-        shutil.copy(self.suite_path, self._symbols_dir)
-        strip = os.environ['STRIP']
-        cmd_helper.RunCmd([strip, self.suite_path, '-o', target_name])
+      target_name = self.suite_path + '_stripped'
+      if not os.path.isfile(target_name):
+        logging.critical('Did not find %s, build target %s',

[frankf, 2014/02/08 01:34:43]

it's more pythonic to raise an exception instead of logging/exiting

[Nico, 2014/02/08 02:13:16]

Agreed for code, but this is UI. Having an error message is a nicer UI than
getting a stack.

[frankf, 2014/02/10 18:27:11]

To be consistent with the rest of pylib, it's better to raise an exception here.

On 2014/02/08 02:13:16, Nico (away Feb 11 - Feb 17) wrote:

+                         target_name, self.suite_name + '_stripped')
+        sys.exit(1)
+
+      target_mtime = os.stat(target_name).st_mtime
+      source_mtime = os.stat(self.suite_path).st_mtime
+      if target_mtime < source_mtime:

[frankf, 2014/02/08 01:34:43]

I'd remove this check. It's a general assumption throughout pylib that your
build is recent.

[frankf, 2014/02/08 01:43:37]

Actually scratch this, I can totally see the pitfal :/

[Nico, 2014/02/08 02:43:45]

Bleh, on a tester bot that gets the binary from a zip file, this can actually
fail. But not having this will be pretty surprising for someone working on these
targets and not expecting having to build the _stripped variant :-/

Maybe on android the targets should be foo_unstripped and foo? Then this check
isn't needed.

[Nico, 2014/02/08 02:47:53]

Hm, this won't work well either. It'd have to be

              'target_name': 'breakpad_unittests_unstripped',
              'product_name': 'breakpad_unittests',

so that the exectuable name of the unstripped thing doesn't change, but then
ninja will (rightfully) complain that there are two rules to build
breakpad_unittests (one for the binary, one for the strip target)

+        logging.critical(
+            'stripped binary older than source binary, build target %s',
+            self.suite_name + '_stripped')
+        sys.exit(1)
+
     test_binary = constants.TEST_EXECUTABLE_DIR + '/' + self.suite_name
     adb.PushIfNeeded(target_name, test_binary)