Re: [Reproducible-builds] Bug#790868: sbuild: Please allow sbuild to use a deterministic build path to build packages

2015-07-03 Thread Maria Valentina Marin
Hi,

On Thu, 2 Jul 2015 16:06:28 +0200 =?iso-8859-1?B?Suly6W15?= Bobbio
 wrote:
> How about using a safeguard here instead of warning users? I'd rather
> not have to answer “it was written” to very angry users who just have
> lost their home directory…
> 
> Maybe fail when the build path that has been set exists and is not
> empty?

Thank you for the idea, I have implemented it in the attached version of
my patch.

Cheers,
akira
>From a540f45de60ffd558a4fea92951a26087bd5694b Mon Sep 17 00:00:00 2001
From: akira 
Date: Mon, 29 Jun 2015 23:53:43 +0200
Subject: [PATCH] Add option to specify a custom build path

Options --build-path and BUILD_PATH allow to specify a custom build
path. This new option is useful for the reproducible builds project [1]
because some packages save the build directory and sbuild by default
uses random build directories to build packages. The TODO note in file
Build.pm was fixed with this patch.

[1] https://wiki.debian.org/ReproducibleBuilds
---
 debian/changelog  |  8 
 lib/Sbuild/Build.pm   | 54 ++-
 lib/Sbuild/Conf.pm|  7 +++
 lib/Sbuild/Options.pm |  3 +++
 man/sbuild.1.in   |  4 
 5 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index a49f5c2..aeca174 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+sbuild (0.65.2-1.1) unstable; urgency=medium
+
+  * Non-maintainer upload.
+  * Add options --build-path and BUILD_PATH which allow to specify a custom
+build path.
+
+ -- akira   Fri, 03 Jul 2015 09:44:31 +0200
+
 sbuild (0.65.2-1) unstable; urgency=medium
 
   * Team upload.
diff --git a/lib/Sbuild/Build.pm b/lib/Sbuild/Build.pm
index cda5cf3..4ff1c9d 100644
--- a/lib/Sbuild/Build.pm
+++ b/lib/Sbuild/Build.pm
@@ -31,6 +31,7 @@ use Errno qw(:POSIX);
 use Fcntl;
 use File::Basename qw(basename dirname);
 use File::Temp qw(tempdir);
+use File::Path qw(make_path);
 use FileHandle;
 use File::Copy qw(); # copy is already exported from Sbuild, so don't export
 		 # anything.
@@ -393,11 +394,54 @@ sub run_chroot_session {
 	}
 
 	$self->set('Chroot Dir', $session->get('Location'));
-	# TODO: Don't hack the build location in; add a means to customise
-	# the chroot directly.  i.e. allow changing of /build location.
-	$self->set('Chroot Build Dir',
-		   tempdir($self->get('Package') . '-XX',
-			   DIR =>  $session->get('Location') . "/build"));
+	if (defined($self->get_conf('BUILD_PATH')) && $self->get_conf('BUILD_PATH')) {
+		my $build_path = $session->get('Location') . "/" . $self->get_conf('BUILD_PATH');
+		$self->set('Chroot Build Dir', $build_path);
+		if (!(-d "$build_path")) {
+			make_path($build_path, {error => \my $err} );
+			if (@$err) {
+my $error;
+for my $diag (@$err) {
+	my ($file, $message) = %$diag;
+	$error .= "mkdir $file: $message\n";
+}
+Sbuild::Exception::Build->throw(
+	error => $error,
+	failstage => "create-session");
+			}
+		} else {
+			my $empty = isEmpty($build_path);
+			if ($empty == 1) {
+Sbuild::Exception::Build->throw(
+	error => "Buildpath: " . $build_path . " is not empty",
+	failstage => "create-session");
+			}
+			elsif ($empty == 2) {
+Sbuild::Exception::Build->throw(
+	error => "Buildpath: " . $build_path . " cannot be read. Insufficient permissions?",
+	failstage => "create-session");
+			}
+		}
+	} else {
+		$self->set('Chroot Build Dir',
+			   tempdir($self->get('Package') . '-XX',
+   DIR =>  $session->get('Location') . "/build"));
+	}
+	sub isEmpty{
+		my ($dirpath) = @_;
+		my $file;
+		if ( opendir my $dfh, $dirpath){
+			while (defined($file = readdir $dfh)){
+next if $file eq '.' or $file eq '..';
+closedir $dfh;
+return 1;
+			}
+			closedir $dfh;
+			return 0;
+		}
+		return 2;
+	}
+
 
 	$self->set('Build Dir', $session->strip_chroot_path($self->get('Chroot Build Dir')));
 
