#37 Improve rpmdev-bumpspec -n NEW (#1545485)
Closed 5 years ago by ngompa. Opened 6 years ago by sergiomb.
sergiomb/rpmdevtools master  into  master

file modified
+8 -9
@@ -37,6 +37,7 @@ 

      def __init__(self, filename, verbose=False, string=None):

          self.verbose = verbose

          self.string = string

+         self.new_version = False

  

          self.filename = filename

          with open(filename) as f:
@@ -123,10 +124,8 @@ 

  

          rpos = vr.find('-')

          if rpos >= 0:  # set custom Release value

-             r = vr[rpos + 1:]

              v = vr[:rpos]

          else:

-             r = "1%{?dist}"

              v = vr

  

          changed = False
@@ -136,12 +135,6 @@ 

                  self.lines[i] = re.sub(

                      r'[^: \t]*$', v, self.lines[i].rstrip()) + '\n'

                  changed = changed or self.lines[i] != original

-             elif self.lines[i].lower().startswith('release:'):

-                 # split and reconstruct to preserve whitespace

-                 split = re.split(r':', self.lines[i].rstrip())

-                 self.lines[i] = split[0] + ':' + \

-                     re.sub(r'[^ \t]*$', r, split[1]) + '\n'

-                 changed = changed or self.lines[i] != original

  

          return changed

  
@@ -173,7 +166,11 @@ 

          relmatch = relre.search(release)

          if not relmatch:  # pattern match failed

              raise BumpSpecError

-         value = str(int(relmatch.group('rel')) + 1)

+         if self.new_version:

+             value = "1"

+             pre = False

+         else:

+             value = str(int(relmatch.group('rel')) + 1)

          post = relmatch.group('post')

  

          new = value + post
@@ -337,7 +334,9 @@ 

              # Not actually a parser error, but... meh.

              parser.error(e)

          if opts.new:

+             s.new_version = True

              changed = s.newVersion(opts.new)

+             s.bumpRelease()

          else:

              s.bumpRelease()

              changed = True

I'm not sure this is exactly the behavior that should exist.

What are you trying to accomplish?

sergio wants the tool to reset the most-significant number of the release tag to 1 while keeping anything at the right side of it. Doing that in a fully automated way would require a much better detection of macros and pre-/post-release versioning schemes than what the tool can handle so far. The offered patch is insufficient. Rationale has been given in the link bz ticket.

Yeah, this would completely break in a lot of cases. This is one of the reasons why I wish Fedora would adopt my proposal for moving snapshot versioning back to the Version field: https://fedoraproject.org/wiki/PackagingDrafts/TildeVersioning

I don't particularly want to make this logic even more complex than it already is.

when we have :

Version:    5.2.8
Release:    6%{?prerel:.%{prerel}}%{?dist}

rpmdev-bumpspec test.spec

Release:    7%{?prerel:.%{prerel}}%{?dist}

but rpmdev-bumpspec test.spec -n someversion, Release is cleaned :

Release:    1%{?dist}

I propose that rpmdev-bumpspec test.spec -n someversion keep the macros like without -n

Release:    1%{?prerel:.%{prerel}}

Sergio,

the tool can recognize the numeric left part of the Release tag and bump it correctly without touching whatever may be right of the most-significant release number. On the contrary, examining the entire Release tag as would be necessary to reset it to 1 correctly, regardless of what may be found to the right of the leading number, is non-trivial and may require more than regular expression based matching. Your patch doesn't do it either.

As a hack, one could cook something based on resetting everything left of the first '%' character in the release tag. It would cover many spec files, but not all.

It would also not work for Mageia, which does Release: %mkrel X instead of Release: X%{?dist}.

It might work with OpenMandriva, which does just Release: X (DistTag is auto-appended to package filename, and stored in the RPM in DistTag attribute automatically).

Hi, summarizing my thoughts.

I have many cases in specs, where we have tags for some proposes

Release:    0.6.%{?date}%{?date:git}%{?rel}%{?dist}
Release:    1%{?prerel:.%{prerel}} 
Release:    2%{?rel_tag}%{?dist}
Release:    2%{?gver}%{?_with_bootstrap:_bootstrap}%{?dist}
Release:    9.%{gitdate}.git%{shortcommit}%{?dist}
Release:    6.%{snapshot}%{?dist}
Release:    15%{?rel_string}%{?dist}

-n clean all, so for me don't work.
if you need it this works like this for something, I don't see where. But we can use another switch, for example -N
Also like we have rpmdev-bumpspec we can't have % macros in Release tag, and I'd like do script that updates some different packages so I can't know how Release tag is composed, so I don't have way to do the script when we bump the release .

The example of the script to update all kde components in RPMFusion.
https://paste.fedoraproject.org/paste/2SXDSPahlAx3uOJf0-z7Dg

To do this properly would more or less require full spec field parsing, which is nuts.

Also, some of these release fields you're showing are not in compliance with Fedora guidelines...

Why so many vastly different release tags schemes? That makes no sense at all. You could unify them all and reduce the final tag definition to something that uses a single macro on its right side. Then you could use -n with a V-R pair.

                  Why so many vastly different release tags schemes? That makes no sense at all. You could unify them all and reduce the final tag definition to something that uses a single macro on its right side. Then you could use -n with a V-R pair.

OK that is a point.
Examples are strictly from RPMFusion , we have different packages and packagers but just talking about packages, it is hard unify all packages, some need bootstrap flag , some have post fixes , some have pre-releases that we like to test, some make releases in a short time , others take many years, even others don't do releases, etc, etc.

PS: I think I confuse you with Michael Cronenworth , though that was the same person, not a problem, just a note ...

I'm going to close this, because I don't think I'll ever take this due to its complexity.

Pull-Request has been closed by ngompa

5 years ago
Metadata