On Tue, Jun 6, 2017 at 8:37 PM, Jeff King <peff@xxxxxxxx> wrote: >> To put your worries to rest we should eliminate remote->fetch_refspec >> altogether and parse refspecs into remote->fetch right away, I'd >> think. After all, that's what's used in most places anyway, and it >> can be easily turned back to a single string where needed (I think in >> only 3 places in builtin/remote.c). > > I don't think we can parse right away without regressing the error > handling. If I have two remotes, one with a bogus refspec, like: > > [remote "one"] > url = ... > fetch = refs/heads/*:refs/remotes/one/* > [remote "two"] > url = ... > fetch = ***bogus*** > > and I do: > > git fetch one > > then read_config() will grab the data for _both_ of them, but only call > remote_get() on the first one. If we parsed the refspecs during > read_config(), we'd parse the bogus remote.two.fetch and die(). > > I guess that's a minor case, but as far as I can tell that's the > motivation for the lazy parsing. Yeah, I know, we'd need a parse_refspec_gently() or something. parse_refspec_internal() already has a 'verify' parameter which prevents it from die()ing while parsing a bogus refspec, but in its current form it doesn't tell us which refspec was bogus, so we'd need a bit more than that to let the user know what's wrong.