Profile support and refactoring

[Osamu Aoki]

Hi,

Thanks for response.  I think direction is getting clear.
(Also I read these codes more carefully now)

On Sat, Jan 09, 2010 at 05:04:23PM -0500, Loïc Minier wrote:
> On Sat, Jan 09, 2010, Osamu Aoki wrote:
...
>  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.

I think you are talking testlib.sh.  PBUILDER_CACHEDIR nicely into the
same convention.  I see:
  export PBUILDER_PKGLIBDIR="${PBUILDER_PKGLIBDIR:-$PBUILDER_ROOT/usr/lib/pbuilder}"
and similar in codes you are touching.  Interesting offset is given
with $PBUILDER_ROOT.  Do we need to do the same for $PBUILDER_CACHEDIR?
Since pbuilderrc is read by pbuilder-loadconfig, is this good place to
define it like:
  export PBUILDER_CACHEDIR="${PBUILDER_CACHEDIR:-$PBUILDER_ROOT/var/cache/pbuilder}"

This is redefined in pbuilder-satisfydepends-aptitude pbuilder-satisfydepends-classic
but not in -gdebi, -experimental.  I guess this is because you are not
touching -gdebi, -experimental.  

Isn't it better to have one code pbuilder-default-path and source it
where needed to be maintainable (pbuilder-loadconfig, pbuilder-checkconfig,
pbuilder-satisfydepends-*).  pbuilder-default-path is basically:

export PBUILDER_PKGLIBDIR="${PBUILDER_PKGLIBDIR:-$PBUILDER_ROOT/usr/lib/pbuilder}"
export PBUILDER_PKGDATADIR="${PBUILDER_PKGDATADIR:-$PBUILDER_ROOT/usr/share/pbuilder}"
export PBUILDER_SYSCONFDIR="${PBUILDER_SYSCONFDIR:-$PBUILDER_ROOT/etc}"
export PBUILDER_CACHEDIR="${PBUILDER_CACHEDIR:-$PBUILDER_ROOT/var/cache/pbuilder}"

pbuilderrc should use all these variables while each associated comment
indicating each default value.

> > >  pbuilder.8:
> > >     -.BI "pbuilder --create [" "options" "]"
>  (No strong opinion on listing short flags at the top; we have almost
>  none of them)

Understood.
 
> > 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)

cowbuilder and pbuilder use "--create" while pbuilder-user-mode-linux uses
"create" in manpage.

Since UML is half dead, I guess junichi's current choice is "--create" syntax.
I will use this style while keeping "create" accepted.

> > >  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.).
...

I get to understand what you are after ...

>  We want to record this setup automatically for the user.

Yes.

> >  * pbuilder-uml-checkparams (independently defined)
> >  * pdebuild-uml-checkparams (independently defined)
...
>  Not sure we have to care too much about these; basic maintenance and
>  responding to bug reports is probably enough.

I agree.
 
> >  * /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.

There are 2 types of "override".  Before and after /etc/pbuilderrc.
Since /etc/pbuilder/ directory only contains pbuilder related things,
repeating "pbuilder" for all files seems redundant.  Imitating dpkg
preinst/postinst, how about:

  /etc/pbuilder/foo.postrc
  /etc/pbuilder/foo.prerc

As for profile support:
>  I wouldn't mind if we don't support profiles in UML mode for now.

I agree.

> > 1. re-factor code using "$PBUILDER_CACHEDIR" etc.
> 
>  Looks good
> 
> >    1.2 standard loading program reads configuration data in the following order
...
I got your point on $PBUILDER_PKGDATADIR"/${PBPROFILE}/pbuilderrc and
$HOME/.pbuilderrc (or likes).  Let's only take care as backward
compatibility.  (Technically, you may be able to run pbuilder like
program as user but with resource demanded, I agree person running this
program is expected to have root access thus /etc is good enough. Maybe
document this deprecated status.)

In light of all these, how about sourcing in following order:

  ${PBPROFILE:+PBUILDER_SYSCONFDIR/pbuilder/$PBPROFILE.prerc}
  ${PBUILDER_PKGDATADIR}/pbuilderrc                (system wide)
  ${PBUILDER_SYSCONFDIR}/pbuilderrc                (created by debconf)
     this contains MIRRORSITE only as default.
  ${PBPROFILE:+$PBUILDER_SYSCONFDIR/pbuilder/$PBPROFILE.postrc} 
  ${HOME}/.pbuilderrc

> > 2. merge *-uml into main code. (next step?)
> 
>  IMO can be done separately.

OK.  Let's not touch it now.
 
> > 3. Add easy access to build environments.
...
> >    /usr/share/pbuilder/sarge/pbuilderric
...
>  Please, no!  

I retract this itself.  But we are thinking very much in the same thing.
(I was thinking to let user "cp" those template profiles together with
MIRRORSITE value.  It may have been a bad idea.  My older patch had
different approach.)

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

With the current source, there is no mechanism to switch default
MIRRORSITE set by system in ${PBUILDER_PKGDATADIR}/pbuilderrc and
MIRRORSITE set by debconf in ${PBUILDER_SYSCONFDIR}/pbuilderrc.  In
order to enable such thing, thee should be some new mechanism to set
MIRRORSITE depending on --distribution.

> > Use ${URL_DEBIAN_ARCHIVE} and ${URL_UBUNTU_ARCHIVE} in these files.
...
>  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.

As I reread codes, I realized the current variable name is MIRRORSITE.

It is handled by function installaptlines in pbuilder-modules.  Quite
honestly, this is one of the most complicate logic part for me.  This is
because this relies too much on conditional branching.  Why not make
this logic more data driven.  

So function installaptlines in pbuilder-modules should source following
corresponding file (if exist) to set default for $MIRRORSITE and
optionally $OTHERMIRROR before sourcing */pbuilderrc etc.

  ${PBUILDER_PKGDATADIR}/distribution/experimental
  ${PBUILDER_PKGDATADIR}/distribution/sid
  ${PBUILDER_PKGDATADIR}/distribution/squeeze
  ${PBUILDER_PKGDATADIR}/distribution/lenny
  ${PBUILDER_PKGDATADIR}/distribution/lenny-backports
  ${PBUILDER_PKGDATADIR}/distribution/volatile
  ${PBUILDER_PKGDATADIR}/distribution/volatile-sloppy
  ${PBUILDER_PKGDATADIR}/distribution/kermic
  ${PBUILDER_PKGDATADIR}/distribution/jaunty

(For experimental, lenny-backports, ..., we set $OTHERMIRROR to these
site while $MIRRORSITE should point to main site like sid or lenny.)

These should also set MIRRORSITE_TYPE to DEBIAN or UBUNTU or any other
values.

We can comment out $MIRRORSITE and $OTHERMIRROR in
${PBUILDER_PKGDATADIR}/pbuilderrc.

Problem is how to cope with debconf result in
${PBUILDER_SYSCONFDIR}/pbuilderrc.

This needs some tweaking in debian/pbuilder.config which does:

 * If MIRRORSITE of the host system contains [Dd]ebian/$ 
   set SYSTEM_TYPE_BY_DEBCONF=DEBIAN
  
 * If MIRRORSITE of the host system contains ubuntu/$ 
   set SYSTEM_TYPE_BY_DEBCONF=UBUNTU
  
 * If MIRRORSITE of the host system contains fobar/$ 
   set SYSTEM_TYPE_BY_DEBCONF=OTHER

And using this with $MIRRORSITE obtained in pbuilder.config, we can make
${PBUILDER_SYSCONFDIR}/pbuilderrc as if:

cat <<EOF ${PBUILDER_SYSCONFDIR}/pbuilderrc
If [ "\$MIRRORSITE_TYPE" = ${SYSTEM_TYPE_BY_DEBCONF} ]; then
   MIRRORSITE="$MIRRORSITE"
fi
EOF

(Actual code is sed script as in pbuilder.config)

Hmmmm... it is a plan.  This should work and expandable to other archive
such as archive.debian.net etc :-)

As for "support of multiple APT repos", I think this can be done simply
creating one template base system with one of --distribution and tweak
it to your need by:
 * --login --save-after-login and edit as you wish.
 * If automation is needed, --exec --save-after-exec, I guess.

It is quite straight forward. 
 
>  Ok; will do
> 
>  taking the plane back to home now

Take care...