Hi, At Wed, 30 Dec 2009 17:28:18 +0100, Loïc Minier wrote: > > [1 <text/plain; iso-8859-1 (8bit)>] > tags 516625 + patch > stop > > Hi > > So I poked this further and did a more complete IPC prototype in shell > (foo.sh, attached) and integrated that into pbuilder in the attached > hackish patch. > > It could be the way forward, but it touches many many things, and it's > particularly fragile stuff. I've been bitten by obscure fd issues a > bunch of times while implementing this. > > Things which remain to be done for this patch: > - implement support in more backends; in particular qemubuilder isn't > supported ATM; I didn't look into it at all; I think cowbuilder works > but didn't check whether anything is closing fds along the way > - move the "-C 5" sudo arg which prevent the fd 4 and 5 from being > closed to some sudo specific place > - proper cleanup (trap) of the tmpdir which holds the fifo > - actually save the ARCHITECTURE in some file or fifo; because it's set > in a subshell, the main process which would use this info does not > have it > - handle the pdebuild-internal code path as well > - do something for the log file which also has the architecture name > > Overall, I think I would love changing pbuilder to have a client/server > model, I think it would allow a much more generic approach, but it is a > quite big amount of work, even after this poc. > > I think for now I'll just add an --architecture flag to pdebuild and > pbuilder (for create). I hope that pbuilder profiles will help avoid > this issue entirely.
Note that qemubuilder is somewhat already client-server model, because we don't quite have a good communication path between inside the qemu session and outside the qemu session. Inside qemu, it's a shell script using stdio (which is hooked up with serial console.) Outside qemu, it's a C program which invokes qemu with appropriate parameter. qemubuilder.c looks for pattern of 'END OF WORK EXIT CODE=' which is output from the shell script inside the qemu session. (used in obtaining the exit code and shutdown). I think the IPC doesn't have to be bidirectional, so in theory pdebuild could send something out in stdout with a special keyword. Having a server/client model is nice as well, but probably something overkill for this specific bug. Let me know what you think. > > Bye > > On Wed, Dec 30, 2009, Loïc Minier wrote: > > On Sun, Nov 29, 2009, Junichi Uekawa wrote: > > > I was thinking along the lines of running something inside the chroot > > > as part of build process, and passing the output back to pdebuild, and > > > using that to get the correct value. > > > > > > However, that's a bit of work to implement, so I have so far deferred > > > doing this. > > > > Yeah it's not trivial; especially since there are two code pathes in > > pdebuild (use-internal versus regular) and multiple builders. Probably > > a nicer design would be to start some kind of pbuilder server in the > > build env and feed commands to it so that we could send a > > "get_architecture" command independently of running the build. > > -- > Loïc Minier > [2 foo.sh <application/x-sh (quoted-printable)>] > > [3 0001-Hackish-IPC-to-query-build-architecture.patch <text/x-diff; us-ascii > (7bit)>] > >From 651249236cb115036b83af9b9f37dd79357f64a8 Mon Sep 17 00:00:00 2001 > From: =?utf-8?q?Lo=C3=AFc=20Minier?= <[email protected]> > Date: Wed, 30 Dec 2009 17:19:42 +0100 > Subject: [PATCH] Hackish IPC to query build architecture > > --- > pbuilder-buildpackage | 26 ++++++++++++++++++++++++-- > pbuilder-buildpackage-funcs | 27 +++++++++++++++++++++++++++ > pdebuild | 34 ++++++++++++++++++++++++++++++++-- > 3 files changed, 83 insertions(+), 4 deletions(-) > > diff --git a/pbuilder-buildpackage b/pbuilder-buildpackage > index 293eecf..062620e 100755 > --- a/pbuilder-buildpackage > +++ b/pbuilder-buildpackage > @@ -24,6 +24,7 @@ set -e > . /usr/lib/pbuilder/pbuilder-checkparams > . /usr/lib/pbuilder/pbuilder-runhooks > . /usr/lib/pbuilder/pbuilder-buildpackage-funcs > +flip_ipc > > PACKAGENAME="$1" > if [ ! -f "$PACKAGENAME" ]; then > @@ -111,13 +112,34 @@ else > exit 1; > fi > > +flip_ipc > +while read command; do > + case "$command" in > + get_host_arch) > + $CHROOTEXEC dpkg-architecture -qDEB_HOST_ARCH > + ;; > + build) > + # XXX should call a function instead > + break > + ;; > + *) > + echo "Invalid command $command" > + ;; > + esac > +done > +flip_ipc > + > log "I: Building the package" > > executehooks "A" > > -DPKG_COMMANDLINE="dpkg-buildpackage -us -uc ${DEBEMAIL:+\"-e$DEBEMAIL\"} > $DEBBUILDOPTS" > - > ( > + # close IPC channels in subshell only > + flip_ipc > + close_ipc > + > + DPKG_COMMANDLINE="dpkg-buildpackage -us -uc ${DEBEMAIL:+\"-e$DEBEMAIL\"} > $DEBBUILDOPTS" > + > : Build process > if [ -n "$TWICE" ]; then > DPKG_COMMANDLINE="$DPKG_COMMANDLINE && $DPKG_COMMANDLINE" > diff --git a/pbuilder-buildpackage-funcs b/pbuilder-buildpackage-funcs > index 023dbca..f21eee2 100644 > --- a/pbuilder-buildpackage-funcs > +++ b/pbuilder-buildpackage-funcs > @@ -104,3 +104,30 @@ function createbuilduser () { > unset LOGNAME || true > fi > } > + > +# use stdout and stdin for IPC and backup the original ones as fd 3 and 4 > +open_ipc() { > + if [ -n "$PBUILDER_IPC" ]; then > + return > + fi > + exec 3>&1 4<&0 > + export PBUILDER_IPC=1 > +} > + > +# exchange IPC fds 3 and 4 with stdin and stdout > +flip_ipc() { > + if [ -z "$PBUILDER_IPC" ]; then > + return > + fi > + exec 5>&3- 3>&1- 1>&5- 5<&4- 4<&0- 0<&5- > +} > + > +# connect original stdin and stdout and close IPC channels > +function close_ipc() { > + if [ -z "$PBUILDER_IPC" ]; then > + return > + fi > + exec 1>&3- 0<&4- > + unset PBUILDER_IPC > +} > + > diff --git a/pdebuild b/pdebuild > index f9e79b5..da12efc 100644 > --- a/pdebuild > +++ b/pdebuild > @@ -31,7 +31,6 @@ fi; > > PKG_SOURCENAME=$(dpkg-parsechangelog|sed -n 's/^Source: //p') > PKG_VERSION=$(dpkg-parsechangelog|sed -n 's/^Version: \(.*:\|\)//p') > -ARCHITECTURE=$(dpkg-architecture -qDEB_HOST_ARCH) > CHANGES="${PKG_SOURCENAME}_${PKG_VERSION}_${ARCHITECTURE}.changes" > > if [ -z "${PBUILDER_BUILD_LOGFILE}" ]; then > @@ -43,6 +42,24 @@ fi > export BUILDRESULTUID=$(id -u) > export BUILDRESULTGID=$(id -g) > > +# use fd 3 and 4 for IPC > +function open_ipc() { > + if [ -n "$PBUILDER_IPC" ]; then > + return > + fi > + exec 3>&1 4<&0 > + export PBUILDER_IPC=1 > +} > + > +# connect original stdin and stdout and close IPC channels > +function close_ipc() { > + if [ -z "$PBUILDER_IPC" ]; then > + return > + fi > + exec 1>&3- 0<&4- > + unset PBUILDER_IPC > +} > + > if [ "${USE_PDEBUILD_INTERNAL}" = 'yes' ]; then > ${PBUILDERROOTCMD} ${PDEBUILD_PBUILDER} --execute > ${extra_configfi...@]/#/--configfile } --bindmounts $(readlink -f ..) "$@" -- > /usr/lib/pbuilder/pdebuild-internal ${PWD} --debbuildopts "" --debbuildopts > "${DEBBUILDOPTS}" --uid "${BUILDRESULTUID}" --gid "${BUILDRESULTGID}" > --pbuildersatisfydepends "$PBUILDERSATISFYDEPENDSCMD" > if [ -d "${BUILDRESULT}" ]; then > @@ -59,7 +76,20 @@ else > log "W: Unmet build-dependency in source" > fi > echo "dpkg-buildpackage -S -us -uc -r${BUILDSOURCEROOTCMD} > $DEBBUILDOPTS" | perl -pe 's/(^|\s)-[bB](\s|$)/$1$2/g' | /bin/bash > - ${PBUILDERROOTCMD} ${PDEBUILD_PBUILDER} --build > ${extra_configfi...@]/#/--configfile } --buildresult "${BUILDRESULT}" > --debbuildopts "" --debbuildopts "${DEBBUILDOPTS}" "$@" > ../"${PKG_SOURCENAME}_${PKG_VERSION}".dsc > + > + open_ipc > + helper() { > + echo get_host_arch > + read ARCHITECTURE > + echo build > + } > + d="`mktemp -dt`" > + fifo="$d/fifo" > + mkfifo "$fifo" > + helper <"$fifo" | ${PBUILDERROOTCMD} -C 5 ${PDEBUILD_PBUILDER} --build > ${extra_configfi...@]/#/--configfile } --buildresult "${BUILDRESULT}" > --debbuildopts "" --debbuildopts "${DEBBUILDOPTS}" "$@" > ../"${PKG_SOURCENAME}_${PKG_VERSION}".dsc >"$fifo" > +log "W: Got architecture $ARCHITECTURE" > + rm -rf "$d" > + close_ipc > fi > > # do signing with optional key specifier > -- > 1.6.3.3 > -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected]
