git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jacob Keller <jacob.keller@gmail.com>
Date
Jun 14, 2022, 06:09 UTC
Message-ID
<CA+P7+xos3vpPjb3m_BkR4Qp3OPf+R+-A1B=jTD3-Q6FeO4RpMg@mail.gmail.com>
In-Reply-To
<Yqfec9yvT3LKomNK@nand.local>
On Mon, Jun 13, 2022 at 6:03 PM Taylor Blau <me@ttaylorr.com> wrote:
Show 34 quoted lines
>
> On Mon, Jun 13, 2022 at 05:32:51PM -0700, Jacob Keller wrote:
> > Fix this by checking negative refspecs inside of get_ref_states. For
> > each ref which matches a negative refspec, copy it into a "skipped" list
> > and remove it from the fetch map. This allows us to show the following
> > output instead:
>
> Seems sensible.
>
> > +     /* handle negative refspecs first */
> > +     for (tail = &fetch_map; *tail; ) {
> > +             ref = *tail;
> > +
> > +             if (omit_name_by_refspec(ref->name, &states->remote->fetch)) {
> > +                     string_list_append(&states->skipped, abbrev_branch(ref->name));
> > +
> > +                     /* Matched a negative refspec, so remove this ref from
> > +                      * consideration for being a new or tracked ref.
> > +                      */
> > +                     *tail = ref->next;
> > +                     free(ref->peer_ref);
> > +                     free(ref);
> > +             } else {
> > +                     tail = &ref->next;
> > +             }
> > +     }
> > +
>
> Not being overly familiar with the "git remote show" code, this
> implementation looks very reasonable to me. If we see a negative
> refspec, we remove it from the fetch_map list and append it to the
> skipped list. Otherwise, we increment our pointer, and continue along
> until we reach the end of the list.
>

The specific way the loop works is similar to other ref looping code but it feels a little odd to me. Still, it seems to be the right approach overall.

Show 13 quoted lines
> > +test_expect_success 'show with negative refspecs' '
> > +     test_when_finished "git -C test config --fixed-value --unset remote.origin.fetch ^refs/heads/main" &&
> > +     (
> > +             cd test &&
> > +             git config --add remote.origin.fetch ^refs/heads/main &&
>
> Doing "git config --unset" outside of the subshell could be avoided by
> ditching the subshell altogether, perhaps with something like:
>
>     test_config -C test remote.origin.fetch ^refs/heads/main &&
>     git -C test remote show origin >actual &&
>     test_cmp test/expect actual
>

I still think that removing the subshell is a good idea here. I'll investigate this.

I also wonder if it would be difficult to enable "--add" semantics for test_config.

> Thanks,
> Taylor
Previous: Jacob KellerNext: Junio C Hamano
Message 6 of 11 in “remote: handle negative refspecs in git remote show”
  1. remote: handle negative refspecs in git remote showJacob Keller, Jun 14, 2022
  2. Taylor BlauJun 14, 2022
  3. Jacob KellerJun 14, 2022
  4. Taylor BlauJun 14, 2022
  5. Jacob KellerJun 16, 2022
  6. Jacob KellerJun 14, 2022
  7. Junio C HamanoJun 15, 2022
  8. Jacob KellerJun 16, 2022
  9. Junio C HamanoJun 16, 2022
  10. Jacob KellerJun 16, 2022
  11. Jacob KellerJun 16, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.