#330 Add new option --name and --namespace to deprecate --module-name
Merged 3 years ago by cqi. Opened 3 years ago by cqi.

file modified
+176 -109
@@ -123,7 +123,7 @@ 

          self._distval = None

          # The distvar rpm value

          self._distvar = None

-         # The rpm epoch of the cloned module

+         # The rpm epoch of the cloned package

          self._epoch = None

          # An authenticated buildsys session

          self._kojisession = None
@@ -133,21 +133,21 @@ 

          self._localarch = None

          # A property to load the mock config

          self._mockconfig = None

-         # The name of the cloned module

+         # The name of the cloned repository

          self._module_name = None

          # The dist git namespace

          self._ns = None

-         # The name of the module from spec file

-         self._module_name_spec = None

-         # The rpm name-version-release of the cloned module

+         # The package name from spec file

+         self._package_name_spec = None

+         # The rpm name-version-release of the cloned package

          self._nvr = None

-         # The rpm release of the cloned module

+         # The rpm release of the cloned package

          self._rel = None

          # The cloned repo object

          self._repo = None

          # The rpm defines used when calling rpm

          self._rpmdefines = None

-         # The specfile in the cloned module

+         # The specfile in the cloned package

          self._spec = None

          # The build target within the buildsystem

          self._target = target
@@ -161,7 +161,7 @@ 

          self._password = None

          # The alternate Koji user to run commands as

          self._runas = None

-         # The rpm version of the cloned module

+         # The rpm version of the cloned package

          self._ver = None

          self.log = log

          # Pushurl or url of remote of branch
@@ -186,6 +186,8 @@ 

          # package name will be sent to lookaside CGI script as 'namespace/name'

          # instead of just name.

          self.lookaside_namespaced = lookaside_namespaced

+         # Deprecates self.module_name

+         self._repo_name = None

  

      # Define properties here

      # Properties allow us to "lazy load" various attributes, which also means
@@ -529,7 +531,7 @@ 

  

      @property

      def localarch(self):

-         """This property ensures the module attribute"""

+         """This property ensures the localarch attribute"""

  

          if not self._localarch:

              self.load_localarch()
@@ -561,19 +563,35 @@ 

          self._mockconfig = '%s-%s' % (self.target, self.localarch)

  

      @property

+     def repo_name(self):

+         """Property to get repository name"""

+         if not self._repo_name:

+             self.load_repo_name()

+         return self._repo_name

+ 

+     @repo_name.setter

+     def repo_name(self, name):

+         """Set repository name"""

+         self._repo_name = name

+ 

+     @property

      def module_name(self):

          """This property ensures the module attribute"""

- 

-         if not self._module_name:

-             self.load_module_name()

-         return self._module_name

+         self.log.warning('Property module_name is deprecated. Please use '

+                          'repo_name instead.')

+         return self.repo_name

  

      @module_name.setter

      def module_name(self, module_name):

-         self._module_name = module_name

+         self.log.warning('Property module_name is deprecated. Please use '

+                          'repo_name instead.')

+         self.repo_name = module_name

  

      def load_module_name(self):

-         """Loads a package module."""

+         return self.load_repo_name()

+ 

+     def load_repo_name(self):

+         """Loads repository name"""

  

          try:

              if self.push_url:
@@ -582,26 +600,26 @@ 

                  # FIXME

                  # if self.distgit_namespaced:

                  #     self._module_name = "/".join(parts.path.split("/")[-2:])

-                 module_name = posixpath.basename(parts.path.strip('/'))

+                 repo_name = posixpath.basename(parts.path.strip('/'))

  

-                 if module_name.endswith('.git'):

-                     module_name = module_name[:-len('.git')]

-                 self._module_name = module_name

+                 if repo_name.endswith('.git'):

+                     repo_name = repo_name[:-len('.git')]

+                 self._repo_name = repo_name

                  return

          except rpkgError:

-             self.log.warning('Failed to get module name from Git url or pushurl')

+             self.log.warning('Failed to get repository name from Git url or pushurl')

  

          self.load_nameverrel()

-         if self._module_name_spec:

-             self._module_name = self._module_name_spec

+         if self._package_name_spec:

+             self._repo_name = self._package_name_spec

              return

  

-         raise rpkgError('Could not find current module name.'

-                         ' Use --module-name.')

+         raise rpkgError('Could not find current repository name.'

+                         ' Use --name and optional --namespace.')

  

      @property

      def ns(self):

-         """This property provides the namespace of the module"""

+         """This property provides the namespace of the repository"""

  

          if not self._ns:

              self.load_ns()
@@ -633,10 +651,16 @@ 

  

      @property

      def ns_module_name(self):

+         self.log.warning('Property ns_module_name is deprecated. Please use '

+                          'ns_repo_name instead.')

+         return self.ns_repo_name

+ 

+     @property

+     def ns_repo_name(self):

          if self.distgit_namespaced:

-             return '%s/%s' % (self.ns, self.module_name)

+             return '%s/%s' % (self.ns, self.repo_name)

          else:

-             return self.module_name

+             return self.repo_name

  

      @property

      def nvr(self):
@@ -649,7 +673,7 @@ 

      def load_nvr(self):

          """This sets the nvr attribute"""

  

-         self._nvr = '%s-%s-%s' % (self.module_name, self.ver, self.rel)

+         self._nvr = '%s-%s-%s' % (self.repo_name, self.ver, self.rel)

  

      @property

      def rel(self):
@@ -659,7 +683,7 @@ 

          return(self._rel)

  

      def load_nameverrel(self):

-         """Set the release of a package module."""

+         """Set the release of a package."""

  

          cmd = ['rpm']

          cmd.extend(self.rpmdefines)
@@ -681,7 +705,7 @@ 

                  self.log.debug(joined_cmd)

                  self.log.error(err)

              raise rpkgError('Could not query n-v-r of %s: %s'

-                             % (self.module_name, e))

+                             % (self.repo_name, e))

          if err:

              self.log.debug('Errors occoured while running following command to get N-V-R-E:')

              self.log.debug(joined_cmd)
