[Instrumented Libraries] Refactor download_build_install.py
- Remove --no-configure. It's not necessary since we can check
if ./configure exists.
- Remove custom_libz builder and replace with --make-targets.
- Update references that mention GYP
- Change most shell_call() usages to pass the command line as
a list instead of a string.
R=thestig
Bug: 1496000
Change-Id: Ie0f56672fd4b919a9b0e08b2e28d25e6b4df7ad9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5009311
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1220710}
NOKEYCHECK=True
GitOrigin-RevId: b99d16f3b1112e73979b1637b11a5ab8477c1526
diff --git a/focal/BUILD.gn b/focal/BUILD.gn
index 604c2bb..4c0004d 100644
--- a/focal/BUILD.gn
+++ b/focal/BUILD.gn
@@ -170,8 +170,8 @@
args += [ "--git-revision=${invoker.git_revision}" ]
}
- if (defined(invoker.no_configure) && invoker.no_configure) {
- args += [ "--no-configure" ]
+ if (defined(invoker.make_targets)) {
+ args += [ "--make-targets=${invoker.make_targets}" ]
}
}
}
@@ -219,8 +219,7 @@
instrumented_library("guest-oslogin") {
git_url = "https://github.com/GoogleCloudPlatform/guest-oslogin.git"
git_revision = "f59b7f38c21b4794282ddf12fd4a6083cd99e1e4"
- # guest-oslogin is built only with `make`.
- no_configure = true
+
# Work around an issue where header files are passed to the linker.
patches = [ "patches/guest-oslogin.diff" ]
}
@@ -451,7 +450,7 @@
# Do not use loadable modules. Same as with Pango, there's no easy way
# to make gdk-pixbuf pick instrumented versions over system-installed
# ones.
- "-Dbuiltin_loaders='all'",
+ "-Dbuiltin_loaders=all",
]
pre_build = "scripts/pre-build/libgdk-pixbuf2.0-0.sh"
@@ -856,5 +855,5 @@
}
instrumented_library("zlib1g") {
- build_method = "custom_zlib"
+ make_targets = [ "libz.so.1.2.11" ]
}
diff --git a/focal/scripts/download_build_install.py b/focal/scripts/download_build_install.py
index db043fe..9c0e50d 100755
--- a/focal/scripts/download_build_install.py
+++ b/focal/scripts/download_build_install.py
@@ -21,29 +21,29 @@
SCRIPT_ABSOLUTE_PATH = os.path.dirname(os.path.abspath(__file__))
def unescape_flags(s):
- """Un-escapes build flags received from GYP.
+ """Un-escapes build flags received from GN.
- GYP escapes build flags as if they are to be inserted directly into a command
+ GN escapes build flags as if they are to be inserted directly into a command
line, wrapping each flag in double quotes. When flags are passed via
CFLAGS/LDFLAGS instead, double quotes must be dropped.
"""
if not s:
- return ''
+ return []
try:
- return ' '.join(ast.literal_eval(s))
+ return ast.literal_eval(s)
except (SyntaxError, ValueError):
- return ' '.join(shlex.split(s))
+ return shlex.split(s)
-def real_path(path_relative_to_gyp):
+def real_path(path_relative_to_gn):
"""Returns the absolute path to a file.
- GYP generates paths relative to the location of the .gyp file, which is one
+ GN generates paths relative to the build directory, which is one
level above the location of this script. This function converts them to
absolute paths.
"""
return os.path.realpath(os.path.join(SCRIPT_ABSOLUTE_PATH, '..',
- path_relative_to_gyp))
+ path_relative_to_gn))
class InstrumentedPackageBuilder(object):
@@ -71,7 +71,7 @@
self._cflags = unescape_flags(args.cflags)
if args.sanitizer_ignorelist:
ignorelist_file = real_path(args.sanitizer_ignorelist)
- self._cflags += ' -fsanitize-blacklist=%s' % ignorelist_file # nocheck
+ self._cflags += ['-fsanitize-blacklist=%s' % ignorelist_file]
self._ldflags = unescape_flags(args.ldflags)
@@ -80,11 +80,11 @@
self._git_url = args.git_url
self._git_revision = args.git_revision
- self._no_configure = args.no_configure
+ self._make_targets = unescape_flags(args.make_targets)
# Initialized later.
- self._source_dir = None
- self._source_archives = None
+ self._source_dir = ''
+ self._source_archives = ''
def init_build_env(self, args_env):
self._build_env = os.environ.copy()
@@ -94,14 +94,14 @@
self._build_env['CC'] = self._cc
self._build_env['CXX'] = self._cxx
- self._build_env['CFLAGS'] = self._cflags
- self._build_env['CXXFLAGS'] = self._cflags
- self._build_env['LDFLAGS'] = self._ldflags
+ self._build_env['CFLAGS'] = ' '.join(self._cflags)
+ self._build_env['CXXFLAGS'] = ' '.join(self._cflags)
+ self._build_env['LDFLAGS'] = ' '.join(self._ldflags)
# libappindicator1 needs this.
self._build_env['CSC'] = '/usr/bin/mono-csc'
- def shell_call(self, command, env=None, cwd=None, ignore_ret_code=False):
+ def shell_call(self, command, env=None, cwd=None, ignore_ret_code=False, shell=False):
"""Wrapper around subprocess.Popen().
Calls command with specific environment and verbosity using
@@ -109,7 +109,7 @@
"""
child = subprocess.Popen(
command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
- env=env, shell=True, cwd=cwd)
+ env=env, shell=shell, cwd=cwd)
stdout = child.communicate()[0].decode('utf-8')
if ignore_ret_code:
if self._verbose:
@@ -127,20 +127,21 @@
Checks out the source code for the package, if required (i.e. unless running
in no-clobber mode). Initializes self._source_dir and self._source_archives.
"""
+ command = ''
get_fresh_source = self._clobber or not os.path.exists(self._working_dir)
if get_fresh_source:
shutil.rmtree(self._working_dir, ignore_errors=True)
os.makedirs(self._working_dir)
if self._git_url:
- command = 'git clone %s' % self._git_url
+ command = ['git', 'clone', self._git_url]
self.shell_call(command, cwd=self._working_dir)
else:
# Download one source package at a time, otherwise, there will
# be connection errors in gnutls_handshake().
lock = open('apt-source-lock', 'w')
fcntl.flock(lock, fcntl.LOCK_EX)
- command = 'apt-get source %s' % self._package
+ command = ['apt-get', 'source', self._package]
self.shell_call(command, cwd=self._working_dir)
fcntl.flock(lock, fcntl.LOCK_UN)
@@ -151,7 +152,7 @@
self._source_component = dirnames[0]
self._source_dir = os.path.join(dirpath, self._source_component, '')
if self._git_url:
- self.shell_call('git checkout %s' % self._git_revision,
+ self.shell_call(['git', 'checkout', self._git_revision],
cwd=self._source_dir)
else:
if len(filenames) == 0:
@@ -163,9 +164,9 @@
def patch_source(self):
for patch in self._patches:
- self.shell_call('patch -p1 -i %s' % patch, cwd=self._source_dir)
+ self.shell_call(['patch', '-p1', '-i', patch], cwd=self._source_dir)
if self._pre_build:
- self.shell_call(self._pre_build, cwd=self._source_dir)
+ self.shell_call([self._pre_build], cwd=self._source_dir)
def copy_source_archives(self):
"""Copies the downloaded source archives to the output dir.
@@ -208,12 +209,12 @@
# Remove downloaded package and generated temporary build files. Failed
# builds intentionally skip this step to help debug build failures.
if self._clobber:
- self.shell_call('rm -rf %s' % self._working_dir)
+ self.shell_call(['rm', '-rf', self._working_dir])
def fix_rpaths(self, directory):
# TODO(eugenis): reimplement fix_rpaths.sh in Python.
script = real_path('scripts/fix_rpaths.sh')
- self.shell_call("%s %s" % (script, directory))
+ self.shell_call([script, directory])
def temp_dir(self):
"""Returns the directory which will be passed to `make install'."""
@@ -232,9 +233,9 @@
# .la files are not needed, nuke them.
# In case --no-static is not supported, nuke any static libraries we built.
self.shell_call(
- 'find %s -name *.la -or -name *.a | xargs rm -f' % self.temp_libdir())
+ 'find %s -name *.la -or -name *.a | xargs rm -f' % self.temp_libdir(), shell=True)
# .pc files are not needed.
- self.shell_call('rm %s/pkgconfig -rf' % self.temp_libdir())
+ self.shell_call(['rm', '-rf', '%s/pkgconfig' % self.temp_libdir()])
def make(self, args, env=None, cwd=None, ignore_ret_code=False):
"""Invokes `make'.
@@ -246,29 +247,30 @@
cwd = self._source_dir
if env is None:
env = self._build_env
- cmd = ['make'] + args
- self.shell_call(' '.join(cmd), env=env, cwd=cwd,
+ self.shell_call(['make'] + args, env=env, cwd=cwd,
ignore_ret_code=ignore_ret_code)
def make_install(self, args, **kwargs):
"""Invokes `make install'."""
self.make(['install'] + args, **kwargs)
- def build_and_install(self, targets=[]):
+ def build_and_install(self):
"""Builds and installs the DSOs.
Builds the package with ./configure + make, installs it to a temporary
location, then moves the relevant files to their permanent location.
"""
- if not self._no_configure:
- configure_cmd = './configure --libdir=/%s/ %s' % (
- self._libdir, self._extra_configure_flags)
+ if os.path.exists(os.path.join(self._source_dir, 'configure')):
+ configure_cmd = [
+ './configure',
+ '--libdir=/%s/' % self._libdir,
+ ] + self._extra_configure_flags
self.shell_call(configure_cmd, env=self._build_env, cwd=self._source_dir)
# Some makefiles use BUILDROOT or INSTALL_ROOT instead of DESTDIR.
args = ['DESTDIR', 'BUILDROOT', 'INSTALL_ROOT']
make_args = ['%s=%s' % (name, self.temp_dir()) for name in args]
- self.make(make_args + targets)
+ self.make(make_args + self._make_targets)
self.make_install(make_args)
@@ -282,7 +284,7 @@
# Now move the contents of the temporary destdir to their final place.
# We only care for the contents of LIBDIR.
self.shell_call('cp %s/* %s/ -rdf' % (self.temp_libdir(),
- self.dest_libdir()))
+ self.dest_libdir()), shell=True)
class DebianBuilder(InstrumentedPackageBuilder):
@@ -299,9 +301,9 @@
self._build_env['CC'] = self._cc
self._build_env['CXX'] = self._cxx
- self._build_env['DEB_CFLAGS_APPEND'] = self._cflags
- self._build_env['DEB_CXXFLAGS_APPEND'] = self._cflags
- self._build_env['DEB_LDFLAGS_APPEND'] = self._ldflags
+ self._build_env['DEB_CFLAGS_APPEND'] = ' '.join(self._cflags)
+ self._build_env['DEB_CXXFLAGS_APPEND'] = ' '.join(self._cflags)
+ self._build_env['DEB_LDFLAGS_APPEND'] = ' '.join(self._ldflags)
self._build_env['DEB_BUILD_OPTIONS'] = \
'nocheck notest nodoc nostrip parallel=%d' % os.cpu_count()
@@ -310,14 +312,15 @@
self.install_packaged_libs()
def build_debian_packages(self):
- configure_cmd = 'dpkg-buildpackage -B -uc'
+ configure_cmd = ['dpkg-buildpackage', '-B', '-uc']
self.shell_call(configure_cmd, env=self._build_env, cwd=self._source_dir)
def install_packaged_libs(self):
for deb_file in self.get_deb_files():
- self.shell_call("dpkg-deb -x %s %s" % (deb_file, self.temp_dir()))
+ self.shell_call(['dpkg-deb', '-x', deb_file, self.temp_dir()])
- dpkg_arch = self.shell_call("dpkg-architecture -qDEB_HOST_MULTIARCH").strip()
+ dpkg_arch_cmd = ['dpkg-architecture', '-qDEB_HOST_MULTIARCH']
+ dpkg_arch = self.shell_call(dpkg_arch_cmd).strip()
lib_dirs = [
"usr/lib/%s" % dpkg_arch,
"lib/%s" % dpkg_arch,
@@ -360,7 +363,7 @@
# libcap2 doesn't have a configure script
build_args = ['CC', 'CXX', 'CFLAGS', 'CXXFLAGS', 'LDFLAGS']
make_args = [
- '%s="%s"' % (name, self._build_env[name]) for name in build_args
+ '%s=%s' % (name, self._build_env[name]) for name in build_args
]
self.make(make_args)
@@ -379,7 +382,7 @@
# Now move the contents of the temporary destdir to their final place.
# We only care for the contents of LIBDIR.
self.shell_call('cp %s/* %s/ -rdf' % (self.temp_libdir(),
- self.dest_libdir()))
+ self.dest_libdir()), shell=True)
class LibcurlBuilder(DebianBuilder):
@@ -413,11 +416,11 @@
def build_and_install(self):
# pciutils doesn't have a configure script
# This build process follows debian/rules.
- self.shell_call('mkdir -p %s-udeb/usr/bin' % self.temp_dir())
+ self.shell_call(['mkdir', '-p', '%s-udeb/usr/bin' % self.temp_dir()])
build_args = ['CC', 'CXX', 'CFLAGS', 'CXXFLAGS', 'LDFLAGS']
make_args = [
- '%s="%s"' % (name, self._build_env[name]) for name in build_args
+ '%s=%s' % (name, self._build_env[name]) for name in build_args
]
make_args += [
'LIBDIR=/%s/' % self._libdir,
@@ -438,31 +441,27 @@
# Now install the DSOs to their final place.
self.shell_call(
'install -m 644 %s/libpci.so* %s' % (self.temp_libdir(),
- self.dest_libdir()))
+ self.dest_libdir()), shell=True)
self.shell_call(
'ln -sf libpci.so.%s %s/libpci.so.3' % (self.package_version(),
- self.dest_libdir()))
+ self.dest_libdir()), shell=True)
class MesonBuilder(InstrumentedPackageBuilder):
def build_and_install(self):
- meson_flags = {
- 'prefix': '/',
- 'libdir': self._libdir,
- 'sbindir': 'bin',
- }
meson_cmd = [
'meson',
'build',
'.',
- ' '.join('--%s %s' % item for item in meson_flags.items()),
+ '--prefix', '/',
+ '--libdir', self._libdir,
+ '--sbindir', 'bin',
'-Db_lundef=false',
- self._extra_configure_flags,
- ]
+ ] + self._extra_configure_flags
- self.shell_call(' '.join(meson_cmd),
+ self.shell_call(meson_cmd,
env=self._build_env, cwd=self._source_dir)
- self.shell_call('ninja -C build install',
+ self.shell_call(['ninja', '-C', 'build', 'install'],
{**self._build_env, 'DESTDIR': self.temp_dir()},
cwd=self._source_dir)
self.post_install()
@@ -475,9 +474,8 @@
'.',
'-DCMAKE_INSTALL_PREFIX=/usr',
'-DCMAKE_INSTALL_LIBDIR=/%s/' % self._libdir,
- self._extra_configure_flags,
- ]
- self.shell_call(' '.join(cmake_cmd), env=self._build_env,
+ ] + self._extra_configure_flags
+ self.shell_call(cmake_cmd, env=self._build_env,
cwd=self._source_dir)
args = ['DESTDIR', 'BUILDROOT', 'INSTALL_ROOT']
@@ -537,15 +535,10 @@
shutil.copy(full_path, self.dest_libdir())
-class ZlibBuilder(InstrumentedPackageBuilder):
- def build_and_install(self):
- super().build_and_install(['libz.so.1.2.11'])
-
-
class StubBuilder(InstrumentedPackageBuilder):
def download_build_install(self):
self._touch(os.path.join(self._destdir, '%s.txt' % self._package))
- self.shell_call('mkdir -p %s' % self.dest_libdir())
+ self.shell_call(['mkdir', '-p', self.dest_libdir()])
self._touch(os.path.join(self.dest_libdir(), '%s.so.0' % self._package))
def _touch(self, path):
@@ -581,9 +574,9 @@
parser.add_argument('--env', default='')
parser.add_argument('--git-url', default='')
parser.add_argument('--git-revision', default='')
- parser.add_argument('--no-configure', action='store_true')
+ parser.add_argument('--make-targets', default='')
- # Ignore all empty arguments because in several cases gyp passes them to the
+ # Ignore all empty arguments because in several cases gn passes them to the
# script, but ArgumentParser treats them as positional arguments instead of
# ignoring (and doesn't have such options).
args = parser.parse_args([arg for arg in sys.argv[1:] if len(arg) != 0])
@@ -603,8 +596,6 @@
builder = LibcurlBuilder(args, clobber)
elif args.build_method == 'custom_libpci3':
builder = Libpci3Builder(args, clobber)
- elif args.build_method == 'custom_zlib':
- builder = ZlibBuilder(args, clobber)
elif args.build_method == 'debian':
builder = DebianBuilder(args, clobber)
elif args.build_method == 'meson':
diff --git a/focal/scripts/fix_rpaths.sh b/focal/scripts/fix_rpaths.sh
index 6b2a0df..94bb445 100755
--- a/focal/scripts/fix_rpaths.sh
+++ b/focal/scripts/fix_rpaths.sh
@@ -4,7 +4,7 @@
# found in the LICENSE file.
# Changes all RPATHs in a given directory from XORIGIN to $ORIGIN
-# See the comment about XORIGIN in instrumented_libraries.gyp
+# See the comment about XORIGIN in BUILD.gn
# Fixes rpath from XORIGIN to $ORIGIN in a single file $1.
function fix_rpath {
diff --git a/focal/scripts/pre-build/autogen.sh b/focal/scripts/pre-build/autogen.sh
index 7ebab4e..9671595 100755
--- a/focal/scripts/pre-build/autogen.sh
+++ b/focal/scripts/pre-build/autogen.sh
@@ -11,7 +11,7 @@
# as that sometimes breaks build. Which is why we have this file.
# Also, some packages may or may not have an autogen script, depending on
-# version. Rather than clutter the GYP file with conditionals, we simply do
+# version. Rather than clutter the GN file with conditionals, we simply do
# nothing if the file is not present.
if [ -x ./autogen.sh ]