Summary: | remove EMERGE_FROM in EAPI=4 | ||
---|---|---|---|
Product: | Portage Development | Reporter: | Brian Harring (RETIRED) <ferringb> |
Component: | Core - Ebuild Support | Assignee: | Portage team <dev-portage> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | mgorny, pms, qa |
Priority: | Normal | ||
Version: | unspecified | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Package list: | Runtime testing required: | --- | |
Bug Depends on: | |||
Bug Blocks: | 358927 | ||
Attachments: |
supress EMERGE_FROM in ebuild/eclass where MERGE_TYPE is available
supress EMERGE_FROM in ebuild/eclass where MERGE_TYPE is available use ebuild_phase_with_hooks more, and fix source_all_bashrcs make repoman disallow EMERGE_FROM in EAPI 4 (like AA and KV) List EMERGE_ as unspecified variables in PMS |
Description
Brian Harring (RETIRED)
![]() I'd like to keep EMERGE_FROM, at least for the near future, because I'd rather not deal with the fallout of user bashrc breakage. It's just not worth the trouble. Changing stuff like this just wastes people's time. If you're concerned about user bashrcs, then export it only *there*. Stop exporting it into ebuild envs leading to compliant PM's breaking because they're not matching portage misbehaviour. This is ignoring that portage looks to export MERGE_TYPE for all EAPIs anyways (meaning they just have to change the var they access). As I've said, I don't care about EAPI<4 usage of EMERGE_FROM- there wasn't an alternative. EAPI>=4 however, continuing this basically dicks the other managers over- I noticed this due to a user mapping back to it. Either way, do what you want in user bashrc env- but stop enabling lock in for EAPI>=4 ebulds/eclasses. (In reply to comment #2) > If you're concerned about user bashrcs, then export it only *there*. Stop > exporting it into ebuild envs leading to compliant PM's breaking because > they're not matching portage misbehaviour. That's an interesting idea. However, the pre/post phase hooks mix things up a bit. This makes it somewhat ugly if we're going to toggle the variable every time that we call a bashrc hook. (In reply to comment #3) > (In reply to comment #2) > > If you're concerned about user bashrcs, then export it only *there*. Stop > > exporting it into ebuild envs leading to compliant PM's breaking because > > they're not matching portage misbehaviour. > > That's an interesting idea. However, the pre/post phase hooks mix things up a > bit. This makes it somewhat ugly if we're going to toggle the variable every > time that we call a bashrc hook. Portage exports MERGE_TYPE regardless of EAPI (which frankly is a bit wrong, but whatever), so that point is moot- they have a variable to rely on. Regardless... I don't give a damn what you export to hooks. I care what you export to ebuild executed code, so your point doesn't apply- do what you want in your hooks (user code), ebuild/eclass code was the point. Few things; 1) if you had issues in naming, you should've brought it up during when it was proposed- you were involved, 2) why are you dragging your feet on *user* configuration- which you can do whatever the hell you want with- when the issue is "remove EMERGE_FROM out of EAPI>=4 ebuild environments"? Your options are there, but best I can tell, you just don't want to fix it- however it has to be fixed else it's bluntly, lock in (and breakages for the alternate managers). Also... what actually is *using* those hook points and is EMERGE_FROM sensitive? by the time of pkg_preinst, the source of the pkg doesn't matter to portage (it's already unpacked to disk) thus there really isn't any swivelling I could see being sane/usable there. Actual examples? (In reply to comment #4) > Portage exports MERGE_TYPE regardless of EAPI (which frankly is a bit wrong, > but whatever), so that point is moot- they have a variable to rely on. No, it's filtered by the config.environ() method: if not eapi_exports_merge_type(eapi): mydict.pop("MERGE_TYPE", None) > Few things; 1) if you had issues in naming, you should've brought it up during > when it was proposed- you were involved, 2) why are you dragging your feet on > *user* configuration- which you can do whatever the hell you want with- when > the issue is "remove EMERGE_FROM out of EAPI>=4 ebuild environments"? EMERGE_FROM and MERGE_TYPE are not mutually exclusive, and I see no reason to force such a constraint. > Your options are there, but best I can tell, you just don't want to fix it- > however it has to be fixed else it's bluntly, lock in (and breakages for the > alternate managers). I've already explained that it's a compatibility issue with user's bashrcs. (In reply to comment #5) > Also... what actually is *using* those hook points and is EMERGE_FROM > sensitive? by the time of pkg_preinst, the source of the pkg doesn't matter to > portage (it's already unpacked to disk) thus there really isn't any swivelling > I could see being sane/usable there. > > Actual examples? Well, pkg_setup is historically a problematic area, since it's called at the beginning of a source build and also at the beginning of a binary package install. So, user bashrc pre/post pkg_setup hooks would be a particular area of concern. (In reply to comment #6) > (In reply to comment #4) > > Portage exports MERGE_TYPE regardless of EAPI (which frankly is a bit wrong, > > but whatever), so that point is moot- they have a variable to rely on. > > No, it's filtered by the config.environ() method: > > if not eapi_exports_merge_type(eapi): > mydict.pop("MERGE_TYPE", None) Code rant follows: the scheduler sets both by default, doebuild sets both if EAPI>=4 (and should be setting only one in that case), and config environ pops MERGE_TYPE if EAPI<4. Also, if invoked via ebuild, both are set for EAPI=4, but none are set for EAPI<4... fun. > > Your options are there, but best I can tell, you just don't want to fix it- > > however it has to be fixed else it's bluntly, lock in (and breakages for the > > alternate managers). > > I've already explained that it's a compatibility issue with user's bashrcs. It isn't; you can still export whatever vars you want for the hook invocations- user code. This is about ebuild/eclass code, which case you shouldn't be exporting EMERGE_FROM during EAPI>=4. > (In reply to comment #5) > > Also... what actually is *using* those hook points and is EMERGE_FROM > > sensitive? by the time of pkg_preinst, the source of the pkg doesn't matter to > > portage (it's already unpacked to disk) thus there really isn't any swivelling > > I could see being sane/usable there. > > > > Actual examples? > > Well, pkg_setup is historically a problematic area, since it's called at the > beginning of a source build and also at the beginning of a binary package > install. So, user bashrc pre/post pkg_setup hooks would be a particular area of > concern. I was looking for actual user bashrcs that were reliant on it, not ebuilds/eclasses- I know of ebuilds that rely on it, they're unaffected by this- pre eapi4, they use emerge_from, eapi4 and above, they use the spec standard- MERGE_TYPE. Basically, what is the actual fallout you're concerned about? Because the fallout of leaving the status quo like this w/ sloppy env's exported to ebuilds/eclasses is perfectly valid alternate package managers getting random breakages because of portage misbehaviour- worse, it's not "fix the eclass/ebuild"- it's code that is embedded into a binpkg so it *lingers* if it's not dealt with quickly. (In reply to comment #7) > It isn't; you can still export whatever vars you want for the hook invocations- > user code. This is about ebuild/eclass code, which case you shouldn't be > exporting EMERGE_FROM during EAPI>=4. It's hardly worth the effort given how intertwined bashrc hooks are with the ebuild/eclass code. > Basically, what is the actual fallout you're concerned about? Because the It could be a major inconvenience for anyone who somehow relies on EMERGE_FROM in bashrc. It's something that's impossible to quantify for a change like this, but the general principle is very simple and easy to demonstrate. > fallout of leaving the status quo like this w/ sloppy env's exported to > ebuilds/eclasses is perfectly valid alternate package managers getting random > breakages because of portage misbehaviour- worse, it's not "fix the > eclass/ebuild"- it's code that is embedded into a binpkg so it *lingers* if > it's not dealt with quickly. You're misplacing fault here. If ebuilds/eclasses use undocumented variables, then they are at fault and should be fixed. It's not the fault of the undocumented variables. (In reply to comment #8) > (In reply to comment #7) > > It isn't; you can still export whatever vars you want for the hook invocations- > > user code. This is about ebuild/eclass code, which case you shouldn't be > > exporting EMERGE_FROM during EAPI>=4. > > It's hardly worth the effort given how intertwined bashrc hooks are with the > ebuild/eclass code. Seriously? Just modify ebuild_phase_with_hooks and use a local EMERGE_FROM. That, presuming this code is borderline sane, exports it just for the hooks. That took all of a minute... > > Basically, what is the actual fallout you're concerned about? Because the > > It could be a major inconvenience for anyone who somehow relies on EMERGE_FROM > in bashrc. It's something that's impossible to quantify for a change like this, > but the general principle is very simple and easy to demonstrate. Hand wavey crap. > > fallout of leaving the status quo like this w/ sloppy env's exported to > > ebuilds/eclasses is perfectly valid alternate package managers getting random > > breakages because of portage misbehaviour- worse, it's not "fix the > > eclass/ebuild"- it's code that is embedded into a binpkg so it *lingers* if > > it's not dealt with quickly. > > You're misplacing fault here. If ebuilds/eclasses use undocumented variables, > then they are at fault and should be fixed. It's not the fault of the > undocumented variables. The fault is in the manager at this point via continuing to enable the broken behaviour- most of the time the devs don't even *know* they're breaking from the spec because of portage's lose env handling on things like this. Pretty much, fix the ebuilds/eclasses (I already did), then fix the cause of the problem- the fault now lies in portage if this is left in place. Either way, letting others comment- you're pretty clear you won't do it, and after having several users pissing/moaning at me for supporting the !@*#ing spec properly I'm not going to back down on this. (In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #7) > > > It isn't; you can still export whatever vars you want for the hook invocations- > > > user code. This is about ebuild/eclass code, which case you shouldn't be > > > exporting EMERGE_FROM during EAPI>=4. > > > > It's hardly worth the effort given how intertwined bashrc hooks are with the > > ebuild/eclass code. > > Seriously? Just modify ebuild_phase_with_hooks and use a local EMERGE_FROM. > That, presuming this code is borderline sane, exports it just for the hooks. > > That took all of a minute... Still, it's unnecessary imo. What you're suggesting it to replace one undocumented variable with another, for questionable benefit. (In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #8) > > > (In reply to comment #7) > > > > It isn't; you can still export whatever vars you want for the hook invocations- > > > > user code. This is about ebuild/eclass code, which case you shouldn't be > > > > exporting EMERGE_FROM during EAPI>=4. > > > > > > It's hardly worth the effort given how intertwined bashrc hooks are with the > > > ebuild/eclass code. > > > > Seriously? Just modify ebuild_phase_with_hooks and use a local EMERGE_FROM. > > That, presuming this code is borderline sane, exports it just for the hooks. > > > > That took all of a minute... > > Still, it's unnecessary imo. What you're suggesting it to replace one > undocumented variable with another, for questionable benefit. Err... you're pretty wildly off base in your understanding of this request. 1) EAPI<=3; ebuild/eclass env has access to EMERGE_FROM in it's current form. 2) under EAPI>=4, MERGE_TYPE, per PMS requirements, is there. 3) under EAPI>=4, to preclude the continuation of this mess, EMERGE_FROM is stripped from ebuild/eclass invocation- when ebuild/eclass code is active, it ain't there. 4) under EAPI>=4, to preserve against the claimed "hooks will break, users will hate it, <insert claim>", w/in ebuild_phase_with_hooks, you tweak it to export EMERGE_FROM for the hooks but not for the actual phase; it's accessible for user configuration, but not ebuilds/eclass I really can't describe it any simpler; no new vars, it's limiting the existing portage mess and gotcha for ebuild/eclass development. Or, and this is far saner, people w/ hooks just use ${MERGE_TYPE:-${EMERGE_FROM}} for accessing it. That works regardless of eapi while shifting away from the current portage mess. I still think the simplest and most reasonable approach would be to leave the portage behavior as it is, and educate ebuild devs if/when they use the undocumented EMERGE_FROM variable in ebuilds or eclasses (which I expect to be a fairly unlikely event). (In reply to comment #12) > I still think the simplest and most reasonable approach would be to leave the > portage behavior as it is, and educate ebuild devs if/when they use the > undocumented EMERGE_FROM variable in ebuilds or eclasses (which I expect to be > a fairly unlikely event). Right, so have those ebuilds/eclasses, minor things like *merging kernel binpkgs* break for the alternate package managers that try to stick to the spec to maintain compatibility. This isn't simpler, nor does it avoid breakage- it enables the situation. Honestly, if that's your views, just state it, it would save time. What's annoying here isn't that an ebuild/eclass did something stupid- the annoyance is that knowing it you're still pushing for keeping status quo- enabling the breakage. Badly phrased, but that's just plain enabling lock in. Gone out of my way to not break portage, to keep compliant, this sort of pushback w/ zero reason is fairly rediculous. Just educate the ebuild devs not to use undocumented variables, and the problem is solved. I'm not interested in convoluted solutions. There are two solutions; you asked for the convoluted mess via claiming userland hooks were at risk- you asked for convoluted bluntly. Telling devs "write pms compliant ebuilds" is great and all, but leaving landmines in place for it however isn't; we have sandbox for a similar reason- to catch dev screwups that are easily overlooked. You've given no real reason thus far for the reticence (nor time wasted on this beyond) "don't wanna" while there is measurable gain to fixing the landmine- both for devs and for general QA. (In reply to comment #15) > There are two solutions; you asked for the convoluted mess via claiming > userland hooks were at risk- you asked for convoluted bluntly. You're dismissing the third solution, where we just educate the ebuild devs not to use undocumented variables. > Telling devs "write pms compliant ebuilds" is great and all, but leaving > landmines in place for it however isn't; we have sandbox for a similar reason- > to catch dev screwups that are easily overlooked. I'm sure all package managers have undocumented variables. They're not problems unless people rely on them, and it's a simple training issue. There's no need to go and remove all such variables. > You've given no real reason thus far for the reticence (nor time wasted on this > beyond) "don't wanna" while there is measurable gain to fixing the landmine- > both for devs and for general QA. Again, I've given the reason about user's bashrcs, but you dismiss it. (In reply to comment #16) > (In reply to comment #15) > > There are two solutions; you asked for the convoluted mess via claiming > > userland hooks were at risk- you asked for convoluted bluntly. > > You're dismissing the third solution, where we just educate the ebuild devs not > to use undocumented variables. Leaving the mess to fester isn't a solution; it *was* left to devs to handle, and they still got it wrong- thus trying to improve the cause of it (portage). > > Telling devs "write pms compliant ebuilds" is great and all, but leaving > > landmines in place for it however isn't; we have sandbox for a similar reason- > > to catch dev screwups that are easily overlooked. > > I'm sure all package managers have undocumented variables. They're not problems > unless people rely on them, and it's a simple training issue. There's no need > to go and remove all such variables. Quite a large amount of the internal variables of managers are prefixed to avoid scenarios like this; this however isn't an "undocumented variable"- EMERGE_FROM was the solution pre EAPI4, MERGE_TYPE is the solution EAPI4 and later. In this particular case, since there *is* a pms correct var- leaving EMERGE_FROM there just makes the situation worse. > > You've given no real reason thus far for the reticence (nor time wasted on this > > beyond) "don't wanna" while there is measurable gain to fixing the landmine- > > both for devs and for general QA. > > Again, I've given the reason about user's bashrcs, but you dismiss it. I addressed it; easiest way is ${MERGE_TYPE:-${EMERGE_FROM}}. I also pointed out that if you're deadset that the hooks must have EMERGE_FROM, you just export it for the *hooks* alone. In other words, user bashrc's can be *addressed* in a fashion preserving the behaviour you want. How exactly is that dismissing it? You can have your same behaviour if you want- the only thing you've been left to arguing against is removing the var from being accessible to ebuild/eclass functionality which I've given very good reasons for. Try again, basically. Obviously we have a disagreement. Feel free to submit it to the council if it's that important to you. Created attachment 274859 [details, diff]
supress EMERGE_FROM in ebuild/eclass where MERGE_TYPE is available
This preserves hook behaviour, while suppressing access to EMERGE_FROM in ebuild/eclass function invocations where MERGE_TYPE is available; in the process improving QA.
Created attachment 274861 [details, diff]
supress EMERGE_FROM in ebuild/eclass where MERGE_TYPE is available
Same patch, just w/ tabs/spaces fixed missed in the first pass.
Created attachment 274863 [details, diff]
use ebuild_phase_with_hooks more, and fix source_all_bashrcs
Can't we just include EMERGE_FROM in the list of unspecified items in PMS <http://dev.gentoo.org/~ulm/pms/4/pms.html#x1-165000B> and add a repoman check for it? (In reply to comment #22) > Can't we just include EMERGE_FROM in the list of unspecified items in PMS > <http://dev.gentoo.org/~ulm/pms/4/pms.html#x1-165000B> Could, although the thing to note is that EMERGE_FROM while non spec, is frankly acceptable in EAPI<4; disallowing it explicitly for those EAPI's isn't sane. While pbins don't support it, they also diverge a bit from traditional binpkg handling last I looked anways. Either way, longer term the patch I uploaded shifts towards removing this issue long term- MERGE_TYPE is there for eapi's going forward, and EMERGE_FROM is still accessible for bashrc hooks and pre EAPI4, while being stored as an internal var for porage's usage. That modification basically castrates new ebuilds/eclasses from invalidly relying on EMERGE_FROM in EAPI>=4; as said, have fixed the affected pkgs already so no real fallout there. > and add a repoman check for it? Not sure if that's particularly viable; EMERGE_FROM usage, while nonspec, is frankly required for EAPI<4 for certain rather important pkgs (kernel, glibc, etc; just do a pquery scan if curious). Cleaning the tree so that a qa check doesn't complain would either require a rather dicey bit of bash (likely to false positive still), or forcing upgrade to EAPI4 (thus accessing MERGE_TYPE only) for the affected pkgs- one of which being glibc, which has to be eapi=0. glibc alone is a no go. Neither option is going to come about quickly, nor do I actually expect those EAPI4 upgrades to occur- either it can't in certain cases, or it's just not worth it. One thing to keep in mind... portage exported env *does* need to be pulled back into properly labeled internal vars; that work has already been occuring, but things like this are good examples of why "just state it's a no go in pms" doesn't fly- alternative implementations get broke by portage bleeding crap into the env. When new functionality is standardized (MERGE_TYPE), we should be suppressing the previous portage var folks relied on to force convergence to the spec; just the same, moving portage vars into PORTAGE_* prefixed (as pkgcore/paludis have done for a long while) precludes ebuilds/eclasses easily becoming localized to one manager. Created attachment 274887 [details, diff] make repoman disallow EMERGE_FROM in EAPI 4 (like AA and KV) (In reply to comment #22) > Can't we just include EMERGE_FROM in the list of unspecified items in PMS > <http://dev.gentoo.org/~ulm/pms/4/pms.html#x1-165000B> and add a repoman check > for it? That sounds good to me. Here's a patch for repoman. Created attachment 274889 [details, diff]
List EMERGE_ as unspecified variables in PMS
Proposed patch for PMS.
The repoman patch is in git now: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=10f6c6d3c028a509ed943ef76633c72dbbcff7f6 (In reply to comment #26) > The repoman patch is in git now: > > http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=10f6c6d3c028a509ed943ef76633c72dbbcff7f6 This is in 2.1.10 and 2.2.0_alpha38. (In reply to comment #25) > Created attachment 274889 [details, diff] > List EMERGE_ as unspecified variables in PMS > > Proposed patch for PMS. Pushed: http://git.overlays.gentoo.org/gitweb/?p=proj/pms.git;a=commit;h=a88d6476e8173dd0183ff1a08280ac7f8664fa63 Is there anything else to be done here? The repoman check was included in portage-2.1.10.3. |