diff --git a/lib/Sbuild/Conf.pm b/lib/Sbuild/Conf.pm
index af89f60..c2132fa 100644
--- a/lib/Sbuild/Conf.pm
+++ b/lib/Sbuild/Conf.pm
@@ -613,6 +613,13 @@ sub setup ($) {
 	CHECK => $validate_directory,
 	HELP => 'This option is deprecated.  Directory for chroot symlinks and sbuild logs.  Defaults to the current directory if unspecified.  It is used as the location of chroot symlinks (obsolete) and for current build log symlinks and some build logs.  There is no default; if unset, it defaults to the current working directory.  $HOME/build is another common configuration.'
 	},
+	'BUILD_PATH'=> {
+	TYPE => 'STRING',
+	VARNAME => 'build_path',
+	GROUP => 'Build options',
+	DEFAULT => undef,
+	HELP => 'By default the package is built in a path of the following format /build/packagename-XX/packagename-version/ where XX is a random ascii string. This option allows one to specify a custom path where the package is built inside the chroot. Notice that the sbuild user in the chroot must have per

Re: [Reproducible-builds] Bug#790868: sbuild: Please allow sbuild to use a deterministic build path to build packages

2015-07-02 Thread Jérémy Bobbio
Hi!

Maria Valentina Marin:
> The attached patch allows users to specify a deterministic build path by
> using the new command line option --build-path or the configuration
> variable $build_path in the ~/.sbuilrc.

I don't know enough of sbuild to comment on the patch, but:

> +.BR \-\-build\-path=\fIstring\fP
> +By default the package is built in a path of the following format
> +/build/packagename-XX/packagename-version/ where XX is a random ascii
> +string. This option allows one to specify a custom path where the package is
> +built inside the chroot. Notice that the sbuild user in the chroot must have
> +permissions to create the path. Common writable locations are subdirectories 
> of
> +/tmp or /build. Caution: the last component of the path will be RECURSIVELY
> +REMOVED after the build is finished. So NEVER specify a build path like /tmp 
> or
> +/home/user because sbuild mounts /tmp and /home from the host into the 
> chroot!
> +Example: If your build path is /tmp/foo then the directory foo and all its
> +content will be removed after the build is finished. If you are running
> +multiple sbuild instances with the same build path in parallel for the same
> +package, make sure that your build path is not in a directory commonly 
> mounted
> +by all sbuild instances (like /tmp or /home). In that case, use for example
> +/build instead. Otherwise, your builds will probably fail or contain wrong
> +content.

How about using a safeguard here instead of warning users? I'd rather
not have to answer “it was written” to very angry users who just have
lost their home directory…

Maybe fail when the build path that has been set exists and is not
empty?

-- 
Lunar.''`. 
lu...@debian.org: :Ⓐ  :  # apt-get install anarchism
`. `'` 
  `-   


signature.asc
Description: Digital signature
___
Reproducible-builds mailing list
Reproducible-builds@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/reproducible-builds

[Reproducible-builds] Bug#790868: sbuild: Please allow sbuild to use a deterministic build path to build packages

2015-07-02 Thread Maria Valentina Marin
Source: sbuild
Version: 0.65.2-1
Severity: wishlist
Tags: patch
User: reproducible-builds@lists.alioth.debian.org
Usertags: toolchain buildpath

Hi,

While working on the "reproducible builds" effort [1], we have noticed
that sbuild by default uses random build directories to build packages.
This makes all packages which save the information of the build
directory unreproducible.

The attached patch allows users to specify a deterministic build path by
using the new command line option --build-path or the configuration
variable $build_path in the ~/.sbuilrc.

The new option build-path might be confused with the deprecated option
build-dir. Is there any better way to name the new option? Or is it not
confusing?

 [1]: https://wiki.debian.org/ReproducibleBuilds
>From 1538db1cbc7442858ee391cf95ccdbdc7d44c543 Mon Sep 17 00:00:00 2001
From: akira 
Date: Mon, 29 Jun 2015 23:53:43 +0200
Subject: [PATCH] Add option to specify a custom build path

Options --build-path and BUILD_PATH allow to specify a custom build
path. This new option is useful for the reproducible builds project [1]
because some packages save the build directory and sbuild by default
uses random build directories to build packages. The TODO note in file
Build.pm was fixed with this patch.

[1] https://wiki.debian.org/ReproducibleBuilds
---
 debian/changelog  |  8 
 lib/Sbuild/Build.pm   | 27 ++-
 lib/Sbuild/Conf.pm|  7 +++
 lib/Sbuild/Options.pm |  3 +++
 man/sbuild.1.in   | 18 ++
 5 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index a49f5c2..09cd5e9 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+sbuild (0.65.2-1.1) unstable; urgency=medium
+
+  * Non-maintainer upload.
+  * Add options --build-path and BUILD_PATH which allow to specify a custom
+build path.
+
+ -- akira   Thu, 02 Jul 2015 12:47:54 +0200
+
 sbuild (0.65.2-1) unstable; urgency=medium
 
   * Team upload.
diff --git a/lib/Sbuild/Build.pm b/lib/Sbuild/Build.pm
index cda5cf3..806eb20 100644
--- a/lib/Sbuild/Build.pm
+++ b/lib/Sbuild/Build.pm
@@ -31,6 +31,7 @@ use Errno qw(:POSIX);
 use Fcntl;
 use File::Basename qw(basename dirname);
 use File::Temp qw(tempdir);
+use File::Path qw(make_path);
 use FileHandle;
 use File::Copy qw(); # copy is already exported from Sbuild, so don't export
 		 # anything.
@@ -393,11 +394,27 @@ sub run_chroot_session {
 	}
 
 	$self->set('Chroot Dir', $session->get('Location'));
-	# TODO: Don't hack the build location in; add a means to customise
-	# the chroot directly.  i.e. allow changing of /build location.
-	$self->set('Chroot Build Dir',
-		   tempdir($self->get('Package') . '-XX',
-			   DIR =>  $session->get('Location') . "/build"));
+	if (defined($self->get_conf('BUILD_PATH')) && $self->get_conf('BUILD_PATH')) {
+		my $build_path = $session->get('Location') . "/" . $self->get_conf('BUILD_PATH');
+		$self->set('Chroot Build Dir', $build_path);
+		if (!(-d "$build_path")) {
+			make_path($build_path, {error => \my $err} );
+			if (@$err) {
+my $error;
+for my $diag (@$err) {
+	my ($file, $message) = %$diag;
+	$error .= "mkdir $file: $message\n";
+}
+Sbuild::Exception::Build->throw(
+	error => $error,
+	failstage => "create-session");
+			}
+		}
+	} else {
+		$self->set('Chroot Build Dir',
+			   tempdir($self->get('Package') . '-XX',
+   DIR =>  $session->get('Location') . "/build"));
+	}
 
 	$self->set('Build Dir', $session->strip_chroot_path($self->get('Chroot Build Dir')));
 
diff --git a/lib/Sbuild/Conf.pm b/lib/Sbuild/Conf.pm
index af89f60..2993647 100644
--- a/lib/Sbuild/Conf.pm
+++ b/lib/Sbuild/Conf.pm
@@ -613,6 +613,13 @@ sub setup ($) {
 	CHECK => $validate_directory,
 	HELP => 'This option is deprecated.  Directory for chroot symlinks and sbuild logs.  Defaults to the current directory if unspecified.  It is used as the location of chroot symlinks (obsolete) and for current build log symlinks and some build logs.  There is no default; if unset, it defaults to the current working directory.  $HOME/build is another common configuration.'
 	},
+	'BUILD_PATH'=> {
+	TYPE => 'STRING',
+	VARNAME => 'build_path',
+	GROUP => 'Build options',
+	DEFAULT => undef,
+	HELP => 'By default the package is built in a path of the following format /build/packagename-XX/packagename-version/ where XX is a random ascii string. This option allows one to specify a custom path where the package is built inside the chroot. Notice that the sbuild user in the chroot must have permissions to create the path. Common writable locations are subdirectories of /tmp or /build. Caution: the last component of the path will be RECURSIVELY REMOVED after the build is finished. So NEVER specify a build path like /tmp or /home/user because sbuild mounts /tmp and /home from the host into the chroot! Example: If your build path is /tmp/foo then the direct