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

[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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: signature
URL: <http://lists.alioth.debian.org/pipermail/buildd-tools-devel/attachments/20170802/dbf2e33c/attachment.sig>