#1736 rpmbuild cleanup
Merged 3 years ago by praiskup. Opened 3 years ago by praiskup.
Unknown source rpmbuild-cleanup  into  main

@@ -34,6 +34,7 @@

  rm -f /var/lib/copr-rpmbuild/pid

  rm -f /var/lib/copr-rpmbuild/main.log

  rm -rf /var/lib/copr-rpmbuild/results

+ rm -rf /var/lib/copr-rpmbuild/workspace/*

  

  shopt -s nullglob

  set -- /etc/copr-builder/hooks/cleanup/*.sh

@@ -221,6 +221,7 @@

  install -d %{buildroot}%mock_config_overrides

  install -d %{buildroot}%{_sharedstatedir}/copr-rpmbuild

  install -d %{buildroot}%{_sharedstatedir}/copr-rpmbuild/results

+ install -d %{buildroot}%{_sharedstatedir}/copr-rpmbuild/workspace

  

  install -d %{buildroot}%{_bindir}

  install -m 755 main.py %{buildroot}%{_bindir}/copr-rpmbuild
@@ -284,6 +285,7 @@

  

  %dir %attr(0775, root, mock) %{_sharedstatedir}/copr-rpmbuild

  %dir %attr(0775, root, mock) %{_sharedstatedir}/copr-rpmbuild/results

+ %dir %attr(0775, root, mock) %{_sharedstatedir}/copr-rpmbuild/workspace

  

  %dir %{_sysconfdir}/copr-rpmbuild

  %config(noreplace) %{_sysconfdir}/copr-rpmbuild/main.ini

@@ -149,6 +149,7 @@

  def read_config(config_path=None):

      config = configparser.RawConfigParser(defaults={

          "resultdir": "/var/lib/copr-rpmbuild/results",

+         "workspace": "/var/lib/copr-rpmbuild/workspace",

          "lockfile": "/var/lib/copr-rpmbuild/lockfile",

          "logfile": "/var/lib/copr-rpmbuild/main.log",

          "pidfile": "/var/lib/copr-rpmbuild/pid",

@@ -1,6 +1,8 @@

  import os

  import errno

  import logging

+ import shutil

+ import stat

  import tempfile

  from ..helpers import string2list

  
@@ -8,11 +10,30 @@

  

  

  class Provider(object):

-     def __init__(self, source_dict, outdir, config):

-         self.outdir = outdir

+     _safe_resultdir = None

+ 

+     def __init__(self, source_dict, config):

+         self.source_dict = source_dict

          self.config = config

  

-         self.workdir = os.path.join(outdir, "obtain-sources")

+         # Where we should produce output, everything there gets copied to

+         # backend once build ends!

+         self.real_resultdir = config.get("main", "resultdir")

+ 

+         # When True, we don't consider the method safe enough to put the results

+         # directly to self.real_resultdir.  So we first put the results below

+         # the self._safe_resultdir.  Note that this may mean that everything

+         # (perhaps large uploaded source RPMs) could end-up in the storage

+         # twice, therefore try to keep it False if possible.

+         self.use_safe_resultdir = False

+ 

+         # Where we can create the temporary directories.  These are

+         # automatically removed when possible when build ends.

+         self.workspace = config.get("main", "workspace")

+ 

+         # A per-task uniquely named working directory.  Ideally all the

+         # work-in-progress stuff should live here.

+         self.workdir = tempfile.mkdtemp(dir=self.workspace, prefix="workdir-")

          try:

              os.mkdir(self.workdir)

          except OSError as e:
@@ -22,6 +43,58 @@

          # Change home directory to workdir and create .rpmmacros there

          os.environ["HOME"] = self.workdir

          self.create_rpmmacros()

+         self.init_provider()

+ 

+     def init_provider(self):

+         """

+         Additional configuration stuff specific to a concrete provider.

+         Automatically called by __init__(), and it is _optional_, therefore we

+         don't raise NotImplementedError in Provider.init_provider() parent.

+         """

+ 

+     @property

+     def resultdir(self):

+         """

+         Create a sub-directory (on demand, when accessed) with permissive

+         permissions to allow user-namespaces (e.g. systemd-nspawn) doing

+         permissions/ownership changes on the files there.

+         """

+         if not self.use_safe_resultdir:

+             return self.real_resultdir

+ 

+         if not self._safe_resultdir:

+             self._safe_resultdir = tempfile.mkdtemp(dir=self.workspace,

+                                                     prefix="safe-resultdir-")

+ 

+             # allow namespaces (even root) to give the files away

+             for directory in [self.workdir, self._safe_resultdir]:

+                 os.chmod(directory, stat.S_IRWXU|stat.S_IRWXO)

+ 

+         return self._safe_resultdir

+ 

+     def copy_insecure_results(self):

+         """

+         Copy the possibly non-removable results to real_resultdir, that will be

+         picked-up by copr-backend.

+         """

+         if not self._safe_resultdir:

+             return

+         shutil.copytree(self._safe_resultdir, self.real_resultdir,

+                         dirs_exist_ok=True)

+ 

+     @staticmethod

+     def _best_effort_cleanup(directory):

+         try:

+             shutil.rmtree(directory)

+         except IOError:

+             log.error("Can not remove the '%s', run copr-builder-cleanup.",

+                       directory)

+ 

+     def cleanup(self):

+         """ Best effort cleanup of the working directories """

+         self._best_effort_cleanup(self.workdir)

+         if self._safe_resultdir:

+             self._best_effort_cleanup(self._safe_resultdir)

  

      def create_rpmmacros(self):

          path = os.path.join(self.workdir, ".rpmmacros")

@@ -20,10 +20,8 @@

  

      workdir = None

  

-     def __init__(self, source_json, outdir, config):

-         super(CustomProvider, self).__init__(source_json, outdir, config)

- 

-         self.outdir = outdir

+     def init_provider(self):

+         source_json = self.source_dict

          self.chroot = source_json.get('chroot')

          self.inner_resultdir = source_json.get('resultdir')

          self.builddeps = source_json.get('builddeps')
@@ -31,18 +29,17 @@

  

          if 'hook_data' in source_json:

              self.hook_payload_url = "{server}/tmp/{tmp}/hook_payload".format(

-                 server=config.get("main", "frontend_url"),

+                 server=self.config.get("main", "frontend_url"),

                  tmp=source_json['tmp'],

              )

  

-         self.workdir = outdir

          self.file_script = os.path.join(self.workdir, 'script')

          with open(self.file_script, 'w') as script:

              script.write(source_json['script'])

  

  

      def produce_srpm(self):

-         mock_config_file = os.path.join(self.outdir, 'mock-config.cfg')

+         mock_config_file = os.path.join(self.resultdir, 'mock-config.cfg')

  

          with open(mock_config_file, 'w') as f:

              # Enable network.
@@ -65,7 +62,7 @@

  

          if self.hook_payload_url:

              chunk_size = 1024

-             hook_payload_file = os.path.join(self.outdir, 'hook_payload')

+             hook_payload_file = os.path.join(self.resultdir, 'hook_payload')

              response = requests.get(self.hook_payload_url, stream=True)

              response.raise_for_status()

  
@@ -101,5 +98,5 @@

  

          helpers.run_cmd(mock + ['--copyout', inner_resultdir, srpm_srcdir])

          helpers.run_cmd(mock + ['--scrub', 'all'])

-         helpers.build_srpm(srpm_srcdir, self.outdir)

+         helpers.build_srpm(srpm_srcdir, self.resultdir)

          shutil.rmtree(srpm_srcdir)

@@ -3,19 +3,22 @@

  """

  

  import os

- 

  from copr_rpmbuild import helpers

- from copr_rpmbuild.providers.base import Provider

+ from .base import Provider

  

  

  class DistGitProvider(Provider):

      """

      DistGit provider, wrapper around copr-distgit-client script

      """

-     def __init__(self, source_dict, outdir, config):

-         super(DistGitProvider, self).__init__(source_dict, outdir, config)

-         self.clone_url = source_dict["clone_url"]

-         self.committish = source_dict.get("committish")

+ 

+     clone_url = None

+     committish = None

+     clone_to = None

+ 

+     def init_provider(self):

+         self.clone_url = self.source_dict["clone_url"]

+         self.committish = self.source_dict.get("committish")

          self.clone_to = os.path.join(

              self.workdir,

              helpers.git_clone_url_basepath(self.clone_url))
@@ -35,5 +38,5 @@

      def produce_srpm(self):

          self.produce_sources()

          helpers.run_cmd([

-             "copr-distgit-client", "srpm", "--outputdir", self.outdir

+             "copr-distgit-client", "srpm", "--outputdir", self.resultdir

          ], cwd=self.clone_to)

@@ -6,8 +6,8 @@

  

  

  class PyPIProvider(Provider):

-     def __init__(self, source_json, outdir, config=None):

-         super(PyPIProvider, self).__init__(source_json, outdir, config)

+     def init_provider(self):

+         source_json = self.source_dict

          self.pypi_package_version = source_json["pypi_package_version"]

          self.pypi_package_name = source_json["pypi_package_name"]

          self.spec_template = source_json.get("spec_template", '')
@@ -24,7 +24,7 @@

          self.tool_presence_check()

  

          cmd = ["pyp2rpm", self.pypi_package_name, "-t", self.spec_template,

-                "--srpm", "-d", self.outdir]

+                "--srpm", "-d", self.resultdir]

  

          for i, python_version in enumerate(self.python_versions):

              if i == 0:

@@ -6,9 +6,8 @@

  

  

  class RubyGemsProvider(Provider):

-     def __init__(self, source_json, outdir, config=None):

-         super(RubyGemsProvider, self).__init__(source_json, outdir, config)

-         self.gem_name = source_json["gem_name"]

+     def init_provider(self):

+         self.gem_name = self.source_dict["gem_name"]

  

      def tool_presence_check(self):

          try:
@@ -20,7 +19,7 @@

      def produce_srpm(self):

          self.tool_presence_check()

  

-         cmd = ["gem2rpm", self.gem_name, "--srpm", "-C", self.outdir, "--fetch"]

+         cmd = ["gem2rpm", self.gem_name, "--srpm", "-C", self.resultdir, "--fetch"]

          result = run_cmd(cmd)

  

          if "Empty tag: License" in result.stderr:

@@ -16,8 +16,9 @@

  

  

  class ScmProvider(Provider):

-     def __init__(self, source_dict, outdir, config):

-         super(ScmProvider, self).__init__(source_dict, outdir, config)

+ 

+     def init_provider(self):

+         source_dict = self.source_dict

          self.scm_type = source_dict.get('type') or 'git'

          self.clone_url = source_dict.get('clone_url')

          self.committish = source_dict.get('committish')
@@ -31,6 +32,9 @@

          self.spec_path = helpers.path_join(

              self.repo_path, os.path.join(self.repo_subdir, self.spec_relpath))

  

+         # make_srpm method can create root-owned files in resultdir

+         self.use_safe_resultdir = self.srpm_build_method == "make_srpm"

+ 

      def generate_rpkg_config(self):

          parsed_clone_url = urlparse(self.clone_url)

          distgit_config_section = None
@@ -81,17 +85,22 @@

  

      def get_rpkg_command(self):

          self.generate_rpkg_config()

-         return ['rpkg', 'srpm', '--outdir', self.outdir, '--spec', self.spec_path]

+         return ['rpkg', 'srpm', '--outdir', self.resultdir, '--spec', self.spec_path]

  

      def get_tito_command(self):

-         return ['tito', 'build', '--srpm', '--output', self.outdir]

+         return ['tito', 'build', '--srpm', '--output', self.resultdir]

  

      def get_tito_test_command(self):

-         return ['tito', 'build', '--test', '--srpm', '--output', self.outdir]

+         return ['tito', 'build', '--test', '--srpm', '--output', self.resultdir]

+ 

+     @staticmethod

+     def _mock_mountpoint(directory):

+         base = os.path.basename(os.path.normpath(directory))

+         return os.path.join("/mnt", base)

  

      def get_make_srpm_command(self):

-         mock_workdir = '/mnt' + self.workdir

-         mock_outdir = '/mnt' + self.outdir

+         mock_workdir = self._mock_mountpoint(self.workdir)

+         mock_resultdir = self._mock_mountpoint(self.resultdir)

          mock_repodir = helpers.path_join(mock_workdir, self.repo_dirname)

          mock_cwd = helpers.path_join(mock_repodir, self.repo_subdir)

          mock_spec_path = helpers.path_join(
@@ -99,12 +108,12 @@

  

          mock_bind_mount_cmd_part = \

              '--plugin-option=bind_mount:dirs=(("{0}", "{1}"), ("{2}", "{3}"))'\

-             .format(self.workdir, mock_workdir, self.outdir, mock_outdir)

+             .format(self.workdir, mock_workdir, self.resultdir, mock_resultdir)

  

          makefile_path = os.path.join(mock_repodir, '.copr', 'Makefile')

          make_srpm_cmd_part = \

              'cd {0}; make -f {1} srpm outdir="{2}" spec="{3}"'\

-             .format(mock_cwd, makefile_path, mock_outdir, mock_spec_path)

+             .format(mock_cwd, makefile_path, mock_resultdir, mock_spec_path)

  

          return ['mock', '--uniqueext', get_mock_uniqueext(),

                  '-r', '/etc/copr-rpmbuild/mock-source-build.cfg',

@@ -10,9 +10,8 @@

  

  

  class UrlProvider(Provider):

-     def __init__(self, source_json, outdir, config=None):

-         super(UrlProvider, self).__init__(source_json, outdir, config)

-         self.url = source_json["url"]

+     def init_provider(self):

+         self.url = self.source_dict["url"]

          self.parsed_url = urlparse(self.url)

  

      def save_spec(self):
@@ -27,12 +26,12 @@

          cmd = ["mock", "-r", "/etc/copr-rpmbuild/mock-source-build.cfg",

                 "--buildsrpm", "--spec", spec_path,

                 "--define", "_disable_source_fetch 0",

-                "--resultdir", self.outdir]

+                "--resultdir", self.resultdir]

          return run_cmd(cmd, cwd=self.workdir)

  

      def download_srpm(self):

          basename = os.path.basename(self.parsed_url.path)

-         filename = os.path.join(self.outdir, basename)

+         filename = os.path.join(self.resultdir, basename)

          response = requests.get(self.url, stream=True)

          if response.status_code != 200:

              raise RuntimeError('Requests get status "{0}" for "{1}"'.format(

file modified
+32
@@ -1,12 +1,44 @@

  [main]

+ # Frontend URL;  Where we donwnload the build tasks from.

A typo here, s/donwnload/download

  frontend_url = https://copr.fedoraproject.org

+ 

+ # Backend URL;  Mostly used to reference the default build-time DNF/YUM

+ # repositories.

  backend_url = https://copr-be.cloud.fedoraproject.org

+ 

+ # Allowed auto-download protocols for SourceX statements in spec file.

+ # See %_disable_source_fetch RPM macro for more info.

  enabled_source_protocols = https ftps

+ 

+ # DistGit fallback, when no distgitX section below is usable.

  distgit_lookaside_url = {scheme}://{netloc}/repo/pkgs/%(ns1)s/%(name)s/%(filename)s/%(hashtype)s/%(hash)s/%(filename)s

  distgit_clone_url = {scheme}://{netloc}/%(module)s

+ 

  # The final %vendor would be e.g. "Fedora Copr - group @copr"

  #rpm_vendor_copr_name = Fedora Copr

  

+ # Where we store build results, so copr-backend can pick them up.

+ #resultdir = /var/lib/copr-rpmbuild/resultdir

+ 

+ # Repository where we do all the heavy work.  We create an uniquely named

+ # "workdir" like (workspace/workdir-XXXXXXXX), and also "safe_resultdir"

+ # (like workspace/safe-resultdir-XXXXXXXX).  Both "workdir" and

+ # "safe_resultdir" are used by various build methods that can create

+ # files with artificial ownership - so cleaning stuff there may need

+ # a call to /bin/copr-builder-cleanup (as root).

+ #workspace = /var/lib/copr-rpmbuild/workspace

+ 

+ # Lock file that guards against concurrent runs of copr-rpmbuild.

+ #lockfile = /var/lib/copr-rpmbuild/lockfile

+ 

+ # The live build log.  This file is continuosly downloaded to copr-backend, and

+ # provided as "builder-live.log" in build results.

+ #logfile = /var/lib/copr-rpmbuild/main.log

+ 

+ # Various supported DistGit instances are configured below for the "rpkg" build

+ # method.  The rpmbuild code iterates through them till it finds an appropriate

+ # distgit_hostname_pattern from the build task "clone_url".

+ 

  [distgit0]

  distgit_hostname_pattern = src.fedoraproject.org

  distgit_lookaside_url = https://src.fedoraproject.org/repo/pkgs/%(ns1)s/%(name)s/%(filename)s/%(hashtype)s/%(hash)s/%(filename)s

file modified
+13 -27
@@ -7,11 +7,8 @@

  import argparse

  import json

  import logging

- import tempfile

  import shutil

  import pprint

- import tempfile

- import stat

  import pipes

  import pkg_resources

  
@@ -148,31 +145,17 @@

          os.makedirs(resultdir)

  

  

- def produce_srpm(task, config, resultdir):

+ def produce_srpm(task, config):

      """

-     create tempdir to allow --private-users=pick with make_srpm

-     that changes permissions on the result directory to out of scope values

+     Use *Provider() classes to create source RPM in config.get("resultdir")

      """

-     tempdir = tempfile.mkdtemp(prefix="copr-rpmbuild-")

-     os.chmod(tempdir, stat.S_IRWXU|stat.S_IRWXO)

      try:

          provider = providers.factory(task["source_type"])(

-             task["source_json"], tempdir, config)

+             task["source_json"], config)

          provider.produce_srpm()

-         for item in os.listdir(tempdir):

-             if item in ["obtain-sources"]:

-                 continue

-             src = os.path.join(tempdir, item)

-             if os.path.isdir(src):

-                 shutil.copytree(src, os.path.join(resultdir, item))

-             else:

-                 shutil.copy(src, resultdir)

+         provider.copy_insecure_results()

      finally:

-         try:

-             shutil.rmtree(tempdir)

-         except Exception:

-             log.error("Can not remove tempdir, "

-                       "https://pagure.io/copr/copr/issue/1258")

+         provider.cleanup()

  

  

  def get_task(args, config, build_config_url_path=None, task_id=None):
@@ -221,9 +204,9 @@

      task = get_task(args, config, build_config_url_path)

      log_task(task)

  

-     resultdir = config.get("main", "resultdir")

-     produce_srpm(task, config, resultdir)

+     produce_srpm(task, config)

  

+     resultdir = config.get("main", "resultdir")

      log.info("Output: {0}".format(

          os.listdir(resultdir)))

  
@@ -254,12 +237,15 @@

      task = get_task(args, config, build_config_url_path, task_id)

      log_task(task)

  

-     sourcedir = tempfile.mkdtemp(prefix="copr-rpmbuild-")

      try:

          distgit = providers.DistGitProvider(

              {"clone_url": task["git_repo"], "committish": task["git_hash"]},

-             sourcedir, config,

+             config,

          )

+ 

+         # Just clone and download sources, don't create source RPM (aka

+         # produce_srpm).  We want to create the source RPM using Mock

+         # in the target chroot.

          distgit.produce_sources()

          resultdir = config.get("main", "resultdir")

          builder = MockBuilder(task, distgit.clone_to, resultdir, config)
@@ -268,7 +254,7 @@

          run_automation_tools(task, resultdir, builder.mock_config_file, log)

      finally:

          builder.archive_configs()

-         shutil.rmtree(sourcedir)

+         distgit.cleanup()

  

  

  def dump_configs(args, config):

file modified
+26 -4
@@ -1,6 +1,7 @@

  import os

  import tempfile

  import unittest

+ import shutil

  

  from copr_rpmbuild import helpers

  
@@ -27,16 +28,37 @@

  """

  

  class TestCase(unittest.TestCase):

-     def test_setup(self):

-         # to be defined in child class

-         pass

+     workdir = None

+     resultdir = None

+     workspace = None

+ 

+     def auto_test_setup(self):

+         """ to be defined in child class """

+ 

+     def auto_test_cleanup(self):

+         """ to be defined in child class """

+ 

+     def config_basic_dirs(self):

+         """ precreate workspace and resultdir """

+         self.workdir = tempfile.mkdtemp(prefix="copr-rpmbuild-produce-srpm-")

+         self.resultdir = os.path.join(self.workdir, "results")

+         self.workspace = os.path.join(self.workdir, "workspace")

+         self.config.set("main", "resultdir", self.resultdir)

+         self.config.set("main", "workspace", self.workspace)

+         os.makedirs(self.workspace)

+         os.makedirs(self.resultdir)

+ 

+     def cleanup_basic_dirs(self):

+         """ cleanup precreated workspace and resultdir """

+         shutil.rmtree(self.workdir)

  

      def setUp(self):

          self.config_path, self.config = self.read_config_data(CONFIG)

-         self.test_setup()

+         self.auto_test_setup()

  

      def tearDown(self):

          os.unlink(self.config_path)

+         self.auto_test_cleanup()

  

      def read_config_data(self, config_data):

          fd, config_path = tempfile.mkstemp()

file modified
+6 -5
@@ -1,3 +1,4 @@

+ import os

  from copr_rpmbuild.providers.base import Provider

  from . import TestCase

  
@@ -13,12 +14,11 @@

      def setUp(self):

          super(TestProvider, self).setUp()

          self.source_json = {}

-         self.resultdir = "/path/to/resultdir"

  

      @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open())

      @mock.patch('copr_rpmbuild.providers.base.os.mkdir')

      def test_create_rpmmacros(self, mock_mkdir, mock_open):

-         provider = Provider(self.source_json, self.resultdir, self.config)

+         provider = Provider(self.source_json, self.config)

          rpmmacros = mock.MagicMock()

          mock_open.return_value = rpmmacros

          provider.create_rpmmacros()
@@ -31,6 +31,7 @@

  

      @mock.patch('copr_rpmbuild.providers.base.os.mkdir')

      @mock.patch('copr_rpmbuild.providers.base.Provider.create_rpmmacros')

-     def test_workdir_in_outdir(self, mock_create_rpmmacros, mock_mkdir):

-         provider = Provider(self.source_json, self.resultdir, self.config)

-         assert provider.workdir == "/path/to/resultdir/obtain-sources"

+     def test_workdir_in_workspace(self, _mock_create_rpmmacros, _mock_mkdir):

+         ws = self.config.get("main", "workspace")

+         provider = Provider(self.source_json, self.config)

+         assert os.path.join(ws, "workdir-") in provider.workdir

@@ -56,7 +56,6 @@

          self.workdir = tempfile.mkdtemp(prefix="copr-distgit-provider-test-")

          self.origin = os.path.join(self.workdir, "origin.git")

          os.mkdir(self.origin)

-         self.outdir = os.path.join(self.workdir, "out")

          os.chdir(self.origin)

          self.lookaside = os.path.join(self.workdir, "lookaside")

          os.mkdir(self.lookaside)
@@ -72,9 +71,16 @@

          ])

          self._setup_configdir()

  

-         self.main_config = configparser.ConfigParser()

-         self.main_config.add_section("main")

-         self.main_config.set("main", "enabled_source_protocols", "file")

+         self.main_config = mc = configparser.ConfigParser()

+         mc.add_section("main")

+         mc.set("main", "enabled_source_protocols", "file")

+         mc.set("main", "resultdir", os.path.join(self.workdir, "results"))

+         mc.set("main", "workspace", os.path.join(self.workdir, "workspace"))

+ 

+         # these normally exist

+         os.makedirs(mc.get("main", "resultdir"))

+         os.makedirs(mc.get("main", "workspace"))

+ 

  

      def teardown_method(self, method):

          _unused_but_needed_for_el6 = (method)
@@ -82,12 +88,14 @@

          self.env_patch.stop()

  

      def test_distgit_method(self):

-         os.mkdir(self.outdir)

          source_dict = {"clone_url": self.origin}

-         dgp = DistGitProvider(source_dict, self.outdir, self.main_config)

+         dgp = DistGitProvider(source_dict, self.main_config)

+         # this is normally created in main.py

          dgp.produce_srpm()

-         assert os.path.exists(os.path.join(self.outdir, "obtain-sources",

-                                            "origin", "datafile"))

+ 

+         # check presence of the cloned file

+         cloned_file = os.path.join(dgp.workdir, "origin", "datafile")

+         assert os.path.exists(cloned_file)

  

  

  @pytest.mark.parametrize('committish', ["main", None, ""])

file modified
+40 -29
@@ -1,55 +1,66 @@

  import os

+ 

  import pytest

- import unittest

- import shutil

  

  from copr_common.enums import BuildSourceEnum

  

  from main import produce_srpm

  

+ from . import TestCase

  

  try:

       from unittest import mock

-      EPEL = False

  except ImportError:

       # Python 2 version depends on mock

       import mock

-      EPEL = True

  

  

- class TestTmpCleanup(unittest.TestCase):

+ class TestTmpCleanup(TestCase):

  

      config = {}

-     resultdir = "/path/to/non/existing/directory"

+     workdir = None

+     resultdir = None

+     workspace = None

+ 

      task = {"source_type": BuildSourceEnum.upload,

              "source_json": {"url": "http://foo.ex/somepackage.spec"}}

  

+     def auto_test_setup(self):

+         self.config_basic_dirs()

+ 

+     def auto_test_cleanup(self):

+         self.cleanup_basic_dirs()

+ 

+     @mock.patch("copr_rpmbuild.providers.base.Provider.cleanup")

      @mock.patch("copr_rpmbuild.providers.spec.UrlProvider.produce_srpm")

-     @mock.patch("main.shutil.rmtree", wraps=shutil.rmtree)

-     def test_produce_srpm_cleanup(self, mock_rmtree, mock_produce_srpm):

-         # Just to be sure, we are starting from zero

-         assert mock_rmtree.call_count == 0

+     def test_produce_srpm_cleanup_no(self, mock_produce_srpm, _cleanup):

+         # Test that we cleanup after successful build

+         produce_srpm(self.task, self.config)

+         # root + resultdir + workspace + not cleaned workdir

+         directories = list(os.walk(self.workdir))

+         assert len(directories) == 4

+ 

+         mock_produce_srpm.side_effect = RuntimeError("Jeeez, something failed")

+         with pytest.raises(RuntimeError):

+             produce_srpm(self.task, self.config)

+ 

+         # .. and plus one more workdir

+         directories = list(os.walk(self.workdir))

+         assert len(directories) == 5

  

+     @mock.patch("copr_rpmbuild.providers.spec.UrlProvider.produce_srpm")

+     def test_produce_srpm_cleanup_yes(self, mock_produce_srpm):

          # Test that we cleanup after successful build

-         produce_srpm(self.task, self.config, self.resultdir)

-         assert mock_rmtree.call_count == (1 if not EPEL else 2)

-         for call in mock_rmtree.call_args_list:

-             args, _ = call

-             assert args[0].startswith("/tmp/copr-rpmbuild-")

- 

-         # Just to check, that on EPEL it recursively removes one directory, 

-         # not two different directories. Do not run this check on Fedora/Python3

-         # because there the directory is removed on one rmtree call.

-         if EPEL:

-             assert mock_rmtree.call_args_list[1][0][0] == \

-                 os.path.join(mock_rmtree.call_args_list[0][0][0], "obtain-sources")

- 

-         # Test that we cleanup after unsuccessful build

+         produce_srpm(self.task, self.config)

+ 

+         # root + resultdir + workspace (cleaned workdir)

+         directories = list(os.walk(self.workdir))

+         assert len(directories) == 3

+ 

          mock_produce_srpm.side_effect = RuntimeError("Jeeez, something failed")

          with pytest.raises(RuntimeError):

-             produce_srpm(self.task, self.config, self.resultdir)

+             produce_srpm(self.task, self.config)

  

-         assert mock_rmtree.call_count == (2 if not EPEL else 4)

-         for call in mock_rmtree.call_args_list:

-             args, _ = call

-             assert args[0].startswith("/tmp/copr-rpmbuild-")

+         # root + resultdir + workspace (cleaned workdir)

+         directories = list(os.walk(self.workdir))

+         assert len(directories) == 3

file modified
+6 -4
@@ -22,7 +22,7 @@

      @mock.patch("{0}.open".format(builtins))

      @mock.patch('copr_rpmbuild.providers.base.os.mkdir')

      def test_init(self, mock_mkdir, mock_open):

-        provider = PyPIProvider(self.source_json, self.resultdir, self.config)

+        provider = PyPIProvider(self.source_json, self.config)

         self.assertEqual(provider.pypi_package_version, "1.52")

         self.assertEqual(provider.pypi_package_name, "motionpaint")

         self.assertEqual(provider.spec_template, "epel7")
@@ -32,8 +32,10 @@

      @mock.patch("{0}.open".format(builtins))

      @mock.patch('copr_rpmbuild.providers.base.os.mkdir')

      def test_produce_srpm(self, mock_mkdir, mock_open, run_cmd):

-         provider = PyPIProvider(self.source_json, "/some/tmp/directory", self.config)

+         provider = PyPIProvider(self.source_json, self.config)

          provider.produce_srpm()

-         assert_cmd = ["pyp2rpm", "motionpaint", "-t", "epel7", "--srpm", "-d", "/some/tmp/directory",

-                       "-b", "2", "-p", "3", "-v", "1.52"]

+         assert_cmd = [

+             "pyp2rpm", "motionpaint", "-t", "epel7", "--srpm",

+             "-d", self.config.get("main", "resultdir"),

+             "-b", "2", "-p", "3", "-v", "1.52"]

          run_cmd.assert_called_with(assert_cmd)

@@ -17,21 +17,21 @@

      def setUp(self):

          super(TestRubyGemsProvider, self).setUp()

          self.source_json = {"gem_name": "A_123"}

-         self.resultdir = "/path/to/resultdir"

  

      @mock.patch("{0}.open".format(builtins))

      @mock.patch('copr_rpmbuild.providers.base.os.mkdir')

      def test_init(self, mock_mkdir, mock_open):

-         provider = RubyGemsProvider(self.source_json, self.resultdir, self.config)

+         provider = RubyGemsProvider(self.source_json, self.config)

          self.assertEqual(provider.gem_name, "A_123")

  

      @mock.patch("copr_rpmbuild.providers.rubygems.run_cmd")

      @mock.patch("{0}.open".format(builtins))

      @mock.patch('copr_rpmbuild.providers.base.os.mkdir')

      def test_produce_srpm(self, mock_mkdir, mock_open, run_cmd):

-         provider = RubyGemsProvider(self.source_json, self.resultdir, self.config)

+         provider = RubyGemsProvider(self.source_json, self.config)

          provider.produce_srpm()

-         assert_cmd = ["gem2rpm", "A_123", "--srpm", "-C", "/path/to/resultdir", "--fetch"]

+         assert_cmd = ["gem2rpm", "A_123", "--srpm",

+                       "-C", self.config.get("main", "resultdir"), "--fetch"]

          run_cmd.assert_called_with(assert_cmd)

  

      @mock.patch("copr_rpmbuild.providers.rubygems.run_cmd")
@@ -42,7 +42,7 @@

                    "Command failed: rpmbuild -bs --nodeps --define '_sourcedir /tmp/gem2rpm-foo-20170905-3367-c2flks'"

                    "--define '_srcrpmdir .' /tmp/gem2rpm-foo-20170905-3367-c2flks/rubygem-foo.spec")

          run_cmd.return_value = Munch({"stderr": stderr})

-         provider = RubyGemsProvider(self.source_json, self.resultdir, self.config)

+         provider = RubyGemsProvider(self.source_json, self.config)

          with pytest.raises(RuntimeError) as ex:

              provider.produce_srpm()

              assert "Not specifying a license means all rights are reserved" in str(ex.exception)

file modified
+45 -19
@@ -1,5 +1,6 @@

  import tempfile

  import os

+ import stat

  import configparser

  import shutil

  
@@ -32,14 +33,13 @@

              "spec": "pkg.spec",

              "srpm_build_method": "rpkg",

          }

-         self.resultdir = "/path/to/resultdir"

  

      @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open())

      @mock.patch('copr_rpmbuild.providers.base.os.mkdir')

      def test_init(self, mock_mkdir, mock_open):

          source_json = self.source_json.copy()

  

-         provider = ScmProvider(source_json, self.resultdir, self.config)

+         provider = ScmProvider(source_json, self.config)

          self.assertEqual(provider.scm_type, "git")

          self.assertEqual(provider.clone_url, "https://example.org/somerepo.git")

          self.assertEqual(provider.committish, "f28")
@@ -53,7 +53,7 @@

  

          source_json["subdirectory"] = "/SOURCES"

          source_json["spec"] = "/SPECS/pkg.spec"

-         provider = ScmProvider(source_json, self.resultdir, self.config)

+         provider = ScmProvider(source_json, self.config)

          self.assertEqual(provider.repo_subdir, "/SOURCES")

          self.assertEqual(provider.spec_relpath, "/SPECS/pkg.spec")

          self.assertEqual(provider.repo_path, os.path.join(provider.workdir, "somerepo"))
@@ -62,6 +62,7 @@

  

      def test_generate_rpkg_config(self):

          tmpdir = tempfile.mkdtemp(prefix="copr-rpmbuild-test-")

+         self.config.set("main", "workspace", tmpdir)

          rpkg_tmpdir = tempfile.mkdtemp(prefix="copr-rpmbuild-test-", dir=tmpdir)

          rpkg_config = open(os.path.join(rpkg_tmpdir, "rpkg.conf.j2"), "w")

          rpkg_config.write(RPKG_CONF_JINJA)
@@ -71,7 +72,7 @@

          source_json["clone_url"] = "http://copr-dist-git.fedorainfracloud.org/git/clime/project/pkg.git"

  

          with mock.patch("copr_rpmbuild.providers.scm.CONF_DIRS", new=[rpkg_tmpdir]):

-             provider = ScmProvider(source_json, tmpdir, self.config)

+             provider = ScmProvider(source_json, self.config)

              rpkg_config_path = provider.generate_rpkg_config()

  

          config = configparser.RawConfigParser()
@@ -83,7 +84,7 @@

          source_json["clone_url"] = "http://unknownurl/git/clime/project/pkg.git"

  

          with mock.patch("copr_rpmbuild.providers.scm.CONF_DIRS", new=[rpkg_tmpdir]):

-             provider = ScmProvider(source_json, tmpdir, self.config)

+             provider = ScmProvider(source_json, self.config)

              rpkg_config_path = provider.generate_rpkg_config()

              self.assertEqual(rpkg_config_path, os.path.join(os.environ['HOME'], '.config', 'rpkg.conf'))

  
@@ -92,16 +93,19 @@

      @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open())

      @mock.patch('copr_rpmbuild.providers.base.os.mkdir')

      def test_get_rpkg_command(self, mock_mkdir, mock_open):

-         provider = ScmProvider(self.source_json, self.resultdir, self.config)

+         provider = ScmProvider(self.source_json, self.config)

          provider.generate_rpkg_config = mock.MagicMock(return_value="/etc/rpkg.conf")

-         assert_cmd = ["rpkg", "srpm", "--outdir", self.resultdir, "--spec", provider.spec_path]

+         assert_cmd = ["rpkg", "srpm",

+                       "--outdir", self.config.get("main", "resultdir"),

+                       "--spec", provider.spec_path]

          self.assertEqual(provider.get_rpkg_command(), assert_cmd)

  

      @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open())

      @mock.patch('copr_rpmbuild.providers.base.os.mkdir')

      def test_get_tito_command(self, mock_mkdir, mock_open):

-         provider = ScmProvider(self.source_json, self.resultdir, self.config)

-         assert_cmd = ["tito", "build", "--srpm", "--output", self.resultdir]

+         provider = ScmProvider(self.source_json, self.config)

+         assert_cmd = ["tito", "build", "--srpm",

+                       "--output", self.config.get("main", "resultdir")]

          self.assertEqual(provider.get_tito_command(), assert_cmd)

  

  
@@ -109,22 +113,44 @@

      @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open())

      @mock.patch('copr_rpmbuild.providers.base.os.mkdir')

      def test_get_tito_test_command(self, mock_mkdir, mock_open, run_cmd_mock):

-         provider = ScmProvider(self.source_json, self.resultdir, self.config)

-         assert_cmd = ["tito", "build", "--test", "--srpm", "--output", self.resultdir]

+         provider = ScmProvider(self.source_json, self.config)

+         assert_cmd = ["tito", "build", "--test", "--srpm",

+                       "--output", self.config.get("main", "resultdir")]

          self.assertEqual(provider.get_tito_test_command(), assert_cmd)

  

      @mock.patch("copr_rpmbuild.providers.scm.get_mock_uniqueext")

      @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open())

-     @mock.patch('copr_rpmbuild.providers.base.os.mkdir')

-     def test_get_make_srpm_command(self, mock_mkdir, mock_open, get_mock_uniqueext_mock):

+     def test_get_make_srpm_command(self, mock_open, get_mock_uniqueext_mock):

+         tmpdir = tempfile.mkdtemp(prefix="copr-rpmbuild-test-")

+         ws = os.path.join(tmpdir, "workspace")

+         rd = os.path.join(tmpdir, "resultdir")

+         os.makedirs(ws)

+         os.makedirs(rd)

+         self.config.set("main", "workspace", ws)

+         self.config.set("main", "resultdir", rd)

+ 

          get_mock_uniqueext_mock.return_value = '2'

-         provider = ScmProvider(self.source_json, self.resultdir, self.config)

-         bind_mount_cmd_part = '--plugin-option=bind_mount:dirs=(("{0}", "/mnt{1}"), ("{2}", "/mnt{3}"))'\

-                               .format(provider.workdir, provider.workdir, self.resultdir, self.resultdir)

-         make_srpm_cmd_part = 'cd /mnt{0}/somerepo/subpkg; make -f /mnt{1}/somerepo/.copr/Makefile srpm '\

-                              'outdir="/mnt{2}" spec="/mnt{3}/somerepo/subpkg/pkg.spec"'\

-                              .format(provider.workdir, provider.workdir, self.resultdir, provider.workdir)

+         self.source_json['srpm_build_method'] = 'make_srpm'

+         provider = ScmProvider(self.source_json, self.config)

+ 

+         for directory in [provider.resultdir, provider.workdir]:

+             assert stat.S_IMODE(os.stat(directory).st_mode) == 0o707

+ 

+         resultdir = provider.resultdir

+         basename = os.path.basename(resultdir)

+         workdir_base = os.path.basename(provider.workdir)

+ 

+         bind_mount_cmd_part = '--plugin-option=bind_mount:dirs=(("{0}", "/mnt/{1}"), ("{2}", "/mnt/{3}"))'\

+                               .format(provider.workdir, workdir_base,

+                                       resultdir, basename)

+         make_srpm_cmd_part = 'cd /mnt/{wb}/somerepo/subpkg; make -f /mnt/{wb}/somerepo/.copr/Makefile srpm '\

+                              'outdir="/mnt/{rb}" spec="/mnt/{wb}/somerepo/subpkg/pkg.spec"'\

+                              .format(

+             wb=workdir_base,

+             rb=basename,

+         )

          assert_cmd = ['mock', '--uniqueext', '2', '-r', '/etc/copr-rpmbuild/mock-source-build.cfg',

                        bind_mount_cmd_part, '--chroot', make_srpm_cmd_part]

  

          self.assertEqual(provider.get_make_srpm_command(), assert_cmd)

+         shutil.rmtree(tmpdir)

file modified
+13 -12
@@ -3,7 +3,6 @@

  import os

  import distro

  import pytest

- import tempfile

  

  try:

      from httmock import urlmatch, HTTMock
@@ -29,14 +28,13 @@

  

  

  class TestUrlProvider(TestCase):

-     def test_setup(self):

+     def auto_test_setup(self):

          self.source_json = {"url": u"http://foo.ex/somepackage.spec"}

-         self.resultdir = "/path/to/resultdir"

  

      @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open())

      @mock.patch('copr_rpmbuild.providers.base.os.mkdir')

      def test_init(self, mock_mkdir, mock_open):

-         provider = UrlProvider(self.source_json, self.resultdir, self.config)

+         provider = UrlProvider(self.source_json, self.config)

          self.assertEqual(provider.url, "http://foo.ex/somepackage.spec")

  

      @mock.patch('requests.get')
@@ -44,43 +42,46 @@

      @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open())

      @mock.patch('copr_rpmbuild.providers.base.os.mkdir')

      def test_produce_srpm(self, mock_mkdir, mock_open, run_cmd, mock_get):

-         provider = UrlProvider(self.source_json, self.resultdir, self.config)

+         provider = UrlProvider(self.source_json, self.config)

          provider.produce_srpm()

          args = [

              'mock', '-r', '/etc/copr-rpmbuild/mock-source-build.cfg',

              '--buildsrpm',

              '--spec', '{0}/somepackage.spec'.format(provider.workdir),

              '--define', '_disable_source_fetch 0',

-             '--resultdir', self.resultdir]

+             '--resultdir', self.config.get("main", "resultdir")]

          run_cmd.assert_called_with(args, cwd=provider.workdir)

  

      @mock.patch('requests.get')

      @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open())

      @mock.patch('copr_rpmbuild.providers.base.os.mkdir')

      def test_save_spec(self, mock_mkdir, mock_open, mock_get):

-         provider = UrlProvider(self.source_json, self.resultdir, self.config)

+         provider = UrlProvider(self.source_json, self.config)

          provider.save_spec()

          mock_open.assert_called_with("{0}/somepackage.spec".format(provider.workdir), "w")

  

  

  class TestUrlProviderQueryString(TestCase):

-     def test_setup(self):

+     def auto_test_setup(self):

          self.json_1 = {

              'url': "http://example.com/"

                     "srelay-0.4.8p3-0.20181224.git688764b.fc10.3sunshine.src.rpm?dl=1",

          }

          self.json_2 = { 'url': "http://example.com/test.spec?a=1&b=2" }

-         self.resultdir = tempfile.mkdtemp(prefix="copr-rpmbuild-test-")

+         self.config_basic_dirs()

+ 

+     def auto_test_cleanup(self):

+         self.cleanup_basic_dirs()

  

      @pytest.mark.skipif(distro.id() in ['rhel', 'centos'] and

                              distro.major_version() == '6',

                          reason='on httmock on rhel6')

      def test_srpm_query_string(self):

          with HTTMock(example_com_match):

-             provider = UrlProvider(self.json_1, self.resultdir, self.config)

+             provider = UrlProvider(self.json_1, self.config)

              provider.produce_srpm()

              file = os.path.join(

-                     self.resultdir,

+                     self.config.get("main", "resultdir"),

                      "srelay-0.4.8p3-0.20181224.git688764b.fc10.3sunshine.src.rpm",

              )

              with open(file, 'r') as f:
@@ -91,7 +92,7 @@

                          reason='on httmock on rhel6')

      def test_spec_query_string(self):

          with HTTMock(example_com_match):

-             provider = UrlProvider(self.json_2, self.resultdir, self.config)

+             provider = UrlProvider(self.json_2, self.config)

              filename = provider.save_spec()

              with open(filename, 'r') as f:

                  assert f.read() == 'some-content'

see particular commits for more info

Build succeeded.

Metadata Update from @praiskup:
- Pull-request tagged with: needs-work

3 years ago

1 new commit added

  • fix 1
3 years ago

Build succeeded.

3 new commits added

  • rpmbuild: clarify and encapsulate Provider's directories
  • rpmbuild: cleanup the Provider class API
  • rpmbuild: better error for cleanup issue#1258
3 years ago

I updated the PR to use dir= argument for mkdtemp, per suggestion ion rhbz#1940456.

Metadata Update from @praiskup:
- Pull-request untagged with: needs-work

3 years ago

Build succeeded.

3 new commits added

  • rpmbuild: clarify and encapsulate Provider's directories
  • rpmbuild: cleanup the Provider class API
  • rpmbuild: better error for cleanup issue#1258
3 years ago

Build succeeded.

TODO: document the directories properly in example config

rebased onto d2afed8

3 years ago

Build succeeded.

4 new commits added

  • rpmbuild: document the options in main.ini file
  • rpmbuild: clarify and encapsulate Provider's directories
  • rpmbuild: cleanup the Provider class API
  • rpmbuild: better error for cleanup issue#1258
3 years ago

Build succeeded.

A typo here, s/donwnload/download

LGTM.
The in-depth documentation is very appreciated, thank you.

Commit e319d6f fixes this pull-request

Pull-Request has been merged by praiskup

3 years ago

Thank you for the review!