On Fri, 16 Mar 2012, Bernhard R. Link wrote:
> > IMO this is not the proper way to extract it automatically. If you want
> > this feature, you should add a unique (and fixed) start/end marker.
> >
> > Maybe something like this ?
> >
> > --- BEGIN DPKG-BUILDFLAGS STATUS ---
> > […]
> > --- END DPKG-BUILDFLAGS STATUS ---
> 
> Things requiring context like that are no fun to parse at all. You
> cannot grep them, a line parser needs additional context.

┏rivendell:~/x/c-a
┗(582)$ cat  ~/tmp/test.txt
before
START
1
2
3
END
after
┏rivendell:~/x/c-a
┗(583)$ sed -ne '/^START/,/^END/ p' ~/tmp/test.txt
START
1
2
3
END

Really it's not difficult.

> > If you really want to keep the "dpkg-builflags: " prefix, then you should
> > use one of the functions exported by Dpkg::ErrorHandling. But I don't
> > think it's required.
> 
> That seems to always want to output some extra (at least a colon) and
> most even seem to have localized messages, so do not look very
> suiteable.

Well, dpkg-buildflags can output additional errors/warnings during the
build so your grep would catch those too. You're thus not able to extract
only the output of the status command.

So if you don't want to go for the above, you should at least use
“print report("status", …)” so that the output ends up always starting
with "dpkg-buildflags: status: " and this can be reasonably extracted
too.

> I've split the first one into preparations to the infrastructure and
> the --status introduction. If you prefer them merged let me know...

It's ok.

> +=item $bf->environment_used($envvar)
> +
> +Records that the given environment variable had influenced
> +or could have influenced (if it had existed or had a different
> +value) the calculated flags.
> +
> +=cut

Please pick a name that start with an action. "mark_used_environment"
or "mark_used_envvar" maybe.

> +sub environment_used {
> +    my ($self, $envvar) = @_;
> +    $self->{'used_envs'}->{$envvar} = 1;

The second "->" is not needed. Drop it.

> +=item my @list = $bf->get_used_environment()

Be consistent with the former method, don't use once "environment_used" and
once "use_environment". Thus I suggest "get_used_environment" or
"get_used_envvar".

Also please return a sorted list to make the order non-random.

> +    my @envvars = $build_flags->get_used_environment();
> +    for my $envvar (@envvars) {
> +     if (exists $ENV{$envvar}) {
> +         printf "dpkg-buildflags: environment variable %s=%s\n",
> +                             $envvar, $ENV{$envvar};
> +     }
> +    }

BTW, I don't really like the usage of plain english in the output.
Plain english ought to be translated... maybe we can use somewhat
meaningful prefixes ?

E: for environment (or ENV:?)
V: for vendor (or VENDOR: ?)
F: for feature (or FEATURE:?)
R: for result or (or RES:? or FLAG:?)

It also makes it easier to reliably extract a sub-part of the output...

> From 6a600d3f9d2b438ca59d0e81f69b91249bd197b1 Mon Sep 17 00:00:00 2001
> From: "Bernhard R. Link" <brl...@debian.org>
> Date: Fri, 16 Mar 2012 11:05:17 +0100
> Subject: [PATCH 4/4] dpkg-buildflags: make --status output which flags are
>  modified by maintainer
> 
> As flags modified by DEB_*_MAINT_* are not reflected by its origin, add
> a new flag to describe flags modified that way.

Do you really see some value to this? You already have the *_MAINT_* variables
in the output.

I don't believe that this is required.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Pre-order a copy of the Debian Administrator's Handbook and help
liberate it: http://debian-handbook.info/liberation/



--
To UNSUBSCRIBE, email to debian-dpkg-bugs-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to