Bug 1047531 - -V gives ERROR: No release value matched: xxx.spec
Summary: -V gives ERROR: No release value matched: xxx.spec
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: rpmdevtools
Version: 21
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Ville Skyttä
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1157418
TreeView+ depends on / blocked
 
Reported: 2013-12-31 15:10 UTC by Mads Kiilerich
Modified: 2014-11-01 16:51 UTC (History)
4 users (show)

Fixed In Version: rpmdevtools-8.5-1.fc21
Clone Of:
: 1157418 (view as bug list)
Environment:
Last Closed: 2014-11-01 16:51:36 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
Fail if nothing to bump was found, no matter if -V or not (704 bytes, patch)
2014-01-01 09:40 UTC, Ville Skyttä
no flags Details | Diff
Take 2: Fail if nothing to bump was found, no matter if -V or not (639 bytes, patch)
2014-01-01 09:43 UTC, Ville Skyttä
no flags Details | Diff
this shall fix it again (1.12 KB, patch)
2014-01-03 22:34 UTC, Michael Schwendt
no flags Details | Diff

Description Mads Kiilerich 2013-12-31 15:10:03 UTC
Description of problem:

running rpmdev-bumpspec with -V on a fine spec fails with
ERROR: No release value matched: xxx.spec

Without -V it works fine.

Looking at the code, it has some special handling to ignore some "Release:" header, and if none is found if will exit(1) when in verbose mode.

I don't know if some special handling of Release is necessary in some cases, but in my case it isn't necessary ... and anyway, adding a -V for verbose should just make it more verbose, not change the behaviour.


Version-Release number of selected component (if applicable):

rpmdevtools-8.4-2.fc20.noarch

This is a change of behaviour since f19, IIRC.

Comment 1 Ville Skyttä 2014-01-01 09:40:04 UTC
Created attachment 844064 [details]
Fail if nothing to bump was found, no matter if -V or not

Looking at the code, I suppose it has always been a bug that the script will exit with code 1 when no value to bump was found, _but only in verbose mode_.

But the current issue of always failing in verbose mode is AFAICS a regression from https://git.fedorahosted.org/cgit/rpmdevtools.git/commit/?id=dd406cf

Michael, does the attached patch look sensible to you?

Comment 2 Ville Skyttä 2014-01-01 09:41:34 UTC
(In reply to Ville Skyttä from comment #1)
> Michael, does the attached patch look sensible to you?

(Better chance to draw attention if Cc'ing people when asking them...)

Comment 3 Ville Skyttä 2014-01-01 09:43:28 UTC
Created attachment 844066 [details]
Take 2: Fail if nothing to bump was found, no matter if -V or not

Comment 4 Michael Schwendt 2014-01-03 22:34:40 UTC
Created attachment 845126 [details]
this shall fix it again

True.

Originally, only the indentation of the sys.exit(1) line had been wrong. With the old code, for any bump the function would have been returned from early. The end of the function would have been reached only when reaching the end of the spec file without having bumped anything.

The attempt at bumping also subpackage Release values in a spec file has messed up the condition when to return from that function. The initial "bumped?" check wasn't too bad but couldn't return from the function prior to reaching the end of the spec file. Since all lines from the spec file must be processed, there must be a single "bumped?" check at the end of the foreach-line loop.

And Ville, your patch is close, but (1) it doesn't define "whatwasbumped" for the case of bumping a normal pattern that matched, and (2) replacing the superfluous "continue" with a "break" for the fallback-case breaks out of the main foreach-line loop and then doesn't process the entire spec file and interferes with -V again.

Comment 5 Michael Schwendt 2014-01-03 22:56:57 UTC
Oh, the "whatwasbumped" related patch is not for F20 but for the changes for bug 1043269 (I haven't thought about that one again yet). It seems it could work, but whether to write to stderr always or only in verbose mode could be changed.

Comment 6 Ville Skyttä 2014-01-06 15:09:53 UTC
Yes, the "whatwasbumped" patch is applied in git, see bug 789330 comment 28, and thus your patch from comment 4 doesn't apply to current git anymore.

Could you clarify what you mean by "it seems it could work"? The "whatwasbumped" patch is not enough alone to fix this bug (nor unfortunately the kernel one), but I believe my patch from comment 3 would work for this particular issue. If the continue -> break change causes problems, the continue could be left in but then we should check if "fallback" is already in "whatwasbumped" before appending it there and would need to teach the code that iterates patids from "whatwasbumped" not to try to look up a _macro_patterns pattern for the fake "fallback" patid.

Would you mind getting a git committer access for "upstream" rpmdevtools and help out directly at least with bumpspec there? My position is a bit awkward as I'm not the author of it nor do I really use it myself for anything. If you can help, please file for gitrpmdevtools access in FAS and I'll gladly approve it.

Comment 7 Michael Schwendt 2014-01-08 01:21:34 UTC
> Could you clarify what you mean by "it seems it could work"?

It referred to your patch (the one attached to this ticket). It could be a valid fix, but I haven't had the git version of the script in front of me (and not in my mind either) and could not draw conclusions just by looking at the patch.

[...]

From all src.rpms and built rpms in the F20 repos including updates-testing and rpmfusion-free+updates, only a very few packages define subpackages with a different %version and/or %release:


Different VR:
['Saaghar', 'linux-firmware', 'openssh', 'texlive']

Only different R:
[]

Only different V:
['antlr3', 'beesu', 'bpg-fonts', 'brltty', 'ding-libs', 'eclipse-linuxtools', 'festival', 'ganglia', 'ghc', 'haskell-platform', 'iaxclient', 'lpg', 'lvm2', 'nhn-nanum-fonts', 'openlmi-scripts', 'perl', 'perl-PBS', 'ruby', 'svxlink', 'texlive', 'thunderbird-lightning', 'trustedqsl', 'xonotic']


A few I've examined do it with macros. Some do it with lots of macros. For others, it isn't even trivial for a human packager to bump them. For example, where a subpackage defines its own %version, one must keep its own %release completely separate from the base package %release (as not to reset it accidentally for base package version upgrades).


The following implements a different and more simple idea than the "whatwasbumped" approach. It also handles kernel.spec and includes the F20 patch I've attached to this ticket:

  http://mschwendt.fedorapeople.org/rpmdev-bumpspec

If we've bumped a macro definition, we're done. No need to search for further Release tags or other macros, since the risk of bumping in competing places is too high. (A new example for that is a subpackage that does "Release: %release", which is superfluous and would have caused the bumpspec script to mess up this tag.)

Comment 8 Raphael Groner 2014-09-06 16:58:06 UTC
Reproducible for me. Vote +1

(In reply to Mads Kiilerich from comment #0)
> Description of problem:
> 
> running rpmdev-bumpspec with -V on a fine spec fails with
> ERROR: No release value matched: xxx.spec
> 
> Without -V it works fine.

Comment 9 Michael Schwendt 2014-09-06 20:47:30 UTC
> Reproducible for me. Vote +1

https://git.fedorahosted.org/cgit/rpmdevtools.git/plain/rpmdev-bumpspec
or the older one in comment 7 contains a fix.

Comment 10 Raphael Groner 2014-09-06 21:34:24 UTC
Please provide a koji scratch build or a package in @updates-testing with the patch applied, so we can test with good quality.

Comment 11 Fedora Update System 2014-10-20 18:48:35 UTC
rpmdevtools-8.5-1.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/rpmdevtools-8.5-1.fc21

Comment 12 Fedora Update System 2014-10-21 17:24:14 UTC
Package rpmdevtools-8.5-1.fc21:
* should fix your issue,
* was pushed to the Fedora 21 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing rpmdevtools-8.5-1.fc21'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2014-13380/rpmdevtools-8.5-1.fc21
then log in and leave karma (feedback).

Comment 13 Raphael Groner 2014-10-23 16:50:30 UTC
Do I understand it rightly, there will be no patch available for F20?

Comment 14 Ville Skyttä 2014-10-24 06:03:33 UTC
Yes and no. A patch will not be available, but it's possible that the 8.5 update will be pushed to F-20 at some point.

Comment 15 Fedora Update System 2014-11-01 16:51:36 UTC
rpmdevtools-8.5-1.fc21 has been pushed to the Fedora 21 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.