On Mon Aug 9, 2021 at 9:16 PM CEST, Junio C Hamano wrote:
> >
> > +# Write a complete fetch command to stdout, suitable for use with `test-tool
> > +# pkt-line`. "want-ref", "want", and "have" values can be given in this order,
> > +# with sections separated by "--".
> > +#
> > +# Examples:
> > +#
> > +# write_fetch_command refs/heads/main
> > +#
> > +# write_fetch_command \
> > +# refs/heads/main \
> > +# -- \
> > +# -- \
> > +# $(git rev-parse x)
> > +#
> > +# write_fetch_command \
> > +# --
> > +# $(git rev-parse a) \
> > +# --
> > +# $(git rev-parse b)
>
> Have a blank line here (or a line with "#" and nothing else) and it
> would become easier to read.
Noted.
>
> > +write_fetch_command () {
> > + write_command fetch &&
> > + echo "0001" &&
> > + echo "no-progress" || return
>
> The "while :" in this helper function are indented with 4 spaces,
> not a single tab.
Noted, sorry.
I can't seem to be able teach `git diff --check` to exit non-zero on whitespace
errors.
> > # c(o/foo) d(o/bar)
> > # \ /
> > # b e(baz) f(main)
> > @@ -97,15 +145,13 @@ test_expect_success 'basic want-ref' '
> > EOF
> > git rev-parse f >expected_commits &&
> >
> > test-tool pkt-line pack >in <<-EOF &&
> > + $(write_fetch_command \
> > + refs/heads/main \
> > + -- \
> > + -- \
> > + $(git rev-parse a) \
> > + )
> > EOF
> > test-tool serve-v2 --stateless-rpc >out <in &&
>
> ... the code that uses the helper needs rewriting to make use of
> it. A failure in $(write_fetch_command) here will not cause the
> caller to stop here. If we had any "git" command used in the
> helper, I would recommand restructuring the caller to do something
> like:
>
> write_fetch_command >pkt-src \
> refs/heads/main \
> -- \
> -- \
> $(git rev-parse a) &&
> test-tool pkt-line pack <pkt-src >in &&
> test-tool serve-v2 --stateless-rpc >out <in &&
>
> but it probably may not be necessary (we only use "echo" there, and
> it probably is not worth worrying about 'echo' failing).
>
> The call to $(git rev-parse a) might fail when rev-parse gets
> broken, and I think the rewritten version would catch such a
> breakage while the one inside $(write_fetch_command) in the here-doc
> would not be.
Heh, this is turning into an exercise to forget everything about bash :/
I didn't know that an error in a nested command substitution would not cause the
outer one to fail. As we can't use a pipe either (as Jonathan Nieder suggests),
I think a redirection like this is indeed necessary. The only other option I
could think of would be assigning rev-parse to a variable, which arguably is
less readable:
have=$(git rew-parse a) &&
test-tool pkt-line pack >in <<-EOF &&
$(write_fetch_command \
refs/heads/main \
-- \
-- \
$have)
EOF
Thanks
