#292 Add revert subcommand
Closed 3 months ago by nphilipp. Opened a year ago by oturpe.
fedora-infra/ oturpe/rpmautospec revert  into  main

file modified
+2 -2
@@ -5,10 +5,10 @@ 

  import sys

  import typing

  

- from .subcommands import changelog, convert, process_distgit, release

+ from .subcommands import changelog, convert, process_distgit, release, revert

  

  

- all_subcmds = (changelog, convert, process_distgit, release)

+ all_subcmds = (changelog, convert, process_distgit, release, revert)

  subcmd_modules_by_name = {}

  

  

file modified
+59
@@ -1,3 +1,5 @@ 

+ import logging

+ import pygit2

  import re

  from collections import namedtuple

  from functools import lru_cache
@@ -8,7 +10,13 @@ 

  # the %autorelease macro including parameters

  AUTORELEASE_MACRO = "autorelease(e:s:pb:n)"

  

+ 

+ log = logging.getLogger(__name__)

+ 

+ 

+ release_re = re.compile(r"^(?P<tag>(?i:Release)\s*:\s*)")

  autorelease_re = re.compile(r"%(?:autorelease(?:\s|$)|\{\??autorelease(?:\s+[^\}]*)?\})")

+ release_autorelease_re = re.compile(release_re.pattern + autorelease_re.pattern, re.MULTILINE)

  changelog_re = re.compile(r"^%changelog(?:\s.*)?$", re.IGNORECASE)

  autochangelog_re = re.compile(r"\s*%(?:autochangelog|\{\??autochangelog\})\s*")

  
@@ -24,6 +32,11 @@ 

      pass

  

  

+ class PartialRpmautospecError(OSError):

+     "Specfile does not have both %rpmautorelease and %rpmautospec"

+     pass

+ 

+ 

  @lru_cache(maxsize=None)

  def check_specfile_features(specpath: Union[Path, str]) -> SpecfileFeatures:

      if not isinstance(specpath, Path):
@@ -79,3 +92,49 @@ 

          retval = retval or features.has_autochangelog

  

      return retval

+ 

+ 

+ def find_repo(path):

+     try:

+         if hasattr(pygit2, "GIT_REPOSITORY_OPEN_NO_SEARCH"):

+             kwargs = {"flags": pygit2.GIT_REPOSITORY_OPEN_NO_SEARCH}

+         else:

+             # pygit2 < 1.4.0

+             kwargs = {}

+         log.debug("Found repository at %s", path)

+         return pygit2.Repository(path, **kwargs)

+     except pygit2.GitError:

+         log.debug("Found no repository at %s", path)

+         return None

+ 

+ 

+ def assert_file_not_modified(repo, file):

+     """Raise an exception if given file is modified in the repository.

+     Files added to repository index are not considered modified.

+     Do nothing if the file is not modified."""

+ 

+     # Remove trailing "/.git/"

+     path = repo.path.rsplit("/", 2)[0]

+ 

+     is_tracked = True

+     try:

+         status = repo.status_file(file.relative_to(path))

+     except KeyError:

+         is_tracked = False

+ 

+     exists = file.exists()

+ 

+     if exists and is_tracked:

+         # Allow converting unmodified, and saved-to-index files.

+         if status not in (pygit2.GIT_STATUS_CURRENT, pygit2.GIT_STATUS_INDEX_MODIFIED):

+             raise FileIsModifiedError(f"File {str(file)!r} is modified")

+     if exists and not is_tracked:

+         raise FileExistsError(

+             f"File {str(file)!r} found in the working directory, but is not tracked"

+         )

+     if not exists and is_tracked:

+         raise FileIsModifiedError(

+             f"File {str(file)!r} is not found in the working directory, but is tracked"

+         )

+     if not exists and not is_tracked:

+         pass

@@ -1,18 +1,23 @@ 

  import logging

- import re

  from pathlib import Path

  from shutil import SpecialFileError

  from typing import List, Optional

  

  import pygit2

  

- from ..misc import autochangelog_re, autorelease_re, changelog_re, FileIsModifiedError

+ from ..misc import (

+     assert_file_not_modified,

+     autochangelog_re,

+     autorelease_re,

+     changelog_re,

+     find_repo,

+     release_re,

+     FileIsModifiedError,

+ )

  

  

  log = logging.getLogger(__name__)

  

