Bug 201771 – sys-apps/portage - unpack() should reject absolute paths + misleading error message

• Product: Portage Development
• Status: RESOLVED FIXED

[Description Jakub Moc (RETIRED) 2007-12-09 14:40:42 UTC]

Well, basically making a mistake in ebuild and passing an absolute tarball path to unpack() made me waste quite some time due to completely misleading message. Since unpack only accepts ./ as path, it should reject all the rest instead with a proper error message.

In case it's really needed, an ebuild to reproduce is at 
http://overlays.gentoo.org/svn/proj/sunrise/reviewed/media-sound/asoundconf/asoundconf-1.0.14.ebuild, just replace unpack ./data.tar.gz with unpack "${S}/data.tar.gz and watch this:

 * ERROR: media-sound/asoundconf-1.0.14 failed.
 * Call stack:
 *               ebuild.sh, line   46:  Called src_unpack
 *             environment, line  164:  Called unpack 'src_unpack'
 *               ebuild.sh, line  324:  Called die
 * The specific snippet of code:
 *   		[ ! -s "${srcdir}${x}" ] && die "$myfail"
 *  The die message:
 *   /var/tmp/portage/media-sound/asoundconf-1.0.14/work/data.tar.gz does not exist

Well... that isn't useful indication of the problem at all:

$ file /var/tmp/portage/media-sound/asoundconf-1.0.14/work/data.tar.gz
/var/tmp/portage/media-sound/asoundconf-1.0.14/work/data.tar.gz: gzip compressed data, from Unix, max compression

[Comment 1 Fabian Groffen 2007-12-09 14:48:09 UTC]

more detailed:

unpack() prefixes its argument with ${DISTDIR} unconditionally unless it starts with "./".  This is fine, but results in the confusing error message when used with an absolute path, as it looks for ${DISTDIR}/abspath/foo.

The error message could be improved by printing ${srcdir}${x} to make it print the full path it is also checking.  However, that results in some bloat that probably shouldn't be triggered at all, by simply spitting out an error message if the argument given to unpack starts with a / (and hence is absolute).

[Comment 2 Fabian Groffen 2007-12-09 14:58:58 UTC]

Created attachment 138096 [details, diff]
suggested patch for unpack()

Attached a suggestion for how to change unpack()'s behaviour to refuse absolute paths.  I don't know if it is useful to allow subdirs in unpack (e.g. unpack somedir/data.tar.bz2), otherwise that could be rejected as well.

[Comment 3 SpanKY 2007-12-09 23:00:15 UTC]

please fix the wording while you're in there

"should not" -> "cannot"

[Comment 4 Zac Medico 2007-12-14 01:15:01 UTC]

This has been released in 2.1.4_rc10.

(In reply to comment #3)
> please fix the wording while you're in there
> 
> "should not" -> "cannot"

done