Hello, On pirmadienis 16 Lapkritis 2009 21:51:17 Joey Hess wrote: > If a packaxge has both a CMakeLists.txt and a GNUmakefile, > dh_auto_configure will currently use the makefile build system, do > nothing, and futher steps will build and install using the GNUmakefile.
No, currently makefile.pm build system *passes through* in configure step. Therefore, if there are CMakeLists.txt and GNUmakefile, dh_auto_configure will auto-select cmake build system. This is valid because your initial dh_auto_configure (7.0.x) had "-x configure" check rather than "-f Makefile" like the rest of steps. Currently "-x configure" gets caught in autotools build system at configure step. > With this change, dh_auto_configure will: > > 1. Find the first auto-buildable build system, which is still makefile. > 2. Hmm, based on your description above, it would find cmake, and use > it, which is a behavior change. However, the code actually skips > doing so, because there is no build step before configure; > makefile is still used. This patch has absolutely no influence on configure step because there is no build steps before it hence no candidates can be generated from them. So the code path remains 100% the same here. Actually, that's the main safety enhancement over my first (and rather bad) attempt to solve this bug. A build system MUST have been auto-selected by a previous step to become a candidate for current one (a variant of your idea to log previously used build system). > Then dh_auto_build will: > > 1. Find the first auto-buildable build system, which is still makefile. Yes. > 2. Find the more specific build system cmake, and use it. > > So it seems this only works because cmake.pm currently inherits the > build step from makefile. Yes, because dh_auto_configure would use cmake AND because cmake inherits from makefile. Both are key concepts here. Read below. > Slightly different story for dh_auto_test: > > 1. First is still makefile. > 2. More specific is again cmake; which is used. > > So, this only works because cmake.pm's test method calls SUPER and > does not do anything that is really cmake specific. That's exactly the point. Answer yourself a question if you want `dh_auto_*` and `dh_auto_* --buildsystem=cmake` to behave differently for a cmake-only package. I guess the answer is no. But even now if we added anything to cmake.pm which was makefile.pm incompatible, that would inevitably happen... Therefore, cmake.pm makes a contract with makefile.pm "not to break simple makefile packages" by inheriting from it. A general rule of OOP programming is that a subclass can only (re)implement or extend the described behaviour of the base class. In other words, cmake.pm (or anything inheriting from makefile.pm for that matter) MUST be able to build simple Makefile based packages (in our case, at least when build system is not manually enforced). Since you have a complete control over auto-buildable build systems in debhelper, the only thing you need to do is to make sure that all build systems deeper in the inheritance tree are able to auto-build packages of their bases, i.e. they do not break their contract. > If cmake had a "cmake-test" program that the test method ran, it could > in this situation break, because the configure step did not use cmake. As I said above, dh_auto_configure chooses cmake at the moment. If it didn't, cmake build system would not be auto-buildable with current debhelper in some cases. However, if e.g. dh_auto_configure was overriden as noop in debian/rules, your scenario could trigger breakage. Therefore, new cmake::test() code, or better yet cmake::check_auto_buildable(test), must determine if cmake::configure() was actually executed before doing anything cmake specific and fallback to behaviour of its base class if it wasn't. I will enhance my #555807 patch to cover this (even though the actualy test() change in there is harmless for simple Makefile based packages). > I think this works for cmake only through a certian measure of luck. > But, it does currently work. > > So the risk of making this change is that we have to beware the above > scenario when making changes to cmake.pm, or when adding future build > systems after cmake that similarly derive from makefile. Fortunately, you have a complete control over this process and you can enforce these "abide contract" rules for all new auto-buildable build systems and enhancements of old ones. It might be a bit a burden, but in my opinion it is worth it. This patch adds new possibilities and consistency to auto selection process, it helps to keep stuff properly encapsulated. On the other hand, currently #555807 patch could be applied to makefile.pm as it is and it would not cause any breakage. But we want everything cmake specific in cmake.pm, don't we? I also attach (improved_bs_autoselection2.diff) even more optimized version of the previous improved_bs_autoselection.diff patch. Functionality-wise, they are identical, just this one does the same with less code and without pointless redundant check_auto_buildable() calls. -- Modestas Vainius <modes...@vainius.eu>
From: Modestas Vainius <modes...@vainius.eu>
Subject: [PATCH] Improve build system auto-selection process
This patch replaces current build system auto-selection process with the
following:
1) Find the first (in @BUILDSYSTEMS order) auto-buildable build system in
the current build step. If found, it becomes the base one. Otherwise,
auto-selection process fails.
2) Look for a more specific (i.e. deeper in the inheritance tree) build system
that is both auto-selectable in any build step coming before the current one
and auto-buildable in current one. If such a build system is found, auto-select
it for current step as well. Otherwise, auto-select the base build system found
in 1). Auto-selection process succeeds.
The patch implements optimized version of the algorithm above.
Signed-off-by: Modestas Vainius <modes...@vainius.eu>
---
Debian/Debhelper/Dh_Buildsystems.pm | 42 +++++++++++++++++++++++++++++++---
t/buildsystems/buildsystem_tests | 4 +--
2 files changed, 39 insertions(+), 7 deletions(-)
diff --git a/Debian/Debhelper/Dh_Buildsystems.pm b/Debian/Debhelper/Dh_Buildsystems.pm
index a9a13a2..747604e 100644
--- a/Debian/Debhelper/Dh_Buildsystems.pm
+++ b/Debian/Debhelper/Dh_Buildsystems.pm
@@ -14,6 +14,8 @@ use File::Spec;
use base 'Exporter';
our @EXPORT=qw(&buildsystems_init &buildsystems_do &load_buildsystem &load_all_buildsystems);
+use constant BUILD_STEPS => qw(configure build test install clean);
+
# Historical order must be kept for backwards compatibility. New
# build systems MUST be added to the END of the list.
our @BUILDSYSTEMS = (
@@ -66,14 +68,46 @@ sub load_buildsystem {
}
else {
# Try to determine build system automatically
- for $system (@BUILDSYSTEMS) {
+ my @buildsyste...@buildsystems;
+ my $base;
+
+ # First of all, determine a base build system for this $step. Take non
+ # auto-buildable build systems out of further consideration.
+ while ($system = shift(@buildsystems)) {
my $inst = create_buildsystem_instance($system, @_);
if ($inst->check_auto_buildable($step)) {
- return $inst;
+ $base = $inst;
+ last;
+ }
+ }
+
+ # Now look for a more specific build system that is auto-buildable in
+ # both any previous and current steps.
+ if ($base && @buildsystems) {
+ for my $prevstep (BUILD_STEPS) {
+ last if ($prevstep eq $step);
+
+ # Find the first build system that is auto-buildable in $prevstep.
+ for $system (@buildsystems) {
+ next unless defined $system;
+ $system = create_buildsystem_instance($system, @_) unless ref $system;
+ if ($system->check_auto_buildable($prevstep)) {
+ # If a candidate build system is both more specific than the
+ # base build system, and auto-buildable in current step, it's
+ # the one we are looking for.
+ if ($system->isa(ref $base) && $system->check_auto_buildable($step)) {
+ return $system;
+ }
+ else {
+ # It's pointless to consider this build system once again
+ $system = undef;
+ }
+ }
+ }
}
}
+ return $base;
}
- return;
}
sub load_all_buildsystems {
@@ -189,7 +223,7 @@ sub buildsystems_do {
$step =~ s/^dh_auto_//;
}
- if (grep(/^\Q$step\E$/, qw{configure build test install clean}) == 0) {
+ if (grep(/^\Q$step\E$/, BUILD_STEPS) == 0) {
error("unrecognized build step: " . $step);
}
diff --git a/t/buildsystems/buildsystem_tests b/t/buildsystems/buildsystem_tests
index 0465a93..fa6891e 100755
--- a/t/buildsystems/buildsystem_tests
+++ b/t/buildsystems/buildsystem_tests
@@ -347,9 +347,7 @@ cleandir $tmpdir;
# CMake
touch "$tmpdir/CMakeLists.txt";
touch "$builddir/Makefile";
-test_autoselection("cmake",
- { configure => "cmake", build => "makefile",
- test => "makefile", install => "makefile", clean => "makefile" }, %tmp);
+test_autoselection("cmake", "cmake", %tmp);
cleandir $tmpdir;
### Test Buildsystem::rmdir_builddir()
--
tg: (1679f2e..) patch/improved_bs_autoselection (depends on: master)
signature.asc
Description: This is a digitally signed message part.
