Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 417229 - [Future EAPI] die if some set of commands are not found
Summary: [Future EAPI] die if some set of commands are not found
Status: RESOLVED WONTFIX
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Core (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: PMS/EAPI
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: future-eapi
  Show dependency tree
 
Reported: 2012-05-23 08:07 UTC by Pacho Ramos
Modified: 2017-09-08 19:41 UTC (History)
2 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Pacho Ramos gentoo-dev 2012-05-23 08:07:30 UTC
This comes from:
https://bugs.gentoo.org/show_bug.cgi?id=417153

and other bugs related with failing to run epatch. Some set of commands are really expected to be present and, then, emerge should immediately die when they are not found instead of simply show a qa warning (that will be hidden for most people).

This commands are (at least):
- epatch -> currently, portage will simply skip patches and this can install a vulnerable/buggy package
- 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

Thanks

Reproducible: Always
Comment 1 Zac Medico gentoo-dev 2012-05-23 08:17:28 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.
Comment 2 Zac Medico gentoo-dev 2012-05-23 08:21:54 UTC
(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?
Comment 3 Pacho Ramos gentoo-dev 2012-05-23 08:26:37 UTC
(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
Comment 4 Zac Medico gentoo-dev 2012-05-23 08:39:18 UTC
(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.
Comment 5 Brian Harring gentoo-dev 2012-05-23 10:26:35 UTC
(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...
Comment 6 Ulrich Müller gentoo-dev 2012-05-23 10:49:20 UTC
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.
Comment 7 Pacho Ramos gentoo-dev 2012-05-23 10:57:51 UTC
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 :-/
Comment 8 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2012-05-23 16:49:50 UTC
(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'...
Comment 9 Pacho Ramos gentoo-dev 2012-05-23 19:25:30 UTC
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
Comment 10 SpanKY gentoo-dev 2012-05-24 05:01:31 UTC
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 ...
Comment 11 Ulrich Müller gentoo-dev 2013-03-16 09:42:58 UTC
(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?
Comment 12 Zac Medico gentoo-dev 2013-03-16 14:47:03 UTC
(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.
Comment 13 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2014-09-06 22:12:33 UTC
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.
Comment 14 Pacho Ramos gentoo-dev 2016-12-10 10:36:40 UTC
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... :)
Comment 15 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2017-09-08 19:41:08 UTC
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.