Subject: Re: [PATCH] remote: handle negative refspecs in git remote show

From: Junio C Hamano <[email-masked]>
Date: Thu, 16 Jun 2022 14:52:17 -0700

Jacob Keller <[email-masked]> writes:

>> This is somewhat curious.  Do we really need to destroy the
>> fetch_map like the above?  I know by removing skipped items from the
>> list, the existing loop (below) can stop having to worry about them,
>> but the caller of get_ref_states() may later want to iterate over
>> the full fetch_map for other reasons (even if the current one does
>> not, a future version of the caller may have a reason to do so that
>> we do not know right now yet).
>>
>
> Good point. I'll fix this. I think we can just move the
> omit_name_by_refspec into the other loop.
>
>> > +
>> >       for (ref = fetch_map; ref; ref = ref->next) {
>> >               if (!ref->peer_ref || !ref_exists(ref->peer_ref->name))
>> >                       string_list_append(&states->new_refs, abbrev_branch(ref->name));
>>
>> IOW, is adding a new condition to this existing loop insufficient?
>>
>
> The tricky part here is that we don't have a simple check, and we're
> currently iterating over all of the refspecs each time. But we have to
> do that regardless so I think this makes sense. Will fix.

Another thing that worries me is that get_stale_heads() will not see
the filtered refs with your original implementation, because you cull
them from the fetch_map in the extra loop upfront.

I do not know offhand what its effect would be, but it probably is
worth testing.  In your original scenario, if we locally have
refs/remotes/jdk19/old and refs/remotes/jdk19/pr/1 (perhaps obtained
before we configured ^refs/pr/* negative refspec), we'd want to see
that pr/1 exists here but will not be updated.

  * remote jdk19
    Fetch URL: [email-masked]:openjdk/jdk19.git
    Push  URL: [email-masked]:openjdk/jdk19.git
    HEAD branch: master
    Remote branches:
      master tracked
      old    stale
      pr/1   stale
      pr/2   skipped
      pr/3   skipped
    Local ref configured for 'git push':
      master pushes to master (fast-forwardable)