Bug#516625: pbuilder: pdebuild signs wrong .changes when building with --arch=i386 on amd64
Junichi Uekawa
dancer at netfort.gr.jp
Thu Dec 31 06:47:25 UTC 2009
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?= <lool at dooz.org>
> 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_CONFIGFILE[@]/#/--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_CONFIGFILE[@]/#/--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_CONFIGFILE[@]/#/--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
>
More information about the Pbuilder-maint
mailing list