Bug 1957928 - Review Request: rpminspect-data-centos - Build deviation compliance tool data files
Summary: Review Request: rpminspect-data-centos - Build deviation compliance tool data...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Brian Lane
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-05-06 18:58 UTC by David Cantrell
Modified: 2021-07-16 01:26 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-06-30 18:04:53 UTC
Type: ---
Embargoed:
bcl: fedora-review+


Attachments (Terms of Use)

Description David Cantrell 2021-05-06 18:58:37 UTC
Spec URL: https://dcantrell.fedorapeople.org/rpminspect-data-centos.spec
SRPM URL: https://dcantrell.fedorapeople.org/rpminspect-data-centos-1.0-1.fc34.src.rpm

Description: CentOS and CentOS Stream specific configuration file for rpminspect
and data files used by the inspections provided by librpminspect.

Fedora Account System Username: dcantrell

This package is similar to rpminspect-data-fedora, except this one is for CentOS.  The package contains data files that define packaging policies for CentOS and CentOS Stream packages.  I am adding it to Fedora because it is possible to run rpminspect anywhere and specify the ruleset for another product.  I want to make rpminspect-data-centos available on Fedora and in EPEL so that you can do CentOS work from a Fedora or EPEL-compatible system more easily.

Comment 1 Brian Lane 2021-05-06 23:16:34 UTC
rpmlint doesn't like including the changelog, but that's a problem with rpmlint not the .spec

Everything else looks good. Builds fine in mock, installs in Fedora 34.

