split get_make_jobserver_status into two functions

I disliked the complexity of the return values, and the boilerplate
code that followed the two calls to the function, to clean/unset
MAKEFLAGS. To solve both, I refactored it into two functions, one simply
tests to see if a jobserver is specified but unavailable, while the other
cleans/unsets MAKEFLAGS.

This loses the ability to pull the jobs-N count out of MAKEFLAGS,
but that was not currently used.

diff --git a/Debian/Debhelper/Buildsystem/makefile.pm b/Debian/Debhelper/Buildsystem/makefile.pm
index f21b2cbd6971c06e634b1ea238cb643b51831c13..159f7c1e43f56139bcdd582743270ba1611c7571 100644 (file)
--- a/Debian/Debhelper/Buildsystem/makefile.pm
+++ b/Debian/Debhelper/Buildsystem/makefile.pm
@@ -7,7 +7,8 @@
 package Debian::Debhelper::Buildsystem::makefile;
 
 use strict;
-use Debian::Debhelper::Dh_Lib qw(escape_shell get_make_jobserver_status);
+use Debian::Debhelper::Dh_Lib qw(escape_shell is_make_jobserver_unavailable
+       clean_makeflags);
 use base 'Debian::Debhelper::Buildsystem';
 
 sub get_makecmd_C {
@@ -37,14 +38,8 @@ sub do_make {
        # Always clean MAKEFLAGS from unavailable jobserver options. If parallel
        # is enabled, do more extensive clean up from all job control specific
        # options
-       my ($status, $makeflags) = get_make_jobserver_status();
-       if ($status eq "jobserver-unavailable" || defined $this->get_parallel()) {
-               if (defined $makeflags) {
-                       $ENV{MAKEFLAGS} = $makeflags;
-               }
-               else {
-                       delete $ENV{MAKEFLAGS} if exists $ENV{MAKEFLAGS};
-               }
+       if (defined $this->get_parallel() || is_make_jobserver_unavailable()) {
+               clean_makeflags();
        }
 
        # Start a new jobserver if parallel building was requested

diff --git a/Debian/Debhelper/Dh_Lib.pm b/Debian/Debhelper/Dh_Lib.pm
index 602b77fbc012f97dae29b069c279f2ea64d12390..ac0d8ab4a05f7ea6f7aa317b8202b7471ab60bec 100644 (file)
--- a/Debian/Debhelper/Dh_Lib.pm
+++ b/Debian/Debhelper/Dh_Lib.pm
@@ -16,7 +16,7 @@ use vars qw(@ISA @EXPORT %dh);
            &compat &addsubstvar &delsubstvar &excludefile &package_arch
            &is_udeb &udeb_filename &debhelper_script_subst &escape_shell
            &inhibit_log &load_log &write_log &dpkg_architecture_value
-           &sourcepackage &get_make_jobserver_status);
+           &sourcepackage &is_make_jobserver_unavailable &clean_makeflags);
 
 my $max_compat=7;
 
@@ -794,44 +794,35 @@ sub debhelper_script_subst {
        }
 }
 
