| Message ID | 20221125095954.4826-3-worldhello.net@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4137c84198be24b9e34eb2b2e0d4743a55629116 |
| Headers | show |
| Series | Fix broken CI on newer github-actions runner image | expand |
Jiang Xin <worldhello.net@gmail.com> writes: > Subject: Re: [PATCH v4 2/4] ci: remove the pipe after "p4 -V" to cache errors "cache" -> "catch", I think. > From: Jiang Xin <zhiyou.jx@alibaba-inc.com> > > When installing p4 as a dependency, we used to pipe output of "p4 -V" > and "p4d -V" to validate the installation and output a condensed version > information. But this would hide potential errors of p4 and would stop > with an empty output. E.g.: p4d version 16.2 running on ubuntu 22.04 > causes sigfaults. ... before it produces any output. > By removing the pipe after "p4 -V" and "p4d -V", we may get a verbose > output, and stop immediately on errors becuase we have "set -e" in "because". > "ci/lib.sh". Since we won't look at these trace logs unless something > fails, so just including the raw output seems most sensible. You started the sentence with "Since", so "so" should be dropped. > Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de> > Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> > --- > ci/install-dependencies.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Well reasoned. Will queue. Thanks. > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh > index feefd6e9bb..97a1a1f574 100755 > --- a/ci/install-dependencies.sh > +++ b/ci/install-dependencies.sh > @@ -83,9 +83,9 @@ esac > if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1 > then > echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)" > - p4d -V | grep Rev. > + p4d -V > echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)" > - p4 -V | grep Rev. > + p4 -V > else > echo >&2 "WARNING: perforce wasn't installed, see above for clues why" > fi
On Sun, Nov 27, 2022 at 8:24 AM Junio C Hamano <gitster@pobox.com> wrote: > > Jiang Xin <worldhello.net@gmail.com> writes: > > > Subject: Re: [PATCH v4 2/4] ci: remove the pipe after "p4 -V" to cache errors > > "cache" -> "catch", I think. oops, it's a typo. I used to copy and paste the commit log to MS word or Apple pages to check for typos, but this step is very cumbersome, and I always forget it. I see this has been fixed in your tree, so there is no need to send v5. -- Jiang Xin
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index feefd6e9bb..97a1a1f574 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -83,9 +83,9 @@ esac if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1 then echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)" - p4d -V | grep Rev. + p4d -V echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)" - p4 -V | grep Rev. + p4 -V else echo >&2 "WARNING: perforce wasn't installed, see above for clues why" fi