Bug#1001403: debhelper: claims it needs perl 5.24, but it seems to require 5.28

2021-12-16 Thread David Ward

On 12/9/2021 11:15 AM, Mattia Rizzolo wrote:

Source: debhelper
Version: 13.5.2

Hi Niels!

I was looking at backporting 13.5.2 all the way to ubuntu 18.04 bionic,
which ships with perl 5.26.

However, it seems that commit 3b2fe5334b88e38197fa24d9459544e50466fc92
"Dh_Lib.pm: Use state feature (by using v5.24)" doesn't make it a
no-change rebuild.

| debian/rules clean
|./run dh clean --without autoreconf --with build-stamp
|Initialization of state variables in list context currently forbidden at 
/build/debhelper-13.5.2ubuntu1~bpo18.04.1/lib/Debian/Debhelper/Dh_Lib.pm line 2026, near 
");"
|BEGIN not safe after errors--compilation aborted at 
/build/debhelper-13.5.2ubuntu1~bpo18.04.1/lib/Debian/Debhelper/Dh_Lib.pm line 
2748.
|Compilation failed in require at ./dh line 15.
|BEGIN failed--compilation aborted at ./dh line 15.
|debian/rules:15: recipe for target 'clean' failed


Whilst it's true that perl 5.24 introduced the state feature, being able
to initialized a persistant hash or array is a feature that has been
introduced with perl 5.28.
https://perldoc.perl.org/perl5280delta#Initialisation-of-aggregate-state-variables


I'm not sure if I should ask you to fully support perl 5.26, or perhaps
just bump the requirement; I'll leave the choice to you, however of
course I'd prefer the former :)


Incidentally, I believe that this is the background of
24fbbbfb116fc07e1ed2a09eb4e5f6713a74e361, so IMHO you should either bump
_that_ `use`, or, if you elect to support 5.26, revert it in favour of
not using `state` in that form.


Commit 24fbbbfb116f was partially reverted:
https://tracker.debian.org/news/1247918/accepted-debhelper-134nmu1-source-into-unstable/

So I agree that either your patch below should be applied, or commit 
3b2fe5334b88 should be reverted, to bring the code in line with the 
stated requirement for Perl 5.24+. (This would also allow it to run on 
EPEL 8.)


Additionally, the remainder of commit 24fbbbfb116f should be reverted, 
since dh_missing only requires Perl 5.24+ (not 5.28+).



Also, if you go the route of not supporting old perl, could you please
consider documenting this in d/control too?


Now, with the little tiny perl knowledge I had I did this.  Following
this change, and reverting 24fbbbfb116fc07e1ed2a09eb4e5f6713a74e361,
debhlper builds fine at the very least.  I'll have to do some tests to
see if it actually works fine too, but I'm optimistic :)