@@ -697,7 +721,7 @@ 

          if len(parts) != 4:

              raise rpkgError('Could not get n-v-r-e from %r'

                              % first_line_output)

-         (self._module_name_spec,

+         (self._package_name_spec,

           self._epoch,

           self._ver,

           self._rel) = parts
@@ -762,7 +786,7 @@ 

  

      @property

      def spec(self):

-         """This property ensures the module attribute"""

+         """This property ensures the spec attribute"""

  

          if not self._spec:

              self.load_spec()
@@ -884,7 +908,7 @@ 

  

      @property

      def mock_results_dir(self):

-         return os.path.join(self.path, "results_%s" % self.module_name,

+         return os.path.join(self.path, "results_%s" % self.repo_name,

                              self.ver, self.rel)

  

      @property
@@ -1120,44 +1144,73 @@ 

  

          return (name, files, uploadfiles)

  

-     def _get_namespace_giturl(self, module):

-         """Get the namespaced git url, if DistGit namespaces enabled

+     def _get_namespace_giturl(self, repo_name):

+         """Get the namespaced git url, if DistGit namespace enabled

  

-         :param str module: a module name

-         :return: giturl with proper namespace.

+         :param str repo_name: a repository name with or without namespace.

+         :return: giturl with proper namespace. If namespace is not in the

+             repository name and dist-git namespace is enabled, default

+             namespace rpms will set.

          :rtype: str

          """

  

          if self.distgit_namespaced:

-             if '/' in module:

-                 giturl = self.gitbaseurl % \

-                     {'user': self.user, 'module': module}

+             if '/' in repo_name:

+                 giturl = self.gitbaseurl % {

+                     'user': self.user,

+                     'repo': repo_name,

+                     # This is for the compatible with old format

+                     'module': repo_name,

+                 }

              else:

                  # Default to rpms namespace for backwards compat

-                 giturl = self.gitbaseurl % \

-                     {'user': self.user, 'module': "rpms/%s" % module}

+                 rpms_repo_name = 'rpms/%s' % repo_name

+                 giturl = self.gitbaseurl % {

+                     'user': self.user,

+                     'repo': rpms_repo_name,

+                     # This is for the compatible with old format

+                     'module': rpms_repo_name,

+                 }

          else:

-             giturl = self.gitbaseurl % \

-                 {'user': self.user, 'module': module}

+             giturl = self.gitbaseurl % {

+                 'user': self.user,

+                 'repo': repo_name,

+                 # This is for the compatible with old format

+                 'module': repo_name

+             }

  

          return giturl

  

-     def _get_namespace_anongiturl(self, module):

-         """Get the namespaced git url, if DistGit namespaces enabled

+     def _get_namespace_anongiturl(self, repo_name):

+         """Get the namespaced git url, if DistGit namespace enabled

  

-         :param str module: a module name

-         :return: anonymous giturl with a proper namespace.

+         :param str repo_name: a repository name with or without namespace.

+         :return: anonymous giturl with a proper namespace. If repository name

+             does not have namespace and dist-git namespace is enabled, default

+             namespace rpms will be set.

          :rtype: str

          """

  

          if self.distgit_namespaced:

-             if '/' in module:

-                 giturl = self.anongiturl % {'module': module}

+             if '/' in repo_name:

+                 giturl = self.anongiturl % {

+                     'repo': repo_name,

+                     # This is for the compatible with old format

+                     'module': repo_name,

+                 }

              else:

+                 rpms_repo_name = 'rpms/%s' % repo_name

                  # Default to rpms namespace for backwards compat

-                 giturl = self.anongiturl % {'module': "rpms/%s" % module}

+                 giturl = self.anongiturl % {

+                     'repo': rpms_repo_name,

+                     # This is for the compatible with old format

+                     'module': rpms_repo_name,

+                 }

          else:

-             giturl = self.anongiturl % {'module': module}

+             giturl = self.anongiturl % {

+                 'repo': repo_name,

+                 'module': repo_name,

+             }

  

          return giturl

  
@@ -1186,7 +1239,7 @@ 

          self.log.info('Tag \'%s\' was created', tagname)

  

      def clean(self, dry=False, useignore=True):

-         """Clean a module checkout of untracked files.

+         """Clean a repository checkout of untracked files.

  

          :param bool dry: Can optionally perform a dry-run. Defaults to `False`.

          :param bool useignore: Can optionally not use the ignore rules.
@@ -1205,11 +1258,11 @@ 

          self._run_command(cmd, cwd=self.path)

          return

  

-     def clone(self, module, path=None, branch=None, bare_dir=None,

+     def clone(self, repo, path=None, branch=None, bare_dir=None,

                anon=False, target=None):

          """Clone a repo, optionally check out a specific branch.

  

-         :param str module: the name of the module to clone

+         :param str repo: the name of the repository to clone.

          :param str path: an optional basedir to perform the clone in.

          :param str branch: an optional name of a branch to checkout instead of

              `<remote>/master`.
@@ -1227,9 +1280,9 @@ 

              self._branch_remote = None

          # construct the git url

          if anon:

-             giturl = self._get_namespace_anongiturl(module)

+             giturl = self._get_namespace_anongiturl(repo)

          else:

-             giturl = self._get_namespace_giturl(module)

+             giturl = self._get_namespace_giturl(repo)

  

          # Create the command

          cmd = ['git', 'clone']
@@ -1261,26 +1314,28 @@ 

  

          self._run_command(cmd, cwd=path)

  

-         base_module = self.get_base_module(module)

-         git_dir = target if target else bare_dir if bare_dir else base_module

+         base_repo = self.get_base_repo(repo)

+         git_dir = target if target else bare_dir if bare_dir else base_repo

          conf_git = git.Git(os.path.join(path, git_dir))

-         self._clone_config(conf_git, module)

+         self._clone_config(conf_git, repo)

  

          return

  

-     def get_base_module(self, module):

-         # Handle namespaced modules

+     def get_base_repo(self, repo):

+         # Handle namespaced repositories

          # Example:

-         #   module: docker/cockpit

+         #   repository: docker/cockpit

+         #       The path will just be os.path.join(path, "cockpit")

+         #   repository: rpms/nodejs

          #       The path will just be os.path.join(path, "cockpit")

-         if "/" in module:

-             return module.split("/")[-1]

-         return module

+         if "/" in repo:

+             return repo.split("/")[-1]

+         return repo

  

-     def clone_with_dirs(self, module, anon=False, target=None):

+     def clone_with_dirs(self, repo, anon=False, target=None):

          """Clone a repo old style with subdirs for each branch.

  

-         :param str module: name of the module to clone

+         :param str repo: name of the repository to clone.

          :param bool anon: whether or not to clone anonymously. Defaults to

              `False`.

          :param str target: an optional name of the folder in which to clone the
@@ -1291,26 +1346,26 @@ 

          self._branch_remote = None

          # Get the full path of, and git object for, our directory of branches

          top_path = os.path.join(self.path,

-                                 target or self.get_base_module(module))

+                                 target or self.get_base_repo(repo))

          top_git = git.Git(top_path)

          repo_path = os.path.join(top_path, 'rpkg.git')

  

          # construct the git url

          if anon:

-             giturl = self._get_namespace_anongiturl(module)

+             giturl = self._get_namespace_anongiturl(repo)

          else:

-             giturl = self._get_namespace_giturl(module)

+             giturl = self._get_namespace_giturl(repo)

  

          # Create our new top directory

          try:

              os.mkdir(top_path)

          except OSError as e:

-             raise rpkgError('Could not create directory for module %s: %s'

-                             % (module, e))

+             raise rpkgError('Could not create directory for repository %s: %s'

+                             % (repo, e))

  

          # Create a bare clone first. This gives us a good list of branches

          try:

-             self.clone(module, top_path, bare_dir=repo_path, anon=anon)

+             self.clone(repo, top_path, bare_dir=repo_path, anon=anon)

          except Exception as e:

              # Clean out our directory

              shutil.rmtree(top_path)
@@ -1342,21 +1397,33 @@ 

          # We don't need this now. Ignore errors since keeping it does no harm

          shutil.rmtree(repo_path, ignore_errors=True)

  

-     def _clone_config(self, conf_git, module):

+     def _clone_config(self, conf_git, repo):

          # Inject ourselves as the git credential helper for https pushing

          conf_git.config('credential.helper', ' '.join(find_me() + ['gitcred']))

          conf_git.config('credential.useHttpPath', 'true')

          # Set any other clone_config

          if self.clone_config:

-             base_module = self.get_base_module(module)

+             base_repo = self.get_base_repo(repo)

              clone_config = self.clone_config.strip() % {

-                 'base_module': base_module, 'module': module}

+                 # base_repo will be just the repository name with namespace

+                 # stripped.

+                 'repo': base_repo,

+                 # repo is the repo argument which is passed in the CLI, which

+                 # could or cound't have namespace. So, if namespace is

+                 # included, it is just the ns_repo, otherwise it is same as the

+                 # base_repo.

+                 'ns_repo': repo,

+ 

+                 # This is for compatible with old format

+                 'base_module': base_repo,

+                 'module': repo

+             }

              for confline in clone_config.splitlines():

                  if confline:

                      conf_git.config(*confline.split())

  

      def commit(self, message=None, file=None, files=[], signoff=False):

-         """Commit changes to a module (optionally found at path)

+         """Commit changes to a repository (optionally found at path)

  

          Requires the caller be a real tty or a message passed.

  
@@ -1414,7 +1481,7 @@ 

              module base directory.

          """

  

-         # Things work better if we're in our module directory

+         # Things work better if we're in our repository directory

          oldpath = os.getcwd()

          os.chdir(self.path)

          # build up the command
@@ -1430,11 +1497,11 @@ 

          os.chdir(oldpath)

          return

  

-     def get_latest_commit(self, module, branch):

-         """Discover the latest commit has for a given module and return it"""

+     def get_latest_commit(self, repo, branch):

+         """Discover the latest commit has for a given repository and return it"""

  

          # This is stupid that I have to use subprocess :/

-         url = self._get_namespace_anongiturl(module)

+         url = self._get_namespace_anongiturl(repo)

          # This cmd below only works to scratch build rawhide

          # We need something better for epel

          cmd = ['git', 'ls-remote', url, 'refs/heads/%s' % branch]
@@ -1448,11 +1515,11 @@ 

              raise rpkgError(e)

          if error:

              raise rpkgError('Got an error finding %s head for %s: %s'

-                             % (branch, module, error))

+                             % (branch, repo, error))

          # Return the hash sum

          if not output:

              raise rpkgError('Could not find remote branch %s for %s'

-                             % (branch, module))

+                             % (branch, repo))

          return output.split()[0]

  

      def gitbuildhash(self, build):
@@ -1531,7 +1598,7 @@ 

                  except ValueError:

                      pass

  

-         # Things work better if we're in our module directory

+         # Things work better if we're in our repository directory

          oldpath = os.getcwd()

          os.chdir(self.path)

  
@@ -1607,7 +1674,7 @@ 

          """

  

          # Create the outfile name based on arguments

-         outfile = '%s-%s-%s.patch' % (self.module_name, self.ver, suffix)

+         outfile = '%s-%s-%s.patch' % (self.repo_name, self.ver, suffix)

  

          # If we want to rediff, the patch file has to already exist

          if rediff and not os.path.exists(os.path.join(self.path, outfile)):
@@ -1616,12 +1683,12 @@ 

  

          # See if there is a source dir to diff in

          if not os.path.isdir(os.path.join(self.path,

-                                           '%s-%s' % (self.module_name,

+                                           '%s-%s' % (self.repo_name,

                                                       self.ver))):

              raise rpkgError('Expanded source dir not found!')

  

          # Setup the command

-         cmd = ['gendiff', '%s-%s' % (self.module_name, self.ver),

+         cmd = ['gendiff', '%s-%s' % (self.repo_name, self.ver),

                 '.%s' % suffix]

  

          # Try to run the command and capture the output
@@ -1737,7 +1804,7 @@ 

              self.log.info("sources file doesn't exist. Source files download skipped.")

              return

  

-         # Default to putting the files where the module is

+         # Default to putting the files where the repository is

          if not outdir:

              outdir = self.path

  
@@ -1756,7 +1823,7 @@ 

          for entry in sourcesf.entries:

              outfile = os.path.join(outdir, entry.file)

              self.lookasidecache.download(

-                 self.ns_module_name if self.lookaside_namespaced else self.module_name,

+                 self.ns_repo_name if self.lookaside_namespaced else self.repo_name,

                  entry.file, entry.hash, outfile,

                  hashtype=entry.hashtype, **args)

  
@@ -1859,13 +1926,13 @@ 

              raise rpkgError('Packages in destination tag %(dest_tag_name)s are not inherited by'

                              ' build tag %(build_tag_name)s' % build_target)

  

-     def construct_build_url(self, module_name=None, commit_hash=None):

+     def construct_build_url(self, repo_name=None, commit_hash=None):

          """Construct build URL with namespaced anongiturl and commit hash

  

-         :param str module_name: name of the module part of the build URL. If

-             omitted, module name with namespace will be guessed from current

-             repository. The given module name will be used in URL directly

-             without guessing namespace.

+         :param str repo_name: name of the repository part of the build URL. If

+             omitted, namespaced name will be guessed from current repository.

+             The given repository name will be used in URL directly without

+             guessing namespace.

          :param str commit_hash: the commit hash appended to build URL. It

              omitted, the latest commit hash got from current repository will be

              used.
@@ -1873,12 +1940,12 @@ 

          :rtype: str

          """

          return '{0}#{1}'.format(

-             self._get_namespace_anongiturl(module_name or self.ns_module_name),

+             self._get_namespace_anongiturl(repo_name or self.ns_repo_name),

              commit_hash or self.commithash)

  

      def build(self, skip_tag=False, scratch=False, background=False,

                url=None, chain=None, arches=None, sets=False, nvr_check=True):

-         """Initiate a build of the module.  Available options are:

+         """Initiate a build.  Available options are:

  

          skip_tag: Skip the tag action after the build

  
@@ -1978,7 +2045,7 @@ 

                                    ' care that I am not able to construct NVR.'

                                    '  I will refer this build by package name'

                                    ' in following messages.')

-                     build_reference = self.module_name

+                     build_reference = self.repo_name

  

          # see if this build has been done.  Does not check builds within

          # a chain
@@ -2057,7 +2124,7 @@ 

              clog.writelines(clog_lines)

  

      def compile(self, arch=None, short=False, builddir=None, nocheck=False):

-         """Run rpmbuild -bc on a module

+         """Run rpmbuild -bc

  

          optionally for a specific arch, or short-circuit it, or

          define an alternate builddir
@@ -2110,7 +2177,7 @@ 

          self.kojisession.uploadWrapper(file, path, callback=callback)

  

      def install(self, arch=None, short=False, builddir=None, nocheck=False):

-         """Run rpm -bi on a module

+         """Run rpm -bi

  

          optionally for a specific arch, short-circuit it, or

          define an alternative builddir
@@ -2146,7 +2213,7 @@ 

          """

  

          # Check for srpm

-         srpm = "%s-%s-%s.src.rpm" % (self.module_name, self.ver, self.rel)

+         srpm = "%s-%s-%s.src.rpm" % (self.repo_name, self.ver, self.rel)

          if not os.path.exists(os.path.join(self.path, srpm)):

              log.warning('No srpm found')

  
@@ -2161,7 +2228,7 @@ 

          if not rpms:

              log.warning('No rpm found')

          cmd = ['rpmlint']

-         default_rpmlintconf = '{0}.rpmlintrc'.format(self.module_name)

+         default_rpmlintconf = '{0}.rpmlintrc'.format(self.repo_name)

          if info:

              cmd.extend(['-i'])

          if rpmlintconf:
@@ -2436,7 +2503,7 @@ 

  

              gitignore.add('/%s' % file_basename)

              self.lookasidecache.upload(

-                 self.ns_module_name if self.lookaside_namespaced else self.module_name,

+                 self.ns_repo_name if self.lookaside_namespaced else self.repo_name,

                  f, file_hash)

  

          sourcesf.write()
@@ -2445,7 +2512,7 @@ 

          self.repo.index.add(['sources', '.gitignore'])

  

      def prep(self, arch=None, builddir=None):

-         """Run `rpmbuild -bp` on a module

+         """Run `rpmbuild -bp`

  

          :param str arch: optional to run prep section for a specific arch. By

              default, local system arch will be used.
@@ -2467,7 +2534,7 @@ 

          self._run_command(cmd, shell=True)

  

      def srpm(self, hashtype=None):

-         """Create an srpm using hashtype from content in the module

+         """Create an srpm using hashtype from content

  

          Requires sources already downloaded. The generated SRPM file will be

          put into package repository directory.
@@ -2478,7 +2545,7 @@ 

  

          self.srpmname = os.path.join(self.path,

                                       "%s-%s-%s.src.rpm"

-                                      % (self.module_name, self.ver, self.rel))

+                                      % (self.repo_name, self.ver, self.rel))

  

          # See if we need to build the srpm

          if os.path.exists(self.srpmname):
@@ -2525,7 +2592,7 @@ 

          else:

              spec = data

          # Replace %{name} with the package name

-         spec = spec.replace("%{name}", self.module_name)

+         spec = spec.replace("%{name}", self.repo_name)

          # Replace %{version} with the package version

          spec = spec.replace("%{version}", self.ver)

  
@@ -2558,7 +2625,7 @@ 

          return [line_num, offset - offset_inc + 1]

  

      def verify_files(self, builddir=None):

-         """Run `rpmbuild -bl` on a module to verify the `%files` section

+         """Run `rpmbuild -bl` to verify the `%files` section

  

          :param str builddir: optionally define an alternate builddir.

          """
@@ -2848,7 +2915,7 @@ 

              # If the scm_url isn't specified, get the remote git URL of the

              # git repo the current user is in

              actual_scm_url = self._get_namespace_anongiturl(

-                 self.ns_module_name)

+                 self.ns_repo_name)

              actual_scm_url = '{0}?#{1}'.format(actual_scm_url, self.commithash)

          return actual_scm_url, actual_branch

  

file modified
+93 -20
@@ -42,6 +42,15 @@ 

      return value

  

  

+ def warning_deprecated_module_name(value):

+     """Warning deprecated of option --module-name"""

+     print('Deprecation warning: --module-name is deprecated and will be '

+           'removed in future version. Use combination of --name and '

+           '--namespace instead.',

+           file=sys.stderr)

+     return value

+ 

+ 

  # Adds `allow_abbrev' feature

  class _ArgumentParser(argparse.ArgumentParser):

      def __init__(self, *args, **kwargs):
@@ -237,6 +246,16 @@ 

              raise rpkgError('Missing kojiconfig and kojiprofile to load Koji '

                              'session. One of them must be specified.')

  

+         if '%(module)s' in items['gitbaseurl']:

+             self.log.warning(

+                 'Format argument module is deprecated in gitbaseurl. '

+                 'Please use "repo" instead.')

+ 

+         if '%(module)s' in items['anongiturl']:

+             self.log.warning(

+                 'Format argument module is deprecated in anongiturl. '

+                 'Please use "repo" instead.')

+ 

          # Create the cmd object

          self._cmd = self.site.Commands(self.args.path,

                                         items['lookaside'],
@@ -258,21 +277,40 @@ 

                                         )

  

          if self.args.module_name:

-             # Module name was specified via argument

+             # Repository name was specified via argument

              if '/' not in self.args.module_name:

-                 self._cmd.module_name = self.args.module_name

+                 self._cmd.repo_name = self.args.module_name

                  if dg_namespaced:

                      # No slash, assume rpms namespace

                      self._cmd.ns = 'rpms'

              else:

-                 self._cmd.ns, self._cmd.module_name = self.args.module_name.rsplit('/', 1)

+                 self._cmd.ns, self._cmd.repo_name = self.args.module_name.rsplit('/', 1)

+ 

+         if self.args.repo_name:

+             self._cmd.module_name = self.args.repo_name

+             if dg_namespaced and self.args.repo_namespace:

+                 self._cmd.ns = self.args.repo_namespace

+             else:

+                 self._cmd.ns = 'rpms'

+ 

          self._cmd.password = self.args.password

          self._cmd.runas = self.args.runas

          self._cmd.debug = self.args.debug

          self._cmd.verbose = self.args.v

-         self._cmd.clone_config = items.get('clone_config')

          self._cmd.lookaside_request_params = items.get('lookaside_request_params')

  

+         clone_config = items.get('clone_config')

+         self._cmd.clone_config = clone_config

+         if clone_config:

+             if '%(base_module)s' in clone_config:

+                 self.log.warning(

+                     'Format argument base_module is deprecated in clone '

+                     'config. Please use "repo" instead.')

+             if '%(module)s' in clone_config:

+                 self.log.warning(

+                     'Format argument module is deprecated in clone config. '

+                     'Please use "ns_repo" instead.')

+ 

      # This function loads the extra stuff once we figure out what site

      # we are

      def do_imports(self, site=None):
@@ -325,13 +363,28 @@ 

                                  'for a while for backward-compatibility. It will be disabled'

                                  ' in future version.')

          # Allow forcing the package name

