#191 Fix rpm command to get changelog from SPEC
Merged 5 years ago by cqi. Opened 5 years ago by cqi.
cqi/rpkg fix-bug-1412224  into  master

file modified
+5
@@ -1,6 +1,11 @@ 

  ChangeLog

  =========

  

+ NEXT

+ ----

+ 

+ - Fix rpm command to get changelog from SPEC - rhbz#1412224 (cqi)

+ 

  v1.49 (2017-02-22)

  ------------------

  

file modified
+4 -2
@@ -1981,8 +1981,10 @@ 

          """Write the latest spec changelog entry to a clog file"""

  

          spec_file = os.path.join(self.path, self.spec)

-         cmd = ['rpm', '-q', '--qf', '%{CHANGELOGTEXT}\n', '--specfile', spec_file]

-         proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)

+         cmd = ['rpm'] + self.rpmdefines + ['-q', '--qf', '"%{CHANGELOGTEXT}\n"',

+                                            '--specfile', '"%s"' % spec_file]

+         proc = subprocess.Popen(' '.join(cmd), shell=True,

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

          stdout, stderr = proc.communicate()

          if proc.returncode > 0:

              raise rpkgError(stderr.strip())

file modified
+14 -4
@@ -72,6 +72,7 @@ 

  

      def setUp(self):

          super(TestClog, self).setUp()

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

  

      def cli_clog(self):

          """Run clog command"""
@@ -86,8 +87,7 @@ 

  

          clog_file = os.path.join(self.cloned_repo_path, 'clog')

          self.assertTrue(os.path.exists(clog_file))

-         with open(clog_file, 'r') as f:

-             clog = f.read().strip()

+         clog = self.read_file(clog_file).strip()

          self.assertEqual('Initial version', clog)

  

      def test_raw_clog(self):
@@ -98,15 +98,25 @@ 

  

          clog_file = os.path.join(self.cloned_repo_path, 'clog')

          self.assertTrue(os.path.exists(clog_file))

-         with open(clog_file, 'r') as f:

-             clog = f.read().strip()

+         clog = self.read_file(clog_file).strip()

          self.assertEqual('- Initial version', clog)

  

+     def test_reference_source_files_in_spec_should_not_break_clog(self):

+         """SPEC containing Source0 or Patch0 should not break clog

+ 

+         This case is reported in bug 1412224

+         """

+         spec_file = os.path.join(self.cloned_repo_path, self.spec_file)

+         spec = self.read_file(spec_file)

+         self.write_file(spec_file, spec.replace('#Source0:', 'Source0: extrafile.txt'))

+         self.test_raw_clog()

+ 

  

  class TestCommit(CliTestCase):

  

      def setUp(self):

          super(TestCommit, self).setUp()

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

          self.make_changes()

  

      def get_last_commit_message(self):

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

      def setUp(self):

          super(ClogTest, self).setUp()

  

-         with open(os.path.join(self.repo_path, self.spec_file), 'w') as specfile:

+         with open(os.path.join(self.cloned_repo_path, self.spec_file), 'w') as specfile:

              specfile.write('''

  Summary: package demo

  Name: pkgtool
@@ -325,12 +325,14 @@ 

  - initial

  ''')

  

-         self.cmd = self.make_commands(self.repo_path)

+         self.clog_file = os.path.join(self.cloned_repo_path, 'clog')

+         self.cmd = self.make_commands()

+         self.checkout_branch(self.cmd.repo, 'eng-rhel-6')

  

      def test_clog(self):

          self.cmd.clog()

  

-         with open(os.path.join(self.repo_path, 'clog'), 'r') as clog:

+         with open(self.clog_file, 'r') as clog:

              clog_lines = clog.readlines()

  

          expected_lines = ['add %changelog section\n',
@@ -340,7 +342,7 @@ 

      def test_raw_clog(self):

          self.cmd.clog(raw=True)

  

-         with open(os.path.join(self.repo_path, 'clog'), 'r') as clog:

+         with open(self.clog_file, 'r') as clog:

              clog_lines = clog.readlines()

  

          expected_lines = ['- add %changelog section\n',

file modified
+2
@@ -27,6 +27,8 @@ 

  Version: 1.2

  Release: 2%{dist}

  License: GPL

+ #Source0:

+ #Patch0:

  Group: Applications/Productivity

  BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)

  %description

Commands.rpmdefines was not added to rpm command to get changelog from
SPEC. This causes SPEC cannot be parsed correctly if it contains SourceN
or PatchN. Like other methods in Commands class that runs rpm,
Commands.rpmdefines should be always added to the rpm command.

Resolves: rhbz#1412224

Signed-off-by: Chenxiong Qi cqi@redhat.com

rebased

5 years ago

This will break if If the path to spec file contains a space. Maybe splitting rpmdefines on first space to create a list like ['--define', '_sourcedir …', …] would be safer?

rebased

5 years ago

This will break if If the path to spec file contains a space. Maybe splitting rpmdefines on first space to create a list like ['--define', '_sourcedir …', …] would be safer?

Hi @lsedlar, fixed this in the same way of load_nameverrel. Thanks for catching this bug.

Hmm, it still does not work with a path with space. For example in the systemd example from original bug report (after renaming parent directory from systemd to break me) I'm getting malformed %include statement. This is failing at rpm level and I can't find a combination of quotes that would fix that. I say let's not worry about it and get the fix out the way it is now.

Okay. I have no idea what %include does. I run the raw rpm by quoting all paths, like --define '_sourcedir "/home/cqi/packages/my fedora packages/systemd"', it still breaks the parse.

It could probably be caused by rpm itself. From line 462, rpm does not handle space within path.

Yeah, could be. The %include statement should just take another file and put it into the spec file. A use case I know of is to move long changelog to a separate file.

The fix is good from my point of view :thumbsup:

this needs to be rebased. It is causing breakage for at least fedora-release package

rebased

5 years ago

rebased

5 years ago

There's a typo here: referernce -> reference. Not a big deal :)

rebased

5 years ago

Rebased and fixed the typo.

rebased

5 years ago

Pull-Request has been merged by cqi

5 years ago

Bug mentioned in my comment is fixed already in rpm 9d38b2f.