Re: should git maintenance prefetch be taught to honor remote.fetch refspec?

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Apr 2, 2021, 22:32 UTC
Message-ID: <CAPig+cRxjLObLvF9kUAuftCxQ+iiFEisagDBWpAAPwdVMUATKQ@mail.gmail.com>
In-Reply-To: <41dc2961-7ba5-a882-3416-45631e2cbb33@gmail.com>

On Fri, Apr 2, 2021 at 4:43 PM Derrick Stolee <stolee@gmail.com> wrote:

Show 9 quoted lines

> I have a branch available [1], but I'm seeing some failures only
> on FreeBSD [2] and I can't understand why that platform is failing
> this test. The current version (as of this writing) does not do
> the substring replacement technique, and hence it just gives up
> on exact matches. I will try the substring approach as an
> alternative and see where that gets me.
>
> [1] https://github.com/gitgitgadget/git/pull/924
> [2] https://github.com/gitgitgadget/git/pull/924/checks?check_run_id=2256079534

The "+" in patterns such as `+refs/heads/\\*:refs/prefetch...` is what is throwing it off. FreeBSD `grep` doesn't seem to like it, though it's not clear why. Escaping it the same way as you escaped "*" doesn't make it work. Replacing "+" with catchall "." does work, so that's one way to fix it.

However, all the escaping you need to do in these refspec patterns to pass them to `grep` is ugly. A much better solution may be to change the `grep` in test-lib-functions.sh:test_subcommand() to `grep -F` to force it to match literally. That way, you can drop all the backslash escaping, including those in front of "[" and "]". A cursory audit of callers test_subcommand() seems to indicate that none of them pass regex patterns, so using `-F` is probably safe and a good idea.

By the way, the `coccinelle` check is also "failing", correctly suggesting that you change:

    strbuf_addf(&replace, ":refs/prefetch/");

to:

    strbuf_addstr(&replace, ":refs/prefetch/");

in `builtin/gc.c`.