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

[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] <= 59999 && 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 59999 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}-XXXXXX.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] <= 59999 && 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 cluttered
over time?

I also don't like the idea of just randomly mixing the /var/cache/apt/archives
directory of the host with multiple sbuild instances which might be running at
the same time. Just imagine sbuild running for different distributions which
have packages with the same name and version but different content. That is
just calling for trouble.

I think a much better option is to use something like apt-cacher-ng. This new
package could easily depend on it and then use that as its default mirror as
already explained 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 think in summary much of what this script does can be achieved by:

 - improving existing documentation
 - 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?

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/20170524/15ca2790/attachment.sig>