[gentoo-dev] New eclass: eapi9-pipestatus.eclass

[gentoo-dev] New eclass: eapi9-pipestatus.eclass

[Ulrich Müller]

[-- Attachment #1.1: Type: text/plain, Size: 970 bytes --]

This implements a pipestatus command, as discussed in bug 566342 [1]
and on IRC.

Subject to approval by the council, the command would be added in
EAPI 9. Its definition in the Package Manager Specification would be
along the lines of:

╓────
║ Tests the shell's pipe status array, i.e. the exit status of the
║ command(s) in the most recently executed foreground pipeline.
║ Returns shell true (0) if all elements are zero, or the last
║ non-zero element otherwise. If called with -v as the first argument,
║ also outputs the pipe status array as a space-separated list.
╙────

The "assert" command could then be banned (either in the same EAPI or
in the following EAPI), typically to be replaced by "pipestatus || die".

I would commit the eclass after the next council meeting, and obviously
only under the condition that the council will pre-approve the command
for EAPI 9.

[1] https://bugs.gentoo.org/566342


[-- Attachment #1.2: eapi9-pipestatus.eclass --]
[-- Type: text/plain, Size: 1556 bytes --]

# Copyright 2024 Gentoo Authors
# Distributed under the terms of the GNU General Public License v2

# @ECLASS: eapi9-pipestatus.eclass
# @MAINTAINER:
# Ulrich Müller <ulm@gentoo.org>
# @AUTHOR:
# Ulrich Müller <ulm@gentoo.org>
# @SUPPORTED_EAPIS: 7 8
# @BLURB: test the PIPESTATUS array
# @DESCRIPTION:
# A stand-alone implementation of a possible future pipestatus command
# (which would be aimed for EAPI 9).  It is meant as a replacement for
# "assert".  In its simplest form it can be called like this:
#
# @CODE
# foo | bar
# pipestatus || die
# @CODE
#
# With the -v option, the command will also echo the pipe status array,
# so it can be assigned to a variable like in the following example:
#
# @CODE
# local status
# foo | bar
# status=$(pipestatus -v) || die "foo | bar failed, status ${status}"
# @CODE

case ${EAPI} in
	7|8) ;;
	*) die "${ECLASS}: EAPI ${EAPI:-0} not supported" ;;
esac

# @FUNCTION: pipestatus
# @USAGE: [-v]
# @RETURN: last non-zero element of PIPESTATUS, or zero if all are zero
# @DESCRIPTION:
# Test the PIPESTATUS array, i.e. the exit status of the command(s)
# in the most recently executed foreground pipeline.  If called with
# option -v, also output the PIPESTATUS array.
pipestatus() {
	local -a status=( "${PIPESTATUS[@]}" )
	local s ret=0

	[[ $# -gt 0 && ${1} != -v || $# -gt 1 ]] \
		&& die "${FUNCNAME}: bad arguments: $@"

	[[ ${1} == -v ]] && echo "${status[@]}"

	for s in "${status[@]}"; do
		[[ ${s} -ne 0 ]] && ret=${s}
	done

	return "${ret}"
}

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]

Re: [gentoo-dev] New eclass: eapi9-pipestatus.eclass

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 2227 bytes --]

On Sun, 2024-11-24 at 13:24 +0100, Ulrich Müller wrote:
> # Copyright 2024 Gentoo Authors
> # Distributed under the terms of the GNU General Public License v2
> 
> # @ECLASS: eapi9-pipestatus.eclass
> # @MAINTAINER:
> # Ulrich Müller <ulm@gentoo.org>
> # @AUTHOR:
> # Ulrich Müller <ulm@gentoo.org>
> # @SUPPORTED_EAPIS: 7 8
> # @BLURB: test the PIPESTATUS array
> # @DESCRIPTION:
> # A stand-alone implementation of a possible future pipestatus command
> # (which would be aimed for EAPI 9).  It is meant as a replacement for
> # "assert".  In its simplest form it can be called like this:
> #
> # @CODE
> # foo | bar
> # pipestatus || die
> # @CODE
> #
> # With the -v option, the command will also echo the pipe status array,
> # so it can be assigned to a variable like in the following example:
> #
> # @CODE
> # local status
> # foo | bar
> # status=$(pipestatus -v) || die "foo | bar failed, status ${status}"

I suppose you may want to put a verbose warning not to put "local"
on the same line, because people are going to do that as an "obvious"
optimization.

