Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-08-05 Thread Michael Stapelberg 
control: block -1 by 870260

Thanks for the thorough review. It took me quite a bit to address all these
comments :).

Find the updated patch attached, and answers inline:

On Wed, Aug 2, 2017 at 8:11 AM, Johannes Schauer  wrote:

> Quoting Michael Stapelberg (2017-08-01 23:15:04)
> > Alright! Patch attached and provided inline, for your convenience:
>
> Cool!
>
> > +if (!defined($ENV{SUDO_USER})) {
> > +die "Please run sudo $0";
> > +}
>
> Should you not rather check the UID instead?
>

Why? We need SUDO_USER later, for the adduser command.


>
> > +system("adduser", "--quiet", "--", $ENV{SUDO_USER}, "sbuild");
>
> I still don't understand the problem with sbuild-adduser?
>

sbuild-adduser prints a whole bunch of extra messages after running
adduser. In fact, looking at its source, its only purpose seems to be to
print these messages after wrapping adduser. These messages don’t make
sense for the use-case at hand, I think:

'''
# Setup tasks for sudo users:

# BUILD
# HOME directory in chroot, user:sbuild, 0770 perms, from
# passwd/group copying to chroot, filtered
# Maybe source 50sbuild, or move into common location.

Next, copy the example sbuildrc file to the home directory of each user and
set the variables for your system:

  cp /usr/share/doc/sbuild/examples/example.sbuildrc /home/michael/.sbuildrc

Now try a build:

  cd /path/to/source
  sbuild-update -ud 
  (or "sbuild-apt  apt-get -f install"
   first if the chroot is broken)
  sbuild -d  _
'''


>
> > +chomp(my $arch = `dpkg --print-architecture`);
> > +
> > +system("sbuild-createchroot",
> > +   "--command-prefix=eatmydata",
>
> There is no --command-prefix option yet. Should this bug not be blocked by
> #870260?
>

Done.


>
> > +   "--include=eatmydata",
> > +   "--alias=UNRELEASED",
> > +   "--alias=sid",
> > +   "unstable",
> > +   "/srv/chroot/unstable-$arch-sbuild",
> > +   "http://localhost:3142/deb.debian.org/debian;);
>
> What happens if any of this fails? You don't catch any errors.
>

I use the autodie module, which catches the error.


>
> What happens if the user runs the script a second time?
>

All actions except for the chroot creation are idempotent.


>
> You could also check some things *before* you attempt them like:
>
>  - is the user already in the sbuild group?
>

adduser is a noop if the user is already in the sbuild group.


>  - does the output path already exist?
>

Done.


>
> > +open(my $fh, ">", "/etc/cron.d/sbuild-debian-dev
> eloper-setup-update-all");
> > +say $fh q|@daily root /usr/share/doc/sbuild/examples
> /sbuild-update-all|;
> > +close($fh);
>
> Why not a simple symlink in /etc/cron.daily/?
>

Wasn’t aware of cron.daily, I use systemd timer units :). Done.


>
> > +say "Now run `newgrp sbuild', or log out and log in again.";
>
> You could tell the user more about what the script did here. For example
> you
> could say things like:
>
>  - added $USER to the sbuild group
>  - created schroot called "unstable" with chroot locatled in /srv/...
>  - created daily schroot upgrade cronjob
>

Done.


>
> The user should also instruct the user that they should not run the script
> again.
>

No; I implemented your other suggestion, so running the script multiple
times is okay.


>
> And you could also print *how* to use sbuild to build packages, for
> example by
> printing:
>
> $ sbuild -d unstable hello
> $ sbuild -d unstable hello.dsc
> $ cd hello && sbuild
>
> > diff --git a/debian/control b/debian/control
> > index 7249e630..7b4bd21b 100644
> > --- a/debian/control
> > +++ b/debian/control
> > @@ -75,6 +75,22 @@ Description: Tool for building Debian binary packages
> > from Debian sources
> >   build-essential packages, plus those in the package build
> >   dependencies.
> >
> > +Package: sbuild-debian-developer-setup
>
> I guess you need the extra package for the apt-cacher-ng dependency? How
> about
> putting the script into the sbuild package and making apt-cacher-ng a
> Recommends?
>
> > +Architecture: all
> > +Depends: sbuild,
> > + apt-cacher-ng,
>
> Will this not also work with the apt-cacher package in the same way?
>

It will, just verified it. Added apt-cacher as an alternative dependency. I
prefer apt-cacher-ng, because it doesn’t ask my questions during
installation.


>
> > + lintian,
>
> I still don't understand why you need lintian as a runtime dependency. To
> run
> lintian, sbuild installs lintian *inside* the chroot. Lintian on the host
> running sbuild is never used.
>

Ah, I didn’t realize that. Removed.


>
> You forgot a dependency on cron.
>

Added.


>
> Doesn't systemd not also provide a cron-replacement? Maybe you could
> provide a
> solution for that to and the depend on "cron | systemd-sysv"
>

Isn’t a better way to do that to depend on the virtual package
“cron-daemon”? Did that instead.


>
> > + ${misc:Depends},
> > + ${perl:Depends},
> > + ${shlibs:Depends}
> >

Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-08-02 Thread Johannes Schauer 
Quoting Michael Stapelberg (2017-08-01 23:15:04)
> Alright! Patch attached and provided inline, for your convenience:

Cool!

> +if (!defined($ENV{SUDO_USER})) {
> +die "Please run sudo $0";
> +}

Should you not rather check the UID instead?

> +system("adduser", "--quiet", "--", $ENV{SUDO_USER}, "sbuild");

I still don't understand the problem with sbuild-adduser?

> +chomp(my $arch = `dpkg --print-architecture`);
> +
> +system("sbuild-createchroot",
> +   "--command-prefix=eatmydata",

There is no --command-prefix option yet. Should this bug not be blocked by
#870260?

> +   "--include=eatmydata",
> +   "--alias=UNRELEASED",
> +   "--alias=sid",
> +   "unstable",
> +   "/srv/chroot/unstable-$arch-sbuild",
> +   "http://localhost:3142/deb.debian.org/debian;);

What happens if any of this fails? You don't catch any errors.

What happens if the user runs the script a second time?

You could also check some things *before* you attempt them like:

 - is the user already in the sbuild group?
 - does the output path already exist?

> +open(my $fh, ">", "/etc/cron.d/sbuild-debian-developer-setup-update-all");
> +say $fh q|@daily root /usr/share/doc/sbuild/examples/sbuild-update-all|;
> +close($fh);

Why not a simple symlink in /etc/cron.daily/?

> +say "Now run `newgrp sbuild', or log out and log in again.";

You could tell the user more about what the script did here. For example you
could say things like:

 - added $USER to the sbuild group
 - created schroot called "unstable" with chroot locatled in /srv/...
 - created daily schroot upgrade cronjob

The user should also instruct the user that they should not run the script
again.

And you could also print *how* to use sbuild to build packages, for example by
printing:

$ sbuild -d unstable hello
$ sbuild -d unstable hello.dsc
$ cd hello && sbuild

> diff --git a/debian/control b/debian/control
> index 7249e630..7b4bd21b 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -75,6 +75,22 @@ Description: Tool for building Debian binary packages
> from Debian sources
>   build-essential packages, plus those in the package build
>   dependencies.
> 
> +Package: sbuild-debian-developer-setup

I guess you need the extra package for the apt-cacher-ng dependency? How about
putting the script into the sbuild package and making apt-cacher-ng a
Recommends?

> +Architecture: all
> +Depends: sbuild,
> + apt-cacher-ng,

Will this not also work with the apt-cacher package in the same way?

> + lintian,

I still don't understand why you need lintian as a runtime dependency. To run
lintian, sbuild installs lintian *inside* the chroot. Lintian on the host
running sbuild is never used.

You forgot a dependency on cron.

Doesn't systemd not also provide a cron-replacement? Maybe you could provide a
solution for that to and the depend on "cron | systemd-sysv"

> + ${misc:Depends},
> + ${perl:Depends},
> + ${shlibs:Depends}
> +Description: Convenience script to set up an sbuild environment for Debian
> Developers
> + Run "sudo sbuild-debian-developer-setup" to add the current user to the
> sbuild
> + group, create an schroot for building packages for Debian unstable, and
> create
> + a cronjob which updates said schroot daily.
> + .
> + This script assumes you are on an un-metered internet connection (daily
> schroot
> + updates might be costly otherwise).

It's nice that you put "debian" in the package name. Why is "debian" not also
part of the executable name?

How about letting the script take two optional arguments: distribution and
suite. Those could default to "debian" and "unstable" and then downstreams
could easily adjust these defaults.

You should provide a man page to the script.

There should be at least the --help command line option which explains how to
use the script.

Thanks!

cheers, josch


signature.asc
Description: signature

Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-08-01 Thread Michael Stapelberg 
Alright! Patch attached and provided inline, for your convenience:

>From b29b1faf6bafb4b6e51b4e99d0febc81abbb543c Mon Sep 17 00:00:00 2001
From: Michael Stapelberg 
Date: Tue, 1 Aug 2017 23:13:36 +0200
Subject: [PATCH] add sbuild-debian-developer-setup package

---
 bin/Makefile.am  |  1 +
 bin/sbuild-debian-developer-setup| 49

 debian/control   | 16 +
 debian/sbuild-debian-developer-setup.install |  1 +
 4 files changed, 67 insertions(+)
 create mode 100755 bin/sbuild-debian-developer-setup
 create mode 100644 debian/sbuild-debian-developer-setup.install

diff --git a/bin/Makefile.am b/bin/Makefile.am
index 138fc0b3..0fcbda09 100644
--- a/bin/Makefile.am
+++ b/bin/Makefile.am
@@ -28,6 +28,7 @@ bin_SCRIPTS = \
  sbuild-abort \
  sbuild-apt \
  sbuild-checkpackages \
+ sbuild-debian-developer-setup   \
  sbuild-update \
  sbuild-upgrade \
  sbuild-distupgrade \
diff --git a/bin/sbuild-debian-developer-setup
b/bin/sbuild-debian-developer-setup
new file mode 100755
index ..82341d28
--- /dev/null
+++ b/bin/sbuild-debian-developer-setup
@@ -0,0 +1,49 @@
+#!/usr/bin/perl
+#
+# Set up sbuild so that packages for Debian unstable can be built and
+# maintenance is done automatically via a daily update cronjob.
+# Copyright © 2017 Michael Stapelberg .
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see
+# .
+#
+###
+
+use strict;
+use warnings;
+use autodie qw(:all);
+use v5.10;
+
+if (!defined($ENV{SUDO_USER})) {
+die "Please run sudo $0";
+}
+
+system("adduser", "--quiet", "--", $ENV{SUDO_USER}, "sbuild");
+
+chomp(my $arch = `dpkg --print-architecture`);
+
+system("sbuild-createchroot",
+   "--command-prefix=eatmydata",
+   "--include=eatmydata",
+   "--alias=UNRELEASED",
+   "--alias=sid",
+   "unstable",
+   "/srv/chroot/unstable-$arch-sbuild",
+   "http://localhost:3142/deb.debian.org/debian;);
+
+open(my $fh, ">", "/etc/cron.d/sbuild-debian-developer-setup-update-all");
+say $fh q|@daily root /usr/share/doc/sbuild/examples/sbuild-update-all|;
+close($fh);
+
+say "Now run `newgrp sbuild', or log out and log in again.";
diff --git a/debian/control b/debian/control
index 7249e630..7b4bd21b 100644
--- a/debian/control
+++ b/debian/control
@@ -75,6 +75,22 @@ Description: Tool for building Debian binary packages
from Debian sources
  build-essential packages, plus those in the package build
  dependencies.

+Package: sbuild-debian-developer-setup
+Architecture: all
+Depends: sbuild,
+ apt-cacher-ng,
+ lintian,
+ ${misc:Depends},
+ ${perl:Depends},
+ ${shlibs:Depends}
+Description: Convenience script to set up an sbuild environment for Debian
Developers
+ Run "sudo sbuild-debian-developer-setup" to add the current user to the
sbuild
+ group, create an schroot for building packages for Debian unstable, and
create
+ a cronjob which updates said schroot daily.
+ .
+ This script assumes you are on an un-metered internet connection (daily
schroot
+ updates might be costly otherwise).
+
 Package: buildd
 Architecture: all
 Depends: adduser,
diff --git a/debian/sbuild-debian-developer-setup.install
b/debian/sbuild-debian-developer-setup.install
new file mode 100644
index ..406b3af9
--- /dev/null
+++ b/debian/sbuild-debian-developer-setup.install
@@ -0,0 +1 @@
+usr/bin/sbuild-debian-developer-setup
-- 
2.13.2


On Tue, Aug 1, 2017 at 11:05 AM, Johannes Schauer  wrote:

> Quoting Michael Stapelberg (2017-08-01 11:04:16)
> > That works for me :).
> >
> > How shall we proceed? Should I prepare a patch against the sbuild
> package?
>
> Yes please!
>
> Put the patch into this bug and then we can talk about it. :)
>



-- 
Best regards,
Michael
From b29b1faf6bafb4b6e51b4e99d0febc81abbb543c Mon Sep 17 00:00:00 2001
From: Michael Stapelberg 
Date: Tue, 1 Aug 2017 23:13:36 +0200
Subject: [PATCH] add sbuild-debian-developer-setup package

---
 bin/Makefile.am  |  1 +
 bin/sbuild-debian-developer-setup| 49 
 debian/control   | 16 +
 debian/sbuild-debian-developer-setup.install |  1 +
 4 files changed, 67

Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-08-01 Thread Michael Stapelberg 
That works for me :).

How shall we proceed? Should I prepare a patch against the sbuild package?

On Tue, Aug 1, 2017 at 11:00 AM, Johannes Schauer  wrote:

> Quoting Michael Stapelberg (2017-08-01 10:51:41)
> > I think the following suggestion takes care of all the concerns you
> brought
> > up: Let’s name it sbuild-debian-developer-setup, describe that the goal
> is to
> > provide an sbuild setup which can build packages for Debian unstable,
> > automates maintenance with its daily update cronjob and assumes an
> un-metered
> > internet connection.
> >
> > Does that sound reasonable?
>
> Okay, you seem to be very enthusiastic about this. Would you like to
> maintain
> that script? Then it could live in the sbuild package but I would not have
> to
> deal with it. ;)
>



-- 
Best regards,
Michael

Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-08-01 Thread Johannes Schauer 
Quoting Michael Stapelberg (2017-08-01 11:04:16)
> That works for me :).
> 
> How shall we proceed? Should I prepare a patch against the sbuild package?

Yes please!

Put the patch into this bug and then we can talk about it. :)


