Bug 604080 – [Future EAPI] Follow POSIX Utility Syntax Guidelines for command options

• Product: Gentoo Hosted Projects
• Status: CONFIRMED ---

[Description Elliot Chandler 2016-12-29 21:10:31 UTC]

Using, for instance, doins -r -- * in an ebuild causes it to fail:

!!! doins: -- does not exist

This may also affect other ebuild functions. It is important to correctly handle this, because running doins * on a package that contains a file named "-r" will cause directories to be installed recursively, which would not be the expected behavior.

This is requested by POSIX utility guideline 10. (http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html)

Thank you!

[Comment 1 Ulrich Müller 2016-12-29 22:30:06 UTC]

I believe this to be a valid issue, but it won't have high priority, for the following reasons. Option parsing in shell scripts is clumsy because there is no standard tool for it. getopt(1) is part of util-linux and therefore not available on all systems. Bash's getopts function doesn't accept GNU long options, which are used by some of the helper functions (like has_version and best_version). Also most ebuild helpers have at only a single option and will simply look at the first argument passed. (So if e.g. the first argument of doins isn't equal to -r but starts with a hyphen, it will be taken as a filename. And yes, this sucks.)

Also an ebuild trying to install a file named -r would be a QA violation by itself, therefore this is a rather theoretical issue.

Tagging with "NeedPatch"; if anyone provides a working implementation for portage then we can consider this for a future EAPI.

[Comment 2 Michał Górny 2016-12-29 22:33:22 UTC]

There is really no need to complexify PMS with unnecessarily complex option parsing. 

The solution is simple: don't use *, use 'doins -r .'. This will save you unnecessary filename substitution. If you really need to play globs, ./* will also work.

[Comment 3 Elliot Chandler 2016-12-29 23:13:36 UTC]

Created attachment 457892 [details, diff]
Patch for bug #604080

I think this is all that is needed, but I haven't tested it yet. (Also, it affects all EAPIs, so in the unlikely event that an ebuild installs a file named "--" as the first argument to doins, it would break that ebuild.)

[Comment 4 Zac Medico 2016-12-29 23:20:52 UTC]

(In reply to kolu bat from comment #3)
> Created attachment 457892 [details, diff] [details, diff]
> Patch for bug #604080
> 
> I think this is all that is needed, but I haven't tested it yet. (Also, it
> affects all EAPIs, so in the unlikely event that an ebuild installs a file
> named "--" as the first argument to doins, it would break that ebuild.)

That patch seem like it should work. We can make it conditional on EAPI with something like this:

if ___eapi_has_doins_double_hyphen && [[ "$1" == "--" ]] ; then
	shift
fi

[Comment 5 Elliot Chandler 2016-12-29 23:31:51 UTC]

Created attachment 457894 [details, diff]
New patch for bug #604080

(In reply to Zac Medico from comment #4)

Yeah, totally. (Here's a patch for that, if anyone wants.)

[Comment 6 Ulrich Müller 2017-01-02 21:45:37 UTC]

(In reply to kolu bat from comment #3)
> Created attachment 457892 [details, diff] [details, diff]
> Patch for bug #604080

And what will happen if an unrecognised option is passed ("doins -a foo bar") or if -r is passed twice ("doins -r -r foo bar" or "doins -rr foo bar")? I would expect the former to abort with an error, and the latter to operate as if -r was specified only once.

Either we should make all helpers (or at least the ones that accept options at all) follow POSIX Utility Syntax Guidelines 3 to 14, or we should leave things as they currently are. I won't be happy about introducing an EAPI dependent change for an incomplete solution.

[Comment 7 Ulrich Müller 2017-01-02 23:04:30 UTC]

Something like the following should work (not tested):

    DOINSRECUR=n
    while getopts "r" opt; do
        case ${opt} in
            r) DOINSRECUR=y ;;
            *) __helpers_die "${helper}: bad option ${opt}"; exit 1 ;;
        esac
    done
    shift $((OPTIND-1))

Note that getopts cannot handle long options, so the {has,best}_version functions would either require special treatment, or would have to change to some one letter option instead of --host-root.