Otherwise people will rely on the behaviour, which leads to issues like bug 480892.
Applying that retroactively might cause a lot of build failures.
No, applying this retroactively might catch a lot of broken ebuilds. Which is a good thing.
(In reply to Julian Ospald (hasufell) from comment #1) > Applying that retroactively might cause a lot of build failures. Yeah, it would have been better if that workaround hadn't been added, in the first place. How about adding a QA warning? That wouldn't cause build failures on users' systems, but still indicate that something is wrong.
the QA warning should include an explanation on how to fix it; they will most likely be ignored otherwise
(In reply to Ciaran McCreesh from comment #2) > No, applying this retroactively might catch a lot of broken ebuilds. Which > is a good thing. Aha, you were running the exact opposite argument when it was about "--disable-silent-rules" in EAPI 5.
So setting DOCS="" is not going to be allowed for ebuilds that use default_src_install? Maybe we should change the default_src_install to allow it?
(In reply to Zac Medico from comment #6) > So setting DOCS="" is not going to be allowed for ebuilds that use > default_src_install? Maybe we should change the default_src_install to allow > it? That will probably not be allowed for current EAPIs. Guaranteed EAPI behavior and all that crap.
if the spec is wrong, fix the spec :)
if you are going to change default_src_install also look at bug 476982 bug 459692
(In reply to Zac Medico from comment #6) > So setting DOCS="" is not going to be allowed for ebuilds that use > default_src_install? default_src_install in EAPI 4 is broken as designed. I wonder how it was possible that it passed, with so many people looking at it. http://article.gmane.org/gmane.linux.gentoo.devel/60575 > Maybe we should change the default_src_install to allow it? Bug 463736. IIUC, that would be for EAPI 6.
I don't see anything in PMS that forbids dodoc with no argument btw: DODOC Installs the given files into a subdirectory ... if no file is given, then it does nothing. it's like: '\forall x\in\emptyset, whatever(x)' is true
(In reply to Julian Ospald (hasufell) from comment #5) > (In reply to Ciaran McCreesh from comment #2) > > No, applying this retroactively might catch a lot of broken ebuilds. Which > > is a good thing. > > Aha, you were running the exact opposite argument when it was about > "--disable-silent-rules" in EAPI 5. No I wasn't. You're confusing "broken ebuild" with "ebuild that breaks when using Portage". The two are entirely different things, and the distinction is important for this kind of case. A broken ebuild is one which either violates the spec or relies upon undefined behaviour.
(In reply to Ciaran McCreesh from comment #12) > (In reply to Julian Ospald (hasufell) from comment #5) > > (In reply to Ciaran McCreesh from comment #2) > > > No, applying this retroactively might catch a lot of broken ebuilds. Which > > > is a good thing. > > > > Aha, you were running the exact opposite argument when it was about > > "--disable-silent-rules" in EAPI 5. > > No I wasn't. You're confusing "broken ebuild" with "ebuild that breaks when > using Portage". The two are entirely different things, and the distinction > is important for this kind of case. A broken ebuild is one which either > violates the spec or relies upon undefined behaviour. You didn't understand what I was referring to. YOU have to prove that the PM change will not break ANY stable ebuild. If you cannot, then it will not be applied.
I don't understand why you're debating here; dodoc with no arg is perfectly fine per PMS and broken PMs should be fixed to be compliant. No ebuild needs to be broken, no animal harmed, etc.
(In reply to Alexis Ballier from comment #11) > I don't see anything in PMS that forbids dodoc with no argument btw: > DODOC Installs the given files into a subdirectory ... > > if no file is given, then it does nothing. The wording of the spec could be more precise. If taken literally, the plural "files" would imply that it's only legal to call it with two or more file operands. I think the intention was to have the usual behaviour of other Unix utilities, namely to disallow calling it without operands. That was the Portage behaviour when the spec was written, and if the spec would intentionally deviate from it, then there should have been a discussion about it. (In reply to Alexis Ballier from comment #14) > I don't understand why you're debating here; dodoc with no arg is perfectly > fine per PMS and broken PMs should be fixed to be compliant. Then all package managers are broken. doins and all its friends check for at least one operand, in all PMs. With the single exception of dodoc in Portage, and there it was added late.
(In reply to Ulrich Müller from comment #15) > (In reply to Alexis Ballier from comment #11) > > I don't see anything in PMS that forbids dodoc with no argument btw: > > DODOC Installs the given files into a subdirectory ... > > > > if no file is given, then it does nothing. > > The wording of the spec could be more precise. If taken literally, the > plural "files" would imply that it's only legal to call it with two or more > file operands. > > I think the intention was to have the usual behaviour of other Unix > utilities, namely to disallow calling it without operands. That was the > Portage behaviour when the spec was written, and if the spec would > intentionally deviate from it, then there should have been a discussion > about it. > We shouldn't need philosophers thinking about the meaning of PMS. > > (In reply to Alexis Ballier from comment #14) > > I don't understand why you're debating here; dodoc with no arg is perfectly > > fine per PMS and broken PMs should be fixed to be compliant. > > Then all package managers are broken. doins and all its friends check for at > least one operand, in all PMs. With the single exception of dodoc in > Portage, and there it was added late. No, PMS is broken, if you define it as "works for the user".
(In reply to Ciaran McCreesh from comment #2) > No, applying this retroactively might catch a lot of broken ebuilds. Which > is a good thing. As long as we don't break stable in the progress; so, a QA warning suffices. (In reply to Ulrich Müller from comment #3) > How about adding a QA warning? That wouldn't cause build failures on users' > systems, but still indicate that something is wrong. +1 and then Tinderbox could catch them all in a reasonable amount of time. Despite that, a large share can be caught with a grep so we can fix them already (unless we will support this particular matter instead of removing it); and by the looks of it, there aren't that much occurrences: https://gist.github.com/TomWij/a7ea452f73ce26e63f75 (In reply to Alexis Ballier from comment #4) > the QA warning should include an explanation on how to fix it; they will > most likely be ignored otherwise How to fix something that was intended? Well, not using the default is an option; but well, that is kind of something against our intentions for something that should be much more simpler. (In reply to Julian Ospald (hasufell) from comment #16) > We shouldn't need philosophers thinking about the meaning of PMS. That's why "The wording of the spec could be more precise" was said. > > Then all package managers are broken. doins and all its friends check for at > > least one operand, in all PMs. With the single exception of dodoc in > > Portage, and there it was added late. > > No, PMS is broken, if you define it as "works for the user". The PMS is not an implementation, but a specification; if the PM implements something that's not specified, that doesn't make the PMS broken. What they want to do here is have the PMS specify it, such that any PM that comes with a "PMS sticker" should "work for the user" for this particular matter.
(In reply to Julian Ospald (hasufell) from comment #16) > We shouldn't need philosophers thinking about the meaning of PMS. Right. http://article.gmane.org/gmane.linux.gentoo.pms/763
I'm not subscribed to gentoo-pms so I'll respond here (In reply to Ulrich Müller from comment #15) > (In reply to Alexis Ballier from comment #11) > > I don't see anything in PMS that forbids dodoc with no argument btw: > > DODOC Installs the given files into a subdirectory ... > > > > if no file is given, then it does nothing. > > The wording of the spec could be more precise. If taken literally, the > plural "files" would imply that it's only legal to call it with two or more > file operands. The plural "files" implies "a list of files" which can very well be empty. The 'universal quantifier' here is made clear with the -r description: 'if the first argument is -r, any subsequent arguments that are directories'... there is a 'any' which explicitly allows none; unless you are claiming 'dodoc -r' is ok and 'dodoc' is not, dodoc without arg is ok by definition. > I think the intention was to have the usual behaviour of other Unix > utilities, namely to disallow calling it without operands. That was the > Portage behaviour when the spec was written, and if the spec would > intentionally deviate from it, then there should have been a discussion > about it. Well, the spec is approved. You cannot base an argument on the intention of what was written. The spec could certainly be more clear, though, and I could certainly argue that the intention was to have dodoc written recursively on a list of files as input. > (In reply to Alexis Ballier from comment #14) > > I don't understand why you're debating here; dodoc with no arg is perfectly > > fine per PMS and broken PMs should be fixed to be compliant. > > Then all package managers are broken. doins and all its friends check for at > least one operand, in all PMs. With the single exception of dodoc in > Portage, and there it was added late. Yes, except calling doins and all its friends without arg is so useless that nobody ever tried :) dodoc without arg has an advantage with eapi4 src_install. (In reply to Ulrich Müller from comment #18) > (In reply to Julian Ospald (hasufell) from comment #16) > > We shouldn't need philosophers thinking about the meaning of PMS. > > Right. > > http://article.gmane.org/gmane.linux.gentoo.pms/763 This is retroactively adding constraints. Such a change (if desired) should be done in a new eapi.
(In reply to Alexis Ballier from comment #19) > > http://article.gmane.org/gmane.linux.gentoo.pms/763 > > This is retroactively adding constraints. Such a change (if desired) should > be done in a new eapi. It's not adding any constraints, but merely a clarification of the wording. No package manager implements the "no parameters" case, so there's no EAPI where you could rely on such behaviour. None of the three package managers' authors has read the spec in the way you claim in comment #11 that it should be read. Can they all be wrong? And no, we're not going to break the specification of all our install functions because of some rare corner case that affects only a handful of ebuilds in the tree.
(In reply to Ulrich Müller from comment #20) > (In reply to Alexis Ballier from comment #19) > > > http://article.gmane.org/gmane.linux.gentoo.pms/763 > > > > This is retroactively adding constraints. Such a change (if desired) should > > be done in a new eapi. > > It's not adding any constraints, but merely a clarification of the wording. > No package manager implements the "no parameters" case, so there's no EAPI > where you could rely on such behaviour. None of the three package managers' > authors has read the spec in the way you claim in comment #11 that it should > be read. Can they all be wrong? http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=blob;f=bin/ebuild-helpers/doins;h=4679e83a9a7140e100da733cb34faad384991a64;hb=HEAD#l10 I agree there is a "gap" in the spec, but authoritatively claiming that it's forbidden doesn't make it forbidden; people started to rely on this for dodoc and nobody has provided a proper solution. The wording could very well be clarified in a way that states 'calling those functions with no argument returns success'. Therefore, yes, you are adding new constraints and it is not 'merely a clarification of the wording'. > And no, we're not going to break the specification of all our install > functions because of some rare corner case that affects only a handful of > ebuilds in the tree. We're not going to break ebuilds because some minor PM happens to implement it in a way that doesn't support this corner case. 'The specification' here is your interpretation of the specification; the specification itself doesn't say anything about it. do* with no arg is allowed by the spec (as in, not explicitly forbidden nor allowed), people started to rely on this, you can't suddenly declare it forbidden and change the behavior of those ebuilds that rely on it. If you don't like it, please do it the proper way: Live with it and propose an improvement for future EAPIs.
(In reply to Alexis Ballier from comment #21) > http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=blob;f=bin/ebuild-helpers/doins;h=4679e83a9a7140e100da733cb34faad384991a64;hb=HEAD#l10 Right, around line 20 you'll find the check for at least one argument: if [ $# -lt 1 ] ; then __helpers_die "${helper}: at least one argument needed" exit 1 fi
(In reply to Julian Ospald (hasufell) from comment #13) > You didn't understand what I was referring to. YOU have to prove that the PM > change will not break ANY stable ebuild. If you cannot, then it will not be > applied. Any ebuild that this breaks is broken anyway (remember, "works for me" doesn't mean it's not broken). All we're doing here is making that breakage visible.
Created attachment 356492 [details, diff] Patch for bin/ebuild-helpers/doins Attached patch adds a QA warning to dodoc.
Created attachment 356494 [details, diff] Patch for bin/ebuild-helpers/doins Sorry, wrong file.
(In reply to Ulrich Müller from comment #22) > (In reply to Alexis Ballier from comment #21) > > http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=blob;f=bin/ebuild-helpers/doins;h=4679e83a9a7140e100da733cb34faad384991a64;hb=HEAD#l10 > > Right, around line 20 you'll find the check for at least one argument: > > if [ $# -lt 1 ] ; then > __helpers_die "${helper}: at least one argument needed" > exit 1 > fi I can submit a patch that fixes this if you wish. You are not even trying to understand my point, so I think I'd better spend my time on other stuff and let you guys with "authority" fix the mess. I'll just ignore any PMS rule that I consider wrong and doesn't come with a proper solution.
(In reply to Alexis Ballier from comment #26) > You are not even trying to understand my point, You've stated your point clearly enough, but that doesn't mean that I'll buy it. Currently all install functions error out if called with no argument (except for dodoc in Portage since some unknown time), and I think that this is the desired behaviour. It's also the behaviour that you'll find throughout in Unix commands (look at chmod, mkdir, rm, or touch, for example) and there's good reason for it. Calling a command with an empty argument list is usually an indication that something went wrong in the caller. We should have very good reason if we want to deviate from this convention. In the current case, the breakage is in the EAPI 4 default_src_install function, so this is what should be fixed. It affects few ebuilds only, so I think it's even a minor problem. IMHO, the problem is certainly not large enough to merit such a far-ranging change as you are suggesting. > so I think I'd better spend my time on other stuff and let you guys with > "authority" fix the mess. I'll just ignore any PMS rule that I consider > wrong and doesn't come with a proper solution. Well, PMS was written by humans and there are bugs in it. Look at the specification of "doman" for another example close-by: "Installs a man page" implies that exactly one argument is allowed. Do you agree that we should fix the spec here? Or should we make package managers comply with the PMS's current wording? (In reply to Alexis Ballier from comment #21) > The wording could very well be clarified in a way that states 'calling those > functions with no argument returns success'. Let's assume for the moment that we did this, and follow it through: Then ebuild authors would be within their right if they added do* statements with an empty argument list. Of course, they would break on users' systems because the PM implements it differently. However, any bugs reported against these ebuilds could be closed as wontfix, because the ebuild would comply with PMS. I wonder what would be the reaction of the "PMS should reflect reality" faction in such a scenario. ;-)
(In reply to Ulrich Müller from comment #27) > (In reply to Alexis Ballier from comment #26) > > You are not even trying to understand my point, > > You've stated your point clearly enough, but that doesn't mean that I'll buy > it. Currently all install functions error out if called with no argument > (except for dodoc in Portage since some unknown time), and I think that this > is the desired behaviour. I agree but I don't think it's worth retroactively breaking stuff. > It's also the behaviour that you'll find > throughout in Unix commands (look at chmod, mkdir, rm, or touch, for > example) and there's good reason for it. Calling a command with an empty > argument list is usually an indication that something went wrong in the > caller. We should have very good reason if we want to deviate from this > convention. In this case (DOCS=""), nothing went wrong and it is the desired behavior because of a flaw in the default src_install. Your analogy doesn't really stand since those unix commands are specified with a mandatory argument. e.g.: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/touch.html touch [-acm] [-r ref_file|-t time|-d date_time] file... > In the current case, the breakage is in the EAPI 4 default_src_install > function, so this is what should be fixed. It affects few ebuilds only, so I > think it's even a minor problem. IMHO, the problem is certainly not large > enough to merit such a far-ranging change as you are suggesting. You are still not understanding my point: There is a weird unspecified zone for the no arg case in PMS. You claim to be clarifying it by forbidding it. I claim to be clarifying it by allowing it. Both changes are the same in nature, are just as much 'far-ranging', they only go in opposite directions. Also, please keep in mind that allowing dodoc with no arg requires less changes than forbiding it. > > so I think I'd better spend my time on other stuff and let you guys with > > "authority" fix the mess. I'll just ignore any PMS rule that I consider > > wrong and doesn't come with a proper solution. > > Well, PMS was written by humans and there are bugs in it. Look at the > specification of "doman" for another example close-by: "Installs a man page" > implies that exactly one argument is allowed. Do you agree that we should > fix the spec here? Or should we make package managers comply with the PMS's > current wording? Yes I agree we should fix the spec. If there is a flaw in PMS then it should be fixed in the most pragmatic way. In this dodoc case, since EAPI4 default src_install relies on this, the pragmatic choice seems to be allowing it. > (In reply to Alexis Ballier from comment #21) > > The wording could very well be clarified in a way that states 'calling those > > functions with no argument returns success'. > > Let's assume for the moment that we did this, and follow it through: Then > ebuild authors would be within their right if they added do* statements with > an empty argument list. Of course, they would break on users' systems > because the PM implements it differently. However, any bugs reported against > these ebuilds could be closed as wontfix, because the ebuild would comply > with PMS. I wonder what would be the reaction of the "PMS should reflect > reality" faction in such a scenario. ;-) There is also common sense and making developer's life easier. A do* function without arg in an ebuild, even legal, is just pure noise. It is just as wrong as adding an ebuild named -r0; even if it was legal, there was no point in doing so except triggering broken corner cases in some PMs. Allowing DOCS='' with default src_install makes developer's life easier. Forbidding it makes them copy/paste some code.
(In reply to Alexis Ballier from comment #28) > I agree but I don't think it's worth retroactively breaking stuff. Nobody wants to break things. We are talking about a QA warning. > [...] > Allowing DOCS='' with default src_install makes developer's life easier. > Forbidding it makes them copy/paste some code. Right, that's the core of the issue, and I hope that everyone agrees that the final goal is to allow such usage. We disagree how things under the surface should work: - We could follow your suggestion and change the PMS to say (quoting from comment #21): 'calling those functions with no argument returns success'. This would mean that _all_ existing implementations of the do* functions would have to be changed. (Even dodoc in Portage would have to be changed, because for EAPI 3 and earlier it still dies if called without argument.) IMHO this is an intrusive change. - Or we could update the PMS so that the no-argument case would be forbidden. dodoc in Portage would get a QA warning (not to break users' systems) and eclass functions currently calling dodoc without arguments would be fixed. In EAPI 6 we would fix the package manager's default src_install function. For EAPI 4 and 5 it doesn't support empty DOCS, but if I believe TomWij's list (see below) then it turns out that this functionality isn't used by any ebuild. (In reply to Tom Wijsman (TomWij) from comment #17) > Despite that, a large share can be caught with a grep so we can fix them > already (unless we will support this particular matter instead of removing > it); and by the looks of it, there aren't that much occurrences: > > https://gist.github.com/TomWij/a7ea452f73ce26e63f75 I've just checked the 20 ebuilds listed. It turns out that they are all using some eclass's src_install function, i.e. none of them is using the package manager's default implementation.
(In reply to Ulrich Müller from comment #29) > > Allowing DOCS='' with default src_install makes developer's life easier. > > Forbidding it makes them copy/paste some code. > > Right, that's the core of the issue, and I hope that everyone agrees that > the final goal is to allow such usage. We disagree how things under the > surface should work: > > - We could follow your suggestion and change the PMS to say (quoting from > comment #21): 'calling those functions with no argument returns success'. > This would mean that _all_ existing implementations of the do* functions > would have to be changed. (Even dodoc in Portage would have to be changed, > because for EAPI 3 and earlier it still dies if called without argument.) > IMHO this is an intrusive change. That way is intrusive, yes. What could be written, which would reflect better reality and be the least retroactive imho, is: In EAPIs 0-5, the behavior of these functions when called without argument is implementation defined. As a special exception, due to a design problem in the default src_install, dodoc in EAPIs 4-5 must not die if called without argument. (In EAPIs 6+, calling these functions without argument is an error.) That way, the QA warning will make more sense and eclasses can be fixed to work with future eapis, which would give a candidate on how to define an eapi6 default src_install.
(In reply to Alexis Ballier from comment #30) > That way is intrusive, yes. What could be written, which would reflect > better reality and be the least retroactive imho, is: > In EAPIs 0-5, the behavior of these functions when called without argument > is implementation defined. As a special exception, due to a design problem > in the default src_install, dodoc in EAPIs 4-5 must not die if called > without argument. I'd rather retroactively fix the spec for default_src_install. It has the same effect (assuming that eclasses would be synced to it). The damage is done in any case, and shifting the blame from Portage to PMS or vice versa doesn't buy us anything.
(In reply to Ulrich Müller from comment #31) > (In reply to Alexis Ballier from comment #30) > > That way is intrusive, yes. What could be written, which would reflect > > better reality and be the least retroactive imho, is: > > In EAPIs 0-5, the behavior of these functions when called without argument > > is implementation defined. As a special exception, due to a design problem > > in the default src_install, dodoc in EAPIs 4-5 must not die if called > > without argument. > > I'd rather retroactively fix the spec for default_src_install. It has the > same effect (assuming that eclasses would be synced to it). That would work for me too; even if fixing the spec is a one line diff, I'm not very comfortable with the fact that this changes the specification of something that was clearly defined (default src_install), while what I have written above only defines something that was not before (even if it adds some crap due to historical design issues). IMHO it boils down to keeping PMS simple, sane and having EAPIs to provide upgrade paths (which were their original goal I think) vs. having PMS set rules in stone and keep the changes minimal, preferably only filling gaps that were overlooked.
(In reply to Ciaran McCreesh from comment #23) > (In reply to Julian Ospald (hasufell) from comment #13) > > You didn't understand what I was referring to. YOU have to prove that the PM > > change will not break ANY stable ebuild. If you cannot, then it will not be > > applied. > > Any ebuild that this breaks is broken anyway (remember, "works for me" > doesn't mean it's not broken). All we're doing here is making that breakage > visible. I am glad you do not have commit access. Voluntarily breaking stable branch is a reason to revoke such access.
If it isn't clear by now, of course I have nothing against a QA warning, although it might give some false positives, but that doesn't sound severe.
(In reply to Julian Ospald (hasufell) from comment #33) > (In reply to Ciaran McCreesh from comment #23) > > (In reply to Julian Ospald (hasufell) from comment #13) > > > You didn't understand what I was referring to. YOU have to prove that the PM > > > change will not break ANY stable ebuild. If you cannot, then it will not be > > > applied. > > > > Any ebuild that this breaks is broken anyway (remember, "works for me" > > doesn't mean it's not broken). All we're doing here is making that breakage > > visible. > > I am glad you do not have commit access. Voluntarily breaking stable branch > is a reason to revoke such access. This is not "breaking stable". If any package has problems with this, it is already broken. All this is doing is changing an invisible error into a visible error so it can be fixed.
(In reply to Ciaran McCreesh from comment #35) > (In reply to Julian Ospald (hasufell) from comment #33) > > (In reply to Ciaran McCreesh from comment #23) > > > (In reply to Julian Ospald (hasufell) from comment #13) > > > > You didn't understand what I was referring to. YOU have to prove that the PM > > > > change will not break ANY stable ebuild. If you cannot, then it will not be > > > > applied. > > > > > > Any ebuild that this breaks is broken anyway (remember, "works for me" > > > doesn't mean it's not broken). All we're doing here is making that breakage > > > visible. > > > > I am glad you do not have commit access. Voluntarily breaking stable branch > > is a reason to revoke such access. > > This is not "breaking stable". If any package has problems with this, it is > already broken. All this is doing is changing an invisible error into a > visible error so it can be fixed. Repeating your nonsense doesn't make it more true.
I suggest that we wait for mgorny's "einstalldocs" function being implemented in eutils. Then eclasses can migrate to this (or be fixed in another fashion not to call dodoc without argument). After that's done, we can see how to proceed further. My suggestion would be to add a QA warning to Portage's dodoc. As for retroactively changing default_src_install, I'm indifferent about that. On the one hand, any retroactive change is nasty and should be avoided. On the other hand, it wouldn't change Portage behaviour; if checking for empty DOCS takes place in dodoc or in the caller doesn't make any difference. (Not so for Paludis, I fear. But as I said before, the damage is already done.) And changing default_src_install would still be much cleaner than the messy spec that would result from making the no-argument behaviour unspecified. IMHO.
(In reply to Ulrich Müller from comment #37) > After that's done, we can see how to proceed further. My suggestion would be > to add a QA warning to Portage's dodoc. Your patch is in git: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=83ac345bb227dc1752a895d53037fce36c9c7966 > As for retroactively changing default_src_install, I'm indifferent about > that. Retroactively changing it sounds reasonable to me.
Clarified PMS wording was approved by the council today: http://git.overlays.gentoo.org/gitweb/?p=proj/pms.git;a=commit;h=8ed75a9fb02c757759132e8bfbdea6cbab09bd98 Since a QA warning in Portage's dodoc implementation is also in place, I think this bug can be closed. Please reopen if you disagree.
