Hi,

I've looked through this patch. I have a few questions for you.

> diff -urN pbuilder-0.189/pbuilder-buildpackage 
> pbuilder-0.189+jackyf1/pbuilder-buildpackage
> --- pbuilder-0.189/pbuilder-buildpackage      2009-05-31 04:41:04.000000000 
> +0300
> +++ pbuilder-0.189+jackyf1/pbuilder-buildpackage      2009-09-27 
> 22:02:57.288519614 +0300
> @@ -21,11 +21,13 @@
>  export LC_ALL=C
>  set -e
>  
> +PACKAGENAME=$1
> +shift
> +
>  . /usr/lib/pbuilder/pbuilder-checkparams
>  . /usr/lib/pbuilder/pbuilder-runhooks
>  . /usr/lib/pbuilder/pbuilder-buildpackage-funcs
>  
> -PACKAGENAME="$1"
>  if [ ! -f "$PACKAGENAME" ]; then
>      log "E: Command line parameter [$PACKAGENAME] is not a valid .dsc file 
> name"
>      exit 1;

Why ?

> diff -urN pbuilder-0.189/pbuilder-buildpackage-funcs 
> pbuilder-0.189+jackyf1/pbuilder-buildpackage-funcs
> --- pbuilder-0.189/pbuilder-buildpackage-funcs        2009-02-26 
> 07:33:11.000000000 +0200
> +++ pbuilder-0.189+jackyf1/pbuilder-buildpackage-funcs        2009-09-27 
> 22:37:20.494949826 +0300
> @@ -37,6 +37,11 @@
>       yes) BUILDOPT="--binary-arch";;
>       *) ;;
>      esac
> +
> +     if [ -n "$USE_CUPT" ]; then
> +             
> PBUILDERSATISFYDEPENDSCMD=/usr/lib/pbuilder/pbuilder-satisfydepends-cupt
> +     fi
> +
>      if "$PBUILDERSATISFYDEPENDSCMD" --control "$1" --chroot "${BUILDPLACE}" 
> --internal-chrootexec "${CHROOTEXEC}" "${BUILDOPT}" ; then
>       :
>      else
> @@ -50,7 +55,12 @@
>      fi
>      # install extra packages to the chroot
>      if [ -n "$EXTRAPACKAGES" ]; then 
> -     $CHROOTEXEC usr/bin/apt-get -y --force-yes install ${EXTRAPACKAGES}
> +             if [ -n "$USE_CUPT" ]; then
> +                     PACKAGE_MANAGER_COMMAND="echo 'y' | /usr/bin/cupt 
> --no-auto-remove"
I think you should fix cupt to accept -y.

> +             else
> +                     PACKAGE_MANAGER_COMMAND="/usr/bin/apt-get -y 
> --force-yes"
> +             fi
> +             $CHROOTEXEC sh -c "$PACKAGE_MANAGER_COMMAND install 
> ${EXTRAPACKAGES}"
>      fi
>  }

I'd rather have cupt wrapper accepting same command-line as apt-get than adding 
if's here...
> diff -urN pbuilder-0.189/pbuilder-updatebuildenv 
> pbuilder-0.189+jackyf1/pbuilder-updatebuildenv
> --- pbuilder-0.189/pbuilder-updatebuildenv    2009-06-19 17:24:13.000000000 
> +0300
> +++ pbuilder-0.189+jackyf1/pbuilder-updatebuildenv    2009-09-27 
> 22:00:41.283468835 +0300
> @@ -34,28 +34,46 @@
>  extractbuildplace
>  $TRAP umountproc_cleanbuildplace_trap exit sighup
>  
> +recover_aptcache
> +
> +if [ -n "$USE_CUPT" ]; then
> +     if ! $CHROOTEXEC /usr/bin/dpkg -l | grep "^ii" | grep cupt; then
> +             log "I: installing cupt package manager";
> +             $CHROOTEXEC apt-get update
> +             $CHROOTEXEC apt-get -y install cupt
> +     fi
> +     PACKAGE_MANAGER=/usr/bin/cupt
> +     PACKAGE_MANAGER_COMMAND="echo 'y' | /usr/bin/cupt -o 
> apt::get::allowunauthenticated=1"
> +else
> +     PACKAGE_MANAGER=/usr/bin/apt-get
> +     PACKAGE_MANAGER_COMMAND="/usr/bin/apt-get -y --force-yes"
> +fi
> +
>  loadhooks
>  log "I: Refreshing the base.tgz "
>  log "I: upgrading packages"
> -$CHROOTEXEC /usr/bin/apt-get update
> +$CHROOTEXEC $PACKAGE_MANAGER update
>  if [ -n "$REMOVEPACKAGES" ]; then
>      $CHROOTEXEC /usr/bin/dpkg --purge $REMOVEPACKAGES
>  fi
> -recover_aptcache
>  
>  $TRAP saveaptcache_umountproc_cleanbuildplace_trap exit sighup
> -$CHROOTEXEC /usr/bin/apt-get -y --force-yes "${force_confn...@]}" 
> dist-upgrade
> +$CHROOTEXEC sh -c "$PACKAGE_MANAGER_COMMAND -o 
> DPkg::Options::=--force-confnew dist-upgrade"

I don't generally like this change to 'sh -c' because it will need another layer
of having to care about quoting.

I guess you should fix cupt.

>  # autoremove: Ignore error in case of etch because apt in etch doesn't
>  # support autoremove. TODO: Do not ignore error when etch is no longer
>  # supported.
> -$CHROOTEXEC /usr/bin/apt-get -y --force-yes autoremove || true
> -$CHROOTEXEC /usr/bin/apt-get -y --force-yes install build-essential dpkg-dev 
> apt aptitude $EXTRAPACKAGES
> +if [ -z "$USE_CUPT" ]; then
> +     $CHROOTEXEC /usr/bin/apt-get -y --force-yes autoremove || true
> +fi
> +
> +FULL_COMMAND="$PACKAGE_MANAGER_COMMAND install build-essential dpkg-dev apt 
> aptitude $EXTRAPACKAGES"
> +$CHROOTEXEC sh -c "$FULL_COMMAND"
>  save_aptcache
>  
>  # optionally auto-clean apt-cache
>  if [ "${AUTOCLEANAPTCACHE}" = "yes" -a -n "$APTCACHE" ]; then
>      log "I: Cleaning the cached apt archive"
> -    $CHROOTEXEC /usr/bin/apt-get autoclean || true
> +    $CHROOTEXEC $PACKAGE_MANAGER autoclean || true
>      find "$APTCACHE/" -maxdepth 1 -name \*.deb | \
>       while read A; do
>           if [ ! -f "$BUILDPLACE/var/cache/apt/archives/"$(basename "$A") -a \
> @@ -71,7 +89,7 @@
>  unloadhooks
>  
>  umountproc
> -$CHROOTEXEC /usr/bin/apt-get clean || true
> +$CHROOTEXEC $PACKAGE_MANAGER clean || true
>  
>  $TRAP cleanbuildplace_trap exit sighup
>  if [ ! "${INTERNAL_BUILD_UML}" = "yes" ]; then



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to