[gentoo-portage-dev] [PATCH 1/2] repoman: update cat/pkg info in header all the time

[gentoo-portage-dev] [PATCH 1/2] repoman: update cat/pkg info in header all the time

[Mike Frysinger]

There are edge cases where repoman's changelog code is not as good as
the existing echangelog.  Mostly related to out of date headers.  Have
the code check the header in more cases not just for missing lines, but
also outdated values all the time.

While we're at it, write some tests!

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 pym/portage/tests/repoman/test_echangelog.py |  101 ++++++++++++++++++++++++++
 pym/repoman/utilities.py                     |   57 ++++++++-------
 2 files changed, 133 insertions(+), 25 deletions(-)
 create mode 100644 pym/portage/tests/repoman/test_echangelog.py

diff --git a/pym/portage/tests/repoman/test_echangelog.py b/pym/portage/tests/repoman/test_echangelog.py
new file mode 100644
index 0000000..71d6d5e
--- /dev/null
+++ b/pym/portage/tests/repoman/test_echangelog.py
@@ -0,0 +1,101 @@
+# Copyright 2012 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+import datetime
+import subprocess
+import sys
+import tempfile
+import time
+
+import portage
+from portage import os
+from portage import shutil
+from portage.tests import TestCase
+from repoman.utilities import UpdateChangeLog
+
+class RepomanEchangelogTestCase(TestCase):
+
+	def setUp(self):
+		super(RepomanEchangelogTestCase, self).setUp()
+
+		self.tmpdir = tempfile.mkdtemp(prefix='repoman.echangelog.')
+
+		self.skel_changelog = os.path.join(self.tmpdir, 'skel.ChangeLog')
+		skel = [
+			'# ChangeLog for <CATEGORY>/<PACKAGE_NAME>\n',
+			'# Copyright 1999-2000 Gentoo Foundation; Distributed under the GPL v2\n',
+			'# $Header: $\n'
+		]
+		self._writelines(self.skel_changelog, skel)
+
+		self.cat = 'mycat'
+		self.pkg = 'mypkg'
+		self.pkgdir = os.path.join(self.tmpdir, self.cat, self.pkg)
+		os.makedirs(self.pkgdir)
+
+		self.header_pkg = '# ChangeLog for %s/%s\n' % (self.cat, self.pkg)
+		self.header_copyright = '# Copyright 1999-%s Gentoo Foundation; Distributed under the GPL v2\n' % \
+			datetime.datetime.now().year
+		self.header_cvs = '# $Header: $\n'
+
+		self.changelog = os.path.join(self.pkgdir, 'ChangeLog')
+
+		self.user = 'Testing User <portage@gentoo.org>'
+
+	def tearDown(self):
+		super(RepomanEchangelogTestCase, self).tearDown()
+		shutil.rmtree(self.tmpdir)
+
+	def _readlines(self, file):
+		with open(file, 'r') as f:
+			return f.readlines()
+
+	def _writelines(self, file, data):
+		with open(file, 'w') as f:
+			f.writelines(data)
+
+	def testRejectRootUser(self):
+		self.assertEqual(UpdateChangeLog(self.pkgdir, 'me <root@gentoo.org>', '', '', '', '', quiet=True), None)
+
+	def testMissingSkelFile(self):
+		# Test missing ChangeLog, but with empty skel (i.e. do nothing).
+		UpdateChangeLog(self.pkgdir, self.user, 'test!', '/does/not/exist', self.cat, self.pkg, quiet=True)
+		actual_cl = self._readlines(self.changelog)
+		self.assertGreater(len(actual_cl[0]), 0)
+
+	def testEmptyChangeLog(self):
+		# Make sure we do the right thing with a 0-byte ChangeLog
+		open(self.changelog, 'w').close()
+		UpdateChangeLog(self.pkgdir, self.user, 'test!', self.skel_changelog, self.cat, self.pkg, quiet=True)
+		actual_cl = self._readlines(self.changelog)
+		self.assertEqual(actual_cl[0], self.header_pkg)
+		self.assertEqual(actual_cl[1], self.header_copyright)
+		self.assertEqual(actual_cl[2], self.header_cvs)
+
+	def testCopyrightUpdate(self):
+		# Make sure updating the copyright line works
+		UpdateChangeLog(self.pkgdir, self.user, 'test!', self.skel_changelog, self.cat, self.pkg, quiet=True)
+		actual_cl = self._readlines(self.changelog)
+		self.assertEqual(actual_cl[1], self.header_copyright)
+
+	def testSkelHeader(self):
+		# Test skel.ChangeLog -> ChangeLog
+		UpdateChangeLog(self.pkgdir, self.user, 'test!', self.skel_changelog, self.cat, self.pkg, quiet=True)
+		actual_cl = self._readlines(self.changelog)
+		self.assertEqual(actual_cl[0], self.header_pkg)
+
+	def testExistingGoodHeader(self):
+		# Test existing ChangeLog (correct values)
+		self._writelines(self.changelog, [self.header_pkg])
+
+		UpdateChangeLog(self.pkgdir, self.user, 'test!', self.skel_changelog, self.cat, self.pkg, quiet=True)
+		actual_cl = self._readlines(self.changelog)
+		self.assertEqual(actual_cl[0], self.header_pkg)
+
+	def testExistingBadHeader(self):
+		# Test existing ChangeLog (wrong values)
+		self._writelines(self.changelog, ['# ChangeLog for \n'])
+
+		UpdateChangeLog(self.pkgdir, self.user, 'test!', self.skel_changelog, self.cat, self.pkg, quiet=True)
+		actual_cl = self._readlines(self.changelog)
+		self.assertEqual(actual_cl[0], self.header_pkg)
diff --git a/pym/repoman/utilities.py b/pym/repoman/utilities.py
index bee67aa..1e07bad 100644
--- a/pym/repoman/utilities.py
+++ b/pym/repoman/utilities.py
@@ -681,7 +681,7 @@ def get_committer_name(env=None):
 	return user
 
 def UpdateChangeLog(pkgdir, user, msg, skel_path, category, package,
-	new=(), removed=(), changed=(), pretend=False):
+	new=(), removed=(), changed=(), pretend=False, quiet=False):
 	"""
 	Write an entry to an existing ChangeLog, or create a new one.
 	Updates copyright year on changed files, and updates the header of
@@ -689,8 +689,8 @@ def UpdateChangeLog(pkgdir, user, msg, skel_path, category, package,
 	"""
 
 	if '<root@' in user:
