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]
> > <= 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.
> 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
