Bug 1444925 - Review Request: python-rpm-generators - Requires and Provides generators for Python RPMs
Summary: Review Request: python-rpm-generators - Requires and Provides generators for ...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Igor Gnatenko
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-04-24 14:40 UTC by Michal Cyprian
Modified: 2017-10-16 22:46 UTC (History)
6 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2017-10-16 22:46:17 UTC
Type: ---
Embargoed:
ignatenko: fedora-review+


Attachments (Terms of Use)

Description Michal Cyprian 2017-04-24 14:40:44 UTC
Spec URL: https://mcyprian.fedorapeople.org/python-rpm-generators.spec
SRPM URL: https://mcyprian.fedorapeople.org/python-rpm-generators-1-1.fc25.src.rpm
Description: This package contains RPM python dependency generators, which were part of the rpm package before. Package rpm_build won't depend on Python 3 after python scripts are moved to separate package. This will also make minimal build root smaller.
Fedora Account System Username: mcyprian

Comment 1 Petr Šabata 2017-04-25 08:56:50 UTC
Thank you so much for doing this :)

Comment 2 Petr Pisar 2017-04-25 14:22:26 UTC
The rpmhome hard-codes a path. Use %{_rpmconfigdir} instead.

Comment 3 Neal Gompa 2017-04-27 01:57:37 UTC
For what it's worth, this is an awful idea, but since the Modularity guys clearly can't separate base runtime from build root, I guess we're faking multibuild packages now...

If you're going to do this instead of doing it the right way, you might as well add "Supplements: (python3-devel and rpm-build and redhat-rpm-config)" so that people don't have to notice you did a bad thing and made this.

