{"thread":{"id":"44973","subject":"[RFC 00/14] Allow fetch-pack to send ref names (globs allowed)","startedAt":"2017-01-25T22:03:24Z","lastAt":"2017-02-23T20:14:52Z","messageCount":30,"participants":["Jonathan Tan","Stefan Beller","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"310222","messageId":"cover.1485381677.git.jonathantanmy@google.com","threadId":"44973","inReplyTo":null,"subject":"[RFC 00/14] Allow fetch-pack to send ref names (globs allowed)","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-25T22:02:53Z","receivedAt":"2017-01-25T22:03:24Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Hello everyone - this is a proposal for a protocol change to allow the\nfetch-pack/upload-pack to converse in terms of ref names (globs\nallowed), and also an implementation of the server (upload-pack) and\nfetch-from-HTTP client (fetch-pack invoked through fetch).\n\nNegotiation currently happens by upload-pack initially sending a list of\nrefs with names and SHA-1 hashes, and then several request/response\npairs in which the request from fetch-pack consists of SHA-1 hashes\n(selected from the initial list). Allowing the request to consist of\nnames instead of SHA-1 hashes increases tolerance to refs changing\n(due to time, and due to having load-balanced servers without strong\nconsistency), and is a step towards eliminating the need for the server\nto send the list of refs first (possibly improving performance).\n\nThe protocol is extended by allowing fetch-pack to send\n\"want-ref \", where is a full name (refs/...) [1], possibly\nincluding glob characters. Upload-pack will inform the client of the\nrefs actually matched by sending \"wanted-ref \" before\nsending the last ACK or NAK.\n\nThis patch set is laid out in this way:\n 1-2:\n Upload-pack, protocol documentation, tests that test upload-pack\n independently. A configuration option is added to control if the\n \"ref-in-want\" capability is advertised. (It is always supported even\n if not advertised - this is so that this feature in multiple\n load-balanced servers can be switched on or off without needing any\n atomic switching.)\n 3:\n Mechanism to test a repo that changes over the negotiation (currently,\n only with the existing mechanism).\n 4-9:\n The current transport mechanism takes in an array of refs which is\n used as both input and output. Since we are planning to extend the\n transport mechanism to also allow the specification of ref names\n (which may include globs, and thus do not have a 1-1 correspondence to\n refs), refactor to make this parameter to be solely an input\n parameter.\n 10-11:\n Changes to fetch-pack (which is used by remote-curl) to support\n \"want-ref\".\n 12-13:\n Changes to the rest (fetch -> transport -> transport-helper ->\n remote-curl) to support \"want-ref\" when fetching from HTTP. The\n transport fetch function signature has been widened to allow passing\n in ref names - transports may use those ref names instead of or in\n addition to refs if they support it. (I chose to preserve refs in the\n function signature because many parts of Git, including remote\n helpers, only understand the old functionality anyway, and because\n precalculating the refs allows some optimizations.)\n 14:\n This is not meant for submission - this is just to show that the tests\n pass if ref-in-want was advertised by default (except for some newly\n added tests that explicitly check for the old behavior).\n\n[1] There has been some discussion about whether the server should\naccept partial ref names, e.g. [2]. In this patch set, I have made the\nserver only accept full names, and it is the responsibility of the\nclient to send the multiple patterns which it wants to match. Quoting\nfrom the commit message of the second patch:\n\n For example, a client could reasonably expand an abbreviated\n name \"foo\" to \"want-ref foo\", \"want-ref refs/heads/foo\", \"want-ref\n refs/tags/foo\", among others, and ensure that at least one such ref has\n been fetched.\n\n[2] <20161024132932.i42rqn2vlpocqmkq@sigill.intra.peff.net>\n\nJonathan Tan (14):\n upload-pack: move parsing of \"want\" line\n upload-pack: allow ref name and glob requests\n upload-pack: test negotiation with changing repo\n fetch: refactor the population of hashes\n fetch: refactor fetch_refs into two functions\n fetch: refactor to make function args narrower\n fetch-pack: put shallow info in out param\n fetch-pack: check returned refs for matches\n transport: put ref oid in out param\n fetch-pack: support partial names and globs\n fetch-pack: support want-ref\n fetch-pack: do not printf after closing stdout\n fetch: send want-ref and receive fetched refs\n DONT USE advertise_ref_in_want=1\n\n Documentation/technical/http-protocol.txt | 20 +-\n Documentation/technical/pack-protocol.txt | 24 +-\n Documentation/technical/protocol-capabilities.txt | 6 +\n builtin/clone.c | 16 +-\n builtin/fetch-pack.c | 64 ++--\n builtin/fetch.c | 178 +++++++---\n fetch-pack.c | 226 +++++++++----\n fetch-pack.h | 6 +-\n remote-curl.c | 91 +++--\n remote.c | 67 +++-\n remote.h | 20 +-\n t/lib-httpd.sh | 1 +\n t/lib-httpd/apache.conf | 8 +\n t/lib-httpd/one-time-sed.sh | 8 +\n t/t5500-fetch-pack.sh | 82 +++++\n t/t5536-fetch-conflicts.sh | 2 +\n t/t5552-upload-pack-ref-in-want.sh | 386 ++++++++++++++++++++++\n transport-helper.c | 105 ++++--\n transport.c | 40 ++-\n transport.h | 23 +-\n upload-pack.c | 117 +++++--\n 21 files changed, 1232 insertions(+), 258 deletions(-)\n create mode 100644 t/lib-httpd/one-time-sed.sh\n create mode 100755 t/t5552-upload-pack-ref-in-want.sh\n\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"310223","messageId":"d0d42b3bb4cf755f122591e191354c53848f197d.1485381677.git.jonathantanmy@google.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"[RFC 02/14] upload-pack: allow ref name and glob requests","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-25T22:02:55Z","receivedAt":"2017-01-25T22:03:28Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Currently, while performing packfile negotiation [1], upload-pack allows\nclients to specify their desired objects only as SHA-1s. This causes:\n(a) vulnerability to failure when an object turns non-existent during\n negotiation, which may happen if, for example, upload-pack is\n provided by multiple Git servers in a load-balancing arrangement,\n and\n(b) dependence on the server first publishing a list of refs with\n associated objects.\n\nTo eliminate (a) and take a step towards eliminating (b), teach\nupload-pack to support requests in the form of ref names and globs (in\naddition to the existing support for SHA-1s) through a new line of the\nform \"want-ref \" where ref is the full name of a ref, a glob\npattern, or a SHA-1. At the conclusion of negotiation, the server will\nwrite \"wanted-ref \" for all requests that have been\nspecified this way.\n\nThe server indicates that it supports this feature by advertising the\ncapability \"ref-in-want\". Advertisement of this capability is by default\ndisabled, but can be enabled through a configuration option.\n\nTo be flexible with respect to client needs, the server does not\nindicate an error if a \"want-ref\" line corresponds to no refs, but\ninstead relies on the client to ensure that what the user needs has been\nfetched. For example, a client could reasonably expand an abbreviated\nname \"foo\" to \"want-ref foo\", \"want-ref refs/heads/foo\", \"want-ref\nrefs/tags/foo\", among others, and ensure that at least one such ref has\nbeen fetched.\n\n[1] pack-protocol.txt\n\nSigned-off-by: Jonathan Tan \n---\n Documentation/technical/http-protocol.txt | 20 +-\n Documentation/technical/pack-protocol.txt | 24 +-\n Documentation/technical/protocol-capabilities.txt | 6 +\n t/t5552-upload-pack-ref-in-want.sh | 295 ++++++++++++++++++++++\n upload-pack.c | 89 ++++++-\n 5 files changed, 411 insertions(+), 23 deletions(-)\n create mode 100755 t/t5552-upload-pack-ref-in-want.sh\n\ndiff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt\nindex 1c561bdd9..162d6bc07 100644\n--- a/Documentation/technical/http-protocol.txt\n+++ b/Documentation/technical/http-protocol.txt\n@@ -316,7 +316,8 @@ to prevent caching of the response.\n \n Servers SHOULD support all capabilities defined here.\n \n-Clients MUST send at least one \"want\" command in the request body.\n+Clients MUST send at least one \"want\" or \"want-ref\" command in the\n+request body.\n Clients MUST NOT reference an id in a \"want\" command which did not\n appear in the response obtained through ref discovery unless the\n server advertises capability `allow-tip-sha1-in-want` or\n@@ -330,7 +331,7 @@ server advertises capability `allow-tip-sha1-in-want` or\n want_list = PKT-LINE(want NUL cap_list LF)\n \t\t *(want_pkt)\n want_pkt = PKT-LINE(want LF)\n- want = \"want\" SP id\n+ want = \"want\" SP id / \"want-ref\" SP name\n cap_list = *(SP capability) SP\n \n have_list = *PKT-LINE(\"have\" SP id LF)\n@@ -352,7 +353,8 @@ C: Build an empty set, `common`, to hold the objects that are later\n determined to be on both ends.\n \n C: Build a set, `want`, of the objects from `advertised` the client\n- wants to fetch, based on what it saw during ref discovery.\n+ wants to fetch, based on what it saw during ref discovery. This is to\n+ be used if the server does not support the ref-in-want capability.\n \n C: Start a queue, `c_pending`, ordered by commit time (popping newest\n first). Add all client refs. When a commit is popped from\n@@ -363,8 +365,8 @@ C: Start a queue, `c_pending`, ordered by commit time (popping newest\n \n C: Send one `$GIT_URL/git-upload-pack` request:\n \n- C: 0032want ...............................\n- C: 0032want ...............................\n+ C: want-ref ...............................\n+ C: want-ref ...............................\n ....\n C: 0032have .............................\n C: 0032have .............................\n@@ -384,7 +386,7 @@ the pkt-line value.\n Commands MUST appear in the following order, if they appear\n at all in the request stream:\n \n-* \"want\"\n+* \"want\" or \"want-ref\"\n * \"have\"\n \n The stream is terminated by a pkt-line flush (`0000`).\n@@ -393,6 +395,9 @@ A single \"want\" or \"have\" command MUST have one hex formatted\n SHA-1 as its value. Multiple SHA-1s MUST be sent by sending\n multiple commands.\n \n+A \"want-ref\" command MUST be followed by a ref name (which may include\n+shell glob characters) or a hex formatted SHA-1.\n+\n The `have` list is created by popping the first 32 commits\n from `c_pending`. Less can be supplied if `c_pending` empties.\n \n@@ -410,6 +415,9 @@ Verify all objects in `want` are directly reachable from refs.\n The server MAY walk backwards through history or through\n the reflog to permit slightly stale requests.\n \n+Treat \"want-ref\" requests as if the equivalent 0 or more \"want\" commands\n+(according to the server's refs) were given instead.\n+\n If no \"want\" objects are received, send an error:\n TODO: Define error if no \"want\" lines are requested.\n \ndiff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt\nindex c59ac9936..b8b2ffdf9 100644\n--- a/Documentation/technical/pack-protocol.txt\n+++ b/Documentation/technical/pack-protocol.txt\n@@ -223,15 +223,20 @@ out of what the server said it could do with the first 'want' line.\n \t\t PKT-LINE(\"deepen-since\" SP timestamp) /\n \t\t PKT-LINE(\"deepen-not\" SP ref)\n \n- first-want = PKT-LINE(\"want\" SP obj-id SP capability-list)\n- additional-want = PKT-LINE(\"want\" SP obj-id)\n+ first-want = PKT-LINE(want SP capability-list)\n+ additional-want = PKT-LINE(want)\n+\n+ want = \"want\" SP obj-id / \"want-ref\" SP name\n \n depth = 1*DIGIT\n ----\n \n-Clients MUST send all the obj-ids it wants from the reference\n-discovery phase as 'want' lines. Clients MUST send at least one\n-'want' command in the request body. Clients MUST NOT mention an\n+Clients may specify the objects it wants by selecting obj-ids from the\n+refs that have been advertised (using \"want\") or by specifying ref names\n+and ref patterns directly (using \"want-ref\"). If the latter, the server\n+will return the list of effective refs used using \"wanted-ref\" lines.\n+Either way, clients MUST send at least one\n+'want' or 'want-ref' command in the request body. Clients MUST NOT mention an\n obj-id in a 'want' command which did not appear in the response\n obtained through ref discovery.\n \n@@ -249,7 +254,7 @@ complete those commits. Commits whose parents are not received as a\n result are defined as shallow and marked as such in the server. This\n information is sent back to the client in the next step.\n \n-Once all the 'want's and 'shallow's (and optional 'deepen') are\n+Once all the wants and 'shallow's (and optional 'deepen') are\n transferred, clients MUST send a flush-pkt, to tell the server side\n that it is done sending the list.\n \n@@ -344,7 +349,9 @@ implementation if we have received at least one \"ACK %s continue\"\n during a prior round. This helps to ensure that at least one common\n ancestor is found before we give up entirely.\n \n-Once the 'done' line is read from the client, the server will either\n+Once the 'done' line is read from the client, the server will send all the\n+refs corresponding to \"want-ref\" lines from the client (prefixed by\n+\"wanted-ref\"), and then either\n send a final 'ACK obj-id' or it will send a 'NAK'. 'obj-id' is the object\n name of the last commit determined to be common. The server only sends\n ACK after 'done' if there is at least one common base and multi_ack or\n@@ -354,9 +361,10 @@ if there is no common base found.\n Then the server will start sending its packfile data.\n \n ----\n- server-response = *ack_multi ack / nak\n+ server-response = *ack_multi *wanted_ref ack / nak\n ack_multi = PKT-LINE(\"ACK\" SP obj-id ack_status)\n ack_status = \"continue\" / \"common\" / \"ready\"\n+ wanted_ref = PKT-LINE(\"wanted-ref\" SP obj-id SP name)\n ack = PKT-LINE(\"ACK\" SP obj-id)\n nak = PKT-LINE(\"NAK\")\n ----\ndiff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt\nindex 26dcc6f50..72a2ff907 100644\n--- a/Documentation/technical/protocol-capabilities.txt\n+++ b/Documentation/technical/protocol-capabilities.txt\n@@ -309,3 +309,9 @@ to accept a signed push certificate, and asks the to be\n included in the push certificate. A send-pack client MUST NOT\n send a push-cert packet unless the receive-pack server advertises\n this capability.\n+\n+ref-in-want\n+-----------\n+\n+If the upload-pack server advertises this capability, fetch-pack may\n+send \"want-ref\" lines.\ndiff --git a/t/t5552-upload-pack-ref-in-want.sh b/t/t5552-upload-pack-ref-in-want.sh\nnew file mode 100755\nindex 000000000..3496af641\n--- /dev/null\n+++ b/t/t5552-upload-pack-ref-in-want.sh\n@@ -0,0 +1,295 @@\n+#!/bin/sh\n+\n+test_description='upload-pack ref-in-want'\n+\n+. ./test-lib.sh\n+\n+line() {\n+\tlen=$(printf '%s' \"$1\" | wc -c) &&\n+\tlen=$((len + 5)) &&\n+\tprintf '%04x%s\\n' $len \"$1\" >>input\n+}\n+\n+flush() {\n+\tprintf '0000'\n+}\n+\n+get_actual_refs() {\n+\tgrep -a wanted-ref output | sed \"s/^.*wanted-ref //\" >actual_refs\n+}\n+\n+get_actual_commits() {\n+\tperl -0777 -p -e \"s/^.*PACK/PACK/s\" o.pack &&\n+\tgit index-pack o.pack &&\n+\tgit verify-pack -v o.idx | grep commit | cut -c-40 | sort >actual_commits\n+}\n+\n+check_output() {\n+\tget_actual_refs &&\n+\ttest_cmp expected_refs actual_refs &&\n+\tget_actual_commits &&\n+\ttest_cmp expected_commits actual_commits\n+}\n+\n+# c(o/foo) d(o/bar)\n+# \\ /\n+# b e(baz) f(master)\n+# \\__ | __/\n+# \\ | /\n+# a\n+test_expect_success 'setup repository' '\n+\ttest_commit a &&\n+\tgit checkout -b o/foo &&\n+\ttest_commit b &&\n+\ttest_commit c &&\n+\tgit checkout -b o/bar b &&\n+\ttest_commit d &&\n+\tgit checkout -b baz a &&\n+\ttest_commit e &&\n+\tgit checkout master &&\n+\ttest_commit f\n+'\n+\n+test_expect_success 'config controls ref-in-want advertisement' '\n+\ttest_config uploadpack.advertiseRefInWant false &&\n+\tprintf \"0000\" | git upload-pack . >output &&\n+\t! grep -a ref-in-want output &&\n+\ttest_config uploadpack.advertiseRefInWant true &&\n+\tprintf \"0000\" | git upload-pack . >output &&\n+\tgrep -a ref-in-want output\n+'\n+\n+test_expect_success 'mix want and want-ref' '\n+\tcat >expected_refs <<-EOF &&\n+\t$(git rev-parse f) refs/heads/master\n+\tEOF\n+\tgit rev-parse e f | sort >expected_commits &&\n+\n+\tline \"want-ref refs/heads/master\" >input &&\n+\tline \"want $(git rev-parse e)\" >>input &&\n+\tflush >>input &&\n+\tline \"have $(git rev-parse a)\" >>input &&\n+\tflush >>input &&\n+\tline \"done\" >>input &&\n+\tgit upload-pack . output &&\n+\tcheck_output\n+'\n+\n+test_expect_success 'want-ref with glob and non-glob' '\n+\tcat >expected_refs <<-EOF &&\n+\t$(git rev-parse f) refs/heads/master\n+\t$(git rev-parse d) refs/heads/o/bar\n+\t$(git rev-parse c) refs/heads/o/foo\n+\tEOF\n+\tgit rev-parse b c d f | sort >expected_commits &&\n+\n+\tline \"want-ref refs/head*/[op]/*\" >input &&\n+\tline \"want-ref refs/heads/master\" >>input &&\n+\tline \"want-ref refs/heads/non-existent/*\" >>input &&\n+\tline \"want-ref refs/heads/also-non-existent\" >>input &&\n+\tflush >>input &&\n+\tline \"have $(git rev-parse a)\" >>input &&\n+\tflush >>input &&\n+\tline \"done\" >>input &&\n+\tgit upload-pack . output &&\n+\tcheck_output\n+'\n+\n+test_expect_success 'want-ref with SHA-1' '\n+\tcat >expected_refs <<-EOF &&\n+\t$(git rev-parse f) $(git rev-parse f)\n+\tEOF\n+\tgit rev-parse f | sort >expected_commits &&\n+\n+\tline \"want-ref $(git rev-parse f)\" >input &&\n+\tflush >>input &&\n+\tline \"have $(git rev-parse a)\" >>input &&\n+\tflush >>input &&\n+\tline \"done\" >>input &&\n+\tgit upload-pack . output &&\n+\tcheck_output\n+'\n+\n+test_expect_success 'want-ref with overlapping glob and non-glob' '\n+\tcat >expected_refs <<-EOF &&\n+\t$(git rev-parse d) refs/heads/o/bar\n+\t$(git rev-parse c) refs/heads/o/foo\n+\tEOF\n+\tgit rev-parse b c d | sort >expected_commits &&\n+\n+\tline \"want-ref refs/heads/o/*\" >input &&\n+\tline \"want-ref refs/heads/o/foo\" >>input &&\n+\tflush >>input &&\n+\tline \"have $(git rev-parse a)\" >>input &&\n+\tflush >>input &&\n+\tline \"done\" >>input &&\n+\tgit upload-pack . output &&\n+\tcheck_output\n+'\n+\n+test_expect_success 'want-ref with overlapping non-glob and SHA-1' '\n+\tcat >expected_refs <<-EOF &&\n+\t$(git rev-parse f) $(git rev-parse f)\n+\t$(git rev-parse f) refs/heads/master\n+\tEOF\n+\tgit rev-parse f | sort >expected_commits &&\n+\n+\tline \"want-ref $(git rev-parse f)\" >input &&\n+\tline \"want-ref refs/heads/master\" >>input &&\n+\tflush >>input &&\n+\tline \"have $(git rev-parse a)\" >>input &&\n+\tflush >>input &&\n+\tline \"done\" >>input &&\n+\tgit upload-pack . output &&\n+\tcheck_output\n+'\n+\n+test_expect_success 'want-ref with overlapping glob and SHA-1' '\n+\tcat >expected_refs <<-EOF &&\n+\t$(git rev-parse f) $(git rev-parse f)\n+\t$(git rev-parse f) refs/heads/master\n+\tEOF\n+\tgit rev-parse f | sort >expected_commits &&\n+\n+\tline \"want-ref $(git rev-parse f)\" >input &&\n+\tline \"want-ref refs/heads/mas*\" >>input &&\n+\tflush >>input &&\n+\tline \"have $(git rev-parse a)\" >>input &&\n+\tflush >>input &&\n+\tline \"done\" >>input &&\n+\tgit upload-pack . output &&\n+\tcheck_output\n+'\n+\n+test_expect_success 'want-ref with overlapping globs' '\n+\tcat >expected_refs <<-EOF &&\n+\t$(git rev-parse d) refs/heads/o/bar\n+\t$(git rev-parse c) refs/heads/o/foo\n+\tEOF\n+\tgit rev-parse b c d | sort >expected_commits &&\n+\n+\tline \"want-ref refs/heads/o/*\" >input &&\n+\tline \"want-ref refs/heads/o/f*\" >>input &&\n+\tflush >>input &&\n+\tline \"have $(git rev-parse a)\" >>input &&\n+\tflush >>input &&\n+\tline \"done\" >>input &&\n+\tgit upload-pack . output &&\n+\tcheck_output\n+'\n+\n+test_expect_success 'want-ref with ref we already have commit for' '\n+\tcat >expected_refs <<-EOF &&\n+\t$(git rev-parse c) refs/heads/o/foo\n+\tEOF\n+\t>expected_commits &&\n+\n+\tline \"want-ref refs/heads/o/foo\" >input &&\n+\tflush >>input &&\n+\tline \"have $(git rev-parse c)\" >>input &&\n+\tflush >>input &&\n+\tline \"done\" >>input &&\n+\tgit upload-pack . output &&\n+\tcheck_output\n+'\n+\n+test_expect_success 'send wanted-ref only at the end of negotiation' '\n+\t# Incomplete input; acknowledge \"have\" with NAK, but no wanted-ref\n+\tline \"want-ref refs/heads/o/foo\" >input &&\n+\tflush >>input &&\n+\tline \"have 1234567890123456789012345678901234567890\" >>input &&\n+\tflush >>input &&\n+\ttest_must_fail git upload-pack . output &&\n+\tgrep -a \"0008NAK\" output &&\n+\ttest_must_fail grep -a \"wanted-ref\" output &&\n+\n+\t# Complete the input, and try again\n+\tline \"have $(git rev-parse c)\" >>input &&\n+\tflush >>input &&\n+\tline \"done\" >>input &&\n+\tflush >>input &&\n+\tgit upload-pack . output &&\n+\tgrep -a \"wanted-ref\" output\n+'\n+\n+test_expect_success 'want-ref with capability declaration' '\n+\tcat >expected_refs <<-EOF &&\n+\t$(git rev-parse c) refs/heads/o/foo\n+\tEOF\n+\tgit rev-parse b c | sort >expected_commits &&\n+\n+\tline \"want-ref refs/heads/o/foo no_progress\" >input &&\n+\tflush >>input &&\n+\tline \"have $(git rev-parse a)\" >>input &&\n+\tflush >>input &&\n+\tline \"done\" >>input &&\n+\tgit upload-pack . output &&\n+\tcheck_output\n+'\n+\n+test_expect_success 'hideRefs' '\n+\ttest_config transfer.hideRefs refs/heads/o/foo &&\n+\n+\tcat >expected_refs <<-EOF &&\n+\t$(git rev-parse d) refs/heads/o/bar\n+\tEOF\n+\tgit rev-parse b d | sort >expected_commits &&\n+\n+\tline \"want-ref refs/heads/o/*\" >input &&\n+\tflush >>input &&\n+\tline \"have $(git rev-parse a)\" >>input &&\n+\tflush >>input &&\n+\tline \"done\" >>input &&\n+\tgit upload-pack . output &&\n+\tcheck_output\n+'\n+\n+test_expect_success 'setup namespaced repo' '\n+\tgit init n &&\n+\t(\n+\t\tcd n &&\n+\t\ttest_commit a &&\n+\t\ttest_commit b &&\n+\t\tgit checkout a &&\n+\t\ttest_commit c &&\n+\t\tgit checkout a &&\n+\t\ttest_commit d &&\n+\t\tgit update-ref refs/heads/ns-no b\n+\t\tgit update-ref refs/namespaces/ns/refs/heads/ns-yes c\n+\t\tgit update-ref refs/namespaces/ns/refs/heads/another d\n+\t)\n+'\n+\n+test_expect_success 'want-ref with namespaces' '\n+\tcat >expected_refs <<-EOF &&\n+\t$(git -C n rev-parse c) refs/heads/ns-yes\n+\tEOF\n+\tgit -C n rev-parse c | sort >expected_commits &&\n+\n+\tline \"want-ref refs/heads/ns-*\" >input &&\n+\tflush >>input &&\n+\tline \"have $(git -C n rev-parse a)\" >>input &&\n+\tflush >>input &&\n+\tline \"done\" >>input &&\n+\tgit --namespace=ns -C n upload-pack . output &&\n+\tcheck_output\n+'\n+\n+test_expect_success 'hideRefs with namespaces' '\n+\ttest_config transfer.hideRefs refs/heads/another &&\n+\n+\tcat >expected_refs <<-EOF &&\n+\t$(git -C n rev-parse c) refs/heads/ns-yes\n+\tEOF\n+\tgit -C n rev-parse c | sort >expected_commits &&\n+\n+\tline \"want-ref refs/heads/ns-*\" >input &&\n+\tflush >>input &&\n+\tline \"have $(git -C n rev-parse a)\" >>input &&\n+\tflush >>input &&\n+\tline \"done\" >>input &&\n+\tgit --namespace=ns -C n upload-pack . output &&\n+\tcheck_output\n+'\n+\n+test_done\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 15c60a204..b88ed8e26 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -62,6 +62,7 @@ static int use_sideband;\n static int advertise_refs;\n static int stateless_rpc;\n static const char *pack_objects_hook;\n+static int advertise_ref_in_want;\n \n static void reset_timeout(void)\n {\n@@ -380,7 +381,25 @@ static int ok_to_give_up(void)\n \treturn 1;\n }\n \n-static int get_common_commits(void)\n+static void write_wanted_ns_refs(const struct string_list *wanted_ns_refs)\n+{\n+\tconst struct string_list_item *item;\n+\tfor_each_string_list_item(item, wanted_ns_refs) {\n+\t\tunsigned char sha1[GIT_SHA1_RAWSZ];\n+\t\tif (!get_sha1_hex(item->string, sha1)) {\n+\t\t\tpacket_write_fmt(1, \"wanted-ref %s %s\\n\", item->string,\n+\t\t\t\t\t item->string);\n+\t\t} else {\n+\t\t\tif (read_ref(item->string, sha1))\n+\t\t\t\tdie(\"unable to read ref %s\", item->string);\n+\t\t\tpacket_write_fmt(1, \"wanted-ref %s %s\\n\",\n+\t\t\t\t\t sha1_to_hex(sha1),\n+\t\t\t\t\t strip_namespace(item->string));\n+\t\t}\n+\t}\n+}\n+\n+static int get_common_commits(const struct string_list *wanted_ns_refs)\n {\n \tunsigned char sha1[20];\n \tchar last_hex[41];\n@@ -442,6 +461,7 @@ static int get_common_commits(void)\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(line, \"done\")) {\n+\t\t\twrite_wanted_ns_refs(wanted_ns_refs);\n \t\t\tif (have_obj.nr > 0) {\n \t\t\t\tif (multi_ack)\n \t\t\t\t\tpacket_write_fmt(1, \"ACK %s\\n\", last_hex);\n@@ -729,7 +749,26 @@ static void deepen_by_rev_list(int ac, const char **av,\n \tpacket_flush(1);\n }\n \n-static void receive_needs(void)\n+static int mark_ref_wanted(const char *ns_ref,\n+\t\t\t const struct object_id *oid, int flags,\n+\t\t\t void *wanted_ns_refs_)\n+{\n+\tstruct string_list *wanted_ns_refs = wanted_ns_refs_;\n+\tstruct object *o;\n+\n+\tif (ref_is_hidden(strip_namespace(ns_ref), ns_ref))\n+\t\treturn 0;\n+\n+\to = parse_object_or_die(oid->hash, ns_ref);\n+\tif (!(o->flags & WANTED)) {\n+\t\to->flags |= WANTED;\n+\t\tadd_object_array(o, NULL, &want_obj);\n+\t}\n+\tstring_list_insert(wanted_ns_refs, ns_ref);\n+\treturn 0;\n+}\n+\n+static void receive_needs(struct string_list *wanted_ns_refs)\n {\n \tstruct object_array shallows = OBJECT_ARRAY_INIT;\n \tstruct string_list deepen_not = STRING_LIST_INIT_DUP;\n@@ -793,8 +832,35 @@ static void receive_needs(void)\n \t\t\tdeepen_rev_list = 1;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (skip_prefix(line, \"want \", &arg) &&\n-\t\t !get_sha1_hex(arg, sha1_buf)) {\n+\t\tif (skip_prefix(line, \"want-ref \", &arg)) {\n+\t\t\tstruct object_id oid;\n+\n+\t\t\tchar *space = strchrnul(arg, ' ');\n+\t\t\tchar *ns_ref = xstrfmt(\"%s%.*s\",\n+\t\t\t\t\t get_git_namespace(),\n+\t\t\t\t\t (int)(space - arg),\n+\t\t\t\t\t arg);\n+\t\t\tif (has_glob_specials(ns_ref))\n+\t\t\t\tfor_each_glob_ref(mark_ref_wanted, ns_ref,\n+\t\t\t\t\t\t wanted_ns_refs);\n+\t\t\telse if (!get_oid_hex(arg, &oid)) {\n+\t\t\t\to = parse_object(oid.hash);\n+\t\t\t\tif (o && !(o->flags & WANTED)) {\n+\t\t\t\t\to->flags |= WANTED;\n+\t\t\t\t\tif (!((allow_unadvertised_object_request & ALLOW_ANY_SHA1) == ALLOW_ANY_SHA1\n+\t\t\t\t\t || is_our_ref(o)))\n+\t\t\t\t\t\thas_non_tip = 1;\n+\t\t\t\t\tadd_object_array(o, NULL, &want_obj);\n+\t\t\t\t}\n+\t\t\t\tmark_ref_wanted(ns_ref, &oid, 0,\n+\t\t\t\t\t\twanted_ns_refs);\n+\t\t\t} else if (!read_ref(ns_ref, oid.hash))\n+\t\t\t\tmark_ref_wanted(ns_ref, &oid, 0,\n+\t\t\t\t\t\twanted_ns_refs);\n+\t\t\tfree(ns_ref);\n+\t\t\tfeatures = space;\n+\t\t} else if (skip_prefix(line, \"want \", &arg) &&\n+\t\t\t !get_sha1_hex(arg, sha1_buf)) {\n \t\t\to = parse_object(sha1_buf);\n \t\t\tif (!o)\n \t\t\t\tdie(\"git upload-pack: not our ref %s\",\n@@ -806,12 +872,11 @@ static void receive_needs(void)\n \t\t\t\t\thas_non_tip = 1;\n \t\t\t\tadd_object_array(o, NULL, &want_obj);\n \t\t\t}\n+\t\t\tfeatures = arg + 40;\n \t\t} else\n \t\t\tdie(\"git upload-pack: protocol error, \"\n \t\t\t \"expected to get sha, not '%s'\", line);\n \n-\t\tfeatures = arg + 40;\n-\n \t\tif (parse_feature_request(features, \"deepen-relative\"))\n \t\t\tdeepen_relative = 1;\n \t\tif (parse_feature_request(features, \"multi_ack_detailed\"))\n@@ -935,7 +1000,7 @@ static int send_ref(const char *refname, const struct object_id *oid,\n \t\tstruct strbuf symref_info = STRBUF_INIT;\n \n \t\tformat_symref_info(&symref_info, cb_data);\n-\t\tpacket_write_fmt(1, \"%s %s%c%s%s%s%s%s agent=%s\\n\",\n+\t\tpacket_write_fmt(1, \"%s %s%c%s%s%s%s%s%s agent=%s\\n\",\n \t\t\t oid_to_hex(oid), refname_nons,\n \t\t\t 0, capabilities,\n \t\t\t (allow_unadvertised_object_request & ALLOW_TIP_SHA1) ?\n@@ -943,6 +1008,8 @@ static int send_ref(const char *refname, const struct object_id *oid,\n \t\t\t (allow_unadvertised_object_request & ALLOW_REACHABLE_SHA1) ?\n \t\t\t\t \" allow-reachable-sha1-in-want\" : \"\",\n \t\t\t stateless_rpc ? \" no-done\" : \"\",\n+\t\t\t advertise_ref_in_want ?\n+\t\t\t\t \" ref-in-want\" : \"\",\n \t\t\t symref_info.buf,\n \t\t\t git_user_agent_sanitized());\n \t\tstrbuf_release(&symref_info);\n@@ -975,6 +1042,7 @@ static int find_symref(const char *refname, const struct object_id *oid,\n static void upload_pack(void)\n {\n \tstruct string_list symref = STRING_LIST_INIT_DUP;\n+\tstruct string_list wanted_ns_refs = STRING_LIST_INIT_DUP;\n \n \thead_ref_namespaced(find_symref, &symref);\n \n@@ -992,11 +1060,12 @@ static void upload_pack(void)\n \tif (advertise_refs)\n \t\treturn;\n \n-\treceive_needs();\n+\treceive_needs(&wanted_ns_refs);\n \tif (want_obj.nr) {\n-\t\tget_common_commits();\n+\t\tget_common_commits(&wanted_ns_refs);\n \t\tcreate_pack_file();\n \t}\n+\tstring_list_clear(&wanted_ns_refs, 0);\n }\n \n static int upload_pack_config(const char *var, const char *value, void *unused)\n@@ -1020,6 +1089,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n \t\tkeepalive = git_config_int(var, value);\n \t\tif (!keepalive)\n \t\t\tkeepalive = -1;\n+\t} else if (!strcmp(\"uploadpack.advertiserefinwant\", var)) {\n+\t\tadvertise_ref_in_want = git_config_bool(var, value);\n \t} else if (current_config_scope() != CONFIG_SCOPE_REPO) {\n \t\tif (!strcmp(\"uploadpack.packobjectshook\", var))\n \t\t\treturn git_config_string(&pack_objects_hook, var, value);\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"310224","messageId":"be83ed22dbea03511bf8f8fe830ca0a365128c05.1485381677.git.jonathantanmy@google.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"[RFC 01/14] upload-pack: move parsing of \"want\" line","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-25T22:02:54Z","receivedAt":"2017-01-25T22:03:32Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Refactor to parse \"want\" lines when the prefix is found. This makes it\neasier to add support for another prefix, which will be done in a\nsubsequent commit.\n\nSigned-off-by: Jonathan Tan \n---\n upload-pack.c | 28 ++++++++++++++--------------\n 1 file changed, 14 insertions(+), 14 deletions(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 7597ba340..15c60a204 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -793,8 +793,20 @@ static void receive_needs(void)\n \t\t\tdeepen_rev_list = 1;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (!skip_prefix(line, \"want \", &arg) ||\n-\t\t get_sha1_hex(arg, sha1_buf))\n+\t\tif (skip_prefix(line, \"want \", &arg) &&\n+\t\t !get_sha1_hex(arg, sha1_buf)) {\n+\t\t\to = parse_object(sha1_buf);\n+\t\t\tif (!o)\n+\t\t\t\tdie(\"git upload-pack: not our ref %s\",\n+\t\t\t\t sha1_to_hex(sha1_buf));\n+\t\t\tif (!(o->flags & WANTED)) {\n+\t\t\t\to->flags |= WANTED;\n+\t\t\t\tif (!((allow_unadvertised_object_request & ALLOW_ANY_SHA1) == ALLOW_ANY_SHA1\n+\t\t\t\t || is_our_ref(o)))\n+\t\t\t\t\thas_non_tip = 1;\n+\t\t\t\tadd_object_array(o, NULL, &want_obj);\n+\t\t\t}\n+\t\t} else\n \t\t\tdie(\"git upload-pack: protocol error, \"\n \t\t\t \"expected to get sha, not '%s'\", line);\n \n@@ -820,18 +832,6 @@ static void receive_needs(void)\n \t\t\tno_progress = 1;\n \t\tif (parse_feature_request(features, \"include-tag\"))\n \t\t\tuse_include_tag = 1;\n-\n-\t\to = parse_object(sha1_buf);\n-\t\tif (!o)\n-\t\t\tdie(\"git upload-pack: not our ref %s\",\n-\t\t\t sha1_to_hex(sha1_buf));\n-\t\tif (!(o->flags & WANTED)) {\n-\t\t\to->flags |= WANTED;\n-\t\t\tif (!((allow_unadvertised_object_request & ALLOW_ANY_SHA1) == ALLOW_ANY_SHA1\n-\t\t\t || is_our_ref(o)))\n-\t\t\t\thas_non_tip = 1;\n-\t\t\tadd_object_array(o, NULL, &want_obj);\n-\t\t}\n \t}\n \n \t/*\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"310225","messageId":"db1db5d0e5563464c09d1678234c9c5e8ae5b2f4.1485381677.git.jonathantanmy@google.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"[RFC 12/14] fetch-pack: do not printf after closing stdout","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-25T22:03:05Z","receivedAt":"2017-01-25T22:03:35Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"In fetch-pack, during a stateless RPC, printf is invoked after stdout is\nclosed. Update the code to not do this, preserving the existing\nbehavior.\n\nSigned-off-by: Jonathan Tan \n---\n builtin/fetch-pack.c | 14 ++++++++++----\n 1 file changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex ae073ab24..24af3b7c5 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -191,10 +191,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t\tprintf(\"connectivity-ok\\n\");\n \t\tfflush(stdout);\n \t}\n-\tclose(fd[0]);\n-\tclose(fd[1]);\n-\tif (finish_connect(conn))\n-\t\treturn 1;\n+\tif (finish_connect(conn)) {\n+\t\tret = 1;\n+\t\tgoto cleanup;\n+\t}\n \n \tret = !ref;\n \n@@ -218,11 +218,17 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t\tret = 1;\n \t}\n \n+\tif (args.stateless_rpc)\n+\t\tgoto cleanup;\n+\n \twhile (ref) {\n \t\tprintf(\"%s %s\\n\",\n \t\t oid_to_hex(&ref->old_oid), ref->name);\n \t\tref = ref->next;\n \t}\n \n+cleanup:\n+\tclose(fd[0]);\n+\tclose(fd[1]);\n \treturn ret;\n }\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"310226","messageId":"eef2b77d88df0db08e4a1505b06e0af2d40143d5.1485381677.git.jonathantanmy@google.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"[RFC 13/14] fetch: send want-ref and receive fetched refs","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-25T22:03:06Z","receivedAt":"2017-01-25T22:03:39Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Teach fetch to send refspecs to the underlying transport, and teach all\ncomponents used by the HTTP transport (remote-curl, transport-helper,\nfetch-pack) to understand and propagate the names and SHA-1s of the refs\nfetched.\n\nSigned-off-by: Jonathan Tan \n---\n builtin/clone.c | 4 +-\n builtin/fetch-pack.c | 8 ++-\n builtin/fetch.c | 100 ++++++++++++++++++++++++++++++-------\n remote-curl.c | 91 ++++++++++++++++++++-------------\n t/t5552-upload-pack-ref-in-want.sh | 4 +-\n transport-helper.c | 74 +++++++++++++++++++++------\n transport.c | 10 ++--\n transport.h | 20 +++++---\n 8 files changed, 229 insertions(+), 82 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 3191da391..765a3a3b6 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1078,7 +1078,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t}\n \n \t\tif (!is_local && !complete_refs_before_fetch) {\n-\t\t\ttransport_fetch_refs(transport, mapped_refs,\n+\t\t\ttransport_fetch_refs(transport, NULL, 0, mapped_refs,\n \t\t\t\t\t &new_remote_refs);\n \t\t\tif (new_remote_refs) {\n \t\t\t\trefs = new_remote_refs;\n@@ -1124,7 +1124,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (is_local)\n \t\tclone_local(path, git_dir);\n \telse if (refs && complete_refs_before_fetch)\n-\t\ttransport_fetch_refs(transport, mapped_refs, NULL);\n+\t\ttransport_fetch_refs(transport, NULL, 0, mapped_refs, NULL);\n \n \tupdate_remote_refs(refs, mapped_refs, remote_head_points_at,\n \t\t\t branch_top.buf, reflog_msg.buf, transport, !is_local);\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 24af3b7c5..ed1608c12 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -35,6 +35,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \tstruct fetch_pack_args args;\n \tstruct sha1_array shallow = SHA1_ARRAY_INIT;\n \tstruct string_list deepen_not = STRING_LIST_INIT_DUP;\n+\tint always_print_refs = 0;\n \n \tpacket_trace_identity(\"fetch-pack\");\n \n@@ -126,6 +127,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t\t\targs.update_shallow = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strcmp(\"--always-print-refs\", arg)) {\n+\t\t\talways_print_refs = 1;\n+\t\t\tcontinue;\n+\t\t}\n \t\tusage(fetch_pack_usage);\n \t}\n \tif (deepen_not.nr)\n@@ -218,7 +223,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t\tret = 1;\n \t}\n \n-\tif (args.stateless_rpc)\n+\tif (args.stateless_rpc && !always_print_refs)\n \t\tgoto cleanup;\n \n \twhile (ref) {\n@@ -226,6 +231,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t\t oid_to_hex(&ref->old_oid), ref->name);\n \t\tref = ref->next;\n \t}\n+\tfflush(stdout);\n \n cleanup:\n \tclose(fd[0]);\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 19e3f40a0..87de00e49 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -302,10 +302,75 @@ static void find_non_local_tags(const struct ref *refs,\n \tstring_list_clear(&remote_refs, 0);\n }\n \n+static void get_effective_refspecs(struct refspec **e_rs, int *e_rs_nr,\n+\t\t\t\t const struct remote *remote,\n+\t\t\t\t const struct refspec *cli_rs, int cli_rs_nr,\n+\t\t\t\t int tags, int *autotags)\n+{\n+\tstatic struct refspec head_refspec;\n+\n+\tconst struct refspec *base_rs;\n+\tint base_rs_nr;\n+\tstruct branch *merge_branch = NULL;\n+\tint i;\n+\n+\tstruct refspec *rs;\n+\tint nr, alloc;\n+\n+\tif (cli_rs_nr) {\n+\t\tbase_rs = cli_rs;\n+\t\tbase_rs_nr = cli_rs_nr;\n+\t} else if (refmap_array) {\n+\t\tdie(\"--refmap option is only meaningful with command-line refspec(s).\");\n+\t} else {\n+\t\t/* Use the defaults */\n+\t\tstruct branch *branch = branch_get(NULL);\n+\t\tint has_merge = branch_has_merge_config(branch);\n+\t\t/* Note: has_merge implies non-NULL branch->remote_name */\n+\t\tif (has_merge && !strcmp(branch->remote_name, remote->name))\n+\t\t\t/*\n+\t\t\t * if the remote we're fetching from is the same\n+\t\t\t * as given in branch..remote, we add the\n+\t\t\t * ref given in branch..merge, too.\n+\t\t\t */\n+\t\t\tmerge_branch = branch;\n+\t\tif (remote &&\n+\t\t (remote->fetch_refspec_nr || merge_branch)) {\n+\t\t\tbase_rs = remote->fetch;\n+\t\t\tbase_rs_nr = remote->fetch_refspec_nr;\n+\t\t} else {\n+\t\t\thead_refspec.src = \"HEAD\";\n+\t\t\tbase_rs = &head_refspec;\n+\t\t\tbase_rs_nr = 1;\n+\t\t}\n+\t}\n+\n+\tfor (i = 0; i < base_rs_nr; i++)\n+\t\tif (base_rs[i].dst && base_rs[i].dst[0]) {\n+\t\t\t*autotags = 1;\n+\t\t\tbreak;\n+\t\t}\n+\n+\talloc = base_rs_nr +\n+\t\t(merge_branch ? merge_branch->merge_nr : 0) +\n+\t\t(tags == TAGS_SET);\n+\trs = xcalloc(alloc, sizeof(*rs));\n+\tmemcpy(rs, base_rs, base_rs_nr * sizeof(*rs));\n+\tnr = base_rs_nr;\n+\tif (merge_branch)\n+\t\tfor (i = 0; i < merge_branch->merge_nr; i++)\n+\t\t\trs[nr++].src = merge_branch->merge[i]->src;\n+\tif (tags == TAGS_SET)\n+\t\trs[nr++] = *tag_refspec;\n+\n+\t*e_rs = rs;\n+\t*e_rs_nr = nr;\n+}\n+\n static struct ref *get_ref_map(const struct remote *remote,\n \t\t\t const struct ref *remote_refs,\n \t\t\t struct refspec *refspecs, int refspec_count,\n-\t\t\t int tags, int *autotags)\n+\t\t\t int tags, int autotags)\n {\n \tint i;\n \tstruct ref *rm;\n@@ -321,11 +386,8 @@ static struct ref *get_ref_map(const struct remote *remote,\n \t\tstruct refspec *fetch_refspec;\n \t\tint fetch_refspec_nr;\n \n-\t\tfor (i = 0; i < refspec_count; i++) {\n+\t\tfor (i = 0; i < refspec_count; i++)\n \t\t\tget_fetch_map(remote_refs, &refspecs[i], &tail, 0);\n-\t\t\tif (refspecs[i].dst && refspecs[i].dst[0])\n-\t\t\t\t*autotags = 1;\n-\t\t}\n \t\t/* Merge everything on the command line (but not --tags) */\n \t\tfor (rm = ref_map; rm; rm = rm->next)\n \t\t\trm->fetch_head_status = FETCH_HEAD_MERGE;\n@@ -372,9 +434,6 @@ static struct ref *get_ref_map(const struct remote *remote,\n \t\t (has_merge && !strcmp(branch->remote_name, remote->name)))) {\n \t\t\tfor (i = 0; i < remote->fetch_refspec_nr; i++) {\n \t\t\t\tget_fetch_map(remote_refs, &remote->fetch[i], &tail, 0);\n-\t\t\t\tif (remote->fetch[i].dst &&\n-\t\t\t\t remote->fetch[i].dst[0])\n-\t\t\t\t\t*autotags = 1;\n \t\t\t\tif (!i && !has_merge && ref_map &&\n \t\t\t\t !remote->fetch[0].pattern)\n \t\t\t\t\tref_map->fetch_head_status = FETCH_HEAD_MERGE;\n@@ -401,7 +460,7 @@ static struct ref *get_ref_map(const struct remote *remote,\n \tif (tags == TAGS_SET)\n \t\t/* also fetch all tags */\n \t\tget_fetch_map(remote_refs, tag_refspec, &tail, 0);\n-\telse if (tags == TAGS_DEFAULT && *autotags)\n+\telse if (tags == TAGS_DEFAULT && autotags)\n \t\tfind_non_local_tags(remote_refs, &ref_map, &tail);\n \n \t/* Now append any refs to be updated opportunistically: */\n@@ -911,13 +970,14 @@ static int quickfetch(struct ref *ref_map)\n \treturn check_connected(iterate_ref_map, &rm, &opt);\n }\n \n-static int fetch_refs(struct transport *transport, struct ref *ref_map,\n-\t\t struct ref **updated_remote_refs)\n+static int fetch_refs(struct transport *transport,\n+\t\t struct refspec *refspecs, int refspec_nr,\n+\t\t struct ref *ref_map, struct ref **updated_remote_refs)\n {\n \tint ret = quickfetch(ref_map);\n \tif (ret)\n-\t\tret = transport_fetch_refs(transport, ref_map,\n-\t\t\t\t\t updated_remote_refs);\n+\t\tret = transport_fetch_refs(transport, refspecs, refspec_nr,\n+\t\t\t\t\t ref_map, updated_remote_refs);\n \tif (ret)\n \t\ttransport_unlock_pack(transport);\n \treturn ret;\n@@ -1068,7 +1128,7 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)\n \ttransport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);\n \ttransport_set_option(transport, TRANS_OPT_DEPTH, \"0\");\n \ttransport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);\n-\tif (!fetch_refs(transport, ref_map, NULL))\n+\tif (!fetch_refs(transport, NULL, 0, ref_map, NULL))\n \t\tconsume_refs(transport, ref_map);\n \n \tif (gsecondary) {\n@@ -1083,6 +1143,10 @@ static int do_fetch(struct transport *transport,\n \tstruct ref *ref_map;\n \tint autotags = (transport->remote->fetch_tags == 1);\n \tint retcode = 0;\n+\n+\tstruct refspec *e_rs;\n+\tint e_rs_nr;\n+\n \tconst struct ref *remote_refs;\n \tstruct ref *new_remote_refs = NULL;\n \n@@ -1103,9 +1167,11 @@ static int do_fetch(struct transport *transport,\n \t\t\tgoto cleanup;\n \t}\n \n+\tget_effective_refspecs(&e_rs, &e_rs_nr, transport->remote,\n+\t\t\t refs, ref_count, tags, &autotags);\n \tremote_refs = transport_get_remote_refs(transport);\n \tref_map = get_ref_map(transport->remote, remote_refs, refs, ref_count,\n-\t\t\t tags, &autotags);\n+\t\t\t tags, autotags);\n \tif (!update_head_ok)\n \t\tcheck_not_current_branch(ref_map);\n \n@@ -1126,7 +1192,7 @@ static int do_fetch(struct transport *transport,\n \t\t\t\t transport->url);\n \t\t}\n \t}\n-\tif (fetch_refs(transport, ref_map, &new_remote_refs)) {\n+\tif (fetch_refs(transport, e_rs, e_rs_nr, ref_map, &new_remote_refs)) {\n \t\tfree_refs(ref_map);\n \t\tretcode = 1;\n \t\tgoto cleanup;\n@@ -1134,7 +1200,7 @@ static int do_fetch(struct transport *transport,\n \tif (new_remote_refs) {\n \t\tfree_refs(ref_map);\n \t\tref_map = get_ref_map(transport->remote, new_remote_refs,\n-\t\t\t\t refs, ref_count, tags, &autotags);\n+\t\t\t\t refs, ref_count, tags, autotags);\n \t\tfree_refs(new_remote_refs);\n \t}\n \ndiff --git a/remote-curl.c b/remote-curl.c\nindex 34a97e732..e78959d47 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -12,6 +12,7 @@\n #include \"credential.h\"\n #include \"sha1-array.h\"\n #include \"send-pack.h\"\n+#include \"refs.h\"\n \n static struct remote *remote;\n /* always ends with a trailing slash */\n@@ -31,7 +32,8 @@ struct options {\n \t\tthin : 1,\n \t\t/* One of the SEND_PACK_PUSH_CERT_* constants. */\n \t\tpush_cert : 2,\n-\t\tdeepen_relative : 1;\n+\t\tdeepen_relative : 1,\n+\t\techo_refs : 1;\n };\n static struct options options;\n static struct string_list cas_options = STRING_LIST_INIT_DUP;\n@@ -139,6 +141,14 @@ static int set_option(const char *name, const char *value)\n \t\telse\n \t\t\treturn -1;\n \t\treturn 0;\n+\t} else if (!strcmp(name, \"echo-refs\")) {\n+\t\tif (!strcmp(value, \"true\"))\n+\t\t\toptions.echo_refs = 1;\n+\t\telse if (!strcmp(value, \"false\"))\n+\t\t\toptions.echo_refs = 0;\n+\t\telse\n+\t\t\treturn -1;\n+\t\treturn 0;\n \n #if LIBCURL_VERSION_NUM >= 0x070a08\n \t} else if (!strcmp(name, \"family\")) {\n@@ -750,7 +760,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads)\n \treturn err;\n }\n \n-static int fetch_dumb(int nr_heads, struct ref **to_fetch)\n+static int fetch_dumb(int nr_heads, const struct ref **to_fetch)\n {\n \tstruct walker *walker;\n \tchar **targets;\n@@ -775,11 +785,24 @@ static int fetch_dumb(int nr_heads, struct ref **to_fetch)\n \t\tfree(targets[i]);\n \tfree(targets);\n \n+\tif (options.echo_refs) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tfor (i = 0; i < nr_heads; i++) {\n+\t\t\tstrbuf_reset(&sb);\n+\t\t\tstrbuf_addf(&sb,\n+\t\t\t\t \"%s %s\\n\",\n+\t\t\t\t oid_to_hex(&to_fetch[i]->old_oid),\n+\t\t\t\t to_fetch[i]->name);\n+\t\t\twrite_or_die(1, sb.buf, sb.len);\n+\t\t}\n+\t}\n+\n \treturn ret ? error(\"fetch failed.\") : 0;\n }\n \n static int fetch_git(struct discovery *heads,\n-\tint nr_heads, struct ref **to_fetch)\n+\tint nr_refspec, const struct refspec *refspecs,\n+\tint nr_heads, const struct ref **to_fetch)\n {\n \tstruct rpc_state rpc;\n \tstruct strbuf preamble = STRBUF_INIT;\n@@ -811,10 +834,15 @@ static int fetch_git(struct discovery *heads,\n \t\t\t\t options.deepen_not.items[i].string);\n \tif (options.deepen_relative && options.depth)\n \t\targv_array_push(&args, \"--deepen-relative\");\n+\tif (options.echo_refs)\n+\t\targv_array_push(&args, \"--always-print-refs\");\n \targv_array_push(&args, url.buf);\n \n-\tfor (i = 0; i < nr_heads; i++) {\n-\t\tstruct ref *ref = to_fetch[i];\n+\tif (refspecs) {\n+\t\tfor (i = 0; i < nr_refspec; i++)\n+\t\t\tpacket_buf_write(&preamble, \"%s\\n\", refspecs[i].src);\n+\t} else {\n+\t\tconst struct ref *ref = to_fetch[i];\n \t\tif (!*ref->name)\n \t\t\tdie(\"cannot fetch by sha1 over smart http\");\n \t\tpacket_buf_write(&preamble, \"%s %s\\n\",\n@@ -837,46 +865,38 @@ static int fetch_git(struct discovery *heads,\n \treturn err;\n }\n \n-static int fetch(int nr_heads, struct ref **to_fetch)\n+static int fetch(int nr_refspec, const struct refspec *refspecs)\n {\n+\tconst struct ref **to_fetch;\n+\tint nr;\n+\tint ret, i;\n \tstruct discovery *d = discover_refs(\"git-upload-pack\", 0);\n+\tget_ref_array(&to_fetch, &nr, d->refs, refspecs, nr_refspec);\n+\n \tif (d->proto_git)\n-\t\treturn fetch_git(d, nr_heads, to_fetch);\n+\t\tret = fetch_git(d, nr_refspec, refspecs, nr, to_fetch);\n \telse\n-\t\treturn fetch_dumb(nr_heads, to_fetch);\n+\t\tret = fetch_dumb(nr, to_fetch);\n+\n+\tfor (i = 0; i < nr; i++) {\n+\t\tfree((void *) to_fetch[i]);\n+\t}\n+\tfree(to_fetch);\n+\n+\treturn ret;\n }\n \n static void parse_fetch(struct strbuf *buf)\n {\n-\tstruct ref **to_fetch = NULL;\n-\tstruct ref *list_head = NULL;\n-\tstruct ref **list = &list_head;\n-\tint alloc_heads = 0, nr_heads = 0;\n+\tstruct refspec *to_fetch = NULL;\n+\tint alloc = 0, nr = 0;\n \n \tdo {\n \t\tconst char *p;\n \t\tif (skip_prefix(buf->buf, \"fetch \", &p)) {\n-\t\t\tconst char *name;\n-\t\t\tstruct ref *ref;\n-\t\t\tstruct object_id old_oid;\n-\n-\t\t\tif (get_oid_hex(p, &old_oid))\n-\t\t\t\tdie(\"protocol error: expected sha/ref, got %s'\", p);\n-\t\t\tif (p[GIT_SHA1_HEXSZ] == ' ')\n-\t\t\t\tname = p + GIT_SHA1_HEXSZ + 1;\n-\t\t\telse if (!p[GIT_SHA1_HEXSZ])\n-\t\t\t\tname = \"\";\n-\t\t\telse\n-\t\t\t\tdie(\"protocol error: expected sha/ref, got %s'\", p);\n-\n-\t\t\tref = alloc_ref(name);\n-\t\t\toidcpy(&ref->old_oid, &old_oid);\n-\n-\t\t\t*list = ref;\n-\t\t\tlist = &ref->next;\n-\n-\t\t\tALLOC_GROW(to_fetch, nr_heads + 1, alloc_heads);\n-\t\t\tto_fetch[nr_heads++] = ref;\n+\t\t\tnr++;\n+\t\t\tALLOC_GROW(to_fetch, nr, alloc);\n+\t\t\tparse_ref_or_pattern(&to_fetch[nr - 1], p);\n \t\t}\n \t\telse\n \t\t\tdie(\"http transport does not support %s\", buf->buf);\n@@ -888,10 +908,8 @@ static void parse_fetch(struct strbuf *buf)\n \t\t\tbreak;\n \t} while (1);\n \n-\tif (fetch(nr_heads, to_fetch))\n+\tif (fetch(nr, to_fetch))\n \t\texit(128); /* error already reported */\n-\tfree_refs(list_head);\n-\tfree(to_fetch);\n \n \tprintf(\"\\n\");\n \tfflush(stdout);\n@@ -1084,6 +1102,7 @@ int cmd_main(int argc, const char **argv)\n \t\t\tprintf(\"option\\n\");\n \t\t\tprintf(\"push\\n\");\n \t\t\tprintf(\"check-connectivity\\n\");\n+\t\t\tprintf(\"echo-refs\\n\");\n \t\t\tprintf(\"\\n\");\n \t\t\tfflush(stdout);\n \t\t} else {\ndiff --git a/t/t5552-upload-pack-ref-in-want.sh b/t/t5552-upload-pack-ref-in-want.sh\nindex 80cf2263a..26e785f3b 100755\n--- a/t/t5552-upload-pack-ref-in-want.sh\n+++ b/t/t5552-upload-pack-ref-in-want.sh\n@@ -345,7 +345,7 @@ test_expect_success 'server is initially ahead - no ref in want' '\n \tgrep \"ERR upload-pack: not our ref\" err\n '\n \n-test_expect_failure 'server is initially ahead - ref in want' '\n+test_expect_success 'server is initially ahead - ref in want' '\n \tgit -C \"$REPO\" config uploadpack.advertiseRefInWant true &&\n \trm -rf local &&\n \tcp -r \"$LOCAL_PRISTINE\" local &&\n@@ -369,7 +369,7 @@ test_expect_success 'server is initially behind - no ref in want' '\n \ttest_cmp expected actual\n '\n \n-test_expect_failure 'server is initially behind - ref in want' '\n+test_expect_success 'server is initially behind - ref in want' '\n \tgit -C \"$REPO\" config uploadpack.advertiseRefInWant true &&\n \trm -rf local &&\n \tcp -r \"$LOCAL_PRISTINE\" local &&\ndiff --git a/transport-helper.c b/transport-helper.c\nindex be0aa6d39..fcd9edcdc 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -28,7 +28,8 @@ struct helper_data {\n \t\tsigned_tags : 1,\n \t\tcheck_connectivity : 1,\n \t\tno_disconnect_req : 1,\n-\t\tno_private_update : 1;\n+\t\tno_private_update : 1,\n+\t\techo_refs : 1;\n \tchar *export_marks;\n \tchar *import_marks;\n \t/* These go from remote name (as in \"list\") to private name */\n@@ -195,6 +196,8 @@ static struct child_process *get_helper(struct transport *transport)\n \t\t\tdata->import_marks = xstrdup(arg);\n \t\t} else if (starts_with(capname, \"no-private-update\")) {\n \t\t\tdata->no_private_update = 1;\n+\t\t} else if (!strcmp(capname, \"echo-refs\")) {\n+\t\t\tdata->echo_refs = 1;\n \t\t} else if (mandatory) {\n \t\t\tdie(\"Unknown mandatory capability %s. This remote \"\n \t\t\t \"helper probably needs newer version of Git.\",\n@@ -383,27 +386,49 @@ static int release_helper(struct transport *transport)\n \treturn res;\n }\n \n+static struct ref *copy_ref_array(struct ref **array, int nr)\n+{\n+\tstruct ref *head = NULL, **tail = &head;\n+\tint i;\n+\tfor (i = 0; i < nr; i++) {\n+\t\t*tail = copy_ref(array[i]);\n+\t\ttail = &(*tail)->next;\n+\t}\n+\treturn head;\n+}\n+\n static int fetch_with_fetch(struct transport *transport,\n-\t\t\t int nr_heads, const struct ref **to_fetch)\n+\t\t\t int nr_refspec, const struct refspec *refspecs,\n+\t\t\t int nr_heads, const struct ref **to_fetch,\n+\t\t\t struct ref **fetched_refs)\n {\n \tstruct helper_data *data = transport->data;\n \tint i;\n \tstruct strbuf buf = STRBUF_INIT;\n-\n-\tfor (i = 0; i < nr_heads; i++) {\n-\t\tconst struct ref *posn = to_fetch[i];\n-\t\tif (posn->status & REF_STATUS_UPTODATE)\n-\t\t\tcontinue;\n-\n-\t\tstrbuf_addf(&buf, \"fetch %s %s\\n\",\n-\t\t\t oid_to_hex(&posn->old_oid),\n-\t\t\t posn->symref ? posn->symref : posn->name);\n+\tint use_echo_refs = data->echo_refs && refspecs;\n+\tstruct ref *fetched = NULL;\n+\n+\tif (use_echo_refs) {\n+\t\tset_helper_option(transport, \"echo-refs\", \"true\");\n+\t\tfor (i = 0; i < nr_refspec; i++)\n+\t\t\tstrbuf_addf(&buf, \"fetch %s\\n\", refspecs[i].src);\n+\t} else {\n+\t\tfor (i = 0; i < nr_heads; i++) {\n+\t\t\tconst struct ref *posn = to_fetch[i];\n+\t\t\tif (posn->status & REF_STATUS_UPTODATE)\n+\t\t\t\tcontinue;\n+\n+\t\t\tstrbuf_addf(&buf, \"fetch %s %s\\n\",\n+\t\t\t\t oid_to_hex(&posn->old_oid),\n+\t\t\t\t posn->symref ? posn->symref : posn->name);\n+\t\t}\n \t}\n \n \tstrbuf_addch(&buf, '\\n');\n \tsendline(data, &buf);\n \n \twhile (1) {\n+\t\tstruct object_id oid;\n \t\tif (recvline(data, &buf))\n \t\t\texit(128);\n \n@@ -418,12 +443,29 @@ static int fetch_with_fetch(struct transport *transport,\n \t\t\t data->transport_options.check_self_contained_and_connected &&\n \t\t\t !strcmp(buf.buf, \"connectivity-ok\"))\n \t\t\tdata->transport_options.self_contained_and_connected = 1;\n-\t\telse if (!buf.len)\n+\t\telse if (use_echo_refs && !get_oid_hex(buf.buf, &oid)\n+\t\t\t && buf.buf[GIT_SHA1_HEXSZ] == ' ') {\n+\t\t\tstruct ref *ref = alloc_ref(buf.buf + GIT_SHA1_HEXSZ + 1);\n+\t\t\toidcpy(&ref->old_oid, &oid);\n+\t\t\tref->next = fetched;\n+\t\t\tfetched = ref;\n+\t\t} else if (!buf.len)\n \t\t\tbreak;\n \t\telse\n \t\t\twarning(\"%s unexpectedly said: '%s'\", data->name, buf.buf);\n \t}\n \tstrbuf_release(&buf);\n+\n+\tif (use_echo_refs) {\n+\t\tif (fetched_refs)\n+\t\t\t*fetched_refs = fetched;\n+\t} else {\n+\t\tassert(fetched == NULL);\n+\t\tif (fetched_refs)\n+\t\t\t*fetched_refs = copy_ref_array((struct ref **) to_fetch,\n+\t\t\t\t\t\t nr_heads);\n+\t}\n+\n \treturn 0;\n }\n \n@@ -657,6 +699,7 @@ static int connect_helper(struct transport *transport, const char *name,\n }\n \n static int fetch(struct transport *transport,\n+\t\t int nr_refspec, const struct refspec *refspecs,\n \t\t int nr_heads, const struct ref **to_fetch,\n \t\t struct ref **fetched_refs)\n {\n@@ -665,8 +708,8 @@ static int fetch(struct transport *transport,\n \n \tif (process_connect(transport, 0)) {\n \t\tdo_take_over(transport);\n-\t\treturn transport->fetch(transport, nr_heads, to_fetch,\n-\t\t\t\t\tfetched_refs);\n+\t\treturn transport->fetch(transport, nr_refspec, refspecs,\n+\t\t\t\t\tnr_heads, to_fetch, fetched_refs);\n \t}\n \n \tcount = 0;\n@@ -688,7 +731,8 @@ static int fetch(struct transport *transport,\n \t\tset_helper_option(transport, \"update-shallow\", \"true\");\n \n \tif (data->fetch)\n-\t\treturn fetch_with_fetch(transport, nr_heads, to_fetch);\n+\t\treturn fetch_with_fetch(transport, nr_refspec, refspecs,\n+\t\t\t\t\tnr_heads, to_fetch, fetched_refs);\n \n \tif (data->import)\n \t\treturn fetch_with_import(transport, nr_heads, to_fetch, fetched_refs);\ndiff --git a/transport.c b/transport.c\nindex 85a4c5369..734c605b1 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -95,6 +95,7 @@ static struct ref *get_refs_from_bundle(struct transport *transport, int for_pus\n }\n \n static int fetch_refs_from_bundle(struct transport *transport,\n+\t\t\t int nr_refspec, const struct refspec *refspecs,\n \t\t\t int nr_heads, const struct ref **to_fetch,\n \t\t\t struct ref **fetched_refs)\n {\n@@ -203,6 +204,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus\n }\n \n static int fetch_refs_via_pack(struct transport *transport,\n+\t\t\t int nr_refspec, const struct refspec *refspecs,\n \t\t\t int nr_heads, const struct ref **to_fetch,\n \t\t\t struct ref **fetched_refs)\n {\n@@ -1096,8 +1098,9 @@ const struct ref *transport_get_remote_refs(struct transport *transport)\n \treturn transport->remote_refs;\n }\n \n-int transport_fetch_refs(struct transport *transport, struct ref *refs,\n-\t\t\t struct ref **fetched_refs)\n+int transport_fetch_refs(struct transport *transport,\n+\t\t\t const struct refspec *refspecs, int refspec_nr,\n+\t\t\t struct ref *refs, struct ref **fetched_refs)\n {\n \tint rc;\n \tint nr_heads = 0, nr_alloc = 0, nr_refs = 0;\n@@ -1136,7 +1139,8 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs,\n \t\t\theads[nr_heads++] = rm;\n \t}\n \n-\trc = transport->fetch(transport, nr_heads, heads, fetched_refs);\n+\trc = transport->fetch(transport, refspec_nr, refspecs,\n+\t\t\t nr_heads, heads, fetched_refs);\n \tif (nop_head) {\n \t\t*nop_tail = *fetched_refs;\n \t\t*fetched_refs = nop_head;\ndiff --git a/transport.h b/transport.h\nindex 326ff9bd6..d7a007d21 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -82,13 +82,20 @@ struct transport {\n \t * Fetch the objects for the given refs. Note that this gets\n \t * an array, and should ignore the list structure.\n \t *\n+\t * The user may provide the array of refspecs used to generate the\n+\t * given refs. If provided, the transport should prefer the refspecs if\n+\t * possible (but may still use the refs for pre-fetch optimizations,\n+\t * for example).\n+\t *\n \t * The transport *may* provide, in fetched_refs, the list of refs that\n-\t * it fetched. If the transport knows anything about the fetched refs\n-\t * that the caller does not know (for example, shallow status or ref\n-\t * hashes), it should provide that list of refs and include that\n-\t * information in the list.\n+\t * it fetched, and must do so if it is different from the given refs.\n+\t * If the transport knows anything about the fetched refs that the\n+\t * caller does not know (for example, shallow status or ref hashes), it\n+\t * should provide that list of refs and include that information in the\n+\t * list.\n \t **/\n \tint (*fetch)(struct transport *transport,\n+\t\t int refspec_nr, const struct refspec *refspecs,\n \t\t int refs_nr, const struct ref **refs,\n \t\t struct ref **fetched_refs);\n \n@@ -234,8 +241,9 @@ int transport_push(struct transport *connection,\n \n const struct ref *transport_get_remote_refs(struct transport *transport);\n \n-int transport_fetch_refs(struct transport *transport, struct ref *refs,\n-\t\t\t struct ref **fetched_refs);\n+int transport_fetch_refs(struct transport *transport,\n+\t\t\t const struct refspec *refspecs, int refspec_nr,\n+\t\t\t struct ref *refs, struct ref **fetched_refs);\n void transport_unlock_pack(struct transport *transport);\n int transport_disconnect(struct transport *transport);\n char *transport_anonymize_url(const char *url);\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"310227","messageId":"057e40c56d7bbaf581c9552b45315087cb0f5ed3.1485381677.git.jonathantanmy@google.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"[RFC 04/14] fetch: refactor the population of hashes","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-25T22:02:57Z","receivedAt":"2017-01-25T22:03:43Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Populate SHA-1 ref hashes in get_ref_map instead of do_fetch. Besides\ntightening scopes of variables in the code, this also prepares for a\nfuture patch where get_ref_map is called multiple times within do_fetch.\n\nSigned-off-by: Jonathan Tan \n---\n builtin/fetch.c | 37 +++++++++++++++++++------------------\n 1 file changed, 19 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex f1570e346..c71d5eb9b 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -316,6 +316,8 @@ static struct ref *get_ref_map(struct transport *transport,\n \n \tconst struct ref *remote_refs = transport_get_remote_refs(transport);\n \n+\tstruct string_list existing_refs = STRING_LIST_INIT_DUP;\n+\n \tif (refspec_count) {\n \t\tstruct refspec *fetch_refspec;\n \t\tint fetch_refspec_nr;\n@@ -411,7 +413,23 @@ static struct ref *get_ref_map(struct transport *transport,\n \t\ttail = &rm->next;\n \t}\n \n-\treturn ref_remove_duplicates(ref_map);\n+\tref_map = ref_remove_duplicates(ref_map);\n+\n+\tfor_each_ref(add_existing, &existing_refs);\n+\tfor (rm = ref_map; rm; rm = rm->next) {\n+\t\tif (rm->peer_ref) {\n+\t\t\tstruct string_list_item *peer_item =\n+\t\t\t\tstring_list_lookup(&existing_refs,\n+\t\t\t\t\t\t rm->peer_ref->name);\n+\t\t\tif (peer_item) {\n+\t\t\t\tstruct object_id *old_oid = peer_item->util;\n+\t\t\t\toidcpy(&rm->peer_ref->old_oid, old_oid);\n+\t\t\t}\n+\t\t}\n+\t}\n+\tstring_list_clear(&existing_refs, 1);\n+\n+\treturn ref_map;\n }\n \n #define STORE_REF_ERROR_OTHER 1\n@@ -1055,14 +1073,10 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)\n static int do_fetch(struct transport *transport,\n \t\t struct refspec *refs, int ref_count)\n {\n-\tstruct string_list existing_refs = STRING_LIST_INIT_DUP;\n \tstruct ref *ref_map;\n-\tstruct ref *rm;\n \tint autotags = (transport->remote->fetch_tags == 1);\n \tint retcode = 0;\n \n-\tfor_each_ref(add_existing, &existing_refs);\n-\n \tif (tags == TAGS_DEFAULT) {\n \t\tif (transport->remote->fetch_tags == 2)\n \t\t\ttags = TAGS_SET;\n@@ -1084,18 +1098,6 @@ static int do_fetch(struct transport *transport,\n \tif (!update_head_ok)\n \t\tcheck_not_current_branch(ref_map);\n \n-\tfor (rm = ref_map; rm; rm = rm->next) {\n-\t\tif (rm->peer_ref) {\n-\t\t\tstruct string_list_item *peer_item =\n-\t\t\t\tstring_list_lookup(&existing_refs,\n-\t\t\t\t\t\t rm->peer_ref->name);\n-\t\t\tif (peer_item) {\n-\t\t\t\tstruct object_id *old_oid = peer_item->util;\n-\t\t\t\toidcpy(&rm->peer_ref->old_oid, old_oid);\n-\t\t\t}\n-\t\t}\n-\t}\n-\n \tif (tags == TAGS_DEFAULT && autotags)\n \t\ttransport_set_option(transport, TRANS_OPT_FOLLOWTAGS, \"1\");\n \tif (prune) {\n@@ -1132,7 +1134,6 @@ static int do_fetch(struct transport *transport,\n \t}\n \n cleanup:\n-\tstring_list_clear(&existing_refs, 1);\n \treturn retcode;\n }\n \n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"310228","messageId":"afe5d7d3f876893fdad318665805df1e056717c6.1485381677.git.jonathantanmy@google.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"[RFC 03/14] upload-pack: test negotiation with changing repo","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-25T22:02:56Z","receivedAt":"2017-01-25T22:03:44Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Make upload-pack report \"not our ref\" errors to the client. (If not, the\nclient would be left waiting for a response when the server is already\ndead.)\n\nAdd tests to check the behavior of upload-pack and fetch-pack when\nupload-pack is served from a changing repository (for example, when\ndifferent servers in a load-balancing agreement participate in the same\nstateless RPC negotiation). This forms a baseline of comparison to the\nref-in-want functionality (which will be introduced in subsequent\ncommits), and ensures that subsequent commits do not change existing\nbehavior.\n\nAs part of this effort, a mechanism to substitute strings in an HTTP\nresponse only on the first invocation is added.\n\nSigned-off-by: Jonathan Tan \n---\n t/lib-httpd.sh | 1 +\n t/lib-httpd/apache.conf | 8 ++++\n t/lib-httpd/one-time-sed.sh | 8 ++++\n t/t5552-upload-pack-ref-in-want.sh | 91 ++++++++++++++++++++++++++++++++++++++\n upload-pack.c | 6 ++-\n 5 files changed, 113 insertions(+), 1 deletion(-)\n create mode 100644 t/lib-httpd/one-time-sed.sh\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 435a37465..84f8efdd4 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -132,6 +132,7 @@ prepare_httpd() {\n \tcp \"$TEST_PATH\"/passwd \"$HTTPD_ROOT_PATH\"\n \tinstall_script broken-smart-http.sh\n \tinstall_script error.sh\n+\tinstall_script one-time-sed.sh\n \n \tln -s \"$LIB_HTTPD_MODULE_PATH\" \"$HTTPD_ROOT_PATH/modules\"\n \ndiff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf\nindex 69174c6e3..ef218ff15 100644\n--- a/t/lib-httpd/apache.conf\n+++ b/t/lib-httpd/apache.conf\n@@ -106,9 +106,14 @@ Alias /auth/dumb/ www/auth/dumb/\n \tSetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}\n \tSetEnv GIT_HTTP_EXPORT_ALL\n \n+\n+\tSetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}\n+\tSetEnv GIT_HTTP_EXPORT_ALL\n+\n ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1\n ScriptAlias /broken_smart/ broken-smart-http.sh/\n ScriptAlias /error/ error.sh/\n+ScriptAliasMatch /one_time_sed/(.*) one-time-sed.sh/$1\n \n \tOptions FollowSymlinks\n \n@@ -118,6 +123,9 @@ ScriptAlias /error/ error.sh/\n \n Options ExecCGI\n \n+\n+\tOptions ExecCGI\n+\n \n \tOptions ExecCGI\n \ndiff --git a/t/lib-httpd/one-time-sed.sh b/t/lib-httpd/one-time-sed.sh\nnew file mode 100644\nindex 000000000..060ec0300\n--- /dev/null\n+++ b/t/lib-httpd/one-time-sed.sh\n@@ -0,0 +1,8 @@\n+#!/bin/sh\n+\n+if [ -e one-time-sed ]; then\n+\t\"$GIT_EXEC_PATH/git-http-backend\" | sed \"$(cat one-time-sed)\"\n+\trm one-time-sed\n+else\n+\t\"$GIT_EXEC_PATH/git-http-backend\"\n+fi\ndiff --git a/t/t5552-upload-pack-ref-in-want.sh b/t/t5552-upload-pack-ref-in-want.sh\nindex 3496af641..80cf2263a 100755\n--- a/t/t5552-upload-pack-ref-in-want.sh\n+++ b/t/t5552-upload-pack-ref-in-want.sh\n@@ -292,4 +292,95 @@ test_expect_success 'hideRefs with namespaces' '\n \tcheck_output\n '\n \n+. \"$TEST_DIRECTORY\"/lib-httpd.sh\n+start_httpd\n+\n+REPO=\"$HTTPD_DOCUMENT_ROOT_PATH/repo\"\n+LOCAL_PRISTINE=\"$(pwd)/local_pristine\"\n+\n+test_expect_success 'setup repos for change-while-negotiating test' '\n+\t(\n+\t\tgit init \"$REPO\" &&\n+\t\tcd \"$REPO\" &&\n+\t\t>.git/git-daemon-export-ok &&\n+\t\ttest_commit m1 &&\n+\t\tgit tag -d m1 &&\n+\n+\t\t# Local repo with many commits (so that negotiation will take\n+\t\t# more than 1 request/response pair)\n+\t\tgit clone \"http://127.0.0.1:$LIB_HTTPD_PORT/smart/repo\" \"$LOCAL_PRISTINE\" &&\n+\t\tcd \"$LOCAL_PRISTINE\" &&\n+\t\tgit checkout -b side &&\n+\t\tfor i in $(seq 1 33); do test_commit s$i; done &&\n+\n+\t\t# Add novel commits to upstream\n+\t\tgit checkout master &&\n+\t\tcd \"$REPO\" &&\n+\t\ttest_commit m2 &&\n+\t\ttest_commit m3 &&\n+\t\tgit tag -d m2 m3\n+\t) &&\n+\tgit -C \"$LOCAL_PRISTINE\" remote set-url origin \"http://127.0.0.1:$LIB_HTTPD_PORT/one_time_sed/repo\"\n+'\n+\n+inconsistency() {\n+\t# Simulate that the server initially reports $2 as the ref\n+\t# corresponding to $1, and after that, $1 as the ref corresponding to\n+\t# $1. This corresponds to the real-life situation where the server's\n+\t# repository appears to change during negotiation, for example, when\n+\t# different servers in a load-balancing arrangement serve (stateless)\n+\t# RPCs during a single negotiation.\n+\tprintf \"s/%s/%s/\" \\\n+\t $(git -C \"$REPO\" rev-parse $1 | tr -d \"\\n\") \\\n+\t $(git -C \"$REPO\" rev-parse $2 | tr -d \"\\n\") \\\n+\t >\"$HTTPD_ROOT_PATH/one-time-sed\"\n+}\n+\n+test_expect_success 'server is initially ahead - no ref in want' '\n+\tgit -C \"$REPO\" config uploadpack.advertiseRefInWant false &&\n+\trm -rf local &&\n+\tcp -r \"$LOCAL_PRISTINE\" local &&\n+\tinconsistency master 1234567890123456789012345678901234567890 &&\n+\ttest_must_fail git -C local fetch 2> err &&\n+\tgrep \"ERR upload-pack: not our ref\" err\n+'\n+\n+test_expect_failure 'server is initially ahead - ref in want' '\n+\tgit -C \"$REPO\" config uploadpack.advertiseRefInWant true &&\n+\trm -rf local &&\n+\tcp -r \"$LOCAL_PRISTINE\" local &&\n+\tinconsistency master 1234567890123456789012345678901234567890 &&\n+\tgit -C local fetch &&\n+\n+\tgit -C \"$REPO\" rev-parse --verify master > expected &&\n+\tgit -C local rev-parse --verify refs/remotes/origin/master > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'server is initially behind - no ref in want' '\n+\tgit -C \"$REPO\" config uploadpack.advertiseRefInWant false &&\n+\trm -rf local &&\n+\tcp -r \"$LOCAL_PRISTINE\" local &&\n+\tinconsistency master \"master^\" &&\n+\tgit -C local fetch &&\n+\n+\tgit -C \"$REPO\" rev-parse --verify \"master^\" > expected &&\n+\tgit -C local rev-parse --verify refs/remotes/origin/master > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_failure 'server is initially behind - ref in want' '\n+\tgit -C \"$REPO\" config uploadpack.advertiseRefInWant true &&\n+\trm -rf local &&\n+\tcp -r \"$LOCAL_PRISTINE\" local &&\n+\tinconsistency master \"master^\" &&\n+\tgit -C local fetch &&\n+\n+\tgit -C \"$REPO\" rev-parse --verify \"master\" > expected &&\n+\tgit -C local rev-parse --verify refs/remotes/origin/master > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+stop_httpd\n+\n test_done\ndiff --git a/upload-pack.c b/upload-pack.c\nindex b88ed8e26..0678c53d6 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -862,9 +862,13 @@ static void receive_needs(struct string_list *wanted_ns_refs)\n \t\t} else if (skip_prefix(line, \"want \", &arg) &&\n \t\t\t !get_sha1_hex(arg, sha1_buf)) {\n \t\t\to = parse_object(sha1_buf);\n-\t\t\tif (!o)\n+\t\t\tif (!o) {\n+\t\t\t\tpacket_write_fmt(1,\n+\t\t\t\t\t\t \"ERR upload-pack: not our ref %s\",\n+\t\t\t\t\t\t sha1_to_hex(sha1_buf));\n \t\t\t\tdie(\"git upload-pack: not our ref %s\",\n \t\t\t\t sha1_to_hex(sha1_buf));\n+\t\t\t}\n \t\t\tif (!(o->flags & WANTED)) {\n \t\t\t\to->flags |= WANTED;\n \t\t\t\tif (!((allow_unadvertised_object_request & ALLOW_ANY_SHA1) == ALLOW_ANY_SHA1\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"310229","messageId":"7b01ee955db8b2d9c6c7e641f7f09436938b2af4.1485381677.git.jonathantanmy@google.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"[RFC 11/14] fetch-pack: support want-ref","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-25T22:03:04Z","receivedAt":"2017-01-25T22:03:48Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Teach fetch-pack to use the want-ref mechanism whenever the server\nadvertises it.\n\nSigned-off-by: Jonathan Tan \n---\n builtin/fetch-pack.c | 5 +-\n fetch-pack.c | 173 ++++++++++++++++++++++++++++++++++++--------------\n fetch-pack.h | 2 +\n t/t5500-fetch-pack.sh | 42 ++++++++++++\n transport.c | 2 +-\n 5 files changed, 175 insertions(+), 49 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 5f14242ae..ae073ab24 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -179,8 +179,9 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \tget_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, &shallow);\n \tget_ref_array(&sought_refs, &nr_sought_refs, ref, sought, nr_sought);\n \n-\tref = fetch_pack(&args, fd, conn, ref, dest, sought_refs, nr_sought_refs,\n-\t\t\t &shallow, pack_lockfile_ptr);\n+\tref = fetch_pack(&args, fd, conn, ref, dest, sought, nr_sought,\n+\t\t\t sought_refs, nr_sought_refs, &shallow,\n+\t\t\t pack_lockfile_ptr);\n \tif (pack_lockfile) {\n \t\tprintf(\"lock %s\\n\", pack_lockfile);\n \t\tfflush(stdout);\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 8cc85c19f..02149c930 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -219,11 +219,19 @@ static void consume_shallow_list(struct fetch_pack_args *args, int fd)\n \t}\n }\n \n-static enum ack_type get_ack(int fd, unsigned char *result_sha1)\n+/*\n+ * Reads an ACK or NAK from fd. If wanted_ref_tail is not NULL, also accepts\n+ * any \"wanted-ref\" lines before that ACK or NAK, writing them to\n+ * wanted_ref_tail.\n+ */\n+static enum ack_type get_ack(int fd, unsigned char *result_sha1,\n+\t\t\t struct ref ***wanted_ref_tail)\n {\n \tint len;\n-\tchar *line = packet_read_line(fd, &len);\n+\tchar *line;\n \tconst char *arg;\n+start:\n+\tline = packet_read_line(fd, &len);\n \n \tif (!len)\n \t\tdie(_(\"git fetch-pack: expected ACK/NAK, got EOF\"));\n@@ -244,7 +252,19 @@ static enum ack_type get_ack(int fd, unsigned char *result_sha1)\n \t\t\treturn ACK;\n \t\t}\n \t}\n-\tdie(_(\"git fetch-pack: expected ACK/NAK, got '%s'\"), line);\n+\tif (wanted_ref_tail) {\n+\t\tstruct object_id oid;\n+\t\tif (skip_prefix(line, \"wanted-ref \", &arg) &&\n+\t\t !get_sha1_hex(arg, oid.hash) && arg[40] == ' ' && arg[41]) {\n+\t\t\tstruct ref *ref = alloc_ref(arg + 41);\n+\t\t\toidcpy(&ref->old_oid, &oid);\n+\t\t\t**wanted_ref_tail = ref;\n+\t\t\t*wanted_ref_tail = &ref->next;\n+\t\t\tgoto start;\n+\t\t}\n+\t\tdie(_(\"git fetch_pack: expected ACK/NAK or wanted-ref, got '%s'\"), line);\n+\t}\n+\tdie(_(\"git fetch_pack: expected ACK/NAK, got '%s'\"), line);\n }\n \n static void send_request(struct fetch_pack_args *args,\n@@ -282,29 +302,55 @@ static int next_flush(struct fetch_pack_args *args, int count)\n \treturn count;\n }\n \n-static int find_common(struct fetch_pack_args *args,\n-\t\t int fd[2], unsigned char *result_sha1,\n-\t\t struct ref *refs)\n+static void write_capabilities(struct strbuf *sb,\n+\t\t\t const struct fetch_pack_args *args)\n {\n-\tint fetching;\n-\tint count = 0, flushes = 0, flush_at = INITIAL_FLUSH, retval;\n-\tconst unsigned char *sha1;\n-\tunsigned in_vain = 0;\n-\tint got_continue = 0;\n-\tint got_ready = 0;\n-\tstruct strbuf req_buf = STRBUF_INIT;\n-\tsize_t state_len = 0;\n+\tif (multi_ack == 2) strbuf_addstr(sb, \" multi_ack_detailed\");\n+\tif (multi_ack == 1) strbuf_addstr(sb, \" multi_ack\");\n+\tif (no_done) strbuf_addstr(sb, \" no-done\");\n+\tif (use_sideband == 2) strbuf_addstr(sb, \" side-band-64k\");\n+\tif (use_sideband == 1) strbuf_addstr(sb, \" side-band\");\n+\tif (args->deepen_relative) strbuf_addstr(sb, \" deepen-relative\");\n+\tif (args->use_thin_pack) strbuf_addstr(sb, \" thin-pack\");\n+\tif (args->no_progress) strbuf_addstr(sb, \" no-progress\");\n+\tif (args->include_tag) strbuf_addstr(sb, \" include-tag\");\n+\tif (prefer_ofs_delta) strbuf_addstr(sb, \" ofs-delta\");\n+\tif (deepen_since_ok) strbuf_addstr(sb, \" deepen-since\");\n+\tif (deepen_not_ok) strbuf_addstr(sb, \" deepen-not\");\n+\tif (agent_supported) strbuf_addf(sb, \" agent=%s\",\n+\t\t\t\t\t git_user_agent_sanitized());\n+}\n \n-\tif (args->stateless_rpc && multi_ack == 1)\n-\t\tdie(_(\"--stateless-rpc requires multi_ack_detailed\"));\n-\tif (marked)\n-\t\tfor_each_ref(clear_marks, NULL);\n-\tmarked = 1;\n+static void write_wants(struct strbuf *sb, const struct fetch_pack_args *args,\n+\t\t\tconst struct refspec *refspecs, int nr_refspec,\n+\t\t\tstruct ref *refs)\n+{\n+\tint capabilities_written = 0;\n \n-\tfor_each_ref(rev_list_insert_ref_oid, NULL);\n-\tfor_each_alternate_ref(insert_one_alternate_ref, NULL);\n+\tif (refspecs) {\n+\t\tint i;\n+\t\tfor (i = 0; i < nr_refspec; i++) {\n+\t\t\tconst char *to_send = (refspecs[i].src && refspecs[i].src[0])\n+\t\t\t\t? refspecs[i].src : \"HEAD\";\n+\t\t\tif (i == 0) {\n+\t\t\t\tstruct strbuf c = STRBUF_INIT;\n+\t\t\t\twrite_capabilities(&c, args);\n+\t\t\t\tpacket_buf_write(sb, \"want-ref %s%s\\n\",\n+\t\t\t\t\t\t to_send, c.buf);\n+\t\t\t\tstrbuf_release(&c);\n+\t\t\t} else\n+\t\t\t\tpacket_buf_write(sb, \"want-ref %s\\n\", to_send);\n+\n+\t\t\t/* write everything that refname_match supports */\n+\t\t\tpacket_buf_write(sb, \"want-ref refs/%s\\n\", to_send);\n+\t\t\tpacket_buf_write(sb, \"want-ref refs/tags/%s\\n\", to_send);\n+\t\t\tpacket_buf_write(sb, \"want-ref refs/heads/%s\\n\", to_send);\n+\t\t\tpacket_buf_write(sb, \"want-ref refs/remotes/%s\\n\", to_send);\n+\t\t\tpacket_buf_write(sb, \"want-ref refs/remotes/%s/HEAD\\n\", to_send);\n+\t\t}\n+\t\treturn;\n+\t}\n \n-\tfetching = 0;\n \tfor ( ; refs ; refs = refs->next) {\n \t\tunsigned char *remote = refs->old_oid.hash;\n \t\tconst char *remote_hex;\n@@ -326,30 +372,41 @@ static int find_common(struct fetch_pack_args *args,\n \t\t}\n \n \t\tremote_hex = sha1_to_hex(remote);\n-\t\tif (!fetching) {\n+\t\tif (!capabilities_written) {\n \t\t\tstruct strbuf c = STRBUF_INIT;\n-\t\t\tif (multi_ack == 2) strbuf_addstr(&c, \" multi_ack_detailed\");\n-\t\t\tif (multi_ack == 1) strbuf_addstr(&c, \" multi_ack\");\n-\t\t\tif (no_done) strbuf_addstr(&c, \" no-done\");\n-\t\t\tif (use_sideband == 2) strbuf_addstr(&c, \" side-band-64k\");\n-\t\t\tif (use_sideband == 1) strbuf_addstr(&c, \" side-band\");\n-\t\t\tif (args->deepen_relative) strbuf_addstr(&c, \" deepen-relative\");\n-\t\t\tif (args->use_thin_pack) strbuf_addstr(&c, \" thin-pack\");\n-\t\t\tif (args->no_progress) strbuf_addstr(&c, \" no-progress\");\n-\t\t\tif (args->include_tag) strbuf_addstr(&c, \" include-tag\");\n-\t\t\tif (prefer_ofs_delta) strbuf_addstr(&c, \" ofs-delta\");\n-\t\t\tif (deepen_since_ok) strbuf_addstr(&c, \" deepen-since\");\n-\t\t\tif (deepen_not_ok) strbuf_addstr(&c, \" deepen-not\");\n-\t\t\tif (agent_supported) strbuf_addf(&c, \" agent=%s\",\n-\t\t\t\t\t\t\t git_user_agent_sanitized());\n-\t\t\tpacket_buf_write(&req_buf, \"want %s%s\\n\", remote_hex, c.buf);\n+\t\t\twrite_capabilities(&c, args);\n+\t\t\tpacket_buf_write(sb, \"want %s%s\\n\", remote_hex, c.buf);\n \t\t\tstrbuf_release(&c);\n+\t\t\tcapabilities_written = 1;\n \t\t} else\n-\t\t\tpacket_buf_write(&req_buf, \"want %s\\n\", remote_hex);\n-\t\tfetching++;\n+\t\t\tpacket_buf_write(sb, \"want %s\\n\", remote_hex);\n \t}\n+}\n+\n+static int find_common(struct fetch_pack_args *args,\n+\t\t int fd[2], unsigned char *result_sha1,\n+\t\t struct strbuf *wants, struct ref **wanted_refs)\n+{\n+\tint count = 0, flushes = 0, flush_at = INITIAL_FLUSH, retval;\n+\tconst unsigned char *sha1;\n+\tunsigned in_vain = 0;\n+\tint got_continue = 0;\n+\tint got_ready = 0;\n+\tstruct strbuf req_buf = STRBUF_INIT;\n+\tsize_t state_len = 0;\n+\n+\tif (args->stateless_rpc && multi_ack == 1)\n+\t\tdie(_(\"--stateless-rpc requires multi_ack_detailed\"));\n+\tif (marked)\n+\t\tfor_each_ref(clear_marks, NULL);\n+\tmarked = 1;\n \n-\tif (!fetching) {\n+\tfor_each_ref(rev_list_insert_ref_oid, NULL);\n+\tfor_each_alternate_ref(insert_one_alternate_ref, NULL);\n+\n+\tstrbuf_swap(&req_buf, wants);\n+\n+\tif (!req_buf.len) {\n \t\tstrbuf_release(&req_buf);\n \t\tpacket_flush(fd[1]);\n \t\treturn 1;\n@@ -435,7 +492,7 @@ static int find_common(struct fetch_pack_args *args,\n \n \t\t\tconsume_shallow_list(args, fd[0]);\n \t\t\tdo {\n-\t\t\t\tack = get_ack(fd[0], result_sha1);\n+\t\t\t\tack = get_ack(fd[0], result_sha1, NULL);\n \t\t\t\tif (ack)\n \t\t\t\t\tprint_verbose(args, _(\"got %s %d %s\"), \"ack\",\n \t\t\t\t\t\t ack, sha1_to_hex(result_sha1));\n@@ -504,7 +561,9 @@ static int find_common(struct fetch_pack_args *args,\n \tif (!got_ready || !no_done)\n \t\tconsume_shallow_list(args, fd[0]);\n \twhile (flushes || multi_ack) {\n-\t\tint ack = get_ack(fd[0], result_sha1);\n+\t\tstruct ref *wr = NULL, **wr_tail = ≀\n+\t\tint ack = get_ack(fd[0], result_sha1, &wr_tail);\n+\t\t*wanted_refs = wr;\n \t\tif (ack) {\n \t\t\tprint_verbose(args, _(\"got %s (%d) %s\"), \"ack\",\n \t\t\t\t ack, sha1_to_hex(result_sha1));\n@@ -835,6 +894,7 @@ static int cmp_ref_by_name(const void *a_, const void *b_)\n static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \t\t\t\t int fd[2],\n \t\t\t\t const struct ref *orig_ref,\n+\t\t\t\t const struct refspec *refspecs, int nr_refspec,\n \t\t\t\t const struct ref **sought, int nr_sought,\n \t\t\t\t struct shallow_info *si,\n \t\t\t\t char **pack_lockfile)\n@@ -843,6 +903,10 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \tunsigned char sha1[20];\n \tconst char *agent_feature;\n \tint agent_len;\n+\tint ref_in_want = 0;\n+\tstruct strbuf wants = STRBUF_INIT;\n+\tstruct ref *wanted_refs = NULL;\n+\tint want_ref_used = 0;\n \n \tsort_ref_list(&ref, ref_compare_name);\n \tQSORT(sought, nr_sought, cmp_ref_by_name);\n@@ -907,17 +971,26 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \t\tdie(_(\"Server does not support --shallow-exclude\"));\n \tif (!server_supports(\"deepen-relative\") && args->deepen_relative)\n \t\tdie(_(\"Server does not support --deepen\"));\n+\tif (server_supports(\"ref-in-want\"))\n+\t\tref_in_want = 1;\n \n \tif (everything_local(args, &ref, sought, nr_sought)) {\n \t\tpacket_flush(fd[1]);\n \t\tgoto all_done;\n \t}\n-\tif (find_common(args, fd, sha1, ref) < 0)\n+\n+\tif (ref_in_want && refspecs) {\n+\t\twrite_wants(&wants, args, refspecs, nr_refspec, NULL);\n+\t\twant_ref_used = 1;\n+\t} else\n+\t\twrite_wants(&wants, args, NULL, 0, ref);\n+\tif (find_common(args, fd, sha1, &wants, &wanted_refs) < 0)\n \t\tif (!args->keep_pack)\n \t\t\t/* When cloning, it is not unusual to have\n \t\t\t * no common commit.\n \t\t\t */\n \t\t\twarning(_(\"no common commits\"));\n+\tstrbuf_release(&wants);\n \n \tif (args->stateless_rpc)\n \t\tpacket_flush(fd[1]);\n@@ -932,6 +1005,13 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \t\tdie(_(\"git fetch-pack: fetch failed.\"));\n \n all_done:\n+\tif (want_ref_used) {\n+\t\tfree_refs(ref);\n+\t\treturn wanted_refs;\n+\t}\n+\n+\tif (wanted_refs)\n+\t\tdie(\"Protocol error: we are not using ref-in-want but server still sends wanted-ref\");\n \treturn ref;\n }\n \n@@ -1082,6 +1162,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,\n \t\t int fd[], struct child_process *conn,\n \t\t const struct ref *ref,\n \t\t const char *dest,\n+\t\t const struct refspec *refspecs, int nr_refspec,\n \t\t const struct ref **sought, int nr_sought,\n \t\t struct sha1_array *shallow,\n \t\t char **pack_lockfile)\n@@ -1098,8 +1179,8 @@ struct ref *fetch_pack(struct fetch_pack_args *args,\n \t\tdie(_(\"no matching remote head\"));\n \t}\n \tprepare_shallow_info(&si, shallow);\n-\tref_cpy = do_fetch_pack(args, fd, ref, sought, nr_sought,\n-\t\t\t\t&si, pack_lockfile);\n+\tref_cpy = do_fetch_pack(args, fd, ref, refspecs, nr_refspec,\n+\t\t\t\tsought, nr_sought, &si, pack_lockfile);\n \treprepare_packed_git();\n \tupdate_shallow(args, ref_cpy, &si);\n \tclear_shallow_info(&si);\ndiff --git a/fetch-pack.h b/fetch-pack.h\nindex 6e4fdbb68..06eb0fb28 100644\n--- a/fetch-pack.h\n+++ b/fetch-pack.h\n@@ -5,6 +5,7 @@\n #include \"run-command.h\"\n \n struct sha1_array;\n+struct refspec;\n \n struct fetch_pack_args {\n \tconst char *uploadpack;\n@@ -38,6 +39,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,\n \t\t int fd[], struct child_process *conn,\n \t\t const struct ref *ref,\n \t\t const char *dest,\n+\t\t const struct refspec *refspecs, int nr_refspec,\n \t\t const struct ref **sought,\n \t\t int nr_sought,\n \t\t struct sha1_array *shallow,\ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex cb1b7d949..18fe23c97 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -563,6 +563,25 @@ test_expect_success 'fetch-pack can fetch refs using a partial name' '\n \tgrep \"$(printf \"%s refs/heads/one\" $(git -C server rev-parse --verify one))\" actual\n '\n \n+test_expect_success 'fetch-pack can fetch refs using a partial name using want-ref' '\n+\trm -rf server &&\n+\tgit init server &&\n+\t(\n+\t\tcd server &&\n+\t\tgit config uploadpack.advertiseRefInWant true\n+\t\ttest_commit 1 &&\n+\t\ttest_commit 2 &&\n+\t\tgit checkout -b one\n+\t) &&\n+\trm -f trace &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git fetch-pack server one >actual &&\n+\techo here &&\n+\tgrep \" want-ref \" trace &&\n+\t! grep \" want \" trace &&\n+\n+\tgrep \"$(printf \"%s refs/heads/one\" $(git -C server rev-parse --verify one))\" actual\n+'\n+\n test_expect_success 'fetch-pack can fetch refs using a glob' '\n \trm -rf server &&\n \tgit init server &&\n@@ -585,6 +604,29 @@ test_expect_success 'fetch-pack can fetch refs using a glob' '\n \tgrep \"$(printf \"%s refs/heads/onc\" $(git -C server rev-parse --verify onc))\" actual\n '\n \n+test_expect_success 'fetch-pack can fetch refs using a glob using want-ref' '\n+\trm -rf server &&\n+\tgit init server &&\n+\t(\n+\t\tcd server &&\n+\t\tgit config uploadpack.advertiseRefInWant true\n+\t\ttest_commit 1 &&\n+\t\ttest_commit 2 &&\n+\t\tgit checkout -b ona &&\n+\t\tgit checkout -b onb &&\n+\t\ttest_commit 3 &&\n+\t\tgit checkout -b onc\n+\t) &&\n+\trm -f trace &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git fetch-pack server \"refs/heads/on*\" >actual &&\n+\tgrep \" want-ref \" trace &&\n+\t! grep \" want \" trace &&\n+\n+\tgrep \"$(printf \"%s refs/heads/ona\" $(git -C server rev-parse --verify ona))\" actual &&\n+\tgrep \"$(printf \"%s refs/heads/onb\" $(git -C server rev-parse --verify onb))\" actual &&\n+\tgrep \"$(printf \"%s refs/heads/onc\" $(git -C server rev-parse --verify onc))\" actual\n+'\n+\n check_prot_path () {\n \tcat >expected <<-EOF &&\n \tDiag: url=$1\ndiff --git a/transport.c b/transport.c\nindex 5ed3fc68e..85a4c5369 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -239,7 +239,7 @@ static int fetch_refs_via_pack(struct transport *transport,\n \n \trefs = fetch_pack(&args, data->fd, data->conn,\n \t\t\t refs_tmp ? refs_tmp : transport->remote_refs,\n-\t\t\t dest, to_fetch, nr_heads, &data->shallow,\n+\t\t\t dest, NULL, 0, to_fetch, nr_heads, &data->shallow,\n \t\t\t &transport->pack_lockfile);\n \tclose(data->fd[0]);\n \tclose(data->fd[1]);\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"310230","messageId":"86a128c5fb710a41791e7183207c4d64889f9307.1485381677.git.jonathantanmy@google.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"[RFC 07/14] fetch-pack: put shallow info in out param","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-25T22:03:00Z","receivedAt":"2017-01-25T22:03:51Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Expand the transport fetch method signature, by adding an out parameter,\nto allow transports to return information about the refs they have\nfetched. Then communicate shallow status information through this\nmechanism instead of by modifying the input list of refs.\n\nThis does require clients to sometimes generate the ref map twice: one\ngeneration from the list of refs provided by the remote (as is currently\ndone) and potentially one regeneration from the new list of refs that\nthe fetch mechanism provides (added in this patch). The double\ngeneration may result in duplicate error messages when a remote-tracking\nbranch seems to track more than one remote branch.\n\nThis is the 1st of 3 patches to eliminate using input refs to\ncommunicate information obtained by the fetch mechanism.\n\nSigned-off-by: Jonathan Tan \n---\n builtin/clone.c | 4 ++--\n builtin/fetch.c | 23 +++++++++++++++++++----\n fetch-pack.c | 18 +++++++++++-------\n remote.c | 12 +++++++++++-\n remote.h | 1 +\n t/t5536-fetch-conflicts.sh | 2 ++\n transport-helper.c | 5 +++--\n transport.c | 32 ++++++++++++++++++++++++++------\n transport.h | 12 ++++++++++--\n 9 files changed, 85 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 5ef81927a..0135c5f1c 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1076,7 +1076,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t}\n \n \t\tif (!is_local && !complete_refs_before_fetch)\n-\t\t\ttransport_fetch_refs(transport, mapped_refs);\n+\t\t\ttransport_fetch_refs(transport, mapped_refs, NULL);\n \n \t\tremote_head = find_ref_by_name(refs, \"HEAD\");\n \t\tremote_head_points_at =\n@@ -1115,7 +1115,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (is_local)\n \t\tclone_local(path, git_dir);\n \telse if (refs && complete_refs_before_fetch)\n-\t\ttransport_fetch_refs(transport, mapped_refs);\n+\t\ttransport_fetch_refs(transport, mapped_refs, NULL);\n \n \tupdate_remote_refs(refs, mapped_refs, remote_head_points_at,\n \t\t\t branch_top.buf, reflog_msg.buf, transport, !is_local);\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex ae7c6daa1..19e3f40a0 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -911,11 +911,13 @@ static int quickfetch(struct ref *ref_map)\n \treturn check_connected(iterate_ref_map, &rm, &opt);\n }\n \n-static int fetch_refs(struct transport *transport, struct ref *ref_map)\n+static int fetch_refs(struct transport *transport, struct ref *ref_map,\n+\t\t struct ref **updated_remote_refs)\n {\n \tint ret = quickfetch(ref_map);\n \tif (ret)\n-\t\tret = transport_fetch_refs(transport, ref_map);\n+\t\tret = transport_fetch_refs(transport, ref_map,\n+\t\t\t\t\t updated_remote_refs);\n \tif (ret)\n \t\ttransport_unlock_pack(transport);\n \treturn ret;\n@@ -1066,7 +1068,7 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)\n \ttransport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);\n \ttransport_set_option(transport, TRANS_OPT_DEPTH, \"0\");\n \ttransport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);\n-\tif (!fetch_refs(transport, ref_map))\n+\tif (!fetch_refs(transport, ref_map, NULL))\n \t\tconsume_refs(transport, ref_map);\n \n \tif (gsecondary) {\n@@ -1082,6 +1084,7 @@ static int do_fetch(struct transport *transport,\n \tint autotags = (transport->remote->fetch_tags == 1);\n \tint retcode = 0;\n \tconst struct ref *remote_refs;\n+\tstruct ref *new_remote_refs = NULL;\n \n \tif (tags == TAGS_DEFAULT) {\n \t\tif (transport->remote->fetch_tags == 2)\n@@ -1123,7 +1126,19 @@ static int do_fetch(struct transport *transport,\n \t\t\t\t transport->url);\n \t\t}\n \t}\n-\tif (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) {\n+\tif (fetch_refs(transport, ref_map, &new_remote_refs)) {\n+\t\tfree_refs(ref_map);\n+\t\tretcode = 1;\n+\t\tgoto cleanup;\n+\t}\n+\tif (new_remote_refs) {\n+\t\tfree_refs(ref_map);\n+\t\tref_map = get_ref_map(transport->remote, new_remote_refs,\n+\t\t\t\t refs, ref_count, tags, &autotags);\n+\t\tfree_refs(new_remote_refs);\n+\t}\n+\n+\tif (consume_refs(transport, ref_map)) {\n \t\tfree_refs(ref_map);\n \t\tretcode = 1;\n \t\tgoto cleanup;\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 601f0779a..9a87ddd3d 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -974,12 +974,13 @@ static int remove_duplicates_in_refs(struct ref **ref, int nr)\n }\n \n static void update_shallow(struct fetch_pack_args *args,\n-\t\t\t struct ref **sought, int nr_sought,\n+\t\t\t struct ref *refs,\n \t\t\t struct shallow_info *si)\n {\n \tstruct sha1_array ref = SHA1_ARRAY_INIT;\n \tint *status;\n \tint i;\n+\tstruct ref *r;\n \n \tif (args->deepen && alternate_shallow_file) {\n \t\tif (*alternate_shallow_file == '\\0') { /* --unshallow */\n@@ -1021,8 +1022,8 @@ static void update_shallow(struct fetch_pack_args *args,\n \tremove_nonexistent_theirs_shallow(si);\n \tif (!si->nr_ours && !si->nr_theirs)\n \t\treturn;\n-\tfor (i = 0; i < nr_sought; i++)\n-\t\tsha1_array_append(&ref, sought[i]->old_oid.hash);\n+\tfor (r = refs; r; r = r->next)\n+\t\tsha1_array_append(&ref, r->old_oid.hash);\n \tsi->ref = &ref;\n \n \tif (args->update_shallow) {\n@@ -1056,12 +1057,15 @@ static void update_shallow(struct fetch_pack_args *args,\n \t * remote is also shallow, check what ref is safe to update\n \t * without updating .git/shallow\n \t */\n-\tstatus = xcalloc(nr_sought, sizeof(*status));\n+\tstatus = xcalloc(ref.nr, sizeof(*status));\n \tassign_shallow_commits_to_refs(si, NULL, status);\n \tif (si->nr_ours || si->nr_theirs) {\n-\t\tfor (i = 0; i < nr_sought; i++)\n+\t\ti = 0;\n+\t\tfor (r = refs; r; r = r->next) {\n \t\t\tif (status[i])\n-\t\t\t\tsought[i]->status = REF_STATUS_REJECT_SHALLOW;\n+\t\t\t\tr->status = REF_STATUS_REJECT_SHALLOW;\n+\t\t\ti++;\n+\t\t}\n \t}\n \tfree(status);\n \tsha1_array_clear(&ref);\n@@ -1090,7 +1094,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,\n \tref_cpy = do_fetch_pack(args, fd, ref, sought, nr_sought,\n \t\t\t\t&si, pack_lockfile);\n \treprepare_packed_git();\n-\tupdate_shallow(args, sought, nr_sought, &si);\n+\tupdate_shallow(args, ref_cpy, &si);\n \tclear_shallow_info(&si);\n \treturn ref_cpy;\n }\ndiff --git a/remote.c b/remote.c\nindex d5eaec737..725a2d39a 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -929,7 +929,7 @@ struct ref *alloc_ref(const char *name)\n \treturn alloc_ref_with_prefix(\"\", 0, name);\n }\n \n-struct ref *copy_ref(const struct ref *ref)\n+struct ref *copy_ref_peerless(const struct ref *ref)\n {\n \tstruct ref *cpy;\n \tsize_t len;\n@@ -941,6 +941,16 @@ struct ref *copy_ref(const struct ref *ref)\n \tcpy->next = NULL;\n \tcpy->symref = xstrdup_or_null(ref->symref);\n \tcpy->remote_status = xstrdup_or_null(ref->remote_status);\n+\tcpy->peer_ref = NULL;\n+\treturn cpy;\n+}\n+\n+struct ref *copy_ref(const struct ref *ref)\n+{\n+\tstruct ref *cpy;\n+\tif (!ref)\n+\t\treturn NULL;\n+\tcpy = copy_ref_peerless(ref);\n \tcpy->peer_ref = copy_ref(ref->peer_ref);\n \treturn cpy;\n }\ndiff --git a/remote.h b/remote.h\nindex 924881169..57d059431 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -131,6 +131,7 @@ struct ref {\n extern struct ref *find_ref_by_name(const struct ref *list, const char *name);\n \n struct ref *alloc_ref(const char *name);\n+struct ref *copy_ref_peerless(const struct ref *ref);\n struct ref *copy_ref(const struct ref *ref);\n struct ref *copy_ref_list(const struct ref *ref);\n void sort_ref_list(struct ref **, int (*cmp)(const void *, const void *));\ndiff --git a/t/t5536-fetch-conflicts.sh b/t/t5536-fetch-conflicts.sh\nindex 2e42cf331..0cb380795 100755\n--- a/t/t5536-fetch-conflicts.sh\n+++ b/t/t5536-fetch-conflicts.sh\n@@ -93,6 +93,8 @@ test_expect_success 'fetch conflict: criss-cross args' '\n \t\tverify_stderr <<-\\EOF\n \t\twarning: refs/remotes/origin/branch1 usually tracks refs/heads/branch1, not refs/heads/branch2\n \t\twarning: refs/remotes/origin/branch2 usually tracks refs/heads/branch2, not refs/heads/branch1\n+\t\twarning: refs/remotes/origin/branch1 usually tracks refs/heads/branch1, not refs/heads/branch2\n+\t\twarning: refs/remotes/origin/branch2 usually tracks refs/heads/branch2, not refs/heads/branch1\n \t\tEOF\n \t)\n '\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 91aed35eb..f3d78bb9e 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -646,14 +646,15 @@ static int connect_helper(struct transport *transport, const char *name,\n }\n \n static int fetch(struct transport *transport,\n-\t\t int nr_heads, struct ref **to_fetch)\n+\t\t int nr_heads, struct ref **to_fetch, struct ref **fetched_refs)\n {\n \tstruct helper_data *data = transport->data;\n \tint i, count;\n \n \tif (process_connect(transport, 0)) {\n \t\tdo_take_over(transport);\n-\t\treturn transport->fetch(transport, nr_heads, to_fetch);\n+\t\treturn transport->fetch(transport, nr_heads, to_fetch,\n+\t\t\t\t\tfetched_refs);\n \t}\n \n \tcount = 0;\ndiff --git a/transport.c b/transport.c\nindex c86ba2eb8..80e007f2f 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -95,7 +95,8 @@ static struct ref *get_refs_from_bundle(struct transport *transport, int for_pus\n }\n \n static int fetch_refs_from_bundle(struct transport *transport,\n-\t\t\t int nr_heads, struct ref **to_fetch)\n+\t\t\t int nr_heads, struct ref **to_fetch,\n+\t\t\t struct ref **fetched_refs)\n {\n \tstruct bundle_transport_data *data = transport->data;\n \treturn unbundle(&data->header, data->fd,\n@@ -202,7 +203,8 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus\n }\n \n static int fetch_refs_via_pack(struct transport *transport,\n-\t\t\t int nr_heads, struct ref **to_fetch)\n+\t\t\t int nr_heads, struct ref **to_fetch,\n+\t\t\t struct ref **fetched_refs)\n {\n \tstruct git_transport_data *data = transport->data;\n \tstruct ref *refs;\n@@ -250,8 +252,12 @@ static int fetch_refs_via_pack(struct transport *transport,\n \tdata->options.self_contained_and_connected =\n \t\targs.self_contained_and_connected;\n \n+\tif (fetched_refs)\n+\t\t*fetched_refs = refs;\n+\telse\n+\t\tfree_refs(refs);\n+\n \tfree_refs(refs_tmp);\n-\tfree_refs(refs);\n \tfree(dest);\n \treturn (refs ? 0 : -1);\n }\n@@ -1090,19 +1096,29 @@ const struct ref *transport_get_remote_refs(struct transport *transport)\n \treturn transport->remote_refs;\n }\n \n-int transport_fetch_refs(struct transport *transport, struct ref *refs)\n+int transport_fetch_refs(struct transport *transport, struct ref *refs,\n+\t\t\t struct ref **fetched_refs)\n {\n \tint rc;\n \tint nr_heads = 0, nr_alloc = 0, nr_refs = 0;\n \tstruct ref **heads = NULL;\n+\tstruct ref *nop_head = NULL, **nop_tail = &nop_head;\n \tstruct ref *rm;\n \n \tfor (rm = refs; rm; rm = rm->next) {\n \t\tnr_refs++;\n \t\tif (rm->peer_ref &&\n \t\t !is_null_oid(&rm->old_oid) &&\n-\t\t !oidcmp(&rm->peer_ref->old_oid, &rm->old_oid))\n+\t\t !oidcmp(&rm->peer_ref->old_oid, &rm->old_oid)) {\n+\t\t\t/* These need to be reported as fetched, but we do not\n+\t\t\t * actually need to fetch them. */\n+\t\t\tif (fetched_refs) {\n+\t\t\t\tstruct ref *nop_ref = copy_ref_peerless(rm);\n+\t\t\t\t*nop_tail = nop_ref;\n+\t\t\t\tnop_tail = &nop_ref->next;\n+\t\t\t}\n \t\t\tcontinue;\n+\t\t}\n \t\tALLOC_GROW(heads, nr_heads + 1, nr_alloc);\n \t\theads[nr_heads++] = rm;\n \t}\n@@ -1120,7 +1136,11 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs)\n \t\t\theads[nr_heads++] = rm;\n \t}\n \n-\trc = transport->fetch(transport, nr_heads, heads);\n+\trc = transport->fetch(transport, nr_heads, heads, fetched_refs);\n+\tif (nop_head) {\n+\t\t*nop_tail = *fetched_refs;\n+\t\t*fetched_refs = nop_head;\n+\t}\n \n \tfree(heads);\n \treturn rc;\ndiff --git a/transport.h b/transport.h\nindex 9820f10b8..b9e7e4656 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -82,11 +82,18 @@ struct transport {\n \t * Fetch the objects for the given refs. Note that this gets\n \t * an array, and should ignore the list structure.\n \t *\n+\t * The transport *may* provide, in fetched_refs, the list of refs that\n+\t * it fetched. If the transport knows anything about the fetched refs\n+\t * that the caller does not know (for example, shallow status), it\n+\t * should provide that list of refs and include that information in the\n+\t * list.\n+\t *\n \t * If the transport did not get hashes for refs in\n \t * get_refs_list(), it should set the old_sha1 fields in the\n \t * provided refs now.\n \t **/\n-\tint (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);\n+\tint (*fetch)(struct transport *transport, int refs_nr, struct ref **refs,\n+\t\t struct ref **fetched_refs);\n \n \t/**\n \t * Push the objects and refs. Send the necessary objects, and\n@@ -230,7 +237,8 @@ int transport_push(struct transport *connection,\n \n const struct ref *transport_get_remote_refs(struct transport *transport);\n \n-int transport_fetch_refs(struct transport *transport, struct ref *refs);\n+int transport_fetch_refs(struct transport *transport, struct ref *refs,\n+\t\t\t struct ref **fetched_refs);\n void transport_unlock_pack(struct transport *transport);\n int transport_disconnect(struct transport *transport);\n char *transport_anonymize_url(const char *url);\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"310231","messageId":"a2262f625eb68cadc82ce042372111c05b4aca32.1485381677.git.jonathantanmy@google.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"[RFC 14/14] DONT USE advertise_ref_in_want=1","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-25T22:03:07Z","receivedAt":"2017-01-25T22:03:53Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"---\n t/t5500-fetch-pack.sh | 2 ++\n upload-pack.c | 2 +-\n 2 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex 18fe23c97..f39dbcab8 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -551,6 +551,7 @@ test_expect_success 'fetch-pack can fetch refs using a partial name' '\n \tgit init server &&\n \t(\n \t\tcd server &&\n+\t\tgit config uploadpack.advertiseRefInWant false\n \t\ttest_commit 1 &&\n \t\ttest_commit 2 &&\n \t\tgit checkout -b one\n@@ -587,6 +588,7 @@ test_expect_success 'fetch-pack can fetch refs using a glob' '\n \tgit init server &&\n \t(\n \t\tcd server &&\n+\t\tgit config uploadpack.advertiseRefInWant false\n \t\ttest_commit 1 &&\n \t\ttest_commit 2 &&\n \t\tgit checkout -b ona &&\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 0678c53d6..4998a8c7e 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -62,7 +62,7 @@ static int use_sideband;\n static int advertise_refs;\n static int stateless_rpc;\n static const char *pack_objects_hook;\n-static int advertise_ref_in_want;\n+static int advertise_ref_in_want = 1;\n \n static void reset_timeout(void)\n {\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"310232","messageId":"17158edf664fc85ae52da66e3de9210a2a645f14.1485381677.git.jonathantanmy@google.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"[RFC 05/14] fetch: refactor fetch_refs into two functions","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-25T22:02:58Z","receivedAt":"2017-01-25T22:03:54Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Refactor the fetch_refs function into a function that does the fetching\nof refs and another function that stores them. This prepares for a\nfuture patch where some processing may be done between those tasks.\n\nSigned-off-by: Jonathan Tan \n---\n builtin/fetch.c | 19 +++++++++++++------\n 1 file changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex c71d5eb9b..43e35c494 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -918,10 +918,16 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)\n \tint ret = quickfetch(ref_map);\n \tif (ret)\n \t\tret = transport_fetch_refs(transport, ref_map);\n-\tif (!ret)\n-\t\tret |= store_updated_refs(transport->url,\n-\t\t\t\ttransport->remote->name,\n-\t\t\t\tref_map);\n+\tif (ret)\n+\t\ttransport_unlock_pack(transport);\n+\treturn ret;\n+}\n+\n+static int consume_refs(struct transport *transport, struct ref *ref_map)\n+{\n+\tint ret = store_updated_refs(transport->url,\n+\t\t\t\t transport->remote->name,\n+\t\t\t\t ref_map);\n \ttransport_unlock_pack(transport);\n \treturn ret;\n }\n@@ -1062,7 +1068,8 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)\n \ttransport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);\n \ttransport_set_option(transport, TRANS_OPT_DEPTH, \"0\");\n \ttransport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);\n-\tfetch_refs(transport, ref_map);\n+\tif (!fetch_refs(transport, ref_map))\n+\t\tconsume_refs(transport, ref_map);\n \n \tif (gsecondary) {\n \t\ttransport_disconnect(gsecondary);\n@@ -1115,7 +1122,7 @@ static int do_fetch(struct transport *transport,\n \t\t\t\t transport->url);\n \t\t}\n \t}\n-\tif (fetch_refs(transport, ref_map)) {\n+\tif (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) {\n \t\tfree_refs(ref_map);\n \t\tretcode = 1;\n \t\tgoto cleanup;\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"310233","messageId":"4fc5ef88ae2135db475c557498e7a805afb693b9.1485381677.git.jonathantanmy@google.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"[RFC 06/14] fetch: refactor to make function args narrower","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-25T22:02:59Z","receivedAt":"2017-01-25T22:03:57Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Refactor find_non_local_tags and get_ref_map to only take the\ninformation they need instead of the entire transport struct. Besides\nimproving code clarity, this also improves their flexibility, which will\nbe needed in a future patch.\n\nSigned-off-by: Jonathan Tan \n---\n builtin/fetch.c | 23 ++++++++++++-----------\n 1 file changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 43e35c494..ae7c6daa1 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -220,7 +220,7 @@ static int will_fetch(struct ref **head, const unsigned char *sha1)\n \treturn 0;\n }\n \n-static void find_non_local_tags(struct transport *transport,\n+static void find_non_local_tags(const struct ref *refs,\n \t\t\tstruct ref **head,\n \t\t\tstruct ref ***tail)\n {\n@@ -230,7 +230,7 @@ static void find_non_local_tags(struct transport *transport,\n \tstruct string_list_item *item = NULL;\n \n \tfor_each_ref(add_existing, &existing_refs);\n-\tfor (ref = transport_get_remote_refs(transport); ref; ref = ref->next) {\n+\tfor (ref = refs; ref; ref = ref->next) {\n \t\tif (!starts_with(ref->name, \"refs/tags/\"))\n \t\t\tcontinue;\n \n@@ -302,7 +302,8 @@ static void find_non_local_tags(struct transport *transport,\n \tstring_list_clear(&remote_refs, 0);\n }\n \n-static struct ref *get_ref_map(struct transport *transport,\n+static struct ref *get_ref_map(const struct remote *remote,\n+\t\t\t const struct ref *remote_refs,\n \t\t\t struct refspec *refspecs, int refspec_count,\n \t\t\t int tags, int *autotags)\n {\n@@ -314,8 +315,6 @@ static struct ref *get_ref_map(struct transport *transport,\n \t/* opportunistically-updated references: */\n \tstruct ref *orefs = NULL, **oref_tail = &orefs;\n \n-\tconst struct ref *remote_refs = transport_get_remote_refs(transport);\n-\n \tstruct string_list existing_refs = STRING_LIST_INIT_DUP;\n \n \tif (refspec_count) {\n@@ -355,8 +354,8 @@ static struct ref *get_ref_map(struct transport *transport,\n \t\t\tfetch_refspec = parse_fetch_refspec(refmap_nr, refmap_array);\n \t\t\tfetch_refspec_nr = refmap_nr;\n \t\t} else {\n-\t\t\tfetch_refspec = transport->remote->fetch;\n-\t\t\tfetch_refspec_nr = transport->remote->fetch_refspec_nr;\n+\t\t\tfetch_refspec = remote->fetch;\n+\t\t\tfetch_refspec_nr = remote->fetch_refspec_nr;\n \t\t}\n \n \t\tfor (i = 0; i < fetch_refspec_nr; i++)\n@@ -365,7 +364,6 @@ static struct ref *get_ref_map(struct transport *transport,\n \t\tdie(\"--refmap option is only meaningful with command-line refspec(s).\");\n \t} else {\n \t\t/* Use the defaults */\n-\t\tstruct remote *remote = transport->remote;\n \t\tstruct branch *branch = branch_get(NULL);\n \t\tint has_merge = branch_has_merge_config(branch);\n \t\tif (remote &&\n@@ -404,7 +402,7 @@ static struct ref *get_ref_map(struct transport *transport,\n \t\t/* also fetch all tags */\n \t\tget_fetch_map(remote_refs, tag_refspec, &tail, 0);\n \telse if (tags == TAGS_DEFAULT && *autotags)\n-\t\tfind_non_local_tags(transport, &ref_map, &tail);\n+\t\tfind_non_local_tags(remote_refs, &ref_map, &tail);\n \n \t/* Now append any refs to be updated opportunistically: */\n \t*tail = orefs;\n@@ -1083,6 +1081,7 @@ static int do_fetch(struct transport *transport,\n \tstruct ref *ref_map;\n \tint autotags = (transport->remote->fetch_tags == 1);\n \tint retcode = 0;\n+\tconst struct ref *remote_refs;\n \n \tif (tags == TAGS_DEFAULT) {\n \t\tif (transport->remote->fetch_tags == 2)\n@@ -1101,7 +1100,9 @@ static int do_fetch(struct transport *transport,\n \t\t\tgoto cleanup;\n \t}\n \n-\tref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);\n+\tremote_refs = transport_get_remote_refs(transport);\n+\tref_map = get_ref_map(transport->remote, remote_refs, refs, ref_count,\n+\t\t\t tags, &autotags);\n \tif (!update_head_ok)\n \t\tcheck_not_current_branch(ref_map);\n \n@@ -1134,7 +1135,7 @@ static int do_fetch(struct transport *transport,\n \tif (tags == TAGS_DEFAULT && autotags) {\n \t\tstruct ref **tail = &ref_map;\n \t\tref_map = NULL;\n-\t\tfind_non_local_tags(transport, &ref_map, &tail);\n+\t\tfind_non_local_tags(remote_refs, &ref_map, &tail);\n \t\tif (ref_map)\n \t\t\tbackfill_tags(transport, ref_map);\n \t\tfree_refs(ref_map);\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"310234","messageId":"27ff4202986d2bb88705f37cab33f171df1427a2.1485381677.git.jonathantanmy@google.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"[RFC 09/14] transport: put ref oid in out param","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-25T22:03:02Z","receivedAt":"2017-01-25T22:04:01Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Return new OID information obtained through fetching in new structs\ninstead of reusing the existing ones. With this change, the input\nstructs are no longer used for output, and are thus marked const.\n\nThis is the 3rd of 3 patches to eliminate using input refs to\ncommunicate information obtained by the fetch mechanism.\n\nSigned-off-by: Jonathan Tan \n---\n builtin/clone.c | 14 ++++++++++++--\n builtin/fetch-pack.c | 4 ++--\n fetch-pack.c | 26 +++++++++++++++-----------\n fetch-pack.h | 2 +-\n transport-helper.c | 34 +++++++++++++++++++++++-----------\n transport.c | 6 +++---\n transport.h | 13 +++++--------\n 7 files changed, 61 insertions(+), 38 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 0135c5f1c..3191da391 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -858,6 +858,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstruct refspec *refspec;\n \tconst char *fetch_pattern;\n \n+\tstruct ref *new_remote_refs = NULL;\n+\n \tpacket_trace_identity(\"clone\");\n \targc = parse_options(argc, argv, prefix, builtin_clone_options,\n \t\t\t builtin_clone_usage, 0);\n@@ -1075,8 +1077,15 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\tbreak;\n \t\t\t}\n \n-\t\tif (!is_local && !complete_refs_before_fetch)\n-\t\t\ttransport_fetch_refs(transport, mapped_refs, NULL);\n+\t\tif (!is_local && !complete_refs_before_fetch) {\n+\t\t\ttransport_fetch_refs(transport, mapped_refs,\n+\t\t\t\t\t &new_remote_refs);\n+\t\t\tif (new_remote_refs) {\n+\t\t\t\trefs = new_remote_refs;\n+\t\t\t\tfree_refs(mapped_refs);\n+\t\t\t\tmapped_refs = wanted_peer_refs(refs, refspec);\n+\t\t\t}\n+\t\t}\n \n \t\tremote_head = find_ref_by_name(refs, \"HEAD\");\n \t\tremote_head_points_at =\n@@ -1148,5 +1157,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tjunk_mode = JUNK_LEAVE_ALL;\n \n \tfree(refspec);\n+\tfree_refs(new_remote_refs);\n \treturn err;\n }\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex f31f874a0..a18fd0c44 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -11,7 +11,7 @@ static const char fetch_pack_usage[] =\n \"[--include-tag] [--upload-pack=] [--depth=] \"\n \"[--no-progress] [--diag-url] [-v] [:] [...]\";\n \n-static void add_sought_entry(struct ref ***sought, int *nr, int *alloc,\n+static void add_sought_entry(const struct ref ***sought, int *nr, int *alloc,\n \t\t\t const char *name)\n {\n \tstruct ref *ref;\n@@ -44,7 +44,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \tint i, ret;\n \tstruct ref *ref = NULL;\n \tconst char *dest = NULL;\n-\tstruct ref **sought = NULL;\n+\tconst struct ref **sought = NULL;\n \tint nr_sought = 0, alloc_sought = 0;\n \tint fd[2];\n \tchar *pack_lockfile = NULL;\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex d4642b05c..8cc85c19f 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -52,6 +52,10 @@ static int non_common_revs, multi_ack, use_sideband;\n #define ALLOW_REACHABLE_SHA1\t02\n static unsigned int allow_unadvertised_object_request;\n \n+/* An arbitrary non-NULL non-const pointer to assign to the util field of\n+ * string list items when we need one. */\n+#define ARBITRARY (&transfer_unpack_limit)\n+\n __attribute__((format (printf, 2, 3)))\n static inline void print_verbose(const struct fetch_pack_args *args,\n \t\t\t\t const char *fmt, ...)\n@@ -556,7 +560,7 @@ static void mark_recent_complete_commits(struct fetch_pack_args *args,\n \n static void filter_refs(struct fetch_pack_args *args,\n \t\t\tstruct ref **refs,\n-\t\t\tstruct ref **sought, int nr_sought)\n+\t\t\tconst struct ref **sought, int nr_sought)\n {\n \tstruct ref *newlist = NULL;\n \tstruct ref **newtail = &newlist;\n@@ -604,16 +608,16 @@ static void filter_refs(struct fetch_pack_args *args,\n \t\tfor (i = 0; i < nr_sought; i++) {\n \t\t\tunsigned char sha1[20];\n \n-\t\t\tref = sought[i];\n+\t\t\tconst struct ref *sref = sought[i];\n \t\t\tif (matched[i])\n \t\t\t\tcontinue;\n-\t\t\tif (get_sha1_hex(ref->name, sha1) ||\n-\t\t\t ref->name[40] != '\\0' ||\n-\t\t\t hashcmp(sha1, ref->old_oid.hash))\n+\t\t\tif (get_sha1_hex(sref->name, sha1) ||\n+\t\t\t sref->name[40] != '\\0' ||\n+\t\t\t hashcmp(sha1, sref->old_oid.hash))\n \t\t\t\tcontinue;\n \n \t\t\tmatched[i] = 1;\n-\t\t\t*newtail = copy_ref(ref);\n+\t\t\t*newtail = copy_ref(sref);\n \t\t\tnewtail = &(*newtail)->next;\n \t\t}\n \t}\n@@ -629,7 +633,7 @@ static void mark_alternate_complete(const struct ref *ref, void *unused)\n \n static int everything_local(struct fetch_pack_args *args,\n \t\t\t struct ref **refs,\n-\t\t\t struct ref **sought, int nr_sought)\n+\t\t\t const struct ref **sought, int nr_sought)\n {\n \tstruct ref *ref;\n \tint retval;\n@@ -831,7 +835,7 @@ static int cmp_ref_by_name(const void *a_, const void *b_)\n static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \t\t\t\t int fd[2],\n \t\t\t\t const struct ref *orig_ref,\n-\t\t\t\t struct ref **sought, int nr_sought,\n+\t\t\t\t const struct ref **sought, int nr_sought,\n \t\t\t\t struct shallow_info *si,\n \t\t\t\t char **pack_lockfile)\n {\n@@ -955,7 +959,7 @@ static void fetch_pack_setup(void)\n \tdid_setup = 1;\n }\n \n-static int remove_duplicates_in_refs(struct ref **ref, int nr)\n+static int remove_duplicates_in_refs(const struct ref **ref, int nr)\n {\n \tstruct string_list names = STRING_LIST_INIT_NODUP;\n \tint src, dst;\n@@ -965,7 +969,7 @@ static int remove_duplicates_in_refs(struct ref **ref, int nr)\n \t\titem = string_list_insert(&names, ref[src]->name);\n \t\tif (item->util)\n \t\t\tcontinue; /* already have it */\n-\t\titem->util = ref[src];\n+\t\titem->util = ARBITRARY;\n \t\tif (src != dst)\n \t\t\tref[dst] = ref[src];\n \t\tdst++;\n@@ -1078,7 +1082,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,\n \t\t int fd[], struct child_process *conn,\n \t\t const struct ref *ref,\n \t\t const char *dest,\n-\t\t struct ref **sought, int nr_sought,\n+\t\t const struct ref **sought, int nr_sought,\n \t\t struct sha1_array *shallow,\n \t\t char **pack_lockfile)\n {\ndiff --git a/fetch-pack.h b/fetch-pack.h\nindex 76f7c719c..6e4fdbb68 100644\n--- a/fetch-pack.h\n+++ b/fetch-pack.h\n@@ -38,7 +38,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,\n \t\t int fd[], struct child_process *conn,\n \t\t const struct ref *ref,\n \t\t const char *dest,\n-\t\t struct ref **sought,\n+\t\t const struct ref **sought,\n \t\t int nr_sought,\n \t\t struct sha1_array *shallow,\n \t\t char **pack_lockfile);\ndiff --git a/transport-helper.c b/transport-helper.c\nindex f3d78bb9e..be0aa6d39 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -384,7 +384,7 @@ static int release_helper(struct transport *transport)\n }\n \n static int fetch_with_fetch(struct transport *transport,\n-\t\t\t int nr_heads, struct ref **to_fetch)\n+\t\t\t int nr_heads, const struct ref **to_fetch)\n {\n \tstruct helper_data *data = transport->data;\n \tint i;\n@@ -477,13 +477,14 @@ static int get_exporter(struct transport *transport,\n }\n \n static int fetch_with_import(struct transport *transport,\n-\t\t\t int nr_heads, struct ref **to_fetch)\n+\t\t\t int nr_heads, const struct ref **to_fetch, struct ref **fetched_refs)\n {\n \tstruct child_process fastimport;\n \tstruct helper_data *data = transport->data;\n \tint i;\n-\tstruct ref *posn;\n+\tconst struct ref *posn;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tstruct ref *fr_head = NULL, **fr_tail = &fr_head;\n \n \tget_helper(transport);\n \n@@ -522,28 +523,38 @@ static int fetch_with_import(struct transport *transport,\n \t * (If no \"refspec\" capability was specified, for historical\n \t * reasons we default to the equivalent of *:*.)\n \t *\n-\t * Store the result in to_fetch[i].old_sha1. Callers such\n+\t * Store the result in old_oid in fetched_refs. Callers such\n \t * as \"git fetch\" can use the value to write feedback to the\n \t * terminal, populate FETCH_HEAD, and determine what new value\n \t * should be written to peer_ref if the update is a\n \t * fast-forward or this is a forced update.\n \t */\n+\tif (!fetched_refs)\n+\t\tgoto cleanup;\n \tfor (i = 0; i < nr_heads; i++) {\n-\t\tchar *private, *name;\n-\t\tposn = to_fetch[i];\n-\t\tif (posn->status & REF_STATUS_UPTODATE)\n+\t\tstruct ref *ref;\n+\t\tchar *private;\n+\t\tconst char *name;\n+\n+\t\tref = copy_ref_peerless(to_fetch[i]);\n+\t\t*fr_tail = ref;\n+\t\tfr_tail = &ref->next;\n+\t\tif (ref->status & REF_STATUS_UPTODATE)\n \t\t\tcontinue;\n-\t\tname = posn->symref ? posn->symref : posn->name;\n+\t\tname = ref->symref ? ref->symref : ref->name;\n \t\tif (data->refspecs)\n \t\t\tprivate = apply_refspecs(data->refspecs, data->refspec_nr, name);\n \t\telse\n \t\t\tprivate = xstrdup(name);\n \t\tif (private) {\n-\t\t\tif (read_ref(private, posn->old_oid.hash) < 0)\n+\t\t\tif (read_ref(private, ref->old_oid.hash) < 0)\n \t\t\t\tdie(\"Could not read ref %s\", private);\n \t\t\tfree(private);\n \t\t}\n \t}\n+\t*fetched_refs = fr_head;\n+\n+cleanup:\n \tstrbuf_release(&buf);\n \treturn 0;\n }\n@@ -646,7 +657,8 @@ static int connect_helper(struct transport *transport, const char *name,\n }\n \n static int fetch(struct transport *transport,\n-\t\t int nr_heads, struct ref **to_fetch, struct ref **fetched_refs)\n+\t\t int nr_heads, const struct ref **to_fetch,\n+\t\t struct ref **fetched_refs)\n {\n \tstruct helper_data *data = transport->data;\n \tint i, count;\n@@ -679,7 +691,7 @@ static int fetch(struct transport *transport,\n \t\treturn fetch_with_fetch(transport, nr_heads, to_fetch);\n \n \tif (data->import)\n-\t\treturn fetch_with_import(transport, nr_heads, to_fetch);\n+\t\treturn fetch_with_import(transport, nr_heads, to_fetch, fetched_refs);\n \n \treturn -1;\n }\ndiff --git a/transport.c b/transport.c\nindex 80e007f2f..5ed3fc68e 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -95,7 +95,7 @@ static struct ref *get_refs_from_bundle(struct transport *transport, int for_pus\n }\n \n static int fetch_refs_from_bundle(struct transport *transport,\n-\t\t\t int nr_heads, struct ref **to_fetch,\n+\t\t\t int nr_heads, const struct ref **to_fetch,\n \t\t\t struct ref **fetched_refs)\n {\n \tstruct bundle_transport_data *data = transport->data;\n@@ -203,7 +203,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus\n }\n \n static int fetch_refs_via_pack(struct transport *transport,\n-\t\t\t int nr_heads, struct ref **to_fetch,\n+\t\t\t int nr_heads, const struct ref **to_fetch,\n \t\t\t struct ref **fetched_refs)\n {\n \tstruct git_transport_data *data = transport->data;\n@@ -1101,7 +1101,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs,\n {\n \tint rc;\n \tint nr_heads = 0, nr_alloc = 0, nr_refs = 0;\n-\tstruct ref **heads = NULL;\n+\tconst struct ref **heads = NULL;\n \tstruct ref *nop_head = NULL, **nop_tail = &nop_head;\n \tstruct ref *rm;\n \ndiff --git a/transport.h b/transport.h\nindex b9e7e4656..326ff9bd6 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -84,15 +84,12 @@ struct transport {\n \t *\n \t * The transport *may* provide, in fetched_refs, the list of refs that\n \t * it fetched. If the transport knows anything about the fetched refs\n-\t * that the caller does not know (for example, shallow status), it\n-\t * should provide that list of refs and include that information in the\n-\t * list.\n-\t *\n-\t * If the transport did not get hashes for refs in\n-\t * get_refs_list(), it should set the old_sha1 fields in the\n-\t * provided refs now.\n+\t * that the caller does not know (for example, shallow status or ref\n+\t * hashes), it should provide that list of refs and include that\n+\t * information in the list.\n \t **/\n-\tint (*fetch)(struct transport *transport, int refs_nr, struct ref **refs,\n+\tint (*fetch)(struct transport *transport,\n+\t\t int refs_nr, const struct ref **refs,\n \t\t struct ref **fetched_refs);\n \n \t/**\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"310235","messageId":"b722c0e310305a7eb4013209122db3a0c7b2052c.1485381677.git.jonathantanmy@google.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"[RFC 08/14] fetch-pack: check returned refs for matches","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-25T22:03:01Z","receivedAt":"2017-01-25T22:04:02Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Instead of setting \"matched\" on matched refs, detect matches by checking\nthe sought refs against the returned refs. Also, since the \"matched\"\nfield in struct ref is now no longer used, remove it.\n\nThis is the 2nd of 3 patches to eliminate using input refs to\ncommunicate information obtained by the fetch mechanism.\n\n(There are possibly ways more efficient than a nested for loop to\naccomplish this. However, since a subsequent patch will compare the\nuser-provided refspecs against the fetched refs directly, and thus\nnecessitating the nested for loop anyway, I decided to use the nested\nfor loop in this patch.)\n\nSigned-off-by: Jonathan Tan \n---\n builtin/fetch-pack.c | 7 ++++++-\n fetch-pack.c | 9 ++++++---\n fetch-pack.h | 2 --\n remote.h | 3 +--\n 4 files changed, 13 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex cfe9e447c..f31f874a0 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -4,6 +4,7 @@\n #include \"remote.h\"\n #include \"connect.h\"\n #include \"sha1-array.h\"\n+#include \"refs.h\"\n \n static const char fetch_pack_usage[] =\n \"git fetch-pack [--all] [--stdin] [--quiet | -q] [--keep | -k] [--thin] \"\n@@ -220,7 +221,11 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t * an error.\n \t */\n \tfor (i = 0; i < nr_sought; i++) {\n-\t\tif (!sought[i] || sought[i]->matched)\n+\t\tstruct ref *r;\n+\t\tfor (r = ref; r; r = r->next)\n+\t\t\tif (!sought[i] || refname_match(sought[i]->name, r->name))\n+\t\t\t\tbreak;\n+\t\tif (r)\n \t\t\tcontinue;\n \t\terror(\"no such remote ref %s\", sought[i]->name);\n \t\tret = 1;\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 9a87ddd3d..d4642b05c 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -562,6 +562,7 @@ static void filter_refs(struct fetch_pack_args *args,\n \tstruct ref **newtail = &newlist;\n \tstruct ref *ref, *next;\n \tint i;\n+\tint *matched = xcalloc(nr_sought, sizeof(*matched));\n \n \ti = 0;\n \tfor (ref = *refs; ref; ref = next) {\n@@ -578,7 +579,7 @@ static void filter_refs(struct fetch_pack_args *args,\n \t\t\t\t\tbreak; /* definitely do not have it */\n \t\t\t\telse if (cmp == 0) {\n \t\t\t\t\tkeep = 1; /* definitely have it */\n-\t\t\t\t\tsought[i]->matched = 1;\n+\t\t\t\t\tmatched[i] = 1;\n \t\t\t\t}\n \t\t\t\ti++;\n \t\t\t}\n@@ -604,19 +605,21 @@ static void filter_refs(struct fetch_pack_args *args,\n \t\t\tunsigned char sha1[20];\n \n \t\t\tref = sought[i];\n-\t\t\tif (ref->matched)\n+\t\t\tif (matched[i])\n \t\t\t\tcontinue;\n \t\t\tif (get_sha1_hex(ref->name, sha1) ||\n \t\t\t ref->name[40] != '\\0' ||\n \t\t\t hashcmp(sha1, ref->old_oid.hash))\n \t\t\t\tcontinue;\n \n-\t\t\tref->matched = 1;\n+\t\t\tmatched[i] = 1;\n \t\t\t*newtail = copy_ref(ref);\n \t\t\tnewtail = &(*newtail)->next;\n \t\t}\n \t}\n \t*refs = newlist;\n+\n+\tfree(matched);\n }\n \n static void mark_alternate_complete(const struct ref *ref, void *unused)\ndiff --git a/fetch-pack.h b/fetch-pack.h\nindex c912e3d32..76f7c719c 100644\n--- a/fetch-pack.h\n+++ b/fetch-pack.h\n@@ -33,8 +33,6 @@ struct fetch_pack_args {\n \n /*\n * sought represents remote references that should be updated from.\n- * On return, the names that were found on the remote will have been\n- * marked as such.\n */\n struct ref *fetch_pack(struct fetch_pack_args *args,\n \t\t int fd[], struct child_process *conn,\ndiff --git a/remote.h b/remote.h\nindex 57d059431..2f7f23d47 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -89,8 +89,7 @@ struct ref {\n \t\tforce:1,\n \t\tforced_update:1,\n \t\texpect_old_sha1:1,\n-\t\tdeletion:1,\n-\t\tmatched:1;\n+\t\tdeletion:1;\n \n \t/*\n \t * Order is important here, as we write to FETCH_HEAD\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"310236","messageId":"8e8ff3cbd7276233e712b4d0049e709c2372cfd7.1485381677.git.jonathantanmy@google.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"[RFC 10/14] fetch-pack: support partial names and globs","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-25T22:03:03Z","receivedAt":"2017-01-25T22:04:06Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Teach fetch-pack to support partial ref names and ref patterns as input.\n\nThis does not use \"want-ref\" yet - support for that will be added in a\nfuture patch.\n\nSigned-off-by: Jonathan Tan \n---\n builtin/fetch-pack.c | 40 ++++++++++++-------------------------\n remote.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++\n remote.h | 16 +++++++++++++++\n t/t5500-fetch-pack.sh | 38 +++++++++++++++++++++++++++++++++++\n 4 files changed, 122 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex a18fd0c44..5f14242ae 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -11,32 +11,12 @@ static const char fetch_pack_usage[] =\n \"[--include-tag] [--upload-pack=] [--depth=] \"\n \"[--no-progress] [--diag-url] [-v] [:] [...]\";\n \n-static void add_sought_entry(const struct ref ***sought, int *nr, int *alloc,\n+static void add_sought_entry(struct refspec **sought, int *nr, int *alloc,\n \t\t\t const char *name)\n {\n-\tstruct ref *ref;\n-\tstruct object_id oid;\n-\n-\tif (!get_oid_hex(name, &oid)) {\n-\t\tif (name[GIT_SHA1_HEXSZ] == ' ') {\n-\t\t\t/* , find refname */\n-\t\t\tname += GIT_SHA1_HEXSZ + 1;\n-\t\t} else if (name[GIT_SHA1_HEXSZ] == '\\0') {\n-\t\t\t; /* , leave sha1 as name */\n-\t\t} else {\n-\t\t\t/* , clear cruft from oid */\n-\t\t\toidclr(&oid);\n-\t\t}\n-\t} else {\n-\t\t/* , clear cruft from get_oid_hex */\n-\t\toidclr(&oid);\n-\t}\n-\n-\tref = alloc_ref(name);\n-\toidcpy(&ref->old_oid, &oid);\n \t(*nr)++;\n \tALLOC_GROW(*sought, *nr, *alloc);\n-\t(*sought)[*nr - 1] = ref;\n+\tparse_ref_or_pattern(&(*sought)[*nr - 1], name);\n }\n \n int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n@@ -44,8 +24,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \tint i, ret;\n \tstruct ref *ref = NULL;\n \tconst char *dest = NULL;\n-\tconst struct ref **sought = NULL;\n+\tstruct refspec *sought = NULL;\n \tint nr_sought = 0, alloc_sought = 0;\n+\tconst struct ref **sought_refs;\n+\tint nr_sought_refs;\n \tint fd[2];\n \tchar *pack_lockfile = NULL;\n \tchar **pack_lockfile_ptr = NULL;\n@@ -195,8 +177,9 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t\t\treturn args.diag_url ? 0 : 1;\n \t}\n \tget_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, &shallow);\n+\tget_ref_array(&sought_refs, &nr_sought_refs, ref, sought, nr_sought);\n \n-\tref = fetch_pack(&args, fd, conn, ref, dest, sought, nr_sought,\n+\tref = fetch_pack(&args, fd, conn, ref, dest, sought_refs, nr_sought_refs,\n \t\t\t &shallow, pack_lockfile_ptr);\n \tif (pack_lockfile) {\n \t\tprintf(\"lock %s\\n\", pack_lockfile);\n@@ -222,12 +205,15 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t */\n \tfor (i = 0; i < nr_sought; i++) {\n \t\tstruct ref *r;\n-\t\tfor (r = ref; r; r = r->next)\n-\t\t\tif (!sought[i] || refname_match(sought[i]->name, r->name))\n+\t\tif (sought[i].pattern)\n+\t\t\tcontinue; /* patterns do not need to match anything */\n+\t\tfor (r = ref; r; r = r->next) {\n+\t\t\tif (refname_match(sought[i].src, r->name))\n \t\t\t\tbreak;\n+\t\t}\n \t\tif (r)\n \t\t\tcontinue;\n-\t\terror(\"no such remote ref %s\", sought[i]->name);\n+\t\terror(\"no such remote ref %s\", sought[i].src);\n \t\tret = 1;\n \t}\n \ndiff --git a/remote.c b/remote.c\nindex 725a2d39a..08f3c910e 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -612,6 +612,39 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \tdie(\"Invalid refspec '%s'\", refspec[i]);\n }\n \n+void parse_ref_or_pattern(struct refspec *refspec, const char *str)\n+{\n+\tstruct object_id oid;\n+\tmemset(refspec, 0, sizeof(*refspec));\n+\n+\tif (!get_oid_hex(str, &oid)) {\n+\t\tif (str[GIT_SHA1_HEXSZ] == ' ') {\n+\t\t\tstruct object_id oid2;\n+\t\t\t/* , find refname */\n+\t\t\trefspec->src = xstrdup(str + GIT_SHA1_HEXSZ + 1);\n+\t\t\tif (!get_oid_hex(refspec->src, &oid2)\n+\t\t\t && !oidcmp(&oid, &oid2))\n+\t\t\t\t/* The name is actually a SHA-1 */\n+\t\t\t\trefspec->exact_sha1 = 1;\n+\t\t} else if (str[GIT_SHA1_HEXSZ] == '\\0') {\n+\t\t\t; /* , leave sha1 as name */\n+\t\t\trefspec->src = xstrdup(str);\n+\t\t\trefspec->exact_sha1 = 1;\n+\t\t} else {\n+\t\t\t/* */\n+\t\t\trefspec->src = xstrdup(str);\n+\t\t}\n+\t} else {\n+\t\t/* */\n+\t\trefspec->src = xstrdup(str);\n+\t}\n+\n+\tif (has_glob_specials(refspec->src)) {\n+\t\trefspec->pattern = 1;\n+\t\trefspec->dst = refspec->src;\n+\t}\n+}\n+\n int valid_fetch_refspec(const char *fetch_refspec_str)\n {\n \tstruct refspec *refspec;\n@@ -1924,6 +1957,28 @@ int get_fetch_map(const struct ref *remote_refs,\n \treturn 0;\n }\n \n+void get_ref_array(const struct ref ***refs, int *nr_ref,\n+\t\t const struct ref *remote_refs,\n+\t\t const struct refspec *refspecs, int nr_refspecs)\n+{\n+\tstruct ref *head = NULL, **tail = &head;\n+\tconst struct ref **array = NULL;\n+\tint nr = 0, alloc = 0;\n+\n+\tstruct ref *r;\n+\tint i;\n+\n+\tfor (i = 0; i < nr_refspecs; i++)\n+\t\tget_fetch_map(remote_refs, &refspecs[i], &tail, 1);\n+\tfor (r = head; r; r = r->next) {\n+\t\tnr++;\n+\t\tALLOC_GROW(array, nr, alloc);\n+\t\tarray[nr - 1] = r;\n+\t}\n+\t*refs = array;\n+\t*nr_ref = nr;\n+}\n+\n int resolve_remote_symref(struct ref *ref, struct ref *list)\n {\n \tif (!ref->symref)\ndiff --git a/remote.h b/remote.h\nindex 2f7f23d47..daca1c65e 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -162,6 +162,13 @@ int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid);\n */\n struct ref *ref_remove_duplicates(struct ref *ref_map);\n \n+/*\n+ * Parse the given ref or ref pattern. If a ref, write a refspec with that ref\n+ * as src, and with an empty dst. If a ref pattern, write a glob refspec with\n+ * that pattern as src and dst.\n+ */\n+void parse_ref_or_pattern(struct refspec *refspec, const char *str);\n+\n int valid_fetch_refspec(const char *refspec);\n struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec);\n \n@@ -192,6 +199,15 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,\n int get_fetch_map(const struct ref *remote_refs, const struct refspec *refspec,\n \t\t struct ref ***tail, int missing_ok);\n \n+/*\n+ * Convenience function to generate an array of refs corresponding to the given\n+ * refspecs. This is equivalent to repeatedly calling get_fetch_map and\n+ * rearranging the returned refs as an array.\n+ */\n+void get_ref_array(const struct ref ***refs, int *nr_ref,\n+\t\t const struct ref *remote_refs,\n+\t\t const struct refspec *refspecs, int nr_refspecs);\n+\n struct ref *get_remote_ref(const struct ref *remote_refs, const char *name);\n \n /*\ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex 505e1b4a7..cb1b7d949 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -547,6 +547,44 @@ test_expect_success 'fetch-pack can fetch a raw sha1' '\n \tgit fetch-pack hidden $(git -C hidden rev-parse refs/hidden/one)\n '\n \n+test_expect_success 'fetch-pack can fetch refs using a partial name' '\n+\tgit init server &&\n+\t(\n+\t\tcd server &&\n+\t\ttest_commit 1 &&\n+\t\ttest_commit 2 &&\n+\t\tgit checkout -b one\n+\t) &&\n+\trm -f trace &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git fetch-pack server one >actual &&\n+\tgrep \" want \" trace &&\n+\t! grep \" want-ref \" trace &&\n+\n+\tgrep \"$(printf \"%s refs/heads/one\" $(git -C server rev-parse --verify one))\" actual\n+'\n+\n+test_expect_success 'fetch-pack can fetch refs using a glob' '\n+\trm -rf server &&\n+\tgit init server &&\n+\t(\n+\t\tcd server &&\n+\t\ttest_commit 1 &&\n+\t\ttest_commit 2 &&\n+\t\tgit checkout -b ona &&\n+\t\tgit checkout -b onb &&\n+\t\ttest_commit 3 &&\n+\t\tgit checkout -b onc\n+\t) &&\n+\trm -f trace &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git fetch-pack server \"refs/heads/on*\" >actual &&\n+\tgrep \" want \" trace &&\n+\t! grep \" want-ref \" trace &&\n+\n+\tgrep \"$(printf \"%s refs/heads/ona\" $(git -C server rev-parse --verify ona))\" actual &&\n+\tgrep \"$(printf \"%s refs/heads/onb\" $(git -C server rev-parse --verify onb))\" actual &&\n+\tgrep \"$(printf \"%s refs/heads/onc\" $(git -C server rev-parse --verify onc))\" actual\n+'\n+\n check_prot_path () {\n \tcat >expected <<-EOF &&\n \tDiag: url=$1\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"310277","messageId":"CAGZ79kb+VVQoimCDCxk1JPtVdDcS0vgi3NgVfo_aZ_=feed8Cw@mail.gmail.com","threadId":"44973","inReplyTo":"db1db5d0e5563464c09d1678234c9c5e8ae5b2f4.1485381677.git.jonathantanmy@google.com","subject":"Re: [RFC 12/14] fetch-pack: do not printf after closing stdout","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-26T00:50:11Z","receivedAt":"2017-01-26T00:50:17Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Jan 25, 2017 at 2:03 PM, Jonathan Tan wrote:\n> In fetch-pack, during a stateless RPC, printf is invoked after stdout is\n> closed. Update the code to not do this, preserving the existing\n> behavior.\n\nThis seems to me as if it could go as an independent\nbugfix(?) or refactoring as this seems to be unclear from the code?\n\n>\n> Signed-off-by: Jonathan Tan \n> ---\n> builtin/fetch-pack.c | 14 ++++++++++----\n> 1 file changed, 10 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\n> index ae073ab24..24af3b7c5 100644\n> --- a/builtin/fetch-pack.c\n> +++ b/builtin/fetch-pack.c\n> @@ -191,10 +191,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n> printf(\"connectivity-ok\\n\");\n> fflush(stdout);\n> }\n> - close(fd[0]);\n> - close(fd[1]);\n> - if (finish_connect(conn))\n> - return 1;\n> + if (finish_connect(conn)) {\n> + ret = 1;\n> + goto cleanup;\n> + }\n>\n> ret = !ref;\n>\n> @@ -218,11 +218,17 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n> ret = 1;\n> }\n>\n> + if (args.stateless_rpc)\n> + goto cleanup;\n> +\n> while (ref) {\n> printf(\"%s %s\\n\",\n> oid_to_hex(&ref->old_oid), ref->name);\n> ref = ref->next;\n> }\n>\n> +cleanup:\n> + close(fd[0]);\n> + close(fd[1]);\n> return ret;\n> }\n> --\n> 2.11.0.483.g087da7b7c-goog\n>\n"},{"id":"310340","messageId":"bcdeacb6-4517-b3de-cafe-27540dd4f7d0@google.com","threadId":"44973","inReplyTo":"CAGZ79kb+VVQoimCDCxk1JPtVdDcS0vgi3NgVfo_aZ_=feed8Cw@mail.gmail.com","subject":"Re: [RFC 12/14] fetch-pack: do not printf after closing stdout","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-26T18:18:27Z","receivedAt":"2017-01-26T18:27:08Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On 01/25/2017 04:50 PM, Stefan Beller wrote:\n> On Wed, Jan 25, 2017 at 2:03 PM, Jonathan Tan wrote:\n>> In fetch-pack, during a stateless RPC, printf is invoked after stdout is\n>> closed. Update the code to not do this, preserving the existing\n>> behavior.\n>\n> This seems to me as if it could go as an independent\n> bugfix(?) or refactoring as this seems to be unclear from the code?\n\nThe subsequent patches in this patch set are dependent on this patch, \nbut it's true that this could be sent out on its own first.\n\nI'm not sure if bugfix is the right word, since the existing behavior is \ncorrect (except perhaps that we rely on the fact that printf after \nclosing stdout does effectively nothing).\n"},{"id":"310365","messageId":"xmqqd1f931g7.fsf@gitster.mtv.corp.google.com","threadId":"44973","inReplyTo":"d0d42b3bb4cf755f122591e191354c53848f197d.1485381677.git.jonathantanmy@google.com","subject":"Re: [RFC 02/14] upload-pack: allow ref name and glob requests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-26T22:23:36Z","receivedAt":"2017-01-26T22:24:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> Currently, while performing packfile negotiation [1], upload-pack allows\n> clients to specify their desired objects only as SHA-1s. This causes:\n> (a) vulnerability to failure when an object turns non-existent during\n> negotiation, which may happen if, for example, upload-pack is\n> provided by multiple Git servers in a load-balancing arrangement,\n> and\n> (b) dependence on the server first publishing a list of refs with\n> associated objects.\n>\n> To eliminate (a) and take a step towards eliminating (b), teach\n> upload-pack to support requests in the form of ref names and globs (in\n> addition to the existing support for SHA-1s) through a new line of the\n> form \"want-ref \" where ref is the full name of a ref, a glob\n> pattern, or a SHA-1. At the conclusion of negotiation, the server will\n> write \"wanted-ref \" for all requests that have been\n> specified this way.\n\nI am not sure if this \"at the conclusion of\" is sensible. It is OK\nto assume that what the client side has is fixed, and it is probably\nOK to desire that what the server side has can change, but at the\nsame time, it feels quite fragile to move the goalpost in between.\n\nStepping back a bit, in an environment that involves multiple server\ninstances that have inconsistent set of refs, can the negotiation\neven be sensibly and safely implemented? The first server the\nclient contacts may, in response to a \"have\", say \"I do have that\ncommit so you do not have to send its ancestors to me. We found one\ncut-off point. Please do explore other lines of histories.\" The\nnext server that concludes the negotiation exchange may not have\nthat commit and will be unable to produce a pack that excludes the\nobjects reachable from that commit---wouldn't that become a problem?\n\nOne way to prevent such a problem from hurting clients may be for\nthese multiple server instances to coordinate and make sure they\nhave a shared perception of the common history among them. Some\npushes may have come to one instance but may not have propagated to\nother instances, and such a commit cannot be accepted as usable\n\"have\" if the servers anticipate that the final client request would\ngo to any of the servers. Otherwise the multiple server arrangement\nwould not work safely, methinks.\n\nAnd if the servers are ensuring the safety using such a mechanism,\nthey can use the same mechanism to restrain \"faster\" instances from\nsending too fresh state of refs that other instances haven't caught\nup to, which would mean they can present a consistent set of refs to\nthe client in the first place, no?\n\nSo I am not sure if the mechanism to request history by refname\ninstead of the tip commit would help the multi-server environment as\nadvertised. It may help solving other problems, though (e.g. like\n\"somebody pushed to update after the initial advertisement was sent\nout\" which can happen even in a single server environment).\n\n> The server indicates that it supports this feature by advertising the\n> capability \"ref-in-want\". Advertisement of this capability is by default\n> disabled, but can be enabled through a configuration option.\n\nOK.\n\n> To be flexible with respect to client needs, the server does not\n> indicate an error if a \"want-ref\" line corresponds to no refs, but\n> instead relies on the client to ensure that what the user needs has been\n> fetched. For example, a client could reasonably expand an abbreviated\n> name \"foo\" to \"want-ref foo\", \"want-ref refs/heads/foo\", \"want-ref\n> refs/tags/foo\", among others, and ensure that at least one such ref has\n> been fetched.\n\nCute. This may be one way to implement the DWIM thing within the\nconstraint of eventually wanting to go to \"client speaks first, the\nserver does not advertise things the client is not interested in\"\nworld. \n\nBut at the same time it may end up bloating the set of refs the\nclient asks instead. Instead of receiving the advertisement and\nthen sending one request after picking the matching one from it,\nthe client needs to send \"refs/{heads,tags,whatever}/foo\".\n"},{"id":"310366","messageId":"CAGZ79kZgx1wQSCQv2xHYK341+nb=EOSTZacoznebNE=_M3JweQ@mail.gmail.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"Re: [RFC 00/14] Allow fetch-pack to send ref names (globs allowed)","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-26T22:15:25Z","receivedAt":"2017-01-26T22:24:39Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Jan 25, 2017 at 2:02 PM, Jonathan Tan wrote:\n> Hello everyone - this is a proposal for a protocol change to allow the\n> fetch-pack/upload-pack to converse in terms of ref names (globs\n> allowed), and also an implementation of the server (upload-pack) and\n> fetch-from-HTTP client (fetch-pack invoked through fetch).\n>\n> Negotiation currently happens by upload-pack initially sending a list of\n> refs with names and SHA-1 hashes, and then several request/response\n> pairs in which the request from fetch-pack consists of SHA-1 hashes\n> (selected from the initial list). Allowing the request to consist of\n> names instead of SHA-1 hashes increases tolerance to refs changing\n> (due to time, and due to having load-balanced servers without strong\n> consistency), and is a step towards eliminating the need for the server\n> to send the list of refs first (possibly improving performance).\n>\n> The protocol is extended by allowing fetch-pack to send\n> \"want-ref \", where is a full name (refs/...) [1], possibly\n> including glob characters. Upload-pack will inform the client of the\n> refs actually matched by sending \"wanted-ref \" before\n> sending the last ACK or NAK.\n\nI have reviewed the patches and think they are a good idea,\ncc'ing Jeff who you linked to and who had some ideas about the protocol as well.\n\nStefan\n"},{"id":"310370","messageId":"20170126230046.aknesybfyzxhx3ia@sigill.intra.peff.net","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"Re: [RFC 00/14] Allow fetch-pack to send ref names (globs allowed)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-01-26T23:00:46Z","receivedAt":"2017-01-26T23:01:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 25, 2017 at 02:02:53PM -0800, Jonathan Tan wrote:\n\n> Negotiation currently happens by upload-pack initially sending a list of\n> refs with names and SHA-1 hashes, and then several request/response\n> pairs in which the request from fetch-pack consists of SHA-1 hashes\n> (selected from the initial list). Allowing the request to consist of\n> names instead of SHA-1 hashes increases tolerance to refs changing\n> (due to time, and due to having load-balanced servers without strong\n> consistency),\n\nInteresting. My big question is: what happens when a ref _does_ change?\nHow does the client handle this?\n\nThe existing uploadpack.allowReachableSHA1InWant is there to work around\nthe problem that an http client may get a ref advertisement in one step,\nand then come back later to do the want/have negotiation, at which point\nthe server has moved on (or maybe it's even a different server). There\nthe client says \"I want sha1 X\", and the server needs to say \"well, X\nisn't my tip now, but it's still acceptable for you to fetch\".\n\nBut this seems to go in the opposite direction. After the advertisement,\nthe client decides \"OK, I want to fetch refs/heads/master which is at\nSHA1 X\". It connects to the server and says \"I want refs/heads/master\".\nLet's say the server has moved its version of the ref to SHA1 Y.\n\nWhat happens? I think the server will say \"wanted-ref master Y\". Does\nthe client just decide to use \"Y\" then? How does that interact with any\ndecisions the client might have made about X? I guess things like\nfast-forwards have to be decided after we fetch the objects anyway\n(since we cannot compute them until we get the pack), so maybe there\naren't any such decisions. I haven't checked.\n\n> and is a step towards eliminating the need for the server\n> to send the list of refs first (possibly improving performance).\n\nI'm not sure it is all that useful towards that end. You still have to\nbreak compatibility so that the client tells the server to suppress the\nref advertisement. After that, it is just a question of asking for the\nrefs. And you have two options:\n\n 1. Ask the server to tell you the values of some subset of the refs,\n pick what you want, and then do the want/have as normal.\n\n 2. Go straight to the want/have, but tell the server the refs you want\n instead of their sha1s.\n\nI think your approach here would lead to (2).\n\nBut (1), besides being closer to how the protocol works now, seems like\nit's more flexible. I can ask about the ref state without necessarily\nhaving to retrieve the objects. How would you write git-ls-remote with\nsuch a system?\n\n> [1] There has been some discussion about whether the server should\n> accept partial ref names, e.g. [2]. In this patch set, I have made the\n> server only accept full names, and it is the responsibility of the\n> client to send the multiple patterns which it wants to match. Quoting\n> from the commit message of the second patch:\n> \n> For example, a client could reasonably expand an abbreviated\n> name \"foo\" to \"want-ref foo\", \"want-ref refs/heads/foo\", \"want-ref\n> refs/tags/foo\", among others, and ensure that at least one such ref has\n> been fetched.\n\nThat has a cost that scales linearly with the number of refs, because\nyou have to ask for each name 6 times. After the discussion you linked,\nI think my preference is more like:\n\n 1. Teach servers to accept a list of patterns from the client\n which will be resolved in order. Unlike your system, the client\n only needs to specify the list once per session, rather than once\n per ref.\n\n 2. (Optional) Give a shorthand for the stock patterns that git has had\n in place for years. That saves some bytes over specifying the\n patterns completely (though it's really not _that_ many bytes, so\n perhaps the complication isn't a big deal).\n\n-Peff\n"},{"id":"310374","messageId":"xmqq8tpx30zq.fsf@gitster.mtv.corp.google.com","threadId":"44973","inReplyTo":"afe5d7d3f876893fdad318665805df1e056717c6.1485381677.git.jonathantanmy@google.com","subject":"Re: [RFC 03/14] upload-pack: test negotiation with changing repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-26T22:33:29Z","receivedAt":"2017-01-26T23:27:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> diff --git a/t/lib-httpd/one-time-sed.sh b/t/lib-httpd/one-time-sed.sh\n> new file mode 100644\n> index 000000000..060ec0300\n> --- /dev/null\n> +++ b/t/lib-httpd/one-time-sed.sh\n> @@ -0,0 +1,8 @@\n> +#!/bin/sh\n> +\n> +if [ -e one-time-sed ]; then\n> +\t\"$GIT_EXEC_PATH/git-http-backend\" | sed \"$(cat one-time-sed)\"\n> +\trm one-time-sed\n> +else\n> +\t\"$GIT_EXEC_PATH/git-http-backend\"\n> +fi\n\nCodingGuidelines?\n\n> +inconsistency() {\n> +\t# Simulate that the server initially reports $2 as the ref\n> +\t# corresponding to $1, and after that, $1 as the ref corresponding to\n> +\t# $1. This corresponds to the real-life situation where the server's\n> +\t# repository appears to change during negotiation, for example, when\n> +\t# different servers in a load-balancing arrangement serve (stateless)\n> +\t# RPCs during a single negotiation.\n> +\tprintf \"s/%s/%s/\" \\\n> +\t $(git -C \"$REPO\" rev-parse $1 | tr -d \"\\n\") \\\n> +\t $(git -C \"$REPO\" rev-parse $2 | tr -d \"\\n\") \\\n> +\t >\"$HTTPD_ROOT_PATH/one-time-sed\"\n\nI'd prefer for the printf'd result to have a final LF (i.e. not\nleaving the resulting one-time-sed with a final incomplete line).\nAlso, do you really need the pipe to tr-d? Doesn't the result of \n$(command substitution) omit the final LF anyway?\n\n $ printf '1 %s 2 %s 3\\n' \"$(echo foo)\" \"$(echo bar)\"; echo OK\n 1 foo 2 bar 3\n OK\n\n> diff --git a/upload-pack.c b/upload-pack.c\n> index b88ed8e26..0678c53d6 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -862,9 +862,13 @@ static void receive_needs(struct string_list *wanted_ns_refs)\n> \t\t} else if (skip_prefix(line, \"want \", &arg) &&\n> \t\t\t !get_sha1_hex(arg, sha1_buf)) {\n> \t\t\to = parse_object(sha1_buf);\n> -\t\t\tif (!o)\n> +\t\t\tif (!o) {\n> +\t\t\t\tpacket_write_fmt(1,\n> +\t\t\t\t\t\t \"ERR upload-pack: not our ref %s\",\n> +\t\t\t\t\t\t sha1_to_hex(sha1_buf));\n> \t\t\t\tdie(\"git upload-pack: not our ref %s\",\n> \t\t\t\t sha1_to_hex(sha1_buf));\n> +\t\t\t}\n\nThis somehow looks like a good thing to do even in production. Am I\nmistaken?\n\n"},{"id":"310377","messageId":"67afbb3b-5d0b-8c0d-3f6e-3f559c68f4bd@google.com","threadId":"44973","inReplyTo":"20170126230046.aknesybfyzxhx3ia@sigill.intra.peff.net","subject":"Re: [RFC 00/14] Allow fetch-pack to send ref names (globs allowed)","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-27T00:26:47Z","receivedAt":"2017-01-27T00:27:44Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Thanks for your comments.\n\nOn 01/26/2017 03:00 PM, Jeff King wrote:\n> On Wed, Jan 25, 2017 at 02:02:53PM -0800, Jonathan Tan wrote:\n>\n>> Negotiation currently happens by upload-pack initially sending a list of\n>> refs with names and SHA-1 hashes, and then several request/response\n>> pairs in which the request from fetch-pack consists of SHA-1 hashes\n>> (selected from the initial list). Allowing the request to consist of\n>> names instead of SHA-1 hashes increases tolerance to refs changing\n>> (due to time, and due to having load-balanced servers without strong\n>> consistency),\n>\n> Interesting. My big question is: what happens when a ref _does_ change?\n> How does the client handle this?\n>\n> The existing uploadpack.allowReachableSHA1InWant is there to work around\n> the problem that an http client may get a ref advertisement in one step,\n> and then come back later to do the want/have negotiation, at which point\n> the server has moved on (or maybe it's even a different server). There\n> the client says \"I want sha1 X\", and the server needs to say \"well, X\n> isn't my tip now, but it's still acceptable for you to fetch\".\n>\n> But this seems to go in the opposite direction. After the advertisement,\n> the client decides \"OK, I want to fetch refs/heads/master which is at\n> SHA1 X\". It connects to the server and says \"I want refs/heads/master\".\n> Let's say the server has moved its version of the ref to SHA1 Y.\n>\n> What happens? I think the server will say \"wanted-ref master Y\". Does\n> the client just decide to use \"Y\" then? How does that interact with any\n> decisions the client might have made about X? I guess things like\n> fast-forwards have to be decided after we fetch the objects anyway\n> (since we cannot compute them until we get the pack), so maybe there\n> aren't any such decisions. I haven't checked.\n\nYes, the server will say \"wanted-ref master Y\". The relevant code \nregarding the decisions the client makes regarding X or Y is in do_fetch \nin builtin/fetch.c.\n\nThere, I can see these decisions done using X:\n - check_not_current_branch (forbidding fetching that modifies the\n current branch) (I just noticed that this has to be done for Y too,\n and will do so)\n - prune refs [*]\n - automatic tag following [*]\n\n[*] X and Y may differ in that one relevant ref appears in one set but \nnot in the other (because a ref was added or removed in the meantime), \ncausing a different result if these decisions were to be done using Y, \nbut I think that it is OK either way.\n\nFetch optimizations (for example, everything_local in fetch-pack.c) that \ncheck if the client really needs to fetch are also done using X, of \ncourse (and if the optimization succeeds, there is no Y).\n\nFast-forwards (and everything else in store_updated_refs) are decided \nusing Y.\n\n>> and is a step towards eliminating the need for the server\n>> to send the list of refs first (possibly improving performance).\n>\n> I'm not sure it is all that useful towards that end. You still have to\n> break compatibility so that the client tells the server to suppress the\n> ref advertisement. After that, it is just a question of asking for the\n> refs. And you have two options:\n>\n> 1. Ask the server to tell you the values of some subset of the refs,\n> pick what you want, and then do the want/have as normal.\n>\n> 2. Go straight to the want/have, but tell the server the refs you want\n> instead of their sha1s.\n>\n> I think your approach here would lead to (2).\n>\n> But (1), besides being closer to how the protocol works now, seems like\n> it's more flexible. I can ask about the ref state without necessarily\n> having to retrieve the objects. How would you write git-ls-remote with\n> such a system?\n\nAssuming a new protocol with the appropriate backwards compatibility \n(which would have to be done for both options), (2) does save a \nrequest/response pair (because we can send the ref names directly as \n\"want-ref\" in the initial request instead of sending ref names in the \ninitial request and then confirming them using \"want \" in the \nsubsequent request). Also, (2) is more tolerant towards refs changing \nover time. (1) might be closer to the current protocol, but I think that \nthe difference is not so significant (only in \"want-ref\" vs \"want\" \nrequest and the \"wanted-ref\" response).\n\nAs for git-ls-remote, I currently think that it would have to use the \nexisting protocol.\n\n>> [1] There has been some discussion about whether the server should\n>> accept partial ref names, e.g. [2]. In this patch set, I have made the\n>> server only accept full names, and it is the responsibility of the\n>> client to send the multiple patterns which it wants to match. Quoting\n>> from the commit message of the second patch:\n>>\n>> For example, a client could reasonably expand an abbreviated\n>> name \"foo\" to \"want-ref foo\", \"want-ref refs/heads/foo\", \"want-ref\n>> refs/tags/foo\", among others, and ensure that at least one such ref has\n>> been fetched.\n>\n> That has a cost that scales linearly with the number of refs, because\n> you have to ask for each name 6 times. After the discussion you linked,\n> I think my preference is more like:\n>\n> 1. Teach servers to accept a list of patterns from the client\n> which will be resolved in order. Unlike your system, the client\n> only needs to specify the list once per session, rather than once\n> per ref.\n>\n> 2. (Optional) Give a shorthand for the stock patterns that git has had\n> in place for years. That saves some bytes over specifying the\n> patterns completely (though it's really not _that_ many bytes, so\n> perhaps the complication isn't a big deal).\n\nI envision that most fetches would be done on a single name (with or \nwithout globs), that expands to 5 (so it is not so crucial to factor out \nthe list of patterns). Having said that, I am open to changing this.\n"},{"id":"310378","messageId":"dc09e446-6d29-8b94-f440-6aa094ab9dc9@google.com","threadId":"44973","inReplyTo":"xmqqd1f931g7.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC 02/14] upload-pack: allow ref name and glob requests","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-27T00:35:31Z","receivedAt":"2017-01-27T00:35:38Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"\n\nOn 01/26/2017 02:23 PM, Junio C Hamano wrote:\n> Jonathan Tan writes:\n>\n>> Currently, while performing packfile negotiation [1], upload-pack allows\n>> clients to specify their desired objects only as SHA-1s. This causes:\n>> (a) vulnerability to failure when an object turns non-existent during\n>> negotiation, which may happen if, for example, upload-pack is\n>> provided by multiple Git servers in a load-balancing arrangement,\n>> and\n>> (b) dependence on the server first publishing a list of refs with\n>> associated objects.\n>>\n>> To eliminate (a) and take a step towards eliminating (b), teach\n>> upload-pack to support requests in the form of ref names and globs (in\n>> addition to the existing support for SHA-1s) through a new line of the\n>> form \"want-ref \" where ref is the full name of a ref, a glob\n>> pattern, or a SHA-1. At the conclusion of negotiation, the server will\n>> write \"wanted-ref \" for all requests that have been\n>> specified this way.\n>\n> I am not sure if this \"at the conclusion of\" is sensible. It is OK\n> to assume that what the client side has is fixed, and it is probably\n> OK to desire that what the server side has can change, but at the\n> same time, it feels quite fragile to move the goalpost in between.\n\nDo you have any specific concerns as to this fragility? Peff mentioned \nsome concerns with the client making some decisions based on the initial \nSHA-1 vs the SHA-1 reported by \"wanted-ref\", to which I replied [1].\n\n> Stepping back a bit, in an environment that involves multiple server\n> instances that have inconsistent set of refs, can the negotiation\n> even be sensibly and safely implemented? The first server the\n> client contacts may, in response to a \"have\", say \"I do have that\n> commit so you do not have to send its ancestors to me. We found one\n> cut-off point. Please do explore other lines of histories.\" The\n> next server that concludes the negotiation exchange may not have\n> that commit and will be unable to produce a pack that excludes the\n> objects reachable from that commit---wouldn't that become a problem?\n\nIt's true that this patch set wouldn't solve this problem. This problem \nonly occurs when there is a commit that the client knows but only a few \nof the servers know (maybe because the client just pushed it to one of \nthem). If, for example, the client does not know a commit and only a few \nof the servers know it (for example, because another user just pushed \nit), this patch set does help. The latter scenario seems like it would \noccur relatively commonly.\n\n> One way to prevent such a problem from hurting clients may be for\n> these multiple server instances to coordinate and make sure they\n> have a shared perception of the common history among them. Some\n> pushes may have come to one instance but may not have propagated to\n> other instances, and such a commit cannot be accepted as usable\n> \"have\" if the servers anticipate that the final client request would\n> go to any of the servers. Otherwise the multiple server arrangement\n> would not work safely, methinks.\n>\n> And if the servers are ensuring the safety using such a mechanism,\n> they can use the same mechanism to restrain \"faster\" instances from\n> sending too fresh state of refs that other instances haven't caught\n> up to, which would mean they can present a consistent set of refs to\n> the client in the first place, no?\n>\n> So I am not sure if the mechanism to request history by refname\n> instead of the tip commit would help the multi-server environment as\n> advertised. It may help solving other problems, though (e.g. like\n> \"somebody pushed to update after the initial advertisement was sent\n> out\" which can happen even in a single server environment).\n\nThis patch set would solve the problem you describe (whether in a single \nserver environment or the coordination between multiple servers that \nprovides \"strong consistency\"). (Although it may not be an important \nproblem to solve, since it is probably OK if the client got a \"slow\" \nversion of the state of the refs.)\n\n>> To be flexible with respect to client needs, the server does not\n>> indicate an error if a \"want-ref\" line corresponds to no refs, but\n>> instead relies on the client to ensure that what the user needs has been\n>> fetched. For example, a client could reasonably expand an abbreviated\n>> name \"foo\" to \"want-ref foo\", \"want-ref refs/heads/foo\", \"want-ref\n>> refs/tags/foo\", among others, and ensure that at least one such ref has\n>> been fetched.\n>\n> Cute. This may be one way to implement the DWIM thing within the\n> constraint of eventually wanting to go to \"client speaks first, the\n> server does not advertise things the client is not interested in\"\n> world.\n>\n> But at the same time it may end up bloating the set of refs the\n> client asks instead. Instead of receiving the advertisement and\n> then sending one request after picking the matching one from it,\n> the client needs to send \"refs/{heads,tags,whatever}/foo\".\n\nThat is true, although I think that the client will typically send only \na few ref names (with or without globs), so the request packet is still \nnot that large.\n\n[1] <67afbb3b-5d0b-8c0d-3f6e-3f559c68f4bd@google.com>\n"},{"id":"310380","messageId":"0c727063-0583-f713-68fb-bd284be696b2@google.com","threadId":"44973","inReplyTo":"xmqq8tpx30zq.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC 03/14] upload-pack: test negotiation with changing repo","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-01-27T00:44:14Z","receivedAt":"2017-01-27T00:51:47Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On 01/26/2017 02:33 PM, Junio C Hamano wrote:\n> Jonathan Tan writes:\n>\n>> diff --git a/t/lib-httpd/one-time-sed.sh b/t/lib-httpd/one-time-sed.sh\n>> new file mode 100644\n>> index 000000000..060ec0300\n>> --- /dev/null\n>> +++ b/t/lib-httpd/one-time-sed.sh\n>> @@ -0,0 +1,8 @@\n>> +#!/bin/sh\n>> +\n>> +if [ -e one-time-sed ]; then\n>> +\t\"$GIT_EXEC_PATH/git-http-backend\" | sed \"$(cat one-time-sed)\"\n>> +\trm one-time-sed\n>> +else\n>> +\t\"$GIT_EXEC_PATH/git-http-backend\"\n>> +fi\n>\n> CodingGuidelines?\n\nThanks - done locally and will send out in the next reroll.\n\n>> +inconsistency() {\n>> +\t# Simulate that the server initially reports $2 as the ref\n>> +\t# corresponding to $1, and after that, $1 as the ref corresponding to\n>> +\t# $1. This corresponds to the real-life situation where the server's\n>> +\t# repository appears to change during negotiation, for example, when\n>> +\t# different servers in a load-balancing arrangement serve (stateless)\n>> +\t# RPCs during a single negotiation.\n>> +\tprintf \"s/%s/%s/\" \\\n>> +\t $(git -C \"$REPO\" rev-parse $1 | tr -d \"\\n\") \\\n>> +\t $(git -C \"$REPO\" rev-parse $2 | tr -d \"\\n\") \\\n>> +\t >\"$HTTPD_ROOT_PATH/one-time-sed\"\n>\n> I'd prefer for the printf'd result to have a final LF (i.e. not\n> leaving the resulting one-time-sed with a final incomplete line).\n> Also, do you really need the pipe to tr-d? Doesn't the result of\n> $(command substitution) omit the final LF anyway?\n>\n> $ printf '1 %s 2 %s 3\\n' \"$(echo foo)\" \"$(echo bar)\"; echo OK\n> 1 foo 2 bar 3\n> OK\n\nDone.\n\n>> diff --git a/upload-pack.c b/upload-pack.c\n>> index b88ed8e26..0678c53d6 100644\n>> --- a/upload-pack.c\n>> +++ b/upload-pack.c\n>> @@ -862,9 +862,13 @@ static void receive_needs(struct string_list *wanted_ns_refs)\n>> \t\t} else if (skip_prefix(line, \"want \", &arg) &&\n>> \t\t\t !get_sha1_hex(arg, sha1_buf)) {\n>> \t\t\to = parse_object(sha1_buf);\n>> -\t\t\tif (!o)\n>> +\t\t\tif (!o) {\n>> +\t\t\t\tpacket_write_fmt(1,\n>> +\t\t\t\t\t\t \"ERR upload-pack: not our ref %s\",\n>> +\t\t\t\t\t\t sha1_to_hex(sha1_buf));\n>> \t\t\t\tdie(\"git upload-pack: not our ref %s\",\n>> \t\t\t\t sha1_to_hex(sha1_buf));\n>> +\t\t\t}\n>\n> This somehow looks like a good thing to do even in production. Am I\n> mistaken?\n\nYes, that's true. If this patch set stalls (for whatever reason), I'll \nspin this off into an independent patch.\n"},{"id":"310381","messageId":"xmqqinp11d3p.fsf@gitster.mtv.corp.google.com","threadId":"44973","inReplyTo":"dc09e446-6d29-8b94-f440-6aa094ab9dc9@google.com","subject":"Re: [RFC 02/14] upload-pack: allow ref name and glob requests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-27T01:54:50Z","receivedAt":"2017-01-27T01:55:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n>> I am not sure if this \"at the conclusion of\" is sensible. It is OK\n>> to assume that what the client side has is fixed, and it is probably\n>> OK to desire that what the server side has can change, but at the\n>> same time, it feels quite fragile to move the goalpost in between.\n>\n> Do you have any specific concerns as to this fragility? Peff mentioned\n> some concerns with the client making some decisions based on the\n> initial SHA-1 vs the SHA-1 reported by \"wanted-ref\", to which I\n> replied [1].\n\nThere were two but I think you are aware of both. One is what Peff\nalready mentioned, the client may want to make the decision before\ngoing through the negotiation. The other is \"moving the goalpost\",\nthe history the last server has may violate the view of the history\ncommon between the server and the client that is established during\nthe negotiation with previous servers.\n\n"},{"id":"311018","messageId":"20170207235350.10718-1-jonathantanmy@google.com","threadId":"44973","inReplyTo":"cover.1485381677.git.jonathantanmy@google.com","subject":"Re: [RFC 00/14] Allow fetch-pack to send ref names (globs allowed)","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-02-07T23:53:50Z","receivedAt":"2017-02-08T00:01:42Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Looking back at the comments I have received in reply, I think that\nthere were two major concerns: (i) the case where a server ACKs a client\n\"have\" line and the client forever thinks that the server has it, but it\nmay not be the case for future servers (or future invocations of the\nsame server), and (ii) what the client does with 2 \"versions\" of remote\nrefs.\n\nFor (i), the issue already exists and as far as I can tell, this patch\nset does not directly impact it, positively or negatively. The\n\"have\"/\"ACK\" part of negotiation is kept the same - the only difference\nin this patch set is that wants can be specified by name instead of by\nSHA-1 hash. This patch set does not help the \"have\"/\"ACK\" part of\nnegotiation, but it helps the \"want\" part.\n\nFor (ii), I have prepared a patch to be squashed, and extended the\ncommit message with an explanation of what is happening. (The commit\nmessage and the patch are appended to this e-mail).\n\n(There was also some discussion of the client being required to send\nexact matches in its \"want-ref\" lines.)\n\nPlease let me know if you have any other opinions or thoughts. It does\nseem to me like such a protocol update (or something similar) would help\nfor large repositories with many ever-changing refs (like refs/changes\nin Gerrit or refs/pull in GitHub) - and the creation of a ref would look\nlike a deletion depending on the order in which we access the servers in\na load-balancing arrangement and the order in which those servers\nsynchronize themselves. And deletion of refs does not work with the\ncurrent protocol, but would work with a protocol that supports wildcards\n(like this one).\n\n-- 8< --\n\nfetch: send want-ref and receive fetched refs\n\nTeach fetch to send refspecs to the underlying transport, and teach all\ncomponents used by the HTTP transport (remote-curl, transport-helper,\nfetch-pack) to understand and propagate the names and SHA-1s of the refs\nfetched.\n\nThe do_fetch method in builtin/fetch.c originally had only one\nremote-local ref map, generated from the already-fetched list of remote\nrefs. This patch introduces a new ref map generated from the list of\nfetched refs. Usages of the list of remote refs or the remote-local ref\nmap are updated as follows:\n - check_not_current_branch (which checks that the current branch is not\n affected by the fetch) is performed both on the original ref map (to\n die before the fetch if we can, as an optimization) and on the new\n ref map (since the new ref map is the one actually applied).\n - Pruning is done based on the new ref map.\n - backfill_tags (for tag following) is performed using the original\n list of remote refs because the new list of fetched refs is not\n guaranteed to contain tag information. (Since backfill_tags performs\n another fetch, it does not need to be fully consistent with the\n just-returned packfile.)\nThe list of remote refs and the remote-local ref map are not otherwise\nused by do_fetch or any function in its invocation chain (fetch_one and\ncmd_fetch).\n---\n builtin/fetch.c | 26 ++++++++++++++------------\n 1 file changed, 14 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 87de00e49..b8432394c 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1177,6 +1177,20 @@ static int do_fetch(struct transport *transport,\n \n \tif (tags == TAGS_DEFAULT && autotags)\n \t\ttransport_set_option(transport, TRANS_OPT_FOLLOWTAGS, \"1\");\n+\tif (fetch_refs(transport, e_rs, e_rs_nr, ref_map, &new_remote_refs)) {\n+\t\tfree_refs(ref_map);\n+\t\tretcode = 1;\n+\t\tgoto cleanup;\n+\t}\n+\tif (new_remote_refs) {\n+\t\tfree_refs(ref_map);\n+\t\tref_map = get_ref_map(transport->remote, new_remote_refs,\n+\t\t\t\t refs, ref_count, tags, autotags);\n+\t\tif (!update_head_ok)\n+\t\t\tcheck_not_current_branch(ref_map);\n+\t\tfree_refs(new_remote_refs);\n+\t}\n+\n \tif (prune) {\n \t\t/*\n \t\t * We only prune based on refspecs specified\n@@ -1192,18 +1206,6 @@ static int do_fetch(struct transport *transport,\n \t\t\t\t transport->url);\n \t\t}\n \t}\n-\tif (fetch_refs(transport, e_rs, e_rs_nr, ref_map, &new_remote_refs)) {\n-\t\tfree_refs(ref_map);\n-\t\tretcode = 1;\n-\t\tgoto cleanup;\n-\t}\n-\tif (new_remote_refs) {\n-\t\tfree_refs(ref_map);\n-\t\tref_map = get_ref_map(transport->remote, new_remote_refs,\n-\t\t\t\t refs, ref_count, tags, autotags);\n-\t\tfree_refs(new_remote_refs);\n-\t}\n-\n \tif (consume_refs(transport, ref_map)) {\n \t\tfree_refs(ref_map);\n \t\tretcode = 1;\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"311132","messageId":"xmqq7f50tdja.fsf@gitster.mtv.corp.google.com","threadId":"44973","inReplyTo":"20170207235350.10718-1-jonathantanmy@google.com","subject":"Re: [RFC 00/14] Allow fetch-pack to send ref names (globs allowed)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-09T00:26:33Z","receivedAt":"2017-02-09T00:26:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> Usages of the list of remote refs or the remote-local ref\n> map are updated as follows:\n> - check_not_current_branch (which checks that the current branch is not\n> affected by the fetch) is performed both on the original ref map (to\n> die before the fetch if we can, as an optimization) and on the new\n> ref map (since the new ref map is the one actually applied).\n\nOK.\n\n> - Pruning is done based on the new ref map.\n\nOK. As that is what eventually gets \"installed\" on the local side,\nit makes sense to become consistent with that set, not the set the\noriginal server gave you.\n\n> - backfill_tags (for tag following) is performed using the original\n> list of remote refs because the new list of fetched refs is not\n> guaranteed to contain tag information. (Since backfill_tags performs\n> another fetch, it does not need to be fully consistent with the\n> just-returned packfile.)\n\nThis smells correct but I need to think about this one a bit more.\n\nOverall I think the strategy is agreeable.\n"},{"id":"312355","messageId":"xmqqo9xtajcu.fsf@gitster.mtv.corp.google.com","threadId":"44973","inReplyTo":"0c727063-0583-f713-68fb-bd284be696b2@google.com","subject":"Re: [RFC 03/14] upload-pack: test negotiation with changing repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-22T23:36:33Z","receivedAt":"2017-02-22T23:36:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n>> This somehow looks like a good thing to do even in production. Am I\n>> mistaken?\n>\n> Yes, that's true. If this patch set stalls (for whatever reason), I'll\n> spin this off into an independent patch.\n\n... which may be needed.\n\nAs to the main goal of this topic, I think the \"ask by refname (with\nglob)\" is very good thing to start the \"client speaks first\" v2\nprotocol, as it would allow us to reduce the size of the initial\nadvertisement for common cases (i.e. remote..fetch is likely\nto list only refs/heads/* on the left hand side of a refspec). And\nadding this to v1 is probably a good first step to make sure the\ncode that is currently used by v1 protocol exchange that will be\nshared with the v2 protocols are prepared to be driven by refname\nwithout knowing the exact object name until the final round.\n\n\n\n"},{"id":"312412","messageId":"20170223184303.26530-1-jonathantanmy@google.com","threadId":"44973","inReplyTo":"xmqqo9xtajcu.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] upload-pack: report \"not our ref\" to client","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-02-23T18:43:03Z","receivedAt":"2017-02-23T18:52:26Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Make upload-pack report \"not our ref\" errors to the client as an \"ERR\" line.\n(If not, the client would be left waiting for a response when the server is\nalready dead.)\n\nSigned-off-by: Jonathan Tan \n---\n\nThanks, here is the independent patch.\n\n upload-pack.c | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 7597ba340..ffb028d62 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -822,9 +822,13 @@ static void receive_needs(void)\n \t\t\tuse_include_tag = 1;\n \n \t\to = parse_object(sha1_buf);\n-\t\tif (!o)\n+\t\tif (!o) {\n+\t\t\tpacket_write_fmt(1,\n+\t\t\t\t\t \"ERR upload-pack: not our ref %s\",\n+\t\t\t\t\t sha1_to_hex(sha1_buf));\n \t\t\tdie(\"git upload-pack: not our ref %s\",\n \t\t\t sha1_to_hex(sha1_buf));\n+\t\t}\n \t\tif (!(o->flags & WANTED)) {\n \t\t\to->flags |= WANTED;\n \t\t\tif (!((allow_unadvertised_object_request & ALLOW_ANY_SHA1) == ALLOW_ANY_SHA1\n-- \n2.11.0.483.g087da7b7c-goog\n\n"},{"id":"312431","messageId":"xmqqshn47jgs.fsf@gitster.mtv.corp.google.com","threadId":"44973","inReplyTo":"20170223184303.26530-1-jonathantanmy@google.com","subject":"Re: [PATCH] upload-pack: report \"not our ref\" to client","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-23T20:14:43Z","receivedAt":"2017-02-23T20:14:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n"}]}