Update make_rpm.py to check whether rpmbuild is present and report an error if not.

Fixes #4065.

Change-Id: I17a5d2ee4befc5e467b0a195311566db246eb167
PiperOrigin-RevId: 183087906

3 files changed, 100 insertions, 27 deletions

diff --git a/tools/build_defs/pkg/make_rpm.py b/tools/build_defs/pkg/make_rpm.py
index 108bd9d972..d98ba88c36 100644
--- a/tools/build_defs/pkg/make_rpm.py
+++ b/tools/build_defs/pkg/make_rpm.py
@@ -26,6 +26,7 @@ import subprocess
 import sys
 from tempfile import mkdtemp
 
+# pylint: disable=g-direct-third-party-import
 from third_party.py import gflags
 
 gflags.DEFINE_string('name', '', 'The name of the software being packaged.')
@@ -126,6 +127,31 @@ def CopyAndRewrite(input_file, output_file, replacements=None):
       output.write(line)
 
 
+def Which(program):
+
+  def IsExe(fpath):
+    return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
+
+  for path in os.environ['PATH'].split(os.pathsep):
+    filename = os.path.join(path, program)
+    if IsExe(filename):
+      return filename
+
+  return None
+
+
+class NoRpmbuildFound(Exception):
+  pass
+
+
+def FindRpmbuild():
+  path = Which('rpmbuild')
+  if path:
+    return path
+  else:
+    raise NoRpmbuildFound()
+
+
 class RpmBuilder(object):
   """A helper class to manage building the RPM file."""
 
@@ -139,6 +165,7 @@ class RpmBuilder(object):
     self.version = GetFlagValue(version)
     self.arch = arch
     self.files = []
+    self.rpmbuild_path = FindRpmbuild()
     self.rpm_path = None
 
   def AddFiles(self, files):
