Summary says it all; ebuilds shouldn't be poking around in ${FILESDIR} from their global scope (sourcing/metadata phase). After sourcing is done, go nuts, but not during. It's very rarely what the ebuild actually wants to do. In terms of chunks of the tree affected, to my knowledge in the last 5 years the only time there was *intentional* $FILESDIR access is in sys-kernel/mips-sources which is attempting some g33/elibs/eblits nastyness. Bug 300370 however reworks their trick so it's no longer reliant on global ${FILESDIR} access. Kind of a no brainer. For this to go in, it has to be retroactive- EAPI isn't known prior to sourcing after all. Non hit either way- pkgcore has set FILESDIR after sourcing pretty much since it's inception, only ebuild to ever go boom during that entire timeframe is mips-sources.
FILESDIR is already only allowed in src_* according to PMS: http://dev.gentoo.org/~tanderson/pms/head/html/pms.html#x1-11800012.1 I think it would be up to each package manager to choose how strictly to enforce this. As far as I remember PMS doesn't specify how violations should be treated for other things, so I don't think there's a need to do so here.
> I think it would be up to each package manager to choose how strictly to > enforce this. As far as I remember PMS doesn't specify how violations should > be treated for other things, so I don't think there's a need to do so here. Leaving strictness of enforcement up to the manager creates incompatibilities between the managers... so stating "the manager can enforce it strictly if they choose" isn't useful if you want to spec a format all can use w/out concern. If in doubt, consider that mips-source hasn't been sourcable for pkgcore a good while now, *despite* pkgcore being compliant to pms. Where it isn't nasty to do, we should be forcing variable creation as needed and that ought to be mandated.
Unfortunately it's Portage policy not to enforce PMS, and the Portage guys have repeatedly refused to make Portage display any kind of warning or error for PMS violations. If you want this to go anywhere, you need to get the Council to mandate that package managers must issue errors for PMS violations where possible.
(In reply to comment #2) > Where it isn't nasty to do, we should be forcing variable creation as needed > and that ought to be mandated. OK, that's a fair enough point of view, but no need to restrict it to just FILESDIR in that case.
(In reply to comment #3) > Unfortunately it's Portage policy not to enforce PMS, and the Portage guys have > repeatedly refused to make Portage display any kind of warning or error for PMS > violations. If you want this to go anywhere, you need to get the Council to > mandate that package managers must issue errors for PMS violations where > possible. I highly doubt the council will back giving you another hammer to beat portage over the head with- further on this one since PMS is intentionally vague in spots, such a hammer is pointless. Hell, *I* wouldn't back the PMS in full on that. That however does not mean baby steps improving the compatibility cannot be undertaken. Case in point, paludis could make FILESDIR only available during the src_* phases- it's intended as the reference implementation of PMS after all.
So, what, you want that hammer to be used only on things that break Pkgcore?
(In reply to comment #6) > So, what, you want that hammer to be used only on things that break Pkgcore? I want you to act more like an adult trying to improve things, rather then taking potshots at things on your current shitlist. Pretty simple question to keep in mind; are you here to improve compatibility between the three, building a spec that is *the* standard for what's required for compliance or are you here just to berate people via PMS? So... we going to move onto a productive discussion that has tangible benefits for PM maintaners and ebuild devs alike, or are you going to stick w/ this whacked attempt to get the council to censure portage for your shits and giggles?
What you're after is already illegal according to PMS, so the only question now is enforcement. Given that Portage ignores every other place in PMS where enforcement is recommended, what makes you think shoving this in would make any difference? Your issue is with Portage, and its lack of any kind of validation or enforcement, not with PMS.
(In reply to comment #8) > What you're after is already illegal according to PMS, so the only question now > is enforcement. Given that Portage ignores every other place in PMS where > enforcement is recommended, what makes you think shoving this in would make any > difference? It's not a question of enforcement- the spec should explicitly disallow this from even occurring. "recommended" is generally worthless in a spec, only useful when recommending an internal implementation detail nothing can rely upon (recommending that an optimization be used for example). Basically, don't be bitching about portage not enforcing 'recommended' things. Make them explicit, then you have a leg to stand on for your bitching- hell, I'd probably even back your bitching. Seriously, 'recommended' in cases like this means "if you want compatibility, aim for the LCD regardless of what this spec tells you". Good way to shoot yourself in the foot, and pretty much worthless in a spec. As for the change, pretty simple really. -Not all variables are meaningful in all phases; variables that are not meaningful in a given phase may be unset or set to any value. +Not all variables are usable in all phases; variables that are not usable in a given phase are to be unset (or set to a value used to intercept invalid usage of these variables). The only odd part about the change needed there is the "or set to a value used to intercept invalid usage of these variables"- that's specifically intended to allow FILESDIR and other vars to be set to a directory where the PMs sandbox mechanism can catch the access and flag at it as bad. Basically intended as a way to convert silent ebuild mistakes into sourcing mistakes, should the manager wish to be more strict. There is one caveat I'm aware of- libtool sets a path based on ECLASSDIR upon sourcing. Enforcing that for normal builds (pkg_setup and friends) seems a bit pointless from where I'm sitting since the real intention of disallowing reliance on it is binpkg compatibility. Seems like the table needs some tweaking for that.
There's currently nothing in PMS that forces package managers to reject invalid anything. If you want to convince the Council that that should change, and that package managers should be required to try to enforce the specification, then feel free to try. But given comment #5, I am under the impression that you don't want package managers to be required to try to enforce the specification, except in certain very specific areas. In any case, there's nothing PMS can do about this on its own. You can either try to convince the Portage people to enforce it even though the spec doesn't mandate it (which they've refused to do for other things previously), or you can ask the Council to allow PMS to start mandating validation.
(In reply to comment #1) > FILESDIR is already only allowed in src_* according to PMS: > http://dev.gentoo.org/~tanderson/pms/head/html/pms.html#x1-11800012.1 What is the meaning of the "Legal in" column? That the variable is only guaranteed to exist in the phases indicated there, or that it points to an existing directory? I'm asking because the table claims that WORKDIR is "legal in" src_* only, whereas everybody uses it in global scope in ebuilds and eclasses. Same for DISTDIR and ECLASSDIR.
Above the table it says: > Not all variables are meaningful in all phases; variables that are not > meaningful in a given phase may be unset or set to any value. So those ebuilds are ok, so long as they don't assume that those variables will contain a valid value at all times.
(In reply to comment #12) > Above the table it says: > > > Not all variables are meaningful in all phases; variables that are not > > meaningful in a given phase may be unset or set to any value. > > So those ebuilds are ok, so long as they don't assume that those variables > will contain a valid value at all times. So what happens if I do S="${WORKDIR}" in global scope? Should I assume that WORKDIR may be unset? This doesn't make sense.
(In reply to comment #10) > There's currently nothing in PMS that forces package managers to reject invalid > anything. If you want to convince the Council that that should change, and that > package managers should be required to try to enforce the specification, then > feel free to try. Enforcing, again, is the wrong term. It implies managers need to do something extra to ensure that ebuilds don't step away from the spec... the spec shouldn't allow for there to be a difference in PM behaviour in core things like this, thus precluding the *possibility* of ebuilds stepping away from a PMS compliant manager. > In any case, there's nothing PMS can do about this on its own. You can either > try to convince the Portage people to enforce it even though the spec doesn't > mandate it (which they've refused to do for other things previously), or you > can ask the Council to allow PMS to start mandating validation. Again, third option. Make the spec preclude fuck ups like this. You don't need the council involvement for this, you just need the PM maintainers (and ebuild devs in some cases) to sign off on changes to the spec. Frankly I oppose running to the council for it since the spec itself is the issue rather then a specific PM- going to the council is a waste of their time since they're not dumb enough to hand carte blanche over- they'll just tell you to solve it w/in the spec and escalate to the council if something cannot be agreed to. The exact same thing PMS was told to do already... Either way, a further option for this is limiting the variable unsetting to just depends/metadata phases- this however requires PMS to specifically label said phase w/in the table. Eliminating those vars during metadata phasing does a pretty good job of having any ebuild screwups go boom up front rather then silently working (and potentially causing issues during regen).
(In reply to comment #13) > So what happens if I do S="${WORKDIR}" in global scope? Should I assume that > WORKDIR may be unset? This doesn't make sense. It means that if you then use S in a phase in which WORKDIR isn't defined, then the value of S is undefined.
(In reply to comment #14) > Enforcing, again, is the wrong term. It implies managers need to do something > extra to ensure that ebuilds don't step away from the spec... the spec > shouldn't allow for there to be a difference in PM behaviour in core things > like this, thus precluding the *possibility* of ebuilds stepping away from a > PMS compliant manager. You want the specification to be changed to require that package managers set certain variables to deliberately invalid values so that ebuilds can't accidentally do something illegal. That's enforcing the spec, just the same as deliberately checking whether or not the format of a particular file is legal according to the spec. > Again, third option. Make the spec preclude fuck ups like this. You don't > need the council involvement for this, you just need the PM maintainers (and > ebuild devs in some cases) to sign off on changes to the spec. As you say in the Summary, it's a retroactive change to the specification. Given that the specification matches Portage behaviour, and that you're after changes to both Portage and ebuilds as a result, this really isn't something that the PMS project has the authority to do on its own. The specification doesn't preclude this specific mistake because the specification doesn't have any language in it requiring package managers to perform any kind of sanity checking or validation. Nor can it, so long as Portage doesn't do anything like that. > Either way, a further option for this is limiting the variable unsetting to > just depends/metadata phases- this however requires PMS to specifically label > said phase w/in the table. But package managers don't currently guarantee they'll unset the value. The specification merely says that they're allowed to, and that by extension any ebuild that relies upon a package manager providing a valid value in all phases is broken. > Eliminating those vars during metadata phasing does a pretty good job of having > any ebuild screwups go boom up front rather then silently working (and > potentially causing issues during regen). I would very much like to see enforcement of this and a huge list of other things PMS requires that ebuilds frequently screw up. However, given that PMS doesn't currently require that package managers try to enforce compliance, and given that the Portage people generally refuse to enforce compliance even when doing so is trivial, there's nothing we can do about it.
All side-issues apart, let's talk about those variables only now. As far as I can see most people agree on the subject. As Ciaran pointed out a retrospective change on PMS needs a council ack, this can be done by mail voting I suppose. If we change the recommendation to a must, we can "force" this on Portage or whatever package manager who does not implement it. Brian agreed to back the proposal, too, so a majority seems to think that a recommendation is a bad idea.
(In reply to comment #16) > The specification doesn't preclude this specific mistake because the > specification doesn't have any language in it requiring package managers to > perform any kind of sanity checking or validation. Nor can it, so long as > Portage doesn't do anything like that. It can- as you like to state "just because portage does something doesn't mean it's right". If all but one agree something should be in the spec, council escalation is the fallback option. Till then you've got no leg to stand on claiming spec lockdown can't be done because portage won't change. > > Either way, a further option for this is limiting the variable unsetting to > > just depends/metadata phases- this however requires PMS to specifically label > > said phase w/in the table. > > But package managers don't currently guarantee they'll unset the value. The > specification merely says that they're allowed to, and that by extension any > ebuild that relies upon a package manager providing a valid value in all phases > is broken. The point I'm trying to beat into your skull here is that leaving the option for up to the manager in cases like this means none can, if they wish to maintain compatibility. It doesn't matter if it's portage, paludis, or pkgcore- all 3 have to do the same LCD thing there if the spec relies on recommended. > > Eliminating those vars during metadata phasing does a pretty good job of having > > any ebuild screwups go boom up front rather then silently working (and > > potentially causing issues during regen). > > I would very much like to see enforcement of this and a huge list of other > things PMS requires that ebuilds frequently screw up. If you've not already, file the tickets for the specific lockdowns to the spec you want- case by case, if people agree, I doubt you'll see serious pushback if the idea isn't whacked. > and given that the Portage people generally refuse to enforce compliance even when > doing so is trivial, there's nothing we can do about it. Spurious claim, and a bit pointless claiming "we can't do it because portage won't enforce what are currently recommendations in the spec". No shit they're not going to do it- majority of PM implementations aim for the LCD of PMS, recommended falls outside of that and will be implemented only when the PM maintainer has a personal interest in it. Worth noting the reason this ticket finally was filed was that I approached zac to see if he agreed with the idea- specifically unsetting the var to block dev screwups, I approached him w/ the intent of verifying at least 2 out of the 3 manager maintainers agreed with the idea prior to pushing it to PMS. Ticket was filed since he agreed with blocking the access, so from where I'm sitting your 'enforcement' complaints need some reevaluating. As said, go file the tickets (or bump them so folks know what the hell you're after), case by case see if there is agreement- doubt you'll get everything you want (doubt I will), but there will be improvements at least. Either way, the portage excuse is irrelevant here.
(In reply to comment #18) > It can- as you like to state "just because portage does something doesn't mean > it's right". Lacking enforcement isn't an obvious bug. Not doing something that no package manager does, and that breaks various ebuilds, is not. > If all but one agree something should be in the spec, council > escalation is the fallback option. Till then you've got no leg to stand on > claiming spec lockdown can't be done because portage won't change. Er, no. We *always* go to Council for retroactive changes and new features. > The point I'm trying to beat into your skull here is that leaving the option > for up to the manager in cases like this means none can, if they wish to > maintain compatibility. It doesn't matter if it's portage, paludis, or > pkgcore- all 3 have to do the same LCD thing there if the spec relies on > recommended. Not true at all. What the spec says is that it's undefined behaviour, and that anything relying upon it is broken. You want to change this to require the package manager to take steps to catch broken behaviour. This is not the kind of thing PMS currently does, and if we start making changes to PMS to force package managers to go out of their way to enforce the spec, there are a lot more things to add. > > I would very much like to see enforcement of this and a huge list of other > > things PMS requires that ebuilds frequently screw up. > > If you've not already, file the tickets for the specific lockdowns to the spec > you want- case by case, if people agree, I doubt you'll see serious pushback if > the idea isn't whacked. You mean like for making Portage enforce profile package.mask being a regular file? > > and given that the Portage people generally refuse to enforce compliance even when > > doing so is trivial, there's nothing we can do about it. > > Spurious claim, and a bit pointless claiming "we can't do it because portage > won't enforce what are currently recommendations in the spec". No shit they're > not going to do it- majority of PM implementations aim for the LCD of PMS, > recommended falls outside of that and will be implemented only when the PM > maintainer has a personal interest in it. So, uh, you want us to put something in PMS that Portage won't do? > Worth noting the reason this ticket finally was filed was that I approached zac > to see if he agreed with the idea- specifically unsetting the var to block dev > screwups, I approached him w/ the intent of verifying at least 2 out of the 3 > manager maintainers agreed with the idea prior to pushing it to PMS. It's not up to them. Portage is allowed to implement stricter handling if it wants, but if you want retroactive changes to PMS and forced changes to package managers, you have to go to the Council. > Ticket was filed since he agreed with blocking the access, so from where I'm > sitting your 'enforcement' complaints need some reevaluating. Will similar enforcement be done on package.mask, package names, version strings, cross-phase function calls, use of ROOT in src_* and so on? I'd like to see that happen. But again, as everyone is telling you, retroactive changes that go against what package managers do require Council approval.
(In reply to comment #19) > (In reply to comment #18) > > The point I'm trying to beat into your skull here is that leaving the option > > for up to the manager in cases like this means none can, if they wish to > > maintain compatibility. It doesn't matter if it's portage, paludis, or > > pkgcore- all 3 have to do the same LCD thing there if the spec relies on > > recommended. > > Not true at all. What the spec says is that it's undefined behaviour, and that > anything relying upon it is broken. You want to change this to require the > package manager to take steps to catch broken behaviour. You're still missing the point- 'undefined' is generally useless, and a source of incompatibility in behaviour. Punting the responsibility to the ebuild author ignores that the spec should be phrased to disallow the problem from even existing. Yes, certain things have to be labeled undefined- not all of them however. > > > I would very much like to see enforcement of this and a huge list of other > > > things PMS requires that ebuilds frequently screw up. > > > > If you've not already, file the tickets for the specific lockdowns to the spec > > you want- case by case, if people agree, I doubt you'll see serious pushback if > > the idea isn't whacked. > > You mean like for making Portage enforce profile package.mask being a regular > file? Bad example- each PM supports things well outside of PMS, and that's fine by me. What is needed in that particular case however is to version the repository in some fashion (profiles/repo_format for example) to control when the behaviour is allowed- no different then PMs supporting alternative EAPIs, just have to mark it somehow. Still, keep filing the tickets (as said, that was one bad example, hopefully you have more that can be used). > > > and given that the Portage people generally refuse to enforce compliance even when > > > doing so is trivial, there's nothing we can do about it. > > > > Spurious claim, and a bit pointless claiming "we can't do it because portage > > won't enforce what are currently recommendations in the spec". No shit they're > > not going to do it- majority of PM implementations aim for the LCD of PMS, > > recommended falls outside of that and will be implemented only when the PM > > maintainer has a personal interest in it. > > So, uh, you want us to put something in PMS that Portage won't do? You *really* missed my point there- if it's a recommendation, you can't rely on people implementing it. Pretty simple. Thus... don't make it a recommendation (especially for new additions to the spec, no point in furthering the issue). > > Worth noting the reason this ticket finally was filed was that I approached zac > > to see if he agreed with the idea- specifically unsetting the var to block dev > > screwups, I approached him w/ the intent of verifying at least 2 out of the 3 > > manager maintainers agreed with the idea prior to pushing it to PMS. > > It's not up to them. Portage is allowed to implement stricter handling if it > wants, but if you want retroactive changes to PMS and forced changes to package > managers, you have to go to the Council. You were bitching portage won't do any enforcement- the point there is that they're open to at least this change. Prior to going to the council one needs to get some consensus- it's there, thus council is the next step. > > Ticket was filed since he agreed with blocking the access, so from where I'm > > sitting your 'enforcement' complaints need some reevaluating. > > Will similar enforcement be done on package.mask, package names, version > strings, cross-phase function calls, use of ROOT in src_* and so on? I'd like > to see that happen. Seperate tickets please... this ticket is already a bit of a clusterfuck via our usual fighting. Personally, I'm thinking get the collection of tweaks built up, then nudge the council for it. Or piecemeal if you prefer (end result is the same).
(In reply to comment #20) > You're still missing the point- 'undefined' is generally useless, and a source > of incompatibility in behaviour. Punting the responsibility to the ebuild > author ignores that the spec should be phrased to disallow the problem from > even existing. > > Yes, certain things have to be labeled undefined- not all of them however. No, undefined means it is illegal for an ebuild to rely upon any particular behaviour. That's as strong as we can go, until you convince the Council to allow PMS to include language forcing package managers to require validation. > > You mean like for making Portage enforce profile package.mask being a regular > > file? > > Bad example- each PM supports things well outside of PMS, and that's fine by > me And Portage supports access to FILESDIR during the metadata phase. Is that fine by you? > > So, uh, you want us to put something in PMS that Portage won't do? > > You *really* missed my point there- if it's a recommendation, you can't rely on > people implementing it. Pretty simple. Thus... don't make it a recommendation > (especially for new additions to the spec, no point in furthering the issue). So you want us to put something in PMS that Portage doesn't do? > > Will similar enforcement be done on package.mask, package names, version > > strings, cross-phase function calls, use of ROOT in src_* and so on? I'd like > > to see that happen. > > Seperate tickets please... this ticket is already a bit of a clusterfuck via > our usual fighting. No, if we're starting to stick "the package manager must ensure" language in PMS, we either do it for a whole load of things all together or not at all. We don't sneak it in for one arbitrary pet feature of yours whilst sidelining everything else where it'll get ignored.
This is fixed in portage-2.1.7.17 and 2.2_rc62.
Seems that this is resolved since a long time, therefore closing.
