From d3bc4c31453aabea565f8991dcbe1d9e1a0fc737 Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Fri, 6 Apr 2018 19:49:20 +0000
Subject: [PATCH 1/2] Rewrite build system to support a "target build system"

Several of the build systems consists of a configure step that
generates a buildscript for another build tool.  Notable examples
being "cmake" and "meson", which even supports multiple backend tools.
This change makes it natively possible for debhelper to support such
build systems with multiple backends.

Note that only build systems with multiple backends have been
rewritten.

Signed-off-by: Niels Thykier <niels@thykier.net>
---
 lib/Debian/Debhelper/Buildsystem.pm       | 117 +++++++++++++++++++++++++++++-
 lib/Debian/Debhelper/Buildsystem/cmake.pm |  33 +++++++--
 lib/Debian/Debhelper/Buildsystem/meson.pm |  20 ++++-
 lib/Debian/Debhelper/Buildsystem/ninja.pm |   8 --
 lib/Debian/Debhelper/Dh_Buildsystems.pm   |  71 +++++++++++++-----
 t/buildsystems/03-bs-auto-buildable.t     |  14 ++--
 6 files changed, 216 insertions(+), 47 deletions(-)

diff --git a/lib/Debian/Debhelper/Buildsystem.pm b/lib/Debian/Debhelper/Buildsystem.pm
index a0ce6367..6e822c45 100644
--- a/lib/Debian/Debhelper/Buildsystem.pm
+++ b/lib/Debian/Debhelper/Buildsystem.pm
@@ -11,15 +11,26 @@ use warnings;
 use Cwd ();
 use File::Spec;
 use Debian::Debhelper::Dh_Lib;