|diff --git a/lib/Debian/Debhelper/Dh_Lib.pm b/lib/Debian/Debhelper/Dh_Lib.pm
|index 2fefad69..30c30256 100644
|--- a/lib/Debian/Debhelper/Dh_Lib.pm
|+++ b/lib/Debian/Debhelper/Dh_Lib.pm
|@@ -2014,6 +2014,8 @@ sub _parse_debian_control {
| # - Takes an optional keyword; if passed, this will return true if the 
keyword is listed in R^3 (Rules-Requires-Root)
| # - If the optional keyword is omitted or not present in R^3 and R^3 is not 
'binary-targets', then returns false
| # - Returns true otherwise (i.e. keyword is in R^3 or R^3 is 'binary-targets')
|+{
|+my %rrr;
| sub should_use_root {
|my ($keyword) = @_;
|my $rrr_env = $ENV{'DEB_RULES_REQUIRES_ROOT'} // 'binary-targets';
|@@ -2023,10 +2025,11 @@ sub should_use_root {
|return 1 if $rrr_env eq 'binary-targets';
|return 0 if not defined($keyword);
|
|-   state %rrr = map { $_ => 1 } split(' ', $rrr_env);
|+   %rrr = map { $_ => 1 } split(' ', $rrr_env) if not %rrr;
|return 1 if exists($rrr{$keyword});
|return 0;
| }
|+}
|
| # Returns the "gain root command" as a list suitable for passing as a part of the 
command to "doit()"
| sub gain_root_cmd {
|@@ -2139,11 +2142,16 @@ sub is_udeb {
|return $package_types{$package} eq 'udeb';
| }
|
|+{
|+my %packages_to_process;
| sub process_pkg {
|my ($package) = @_;
|-   state %packages_to_process = map { $_ => 1 } @{$dh{DOPACKAGES}};
|+   if (not %packages_to_process) {
|+   %packages_to_process = map { $_ => 1 } @{$dh{DOPACKAGES}};
|+   }
|return $packages_to_process{$package} // 0;
| }
|+}
|
| # Only useful for dh(1)
| sub bd_dh_sequences {
|@@ -2938,12 +2946,15 @@ sub perl_cross_incdir {
|return $incdir;
| }
|
|+{
|+my %known_packages;
| sub is_known_package {
|my ($package) = @_;
|-   state %known_packages = map { $_ => 1 } getpackages();
|+   %known_packages = map { $_ => 1 } getpackages() if not %known_packages;
|return 1 if exists($known_packages{$package});
|return 0
| }
|+}
|
| sub assert_opt_is_known_package {
|my ($package, $method) = @_;
|

/




Bug#1001403: debhelper: claims it needs perl 5.24, but it seems to require 5.28

2021-12-09 Thread Mattia Rizzolo
Source: debhelper
Version: 13.5.2

Hi Niels!

I was looking at backporting 13.5.2 all the way to ubuntu 18.04 bionic,
which ships with perl 5.26.

However, it seems that commit 3b2fe5334b88e38197fa24d9459544e50466fc92
"Dh_Lib.pm: Use state feature (by using v5.24)" doesn't make it a
no-change rebuild.

| debian/rules clean
|./run dh clean --without autoreconf --with build-stamp
|Initialization of state variables in list context currently forbidden at 
/build/debhelper-13.5.2ubuntu1~bpo18.04.1/lib/Debian/Debhelper/Dh_Lib.pm line 
2026, near ");"
|BEGIN not safe after errors--compilation aborted at 
/build/debhelper-13.5.2ubuntu1~bpo18.04.1/lib/Debian/Debhelper/Dh_Lib.pm line 
2748.
|Compilation failed in require at ./dh line 15.
|BEGIN failed--compilation aborted at ./dh line 15.
|debian/rules:15: recipe for target 'clean' failed


Whilst it's true that perl 5.24 introduced the state feature, being able
to initialized a persistant hash or array is a feature that has been
introduced with perl 5.28.
https://perldoc.perl.org/perl5280delta#Initialisation-of-aggregate-state-variables


I'm not sure if I should ask you to fully support perl 5.26, or perhaps
just bump the requirement; I'll leave the choice to you, however of
course I'd prefer the former :)


Incidentally, I believe that this is the background of
24fbbbfb116fc07e1ed2a09eb4e5f6713a74e361, so IMHO you should either bump
_that_ `use`, or, if you elect to support 5.26, revert it in favour of
not using `state` in that form.

Also, if you go the route of not supporting old perl, could you please
consider documenting this in d/control too?


Now, with the little tiny perl knowledge I had I did this.  Following
this change, and reverting 24fbbbfb116fc07e1ed2a09eb4e5f6713a74e361,
debhlper builds fine at the very least.  I'll have to do some tests to
see if it actually works fine too, but I'm optimistic :)


|diff --git a/lib/Debian/Debhelper/Dh_Lib.pm b/lib/Debian/Debhelper/Dh_Lib.pm
|index 2fefad69..30c30256 100644
|--- a/lib/Debian/Debhelper/Dh_Lib.pm
|+++ b/lib/Debian/Debhelper/Dh_Lib.pm
|@@ -2014,6 +2014,8 @@ sub _parse_debian_control {
| # - Takes an optional keyword; if passed, this will return true if the 
keyword is listed in R^3 (Rules-Requires-Root)
| # - If the optional keyword is omitted or not present in R^3 and R^3 is not 
'binary-targets', then returns false
| # - Returns true otherwise (i.e. keyword is in R^3 or R^3 is 'binary-targets')
|+{
|+my %rrr;
| sub should_use_root {
|my ($keyword) = @_;
|my $rrr_env = $ENV{'DEB_RULES_REQUIRES_ROOT'} // 'binary-targets';
|@@ -2023,10 +2025,11 @@ sub should_use_root {
|return 1 if $rrr_env eq 'binary-targets';
|return 0 if not defined($keyword);
| 
|-   state %rrr = map { $_ => 1 } split(' ', $rrr_env);
|+   %rrr = map { $_ => 1 } split(' ', $rrr_env) if not %rrr;
|return 1 if exists($rrr{$keyword});
|return 0;
| }
|+}
| 
| # Returns the "gain root command" as a list suitable for passing as a part of 
the command to "doit()"
| sub gain_root_cmd {
|@@ -2139,11 +2142,16 @@ sub is_udeb {
|return $package_types{$package} eq 'udeb';
| }
| 
|+{
|+my %packages_to_process;
| sub process_pkg {
|my ($package) = @_;
|-   state %packages_to_process = map { $_ => 1 } @{$dh{DOPACKAGES}};
|+   if (not %packages_to_process) {
|+   %packages_to_process = map { $_ => 1 } @{$dh{DOPACKAGES}};
|+   }
|return $packages_to_process{$package} // 0;
| }
|+}
| 
| # Only useful for dh(1)
| sub bd_dh_sequences {
|@@ -2938,12 +2946,15 @@ sub perl_cross_incdir {
|return $incdir;
| }
| 
|+{
|+my %known_packages;
| sub is_known_package {
|my ($package) = @_;
|-   state %known_packages = map { $_ => 1 } getpackages();
|+   %known_packages = map { $_ => 1 } getpackages() if not %known_packages;
|return 1 if exists($known_packages{$package});
|return 0
| }
|+}
| 
| sub assert_opt_is_known_package {
|my ($package, $method) = @_;
|

-- 
regards,
Mattia Rizzolo

GPG Key: 66AE 2B4A FCCF 3F52 DA18  4D18 4B04 3FCD B944 4540  .''`.
More about me:  https://mapreri.org : :'  :
Launchpad user: https://launchpad.net/~mapreri  `. `'`
Debian QA page: https://qa.debian.org/developer.php?login=mattia  `-


signature.asc
Description: PGP signature