Bug 691776 – portage-2.3.71: unpack should unconditionally die if an unpacker returns an error

• Product: Portage Development
• Status: RESOLVED FIXED

[Description Ulrich Müller 2019-08-08 20:14:48 UTC]

unpack calls __helpers_die if an unpacker returns with an error.

PMS says in https://projects.gentoo.org/pms/7/pms.html#x1-13500012.3.15:
"If unpacking a supported file format fails, unpack shall abort the build process."

So unpack should call die, not __helpers_die. (For some reason, 7z does that already, as only unpacker).

[Comment 1 Ulrich Müller 2019-08-12 13:11:56 UTC]

Created attachment 586608 [details, diff]
unpack: Unconditionally die if an unpacker returns an error.

[Comment 2 Mike Gilbert 2019-08-12 16:18:14 UTC]

Is unpack() not supposed to respect "nonfatal"?

https://projects.gentoo.org/pms/7/pms.html#x1-12100012.3.1

[Comment 3 Ulrich Müller 2019-08-12 20:21:09 UTC]

(In reply to Mike Gilbert from comment #2)
> Is unpack() not supposed to respect "nonfatal"?
> 
> https://projects.gentoo.org/pms/7/pms.html#x1-12100012.3.1

No. unpack() already died unconditionally in EAPI 0, therefore nonfatal behaviour wasn't added to the spec. You can also deduce the correct behaviour from the default src_unpack implementation for EAPI 0, which simply calls unpack without checking for error status.

AFAICS, the current status is that default_src_unpack in EAPIs 0 to 2 doesn't do any error checking at all, which doesn't look right to me.

This commit broke it:

commit 525e69351d45621c34a9326fcbc11ca592cb6539
Author: Michał Górny <[e-mail]>
Date:   Sun Nov 30 13:12:58 2014 +0100

    Respect nonfatal in unpack(), econf() and einstall()

[Comment 4 Ulrich Müller 2019-08-12 20:23:14 UTC]

Created attachment 586692 [details, diff]
unpack: Unconditionally die if an unpacker returns an error.

Presumably, it will be cleaner to revert commit 5761a2ec (as far as it affects unpack).

[Comment 5 Larry the Git Cow 2019-08-14 02:08:30 UTC]

The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/proj/portage.git/commit/?id=17ecafa949c87a6f2a2d2c98c7de18ed06f08f2f

commit 17ecafa949c87a6f2a2d2c98c7de18ed06f08f2f
Author:     Ulrich Müller <[e-mail]>
AuthorDate: 2019-08-12 20:20:16 +0000
Commit:     Zac Medico <[e-mail]>
CommitDate: 2019-08-14 02:02:44 +0000

    unpack: Unconditionally die if an unpacker returns an error.
    
    As specified by PMS: "If unpacking a supported file format fails,
    unpack shall abort the build process."
    https://projects.gentoo.org/pms/7/pms.html#x1-13500012.3.15:
    
    This partially reverts commit 525e69351d45621c34a9326fcbc11ca592cb6539,
    as far as unpack() is concerned.
    
    Bug: https://bugs.gentoo.org/691776
    Signed-off-by: Ulrich Müller <[e-mail]>
    Signed-off-by: Zac Medico <[e-mail]>

 bin/isolated-functions.sh | 13 ++-----
 bin/phase-helpers.sh      | 87 +++++++++++++----------------------------------
 2 files changed, 27 insertions(+), 73 deletions(-)

[Comment 6 Zac Medico 2019-08-14 02:08:52 UTC]

Thanks!

[Comment 7 Ulrich Müller 2019-08-14 03:18:12 UTC]

(In reply to Ulrich Müller from comment #4)
> Created attachment 586692 [details, diff] [details, diff]
> unpack: Unconditionally die if an unpacker returns an error.
> 
> Presumably, it will be cleaner to revert commit 5761a2ec (as far as it
> affects unpack).

This should have read "commit 525e6935" of course.
And I see that you've noticed and corrected it before pushing. :-)

[Comment 8 Larry the Git Cow 2019-08-19 05:06:23 UTC]

The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=08557524dc6c8eec3a366e43ab2587d2cdd8f133

commit 08557524dc6c8eec3a366e43ab2587d2cdd8f133
Author:     Zac Medico <[e-mail]>
AuthorDate: 2019-08-19 04:24:07 +0000
Commit:     Zac Medico <[e-mail]>
CommitDate: 2019-08-19 05:06:15 +0000

    sys-apps/portage: Bump to version 2.3.72
    
     #463952 glsa-check: install in /usr/bin
     #646090 preserve-libs: get dep graph from EROOT
     #690484 detect internal collisions for /usr merge
     #690786 repoman: support metadata/layout.conf restrict-allowed
     #691776 unpack: Unconditionally die if an unpacker returns an error
     #691638 Show get/setfattr stderr
     #692024 econf: Unconditionally die on error in EAPIs 0 to 3
     #692262 QA Notice: EXPORT_FUNCTIONS is called before inherit in
             kernel-2.eclass
     #692412 emerge IndexError for ambiguous package atom with pypy
    
    Bug: https://bugs.gentoo.org/691278
    Bug: https://bugs.gentoo.org/463952
    Bug: https://bugs.gentoo.org/646090
    Bug: https://bugs.gentoo.org/690484
    Bug: https://bugs.gentoo.org/690786
    Bug: https://bugs.gentoo.org/691776
    Bug: https://bugs.gentoo.org/691638
    Bug: https://bugs.gentoo.org/692024
    Bug: https://bugs.gentoo.org/692262
    Bug: https://bugs.gentoo.org/692412
    Package-Manager: Portage-2.3.71, Repoman-2.3.17
    Signed-off-by: Zac Medico <[e-mail]>

 sys-apps/portage/Manifest              |   1 +
 sys-apps/portage/portage-2.3.72.ebuild | 264 +++++++++++++++++++++++++++++++++
 2 files changed, 265 insertions(+)