Re: [rfc] leverage -x/-X tar call to pass more tar options
Hi,
Pierre Habouzit wrote:
> The point is that doing chained pipes is pretty annoying from C
> (especially if you want to use parallelism in the process), plus it kind
> of make sense for many kind of operations where you don't need
> --fsys-tarfile anymore: e.g. extracting a single file can be done with:
>
> dpkg-deb -x file.deb some-dir ./path/to/file/to/extract
I have wished for something like this before.
> +++ b/dpkg-deb/extract.c
> @@ -325,7 +329,17 @@ void extracthalf(const char *debar, const char *directory,
>
> unsetenv("TAR_OPTIONS");
>
> - execlp(TAR, "tar", buffer, "-", NULL);
> + for (i = 0; taropts && taropts[i]; i++);
Produces a warning from gcc.
> + arg = targv = m_malloc((5 + i + 1) * sizeof(targv[0]));
Why not (4 + i) * sizeof(targv[0])?
> + *arg++ = "tar";
> + *arg++ = buffer;
> + *arg++ = "-";
> + if (i) {
Needed for the !taropts case. Good.
> + mempcpy(arg, taropts, i * sizeof(taropts[0]));
> + arg += i;
> + }
> + *arg++ = NULL;
> + execvp(TAR, (char *const *)targv);
> ohshite(_("failed to exec tar"));
> }
> close(p2[0]);
I couldn’t find any other uses of mempcpy in dpkg, so it might be better
to use memcpy() or use a wrapper like git does.
> diff --git a/dpkg-deb/main.c b/dpkg-deb/main.c
> index 319e715..8238720 100644
> --- a/dpkg-deb/main.c
> +++ b/dpkg-deb/main.c
> @@ -78,8 +78,10 @@ usage(const struct cmdinfo *cip, const char *value)
> " -W|--show <deb> Show information on package(s)\n"
> " -f|--field <deb> [<cfield> ...] Show field(s) to stdout.\n"
> " -e|--control <deb> [<directory>] Extract control info.\n"
> -" -x|--extract <deb> <directory> Extract files.\n"
> -" -X|--vextract <deb> <directory> Extract & list files.\n"
> +" -x|--extract <deb> <directory> [tar-options...]\n"
> +" Extract files.\n"
> +" -X|--vextract <deb> <directory> [tar-options...]\n"
> +" Extract & list files.\n"
> " --fsys-tarfile <deb> Output filesystem tarfile.\n"
> "\n"));
>
Might be worth hinting that myopt will try to consume any tar option
flags itself. Something like:
" -x|--extract <deb> <directory> [[--] tar-options...]\n"
" Extract files.\n"
" -X|--vextract <deb> <directory> [[--] tar-options...]\n"
" Extract & list files.\n"
> diff --git a/man/dpkg-deb.1 b/man/dpkg-deb.1
> index ec451ef..c5db6e7 100644
> --- a/man/dpkg-deb.1
> +++ b/man/dpkg-deb.1
> @@ -123,9 +123,11 @@ package archive. It is currently produced in the format generated by
> .BR tar 's
> verbose listing.
> .TP
> -.BR \-x ", " \-\-extract " \fIarchive directory\fP"
> +.BR \-x ", " \-\-extract " \fIarchive directory\fP [\fItar-options\fP...]"
> Extracts the filesystem tree from a package archive into the specified
> -directory.
> +directory. Additionnal arguments are passed straight to
> +.BR tar (1)
> + .
Likewise.
>
> Note that extracting a package to the root directory will
> .I not
> @@ -137,7 +139,7 @@ to install packages.
> (but not its parents) will be created if necessary, and its permissions
> modified to match the contents of the package.
> .TP
> -.BR \-X ", " \-\-vextract " \fIarchive directory\fP"
> +.BR \-X ", " \-\-vextract " \fIarchive directory\fP [\fItar-options\fP...]"
> Is like
> .BR \-\-extract " (" \-x ")"
> but prints a listing of the files extracted as it goes.
Likewise.
With whatever subset of the changes suggested seems suitable,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.
diff --git a/dpkg-deb/extract.c b/dpkg-deb/extract.c
index 914224b..2f6ac57 100644
--- a/dpkg-deb/extract.c
+++ b/dpkg-deb/extract.c
@@ -329,13 +329,18 @@ void extracthalf(const char *debar, const char *directory,
unsetenv("TAR_OPTIONS");
- for (i = 0; taropts && taropts[i]; i++);
- arg = targv = m_malloc((5 + i + 1) * sizeof(targv[0]));
+ for (i = 0; taropts && taropts[i]; i++)
+ ; /* just counting */
+ arg = targv = m_malloc((
+ 3 + /* tar tvf - */
+ i + /* taropts */
+ 1 /* terminating NULL */
+ ) * sizeof(targv[0]));
*arg++ = "tar";
*arg++ = buffer;
*arg++ = "-";
if (i) {
- mempcpy(arg, taropts, i * sizeof(taropts[0]));
+ memcpy(arg, taropts, i * sizeof(taropts[0]));
arg += i;
}
*arg++ = NULL;
diff --git a/dpkg-deb/main.c b/dpkg-deb/main.c
index 8238720..3ad20e6 100644
--- a/dpkg-deb/main.c
+++ b/dpkg-deb/main.c
@@ -78,9 +78,9 @@ usage(const struct cmdinfo *cip, const char *value)
" -W|--show <deb> Show information on package(s)\n"
" -f|--field <deb> [<cfield> ...] Show field(s) to stdout.\n"
" -e|--control <deb> [<directory>] Extract control info.\n"
-" -x|--extract <deb> <directory> [tar-options...]\n"
+" -x|--extract <deb> <directory> [[--] tar-options...]\n"
" Extract files.\n"
-" -X|--vextract <deb> <directory> [tar-options...]\n"
+" -X|--vextract <deb> <directory> [[--] tar-options...]\n"
" Extract & list files.\n"
" --fsys-tarfile <deb> Output filesystem tarfile.\n"
"\n"));
diff --git a/man/dpkg-deb.1 b/man/dpkg-deb.1
index c5db6e7..c267734 100644
--- a/man/dpkg-deb.1
+++ b/man/dpkg-deb.1
@@ -123,11 +123,13 @@ package archive. It is currently produced in the format generated by
.BR tar 's
verbose listing.
.TP
-.BR \-x ", " \-\-extract " \fIarchive directory\fP [\fItar-options\fP...]"
+.BR \-x ", " \-\-extract " \fIarchive directory\fP [[\-\-] \fItar-options\fP...]"
Extracts the filesystem tree from a package archive into the specified
-directory. Additionnal arguments are passed straight to
-.BR tar (1)
- .
+directory. Any arguments left over after dpkg\-deb processes its own are
+passed to
+.BR tar "(1)."
+Option flags (e.g., \-\-exclude) are only passed to tar if preceded by
+\-\- on the command line.
Note that extracting a package to the root directory will
.I not
@@ -139,7 +141,7 @@ to install packages.
(but not its parents) will be created if necessary, and its permissions
modified to match the contents of the package.
.TP
-.BR \-X ", " \-\-vextract " \fIarchive directory\fP [\fItar-options\fP...]"
+.BR \-X ", " \-\-vextract " \fIarchive directory\fP [[\-\-] \fItar-options\fP...]"
Is like
.BR \-\-extract " (" \-x ")"
but prints a listing of the files extracted as it goes.
--
Reply to: