Re: [patch] always send the full name on the --status-fd
On Mon, Oct 21, 2013 at 07:54:22AM +0200, Guillem Jover wrote:
> On Tue, 2013-10-15 at 19:39:32 +0200, Michael Vogt wrote:
> > On Tue, Oct 15, 2013 at 05:39:39PM +0200, Guillem Jover wrote:
> > > Help the progress reporting code as in making it simpler or something
> > > else?
> >
> > It would help making the current code simpler and more consistent. But
> > apt has enough information to work without it.
I'm actually not so sure anymore if it is not required for correctness
too, out-of-order messages like disappearing packages may need to be
fully qualified, but I haven't investigated further as I hope we can
get a new status-fd format :)
[..]
> > > I guess I still have the same issue with this as when devising the
> > > current rationale for this: backward-compatibility with other dpkg
> > > users not switching to multiarch.
> >
> > This could be controlled via e.g. a switch, but given that apt would
> > have to support the old behavior for some time I guess the net win is
> > very small (for apt).
>
> If we are going to add a new format, we might as well add any
> additional information that frontends might find useful and that makes
> sense sending. We can also cleanup the format and make it more sane
> and consistent. Perhaps also in deb822-style, inspired from the
> APT::Status-deb822-Fd comment in apt's changelog. :) But perhaps
> that complicates the parsing too much?
Great, I think that is a good idea, apt has a parser for deb822, so it
should not complicate stuff on our side. I attached a patch that
outlines what it could look like. Feedback very welcome, it probably
needs some more more work, propably something like
--assert-status-deb822 (unless I add a hard dependency) and I'm not
sure if the added test is a useful addition for the repo (it was/is
for me during development).
> I'm not sure what's your policy regarding backward compat, and when
> you consider a hard dependency on a newer dpkg version is enough to be
> able to remove old code, but it should eventually make things better.
> dpkg will not have that luxiry though. :)
[..]
Indeed.
> > > (As an aside, I see the code is not keying on "status:" so if there's a
> > > new record type introduced, apt might misbehave.)
> >
> > Correct, thanks for pointing this out. The code is currently getting
> > reworked and this was one of the issues fixed.
>
> I might not be looking at the right place or maybe the code has not
> been pushed, but I don't see a fix for this?
[..]
I was only in my "mvo" branch, not merged when you looked at it. But
its there now.
> In any case as mentioned above, I'm absolutely fine with adding an
> explicit option to select a new status format. Either --status-fd-format
> or similar. We'd need to come up first with that'd frontends need from
> it.
Great, hope the attached patch gives us a starting point, with deb822
it should easy enough to expand what we send.
Cheers,
Michael
>From e2e0b2bbe79ed92866f3198ecb32864706340a52 Mon Sep 17 00:00:00 2001
From: Michael Vogt <mvo@debian.org>
Date: Wed, 23 Oct 2013 10:24:26 +0200
Subject: [PATCH] initial version (rought)
---
lib/dpkg/dbmodify.c | 4 ++
lib/dpkg/dpkg.h | 3 ++
lib/dpkg/libdpkg.map | 2 +
lib/dpkg/log.c | 58 ++++++++++++++++++----
src/configure.c | 8 +++
src/errors.c | 3 ++
src/help.c | 4 ++
src/main.c | 14 ++++++
test/integration/debs/minimal_1.0_all.deb | Bin 0 -> 1222 bytes
test/integration/test-dpkg-status-fd-deb822 | 73 ++++++++++++++++++++++++++++
10 files changed, 159 insertions(+), 10 deletions(-)
create mode 100644 test/integration/debs/minimal_1.0_all.deb
create mode 100755 test/integration/test-dpkg-status-fd-deb822
diff --git a/lib/dpkg/dbmodify.c b/lib/dpkg/dbmodify.c
index 61cfcb3..e015423 100644
--- a/lib/dpkg/dbmodify.c
+++ b/lib/dpkg/dbmodify.c
@@ -412,6 +412,10 @@ void modstatdb_note(struct pkginfo *pkg) {
versiondescribe(&pkg->installed.version, vdew_nonambig));
statusfd_send("status: %s: %s", pkg_name(pkg, pnaw_nonambig),
statusinfos[pkg->status].name);
+ statusfd_send_deb822("Status: status\n"
+ "Package: %s\n"
+ "Action: %s\n", pkg_name(pkg, pnaw_always),
+ statusinfos[pkg->status].name);
if (cstatus >= msdbrw_write)
modstatdb_note_core(pkg);
diff --git a/lib/dpkg/dpkg.h b/lib/dpkg/dpkg.h
index 55b373b..47a3fda 100644
--- a/lib/dpkg/dpkg.h
+++ b/lib/dpkg/dpkg.h
@@ -127,7 +127,10 @@ extern const char *log_file;
void log_message(const char *fmt, ...) DPKG_ATTR_PRINTF(1);
void statusfd_add(int fd);
+void statusfd_add_deb822(int fd);
void statusfd_send(const char *fmt, ...) DPKG_ATTR_PRINTF(1);
+void statusfd_send_deb822(const char *fmt, ...) DPKG_ATTR_PRINTF(1);
+
/*** cleanup.c ***/
diff --git a/lib/dpkg/libdpkg.map b/lib/dpkg/libdpkg.map
index 318e997..5bbc23a 100644
--- a/lib/dpkg/libdpkg.map
+++ b/lib/dpkg/libdpkg.map
@@ -173,7 +173,9 @@ LIBDPKG_PRIVATE {
# Action logging
statusfd_add;
+ statusfd_add_deb822;
statusfd_send;
+ statusfd_send_deb822;
# Progress report support
progress_init;
diff --git a/lib/dpkg/log.c b/lib/dpkg/log.c
index ce9d1ea..796ea43 100644
--- a/lib/dpkg/log.c
+++ b/lib/dpkg/log.c
@@ -76,9 +76,10 @@ struct pipef {
};
static struct pipef *status_pipes = NULL;
+static struct pipef *status_pipes_deb822 = NULL;
-void
-statusfd_add(int fd)
+static void
+statusfd_add_internal(int fd, struct pipef **to_pipes)
{
struct pipef *pipe_new;
@@ -86,15 +87,37 @@ statusfd_add(int fd)
pipe_new = nfmalloc(sizeof(struct pipef));
pipe_new->fd = fd;
- pipe_new->next = status_pipes;
- status_pipes = pipe_new;
+ pipe_new->next = *to_pipes;
+ *to_pipes = pipe_new;
+}
+
+void
+statusfd_add(int fd)
+{
+ statusfd_add_internal(fd, &status_pipes);
+}
+
+void
+statusfd_add_deb822(int fd)
+{
+ statusfd_add_internal(fd, &status_pipes_deb822);
+}
+
+static void
+statusfd_send_to_pipes(struct pipef *to_pipes, struct varbuf vb)
+{
+ struct pipef *pipef;
+ for (pipef = to_pipes; pipef; pipef = pipef->next) {
+ if (fd_write(pipef->fd, vb.buf, vb.used) < 0)
+ ohshite(_("unable to write to status fd %d"),
+ pipef->fd);
+ }
}
void
statusfd_send(const char *fmt, ...)
{
static struct varbuf vb;
- struct pipef *pipef;
va_list args;
if (!status_pipes)
@@ -109,9 +132,24 @@ statusfd_send(const char *fmt, ...)
varbuf_add_char(&vb, '\n');
va_end(args);
- for (pipef = status_pipes; pipef; pipef = pipef->next) {
- if (fd_write(pipef->fd, vb.buf, vb.used) < 0)
- ohshite(_("unable to write to status fd %d"),
- pipef->fd);
- }
+ statusfd_send_to_pipes(status_pipes, vb);
+}
+
+void
+statusfd_send_deb822(const char *fmt, ...)
+{
+ static struct varbuf vb;
+ va_list args;
+
+ if (!status_pipes_deb822)
+ return;
+
+ va_start(args, fmt);
+ varbuf_reset(&vb);
+ varbuf_vprintf(&vb, fmt, args);
+ // append final \n
+ varbuf_add_char(&vb, '\n');
+ va_end(args);
+
+ statusfd_send_to_pipes(status_pipes_deb822, vb);
}
diff --git a/src/configure.c b/src/configure.c
index fd54582..a676606 100644
--- a/src/configure.c
+++ b/src/configure.c
@@ -699,6 +699,14 @@ promptconfaction(struct pkginfo *pkg, const char *cfgfile,
statusfd_send("status: %s : %s : '%s' '%s' %i %i ",
cfgfile, "conffile-prompt",
realold, realnew, useredited, distedited);
+ statusfd_send_deb822("Status: %s\n"
+ "ConfigFile: configfile-prompt\n"
+ "ConfigOld: %s\n"
+ "ConfigNew: %s\n"
+ "UserEdited: %i\n"
+ "DistEdited: %i\n",
+ cfgfile, realold, realnew, useredited,
+ distedited);
do {
/* Flush the terminal's input in case the user involuntarily
diff --git a/src/errors.c b/src/errors.c
index 9dc6724..1d0d142 100644
--- a/src/errors.c
+++ b/src/errors.c
@@ -60,6 +60,9 @@ void print_error_perpackage(const char *emsg, const char *arg) {
notice(_("error processing %s (--%s):\n %s"), arg, cipaction->olong, emsg);
statusfd_send("status: %s : %s : %s", arg, "error", emsg);
+ statusfd_send_deb822("Status: error\n"
+ "Package: %s\n"
+ "Message: %s\n", arg, emsg);
nr= malloc(sizeof(struct error_report));
if (!nr) {
diff --git a/src/help.c b/src/help.c
index 2af873c..2e41e14 100644
--- a/src/help.c
+++ b/src/help.c
@@ -403,4 +403,8 @@ log_action(const char *action, struct pkginfo *pkg, struct pkgbin *pkgbin)
versiondescribe(&pkg->available.version, vdew_nonambig));
statusfd_send("processing: %s: %s", action,
pkgbin_name(pkg, pkgbin, pnaw_nonambig));
+ statusfd_send_deb822("Status: processing\n"
+ "Action: %s\n"
+ "Package: %s\n", action,
+ pkgbin_name(pkg, pkgbin, pnaw_always));
}
diff --git a/src/main.c b/src/main.c
index 9a1c443..59f4025 100644
--- a/src/main.c
+++ b/src/main.c
@@ -148,6 +148,7 @@ usage(const struct cmdinfo *ci, const char *value)
" Just say what we would do - don't do it.\n"
" -D|--debug=<octal> Enable debugging (see -Dhelp or --debug=help).\n"
" --status-fd <n> Send status change updates to file descriptor <n>.\n"
+" --status-fd-deb822 <n> Send status change updates in deb822 format to file descriptor <n>.\n"
" --status-logger=<command> Send status change updates to <command>'s stdin.\n"
" --log=<filename> Log status changes and actions to <filename>.\n"
" --ignore-depends=<package>,...\n"
@@ -403,6 +404,18 @@ static void setpipe(const struct cmdinfo *cip, const char *value) {
statusfd_add(v);
}
+static void setpipe_deb822(const struct cmdinfo *cip, const char *value) {
+ long v;
+ char *ep;
+
+ errno = 0;
+ v = strtol(value, &ep, 0);
+ if (value == ep || *ep || v < 0 || v > INT_MAX || errno != 0)
+ badusage(_("invalid integer for --%s: `%.250s'"),cip->olong,value);
+
+ statusfd_add_deb822(v);
+}
+
static bool
is_invoke_action(enum action action)
{
@@ -686,6 +699,7 @@ static const struct cmdinfo cmdinfos[]= {
{ "verify-format", 0, 1, NULL, NULL, set_verify_format },
{ "status-logger", 0, 1, NULL, NULL, set_invoke_hook, 0, &status_loggers_tail },
{ "status-fd", 0, 1, NULL, NULL, setpipe, 0 },
+ { "status-fd-deb822", 0, 1, NULL, NULL, setpipe_deb822, 0 },
{ "log", 0, 1, NULL, &log_file, NULL, 0 },
{ "pending", 'a', 0, &f_pending, NULL, NULL, 1 },
{ "recursive", 'R', 0, &f_recursive, NULL, NULL, 1 },
diff --git a/test/integration/debs/minimal_1.0_all.deb b/test/integration/debs/minimal_1.0_all.deb
new file mode 100644
index 0000000000000000000000000000000000000000..9098969cc1274dc56e3ee0143249d3a5ddd1bb33
GIT binary patch
literal 1222
zcmY$iNi0gvu;WTeP0CEn(@o0EODw8XP*5;5wlFd^G&MIgQ&2Df@?oT*fq|KciGl(U
zK|unSk)8opa(-S(QGSkINn(*+dKF>)re>CK{qo%$3?RV7{PxP(yekF*3=eFdYqnY6
z^_6+Pp2<bED@0{Um+qap6_X@{tT)MSeR$;ZhXfl=zuQwaf*xM^wD{+e&@G+D#`iwH
zDcfFIP#BPMv&y$i=ikSw4W1I-la<m6&m8`%^t^`o<}<-F=eW5y96a|Te(mqOpX;xb
zU6**fR#I-q|7_>?uj|7;?EdR7B-Y1$<jQ~ca=lspiw-ACObuHsv4}rouF)@vNx3p}
zZ!8O!leWLT!G*{9OPI#n!|ZPj_ZV_imzuT;e0a0?;PeXBW0?omZoJ$5!lymHOd>$;
z?bYWW%vLX|?I@AG{ljia=ee~;VLw&x%(%v`_*c86H}vw^P1f-(Y=+gB6Hn;ccun|h
zq1V1^etS{*=UJQN44O8Ji5WYu-oj#TZp$$vuZ-(efVjk7y-g3qo)^|gH5x1bihIeY
z7g+tGck3!Su^n67%KaZKI?md1ba$zXLA%Q4-Fy;a*SRZ7_6hREYE+%gkG}9M?Xp3n
zv1I%GeKo!R`hLo%K6`fY@4=t`-Cv&n^#9cK?7!g@PI1rMX1>~nVwq=BZ-=Cw3_6y2
zCW!B0%I&$Y4{yBhdHGG=@!apfzC3yKclYJm7SpaCZoS}n;jWR_W2SLV)?IzuOw+qC
zzG0d8uEPgQ0%vblbBo-xMq`qI-sC-Jil!}(?AFnLx>xYbmWZog?GA0>d8y*feK7FK
zjHIJodpsjmE2f5QpS@zL-Ja?#|EA`0D7FYVp%4@PA1zP+&8^6w!N9<kl30=mPjbNY
zL@?EwLDI5<LIM|3s+-~Hf7?Lhc=5aC6$cu7FCHs<BxPup>$ITv^b*EfH+|(j+oZ$G
zn7-V8?5E%;=rb#9d(QjvjgNjziU0ThrreT~ml7FOpVmC_+SLBomQ!5maTn*YPX_DT
zD(5x$C|&p&X5GNzeS6QAcjfQnHJi75){8a%Kj&$p<<I_S_rq@`?y~#0b4u5r<KOJJ
zx$ctt=c=~i-})o}m*l<bG<%u#H{FZt(|)i2$KB_jKeu0g?mylCpLy?Y{`dJ|Q~k-x
zX$;E$z0=v%u5Ev?`_ZKZ-yghnt(L80n_*@Bu1o9#SFFC`zrcB?>jPgdb4fnU@sppw
z`=>mY>(Bm!bMq?>v%k?T@%<sC-28rFv8vh8H_aQb@0B~iSCji{8y~l<)D^kIDf{`=
z#U5_iH}lmaA-=k(KP~1}My3`U*8So5z<hkSg3%ALR~;O&vKF_~tAF1VxOdh!I5P6_
zpW83bTur@t?8X0^lmBSD{k7+6Ke+i|x0u^z_sn_6r+)gm|4Ds_x%VFCKh3}86<_?*
z-}Ha?sr%QD=^Xrj`nmnqiht*eJipqrMN990@_SXgYii=}I{O!2&m4XxZgMOyCofoC
Ufujk6D&F2<+Pe4z11KQ?0RD?GVE_OC
literal 0
HcmV?d00001
diff --git a/test/integration/test-dpkg-status-fd-deb822 b/test/integration/test-dpkg-status-fd-deb822
new file mode 100755
index 0000000..2b99355
--- /dev/null
+++ b/test/integration/test-dpkg-status-fd-deb822
@@ -0,0 +1,73 @@
+#!/bin/sh
+
+set -e
+
+BASEDIR=$(dirname $(readlink -f $0))
+TMP_ROOT=$(mktemp -d)
+trap "rm -rf $TMP_ROOT" HUP INT QUIT ILL ABRT FPE SEGV PIPE TERM
+
+if [ -x "${BASEDIR}/../../src/dpkg" ]; then
+ DPKG="${BASEDIR}/../../src/dpkg"
+elif [ -x "${BASEDIR}/../../build/src/dpkg" ]; then
+ DPKG="${BASEDIR}/../../build/src/dpkg"
+else
+ echo "Can't find dpkg build"
+ exit 1
+fi
+
+# help dpkg to find its stuff
+mkdir -p $TMP_ROOT/var/lib/dpkg/updates \
+ $TMP_ROOT/var/lib/dpkg/triggers \
+ $TMP_ROOT/var/log \
+ $TMP_ROOT/var/lib/dpkg/info
+touch $TMP_ROOT/var/lib/dpkg/status
+touch $TMP_ROOT/var/lib/dpkg/available
+touch $TMP_ROOT/var/log/dpkg.log
+
+DPKG="fakeroot $DPKG --root=$TMP_ROOT --force-not-root --force-bad-path --log=${TMP_ROOT}/var/log/dpkg.log"
+
+DPKG_PROGRESS_LOG="$TMP_ROOT/dpkg-progress.log"
+DPKG_PROGRESS_LOG_E="$TMP_ROOT/dpkg-progress-expected.log"
+
+exec 3> $DPKG_PROGRESS_LOG
+$DPKG --status-fd-deb822=3 -i $BASEDIR/debs/minimal_1.0_all.deb >/dev/null
+cat > $DPKG_PROGRESS_LOG_E <<EOF
+Status: processing
+Action: install
+Package: minimal:all
+
+Status: status
+Package: minimal:all
+Action: half-installed
+
+Status: status
+Package: minimal:all
+Action: unpacked
+
+Status: status
+Package: minimal:all
+Action: unpacked
+
+Status: processing
+Action: configure
+Package: minimal:all
+
+Status: status
+Package: minimal:all
+Action: unpacked
+
+Status: status
+Package: minimal:all
+Action: half-configured
+
+Status: status
+Package: minimal:all
+Action: installed
+
+EOF
+if ! diff -u $DPKG_PROGRESS_LOG $DPKG_PROGRESS_LOG_E; then
+ echo "Got unexpected progress"
+ exit 1
+fi
+
+echo "PASS"
--
1.8.3.2
Reply to: