#628 Refuse import of packages processed by rpmautospec
Merged a year ago by onosek. Opened 2 years ago by zbyszek.
zbyszek/rpkg rpmautospec-reject  into  master

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

  from pyrpkg.spec import SpecFile

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

                            is_file_tracked, is_lookaside_eligible_file,

-                           log_result)

+                           spec_file_processed_by_rpmautospec, log_result)

  

  from .gitignore import GitIgnore

  
@@ -1445,10 +1445,16 @@ 

                      # the test of eligibility

                      self.log.warning("SRPM file contains a directory: '{0}'. "

                                       "Skipping it.".format(file))

+ 

                  if is_lookaside_eligible_file(file, target_dir):

                      uploadfiles.append(file)

                  else:

                      files.append(file)

+ 

+                 if (file == self.spec and

+                     spec_file_processed_by_rpmautospec(file, target_dir)):

+                     raise rpkgError('srpm was processed by rpmautospec')

+ 

          finally:

              shutil.rmtree(target_dir)

  

file modified
+20
@@ -320,3 +320,23 @@ 

      # output contains encoding ("binary", "us-ascii", ...)

      encoding = output.strip()  # strip newline at the end

      return encoding == "binary"

+ 

+ def spec_file_processed_by_rpmautospec(file_name, dir_path=None):

+     file_path = os.path.join(dir_path or "", file_name)

+ 

+     try:

+         contents = open(file_path).readlines()

+     except Exception:

+         # if we can't read it, let's assume the answer is "no".

+         return False

+ 

+     # Check for the %autorelease header prepended to the file

+     if any('START: Set by rpmautospec' in line for line in contents[:10]):

+         return True

+ 

+     # It seems that currently there's no mechanism to detect

+     # %autochangelog processing. But most packages would use both

+     # %autochangelog and %autorelease together, so we should catch

+     # most cases by checking for %autorelease only.

+     # https://pagure.io/fedora-infra/rpmautospec/issue/269

+     return False

People occasionally try to do 'fedpkg/rpm import' on an srpm which was
built from a .spec file with %autorelease and %autochangelog fields.
rpmautospec works by replacing those macros with generated content.
Importing and srpm after such processing is generally the wrong thing
to do, because for %autorelease and %autochangelog to work, the spec
file with (unexpanded) macros must be stored in dist-git. But even if
somebody wanted to stop using rpmautospec, and wanted to import the
processed spec file, the headers that were inserted by rpmautospec
should be removed. Thus, just refuse such imports.

Note that I also opened https://pagure.io/fedora-infra/rpmautospec/issue/269
to provide an "official" way to detect that the file was processed.
But even the current imperfect check is better than nothing because
it seems to be a fairly common mistake because 'fedpkg import' is
a very common workflow, and %autorelease/%autochangelog are becoming
the norm too.

Test:
$ PYTHONPATH=$HOME/python/rpkg fedpkg import --offline package-notes-0.5-3.fc38.src.rpm
Could not execute import_srpm: srpm was processed by rpmautospec

Signed-off-by: Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl

pretty please pagure-ci rebuild

2 years ago

Hi. I looked at your code.
fedpkg import allows running in a repository without any specfile (in case of a new repository). In this situation self.spec fails (IncompleteLayout is selected).
What about checking every specfile found in the srpm instead? I can make this change, just confirm whether it is OK for you.

Please do! I'm not familiar with the code, so it's hard for me to see the "big picture" and all the corner cases.

Commit 3087dd7 fixes this pull-request

Pull-Request has been merged by onosek

a year ago
Metadata