[RFC/PATCH 0/5] remote: eliminate remote->{fetch,push}_refspec and lazy parsing of refspecs

'struct remote' stores refspecs twice: once in their original string form in remote->{fetch,push}_refspecs and once in their parsed form in remote->{fetch,push}. This is necessary, because we need the refspecs for lazy parsing after we finished reading the configuration: we don't want to die() on a bogus refspec while reading the configuration of a remote we are not going to access at all.

However, storing refspecs in both forms has some drawbacks:

  - The same information is stored twice, wasting memory.
  - remote->{fetch,push}_refspecs, i.e. the string arrays are
    conveniently ALLOC_GROW()-able with associated
    {fetch,push}_refspec_{nr,alloc} fields, but remote->{fetch,push}
    are not.
  - Wherever remote->{fetch,push} are accessed, the number of parsed
    refspecs in there is specified by remote->{fetch,push}_refspec_nr.
    This requires us to keep the two arrays in sync and makes adding
    additional refspecs cumbersome and error prone.

  - And worst of all, it pissed me off while working on
    sg/clone-refspec-from-command-line-config ;)

So here is my crack at getting rid of them.

The idea is to parse refspecs gently while reading the configuration: this way we won't need to store all refspecs as strings, and won't die() on a bogus refspec right away. A bogus refspec, if there's one, will be stored in the remote it belongs to, so it will be available later when that remote is accessed and can be used in the error message.

At the end of the series the remote API will have public functions add_{fetch,push}_refspec() for, well, adding refspecs, including parsing, to those that were already present in the configuration. I'm not sure what the use case for adding extra push refspecs could be, but sg/clone-refspec-from-command-line-config has shown that there is a need for adding fetch refspecs, and this way they are at least symmetrical. Though I don't see any other use case for adding extra fetch refspecs, either. Anyway, there's less subtlety without the need to keep the two arrays ->fetch and ->fetch_refspec in sync.

The first three patches are preparation. Patch 3 was done with --patience, otherwise it's unreadable. -w will help even more to see what's going on. The last patch's commit message is rather terse... but I deemed it sufficient for an initial RFC; will expand it later, if there's an agreement that this approach is worth pursuing.

This applies on top of a merge of master and the fresh reroll (v5) of sg/clone-refspec-from-command-line-config:

  http://public-inbox.org/git/20170616173849.8071-1-szeder.dev@gmail.com/T/#m7f9c81fb415920acc2af93f432aecfa52477e6b8

It's also available at:

  https://github.com/szeder/git remote-no-string-refspecs

Best, Gábor

SZEDER Gábor (5):
  remote: don't use remote->{fetch,push}_refspec
  remote.c: don't pass copies of refspecs to add_{fetch,push}_refspec()
  remote.c: extract a helper function to parse a single refspec
  remote.c: eliminate remote->fetch_refspec
  remote.c: eliminate remote->push_refspec

 builtin/clone.c  |   7 +-
 builtin/fetch.c  |  20 ++--
 builtin/push.c   |  12 ++-
 builtin/remote.c |  47 ++++++---
 remote.c         | 316 +++++++++++++++++++++++++++++++------------------------
 remote.h         |  31 ++++--
 6 files changed, 248 insertions(+), 185 deletions(-)