#509 layout module
Merged 3 years ago by onosek. Opened 3 years ago by lrossett.
lrossett/rpkg layout-module  into  master

file modified
+18 -13
@@ -49,6 +49,7 @@ 

  from pyrpkg.utils import (cached_property, extract_srpm, find_me,

                            is_file_tracked, is_lookaside_eligible_file,

                            log_result)

+ from pyrpkg import layout

  

  from .gitignore import GitIgnore

  
@@ -212,6 +213,8 @@ 

          self.block_retire_ns = ['rpms']

          # Git excludes patterns

          self.git_excludes = git_excludes or []

+         # Layout setup

+         self.layout = layout.build(self.path)

  

      # Define properties here

      # Properties allow us to "lazy load" various attributes, which also means
@@ -759,14 +762,16 @@ 

              raise rpkgError('Could not find the base OS ver from branch name'

                              ' %s. Consider using --release option' %

                              self.branch_merge)

+         if self.layout is None:

+             raise rpkgError('Unexpected error while loading the package.')

          self._distvar, self._distval = osver.split('-')

          self._distval = self._distval.replace('.', '_')

          self._disttag = 'el%s' % self._distval

-         self._rpmdefines = ["--define '_sourcedir %s'" % self.path,

-                             "--define '_specdir %s'" % self.path,

-                             "--define '_builddir %s'" % self.path,

-                             "--define '_srcrpmdir %s'" % self.path,

-                             "--define '_rpmdir %s'" % self.path,

+         self._rpmdefines = ["--define '_sourcedir %s'" % self.layout.sourcedir,

+                             "--define '_specdir %s'" % self.layout.specdir,

+                             "--define '_builddir %s'" % self.layout.builddir,

+                             "--define '_srcrpmdir %s'" % self.layout.srcrpmdir,

+                             "--define '_rpmdir %s'" % self.layout.rpmdir,

                              "--define 'dist .%s'" % self._disttag,

                              "--define '%s %s'" % (self._distvar,

                                                    self._distval.split('_')[0]),
@@ -784,11 +789,14 @@ 

      def load_spec(self):

          """This sets the spec attribute"""

  

+         if self.layout is None:

+             raise rpkgError('Spec file is not available')

+ 

          if self.is_retired():

              raise rpkgError('This package or module is retired. The action has stopped.')

  

          # Get a list of files in the path we're looking at

-         files = os.listdir(self.path)

+         files = os.listdir(self.layout.specdir)

          # Search the files for the first one that ends with ".spec"

          for f in files:

              if f.endswith('.spec') and not f.startswith('.'):
@@ -1015,7 +1023,9 @@ 

  

      @property

      def sources_filename(self):

-         return os.path.join(self.path, 'sources')

+         if self.layout is None:

+             return os.path.join(self.path, 'sources')

+         return os.path.join(self.path, self.layout.sources_file_template)

  

      @property

      def osbs_config_filename(self):
@@ -3188,12 +3198,7 @@ 

          The state is indicated by present of files 'dead.package'

          or 'dead.module'.

          """

-         marker = 'dead.package'

-         if os.path.isfile(os.path.join(self.path, marker)):

-             return marker

-         marker = 'dead.module'

-         if os.path.isfile(os.path.join(self.path, marker)):

-             return marker

+         return self.layout.is_retired()

  

      def retire(self, message):

          """Delete all tracked files and commit a new dead.package file for rpms

file modified
+5
@@ -62,3 +62,8 @@ 

  

      def __unicode__(self):

          return six.text_type(self.message)

+ 

+ 

+ class LayoutError(rpkgError):

+     """Raised when something went wrong while parsing/loading a layout"""

+     pass

@@ -0,0 +1,30 @@ 

+ # Copyright (c) 2015 - Red Hat Inc.

+ #

+ # This program is free software; you can redistribute it and/or modify it

+ # under the terms of the GNU General Public License as published by the

+ # Free Software Foundation; either version 2 of the License, or (at your

+ # option) any later version.  See http://www.gnu.org/copyleft/gpl.html for

+ # the full text of the license.

+ 

+ """package layout management"""

+ 

+ 

+ from pyrpkg.errors import LayoutError

+ from .base import MetaLayout

+ from .layouts import DistGitLayout, SRPMLayout # noqa

+ 

+ 

+ def build(path):

+     """

+     Tries to create a layout instance based on MetaLayout._layouts

+     and will return an instance in the first successfull attempt to

+     create a new object from the MetaLayout._layouts list.

+ 

+     Returns None if no layouts can be created.

+     """

+     for layout in MetaLayout._layouts:

+         try:

+             return layout.from_path(path)

+         except LayoutError:

+             continue

+     return None

@@ -0,0 +1,93 @@ 

+ # Copyright (c) 2015 - Red Hat Inc.

+ #

+ # This program is free software; you can redistribute it and/or modify it

+ # under the terms of the GNU General Public License as published by the

+ # Free Software Foundation; either version 2 of the License, or (at your

+ # option) any later version.  See http://www.gnu.org/copyleft/gpl.html for

+ # the full text of the license.

+ 

+ """base and meta package layout classes"""

+ 

+ 

+ import os

+ import abc

+ from abc import abstractmethod, ABCMeta

+ 

+ import six

+ 

+ from pyrpkg.errors import LayoutError

+ 

+ 

+ if six.PY2:

+     ABC = abc.ABCMeta('ABC', (object,), {'__slots__': ()})

+ else:

+     ABC = abc.ABC

+ 

+ 

+ class MetaLayout(type):

+     """

+     Layout meta class that keeps track of all layout subclasses.

+ 

+     Classes are appended in the _layouts property in order of code execution.

+     """

+     def __init__(cls, name, bases, dct):

+         """

+         This method registers a subclass in a class property (_layouts)

+         so those can be used as valid layout classes.

+         """

+         if not hasattr(cls, '_layouts'):

+             MetaLayout._layouts = []

+         if cls not in cls._layouts and cls.__name__ != 'BaseLayout':

+             MetaLayout._layouts.append(cls)

+         super(MetaLayout, cls).__init__(name, bases, dct)

+ 

+ 

+ class ABCMetaLayout(MetaLayout, ABCMeta):

+     """

+     A mixin metaclass to enable usage of both Metalayout and ABC meta classes as a single metaclass.

+     """

+     pass

+ 

+ 

+ @six.add_metaclass(ABCMetaLayout)

+ class BaseLayout(ABC):

+     """

+     The abstract class to be inherited from when implemeting specific layouts.

+ 

+     Every subclass will be registered in the MetaLayout class.

+ 

+     Inherited classes will be registered in order or code execution.

+     """

+     root_dir = None

+     sourcedir = None

+     specdir = None

+     builddir = None

+     rpmdir = None

+     srcrpmdir = None

+     sources_file_template = None

+ 

+     @classmethod

+     @abstractmethod

+     def from_path(cls, path):

+         """

+         Class constructor based on a package path.

+ 

+         This method's implementation is madatory and

+         should return an instance of the object class.

+ 

+         It should return None if it can't read the path

+         or if the dir path contains an invalid layout.

+         """

+         if not os.path.exists(path):

+             raise LayoutError('package path does not exist')

+ 

+     def is_retired(self):

+         """

+         Checks whether package or module is already retired.

+         The state is indicated by present of files 'dead.package'

+         or 'dead.module'.

+         """

+         for fname in ['dead.package', 'dead.module']:

+             if os.path.exists('%s/%s' % (self.root_dir, fname)):

+                 return fname

+         return None

@@ -0,0 +1,80 @@ 

+ # Copyright (c) 2015 - Red Hat Inc.

+ #

+ # This program is free software; you can redistribute it and/or modify it

+ # under the terms of the GNU General Public License as published by the

+ # Free Software Foundation; either version 2 of the License, or (at your

+ # option) any later version.  See http://www.gnu.org/copyleft/gpl.html for

+ # the full text of the license.

+ 

+ """package layout implementation"""

+ 

+ 

+ import os

+ 

+ from .base import BaseLayout

+ from pyrpkg.errors import LayoutError

+ 

+ 

+ class DistGitLayout(BaseLayout):

+     """

+     This class represents a dist-git package layout.

+     """

+     def __init__(self, root_dir=None, sources_file_template='sources'):

+         """

+         Default class constructor to create a new object instance.

+         """

+         self.root_dir = root_dir

+         self.sourcedir = root_dir

+         self.specdir = root_dir

+         self.builddir = root_dir

+         self.rpmdir = root_dir

+         self.srcrpmdir = root_dir

+         self.sources_file_template = sources_file_template

+ 

+     @classmethod

+     def from_path(cls, path):

+         """

+         Creates a new object instance from a valid path in the file system.

+ 

+         Returns none if path or package structure is invalid.

+         """

+         super(DistGitLayout, cls).from_path(path)

+ 

+         if len([f for f in os.listdir(path) if f.endswith('.spec')]) == 0:

+             raise LayoutError('spec file not found.')

+         sources = 'sources' if os.path.exists('%s/sources' % path) else None

+         return cls(root_dir=path, sources_file_template=sources)

+ 

+ 

+ class SRPMLayout(BaseLayout):

+     """

+     This class represents an exposed source RPM package layout.

+     """

+     def __init__(self, root_dir=None, sources_file_template='.{0.repo_name}.metadata'):

+         """

+         Default class constructor to create a new object instance.

+         """

+         self.root_dir = root_dir

+         self.sourcedir = os.path.join(root_dir, 'SOURCES')

+         self.specdir = os.path.join(root_dir, 'SPECS')

+         self.builddir = os.path.join(root_dir, 'BUILD')

+         self.rpmdir = os.path.join(root_dir, 'RPMS')

+         self.srcrpmdir = os.path.join(root_dir, 'SRPMS')

+         self.sources_file_template = sources_file_template

+ 

+     @classmethod

+     def from_path(cls, path):

+         """

+         Creates a new object instance from a valid path in the file system.

+ 

+         Returns none if path or package structure is invalid.

+         """

+         super(SRPMLayout, cls).from_path(path)

+ 

+         if not os.path.exists(os.path.join(path, 'SPECS')):

+             raise LayoutError('SPECS dir not found.')

+         if len([f for f in os.listdir(os.path.join(path, 'SPECS')) if f.endswith('.spec')]) == 0:

+             raise LayoutError('spec file not found.')

+         if len([f for f in os.listdir(path) if f.endswith('.metadata')]) == 0:

+             raise LayoutError('metadata file not found.')

+         return cls(root_dir=path)

empty or binary file added
empty or binary file added
empty or binary file added
empty or binary file added
empty or binary file added
empty or binary file added
empty or binary file added
empty or binary file added
empty or binary file added
empty or binary file added
empty or binary file added
empty or binary file added
empty or binary file added
empty or binary file added
file modified
+15 -7
@@ -19,7 +19,7 @@ 

  import koji_cli.lib

  import pyrpkg.cli

  import utils

- from pyrpkg import Commands, Modulemd, rpkgError

+ from pyrpkg import Commands, Modulemd, rpkgError, layout

  from utils import CommandTestCase, FakeThreadPool

  

  try:
@@ -64,6 +64,11 @@ 

      return(os.path.basename(rpm_file)[:-len('.src.rpm')])

  

  

+ class MockLayout(layout.DistGitLayout):

+     def is_retired(self):

+         return None

+ 

+ 

  class CliTestCase(CommandTestCase):

  

      def new_cli(self, cfg=None):
@@ -1561,11 +1566,13 @@ 

      @patch('os.path.exists', return_value=False)

      def test_use_mock_config_got_from_koji(

              self, exists, config_dir_other, config_dir_basic):

-         config_dir_basic.return_value = '/path/to/config-dir'

+         mock_layout = layout.DistGitLayout(root_dir=self.cloned_repo_path)

+         with patch('pyrpkg.layout.build', return_value=mock_layout):

+             config_dir_basic.return_value = '/path/to/config-dir'

  

-         cli_cmd = ['rpkg', '--path', self.cloned_repo_path,

-                    '--release', 'rhel-7', 'mockbuild']

-         self.mockbuild(cli_cmd)

+             cli_cmd = ['rpkg', '--path', self.cloned_repo_path,

+                        '--release', 'rhel-7', 'mockbuild']

+             self.mockbuild(cli_cmd)

  

          args, kwargs = self.mock_run_command.call_args

          cmd_to_execute = args[0]
@@ -3449,8 +3456,9 @@ 

      @patch('pyrpkg.Commands.nvr', new_callable=PropertyMock)

      @patch('pyrpkg.Commands._run_command')

      def test_option_srpm_by_generate_srpm_from_repo(self, _run_command, nvr):

-         nvr.return_value = 'docpkg-0.1-1.fc28'

-         self.assert_option_srpm_use()

+         with patch('pyrpkg.layout.build', return_value=MockLayout(root_dir=self.cloned_repo_path)):

+             nvr.return_value = 'docpkg-0.1-1.fc28'

+             self.assert_option_srpm_use()

  

          args, kwargs = _run_command.call_args

          self.assertEqual({'shell': True}, kwargs)

@@ -0,0 +1,50 @@ 

+ import os

+ import unittest

+ 

+ from pyrpkg.layout import layouts

+ from pyrpkg import errors

+ 

+ 

+ fixtures_dir = os.path.join(os.path.dirname(__file__), 'fixtures')

+ 

+ 

+ class DistGitLayoutTestCase(unittest.TestCase):

+     def setUp(self):

+         self.workdir = os.path.join(fixtures_dir, 'layouts/dist-git')

+         self.layout = layouts.DistGitLayout.from_path(self.workdir)

+ 

+     def test_layout_data(self):

+         self.assertEqual(self.layout.sourcedir, self.workdir)

+         self.assertEqual(self.layout.specdir, self.workdir)

+         self.assertEqual(self.layout.specdir, self.workdir)

+         self.assertEqual(self.layout.root_dir, self.workdir)

+         self.assertEqual(self.layout.builddir, self.workdir)

+         self.assertEqual(self.layout.rpmdir, self.workdir)

+         self.assertEqual(self.layout.srcrpmdir, self.workdir)

+         self.assertEqual(self.layout.sources_file_template, 'sources')

+ 

+     def test_layout_retired(self):

+         self.assertEqual(None, self.layout.is_retired())

+ 

+ 

+ class DistGitLayoutErrorsTestCase(unittest.TestCase):

+     def setUp(self):

+         self.workdir = os.path.join(fixtures_dir, 'layouts')

+ 

+     def test_path_error(self):

+         with self.assertRaises(errors.LayoutError) as e:

+             layouts.DistGitLayout.from_path(os.path.join(self.workdir, 'notfound'))

+         self.assertEqual('package path does not exist', e.exception.args[0])

+ 

+     def test_specless_error(self):

+         with self.assertRaises(errors.LayoutError) as e:

+             layouts.DistGitLayout.from_path(os.path.join(self.workdir, 'specless'))

+         self.assertEqual('spec file not found.', e.exception.args[0])
-        self.assertEqual('spec file not found.', e.exception.args[0])
+        self.assertEqual('package path does not exist', e.exception.args[0])

super method raises its exception (from "abstract" method) earlier than comparation in a specific method is reached. In the current situation, input of the test can not be not existing dir.

+ 

+     def test_dead_module_error(self):

+         layout = layouts.DistGitLayout.from_path(os.path.join(self.workdir, 'dead-module'))

+         self.assertEqual('dead.module', layout.is_retired())

+ 

+     def test_dead_package_error(self):

+         layout = layouts.DistGitLayout.from_path(os.path.join(self.workdir, 'dead-package'))

+         self.assertEqual('dead.package', layout.is_retired())

@@ -0,0 +1,49 @@ 

+ import os

+ import unittest

+ 

+ from pyrpkg.layout import layouts

+ from pyrpkg import errors

+ 

+ 

+ fixtures_dir = os.path.join(os.path.dirname(__file__), 'fixtures')

+ 

+ 

+ class SRPMLayoutTestCase(unittest.TestCase):

+     def setUp(self):

+         self.workdir = os.path.join(fixtures_dir, 'layouts/srpm')

+         self.layout = layouts.SRPMLayout.from_path(self.workdir)

+ 

+     def test_layout_data(self):

+         self.assertEqual(self.layout.sourcedir, os.path.join(self.workdir, 'SOURCES'))

+         self.assertEqual(self.layout.specdir, os.path.join(self.workdir, 'SPECS'))

+         self.assertEqual(self.layout.root_dir, self.workdir)

+         self.assertEqual(self.layout.builddir, os.path.join(self.workdir, 'BUILD'))

+         self.assertEqual(self.layout.rpmdir, os.path.join(self.workdir, 'RPMS'))

+         self.assertEqual(self.layout.srcrpmdir, os.path.join(self.workdir, 'SRPMS'))

+         self.assertEqual(self.layout.sources_file_template, '.{0.repo_name}.metadata')

+ 

+     def test_layout_retired(self):

+         self.assertEqual(None, self.layout.is_retired())

+ 

+ 

+ class SRPMLayoutErrorsTestCase(unittest.TestCase):

+     def setUp(self):

+         self.workdir = os.path.join(fixtures_dir, 'layouts')

+ 

+     def test_path_error(self):

+         with self.assertRaises(errors.LayoutError) as e:

+             layouts.SRPMLayout.from_path(os.path.join(self.workdir, 'notfound'))

+         self.assertEqual('package path does not exist', e.exception.args[0])

+ 

+     def test_specless_error(self):

+         with self.assertRaises(errors.LayoutError) as e:

+             layouts.SRPMLayout.from_path(os.path.join(self.workdir, 'srpm-specless'))

+         self.assertEqual('spec file not found.', e.exception.args[0])
-        self.assertEqual('spec file not found.', e.exception.args[0])
+        self.assertEqual('SPECS dir not found.', e.exception.args[0])

Unlike the previous failed test case, we have dir that exists (srpm-specless). So it ends with raising exception during SPECS directory test.

+ 

+     def test_dead_module_error(self):

+         layout = layouts.SRPMLayout.from_path(os.path.join(self.workdir, 'srpm-dead-module'))

+         self.assertEqual('dead.module', layout.is_retired())

+ 

+     def test_dead_package_error(self):

+         layout = layouts.SRPMLayout.from_path(os.path.join(self.workdir, 'srpm-dead-package'))

+         self.assertEqual('dead.package', layout.is_retired())

Signed-off-by: lrossett lrossett@redhat.com
This PR is a continuation of the work done on https://pagure.io/rpkg/pull-request/393 by @bstinson.

There are still a few code pieces that need to be changed but I wanted to get an early feedback for the proposed solution.

Proposal

  • Adds a Layout metaclass and base class to be inherited by other layout implementation classes;
  • Every class that inherits from BaseLayout gets automatically added into the metaclass to be used by the build function;
  • Layout classes are added by order of code definition in the _layouts list;

rebased onto ef229c2c5c1555f3624f9673387a6cc9038dc931

3 years ago

rebased onto 9d58a539634ac1f48e3855f01945a34fc5d45c0d

3 years ago

rebased onto afcc2bc1975462699b378cbc6890167954e7d802

3 years ago

rebased onto 91da14d4946ec3ba7a2c8e48e625b30e3b768cbe

3 years ago

rebased onto 26141414394e75849966e6b84faf549d84dfc61e

3 years ago

pretty please pagure-ci rebuild

3 years ago

can wildcard be avoided somehow?

May I ask you to add a comment to the commit message? Something describing the purpose, like the description of the PR itself (or some text from the original #393).

I have noticed that in tests/test_cli.py there is self.srcrpmdir used. Doesn't it conflict with newly moved variable to self.layout.srcrpmdir?

isn't self.srcrpmdir just a property of the test class that is used in a cli test case?

rebased onto 81a0ee184e052f34d3c98abc339641efde3bdb7e

3 years ago

rebased onto 31c7c6535b2b50b584d3bd5f635a2cc5f2d49bc0

3 years ago
-        self.assertEqual('spec file not found.', e.exception.args[0])
+        self.assertEqual('package path does not exist', e.exception.args[0])

super method raises its exception (from "abstract" method) earlier than comparation in a specific method is reached. In the current situation, input of the test can not be not existing dir.

-        self.assertEqual('spec file not found.', e.exception.args[0])
+        self.assertEqual('SPECS dir not found.', e.exception.args[0])

Unlike the previous failed test case, we have dir that exists (srpm-specless). So it ends with raising exception during SPECS directory test.

Above suggestions should resolve failing unittests without changing the code itself. But if the intended behaviour was different, please, modify the code in Layout classes.

isn't self.srcrpmdir just a property of the test class that is used in a cli test case?

OK, it looks you are right. It doesn't colide with the new functionality.

ok I will apply those changes but I am confused because tox was working locally and applying it made it fail now

rebased onto a95bad2

3 years ago

I think the problem was that those empty folders were not being committed so I added an empty file (.gitkeep) in those dirs.

I think the problem was that those empty folders were not being committed so I added an empty file (.gitkeep) in those dirs.

Yes, it looks like it. That is great. We can move further. I will merge it.

Pull-Request has been merged by onosek

3 years ago
Changes Summary 32
+18 -13
file changed
pyrpkg/__init__.py
+5 -0
file changed
pyrpkg/errors.py
+30
file added
pyrpkg/layout/__init__.py
+93
file added
pyrpkg/layout/base.py
+80
file added
pyrpkg/layout/layouts.py
+0
file added
tests/fixtures/layouts/dead-module/dead.module
+0
file added
tests/fixtures/layouts/dead-module/foobar.spec
+0
file added
tests/fixtures/layouts/dead-package/dead.package
+0
file added
tests/fixtures/layouts/dead-package/foobar.spec
+0
file added
tests/fixtures/layouts/dist-git/foobar.spec
+0
file added
tests/fixtures/layouts/dist-git/foobar.txt
+0
file added
tests/fixtures/layouts/dist-git/sources
+0
file added
tests/fixtures/layouts/specless/.gitkeep
+0
file added
tests/fixtures/layouts/srpm-dead-module/.foobar.metadata
+0
file added
tests/fixtures/layouts/srpm-dead-module/SOURCES/foobar-firstcommit.patch
+0
file added
tests/fixtures/layouts/srpm-dead-module/SPECS/foobar.spec
+0
file added
tests/fixtures/layouts/srpm-dead-module/dead.module
+0
file added
tests/fixtures/layouts/srpm-dead-package/.foobar.metadata
+0
file added
tests/fixtures/layouts/srpm-dead-package/SOURCES/foobar-firstcommit.patch
+0
file added
tests/fixtures/layouts/srpm-dead-package/SPECS/foobar.spec
+0
file added
tests/fixtures/layouts/srpm-dead-package/dead.package
+0
file added
tests/fixtures/layouts/srpm-dead-package/srpm/.foobar.metadata
+0
file added
tests/fixtures/layouts/srpm-dead-package/srpm/SOURCES/foobar-firstcommit.patch
+0
file added
tests/fixtures/layouts/srpm-dead-package/srpm/SPECS/foobar.spec
+0
file added
tests/fixtures/layouts/srpm-specless/.foobar.metadata
+0
file added
tests/fixtures/layouts/srpm-specless/SPECS/.gitkeep
+0
file added
tests/fixtures/layouts/srpm/.foobar.metadata
+0
file added
tests/fixtures/layouts/srpm/SOURCES/foobar-firstcommit.patch
+0
file added
tests/fixtures/layouts/srpm/SPECS/foobar.spec
+15 -7
file changed
tests/test_cli.py
+50
file added
tests/test_layout_distgit.py
+49
file added
tests/test_layout_srpm.py