- release_re = re.compile(r"^(?P<tag>(?i:Release)\s*:\s*)")

- 

  

  class PkgConverter:

      def __init__(self, spec_or_path: Path):
@@ -41,28 +46,9 @@ 

  

          log.debug("Working on spec file %s", self.specfile)

  

-         try:

-             if hasattr(pygit2, "GIT_REPOSITORY_OPEN_NO_SEARCH"):

-                 kwargs = {"flags": pygit2.GIT_REPOSITORY_OPEN_NO_SEARCH}

-             else:

-                 # pygit2 < 1.4.0

-                 kwargs = {}

-             self.repo = pygit2.Repository(self.path, **kwargs)

-             log.debug("Found repository at %s", self.path)

-         except pygit2.GitError:

-             self.repo = None

-             log.debug("Found no repository at %s", self.path)

- 

+         self.repo = find_repo(self.path)

          if self.repo is not None:

-             try:

-                 spec_status = self.repo.status_file(self.specfile.relative_to(self.path))

-             except KeyError:

-                 raise FileExistsError(

-                     f"Spec file {str(self.specfile)!r} is in a repository, but isn't tracked"

-                 )

-             # Allow converting unmodified, and saved-to-index files.

-             if spec_status not in (pygit2.GIT_STATUS_CURRENT, pygit2.GIT_STATUS_INDEX_MODIFIED):

-                 raise FileIsModifiedError(f"Spec file {str(self.specfile)!r} is modified")

+             assert_file_not_modified(self.repo, self.specfile)

              try:

                  self.repo.status_file("changelog")

              except KeyError:

@@ -0,0 +1,82 @@ 

+ import argparse

+ import fileinput

+ import logging

+ import sys

+ from contextlib import suppress

+ from pathlib import Path

+ 

+ from ..misc import (

+     assert_file_not_modified,

+     autochangelog_re,

+     check_specfile_features,

+     find_repo,

+     release_autorelease_re,

+     PartialRpmautospecError,

+ )

+ from ..pkg_history import PkgHistoryProcessor

+ from .changelog import collate_changelog

+ 

+ log = logging.getLogger(__name__)

+ 

+ 

+ def register_subcommand(subparsers: argparse._SubParsersAction) -> str:

+     subcmd_name = "revert"

+     revert_parser = subparsers.add_parser(

+         subcmd_name,

+         help="Convert an rpmautospec enabled specfile to use a static Release: and %%changelog",

+     )

+     revert_parser.add_argument(

+         "spec_or_path",

+         default=Path("."),

+         type=Path,

+         nargs="?",

+         help="Path to package worktree or the spec file within",

+     )

+     return subcmd_name

+ 

+ 

+ def replace(

+     file: Path,

+     release: str,

+     changelog: str,

+ ) -> Path:

+     with fileinput.input(file, inplace=True, backup=".rpmautospec") as fi:

+         for index, line in enumerate(fi):

+             match = release_autorelease_re.match(line)

+             if match:

+                 log.info("%s:%s: replacing Release", file, index + 1)

+                 line = match.group(1) + release + "\n"

+             if autochangelog_re.match(line):

+                 log.info("%s:%s: replacing %%autochangelog", file, index + 1)

+                 sys.stdout.write(changelog)

+                 break

+             sys.stdout.write(line)

+     return Path(str(file) + ".rpmautospec")

+ 

+ 

+ def main(params: argparse.Namespace) -> None:

+     processor = PkgHistoryProcessor(params.spec_or_path)

+ 

+     features = check_specfile_features(processor.specfile)

+     if not features.has_autorelease and not features.has_autochangelog:

+         log.info(f"{str(processor.specfile)!r} does not use rpmautospec. Nothing to do.")

+         return

+     elif not features.has_autorelease or not features.has_autochangelog:

+         raise PartialRpmautospecError(

+             f"{str(processor.specfile)!r} does not have both %autorelease and %autochangelog."

+         )

+ 

+     repo = find_repo(processor.path)

+     if repo is not None:

+         assert_file_not_modified(repo, processor.specfile)

+         changelog = processor.specfile.parent / "changelog"

+         assert_file_not_modified(repo, changelog)

+ 

+     visitors = [processor.release_number_visitor, processor.changelog_visitor]

