[Reproducible-commits] [dpkg] 25/74: Revert "Dpkg::Conf: Switch implementation to be hash based"

Mattia Rizzolo mattia at debian.org
Sun Jul 3 22:22:53 UTC 2016


This is an automated email from the git hooks/post-receive script.

mattia pushed a commit to annotated tag 1.18.8
in repository dpkg.

commit 24a4f968635d60dde8bd2077d652096119e8d4f9
Author: Guillem Jover <guillem at debian.org>
Date:   Sun May 22 17:10:59 2016 +0200

    Revert "Dpkg::Conf: Switch implementation to be hash based"
    
    This reverts commit 94e241761c06ab112ec3e899dd9449784928c6c5.
    
    This change broke backwards compatibility in multiple ways. The
    format_argv option was set by default, the order was not preserved,
    which was important for dpkg.cfg files, and duplicate option names
    stopped being supported.
    
    Add regression tests to avoid similar changes in the future.
    
    Closes: #824938
---
 debian/changelog                 |  6 ++++
 scripts/Dpkg/Conf.pm             | 78 +++++++++++++---------------------------
 scripts/t/Dpkg_Conf.t            | 64 ++++++++++++++++-----------------
 scripts/t/Dpkg_Conf/config-mixed |  5 ++-
 4 files changed, 66 insertions(+), 87 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index 9d6642c..df76d26 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -13,6 +13,12 @@ dpkg (1.18.8) UNRELEASED; urgency=medium
       dpkg-parsechangelog's output.
     - Validate source version in set_version_substvars()'s Dpkg::Substvars
       method.
+    - Revert "Dpkg::Conf: Switch implementation to be hash based", as this
+      change broke backwards compatibility in multiple ways. The format_argv
+      option was set by default, the order was not preserved, which was
+      important for dpkg.cfg files, and duplicate option names stopped being
+      supported. Add regression tests to avoid similar changes in the future.
+      Closes: #824938
   * Test suite:
     - Bump perlcritic ValuesAndExpressions::RequireNumberSeparators minimum
       to 99999.
diff --git a/scripts/Dpkg/Conf.pm b/scripts/Dpkg/Conf.pm
index 8e49147..bf9a613 100644
--- a/scripts/Dpkg/Conf.pm
+++ b/scripts/Dpkg/Conf.pm
@@ -18,7 +18,9 @@ package Dpkg::Conf;
 use strict;
 use warnings;
 
-our $VERSION = '1.02';
+our $VERSION = '1.03';
+
+use Carp;
 
 use Dpkg::Gettext;
 use Dpkg::ErrorHandling;
@@ -58,7 +60,7 @@ sub new {
     my $class = ref($this) || $this;
 
     my $self = {
-	options => {},
+	options => [],
 	allow_short => 0,
     };
     foreach my $opt (keys %opts) {
@@ -79,46 +81,19 @@ Returns the list of options that can be parsed like @ARGV.
 
 sub get_options {
     my $self = shift;
-    my @options;
-
-    foreach my $name (sort keys %{$self->{options}}) {
-        my $value = $self->{options}->{$name};
 
-        $name = "--$name" unless $name =~ /^-/;
-        if (defined $value) {
-            push @options, "$name=$value";
-        } else {
-            push @options, $name;
-        }
-    }
-
-    return @options;
+    return @{$self->{options}};
 }
 
-=item $value = $conf->get($name)
-
-Returns the value of option $name, where the option name should not have "--"
-prefixed. If the option is not present the function will return undef.
-
-=cut
+# These functions existed for a brief time, but do not mesh well with
+# repeated options.
 
 sub get {
-    my ($self, $name) = @_;
-
-    return $self->{options}->{$name};
+    croak 'obsolete function, use get_options instead';
 }
 
-=item $conf->set($name, $value)
-
-Set the $value of option $name, where the option name should not have "--"
-prefixed.
-
-=cut
-
 sub set {
-    my ($self, $name, $value) = @_;
-
-    $self->{options}->{$name} = $value;
+    croak 'obsolete function, use get_options instead';
 }
 
 =item $conf->load($file)
@@ -150,11 +125,13 @@ sub parse {
 	}
 	if (/^([^=]+)(?:=(.*))?$/) {
 	    my ($name, $value) = ($1, $2);
+	    $name = "--$name" unless $name =~ /^-/;
 	    if (defined $value) {
 		$value =~ s/^"(.*)"$/$1/ or $value =~ s/^'(.*)'$/$1/;
+		push @{$self->{options}}, "$name=$value";
+	    } else {
+		push @{$self->{options}}, $name;
 	    }
-	    $name =~ s/^--//;
-	    $self->{options}->{$name} = $value;
 	    $count++;
 	} else {
 	    warning(g_('invalid syntax for option in %s, line %d'), $desc, $.);
@@ -167,8 +144,6 @@ sub parse {
 
 Filter the list of options, either removing or keeping all those that
 return true when &$opts{remove}($option) or &opts{keep}($option) is called.
-If $opts{format_argv} is true the long options passed to the filter
-functions will have "--" prefixed.
 
 =cut
 
@@ -177,16 +152,10 @@ sub filter {
     my $remove = $opts{remove} // sub { 0 };
     my $keep = $opts{keep} // sub { 1 };
 
-    foreach my $name (keys %{$self->{options}}) {
-        my $option = $name;
+    croak 'obsolete option format_argv' if exists $opts{format_argv};
 
-        if ($opts{format_argv}) {
-            $option = "--$name" unless $name =~ /^-/;
-        }
-        if (&$remove($option) or not &$keep($option)) {
-            delete $self->{options}->{$name};
-        }
-    }
+    @{$self->{options}} = grep { not &$remove($_) and &$keep($_) }
+                               @{$self->{options}};
 }
 
 =item $string = $conf->output($fh)
@@ -207,12 +176,9 @@ Save the options in a file.
 sub output {
     my ($self, $fh) = @_;
     my $ret = '';
-
-    foreach my $name (sort keys %{$self->{options}}) {
-	my $value = $self->{options}->{$name};
-
-	my $opt = $name;
-	$opt .= " = \"$value\"" if defined $value;
+    foreach my $opt ($self->get_options()) {
+	$opt =~ s/^--//;
+	$opt =~ s/^([^=]+)=(.*)$/$1 = "$2"/;
 	$opt .= "\n";
 	print { $fh } $opt if defined $fh;
 	$ret .= $opt;
@@ -224,6 +190,12 @@ sub output {
 
 =head1 CHANGES
 
+=head2 Version 1.03 (dpkg 1.18.8)
+
+Obsolete option: 'format_argv' in $conf->filter().
+
+Obsolete methods: $conf->get(), $conf->set().
+
 =head2 Version 1.02 (dpkg 1.18.5)
 
 New option: Accept new option 'format_argv' in $conf->filter().
diff --git a/scripts/t/Dpkg_Conf.t b/scripts/t/Dpkg_Conf.t
index 3f8070e..c6f0510 100644
--- a/scripts/t/Dpkg_Conf.t
+++ b/scripts/t/Dpkg_Conf.t
@@ -16,7 +16,7 @@
 use strict;
 use warnings;
 
-use Test::More tests => 14;
+use Test::More tests => 9;
 
 BEGIN {
     use_ok('Dpkg::Conf');
@@ -28,17 +28,21 @@ my $datadir = $srcdir . '/t/Dpkg_Conf';
 my ($conf, $count, @opts);
 
 my @expected_long_opts = (
-'--l=v',
-'--option-dash=value-dash',
 '--option-double-quotes=value double quotes',
-'--option-equal=value-equal=subvalue-equal',
-'--option-indent=value-indent',
-'--option-name=value-name',
-'--option-noequal=value-noequal',
-'--option-simple',
 '--option-single-quotes=value single quotes',
 '--option-space=value words space',
-);
+qw(
+--option-dupe=value1
+--option-name=value-name
+--option-indent=value-indent
+--option-equal=value-equal=subvalue-equal
+--option-noequal=value-noequal
+--option-dupe=value2
+--option-simple
+--option-dash=value-dash
+--option-dupe=value3
+--l=v
+));
 my @expected_short_opts = qw(
 -o=vd
 -s
@@ -48,55 +52,50 @@ $conf = Dpkg::Conf->new();
 local $SIG{__WARN__} = sub { };
 $count = $conf->load("$datadir/config-mixed");
 delete $SIG{__WARN__};
-is($count, 10, 'Load a config file, only long options');
+is($count, 13, 'Load a config file, only long options');
 
 @opts = $conf->get_options();
 is_deeply(\@opts, \@expected_long_opts, 'Parse long options');
 
 $conf = Dpkg::Conf->new(allow_short => 1);
 $count = $conf->load("$datadir/config-mixed");
-is($count, 12, 'Load a config file, mixed options');
+is($count, 15, 'Load a config file, mixed options');
 
 @opts = $conf->get_options();
-my @expected_mixed_opts = ( @expected_short_opts, @expected_long_opts );
+my @expected_mixed_opts = ( @expected_long_opts, @expected_short_opts );
 is_deeply(\@opts, \@expected_mixed_opts, 'Parse mixed options');
 
 my $expected_mixed_output = <<'MIXED';
--o = "vd"
--s
-l = "v"
-option-dash = "value-dash"
 option-double-quotes = "value double quotes"
-option-equal = "value-equal=subvalue-equal"
-option-indent = "value-indent"
+option-single-quotes = "value single quotes"
+option-space = "value words space"
+option-dupe = "value1"
 option-name = "value-name"
+option-indent = "value-indent"
+option-equal = "value-equal=subvalue-equal"
 option-noequal = "value-noequal"
+option-dupe = "value2"
 option-simple
-option-single-quotes = "value single quotes"
-option-space = "value words space"
+option-dash = "value-dash"
+option-dupe = "value3"
+l = "v"
+-o = "vd"
+-s
 MIXED
 
 is($conf->output, $expected_mixed_output, 'Output mixed options');
 
-is($conf->get('-o'), 'vd', 'Get option -o');
-is($conf->get('option-dash'), 'value-dash', 'Get option-dash');
-is($conf->get('option-space'), 'value words space', 'Get option-space');
-
-is($conf->get('manual-option'), undef, 'Get non-existent option');
-$conf->set('manual-option', 'manual value');
-is($conf->get('manual-option'), 'manual value', 'Get manual option');
-
 my $expected_filter;
 
 $expected_filter = <<'FILTER';
+l = "v"
 -o = "vd"
 -s
-l = "v"
 FILTER
 
 $conf = Dpkg::Conf->new(allow_short => 1);
 $conf->load("$datadir/config-mixed");
-$conf->filter(format_argv => 1, remove => sub { $_[0] =~ m/^--option/ });
+$conf->filter(remove => sub { $_[0] =~ m/^--option/ });
 is($conf->output, $expected_filter, 'Filter remove');
 
 $expected_filter = <<'FILTER';
@@ -106,7 +105,7 @@ FILTER
 
 $conf = Dpkg::Conf->new(allow_short => 1);
 $conf->load("$datadir/config-mixed");
-$conf->filter(keep => sub { $_[0] =~ m/^option-[a-z]+-quotes/ });
+$conf->filter(keep => sub { $_[0] =~ m/^--option-[a-z]+-quotes/ });
 is($conf->output, $expected_filter, 'Filter keep');
 
 $expected_filter = <<'FILTER';
@@ -115,8 +114,7 @@ FILTER
 
 $conf = Dpkg::Conf->new(allow_short => 1);
 $conf->load("$datadir/config-mixed");
-$conf->filter(format_argv => 1,
-              remove => sub { $_[0] =~ m/^--option/ },
+$conf->filter(remove => sub { $_[0] =~ m/^--option/ },
               keep => sub { $_[0] =~ m/^--/ });
 is($conf->output, $expected_filter, 'Filter keep and remove');
 
diff --git a/scripts/t/Dpkg_Conf/config-mixed b/scripts/t/Dpkg_Conf/config-mixed
index d149d38..87c0405 100644
--- a/scripts/t/Dpkg_Conf/config-mixed
+++ b/scripts/t/Dpkg_Conf/config-mixed
@@ -4,6 +4,7 @@ option-double-quotes = "value double quotes"
 option-single-quotes = 'value single quotes'
 option-space  = 	 value words space	  
 
+option-dupe=value1
 option-name=value-name		  
 	  
 	  # Indented comment.
@@ -11,10 +12,12 @@ option-name=value-name
 
 option-equal=value-equal=subvalue-equal
 option-noequal value-noequal
+option-dupe=value2
 option-simple
 
-# Fully spelled out option.
+# Fully spelled out options.
 --option-dash=value-dash
+--option-dupe=value3
 
 # Long option with one character name.
 l=v

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/reproducible/dpkg.git



More information about the Reproducible-commits mailing list