Bug#520615: Option to act on the packages not logged yet
Hello,
On 2009 m. March 23 d., Monday 01:42:26 you wrote:
> > --- a/dh
> > +++ b/dh
> > @@ -486,17 +486,7 @@ sub run {
> > =20
> > sub loadlog {
> > my $package=3Dshift;
> > - my $ext=3Dpkgext($package);
> > -=09
> > - my @log;
> > - open(LOG, "<", "debian/${ext}debhelper.log") || return;
> > - while () {
> >-chomp;
> >-push @log, $_;
> >-$logged{$package}{$_}=3D1;
> >-}
> >-close LOG;
> >-return @log;
> >+return Debian::Debhelper::Dh_Lib::load_log($package, \%logged);
>%logged is not defined here, shouldn't the second parameter just be
>%undef?
%logged is defined globally (it is even visible in the latest edition of the
patch)
> > +sub is_in_log {
> > + my $package=shift;
> > + my %db;
> > + my $cmd=basename($0);
> > +
> > + load_log($package, \%db);
> > + return exists $db{$package}{$cmd};
> > +}
>
> Looking at this closer, it seems unnecessarily overcomplicated to pass
> $db to load_log and build the complex hash structure. With the function
> just returning a list, all that's needed is:
>
> grep { $_ eq basename($0) } load_log($package);
>
> Probably no need for the is_in_log function when it's that simple.
Done. Interdiff below, full fixed version of the patch attached.
diff --git a/Debian/Debhelper/Dh_Getopt.pm b/Debian/Debhelper/Dh_Getopt.pm
index 3af3c5b..5585a54 100644
--- a/Debian/Debhelper/Dh_Getopt.pm
+++ b/Debian/Debhelper/Dh_Getopt.pm
@@ -224,7 +224,8 @@ sub parseopts {
my $package;
my %packages_seen;
foreach $package (@{$dh{DOPACKAGES}}) {
- if (defined($dh{EXCLUDE_LOGGED}) && is_in_log($package)) {
+ if (defined($dh{EXCLUDE_LOGGED}) &&
+ grep { $_ eq basename($0) } load_log($package)) {
$exclude_package{$package}=1;
}
if (! $exclude_package{$package}) {
diff --git a/Debian/Debhelper/Dh_Lib.pm b/Debian/Debhelper/Dh_Lib.pm
index e759465..23e81a6 100644
--- a/Debian/Debhelper/Dh_Lib.pm
+++ b/Debian/Debhelper/Dh_Lib.pm
@@ -15,7 +15,7 @@ use vars qw(@ISA @EXPORT %dh);
&filedoublearray &getpackages &basename &dirname &xargs %dh
&compat &addsubstvar &delsubstvar &excludefile &package_arch
&is_udeb &udeb_filename &debhelper_script_subst &escape_shell
- &inhibit_log &is_in_log);
+ &inhibit_log &load_log);
my $max_compat=7;
@@ -136,15 +136,6 @@ sub inhibit_log {
$write_log=0;
}
-sub is_in_log {
- my $package=shift;
- my %db;
- my $cmd=basename($0);
-
- load_log($package, \%db);
- return exists $db{$package}{$cmd};
-}
-
# Pass it an array containing the arguments of a shell command like would
# be run by exec(). It turns that into a line like you might enter at the
# shell, escaping metacharacters and quoting arguments that contain spaces.
diff --git a/dh b/dh
index 542eb12..ab7ddb0 100755
--- a/dh
+++ b/dh
@@ -376,7 +376,7 @@ while (@ARGV_orig) {
my %logged;
my %startpoint;
foreach my $package (@packages) {
- my @log=loadlog($package);
+ my @log=load_log($package, \%logged);
if ($dh{AFTER}) {
# Run commands in the sequence that come after the
# specified command.
@@ -484,11 +484,6 @@ sub run {
}
}
-sub loadlog {
- my $package=shift;
- return Debi
Bug#520615: Option to act on the packages not logged yet
Modestas Vainius wrote:
> +sub load_log {
> + my ($package, $db)=...@_;
> + my $ext=pkgext($package);
> +
> + my @log;
> + open(LOG, "<", "debian/${ext}debhelper.log") || return;
> + while () {
> + chomp;
> + push @log, $_;
> + $db->{$package}{$_}=1 if (defined $db);
> + }
> + close LOG;
> + return @log;
> +}
>
> sub write_log {
> my $cmd=shift;
> @@ -121,6 +136,15 @@ sub inhibit_log {
> $write_log=0;
> }
>
> +sub is_in_log {
> + my $package=shift;
> + my %db;
> + my $cmd=basename($0);
> +
> + load_log($package, \%db);
> + return exists $db{$package}{$cmd};
> +}
Looking at this closer, it seems unnecessarily overcomplicated to pass
$db to load_log and build the complex hash structure. With the function
just returning a list, all that's needed is:
grep { $_ eq basename($0) } load_log($package);
Probably no need for the is_in_log function when it's that simple.
--
see shy jo
signature.asc
Description: Digital signature
Bug#520615: Option to act on the packages not logged yet
Modestas Vainius wrote:
> --- a/dh
> +++ b/dh
> @@ -486,17 +486,7 @@ sub run {
>
> sub loadlog {
> my $package=shift;
> - my $ext=pkgext($package);
> -
> - my @log;
> - open(LOG, "<", "debian/${ext}debhelper.log") || return;
> - while () {
> - chomp;
> - push @log, $_;
> - $logged{$package}{$_}=1;
> - }
> - close LOG;
> - return @log;
> + return Debian::Debhelper::Dh_Lib::load_log($package, \%logged);
%logged is not defined here, shouldn't the second parameter just be
%undef?
--
see shy jo
signature.asc
Description: Digital signature
Bug#520615: Option to act on the packages not logged yet
Hello,
On 2009 m. March 21 d., Saturday 18:46:31 Modestas Vainius wrote:
> fixed patch is attached.
Removed bogus "|| return;" from load_log. Full version of fixed (2) patch
attached. Sorry.
--- a/Debian/Debhelper/Dh_Lib.pm
+++ b/Debian/Debhelper/Dh_Lib.pm
@@ -106,7 +106,7 @@ sub END {
sub load_log {
my ($package, $db)=...@_;
- my $ext=pkgext($package) || return;
+ my $ext=pkgext($package);
my @log;
open(LOG, "<", "debian/${ext}debhelper.log") || return;
--
Modestas Vainius
From 658900b65052c055e9b48bbd26c9e9f07481b167 Mon Sep 17 00:00:00 2001
From: Modestas Vainius
Date: Sat, 21 Mar 2009 18:51:16 +0200
Subject: [PATCH] Add a global --remaining-packages option.
Add a global --remaining-packages option which allows to skip the command on
the packages which it has already been run on (i.e. if the command helper is
already present in the package debhelper log).
Signed-off-by: Modestas Vainius
---
Debian/Debhelper/Dh_Getopt.pm |5 +
Debian/Debhelper/Dh_Lib.pm| 28 ++--
debhelper.pod |8
dh| 12 +---
4 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/Debian/Debhelper/Dh_Getopt.pm b/Debian/Debhelper/Dh_Getopt.pm
index ef94e94..3af3c5b 100644
--- a/Debian/Debhelper/Dh_Getopt.pm
+++ b/Debian/Debhelper/Dh_Getopt.pm
@@ -91,6 +91,8 @@ sub getoptions {
"N=s" => \&ExcludePackage,
"no-package=s" => \&ExcludePackage,
+ "remaining-packages" => \$dh{EXCLUDE_LOGGED},
+
"dbg-package=s" => \&AddDebugPackage,
"s" => \&AddPackage,
@@ -222,6 +224,9 @@ sub parseopts {
my $package;
my %packages_seen;
foreach $package (@{$dh{DOPACKAGES}}) {
+ if (defined($dh{EXCLUDE_LOGGED}) && is_in_log($package)) {
+ $exclude_package{$package}=1;
+ }
if (! $exclude_package{$package}) {
if (! exists $packages_seen{$package}) {
$packages_seen{$package}=1;
diff --git a/Debian/Debhelper/Dh_Lib.pm b/Debian/Debhelper/Dh_Lib.pm
index d481128..e759465 100644
--- a/Debian/Debhelper/Dh_Lib.pm
+++ b/Debian/Debhelper/Dh_Lib.pm
@@ -15,7 +15,7 @@ use vars qw(@ISA @EXPORT %dh);
&filedoublearray &getpackages &basename &dirname &xargs %dh
&compat &addsubstvar &delsubstvar &excludefile &package_arch
&is_udeb &udeb_filename &debhelper_script_subst &escape_shell
- &inhibit_log);
+ &inhibit_log &is_in_log);
my $max_compat=7;
@@ -102,7 +102,22 @@ sub END {
if ($? == 0 && $write_log) {
write_log(basename($0), @{$dh{DOPACKAGES}});
}
-}
+}
+
+sub load_log {
+ my ($package, $db)=...@_;
+ my $ext=pkgext($package);
+
+ my @log;
+ open(LOG, "<", "debian/${ext}debhelper.log") || return;
+ while () {
+ chomp;
+ push @log, $_;
+ $db->{$package}{$_}=1 if (defined $db);
+ }
+ close LOG;
+ return @log;
+}
sub write_log {
my $cmd=shift;
@@ -121,6 +136,15 @@ sub inhibit_log {
$write_log=0;
}
+sub is_in_log {
+ my $package=shift;
+ my %db;
+ my $cmd=basename($0);
+
+ load_log($package, \%db);
+ return exists $db{$package}{$cmd};
+}
+
# Pass it an array containing the arguments of a shell command like would
# be run by exec(). It turns that into a line like you might enter at the
# shell, escaping metacharacters and quoting arguments that contain spaces.
diff --git a/debhelper.pod b/debhelper.pod
index 77ace5c..dad286e 100644
--- a/debhelper.pod
+++ b/debhelper.pod
@@ -118,6 +118,14 @@ are not architecture independent.
Do not act on the specified package even if an -a, -i, or -p option lists
the package as one that should be acted on.
+=item B<--remaining-packages>
+
+Do not act on the packages which have already been acted on by this debhelper
+command earlier (i.e. if the command is present in the package debhelper log).
+For example, if you need to call the command with special options only for a
+couple of binary packages, pass this option to the last call of the command to
+process the rest of packages with default settings.
+
=item B<--ignore=>I
Ignore the specified file. This can be used if debian/ contains a debhelper
diff --git a/dh b/dh
index 8639ed0..542eb12 100755
--- a/dh
+++ b/dh
@@ -486,17 +486,7 @@ sub run {
sub loadlog {
my $package=shift;
- my $ext=pkgext($package);
-
- my @log;
- open(LOG, "<", "debian/${ext}debhelper.log") || return;
- while () {
- chomp;
- push @log, $_;
- $logged{$package}{$_}=1;
- }
- close LOG;
- return @log;
+ return Debian::Debhelper::Dh_Lib::load_log($package, \%logged);
}
sub writelog {
--
1.6.2.1
signature.asc
Description: This is a digitally signed message part.
Bug#520615: Option to act on the packages not logged yet
Hello,
fixed patch is attached.
--
Modestas Vainius
From 830d6634ed3525cf2e3e816880971f8d5ff29dc6 Mon Sep 17 00:00:00 2001
From: Modestas Vainius
Date: Sat, 21 Mar 2009 18:43:28 +0200
Subject: [PATCH] Add a global --remaining-packages option.
Add a global --remaining-packages option which allows to skip the command on
the packages which it has already been run on (i.e. if the command helper is
already present in the package debhelper log).
Signed-off-by: Modestas Vainius
---
Debian/Debhelper/Dh_Getopt.pm |5 +
Debian/Debhelper/Dh_Lib.pm| 28 ++--
debhelper.pod |8
dh| 12 +---
4 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/Debian/Debhelper/Dh_Getopt.pm b/Debian/Debhelper/Dh_Getopt.pm
index ef94e94..3af3c5b 100644
--- a/Debian/Debhelper/Dh_Getopt.pm
+++ b/Debian/Debhelper/Dh_Getopt.pm
@@ -91,6 +91,8 @@ sub getoptions {
"N=s" => \&ExcludePackage,
"no-package=s" => \&ExcludePackage,
+ "remaining-packages" => \$dh{EXCLUDE_LOGGED},
+
"dbg-package=s" => \&AddDebugPackage,
"s" => \&AddPackage,
@@ -222,6 +224,9 @@ sub parseopts {
my $package;
my %packages_seen;
foreach $package (@{$dh{DOPACKAGES}}) {
+ if (defined($dh{EXCLUDE_LOGGED}) && is_in_log($package)) {
+ $exclude_package{$package}=1;
+ }
if (! $exclude_package{$package}) {
if (! exists $packages_seen{$package}) {
$packages_seen{$package}=1;
diff --git a/Debian/Debhelper/Dh_Lib.pm b/Debian/Debhelper/Dh_Lib.pm
index d481128..fd96b77 100644
--- a/Debian/Debhelper/Dh_Lib.pm
+++ b/Debian/Debhelper/Dh_Lib.pm
@@ -15,7 +15,7 @@ use vars qw(@ISA @EXPORT %dh);
&filedoublearray &getpackages &basename &dirname &xargs %dh
&compat &addsubstvar &delsubstvar &excludefile &package_arch
&is_udeb &udeb_filename &debhelper_script_subst &escape_shell
- &inhibit_log);
+ &inhibit_log &is_in_log);
my $max_compat=7;
@@ -102,7 +102,22 @@ sub END {
if ($? == 0 && $write_log) {
write_log(basename($0), @{$dh{DOPACKAGES}});
}
-}
+}
+
+sub load_log {
+ my ($package, $db)=...@_;
+ my $ext=pkgext($package) || return;
+
+ my @log;
+ open(LOG, "<", "debian/${ext}debhelper.log") || return;
+ while () {
+ chomp;
+ push @log, $_;
+ $db->{$package}{$_}=1 if (defined $db);
+ }
+ close LOG;
+ return @log;
+}
sub write_log {
my $cmd=shift;
@@ -121,6 +136,15 @@ sub inhibit_log {
$write_log=0;
}
+sub is_in_log {
+ my $package=shift;
+ my %db;
+ my $cmd=basename($0);
+
+ load_log($package, \%db);
+ return exists $db{$package}{$cmd};
+}
+
# Pass it an array containing the arguments of a shell command like would
# be run by exec(). It turns that into a line like you might enter at the
# shell, escaping metacharacters and quoting arguments that contain spaces.
diff --git a/debhelper.pod b/debhelper.pod
index 77ace5c..dad286e 100644
--- a/debhelper.pod
+++ b/debhelper.pod
@@ -118,6 +118,14 @@ are not architecture independent.
Do not act on the specified package even if an -a, -i, or -p option lists
the package as one that should be acted on.
+=item B<--remaining-packages>
+
+Do not act on the packages which have already been acted on by this debhelper
+command earlier (i.e. if the command is present in the package debhelper log).
+For example, if you need to call the command with special options only for a
+couple of binary packages, pass this option to the last call of the command to
+process the rest of packages with default settings.
+
=item B<--ignore=>I
Ignore the specified file. This can be used if debian/ contains a debhelper
diff --git a/dh b/dh
index 8639ed0..542eb12 100755
--- a/dh
+++ b/dh
@@ -486,17 +486,7 @@ sub run {
sub loadlog {
my $package=shift;
- my $ext=pkgext($package);
-
- my @log;
- open(LOG, "<", "debian/${ext}debhelper.log") || return;
- while () {
- chomp;
- push @log, $_;
- $logged{$package}{$_}=1;
- }
- close LOG;
- return @log;
+ return Debian::Debhelper::Dh_Lib::load_log($package, \%logged);
}
sub writelog {
--
1.6.2.1
signature.asc
Description: This is a digitally signed message part.
Bug#520615: Option to act on the packages not logged yet
Modestas Vainius wrote:
> + "exclude-logged" => \$dh{EXCLUDE_LOGGED},
> + "remaining-packages" => \$dh{EXCLUDE_LOGGED},
I don't see a reason to have two names for the same option.
--exclude-logged somewhat exposes internals so I prefer the other one.
> +sub is_helper_in_log {
> + my $cmd=shift;
> + my @packag...@_;
> +
> + foreach my $package (@packages) {
> + my $ext=pkgext($package);
> + if ($ext && open(LOG, "debian/${ext}debhelper.log")) {
> + if (grep /^\Q$cmd\E$/, ) {
> + close LOG;
> + return $package;
> + }
> + close LOG;
> + }
> + }
> + return undef;
> +}
This is fairly much a duplicate of loadlog from dh, and I think that
both dh and is_in_log could use the same function with some reworking.
> +=item B<--exclude-logged>, B<--remaining-packages>
> +
> +Do not act on the packages which have already been acted on by this helper
> +earlier (i.e. the helper is present in the package debhelper log). Useful
> +for invoking the helper on the packages remaining unprocessed after calling
> +the helper with special options on a few specific packages.
The docs call them denhelper commands, not helpers.
--
see shy jo
signature.asc
Description: Digital signature
Bug#520615: Option to act on the packages not logged yet
Package: debhelper
Version: 7.2.6
Severity: wishlist
Tags: patch
Hello,
lets suppose there are 5 binary packages: foo, bar, abc, def, ghi.
Consider this debian/rules snippet:
override_dh_foo:
dh_foo -pfoo --option1
dh_foo -pbar --option2
# The following is not resistant to the addition of new binary packages.
# The call will get ugly if package list is long
dh_foo -pabc -pdef -pghi
# The same thing could be achieved with the following, but it will get
# ugly if there are many packages like foo and bar.
dh_foo -Nfoo -Nbar
Both of these solutions seem to be pretty incovenient and suffer from
information duplication. The patch attached adds the --exclude-logged
(--remaining-packages) option which allows to write the same thing as:
override_dh_foo:
dh_foo -pfoo --option1
dh_foo -pbar --option2
dh_foo --remaining-packages
In my opinion, this is much cleaner. This option is similar what dh itself
does, but:
1) --remaining-packages can be useful without dh;
2) you cannot use dh --until dh_foo in place of --remaining-packages because:
a) there is no way to know the sequence name in the override (which in itself
could be a wish for dh exporting DH_SEQUENCE_NAME env. variable. Who knows,
might be useful for somebody).
b) dh --until dh_foo would call the override again -> endless loop.
The patch is against current debhelper master aka
ce53a5276c9f3a388dce10f442c3c1659e3bccdb.
-- System Information:
Debian Release: squeeze/sid
APT prefers unstable
APT policy: (500, 'unstable'), (500, 'testing'), (101, 'experimental')
Architecture: amd64 (x86_64)
Kernel: Linux 2.6.28-1-amd64 (SMP w/1 CPU core)
Locale: LANG=lt_LT.UTF-8, LC_CTYPE=lt_LT.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash
Versions of packages debhelper depends on:
ii binutils 2.19.1-1 The GNU assembler, linker and bina
ii dpkg-dev 1.15.0 Debian package development tools
ii file 4.26-2 Determines file type using "magic"
ii html2text 1.3.2a-13 advanced HTML to text converter
ii man-db2.5.5-1on-line manual pager
ii perl 5.10.0-19 Larry Wall's Practical Extraction
ii po-debconf1.0.15 manage translated Debconf template
debhelper recommends no packages.
Versions of packages debhelper suggests:
pn dh-make(no description available)
-- no debconf information
>From 7bfd78f21579223231449449df29e5dd3b95b005 Mon Sep 17 00:00:00 2001
From: Modestas Vainius
Date: Sat, 21 Mar 2009 13:20:20 +0200
Subject: [PATCH] Add a global --exclude-logged (--remaining-packages) option.
Add a global --exclude-logged (--remaining-packages) option which allows to
skip the helper on the packages which it has already been run on (i.e. the
helper is already present in the package debhelper log). Useful for invoking
the helper on the packages remaining unprocessed after calling the helper with
special options on a few specific packages.
Signed-off-by: Modestas Vainius
---
Debian/Debhelper/Dh_Getopt.pm |6 ++
Debian/Debhelper/Dh_Lib.pm| 23 ++-
debhelper.pod |7 +++
3 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/Debian/Debhelper/Dh_Getopt.pm b/Debian/Debhelper/Dh_Getopt.pm
index ef94e94..116e74d 100644
--- a/Debian/Debhelper/Dh_Getopt.pm
+++ b/Debian/Debhelper/Dh_Getopt.pm
@@ -91,6 +91,9 @@ sub getoptions {
"N=s" => \&ExcludePackage,
"no-package=s" => \&ExcludePackage,
+ "exclude-logged" => \$dh{EXCLUDE_LOGGED},
+ "remaining-packages" => \$dh{EXCLUDE_LOGGED},
+
"dbg-package=s" => \&AddDebugPackage,
"s" => \&AddPackage,
@@ -222,6 +225,9 @@ sub parseopts {
my $package;
my %packages_seen;
foreach $package (@{$dh{DOPACKAGES}}) {
+ if (defined($dh{EXCLUDE_LOGGED}) && is_in_log($package)) {
+ $exclude_package{$package}=1;
+ }
if (! $exclude_package{$package}) {
if (! exists $packages_seen{$package}) {
$packages_seen{$package}=1;
diff --git a/Debian/Debhelper/Dh_Lib.pm b/Debian/Debhelper/Dh_Lib.pm
index d481128..61510e2 100644
--- a/Debian/Debhelper/Dh_Lib.pm
+++ b/Debian/Debhelper/Dh_Lib.pm
@@ -15,7 +15,7 @@ use vars qw(@ISA @EXPORT %dh);
&filedoublearray &getpackages &basename &dirname &xargs %dh
&compat &addsubstvar &delsubstvar &excludefile &package_arch
&is_udeb &udeb_filename &debhelper_script_subst &escape_shell
- &inhibit_log);
+ &inhibit_log &is_helper_in_log &is_in_log);
my $max_compat=7;
@@ -121,6 +121,27 @@ sub inhibit_log {
$write_log=0;
}
+sub is_helper_in_log {
+
