Re: [gentoo-dev] [PATCH 2/8] Use bash redirection to run 'tee' rather than simple pipes.

[Michał Górny]

On Thu, 28 Feb 2013 05:09:15 -0800
Alec Warner <antarus@g.o> wrote:

> On Wed, Feb 27, 2013 at 1:43 PM, Michał Górny <mgorny@g.o> 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