Re: [PATCH v3 5/8] fetch: refactor fetch_refs into two functions
From: Jonathan Nieder <hidden>
Date: 2018-06-22 21:42:46
Brandon Williams wrote:
quoted hunk ↗ jump to hunk
--- a/builtin/fetch.c +++ b/builtin/fetch.c@@ -967,10 +967,16 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map) int ret = quickfetch(ref_map); if (ret) ret = transport_fetch_refs(transport, ref_map); - if (!ret) - ret |= store_updated_refs(transport->url, - transport->remote->name, - ref_map); + if (ret) + transport_unlock_pack(transport); + return ret; +} + +static int consume_refs(struct transport *transport, struct ref *ref_map) +{ + int ret = store_updated_refs(transport->url, + transport->remote->name, + ref_map); transport_unlock_pack(transport); return ret; }
[...]
- fetch_refs(transport, ref_map); + if (!fetch_refs(transport, ref_map)) + consume_refs(transport, ref_map);
Ah, I missed something in my previous reply. If transport_fetch_refs succeeds and store_updated_refs fails, then in the old code, transport_unlock_pack would clean up by removing the no longer needed .keep file. In the new code, that's consume_refs's responsibility, which I find much nicer. It's probably worth mentioning that in the commit message as well. Thanks again, Jonathan