git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Brandon Williams <bmwill@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/2] refspec: consolidate ref-prefix generation logic
Date: Wed, 30 May 2018 18:07:39 -0700
Message-ID: <20180531010739.GB36515@aiede.svl.corp.google.com> (raw)
In-Reply-To: <20180531004309.GA36515@aiede.svl.corp.google.com>

Jonathan Nieder wrote:
> Brandon Williams wrote:

>> When using protocol v2 a client constructs a list of ref-prefixes which
>> are sent across the wire so that the server can do server-side filtering
>> of the ref-advertisement.  The logic that does this exists for both
>> fetch and push (even though no push support for v2 currently exists yet)
>> and is roughly the same so lets consolidate this logic and make it
>> general enough that it can be used for both the push and fetch cases.
>>
>> Signed-off-by: Brandon Williams <bmwill@google.com>
>> ---
>>  builtin/fetch.c | 13 +------------
>>  refspec.c       | 29 +++++++++++++++++++++++++++++
>>  refspec.h       |  4 ++++
>>  transport.c     | 21 +--------------------
>>  4 files changed, 35 insertions(+), 32 deletions(-)
>
> I assume this is meant to be a refactoring with no functional change.
> Alas, it's causing fetch-by-SHA-1 to fail for me:
[...]
>  $ bin-wrappers/git -C /tmp/r -c protocol.version=2 fetch \
> 	https://kernel.googlesource.com/pub/scm/git/git \
> 	6373cb598e1a4e0340583ad75d5abba01ff79774
>  fatal: no matching remote head

Backtrace:

 #0  0x0000555555671e09 in fetch_pack (args=0x7fffffffda80, fd=0x555555a6a9b0, conn=0x555555a65780, ref=0x0, 
     dest=0x555555a64900 "https://kernel.googlesource.com/pub/scm/git/git", sought=0x555555a69a80, nr_sought=1, shallow=0x555555a6a9d8, 
     pack_lockfile=0x555555a631c8, version=protocol_v2) at fetch-pack.c:1562
 #1  0x00005555557290b9 in fetch_refs_via_pack (transport=0x555555a63190, nr_heads=1, to_fetch=0x555555a69a80) at transport.c:326
 #2  0x000055555572b712 in transport_fetch_refs (transport=0x555555a63190, refs=0x555555a65520) at transport.c:1247
 #3  0x000055555559a2a6 in fetch_refs (transport=0x555555a63190, ref_map=0x555555a65520) at builtin/fetch.c:942
 #4  0x000055555559ac3e in do_fetch (transport=0x555555a63190, rs=0x7fffffffdc30) at builtin/fetch.c:1157
 #5  0x000055555559b535 in fetch_one (remote=0x555555a63870, argc=1, argv=0x7fffffffdff8, prune_tags_ok=0) at builtin/fetch.c:1398
 #6  0x000055555559b9a3 in cmd_fetch (argc=1, argv=0x7fffffffdff8, prefix=0x0) at builtin/fetch.c:1487
 #7  0x000055555556764b in run_builtin (p=0x555555a0fa30 <commands+816>, argc=3, argv=0x7fffffffdff0) at git.c:350
 #8  0x000055555556795f in handle_builtin (argc=3, argv=0x7fffffffdff0) at git.c:565
 #9  0x0000555555567b07 in run_argv (argcp=0x7fffffffde8c, argv=0x7fffffffde80) at git.c:617
 #10 0x0000555555567cbc in cmd_main (argc=3, argv=0x7fffffffdff0) at git.c:694
 #11 0x000055555560774c in main (argc=8, argv=0x7fffffffdfc8) at common-main.c:45

Both refs_tmp and transport->remote_refs are NULL.

When refspec_ref_prefixes gets called, it produces the usual
argv_array with 6373cb598e1a4e0340583ad75d5abba01ff79774,
refs/6373cb598e1a4e0340583ad75d5abba01ff79774, etc.

In the old code, this doesn't happen because we trigger the
item->exact_sha1 test.

Would something like the following make sense?  It passes tests and
appears to avoid trouble but I haven't looked any closer than that.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

