| Summary: | [Future EAPI] die if some set of commands are not found | ||
|---|---|---|---|
| Product: | Portage Development | Reporter: | Pacho Ramos <pacho> |
| Component: | Core | Assignee: | PMS/EAPI <pms> |
| Status: | RESOLVED WONTFIX | ||
| Severity: | normal | CC: | esigra, mgorny |
| Priority: | Normal | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | |||
| Bug Blocks: | 174380 | ||
|
Description
Pacho Ramos
2012-05-23 08:07:30 UTC
(In reply to comment #0) ? emerge should immediately die when > they are not found instead of simply show a qa warning (that will be hidden > for most people). Making it die would have to be an EAPI extension. There was some related discussion about using a bash trap for errors like this, in bug 138792. > This commands are (at least): > - epatch -> currently, portage will simply skip patches and this can install > a vulnerable/buggy package A repoman check is already planned for this (bug 417159). > - enewuser/enewgroup -> currently portage will install the package even if > it won't run at all, and people will need to rebuild it after fixed > - probably more that I am forgetting We can add a repoman check for this too. (In reply to comment #1) > > - enewuser/enewgroup -> currently portage will install the package even if > > it won't run at all, and people will need to rebuild it after fixed > > - probably more that I am forgetting > > We can add a repoman check for this too. Oh, I see bug 417231 now. (In reply to comment #1) > (In reply to comment #0) > ? emerge should immediately die when > > they are not found instead of simply show a qa warning (that will be hidden > > for most people). > > Making it die would have to be an EAPI extension. There was some related > discussion about using a bash trap for errors like this, in bug 138792. So, do you want to re-assign this to pms-bugs@g.o, requesting an EAPI extension? (In reply to comment #1) > (In reply to comment #0) > ? emerge should immediately die when > > they are not found instead of simply show a qa warning (that will be hidden > > for most people). > > Making it die would have to be an EAPI extension. There was some related > discussion about using a bash trap for errors like this, in bug 138792. > In that case, I would like to see this discussed for future-eapis and, until then, should we explicitly add "|| die" for this uncovered commands? > > This commands are (at least): > > - epatch -> currently, portage will simply skip patches and this can install > > a vulnerable/buggy package > > A repoman check is already planned for this (bug 417159). > > > - enewuser/enewgroup -> currently portage will install the package even if > > it won't run at all, and people will need to rebuild it after fixed > > - probably more that I am forgetting > > We can add a repoman check for this too. Yes, I reported a bug for it too some minutes ago (In reply to comment #3) > In that case, I would like to see this discussed for future-eapis and, until > then, should we explicitly add "|| die" for this uncovered commands? Well, I think a repoman check is probably sufficient. (In reply to comment #4) > (In reply to comment #3) > > In that case, I would like to see this discussed for future-eapis and, until > > then, should we explicitly add "|| die" for this uncovered commands? > > Well, I think a repoman check is probably sufficient. Agreed, although that's not saying I like it. Realistically, that functionality belongs in the manager anyways... We'll never be able to catch any possible mistake. And there's really no excuse if an ebuild is calling epatch without inheriting eutils. So, keep it simple please. +1 for a repoman check. Why not standardize then usage of "|| die" with epatch and others like we already do with "sed", for example
epatch "${FILESDIR}/1.patch" || die
would be the idea :-/
(In reply to comment #7) > Why not standardize then usage of "|| die" with epatch and others like we > already do with "sed", for example > > epatch "${FILESDIR}/1.patch" || die > > would be the idea :-/ Because people were too lazy to type '|| die'. So we invented a hack of packing random 'die' calls into our commands. What would be a better solution? Maybe a nicely working 'set -e'... Other option would be that, since portage is currently able to show a QA warning when a command is not found, it could also die at that step (before installing, that is the important think). The problem would be false positives... but I am not sure if there are so much packages having "valid" command not found issues :/ Regarding usage of die, I was talking to do that as "policy" (like done with other commands like sed) for old eapis i've posted repoman patches to implement eclass checking and seems to be pretty good ... so if we're happy with that, once it gets released, devs will quickly fix their own packages ... (In reply to comment #10) > i've posted repoman patches to implement eclass checking and seems to be > pretty good ... so if we're happy with that, once it gets released, devs > will quickly fix their own packages ... Have these repoman patches been released? (In reply to comment #11) > (In reply to comment #10) > > i've posted repoman patches to implement eclass checking and seems to be > > pretty good ... so if we're happy with that, once it gets released, devs > > will quickly fix their own packages ... > > Have these repoman patches been released? In portage-2.2, they are treated as warnings. I don't think it should be enabled in stable until we make the checks use data parsed directly from eclasses. I'd say we could achieve this quite easily using command_not_found_handle() trick. However, there are two problems here: 1. Some eclasses/ebuilds actually just try to run commands and accept failures rather than checking for their existence first. This is fixable with EAPI bump but will make the code a bit longer. 2. I don't know if Portage internals don't rely on that somewhere. This is even more fixable :). As I see it, we certainly can do this. Well, I just reminded this today because of a failing eautoreconf call due to forgotten inherit of autotools.eclass, I am not sure if this could be reviewed for eapi7 or... :) We've discussed this today, and ulm pointed out that the implicit handling of command-not-found errors would suppress explicit '|| die' calls which -- even if not exactly meant to handle missing commands -- could provide more helpful error messages. Instead, we may look into enforcing this more on Portage end / as a Gentoo policy. |