-# A helper subroutine for detecting (based on MAKEFLAGS) if make jobserver 
-# is enabled, if it is available or MAKEFLAGS contains "jobs" option.
-# It returns current status (jobserver, jobserver-unavailable or jobs-N where
-# N is number of jobs, 0 if infinite) and MAKEFLAGS cleaned up from 
-# job control options.
-sub get_make_jobserver_status {
-       my $jobsre = qr/(?:^|\s)(?:(?:-j\s*|--jobs(?:=|\s+))(\d+)?|--jobs)\b/;
-       my $status = "";
-       my $makeflags;
+# Checks if make's jobserver is enabled via MAKEFLAGS, but
+# the FD used to communicate with it is actually not available.
+sub is_make_jobserver_unavailable {
+       if (exists $ENV{MAKEFLAGS} && 
+           $ENV{MAKEFLAGS} =~ /(?:^|\s)--jobserver-fds=(\d+)/) {
+               if (!open(my $in, "<&$1")) {
+                       return 1; # unavailable
+               }
+               else {
+                       close $in;
+                       return 0; # available
+               }
+       }
+
+       return; # no jobserver specified
+}
 
+# Cleans out job control options from MAKEFLAGS.
+sub clean_makeflags {
        if (exists $ENV{MAKEFLAGS}) {
-               $makeflags = $ENV{MAKEFLAGS};
-               if ($makeflags =~ /(?:^|\s)--jobserver-fds=(\d+)/) {
-                       $status = "jobserver";
-                       if (!open(my $in, "<&", "$1")) {
-                               # Job server is unavailable
-                               $status .= "-unavailable";
-                       }
-                       else {
-                               close $in;
-                       }
-                       # Clean makeflags up
-                       $makeflags =~ s/(?:^|\s)--jobserver-fds=\S+//g;
-                       $makeflags =~ s/(?:^|\s)-j\b//g;
+               if ($ENV{MAKEFLAGS} =~ /(?:^|\s)--jobserver-fds=(\d+)/) {
+                       $ENV{MAKEFLAGS} =~ s/(?:^|\s)--jobserver-fds=\S+//g;
+                       $ENV{MAKEFLAGS} =~ s/(?:^|\s)-j\b//g;
                }
-               elsif (my @m = ($makeflags =~ /$jobsre/g)) {
-                       # Job count is specified in MAKEFLAGS. Whenever make reads it, a new
-                       # jobserver will be started. Job count returned is 0 if infinite.
-                       $status = "jobs-" . (defined $m[$#m] ? $m[$#m] : "0");
-                       # Clean makeflags up from "jobs" option(s)
-                       $makeflags =~ s/$jobsre//g;
+               else {
+                       $ENV{MAKEFLAGS} =~ s/(?:^|\s)(?:(?:-j\s*|--jobs(?:=|\s+))(\d+)?|--jobs)\b//g;
                }
+               delete $ENV{MAKEFLAGS} if $ENV{MAKEFLAGS} =~ /^\s*$/;
        }
-       if ($status) {
-               # MAKEFLAGS could be unset if it is empty
-               $makeflags = undef if $makeflags =~ /^\s*$/;
-       }
-       return wantarray ? ($status, $makeflags) : $status;
 }
 
 1

diff --git a/dh b/dh
index 9d1fca0eaa073dcc34c8feb619101b3d3f948c42..a233db4a8ef44369ffd521b177e5ce2d775d4256 100755 (executable)
--- a/dh
+++ b/dh
@@ -242,15 +242,8 @@ inhibit_log();
 # make has closed jobserver FDs). In such a case, MAKEFLAGS is cleaned from the
 # offending --jobserver-fds option in order to prevent further make invocations
 # from spitting warnings and disabling job support.
-my ($status, $makeflags) = get_make_jobserver_status();
-if ($status eq "jobserver-unavailable") {
-       # Stop make from spitting pointless job control warnings
-       if (defined $makeflags) {
-               $ENV{MAKEFLAGS} = $makeflags;
-       }
-       else {
-               delete $ENV{MAKEFLAGS};
-       }
+if (is_make_jobserver_unavailable()) {
+       clean_makeflags();
        # Enable parallel (no maximum) if the package doesn't since it appears this
        # dh is called via dpkg-buildpackage -jX.
        $dh{PARALLEL} = 0 if !defined $dh{PARALLEL};

diff --git a/t/buildsystems/buildsystem_tests b/t/buildsystems/buildsystem_tests
index 41c0f977ecfb930674a18b9d78b3816479417fd3..2f9a146e6e33e0acf31e2dbcb74391af75a1ce15 100755 (executable)
--- a/t/buildsystems/buildsystem_tests
+++ b/t/buildsystems/buildsystem_tests
@@ -1,6 +1,6 @@
 #!/usr/bin/perl
 
-use Test::More tests => 273;
+use Test::More tests => 281;
 
 use strict;
 use warnings;
@@ -484,39 +484,47 @@ ok ( ! -e 'bld', "bld got deleted too" );
 #### Test parallel building and related options / routines
 @tmp = ( $ENV{MAKEFLAGS}, $ENV{DEB_BUILD_OPTIONS} );
 
-# Test get_make_jobserver_status() sub
+# Test is_make_jobserver_unavailable and clean_makeflags.
 
 $ENV{MAKEFLAGS} = "--jobserver-fds=103,104 -j";
-is_deeply( [ get_make_jobserver_status() ], [ "jobserver-unavailable", undef ],
-       "get_make_jobserver_status(): unavailable jobserver, unset makeflags" );
+ok(is_make_jobserver_unavailable(), "unavailable jobserver" );
+clean_makeflags();
+ok(! exists $ENV{MAKEFLAGS}, "unset makeflags");
 
 $ENV{MAKEFLAGS} = "-a --jobserver-fds=103,104 -j -b";
-is_deeply( [ get_make_jobserver_status() ], [ "jobserver-unavailable", "-a -b" ],
-       "get_make_jobserver_status(): unavailable jobserver, clean makeflags" );
+ok(is_make_jobserver_unavailable(), "unavailable jobserver" );
+clean_makeflags();
+is($ENV{MAKEFLAGS}, "-a -b", "clean makeflags");
 
 $ENV{MAKEFLAGS} = " --jobserver-fds=1,2 -j  ";
-is_deeply( [ get_make_jobserver_status() ], [ "jobserver", undef ],
-       "get_make_jobserver_status(): jobserver (available), clean makeflags" );
+ok(! is_make_jobserver_unavailable(), "available jobserver" );
+clean_makeflags();
+ok(! exists $ENV{MAKEFLAGS}, "unset makeflags");
 
 $ENV{MAKEFLAGS} = "-a -j -b";
-is_deeply( [ get_make_jobserver_status() ], [ "jobs-0", "-a -b" ],
-       "get_make_jobserver_status(): -j" );
+ok(! is_make_jobserver_unavailable(), "no specified jobserver");
+clean_makeflags();
+is($ENV{MAKEFLAGS}, "-a -b", "clean makeflags");
 
 $ENV{MAKEFLAGS} = "-a --jobs -b";
-is_deeply( [ get_make_jobserver_status() ], [ "jobs-0", "-a -b" ],
-       "get_make_jobserver_status(): --jobs" );
+ok(! is_make_jobserver_unavailable(), "no specified jobserver");
+clean_makeflags();
+is($ENV{MAKEFLAGS}, "-a -b", "clean makeflags");
 
 $ENV{MAKEFLAGS} = "-j6";
-is_deeply( [ get_make_jobserver_status() ], [ "jobs-6", undef ],
-       "get_make_jobserver_status(): -j6" );
+ok(! is_make_jobserver_unavailable(), "no specified jobserver");
+clean_makeflags();
+ok(! exists $ENV{MAKEFLAGS}, "unset makeflags");
 
 $ENV{MAKEFLAGS} = "-a -j6 --jobs=7";
-is_deeply( [ get_make_jobserver_status() ], [ "jobs-7", "-a" ],
-       "get_make_jobserver_status(): -j6 --jobs=7" );
+ok(! is_make_jobserver_unavailable(), "no specified jobserver");
+clean_makeflags();
+is($ENV{MAKEFLAGS}, "-a", "clean makeflags");
 
 $ENV{MAKEFLAGS} = "-j6 --jobserver-fds=5,6 --jobs=8";
-is_deeply( [ get_make_jobserver_status() ], [ "jobserver-unavailable", "-j6 --jobs=8" ],
-       "get_make_jobserver_status(): mixed jobserver and -j/--jobs" );
+ok(is_make_jobserver_unavailable(), "unavailable jobserver");
+clean_makeflags();
+is($ENV{MAKEFLAGS}, "-j6 --jobs=8", "jobserver options removed");
 
 # Test parallel building with makefile build system.
 $ENV{MAKEFLAGS} = "";