-         self.parser.add_argument('--module-name',

-                                  help=('Override the module name. Otherwise'

-                                        ' it is discovered from: Git push URL'

-                                        ' or Git URL (last part of path with'

-                                        ' .git extension removed) or from name'

-                                        ' macro in spec file. In that order.')

-                                  )

+         self.parser.add_argument(

+             '--module-name',

+             type=warning_deprecated_module_name,

+             help='Deprecated. Use combination of --name and --namespace. '

+                  'Override the module name. Otherwise it is discovered from: '

+                  'Git push URL or Git URL (last part of path with .git '

+                  'extension removed) or from name macro in spec file. In that '

+                  'order.')

+         self.parser.add_argument(

+             '--name',

+             metavar='NAME',

+             dest='repo_name',

+             help='Override repository name. Use --namespace option to change '

+                  'namespace. If not specified, name is discovered from Git '

+                  'push URL or Git URL (last part of path with .git extension '

+                  'removed) or from Name macro in spec file, in that order.')

+         self.parser.add_argument(

+             '--namespace',

+             metavar='NAMESPACE',

+             dest='repo_namespace',

+             help='The package repository namespace. If omitted, default to '

+                  'rpms if namespace is enabled.')

          # Override the  discovered user name

          self.parser.add_argument('--user', default=None,

                                   help='Override the discovered user name')
