summaryrefslogtreecommitdiff
path: root/fetch-pack.c
AgeCommit message (Collapse)AuthorFilesLines
2017-03-14Merge branch 'mm/fetch-show-error-message-on-unadvertised-object'Libravatar Junio C Hamano1-13/+38
"git fetch" that requests a commit by object name, when the other side does not allow such an request, failed without much explanation. * mm/fetch-show-error-message-on-unadvertised-object: fetch-pack: add specific error for fetching an unadvertised object fetch_refs_via_pack: call report_unmatched_refs fetch-pack: move code to report unmatched refs to a function
2017-03-02fetch-pack: add specific error for fetching an unadvertised objectLibravatar Matt McCutchen1-15/+27
Enhance filter_refs (which decides whether a request for an unadvertised object should be sent to the server) to record a new match status on the "struct ref" when a request is not allowed, and have report_unmatched_refs check for this status and print a special error message, "Server does not allow request for unadvertised object". Signed-off-by: Matt McCutchen <matt@mattmccutchen.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-02fetch-pack: move code to report unmatched refs to a functionLibravatar Matt McCutchen1-0/+13
Prepare to reuse this code in transport.c for "git fetch". While we're here, internationalize the existing error message. Signed-off-by: Matt McCutchen <matt@mattmccutchen.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-08fetch-pack: cache results of for_each_alternate_refLibravatar Jeff King1-10/+42
We may run for_each_alternate_ref() twice, once in find_common() and once in everything_local(). This operation can be expensive, because it involves running a sub-process which must freshly load all of the alternate's refs from disk. Let's cache and reuse the results between the two calls. We can make some optimizations based on the particular use pattern in fetch-pack to keep our memory usage down. The first is that we only care about the sha1s, not the refs themselves. So it's OK to store only the sha1s, and to suppress duplicates. The natural fit would therefore be a sha1_array. However, sha1_array's de-duplication happens only after it has read and sorted all entries. It still stores each duplicate. For an alternate with a large number of refs pointing to the same commits, this is a needless expense. Instead, we'd prefer to eliminate duplicates before putting them in the cache, which implies using a hash. We can further note that fetch-pack will call parse_object() on each alternate sha1. We can therefore keep our cache as a set of pointers to "struct object". That gives us a place to put our "already seen" bit with an optimized hash lookup. And as a bonus, the object stores the sha1 for us, so pointer-to-object is all we need. There are two extra optimizations I didn't do here: - we actually store an array of pointer-to-object. Technically we could just walk the obj_hash table looking for entries with the ALTERNATE flag set (because our use case doesn't care about the order here). But that hash table may be mostly composed of non-ALTERNATE entries, so we'd waste time walking over them. So it would be a slight win in memory use, but a loss in CPU. - the items we pull out of the cache are actual "struct object"s, but then we feed "obj->sha1" to our sub-functions, which promptly call parse_object(). This second parse is cheap, because it starts with lookup_object() and will bail immediately when it sees we've already parsed the object. We could save the extra hash lookup, but it would involve refactoring the functions we call. It may or may not be worth the trouble. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-08for_each_alternate_ref: pass name/oid instead of ref structLibravatar Jeff King1-4/+8
Breaking down the fields in the interface makes it easier to change the backend of for_each_alternate_ref to something that doesn't use "struct ref" internally. The only field that callers actually look at is the oid, anyway. The refname is kept in the interface as a plausible thing for future code to want. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-11fetch-pack.c: correct command at the beginning of an error messageLibravatar Ralf Thielow1-1/+1
One error message in fetch-pack.c uses 'git fetch_pack' at the beginning which is not a git command. Use 'git fetch-pack' instead. Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10Merge branch 'nd/shallow-deepen'Libravatar Junio C Hamano1-73/+96
The existing "git fetch --depth=<n>" option was hard to use correctly when making the history of an existing shallow clone deeper. A new option, "--deepen=<n>", has been added to make this easier to use. "git clone" also learned "--shallow-since=<date>" and "--shallow-exclude=<tag>" options to make it easier to specify "I am interested only in the recent N months worth of history" and "Give me only the history since that version". * nd/shallow-deepen: (27 commits) fetch, upload-pack: --deepen=N extends shallow boundary by N commits upload-pack: add get_reachable_list() upload-pack: split check_unreachable() in two, prep for get_reachable_list() t5500, t5539: tests for shallow depth excluding a ref clone: define shallow clone boundary with --shallow-exclude fetch: define shallow boundary with --shallow-exclude upload-pack: support define shallow boundary by excluding revisions refs: add expand_ref() t5500, t5539: tests for shallow depth since a specific date clone: define shallow clone boundary based on time with --shallow-since fetch: define shallow boundary with --shallow-since upload-pack: add deepen-since to cut shallow repos based on time shallow.c: implement a generic shallow boundary finder based on rev-list fetch-pack: use a separate flag for fetch in deepening mode fetch-pack.c: mark strings for translating fetch-pack: use a common function for verbose printing fetch-pack: use skip_prefix() instead of starts_with() upload-pack: move rev-list code out of check_non_tip() upload-pack: make check_non_tip() clean things up on error upload-pack: tighten number parsing at "deepen" lines ...
2016-10-10Merge branch 'rs/qsort'Libravatar Junio C Hamano1-1/+1
We call "qsort(array, nelem, sizeof(array[0]), fn)", and most of the time third parameter is redundant. A new QSORT() macro lets us omit it. * rs/qsort: show-branch: use QSORT use QSORT, part 2 coccicheck: use --all-includes by default remove unnecessary check before QSORT use QSORT add QSORT
2016-09-29use QSORTLibravatar René Scharfe1-1/+1
Apply the semantic patch contrib/coccinelle/qsort.cocci to the code base, replacing calls of qsort(3) with QSORT. The resulting code is shorter and supports empty arrays with NULL pointers. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-23fetch-pack: do not reset in_vain on non-novel acksLibravatar Jonathan Tan1-2/+9
The MAX_IN_VAIN mechanism was introduced in commit f061e5f ("fetch-pack: give up after getting too many "ack continue"", 2006-05-24) to stop ref negotiation if a number of consecutive "have"s have been sent with no corresponding new acks. This is to stop the client from digging too deep in an irrelevant side branch in vain without ever finding a common ancestor. A use case (as described in that commit) is the scenario in which the local repository has more roots than the remote repository. However, during a negotiation in which stateless RPCs are used, MAX_IN_VAIN will (almost) never trigger (in the more-roots scenario above and others) because in each new request, the client has to inform the server of objects it already has and knows the server has (to remind the server of the state), which the server then acks. Make fetch-pack only consider, as new acks for the purpose of MAX_IN_VAIN, acks for objects for which the client has never received an ack before in this session. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-19fetch-pack: grow stateless RPC windows exponentiallyLibravatar Jonathan Tan1-7/+12
When updating large repositories, the LARGE_FLUSH limit (that is, the limit at which the window growth strategy switches from exponential to linear) is reached quite quickly. Use a conservative exponential growth strategy when that limit is reached instead (and increase LARGE_FLUSH so that there is no regression in window size). This optimization is only applied during stateless RPCs to avoid the issue raised and fixed in commit 44d8dc54 (Fix potential local deadlock during fetch-pack, 2011-03-29). Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-13fetch, upload-pack: --deepen=N extends shallow boundary by N commitsLibravatar Nguyễn Thái Ngọc Duy1-0/+3
In git-fetch, --depth argument is always relative with the latest remote refs. This makes it a bit difficult to cover this use case, where the user wants to make the shallow history, say 3 levels deeper. It would work if remote refs have not moved yet, but nobody can guarantee that, especially when that use case is performed a couple months after the last clone or "git fetch --depth". Also, modifying shallow boundary using --depth does not work well with clones created by --since or --not. This patch fixes that. A new argument --deepen=<N> will add <N> more (*) parent commits to the current history regardless of where remote refs are. Have/Want negotiation is still respected. So if remote refs move, the server will send two chunks: one between "have" and "want" and another to extend shallow history. In theory, the client could send no "want"s in order to get the second chunk only. But the protocol does not allow that. Either you send no want lines, which means ls-remote; or you have to send at least one want line that carries deep-relative to the server.. The main work was done by Dongcan Jiang. I fixed it up here and there. And of course all the bugs belong to me. (*) We could even support --deepen=<N> where <N> is negative. In that case we can cut some history from the shallow clone. This operation (and --depth=<shorter depth>) does not require interaction with remote side (and more complicated to implement as a result). Helped-by: Duy Nguyen <pclouds@gmail.com> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Dongcan Jiang <dongcan.jiang@gmail.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-13fetch: define shallow boundary with --shallow-excludeLibravatar Nguyễn Thái Ngọc Duy1-1/+14
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-13fetch: define shallow boundary with --shallow-sinceLibravatar Nguyễn Thái Ngọc Duy1-1/+11
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-13fetch-pack: use a separate flag for fetch in deepening modeLibravatar Nguyễn Thái Ngọc Duy1-6/+8
The shallow repo could be deepened or shortened when then user gives --depth. But in future that won't be the only way to deepen/shorten a repo. Stop relying on args->depth in this mode. Future deepening methods can simply set this flag on instead of updating all these if expressions. The new name "deepen" was chosen after the command to define shallow boundary in pack protocol. New commands also follow this tradition. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-13fetch-pack.c: mark strings for translatingLibravatar Nguyễn Thái Ngọc Duy1-38/+37
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-13fetch-pack: use a common function for verbose printingLibravatar Nguyễn Thái Ngọc Duy1-46/+42
This reduces the number of "if (verbose)" which makes it a bit easier to read imo. It also makes it easier to redirect all these printouts, to a file for example. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-04-20fetch-pack: isolate sigpipe in demuxer threadLibravatar Jeff King1-3/+1
In commit 9ff18fa (fetch-pack: ignore SIGPIPE in sideband demuxer, 2016-02-24), we started using sigchain_push() to ignore SIGPIPE in the async demuxer thread. However, this is rather clumsy, as it ignores SIGPIPE for the entire process, including the main thread. At the time we didn't have any per-thread signal support, but we now we do. Let's use it. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-02-25fetch-pack: ignore SIGPIPE in sideband demuxerLibravatar Jeff King1-1/+5
If the other side feeds us a bogus pack, index-pack (or unpack-objects) may die early, before consuming all of its input. As a result, the sideband demuxer may get SIGPIPE (racily, depending on whether our data made it into the pipe buffer or not). If this happens and we are compiled with pthread support, it will take down the main thread, too. This isn't the end of the world, as the main process will just die() anyway when it sees index-pack failed. But it does mean we don't get a chance to say "fatal: index-pack failed" or similar. And it also means that we racily fail t5504, as we sometimes die() and sometimes are killed by SIGPIPE. So let's ignore SIGPIPE while demuxing the sideband. We are already careful to check the return value of write(), so we won't waste time writing to a broken pipe. The caller will notice the error return from the async thread, though in practice we don't even get that far, as we die() as soon as we see that index-pack failed. The non-sideband case is already fine; we let index-pack read straight from the socket, so there is no SIGPIPE at all. Technically the non-threaded async case is also OK without this (the forked async process gets SIGPIPE), but it's not worth distinguishing from the threaded case here. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-11-20Remove get_object_hash.Libravatar brian m. carlson1-2/+2
Convert all instances of get_object_hash to use an appropriate reference to the hash member of the oid member of struct object. This provides no functional change, as it is essentially a macro substitution. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
2015-11-20Convert struct object to object_idLibravatar brian m. carlson1-1/+1
struct object is one of the major data structures dealing with object IDs. Convert it to use struct object_id instead of an unsigned char array. Convert get_object_hash to refer to the new member as well. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
2015-11-20Add several uses of get_object_hash.Libravatar brian m. carlson1-2/+2
Convert most instances where the sha1 member of struct object is dereferenced to use get_object_hash. Most instances that are passed to functions that have versions taking struct object_id, such as get_sha1_hex/get_oid_hex, or instances that can be trivially converted to use struct object_id instead, are not converted. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
2015-11-20Convert struct ref to use object_id.Libravatar brian m. carlson1-9/+9
Use struct object_id in three fields in struct ref and convert all the necessary places that use it. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
2015-10-05fetch-pack: use argv_array for index-pack / unpack-objectsLibravatar Jeff King1-29/+27
This cleans up a magic number that must be kept in sync with the rest of the code (the number of argv slots). It also lets us drop some fixed buffers and an sprintf (since we can now use argv_array_pushf). We do still have to keep one fixed buffer for calling gethostname, but at least now the size computations for it are much simpler. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10memoize common git-path "constant" filesLibravatar Jeff King1-1/+1
One of the most common uses of git_path() is to pass a constant, like git_path("MERGE_MSG"). This has two drawbacks: 1. The return value is a static buffer, and the lifetime is dependent on other calls to git_path, etc. 2. There's no compile-time checking of the pathname. This is OK for a one-off (after all, we have to spell it correctly at least once), but many of these constant strings appear throughout the code. This patch introduces a series of functions to "memoize" these strings, which are essentially globals for the lifetime of the program. We compute the value once, take ownership of the buffer, and return the cached value for subsequent calls. cache.h provides a helper macro for defining these functions as one-liners, and defines a few common ones for global use. Using a macro is a little bit gross, but it does nicely document the purpose of the functions. If we need to touch them all later (e.g., because we learned how to change the git_dir variable at runtime, and need to invalidate all of the stored values), it will be much easier to have the complete list. Note that the shared-global functions have separate, manual declarations. We could do something clever with the macros (e.g., expand it to a declaration in some places, and a declaration _and_ a definition in path.c). But there aren't that many, and it's probably better to stay away from too-magical macros. Likewise, if we abandon the C preprocessor in favor of generating these with a script, we could get much fancier. E.g., normalizing "FOO/BAR-BAZ" into "git_path_foo_bar_baz". But the small amount of saved typing is probably not worth the resulting confusion to readers who want to grep for the function's definition. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-07-01Merge branch 'me/fetch-into-shallow-safety'Libravatar Junio C Hamano1-1/+1
"git fetch --depth=<depth>" and "git clone --depth=<depth>" issued a shallow transfer request even to an upload-pack that does not support the capability. * me/fetch-into-shallow-safety: fetch-pack: check for shallow if depth given
2015-06-17fetch-pack: check for shallow if depth givenLibravatar Mike Edgar1-1/+1
When a repository is first fetched as a shallow clone, either by git-clone or by fetching into an empty repo, the server's capabilities are not currently consulted. The client will send shallow requests even if the server does not understand them, and the resulting error may be unhelpful to the user. This change pre-emptively checks so we can exit with a helpful error if necessary. Signed-off-by: Mike Edgar <adgar@google.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-05Merge branch 'bc/object-id'Libravatar Junio C Hamano1-8/+21
for_each_ref() callback functions were taught to name the objects not with "unsigned char sha1[20]" but with "struct object_id". * bc/object-id: (56 commits) struct ref_lock: convert old_sha1 member to object_id warn_if_dangling_symref(): convert local variable "junk" to object_id each_ref_fn_adapter(): remove adapter rev_list_insert_ref(): remove unneeded arguments rev_list_insert_ref_oid(): new function, taking an object_oid mark_complete(): remove unneeded arguments mark_complete_oid(): new function, taking an object_oid clear_marks(): rewrite to take an object_id argument mark_complete(): rewrite to take an object_id argument send_ref(): convert local variable "peeled" to object_id upload-pack: rewrite functions to take object_id arguments find_symref(): convert local variable "unused" to object_id find_symref(): rewrite to take an object_id argument write_one_ref(): rewrite to take an object_id argument write_refs_to_temp_dir(): convert local variable sha1 to object_id submodule: rewrite to take an object_id argument shallow: rewrite functions to take object_id arguments handle_one_ref(): rewrite to take an object_id argument add_info_ref(): rewrite to take an object_id argument handle_one_reflog(): rewrite to take an object_id argument ...
2015-05-25rev_list_insert_ref(): remove unneeded argumentsLibravatar Michael Haggerty1-3/+3
Now that the function is not being used as an each_ref_sha1_fn, we can delete the unused arguments in its signature. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-25rev_list_insert_ref_oid(): new function, taking an object_oidLibravatar Michael Haggerty1-3/+7
This function can be used with for_each_ref() without having to be wrapped. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-25mark_complete(): remove unneeded argumentsLibravatar Michael Haggerty1-3/+3
Now that the function is not being used as an each_ref_sha1_fn, we can delete the unused arguments in its signature. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-25mark_complete_oid(): new function, taking an object_oidLibravatar Michael Haggerty1-4/+7
This function can be used with for_each_ref() without having to be wrapped. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-25clear_marks(): rewrite to take an object_id argumentLibravatar Michael Haggerty1-8/+5
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-25each_ref_fn: change to take an object_id parameterLibravatar Michael Haggerty1-4/+13
Change typedef each_ref_fn to take a "const struct object_id *oid" parameter instead of "const unsigned char *sha1". To aid this transition, implement an adapter that can be used to wrap old-style functions matching the old typedef, which is now called "each_ref_sha1_fn"), and make such functions callable via the new interface. This requires the old function and its cb_data to be wrapped in a "struct each_ref_fn_sha1_adapter", and that object to be used as the cb_data for an adapter function, each_ref_fn_adapter(). This is an enormous diff, but most of it consists of simple, mechanical changes to the sites that call any of the "for_each_ref" family of functions. Subsequent to this change, the call sites can be rewritten one by one to use the new interface. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-22upload-pack: optionally allow fetching reachable sha1Libravatar Fredrik Medley1-1/+9
With uploadpack.allowReachableSHA1InWant configuration option set on the server side, "git fetch" can make a request with a "want" line that names an object that has not been advertised (likely to have been obtained out of band or from a submodule pointer). Only objects reachable from the branch tips, i.e. the union of advertised branches and branches hidden by transfer.hideRefs, will be processed. Note that there is an associated cost of having to walk back the history to check the reachability. This feature can be used when obtaining the content of a certain commit, for which the sha1 is known, without the need of cloning the whole repository, especially if a shallow fetch is used. Useful cases are e.g. repositories containing large files in the history, fetching only the needed data for a submodule checkout, when sharing a sha1 without telling which exact branch it belongs to and in Gerrit, if you think in terms of commits instead of change numbers. (The Gerrit case has already been solved through allowTipSHA1InWant as every Gerrit change has a ref.) Signed-off-by: Fredrik Medley <fredrik.medley@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-22upload-pack: prepare to extend allow-tip-sha1-in-wantLibravatar Fredrik Medley1-3/+6
To allow future extensions, e.g. allowing non-tip sha1, replace the boolean allow_tip_sha1_in_want variable with the flag-style allow_request_with_bare_object_name variable. Signed-off-by: Fredrik Medley <fredrik.medley@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-19fetch-pack: remove dead assignment to ref->new_sha1Libravatar Jeff King1-3/+0
In everything_local(), we used to assign the current ref's value found in ref->old_sha1 to ref->new_sha1 when we already have all the necessary objects to complete the history leading to that commit. This copying was broken at 49bb805e (Do not ask for objects known to be complete., 2005-10-19) and ever since we instead stuffed a random bytes in ref->new_sha1 here. No code complained or failed due to this breakage. It turns out that no code path that comes after this assignment even looks at ref->new_sha1 at all. - The only caller of everything_local(), do_fetch_pack(), returns this list of refs, whose element has bogus new_sha1 values, to its caller. It does not look at the elements itself, but does pass them to find_common, which looks only at the name and old_sha1 fields. - The only caller of do_fetch_pack(), fetch_pack(), returns this list to its caller. It does not look at the elements nor act on them. - One of the two callers of fetch_pack() is cmd_fetch_pack(), the top-level that implements "git fetch-pack". The only thing it looks at in the elements of the returned ref list is the old_sha1 and name fields. - The other caller of fetch_pack() is fetch_refs_via_pack() in the transport layer, which is a helper that implements "git fetch". It only cares about whether the returned list is empty (i.e. failed to fetch anything). Just drop the bogus assignment, that is not even necessary. The remote-tracking refs are updated based on a different list and not using the ref list being manipulated by this code path; the caller do_fetch_pack() created a copy of that real ref list and passed the copy down to this function, and modifying the elements here does not affect anything. Noticed-by: Kyle J. McKay <mackyle@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-19filter_ref: make a copy of extra "sought" entriesLibravatar Jeff King1-3/+2
If the server supports allow_tip_sha1_in_want, we add any unmatched raw-sha1 entries in our "sought" list of refs to the list of refs we will ask the other side for. We do so by inserting the original "struct ref" directly into our list, rather than making a copy. This has several problems. The most minor problem is that one cannot ever free the resulting list; it contains structs that are copies of the remote refs (made earlier by fetch_pack) along with sought refs that are referenced elsewhere. But more importantly that we set the ref->next pointer to NULL, chopping off the remainder of any existing list that the ref was a part of. We get the set of "sought" refs in an array rather than a linked list, but that array is often in turn generated from a list. The test modification in t5516 demonstrates this. Rather than fetching just an exact sha1, we fetch that sha1 plus another ref: - we build a linked list of refs to fetch when do_fetch calls get_ref_map; the exact sha1 is first, followed by the named ref ("refs/heads/extra" in this case). - we pass that linked list to transport_fetch_ref, which squashes it into an array of pointers - that array goes to fetch_pack, which calls filter_ref. There we generate the want list from a mix of what the remote side has advertised, and the "sought" entry for the exact sha1. We set the sought entry's "next" pointer to NULL. - after we return from transport_fetch_refs, we then try to update the refs by following the linked list. But our list is now truncated, and we do not update refs/heads/extra at all. We can fix this by making a copy of the ref. There's nothing that fetch_pack does to it that must be reflected in the original "sought" list (and indeed, if that were the case we would have a serious bug, because it is only exact-sha1 entries which are treated this way). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-19filter_ref: avoid overwriting ref->old_sha1 with garbageLibravatar Jeff King1-1/+5
If the server supports allow_tip_sha1_in_want, then fetch-pack's filter_refs function tries to check whether a ref is a request for a straight sha1 by running: if (get_sha1_hex(ref->name, ref->old_sha1)) ... I.e., we are using get_sha1_hex to ask "is this ref name a sha1?". If it is true, then the contents of ref->old_sha1 will end up unchanged. But if it is false, then get_sha1_hex makes no guarantees about what it has written. With a ref name like "abcdefoo", we would overwrite 3 bytes of ref->old_sha1 before realizing that it was not a sha1. This is likely not a problem in practice, as anything in refs->name (besides a sha1) will start with "refs/", meaning that we would notice on the first character that there is a problem. Still, we are making assumptions about the state left in the output when get_sha1_hex returns an error (e.g., it could start from the end of the string, or error check the values only once they were placed in the output). It's better to be defensive. We could just check that we have exactly 40 characters of sha1. But let's be even more careful and make sure that we have a 40-char hex refname that matches what is in old_sha1. This is perhaps overly defensive, but spells out our assumptions clearly. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-01lockfile.h: extract new header file for the functions in lockfile.cLibravatar Michael Haggerty1-0/+1
Move the interface declaration for the functions in lockfile.c from cache.h to a new file, lockfile.h. Add #includes where necessary (and remove some redundant includes of cache.h by files that already include builtin.h). Move the documentation of the lock_file state diagram from lockfile.c to the new header file. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-09-11Merge branch 'rs/child-process-init'Libravatar Junio C Hamano1-2/+1
Code clean-up. * rs/child-process-init: run-command: inline prepare_run_command_v_opt() run-command: call run_command_v_opt_cd_env() instead of duplicating it run-command: introduce child_process_init() run-command: introduce CHILD_PROCESS_INIT
2014-08-20run-command: introduce CHILD_PROCESS_INITLibravatar René Scharfe1-2/+1
Most struct child_process variables are cleared using memset first after declaration. Provide a macro, CHILD_PROCESS_INIT, that can be used to initialize them statically instead. That's shorter, doesn't require a function call and is slightly more readable (especially given that we already have STRBUF_INIT, ARGV_ARRAY_INIT etc.). Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-07fetchpack.c: replace `git_config()` with `git_config_get_*()` familyLibravatar Tanay Abhra1-27/+8
Use `git_config_get_*()` family instead of `git_config()` to take advantage of the config-set API which provides a cleaner control flow. Signed-off-by: Tanay Abhra <tanayabh@gmail.com> Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-07-09Merge branch 'jk/skip-prefix'Libravatar Junio C Hamano1-10/+14
* jk/skip-prefix: http-push: refactor parsing of remote object names imap-send: use skip_prefix instead of using magic numbers use skip_prefix to avoid repeated calculations git: avoid magic number with skip_prefix fetch-pack: refactor parsing in get_ack fast-import: refactor parsing of spaces stat_opt: check extra strlen call daemon: use skip_prefix to avoid magic numbers fast-import: use skip_prefix for parsing input use skip_prefix to avoid repeating strings use skip_prefix to avoid magic numbers transport-helper: avoid reading past end-of-string fast-import: fix read of uninitialized argv memory apply: use skip_prefix instead of raw addition refactor skip_prefix to return a boolean avoid using skip_prefix as a boolean daemon: mark some strings as const parse_diff_color_slot: drop ofs parameter
2014-06-20fetch-pack: refactor parsing in get_ackLibravatar Jeff King1-6/+9
There are several uses of the magic number "line+45" when parsing ACK lines from the server, and it's rather unclear why 45 is the correct number. We can make this more clear by keeping a running pointer as we parse, using skip_prefix to jump past the first "ACK ", then adding 40 to jump past get_sha1_hex (which is still magical, but hopefully 40 is less magical to readers of git code). Note that this actually puts us at line+44. The original required some character between the sha1 and further ACK flags (it is supposed to be a space, but we never enforced that). We start our search for flags at line+44, which meanas we are slightly more liberal than the old code. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-20use skip_prefix to avoid magic numbersLibravatar Jeff King1-4/+5
It's a common idiom to match a prefix and then skip past it with a magic number, like: if (starts_with(foo, "bar")) foo += 3; This is easy to get wrong, since you have to count the prefix string yourself, and there's no compiler check if the string changes. We can use skip_prefix to avoid the magic numbers here. Note that some of these conversions could be much shorter. For example: if (starts_with(arg, "--foo=")) { bar = arg + 6; continue; } could become: if (skip_prefix(arg, "--foo=", &bar)) continue; However, I have left it as: if (skip_prefix(arg, "--foo=", &v)) { bar = v; continue; } to visually match nearby cases which need to actually process the string. Like: if (skip_prefix(arg, "--foo=", &v)) { bar = atoi(v); continue; } Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-09Use starts_with() for C strings instead of memcmp()Libravatar René Scharfe1-1/+1
Convert three cases of checking for a constant prefix using memcmp() to starts_with(). This way there is no need for magic string length constants and we avoid running over the end of the string should it be shorter than the prefix. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-04-03Merge branch 'nd/log-show-linear-break'Libravatar Junio C Hamano1-0/+1
Attempts to show where a single-strand-of-pearls break in "git log" output. * nd/log-show-linear-break: log: add --show-linear-break to help see non-linear history object.h: centralize object flag allocation
2014-03-25object.h: centralize object flag allocationLibravatar Nguyễn Thái Ngọc Duy1-0/+1
While the field "flags" is mainly used by the revision walker, it is also used in many other places. Centralize the whole flag allocation to one place for a better overview (and easier to move flags if we have too). Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-03-21Merge branch 'jk/shallow-update-fix'Libravatar Junio C Hamano1-11/+0
Serving objects from a shallow repository needs to write a new file to hold the temporary shallow boundaries but it was not cleaned when we exit due to die() or a signal. * jk/shallow-update-fix: shallow: verify shallow file after taking lock shallow: automatically clean up shallow tempfiles shallow: use stat_validity to check for up-to-date file