From d74eeb394155b199a81e8feee2163f3894d2509d Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Mon, 16 Nov 2020 11:19:19 -0500 Subject: [PATCH 1/3] MockModuleBuilder: Don't rewrite mock config to add SCM options When we want to pass in SCM options particular to a specific module build, do that on the mock command line rather than by modifying mock.cfg - this avoids invalidating the root cache. --- .../builder/MockModuleBuilder.py | 66 ++++++++++--------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/module_build_service/builder/MockModuleBuilder.py b/module_build_service/builder/MockModuleBuilder.py index c4804b7c..d4a3d888 100644 --- a/module_build_service/builder/MockModuleBuilder.py +++ b/module_build_service/builder/MockModuleBuilder.py @@ -811,42 +811,44 @@ class SRPMBuilder(BaseBuilder): class SCMBuilder(BaseBuilder): def __init__(self, config, resultsdir, source, artifact_name): super(SCMBuilder, self).__init__(config, resultsdir) - with open(config, "a") as f: - repo_path, branch = source.split("?#") - distgit_cmds = self._get_distgit_commands(source) - # Supply the artifact name for "{0}" and the full path to the repo for "{repo_path}" - distgit_get = distgit_cmds[0].format(artifact_name, repo_path=repo_path) - # mock-scm cannot checkout particular commit hash, but only branch. - # We therefore use a command that combines the distgit-command with - # checking out a particular commit hash. - # See https://bugzilla.redhat.com/show_bug.cgi?id=1459437 for - # more info. Once mock-scm supports this feature, we can remove - # this code. - distgit_get_branch = "sh -c {}'; git -C {} checkout {}'".format( - pipes.quote(distgit_get), artifact_name, branch) + repo_path, branch = source.split("?#") + distgit_cmds = self._get_distgit_commands(source) + # Supply the artifact name for "{0}" and the full path to the repo for "{repo_path}" + distgit_get = distgit_cmds[0].format(artifact_name, repo_path=repo_path) - f.writelines([ - "config_opts['scm'] = True\n", - "config_opts['scm_opts']['method'] = 'distgit'\n", - "config_opts['scm_opts']['package'] = '{}'\n".format(artifact_name), - "config_opts['scm_opts']['distgit_get'] = {!r}\n".format(distgit_get_branch), - ]) + # mock-scm cannot checkout particular commit hash, but only branch. + # We therefore use a command that combines the distgit-command with + # checking out a particular commit hash. + # See https://bugzilla.redhat.com/show_bug.cgi?id=1459437 for + # more info. Once mock-scm supports this feature, we can remove + # this code. + distgit_get_branch = "sh -c {}'; git -C {} checkout {}'".format( + pipes.quote(distgit_get), artifact_name, branch) - # Set distgit_src_get only if it's defined. - if distgit_cmds[1]: - f.write( - "config_opts['scm_opts']['distgit_src_get'] = '{}'\n".format(distgit_cmds[1])) + scm_options = [ + ('method', 'distgit'), + ('package', artifact_name), + ('distgit_get', distgit_get_branch), + ] - # The local git repositories cloned by `fedpkg clone` typically do not have - # the tarballs with sources committed in a git repo. They normally live in lookaside - # cache on remote server, but we should not try getting them from there for true - # local builds. - # Instead, get them from local path with git repository by passing that path to Mock - # using the `ext_src_dir`. - if repo_path.startswith("file://"): - src_dir = repo_path[len("file://"):] - f.write("config_opts['scm_opts']['ext_src_dir'] = '{}'\n".format(src_dir)) + # Set distgit_src_get only if it's defined. + if distgit_cmds[1]: + scm_options.append(('distgit_src_get', distgit_cmds[1])) + + # The local git repositories cloned by `fedpkg clone` typically do not have + # the tarballs with sources committed in a git repo. They normally live in lookaside + # cache on remote server, but we should not try getting them from there for true + # local builds. + # Instead, get them from local path with git repository by passing that path to Mock + # using the `ext_src_dir`. + if repo_path.startswith("file://"): + src_dir = repo_path[len("file://"):] + scm_options.append(('ext_src_dir', src_dir)) + + self.cmd.append('--scm-enable') + for name, value in scm_options: + self.cmd.extend(('--scm-option', '{}={}'.format(name, value))) def _get_distgit_commands(self, source): for host, cmds in conf.distgits.items(): From f09d7cfe7f477fb1f8bf5d78a4c859e78d5eab42 Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Mon, 16 Nov 2020 11:29:05 -0500 Subject: [PATCH 2/3] MockModuleBuilder: manage timestamps on mock config files The mod time for the mock configuration file is used to determine whether the root cache is out-of-date or not, so we want to avoid changing the configuration timestamps when we don't change content when we're just writing a per-thread mock configuration file again with no substantive changes. We do this by only updating the master mock.cfg file when we're actually adding content, and propagating its mod time to the per-thread configuration files. --- .../builder/MockModuleBuilder.py | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/module_build_service/builder/MockModuleBuilder.py b/module_build_service/builder/MockModuleBuilder.py index d4a3d888..0f610b67 100644 --- a/module_build_service/builder/MockModuleBuilder.py +++ b/module_build_service/builder/MockModuleBuilder.py @@ -468,7 +468,7 @@ class MockModuleBuilder(GenericBuilder): self.enabled_modules = config_opts["module_enable"] self.releasever = config_opts["releasever"] - def _write_mock_config(self): + def _write_mock_config(self, update_main=False): """ Writes Mock config file to local file. """ @@ -483,11 +483,15 @@ class MockModuleBuilder(GenericBuilder): config = config.replace("$enabled_modules", str(self.enabled_modules)) config = config.replace("$releasever", str(self.releasever)) - # We write the most recent config to "mock.cfg", so thread-related - # configs can be later (re-)generated from it using _load_mock_config. - outfile = os.path.join(self.configdir, "mock.cfg") - with open(outfile, "w") as f: - f.write(config) + + mock_cfg_path = os.path.join(self.configdir, "mock.cfg") + if update_main or not os.path.exists(mock_cfg_path): + # We write a config to "mock.cfg", so thread-related + # configs can be later (re-)generated from it using _load_mock_config. + with open(mock_cfg_path, "w") as f: + f.write(config) + + mtime = os.path.getmtime(mock_cfg_path) # Write the config to thread-related configuration file. outfile = os.path.join( @@ -495,6 +499,8 @@ class MockModuleBuilder(GenericBuilder): with open(outfile, "w") as f: f.write(config) + os.utime(outfile, (mtime, mtime)) + def buildroot_connect(self, groups): self._load_mock_config() self.groups = list(set().union(groups["build"], self.groups)) @@ -525,7 +531,7 @@ class MockModuleBuilder(GenericBuilder): if artifact and artifact.startswith("module-build-macros"): self._load_mock_config() self.groups.append("module-build-macros") - self._write_mock_config() + self._write_mock_config(update_main=True) events.scheduler.add(repos_done_handler, ("fake_msg", self.tag_name + "-build")) @@ -586,7 +592,7 @@ class MockModuleBuilder(GenericBuilder): baseurl = "file://" + repo_dir self._add_repo(repo_name, baseurl) - self._write_mock_config() + self._write_mock_config(update_main=True) def _send_build_change(self, state, source, build_id): from module_build_service.scheduler.handlers.components import ( From 48c4c75d7377c4629f7a0b1bcb90a9c0cb971cf5 Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Mon, 16 Nov 2020 11:32:49 -0500 Subject: [PATCH 3/3] MockModuleBuilder: Share root cache between threads The different build threads all are using the same basic build root contents, so there's no reason to use separate caches - point the root cache plugin for mock to a single location. (There's locking inside Mock for updating the root cache.) --- module_build_service/builder/MockModuleBuilder.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/module_build_service/builder/MockModuleBuilder.py b/module_build_service/builder/MockModuleBuilder.py index 0f610b67..ccfc5f29 100644 --- a/module_build_service/builder/MockModuleBuilder.py +++ b/module_build_service/builder/MockModuleBuilder.py @@ -6,6 +6,7 @@ import os import pipes import re import subprocess +from textwrap import dedent import threading import dnf @@ -483,6 +484,11 @@ class MockModuleBuilder(GenericBuilder): config = config.replace("$enabled_modules", str(self.enabled_modules)) config = config.replace("$releasever", str(self.releasever)) + config += dedent("""\ + config_opts.setdefault('plugin_conf', {}) + config_opts['plugin_conf'].setdefault('root_cache_opts', {}) + config_opts['plugin_conf']['root_cache_opts']['dir'] = "{{cache_topdir}}/%s/root_cache/" + """) % self.tag_name mock_cfg_path = os.path.join(self.configdir, "mock.cfg") if update_main or not os.path.exists(mock_cfg_path):