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

guillem pushed a commit to branch master
in repository dpkg.

commit 981a18c37036b68f368b0bfab71d2a984abba9e6
Author: Guillem Jover <guil...@debian.org>
Date:   Sun Apr 8 22:43:09 2018 +0200

    Dpkg::Version: Fix bool overload behavior
    
    The current bool overload has broken semantics, because it considers the
    version "0" to be false.
    
    The bool overload used to have sane semantics (equivalent to is_valid())
    before commit 5b9f353b2940de751df47036608afbe71992d622, but there it got
    changed to return the stringified version if it was valid, or undef
    otherwise, to fix a problem within dpkg-shlibdeps, instead of properly
    fixing the local-only problem in the tool. This makes the overload hard
    to use, and broke existing callers from external projects.
    
    We will emit a warning until dpkg 1.20.x to notify of the semantic change
    in case there is code relying on the broken semantics. For fixed code the
    warning can then be quiesced with:
    
      no warnings qw(Dpkg::Version::semantic_change::overload::bool);
    
    Closes: #895004
---
 debian/changelog          |  4 ++++
 scripts/Dpkg/Version.pm   | 20 +++++++++++++++++---
 scripts/dpkg-shlibdeps.pl |  3 ++-
 scripts/t/Dpkg_Version.t  | 17 ++++++++---------
 t/pod-spell.t             |  1 +
 5 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index 5098e7f..56b4c9e 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -89,6 +89,10 @@ dpkg (1.19.1) UNRELEASED; urgency=medium
     - Dpkg::Vendor::Debian: Mark riscv64 as having gcc builtin PIE.
     - Dpkg::Shlibs::Objdump: Fix ELF program detection, for PIE binaries and
       executable libraries.
+    - Dpkg::Version: Fix bool overload behavior back to be an is_valid()
+      alias. Emit a specific perl warning until 1.20.x so that users can check
+      whether the semantic change has any impact on the code, which can then
+      be quiesced. Closes: #895004
   * Documentation:
     - Update gettext minimal version in README.
     - Add a missing dot on the dpkg-buildflags(1) «lfs» feature paragraph.
diff --git a/scripts/Dpkg/Version.pm b/scripts/Dpkg/Version.pm
index 477082b..a06ba26 100644
--- a/scripts/Dpkg/Version.pm
+++ b/scripts/Dpkg/Version.pm
@@ -20,6 +20,7 @@ package Dpkg::Version;
 
 use strict;
 use warnings;
+use warnings::register qw(semantic_change::overload::bool);
 
 our $VERSION = '1.01';
 our @EXPORT = qw(
@@ -55,7 +56,12 @@ use overload
     '<=>' => \&_comparison,
     'cmp' => \&_comparison,
     '""'  => sub { return $_[0]->as_string(); },
-    'bool' => sub { return $_[0]->as_string() if $_[0]->is_valid(); },
+    'bool' => sub {
+        warnings::warnif('Dpkg::Version::semantic_change::overload::bool',
+                         'bool overload behavior has changed back to be ' .
+                         'an is_valid() alias');
+        return $_[0]->is_valid();
+    },
     'fallback' => 1;
 
 =encoding utf8
@@ -121,8 +127,16 @@ sub new {
 =item boolean evaluation
 
 When the Dpkg::Version object is used in a boolean evaluation (for example
-in "if ($v)" or "$v || 'default'") it returns its string representation
-if the version stored is valid ($v->is_valid()) and undef otherwise.
+in "if ($v)" or "$v ? \"$v\" : 'default'") it returns true if the version
+stored is valid ($v->is_valid()) and false otherwise.
+
+B<Notice>: Between dpkg 1.15.7.2 and 1.19.1 this overload used to return
+$v->as_string() if $v->is_valid(), a breaking change in behavior that caused
+"0" versions to be evaluated as false. To catch any possibly intended code
+that relied on those semantics, this overload will emit a warning with
+category "Dpkg::Version::semantic_change::overload::bool" until dpkg 1.20.x.
+Once fixed, or for already valid code the warning can be quiesced with
+C<no warnings qw(Dpkg::Version::semantic_change::overload::bool)>.
 
 =item $v->is_valid()
 
diff --git a/scripts/dpkg-shlibdeps.pl b/scripts/dpkg-shlibdeps.pl
index 323fba9..4b18545 100755
--- a/scripts/dpkg-shlibdeps.pl
+++ b/scripts/dpkg-shlibdeps.pl
@@ -546,7 +546,8 @@ foreach my $field (reverse @depfields) {
            map {
                # Translate dependency templates into real dependencies
                my $templ = $_;
-               if ($dependencies{$field}{$templ}) {
+               if ($dependencies{$field}{$templ}->is_valid() and
+                   $dependencies{$field}{$templ}->as_string()) {
                    $templ =~ s/#MINVER#/(>= $dependencies{$field}{$templ})/g;
                } else {
                    $templ =~ s/#MINVER#//g;
diff --git a/scripts/t/Dpkg_Version.t b/scripts/t/Dpkg_Version.t
index 78db7ae..17e9b4b 100644
--- a/scripts/t/Dpkg_Version.t
+++ b/scripts/t/Dpkg_Version.t
@@ -20,6 +20,7 @@ use Test::More;
 
 use Dpkg::ErrorHandling;
 use Dpkg::IPC;
+use Dpkg::Version;
 
 report_options(quiet_warnings => 1);
 
@@ -30,7 +31,7 @@ my @ops = ('<', '<<', 'lt',
           '>=', 'ge',
           '>', '>>', 'gt');
 
-plan tests => scalar(@tests) * (3 * scalar(@ops) + 4) + 30;
+plan tests => scalar(@tests) * (3 * scalar(@ops) + 4) + 27;
 
 sub dpkg_vercmp {
      my ($a, $cmp, $b) = @_;
@@ -57,8 +58,6 @@ sub obj_vercmp {
      return $a gt $b if $cmp eq 'gt';
 }
 
-use_ok('Dpkg::Version');
-
 my $truth = {
     '-1' => {
        '<<' => 1, 'lt' => 1,
@@ -83,6 +82,12 @@ my $truth = {
     },
 };
 
+# XXX: Some of the tests check the bool overload, which currently emits
+# the semantic_change warning. Disable it until we stop emitting the
+# warning in dpkg 1.20.x.
+## no critic (TestingAndDebugging::ProhibitNoWarnings)
+no warnings(qw(Dpkg::Version::semantic_change::overload::bool));
+
 # Handling of empty/invalid versions
 my $empty = Dpkg::Version->new('');
 ok($empty eq '', "Dpkg::Version->new('') eq ''");
@@ -128,12 +133,6 @@ ok(!$ver->is_native(), 'upstream version w/ revision is 
not native');
 $ver = Dpkg::Version->new('1.0-1.0-1');
 ok(!$ver->is_native(), 'upstream version w/ dash and revision is not native');
 
-# Other tests
-$ver = Dpkg::Version->new('1.2.3-4');
-is($ver || 'default', '1.2.3-4', 'bool eval returns string representation');
-$ver = Dpkg::Version->new('0');
-is($ver || 'default', 'default', 'bool eval of version 0 is still false...');
-
 # Comparisons
 foreach my $case (@tests) {
     my ($a, $b, $res) = split ' ', $case;
diff --git a/t/pod-spell.t b/t/pod-spell.t
index 6d3f7ff..111d592 100644
--- a/t/pod-spell.t
+++ b/t/pod-spell.t
@@ -83,6 +83,7 @@ modelines
 multiarch
 nocheck
 qa
+quiesced
 reportfile
 rfc822
 sig

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

Reply via email to