remote: eliminate remote->{fetch,push}_refspec and lazy parsing of refspecs
Get series with b4
b4 am [email protected]Versions newest first
- v1 Latest5 patches 6 repliesIn review
Patches in v1 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/5 | remote: don't use remote->{fetch,push}_refspecbuiltin/ remote.c remote.h | +37 | 1 reply | |
| 2/5 | remote.c: don't pass copies of refspecs to add_{fetch,push}_refspec()builtin/ remote.c | +15 | 0 replies | |
| 3/5 | remote.c: extract a helper function to parse a single refspecremote.c | +122 | 0 replies | |
| 4/5 | remote.c: eliminate remote->fetch_refspecbuiltin/ builtin/ builtin/ +2 more | +69 | 0 replies | |
| 5/5 | remote.c: eliminate remote->push_refspecbuiltin/ builtin/ remote.c +1 more | +34 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [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/[email protected]/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(-)
Signature · 1 line
2.13.1.505.g7cc9fcafb