Bug#601590: [debhelper] debhelper: cmake buildsystem does not clean builddir if cancelled during configure

2011-07-21 Thread gregory hainaut
On Thu, 21 Jul 2011 14:53:09 +0300
Modestas Vainius  wrote:

> Well, ok. In either case, the second hunk of your patch is redundant.
> That "clean" code is already implicit due to inheritance from
> makefile buildsystem.
> 
> However, this bug is not about cmake actually. I think it applies to
> autoconf (or any other buildsystem inheriting from makefile) as well.
> So ideally it should be solved at makefile buildsystem level. Maybe
> something like an attached patch.
> 

Indeed a better fix. Work well in my testcase.

Cheers
Gregory


signature.asc
Description: PGP signature


Bug#601590: [debhelper] debhelper: cmake buildsystem does not clean builddir if cancelled during configure

2011-07-21 Thread Modestas Vainius
Hello,

On ketvirtadienis 21 Liepa 2011 12:23:29 Gregory Hainaut wrote:
> Well in this corner-case situation the idea was to remove the buildir, not
> to call `make clean`. Actually the current code never call `make clean`
> because rmdir_builddir nukes the builddir that contains the makefile.
> Previous patch will not change this behavior. In my opinion for cmake
> rmdir_builddir is enough but I wanted to keep change as small as possible.
> 
> The current use case:
> 1/ run dh_auto_configure but interrupt before the end (error or manually
> interrupt). It will create a builddir but without makefile. The builddir
> will contains various files (binary and text).
> ...
> 2/ then do dh_auto_clean (rebuild of package for example). For the moment
> the builddir is left untouched actually dh_auto_build will do nothing
> because it call the default clean function (from memory, inside
> BuildSystem.pm). Note, if dh_auto_configure finished sucessfully, the
> buildir is properly remove during dh_auto_clean (this time clean comes from
> makefile.pm).

Well, ok. In either case, the second hunk of your patch is redundant. That 
"clean" code is already implicit due to inheritance from makefile buildsystem.

However, this bug is not about cmake actually. I think it applies to autoconf 
(or any other buildsystem inheriting from makefile) as well. So ideally it 
should be solved at makefile buildsystem level. Maybe something like an 
attached patch.