signature.asc
Description: signature

Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-08-01 Thread Johannes Schauer 
Quoting Michael Stapelberg (2017-08-01 10:51:41)
> I think the following suggestion takes care of all the concerns you brought
> up: Let’s name it sbuild-debian-developer-setup, describe that the goal is to
> provide an sbuild setup which can build packages for Debian unstable,
> automates maintenance with its daily update cronjob and assumes an un-metered
> internet connection.
> 
> Does that sound reasonable?

Okay, you seem to be very enthusiastic about this. Would you like to maintain
that script? Then it could live in the sbuild package but I would not have to
deal with it. ;)


signature.asc
Description: signature

Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-08-01 Thread Michael Stapelberg 
On Mon, Jul 31, 2017 at 4:24 PM, Johannes Schauer  wrote:

> Quoting Michael Stapelberg (2017-07-31 14:19:16)
> > Unless I’m mistaken, the following is what we’d need to recommend to new
> > users:
> >
> > % sudo apt install sbuild apt-cacher-ng lintian
>
> Why install lintian?
>

So that it is available to be used, as per the changed default?


>
> > % sudo adduser --quiet -- "$USER" sbuild
>
> Better:
>
> sudo sbuild-adduser $USER
>

sbuild-adduser gives instructions which don’t apply in our case (AFAIR).


>
> > % sudo sbuild-createchroot \
> >   --command-prefix=eatmydata \
> >   --include=eatmydata \
> >   --alias=UNRELEASED \
> >   --alias=sid \
> >   unstable \
> >   /srv/chroot/unstable-amd64-sbuild \
> >   http://localhost:3142/deb.debian.org/debian
>
> That is *if* the machine of the user is amd64.
>

Substitute $(dpkg --print-architecture).


>
> Also, this part would be Debian-specific. Downstream distributions would
> have
> to adapt the alias and mirror values.
>

Yes.


>
> Also, didn't you also propose to make the schroot be run in a tmpfs the
> default? In that case, eatmydata would be quite pointless, no?
>

I proposed suggesting to build in tmpfs, not doing it by default. If you
think people in general have enough RAM for that to be a good idea, I’ll
happily change it and get rid of eatmydata.


>
> > % echo 15 */6 * * * root /usr/share/doc/sbuild/examples/sbuild-update-all
> |
> > sudo tee /etc/cron.d/sbuild-update-all
>
> Every six hours? I find that a bit excessive. This should certainly be
>

This is the first example
from /usr/share/doc/sbuild/examples/sbuild-update-all itself, which I
assumed was a recommendation. We could dial it down to once a day.


> configurable. Not everybody is behind an internet connection which is fast
>

It is configurable, just edit the file afterwards :).


> enough and/or where one doesn't pay per MB.
>

True, but I’d wager most DMs/DDs (the primary target audience of this
endavour) are. We can slap a fat warning on top of the package description
to clarify that we’re operating under the assumption.


>
> > % newgrp sbuild
>
> This would only have an effect on the currently open terminal and would
> have to
> be executed again on every new terminal session until the user *really*
> logs
> out and in again.
>

Yes. We can tell the user about this fact, but running newgrp seems like a
good idea nevertheless.


>
> > That seems quite involved over, say, “apt install sbuild-setup &&
> > sbuild-setup unstable”.
> >
> > Hence, I’d definitely appreciate a script which does all the over having
> to
> > refer to a wiki page and copy long commands.
>
> Except that the sbuild-setup command would need to become quite complex
> because
> it the user has to be able to control:
>
>  - how to setup schroot (overlayfs? tmpfs?)
>

The default.


>  - where to put the chroots
>

The default.


>  - which distribution aliases (distribution specific)
>

sid for Debian. Downstream can change this.


>  - which extra packages to include (like eatmydata)
>

Only eatmydata.


>  - whether this is the first run or not (warn if the script is run for a
> second
>time)
>  - how often to update the chroots via cron
>

Once a day, change the crontab file if you want to change that.

To summarize: the point of this script is to provide an easy way to get
sbuild for people who don’t care. People who do care about any of these
details should not use it. I think there is significant value in providing
an easy path, if only as a stop-gap until the user gets around to looking
into this subject area in more depth, and creating their own preferred
setup.


>
> And then we have a script with a complexity which is close to where
> sbuild-createchroot already is.
>
> Or are you actually convinced that it is possible to find a set of defaults
> which fits even half the userbase of sbuild?
>
> Since we are down to two mandatory (and two optional) commands after
> running
> "apt install sbuild", I'd argue that a superior solution would be to
> improve
> the documentation of which commands to run for a "typical" setup. I fear
> that
> trying to create a "one-size-fits-all" script can have many unintended
> side-effects (thinking of users behind bad or costly internet, who use
> schroot
> for other purposes, who don't want to install another deamon like
> apt-cacher-ng, who are not building for Debian but for downstreams...).
> I'm not
> convinced that the time that the user would invest to *really* understand
> the
> things that an sbuild-setup script is actually doing would not be better
> spent
> in learning how to use the individual tools.
>

I think the following suggestion takes care of all the concerns you brought
up: Let’s name it sbuild-debian-developer-setup, describe that the goal is
to provide an sbuild setup which can build packages for Debian unstable,
automates maintenance with its daily update cronjob and assumes an
un-metered internet connection.

Does that

Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-07-31 Thread Johannes Schauer 
Quoting Michael Stapelberg (2017-07-31 14:19:16)
> Unless I’m mistaken, the following is what we’d need to recommend to new
> users:
> 
> % sudo apt install sbuild apt-cacher-ng lintian

Why install lintian?

> % sudo adduser --quiet -- "$USER" sbuild

Better:

sudo sbuild-adduser $USER

> % sudo sbuild-createchroot \
>   --command-prefix=eatmydata \
>   --include=eatmydata \
>   --alias=UNRELEASED \
>   --alias=sid \
>   unstable \
>   /srv/chroot/unstable-amd64-sbuild \
>   http://localhost:3142/deb.debian.org/debian

That is *if* the machine of the user is amd64.

Also, this part would be Debian-specific. Downstream distributions would have
to adapt the alias and mirror values.

Also, didn't you also propose to make the schroot be run in a tmpfs the
default? In that case, eatmydata would be quite pointless, no?

> % echo 15 */6 * * * root /usr/share/doc/sbuild/examples/sbuild-update-all |
> sudo tee /etc/cron.d/sbuild-update-all

Every six hours? I find that a bit excessive. This should certainly be
configurable. Not everybody is behind an internet connection which is fast
enough and/or where one doesn't pay per MB.

> % newgrp sbuild

This would only have an effect on the currently open terminal and would have to
be executed again on every new terminal session until the user *really* logs
out and in again.

> That seems quite involved over, say, “apt install sbuild-setup &&
> sbuild-setup unstable”.
> 
> Hence, I’d definitely appreciate a script which does all the over having to
> refer to a wiki page and copy long commands.

Except that the sbuild-setup command would need to become quite complex because
it the user has to be able to control:

 - how to setup schroot (overlayfs? tmpfs?)
 - where to put the chroots
 - which distribution aliases (distribution specific)
 - which extra packages to include (like eatmydata)
 - whether this is the first run or not (warn if the script is run for a second
   time)
 - how often to update the chroots via cron

And then we have a script with a complexity which is close to where
sbuild-createchroot already is.

Or are you actually convinced that it is possible to find a set of defaults
which fits even half the userbase of sbuild?

