diff --git a/README.md b/README.md index ad4ce21..4ecbb98 100644 --- a/README.md +++ b/README.md @@ -1,10 +1,11 @@ -# importpatches.py +# importpatches.py and exportpatches.py -A command to update Fedora Python dist-git spec & patches from a Git repository +Commands to update Fedora Python dist-git spec & patches from/to a Git repository Meant to be used with a local clone of [fedora-python/cpython] which includes tags like `vX.Y.Z` (upstream releases) and branches like `fedora-X.Y` (`vX.Y.Z` + commits for individual patches). +The exportpatches script assumes that the remote is named `fedora-python`. The summary lines of patch commits must start with `NNNNN: `, where NNNNN is the patch number (registered in the [patch registry]). @@ -13,6 +14,19 @@ The rest of the commit message should be usable in the spec. (It also mostly works with the `fedora-2.7` branch, which uses different conventions.) +Alternatively, a **numberless mode** is supported (intended for Python 3.10+, +which no longer has the specially-handled patch 189): if none of the commits +being imported have a `NNNNN: ` prefix, `importpatches` switches to +numberless mode automatically. Patch filenames are always freshly generated +(no NNNNN- prefix, no filename-stability lookup), and the spec declares bare +`Patch:` (or, with `--rhel8-compat`, sequential `Patch1:`, `Patch2:`, ... with +no semantic meaning, for RPM on RHEL 8, which doesn't support bare `Patch:` +tags). `exportpatches` auto-detects numberless mode too, from the spec's +`Patch` declarations and patch filenames (rather than commit messages, since +it hasn't created any commits yet), and never adds a number (real or RHEL 8 +compat) to a commit message in that mode. Commits/patches must not mix +numbered and numberless conventions; both scripts error out if they do. + [fedora-python/cpython]: https://github.com/fedora-python/cpython [patch registry]: https://fedoraproject.org/wiki/SIGs/Python/PythonPatches @@ -21,15 +35,17 @@ conventions.) - [click](https://pypi.org/project/click/) (`dnf install python3-click`) - [rpmautospec](https://docs.pagure.org/rpmautospec/) (`dnf install python3-rpmautospec`) +- [pytest](https://pypi.org/project/pytest/) (`dnf install python3-pytest`), only needed to run the test suite ## Setup -Add the script to your `$PATH`, for example: +Add the scripts to your `$PATH`, for example: ln -s $PWD/importpatches.py ~/.local/bin/importpatches + ln -s $PWD/exportpatches.py ~/.local/bin/exportpatches -The script needs to know where your local clone of `fedora-python/cpython` is, +The scripts need to know where your local clone of `fedora-python/cpython` is, and uses Git configuration as a default. In your clone of dist-git, run `git config importpatches.upstream .../cpython`. @@ -40,13 +56,48 @@ to avoid the need to set this in all dist-git clones of Pythons. ## Usage -Run `importpatches.py` without arguments in your clone of dist-git. +Run `importpatches.py`/`exportpatches.py` without arguments in your clone of dist-git. If the defaults don't work for you, run with `--help` to see the options. +## What it does + +### importpatches + +importpatches takes commits between a base tag and head tag +in the [fedora-python/cpython] clone and turns them into patch files +plus updated spec comments in the current dist-git checkout. +Each commit is formatted into a *.patch file (named NNNNN-... where possible) +and its Git patch-id is written into the spec comment above +the PatchNNNNN: line, along with the commit message body. +It replaces the whole patches section between the +`(Patches taken from github.com/fedora-python/cpython)` and +`(New patches go here ^^^)` markers, removing old patch files and moving +the new ones into place. + +In numberless mode (auto-detected, see above), patch filenames are always +generated fresh from the commit summary instead of being matched by number, +and the spec gets bare `Patch:` lines (or `Patch1:`, `Patch2:`, ... with +`--rhel8-compat`, which is a no-op in numbered mode). + + +### exportpatches + +exportpatches is an inverse of importpatches: it takes patches listed +in the dist-git spec and applies them onto a local clone of [fedora-python/cpython], +producing the fedora-X.Y branch from the upstream vX.Y.Z tag. +Each patch becomes one commit, with the summary line prefixed by its patch +number (NNNNN: ) as required by the patch registry. +It then tags the result and pushes the branch and tag to the fedora remote. + +In numberless mode (auto-detected from the spec's `Patch` declarations, see +above), no number is ever prefixed onto a commit's summary line, even if the +spec uses the `--rhel8-compat` sequential `Patch1:`, `Patch2:`, ... scheme. + + ## Git hash IDs -The script adds Git hash IDs to the spec file. +The importpatches script adds Git hash IDs to the spec file. These are hashes of the patch content, ignoring tings like context lines and comments. When one of these changes, pay special atttention to the patch diff. @@ -60,6 +111,11 @@ via `rpmautospec.calculate_release()` instead of parsing it from the spec or querying `rpm`. +## Running tests + + python3 -m pytest + + ## License The script is available under the MIT license. May it serve you well. diff --git a/exportpatches.py b/exportpatches.py index 5c7bf04..ffbed4c 100755 --- a/exportpatches.py +++ b/exportpatches.py @@ -7,23 +7,91 @@ import readline import tempfile import os +import dataclasses +import enum import click # dnf install python3-click from rpmautospec import specfile_uses_rpmautospec, calculate_release REPO_KEY = 'importpatches.upstream' +PATCH_LINE_RE = re.compile(r'^Patch(?P[0-9]{1,5})?:\s*(?P\S+)') +NUMBERED_FILENAME_RE = re.compile(r'^\d{5}-') + + +class Mode(enum.Enum): + NUMBERED = 'numbered' + NUMBERLESS = 'numberless' + + +class ExportModeDetectionError(ValueError): + """Raised when a spec's Patch declarations are ambiguous/inconsistent""" + + +@dataclasses.dataclass +class SpecPatch: + """A single Patch declaration parsed from the spec file""" + number: int | None + value: str + + +def find_duplicate_patch_numbers(patches): + """Return the set of patch numbers that appear more than once in + `patches` (a list of SpecPatch); ignores entries with no number.""" + seen = set() + duplicates = set() + for p in patches: + if p.number is None: + continue + if p.number in seen: + duplicates.add(p.number) + seen.add(p.number) + return duplicates + + +def detect_export_mode(patches): + """Detect whether spec Patch declarations use numbered or numberless + conventions + + `patches` is a list of SpecPatch. URL-valued patches (typically added + temporarily by hand, e.g. for testing an upstream fix before it's + imported properly) are excluded from detection and never influence the + result either way. Among the remaining, local-file patches: a 5-digit + filename prefix is the primary signal for Mode.NUMBERED (legacy). + Otherwise, bare Patch: declarations, or PatchN: numbers present only in + the spec (the RHEL 8 compatibility scheme, which has no semantic + meaning, so numbers need not be contiguous or in any particular order + -- patches may have been manually removed, leaving gaps), mean + Mode.NUMBERLESS. A mix of bare and numbered declarations is ambiguous + and raises ExportModeDetectionError. + """ + if not patches: + return Mode.NUMBERED + + local_patches = [p for p in patches if '://' not in p.value] + if not local_patches: + return Mode.NUMBERED + + filenames = [p.value.rsplit('/', 1)[-1] for p in local_patches] + if any(NUMBERED_FILENAME_RE.match(fn) for fn in filenames): + return Mode.NUMBERED + + numbers = [p.number for p in local_patches] + if all(n is None for n in numbers) or all(n is not None for n in numbers): + return Mode.NUMBERLESS + raise ExportModeDetectionError( + 'Cannot determine numbering mode from spec Patch declarations: no ' + 'filename has a 5-digit number prefix, but Patch tag numbers are a ' + f'mix of numbered and bare declarations: {numbers!r}' + ) -def removeprefix(self, prefix, regex=False): - if regex: - return re.sub(r'^{0}'.format(prefix), '', self) +def removeprefix(self, prefix): + # PEP-616 backport + if self.startswith(prefix): + return self[len(prefix):] else: - # PEP-616 backport - if self.startswith(prefix): - return self[len(prefix):] - else: - return self + return self def run(*args, echo_stdout=True, **kwargs): """Like subprocess.run, but with logging and more appropriate defaults""" @@ -57,6 +125,12 @@ def add_redir(kwarg_name, symbol): return result +def tag_exists(tag): + """Whether `tag` already exists in the repository (cwd)""" + repo_tags = run(*shlex.split(f"git tag --list {tag}"), echo_stdout=False) + return any(repo_tag.startswith(tag) for repo_tag in repo_tags.stdout.split('\n')) + + @click.command(context_settings={'help_option_names': ['-h', '--help']}) @click.option( '-r', '--repo', default=None, metavar='REPO', @@ -87,10 +161,15 @@ def add_redir(kwarg_name, symbol): '-t', '--tag', default=None, metavar='XY', help="Custom tag, e.g. fedora-3.13.0-1" ) +@click.option( + '--no-push', is_flag=True, default=False, + help="Tag the result but skip pushing to the remote, and skip the " + + "interactive push confirmation. Useful when testing exportpatches." +) @click.argument( 'spec', default=None, required=False, type=Path, ) -def main(spec, repo, base, branch, python_version, release, tag): +def main(spec, repo, base, branch, python_version, release, tag, no_push): """ Update cpython Git repository with patches from dist-git spec @@ -106,6 +185,13 @@ def main(spec, repo, base, branch, python_version, release, tag): PatchNNNNN: /, where NNNNN is a patch number from: https://fedoraproject.org/wiki/SIGs/Python/PythonPatches + Numberless mode is also supported and auto-detected from the spec: if + no Patch filename has a NNNNN- prefix, declarations may be either bare + Patch: /, or PatchN: / with N forming a plain + 1..N sequence (the RHEL 8 compatibility scheme, which has no semantic + meaning). In numberless mode, no number is ever added to a commit + message, even the RHEL 8 compatibility ones. + When exportpatches successfuly finishes, it is expected to run importpatches to import patch to the spec file in a standardized form. @@ -205,24 +291,46 @@ def main(spec, repo, base, branch, python_version, release, tag): click.secho(f'Assuming --release={release}', fg='yellow') with spec.open() as f: - patches = {} + patches = [] for line in f: line = line.strip() if line.startswith('Patch'): - try: - patch_number = removeprefix(re.match("^Patch[0-9]{1,5}:", -line).group(), 'Patch') - except AttributeError: + match = PATCH_LINE_RE.match(line) + if not match: click.secho( - "Patch number is missing.", + f"Could not parse Patch line: {line}", fg='red', ) exit(1) - update = {patch_number : removeprefix(line, 'Patch[0-9]*: *', -regex=True)} - patches.update(**update) + number_str = match.group('number') + patches.append(SpecPatch( + number=int(number_str) if number_str else None, + value=match.group('value'), + )) click.secho(f'Found {len(patches)} ({patches}) patches from spec file', fg='yellow') + duplicate_numbers = find_duplicate_patch_numbers(patches) + if duplicate_numbers: + click.secho( + f'Duplicate Patch numbers found in spec: {sorted(duplicate_numbers)}', + fg='red', + ) + exit(1) + + try: + mode = detect_export_mode(patches) + except ExportModeDetectionError as e: + click.secho(str(e), fg='red') + exit(1) + click.secho(f'Detected mode: {mode.value}', fg='yellow') + + if mode == Mode.NUMBERED: + # Bare Patch: lines are only legitimate in numberless mode. + for record in patches: + if record.number is None: + click.secho("Patch number is missing.", fg='red') + exit(1) + click.secho(f'Changing working directory to {repo}', fg='yellow') os.chdir(repo) path = str(spec).rsplit('/',1)[0] @@ -263,12 +371,12 @@ def main(spec, repo, base, branch, python_version, release, tag): *shlex.split(f"git reset --hard {base}") ) - for patch_number, patch in patches.items(): + for record in patches: head_hash = run( *shlex.split(f"git rev-parse HEAD") ) - patch_filename = patch.rsplit('/', 1)[-1] + patch_filename = record.value.rsplit('/', 1)[-1] try: proc = run( *shlex.split(f"git am --committer-date-is-author-date {path}/{patch_filename}") @@ -283,14 +391,17 @@ def main(spec, repo, base, branch, python_version, release, tag): *shlex.split(f"git log --format=%B -n 1"), stdout=subprocess.PIPE ) - # checking if patch number is present at the beginning of the - # commit message - pattern = re.compile(r"^[0-9]{5}:") - if not pattern.match(proc.stdout): - patch_number_with_padding = patch_number.rjust(5, '0') - proc = run( - 'git', 'commit', '--amend', '-m', f'{patch_number_with_padding}: {proc.stdout}' - ) + if mode == Mode.NUMBERED: + # checking if patch number is present at the beginning of the + # commit message + pattern = re.compile(r"^[0-9]{5}:") + if not pattern.match(proc.stdout): + patch_number_with_padding = str(record.number).rjust(5, '0') + proc = run( + 'git', 'commit', '--amend', '-m', f'{patch_number_with_padding}: {proc.stdout}' + ) + # In numberless mode (bare Patch: or RHEL 8 compat PatchN:), + # never insert any number into the commit message. head1_hash = run( *shlex.split(f"git rev-parse HEAD^1") ) @@ -304,37 +415,41 @@ def main(spec, repo, base, branch, python_version, release, tag): if tag == None: tag = f'fedora-{upstream_version}-{release}' - while(True): - click.secho(f'Checking if tag ({tag}) already exists', fg='yellow') - repo_tags = run( - *shlex.split(f"git tag --list {tag}"), - echo_stdout=False - ) - tag_exists = False - for repo_tag in repo_tags.stdout.split('\n'): - if repo_tag.startswith(f"{tag}"): - tag_exists = True - if tag_exists: + if no_push: + # Avoid the interactive tag-collision prompt below too, since + # --no-push is meant for non-interactive testing. + if tag_exists(tag): click.secho( - f"Tag ({tag}) already exists in the repository.", + f"Tag ({tag}) already exists; --no-push given, skipping tag creation.", fg='yellow', ) - click.secho(f"Create a new tag? [y/n]", fg='yellow') - c = input() - if c == 'y': - tag = input("Tag name: ") - else: + else: + click.secho(f"About to tag the current state of repository with {tag}.", fg='yellow') + run(*shlex.split(f"git tag {tag}")) + else: + while(True): + click.secho(f'Checking if tag ({tag}) already exists', fg='yellow') + if tag_exists(tag): click.secho( - f"Exiting...", - fg='red', + f"Tag ({tag}) already exists in the repository.", + fg='yellow', ) - exit(1) - else: - break + click.secho(f"Create a new tag? [y/n]", fg='yellow') + c = input() + if c == 'y': + tag = input("Tag name: ") + else: + click.secho( + f"Exiting...", + fg='red', + ) + exit(1) + else: + break - click.secho(f"About to tag the current state of repository with {tag}.", fg='yellow') + click.secho(f"About to tag the current state of repository with {tag}.", fg='yellow') - run(*shlex.split(f"git tag {tag}")) + run(*shlex.split(f"git tag {tag}")) click.secho( f"Following commands will push the changes:", @@ -342,24 +457,28 @@ def main(spec, repo, base, branch, python_version, release, tag): ) print(f"git push fedora-python {tag}") print(f"git push --force -u fedora-python fedora-{python_version}") - click.secho( - f"Do you wish to continue? [y/n]", - fg='yellow', - ) - c = input() - if c == 'y': - proc = run( - *shlex.split(f"git push fedora-python {tag}") - ) - proc = run( - *shlex.split(f"git push --force -u fedora-python fedora-{python_version}") - ) + + if no_push: + click.secho("--no-push given, skipping push.", fg='yellow') else: click.secho( - f"Exiting...", - fg='red', + f"Do you wish to continue? [y/n]", + fg='yellow', ) - exit(1) + c = input() + if c == 'y': + proc = run( + *shlex.split(f"git push fedora-python {tag}") + ) + proc = run( + *shlex.split(f"git push --force -u fedora-python fedora-{python_version}") + ) + else: + click.secho( + f"Exiting...", + fg='red', + ) + exit(1) click.secho('OK', fg='green') diff --git a/importpatches.py b/importpatches.py index d10d6ce..bf79f6b 100755 --- a/importpatches.py +++ b/importpatches.py @@ -6,6 +6,7 @@ import shlex import re import dataclasses +import enum from textwrap import dedent import tempfile import shutil @@ -60,26 +61,73 @@ def removeprefix(self, prefix): return self -@dataclasses.dataclass -class PatchInformation: - """All information needed about a patch""" - number: int - patch_id: str - comment: str - filename: Path - trailer: str = '' +class Mode(enum.Enum): + NUMBERED = 'numbered' + NUMBERLESS = 'numberless' -def handle_patch(repo, commit_id, *, tempdir, python_version): - """Handle a single patch, writing it to `tempdir` and returning info - """ - message = run( - 'git', 'show', '-s', '--format=%B', commit_id, - cwd=repo, - ).stdout.strip() - summary, _, message_body = message.partition('\n') - match = PATCH_NUMBER_RE.match(summary) +class ModeDetectionError(ValueError): + """Raised when commits mix numbered and numberless conventions""" + + +def find_dirty_style_number(summary, message): + """Resolve the patch number for an 'old and dirty' style commit summary + (a bare patch filename), or None if none can be found""" + match = re.search(r'\d{5,}', message) if match: + return int(match.group(0)) + return SPECIAL_PATCH_NUMBERS.get(summary) + + +def detect_import_mode(messages, log): + """Detect whether commits use numbered or numberless conventions + + `messages` maps commit_id -> full commit message text (as returned by + `git show -s --format=%B`). `log` is the list of commit ids to inspect + (order doesn't matter for detection). + + Returns Mode.NUMBERED or Mode.NUMBERLESS. Raises ModeDetectionError if + commits mix both conventions. + """ + numbered_ids = [] + numberless_ids = [] + for commit_id in log: + message = messages[commit_id] + summary = message.partition('\n')[0] + if PATCH_NUMBER_RE.match(summary): + numbered_ids.append(commit_id) + elif summary.endswith('.patch') and FLIENAME_SAFE_RE.match(summary) and \ + find_dirty_style_number(summary, message) is not None: + # "old and dirty" Python 2 style commits always carry a number + # (in the message body or SPECIAL_PATCH_NUMBERS), so they count + # as numbered too. + numbered_ids.append(commit_id) + else: + numberless_ids.append(commit_id) + + if numbered_ids and numberless_ids: + raise ModeDetectionError( + 'Commits mix numbered and numberless conventions; cannot ' + 'auto-detect mode.\n' + 'Numbered-looking commits: ' + + ', '.join(c[:9] for c in numbered_ids) + '\n' + 'Numberless-looking commits: ' + + ', '.join(c[:9] for c in numberless_ids) + ) + if numberless_ids: + return Mode.NUMBERLESS + return Mode.NUMBERED + + +def determine_patch_number_and_filename(commit_id, summary, message, mode): + """Determine a patch's number (or None) and filename + + In Mode.NUMBERED, an existing NNNNN-*.patch file is reused if found, + to keep filenames stable; otherwise (or in Mode.NUMBERLESS) a fresh + filename is generated from the commit summary. + """ + if mode == Mode.NUMBERED and PATCH_NUMBER_RE.match(summary): + match = PATCH_NUMBER_RE.match(summary) number = int(match.group(1)) paths = list(Path('.').glob(f'{number:05d}-*.patch')) if len(paths) == 0: @@ -91,21 +139,41 @@ def handle_patch(repo, commit_id, *, tempdir, python_version): exit( 'More than one patch file matches {number}: {paths_msg}' ) - elif summary.endswith('.patch') and FLIENAME_SAFE_RE.match(summary): + elif mode == Mode.NUMBERED and summary.endswith('.patch') and \ + FLIENAME_SAFE_RE.match(summary): path = Path(summary) - match = re.search(r'\d{5,}', message) - if match: - number = int(str(match.group(0))) - elif summary in SPECIAL_PATCH_NUMBERS: - number = SPECIAL_PATCH_NUMBERS[summary] - else: + number = find_dirty_style_number(summary, message) + if number is None: exit( f'Cannot find patch number in {commit_id[:9]}: {summary}' ) + elif mode == Mode.NUMBERLESS: + number = None + path = Path(slugify(summary) + '.patch') else: exit( f'Cannot derive patch filename from {commit_id[:9]}: {summary}' ) + return number, path + + +@dataclasses.dataclass +class PatchInformation: + """All information needed about a patch""" + patch_id: str + comment: str + filename: Path + number: int | None = None + trailer: str = '' + + +def handle_patch(repo, commit_id, message, *, tempdir, python_version, mode): + """Handle a single patch, writing it to `tempdir` and returning info + """ + summary, _, message_body = message.partition('\n') + number, path = determine_patch_number_and_filename( + commit_id, summary, message, mode, + ) patch_path = tempdir / path.name @@ -124,7 +192,7 @@ def handle_patch(repo, commit_id, *, tempdir, python_version): hash_id = run('git', 'patch-id', '--stable', stdin=f).stdout.split()[0] spec_comment = [] - if summary.endswith('.patch'): + if summary.endswith('.patch') and number is not None: message_body = removeprefix(message_body.strip(), f'{number:05d} #\n') else: spec_comment.append(re.sub(PATCH_NUMBER_RE, '', summary)) @@ -141,8 +209,11 @@ def handle_patch(repo, commit_id, *, tempdir, python_version): trailer = '' return PatchInformation( - number, hash_id, '\n'.join(spec_comment).strip(), path.name, - trailer, + patch_id=hash_id, + comment='\n'.join(spec_comment).strip(), + filename=path.name, + number=number, + trailer=trailer, ) @@ -228,10 +299,17 @@ def add_redir(kwarg_name, symbol): '-v', '--python-version', default=None, metavar='X.Y', help="Python version, e.g. 3.10 (default extracted from spec name)" ) +@click.option( + '--rhel8-compat', is_flag=True, default=False, + help="In numberless mode, write sequential fake Patch1:, Patch2:, ... " + + "numbers in the spec (not zero-padded, not part of the filename " + + "or comment) for compatibility with RPM on RHEL 8, which doesn't " + + "support bare 'Patch:' tags. No effect in numbered mode." +) @click.argument( 'spec', default=None, required=False, type=Path, ) -def main(spec, repo, base, head, python_version): +def main(spec, repo, base, head, python_version, rhel8_compat): """Update Fedora Python dist-git spec & patches from a Git repository Meant to be run in a local clone of Fedora's pythonX.Y dist-git. @@ -263,6 +341,13 @@ def main(spec, repo, base, head, python_version): Patch 189 is handled specially: version numbers of bundled packages are extracted from it. + If none of the commits between TAG and BRANCH have a NNNNN: prefix, + numberless mode is used instead: commit summaries are plain text, + patch filenames are always freshly generated from the summary, and + the spec declares bare ``Patch:`` (or, with --rhel8-compat, sequential + ``Patch1:``, ``Patch2:``, ... with no semantic meaning). Commits must + not mix numbered and numberless conventions. + Note that patch files are read and written from the current directory, regardless of the --repo option. @@ -382,19 +467,52 @@ def cyan(text): 'was selected; try giving -c explicitly.' ) + messages = { + commit_id: run( + 'git', 'show', '-s', '--format=%B', commit_id, + cwd=repo, echo_stdout=False, + ).stdout.strip() + for commit_id in log + } + try: + mode = detect_import_mode(messages, log) + except ModeDetectionError as e: + exit(str(e)) + click.secho(f'Detected mode: {mode.value}', fg='yellow') + patches_section = [] + rhel8_number = 0 + seen_filenames = {} for commit_id in reversed(log): result = handle_patch( - repo, commit_id, tempdir=tempdir, - python_version=python_version, + repo, commit_id, messages[commit_id], tempdir=tempdir, + python_version=python_version, mode=mode, ) + if result.filename in seen_filenames: + exit( + f'Patch filename {result.filename} would be generated ' + + f'for both {seen_filenames[result.filename][:9]} and ' + + f'{commit_id[:9]}; rename one of the commits so their ' + + 'summaries produce distinct filenames.' + ) + seen_filenames[result.filename] = commit_id comment = '\n'.join( f'# {l}' if l else '#' for l in result.comment.splitlines() ) + if result.number is not None: + header = f'# {result.number:05d} # {result.patch_id}' + patch_tag = f'Patch{result.number}: {result.filename}' + else: + header = f'# {result.patch_id}' + if rhel8_compat: + rhel8_number += 1 + patch_tag = f'Patch{rhel8_number}: {result.filename}' + else: + patch_tag = f'Patch: {result.filename}' section = dedent(f""" - # {result.number:05d} # {result.patch_id} + {header} %s - Patch{result.number}: {result.filename} + {patch_tag} """) % comment.replace('%', '%%') if result.trailer: section = section.rstrip() + result.trailer diff --git a/tests/test_exportpatches.py b/tests/test_exportpatches.py new file mode 100644 index 0000000..bbb6918 --- /dev/null +++ b/tests/test_exportpatches.py @@ -0,0 +1,117 @@ +import pytest + +import exportpatches as ep + + +def P(number, value): + return ep.SpecPatch(number=number, value=value) + + +# --- PATCH_LINE_RE --- + +def test_patch_line_re_numbered(): + m = ep.PATCH_LINE_RE.match('Patch00042: 00042-foo.patch') + assert m.group('number') == '00042' + assert m.group('value') == '00042-foo.patch' + + +def test_patch_line_re_bare(): + m = ep.PATCH_LINE_RE.match('Patch: foo.patch') + assert m.group('number') is None + assert m.group('value') == 'foo.patch' + + +def test_patch_line_re_rhel8_compat(): + m = ep.PATCH_LINE_RE.match('Patch1: foo.patch') + assert m.group('number') == '1' + + +# --- detect_export_mode --- + +def test_detect_export_mode_numbered_by_filename_prefix(): + patches = [P(1, '00001-foo.patch'), P(2, '00002-bar.patch')] + assert ep.detect_export_mode(patches) == ep.Mode.NUMBERED + + +def test_detect_export_mode_numbered_with_trailing_legacy_unnumbered_entry(): + # legacy: manually added trailing patch, no filename/commit number yet + patches = [P(1, '00001-foo.patch'), P(2, 'bar.patch')] + assert ep.detect_export_mode(patches) == ep.Mode.NUMBERED + + +def test_detect_export_mode_only_url_value_defaults_numbered(): + # no local patches to detect anything from; arbitrary default, matches + # the "no patches at all" case + patches = [P(42, 'https://bugzilla.redhat.com/attachment.cgi?id=999999')] + assert ep.detect_export_mode(patches) == ep.Mode.NUMBERED + + +def test_detect_export_mode_url_value_does_not_affect_numberless_detection(): + # a temporary, hand-added URL-based patch (e.g. testing an upstream fix + # before it's imported properly) must not flip detection away from + # what the other, local patches indicate + patches = [ + P(None, 'foo.patch'), + P(5, 'https://bugzilla.redhat.com/attachment.cgi?id=999999'), + ] + assert ep.detect_export_mode(patches) == ep.Mode.NUMBERLESS + + +def test_detect_export_mode_url_value_does_not_force_numberless(): + # same, but the other local patches are legacy-numbered + patches = [ + P(1, '00001-foo.patch'), + P(5, 'https://bugzilla.redhat.com/attachment.cgi?id=999999'), + ] + assert ep.detect_export_mode(patches) == ep.Mode.NUMBERED + + +def test_detect_export_mode_numberless_bare(): + patches = [P(None, 'foo.patch'), P(None, 'bar.patch')] + assert ep.detect_export_mode(patches) == ep.Mode.NUMBERLESS + + +def test_detect_export_mode_numberless_rhel8_compat_sequence(): + patches = [P(1, 'foo.patch'), P(2, 'bar.patch'), P(3, 'baz.patch')] + assert ep.detect_export_mode(patches) == ep.Mode.NUMBERLESS + + +def test_detect_export_mode_numberless_rhel8_compat_out_of_order(): + # RHEL 8 compat numbers have no semantic meaning, so reordering the + # spec lines without renumbering must still be recognized as numberless + patches = [P(2, 'bar.patch'), P(1, 'foo.patch'), P(3, 'baz.patch')] + assert ep.detect_export_mode(patches) == ep.Mode.NUMBERLESS + + +def test_detect_export_mode_ambiguous_mixed_none_and_number_raises(): + patches = [P(None, 'foo.patch'), P(1, 'bar.patch')] + with pytest.raises(ep.ExportModeDetectionError): + ep.detect_export_mode(patches) + + +def test_detect_export_mode_numberless_rhel8_compat_with_gaps(): + # a patch may have been manually removed, leaving a gap; RHEL 8 compat + # numbers have no semantic meaning so this must still be numberless + patches = [P(1, 'foo.patch'), P(3, 'bar.patch')] + assert ep.detect_export_mode(patches) == ep.Mode.NUMBERLESS + + +def test_detect_export_mode_no_patches_defaults_numbered(): + assert ep.detect_export_mode([]) == ep.Mode.NUMBERED + + +# --- find_duplicate_patch_numbers --- + +def test_find_duplicate_patch_numbers_none_duplicated(): + patches = [P(1, 'foo.patch'), P(2, 'bar.patch')] + assert ep.find_duplicate_patch_numbers(patches) == set() + + +def test_find_duplicate_patch_numbers_detects_duplicate(): + patches = [P(1, 'foo.patch'), P(1, 'bar.patch'), P(2, 'baz.patch')] + assert ep.find_duplicate_patch_numbers(patches) == {1} + + +def test_find_duplicate_patch_numbers_ignores_bare_entries(): + patches = [P(None, 'foo.patch'), P(None, 'bar.patch')] + assert ep.find_duplicate_patch_numbers(patches) == set() diff --git a/tests/test_importpatches.py b/tests/test_importpatches.py new file mode 100644 index 0000000..d5f36bd --- /dev/null +++ b/tests/test_importpatches.py @@ -0,0 +1,108 @@ +from pathlib import Path + +import pytest + +import importpatches as ip + + +def msg(summary, body=''): + return summary + ('\n' + body if body else '') + + +# --- detect_import_mode --- + +def test_detect_mode_all_numbered(): + log = ['c1', 'c2'] + messages = {'c1': msg('00001: First'), 'c2': msg('00002: Second')} + assert ip.detect_import_mode(messages, log) == ip.Mode.NUMBERED + + +def test_detect_mode_all_numberless(): + log = ['c1', 'c2'] + messages = {'c1': msg('Fix something'), 'c2': msg('Fix something else')} + assert ip.detect_import_mode(messages, log) == ip.Mode.NUMBERLESS + + +def test_detect_mode_dirty_style_counts_as_numbered(): + log = ['c1', 'c2'] + messages = { + 'c1': msg('00001: First'), + 'c2': msg('python-2.6-rpath.patch', '# 00016 #\nBody'), + } + assert ip.detect_import_mode(messages, log) == ip.Mode.NUMBERED + + +def test_detect_mode_filename_shaped_numberless_summary_stays_numberless(): + # a numberless commit summary that happens to be filename-safe and end + # in '.patch', but carries no resolvable dirty-style number, must not + # be confused with a genuine "old and dirty" commit + log = ['c1'] + messages = {'c1': msg('fix-thing.patch', 'Body with no big number')} + assert ip.detect_import_mode(messages, log) == ip.Mode.NUMBERLESS + + +def test_detect_mode_mixture_raises(): + log = ['c1', 'c2'] + messages = {'c1': msg('00001: First'), 'c2': msg('Fix something else')} + with pytest.raises(ip.ModeDetectionError): + ip.detect_import_mode(messages, log) + + +def test_detect_mode_no_commits_defaults_numbered(): + assert ip.detect_import_mode({}, []) == ip.Mode.NUMBERED + + +# --- determine_patch_number_and_filename --- + +def test_numbered_existing_patch_keeps_filename(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / '00003-old-name.patch').touch() + number, path = ip.determine_patch_number_and_filename( + 'deadbeef', '00003: Fix the thing', '00003: Fix the thing', ip.Mode.NUMBERED, + ) + assert number == 3 + assert path == Path('00003-old-name.patch') + + +def test_numbered_new_patch_slugifies(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + number, path = ip.determine_patch_number_and_filename( + 'deadbeef', '00003: Fix the thing', '00003: Fix the thing', ip.Mode.NUMBERED, + ) + assert number == 3 + # slugify() operates on the whole summary, so the number prefix ends up + # baked into the filename here (pre-existing behavior, unchanged). + assert path == Path('00003-fix-the-thing.patch') + + +def test_numberless_always_slugifies_ignoring_lookalike_file(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / '00003-fix-the-thing.patch').touch() # decoy, must be ignored + number, path = ip.determine_patch_number_and_filename( + 'deadbeef', 'Fix the thing', 'Fix the thing', ip.Mode.NUMBERLESS, + ) + assert number is None + assert path == Path('fix-the-thing.patch') + + +def test_numberless_filename_shaped_summary_does_not_crash(monkeypatch): + # mirrors test_detect_mode_filename_shaped_numberless_summary_stays_numberless: + # once mode is NUMBERLESS, a '.patch'-ending filename-safe summary must + # not be routed into the dirty-style number lookup and crash + number, path = ip.determine_patch_number_and_filename( + 'deadbeef', 'fix-thing.patch', 'fix-thing.patch\n\nBody with no big number', + ip.Mode.NUMBERLESS, + ) + assert number is None + assert path == Path('fix-thing-patch.patch') + + +def test_numberless_never_calls_glob(monkeypatch): + def boom(*a, **k): + raise AssertionError('glob must not be called in numberless mode') + monkeypatch.setattr(Path, 'glob', boom) + number, path = ip.determine_patch_number_and_filename( + 'deadbeef', 'Fix the thing', 'Fix the thing', ip.Mode.NUMBERLESS, + ) + assert number is None + assert path == Path('fix-the-thing.patch') diff --git a/tests/test_integration.py b/tests/test_integration.py new file mode 100644 index 0000000..b4f7800 --- /dev/null +++ b/tests/test_integration.py @@ -0,0 +1,451 @@ +""" +End-to-end integration tests: run importpatches.py/exportpatches.py as real +subprocesses against scratch upstream/dist-git repos built under tmp_path. +""" + +import os +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +IMPORTPATCHES = REPO_ROOT / 'importpatches.py' +EXPORTPATCHES = REPO_ROOT / 'exportpatches.py' + +SPEC_FILENAME = 'python1.0.spec' +UPSTREAM_TAG = 'v1.0.0' +FEDORA_BRANCH = 'fedora-1.0' + +SPEC_TEMPLATE = """\ +%global upstream_version 1.0.0 +Name: python1.0 +Version: 1.0.0 +Release: %autorelease +Summary: ... + +License: MIT + +Source0: ... + +# (Patches taken from github.com/fedora-python/cpython) +{existing}# (New patches go here ^^^) + +%description +... + +%prep +%autosetup -S git_am + +%changelog +%autochangelog +""" + + +@pytest.fixture(scope='session') +def env(): + e = os.environ.copy() + e.update( + GIT_AUTHOR_NAME='Test', GIT_AUTHOR_EMAIL='t@example.com', + GIT_COMMITTER_NAME='Test', GIT_COMMITTER_EMAIL='t@example.com', + LANG='C.utf-8', + ) + return e + + +def git(args, cwd, env): + return subprocess.run( + ['git', *args], cwd=cwd, env=env, + check=True, capture_output=True, text=True, + ) + + +@pytest.fixture(scope='session') +def upstream_template(tmp_path_factory, env): + """Build the pristine scratch upstream repo once per test session.""" + path = tmp_path_factory.mktemp('upstream-template') + git(['init', '-q'], path, env) + (path / 'main.py').write_text("print('hello')\n") + git(['add', 'main.py'], path, env) + git(['commit', '-q', '-m', 'Initial upstream commit'], path, env) + git(['tag', UPSTREAM_TAG], path, env) + git(['branch', FEDORA_BRANCH, UPSTREAM_TAG], path, env) + return path + + +@pytest.fixture(scope='session') +def distgit_template(tmp_path_factory, env): + """Build the pristine scratch dist-git repo once per test session. + + Deliberately does not set importpatches.upstream here: every test's + `distgit` fixture copies this template and points the config at its own + freshly-copied `upstream`, so setting it here would just be overwritten. + """ + path = tmp_path_factory.mktemp('distgit-template') + write_spec(path, existing='') + git(['init', '-q'], path, env) + git(['add', '-A'], path, env) + git(['commit', '-q', '-m', 'init'], path, env) + return path + + +@pytest.fixture +def upstream(tmp_path, upstream_template): + """A scratch fedora-python/cpython-like repo, tagged v1.0.0, with an + (empty) fedora-1.0 branch ready for patches.""" + path = tmp_path / 'upstream' + shutil.copytree(upstream_template, path) + return path + + +@pytest.fixture +def distgit(tmp_path, distgit_template, upstream, env): + """A scratch dist-git checkout, pointing importpatches.upstream at + `upstream`, with an empty patches section.""" + path = tmp_path / 'distgit' + shutil.copytree(distgit_template, path) + git(['config', 'importpatches.upstream', str(upstream)], path, env) + return path + + +def write_spec(distgit, existing): + (distgit / SPEC_FILENAME).write_text(SPEC_TEMPLATE.format(existing=existing)) + + +def commit_spec(distgit, env, message='update patches'): + git(['add', '-A'], distgit, env) + git(['commit', '-q', '-m', message], distgit, env) + + +def add_commits(upstream, env, branch, messages): + """Append one commit per message onto `branch`, each editing main.py.""" + git(['switch', '-q', branch], upstream, env) + main_py = upstream / 'main.py' + for i, message in enumerate(messages): + with main_py.open('a') as f: + f.write(f'line{i}\n') + git(['commit', '-q', '-am', message], upstream, env) + + +def build_patch_files(upstream, distgit, env, entries): + """entries: list of (commit message, target filename in distgit). + + Creates one commit per entry (each appending a line to main.py) on a + throwaway 'scratch' branch off v1.0.0, and writes each commit's + `git format-patch` output into distgit/filename, ready for `git am`. + """ + git(['switch', '-q', '-c', 'scratch', UPSTREAM_TAG], upstream, env) + main_py = upstream / 'main.py' + for i, (message, filename) in enumerate(entries): + with main_py.open('a') as f: + f.write(f'line{i}\n') + git(['commit', '-q', '-am', message], upstream, env) + commit = git(['rev-parse', 'HEAD'], upstream, env).stdout.strip() + patch_text = subprocess.run( + ['git', 'format-patch', '--stdout', '-1', '--minimal', '--patience', + '--zero-commit', '--no-signature', '--keep-subject', commit], + cwd=upstream, env=env, check=True, capture_output=True, text=True, + ).stdout + (distgit / filename).write_text(patch_text) + + +def run_importpatches(distgit, upstream, env, *extra_args): + args = [ + '--repo', str(upstream), + '--base', UPSTREAM_TAG, + '--head', FEDORA_BRANCH, + '--python-version', '1.0', + *extra_args, + str(distgit / SPEC_FILENAME), + ] + return subprocess.run( + [sys.executable, str(IMPORTPATCHES), *args], + cwd=distgit, env=env, capture_output=True, text=True, + stdin=subprocess.DEVNULL, + ) + + +def run_exportpatches(distgit, upstream, env, *extra_args): + args = [ + '--repo', str(upstream), + '--base', UPSTREAM_TAG, + '--branch', FEDORA_BRANCH, + '--python-version', '1.0', + '--release', '1', + '--no-push', + *extra_args, + str(distgit / SPEC_FILENAME), + ] + return subprocess.run( + [sys.executable, str(EXPORTPATCHES), *args], + cwd=distgit, env=env, capture_output=True, text=True, + stdin=subprocess.DEVNULL, + ) + + +def commit_subjects(upstream, env, branch=FEDORA_BRANCH, base=UPSTREAM_TAG): + result = git(['log', '--format=%s', branch, '^' + base], upstream, env) + return result.stdout.splitlines() + + +# --- importpatches --- + +def test_import_numberless_creates_bare_patch_declarations(distgit, upstream, env): + add_commits(upstream, env, FEDORA_BRANCH, [ + 'Add a numberless feature flag', + 'Fix a numberless bug', + ]) + + result = run_importpatches(distgit, upstream, env) + + assert result.returncode == 0, result.stderr + spec = (distgit / SPEC_FILENAME).read_text() + assert 'Patch: add-a-numberless-feature-flag.patch' in spec + assert 'Patch: fix-a-numberless-bug.patch' in spec + assert 'Patch1' not in spec and 'Patch2' not in spec + assert (distgit / 'add-a-numberless-feature-flag.patch').exists() + assert (distgit / 'fix-a-numberless-bug.patch').exists() + + +def test_import_numberless_removes_stale_numbered_patch(distgit, upstream, env): + write_spec(distgit, existing=( + '# 00099 # deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n' + '# Some old numbered patch\n' + 'Patch99: 00099-some-old-numbered-patch.patch\n' + )) + (distgit / '00099-some-old-numbered-patch.patch').touch() + commit_spec(distgit, env, 'seed a stale numbered patch') + add_commits(upstream, env, FEDORA_BRANCH, ['Add a numberless feature flag']) + + result = run_importpatches(distgit, upstream, env) + + assert result.returncode == 0, result.stderr + spec = (distgit / SPEC_FILENAME).read_text() + assert 'Patch99' not in spec + assert 'Patch: add-a-numberless-feature-flag.patch' in spec + assert not (distgit / '00099-some-old-numbered-patch.patch').exists() + + +def test_import_rhel8_compat_sequential_numbers(distgit, upstream, env): + add_commits(upstream, env, FEDORA_BRANCH, [ + 'Add a numberless feature flag', + 'Fix a numberless bug', + ]) + + result = run_importpatches(distgit, upstream, env, '--rhel8-compat') + + assert result.returncode == 0, result.stderr + spec = (distgit / SPEC_FILENAME).read_text() + assert 'Patch1: add-a-numberless-feature-flag.patch' in spec + assert 'Patch2: fix-a-numberless-bug.patch' in spec + # RHEL 8 compat numbers must not leak into filenames or comments + assert not (distgit / '00001-add-a-numberless-feature-flag.patch').exists() + assert '# 00001' not in spec and '# 00002' not in spec + + +def test_import_mixture_of_numbered_and_numberless_errors(distgit, upstream, env): + original_spec = (distgit / SPEC_FILENAME).read_text() + add_commits(upstream, env, FEDORA_BRANCH, [ + '00001: A numbered patch', + 'A numberless patch', + ]) + + result = run_importpatches(distgit, upstream, env) + + assert result.returncode != 0 + assert 'mix' in (result.stdout + result.stderr).lower() + # nothing should have been written before mode detection errored out + assert (distgit / SPEC_FILENAME).read_text() == original_spec + assert not list(distgit.glob('*.patch')) + + +def test_import_numbered_mode_regression(distgit, upstream, env): + add_commits(upstream, env, FEDORA_BRANCH, ['00001: A numbered patch']) + + result = run_importpatches(distgit, upstream, env) + + assert result.returncode == 0, result.stderr + spec = (distgit / SPEC_FILENAME).read_text() + assert 'Patch1: 00001-a-numbered-patch.patch' in spec + assert (distgit / '00001-a-numbered-patch.patch').exists() + + +def test_import_numberless_filename_collision_errors(distgit, upstream, env): + original_spec = (distgit / SPEC_FILENAME).read_text() + add_commits(upstream, env, FEDORA_BRANCH, [ + 'Fix build issue', + 'Fix build issue', + ]) + + result = run_importpatches(distgit, upstream, env) + + assert result.returncode != 0 + assert 'fix-build-issue.patch' in (result.stdout + result.stderr) + assert (distgit / SPEC_FILENAME).read_text() == original_spec + assert not list(distgit.glob('*.patch')) + + +def test_import_numberless_patch_ending_summary_does_not_crash(distgit, upstream, env): + # A numberless commit summary that happens to end in '.patch' but isn't + # filename-safe (contains spaces) must not be confused with the + # "old and dirty" dirty-style convention. + add_commits(upstream, env, FEDORA_BRANCH, ['Update foo bar.patch']) + + result = run_importpatches(distgit, upstream, env) + + assert result.returncode == 0, result.stderr + spec = (distgit / SPEC_FILENAME).read_text() + assert 'Patch: update-foo-bar-patch.patch' in spec + assert 'Update foo bar.patch' in spec + assert (distgit / 'update-foo-bar-patch.patch').exists() + + +# --- exportpatches --- + +def test_export_numbered_mode_inserts_number_for_trailing_unnumbered_patch( + distgit, upstream, env, +): + build_patch_files(upstream, distgit, env, [ + ('00001: Existing numbered patch', '00001-existing-numbered.patch'), + ('Add a new thing', 'something.patch'), + ]) + write_spec(distgit, existing=( + 'Patch1: 00001-existing-numbered.patch\n' + 'Patch666: something.patch\n' + )) + commit_spec(distgit, env) + + result = run_exportpatches(distgit, upstream, env, '--tag', 'test-numbered-1') + + assert result.returncode == 0, result.stderr + assert commit_subjects(upstream, env) == [ + '00666: Add a new thing', + '00001: Existing numbered patch', + ] + + +def test_export_numberless_bare_never_inserts_number(distgit, upstream, env): + build_patch_files(upstream, distgit, env, [ + ('Add a numberless feature flag', 'add-a-numberless-feature-flag.patch'), + ('Fix a numberless bug', 'fix-a-numberless-bug.patch'), + ]) + write_spec(distgit, existing=( + 'Patch: add-a-numberless-feature-flag.patch\n' + 'Patch: fix-a-numberless-bug.patch\n' + )) + commit_spec(distgit, env) + + result = run_exportpatches(distgit, upstream, env, '--tag', 'test-numberless-1') + + assert result.returncode == 0, result.stderr + assert commit_subjects(upstream, env) == [ + 'Fix a numberless bug', + 'Add a numberless feature flag', + ] + + +def test_export_numberless_rhel8_compat_never_inserts_number(distgit, upstream, env): + build_patch_files(upstream, distgit, env, [ + ('Add a numberless feature flag', 'add-a-numberless-feature-flag.patch'), + ('Fix a numberless bug', 'fix-a-numberless-bug.patch'), + ]) + write_spec(distgit, existing=( + 'Patch1: add-a-numberless-feature-flag.patch\n' + 'Patch2: fix-a-numberless-bug.patch\n' + )) + commit_spec(distgit, env) + + result = run_exportpatches(distgit, upstream, env, '--tag', 'test-numberless-2') + + assert result.returncode == 0, result.stderr + assert commit_subjects(upstream, env) == [ + 'Fix a numberless bug', + 'Add a numberless feature flag', + ] + + +def test_export_numberless_rhel8_compat_numbers_out_of_order(distgit, upstream, env): + # RHEL 8 compat numbers have no semantic meaning, so they need not + # ascend in spec order; application order still follows spec order. + build_patch_files(upstream, distgit, env, [ + ('Add a numberless feature flag', 'add-a-numberless-feature-flag.patch'), + ('Fix a numberless bug', 'fix-a-numberless-bug.patch'), + ]) + write_spec(distgit, existing=( + 'Patch2: add-a-numberless-feature-flag.patch\n' + 'Patch1: fix-a-numberless-bug.patch\n' + )) + commit_spec(distgit, env) + + result = run_exportpatches(distgit, upstream, env, '--tag', 'test-numberless-3') + + assert result.returncode == 0, result.stderr + assert commit_subjects(upstream, env) == [ + 'Fix a numberless bug', + 'Add a numberless feature flag', + ] + + +def test_export_duplicate_patch_numbers_errors(distgit, upstream, env): + write_spec(distgit, existing=( + 'Patch5: foo.patch\n' + 'Patch5: bar.patch\n' + )) + commit_spec(distgit, env) + + result = run_exportpatches(distgit, upstream, env, '--tag', 'test-duplicate') + + assert result.returncode != 0 + assert 'duplicate' in (result.stdout + result.stderr).lower() + assert commit_subjects(upstream, env) == [] + + +def test_export_no_push_skips_push(distgit, upstream, env): + build_patch_files(upstream, distgit, env, [ + ('00001: A numbered patch', '00001-a-numbered-patch.patch'), + ]) + write_spec(distgit, existing='Patch1: 00001-a-numbered-patch.patch\n') + commit_spec(distgit, env) + + # No 'fedora-python' remote exists in the scratch upstream repo, so a + # real push attempt would crash the script (unhandled CalledProcessError); + # exiting 0 here is itself proof no push was attempted. + result = run_exportpatches(distgit, upstream, env, '--tag', 'test-no-push') + + assert result.returncode == 0, result.stderr + assert 'skipping push' in result.stdout.lower() + assert 'Traceback' not in result.stderr + tags = git(['tag', '--list', 'test-no-push'], upstream, env).stdout + assert 'test-no-push' in tags + + +def test_export_no_push_skips_existing_tag_instead_of_moving_it(distgit, upstream, env): + build_patch_files(upstream, distgit, env, [ + ('Add a numberless feature flag', 'add-a-numberless-feature-flag.patch'), + ('Fix a numberless bug', 'fix-a-numberless-bug.patch'), + ]) + + write_spec(distgit, existing='Patch: add-a-numberless-feature-flag.patch\n') + commit_spec(distgit, env, 'one patch') + result1 = run_exportpatches(distgit, upstream, env, '--tag', 'test-no-move') + assert result1.returncode == 0, result1.stderr + tag_commit_1 = git(['rev-list', '-n', '1', 'test-no-move'], upstream, env).stdout.strip() + + write_spec(distgit, existing=( + 'Patch: add-a-numberless-feature-flag.patch\n' + 'Patch: fix-a-numberless-bug.patch\n' + )) + commit_spec(distgit, env, 'two patches') + result2 = run_exportpatches(distgit, upstream, env, '--tag', 'test-no-move') + + assert result2.returncode == 0, result2.stderr + assert 'skipping tag creation' in result2.stdout.lower() + tag_commit_2 = git(['rev-list', '-n', '1', 'test-no-move'], upstream, env).stdout.strip() + assert tag_commit_2 == tag_commit_1 + # the branch itself did move forward to include the second patch + assert commit_subjects(upstream, env) == [ + 'Fix a numberless bug', + 'Add a numberless feature flag', + ]