+     result = processor.run(visitors=visitors)

+     release = result["release-complete"] + "%{?dist}"

+     changelog = collate_changelog(result)

+     bakfile = replace(processor.specfile, release, changelog)

+     with suppress(OSError):

+         processor.path.joinpath("changelog").unlink()

+         bakfile.unlink()

@@ -3,7 +3,6 @@ 

  """

  

  import logging

- import re

  from types import SimpleNamespace

  from unittest import mock

  
@@ -11,10 +10,11 @@ 

  import pytest

  

  from rpmautospec.subcommands import convert

- from rpmautospec.misc import autorelease_re, autochangelog_re, FileIsModifiedError

- 

- release_autorelease_re = re.compile(

-     convert.release_re.pattern + autorelease_re.pattern, re.MULTILINE

+ from rpmautospec.misc import (

+     autorelease_re,

+     autochangelog_re,

+     release_autorelease_re,

+     FileIsModifiedError,

  )

  

  

@@ -0,0 +1,216 @@ 

+ """

+ Test the rpmautospec.subcommands.revert module

+ """

+ import pytest

+ import re

+ 

+ from types import SimpleNamespace

+ 

+ from rpmautospec.subcommands import revert

+ from rpmautospec.misc import (

+     autochangelog_re,

+     release_autorelease_re,

+     FileIsModifiedError,

+     PartialRpmautospecError,

+ )

+ 

+ base_release_re = re.compile(r"(?i:Release)\s*:\s*(.+)%{\?dist}")

+ 

+ # Expected tail of %changelog, and thus the whole specfile,

+ # when the test specfile is reverted outside of Git repository.

+ # The changelog entry contains a changing date,

+ # so only the fixed tail is recorded here.

+ norepo_changelog_text = """John Doe <packager@example.com> - 1.0-1

+ - Uncommitted changes"""

+ 

+ repo_changelog_text = """* Wed Feb 22 2023 John Doe <packager@example.com> - 1.1-1

+ - Version 1.1 from changelog

+ 

+ * Tue Feb 21 2023 John Doe <packager@example.com> - 1.0-1

+ - Version 1.0 from changelog"""

+ 

+ 

+ def assert_no_rpmautospec(specfile):

+     """Assert that rpmautospec is not used in given specfile."""

+     assert not release_autorelease_re.search(specfile.read_text())

+     assert not autochangelog_re.search(specfile.read_text())

+ 

+ 

+ def assert_release_number(specfile, expected_base_release):

+     """Assert that specfile has the expected release number."""

+     match = base_release_re.search(specfile.read_text())

+     assert match is not None

+     assert match.group(1) == expected_base_release

+ 

+ 

+ def test_revert_simple(specfile, repo):

+     args = SimpleNamespace(spec_or_path=specfile)

+     revert.main(args)

+ 

+     assert_no_rpmautospec(specfile)

+     assert_release_number(specfile, "2")

+     assert specfile.read_text().endswith(

+         """Jane Doe <jane.doe@example.com> - 1.0-1

+ - Initial commit"""

+     )

+ 

+ 

+ def test_revert_changelog_at_head(specfile, repo):

+     # The changelog file has already been added:

+     changelog = specfile.parent / "changelog"

+     changelog.write_text(repo_changelog_text)

+     repo.index.add("changelog")

+     repo.index.write()

+     repo.create_commit(

+         repo.head.name,

+         repo.default_signature,

+         repo.default_signature,

+         "Add changelog",

+         repo.index.write_tree(),

+         [repo.head.target],

+     )

+ 

+     args = SimpleNamespace(spec_or_path=specfile)

+     revert.main(args)

+     print(specfile.read_text())

+ 

+     assert_no_rpmautospec(specfile)

+     assert base_release_re.search(specfile.read_text()).group(1) == "3"

+     assert specfile.read_text().endswith(repo_changelog_text)

+     assert not changelog.exists()

+ 

+ 

+ def test_revert_changelog_at_old_commit(specfile, repo):

+     changelog = specfile.parent / "changelog"

+     changelog.write_text(repo_changelog_text)

+     repo.index.add("changelog")

+     repo.index.write()

+     repo.create_commit(

+         repo.head.name,

+         repo.default_signature,

+         repo.default_signature,

+         "Add changelog",

+         repo.index.write_tree(),

+         [repo.head.target],

+     )

+     repo.create_commit(

+         repo.head.name,

+         repo.default_signature,

+         repo.default_signature,

+         "Next change",

+         repo.index.write_tree(),

+         [repo.head.target],

+     )

+ 

+     args = SimpleNamespace(spec_or_path=specfile)

+     revert.main(args)

+     print(specfile.read_text())

+ 

+     assert_no_rpmautospec(specfile)

+     assert base_release_re.search(specfile.read_text()).group(1) == "4"

+     expected_latest_entry = """Jane Doe <jane.doe@example.com> - 1.0-4