+use Debian::Debhelper::Dh_Buildsystems qw(load_buildsystem);
 
 # Build system name. Defaults to the last component of the class
 # name. Do not override this method unless you know what you are
 # doing.
 sub NAME {
-	my $this=shift;
-	my $class = ref($this) || $this;
+	my ($this) = @_;
+	my $class = ref($this);
+	my $target_name;
+	if ($class) {
+		if ($this->IS_GENERATOR_BUILD_SYSTEM) {
+			$target_name = $this->{'targetbuildsystem'}->NAME;
+		}
+	} else {
+		$class = $this;
+	}
 	if ($class =~ m/^.+::([^:]+)$/) {
-		return $1;
+		my $name = $1;
+		return "${name}+${target_name}" if defined($target_name);
+		return $name;
 	}
 	else {
 		error("Ä±nvalid build system class name: $class");
@@ -37,6 +48,43 @@ sub DEFAULT_BUILD_DIRECTORY {
 	"obj-" . dpkg_architecture_value("DEB_HOST_GNU_TYPE");
 }
 
+# Return 1 if the build system generator
+sub IS_GENERATOR_BUILD_SYSTEM {
+	return 0;
+}
+
+# Generator build-systems only
+# The name of the supported target systems.  The first one is
+# assumed to be the default if DEFAULT_TARGET_BUILD_SYSTEM is
+# not overridden.
+sub SUPPORTED_TARGET_BUILD_SYSTEMS {
+	error("class lacking SUPPORTED_TARGET_BUILD_SYSTEMS");
+}
+
+# Generator build-systems only
+# Name of default target build system if target is unspecified
+#  (e.g. --buildsystem=cmake instead of cmake+makefile).
+sub DEFAULT_TARGET_BUILD_SYSTEM {
+	my ($this) = @_;
+	my @targets = $this->SUPPORTED_TARGET_BUILD_SYSTEMS;
+	# Assume they are listed in order.
+	return $targets[0];
+}
+
+# For regular build systems, the same as DESCRIPTION
+# For generator based build systems, the DESCRIPTION of the generator build
+# system + the target build system.  Do not override this method unless you
+# know what you are doing.
+sub FULL_DESCRIPTION {
+	my ($this) = @_;
+	my $description = $this->DESCRIPTION;
+	return $description if not exists($this->{'targetbuildsystem'});
+	my $target_build_system = $this->{'targetbuildsystem'};
+	return $description if not defined($target_build_system);
+	my $target_desc = $target_build_system->FULL_DESCRIPTION;
+	return "${description} combined with ${target_desc}";
+}
+
 # Constructs a new build system object. Named parameters:
 # - sourcedir-     specifies source directory (relative to the current (top)
 #                  directory) where the sources to be built live. If not
@@ -46,6 +94,8 @@ sub DEFAULT_BUILD_DIRECTORY {
 #                  DEFAULT_BUILD_DIRECTORY directory will be used.
 # - parallel -     max number of parallel processes to be spawned for building
 #                  sources (-1 = unlimited; 1 = no parallel)
+# - targetbuildsystem -     The target build system for generator based build
+#                           systems.  Only set for generator build systems.
 # Derived class can override the constructor to initialize common object
 # parameters. Do NOT use constructor to execute commands or otherwise
 # configure/setup build environment. There is absolutely no guarantee the
@@ -58,6 +108,7 @@ sub new {
 	                   builddir => undef,
 	                   parallel => undef,
 	                   cwd => Cwd::getcwd() }, $class);
+	my $target_bs_name;
 
 	if (exists $opts{sourcedir}) {
 		# Get relative sourcedir abs_path (without symlinks)
@@ -73,6 +124,19 @@ sub new {
 	if (defined $opts{parallel}) {
 		$this->{parallel} = $opts{parallel};
 	}
+	if (exists $opts{targetbuildsystem}) {
+		$target_bs_name = $opts{targetbuildsystem};
+	}
+
+	$target_bs_name //= $this->DEFAULT_TARGET_BUILD_SYSTEM if $this->IS_GENERATOR_BUILD_SYSTEM;
+
+	if (defined($target_bs_name)) {
+		my %target_opts = %opts;
+		delete($target_opts{'targetbuildsystem'});
+		my $target_system = load_buildsystem($target_bs_name, undef, %target_opts);
+		$this->set_targetbuildsystem($target_system);
+	}
+
 	return $this;
 }
 
@@ -104,6 +168,27 @@ sub _set_builddir {
 	return $builddir;
 }
 
+sub set_targetbuildsystem {
+	my ($this, $target_system) = @_;
+	my $ok = 0;
+	my $target_bs_name = $target_system->NAME;
+	if (not $this->IS_GENERATOR_BUILD_SYSTEM) {
+		my $name = $this->NAME;
+		error("Buildsystem ${name} is not a generator build system");
+	}
+	for my $supported_bs_name ($this->SUPPORTED_TARGET_BUILD_SYSTEMS) {
+		if ($supported_bs_name eq $target_bs_name) {
+			$ok = 1;
+			last;
+		}
+	}
+	if (not $ok) {
+		my $name = $this->NAME;
+		error("Buildsystem ${name} does not support ${target_bs_name} as target build system.");
+	}
+	$this->{targetbuildsystem} = $target_system
+}
+
 # This instance method is called to check if the build system is able
 # to build a source package. It will be called during the build
 # system auto-selection process, inside the root directory of the debian
@@ -412,6 +497,9 @@ sub pre_building_step {
 		    " does not support building out of source tree. In source building enforced.");
 		delete $this->{warn_insource};
 	}
+	if ($this->IS_GENERATOR_BUILD_SYSTEM) {
+		$this->{targetbuildsystem}->pre_building_step(@_);
+	}
 }
 
 # Instance method that is called after performing any step (see below).
@@ -420,6 +508,9 @@ sub pre_building_step {
 sub post_building_step {
 	my $this=shift;
 	my ($step)=@_;
+	if ($this->IS_GENERATOR_BUILD_SYSTEM) {
+		$this->{targetbuildsystem}->post_building_step(@_);
+	}
 }
 
 # The instance methods below provide support for configuring,
@@ -430,26 +521,44 @@ sub post_building_step {
 # implement build system specific steps needed to build the
 # source. Arbitrary number of custom step arguments might be
 # passed. Default implementations do nothing.
+#
+# Note: For generator build systems, the default is to
+# delegate the step to the target build system for all
+# steps except configure.
 sub configure {
 	my $this=shift;
 }
 
 sub build {
 	my $this=shift;
+	if ($this->IS_GENERATOR_BUILD_SYSTEM) {
+		$this->{targetbuildsystem}->build(@_);
+	}
 }
 
 sub test {
 	my $this=shift;
+	if ($this->IS_GENERATOR_BUILD_SYSTEM) {
+		$this->{targetbuildsystem}->test(@_);
+	}
 }
 
 # destdir parameter specifies where to install files.
 sub install {
 	my $this=shift;
-	my $destdir=shift;
+	my ($destdir) = @_;
+
+	if ($this->IS_GENERATOR_BUILD_SYSTEM) {
+		$this->{targetbuildsystem}->install(@_);
+	}
 }
 
 sub clean {
 	my $this=shift;
+
+	if ($this->IS_GENERATOR_BUILD_SYSTEM) {
+		$this->{targetbuildsystem}->clean(@_);
+	}
 }
 
 1
diff --git a/lib/Debian/Debhelper/Buildsystem/cmake.pm b/lib/Debian/Debhelper/Buildsystem/cmake.pm
index 5e3ce082..a342e436 100644
--- a/lib/Debian/Debhelper/Buildsystem/cmake.pm
+++ b/lib/Debian/Debhelper/Buildsystem/cmake.pm
@@ -9,7 +9,7 @@ package Debian::Debhelper::Buildsystem::cmake;
 use strict;
 use warnings;
 use Debian::Debhelper::Dh_Lib qw(compat dpkg_architecture_value error is_cross_compiling);
-use parent qw(Debian::Debhelper::Buildsystem::makefile);
+use parent qw(Debian::Debhelper::Buildsystem);
 
 my @STANDARD_CMAKE_FLAGS = qw(
   -DCMAKE_INSTALL_PREFIX=/usr
@@ -31,12 +31,26 @@ sub DESCRIPTION {
 	"CMake (CMakeLists.txt)"
 }
 
+sub IS_GENERATOR_BUILD_SYSTEM {
+	return 1;
+}
+
+sub SUPPORTED_TARGET_BUILD_SYSTEMS {
+	return qw(makefile);
+}
+
 sub check_auto_buildable {
 	my $this=shift;
 	my ($step)=@_;
 	if (-e $this->get_sourcepath("CMakeLists.txt")) {
 		my $ret = ($step eq "configure" && 1) ||
-		          $this->SUPER::check_auto_buildable(@_);
+		          $this->{targetbuildsystem}->check_auto_buildable(@_);
+		if ($step eq "clean" && defined($this->get_builddir())) {
+			# Assume that the package can be cleaned (i.e. the build directory can
+			# be removed) as long as it is built out-of-source tree and can be
+			# configured.
+			$ret++ if not $ret;
+		}
 		# Existence of CMakeCache.txt indicates cmake has already
 		# been used by a prior build step, so should be used
 		# instead of the parent makefile class.
@@ -111,13 +125,16 @@ sub configure {
 
 sub test {
 	my $this=shift;
-
-	# Unlike make, CTest does not have "unlimited parallel" setting (-j implies
-	# -j1). So in order to simulate unlimited parallel, allow to fork a huge
-	# number of threads instead.
-	my $parallel = ($this->get_parallel() > 0) ? $this->get_parallel() : 999;
+	my $target = $this->{targetbuildsystem};
 	$ENV{CTEST_OUTPUT_ON_FAILURE} = 1;
-	return $this->SUPER::test(@_, "ARGS+=-j$parallel");
+	if ($target->NAME eq 'makefile') {
+		# Unlike make, CTest does not have "unlimited parallel" setting (-j implies
+		# -j1). So in order to simulate unlimited parallel, allow to fork a huge
+		# number of threads instead.
+		my $parallel = ($this->get_parallel() > 0) ? $this->get_parallel() : 999;
+		push(@_, "ARGS+=-j$parallel")
+	}
+	return $this->SUPER::test(@_);
 }
 
 1
diff --git a/lib/Debian/Debhelper/Buildsystem/meson.pm b/lib/Debian/Debhelper/Buildsystem/meson.pm
index f157296f..91891485 100644
--- a/lib/Debian/Debhelper/Buildsystem/meson.pm
+++ b/lib/Debian/Debhelper/Buildsystem/meson.pm
@@ -8,12 +8,21 @@ package Debian::Debhelper::Buildsystem::meson;
 use strict;
 use warnings;
 use Debian::Debhelper::Dh_Lib qw(dpkg_architecture_value is_cross_compiling doit warning error generated_file);
-use parent qw(Debian::Debhelper::Buildsystem::ninja);
+use parent qw(Debian::Debhelper::Buildsystem);
 
 sub DESCRIPTION {
 	"Meson (meson.build)"
 }
 
+sub IS_GENERATOR_BUILD_SYSTEM {
+	return 1;
+}
+
+sub SUPPORTED_TARGET_BUILD_SYSTEMS {
+	return qw(ninja);
+}
+
+
 sub check_auto_buildable {
 	my $this=shift;
 	my ($step)=@_;
@@ -22,7 +31,14 @@ sub check_auto_buildable {
 
 	# Handle configure explicitly; inherit the rest
 	return 1 if $step eq "configure";
-	return $this->SUPER::check_auto_buildable(@_);
+	my $ret = $this->{targetbuildsystem}->check_auto_buildable(@_);
+	if ($ret == 0 and $step eq 'clean' and defined($this->get_builddir())) {
+		# Assume that the package can be cleaned (i.e. the build directory can
+		# be removed) as long as it is built out-of-source tree and can be
+		# configured.
+		$ret++;
+	}
+	return $ret;
 }
 
 sub new {
diff --git a/lib/Debian/Debhelper/Buildsystem/ninja.pm b/lib/Debian/Debhelper/Buildsystem/ninja.pm
index 8ebc95cd..c08ff166 100644
--- a/lib/Debian/Debhelper/Buildsystem/ninja.pm
+++ b/lib/Debian/Debhelper/Buildsystem/ninja.pm
@@ -30,14 +30,6 @@ sub check_auto_buildable {
 		# This is always called in the source directory, but generally
 		# Ninja files are created (or live) in the build directory.
 		return 1;
-	} elsif ($step eq "clean" && defined $this->get_builddir() &&
-	         $this->check_auto_buildable("configure"))
-	{
-		# Assume that the package can be cleaned (i.e. the build directory can
-		# be removed) as long as it is built out-of-source tree and can be
-		# configured. This is useful for derivative buildsystems which
-		# generate Ninja files.
-		return 1;
 	}
 	return 0;
 }
diff --git a/lib/Debian/Debhelper/Dh_Buildsystems.pm b/lib/Debian/Debhelper/Dh_Buildsystems.pm
index 7d4b421c..7f3a96b4 100644
--- a/lib/Debian/Debhelper/Dh_Buildsystems.pm
+++ b/lib/Debian/Debhelper/Dh_Buildsystems.pm
@@ -25,11 +25,11 @@ our @BUILDSYSTEMS = (
 	"makefile",
 	"python_distutils",
 	(compat(7) ? "perl_build" : ()),
-	"cmake",
+	"cmake+makefile",
 	"ant",
 	"qmake",
 	"qmake_qt4",
-	"meson",
+	"meson+ninja",
 	"ninja",
 );
 
@@ -45,13 +45,21 @@ my $opt_list;
 my $opt_parallel;
 
 sub create_buildsystem_instance {
-	my ($system, $required, %bsopts) = @_;
-	my $module = "Debian::Debhelper::Buildsystem::$system";
+	my ($full_name, $required, %bsopts) = @_;
+	my @parts = split(m{[+]}, $full_name, 2);
+	my $name = $parts[0];
+	my $module = "Debian::Debhelper::Buildsystem::$name";
+	if (@parts > 1) {
+		if (exists($bsopts{'targetbuildsystem'})) {
+			error("Conflicting target buildsystem for ${name} (load as ${full_name}, but target configured in bsopts)");
+		}
+		$bsopts{'targetbuildsystem'} = $parts[1];
+	}
 
 	eval "use $module";
 	if ($@) {
 		return if not $required;
-		error("unable to load build system class '$system': $@");
+		error("unable to load build system class '$name': $@");
 	}
 
 	if (!exists $bsopts{builddir} && defined $opt_builddir) {
@@ -73,9 +81,15 @@ sub autoselect_buildsystem {
 	my $selected_level = 0;
 
 	foreach my $inst (@_) {
-		# Only derived (i.e. more specific) build system can be
-		# considered beyond the currently selected one.
-		next if defined $selected && !$inst->isa(ref $selected);
+		# Only  more specific build system can be considered beyond
+		# the currently selected one.
+		if (defined($selected)) {
+			my $ok = $inst->isa(ref($selected)) ? 1 : 0;
+			if (not $ok and $inst->IS_GENERATOR_BUILD_SYSTEM) {
+				$ok = 1 if $inst->{targetbuildsystem}->NAME eq $selected->NAME;
+			}
+			next if not $ok;
+		}
 
 		# If the build system says it is auto-buildable at the current
 		# step and it can provide more specific information about its
@@ -121,7 +135,8 @@ sub load_buildsystem {
 
 sub load_all_buildsystems {
 	my $incs=shift || \@INC;
-	my (%buildsystems, @buildsystems);
+	my %opts = @_;
+	my (%buildsystems, %genbuildsystems, @buildsystems);
 
 	foreach my $inc (@$incs) {
 		my $path = File::Spec->catdir($inc, "Debian/Debhelper/Buildsystem");
@@ -129,8 +144,19 @@ sub load_all_buildsystems {
 			foreach my $module_path (glob "$path/*.pm") {
 				my $name = basename($module_path);
 				$name =~ s/\.pm$//;
-				next if exists $buildsystems{$name};
-				$buildsystems{$name} = create_buildsystem_instance($name, 1, @_);
+				next if exists $buildsystems{$name} or exists $genbuildsystems{$name};
+				my $system = create_buildsystem_instance($name, 1, %opts);
+				if ($system->IS_GENERATOR_BUILD_SYSTEM) {
+					$genbuildsystems{$name} = 1;
+					for my $target_name ($system->SUPPORTED_TARGET_BUILD_SYSTEMS) {
+						my $full_name = "${name}+${target_name}";
+						my $full_system = create_buildsystem_instance($name, 1, %opts,
+							'targetbuildsystem' => $target_name);
+						$buildsystems{$full_name} = $full_system;
+					}
+				} else {
+					$buildsystems{$name} = $system;
+				}
 			}
 		}
 	}
@@ -209,22 +235,31 @@ sub buildsystems_list {
 	my @buildsystems = load_all_buildsystems();
 	my %auto_selectable = map { $_ => 1 } @THIRD_PARTY_BUILDSYSTEMS;
 	my $auto = autoselect_buildsystem($step, grep { ! $_->{thirdparty} || $auto_selectable{$_->NAME} } @buildsystems);
-	my $specified;
+	my $specified_text;
+
+	if ($opt_buildsys) {
+		for my $inst (@buildsystems) {
+			my $full_name = $inst->NAME;
+			if ($full_name eq $opt_buildsys) {
+				$specified_text = $full_name;
+			} elsif ($inst->IS_GENERATOR_BUILD_SYSTEM and ref($inst)->NAME eq $opt_buildsys) {
+				my $default = $inst->DEFAULT_TARGET_BUILD_SYSTEM;
+				$specified_text = "${opt_buildsys}+${default} (default for ${opt_buildsys})";
+			}
+		}
+	}
 
 	# List build systems (including auto and specified status)
 	foreach my $inst (@buildsystems) {
-		if (! defined $specified && defined $opt_buildsys && $opt_buildsys eq $inst->NAME()) {
-			$specified = $inst;
-		}
-		printf("%-20s %s", $inst->NAME(), $inst->DESCRIPTION());
+		printf("%-20s %s", $inst->NAME(), $inst->FULL_DESCRIPTION());
 		print " [3rd party]" if $inst->{thirdparty};
 		print "\n";
 	}
 	print "\n";
 	print "Auto-selected: ", $auto->NAME(), "\n" if defined $auto;
-	print "Specified: ", $specified->NAME(), "\n" if defined $specified;
+	print "Specified: ", $specified_text, "\n" if defined $specified_text;
 	print "No system auto-selected or specified\n"
-		if ! defined $auto && ! defined $specified;
+		if ! defined $auto && ! defined $specified_text;
 }
 
 sub buildsystems_do {
diff --git a/t/buildsystems/03-bs-auto-buildable.t b/t/buildsystems/03-bs-auto-buildable.t
index c1229dc9..082e38bf 100755
--- a/t/buildsystems/03-bs-auto-buildable.t
+++ b/t/buildsystems/03-bs-auto-buildable.t
@@ -90,7 +90,7 @@ sub run_auto_buildable_tests {
 	rm_files("${sourcedir}/configure");
 
 	create_empty_file("${sourcedir}/CMakeLists.txt");
-	test_check_auto_buildable($bs{cmake}, "CMakeLists.txt", { configure => 1, clean => 1 });
+	test_check_auto_buildable($bs{'cmake+makefile'}, "CMakeLists.txt", { configure => 1, clean => 1 });
 	rm_files("${sourcedir}/CMakeLists.txt");
 
 	create_empty_file("${sourcedir}/Makefile.PL");
@@ -98,7 +98,7 @@ sub run_auto_buildable_tests {
 	rm_files("${sourcedir}/Makefile.PL");
 
 	create_empty_file("${sourcedir}/meson.build");
-	test_check_auto_buildable($bs{meson}, "meson.build", { configure => 1, clean => 1 });
+	test_check_auto_buildable($bs{'meson+ninja'}, "meson.build", { configure => 1, clean => 1 });
 	# Leave meson.build
 
 	create_empty_file("${builddir}/build.ninja");
@@ -106,7 +106,7 @@ sub run_auto_buildable_tests {
 	# Leave ninja.build
 
 	# Meson + ninja
-	test_check_auto_buildable($bs{meson}, "meson.build+build.ninja", { configure => 1, build => 1, clean => 1, install => 1, test => 1 });
+	test_check_auto_buildable($bs{'meson+ninja'}, "meson.build+build.ninja", { configure => 1, build => 1, clean => 1, install => 1, test => 1 });
 	rm_files("${sourcedir}/meson.build", "${builddir}/build.ninja");
 
 	# With Makefile
@@ -120,9 +120,9 @@ sub run_auto_buildable_tests {
 
 	# ... +cmake
 	create_empty_file("${sourcedir}/CMakeLists.txt");
-	test_check_auto_buildable($bs{cmake}, "CMakeLists.txt+Makefile", 1);
+	test_check_auto_buildable($bs{'cmake+makefile'}, "CMakeLists.txt+Makefile", 1);
 	create_empty_file("$builddir/CMakeCache.txt"); # strong evidence that cmake was run
-	test_check_auto_buildable($bs{cmake}, "CMakeCache.txt+Makefile", 2);
+	test_check_auto_buildable($bs{'cmake+makefile'}, "CMakeCache.txt+Makefile", 2);
 	rm_files("${builddir}/Makefile", "${sourcedir}/CMakeLists.txt");
 
 	# Makefile.PL forces in-source
@@ -189,7 +189,7 @@ sub run_autoselection_tests {
 	# CMake
 	create_empty_file("${sourcedir}/CMakeLists.txt");
 	test_autoselection("cmake without CMakeCache.txt",
-					   { configure => "cmake", build => "makefile",
+					   { configure => "cmake+makefile", build => "makefile",
 						 test => "makefile", install => "makefile",
 						 clean => "makefile"
 					   },
@@ -201,7 +201,7 @@ sub run_autoselection_tests {
 
 	create_empty_file("${sourcedir}/CMakeLists.txt");
 	test_autoselection("cmake with CMakeCache.txt",
-					   "cmake",
+					   "cmake+makefile",
 					   %options,
 					   code_configure => sub {
 						   create_empty_file("$builddir/Makefile");
-- 
2.16.3