@@ -538,11 +591,11 @@ 

          """Register the clone target and co alias"""

  

          clone_parser = self.subparsers.add_parser(

-             'clone', help='Clone and checkout a module',

-             description='This command will clone the named module from the '

-                         'configured repository base URL. By default it will '

-                         'also checkout the master branch for your working '

-                         'copy.')

+             'clone', help='Clone and checkout a repository',

+             description='This command will clone the named repository from '

+                         'the configured repository base URL. By default it '

+                         'will also checkout the master branch for your '

+                         'working copy.')

          # Allow an old style clone with subdirs for branches

          clone_parser.add_argument(

              '--branches', '-B', action='store_true',
@@ -556,11 +609,11 @@ 

              help='Check out a module anonymously')

          # store the module to be cloned

          clone_parser.add_argument(

-             'module', nargs=1, help='Name of the module to clone')

+             'repo', nargs=1, help='Name of the repository to clone')

          # Eventually specify where to clone the module

          clone_parser.add_argument(

              "clone_target", default=None, nargs="?",

-             help='Directory in which to clone the module')

+             help='Directory in which to clone the repository')

          clone_parser.set_defaults(command=self.clone)

  

          # Add an alias for historical reasons
@@ -1480,11 +1533,11 @@ 

  

      def clone(self):

          if self.args.branches:

