On Fri, Apr 02, 2021 at 03:09:01PM -0700, Junio C Hamano wrote: > Tom Saeger <tom.saeger@xxxxxxxxxx> writes: > > > On Fri, Apr 02, 2021 at 05:07:09PM -0400, Derrick Stolee wrote: > >> On 4/2/2021 4:43 PM, Derrick Stolee wrote: > >> > On 4/2/2021 2:27 PM, Tom Saeger wrote: > >> >> generally isn't it still changing the right-hand side of refspec? > >> >> > >> >> replacing ":refs/" with ":refs/prefetch/" > >> > > >> > Right, this substring replacement might be easiest to achieve. The > >> > 'struct refspec' doesn't make it incredibly easy. Perhaps skipping > >> > the refspec parsing and just doing that substring swap directly from > >> > the config value might be the best approach. > >> > > >> >> This would still work for refspecs with negative patterns right? > >> > > >> > One of the issues is that negative patterns have no ":refs/" > >> > substring. > >> > > >> > The other issue is that exact matches (no "*") have an exact > >> > string in the destination, too, so replacing the _entire_ > >> > destination with "refs/prefetch/<remote>/*" breaks the refspec. > >> > I think the substring approach will still work here. > >> > >> I updated my branch with the substring approach, which is > >> probably the better solution. Please give it a try. I don't > >> expect that change to help the FreeBSD build, but we will see. > > > > > > This worked for all the scenarios I tried, which had both negatives and > > multi remote fetch values. > > > > Looks good! > > > > Reviewed-by: Tom Saeger <tom.saeger@xxxxxxxxxx> > > That sounds more like tested-by, but anyway, thanks for working well > together. Tested-by: Tom Saeger <tom.saeger@xxxxxxxxxx> works for me, I did review the code but perhaps it's best to leave reviews to others. > > > > > > >> > >> > [1] https://github.com/gitgitgadget/git/pull/924 > >> > [2] https://github.com/gitgitgadget/git/pull/924/checks?check_run_id=2256079534 > >> > >> Thanks, > >> -Stolee