@@ -171,7 +198,7 @@ class RpmBuilder(object):
     """Call rpmbuild with the correct arguments."""
 
     args = [
-        'rpmbuild',
+        self.rpmbuild_path,
         '--define',
         '_topdir %s' % dirname,
         '--define',
@@ -218,9 +245,13 @@ class RpmBuilder(object):
 
 
 def main(argv=()):
-  builder = RpmBuilder(FLAGS.name, FLAGS.version, FLAGS.arch)
-  builder.AddFiles(argv[1:])
-  return builder.Build(FLAGS.spec_file, FLAGS.out_file)
+  try:
+    builder = RpmBuilder(FLAGS.name, FLAGS.version, FLAGS.arch)
+    builder.AddFiles(argv[1:])
+    return builder.Build(FLAGS.spec_file, FLAGS.out_file)
+  except NoRpmbuildFound:
+    print('ERROR: rpmbuild is required but is not present in PATH')
+    return 1
 
 
 if __name__ == '__main__':
diff --git a/tools/build_defs/pkg/make_rpm_test.py b/tools/build_defs/pkg/make_rpm_test.py
index 8c871725c0..c384735949 100644
--- a/tools/build_defs/pkg/make_rpm_test.py
+++ b/tools/build_defs/pkg/make_rpm_test.py
@@ -17,12 +17,29 @@ from __future__ import absolute_import
 from __future__ import division
 from __future__ import print_function
 
+import contextlib
 import os
 import unittest
 
 from tools.build_defs.pkg import make_rpm
 
 
+@contextlib.contextmanager
+def PrependPath(dirs):
+  with ReplacePath(dirs + [os.environ['PATH']]):
+    yield
+
+
+@contextlib.contextmanager
+def ReplacePath(dirs):
+  original_path = os.environ['PATH']
+  try:
+    os.environ['PATH'] = os.pathsep.join(dirs)
+    yield
+  finally:
+    os.environ['PATH'] = original_path
+
+
 def WriteFile(filename, *contents):
   with open(filename, 'w') as text_file:
     text_file.write('\n'.join(contents))
@@ -80,31 +97,55 @@ class MakeRpmTest(unittest.TestCase):
       self.assertCountEqual(['Some: data1a', 'Other: data2', 'More: data3a'],
                             FileContents('out.txt'))
 
+  def testFindRpmbuild_present(self):
+    with make_rpm.Tempdir() as outer:
+      dummy = os.sep.join([outer, 'rpmbuild'])
+      WriteFile(dummy, 'dummy rpmbuild')
+      os.chmod(dummy, 0o777)
+      with PrependPath([outer]):
+        path = make_rpm.FindRpmbuild()
+        self.assertEqual(dummy, path)
+
+  def testFindRpmbuild_missing(self):
+    with make_rpm.Tempdir() as outer:
+      with ReplacePath([outer]):
+        with self.assertRaises(make_rpm.NoRpmbuildFound) as context:
+          make_rpm.FindRpmbuild()
+        self.assertIsNotNone(context)
+
   def testSetupWorkdir(self):
-    builder = make_rpm.RpmBuilder('test', '1.0', 'x86')
     with make_rpm.Tempdir() as outer:
-      # Create spec_file, test files.
-      WriteFile('test.spec', 'Name: test', 'Version: 0.1', 'Summary: test data')
-      WriteFile('file1.txt', 'Hello')
-      WriteFile('file2.txt', 'Goodbye')
-      builder.AddFiles(['file1.txt', 'file2.txt'])
-
-      with make_rpm.Tempdir():
-        # Call RpmBuilder.
-        builder.SetupWorkdir('test.spec', outer)
-
-        # Make sure files exist.
-        self.assertTrue(DirExists('SOURCES'))
-        self.assertTrue(DirExists('BUILD'))
-        self.assertTrue(DirExists('TMP'))
-        self.assertTrue(FileExists('test.spec'))
-        self.assertCountEqual(
-            ['Name: test', 'Version: 1.0', 'Summary: test data'],
-            FileContents('test.spec'))
-        self.assertTrue(FileExists('BUILD/file1.txt'))
-        self.assertCountEqual(['Hello'], FileContents('BUILD/file1.txt'))
-        self.assertTrue(FileExists('BUILD/file2.txt'))
-        self.assertCountEqual(['Goodbye'], FileContents('BUILD/file2.txt'))
+      dummy = os.sep.join([outer, 'rpmbuild'])
+      WriteFile(dummy, 'dummy rpmbuild')
+      os.chmod(dummy, 0o777)
+
+      with PrependPath([outer]):
+        # Create the builder and exercise it.
+        builder = make_rpm.RpmBuilder('test', '1.0', 'x86')
+
+        # Create spec_file, test files.
+        WriteFile('test.spec', 'Name: test', 'Version: 0.1',
+                  'Summary: test data')
+        WriteFile('file1.txt', 'Hello')
+        WriteFile('file2.txt', 'Goodbye')
+        builder.AddFiles(['file1.txt', 'file2.txt'])
+
+        with make_rpm.Tempdir():
+          # Call RpmBuilder.
+          builder.SetupWorkdir('test.spec', outer)
+
+          # Make sure files exist.
+          self.assertTrue(DirExists('SOURCES'))
+          self.assertTrue(DirExists('BUILD'))
+          self.assertTrue(DirExists('TMP'))
+          self.assertTrue(FileExists('test.spec'))
+          self.assertCountEqual(
+              ['Name: test', 'Version: 1.0', 'Summary: test data'],
+              FileContents('test.spec'))
+          self.assertTrue(FileExists('BUILD/file1.txt'))
+          self.assertCountEqual(['Hello'], FileContents('BUILD/file1.txt'))
+          self.assertTrue(FileExists('BUILD/file2.txt'))
+          self.assertCountEqual(['Goodbye'], FileContents('BUILD/file2.txt'))
 
 
 if __name__ == '__main__':
diff --git a/tools/build_defs/pkg/rpm.bzl b/tools/build_defs/pkg/rpm.bzl
index 2322dfae77..2db0e82648 100644
--- a/tools/build_defs/pkg/rpm.bzl
+++ b/tools/build_defs/pkg/rpm.bzl
@@ -54,6 +54,7 @@ def _pkg_rpm_impl(ctx):
   # TODO(katre): Generate a source RPM.
   ctx.action(
       executable = ctx.executable._make_rpm,
+      use_default_shell_env = True,
       arguments = args,
       inputs = files,
       outputs = [ctx.outputs.rpm],