As part of the move away from MD5 for the source files hashes in the lookaside cache, we have decided to change the format of the 'sources' files, so it includes the hash type, to make it easier to identify which hash was used.
We agreed on using the BSD-style format, as returned on Linux by passing the extra --tag option to commands like md5sum, sha512sumn, etc.
In addition, we decided to forbid mixing hash types in a single sources file, forcing maintainers to use `fedpkg new-sources` the first time they upload an additional source file.
This 4 patches series implements all that, with loads of unit tests, and some code cleanup.
src/pyrpkg/__init__.py | 60 ++++----- src/pyrpkg/sources.py | 123 ++++++++++++------ test/test_sources.py | 334 ++++++++++++++++++++++++++++++++++-------------- 3 files changed, 359 insertions(+), 158 deletions(-)
From: Mathieu Bridon bochecha@daitauha.fr
The API was awkward to use, to test, and hard to extend to support the new file format we are targetting.
The new code is fully tested, and offers a simpler object API which should be easier to work with. --- src/pyrpkg/__init__.py | 45 ++++----- src/pyrpkg/sources.py | 92 +++++++++++------- test/test_sources.py | 255 +++++++++++++++++++++++++++++++------------------ 3 files changed, 234 insertions(+), 158 deletions(-)
diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py index 5c4dbc5..7ecc8b4 100644 --- a/src/pyrpkg/__init__.py +++ b/src/pyrpkg/__init__.py @@ -38,7 +38,7 @@ try: except ImportError: pass
-import pyrpkg.sources +from pyrpkg.sources import SourcesFile
# Define our own error class @@ -1568,16 +1568,20 @@ class Commands(object): # Default to putting the files where the module is if not outdir: outdir = self.path - for (csum, file) in self._read_sources(): + + sourcesf = SourcesFile(self.sources_filename) + + for entry in sourcesf.entries: # See if we already have a valid copy downloaded - outfile = os.path.join(outdir, file) + outfile = os.path.join(outdir, entry.file) if os.path.exists(outfile): - if self._verify_file(outfile, csum, self.lookasidehash): + if self._verify_file(outfile, entry.hash, entry.hashtype): continue - self.log.info("Downloading %s" % (file)) + self.log.info("Downloading %s" % (entry.file)) url = '%s/%s/%s/%s/%s' % (self.lookaside, self.module_name, - file.replace(' ', '%20'), - csum, file.replace(' ', '%20')) + entry.file.replace(' ', '%20'), + entry.hash, + entry.file.replace(' ', '%20')) # These options came from Makefile.common. # Probably need to support wget as well command = ['curl', '-H', 'Pragma:', '-o', outfile, '-R', '-S', @@ -1586,8 +1590,8 @@ class Commands(object): command.append('-s') command.append(url) self._run_command(command) - if not self._verify_file(outfile, csum, self.lookasidehash): - raise rpkgError('%s failed checksum' % file) + if not self._verify_file(outfile, entry.hash, entry.hashtype): + raise rpkgError('%s failed checksum' % entry.file) return
def switch_branch(self, branch, fetch=True): @@ -2226,18 +2230,6 @@ class Commands(object): config_dir) self._cleanup_tmp_dir(config_dir)
- def _read_sources(self): - """Parses 'sources' file""" - with open(self.sources_filename, 'rb') as sources_fp: - reader = pyrpkg.sources.reader(sources_fp) - return [a for a in reader] - - def _write_sources(self, rows): - """Writes 'sources' file""" - with open(self.sources_filename, 'wb') as sources_fp: - writer = pyrpkg.sources.writer(sources_fp) - writer.writerows(rows) - def upload(self, files, replace=False): """Upload source file(s) in the lookaside cache
@@ -2247,11 +2239,7 @@ class Commands(object): oldpath = os.getcwd() os.chdir(self.path)
- # Decide to overwrite or append to sources: - if replace or not os.path.exists(self.sources_filename): - sources = [] - else: - sources = self._read_sources() + sourcesf = SourcesFile(self.sources_filename, replace=replace)
# Will add new sources to .gitignore if they are not already there. gitignore = GitIgnore(os.path.join(self.path, '.gitignore')) @@ -2262,8 +2250,7 @@ class Commands(object): file_hash = self._hash_file(f, self.lookasidehash) self.log.info("Uploading: %s %s" % (file_hash, f)) file_basename = os.path.basename(f) - if (file_hash, file_basename) not in sources: - sources.append((file_hash, file_basename)) + sourcesf.add_entry(self.lookasidehash, file_basename, file_hash)
# Add this file to .gitignore if it's not already there: if not gitignore.match(file_basename): @@ -2278,7 +2265,7 @@ class Commands(object): self._do_curl(file_hash, f) uploaded.append(file_basename)
- self._write_sources(sources) + sourcesf.write()
# Write .gitignore with the new sources if anything changed: gitignore.write() diff --git a/src/pyrpkg/sources.py b/src/pyrpkg/sources.py index c4263ad..e987fa6 100644 --- a/src/pyrpkg/sources.py +++ b/src/pyrpkg/sources.py @@ -1,55 +1,75 @@ """ -Our so-called sources file is simple text-based line-oriented file format. Each -line represents one file and has two fields: file hash and base name of the -file. Field separator is two spaces and Unix end-of-lines. +Our so-called sources file is simple text-based line-oriented file format.
-This sources module implements API similar to csv module from standard library -to read and write data in sources file format. +Each line represents one source file and is in the same format as the output +of commands like `md5sum filename`: + + hash filename + +This module implements a simple API to read these files, parse lines into +entries, and write these entries to the file in the proper format. """
-class Reader(object): - def __init__(self, sourcesfile): - self.sourcesfile = sourcesfile - self._sourcesiter = None +import os +import re
- def __iter__(self): - for entry in self.sourcesfile: - yield _parse_line(entry)
+class MalformedLineError(Exception): + pass
-class Writer(object): - def __init__(self, sourcesfile): + +class SourcesFile(object): + def __init__(self, sourcesfile, replace=False): self.sourcesfile = sourcesfile + self.entries = [] + + if not replace: + if not os.path.exists(sourcesfile): + return + + with open(sourcesfile) as f: + for line in f: + entry = self.parse_line(line) + + if entry and entry not in self.entries: + self.entries.append(entry) + + def parse_line(self, line): + stripped = line.strip() + + if not stripped: + return
- def writerow(self, row): - self.sourcesfile.write("%s\n" % _format_line(row)) + try: + hash, file = stripped.split(' ', 1)
- def writerows(self, rows): - for row in rows: - self.writerow(row) + except ValueError: + raise MalformedLineError(line)
+ return SourceFileEntry('md5', file, hash)
-def reader(sourcesfile): - return Reader(sourcesfile) + def add_entry(self, hashtype, file, hash): + entry = SourceFileEntry(hashtype, file, hash)
+ if entry not in self.entries: + self.entries.append(entry)
-def writer(sourcesfile): - return Writer(sourcesfile) + def write(self): + with open(self.sourcesfile, 'w') as f: + for entry in self.entries: + f.write(str(entry))
-def _parse_line(line): - stripped_line = line.strip() - if not stripped_line: - return [] - entries = stripped_line.split(' ', 1) - if len(entries) != 2: - raise ValueError("Malformed line: %r." % line) - return entries +class SourceFileEntry(object): + def __init__(self, hashtype, file, hash): + self.hashtype = hashtype.lower() + self.hash = hash + self.file = file
+ def __str__(self): + return '%s %s\n' % (self.hash, self.file)
-def _format_line(entry): - if len(entry) != 0 and len(entry) != 2: - raise ValueError("Incorrect number of fields for entry: %r." - % (entry,)) - return " ".join(entry) + def __eq__(self, other): + return ((self.hashtype, self.hash, self.file) == + (other.hashtype, other.hash, other.file)) diff --git a/test/test_sources.py b/test/test_sources.py index 997ab83..2b281e7 100644 --- a/test/test_sources.py +++ b/test/test_sources.py @@ -1,6 +1,9 @@ import os +import random +import shutil +import string import sys -import StringIO +import tempfile import unittest
old_path = list(sys.path) @@ -10,98 +13,164 @@ from pyrpkg import sources sys.path = old_path
-class formatLineTestCase(unittest.TestCase): - def test_wrong_number_of_fields(self): - WRONG_ENTRIES = [ - ('foo'), - ('foo', 'bar', 'foo'), - ] - for entry in WRONG_ENTRIES: - self.assertRaises(ValueError, sources._format_line, entry) - - def test_empty_entry(self): - self.assertEqual('', sources._format_line(())) - - def test_correct_entry(self): - CORRECT_ENTRIES = [ - (['foo', 'bar'], ('foo bar')), - ] - for entry, line in CORRECT_ENTRIES: - self.assertEqual(line, - sources._format_line(entry)) - - -class parseLineTestCase(unittest.TestCase): - def test_wrong_number_of_parts(self): - WRONG_LINES = [ - 'foo\n', - 'foo \n', - 'foo bar\n', - ] - for line in WRONG_LINES: - self.assertRaises(ValueError, sources._parse_line, line) - - def test_empty_line(self): - EMPTY_LINES = [ - '', - '\n', - ' \n', - ] - for line in EMPTY_LINES: - self.assertEqual([], sources._parse_line(line)) - - def test_correct_line(self): - CORRECT_LINES = [ - ('foo bar\n', ['foo', 'bar']), - ('foo bar\n', ['foo', ' bar']) - ] - for line, entry in CORRECT_LINES: - self.assertEqual(entry, sources._parse_line(line)) - - -class ReaderTestCase(unittest.TestCase): - def test_empty_sources(self): - EMPTY_SOURCES = [ - ('', []), - ('\n', [[]]), - (' \n', [[]]), - ('\n\n', [[], []]), - (' \n ', [[], []]), - ] - for buffer, entries in EMPTY_SOURCES: - fp = StringIO.StringIO(buffer) - reader = sources.Reader(fp) - self.assertEqual(entries, [a for a in reader]) - fp.close() - - def test_correct_sources(self): - CORRECT_SOURCES = [ - ('foo bar\n', [['foo', 'bar']]), - ('foo bar\nfooo baaar\n', [['foo', 'bar'], - ['fooo', 'baaar'], - ]), - ] - for buffer, entries in CORRECT_SOURCES: - fp = StringIO.StringIO(buffer) - reader = sources.Reader(fp) - self.assertEqual(entries, [a for a in reader]) - fp.close() - - -class WriterTestCase(unittest.TestCase): - def test_writerows(self): - CORRECT_SOURCES = [ - ([['foo', 'bar']], 'foo bar\n'), - ([['foo', 'bar'], - ['fooo', 'baaar'], - ], 'foo bar\nfooo baaar\n'), - ] - for entries, buffer in CORRECT_SOURCES: - fp = StringIO.StringIO() - writer = sources.Writer(fp) - writer.writerows(entries) - self.assertEqual(fp.getvalue(), buffer) - fp.close() +class SourceFileEntryTestCase(unittest.TestCase): + def test_entry(self): + e = sources.SourceFileEntry('md5', 'afile', 'ahash') + expected = 'ahash afile\n' + self.assertEqual(str(e), expected) + + +class SourcesFileTestCase(unittest.TestCase): + def setUp(self): + self.workdir = tempfile.mkdtemp(prefix='rpkg-tests.') + self.sourcesfile = os.path.join(self.workdir, self._testMethodName) + + def tearDown(self): + shutil.rmtree(self.workdir) + + def test_parse_empty_line(self): + s = sources.SourcesFile(self.sourcesfile) + entry = s.parse_line('') + self.assertIsNone(entry) + + def test_parse_eol_line(self): + s = sources.SourcesFile(self.sourcesfile) + entry = s.parse_line('\n') + self.assertIsNone(entry) + + def test_parse_whitespace_line(self): + s = sources.SourcesFile(self.sourcesfile) + entry = s.parse_line(' \n') + self.assertIsNone(entry) + + def test_parse_entry_line(self): + s = sources.SourcesFile(self.sourcesfile) + + line = 'ahash afile\n' + entry = s.parse_line(line) + + self.assertTrue(isinstance(entry, sources.SourceFileEntry)) + self.assertEqual(entry.hashtype, 'md5') + self.assertEqual(entry.hash, 'ahash') + self.assertEqual(entry.file, 'afile') + self.assertEqual(str(entry), line) + + def test_parse_wrong_lines(self): + s = sources.SourcesFile(self.sourcesfile) + + lines = ['ahash', + 'ahash ', + 'ahash afile', + ] + + for line in lines: + with self.assertRaises(sources.MalformedLineError): + s.parse_line(line) + + def test_open_new_file(self): + s = sources.SourcesFile(self.sourcesfile) + self.assertEqual(len(s.entries), 0) + + def test_open_empty_file(self): + open(self.sourcesfile, 'w').write('') + s = sources.SourcesFile(self.sourcesfile) + self.assertEqual(len(s.entries), 0) + + def test_open_existing_file(self): + lines = ['ahash afile\n', 'anotherhash anotherfile\n'] + + with open(self.sourcesfile, 'w') as f: + for line in lines: + f.write(line) + + s = sources.SourcesFile(self.sourcesfile) + + for i, entry in enumerate(s.entries): + self.assertTrue(isinstance(entry, sources.SourceFileEntry)) + self.assertEqual(str(entry), lines[i]) + + def test_open_existing_file_with_wrong_line(self): + line = 'some garbage here\n' + + with open(self.sourcesfile, 'w') as f: + f.write(line) + + with self.assertRaises(sources.MalformedLineError): + return sources.SourcesFile(self.sourcesfile) + + def test_add_entry(self): + s = sources.SourcesFile(self.sourcesfile) + self.assertEqual(len(s.entries), 0) + + s.add_entry('md5', 'afile', 'ahash') + self.assertEqual(len(s.entries), 1) + self.assertEqual(str(s.entries[-1]), 'ahash afile\n') + + s.add_entry('md5', 'anotherfile', 'anotherhash') + self.assertEqual(len(s.entries), 2) + self.assertEqual(str(s.entries[-1]), 'anotherhash anotherfile\n') + + def test_add_entry_twice(self): + s = sources.SourcesFile(self.sourcesfile) + self.assertEqual(len(s.entries), 0) + + s.add_entry('md5', 'afile', 'ahash') + self.assertEqual(len(s.entries), 1) + self.assertEqual(str(s.entries[-1]), 'ahash afile\n') + + s.add_entry('md5', 'afile', 'ahash') + self.assertEqual(len(s.entries), 1) + + def test_write_new_file(self): + s = sources.SourcesFile(self.sourcesfile) + self.assertEqual(len(s.entries), 0) + + s.add_entry('md5', 'afile', 'ahash') + s.add_entry('md5', 'anotherfile', 'anotherhash') + s.write() + + with open(self.sourcesfile) as f: + lines = f.readlines() + + self.assertEqual(len(lines), 2) + self.assertEqual(lines[0], 'ahash afile\n') + self.assertEqual(lines[1], 'anotherhash anotherfile\n') + + def test_write_adding_a_line(self): + lines = ['ahash afile\n', 'anotherhash anotherfile\n'] + + with open(self.sourcesfile, 'w') as f: + for line in lines: + f.write(line) + + s = sources.SourcesFile(self.sourcesfile) + s.add_entry('md5', 'thirdfile', 'thirdhash') + s.write() + + with open(self.sourcesfile) as f: + lines = f.readlines() + + self.assertEqual(len(lines), 3) + self.assertEqual(lines[0], 'ahash afile\n') + self.assertEqual(lines[1], 'anotherhash anotherfile\n') + self.assertEqual(lines[2], 'thirdhash thirdfile\n') + + def test_write_over(self): + lines = ['ahash afile\n', 'anotherhash anotherfile\n'] + + with open(self.sourcesfile, 'w') as f: + for line in lines: + f.write(line) + + s = sources.SourcesFile(self.sourcesfile, replace=True) + s.add_entry('md5', 'thirdfile', 'thirdhash') + s.write() + + with open(self.sourcesfile) as f: + lines = f.readlines() + + self.assertEqual(len(lines), 1) + self.assertEqual(lines[0], 'thirdhash thirdfile\n')
if __name__ == '__main__':
From: Mathieu Bridon bochecha@daitauha.fr
We are eventually going to move away from md5 for the sources. However, in order to make the migration (and future ones) easier, we want to indicate on each line of the 'sources' file what is the hash function used to compute the hash of the file.
Fortunately, the md5sum/sha512sum/... utilities support two file formats as their inputs and outputs:
* the current format: `ahash afile`
* the BSD-style format, obtained with the `--tag` option: `HASHTYPE (afile) = ahash
This second format is perfect for us, so this commit moves our 'sources' file handling to it.
A couple of notes:
* we preserve compatibility with existing files, so lines in the old format are still read * we now only ever write lines in the new format, which means that when reading an existing file, all currently present lines will be converted to the new format when writing back to the file --- src/pyrpkg/sources.py | 21 ++++++++++-- test/test_sources.py | 92 ++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 99 insertions(+), 14 deletions(-)
diff --git a/src/pyrpkg/sources.py b/src/pyrpkg/sources.py index e987fa6..340c3e1 100644 --- a/src/pyrpkg/sources.py +++ b/src/pyrpkg/sources.py @@ -2,7 +2,12 @@ Our so-called sources file is simple text-based line-oriented file format.
Each line represents one source file and is in the same format as the output -of commands like `md5sum filename`: +of commands like `md5sum --tag filename`: + + hashtype (filename) = hash + +To preserve backwards compatibility, lines can also be in the older format, +which corresponds to the output of commands like `md5sum filename`:
hash filename
@@ -15,6 +20,10 @@ import os import re
+LINE_PATTERN = re.compile( + r'^(?P<hashtype>[^ ]+?) ((?P<file>[^ )]+?)) = (?P<hash>[^ ]+?)$') + + class MalformedLineError(Exception): pass
@@ -41,6 +50,12 @@ class SourcesFile(object): if not stripped: return
+ m = LINE_PATTERN.match(stripped) + if m is not None: + return SourceFileEntry(m.group('hashtype'), m.group('file'), + m.group('hash')) + + # Try falling back on the old format try: hash, file = stripped.split(' ', 1)
@@ -68,7 +83,9 @@ class SourceFileEntry(object): self.file = file
def __str__(self): - return '%s %s\n' % (self.hash, self.file) + return '%s (%s) = %s\n' % (self.hashtype.upper(), self.file, + self.hash) +
def __eq__(self, other): return ((self.hashtype, self.hash, self.file) == diff --git a/test/test_sources.py b/test/test_sources.py index 2b281e7..fb91b3e 100644 --- a/test/test_sources.py +++ b/test/test_sources.py @@ -16,7 +16,7 @@ sys.path = old_path class SourceFileEntryTestCase(unittest.TestCase): def test_entry(self): e = sources.SourceFileEntry('md5', 'afile', 'ahash') - expected = 'ahash afile\n' + expected = 'MD5 (afile) = ahash\n' self.assertEqual(str(e), expected)
@@ -43,10 +43,23 @@ class SourcesFileTestCase(unittest.TestCase): entry = s.parse_line(' \n') self.assertIsNone(entry)
- def test_parse_entry_line(self): + def test_parse_old_style_line(self): s = sources.SourcesFile(self.sourcesfile)
line = 'ahash afile\n' + newline = 'MD5 (afile) = ahash\n' + entry = s.parse_line(line) + + self.assertTrue(isinstance(entry, sources.SourceFileEntry)) + self.assertEqual(entry.hashtype, 'md5') + self.assertEqual(entry.hash, 'ahash') + self.assertEqual(entry.file, 'afile') + self.assertEqual(str(entry), newline) + + def test_parse_entry_line(self): + s = sources.SourcesFile(self.sourcesfile) + + line = 'MD5 (afile) = ahash\n' entry = s.parse_line(line)
self.assertTrue(isinstance(entry, sources.SourceFileEntry)) @@ -61,6 +74,8 @@ class SourcesFileTestCase(unittest.TestCase): lines = ['ahash', 'ahash ', 'ahash afile', + 'SHA512 (afile) = ahash garbage', + 'MD5 SHA512 (afile) = ahash', ]
for line in lines: @@ -76,8 +91,23 @@ class SourcesFileTestCase(unittest.TestCase): s = sources.SourcesFile(self.sourcesfile) self.assertEqual(len(s.entries), 0)
- def test_open_existing_file(self): + def test_open_existing_file_with_old_style_lines(self): lines = ['ahash afile\n', 'anotherhash anotherfile\n'] + newlines = ['MD5 (afile) = ahash\n', + 'MD5 (anotherfile) = anotherhash\n'] + + with open(self.sourcesfile, 'w') as f: + for line in lines: + f.write(line) + + s = sources.SourcesFile(self.sourcesfile) + + for i, entry in enumerate(s.entries): + self.assertTrue(isinstance(entry, sources.SourceFileEntry)) + self.assertEqual(str(entry), newlines[i]) + + def test_open_existing_file(self): + lines = ['MD5 (afile) = ahash\n', 'MD5 (anotherfile) = anotherhash\n']
with open(self.sourcesfile, 'w') as f: for line in lines: @@ -89,6 +119,44 @@ class SourcesFileTestCase(unittest.TestCase): self.assertTrue(isinstance(entry, sources.SourceFileEntry)) self.assertEqual(str(entry), lines[i])
+ def test_open_existing_file_with_mixed_lines(self): + lines = ['ahash afile\n', + 'anotherhash anotherfile\n', + 'MD5 (thirdfile) = thirdhash\n', + ] + expected = [ + 'MD5 (afile) = ahash\n', + 'MD5 (anotherfile) = anotherhash\n', + 'MD5 (thirdfile) = thirdhash\n', + ] + + with open(self.sourcesfile, 'w') as f: + for line in lines: + f.write(line) + + s = sources.SourcesFile(self.sourcesfile) + + for i, entry in enumerate(s.entries): + self.assertTrue(isinstance(entry, sources.SourceFileEntry)) + self.assertEqual(str(entry), expected[i]) + + def test_open_existing_file_with_identical_entries_old_and_new(self): + lines = ['ahash afile\n', + 'MD5 (afile) = ahash\n', + ] + + with open(self.sourcesfile, 'w') as f: + for line in lines: + f.write(line) + + s = sources.SourcesFile(self.sourcesfile) + + self.assertEqual(len(s.entries), 1) + self.assertEqual(s.entries[0].hashtype, 'md5') + self.assertEqual(s.entries[0].file, 'afile') + self.assertEqual(s.entries[0].hash, 'ahash') + self.assertEqual(str(s.entries[0]), lines[-1]) + def test_open_existing_file_with_wrong_line(self): line = 'some garbage here\n'
@@ -104,11 +172,11 @@ class SourcesFileTestCase(unittest.TestCase):
s.add_entry('md5', 'afile', 'ahash') self.assertEqual(len(s.entries), 1) - self.assertEqual(str(s.entries[-1]), 'ahash afile\n') + self.assertEqual(str(s.entries[-1]), 'MD5 (afile) = ahash\n')
s.add_entry('md5', 'anotherfile', 'anotherhash') self.assertEqual(len(s.entries), 2) - self.assertEqual(str(s.entries[-1]), 'anotherhash anotherfile\n') + self.assertEqual(str(s.entries[-1]), 'MD5 (anotherfile) = anotherhash\n')
def test_add_entry_twice(self): s = sources.SourcesFile(self.sourcesfile) @@ -116,7 +184,7 @@ class SourcesFileTestCase(unittest.TestCase):
s.add_entry('md5', 'afile', 'ahash') self.assertEqual(len(s.entries), 1) - self.assertEqual(str(s.entries[-1]), 'ahash afile\n') + self.assertEqual(str(s.entries[-1]), 'MD5 (afile) = ahash\n')
s.add_entry('md5', 'afile', 'ahash') self.assertEqual(len(s.entries), 1) @@ -133,8 +201,8 @@ class SourcesFileTestCase(unittest.TestCase): lines = f.readlines()
self.assertEqual(len(lines), 2) - self.assertEqual(lines[0], 'ahash afile\n') - self.assertEqual(lines[1], 'anotherhash anotherfile\n') + self.assertEqual(lines[0], 'MD5 (afile) = ahash\n') + self.assertEqual(lines[1], 'MD5 (anotherfile) = anotherhash\n')
def test_write_adding_a_line(self): lines = ['ahash afile\n', 'anotherhash anotherfile\n'] @@ -151,9 +219,9 @@ class SourcesFileTestCase(unittest.TestCase): lines = f.readlines()
self.assertEqual(len(lines), 3) - self.assertEqual(lines[0], 'ahash afile\n') - self.assertEqual(lines[1], 'anotherhash anotherfile\n') - self.assertEqual(lines[2], 'thirdhash thirdfile\n') + self.assertEqual(lines[0], 'MD5 (afile) = ahash\n') + self.assertEqual(lines[1], 'MD5 (anotherfile) = anotherhash\n') + self.assertEqual(lines[2], 'MD5 (thirdfile) = thirdhash\n')
def test_write_over(self): lines = ['ahash afile\n', 'anotherhash anotherfile\n'] @@ -170,7 +238,7 @@ class SourcesFileTestCase(unittest.TestCase): lines = f.readlines()
self.assertEqual(len(lines), 1) - self.assertEqual(lines[0], 'thirdhash thirdfile\n') + self.assertEqual(lines[0], 'MD5 (thirdfile) = thirdhash\n')
if __name__ == '__main__':
From: Mathieu Bridon bochecha@daitauha.fr
There theoretically isn't any problem with mixing various hashes, as far as rpkg is concerned.
However, taking for example the following file:
$ cat sources MD5 (pygit2-0.22.0.tar.gz) = 31dc65b3cbe6e39d75a39db86dd7cd8c SHA512 (pygit2-0.22.0.tar.gz) =2079adc3285bf08fc7227b412b9d3dc[...]
When someone tries verifying it with `md5sum -c` they would get:
$ md5sum -c sources pygit2-0.22.0.tar.gz: OK md5sum: WARNING: 1 line is improperly formatted
They'd need to also to check the file with `sha512sum -c`, but then they would get:
$ sha512sum -c sources pygit2-0.22.0.tar.gz: OK sha512sum: WARNING: 1 line is improperly formatted
This is clearly not ideal, and it might cause confusion.
Mixing might happen when a file contains lines with the old hash, then after the migration a packager tries to upload an additional source file.
This commit prevents mixing, by forcing packagers to use `new-sources` and redo the source file entirely with the new hash.
It's a one time annoyance, but in addition to reducing future confusion, it helps moving towards the goal of completely dropping MD5. --- src/pyrpkg/__init__.py | 20 ++++++++++++++++++-- src/pyrpkg/sources.py | 18 ++++++++++++++++-- test/test_sources.py | 11 +++++++++++ 3 files changed, 45 insertions(+), 4 deletions(-)
diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py index 7ecc8b4..57d9b75 100644 --- a/src/pyrpkg/__init__.py +++ b/src/pyrpkg/__init__.py @@ -38,7 +38,7 @@ try: except ImportError: pass
-from pyrpkg.sources import SourcesFile +from pyrpkg.sources import HashtypeMixingError, SourcesFile
# Define our own error class @@ -2250,7 +2250,23 @@ class Commands(object): file_hash = self._hash_file(f, self.lookasidehash) self.log.info("Uploading: %s %s" % (file_hash, f)) file_basename = os.path.basename(f) - sourcesf.add_entry(self.lookasidehash, file_basename, file_hash) + + try: + sourcesf.add_entry(self.lookasidehash, file_basename, + file_hash) + except HashtypeMixingError as e: + msg = '\n'.join([ + 'Can not upload a new source file with a %(newhash)s ' + 'hash, as the "%(sources)s" file contains at least one ' + 'line with a %(existinghash)s hash.', '', + 'Please redo the whole "%(sources)s" file using:', + ' `%(arg0)s new-sources file1 file2 ...`']) % { + 'newhash': e.new_hashtype, + 'existinghash': e.existing_hashtype, + 'sources': self.sources_filename, + 'arg0': sys.argv[0], + } + raise rpkgError(msg)
# Add this file to .gitignore if it's not already there: if not gitignore.match(file_basename): diff --git a/src/pyrpkg/sources.py b/src/pyrpkg/sources.py index 340c3e1..9c3b22e 100644 --- a/src/pyrpkg/sources.py +++ b/src/pyrpkg/sources.py @@ -24,6 +24,14 @@ LINE_PATTERN = re.compile( r'^(?P<hashtype>[^ ]+?) ((?P<file>[^ )]+?)) = (?P<hash>[^ ]+?)$')
+class HashtypeMixingError(Exception): + def __init__(self, existing_hashtype, new_hashtype): + super(HashtypeMixingError, self).__init__() + + self.existing_hashtype = existing_hashtype + self.new_hashtype = new_hashtype + + class MalformedLineError(Exception): pass
@@ -67,8 +75,14 @@ class SourcesFile(object): def add_entry(self, hashtype, file, hash): entry = SourceFileEntry(hashtype, file, hash)
- if entry not in self.entries: - self.entries.append(entry) + for e in self.entries: + if entry.hashtype != e.hashtype: + raise HashtypeMixingError(e.hashtype, entry.hashtype) + + if entry == e: + return + + self.entries.append(entry)
def write(self): with open(self.sourcesfile, 'w') as f: diff --git a/test/test_sources.py b/test/test_sources.py index fb91b3e..ba90427 100644 --- a/test/test_sources.py +++ b/test/test_sources.py @@ -189,6 +189,17 @@ class SourcesFileTestCase(unittest.TestCase): s.add_entry('md5', 'afile', 'ahash') self.assertEqual(len(s.entries), 1)
+ def test_add_entry_mixing_hashtypes(self): + s = sources.SourcesFile(self.sourcesfile) + self.assertEqual(len(s.entries), 0) + + s.add_entry('md5', 'afile', 'ahash') + self.assertEqual(len(s.entries), 1) + self.assertEqual(str(s.entries[-1]), 'MD5 (afile) = ahash\n') + + with self.assertRaises(sources.HashtypeMixingError): + s.add_entry('sha512', 'anotherfile', 'anotherhash') + def test_write_new_file(self): s = sources.SourcesFile(self.sourcesfile) self.assertEqual(len(s.entries), 0)
From: Mathieu Bridon bochecha@daitauha.fr
--- src/pyrpkg/__init__.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py index 57d9b75..a5f02ea 100644 --- a/src/pyrpkg/__init__.py +++ b/src/pyrpkg/__init__.py @@ -1578,10 +1578,9 @@ class Commands(object): if self._verify_file(outfile, entry.hash, entry.hashtype): continue self.log.info("Downloading %s" % (entry.file)) + urled_file = entry.file.replace(' ', '%20') url = '%s/%s/%s/%s/%s' % (self.lookaside, self.module_name, - entry.file.replace(' ', '%20'), - entry.hash, - entry.file.replace(' ', '%20')) + urled_file, entry.hash, urled_file) # These options came from Makefile.common. # Probably need to support wget as well command = ['curl', '-H', 'Pragma:', '-o', outfile, '-R', '-S',
All patches looked ok so I've merged them in upstream and released rpkg-1.31. I've rebuilt rpkg in Fedora too: rpkg-1.31-1.fc23 and rpkg-1.31-1.fc22 (by now).
Thanks!
On Fri, 2015-03-06 at 17:31 +0100, Pavol Babincak wrote:
All patches looked ok so I've merged them in upstream and released rpkg-1.31. I've rebuilt rpkg in Fedora too: rpkg-1.31-1.fc23 and rpkg-1.31-1.fc22 (by now).
This might be problematic.
Say I run this new rpkg on Fedora 22, and upload a new source tarball in the f21 branch.
The sources file will be in the new format.
In Koji, when building the package, inside the mock buildroot, the previous rpkg gets installed, which won't be able to read the sources file in the new format.
You should push it to all Fedora branches at the same time.
On 03/06/2015 06:13 PM, Mathieu Bridon wrote:
On Fri, 2015-03-06 at 17:31 +0100, Pavol Babincak wrote:
All patches looked ok so I've merged them in upstream and released rpkg-1.31. I've rebuilt rpkg in Fedora too: rpkg-1.31-1.fc23 and rpkg-1.31-1.fc22 (by now).
This might be problematic.
Say I run this new rpkg on Fedora 22, and upload a new source tarball in the f21 branch.
The sources file will be in the new format.
In Koji, when building the package, inside the mock buildroot, the previous rpkg gets installed, which won't be able to read the sources file in the new format.
You should push it to all Fedora branches at the same time.
Thanks for pointing this out. I realized this might break building packages on custom buildsystems if they aren't prepared for this. Package for fetching sources in buildroots of a build system needs to know the new (BSD-like) sources format before developers start to commit sources in this format. People might have an alternative implementation for fetching the sources in their buildroots similar to Fedora's fedpkg-minimal. So upgrading to new rpkg might not be a proper solution for them.
We'll need to take different approach then. By default pyrpkg will be backwards compatible and write sources in old format with md5 hash. pyrpkg's clients (e.g. fedpkg) will have a way to switch to new sources file format if their buildsystem infrastructure is prepared for that.
In Fedora this means I need to finish my implementation of new sources format in fedpkg-minimal and only after this package will be in all supported buildroots we can switch fedpkg's default behaviour.
I've untagged rpkg build from rawhide so developers won't rewrite their sources files in the way which Koji buildroots doesn't understand yet.
On Fri, 2015-03-06 at 19:35 +0100, Pavol Babincak wrote:
We'll need to take different approach then. By default pyrpkg will be backwards compatible and write sources in old format with md5 hash. pyrpkg's clients (e.g. fedpkg) will have a way to switch to new sources file format if their buildsystem infrastructure is prepared for that.
In Fedora this means I need to finish my implementation of new sources format in fedpkg-minimal and only after this package will be in all supported buildroots we can switch fedpkg's default behaviour.
I've untagged rpkg build from rawhide so developers won't rewrite their sources files in the way which Koji buildroots doesn't understand yet.
Seems like the best for now.
I'll send new patches that implement what you described above.
From: Mathieu Bridon bochecha@daitauha.fr
Why did I submit this like that in the first place? :x --- src/pyrpkg/sources.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/pyrpkg/sources.py b/src/pyrpkg/sources.py index 9c3b22e..ea49ea6 100644 --- a/src/pyrpkg/sources.py +++ b/src/pyrpkg/sources.py @@ -92,9 +92,9 @@ class SourcesFile(object):
class SourceFileEntry(object): def __init__(self, hashtype, file, hash): - self.hashtype = hashtype.lower() - self.hash = hash - self.file = file + self.hashtype = hashtype.lower() + self.hash = hash + self.file = file
def __str__(self): return '%s (%s) = %s\n' % (self.hashtype.upper(), self.file,
From: Mathieu Bridon bochecha@daitauha.fr
Not all rpkg-based tools will be migrating to the new sources file format at the same time, if ever.
For example, if rpkg was updated only in Rawhide, then a package maintainer who uses Rawhide would commit sources file in the new format, and another maintainer (or Koji!) on an older Fedora version would not be able to read the new file.
So we would need to upgrade rpkg in all versions of Fedora at the same time.
But then all Fedora downstreams would also need to have the new rpkg at the same time, or at least before maintainers start pushing new-format files.
This kind of lock-in can't happen at the rpkg level.
Instead, this new patch:
* makes rpkg able to write files in either format * makes rpkg write files in the old format by default (to preserve compatibility) * allows rpkg-based tools to specify whether they want to use the new format.
This means we can upgrade rpkg more easily everywhere, taking our time. Eventually, Fedora can decide to move to the new format simply by overriding pyrpkg.Commands.source_entry_type to 'bsd' in fedpkg.Commands --- src/pyrpkg/__init__.py | 7 +++++-- src/pyrpkg/sources.py | 20 +++++++++++------- test/test_sources.py | 55 +++++++++++++++++++++++++++++++++----------------- 3 files changed, 54 insertions(+), 28 deletions(-)
diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py index a5f02ea..60c1a6e 100644 --- a/src/pyrpkg/__init__.py +++ b/src/pyrpkg/__init__.py @@ -158,6 +158,8 @@ class Commands(object): self._branch_remote = None # Name of default remote to be used for new clone self.default_branch_remote = 'origin' + # Default sources file output format type + self.source_entry_type = 'old'
# Define properties here # Properties allow us to "lazy load" various attributes, which also means @@ -1569,7 +1571,7 @@ class Commands(object): if not outdir: outdir = self.path
- sourcesf = SourcesFile(self.sources_filename) + sourcesf = SourcesFile(self.sources_filename, self.source_entry_type)
for entry in sourcesf.entries: # See if we already have a valid copy downloaded @@ -2238,7 +2240,8 @@ class Commands(object): oldpath = os.getcwd() os.chdir(self.path)
- sourcesf = SourcesFile(self.sources_filename, replace=replace) + sourcesf = SourcesFile(self.sources_filename, self.source_entry_type, + replace=replace)
# Will add new sources to .gitignore if they are not already there. gitignore = GitIgnore(os.path.join(self.path, '.gitignore')) diff --git a/src/pyrpkg/sources.py b/src/pyrpkg/sources.py index ea49ea6..a3ff1b1 100644 --- a/src/pyrpkg/sources.py +++ b/src/pyrpkg/sources.py @@ -37,8 +37,10 @@ class MalformedLineError(Exception):
class SourcesFile(object): - def __init__(self, sourcesfile, replace=False): + def __init__(self, sourcesfile, entry_type, replace=False): self.sourcesfile = sourcesfile + self.entry_type = {'old': SourceFileEntry, + 'bsd': BSDSourceFileEntry}[entry_type] self.entries = []
if not replace: @@ -60,7 +62,7 @@ class SourcesFile(object):
m = LINE_PATTERN.match(stripped) if m is not None: - return SourceFileEntry(m.group('hashtype'), m.group('file'), + return self.entry_type(m.group('hashtype'), m.group('file'), m.group('hash'))
# Try falling back on the old format @@ -70,10 +72,10 @@ class SourcesFile(object): except ValueError: raise MalformedLineError(line)
- return SourceFileEntry('md5', file, hash) + return self.entry_type('md5', file, hash)
def add_entry(self, hashtype, file, hash): - entry = SourceFileEntry(hashtype, file, hash) + entry = self.entry_type(hashtype, file, hash)
for e in self.entries: if entry.hashtype != e.hashtype: @@ -97,10 +99,14 @@ class SourceFileEntry(object): self.file = file
def __str__(self): - return '%s (%s) = %s\n' % (self.hashtype.upper(), self.file, - self.hash) - + return '%s %s\n' % (self.hash, self.file)
def __eq__(self, other): return ((self.hashtype, self.hash, self.file) == (other.hashtype, other.hash, other.file)) + + +class BSDSourceFileEntry(SourceFileEntry): + def __str__(self): + return '%s (%s) = %s\n' % (self.hashtype.upper(), self.file, + self.hash) diff --git a/test/test_sources.py b/test/test_sources.py index 13f7c06..576bd2c 100644 --- a/test/test_sources.py +++ b/test/test_sources.py @@ -14,6 +14,11 @@ sys.path = old_path class SourceFileEntryTestCase(unittest.TestCase): def test_entry(self): e = sources.SourceFileEntry('md5', 'afile', 'ahash') + expected = 'ahash afile\n' + self.assertEqual(str(e), expected) + + def test_bsd_style_entry(self): + e = sources.BSDSourceFileEntry('md5', 'afile', 'ahash') expected = 'MD5 (afile) = ahash\n' self.assertEqual(str(e), expected)
@@ -27,22 +32,34 @@ class SourcesFileTestCase(unittest.TestCase): shutil.rmtree(self.workdir)
def test_parse_empty_line(self): - s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd') entry = s.parse_line('') self.assertIsNone(entry)
def test_parse_eol_line(self): - s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd') entry = s.parse_line('\n') self.assertIsNone(entry)
def test_parse_whitespace_line(self): - s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd') entry = s.parse_line(' \n') self.assertIsNone(entry)
def test_parse_old_style_line(self): - s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'old') + + line = 'ahash afile\n' + entry = s.parse_line(line) + + self.assertTrue(isinstance(entry, sources.SourceFileEntry)) + self.assertEqual(entry.hashtype, 'md5') + self.assertEqual(entry.hash, 'ahash') + self.assertEqual(entry.file, 'afile') + self.assertEqual(str(entry), line) + + def test_migrate_old_style_line(self): + s = sources.SourcesFile(self.sourcesfile, 'bsd')
line = 'ahash afile\n' newline = 'MD5 (afile) = ahash\n' @@ -55,7 +72,7 @@ class SourcesFileTestCase(unittest.TestCase): self.assertEqual(str(entry), newline)
def test_parse_entry_line(self): - s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd')
line = 'MD5 (afile) = ahash\n' entry = s.parse_line(line) @@ -67,7 +84,7 @@ class SourcesFileTestCase(unittest.TestCase): self.assertEqual(str(entry), line)
def test_parse_wrong_lines(self): - s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd')
lines = ['ahash', 'ahash ', @@ -81,12 +98,12 @@ class SourcesFileTestCase(unittest.TestCase): s.parse_line(line)
def test_open_new_file(self): - s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd') self.assertEqual(len(s.entries), 0)
def test_open_empty_file(self): open(self.sourcesfile, 'w').write('') - s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd') self.assertEqual(len(s.entries), 0)
def test_open_existing_file_with_old_style_lines(self): @@ -98,7 +115,7 @@ class SourcesFileTestCase(unittest.TestCase): for line in lines: f.write(line)
- s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd')
for i, entry in enumerate(s.entries): self.assertTrue(isinstance(entry, sources.SourceFileEntry)) @@ -111,7 +128,7 @@ class SourcesFileTestCase(unittest.TestCase): for line in lines: f.write(line)
- s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd')
for i, entry in enumerate(s.entries): self.assertTrue(isinstance(entry, sources.SourceFileEntry)) @@ -132,7 +149,7 @@ class SourcesFileTestCase(unittest.TestCase): for line in lines: f.write(line)
- s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd')
for i, entry in enumerate(s.entries): self.assertTrue(isinstance(entry, sources.SourceFileEntry)) @@ -147,7 +164,7 @@ class SourcesFileTestCase(unittest.TestCase): for line in lines: f.write(line)
- s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd')
self.assertEqual(len(s.entries), 1) self.assertEqual(s.entries[0].hashtype, 'md5') @@ -162,10 +179,10 @@ class SourcesFileTestCase(unittest.TestCase): f.write(line)
with self.assertRaises(sources.MalformedLineError): - return sources.SourcesFile(self.sourcesfile) + return sources.SourcesFile(self.sourcesfile, 'bsd')
def test_add_entry(self): - s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd') self.assertEqual(len(s.entries), 0)
s.add_entry('md5', 'afile', 'ahash') @@ -177,7 +194,7 @@ class SourcesFileTestCase(unittest.TestCase): self.assertEqual(str(s.entries[-1]), 'MD5 (anotherfile) = anotherhash\n')
def test_add_entry_twice(self): - s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd') self.assertEqual(len(s.entries), 0)
s.add_entry('md5', 'afile', 'ahash') @@ -188,7 +205,7 @@ class SourcesFileTestCase(unittest.TestCase): self.assertEqual(len(s.entries), 1)
def test_add_entry_mixing_hashtypes(self): - s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd') self.assertEqual(len(s.entries), 0)
s.add_entry('md5', 'afile', 'ahash') @@ -199,7 +216,7 @@ class SourcesFileTestCase(unittest.TestCase): s.add_entry('sha512', 'anotherfile', 'anotherhash')
def test_write_new_file(self): - s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd') self.assertEqual(len(s.entries), 0)
s.add_entry('md5', 'afile', 'ahash') @@ -220,7 +237,7 @@ class SourcesFileTestCase(unittest.TestCase): for line in lines: f.write(line)
- s = sources.SourcesFile(self.sourcesfile) + s = sources.SourcesFile(self.sourcesfile, 'bsd') s.add_entry('md5', 'thirdfile', 'thirdhash') s.write()
@@ -239,7 +256,7 @@ class SourcesFileTestCase(unittest.TestCase): for line in lines: f.write(line)
- s = sources.SourcesFile(self.sourcesfile, replace=True) + s = sources.SourcesFile(self.sourcesfile, 'bsd', replace=True) s.add_entry('md5', 'thirdfile', 'thirdhash') s.write()
From: Mathieu Bridon bochecha@daitauha.fr
In addition to being just cleaner, this removes the only warning when running the tests for the pyrpkg.sources module with Python3.
Not that we're anywhere ready to have pyrpkg run with Python3, but any little step in that direction is a good thing.
Plus it means pyrpkg.sources works fine with both Python2 and Python3! --- test/test_sources.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/test/test_sources.py b/test/test_sources.py index 576bd2c..747abe2 100644 --- a/test/test_sources.py +++ b/test/test_sources.py @@ -102,7 +102,9 @@ class SourcesFileTestCase(unittest.TestCase): self.assertEqual(len(s.entries), 0)
def test_open_empty_file(self): - open(self.sourcesfile, 'w').write('') + with open(self.sourcesfile, 'w') as f: + f.write('') + s = sources.SourcesFile(self.sourcesfile, 'bsd') self.assertEqual(len(s.entries), 0)
On Sat, 2015-03-07 at 16:20 +0800, Mathieu Bridon wrote:
On Fri, 2015-03-06 at 19:35 +0100, Pavol Babincak wrote:
We'll need to take different approach then. By default pyrpkg will be backwards compatible and write sources in old format with md5 hash. pyrpkg's clients (e.g. fedpkg) will have a way to switch to new sources file format if their buildsystem infrastructure is prepared for that.
In Fedora this means I need to finish my implementation of new sources format in fedpkg-minimal and only after this package will be in all supported buildroots we can switch fedpkg's default behaviour.
I've untagged rpkg build from rawhide so developers won't rewrite their sources files in the way which Koji buildroots doesn't understand yet.
Seems like the best for now.
I'll send new patches that implement what you described above.
Patch 2/3 in the new series I just sent does just that:
- pyrpkg.sources can read/write sources files both in the current format and the new (BSD-style) format - pyrpkg defaults to writing sources files in the current format, to preserve compatibility - pyrpkg-based applications (like fedpkg) can override this easily if they want to move to writing sources files in the new (BSD-style) format
I believe with these patches merged you could make a new release of rpkg, build it and update it for Fedora in all branches (including EPEL), and nothing would change... except we'd now have the possibility to eventually move to the new format whenever we decided to.
We should even be able to only move to the new format in newer Fedora only if we wanted to, but it could be confusing, as the sources file would be written in a given format based on the Fedora release the packager is running, not the target Fedora branch for the build.
But it would all work nevertheless, as long as pyrpkg had been updated in all branches.
On 03/19/2015 12:16 PM, Mathieu Bridon wrote:
On Sat, 2015-03-07 at 16:20 +0800, Mathieu Bridon wrote:
On Fri, 2015-03-06 at 19:35 +0100, Pavol Babincak wrote:
We'll need to take different approach then. By default pyrpkg will be backwards compatible and write sources in old format with md5 hash. pyrpkg's clients (e.g. fedpkg) will have a way to switch to new sources file format if their buildsystem infrastructure is prepared for that.
In Fedora this means I need to finish my implementation of new sources format in fedpkg-minimal and only after this package will be in all supported buildroots we can switch fedpkg's default behaviour.
I've untagged rpkg build from rawhide so developers won't rewrite their sources files in the way which Koji buildroots doesn't understand yet.
Seems like the best for now.
I'll send new patches that implement what you described above.
Patch 2/3 in the new series I just sent does just that:
- pyrpkg.sources can read/write sources files both in the current format and the new (BSD-style) format
- pyrpkg defaults to writing sources files in the current format, to preserve compatibility
- pyrpkg-based applications (like fedpkg) can override this easily if they want to move to writing sources files in the new (BSD-style) format
Patches looked fine and tests passed so merged and released new rpkg - 1.32.
rpkg built in rawhide by now. I'll do the rest later. Hopefully this week.
On 02/16/2015 10:33 PM, Mathieu Bridon wrote:
As part of the move away from MD5 for the source files hashes in the lookaside cache, we have decided to change the format of the 'sources' files, so it includes the hash type, to make it easier to identify which hash was used.
We agreed on using the BSD-style format, as returned on Linux by passing the extra --tag option to commands like md5sum, sha512sumn, etc.
In addition, we decided to forbid mixing hash types in a single sources file, forcing maintainers to use `fedpkg new-sources` the first time they upload an additional source file.
are these changes backwards compatible?
This 4 patches series implements all that, with loads of unit tests, and some code cleanup.
src/pyrpkg/__init__.py | 60 ++++----- src/pyrpkg/sources.py | 123 ++++++++++++------ test/test_sources.py | 334 ++++++++++++++++++++++++++++++++++-------------- 3 files changed, 359 insertions(+), 158 deletions(-)
https://fedorahosted.org/rel-eng/ticket/5846
-- buildsys mailing list buildsys@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/buildsys
On Thu, 2015-02-19 at 19:21 -0500, Mike McLean wrote:
On 02/16/2015 10:33 PM, Mathieu Bridon wrote:
As part of the move away from MD5 for the source files hashes in the lookaside cache, we have decided to change the format of the 'sources' files, so it includes the hash type, to make it easier to identify which hash was used.
We agreed on using the BSD-style format, as returned on Linux by passing the extra --tag option to commands like md5sum, sha512sumn, etc.
In addition, we decided to forbid mixing hash types in a single sources file, forcing maintainers to use `fedpkg new-sources` the first time they upload an additional source file.
are these changes backwards compatible?
I always get confused with the meaning of "backwards" vs "forward" compatible, so let me state both. :)
Older rpkg will not work with files in the new format, they will just raise exceptions trying to read them.
However, after being patched, rpkg still will be able to read files in the old format. In fact, that's how files get converted to the new format.
Does that answer your question?
buildsys@lists.fedoraproject.org
-
Mathieu Bridon -
Mike McLean -
Pavol Babincak