+ - Next change"""

+     assert specfile.read_text().endswith("%s\n\n%s" % (expected_latest_entry, repo_changelog_text))

+     assert not changelog.exists()

+ 

+ 

+ @pytest.mark.parametrize("changelog", [repo_changelog_text])

+ @pytest.mark.parametrize("release", ["Release: 7%{?dist}"])

+ def test_nothing_to_do(specfile, repo):

+     args = SimpleNamespace(spec_or_path=specfile)

+     revert.main(args)

+ 

+     assert_no_rpmautospec(specfile)

+     assert_release_number(specfile, "7")

+     assert specfile.read_text().endswith("%s\n" % repo_changelog_text)

+ 

+ 

+ def test_norepo(specfile):

+     args = SimpleNamespace(spec_or_path=specfile)

+     revert.main(args)

+ 

+     assert_no_rpmautospec(specfile)

+     assert_release_number(specfile, "1")

+     assert specfile.read_text().endswith(norepo_changelog_text)

+ 

+ 

+ @pytest.mark.parametrize("changelog", [repo_changelog_text])

+ def test_error_autorelease_only(specfile, repo):

+     args = SimpleNamespace(spec_or_path=specfile)

+     with pytest.raises(

+         PartialRpmautospecError, match="does not have both %autorelease and %autochangelog"

+     ):

+         revert.main(args)

+ 

+ 

+ @pytest.mark.parametrize("release", ["Release: 7%{?dist}"])

+ def test_error_autochangelog_only(specfile, repo):

+     args = SimpleNamespace(spec_or_path=specfile)

+     with pytest.raises(

+         PartialRpmautospecError, match="does not have both %autorelease and %autochangelog"

+     ):

+         revert.main(args)

+ 

+ 

+ def test_error_specfile_modified(specfile, repo):

+     # The spec file has been modified without committing it:

+     with specfile.open("a") as f:

+         print("Modified", file=f)

+     args = SimpleNamespace(spec_or_path=specfile)

+     with pytest.raises(FileIsModifiedError, match="is modified"):

+         revert.main(args)

+ 

+ 

+ def test_error_specfile_deleted(specfile, repo):

+     # The spec file has been deleted:

+     specfile.unlink()

+     args = SimpleNamespace(spec_or_path=specfile)

+     with pytest.raises(FileNotFoundError, match="doesn't exist"):

+         revert.main(args)

+ 

+ 

+ def test_error_changelog_modified(specfile, repo):

+     # The changelog file has already been added:

+     changelog = specfile.parent / "changelog"

+     changelog.write_text(repo_changelog_text)

+     repo.index.add("changelog")

+     repo.index.write()

+     repo.create_commit(

+         repo.head.name,

+         repo.default_signature,

+         repo.default_signature,

+         "Add changelog",

+         repo.index.write_tree(),

+         [repo.head.target],

+     )

+ 

+     # The changelog has been modified without committing it:

+     with changelog.open("a") as f:

+         print("- Another change", file=f)

+     args = SimpleNamespace(spec_or_path=specfile)

+     with pytest.raises(FileIsModifiedError, match="is modified"):

+         revert.main(args)

+ 

+ 

+ def test_error_changelog_deleted(specfile, repo):

+     # The changelog file has already been added:

+     changelog = specfile.parent / "changelog"

+     changelog.write_text(repo_changelog_text)

+     repo.index.add("changelog")

+     repo.index.write()

+     repo.create_commit(

+         repo.head.name,

+         repo.default_signature,

+         repo.default_signature,

+         "Add changelog",

+         repo.index.write_tree(),

+         [repo.head.target],

+     )

+ 

+     # The changelog has been deleted without committing:

+     changelog.unlink()

+     args = SimpleNamespace(spec_or_path=specfile)

+     with pytest.raises(

+         FileIsModifiedError, match="is not found in the working directory, but is tracked"

+     ):

+         revert.main(args)

Use of rpmautospec is not mandatory in Fedora, even though with
Rpmautospec by Default it is recommended 1. In case a package
using rpmautospec is transferred to a maintainer who does not want
to use it, there is a need to revert the package back to
traditional release and changelog management. The new revert
subcommand does that.

Only simple specfiles that look like this are handled:

Release:    %autorelease

%changelog
%autochangelog

If the changelog file exists, it is deleted and its contents are
moved to specfile %changelog as expected.

The revert subcommand performs similar checks for local
modifications as convert subcommand. An error is if there are local
changes on top of repository contents.

Relates: https://pagure.io/fesco/issue/2930
Relates: https://github.com/fedora-eln/distrobaker/issues/12
Signed-off-by: Otto Liljalaakso otto.liljalaakso@iki.fi
Co-authored-by: Maxwell G gotmax@e.email

This revives #287, the same code with minor changes and tests added.
Only success cases are tested for now.
I try to add some tests for different failures as well soon.

rebased onto 474fa9ffce075808122819761daa2b2386b54968

a year ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/2e9fe8cebb964a04ba2a5da5b06a4a94

So a big question for me is whether an automatic commit should be done. I think that in most cases we'd want this, but it's probably better to not do this in this PR. Let's get the basic functionality merged, which is already quite useful, and then we can discuss autocommit separately.

 +     if not features.has_autorelease and not features.has_autochangelog:
+         log.info(f"{str(processor.specfile)!r} does not use rpmautospec. Nothing to do.")
+         return 

I'm pretty sure this needs to fail with an error. People might use this in scripting and such, and it's not enough to emit a message.

I tried this on a repo that had a changelog file, and while .spec was updated as expected, the changelog file was not touched. It should be removed, so that if the user does git commit -a, the stray file is not left behind.

I tested this with a few repos, and it seems to work as expected. Thanks for working on this.

So a big question for me is whether an automatic commit should be done. I think that in most cases we'd want this, but it's probably better to not do this in this PR. Let's get the basic functionality merged, which is already quite useful, and then we can discuss autocommit separately.

I have no strong opinions here, I can leave the commit part out initially if so desired.
It also occurs to me that the correct choice forrevert here is probably the same as for convert.
Their use cases are comparable:
One is for a maintainer who wants to use rpmautospec and adopts a package that is not using it,
the other for a maintainer who does not want to use rpmautospec and adopts a package that is using it.

+ if not features.has_autorelease and not features.has_autochangelog: + log.info(f"{str(processor.specfile)!r} does not use rpmautospec. Nothing to do.") + return
I'm pretty sure this needs to fail with an error. People might use this in scripting and such, and it's not enough to emit a message.

Makes sense to me, however again the situation here is comparable to convert.
Currently, it does not bother with errors on nothing-to-do (and the output does not look right):

$ rpmautospec convert
'/home/otto/src/Jakelut/Fedora/src.fedoraproject.org/rpms/iir1/iir1.spec' is already using %autochangelog
'/home/otto/src/Jakelut/Fedora/src.fedoraproject.org/rpms/iir1/iir1.spec' is already using %autorelease
Nothing to commit
Converted to .
$ echo $?
0

I tried this on a repo that had a changelog file, and while .spec was updated as expected, the changelog file was not touched. It should be removed, so that if the user does git commit -a, the stray file is not left behind.

Good point, I'll do something about that.

I tested this with a few repos, and it seems to work as expected. Thanks for working on this.

Thanks for testing!

rebased onto 5e80cef74b1ceba128cbc6c150a9d55e41d82dad

a year ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/407491e9ef3545bcbdce26173513b8b4

2 new commits added

  • Add revert subcommand
  • Move some regexs to misc.py
a year ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/3f22bfa03a8b46ae905769490e7f3f12

Here is the next revision.
Changes:

  • changelog was left behind in your test, because the script defaulted to backup.
    Backup included not deleting the inlined changelog.
    Since rpmautospec is mainly intended to be run inside a Git repository,
    separate backup is not needed,
    and also other subcommands do not have such feature.
    So I just deleted the whole backup thing.

  • The case where specfile only had %autochangelog but not %autorelease crashed somewhere deep inside PkgHistoryProcessor.
    Instead of trying to somehow make it work,
    I just added checking that both are present before starting.
    A specfile that only have one are most probably corrupted,
    so trying to process it would be a garbage-in-garbage-out situation anyhow.

  • I think enough test cases.

Still to do or to think about:

  • To commit or not to commit?
  • Is "nothing to do" an error?
  • convert refuses to start if the repository is dirty. Should revert be the same?
  • Figure out why Zuul is failing — though the error does not look like it was caused by these changes.

To commit or not to commit?

As you wrote, we're generally always inside of a repo. I think it doesn't make much sense to not commit if we're inside of a repo. But as I wrote above, I think the feature is useful even without that, so I recommend leaving that part for later.

Is "nothing to do" an error?

Generally, yes, this is how UNIX commands work. You specified that something should happen, but it can't, so you get an error. Obviously there are exceptions, some commands quietly do nothing if the operation is already done, but that mostly applies to idempotent commands that would reasonably be executed multiple times. I don't think revert is like that.

convert refuses to start if the repository is dirty. Should revert be the same?

If you're running inside of a git repo, and you have some modified state, it's likely that the modification will interfere with the state. If you really want to mix the two types of changes, then git stash && rpmautospec revert && git stash pop is good enough of a workaround. I also think that consistency in the CLI is very important for user happiness. Both commands should exhibit the same safety checks as far as reasonable.

Figure out why Zuul is failing — though the error does not look like it was caused by these changes.

Yeah, it's failing in the same way in other places. I filed https://pagure.io/fedora-infra/rpmautospec/pull-request/294 to help diagnose those failures.

3 new commits added

  • Add revert subcommand
  • Move some utilities from subcommands/convert.py to misc.py
  • Move some regexs to misc.py
a year ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/a956298615114b82bd465cc9614cf77b

3 new commits added

  • Add revert subcommand
  • Move some utilities from subcommands/convert.py to misc.py
  • Move some regexs to misc.py
a year ago

Thank you for the feedback, once again.

I started implementing the dirty repository checks.
The latest push has incomplete work for that.

For the "nothing to do" error, maybe a separate issue should be created for it,
so that it can be fixed (or rejected by the maintainers) for all subcommands at once.

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/ff9826b3e8604a0d8110999d19cc6a54

@oturpe You wrote "incomplete", so I didn't review. Please let me know when you want another round of reviews.

That is right, things are mostly ready, but unfortunately I have not had the time to do the finishing touches.
I will ping you when I get around to do them.

3 new commits added

  • Add revert subcommand
  • Move some utilities from subcommands/convert.py to misc.py
  • Move some regexs to misc.py
a year ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/b88cae73eb634bdc8b3f9885de5f864f

Ok, the latest batch is in.
I think this is has reached a point where it could be merged, please review!

To recap the earlier discussion, these two items seem like good ideas,
but have to been added yet (reasoning in parenthesis):

  • Committing changes after success (can be added later)
  • Returning with error in case there is nothing to revert (should be done for the other subcommands as well, at least convert has the same problem)

By the way, I found an interesting case regarding the latter point.
Consider a specfile with this (adapted from nodejs18):

%global baserelease %autorelease
Release:    %baserelease

Right now, revert does nothing to such specfile and returns success.
That is wrong, the specfile is still using rpmautospec.
So returning an error when nothing is done would be safer.
I am sure that many similar cases can constructed, and will also be seen in the wild.
I almost went on to fix this immediately,
but then decided to wait for a maintainer review before doing still more changes.

I would love to see this land, as I need this for being able to build packages properly from Fedora in the openSUSE Build Service (which has no access to Git period and does not work off the Git SCM directly like Koji does).

@ngompa it would be useful if you could try the pull request with some of your packages and check if this does what you need.

Even more useful would be to get a review from a maintainer :smile:

rebased onto 846a5ff

a year ago

Sorry for the late reply 🫣. As I moved the project repository to GitHub, so I’ll close this PR. Would you please resubmit it there if you’re still interested? Thanks!

Pull-Request has been closed by nphilipp

3 months ago