python-distutils-ng (what a disgusting name...) installs compiled files in src_install() making them owned by PM. The python.eclass used to create them during postinst(). Thus, when migrating an ebuild from latter to the former, one forces all users to get a lot of conflicts.
I like the name. I've got answer from Zac when I asked about this: """ However, FEATURES="protect-owned" is enabled by default, and it will work fine if the .pyo and .pyc files are not owned by anything. I don't know why people use FEATURES="collision-protect", but at least you can tell them that they'll still have a reasonable level of protection from FEATURES="protect-owned" (it protect against file collisions between packages). """ I'll add a warning message to eclass when it detects collision-protect in FEATURES. And in my personal opinion: previous behavior that nobody owned the files, but "somehow" package manager touched/removed them sucked.
Portage does spit out a pretty horrid warning, even with collision-protect disabled.
(In reply to comment #2) > Portage does spit out a pretty horrid warning, even with collision-protect > disabled. Yes, I'm aware of this. On the positive side it's only one time warning, with "protect-owned" portage does not abort the process. I'd love to have something along 'uninstall previous version *before* installing new one'.
I've modified the eclass to die with better error than plain collision error: Index: python-distutils-ng.eclass =================================================================== RCS file: /var/cvsroot/gentoo-x86/eclass/python-distutils-ng.eclass,v retrieving revision 1.9 diff -u -r1.9 python-distutils-ng.eclass --- python-distutils-ng.eclass 30 Mar 2012 16:41:40 -0000 1.9 +++ python-distutils-ng.eclass 3 Apr 2012 19:12:30 -0000 @@ -51,7 +51,7 @@ # Set the value to "yes" to skip compilation and/or optimization of Python # modules. -EXPORT_FUNCTIONS src_prepare src_configure src_compile src_test src_install +EXPORT_FUNCTIONS pkg_pretend src_prepare src_configure src_compile src_test src_install case "${EAPI}" in 0|1|2|3) @@ -288,6 +288,17 @@ fi } +# Phase function: pkg_pretend +python-distutils-ng_pkg_pretend() { + if has "collision-protect" ${FEATURES}; then + eerror "Due to previous eclass compiling Python files outside of src_install" + eerror "(and not recording resulting .pyc and .pyo files as owned by any package)" + eerror "merging this package with \"collision-protect\" in FEATURES will result" + eerror "in an error, please switch to using \"protect-owned\" instead." + die "\"collision-protect\" in FEATURES detected" + fi +} + # Phase function: src_prepare python-distutils-ng_src_prepare() { [[ "${PYTHON_OPTIONAL}" = "yes" ]] && { use python || return; }
Erm, and you just changed the public API of the eclass. That's not a good eclass quality indicator.
(In reply to comment #5) > Erm, and you just changed the public API of the eclass. That's not a good > eclass quality indicator. I think you're overreacting - the eclass is very young, it's used by two ebuilds in the tree (few more outside of it). Also it's not that once written you can't change anything in eclass, they are not set in stone -- eclasses will change over time.
Nothing of any significance is actually using this eclass yet, so not a big deal. Having a pkg_pretend check for every python package is probably not the greatest idea, however.
(In reply to comment #7) > Nothing of any significance is actually using this eclass yet, so not a big > deal. > > Having a pkg_pretend check for every python package is probably not the > greatest idea, however. I agree Mike, we can remove this check when majority of packages have been migrated and number of affected users would be small. On the other hand this check is pretty fast and shouldn't be very inconvenient.
(In reply to comment #8) > (In reply to comment #7) > > Nothing of any significance is actually using this eclass yet, so not a big > > deal. > > > > Having a pkg_pretend check for every python package is probably not the > > greatest idea, however. > > I agree Mike, we can remove this check when majority of packages have been > migrated and number of affected users would be small. On the other hand this > check is pretty fast and shouldn't be very inconvenient. Time it. This isn't going to be cheap. Reopening; a plan of "make pkg_pretend explode across an eclass transition" is extremely unfriendly; nor should your plan be "harass people to use a different features functionality". http://devmanual.gentoo.org/ebuild-writing/eapi/index.html, weak blocking file collisions is a potential resolution, although that's not guranteed by PMS... Also, just a sidenote; one of the major faults w/ the python eclass was stupid shit being pulled where hacks were jammed in; this reeks of a lesser form of that mess. Really, really would like to see the new work not wind up replicating the same shortsightedness python.eclass displayed (like your new work, but not liking this resolution to say the least).
(In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #7) > > > Nothing of any significance is actually using this eclass yet, so not a big > > > deal. > > > > > > Having a pkg_pretend check for every python package is probably not the > > > greatest idea, however. > > > > I agree Mike, we can remove this check when majority of packages have been > > migrated and number of affected users would be small. On the other hand this > > check is pretty fast and shouldn't be very inconvenient. > > Time it. This isn't going to be cheap. Ok (test environment: dual core mobile pentium SU4100 1.3GHz, 3 GB of memory, WDC 5400rpm SATA disk): With the check: nelchael@s-lappy ~/.../dev-python/python-gflags$ for i in {1..9}; do echo -n "${i} >>> "; ( time ebuild python-gflags-2.0.ebuild manifest clean install > /dev/null ) 2>&1 | grep real; done 1 >>> real 0m6.210s 2 >>> real 0m6.132s 3 >>> real 0m6.128s 4 >>> real 0m6.296s 5 >>> real 0m6.171s 6 >>> real 0m6.141s 7 >>> real 0m6.259s 8 >>> real 0m6.125s 9 >>> real 0m6.135s AVERAGE = 6.177 Without the check: nelchael@s-lappy ~/.../dev-python/python-gflags$ for i in {1..9}; do echo -n "${i} >>> "; ( time ebuild python-gflags-2.0.ebuild manifest clean install > /dev/null ) 2>&1 | grep real; done 1 >>> real 0m6.268s 2 >>> real 0m6.135s 3 >>> real 0m6.191s 4 >>> real 0m6.189s 5 >>> real 0m6.130s 6 >>> real 0m6.126s 7 >>> real 0m6.221s 8 >>> real 0m6.140s 9 >>> real 0m6.111s AVERAGE = 6.168 For this run it's 9 ms slower than with the check (0.146%), for some other runs the version with the check was faster (!). For other ebuilds when there's actually something to build it will be even less significant. > Reopening; a plan of "make pkg_pretend explode across an eclass transition" > is extremely unfriendly; nor should your plan be "harass people to use a > different features functionality". > > http://devmanual.gentoo.org/ebuild-writing/eapi/index.html, weak blocking > file collisions is a potential resolution, although that's not guranteed by > PMS... I'm not harassing anyone, it's your opinion. Please reopen *with a patch* - I'd appreciate any real help with this issue. > Also, just a sidenote; one of the major faults w/ the python eclass was > stupid shit being pulled where hacks were jammed in; this reeks of a lesser > form of that mess. Really, really would like to see the new work not wind > up replicating the same shortsightedness python.eclass displayed (like your > new work, but not liking this resolution to say the least). I couldn't be more clear on this: my main goal is simplicity of eclass and ebuilds using it (even if it leads to lack of functionality in some areas). Also keep in mind that this is not a "feature" that I've added happily, it's just trying to work around the issue created by previous eclass.
(In reply to comment #10) <snip a lot of statistics that aren't actually measuring the point> > For other ebuilds when > there's actually something to build it will be even less significant. You're not actually testing anything of value; you're contrasting the extra cost of parsing the function for a *metadata generation*, rather than timing the cost of invoking pkg_pretend (single threaded) for each eclass consumer *during resolution of all future versions of those eclass consumers*; that means what was previously pure python/C++, now has to shell out to bash for every version. That is not cheap. You didn't actually test what I pointed out. If you want to argue against what I stated, please test what I actually stated. > > Reopening; a plan of "make pkg_pretend explode across an eclass transition" > > is extremely unfriendly; nor should your plan be "harass people to use a > > different features functionality". > > > > http://devmanual.gentoo.org/ebuild-writing/eapi/index.html, weak blocking > > file collisions is a potential resolution, although that's not guranteed by > > PMS... > > I'm not harassing anyone, it's your opinion. Please reopen *with a patch* - > I'd appreciate any real help with this issue. It's not my opinion; you're proposing a pathway that involves a shitton of collisions for all pkgs crossing that version gap. It's a QA matter, not opinion. Depending on the view of weak blockers allowing collisions (vs PMS definition that says nothing of the sort), there is a way around it. Sticking the head in the sand and going "that's just like, your opinion, man" doesn't fly. > > Also, just a sidenote; one of the major faults w/ the python eclass was > > stupid shit being pulled where hacks were jammed in; this reeks of a lesser > > form of that mess. Really, really would like to see the new work not wind > > up replicating the same shortsightedness python.eclass displayed (like your > > new work, but not liking this resolution to say the least). > > I couldn't be more clear on this: my main goal is simplicity of eclass and > ebuilds using it (even if it leads to lack of functionality in some areas). > Also keep in mind that this is not a "feature" that I've added happily, it's > just trying to work around the issue created by previous eclass. Like it or not, you have to deal with this. It sucks I know- but your course you're attempting isn't going to fly and I'm trying *nicely* to point that out (and give you alternatives). Leave it open, and get QA to sign off on tree wide collisions if you really want to force the FEATURE check (which is bad QA anyways since FEATURES shouldn't be relied upon like this); else look into the weak blocker route. With respect, this isn't a "I'm just going to do what I intend unless you give a patch"- you want to have pyc generated during src_install (which I'm heavily in favor for- pushed for mtime in eapi3 after all), you need to sort the issues *prior*, not break it and say "submit a patch".
(In reply to comment #11) > (In reply to comment #10) > <snip a lot of statistics that aren't actually measuring the point> Ok. > > For other ebuilds when > > there's actually something to build it will be even less significant. > > You're not actually testing anything of value; you're contrasting the extra > cost of parsing the function for a *metadata generation*, rather than timing > the cost of invoking pkg_pretend (single threaded) for each eclass consumer > *during resolution of all future versions of those eclass consumers*; that > means what was previously pure python/C++, now has to shell out to bash for > every version. That is not cheap. > > You didn't actually test what I pointed out. If you want to argue against > what I stated, please test what I actually stated. Hmm.. how I'm supposed to test "*all future* versions of those eclass consumers"? Tests like above allow some estimates. > > > Reopening; a plan of "make pkg_pretend explode across an eclass transition" > > > is extremely unfriendly; nor should your plan be "harass people to use a > > > different features functionality". > > > > > > http://devmanual.gentoo.org/ebuild-writing/eapi/index.html, weak blocking > > > file collisions is a potential resolution, although that's not guranteed by > > > PMS... > > > > I'm not harassing anyone, it's your opinion. Please reopen *with a patch* - > > I'd appreciate any real help with this issue. > > It's not my opinion; you're proposing a pathway that involves a shitton of > collisions for all pkgs crossing that version gap. > > It's a QA matter, not opinion. I was referring to your expression of "harass people", do you see the eerror & die as a harassment of people? "Harassment" has very strict and definitive meaning. > Depending on the view of weak blockers allowing collisions (vs PMS > definition that says nothing of the sort), there is a way around it. > > Sticking the head in the sand and going "that's just like, your opinion, > man" doesn't fly. I'm not doing that, I suggested a solution (or according to you "harassment"), you disagree - you have right to do so, I'm fine with it. > > > Also, just a sidenote; one of the major faults w/ the python eclass was > > > stupid shit being pulled where hacks were jammed in; this reeks of a lesser > > > form of that mess. Really, really would like to see the new work not wind > > > up replicating the same shortsightedness python.eclass displayed (like your > > > new work, but not liking this resolution to say the least). > > > > I couldn't be more clear on this: my main goal is simplicity of eclass and > > ebuilds using it (even if it leads to lack of functionality in some areas). > > Also keep in mind that this is not a "feature" that I've added happily, it's > > just trying to work around the issue created by previous eclass. > > Like it or not, you have to deal with this. It sucks I know- but your > course you're attempting isn't going to fly and I'm trying *nicely* to point > that out (and give you alternatives). I disagree with "nicely" in the above sentence, but it's just my opinion. > Leave it open, and get QA to sign off on tree wide collisions if you really > want to force the FEATURE check (which is bad QA anyways since FEATURES > shouldn't be relied upon like this); else look into the weak blocker route. To quote you from comment #9: "weak blocking file collisions is a potential resolution, although that's not guranteed by PMS" So does it work or not? > With respect, this isn't a "I'm just going to do what I intend unless you > give a patch"- you want to have pyc generated during src_install (which I'm > heavily in favor for- pushed for mtime in eapi3 after all), you need to sort > the issues *prior*, not break it and say "submit a patch". Two things: - I didn't break anything, at least yet - I'd bet that time wasted on this bug by both of us greatly exceeds how long it would take to create a patch I've tried adding "!!<${CATEGORY}/${PF}" to DEPEND in ebuild, and it still dies with collision error. It's clear I'm misunderstanding you or the solution doesn't work - can you help me with this?
(In reply to comment #12) > Hmm.. how I'm supposed to test "*all future* versions of those eclass > consumers"? Tests like above allow some estimates. As far as I know, your intent is to replace python.eclass w/ your work. If that *is* the intent, then you could just mangle python eclass and stick a pkg_pretend into it for EAPI=4, and time emerge resolutions (specifically -e) for before/after. Add the basic 'has $FEATURES blah' bit (since has's implementation can be linear and slow as hell depending on the PM) to get some actual execution; from there, time before and after. Also... and this just dawned on me; the pkg_pretend route explicitly requires all consumers of that eclass to be EAPI=4 (since anything else would allow arbitrary explosions for people crossing the version where the eclass shifted from python to yours). You sure you've thought that fully through in terms of implications for the ebuilds/deps, and interaction w/ other eclasses? > I was referring to your expression of "harass people", do you see the eerror > & die as a harassment of people? "Harassment" has very strict and definitive > meaning. Sorry; 'harass' is accurate, but you're reading it more strongly than I intended. I say 'harass' here because it basically blows up resolutions/merges in peoples face if they have the feature enabled. Doing what you're suggesting effectively would kill off FEATURES=collision-protect usage, at best resulting in people doing FEATURES=protect-owned (which I suspect has worse performance also). I say 'effectively' because ebuilds would gradually migrate to the new eclass- meaning during week 1, a user gets told "suppress collision-protect" via a blown up resolution; week 2, a new pkg has gone to your eclass, they get the same thing. Week 3, same thing. You get the idea. It would get annoying *very quickly* from a user experience standpoint, eventually forcing the user to switch to protect-owned in full rather than trying to run collision-protect (note: devs should be running collision-protect rather than protect-owned for QA reasons). > > Like it or not, you have to deal with this. It sucks I know- but your > > course you're attempting isn't going to fly and I'm trying *nicely* to point > > that out (and give you alternatives). > > I disagree with "nicely" in the above sentence, but it's just my opinion. Well, relatively 'nicely'. Not trying to be a dick, just trying to ensure this doesn't get overlooked/swept under the rug and is properly dealt with. > > Leave it open, and get QA to sign off on tree wide collisions if you really > > want to force the FEATURE check (which is bad QA anyways since FEATURES > > shouldn't be relied upon like this); else look into the weak blocker route. > > To quote you from comment #9: > "weak blocking file collisions is a potential resolution, although that's > not guranteed by PMS" > > So does it work or not? Probably for portage, but probably not for paludis/pkgcore; the kicker there is pkgcore/paludis are adhering to the spec. The question there is whether or not the spec should be updated w/ that requirement- if so, you can use that route to deal with your issue. If not, then another solution is needed. > Two things: > - I didn't break anything, at least yet > - I'd bet that time wasted on this bug by both of us greatly exceeds how > long it would take to create a patch 1) If it were simple, you'd have either a "go do xyz" set of instructions from me, or a patch. 2) It *will* break shit, and the plan of moving a sizable chunk of the tree to this eclass means it needs being dealt with. We can't wait till it's an issue, need to have a sane plan there (and yes, I'm aware that sucks to do- that said the work is worth the effort since it increases python.eclass deprecation). > I've tried adding "!!<${CATEGORY}/${PF}" to DEPEND in ebuild, and it still > dies with collision error. Should be RDEPEND offhand; suggest pinging zmedico and getting his comments/confirmation of weak blocker behaviour there (devmanual was written against portage behaviour). > It's clear I'm misunderstanding you or the solution doesn't work - can you > help me with this? I'm a bit laggy right now, but I'm willing to help where I can. :)
Could you remove this 'die' idiocy? I can handle collisions myself. What I hate, is ebuild telling me to disable collision-protect when I already removed colliding files. And you are now forcing every user out there to disable it to use your eclass.
Ah, and please use the mailing list to send changes to eclass for review. So people can react before they end up in a bad shithole.
FYI: after discussion on gentoo-dev the eclass now has ewarn instead of eerror/die.
This should fix it: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=33545ea18e8816addb3c54bb26a0cc788b8512e6
Thank you Zac for committing this, when it gets included in portage release I'll add !<sys-apps/portage-2.1.10.xx to DEPEND to eclass.
I've combined the fnmatch support into the COLLISION_IGNORE variable, so we only need one variable: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=bc18e1e7c3af475412e997489058c58942ebfdcb
This is released for testing in 2.2.0_alpha102.
(In reply to comment #20) > This is released for testing in 2.2.0_alpha102. Do you plan on backporting those changes to 2.1.10.x?
Yes, it will be in portage-2.1.10.58 which I plan to release in about 2 weeks.
(In reply to comment #22) > Yes, it will be in portage-2.1.10.58 which I plan to release in about 2 > weeks. Awesome! I'll add !<2.1.10.58 to DEPEND then.
It's released in portage-2.1.10.58 now.
I'd like to add following code to eclass, does it look ok to you? $ cvs diff python-distutils-ng.eclass Index: python-distutils-ng.eclass =================================================================== RCS file: /var/cvsroot/gentoo-x86/eclass/python-distutils-ng.eclass,v retrieving revision 1.19 diff -u -r1.19 python-distutils-ng.eclass --- python-distutils-ng.eclass 6 May 2012 03:20:45 -0000 1.19 +++ python-distutils-ng.eclass 14 May 2012 19:18:45 -0000 @@ -66,6 +66,8 @@ die "Unsupported EAPI=${EAPI} (unknown) for python-distutils-ng.eclass" ;; esac +DEPEND="${DEPEND} !<sys-apps/portage-2.1.10.58" + # @FUNCTION: _python-distutils-ng_get_binary_for_implementation # @USAGE: implementation # @RETURN: Full path to Python binary for given implementation.
(In reply to comment #25) > I'd like to add following code to eclass, does it look ok to you? That looks fine.
(In reply to comment #26) > (In reply to comment #25) > > I'd like to add following code to eclass, does it look ok to you? > > That looks fine. Thank you for the review, I've just committed the code -- I think this solves the bug :)