Since we are down to two mandatory (and two optional) commands after running
"apt install sbuild", I'd argue that a superior solution would be to improve
the documentation of which commands to run for a "typical" setup. I fear that
trying to create a "one-size-fits-all" script can have many unintended
side-effects (thinking of users behind bad or costly internet, who use schroot
for other purposes, who don't want to install another deamon like
apt-cacher-ng, who are not building for Debian but for downstreams...). I'm not
convinced that the time that the user would invest to *really* understand the
things that an sbuild-setup script is actually doing would not be better spent
in learning how to use the individual tools.

What do you think?

cheers, josch


signature.asc
Description: signature

Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-07-31 Thread Michael Stapelberg 
On Sun, Jul 30, 2017 at 7:53 PM, Johannes Schauer  wrote:

> Hi,
>
> Quoting Michael Stapelberg (2017-06-02 18:23:02)
> > Thanks for the review. Answers inline:
>
> sorry for the delay. I'm under a pile of work and this wasn't on the top
> of my
> todo list. But let me not stall your work:
>
> > > > # Enable eatmydata: occasionally losing a test build is preferable
> over
> > > longer
> > > > # build times and disk wear.
> > > > echo "command-prefix=eatmydata" >> "${tmp}"
> > >
> > > I don't see sbuild-createchroot mangling with command-prefix at all. So
> > > why do
> > > you filter it out initially?
> > >
> > > Would it not be better to add a command line option to
> sbuild-createchroot
> > > which allows setting a custom command-prefix? Then you would not need
> to
> > > manually mangle the created config file at all and lots of the
> problems I
> > > mentioned above and other fragile things would go away.
> > Sure. Can you take care of this or do you want me to send a patch?
>
> Please file a new bug with a patch.
>

Done: https://bugs.debian.org/870260


>
> > > > chmod 644 "${tmp}"
> > > > mv "${tmp}" "${config}"
> > > >
> > > > # Copy a modified example sbuildrc config file
> > > > for homedir in $(getent passwd | perl -F: -nlE '$F[2] >= 1000 &&
> $F[2]
> > > <= 5 && say $F[5]')
> > > > do
> > > > userconfig="${homedir}/.sbuildrc"
> > > > if [ ! -e "${userconfig}" ]
> > > > then
> > > > (grep -v -E "^(# don't remove this|1;\$)"
> /usr/share/doc/sbuild/examples/example.sbuildrc
> > > && cat /usr/share/doc/sbuild-debian-setup/sbuildrc) > "${userconfig}"
> > > > fi
> > > > done
> > >
> > > Instead of crafting a custom sbuildrc, we can also talk about changing
> the
> > > existing defaults.
> > >
> > > For example you set "$build_arch_all = 1;" I can certainly see how it
> makes
> > > sense to change this default for sbuild but keep it at 0 for buildd.
> > Sounds good. Same question: can you take care of this, or should I send a
> > patch?
>
> For personal users of sbuild (users of sbuild and not buildd) they will
> probably always want to build arch:all packages. So lets change the
> default!
>
> Again, a short bug with patch is appreciated.
>

Done: https://bugs.debian.org/870263


>
> > > Then you set "$build_source = 1;" but I don't know why you would need
> this.
> > > Sbuild is not there to build the source package. The source package is
> its
> > > input not its output. Why would you want sbuild to build it again?
> > The wiki page contained some confusing advice, which I now removed.
>
> Thanks!
>
> > > Lastly, you set "$run_lintian = 1;" which I also think would be a
> sensible
> > > thing to change in the defaults.
> > Sounds good, see above.
>
> You can include that in the other bug.
>

See above.


>
> > > > if [ ! -e "/etc/schroot/setup.d/04tmpfs" ]
> > > > then
> > > > echo ""
> > > > echo "  If you can spare the RAM, you can enable building in
> tmpfs
> > > using:"
> > > > echo ""
> > > > echo "sudo ln -s /etc/schroot/setup-available.d
> /overlays-in-tmpfs
> > > /etc/schroot/setup.d/04tmpfs"
> > > > echo ""
> > > > fi
> > >
> > > This will affect *all* schroot sessions and not just those used by
> sbuild.
> > > As such, this should rather go into documentation for schroot. But
> maybe
> > > there is a way to make this mounting specific to sbuild schroots?
> > I don’t know, you’re the expert :). I think users who know what schroot
> is
> > (to me it’s “the thing which sbuild uses”) and who use it for anything
> but
> > sbuild will realize what’s happening here. We could add some wording to
> > that extent.
>
> More docs in this direction would be appreciated.
>
> > > I think in summary much of what this script does can be achieved by:
> > >
> > >  - improving existing documentation
> > I updated the wiki page based on your review.
>
> Thanks a lot!
>
> > >  - changing configuration defaults to more sensible values
> > >  - adding more options to existing tools like sbuild-createchroot
> > >  - using existing tools instead of using creative ways to work around
> them
> > >(apt-cacher-ng)
> > >
> > > If we do all that, what is left that a new script should take care of?
> > The following things are still left:
> >
> > • Adding the user(s) to the sbuild group
>
> That's just one call to sbuild-adduser
>
> > • Creating the chroot with all the options we discussed
>
> Yet another single call to sbuild-createchroot
>
> > • Suggesting to build in tmpfs
>
> I'd consider that an optional tip that can be listed in the docs somewhere.
>
> > • Periodically updating the schroots (not strictly speaking part of the
> > script itself, but listing it here anyway)
>
> This can be done using the cron script from
> /usr/share/doc/sbuild/examples/sbuild-update-all
>
> Bug #870102 is about making this cron script more visible by adding better
> documentation.
>
> So all in all, we are down to running two scripts to setup sbuild. One of
>

Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-07-30 Thread Johannes Schauer 
Hi,

Quoting Michael Stapelberg (2017-06-02 18:23:02)
> Thanks for the review. Answers inline:

sorry for the delay. I'm under a pile of work and this wasn't on the top of my
todo list. But let me not stall your work:

> > > # Enable eatmydata: occasionally losing a test build is preferable over
> > longer
> > > # build times and disk wear.
> > > echo "command-prefix=eatmydata" >> "${tmp}"
> >
> > I don't see sbuild-createchroot mangling with command-prefix at all. So
> > why do
> > you filter it out initially?
> >
> > Would it not be better to add a command line option to sbuild-createchroot
> > which allows setting a custom command-prefix? Then you would not need to
> > manually mangle the created config file at all and lots of the problems I
> > mentioned above and other fragile things would go away.
> Sure. Can you take care of this or do you want me to send a patch?

Please file a new bug with a patch.