Comment 2 Neal Gompa 2021-05-07 11:43:13 UTC
(In reply to Brian Lane from comment #1)
> rpmlint doesn't like including the changelog, but that's a problem with
> rpmlint not the .spec
> 
> Everything else looks good. Builds fine in mock, installs in Fedora 34.

No, this is a problem in general. This is not acceptable for Fedora.

Comment 3 Neal Gompa 2021-05-07 11:45:00 UTC
And now I've checked and found out this was done on rpminspect-data-fedora and rpminspect. :(

Comment 4 David Cantrell 2021-05-07 13:04:54 UTC
(In reply to Neal Gompa from comment #2)
> (In reply to Brian Lane from comment #1)
> > rpmlint doesn't like including the changelog, but that's a problem with
> > rpmlint not the .spec
> > 
> > Everything else looks good. Builds fine in mock, installs in Fedora 34.
> 
> No, this is a problem in general. This is not acceptable for Fedora.

I don't see anything in the packaging guidelines that prohibit this.  The guidelines discuss the format of the changelog content, which I have made sure to do so that entries do not simply say "Upgrade to program-1.2.3".

Comment 5 Neal Gompa 2021-05-07 13:20:46 UTC
It was already asked before and rejected: https://pagure.io/packaging-committee/pull-request/942

Comment 6 Neal Gompa 2021-05-07 13:21:51 UTC
Also, another issue with this package:

> %setup -q -n rpminspect-data-centos-1.0

This should be macroized, or it will fail every time the spec is bumped.

Comment 7 Neal Gompa 2021-05-07 13:23:27 UTC
> Source0:        rpminspect-data-centos-1.0.tar.xz

Please turn this into a proper SourceURL that is macroized so bumping versions will work with normal workflows.

Comment 8 David Cantrell 2021-05-07 17:49:29 UTC
(In reply to Neal Gompa from comment #5)
> It was already asked before and rejected:
> https://pagure.io/packaging-committee/pull-request/942

Yeah, I've read that.  But there's still nothing in the packaging guidelines for Fedora that explicitly say "don't do this."

I think you should understand the workflow that I'm using.  I fail to see how %include is a problem here because the SRPM is built from the dist-git branch now and the spec file is processed accordingly.  The resulting RPMs contain the changelog entries in the RPM headers, so all of the data is there.  The resulting SRPM also includes the 'changelog' file which gets installed if you ever install the source RPM locally and rebuild the package.  I see all of the comments in the PR you linked, but I have yet to see any actual problem from the workflow I have.  Maybe I'm missing something.

The packaging aspect of rpminspect and the data packages are downstream from the projects.  I make a release upstream and then an RPM spec file style changelog block is generated.  This is combined with any existing 'changelog' file in the downstream repo.  I then have 'make koji' in the upstream project which builds that new release for each target release of Fedora and EPEL.  I do this so the changelog that appears in the RPM spec is one-way from the upstream repo.  I don't want to maintain a changelog in each Fedora and EPEL branch.  Now since there are rebuilds and other people touching packages, my 'make koji' step sucks in whatever %changelog entries got added to the spec file and prepends them the existing changelog and then the new release's changelog lines are added on top of that.  All of this is done so I can be out of the business of maintaining RPM style spec file changelogs but still provide that information to users.

I'll change %setup and Source0 to use macros, but to be clear, these would never change without first making a new release upstream and running 'make koji'.  The spec file is generated at release time.  It doesn't matter to me though, I'll change it to the macros.

I do all of the above to avoid "backcommitting" spec file changelogs to the upstream project.  The entire desire I have is to not have a spec file changelog in two places.  It's autogenerated from upstream and goes in the downstream repo, combined with whatever happened downstream independent of upstream work.  I put together this workflow to provide information of substance in the RPM spec file changelog because people expressed interest in that (in fact, you specifically did, and your reasons for it made sense...I just wanted to make it not-a-manual-process).  Prior to any of this, I just had a %changelog with a single link to the github project page history.

Comment 9 Neal Gompa 2021-05-07 18:14:34 UTC
(In reply to David Cantrell from comment #8)
> (In reply to Neal Gompa from comment #5)
> > It was already asked before and rejected:
> > https://pagure.io/packaging-committee/pull-request/942
> 
> Yeah, I've read that.  But there's still nothing in the packaging guidelines
> for Fedora that explicitly say "don't do this."
> 
> I think you should understand the workflow that I'm using.  I fail to see
> how %include is a problem here because the SRPM is built from the dist-git
> branch now and the spec file is processed accordingly.  The resulting RPMs
> contain the changelog entries in the RPM headers, so all of the data is
> there.  The resulting SRPM also includes the 'changelog' file which gets
> installed if you ever install the source RPM locally and rebuild the
> package.  I see all of the comments in the PR you linked, but I have yet to
> see any actual problem from the workflow I have.  Maybe I'm missing
> something.
> 

This really sounds like you should just have an upstream changelog file that gets pulled in as a doc. The point of the %changelog in the package is to detail the packaging changes. I know people do mix the two, but the fundamental assumption for RPM changelogs that most people have is that they detail the changes done to the package, and the changes to the software is inside the package as a file.

The problem I generally have with your method of changelogs is the usage of %include, which just makes it messy, but it's included as a source, so...

*sigh*

Comment 10 David Cantrell 2021-05-07 18:48:50 UTC
(In reply to Neal Gompa from comment #9)
> (In reply to David Cantrell from comment #8)
> > (In reply to Neal Gompa from comment #5)
> > > It was already asked before and rejected:
> > > https://pagure.io/packaging-committee/pull-request/942
> > 
> > Yeah, I've read that.  But there's still nothing in the packaging guidelines
> > for Fedora that explicitly say "don't do this."
> > 
> > I think you should understand the workflow that I'm using.  I fail to see
> > how %include is a problem here because the SRPM is built from the dist-git
> > branch now and the spec file is processed accordingly.  The resulting RPMs
> > contain the changelog entries in the RPM headers, so all of the data is
> > there.  The resulting SRPM also includes the 'changelog' file which gets
> > installed if you ever install the source RPM locally and rebuild the
> > package.  I see all of the comments in the PR you linked, but I have yet to
> > see any actual problem from the workflow I have.  Maybe I'm missing
> > something.
> > 
> 
> This really sounds like you should just have an upstream changelog file that
> gets pulled in as a doc. The point of the %changelog in the package is to
> detail the packaging changes. I know people do mix the two, but the
> fundamental assumption for RPM changelogs that most people have is that they
> detail the changes done to the package, and the changes to the software is
> inside the package as a file.

OK, so I've heard arguments in both directions here.  The %changelog should include packaging changes, but it should also include summaries of changes of significance.  I can see arguments for both.  In the case of these rpminspect packages, nothing of consequence is going to really happen packaging wise.  They are all very simple.  Are you saying the %changelog can just simply be "- Upgrade to rpminspect-data-centos-1.0" and leave it at that?  I can add a %doc which is a generated ChangeLog from the git log.

A comparison can be made with anaconda where we have always maintained the RPM %changelog detailing all of the changes for each release.  I felt that same idea applied here.  At least in the case of the rpminspect data packages, maybe not necessarily the program.

What would you prefer I do for the %changelog here?  Maybe I am overcomplicating things because I felt people wanted to see the detailed changes via "rpm -q --changelog PACKAGE".

> The problem I generally have with your method of changelogs is the usage of
> %include, which just makes it messy, but it's included as a source, so...
> 
> *sigh*

I get that and I don't want to make things confusing for people.  My issue is I'm trying to understand the technical failures by using %include.  Style opinions vary across all developers, so I get that.  You and I can just disagree on style, which is fine.  The failure I have been able to reproduce using %include is if you are rebuilding locally where _topdir gets redefined in your environment and rpmbuild then cannot find the changelog to include.  It won't fail, it just won't include the changelog.  But the default install of rpm, the use of rpmdev-setuptree for local development, and the use of fedpkg using dist-git all result in the desired output.  fedpkg local can be messed up, so I assume that's a reasonable failure to avoid.

I am reworking how I generate this package and will post an update later today.  Once I get things sorted out with this one, I will propagate the changes over to the other rpminspect packages for consistency.

Comment 11 Neal Gompa 2021-05-07 21:35:59 UTC
(In reply to David Cantrell from comment #10)
> (In reply to Neal Gompa from comment #9)
> > (In reply to David Cantrell from comment #8)
> > > (In reply to Neal Gompa from comment #5)
> > > > It was already asked before and rejected:
> > > > https://pagure.io/packaging-committee/pull-request/942
> > > 
> > > Yeah, I've read that.  But there's still nothing in the packaging guidelines
> > > for Fedora that explicitly say "don't do this."
> > > 
> > > I think you should understand the workflow that I'm using.  I fail to see
> > > how %include is a problem here because the SRPM is built from the dist-git
> > > branch now and the spec file is processed accordingly.  The resulting RPMs
> > > contain the changelog entries in the RPM headers, so all of the data is
> > > there.  The resulting SRPM also includes the 'changelog' file which gets
> > > installed if you ever install the source RPM locally and rebuild the
> > > package.  I see all of the comments in the PR you linked, but I have yet to
> > > see any actual problem from the workflow I have.  Maybe I'm missing
> > > something.
> > > 
> > 
> > This really sounds like you should just have an upstream changelog file that
> > gets pulled in as a doc. The point of the %changelog in the package is to
> > detail the packaging changes. I know people do mix the two, but the
> > fundamental assumption for RPM changelogs that most people have is that they
> > detail the changes done to the package, and the changes to the software is
> > inside the package as a file.
> 
> OK, so I've heard arguments in both directions here.  The %changelog should
> include packaging changes, but it should also include summaries of changes
> of significance.  I can see arguments for both.  In the case of these
> rpminspect packages, nothing of consequence is going to really happen
> packaging wise.  They are all very simple.  Are you saying the %changelog
> can just simply be "- Upgrade to rpminspect-data-centos-1.0" and leave it at
> that?  I can add a %doc which is a generated ChangeLog from the git log.
> 
> A comparison can be made with anaconda where we have always maintained the
> RPM %changelog detailing all of the changes for each release.  I felt that
> same idea applied here.  At least in the case of the rpminspect data
> packages, maybe not necessarily the program.
> 
> What would you prefer I do for the %changelog here?  Maybe I am
> overcomplicating things because I felt people wanted to see the detailed
> changes via "rpm -q --changelog PACKAGE".
> 

I think that as long as nobody is specifically asking for it, I wouldn't do it that way. As far as I know, the reason Anaconda does that is that it's part of the tito-based workflow they adopted. If you *want* to, then by all means, but the general guidance I've gotten over the years is that having the software changelog in the docdir is more than sufficient for those kinds of changes.

> > The problem I generally have with your method of changelogs is the usage of
> > %include, which just makes it messy, but it's included as a source, so...
> > 
> > *sigh*
> 
> I get that and I don't want to make things confusing for people.  My issue
> is I'm trying to understand the technical failures by using %include.  Style
> opinions vary across all developers, so I get that.  You and I can just
> disagree on style, which is fine.  The failure I have been able to reproduce
> using %include is if you are rebuilding locally where _topdir gets redefined
> in your environment and rpmbuild then cannot find the changelog to include. 
> It won't fail, it just won't include the changelog.  But the default install
> of rpm, the use of rpmdev-setuptree for local development, and the use of
> fedpkg using dist-git all result in the desired output.  fedpkg local can be
> messed up, so I assume that's a reasonable failure to avoid.
> 
> I am reworking how I generate this package and will post an update later
> today.  Once I get things sorted out with this one, I will propagate the
> changes over to the other rpminspect packages for consistency.

It's basically the topdir thing that's a problem. Otherwise, *shrug*...

Comment 12 David Cantrell 2021-06-24 15:30:27 UTC
To close the loop on this...

(In reply to Neal Gompa from comment #11)
> (In reply to David Cantrell from comment #10)
> > (In reply to Neal Gompa from comment #9)
> > > (In reply to David Cantrell from comment #8)
> > > > (In reply to Neal Gompa from comment #5)
> > > > > It was already asked before and rejected:
> > > > > https://pagure.io/packaging-committee/pull-request/942
> > > > 
> > > > Yeah, I've read that.  But there's still nothing in the packaging guidelines
> > > > for Fedora that explicitly say "don't do this."
> > > > 
> > > > I think you should understand the workflow that I'm using.  I fail to see
> > > > how %include is a problem here because the SRPM is built from the dist-git
> > > > branch now and the spec file is processed accordingly.  The resulting RPMs
> > > > contain the changelog entries in the RPM headers, so all of the data is
> > > > there.  The resulting SRPM also includes the 'changelog' file which gets
> > > > installed if you ever install the source RPM locally and rebuild the
> > > > package.  I see all of the comments in the PR you linked, but I have yet to
> > > > see any actual problem from the workflow I have.  Maybe I'm missing
> > > > something.
> > > > 
> > > 
> > > This really sounds like you should just have an upstream changelog file that
> > > gets pulled in as a doc. The point of the %changelog in the package is to
> > > detail the packaging changes. I know people do mix the two, but the
> > > fundamental assumption for RPM changelogs that most people have is that they
> > > detail the changes done to the package, and the changes to the software is
> > > inside the package as a file.
> > 
> > OK, so I've heard arguments in both directions here.  The %changelog should
> > include packaging changes, but it should also include summaries of changes
> > of significance.  I can see arguments for both.  In the case of these
> > rpminspect packages, nothing of consequence is going to really happen
> > packaging wise.  They are all very simple.  Are you saying the %changelog
> > can just simply be "- Upgrade to rpminspect-data-centos-1.0" and leave it at
> > that?  I can add a %doc which is a generated ChangeLog from the git log.
> > 
> > A comparison can be made with anaconda where we have always maintained the
> > RPM %changelog detailing all of the changes for each release.  I felt that
> > same idea applied here.  At least in the case of the rpminspect data
> > packages, maybe not necessarily the program.
> > 
> > What would you prefer I do for the %changelog here?  Maybe I am
> > overcomplicating things because I felt people wanted to see the detailed
> > changes via "rpm -q --changelog PACKAGE".
> > 
> 
> I think that as long as nobody is specifically asking for it, I wouldn't do
> it that way. As far as I know, the reason Anaconda does that is that it's
> part of the tito-based workflow they adopted. If you *want* to, then by all
> means, but the general guidance I've gotten over the years is that having
> the software changelog in the docdir is more than sufficient for those kinds
> of changes.

That's not the reason anaconda uses the RPM %changelog that way.  They may not do this anymore.  I started working on anaconda in 2005 and the unique thing about the releases is that there was no upstream tarball release of anaconda.  A release was made in SRPM format and built in Fedora directly.  We used the %changelog block as our project's changelog.  This is different than many other projects, but it's what was going on when I joined the project.  Having worked on anaconda for so long, I began to like the summarized %changelog entries that reflected changes that actually went in to the code as opposed to spec file changes or other such stuff like that.

I have not looked so I don't know if the anaconda team still does that, but at least that's the reason we were doing that for so long.

> > > The problem I generally have with your method of changelogs is the usage of
> > > %include, which just makes it messy, but it's included as a source, so...
> > > 
> > > *sigh*
> > 
> > I get that and I don't want to make things confusing for people.  My issue
> > is I'm trying to understand the technical failures by using %include.  Style
> > opinions vary across all developers, so I get that.  You and I can just
> > disagree on style, which is fine.  The failure I have been able to reproduce
> > using %include is if you are rebuilding locally where _topdir gets redefined
> > in your environment and rpmbuild then cannot find the changelog to include. 
> > It won't fail, it just won't include the changelog.  But the default install
> > of rpm, the use of rpmdev-setuptree for local development, and the use of
> > fedpkg using dist-git all result in the desired output.  fedpkg local can be
> > messed up, so I assume that's a reasonable failure to avoid.
> > 
> > I am reworking how I generate this package and will post an update later
> > today.  Once I get things sorted out with this one, I will propagate the
> > changes over to the other rpminspect packages for consistency.
> 
> It's basically the topdir thing that's a problem. Otherwise, *shrug*...

Ehhhh, ok.  I mean, I kind of consider that a featureful flaw of rpmbuild anyway.  If you go mucking around with topdir, you're bound to hit some problems and my personal view is if you're doing that and it breaks, you get to keep the pieces.  The only thing I've been able to reproduce is not getting a %changelog, which isn't fatal for RPM.

All that aside, I have reworked how I am automating builds for Copr and released builds in Koji so it's no longer generating a changelog like I had been.  It now generates a single "- Upgrade to NAME-VER" entry and prepends that to whatever it finds in the downstream git repo for the package.  The combined changelog block is committed downstream so the growing changelogs are not pushed back upstream but just live on in their respective branches in package git.  To me this gets to what you were suggesting while also retaining any changelog modifications that appear downstream *and* not having to keep spec files in sync in two locations.

New spec file and SRPM posted:
Spec: https://dcantrell.fedorapeople.org/rpminspect-data-centos.spec
SRPM: https://dcantrell.fedorapeople.org/rpminspect-data-centos-1.0-1.fc34.src.rpm

I know the package already has review+, but I wanted to make this change before getting it in to the distribution.  If this is satisfactory for all parties who have commented, I will go forth with it.  I will also make these changelog adjustments for the existing rpminspect-data-fedora package as well.

Comment 13 Brian Lane 2021-06-24 15:58:13 UTC
Looks good to me!

Comment 14 Gwyn Ciesla 2021-06-24 21:01:33 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rpminspect-data-centos

Comment 15 Fedora Update System 2021-06-30 17:16:40 UTC
FEDORA-2021-471f0e3569 has been submitted as an update to Fedora 33. https://bodhi.fedoraproject.org/updates/FEDORA-2021-471f0e3569

Comment 16 Fedora Update System 2021-06-30 17:28:34 UTC
FEDORA-2021-52307b5ee2 has been submitted as an update to Fedora 34. https://bodhi.fedoraproject.org/updates/FEDORA-2021-52307b5ee2

Comment 17 Fedora Update System 2021-06-30 17:46:50 UTC
FEDORA-EPEL-2021-a087106e96 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-a087106e96

Comment 18 Fedora Update System 2021-06-30 18:02:36 UTC
FEDORA-EPEL-2021-20a66b3776 has been submitted as an update to Fedora EPEL 8. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-20a66b3776

Comment 19 David Cantrell 2021-06-30 18:04:53 UTC
Builds submitted for F33, F34, EPEL7, and EPEL8.  Also built for rawhide.

Comment 20 Fedora Update System 2021-07-01 01:04:24 UTC
FEDORA-2021-471f0e3569 has been pushed to the Fedora 33 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2021-471f0e3569 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-471f0e3569

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 21 Fedora Update System 2021-07-01 01:18:11 UTC
FEDORA-2021-52307b5ee2 has been pushed to the Fedora 34 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2021-52307b5ee2 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-52307b5ee2

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 22 Fedora Update System 2021-07-01 01:18:47 UTC
FEDORA-EPEL-2021-20a66b3776 has been pushed to the Fedora EPEL 8 testing repository.

You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-20a66b3776

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 23 Fedora Update System 2021-07-01 01:51:34 UTC
FEDORA-EPEL-2021-a087106e96 has been pushed to the Fedora EPEL 7 testing repository.

You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-a087106e96

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 24 Fedora Update System 2021-07-09 00:46:02 UTC
FEDORA-2021-471f0e3569 has been pushed to the Fedora 33 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 25 Fedora Update System 2021-07-09 01:01:07 UTC
FEDORA-2021-52307b5ee2 has been pushed to the Fedora 34 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 26 Fedora Update System 2021-07-16 01:15:38 UTC
FEDORA-EPEL-2021-20a66b3776 has been pushed to the Fedora EPEL 8 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 27 Fedora Update System 2021-07-16 01:26:35 UTC
FEDORA-EPEL-2021-a087106e96 has been pushed to the Fedora EPEL 7 stable repository.
If problem still persists, 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.