Comment 4 Petr Šabata 2017-04-27 10:00:10 UTC
(In reply to Neal Gompa from comment #3)
> For what it's worth, this is an awful idea, but since the Modularity guys
> clearly can't separate base runtime from build root, I guess we're faking
> multibuild packages now...
> 
> If you're going to do this instead of doing it the right way, you might as
> well add "Supplements: (python3-devel and rpm-build and redhat-rpm-config)"
> so that people don't have to notice you did a bad thing and made this.

I might be missing quite a lot here.  Your comment was fairly vague.

Faking multibuild packages?  What do you mean by that?

What's wrong about separating language-specific dependency generators from a generic package build tool?  What's the right away in your book?  Or are you commenting on the transitive hard dependency on the generators, which is different from how it was done with Perl, for example?

While a reverse soft dependency wouldn't hurt anything, why would you want it?

Comment 5 Neal Gompa 2017-04-27 11:14:12 UTC
(In reply to Petr Šabata from comment #4)
> (In reply to Neal Gompa from comment #3)
> > For what it's worth, this is an awful idea, but since the Modularity guys
> > clearly can't separate base runtime from build root, I guess we're faking
> > multibuild packages now...
> > 
> > If you're going to do this instead of doing it the right way, you might as
> > well add "Supplements: (python3-devel and rpm-build and redhat-rpm-config)"
> > so that people don't have to notice you did a bad thing and made this.
> 
> I might be missing quite a lot here.  Your comment was fairly vague.
> 
> Faking multibuild packages?  What do you mean by that?
> 

multibuild is a concept that SUSE has with their OBS where you can have multiple spec files associated with a single set of sources, which build multiple source and binary packages[1]. Combined with source services, it's not completely terrible, though it can make it harder to build outside of OBS, depending on how it's done. This is a poor man's version of that, but it's too decoupled to be useful, since they must be manually updated independently, but in lockstep with each other.

[1]: http://openbuildservice.org/help/manuals/obs-reference-guide/cha.obs.multibuild.html

> What's wrong about separating language-specific dependency generators from a
> generic package build tool?  What's the right away in your book?  Or are you
> commenting on the transitive hard dependency on the generators, which is
> different from how it was done with Perl, for example?
> 

This particular dependency generator is part of RPM itself, and I as the upstream developer of it, prefer it to be maintained with RPM.

Additionally, The *only* reason this is being separated is because the modularity guys want base runtime = buildroot, which is a completely wrong characterization to begin with. They freaked out because python3-setuptools is a requirement for the new dependency generator, because otherwise there's no way to reliably parse the Python metadata.

I don't even agree with how we handled Perl, either. Perl dependency generators are still part of RPM, just like my Python one, and splitting them like this just promotes people monkey-patching and not upstreaming fixes.

> While a reverse soft dependency wouldn't hurt anything, why would you want
> it?

Because then when normal humans want to build Python packages, they don't have to care that this split happened. We probably have to edit all the Fedora packages anyway, because weak deps are disabled in mock builds. Either that, or python3-devel gains a Requires on python3-rpm-generators, which would allow us to not have to mass edit the spec files. I'd probably go for the latter, personally, because then it's a "nothing changes" scenario everywhere.

Comment 6 Petr Šabata 2017-04-27 13:15:09 UTC
(In reply to Neal Gompa from comment #5)
> (In reply to Petr Šabata from comment #4)
> > (In reply to Neal Gompa from comment #3)
> > > For what it's worth, this is an awful idea, but since the Modularity guys
> > > clearly can't separate base runtime from build root, I guess we're faking
> > > multibuild packages now...
> > > 
> > > If you're going to do this instead of doing it the right way, you might as
> > > well add "Supplements: (python3-devel and rpm-build and redhat-rpm-config)"
> > > so that people don't have to notice you did a bad thing and made this.
> > 
> > I might be missing quite a lot here.  Your comment was fairly vague.
> > 
> > Faking multibuild packages?  What do you mean by that?
> > 
> 
> multibuild is a concept that SUSE has with their OBS where you can have
> multiple spec files associated with a single set of sources, which build
> multiple source and binary packages[1]. Combined with source services, it's
> not completely terrible, though it can make it harder to build outside of
> OBS, depending on how it's done. This is a poor man's version of that, but
> it's too decoupled to be useful, since they must be manually updated
> independently, but in lockstep with each other.
> 
> [1]:
> http://openbuildservice.org/help/manuals/obs-reference-guide/cha.obs.
> multibuild.html

Associating sources with packages is done via the sources files in dist-git; multiple packages and branches can use the same sources.  I'm not familiar with SUSE so perhaps I'm misunderstanding; I'll read the link.  Thanks.

> > What's wrong about separating language-specific dependency generators from a
> > generic package build tool?  What's the right away in your book?  Or are you
> > commenting on the transitive hard dependency on the generators, which is
> > different from how it was done with Perl, for example?
> > 
> 
> This particular dependency generator is part of RPM itself, and I as the
> upstream developer of it, prefer it to be maintained with RPM.
> 
> Additionally, The *only* reason this is being separated is because the
> modularity guys want base runtime = buildroot, which is a completely wrong
> characterization to begin with. They freaked out because python3-setuptools
> is a requirement for the new dependency generator, because otherwise there's
> no way to reliably parse the Python metadata.
> 
> I don't even agree with how we handled Perl, either. Perl dependency
> generators are still part of RPM, just like my Python one, and splitting
> them like this just promotes people monkey-patching and not upstreaming
> fixes.

Ah, a clash of opinions :) I believe such generators should be maintained separately, in both downstream and upstream.  I supported the Perl change and I welcome this one as well.

I happen to be one of "the modularity guys" and while I definitely dislike the Python generators currently being part of rpm-build and depending on the full Python installation and yes, it was what, at least partially, ignited a brief discussion about this change, it's not the only reason here.  I dare to say the reason is that some people actually think it's a Good Thing going forward.  Again, I do, for instance.

> > While a reverse soft dependency wouldn't hurt anything, why would you want
> > it?
> 
> Because then when normal humans want to build Python packages, they don't
> have to care that this split happened. We probably have to edit all the
> Fedora packages anyway, because weak deps are disabled in mock builds.
> Either that, or python3-devel gains a Requires on python3-rpm-generators,
> which would allow us to not have to mass edit the spec files. I'd probably
> go for the latter, personally, because then it's a "nothing changes"
> scenario everywhere.

I see.

Yes, there are two main ways to do this.  One is to add the generators build dependency to every Python package (or more precisely, every Python package whose maintainer wishes to use this feature), which I think is more flexible and is what was done for Perl but would be a massive undertaking.  Another is to add that dependency to python*-devel which is what the Python team plans to do, from what I heard.  So it should be relatively painless.

Comment 7 Tomas Orsava 2017-05-02 14:38:33 UTC
> > > While a reverse soft dependency wouldn't hurt anything, why would you want
> > > it?
> > 
> > Because then when normal humans want to build Python packages, they don't
> > have to care that this split happened. We probably have to edit all the
> > Fedora packages anyway, because weak deps are disabled in mock builds.
> > Either that, or python3-devel gains a Requires on python3-rpm-generators,
> > which would allow us to not have to mass edit the spec files. I'd probably
> > go for the latter, personally, because then it's a "nothing changes"
> > scenario everywhere.
> 
> I see.
> 
> Yes, there are two main ways to do this.  One is to add the generators build
> dependency to every Python package (or more precisely, every Python package
> whose maintainer wishes to use this feature), which I think is more flexible
> and is what was done for Perl but would be a massive undertaking.  Another
> is to add that dependency to python*-devel which is what the Python team
> plans to do, from what I heard.  So it should be relatively painless.

Correct, that's exactly how we plan to implement this. Both python2-devel and python3-devel will have a Requires: python-rpm-generators.

And the python-rpm-generators will be a Python 3—only package, so in essence you'll need Python 3 in the buildroot if you will want to build a Python 2 package. (That is how it is now anyway, so it's essentially a no-change change for Python 2.)

Comment 8 Tomas Orsava 2017-05-03 15:56:25 UTC
============================================
=== I'm taking over this package request ===
============================================

Spec URL:
https://raw.githubusercontent.com/torsava/misc/master/fedora-package-request/python-rpm-generators/python-rpm-generators.spec

SRPM URL:
https://github.com/torsava/misc/raw/master/fedora-package-request/python-rpm-generators/python-rpm-generators-4.13.0.1-1.fc24.src.rpm

Patch files:
https://github.com/torsava/misc/tree/master/fedora-package-request/python-rpm-generators


Koji build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=19388660

Koji build in bootsrapping mode:
https://koji.fedoraproject.org/koji/taskinfo?taskID=19388667


Description:
This package provides scripts that analyse Python binary RPM packages and add
appropriate Provides and Requires tags to them.


Fedora Account System Username: torsava

Comment 10 Igor Gnatenko 2017-05-11 10:17:47 UTC
* Why subpackage is named as python3-? It doesn't make any sense for me.
* What about adding Conflicts for different versions of RPM? This will make sure that we update it along with RPM (since upstream is same).
* Why not to call package python-generators? Same way as we did it with perl.
* What's the migration plan? Will we add Requires to rpm-build or will ask packagers to add BuildRequires: python-generators or ...?

Comment 11 Igor Gnatenko 2017-05-11 10:18:32 UTC
Apart from few notes above, I have nothing to complain about.

Comment 12 Tomas Orsava 2017-05-11 12:00:56 UTC
(In reply to Igor Gnatenko from comment #10)
> * Why subpackage is named as python3-? It doesn't make any sense for me.

One RPM/subpackage can contain only Python 2 or Python 3 compiled files, not both at the same time. There was a discussion about providing both Python 2 and Python 3 subpackages, but with Python 2 EOL in sight we decided it wasn't necessary.

Instead `python2-devel` will depend on `python3-rpm-generators`, which will mean you will drag Python 3 into the buildroot to build Python 2 packages, however we decided that isn't an issue.


> * What about adding Conflicts for different versions of RPM? This will make
> sure that we update it along with RPM (since upstream is same).

I don't believe there really is a need to tightly couple them like so. It would complicate updating and building of both rpm and this package.


> * Why not to call package python-generators? Same way as we did it with perl.

There is a package called `python-rpm-macros` so I chose to follow it's naming precedent.


> * What's the migration plan? Will we add Requires to rpm-build or will ask
> packagers to add BuildRequires: python-generators or ...?

Currently all Python packages have to have a BuildRequires on pythonX-devel. So the migration plan is to add a Requires: python3-rpm-generators to both devel subpackages. That way nothing will change from the point of view of individual packages.

Comment 13 Igor Gnatenko 2017-05-15 12:30:34 UTC
> One RPM/subpackage can contain only Python 2 or Python 3 compiled files, not both at the same time. There was a discussion about providing both Python 2 and Python 3 subpackages, but with Python 2 EOL in sight we decided it wasn't necessary.

it was more about naming rather than content... if you would name `python-rpm-generators`, it would be still containing python3 compiled files..

Anyhow, naming is not something which could block review...


So, approving. Please, coordinate with RPM packaging this.

Comment 14 Tomas Orsava 2017-05-15 12:44:00 UTC
(In reply to Igor Gnatenko from comment #13)
> > One RPM/subpackage can contain only Python 2 or Python 3 compiled files, not both at the same time. There was a discussion about providing both Python 2 and Python 3 subpackages, but with Python 2 EOL in sight we decided it wasn't necessary.
> 
> it was more about naming rather than content... if you would name
> `python-rpm-generators`, it would be still containing python3 compiled
> files..

Oh I see! I agree with you that the python- prefix would be better in this particular case, it would break the Fedora Packaging Guidelines for Python which mandate that the `python-` prefix is reserved only for Python 2 packages until the default is switched to Python 3 sometime in the future (likely 2020). 

> 
> 
> So, approving. Please, coordinate with RPM packaging this.

Thanks! When the package gets to the repos, I'll follow up with a BZ and a patch for the `rpm` package as well as the Python 2 and 3 packages.

Comment 15 Gwyn Ciesla 2017-05-15 13:03:10 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/python-rpm-generators

Comment 16 Tomas Orsava 2017-05-15 14:54:04 UTC
Build in rawhide:

https://koji.fedoraproject.org/koji/taskinfo?taskID=19570035


Will close the issue when the python2, python3 and rpm packages are updated as well.

Comment 17 Panu Matilainen 2017-05-23 09:43:00 UTC
The coordination on this thing could've gone better: I only discovered this initiative has moved forward due to my rawhide builds in COPR failing because of implicit file conflicts on the generators.

Python generators ripped out of rpm as of rpm-4.13.0.1-22.fc27.

Comment 18 Tomas Orsava 2017-05-23 13:24:46 UTC
I was just about to send a BZ with a patch to excise the generators out of rpm. I was waiting until now so that the python-rpm-generators and the added dependencies to python2-devel and python3-devel propagate to all the buildroots.

However, I've tested it several times and the packages do not conflict because the content of the files is identical. The python3-rpm-generators package only conflicts with older versions of RPM (rpm-build < 4.13.0.1-2) because there the files have a different content.

Can you show me the COPR repo where you got the failures? There must be something going on.

Comment 19 Panu Matilainen 2017-05-26 07:48:22 UTC
My COPR repo has upstream git master version of rpm which differs from what Fedora ships.

It's much better to be up-front about such things, dropping a quick note or just adding us to CC of this bug would've made a worlds of difference and would've taken much less of your time than this "aftermath". Doing it like this felt - wholly unnecessarily - like trying to sneak something in behind our back. But what's done is done, lets move on...

Comment 20 Tomas Orsava 2017-05-26 14:25:13 UTC
Oh, I apologise if it felt that way, that was not my intention at all. I thought the course of action was decided in the rpm BZ#1410631, but now I realize that it took some time before I had time to implement it and I could have posted an update there that the work is on the way.


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