Profile support and refactoring

[Loïc Minier]

On Sat, Jan 09, 2010, Osamu Aoki wrote:
> Good idea. (Actually, there are many // in path even without this.  This
> type of code cleaning may be good idea across pbuilder.)

 Agreed; can happen progressively or as a separate effort

> I thought about it too.  But using expression like "some defaults
> settings" are quite vague.  What is wrong to say "all default settings
> containing '/var/cache/pbuilder' will be moved to
> '/var/cache/pbuilder/${PBPROFILE}'?

 That is fine; thanks

> By the way, What do you think about factoring some related variables like:
> PBUILDER_CACHEDIR=/var/cache/pbuilder
> 
> If this is defined in /usr/share/pbuilder/pbuilderrc and used across
> code consistently across pbuilder, it becomes cleaner. (I did not do
> this to minimize patch size for easy initial code review.) I will get
> back to this below.

 I fully agree; in fact, I've started changing some uses of e.g.
 /usr/lib/pbuilder because I needed to override the directory at runtime
 (for the testsuite).  See PBUILDER_PKGLIBDIR PBUILDER_PKGDATADIR
 PBUILDER_SYSCONFDIR etc.

> >  pbuilder.8:
> >     -.BI "pbuilder --create [" "options" "]"
> >     +.BI "pbuilder [--mode pbmode] --create [" "options" "]"
> >  don't think you actually need to mention that here; just mention the
> >  flag later.  We can mention the new feature in the debian/NEWS to get
> >  some attention.
> 
> But isn't it the purpose of manpage to explain syntax of command here.
> Dropping these altogether seems bad idea.  But I understand these looks
> crowded.

 It is, but the many long flags aren't all listed at the top, so for
 consistency the new flag shouldn't; I think a full list of flags would
 be useless because unreadable.

> >  pbuilder/pbuilderrc: I think the flag should be shorter because it will
> >  be used frequently; how about "-m" for mode, or "-p" for profile?
> 
> They are.  As I come to think of it, maybe manpage examples should use
> short form as main form.....
> 
>  +.BI "pbuilder [--p profile] --create [" "options" "]"
> 
> Then it is easier to read and explain.

 (No strong opinion on listing short flags at the top; we have almost
 none of them)

> Actually, I always wondered which syntax is recommended and which is
> deprecated backward compatibility.  I.e., "--create" or "create".
> Although both seem to be allowed, manpage list one of them.

 (No idea)

> >  So if you check my initial proposal, I propose *recording* more things
> >  in the mode / profile: the target distribution for instance.  I think
> >  that "pbuilder -p foo create --distribution squeeze" should create a
> >  foo.pbuilderrc which sets DISTRIBUTION=squeeze so that subsequent
> >  commands keep using that (or --mirror, etc.).  What do you think of
> >  this part?  Is this something you would be interested in implementing?
> >  Or if you have no interest, would you mind if I did it?
> 
> Although I mentioned some to BTS, let me go over.
> 
> I avoided this in order to make "-p profile" feature is minimal and
> easily accepted.  As I said, --configfile option can be used with
> "create" so making new feature like this is minimal advantage if one
> wishes to tweak /etc/apt/sources.list to add testing-security-updates
> etc.  These can be done better by accessing cheroot directly by user or
> via hook script using the generic infrastructure.  Too much special
> features will not benefit user.

 I don't see that as a special feature; I see that as doing the expected
 thing.  Here's my logic: *Why* does one create a profile?  because the
 pbuilder setup differs.  We want to record this setup automatically for
 the user.

 I think we can close dozen of bugs by implementing this; in fact I have
 a long list of tabs open on these bugs in my browser to remind me of
 this  :)

 However, I'm happy to complete this support if you like.

> Let's see where they are loaded from:
> 
>  * pbuilder-loadconfig
>    "$PBUILDER_PKGDATADIR"/pbuilderrc
>    "$PBUILDER_SYSCONFDIR"/pbuilderrc
>    "$HOME"/.pbuilderrc  
> 
>  * pbuilder-checkparams (source pbuilder-loadconfig)
> 
>  * pdebuild-checkparams (source pbuilder-loadconfig)

 Ack

>  * pbuilder-uml-checkparams (independently defined)
>    /usr/share/pbuilder/pbuilder-uml.conf
>    /etc/pbuilder/pbuilder-uml.conf 
>    ${HOME}/.pbuilderrc
>    ${HOME}/.pbuilder-umlrc
> 
>  * pdebuild-uml-checkparams (independently defined)
>    /usr/share/pbuilder/pbuilder-uml.conf 
>    /etc/pbuilder/pbuilder-uml.conf 
>    ${HOME}/.pbuilderrc 
>    ${HOME}/.pbuilder-umlrc

 Not sure we have to care too much about these; basic maintenance and
 responding to bug reports is probably enough.

>  * /etc/pbuilder
>    it contains /etc/pbuilder/pbuilderrc -> ../pbuilderrc
>    (This looks better place to keep real config)

 I think that should be the default system config for all profiles (or
 the default profile).

 I would like to have /etc/pbuilder/foo.pbuilderrc files which would
 override /etc/pbuilderrc.

> Somewhat inconsistent as I see.  Also having similar code for normal and uml
> version is prone for inconsistency.

 Agreed

> As I read source for *-uml, I realize refactoring these configuration loading
> scheme may be good idea to make -p work for *-uml.  (I now see *-uml can not
> use profile feature yet.)

 I wouldn't mind if we don't support profiles in UML mode for now.  UML
 is almost dead upstream; I recently looked a bit into it, and it seems
 it's still building, but most people are using kvm or qemu instead now.
 When I used it some years ago, it was terribly slow too.

> 1. re-factor code using "$PBUILDER_CACHEDIR" etc.

 Looks good

>    1.2 standard loading program reads configuration data in the following order
>    "$PBUILDER_PKGDATADIR"/pbuilderrc
>    "$PBUILDER_SYSCONFDIR"/pbuilder/pbuilderrc
>    "$HOME"/.pbuilder/pbuilderrc
>    "$PBUILDER_PKGDATADIR"/${PBPROFILE}pbuilderrc           (if -n ${PBPROFILE})
>    "$PBUILDER_SYSCONFDIR"/pbuilder/${PBPROFILE}pbuilderrc  (if -n ${PBPROFILE})
>    "$HOME"/.pbuilder/${PBPROFILE}pbuilderrc                (if -n ${PBPROFILE})

 Hmm.  I don't think we need
 "$PBUILDER_PKGDATADIR"/${PBPROFILE}pbuilderrc; only packages should
 ship files in this place and profiles are created by end-users.  Also,
 I hate ~/.pbuilderrc; I think pbuilder is a system-service because of
 the directories it uses, the root permission it requires, and the
 insecurity of the chroot setup under a home dir.  So I think it might
 make sense to keep the current order and add
 etc/pbuilder/${PBPROFILE}.pbuilderrc after etc/pbuilderrc

> 2. merge *-uml into main code. (next step?)

 IMO can be done separately.

> 3. Add easy access to build environments.
> 
> If we do refactoring as described above, then we can also ship template
> configurations for each typical pbuilder scenario/profile:
>    /usr/share/pbuilder/sarge/pbuilderrc
>    /usr/share/pbuilder/lenny/pbuilderrc
>    /usr/share/pbuilder/sid/pbuilderrc
>    /usr/share/pbuilder/experimental/pbuilderrc
>    /usr/share/pbuilder/kermic/pbuilderrc
>    /usr/share/pbuilder/jaunty/pbuilderrc

 Please, no!  That's *exactly* why I want to record what you pass on the
 command-line in the profile!  pbuilder should just use proper defaults
 if you pass this or that distribution, i.e. if you pass a Debian distro
 it should use a Debian mirror and an Ubuntu distro an Ubuntu mirror.

 So for instance:
    pbuilder -p karmic create --distribution karmic
 would default to using an Ubuntu mirror and would create
 /etc/pbuilder/karmic.pbuilderrc with DISTRIBUTION=karmic.

 If instead you are interested in having e.g. two pbuilder setups for
 main and main + contrib, you could:
    pbuilder -p contrib create --components "main contrib"
 and that would create /etc/pbuilder/contrib.pbuilderrc with
 COMPONENTS=main contrib.

 If instead you want to build karmic i386 under amd64, you could:
    pbuilder -p karmic-i386 create --distribution karmic \
        --architecture i386
 etc.

> Use ${URL_DEBIAN_ARCHIVE} and ${URL_UBUNTU_ARCHIVE} in these files.
> Define as  ${URL_DEBIAN_ARCHIVE:-some default value} and
> ${URL_UBUNTU_ARCHIVE:-some default value} in
> /usr/share/pbuilder/pbuilderrc to allow local customization across all
> releases, so we can override it from /etc/pbuilder/pbuilderrc or
> $HOME/.pbuilder/pbuilderrc.  (The order of configuration file parsing is
> important for this to work.)

 Sure.  Well probably you want _MIRROR instead of URL_ARCHIVE, but
 that's fine.  The way I see it, we should change the defaults code to
 set MIRROR depending on the input.

> Now we can do
> 
> $ pbuilder -p hardy create
>   ...
> 
> $ pbuilder -sarge create --uml
> 
> This should be much easier than reading manual and setting all arguments
> to pbuilder.

 Except this assumes one profile for one dist: it would be hard to
 extend the model of dist == profile for complex setups such as
 backports, or different architectures.

> As for "support of multiple APT repos", I think this can get to
> complicated and most easily achieved by (as I discussed):
>  * copying similar configuration from /usr/share/pbuilder/*/pbuilderrc to
>    /etc/pbuilder/*/pbuilderrc 
>  * --login --save-after-login and edit as you wish.
>  * If automation is needed, --exec or is another way, I guess.

 Yeah, that's harder; didn't give much thought into that yet.

> Please do it with "profile-support" branch after looking at latest code
> so the merged code looks clean.  I appreciate your help.  (When you are
> removing osamu branch at shared host, give us fair warning.)

 Ok; will do

 taking the plane back to home now

   Thanks for the write up!
-- 
Loïc Minier