-		err = 'Please set ECHANGELOG_USER or run as non-root'
-		logging.critical(err)
+		if not quiet:
+			logging.critical('Please set ECHANGELOG_USER or run as non-root')
 		return None
 
 	# ChangeLog times are in UTC
@@ -711,24 +711,13 @@ def UpdateChangeLog(pkgdir, user, msg, skel_path, category, package,
 	old_header_lines = []
 	header_lines = []
 
+	clold_file = None
 	try:
 		clold_file = io.open(_unicode_encode(cl_path,
 			encoding=_encodings['fs'], errors='strict'),
 			mode='r', encoding=_encodings['repo.content'], errors='replace')
 	except EnvironmentError:
-		clold_file = None
-
-	clskel_file = None
-	if clold_file is None:
-		# we will only need the ChangeLog skeleton if there is no
-		# ChangeLog yet
-		try:
-			clskel_file = io.open(_unicode_encode(skel_path,
-				encoding=_encodings['fs'], errors='strict'),
-				mode='r', encoding=_encodings['repo.content'],
-				errors='replace')
-		except EnvironmentError:
-			pass
+		pass
 
 	f, clnew_path = mkstemp()
 
@@ -736,17 +725,35 @@ def UpdateChangeLog(pkgdir, user, msg, skel_path, category, package,
 	try:
 		if clold_file is not None:
 			# retain header from old ChangeLog
+			first_line = True
 			for line in clold_file:
-				line_strip =  line.strip()
+				line_strip = line.strip()
 				if line_strip and line[:1] != "#":
 					clold_lines.append(line)
 					break
+				# always make sure cat/pkg is up-to-date in case we are
+				# moving packages around, or copied from another pkg, or ...
+				if first_line:
+					if line.startswith('# ChangeLog for'):
+						line = '# ChangeLog for %s/%s\n' % (category, package)
+					first_line = False
 				old_header_lines.append(line)
 				header_lines.append(_update_copyright_year(year, line))
 				if not line_strip:
 					break
 
-		elif clskel_file is not None:
+		clskel_file = None
+		if not header_lines:
+			# delay opening this until we find we need a header
+			try:
+				clskel_file = io.open(_unicode_encode(skel_path,
+					encoding=_encodings['fs'], errors='strict'),
+					mode='r', encoding=_encodings['repo.content'],
+					errors='replace')
+			except EnvironmentError:
+				pass
+
+		if clskel_file is not None:
 			# read skel.ChangeLog up to first empty line
 			for line in clskel_file:
 				line_strip = line.strip()
@@ -806,7 +813,7 @@ def UpdateChangeLog(pkgdir, user, msg, skel_path, category, package,
 			errors='backslashreplace')
 
 		for line in clnew_lines:
-			f.write(line)
+			f.write(_unicode_decode(line))
 
 		# append stuff from old ChangeLog
 		if clold_file is not None:
@@ -837,12 +844,12 @@ def UpdateChangeLog(pkgdir, user, msg, skel_path, category, package,
 			clold_file.close()
 		f.close()
 
-		# show diff (do we want to keep on doing this, or only when
-		# pretend?)
-		for line in difflib.unified_diff(clold_lines, clnew_lines,
-			fromfile=cl_path, tofile=cl_path, n=0):
-			util.writemsg_stdout(line, noiselevel=-1)
-		util.writemsg_stdout("\n", noiselevel=-1)
+		# show diff
+		if not quiet:
+			for line in difflib.unified_diff(clold_lines, clnew_lines,
+					fromfile=cl_path, tofile=cl_path, n=0):
+				util.writemsg_stdout(line, noiselevel=-1)
+			util.writemsg_stdout("\n", noiselevel=-1)
 
 		if pretend:
 			# remove what we've done
-- 
1.7.9.7

[gentoo-portage-dev] [PATCH 2/2] repoman: handle trailing newlines better

[Mike Frysinger]

Automatically strip trailing newlines from the ChangeLog, and be
better about not adding them in the first place (still not perfect,
but getting there).

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 pym/portage/tests/repoman/test_echangelog.py |    9 +++++++++
 pym/repoman/utilities.py                     |   11 ++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/pym/portage/tests/repoman/test_echangelog.py b/pym/portage/tests/repoman/test_echangelog.py
index 71d6d5e..74aa1c5 100644
--- a/pym/portage/tests/repoman/test_echangelog.py
+++ b/pym/portage/tests/repoman/test_echangelog.py
@@ -83,6 +83,7 @@ class RepomanEchangelogTestCase(TestCase):
 		UpdateChangeLog(self.pkgdir, self.user, 'test!', self.skel_changelog, self.cat, self.pkg, quiet=True)
 		actual_cl = self._readlines(self.changelog)
 		self.assertEqual(actual_cl[0], self.header_pkg)
+		self.assertNotEqual(actual_cl[-1], '\n')
 
 	def testExistingGoodHeader(self):
 		# Test existing ChangeLog (correct values)
@@ -99,3 +100,11 @@ class RepomanEchangelogTestCase(TestCase):
 		UpdateChangeLog(self.pkgdir, self.user, 'test!', self.skel_changelog, self.cat, self.pkg, quiet=True)
 		actual_cl = self._readlines(self.changelog)
 		self.assertEqual(actual_cl[0], self.header_pkg)
+
+	def testTrailingNewlines(self):
+		# Make sure trailing newlines get chomped.
+		self._writelines(self.changelog, ['#\n', 'foo\n', '\n', 'bar\n', '\n', '\n'])
+
+		UpdateChangeLog(self.pkgdir, self.user, 'test!', self.skel_changelog, self.cat, self.pkg, quiet=True)
+		actual_cl = self._readlines(self.changelog)
+		self.assertNotEqual(actual_cl[-1], '\n')
diff --git a/pym/repoman/utilities.py b/pym/repoman/utilities.py
index 1e07bad..013858a 100644
--- a/pym/repoman/utilities.py
+++ b/pym/repoman/utilities.py
@@ -807,7 +807,9 @@ def UpdateChangeLog(pkgdir, user, msg, skel_path, category, package,
 		for line in textwrap.wrap(msg, 80, \
 				initial_indent='  ', subsequent_indent='  '):
 			clnew_lines.append(_unicode_decode('%s\n' % line))
-		clnew_lines.append(_unicode_decode('\n'))
+		# Don't append a trailing newline if the file is new.
+		if clold_file is not None:
+			clnew_lines.append(_unicode_decode('\n'))
 
 		f = io.open(f, mode='w', encoding=_encodings['repo.content'],
 			errors='backslashreplace')
@@ -839,9 +841,12 @@ def UpdateChangeLog(pkgdir, user, msg, skel_path, category, package,
 			# in the unified_diff call below.
 			clold_lines = old_header_lines + clold_lines
 
-			for line in clold_file:
-				f.write(line)
+			# Trim any trailing newlines.
+			lines = clold_file.readlines()
 			clold_file.close()
+			while lines and lines[-1] == '\n':
+				del lines[-1]
+			f.writelines(lines)
 		f.close()
 
 		# show diff
-- 
1.7.9.7

Re: [gentoo-portage-dev] [PATCH 1/2] repoman: update cat/pkg info in header all the time

[Zac Medico]

On 06/12/2012 03:13 PM, Mike Frysinger wrote:
> There are edge cases where repoman's changelog code is not as good as
> the existing echangelog.  Mostly related to out of date headers.  Have
> the code check the header in more cases not just for missing lines, but
> also outdated values all the time.
> 
> While we're at it, write some tests!
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  pym/portage/tests/repoman/test_echangelog.py |  101 ++++++++++++++++++++++++++
>  pym/repoman/utilities.py                     |   57 ++++++++-------
>  2 files changed, 133 insertions(+), 25 deletions(-)
>  create mode 100644 pym/portage/tests/repoman/test_echangelog.py

Looks good to me.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH 2/2] repoman: handle trailing newlines better

[Zac Medico]

On 06/12/2012 03:13 PM, Mike Frysinger wrote:
> Automatically strip trailing newlines from the ChangeLog, and be
> better about not adding them in the first place (still not perfect,
> but getting there).
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  pym/portage/tests/repoman/test_echangelog.py |    9 +++++++++
>  pym/repoman/utilities.py                     |   11 ++++++++---
>  2 files changed, 17 insertions(+), 3 deletions(-)

Looks good to me.
-- 
Thanks,
Zac