From: "Michał Górny" <mgorny@gentoo.org>
To: gentoo-dev@lists.gentoo.org
Cc: antarus@gentoo.org
Subject: Re: [gentoo-dev] [PATCH 2/8] Use bash redirection to run 'tee' rather than simple pipes.
Date: Thu, 28 Feb 2013 15:53:23 +0100 [thread overview]
Message-ID: <20130228155323.60e68d2a@pomiocik.lan> (raw)
In-Reply-To: <CAAr7Pr-=Sp7VVbfewD=7DS2huwutSLshHSwq_LdVEWJLLxYEeA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2984 bytes --]
On Thu, 28 Feb 2013 05:09:15 -0800
Alec Warner <antarus@gentoo.org> wrote:
> On Wed, Feb 27, 2013 at 1:43 PM, Michał Górny <mgorny@gentoo.org> wrote:
> > This allows us to spawn 'tee' as separate process while keeping
> > the function code executed in the main shell.
>
> Can you explain why this is 'better'? I'd prefer way more
> documentation in the code itself. You are using a number of what I'd
> consider 'banned' constructs and you don't go very far out of your way
> to explain why you cannot live without them.
Well, if we used simple pipes, like:
foo | tee ...
Then any variable exported by foo would be discarded. This broke a few
ebuilds already.
By banned constructs, do you mean 'eval'? I hate it too but wasn't able
to solve it anyhow better.
> > gx86/eclass/multibuild.eclass | 19 ++++++++++---------
> > 1 file changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/gx86/eclass/multibuild.eclass b/gx86/eclass/multibuild.eclass
> > index 716d34f..d42b8a7 100644
> > --- a/gx86/eclass/multibuild.eclass
> > +++ b/gx86/eclass/multibuild.eclass
> > @@ -108,18 +108,20 @@ multibuild_foreach() {
> > local MULTIBUILD_VARIANT=${v}
> > local MULTIBUILD_ID=${prev_id}${v}
> > local BUILD_DIR=${bdir%%/}-${v}
> > + local log_fd
> >
> > - einfo "${v}: running ${@}" \
> > - | tee -a "${T}/build-${MULTIBUILD_ID}.log"
> > + # redirect_alloc_fd accepts files only. so we need to open
> > + # a random file and then reuse the fd for logger process.
> > + redirect_alloc_fd log_fd /dev/null
> > + eval "exec ${log_fd}> "'>(exec tee -a "${T}/build-${MULTIBUILD_ID}.log")'
> The quoting here is a nightmare.
> Do the leading single quotes have to be *right next to* the double quotes?
> Why is there a space between the first > and the "?
> vim hi-lighting makes this more readable, but still weird looking.
Yes, I know. I'm thinking of trying to put it all into a single eval,
and just use \$ rather than mixing single and double quotes.
> Also please document clearly why 'eval' is required (it is not clear
> to me..or I suspect most readers ;p)
Well, for the very simple reason that 'fd=2; echo 11 >&${fd}' doesn't
work. The eval simply changes that into 'echo 11>&2'.
> Are we not just making an fd and pointing the 'read end' at a tee subprocess?
Yes, exactly that.
> > +
> > + eval 'einfo "${v}: running ${@}" >&'${log_fd}' 2>&1'
> And again here, why can't we just redirect directly to log_fd?
>
> >
> > # _multibuild_parallel() does redirection internally.
> > # this is a hidden API to avoid writing multilib_foreach twice.
> We do not use the _multibuild_parellel stuff anymore...is this comment relevant?
You are correct, my mistake.
--
Best regards,
Michał Górny
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]
next prev parent reply other threads:[~2013-02-28 14:53 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-27 21:41 [gentoo-dev] [RFC] multibuild.eclass -- a generic pluggable framework to handle multi-variant builds Michał Górny
2013-02-27 21:43 ` [gentoo-dev] [PATCH 1/8] Initial version of multibuild eclass Michał Górny
2013-02-27 21:43 ` [gentoo-dev] [PATCH 2/8] Use bash redirection to run 'tee' rather than simple pipes Michał Górny
2013-02-28 13:09 ` Alec Warner
2013-02-28 14:53 ` Michał Górny [this message]
2013-02-27 21:43 ` [gentoo-dev] [PATCH 3/8] Avoid writing outside WORKDIR if S=${WORKDIR} Michał Górny
2013-02-27 21:43 ` [gentoo-dev] [PATCH 4/8] Convert multilib-build to use multibuild Michał Górny
2013-02-27 21:43 ` [gentoo-dev] [PATCH 5/8] python-r1: calculate final list of enabled impls for foreach Michał Górny
2013-02-27 21:43 ` [gentoo-dev] [PATCH 6/8] Convert python-r1 to use multibuild Michał Górny
2013-02-27 21:43 ` [gentoo-dev] [PATCH 7/8] Move run_in_build_dir() to multibuild.eclass Michał Górny
2013-02-27 21:43 ` [gentoo-dev] [PATCH 8/8] fftw: example use of multibuild in ebuild Michał Górny
2013-03-02 23:19 ` Christoph Junghans
2013-03-02 21:42 ` [gentoo-dev] Further changes to multibuild.eclass Michał Górny
2013-03-02 21:42 ` [gentoo-dev] [PATCH 1/4] multibuild: print only 'public' part of command-line Michał Górny
2013-03-02 22:52 ` Alec Warner
2013-03-02 23:03 ` Michał Górny
2013-03-02 21:42 ` [gentoo-dev] [PATCH 2/4] multibuild: add multibuild_for_best_variant() Michał Górny
2013-03-02 21:42 ` [gentoo-dev] [PATCH 3/4] multilib-build: introduce multilib_for_best_abi() Michał Górny
2013-03-02 21:42 ` [gentoo-dev] [PATCH 4/4] distutils-r1: reuse multibuild_for_best_variant Michał Górny
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130228155323.60e68d2a@pomiocik.lan \
--to=mgorny@gentoo.org \
--cc=antarus@gentoo.org \
--cc=gentoo-dev@lists.gentoo.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox