[PATCH 1/9] add profile support to pbuilder

[Osamu Aoki]

Hi,

Thanks for the review of non-functioning codes :-)

Since then, I cleaned source a bit to get it to work and learning 
"git rebase -i master".

On Fri, Jan 22, 2010 at 11:55:58PM +0100, Loïc Minier wrote:
> On Sun, Jan 17, 2010, Loïc Minier wrote:
> > +function preload_config () {
> > +    while [ -n "$1" ]; do 
> > +	case "$1" in 
> > +	    --profile|-p)
> > +		PROFILE="$2";
> > +		shift; shift;
...
> > +	esac
> > +    done
> > +}

>  I find this is a bit heavy just for the --profile concept; it is a bit
>  like --configfile which doesn't require this; the --configfile
>  "EXTRA_CONFIGFILE" concept is not pretty either though.   :-/

I understand your concern.  But there seems to be 2 choices:
 * make this --profile to be the first option before COMMANDS.
 * make this --profile to be the option losded before all other options.
 
>  I think it would be ok if you would just source the profile at
>  --profile time.  

You mean to keep /usr/share/pbuilder/pbuilderrc and /etc/pbuilderrc and
overide setting of these when --profile is found.  Hmmm...

> This will override things like ~/.pbuilderrc, but then
>  --profile is a command-line argument, and things which are set in the
>  profile config file should not conflict too much with things in
>  ~/.pbuilderrc.

But then we loose custom configuration capability ... let me think a bit
more.

> > --- a/pbuilder-loadconfig
> > +++ b/pbuilder-loadconfig
> > @@ -18,10 +18,17 @@
> >  #   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> >  
> >  export PBUILDER_PKGDATADIR="${PBUILDER_PKGDATADIR:-$PBUILDER_ROOT/usr/share/pbuilder}"
> > -export PBUILDER_SYSCONFDIR="${PBUILDER_SYSCONFDIR:-$PBUILDER_ROOT/etc}"
> > +export PBUILDER_SYSCONFDIR="${PBUILDER_SYSCONFDIR:-$PBUILDER_ROOT/etc/pbuilder}"
> > +export PBUILDER_CACHEDIR="${PBUILDER_CACHEDIR:-$PBUILDER_ROOT/var/cache/pbuilder}"
> 
>  I see you reverted that one later.  I tried sticking to GNU style vars;
>  see info Automake "Standard Directory Variables", but there are more
>  vars than just these such as pkglibdir, pkgincludedir etc.

I understand you are talking PBUILDER_SYSCONFDIR. That is /etc now.
Unless I revert it, test suites rejected.

If there is something better for PBUILDER_CACHEDIR, let me know.

Just tell me what are the better choices in accepted styles.

> > -for RCFILE in "$PBUILDER_PKGDATADIR"/pbuilderrc "$PBUILDER_SYSCONFDIR"/pbuilderrc "$HOME"/.pbuilderrc; do
> > -    if [ -f "$RCFILE" ]; then 
> > +for RCFILE in \
> > +    "${PBUILDER_PKGDATADIR}/pbuilderrc" \
> > +    "${PBUILDER_SYSCONFDIR}/pbuilderrc" \
> > +    "${PROFILE:+$PBUILDER_SYSCONFDIR/$PROFILE.rc}" \
> > +    "${HOME}/.pbuilderrc"; do
> > +    if [ -z "$RCFILE" ]; then 
> > +        : # If no profile set, do nothing and normal
> > +    elif [ -f "$RCFILE" ]; then 
> 
>  (See above on profile config handling.)

Your idea is sourcing while --profile is proessed in normal way (not
preloading).  But key contribution of --profile is not this part.

It defines $PROFILE which changes from
  /var/cache/pbuilder/
to
  /var/cache/pbuilder/<profile-value>/

That is the core of my patches.

> > --- a/pbuilder.8
> > +++ b/pbuilder.8
> > @@ -399,6 +399,24 @@ This is useful when you keep a aptcache directory for each distribution
> >  and want to keep the size of the aptcache down.
> >  
> >  .TP
> > +.BI "\-\-profile|\-p " "profile"
> > +Move default pbuilder data cache directories from thier normal
> > +.B "/var/cache/pbuilder/"
> > +to
> > +.BI "/var/cache/pbuilder/" "profile" "/"
> 
>  "their"
> 
>  I think the description should be generic instead of specifically
>  pointing at "/var/cache/pbuilder/".  For instance you could mention
>  that profile allow maintaining multiple build environments with
>  different APT configurations, separate build dirs etc.

I see.  Your idea of --profile is simply source extra configuration
file. That is where the difference comes in.

My way of --profile changes default behaviors in group.  Then standard
configuration and /etc/pbuilder/<profile-name>.rc can override it.

I wanted:

$ sudo pbuilder --create --profile foo --distribution jaunty
$ sudo pbuilder --login --profile foo --save-after-login
 ... customize like changing apt line to karmic
$ sudo pbuilder --update --profile foo
$ cd path/to/source/
$ pdebuild --profile foo
$ cd /var/cache/foo/result

I did not want to do too much pbuilder specific configuration.

> > --- a/pbuilderrc
> > +++ b/pbuilderrc
> > @@ -1,17 +1,32 @@
> > +# Default values set by the calling program:
> > +#   ${PROFILE}             ""
...
>  Probably not the ideal place to have documentation; also, let's avoid
>  duplicating these values -- these are set in multiple places already.

Understood.  Fixed.
 
>  Other changes look good.
> 
> > --- a/pbuilderrc.5
> > +++ b/pbuilderrc.5
> > +This moves default pbuilder data cache directories from thier normal
> > +.B "/var/cache/pbuilder/"
> 
>  Ditto; and I think I'd document "PROFILE" as a valid way to set the
>  default profile.  You might want to set PROFILE to "sid" by default and
>  override on the command-line.

Although we can set it, I fail to understand intent.  Maybe we are
expectingdifferent thing with PROFILE.

Thanks.  I will include ones I understood and publish nit just concept
but working set of patch later.

Osamu