'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@xxxxxxxxx/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(-)
--
2.13.1.505.g7cc9fcafb