-             self.cmd.clone_with_dirs(self.args.module[0],

+             self.cmd.clone_with_dirs(self.args.repo[0],

                                       anon=self.args.anonymous,

                                       target=self.args.clone_target)

          else:

-             self.cmd.clone(self.args.module[0],

+             self.cmd.clone(self.args.repo[0],

                             branch=self.args.branch,

                             anon=self.args.anonymous,

                             target=self.args.clone_target)
@@ -2074,6 +2127,26 @@ 

          # Parse the args

          self.args = self.parser.parse_args()

  

+         if self.args.module_name and self.args.repo_name:

+             self.parser.error(

+                 'argument --name: not allowed with --module-name')

+ 

+         if self.args.module_name and self.args.repo_namespace:

+             self.parser.error(

+                 'argument --namespace: not allowed with --module-name')

+ 

+         if not self.args.repo_name and self.args.repo_namespace:

+             self.parser.error(

+                 'missing --name: using --namespace requires --name option')

+ 

+         if self.args.repo_namespace:

+             if self.config.has_option(self.name, 'distgit_namespaces'):

+                 namespaces = self.config.get(

+                     self.name, 'distgit_namespaces').split()

+                 if self.args.repo_namespace not in namespaces:

+                     self.parser.error('namespace {0} is not valid'.format(

+                         self.args.repo_namespace))

+ 

          if self.args.user:

              self.user = self.args.user

          else:

@@ -0,0 +1,13 @@ 

+ [rpkg]

+ lookaside = http://localhost/repo/pkgs

+ lookasidehash = md5

+ lookaside_cgi = https://localhost/repo/pkgs/upload.cgi

+ gitbaseurl = ssh://%(user)s@localhost/%(module)s

+ anongiturl = git://localhost/%(module)s

+ branchre = f\d$|f\d\d$|el\d$|olpc\d$|master$

+ kojiprofile = koji

+ build_client = koji

+ clone_config =

+   bz.default-component %(module)s

+ distgit_namespaced = True

+ distgit_namespaces = rpms modules containers

file modified
+121 -36
@@ -81,9 +81,22 @@ 

              self.run_cmd(cmd, cwd=repo_path,

                           stdout=subprocess.PIPE, stderr=subprocess.PIPE)

  

+     def get_absolute_conf_filename(self, filename):

+         """Find and return absolute conf file from fixtures"""

+         abs_filename = os.path.join(

+             os.path.dirname(__file__), 'fixtures', filename)

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

+             raise RuntimeError(

+                 'Fixture conf file {0} does not exist.'.format(abs_filename))

+         return abs_filename

+ 

  

  class TestModuleNameOption(CliTestCase):

  

+     def setUp(self):

+         super(TestModuleNameOption, self).setUp()

+         self.conf_file = self.get_absolute_conf_filename('rpkg-ns.conf')

+ 

      def get_cmd(self, module_name, cfg=None):

          cmd = ['rpkg', '--path', self.cloned_repo_path, '--module-name', module_name, 'verrel']

          with patch('sys.argv', new=cmd):
@@ -92,36 +105,28 @@ 

  

      def test_non_namespaced(self):

          cmd = self.get_cmd('foo')

-         self.assertEqual(cmd._module_name, 'foo')

-         self.assertEqual(cmd.ns_module_name, 'foo')

+         self.assertEqual(cmd._repo_name, 'foo')

+         self.assertEqual(cmd.ns_repo_name, 'foo')

  

      def test_just_module_name(self):

-         cmd = self.get_cmd(

-             'foo',

-             os.path.join(os.path.dirname(__file__), 'fixtures', 'rpkg-ns.conf'))

-         self.assertEqual(cmd._module_name, 'foo')

-         self.assertEqual(cmd.ns_module_name, 'rpms/foo')

+         cmd = self.get_cmd('foo', self.conf_file)

+         self.assertEqual(cmd._repo_name, 'foo')

+         self.assertEqual(cmd.ns_repo_name, 'rpms/foo')

  

      def test_explicit_default(self):

-         cmd = self.get_cmd(

-             'rpms/foo',

-             os.path.join(os.path.dirname(__file__), 'fixtures', 'rpkg-ns.conf'))

-         self.assertEqual(cmd._module_name, 'foo')

-         self.assertEqual(cmd.ns_module_name, 'rpms/foo')

+         cmd = self.get_cmd('rpms/foo', self.conf_file)

+         self.assertEqual(cmd._repo_name, 'foo')

+         self.assertEqual(cmd.ns_repo_name, 'rpms/foo')

  

      def test_with_namespace(self):

-         cmd = self.get_cmd(

-             'container/foo',

-             os.path.join(os.path.dirname(__file__), 'fixtures', 'rpkg-ns.conf'))

-         self.assertEqual(cmd._module_name, 'foo')

-         self.assertEqual(cmd.ns_module_name, 'container/foo')

+         cmd = self.get_cmd('container/foo', self.conf_file)

+         self.assertEqual(cmd._repo_name, 'foo')

+         self.assertEqual(cmd.ns_repo_name, 'container/foo')

  

      def test_with_nested_namespace(self):

-         cmd = self.get_cmd(

-             'user/project/foo',

-             os.path.join(os.path.dirname(__file__), 'fixtures', 'rpkg-ns.conf'))

-         self.assertEqual(cmd._module_name, 'foo')

-         self.assertEqual(cmd.ns_module_name, 'user/project/foo')

+         cmd = self.get_cmd('user/project/foo', self.conf_file)

+         self.assertEqual(cmd._repo_name, 'foo')

+         self.assertEqual(cmd.ns_repo_name, 'user/project/foo')

  

  

  class TestKojiConfigBackwardCompatibility(CliTestCase):
@@ -246,7 +251,7 @@ 

          This test can be delete after kojiconfig is removed eventually.

          """

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

-                    '--module-name', 'mycontainer',

+                    '--name', 'mycontainer',

                     'container-build']

  

          cfg_file = os.path.join(os.path.dirname(__file__),
@@ -864,7 +869,7 @@ 

      def test_lint(self, _run_command):

          self.checkout_branch(git.Repo(self.cloned_repo_path), 'eng-rhel-7')

  

-         cli_cmd = ['rpkg', '--module-name', 'docpkg', '--path', self.cloned_repo_path, 'lint']

+         cli_cmd = ['rpkg', '--name', 'docpkg', '--path', self.cloned_repo_path, 'lint']

  

          with patch('sys.argv', new=cli_cmd):

              cli = self.new_cli()
@@ -877,7 +882,7 @@ 

      def test_lint_warning_with_info(self, _run_command):

          self.checkout_branch(git.Repo(self.cloned_repo_path), 'eng-rhel-7')

  

-         cli_cmd = ['rpkg', '--module-name', 'docpkg', '--path', self.cloned_repo_path,

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

                     'lint', '--info']

  

          with patch('sys.argv', new=cli_cmd):
@@ -894,7 +899,7 @@ 

          lint_config_path = os.path.join(self.cloned_repo_path, 'docpkg.rpmlintrc')

          open(lint_config_path, 'a').close()

  

-         cli_cmd = ['rpkg', '--module-name', 'docpkg', '--path', self.cloned_repo_path, 'lint']

+         cli_cmd = ['rpkg', '--name', 'docpkg', '--path', self.cloned_repo_path, 'lint']

  

          with patch('sys.argv', new=cli_cmd):

              cli = self.new_cli()
@@ -912,7 +917,7 @@ 

          deprecated_lint_config_path = os.path.join(self.cloned_repo_path, '.rpmlint')

          open(deprecated_lint_config_path, 'a').close()

  

-         cli_cmd = ['rpkg', '--module-name', 'docpkg', '--path', self.cloned_repo_path, 'lint']

+         cli_cmd = ['rpkg', '--name', 'docpkg', '--path', self.cloned_repo_path, 'lint']

  

          with patch('sys.argv', new=cli_cmd):

              cli = self.new_cli()
@@ -930,7 +935,7 @@ 

          custom_lint_config_path = os.path.join(self.cloned_repo_path, 'custom.rpmlint')

          open(custom_lint_config_path, 'a').close()

  

-         cli_cmd = ['rpkg', '--module-name', 'docpkg', '--path', self.cloned_repo_path,

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

                     'lint', '--rpmlintconf', 'custom.rpmlint']

  

          with patch('sys.argv', new=cli_cmd):
@@ -1254,7 +1259,7 @@ 

          super(TestImportSrpm, self).tearDown()

  

      def assert_import_srpm(self, target_repo):

-         cli_cmd = ['rpkg', '--path', target_repo, '--module-name', 'docpkg',

+         cli_cmd = ['rpkg', '--path', target_repo, '--name', 'docpkg',

                     'import', '--skip-diffs', self.srpm_file]

  

          with patch('sys.argv', new=cli_cmd):
@@ -1646,10 +1651,10 @@ 

          self.Popen_patcher = patch('subprocess.Popen')

          self.mock_Popen = self.Popen_patcher.start()

  

-         self.module_name_patcher = patch('pyrpkg.Commands.module_name',

-                                          new_callable=PropertyMock,

-                                          return_value='docpkg')

-         self.mock_module_name = self.module_name_patcher.start()

+         self.repo_name_patcher = patch('pyrpkg.Commands.repo_name',

+                                        new_callable=PropertyMock,

+                                        return_value='docpkg')

+         self.mock_repo_name = self.repo_name_patcher.start()

  

          self.ver_patcher = patch('pyrpkg.Commands.ver',

                                   new_callable=PropertyMock,
@@ -1658,7 +1663,7 @@ 

  

      def tearDown(self):

          self.ver_patcher.stop()

-         self.module_name_patcher.stop()

+         self.repo_name_patcher.stop()

          self.Popen_patcher.stop()

          self.repo_patcher.stop()

          super(TestPatch, self).tearDown()
@@ -2113,12 +2118,12 @@ 

      Name:       python-dns

      NVR:        None

      State:      FAILED

-     Koji Task:  

+     Koji Task:

  

      Name:       python-cryptography

      NVR:        None

      State:      FAILED

-     Koji Task:  

+     Koji Task:

  """  # noqa

          self.maxDiff = None

          self.assertEqual(self.sort_lines(expected_output),
@@ -2513,3 +2518,83 @@ 

  

              six.assertRaisesRegex(self, rpkgError, r'mbs-manager is missing.+',

                                    cli.module_build_local)

+ 

+ 

+ class TestOptionNameAndNamespace(CliTestCase):

+     """Test CLI option --name and --namespace"""

+ 

+     @patch('sys.stderr', new_callable=six.StringIO)

+     def test_name_exclusive_with_module_name(self, stderr):

+         cli = [

+             'rpkg', '--module-name', 'somepkg', '--name', 'somepkg',

+             'request-repo'

+         ]

+         with patch('sys.argv', new=cli):

+             with self.assertRaises(SystemExit):

+                 self.new_cli()

+ 

+     @patch('sys.stderr', new_callable=six.StringIO)

+     def test_namespace_exclusive_with_module_name(self, stderr):

+         cli = [

+             'rpkg', '--module-name', 'somepkg', '--namespace', 'modules',

+             'request-repo'

+         ]

+         with patch('sys.argv', new=cli):

+             with self.assertRaises(SystemExit):

+                 self.new_cli()

+ 

+     @patch('sys.stderr', new_callable=six.StringIO)

+     def test_namespace_not_in_configured_distgit_namespaces(self, stderr):

+         conf_file = self.get_absolute_conf_filename(

+             'rpkg-has-distgit-namespaces.conf')

+         cli = [

+             'rpkg', '--path', self.cloned_repo_path,

+             '--name', 'somepkg', '--namespace', 'somenamespace',

+             'srpm'

+         ]

+         with patch('sys.argv', new=cli):

+             with self.assertRaises(SystemExit):

+                 self.new_cli(conf_file)

+ 

+     def test_namespace_in_configured_distgit_namespaces(self):

+         conf_file = self.get_absolute_conf_filename(

+             'rpkg-has-distgit-namespaces.conf')

+         cli = [

+             'rpkg', '--path', self.cloned_repo_path,

+             '--name', 'somepkg', '--namespace', 'modules',

+             'srpm'

+         ]

+         with patch('sys.argv', new=cli):

+             cli = self.new_cli(conf_file)

+             self.assertEqual('modules', cli.args.repo_namespace)

+ 

+     def test_only_name(self):

+         cli = [

+             'rpkg', '--path', self.cloned_repo_path,

+             '--name', 'somepkg', 'srpm'

+         ]

+         with patch('sys.argv', new=cli):

+             cli = self.new_cli()

+             self.assertEqual('somepkg', cli.cmd.module_name)

+             self.assertEqual('rpms', cli.cmd.ns)

+ 

+     def test_name_and_namespace_with_distgit_namespace_disabled(self):

+         cli = [

+             'rpkg', '--path', self.cloned_repo_path,

+             '--name', 'somepkg', '--namespace', 'modules', 'srpm'

+         ]

+         with patch('sys.argv', new=cli):

+             cli = self.new_cli()

+             self.assertEqual('somepkg', cli.cmd.module_name)

+             self.assertEqual('rpms', cli.cmd.ns)

+ 

+     def test_name_and_namespace_with_distgit_namespace_enabled(self):

+         cli = [

+             'rpkg', '--path', self.cloned_repo_path,

+             '--name', 'somepkg', '--namespace', 'modules', 'srpm'

+         ]

+         with patch('sys.argv', new=cli):

+             abs_filename = self.get_absolute_conf_filename('rpkg-ns.conf')

+             cli = self.new_cli(abs_filename)

+             self.assertEqual('somepkg', cli.cmd.module_name)

+             self.assertEqual('modules', cli.cmd.ns)

file modified
+4 -4
@@ -84,7 +84,7 @@ 

      def test_load_from_spec(self):

          """Ensure name, version, release can be loaded from a valid SPEC"""

          self.cmd.load_nameverrel()

-         self.assertEqual('docpkg', self.cmd._module_name_spec)

+         self.assertEqual('docpkg', self.cmd._package_name_spec)

          self.assertEqual('0', self.cmd._epoch)

          self.assertEqual('1.2', self.cmd._ver)

          self.assertEqual('2.el6', self.cmd._rel)
@@ -116,7 +116,7 @@ 

          cmd = self.make_commands(path=cloned_repo_dir)

  

          cmd.load_nameverrel()

-         self.assertEqual('docpkg', cmd._module_name_spec)

+         self.assertEqual('docpkg', cmd._package_name_spec)

          self.assertEqual('0', cmd._epoch)

          self.assertEqual('1.2', cmd._ver)

          self.assertEqual('2.el6', cmd._rel)
@@ -142,7 +142,7 @@ 

                          content=utils.spec_file_echo_text)

  

          self.cmd.load_nameverrel()

-         self.assertEqual('docpkg', self.cmd._module_name_spec)

+         self.assertEqual('docpkg', self.cmd._package_name_spec)

          self.assertEqual('0', self.cmd._epoch)

          self.assertEqual('1.2', self.cmd._ver)

          self.assertEqual('2.el6', self.cmd._rel)
@@ -695,7 +695,7 @@ 

  class TestConstructBuildURL(CommandTestCase):

      """Test Commands.construct_build_url"""

  

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

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

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

      def test_construct_url(self, commithash, ns_module_name):

          commithash.return_value = '12345'

--module-name can still be used but deprecation message is printed. As a result, a use case of --module-name, e.g. fedpkg --module-name modules/somemodule request-repo, could be replaced with fedpkg --name somemodule --namespace modules request-repo.

In addition, if repo_namespaces is configured, for example in fedpkg.conf, value of --namespace will be validated.

Fixes #301

1 new commit added

  • Massive replacement of module
3 years ago

Pretty please pagure-ci rebuild

Typo: omtted -> omitted

This seems inaccurate. It's used not only for cloning, but in other operations as well (like verrel). Can it reuse the original help from --module-name? Something like Override repository name. Use --namespace option to change namespace. If not specified, name is discovered from Git push URL or Git URL (last part of path with .git extension removed) or from Name macro in spec file, in that order.

Can the error be changed to missing --name: using --namespace requires --name option

Mostly looks good to me. I have a concern about the deprecation warnings in module_name property. Those will be printed to users, but they don't really have any way to fix that. I'm sure some people will be confused by that, and there will be bugs opened about it.

It's inaccurate indeed. I like your version, it's clear enough. Thanks :)

Mostly looks good to me. I have a concern about the deprecation warnings in module_name property. Those will be printed to users, but they don't really have any way to fix that. I'm sure some people will be confused by that, and there will be bugs opened about it.

Deprecation of --module-name and property Commands.module_name will be mentioned in released notes, which is for developers. Printing deprecation warnings would be a last way to notify the deprecation. How about log the message in property module_name in debug level? Or we can just keep them and user is able to run with -q option.

1 new commit added

  • Fix typo and reword option help and deprecation message
3 years ago

These two conditions will crash if clone config is not set. Running fedpkg tests against this PR runs into it.

Rhpkg tests are not passing against this PR, but that looks like some mock going wrong in the test suite itself.

Yeah, rhpkg needs to update in order to work with this patch. There is a PR for fedpkg for that as well.

Mostly looks good to me. The last issue is the inline comment in cli.py about clone config. As is, this change will require all downstreams to define some clone configuration. Can the condition be updated to work even when clone_config is None?

1 new commit added

  • Check old format args only if there is clone config
3 years ago

Pretty please pagure-ci rebuild

@lsedlar Done. I actually fixed it but forgot to push :)

Looks good to me :thumbsup:

rebased onto 33e1b61

3 years ago

Rebased on master. Conflicts are resolved.

Pretty please pagure-ci rebuild

Pull-Request has been merged by cqi

3 years ago