#700 *pkg import: check specfile matches the repo name
Merged 2 years ago by onosek. Opened 2 years ago by onosek.
onosek/rpkg check_specfile  into  master

file modified
+17 -3
@@ -46,8 +46,8 @@ 

  

  from pyrpkg import layout

  from pyrpkg.errors import (AlreadyUploadedError, HashtypeMixingError,

-                            NoSourcesError, UnknownTargetError, rpkgAuthError,

-                            rpkgError)

+                            NoSourcesError, SpecfileDoesntMatchRepoNameError,

+                            UnknownTargetError, rpkgAuthError, rpkgError)

  from pyrpkg.lookaside import CGILookasideCache

  from pyrpkg.sources import SourcesFile

  from pyrpkg.spec import SpecFile
@@ -2001,13 +2001,15 @@ 

              raise rpkgError('Could not find hash of build %s' % build)

          return (hash)

  

-     def import_srpm(self, srpm):

+     def import_srpm(self, srpm, check_specfile_matches_repo_name=True):

          """Import the contents of an srpm into a repo.

  

          This function will add/remove content to match the srpm,

          upload new files to the lookaside, and stage the changes.

  

          :param str srpm: file to import contents from.

+         :param bool check_specfile_matches_repo_name: check specfile

+             in SRPM matches the repository name

          :return: a list of files to upload.

          :rtype: list

          """
@@ -2059,6 +2061,18 @@ 

                  self.repo.index.remove([file])

                  os.remove(file)

  

+         specfile_matches_repo_name = False

+         for file in files:

+             if self.repo_name + '.spec' == file:

The check looks reasonable to me but you forgot to strip the namespace from self.repo_name and therefore this check fails when trying to import packages to Copr DistGit.

Please see https://github.com/fedora-copr/copr/issues/3365

Temporarily I am disabling the check in our code but I think it would be nice to eventually have it.
https://github.com/fedora-copr/copr/issues/3365

Can you please add support for namespaced repositories here? :-)

Should I create a new rpkg issue for this?

+                 specfile_matches_repo_name = True

+         if check_specfile_matches_repo_name and not specfile_matches_repo_name:

+             raise SpecfileDoesntMatchRepoNameError(

+                 "Error: Specfile '{0}' doesn't match repo name '{1}'. This check "

+                 "can be omitted by adding argument '--do-not-check-specfile-name'".format(

+                     ", ".join(file for file in files if file.endswith('.spec')),

+                     self.repo_name)

+             )

+ 

          try:

              self.log.debug("Extracting srpm '{0}'".format(srpm))

              output, err = extract_srpm(srpm)

file modified
+9 -1
@@ -991,6 +991,12 @@ 

          import_srpm_parser.add_argument(

              '--offline', help='Do not upload files into lookaside cache',

              action='store_true')

+         import_srpm_parser.add_argument(

+             '--do-not-check-specfile-name',

+             action='store_true',

+             default=False,

+             help="Do not check whether specfile in SRPM matches "

+                  "the repository name")

          import_srpm_parser.add_argument('srpm', help='Source rpm to import')

          import_srpm_parser.set_defaults(command=self.import_srpm)

  
@@ -2385,7 +2391,9 @@ 

          print(self.cmd.giturl())

  

      def import_srpm(self):

-         uploadfiles = self.cmd.import_srpm(self.args.srpm)

+         uploadfiles = self.cmd.import_srpm(

+             self.args.srpm,

+             check_specfile_matches_repo_name=not self.args.do_not_check_specfile_name)

          if uploadfiles:

              try:

                  self.cmd.upload(uploadfiles, replace=True, offline=self.args.offline)

file modified
+7
@@ -78,3 +78,10 @@ 

  class NoSourcesError(rpkgError):

      """Raised when there are no sources"""

      pass

+ 

+ 

+ class SpecfileDoesntMatchRepoNameError(rpkgError):

+     """Raised when srpm doesn't contain specfile that matches

+     with repository name

+     """

+     pass

file modified
+3 -3
@@ -128,7 +128,7 @@ 

  

      def raise_upload_error(self, http_status):

          messages = {

-             http_client.UNAUTHORIZED: 'Request is unauthorized.',

+             http_client.UNAUTHORIZED: 'Dist-git request is unauthorized.',

              http_client.INTERNAL_SERVER_ERROR: 'Error occurs inside the server.',

          }

          default = 'Fail to upload files. Server returns status {0}'.format(http_status)
@@ -194,7 +194,7 @@ 

          if status != 200:

              self.log.info('Remove downloaded invalid file %s', outfile)

              os.remove(outfile)

-             raise DownloadError('Server returned status code %d' % status)

+             raise DownloadError('Dist-git server returned status code %d' % status)

  

          os.utime(outfile, (tstamp, tstamp))

  
@@ -406,7 +406,7 @@ 

                      try:

                          return function(*args, **kwargs)

                      except wait_on as e:

-                         self.log.warn("Network error: %s" % (e))

+                         self.log.warning("Network error: %s" % (e))

                          attempts_left -= 1

                          self.log.debug("Attempt %d/%d has failed."

                                         % (attempts_all - attempts_left, attempts_all))

file modified
+1 -1
@@ -1840,7 +1840,7 @@ 

                  cli.import_srpm()  # no exception should be raised

  

      def test_import_srpm_processed_by_rpmautospec(self):

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

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

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

  

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

Specfile in SRPM is check whether has the same name as the dist-git
repository name. It should prevent having multiple similar
repositories accidentally created.
This behavior can be ommited by adding argument after import
command: '--do-not-check-specfile-name'.

JIRA: RHELCMP-12669
Fixes: https://pagure.io/fedpkg/issue/529
Relates: https://pagure.io/releng/issue/7523

Signed-off-by: Ondřej Nosek onosek@redhat.com

I assume this will end execution of fedpkg, therefore it should rather be "Error" IMHO.

rebased onto 13623db

2 years ago
  • OK, changed to Error.
  • During testing, I realized, that the specfile check was performed after extracting files from SRPM. I moved it before the extract operation.
  • Added some minor changes like clarifying error messages.

Pull-Request has been merged by onosek

2 years ago

The check looks reasonable to me but you forgot to strip the namespace from self.repo_name and therefore this check fails when trying to import packages to Copr DistGit.

Please see https://github.com/fedora-copr/copr/issues/3365

Temporarily I am disabling the check in our code but I think it would be nice to eventually have it.
https://github.com/fedora-copr/copr/issues/3365

Can you please add support for namespaced repositories here? :-)

Should I create a new rpkg issue for this?