> # @CODE
> 
> case ${EAPI} in
> 	7|8) ;;
> 	*) die "${ECLASS}: EAPI ${EAPI:-0} not supported" ;;
> esac
> 
> # @FUNCTION: pipestatus
> # @USAGE: [-v]
> # @RETURN: last non-zero element of PIPESTATUS, or zero if all are zero
> # @DESCRIPTION:
> # Test the PIPESTATUS array, i.e. the exit status of the command(s)
> # in the most recently executed foreground pipeline.  If called with
> # option -v, also output the PIPESTATUS array.
> pipestatus() {
> 	local -a status=( "${PIPESTATUS[@]}" )
> 	local s ret=0
> 
> 	[[ $# -gt 0 && ${1} != -v || $# -gt 1 ]] \

Please use parentheses when you combine && and ||, if only for the sake
of readability.

> 		&& die "${FUNCNAME}: bad arguments: $@"

Replace the '\' with the '&&'.

> 
> 	[[ ${1} == -v ]] && echo "${status[@]}"
> 
> 	for s in "${status[@]}"; do
> 		[[ ${s} -ne 0 ]] && ret=${s}

I suppose it's just my C-foo talking and completely needless
optimization here, but it really itches me to iterate the array
backwards and return on the first match.

> 	done
> 
> 	return "${ret}"
> }

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [gentoo-dev] New eclass: eapi9-pipestatus.eclass

[Ulrich Müller]

[-- Attachment #1: Type: text/plain, Size: 1953 bytes --]

>>>>> On Sun, 24 Nov 2024, Michał Górny wrote:

>> # @CODE
>> # local status
>> # foo | bar
>> # status=$(pipestatus -v) || die "foo | bar failed, status ${status}"

> I suppose you may want to put a verbose warning not to put "local"
> on the same line, because people are going to do that as an "obvious"
> optimization.

Thank you for the feedback. I have added the following:

# Caveat: "pipestatus" must be the next command following the pipeline.
# In particular, the "local" declaration must be before the pipeline,
# otherwise it will reset the status.

>> [[ $# -gt 0 && ${1} != -v || $# -gt 1 ]] \

> Please use parentheses when you combine && and ||, if only for the sake
> of readability.

I've replaced it by:

    [[ $# -gt 0 && ( ${1} != -v || $# -gt 1 ) ]]

Note that

    [[ ( $# -gt 0 && ${1} != -v ) || $# -gt 1 ]]

would also work, i.e. order doesn't matter (the reason is that "-gt 1"
implies "-gt 0").

>> && die "${FUNCNAME}: bad arguments: $@"

> Replace the '\' with the '&&'.

We don't have any policy on this, and IMHO it is clearer to split the
line before the operator. (This is also what the GNU coding standards
say, and what is used in mathematical typesetting.)

>> [[ ${1} == -v ]] && echo "${status[@]}"
>> 
>> for s in "${status[@]}"; do
>> [[ ${s} -ne 0 ]] && ret=${s}

> I suppose it's just my C-foo talking and completely needless
> optimization here, but it really itches me to iterate the array
> backwards and return on the first match.

I had considered this and decided against it, see the last paragraph
of https://bugs.gentoo.org/566342#c13:

| Alternatively, one could loop backwards over the array (and return
| for the first nonzero status) but it would be more complicated.
| I don't think that would be more efficient because the normal case
| is that all statūs are zero. Plus, pipelines rarely consist of more
| than 3 commands.

Ulrich

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]

Re: [gentoo-dev] New eclass v2: eapi9-pipestatus.eclass

[Ulrich Müller]

[-- Attachment #1.1: Type: text/plain, Size: 656 bytes --]

>>>>> On Sun, 24 Nov 2024, Ulrich Müller wrote:

> This implements a pipestatus command, as discussed in bug 566342 [1]
> and on IRC.

> ╓────
> ║ Tests the shell's pipe status array, i.e. the exit status of the
> ║ command(s) in the most recently executed foreground pipeline.
> ║ Returns shell true (0) if all elements are zero, or the last
> ║ non-zero element otherwise. If called with -v as the first argument,
> ║ also outputs the pipe status array as a space-separated list.
> ╙────

Updated version:
- No functional changes
- Minor style changes
- Added a comment

Thanks again for the feedback.


[-- Attachment #1.2: eapi9-pipestatus.eclass --]
[-- Type: text/plain, Size: 1757 bytes --]

# Copyright 2024 Gentoo Authors
# Distributed under the terms of the GNU General Public License v2

# @ECLASS: eapi9-pipestatus.eclass
# @MAINTAINER:
# Ulrich Müller <ulm@gentoo.org>
# @AUTHOR:
# Ulrich Müller <ulm@gentoo.org>
# @SUPPORTED_EAPIS: 7 8
# @BLURB: test the PIPESTATUS array
# @DESCRIPTION:
# A stand-alone implementation of a possible future pipestatus command
# (which would be aimed for EAPI 9).  It is meant as a replacement for
# "assert".  In its simplest form it can be called like this:
#
# @CODE
# foo | bar
# pipestatus || die
# @CODE
#
# With the -v option, the command will also echo the pipe status array,
# so it can be assigned to a variable like in the following example:
#
# @CODE
# local status
# foo | bar
# status=$(pipestatus -v) || die "foo | bar failed, status ${status}"
# @CODE
#
# Caveat: "pipestatus" must be the next command following the pipeline.
# In particular, the "local" declaration must be before the pipeline,
# otherwise it would reset the status.

case ${EAPI} in
	7|8) ;;
	*) die "${ECLASS}: EAPI ${EAPI:-0} not supported" ;;
esac

# @FUNCTION: pipestatus
# @USAGE: [-v]
# @RETURN: last non-zero element of PIPESTATUS, or zero if all are zero
# @DESCRIPTION:
# Test the PIPESTATUS array, i.e. the exit status of the command(s)
# in the most recently executed foreground pipeline.  If called with
# option -v, also output the PIPESTATUS array.
pipestatus() {
	local status=( "${PIPESTATUS[@]}" )
	local s ret=0 verbose=""

	[[ ${1} == -v ]] && { verbose=1; shift; }
	[[ $# -ne 0 ]] && die "usage: ${FUNCNAME} [-v]"

	for s in "${status[@]}"; do
		[[ ${s} -ne 0 ]] && ret=${s}
	done

	[[ ${verbose} ]] && echo "${status[@]}"

	return "${ret}"
}

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]

Re: [gentoo-dev] New eclass: eapi9-pipestatus.eclass

[Florian Schmaus]

[-- Attachment #1.1.1: Type: text/plain, Size: 2005 bytes --]

On 24/11/2024 13.24, Ulrich Müller wrote:
> This implements a pipestatus command, as discussed in bug 566342 [1]
> and on IRC.
> 
> Subject to approval by the council, the command would be added in
> EAPI 9. Its definition in the Package Manager Specification would be
> along the lines of:
> 
> ╓────
> ║ Tests the shell's pipe status array, i.e. the exit status of the
> ║ command(s) in the most recently executed foreground pipeline.
> ║ Returns shell true (0) if all elements are zero, or the last
> ║ non-zero element otherwise. If called with -v as the first argument,
> ║ also outputs the pipe status array as a space-separated list.
> ╙────

Thanks again for putting effort into this ulm.

When this was discussed in #-qa one initial version of pipestatus() had 
support for specifying non-zero exit statues as success indication for 
certain commands in the pipe. The prime example being 'grep' returning 1 
if no input matched.

One proposed version had support for this, but it was removed in later 
version and the discussion in #-qa shifted towards how the value of 
PIPESTATUS can be preserved to be included in a potential error message.

I agree that we should drop 'assert' and that dropping it requires a 
sensible named alternative. And the proposed version of pipestatus() is 
a functional equivalent alternative.

However, I am not sure why the proposed pipestatus() does no longer 
include support for specifying non-zero exit statuses as success indication.

I looked forward to use pipestatus() in texlive-modules.eclass [1], but 
since the pipe includes a 'grep', which is not uncommon, I can not use 
what is currently proposed.

It seems strange to me to go this far, but then drop the ball on the 
last meter.

Could we please consider re-adding support for this?

- Flow



1: 
https://github.com/gentoo/gentoo/blob/11b21e623fcff8948cbbd33854ffdb2a51b2840b/eclass/texlive-module.eclass#L540-L549

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 21567 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [gentoo-dev] New eclass: eapi9-pipestatus.eclass

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 301 bytes --]

On Wed, 2024-11-27 at 11:41 +0100, Florian Schmaus wrote:
> I looked forward to use pipestatus() in texlive-modules.eclass [1], but 
> since the pipe includes a 'grep', which is not uncommon, I can not use 
> what is currently proposed.

Use sed instead.

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [gentoo-dev] New eclass: eapi9-pipestatus.eclass

[Ulrich Müller]

[-- Attachment #1: Type: text/plain, Size: 1951 bytes --]

>>>>> On Wed, 27 Nov 2024, Florian Schmaus wrote:

>> ╓────
>> ║ Tests the shell's pipe status array, i.e. the exit status of the
>> ║ command(s) in the most recently executed foreground pipeline.
>> ║ Returns shell true (0) if all elements are zero, or the last
>> ║ non-zero element otherwise. If called with -v as the first argument,
>> ║ also outputs the pipe status array as a space-separated list.
>> ╙────

> Thanks again for putting effort into this ulm.

> When this was discussed in #-qa one initial version of pipestatus()
> had support for specifying non-zero exit statues as success indication
> for certain commands in the pipe. The prime example being 'grep'
> returning 1 if no input matched.

> One proposed version had support for this, but it was removed in later
> version and the discussion in #-qa shifted towards how the value of
> PIPESTATUS can be preserved to be included in a potential error
> message.

> I agree that we should drop 'assert' and that dropping it requires a
> sensible named alternative. And the proposed version of pipestatus()
> is a functional equivalent alternative.

> However, I am not sure why the proposed pipestatus() does no longer
> include support for specifying non-zero exit statuses as success
> indication.

> I looked forward to use pipestatus() in texlive-modules.eclass [1],
> but since the pipe includes a 'grep', which is not uncommon, I can not
> use what is currently proposed.

> It seems strange to me to go this far, but then drop the ball on the
> last meter.

> Could we please consider re-adding support for this?

I have dropped the feature because feedback in bug 566342 was very
negative. The main argument was it would unnecessarily complicate the
function, in order to account for a corner case.

In the discussion it was suggested to introduce a wrapper instead;
this is now bug 945110.

Ulrich

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]