-- 
Modestas Vainius 
From d04fc959415b7b8f27636b8bab1414de69339a10 Mon Sep 17 00:00:00 2001
From: Modestas Vainius 
Date: Thu, 21 Jul 2011 14:48:57 +0300
Subject: [PATCH] makefile.pm: remove build directory even if Makefile does
 not exist yet.

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 Makefiles.
---
 Debian/Debhelper/Buildsystem/makefile.pm |   22 +-
 t/buildsystems/buildsystem_tests |2 +-
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/Debian/Debhelper/Buildsystem/makefile.pm b/Debian/Debhelper/Buildsystem/makefile.pm
index d4b68e4..f38387b 100644
--- a/Debian/Debhelper/Buildsystem/makefile.pm
+++ b/Debian/Debhelper/Buildsystem/makefile.pm
@@ -69,11 +69,23 @@ sub check_auto_buildable {
 	my $this=shift;
 	my ($step) = @_;
 
-	# This is always called in the source directory, but generally
-	# Makefiles are created (or live) in the the build directory.
-	return (-e $this->get_buildpath("Makefile") ||
-	-e $this->get_buildpath("makefile") ||
-	-e $this->get_buildpath("GNUmakefile")) ? 1 : 0;
+	if (-e $this->get_buildpath("Makefile") ||
+	-e $this->get_buildpath("makefile") ||
+	-e $this->get_buildpath("GNUmakefile"))
+	{
+		# This is always called in the source directory, but generally
+		# Makefiles are created (or live) in the 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 Makefiles.
+		return 1;
+	}
+	return 0;
 }
 
 sub build {
diff --git a/t/buildsystems/buildsystem_tests b/t/buildsystems/buildsystem_tests
index c6f23ca..3b45ac5 100755
--- a/t/buildsystems/buildsystem_tests
+++ b/t/buildsystems/buildsystem_tests
@@ -256,7 +256,7 @@ touch "$tmpdir/configure", 0755;
 test_check_auto_buildable($bs{autoconf}, "configure", { configure => 1 });
 
 touch "$tmpdir/CMakeLists.txt";
-test_check_auto_buildable($bs{cmake}, "CMakeLists.txt", { configure => 1 });
+test_check_auto_buildable($bs{cmake}, "CMakeLists.txt", { configure => 1, clean => 1 });
 
 touch "$tmpdir/Makefile.PL";
 test_check_auto_buildable($bs{perl_makemaker}, "Makefile.PL", { configure => 1 });
-- 
1.7.5.4



signature.asc
Description: This is a digitally signed message part.


Bug#601590: [debhelper] debhelper: cmake buildsystem does not clean builddir if cancelled during configure

2011-07-21 Thread Gregory Hainaut
On Thu, Jul 21, 2011 at 8:35 AM, Modestas Vainius  wrote:

> Hello,
>
> On ketvirtadienis 21 Liepa 2011 00:22:20 gregory hainaut wrote:
> > Package: debhelper
> > Version: 8.9.1
> > Tags: patch
> >
> > Hello all,
> >
> > I'm facing the same issue for a personal project.
> > In the correct run, debhelper detect the clean phase as a makefile
> > build. However in our case the Makefile did not exist. The logic to
> > detect the cmake build is only enabled during the configure phase. So
> > debhelper fails to detect the build system.
> >
> > I attach a small patch to change the behavior. I do a quick test and
> > seem to be fine.
> > 1/ It enable the detection of the cmake build system during the clean
> > phase.
> > 2/ I copy past the clean function from makefile.pm to cmake.pm.
>
> It makes no sense to call `make clean` if Makefile does not exist. Probably
> you didn't pass source or build directory properly.
>
> --
> Modestas Vainius 
>

Hello,

Well in this corner-case situation the idea was to remove the buildir, not
to call `make clean`. Actually the current code never call `make clean`
because rmdir_builddir nukes the builddir that contains the makefile.
Previous patch will not change this behavior. In my opinion for cmake
rmdir_builddir is enough but I wanted to keep change as small as possible.

The current use case:
1/ run dh_auto_configure but interrupt before the end (error or manually
interrupt). It will create a builddir but without makefile. The builddir
will contains various files (binary and text).
...
2/ then do dh_auto_clean (rebuild of package for example). For the moment
the builddir is left untouched actually dh_auto_build will do nothing
because it call the default clean function (from memory, inside
BuildSystem.pm). Note, if dh_auto_configure finished sucessfully, the
buildir is properly remove during dh_auto_clean (this time clean comes from
makefile.pm).

Best regards,
Gregory


Bug#601590: [debhelper] debhelper: cmake buildsystem does not clean builddir if cancelled during configure

2011-07-20 Thread Modestas Vainius
Hello,

On ketvirtadienis 21 Liepa 2011 00:22:20 gregory hainaut wrote:
> Package: debhelper
> Version: 8.9.1
> Tags: patch
> 
> Hello all,
> 
> I'm facing the same issue for a personal project.
> In the correct run, debhelper detect the clean phase as a makefile
> build. However in our case the Makefile did not exist. The logic to
> detect the cmake build is only enabled during the configure phase. So
> debhelper fails to detect the build system.
> 
> I attach a small patch to change the behavior. I do a quick test and
> seem to be fine.
> 1/ It enable the detection of the cmake build system during the clean
> phase.
> 2/ I copy past the clean function from makefile.pm to cmake.pm.

It makes no sense to call `make clean` if Makefile does not exist. Probably 
you didn't pass source or build directory properly.

-- 
Modestas Vainius 


signature.asc
Description: This is a digitally signed message part.


Bug#601590: [debhelper] debhelper: cmake buildsystem does not clean builddir if cancelled during configure

2011-07-20 Thread gregory hainaut
Package: debhelper
Version: 8.9.1
Tags: patch

Hello all,

I'm facing the same issue for a personal project.
In the correct run, debhelper detect the clean phase as a makefile
build. However in our case the Makefile did not exist. The logic to
detect the cmake build is only enabled during the configure phase. So
debhelper fails to detect the build system.

I attach a small patch to change the behavior. I do a quick test and
seem to be fine.
1/ It enable the detection of the cmake build system during the clean
phase.
2/ I copy past the clean function from makefile.pm to cmake.pm.

Hope it help.

Best regards,
Gregory--- /usr/share/perl5/Debian/Debhelper/Buildsystem/cmake.pm	2011-07-20 23:11:12.0 +0200
+++ /root/cmake.pm	2011-07-20 23:11:05.0 +0200
@@ -17,7 +17,7 @@
 	my $this=shift;
 	my ($step)=@_;
 	if (-e $this->get_sourcepath("CMakeLists.txt")) {
-		my $ret = ($step eq "configure" && 1) ||
+		my $ret = ($step eq "configure" && 1) || ($step eq "clean" && 1) ||
 		  $this->SUPER::check_auto_buildable(@_);
 		# Existence of CMakeCache.txt indicates cmake has already
 		# been used by a prior build step, so should be used
@@ -62,4 +62,11 @@
 	return $this->SUPER::test(@_);
 }
 
+sub clean {
+	my $this=shift;
+	if (!$this->rmdir_builddir()) {
+		$this->make_first_existing_target(['distclean', 'realclean', 'clean'], @_);
+	}
+}
+
 1