--- i/refspec.c
+++ w/refspec.c
@@ -202,6 +202,8 @@ void refspec_ref_prefixes(const struct refspec *rs,
 		const struct refspec_item *item = &rs->items[i];
 		const char *prefix = NULL;
 
+		if (item->exact_sha1)
+			continue;
 		if (rs->fetch == REFSPEC_FETCH)
 			prefix = item->src;
 		else if (item->dst)

  reply index

Thread overview: 112+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-14 21:55 [PATCH 00/35] refactoring refspecs Brandon Williams
2018-05-14 21:55 ` [PATCH 01/35] refspec: move refspec parsing logic into its own file Brandon Williams
2018-05-15  8:06   ` Junio C Hamano
2018-05-15 16:51     ` Brandon Williams
2018-05-16  0:40       ` Junio C Hamano
2018-05-14 21:55 ` [PATCH 02/35] refspec: factor out parsing a single refspec Brandon Williams
2018-05-14 21:55 ` [PATCH 03/35] refspec: rename struct refspec to struct refspec_item Brandon Williams
2018-05-15  8:17   ` Junio C Hamano
2018-05-15 18:19     ` Brandon Williams
2018-05-14 21:55 ` [PATCH 04/35] refspec: introduce struct refspec Brandon Williams
2018-05-15  9:37   ` Junio C Hamano
2018-05-15 18:37     ` Brandon Williams
2018-05-14 21:55 ` [PATCH 05/35] refspec: convert valid_fetch_refspec to use parse_refspec Brandon Williams
2018-05-15  9:41   ` Junio C Hamano
2018-05-14 21:55 ` [PATCH 06/35] submodule--helper: convert push_check to use struct refspec Brandon Williams
2018-05-14 21:55 ` [PATCH 07/35] pull: convert get_tracking_branch to use refspec_item_init Brandon Williams
2018-05-14 21:55 ` [PATCH 08/35] transport: convert transport_push to use struct refspec Brandon Williams
2018-05-14 21:56 ` [PATCH 09/35] remote: convert check_push_refs " Brandon Williams
2018-05-14 21:56 ` [PATCH 10/35] remote: convert match_push_refs " Brandon Williams
2018-05-14 21:56 ` [PATCH 11/35] clone: convert cmd_clone to use refspec_item_init Brandon Williams
2018-05-14 21:56 ` [PATCH 12/35] fast-export: convert to use struct refspec Brandon Williams
2018-05-14 21:56 ` [PATCH 13/35] remote: convert push refspecs to " Brandon Williams
2018-05-14 21:56 ` [PATCH 14/35] remote: convert fetch " Brandon Williams
2018-05-15  8:31   ` Ævar Arnfjörð Bjarmason
2018-05-15 17:57     ` Brandon Williams
2018-05-14 21:56 ` [PATCH 15/35] transport-helper: convert to use " Brandon Williams
2018-05-14 21:56 ` [PATCH 16/35] fetch: convert fetch_one " Brandon Williams
2018-05-14 21:56 ` [PATCH 17/35] fetch: convert refmap " Brandon Williams
2018-05-14 21:56 ` [PATCH 18/35] refspec: remove the deprecated functions Brandon Williams
2018-05-14 21:56 ` [PATCH 19/35] fetch: convert do_fetch to take a struct refspec Brandon Williams
2018-05-14 21:56 ` [PATCH 20/35] fetch: convert get_ref_map " Brandon Williams
2018-05-14 21:56 ` [PATCH 21/35] fetch: convert prune_refs " Brandon Williams
2018-05-14 21:56 ` [PATCH 22/35] remote: convert get_stale_heads " Brandon Williams
2018-05-14 21:56 ` [PATCH 23/35] remote: convert apply_refspecs " Brandon Williams
2018-05-14 21:56 ` [PATCH 24/35] remote: convert query_refspecs " Brandon Williams
2018-05-14 21:56 ` [PATCH 25/35] remote: convert get_ref_match " Brandon Williams
2018-05-14 21:56 ` [PATCH 26/35] remote: convert match_explicit_refs " Brandon Williams
2018-05-14 21:56 ` [PATCH 27/35] push: check for errors earlier Brandon Williams
2018-05-14 21:56 ` [PATCH 28/35] push: convert to use struct refspec Brandon Williams
2018-05-14 21:56 ` [PATCH 29/35] transport: convert transport_push to take a " Brandon Williams
2018-05-14 21:56 ` [PATCH 30/35] send-pack: store refspecs in " Brandon Williams
2018-05-14 21:56 ` [PATCH 31/35] transport: remove transport_verify_remote_names Brandon Williams
2018-05-14 21:56 ` [PATCH 32/35] http-push: store refspecs in a struct refspec Brandon Williams
2018-05-14 21:56 ` [PATCH 33/35] remote: convert match_push_refs to take " Brandon Williams
2018-05-14 21:56 ` [PATCH 34/35] remote: convert check_push_refs " Brandon Williams
2018-05-14 21:56 ` [PATCH 35/35] submodule: convert push_unpushed_submodules " Brandon Williams
2018-05-15  8:11   ` Ævar Arnfjörð Bjarmason
2018-05-15 16:52     ` Stefan Beller
2018-05-15 16:59     ` Brandon Williams
2018-05-14 23:08 ` [PATCH 00/35] refactoring refspecs Stefan Beller
2018-05-15  8:05 ` Junio C Hamano
2018-05-15  8:39 ` Ævar Arnfjörð Bjarmason
2018-05-15 18:01   ` Brandon Williams
2018-05-16 22:57 ` [PATCH v2 00/36] " Brandon Williams
2018-05-16 22:57   ` [PATCH v2 01/36] refspec: move refspec parsing logic into its own file Brandon Williams
2018-05-16 22:57   ` [PATCH v2 02/36] refspec: rename struct refspec to struct refspec_item Brandon Williams
2018-05-16 22:57   ` [PATCH v2 03/36] refspec: factor out parsing a single refspec Brandon Williams
2018-05-16 22:57   ` [PATCH v2 04/36] refspec: introduce struct refspec Brandon Williams
2018-05-16 22:57   ` [PATCH v2 05/36] refspec: convert valid_fetch_refspec to use parse_refspec Brandon Williams
2018-06-03 17:13     ` Martin Ågren
2018-06-04 14:43       ` [PATCH] refspec: initalize `refspec_item` in `valid_fetch_refspec()` Martin Ågren
2018-06-04 17:36         ` Brandon Williams
2018-06-04 21:55         ` Ævar Arnfjörð Bjarmason
2018-06-05  5:10           ` Martin Ågren
2018-06-05 16:29           ` Brandon Williams
2018-06-05 19:54             ` [PATCH 0/3] refspec: refactor & fix free() behavior Ævar Arnfjörð Bjarmason
2018-06-05 19:58               ` Brandon Williams
2018-06-05 20:20                 ` Martin Ågren
2018-06-05 19:54             ` [PATCH 1/3] refspec: s/refspec_item_init/&_or_die/g Ævar Arnfjörð Bjarmason
2018-06-05 19:54             ` [PATCH 2/3] refspec: add back a refspec_item_init() function Ævar Arnfjörð Bjarmason
2018-06-05 19:54             ` [PATCH 3/3] refspec: initalize `refspec_item` in `valid_fetch_refspec()` Ævar Arnfjörð Bjarmason
2018-05-16 22:57   ` [PATCH v2 06/36] submodule--helper: convert push_check to use struct refspec Brandon Williams
2018-05-16 22:57   ` [PATCH v2 07/36] pull: convert get_tracking_branch to use refspec_item_init Brandon Williams
2018-05-16 22:57   ` [PATCH v2 08/36] transport: convert transport_push to use struct refspec Brandon Williams
2018-05-16 22:57   ` [PATCH v2 09/36] remote: convert check_push_refs " Brandon Williams
2018-05-16 22:57   ` [PATCH v2 10/36] remote: convert match_push_refs " Brandon Williams
2018-05-16 22:57   ` [PATCH v2 11/36] clone: convert cmd_clone to use refspec_item_init Brandon Williams
2018-05-16 22:57   ` [PATCH v2 12/36] fast-export: convert to use struct refspec Brandon Williams
2018-05-16 22:58   ` [PATCH v2 13/36] remote: convert push refspecs to " Brandon Williams
2018-05-16 22:58   ` [PATCH v2 14/36] remote: convert fetch " Brandon Williams
2018-05-16 22:58   ` [PATCH v2 15/36] remote: remove add_prune_tags_to_fetch_refspec Brandon Williams
2018-05-16 22:58   ` [PATCH v2 16/36] transport-helper: convert to use struct refspec Brandon Williams
2018-05-16 22:58   ` [PATCH v2 17/36] fetch: convert fetch_one " Brandon Williams
2018-05-16 22:58   ` [PATCH v2 18/36] fetch: convert refmap " Brandon Williams
2018-05-16 22:58   ` [PATCH v2 19/36] refspec: remove the deprecated functions Brandon Williams
2018-05-16 22:58   ` [PATCH v2 20/36] fetch: convert do_fetch to take a struct refspec Brandon Williams
2018-05-16 22:58   ` [PATCH v2 21/36] fetch: convert get_ref_map " Brandon Williams
2018-05-16 22:58   ` [PATCH v2 22/36] fetch: convert prune_refs " Brandon Williams
2018-05-16 22:58   ` [PATCH v2 23/36] remote: convert get_stale_heads " Brandon Williams
2018-05-16 22:58   ` [PATCH v2 24/36] remote: convert apply_refspecs " Brandon Williams
2018-05-16 22:58   ` [PATCH v2 25/36] remote: convert query_refspecs " Brandon Williams
2018-05-16 22:58   ` [PATCH v2 26/36] remote: convert get_ref_match " Brandon Williams
2018-05-16 22:58   ` [PATCH v2 27/36] remote: convert match_explicit_refs " Brandon Williams
2018-05-16 22:58   ` [PATCH v2 28/36] push: check for errors earlier Brandon Williams
2018-05-16 22:58   ` [PATCH v2 29/36] push: convert to use struct refspec Brandon Williams
2018-05-16 22:58   ` [PATCH v2 30/36] transport: convert transport_push to take a " Brandon Williams
2018-05-16 22:58   ` [PATCH v2 31/36] send-pack: store refspecs in " Brandon Williams
2018-05-16 22:58   ` [PATCH v2 32/36] transport: remove transport_verify_remote_names Brandon Williams
2018-05-16 22:58   ` [PATCH v2 33/36] http-push: store refspecs in a struct refspec Brandon Williams
2018-05-16 22:58   ` [PATCH v2 34/36] remote: convert match_push_refs to take " Brandon Williams
2018-05-16 22:58   ` [PATCH v2 35/36] remote: convert check_push_refs " Brandon Williams
2018-05-16 22:58   ` [PATCH v2 36/36] submodule: convert push_unpushed_submodules " Brandon Williams
2018-05-16 23:48   ` [PATCH 0/2] generating ref-prefixes for configured refspecs Brandon Williams
2018-05-16 23:48     ` [PATCH 1/2] refspec: consolidate ref-prefix generation logic Brandon Williams
2018-05-31  0:43       ` Jonathan Nieder
2018-05-31  1:07         ` Jonathan Nieder [this message]
2018-05-31  7:23       ` [PATCH] fetch: do not pass ref-prefixes for fetch by exact SHA1 Jonathan Nieder
2018-05-31 15:44         ` Brandon Williams
2018-06-01  2:12         ` Junio C Hamano
2018-06-01  2:49           ` Jonathan Nieder
2018-05-16 23:48     ` [PATCH 2/2] fetch: generate ref-prefixes when using a configured refspec Brandon Williams
2018-05-17 21:32     ` [PATCH 0/2] generating ref-prefixes for configured refspecs Junio C Hamano

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180531010739.GB36515@aiede.svl.corp.google.com \
    --to=jrnieder@gmail.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox