Hi Junichi, Junichi Uekawa wrote: > 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 ? Because unless I do that, /usr/lib/pbuilder/pbuilder-checkparams will fail to recognize '--cupt' param, as it will see only the name of .dsc to build.
>> 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. Well, I don't have to, however I will. But anyway, what's wrong with current variant? > >> + 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... Why have I? Extra lines? Cupt is other package manager, and though it has similar commands and options, but it doesn't need to follow apt-get precisely. >> 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. However you have several other places in code with 'sh -c' here and there. > I guess you should fix cupt. Why? I will probably implement '-y' switch later, but again, I see nothing wrong in having "echo 'y'", at least in starter patch. -- Eugene V. Lyubimkin aka JackYF, JID: jackyf.devel(maildog)gmail.com C++/Perl developer, Debian Developer
signature.asc
Description: OpenPGP digital signature