Bug 1410403
Summary: | fedpkg-copr: don't depend on fedpkg | ||
---|---|---|---|
Product: | [Community] Copr | Reporter: | Pavel Raiskup <praiskup> |
Component: | backend | Assignee: | clime |
Status: | CLOSED CURRENTRELEASE | QA Contact: | |
Severity: | unspecified | Docs Contact: | |
Priority: | unspecified | ||
Version: | unspecified | CC: | clime, praiskup |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | If docs needed, set a value | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2017-08-03 13:08:15 UTC | Type: | Bug |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Pavel Raiskup
2017-01-05 12:23:06 UTC
(In reply to Pavel Raiskup from comment #0) > we can easily differ from Fedora specific guidelines (e.g. dist-git branch > names). Read that rather like Fedora/Koji-specific build-system _configuration_, this has nothing to do with Fedora Packaging Guidelines. Well, dropping fedpkg means dropping load_rpmdefines functionality that we inherit in fedpkg-copr - that means dropping support for Fedora dist-git branch naming. For COPR deployments that currently use Fedora branch naming (like Fedora COPR), that would be a problem. (In reply to clime from comment #2) > Well, dropping fedpkg means dropping load_rpmdefines functionality that we > inherit in fedpkg-copr load_rpmdefines() is also implemented in pyrpkg, and sounds like there's only one (pretty small) difference in it's output: | - "--define 'dist .%s'" % self._disttag, | + "--define 'dist .%s'" % self.dist, | "--define '%s %s'" % (self._distvar, | - self._distval.split('_')[0]), | - # int and float this to remove the decimal | - "--define '%s 1'" % self._disttag] | + self._distval), | + "--eval '%%undefine %s'" % self._distunset, | + "--define '%s 1'" % self.dist] The 'self.dist' is trivial to compute, I think in Copr we already re-calculate this stuff at least for mageia. (In reply to clime from comment #2) > For COPR deployments that currently use Fedora branch naming (like > Fedora COPR), that would be a problem. I think it is useful for Fedora Copr (such deployment) to not depend on fedpkg with this regard; so such deployments would benefit from re-implementing the "branch to dist" mapping, so it is immutable by fedpkg folks. Does that make sense ^^, so should I assign this bug to myself and propose PR (possibly fix the https://pagure.io/copr/copr/pull-request/11)? (In reply to Pavel Raiskup from comment #3) > (In reply to clime from comment #2) > > Well, dropping fedpkg means dropping load_rpmdefines functionality that we > > inherit in fedpkg-copr > > load_rpmdefines() is also implemented in pyrpkg, and sounds like there's only > one (pretty small) difference in it's output: load_rpmdefines in fedpkg is pretty much completely reimplemented (extended a lot if compared to pyrpkg) containing definitions for various Fedora dist-git branches such as 'f\d\d$' or 'el\d$'. This is what we need currently for fedpkg-copr to operate correctly in copr-backend when Fedora dist-git naming is used. >I think it is useful for Fedora Copr (such deployment) to not depend on >fedpkg with this regard; so such deployments would benefit from >re-implementing the "branch to dist" mapping, so it is immutable by fedpkg >folks. I am not getting this. Do you plan to reimplement what is in fedpkg into our dist-git-client (called fedpkg-copr currently)? (In reply to clime from comment #4) > load_rpmdefines in fedpkg is pretty much completely reimplemented > (extended a lot if compared to pyrpkg) containing definitions for > various Fedora dist-git branches such as 'f\d\d$' or 'el\d$'. This is > what we need currently for fedpkg-copr to operate correctly in > copr-backend when Fedora dist-git naming is used. But just for "branch -> dist" mapping (we don't need anything else!), the dependency on fedpkg is IMO too much (taking into account that we reimplement the mapping anyway, even though "only" temporarily). The branching policy in "general" Copr deployment is different topic from branching in Fedora. > I am not getting this. Do you plan to reimplement what is in fedpkg into > our dist-git-client (called fedpkg-copr currently)? I think some light-weight version of it, yes. To have only that part which we necessarily need and drop the 'fedpkg' requirement. Maybe, if you consider dependency on 'fedpkg' important -- we could inherit from 'pyrpkg' classes (and inherit from 'fedpkg' classes only optionally). I believe that 'fedpkg' is *not* desired to be library at all, btw. There's actually only one version of fedpkg (no python2 and python3 version), which might look like there's need for completely different pythonX-fedpkg library providing "only" the actual fedoras "branch -> dist" mapping. But still, this should be optional for Copr. Ah, pyrpkg is python2 only, too. But that's supposed to be library. (In reply to Pavel Raiskup from comment #5) I am not getting this. Do you plan to reimplement what is in fedpkg into > > our dist-git-client (called fedpkg-copr currently)? > > I think some light-weight version of it, yes. To have only that part which > we > necessarily need and drop the 'fedpkg' requirement. I don't really see a reason for dropping fedpkg. I do not consider its API to be that unstable to necessitate a change. That's not really API though, and it brings just constraints (zero additional functionality from Copr POV, no flexibility), e.g. 'master' == rawhide. Without pushing you somewhere you don't want to be, is it OK to make fedpkg "optional" dependency (installed by ansible instead of Requires)? Also note that 'mga' proposal in https://pagure.io/fedpkg/pull-request/25 goes totally out of scope fedpkg -- I would be really surprised if there was small motivation for merge. Fedpkg is really Fedora oriented, I would expect something like 'mgapkg' tool. (In reply to Pavel Raiskup from comment #9) > That's not really API though, and it brings just constraints (zero > additional functionality from Copr POV, no flexibility), e.g. 'master' == > rawhide. > > Without pushing you somewhere you don't want to be, is it OK to make > fedpkg "optional" dependency (installed by ansible instead of Requires)? I don't get the point of the change. We need functionality that we inherit from fedpkg because of the support for Fedora named branches and to duplicate code that is already there seems not to be great. (In reply to clime from comment #11) > I don't get the point of the change. We need functionality that we > inherit from fedpkg because of the support for Fedora named branches That functionality is needed solely for one instance Fedora Copr (and I bet even there it is temporary anyways, with the huge containerization/taskotron/modularity efforts I would be surprised if the branching scheme survived forever). > and to duplicate code that is already there seems not to be great. Ok, but we duplicate it anyways (temporarily) but the PR is there for a long time already.. --- Argh, I hit "submit" too early .. I agree that duplicating the functionality is a bad idea. But as no other Copr instance will benefit from 'fedpkg' dependency, could I hack on making it optional? (In reply to Pavel Raiskup from comment #13) > Argh, I hit "submit" too early .. I agree that duplicating the functionality > is a bad idea. But as no other Copr instance will benefit from 'fedpkg' > dependency, could I hack on making it optional? Do as you like. I think much better direction is to try to employ `mock --buildsrpm` and get rid of the whole fedpkg-copr, fedpkg, pyrpkg stack at once while keeping support for Fedora named branches. This is probably what I will look at, if noone else will be faster, because I find it very tempting to build srpm in that same chroot where the rpm is built, instead of building it outside of the chroot and then passing it into the chroot for rpm-building as is the case now. Even if that didn't work, removing fedpkg dependency from fedpkg-copr package is not bringing anything and I would suggest to make a new script aside as a proof of concept that in some way (re-)implements fedpkg-copr's functionality as it stands now and is used by copr-backend to obtain srpms from copr-dist-git. (In reply to clime from comment #14) > I think much better direction is to try to employ `mock --buildsrpm` and > get rid of the whole fedpkg-copr, fedpkg, pyrpkg stack at once while > keeping support for Fedora named branches. At least 'pyrpkg' is still needed to clone the package and download sources. > This is probably what I will look at, if noone else will be faster, > because I find it very tempting to build srpm in that same chroot where > the rpm is built, instead of building it outside of the chroot and then > passing it into the chroot for rpm-building as is the case now. Note that "minimal" buildroot might be modified (explicitly added packages by copr maintainer. Also is "clean" needed between mock "--buildsrpm" and "--rebuild"? If yes, it will take additional time to generate source rpm, and if not, one should ensure that the mock chroot is not modified some ugly way so --rebuild goes to FTBFS. I pretty much like your idea about 'mock --buildsrpm' from long term perspective -- we should generate one SRPM for e.g. Fedora 25 and then build this SRPM against all Fedora 25 architectures enabled in copr (aka koji/brew style). > Even if that didn't work, removing fedpkg dependency from fedpkg-copr > package is not bringing anything and I would suggest to make a new > script aside as a proof of concept that in some way (re-)implements > fedpkg-copr's functionality as it stands now and is used by copr-backend > to obtain srpms from copr-dist-git. Good idea. Let's call that script 'copkg'. PoC: https://github.com/praiskup/copr-dist-git-client-poc You can try e.g. this against Fedora copr: ./prep-srpm --config config-copr.sh --module praiskup/ping/quick-package --revision=5ce6ec838e22df0c5dfbb3ca042347a04829f5c1 Note that I'm not sure about %dist and %fedora/%rhel/.. macros. If there was usage for them, it is trivial to implement the mapping according to our needs. (In reply to clime from comment #14) > I would suggest to make a new script aside as a proof of concept that in some > way (re-)implements fedpkg-copr's functionality as it stands now and is used > by copr-backend to obtain srpms from copr-dist-git. Here is another attempt, btw. using 'mock --buildsrpm' as suggested in previous discussion [1]. I kind of dislike this fact, but as long as we support mageia (and they did some unfortunate decisions complicating generating of source RPMs on non-mageia boxes [2] -- I guess that's distro bug), the 'mock --buildsrpm' is needed (and used ATM in the concept). There's one neat thing about this copr-builder script: When we use it together with [3] pull request, we have live build logs almost for free :). That's because with builder-side script it is much easier to work with mock to produce readable logs. There are some incompatibility changes like moving from 'mockchain' to 'mock' (I'm still not sure whether this is needed) and using 'copr mock-config' feature to get the mock configuration (so at least [4] is needed too). I'll cover pros/cons in upcoming pull-request (once done) against copr-builder which should (at least optionally) utilize this script. [1] https://github.com/praiskup/copr-bulider [2] https://pagure.io/copr/copr/c/13b53c9d6706 [3] https://pagure.io/copr/copr/pull-request/44 fedpkg-copr is no longer used in COPR and has been deprecated as a package. |