Re: [Buildroot] [PATCH 2/2] support/download: fix the cargo post-process in face of failed vendoring

[Peter Korsgaard <peter@korsgaard.com>]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > In commit 04154a651729 (support/download/cargo-post-process: cargo
 > output for vendor config), we switched away from our hand-crafted
 > cargo.toml mangling, to use cargo itself to update that file.

 > In doing so, we enabled the shell pipefail option, so that we could
 > catch cargo failures, while redirecting its output through tee to the
 > cargo.toml.

 > However, pipefail is overzealous, and will hit us even for pipes we do
 > not want to globally fail, like the one that actually checks whether an
 > archive is already vendored or not:

 >     if tar tf "${output}" | grep -q "^[^/]*/VENDOR" ; then
 >         ...

 > with pipefail, the above may always fail:
 >   - if the tarball is already vendored, grep will exit on the first
 >     match because of -q (it only needs a single match to decide that its
 >     return code will be zero), so the | will get closed, and tar may
 >     get -EPIPE before it had a chance to finish listing the archive, and
 >     thus would terminate in error;
 >   - if the tarball is not vendored, grep will exit in error.

 > It turns out that the tee was only added so that we could see the
 > messages emitted by cargo, and still fill the cargo.tom with the output
 > of cargo.

 > But that's a bit overkill: the cargo messages are going to stderr, and
 > the blurb to add to cargo.toml to stdout, so we just need to redirect
 > stdout.

 > Yes, we do not see what cargo added to cargo.toml, but that is not so
 > interesting.

 > Still, cargo ends its messages with a suggestion for the user to modify
 > cargo.toml, with:

 >     To use vendored sources, add this to your .cargo/config.toml for this project:

 > But since we've already redirected that to cargo.toml, there is nothing
 > for the user to edit, so the above can get confusing. Emit a little
 > blurb that states that everything is under control.

 > And then we can drop pipefail.

 > Note: the go-post-process initially had pipefail too, but it was dropped
 > in bfd1a31d0e59 (support/download/go-post-process: drop -o pipefail) as
 > it was causing spurrious breakage when extracting the archive before
 > vendoring, so it is only reasonable that we also remove it from the
 > cargo-post-process.

 > Reported-by: Peter Korsgaard <peter@korsgaard.com>
 > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
 > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > Cc: Simon Richter <simon.richter@ptwdosimetry.com>

Committed, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot