incoherent-version-in-changelog is factoring .%dist in %{release}

[brianjmurrell]

I am getting a W: incoherent-version-in-changelog 1.15.0~rc3-2 ['1.15.0~rc3-2.suse.lp153', '1.15.0~rc3-2.suse.lp153'] warning.

This is due to Release: 2%{?dist} in the specfile. But I cannot include either %{dist} or even %{release} in the changelog version or else I get W: macro-in-%changelog %{release}. I certainly cannot hard code a given distro's %{dist} value either.

It seems like

rpmlint/rpmlint/checks/TagsCheck.py

Lines 549 to 562 in b749f3a

	expected = [version + '-' + release]
	if epoch is not None: # regardless of use_epoch
	expected[0] = str(epoch) + ':' + expected[0]
	# Allow EVR in changelog without release extension,
	# the extension is often a macro or otherwise dynamic.
	if self.release_ext:
	expected.append(self.extension_regex.sub('', expected[0]))
	# Check if a package does not have a version that is
	# compatible with epoch:vesrion-release tuple
	if ret.group(1) not in expected:
	if len(expected) == 1:
	expected = expected[0]
	self.output.add_info('W', pkg, 'incoherent-version-in-changelog',
	ret.group(1), expected)

needs to account for a possible .%{?dist} in Release:.

[marxin]

This is due to Release: 2%{?dist} in the specfile.

It seems strange adding %{dist} to Release.

But I cannot include either %{dist} or even %{release} in the changelog version or else I get W: macro-in-%changelog %{release}. I certainly cannot hard code a given distro's %{dist} value either.

Well, I can imagine supporting any of the 2 macros in %changelog but I would like to see your use-case?

[brianjmurrell]

It seems strange adding %{dist} to Release.

To just pick a specfile at random:

https://src.fedoraproject.org/rpms/autofs/blob/rawhide/f/autofs.spec#_15

I think you will find that all Fedora, RHEL (and therefore all EL downstreams) and SUSE specfiles do that.

Well, I can imagine supporting any of the 2 macros in %changelog but I would like to see your use-case?

As above? But honestly, I'd rather not use %{release} as it defeats the purpose of the changelog and having to add %{?dist} to every changelog entry just seems silly.

It seems far more useful for expected to just allow for %{version}-%{release} with .%{dist} removed. But yes, how do you know what %{dist} is for a SUSE RPM when running rpmlint on say, a Fedora machine. I can only suppose you need to keep a list of common %{dist}s.

[marxin]

To just pick a specfile at random:

https://src.fedoraproject.org/rpms/autofs/blob/rawhide/f/autofs.spec#_15

I think you will find that all Fedora, RHEL (and therefore all EL downstreams) and SUSE specfiles do that.

All right, so then let's allow %{dist} in Release then. Do you want to come up with a pull request?

[brianjmurrell]

Do you want to come up with a pull request?

Unfortunately this is a $day_job issue and I have too many other deadlines looming to have to take on another yak to shave.

But there is a different kind of consistency issue here. Why do I only see this while parsing a suse RPM and not a, say, EL7 or EL8 RPM? They are all built from the same specfile and therefore all have %{?dist} expanded in their Release: but no in their %changelog entries.

What is special about the el7 and el8 %{?dist}s that they don't raise this issue?

[marxin]

Please upload somewhere RPM files so that I can observe the difference.

[brianjmurrell]

Hrm. I don't really have anywhere I can upload them to.

If you use mock to build https://github.com/daos-stack/dpdk/blob/1231cac43173877aae6b152ded5523d80809bac9/dpdk.spec on each of opensuse-leap-15.3-x86_64 and rocky+epel-8-x86_64 you will get RPMs you can compare.

[marxin]

Hrm. I don't really have anywhere I can upload them to.

Please zip them and attach them to this Issue.

[Conan-Kudo]

@brianjmurrell Do you have a COPR project where this was built? That would also work, since they use the same Mock definitions.

[marxin]

Can you please attach the problematic RPM file?

[brianjmurrell]

libfabric-1.15.1-3.suse.lp153.x86_64.rpm.tar.gz

[marxin]

Thanks for the file.

So can you please experiment with something like:

diff --git a/rpmlint/checks/SpecCheck.py b/rpmlint/checks/SpecCheck.py
index 997d974c..739840be 100644
--- a/rpmlint/checks/SpecCheck.py
+++ b/rpmlint/checks/SpecCheck.py
@@ -436,7 +436,7 @@ class SpecCheck(AbstractCheck):
                                          '%%changelog: %s' % deptoken)
                 for match in self.macro_regex.findall(line):
                     res = re.match('%+', match)
-                    if len(res.group(0)) % 2 and match != '%autochangelog':
+                    if len(res.group(0)) % 2 and match not in ('%autochangelog', '%release', '%dist'):
                         self.output.add_info('W', pkg, 'macro-in-%changelog', match)
             else:
                 if not depscript_override:

which should not warn about %release nor %dist macros in %changelog.

[Conan-Kudo]

The %release macro should never be in the changelog, nor should %dist.

[Conan-Kudo]

@brianjmurrell In the case of writing out EVRs in the changelog entry, you should omit the %dist macro, because that value changes depending on who it is built for. So instead of 1.0-1%{?dist}, just write 1.0-1.

Basically, this is not a bug, and this is working as intended.

[brianjmurrell]

@Conan-Kudo Indeed, I completely agree. While it's been a while since I filed this ticket, because I generally agree with you I can only imagine that I tried including %{dist} to try to appease some other rpmlint error. I will try to revisit this.

Indeed, if you go re-read the original comment here, I do say:

I cannot include either %{dist} or even %{release} in the changelog version

So I acknowledge that I don't really want to do that,

The title of this issue even is incoherent-version-in-changelog is factoring .%dist in %{release} which is describing that it's not my attempting to include .%{dist} in the %changelog that is at the root of the problem but rather that rpmlint is having a fit because %{dist} is in the Release:, and that is disagreeing with what is in the %changelog entry's version (when it does not try to include .%{dist} in it.

[brianjmurrell]

if len(res.group(0)) % 2 and match not in ('%autochangelog', '%release', '%dist'):

But per @Conan-Kudo (and I agree with him), I shouldn't actually be putting the %dist macro into the %changelog entry, so this solution seems to simply allow the hacky work-around rather than addressing the root problem of having the %dist in the Release: being factored into the comparison of the version in the %changelog.

That said, the work-around does work. It just doesn't seem to be the correct solution where the correct solution, I would think, is to disable the factoring of %dist into Release: when comparing the version with the one in the %changelog.

[brianjmurrell]

Any further thoughts given the last couple of comments?

[marxin]

Let's use the workaround for now. The issue is correct, but has very low priority for me, sorry.

[brianjmurrell]

What is the workaround exactly?

[marxin]

The one mentioned in #856 (comment) ?

[brianjmurrell]

Do you mean using %{dist} in the %changelog entry? It seems count-productive to break the data just to appease a broken validation tool.

I think I'd sooner add an ignore in the rpmlintrc than to start breaking the real-world data just to appease the broken validator.

So I guess that is what I will do until this issue is fixed.

Thanks for all of the input.

[marxin]

I think I'd sooner add an ignore in the rpmlintrc than to start breaking the real-world data just to appease the broken validator.

Yes, that would be the easiest approach.

Thanks for the discussion.