Re: [PATCH] fetch-pack: do not take shallow lock unnecessarily

> In other parts of the code we often have an early exit instead of
> setting a variable and reacting to that in the end, i.e. what
> do you think about:
> 
> static void receive_shallow_info(struct fetch_pack_args *args,
>     struct packet_reader *reader)
> {
>      process_section_header(reader, "shallow-info", 0);
> +    if (reader->status == PACKET_READ_FLUSH ||
> +        reader->status == PACKET_READ_DELIM)
> +            /* useful comment why empty sections appear */
> +            return;
>     while (packet_reader_read(reader) == PACKET_READ_NORMAL) {
>     ...
> 
> instead? This would allow us to keep the rest of the function
> relatively simple as well as we'd have a dedicated space where
> we can explain why empty sections need to be treated specially.

> > +test_expect_success 'ensure that multiple fetches in same process from a shallow repo works' '
> > +       rm -rf server client trace &&
> > +
> > +       test_create_repo server &&
> > +       test_commit -C server one &&
> > +       test_commit -C server two &&
> > +       test_commit -C server three &&
> > +       git clone --shallow-exclude two "file://$(pwd)/server" client &&
> > +
> > +       git -C server tag -a -m "an annotated tag" twotag two &&
> > +
> > +       # Triggers tag following (thus, 2 fetches in one process)
> > +       GIT_TRACE_PACKET="$(pwd)/trace" git -C client -c protocol.version=2 \
> > +               fetch --shallow-exclude one origin &&
> > +       # Ensure that protocol v2 is used
> > +       grep "fetch< version 2" trace
> > +'
> 
> Would we also need to ensure tags 'one' and 'three',
> but not 'two' are present?
> (What error condition do we see without this patch?)