> > > chmod 644 "${tmp}"
> > > mv "${tmp}" "${config}"
> > >
> > > # Copy a modified example sbuildrc config file
> > > for homedir in $(getent passwd | perl -F: -nlE '$F[2] >= 1000 && $F[2]
> > <= 5 && say $F[5]')
> > > do
> > > userconfig="${homedir}/.sbuildrc"
> > > if [ ! -e "${userconfig}" ]
> > > then
> > > (grep -v -E "^(# don't remove this|1;\$)" 
> > > /usr/share/doc/sbuild/examples/example.sbuildrc
> > && cat /usr/share/doc/sbuild-debian-setup/sbuildrc) > "${userconfig}"
> > > fi
> > > done
> >
> > Instead of crafting a custom sbuildrc, we can also talk about changing the
> > existing defaults.
> >
> > For example you set "$build_arch_all = 1;" I can certainly see how it makes
> > sense to change this default for sbuild but keep it at 0 for buildd.
> Sounds good. Same question: can you take care of this, or should I send a
> patch?

For personal users of sbuild (users of sbuild and not buildd) they will
probably always want to build arch:all packages. So lets change the default!

Again, a short bug with patch is appreciated.

> > Then you set "$build_source = 1;" but I don't know why you would need this.
> > Sbuild is not there to build the source package. The source package is its
> > input not its output. Why would you want sbuild to build it again?
> The wiki page contained some confusing advice, which I now removed.

Thanks!

> > Lastly, you set "$run_lintian = 1;" which I also think would be a sensible
> > thing to change in the defaults.
> Sounds good, see above.

You can include that in the other bug.

> > > if [ ! -e "/etc/schroot/setup.d/04tmpfs" ]
> > > then
> > > echo ""
> > > echo "  If you can spare the RAM, you can enable building in tmpfs
> > using:"
> > > echo ""
> > > echo "sudo ln -s /etc/schroot/setup-available.d/overlays-in-tmpfs
> > /etc/schroot/setup.d/04tmpfs"
> > > echo ""
> > > fi
> >
> > This will affect *all* schroot sessions and not just those used by sbuild.
> > As such, this should rather go into documentation for schroot. But maybe
> > there is a way to make this mounting specific to sbuild schroots?
> I don’t know, you’re the expert :). I think users who know what schroot is
> (to me it’s “the thing which sbuild uses”) and who use it for anything but
> sbuild will realize what’s happening here. We could add some wording to
> that extent.

More docs in this direction would be appreciated.

> > I think in summary much of what this script does can be achieved by:
> >
> >  - improving existing documentation
> I updated the wiki page based on your review.

Thanks a lot!

> >  - changing configuration defaults to more sensible values
> >  - adding more options to existing tools like sbuild-createchroot
> >  - using existing tools instead of using creative ways to work around them
> >(apt-cacher-ng)
> >
> > If we do all that, what is left that a new script should take care of?
> The following things are still left:
> 
> • Adding the user(s) to the sbuild group

That's just one call to sbuild-adduser

> • Creating the chroot with all the options we discussed

Yet another single call to sbuild-createchroot

> • Suggesting to build in tmpfs

I'd consider that an optional tip that can be listed in the docs somewhere.

> • Periodically updating the schroots (not strictly speaking part of the
> script itself, but listing it here anyway)

This can be done using the cron script from
/usr/share/doc/sbuild/examples/sbuild-update-all

Bug #870102 is about making this cron script more visible by adding better
documentation.

So all in all, we are down to running two scripts to setup sbuild. One of that
is only required once after installation while the other can be run multiple
times to setup multiple chroots. I don't think a wrapper script for these two
scripts makes much sense. Lets rather better document which two commands one
needs to run after installing sbuild for the first time.

Thanks!

cheers, josch


signature.asc
Description: signature

Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-07-30 Thread Michael Stapelberg 
josch, friendly ping? :)

On Fri, Jun 2, 2017 at 6:23 PM, Michael Stapelberg 
wrote:

> Thanks for the review. Answers inline:
>
> On Wed, May 24, 2017 at 11:03 AM, Johannes Schauer 
> wrote:
>
>> Hi,
>>
>> Quoting Geert Stappers (2017-05-21 08:43:31)
>> > the debian/postinst now here inline
>>
>> thanks, that allows me to comment easily.
>>
>> > # Add to group sbuild all “dynamically allocated user accounts†; see
>> > # https://www.debian.org/doc/debian-policy/ch-opersys.html
>> > for user in $(getent passwd | perl -F: -nlE '$F[2] >= 1000 && $F[2] <=
>> 5 && say $F[0]')
>> > do
>> > # Strictly speaking, we should use sbuild-adduser, but
>> sbuild-adduser prints
>> > # setup instructions to STDERR which do not make sense in the
>> context of
>> > # this package. Filtering STDERR is cumbersome, so we call adduser
>> directly.
>> > adduser --quiet -- "$user" sbuild
>> > done
>>
>> I'm doubtful whether adding *all* users with ids between 1000 and 5
>> to the
>> sbuild group is something sensible to do. This would give *all* these
>> users
>> instant access to schroot-based chroots. I don't think this is something
>> anybody expects when running a script, much less something that anybody
>> expects
>> to happen in a package's postinstall.
>>
>> > # Create a chroot if it does not already exist
>> > chroot="unstable-$(dpkg --print-architecture)-sbuild"
>> > if ! schroot -i -c chroot:${chroot} >/dev/null 2>&1
>> > then
>> > sbuild-createchroot \
>> > --include=eatmydata,ccache,gnupg \
>>
>> Why include ccache? I would not expect that a single build will compile
>> the
>> same source multiple times. Without bind-mounting the cache directory
>> from the
>> outside this will probably not help much.
>>
>> Why include gnupg?
>>
>
> This was copy from the wiki page. Removed.
>
>
>>
>> > unstable \
>> > /srv/chroot/${chroot} \
>> > http://deb.debian.org/debian
>>
>> Why not let this new package depend on apt-cacher-ng and let
>> http://127.0.0.1:3142/deb.debian.org/debian be the default mirror?
>>
>
> Done.
>
>
>>
>> > # At this point, sbuild-createchroot created an schroot
>> configuration with a
>> > # random suffix, e.g. 
>> > /etc/schroot/chroot.d/unstable-amd64-sbuild-pyViYe.
>> As
>> > # sbuild-createchroot recommends, we rename that file before making
>> > # adjustments.
>> > mv /etc/schroot/chroot.d/${chroot}-* /etc/schroot/chroot.d/${chroot
>> }
>>
>> What happens with any existing /etc/schroot/chroot.d/${chroot}? You just
>> overwrite it?
>>
>> What happens if more than one file match /etc/schroot/chroot.d/${chroot
>> }-*?
>>
>> > fi
>> >
>> > # schroot config customizations:
>> > config=/etc/schroot/chroot.d/${chroot}
>> > tmp=$(mktemp ${config}-XX.dpkg-tmp)
>> > trap 'rm -f "${tmp}"' TERM INT EXIT QUIT
>> >
>> > grep -v -E '^(aliases|command-prefix)=' "${config}" > "${tmp}"
>> >
>> > # For convenience, treat UNRELEASED as an alias for unstable (so that
>> > # debian/changelog files containing UNRELEASED do not need to be
>> modified before
>> > # building). Also sid, because it is short to type when specifying -d.
>> > echo "aliases=UNRELEASED,sid" >> "${tmp}"
>>
>> You can achieve the same effect using sbuild-createchroot by using the
>> --alias
>> option. No need to manually mangle the schroot config.
>>
>
> Done.
>
>
>>
>> > # Enable eatmydata: occasionally losing a test build is preferable over
>> longer
>> > # build times and disk wear.
>> > echo "command-prefix=eatmydata" >> "${tmp}"
>>
>> I don't see sbuild-createchroot mangling with command-prefix at all. So
>> why do
>> you filter it out initially?
>>
>> Would it not be better to add a command line option to sbuild-createchroot
>> which allows setting a custom command-prefix? Then you would not need to
>> manually mangle the created config file at all and lots of the problems I
>> mentioned above and other fragile things would go away.
>>
>
> Sure. Can you take care of this or do you want me to send a patch?
>
>
>>
>> > chmod 644 "${tmp}"
>> > mv "${tmp}" "${config}"
>> >
>> > # Copy a modified example sbuildrc config file
>> > for homedir in $(getent passwd | perl -F: -nlE '$F[2] >= 1000 && $F[2]
>> <= 5 && say $F[5]')
>> > do
>> > userconfig="${homedir}/.sbuildrc"
>> > if [ ! -e "${userconfig}" ]
>> > then
>> > (grep -v -E "^(# don't remove this|1;\$)"
>> /usr/share/doc/sbuild/examples/example.sbuildrc && cat
>> /usr/share/doc/sbuild-debian-setup/sbuildrc) > "${userconfig}"
>> > fi
>> > done
>>
>> Instead of crafting a custom sbuildrc, we can also talk about changing the
>> existing defaults.
>>
>> For example you set "$build_arch_all = 1;" I can certainly see how it
>> makes
>> sense to change this default for sbuild but keep it at 0 for buildd.
>>
>
> Sounds good. Same question: can you take care of this, or should I send a
> patch?
>
>
>>
>> Then you set "$build_source = 1;" but I don't

Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-06-02 Thread Michael Stapelberg 
Thanks for the review. Answers inline:

On Wed, May 24, 2017 at 11:03 AM, Johannes Schauer  wrote:

> Hi,
>
> Quoting Geert Stappers (2017-05-21 08:43:31)
> > the debian/postinst now here inline
>
> thanks, that allows me to comment easily.
>
> > # Add to group sbuild all “dynamically allocated user accounts†; see
> > # https://www.debian.org/doc/debian-policy/ch-opersys.html
> > for user in $(getent passwd | perl -F: -nlE '$F[2] >= 1000 && $F[2] <=
> 5 && say $F[0]')
> > do
> > # Strictly speaking, we should use sbuild-adduser, but
> sbuild-adduser prints
> > # setup instructions to STDERR which do not make sense in the
> context of
> > # this package. Filtering STDERR is cumbersome, so we call adduser
> directly.
> > adduser --quiet -- "$user" sbuild
> > done
>
> I'm doubtful whether adding *all* users with ids between 1000 and 5 to
> the
> sbuild group is something sensible to do. This would give *all* these users
> instant access to schroot-based chroots. I don't think this is something
> anybody expects when running a script, much less something that anybody
> expects
> to happen in a package's postinstall.
>
> > # Create a chroot if it does not already exist
> > chroot="unstable-$(dpkg --print-architecture)-sbuild"
> > if ! schroot -i -c chroot:${chroot} >/dev/null 2>&1
> > then
> > sbuild-createchroot \
> > --include=eatmydata,ccache,gnupg \
>
> Why include ccache? I would not expect that a single build will compile the
> same source multiple times. Without bind-mounting the cache directory from
> the
> outside this will probably not help much.
>
> Why include gnupg?
>

This was copy from the wiki page. Removed.


>
> > unstable \
> > /srv/chroot/${chroot} \
> > http://deb.debian.org/debian
>
> Why not let this new package depend on apt-cacher-ng and let
> http://127.0.0.1:3142/deb.debian.org/debian be the default mirror?
>

Done.


>
> > # At this point, sbuild-createchroot created an schroot
> configuration with a
> > # random suffix, e.g. 
> > /etc/schroot/chroot.d/unstable-amd64-sbuild-pyViYe.
> As
> > # sbuild-createchroot recommends, we rename that file before making
> > # adjustments.
> > mv /etc/schroot/chroot.d/${chroot}-* /etc/schroot/chroot.d/${chroot}
>
> What happens with any existing /etc/schroot/chroot.d/${chroot}? You just
> overwrite it?
>
> What happens if more than one file match /etc/schroot/chroot.d/${
> chroot}-*?
>
> > fi
> >
> > # schroot config customizations:
> > config=/etc/schroot/chroot.d/${chroot}
> > tmp=$(mktemp ${config}-XX.dpkg-tmp)
> > trap 'rm -f "${tmp}"' TERM INT EXIT QUIT
> >
> > grep -v -E '^(aliases|command-prefix)=' "${config}" > "${tmp}"
> >
> > # For convenience, treat UNRELEASED as an alias for unstable (so that
> > # debian/changelog files containing UNRELEASED do not need to be
> modified before
> > # building). Also sid, because it is short to type when specifying -d.
> > echo "aliases=UNRELEASED,sid" >> "${tmp}"
>
> You can achieve the same effect using sbuild-createchroot by using the
> --alias
> option. No need to manually mangle the schroot config.
>

Done.


>
> > # Enable eatmydata: occasionally losing a test build is preferable over
> longer
> > # build times and disk wear.
> > echo "command-prefix=eatmydata" >> "${tmp}"
>
> I don't see sbuild-createchroot mangling with command-prefix at all. So
> why do
> you filter it out initially?
>
> Would it not be better to add a command line option to sbuild-createchroot
> which allows setting a custom command-prefix? Then you would not need to
> manually mangle the created config file at all and lots of the problems I
> mentioned above and other fragile things would go away.
>

Sure. Can you take care of this or do you want me to send a patch?


>
> > chmod 644 "${tmp}"
> > mv "${tmp}" "${config}"
> >
> > # Copy a modified example sbuildrc config file
> > for homedir in $(getent passwd | perl -F: -nlE '$F[2] >= 1000 && $F[2]
> <= 5 && say $F[5]')
> > do
> > userconfig="${homedir}/.sbuildrc"
> > if [ ! -e "${userconfig}" ]
> > then
> > (grep -v -E "^(# don't remove this|1;\$)" 
> > /usr/share/doc/sbuild/examples/example.sbuildrc
> && cat /usr/share/doc/sbuild-debian-setup/sbuildrc) > "${userconfig}"
> > fi
> > done
>
> Instead of crafting a custom sbuildrc, we can also talk about changing the
> existing defaults.
>
> For example you set "$build_arch_all = 1;" I can certainly see how it makes
> sense to change this default for sbuild but keep it at 0 for buildd.
>

Sounds good. Same question: can you take care of this, or should I send a
patch?


>
> Then you set "$build_source = 1;" but I don't know why you would need this.
> Sbuild is not there to build the source package. The source package is its
> input not its output. Why would you want sbuild to build it again?
>

The wiki page contained some confusing advice, which I now removed.


>
> Lastly, you set "$run_lintian

Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-05-24 Thread Johannes Schauer 
Hi,

Quoting Geert Stappers (2017-05-21 08:43:31)
> the debian/postinst now here inline

thanks, that allows me to comment easily.

> # Add to group sbuild all “dynamically allocated user accounts”; see
> # https://www.debian.org/doc/debian-policy/ch-opersys.html
> for user in $(getent passwd | perl -F: -nlE '$F[2] >= 1000 && $F[2] <= 5 
> && say $F[0]')
> do
> # Strictly speaking, we should use sbuild-adduser, but sbuild-adduser 
> prints
> # setup instructions to STDERR which do not make sense in the context of
> # this package. Filtering STDERR is cumbersome, so we call adduser 
> directly.
> adduser --quiet -- "$user" sbuild
> done

I'm doubtful whether adding *all* users with ids between 1000 and 5 to the
sbuild group is something sensible to do. This would give *all* these users
instant access to schroot-based chroots. I don't think this is something
anybody expects when running a script, much less something that anybody expects
to happen in a package's postinstall.

> # Create a chroot if it does not already exist
> chroot="unstable-$(dpkg --print-architecture)-sbuild"
> if ! schroot -i -c chroot:${chroot} >/dev/null 2>&1
> then
> sbuild-createchroot \
> --include=eatmydata,ccache,gnupg \

Why include ccache? I would not expect that a single build will compile the
same source multiple times. Without bind-mounting the cache directory from the
outside this will probably not help much.

Why include gnupg?

> unstable \
> /srv/chroot/${chroot} \
> http://deb.debian.org/debian

Why not let this new package depend on apt-cacher-ng and let
http://127.0.0.1:3142/deb.debian.org/debian be the default mirror?

> # At this point, sbuild-createchroot created an schroot configuration 
> with a
> # random suffix, e.g. /etc/schroot/chroot.d/unstable-amd64-sbuild-pyViYe. 
> As
> # sbuild-createchroot recommends, we rename that file before making
> # adjustments.
> mv /etc/schroot/chroot.d/${chroot}-* /etc/schroot/chroot.d/${chroot}

What happens with any existing /etc/schroot/chroot.d/${chroot}? You just
overwrite it?

What happens if more than one file match /etc/schroot/chroot.d/${chroot}-*?

> fi
> 
> # schroot config customizations:
> config=/etc/schroot/chroot.d/${chroot}
> tmp=$(mktemp ${config}-XX.dpkg-tmp)
> trap 'rm -f "${tmp}"' TERM INT EXIT QUIT
> 
> grep -v -E '^(aliases|command-prefix)=' "${config}" > "${tmp}"
> 
> # For convenience, treat UNRELEASED as an alias for unstable (so that
> # debian/changelog files containing UNRELEASED do not need to be modified 
> before
> # building). Also sid, because it is short to type when specifying -d.
> echo "aliases=UNRELEASED,sid" >> "${tmp}"

You can achieve the same effect using sbuild-createchroot by using the --alias
option. No need to manually mangle the schroot config.

> # Enable eatmydata: occasionally losing a test build is preferable over longer
> # build times and disk wear.
> echo "command-prefix=eatmydata" >> "${tmp}"

I don't see sbuild-createchroot mangling with command-prefix at all. So why do
you filter it out initially?

Would it not be better to add a command line option to sbuild-createchroot
which allows setting a custom command-prefix? Then you would not need to
manually mangle the created config file at all and lots of the problems I
mentioned above and other fragile things would go away.

> chmod 644 "${tmp}"
> mv "${tmp}" "${config}"
> 
> # Copy a modified example sbuildrc config file
> for homedir in $(getent passwd | perl -F: -nlE '$F[2] >= 1000 && $F[2] <= 
> 5 && say $F[5]')
> do
> userconfig="${homedir}/.sbuildrc"
> if [ ! -e "${userconfig}" ]
> then
> (grep -v -E "^(# don't remove this|1;\$)" 
> /usr/share/doc/sbuild/examples/example.sbuildrc && cat 
> /usr/share/doc/sbuild-debian-setup/sbuildrc) > "${userconfig}"
> fi
> done

Instead of crafting a custom sbuildrc, we can also talk about changing the
existing defaults.

For example you set "$build_arch_all = 1;" I can certainly see how it makes
sense to change this default for sbuild but keep it at 0 for buildd.

Then you set "$build_source = 1;" but I don't know why you would need this.
Sbuild is not there to build the source package. The source package is its
input not its output. Why would you want sbuild to build it again?

Lastly, you set "$run_lintian = 1;" which I also think would be a sensible
thing to change in the defaults.

As a result, you would not need to manually craft a custom sbuildrc anymore.

> # bind-mount the apt archive cache into chroots, so that packages are 
> downloaded
> # only once. The assumption is that users will not typically have a local apt
> # mirror or caching proxy.
> if ! grep -q '^/var/cache/apt/archives' /etc/schroot/sbuild/fstab
> then
> echo "/var/cache/apt/archives /var/cache/apt/archives none rw,bind 0 0" \
>  >>/etc/schroot/sbuild/fstab
> fi

And who cleans up /var/cache/apt/archives regularly when it becomes

Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-05-24 Thread Ansgar Burchardt 
Hi,

Michael Stapelberg  writes:
> Doing it in a script is one more step. The point of this endavour is to
> make the setup as simple as possible.

I think creating a chroot in postinst is not good.  Mostly because
maintainer scripts should never fail, but this is too likely to fail
(for example when there are network connection issues).

Maintainer scripts must also *never* write to user home
directories. Besides security issues this will also make maintainer
scripts just fail.

Just providing a script in the existing sbuild package to more easily
setup a chroot with a standard configuration seems to be the better
option to me.

Ansgar

Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-05-21 Thread Michael Stapelberg 
Doing it in a script is one more step. The point of this endavour is to
make the setup as simple as possible.

On that note, the awkward newgrp step afterwards still irks me.

On Sun, May 21, 2017 at 8:43 AM, Geert Stappers 
wrote:

> On Sat, May 20, 2017 at 11:34:37PM +0200, Michael Stapelberg wrote:
> >
> > Find attached the first draft of my suggestion. I implemented it as a
> > separate package purely so that I can build it more quickly, but I assume
> > we???d want to fold this into src:sbuild eventually.
> >
> > The resulting package (I built it using dpkg-buildpackage -b) depends on
> > sbuild, schroot, debootstrap, perl-base, lintian. Upon installation, it
> > will create an unstable sbuild schroot, modify its configuration, add all
> > users to sbuild and create a modified ~/.sbuildrc for all users.
> >
> > If you want to read through the entire behavior, I recommend the
> > entrypoints debian/postinst and update-sbuild-chroots.
>
> the debian/postinst now here inline
>
>
> #!/bin/sh
>
> set -e
>
> #DEBHELPER#
>
> # TODO: make this package pass piuparts
>
> # Add to group sbuild all “dynamically allocated user accounts”; see
> # https://www.debian.org/doc/debian-policy/ch-opersys.html
> for user in $(getent passwd | perl -F: -nlE '$F[2] >= 1000 && $F[2] <=
> 5 && say $F[0]')
> do
> # Strictly speaking, we should use sbuild-adduser, but sbuild-adduser
> prints
> # setup instructions to STDERR which do not make sense in the context
> of
> # this package. Filtering STDERR is cumbersome, so we call adduser
> directly.
> adduser --quiet -- "$user" sbuild
> done
>
> # TODO: maybe add a “setup” suffix into the generated chroot name for
> easier trouble-shooting (we’ll immediately know who created the chroot
> initially)
>
> # Create a chroot if it does not already exist
> chroot="unstable-$(dpkg --print-architecture)-sbuild"
> if ! schroot -i -c chroot:${chroot} >/dev/null 2>&1
> then
> sbuild-createchroot \
> --include=eatmydata,ccache,gnupg \
> unstable \
> /srv/chroot/${chroot} \
> http://deb.debian.org/debian
>
> # At this point, sbuild-createchroot created an schroot configuration
> with a
> # random suffix, e.g. /etc/schroot/chroot.d/unstable-amd64-sbuild-pyViYe.
> As
> # sbuild-createchroot recommends, we rename that file before making
> # adjustments.
> mv /etc/schroot/chroot.d/${chroot}-* /etc/schroot/chroot.d/${chroot}
> fi
>
> # schroot config customizations:
> config=/etc/schroot/chroot.d/${chroot}
> tmp=$(mktemp ${config}-XX.dpkg-tmp)
> trap 'rm -f "${tmp}"' TERM INT EXIT QUIT
>
> grep -v -E '^(aliases|command-prefix)=' "${config}" > "${tmp}"
>
> # For convenience, treat UNRELEASED as an alias for unstable (so that
> # debian/changelog files containing UNRELEASED do not need to be modified
> before
> # building). Also sid, because it is short to type when specifying -d.
> echo "aliases=UNRELEASED,sid" >> "${tmp}"
>
> # Enable eatmydata: occasionally losing a test build is preferable over
> longer
> # build times and disk wear.
> echo "command-prefix=eatmydata" >> "${tmp}"
>
> chmod 644 "${tmp}"
> mv "${tmp}" "${config}"
>
> # Copy a modified example sbuildrc config file
> for homedir in $(getent passwd | perl -F: -nlE '$F[2] >= 1000 && $F[2] <=
> 5 && say $F[5]')
> do
> userconfig="${homedir}/.sbuildrc"
> if [ ! -e "${userconfig}" ]
> then
> (grep -v -E "^(# don't remove this|1;\$)" 
> /usr/share/doc/sbuild/examples/example.sbuildrc
> && cat /usr/share/doc/sbuild-debian-setup/sbuildrc) > "${userconfig}"
> fi
> done
>
> # bind-mount the apt archive cache into chroots, so that packages are
> downloaded
> # only once. The assumption is that users will not typically have a local
> apt
> # mirror or caching proxy.
> if ! grep -q '^/var/cache/apt/archives' /etc/schroot/sbuild/fstab
> then
> echo "/var/cache/apt/archives /var/cache/apt/archives none rw,bind 0
> 0" \
>  >>/etc/schroot/sbuild/fstab
> fi
>
> if [ ! -e "/etc/schroot/setup.d/04tmpfs" ]
> then
> echo ""
> echo "  If you can spare the RAM, you can enable building in tmpfs
> using:"
> echo ""
> echo "sudo ln -s /etc/schroot/setup-available.d/overlays-in-tmpfs
> /etc/schroot/setup.d/04tmpfs"
> echo ""
> fi
>
> exit 0
>
> > I tested this package on my notebook, which is a Debian installation on
> > which I never had sbuild installed, so I???m reasonably confident that
> the
> > package works ??? at least to the point that one gets an sbuild
> installation
> > that builds packages.
> >
> > I???d be happy about any feedback. Thanks!
>
>
> What was/is the idea of doing this in an additional packages?
> Why not in an additional script?
>
>
> Groeten
> Geert Stappers
> --
> Leven en laten leven
>



-- 
Best regards,
Michael

Bug#859867: [buildd-tools-devel] Bug#859867: Bug#859867: Please add a package which automatically configures sbuild for Debian packaging

2017-05-21 Thread Geert Stappers 
On Sat, May 20, 2017 at 11:34:37PM +0200, Michael Stapelberg wrote:
> 
> Find attached the first draft of my suggestion. I implemented it as a
> separate package purely so that I can build it more quickly, but I assume
> we???d want to fold this into src:sbuild eventually.
> 
> The resulting package (I built it using dpkg-buildpackage -b) depends on
> sbuild, schroot, debootstrap, perl-base, lintian. Upon installation, it
> will create an unstable sbuild schroot, modify its configuration, add all
> users to sbuild and create a modified ~/.sbuildrc for all users.
> 
> If you want to read through the entire behavior, I recommend the
> entrypoints debian/postinst and update-sbuild-chroots.

the debian/postinst now here inline


#!/bin/sh

set -e

#DEBHELPER#

# TODO: make this package pass piuparts

# Add to group sbuild all “dynamically allocated user accounts”; see
# https://www.debian.org/doc/debian-policy/ch-opersys.html
for user in $(getent passwd | perl -F: -nlE '$F[2] >= 1000 && $F[2] <= 5 && 
say $F[0]')
do
# Strictly speaking, we should use sbuild-adduser, but sbuild-adduser prints
# setup instructions to STDERR which do not make sense in the context of
# this package. Filtering STDERR is cumbersome, so we call adduser directly.
adduser --quiet -- "$user" sbuild
done

# TODO: maybe add a “setup” suffix into the generated chroot name for 
easier trouble-shooting (we’ll immediately know who created the chroot 
initially)

# Create a chroot if it does not already exist
chroot="unstable-$(dpkg --print-architecture)-sbuild"
if ! schroot -i -c chroot:${chroot} >/dev/null 2>&1
then
sbuild-createchroot \
--include=eatmydata,ccache,gnupg \
unstable \
/srv/chroot/${chroot} \
http://deb.debian.org/debian

# At this point, sbuild-createchroot created an schroot configuration with a
# random suffix, e.g. /etc/schroot/chroot.d/unstable-amd64-sbuild-pyViYe. As
# sbuild-createchroot recommends, we rename that file before making
# adjustments.
mv /etc/schroot/chroot.d/${chroot}-* /etc/schroot/chroot.d/${chroot}
fi

# schroot config customizations:
config=/etc/schroot/chroot.d/${chroot}
tmp=$(mktemp ${config}-XX.dpkg-tmp)
trap 'rm -f "${tmp}"' TERM INT EXIT QUIT

grep -v -E '^(aliases|command-prefix)=' "${config}" > "${tmp}"

# For convenience, treat UNRELEASED as an alias for unstable (so that
# debian/changelog files containing UNRELEASED do not need to be modified before
# building). Also sid, because it is short to type when specifying -d.
echo "aliases=UNRELEASED,sid" >> "${tmp}"

# Enable eatmydata: occasionally losing a test build is preferable over longer
# build times and disk wear.
echo "command-prefix=eatmydata" >> "${tmp}"

chmod 644 "${tmp}"
mv "${tmp}" "${config}"

# Copy a modified example sbuildrc config file
for homedir in $(getent passwd | perl -F: -nlE '$F[2] >= 1000 && $F[2] <= 5 
&& say $F[5]')
do
userconfig="${homedir}/.sbuildrc"
if [ ! -e "${userconfig}" ]
then
(grep -v -E "^(# don't remove this|1;\$)" 
/usr/share/doc/sbuild/examples/example.sbuildrc && cat 
/usr/share/doc/sbuild-debian-setup/sbuildrc) > "${userconfig}"
fi
done

# bind-mount the apt archive cache into chroots, so that packages are downloaded
# only once. The assumption is that users will not typically have a local apt
# mirror or caching proxy.
if ! grep -q '^/var/cache/apt/archives' /etc/schroot/sbuild/fstab
then
echo "/var/cache/apt/archives /var/cache/apt/archives none rw,bind 0 0" \
 >>/etc/schroot/sbuild/fstab
fi

if [ ! -e "/etc/schroot/setup.d/04tmpfs" ]
then
echo ""
echo "  If you can spare the RAM, you can enable building in tmpfs using:"
echo ""
echo "sudo ln -s /etc/schroot/setup-available.d/overlays-in-tmpfs 
/etc/schroot/setup.d/04tmpfs"
echo ""
fi

exit 0

> I tested this package on my notebook, which is a Debian installation on
> which I never had sbuild installed, so I???m reasonably confident that the
> package works ??? at least to the point that one gets an sbuild installation
> that builds packages.
> 
> I???d be happy about any feedback. Thanks!


What was/is the idea of doing this in an additional packages?
Why not in an additional script?


Groeten
Geert Stappers
-- 
Leven en laten leven