[PATCH v2 3/8] upload-pack: test negotiation with changing repository

(which will be introduced to the client in subsequent commits), and ensures that subsequent commits do not change existing behavior. As part of this effort, a mechanism to substitute strings in a single HTTP response is added. Signed-off-by: Brandon Williams --- t/lib-httpd.sh

[PATCH v2 8/8] fetch-pack: implement ref-in-want

Implement ref-in-want on the client side so that when a server supports the "ref-in-want" feature, a client will send "want-ref" lines for each reference the client wants to fetch. Signed-off-by: Brandon Williams --- fetch-pack.c

[PATCH v2 1/8] test-pkt-line: add unpack-sideband subcommand

Add an 'unpack-sideband' subcommand to the test-pkt-line helper to enable unpacking packet line data sent multiplexed using a sideband. Signed-off-by: Brandon Williams --- t/helper/test-pkt-line.c | 37 + 1 file changed, 37 insertions(+) diff

[PATCH v2 4/8] fetch: refactor the population of peer ref OIDs

Populate peer ref OIDs in get_ref_map instead of do_fetch. Besides tightening scopes of variables in the code, this also prepares for get_ref_map being able to be called multiple times within do_fetch. Signed-off-by: Brandon Williams --- builtin/fetch.c | 36

[PATCH v2 6/8] fetch: refactor to make function args narrower

struct. Signed-off-by: Brandon Williams --- builtin/fetch.c | 52 - 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index ee8b87c78..b600e1f10 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c

Re: [PATCH v2 1/8] fetch-pack: split up everything_local()

gt; for_each_cached_alternate(insert_one_alternate_object); > > /* Filter 'ref' by 'sought' and those that aren't local > */ > - if (everything_local(args, &ref, sought, nr_sought)) > + mark_complete_and_common_ref(args, &ref); > + filter_refs(args, &ref, sought, nr_sought); > + if (everything_local(args, &ref)) > state = FETCH_DONE; > else > state = FETCH_SEND_REQUEST; > -- > 2.17.0.768.g1526ddbba1.dirty > -- Brandon Williams

Re: [PATCH v2 3/8] fetch-pack: directly end negotiation if ACK ready

} > } > done: > @@ -1300,7 +1300,6 @@ static int process_acks(struct packet_reader *reader, > struct oidset *common) > } > > if (!strcmp(reader->line, "ready")) { > - clear_prio_queue(&rev_list); > received_ready = 1; > continue; > } > -- > 2.17.0.768.g1526ddbba1.dirty > -- Brandon Williams

Re: [PATCH v2 4/8] fetch-pack: use ref adv. to prune "have" sent

col v0, then everything_local() would enqueue it with > COMMON_REF | SEEN. The addition of COMMON_REF ensures that its parents > are not sent as "have" lines. > > Change the order in do_fetch_pack_v2() to be consistent with > do_fetch_pack(), and to avoid sending unnecessary "have" lines. > > Signed-off-by: Jonathan Tan > --- -- Brandon Williams

Re: [PATCH v2 3/8] fetch-pack: directly end negotiation if ACK ready

On 06/14, Brandon Williams wrote: > On 06/06, Jonathan Tan wrote: > > When "ACK %s ready" is received, find_common() clears rev_list in an > > attempt to stop further "have" lines from being sent [1]. It is much > > more readable to explicitly break from th

Re: [PATCH v2 5/8] fetch-pack: make negotiation-related vars local

, > marked = 1; > > /* Filter 'ref' by 'sought' and those that aren't local > */ > - mark_complete_and_common_ref(args, &ref); > + mark_complete_and_common_ref(&data, args, &ref); > filter_refs(args, &ref, sought, nr_sought); > if (everything_local(args, &ref)) > state = FETCH_DONE; > else > state = FETCH_SEND_REQUEST; > > - for_each_ref(rev_list_insert_ref_oid, NULL); > - for_each_cached_alternate(insert_one_alternate_object); > + for_each_ref(rev_list_insert_ref_oid, &data); > + for_each_cached_alternate(&data, > + insert_one_alternate_object); > break; > case FETCH_SEND_REQUEST: > - if (send_fetch_request(fd[1], args, ref, &common, > + if (send_fetch_request(&data, fd[1], args, ref, &common, > &haves_to_send, &in_vain)) > state = FETCH_GET_PACK; > else > @@ -1411,7 +1425,7 @@ static struct ref *do_fetch_pack_v2(struct > fetch_pack_args *args, > break; > case FETCH_PROCESS_ACKS: > /* Process ACKs/NAKs */ > - switch (process_acks(&reader, &common)) { > + switch (process_acks(&data, &reader, &common)) { > case 2: > state = FETCH_GET_PACK; > break; > -- > 2.17.0.768.g1526ddbba1.dirty > -- Brandon Williams

Re: [PATCH v2 8/8] negotiator/default: use better style in comments

he next rev to send, ignoring the common. > -*/ > - > + * Get the next rev to send, ignoring the common. > + */ > static const struct object_id *get_rev(struct data *data) > { > struct commit *commit = NULL; > -- > 2.17.0.768.g1526ddbba1.dirty > Don't have this be a separate patch, squash it into the previous patch. -- Brandon Williams

Re: [PATCH v2 00/31] object-store: lookup_commit

a look at the series and it looks good. I'm glad we're getting closer to this set of series being completed. Thanks for pushing this through :) -- Brandon Williams

Re: [PATCH v2 2/8] upload-pack: implement ref-in-want

On 06/14, Stefan Beller wrote: > Hi Brandon, > On Wed, Jun 13, 2018 at 2:39 PM Brandon Williams wrote: > > negotiation, which may happen if, for example, the desired repository is > > provided by multiple Git servers in a load-balancing arrangement. > > ... and the repo

Re: [PATCH v2 1/8] test-pkt-line: add unpack-sideband subcommand

On 06/14, Stefan Beller wrote: > On Wed, Jun 13, 2018 at 2:39 PM Brandon Williams wrote: > > > > Add an 'unpack-sideband' subcommand to the test-pkt-line helper to > > enable unpacking packet line data sent multiplexed using a sideband. > > > > Sign

Re: [PATCH v2 8/8] fetch-pack: implement ref-in-want

On 06/14, Stefan Beller wrote: > On Wed, Jun 13, 2018 at 2:39 PM Brandon Williams wrote: > > > +static void receive_wanted_refs(struct packet_reader *reader, struct ref > > *refs) > > +{ > ... > > + > > + for (r = refs; r; r = r->next) { &

Re: [PATCH 10/35] commit: add repository argument to lookup_commit

y passing > > 'struct index_state *' throughout library code, and sometimes I pass > > 'struct repository *' instead when I see that code uses more things > > that just the index. And I have started to replace the_repository in > > some places with a function argument. > > > > If some of my patches come first while you have not finished > > repository conversion (very likely), you and I will have to pay > > attention to this more often. -- Brandon Williams

Re: [PATCH v2 0/8] ref-in-want

On 06/15, Junio C Hamano wrote: > Brandon Williams writes: > > > Changes in v2: > > * issuing a want-ref line to a ref which doesn't exist is just ignored. > > * fixed some typos > > Do I lock some (logical) prerequisite changes? On 2.18-rc2 they > appl

Re: Adding nested repository with slash adds files instead of gitlink

.c, which has > been replaced with something else in bw/pathspec-sans-the-index. If > you have time, try a version without those changes (e.g. v2.13 or > before) to see if it's a possible culprit. I just tested this with v2.13 and saw the same issue. I don't actually think this ever worked in the way you want it to Heiko. Maybe git add needs to be taught to be more intelligent when trying to add a submodule which doesn't exist in the index. -- Brandon Williams

Re: [PATCH 00/15] Kill the_index part 1, expose it

uess I've gotten pretty good at getting people to go down rabbit holes. First Stefan with the object store refactoring and now you with the index stuff. All because I wanted git to be more object oriented and have less global state ;) -- Brandon Williams

Re: [PATCH 15/15] cache.h: flip NO_THE_INDEX_COMPATIBILITY_MACROS switch

c b/pathspec.c > index 27cd606786..6997707477 100644 > --- a/pathspec.c > +++ b/pathspec.c > @@ -1,4 +1,3 @@ > -#define NO_THE_INDEX_COMPATIBILITY_MACROS > #include "cache.h" > #include "config.h" > #include "dir.h" > diff --git a/read-cache.c b/read-cache.c > index 372588260e..2a84ad0797 100644 > --- a/read-cache.c > +++ b/read-cache.c > @@ -3,7 +3,6 @@ > * > * Copyright (C) Linus Torvalds, 2005 > */ > -#define NO_THE_INDEX_COMPATIBILITY_MACROS > #include "cache.h" > #include "config.h" > #include "tempfile.h" > diff --git a/submodule.c b/submodule.c > index 939d6870ec..c6ae29379d 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -1,4 +1,3 @@ > -#define NO_THE_INDEX_COMPATIBILITY_MACROS > > #include "cache.h" > #include "repository.h" > diff --git a/t/helper/test-dump-untracked-cache.c > b/t/helper/test-dump-untracked-cache.c > index bd92fb305a..56a5ce8abb 100644 > --- a/t/helper/test-dump-untracked-cache.c > +++ b/t/helper/test-dump-untracked-cache.c > @@ -1,3 +1,4 @@ > +#define USE_THE_INDEX_COMPATIBILITY_MACROS > #include "cache.h" > #include "dir.h" > > diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h > index 7116ddfb94..a7ff69e9f3 100644 > --- a/t/helper/test-tool.h > +++ b/t/helper/test-tool.h > @@ -1,6 +1,8 @@ > #ifndef __TEST_TOOL_H__ > #define __TEST_TOOL_H__ > > +#define USE_THE_INDEX_COMPATIBILITY_MACROS > + > int cmd__chmtime(int argc, const char **argv); > int cmd__config(int argc, const char **argv); > int cmd__ctype(int argc, const char **argv); > diff --git a/tree.c b/tree.c > index 244eb5e665..b5ed7bc22b 100644 > --- a/tree.c > +++ b/tree.c > @@ -1,4 +1,3 @@ > -#define NO_THE_INDEX_COMPATIBILITY_MACROS > #include "cache.h" > #include "cache-tree.h" > #include "tree.h" > diff --git a/unpack-trees.c b/unpack-trees.c > index 3a85a02a77..fd09e812a2 100644 > --- a/unpack-trees.c > +++ b/unpack-trees.c > @@ -1,4 +1,3 @@ > -#define NO_THE_INDEX_COMPATIBILITY_MACROS > #include "cache.h" > #include "argv-array.h" > #include "repository.h" > -- > 2.18.0.rc0.333.g22e6ee6cdf > -- Brandon Williams

Re: [PATCH v2 2/2] fetch: send "refs/tags/" prefix upon CLI refspecs

> > > How is the tag following documented (i.e. when is the user least > > surprised that we do not tag-follow all and unconditionally)? > > It's documented under the --no-tags option in the man page for git > fetch. I'm not sure what you mean by the user being surprised. -- Brandon Williams

Re: [PATCH 0/8] ref-in-want

in the past with the shallow features and later capabilities were added to add different ways to request shallow fetches. That being said, if we find that this feature doesn't work as-is and needs the extra complexity of patterns from the start then they should be added. But it doesn't seem like there's a concrete reason at the moment. -- Brandon Williams

Re: [PATCH v2 7/8] fetch-pack: put shallow info in output parameter

for (r = refs; r; r = r->next, i++) > > if (status[i]) > > - sought[i]->status = REF_STATUS_REJECT_SHALLOW; > > + r->status = REF_STATUS_REJECT_SHALLOW; > > You use i here without initializing it to 0. t5703 also fails with this > patch - probably related to this, but I didn't check. Oh yeah that's definitely a bug, thanks for catching that. > > If you initialize i here, I don't think you need to initialize it to 0 > at the top of this function. -- Brandon Williams

Re: [PATCH v2 2/8] upload-pack: implement ref-in-want

equest "refs/tags/*" is too broad of a request and could be requesting 100s of tags when all we really wanted was to get the one or two new tags which are present on the remote (because we already have all the other tags present locally). So I think the best way is to limit these patterns to the ls-refs request where we can then discover the few tags we're missing and then request just those tags explicitly, keeping the resulting packfile smaller. Thoughts? -- Brandon Williams

Re: [PATCH v2 2/8] upload-pack: implement ref-in-want

On 06/19, Junio C Hamano wrote: > Brandon Williams writes: > > > I also think that we should keep this first implementation of > > ref-in-want simple and *not* include patterns, even if that's what we > > may want someday down the road. Adding a new capability in

Re: [PATCH 0/8] ref-in-want

hat v1 of the series did and have it be a hard error? I've gone back and forth on what I think we should do so I'd like to hear at least one more opinion :) -- Brandon Williams

[PATCH v3 3/8] upload-pack: test negotiation with changing repository

(which will be introduced to the client in subsequent commits), and ensures that subsequent commits do not change existing behavior. As part of this effort, a mechanism to substitute strings in a single HTTP response is added. Signed-off-by: Brandon Williams --- t/lib-httpd.sh

[PATCH v3 2/8] upload-pack: implement ref-in-want

he form of ref names through a new "want-ref " parameter. At the conclusion of negotiation, the server will send a list of all of the wanted references (as provided by "want-ref" lines) in addition to the generated packfile. Signed-off-by: Brandon Williams --- Docum

[PATCH v3 4/8] fetch: refactor the population of peer ref OIDs

Populate peer ref OIDs in get_ref_map instead of do_fetch. Besides tightening scopes of variables in the code, this also prepares for get_ref_map being able to be called multiple times within do_fetch. Signed-off-by: Brandon Williams --- builtin/fetch.c | 36

[PATCH v3 1/8] test-pkt-line: add unpack-sideband subcommand

Add an 'unpack-sideband' subcommand to the test-pkt-line helper to enable unpacking packet line data sent multiplexed using a sideband. Signed-off-by: Brandon Williams --- t/helper/test-pkt-line.c | 33 + 1 file changed, 33 insertions(+) diff --git

[PATCH v3 5/8] fetch: refactor fetch_refs into two functions

Refactor the fetch_refs function into a function that does the fetching of refs and another function that stores them. Signed-off-by: Brandon Williams --- builtin/fetch.c | 19 +-- 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c

[PATCH v3 6/8] fetch: refactor to make function args narrower

struct. Signed-off-by: Brandon Williams --- builtin/fetch.c | 52 - 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index ee8b87c78..b600e1f10 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c

[PATCH v3 7/8] fetch-pack: put shallow info in output parameter

sometimes generate the ref map twice: once from the list of refs provided by the remote (as is currently done) and potentially once from the new list of refs that the fetch mechanism provides. Signed-off-by: Brandon Williams --- builtin/clone.c | 4 ++-- builtin/fetch.c | 28

[PATCH v3 0/8] ref-in-want

ists is a hard error on the server. I went back and forth many times on what the right thing to do here is and decided that a hard error works much cleaner for the time being. * Some typos. Brandon Williams (8): test-pkt-line: add unpack-sideband subcommand upload-pack: implement ref-i

[PATCH v3 8/8] fetch-pack: implement ref-in-want

Implement ref-in-want on the client side so that when a server supports the "ref-in-want" feature, a client will send "want-ref" lines for each reference the client wants to fetch. Signed-off-by: Brandon Williams --- fetch-pack.c

[PATCH v2] docs: link to gitsubmodules

Add a link to gitsubmodules(7) under the `submodule.active` entry in git-config(1). Signed-off-by: Brandon Williams --- Documentation/config.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index ab641bf5a

Re: [PATCH 2/7] submodule-config: add helper function to get 'fetch' config from .gitmodules

= > parse_fetch_recurse_submodules_arg(var, value); There shouldn't be a space before "->" in this line. -- Brandon Williams

Re: [PATCH 3/7] submodule-config: add helper to get 'update-clone' config from .gitmodules

27;config_from_gitmodules' for > options not strictly related to submodules: "submodule.fetchobjs" does s/fetchobjs/fetchjobs -- Brandon Williams

Re: [PATCH 6/7] submodule-config: reuse config_from_gitmodules in repo_read_gitmodules

red > in > - * the repository. > - * > - * Runs the provided config function on the '.gitmodules' file found in the > - * working directory. > - */ > -static void config_from_gitmodules(config_fn_t fn, struct repository *repo, > void *data) > -{ > - if (repo->worktree) { > - char *file = repo_worktree_path(repo, GITMODULES_FILE); > - git_config_from_file(fn, file, data); > - free(file); > - } > -} > - > struct fetch_config { > int *max_children; > int *recurse_submodules; > -- > 2.18.0 > -- Brandon Williams

Re: [PATCH 7/7] submodule-config: cleanup backward compatibility helpers

_clone_config config = { + .fetchjobs = max_jobs, + .recurse_submodules = NULL, + }; + config_from_gitmodules(gitmodules_fetch_clone_config, the_repository, + &config); } -- Brandon Williams

Re: [PATCH 0/7] Restrict the usage of config_from_gitmodules()

series it looks like you should be in a good place to read the .gitmodules file from other places (not just in the worktree). As you've mentioned here I also agree we could do without the last patch but I'll leave that up to you. Other than a couple typos I found I think this series looks good! Thanks again for revisiting this. -- Brandon Williams

Re: [PATCH 0/7] Restrict the usage of config_from_gitmodules()

On 06/22, Antonio Ospite wrote: > On Fri, 22 Jun 2018 10:13:10 -0700 > Brandon Williams wrote: > > [...] > > Thanks for working on this. I think its a good approach and the end > > result makes it much harder for arbitrary config to sneak back in to the > > .gi

Re: [PATCH v3 8/8] fetch-pack: implement ref-in-want

On 06/22, Jonathan Nieder wrote: > Hi, > > Brandon Williams wrote: > > > Implement ref-in-want on the client side so that when a server supports > > the "ref-in-want" feature, a client will send "want-ref" lines for each > > reference the cli

Re: [PATCH v3 7/8] fetch-pack: put shallow info in output parameter

ds); > > + rc = transport->vtable->fetch(transport, nr_heads, heads, fetched_refs); > > + if (fetched_refs && nop_head) { > > + *nop_tail = *fetched_refs; > > + *fetched_refs = nop_head; > > + } > > > > free(heads); > > return rc; > > And sometimes, even if we are merely simulating the fetching of refs, we > still need to report those refs in fetched_refs. This is correct. > > I also see that t5703 now passes. > > Besides enabling the writing of subsequent patches, I see that this also > makes the API clearer in that the input refs to transport_fetch_refs() > are not overloaded to output shallow information. Other than the " = 0" > change above, this patch looks good to me. Perfect, I'll just drop the " = 0" part (making the diff slightly smaller) -- Brandon Williams

Re: [PATCH v3 2/8] upload-pack: implement ref-in-want

oid; > > + struct string_list_item *item; > > + struct object *o; > > + > > + if (read_ref(arg, &oid)) > > + die("unknown ref %s", arg); > > One more thing - if you're planning to "die" here, als

Re: [PATCH 0/2] Object store refactoring: make bitmap_git not global

As a bonus, the API is also improved, > and global state reduced. I've reviewed this series and haven't found any issues. Reviewed-by: Brandon Williams -- Brandon Williams

Re: [PATCH v3 0/7] Refactor fetch negotiation into its own API

tiation_state") >for the struct, instead of "struct data" > - squashed patch 8 into patch 7; this means that the comments are not >moved verbatim, but for the reviewer, verbatim-ness of comments is >probably not that important anyway Thanks this series looks good now. Reviewed-by: Brandon Williams -- Brandon Williams

[PATCH v4 0/8] ref-in-want

Changes in v4 are fairly minor. There are a few documentation changes, commit message updates, as well as a few small style tweaks based on reviewer comments. Brandon Williams (8): test-pkt-line: add unpack-sideband subcommand upload-pack: implement ref-in-want upload-pack: test

[PATCH v4 1/8] test-pkt-line: add unpack-sideband subcommand

Add an 'unpack-sideband' subcommand to the test-pkt-line helper to enable unpacking packet line data sent multiplexed using a sideband. Signed-off-by: Brandon Williams --- t/helper/test-pkt-line.c | 33 + 1 file changed, 33 insertions(+) diff --git

[PATCH v4 2/8] upload-pack: implement ref-in-want

he form of ref names through a new "want-ref " parameter. At the conclusion of negotiation, the server will send a list of all of the wanted references (as provided by "want-ref" lines) in addition to the generated packfile. Signed-off-by: Brandon Williams --- Docum

[PATCH v4 6/8] fetch: refactor to make function args narrower

struct. Signed-off-by: Brandon Williams --- builtin/fetch.c | 52 - 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 2fabfed0e..bda00e826 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c

[PATCH v4 4/8] fetch: refactor the population of peer ref OIDs

Populate peer ref OIDs in get_ref_map instead of do_fetch. Besides tightening scopes of variables in the code, this also prepares for get_ref_map being able to be called multiple times within do_fetch. Signed-off-by: Brandon Williams --- builtin/fetch.c | 36

[PATCH v4 5/8] fetch: refactor fetch_refs into two functions

Refactor the fetch_refs function into a function that does the fetching of refs and another function that stores them. This is in preparation for allowing additional processing of the fetched refs before updating the local ref store. Signed-off-by: Brandon Williams --- builtin/fetch.c | 23

[PATCH v4 7/8] fetch-pack: put shallow info in output parameter

sometimes generate the ref map twice: once from the list of refs provided by the remote (as is currently done) and potentially once from the new list of refs that the fetch mechanism provides. Signed-off-by: Brandon Williams --- builtin/clone.c | 4 ++-- builtin/fetch.c | 28

[PATCH v4 8/8] fetch-pack: implement ref-in-want

and the value of that ref will be sent at the termination of negotiation, just before a packfile is sent. More information on the ref-in-want feature can be found in Documentation/technical/protocol-v2.txt. Signed-off-by: Brandon Williams --- fetch-pack.c | 35 +++

[PATCH v4 3/8] upload-pack: test negotiation with changing repository

(which will be introduced to the client in subsequent commits), and ensures that subsequent commits do not change existing behavior. As part of this effort, a mechanism to substitute strings in a single HTTP response is added. Signed-off-by: Brandon Williams --- t/lib-httpd.sh

Re: [PATCH v2 0/6] Restrict the usage of config_from_gitmodules to submodule-config

ace before an arrow operator in patch 2 > * Fix a typo in the commit message of patch 3: s/fetchobjs/fetchjobs > * Add a note in the commit message of patch 6 about checking the > worktree before loading .gitmodules > * Drop patch 7, it was meant as a cleanup but resulted in parsing the > .gitmodules file twice Thanks for making these changes, this version looks good to me! -- Brandon Williams

[PATCH v5 1/8] test-pkt-line: add unpack-sideband subcommand

Add an 'unpack-sideband' subcommand to the test-pkt-line helper to enable unpacking packet line data sent multiplexed using a sideband. Signed-off-by: Brandon Williams --- t/helper/test-pkt-line.c | 33 + 1 file changed, 33 insertions(+) diff --git

[PATCH v5 3/8] upload-pack: test negotiation with changing repository

(which will be introduced to the client in subsequent commits), and ensures that subsequent commits do not change existing behavior. As part of this effort, a mechanism to substitute strings in a single HTTP response is added. Signed-off-by: Brandon Williams --- t/lib-httpd.sh

[PATCH v5 2/8] upload-pack: implement ref-in-want

he form of ref names through a new "want-ref " parameter. At the conclusion of negotiation, the server will send a list of all of the wanted references (as provided by "want-ref" lines) in addition to the generated packfile. Signed-off-by: Brandon Williams --- Docum

[PATCH v5 4/8] fetch: refactor the population of peer ref OIDs

Populate peer ref OIDs in get_ref_map instead of do_fetch. Besides tightening scopes of variables in the code, this also prepares for get_ref_map being able to be called multiple times within do_fetch. Signed-off-by: Brandon Williams --- builtin/fetch.c | 36

[PATCH v5 5/8] fetch: refactor fetch_refs into two functions

Refactor the fetch_refs function into a function that does the fetching of refs and another function that stores them. This is in preparation for allowing additional processing of the fetched refs before updating the local ref store. Signed-off-by: Brandon Williams --- builtin/fetch.c | 23

[PATCH v5 6/8] fetch: refactor to make function args narrower

struct. Signed-off-by: Brandon Williams --- builtin/fetch.c | 52 - 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 2fabfed0e..bda00e826 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c

[PATCH v5 7/8] fetch-pack: put shallow info in output parameter

sometimes generate the ref map twice: once from the list of refs provided by the remote (as is currently done) and potentially once from the new list of refs that the fetch mechanism provides. Signed-off-by: Brandon Williams --- builtin/clone.c | 4 ++-- builtin/fetch.c | 28

[PATCH v5 8/8] fetch-pack: implement ref-in-want

and the value of that ref will be sent at the termination of negotiation, just before a packfile is sent. More information on the ref-in-want feature can be found in Documentation/technical/protocol-v2.txt. Signed-off-by: Brandon Williams --- fetch-pack.c

[PATCH v5 0/8] ref-in-want

Changes in v5: * Added a comment explaining the one-time-sed.sh script * Added a number of tests per reviewer feedback * Fixed a typo in documentation Brandon Williams (8): test-pkt-line: add unpack-sideband subcommand upload-pack: implement ref-in-want upload-pack: test negotiation with

Re: [PATCH v5 2/8] upload-pack: implement ref-in-want

On 06/26, Junio C Hamano wrote: > Brandon Williams writes: > > > +wanted-refs section > > + * This section is only included if the client has requested a > > + ref using a 'want-ref' line and if a packfile section is also > > + included

Re: [PATCH v5 3/8] upload-pack: test negotiation with changing repository

On 06/26, Junio C Hamano wrote: > Brandon Williams writes: > > > diff --git a/t/lib-httpd/one-time-sed.sh b/t/lib-httpd/one-time-sed.sh > > new file mode 100644 > > index 0..8a9a5aca0 > > --- /dev/null > > +++ b/t/lib-httpd/one-time-sed.sh

Re: [PATCH v5 7/8] fetch-pack: put shallow info in output parameter

On 06/26, Junio C Hamano wrote: > Brandon Williams writes: > > > Expand the transport fetch method signature, by adding an output > > parameter, to allow transports to return information about the refs they > > have fetched. Then communicate shallow status information th

Re: [PATCH v5 8/8] fetch-pack: implement ref-in-want

no want-ref line is going to be sent because we're requesting an exact OID here, not a ref. But I can add checks for want-ref in the tests that should be sending want-ref. > > Other than that (and my other comments), this patch series looks good. -- Brandon Williams

Re: [PATCH v5 2/8] upload-pack: implement ref-in-want

On 06/27, Junio C Hamano wrote: > Brandon Williams writes: > > >> > +* The server SHOULD NOT send any refs which were not requested > >> > + using 'want-ref' lines and a client MUST ignore refs which > >> > + weren

[PATCH v6 0/8] ref-in-want

-ref section. Also changed the docs around the server's responsibility with the refs that are sent during this section. Brandon Williams (8): test-pkt-line: add unpack-sideband subcommand upload-pack: implement ref-in-want upload-pack: test negotiation with changing repository

[PATCH v6 5/8] fetch: refactor fetch_refs into two functions

Refactor the fetch_refs function into a function that does the fetching of refs and another function that stores them. This is in preparation for allowing additional processing of the fetched refs before updating the local ref store. Signed-off-by: Brandon Williams --- builtin/fetch.c | 23

[PATCH v6 2/8] upload-pack: implement ref-in-want

he form of ref names through a new "want-ref " parameter. At the conclusion of negotiation, the server will send a list of all of the wanted references (as provided by "want-ref" lines) in addition to the generated packfile. Signed-off-by: Brandon Williams --- Docum

[PATCH v6 3/8] upload-pack: test negotiation with changing repository

(which will be introduced to the client in subsequent commits), and ensures that subsequent commits do not change existing behavior. As part of this effort, a mechanism to substitute strings in a single HTTP response is added. Signed-off-by: Brandon Williams --- t/lib-httpd.sh

[PATCH v6 7/8] fetch-pack: put shallow info in output parameter

sometimes generate the ref map twice: once from the list of refs provided by the remote (as is currently done) and potentially once from the new list of refs that the fetch mechanism provides. Signed-off-by: Brandon Williams --- builtin/clone.c | 4 ++-- builtin/fetch.c | 28

[PATCH v6 4/8] fetch: refactor the population of peer ref OIDs

Populate peer ref OIDs in get_ref_map instead of do_fetch. Besides tightening scopes of variables in the code, this also prepares for get_ref_map being able to be called multiple times within do_fetch. Signed-off-by: Brandon Williams --- builtin/fetch.c | 36

[PATCH v6 6/8] fetch: refactor to make function args narrower

struct. Signed-off-by: Brandon Williams --- builtin/fetch.c | 52 - 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 2fabfed0e..bda00e826 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c

[PATCH v6 1/8] test-pkt-line: add unpack-sideband subcommand

Add an 'unpack-sideband' subcommand to the test-pkt-line helper to enable unpacking packet line data sent multiplexed using a sideband. Signed-off-by: Brandon Williams --- t/helper/test-pkt-line.c | 33 + 1 file changed, 33 insertions(+) diff --git

[PATCH v6 8/8] fetch-pack: implement ref-in-want

and the value of that ref will be sent at the termination of negotiation, just before a packfile is sent. More information on the ref-in-want feature can be found in Documentation/technical/protocol-v2.txt. Signed-off-by: Brandon Williams --- fetch-pack.c

Re: [PATCH v2] fetch-pack: support negotiation tip whitelist

@@ static int fetch(struct transport *transport, > transport, "filter", > data->transport_options.filter_options.filter_spec); > > + if (data->transport_options.negotiation_tips) > + warning("Ignoring --negotiation-tip because the protocol does > not support it."); > + > if (data->fetch) > return fetch_with_fetch(transport, nr_heads, to_fetch); > > diff --git a/transport.c b/transport.c > index a32da30dee..9f10f8ad9f 100644 > --- a/transport.c > +++ b/transport.c > @@ -318,6 +318,7 @@ static int fetch_refs_via_pack(struct transport > *transport, > args.filter_options = data->options.filter_options; > args.stateless_rpc = transport->stateless_rpc; > args.server_options = transport->server_options; > + args.negotiation_tips = data->options.negotiation_tips; > > if (!data->got_remote_heads) > refs_tmp = get_refs_via_connect(transport, 0, NULL); > diff --git a/transport.h b/transport.h > index 7792b08582..d31be5be63 100644 > --- a/transport.h > +++ b/transport.h > @@ -25,6 +25,16 @@ struct git_transport_options { > const char *receivepack; > struct push_cas_option *cas; > struct list_objects_filter_options filter_options; > + > + /* > + * This is only used during fetch. See the documentation of > + * negotiation_tips in struct fetch_pack_args. > + * > + * This field is only supported by transports that support connect or > + * stateless_connect. Set this field directly instead of using > + * transport_set_option(). > + */ > + struct oid_array *negotiation_tips; > }; > > enum transport_family { > -- > 2.18.0.rc2.346.g013aa6912e-goog > -- Brandon Williams

Re: [PATCH v2] fetch-pack: support negotiation tip whitelist

fetch is a perfect example of supporting all three. I can do git fetch origin SHA1 git fetch origin master git fetch origin refs/heads/*:refs/heads/* -- Brandon Williams

Re: [PATCH v3] fetch-pack: support negotiation tip whitelist

fetched. > ++ > +This option may be specified more than once; if so, Git will report > +commits reachable from any of the given commits. > ++ > +The argument to this option may be a glob on ref names, a ref, or the > (possibly > +abbreviated SHA-1 of a commit. Specifying a glob is equivalent to specifying > +this option multiple times, one for each matching ref name. I think you're missing a closing ')' Aside from that nit this patch looks good, thanks! -- Brandon Williams

Re: [PATCH v3 00/32] object-store: lookup_commit

uch easier to handle errors. This work needs to continue if we want to improve the submodule experience in git. Yes this might mean there are a few more conflicts when merging a series (because of the scope of these refactorings) but it is well worth it given what the end-state will look like. -- Brandon Williams

Re: [PATCH 00/12] Kill the_index part2, header file cleanup

nned through the series and it looks good. Again, thanks for putting in the work to get this done. I'm looking forward to the end product :) -- Brandon Williams

Re: [PATCH v4 6/9] diff.c: add a blocks mode for moved code detection

ernativeDimmed "normal blue" && > test_config color.diff.newMovedAlternativeDimmed "normal yellow" && > + git diff HEAD --no-renames --color-moved=blocks --color >actual.raw && > + grep -v "index" actual.raw | test_decode_color >actual && > + cat <<-\EOF >expected && > + diff --git a/lines.txt b/lines.txt > + --- a/lines.txt > + +++ b/lines.txt > + @@ -1,16 +1,16 @@ > + -long line 1 > + -long line 2 > + -long line 3 > + line 4 > + line 5 > + line 6 > + line 7 > + line 8 > + line 9 > + +long line 1 > + +long line 2 > + +long line 3 > + +long line 14 > + +long line 15 > + +long line 16 > + line 10 > + line 11 > + line 12 > + line 13 > + -long line 14 > + -long line 15 > + -long line 16 > + EOF > + test_cmp expected actual > + Theres an empty line here. Not worth fixing if its the only issue though. -- Brandon Williams

Re: [PATCH v4 7/9] diff.c: decouple white space treatment from move detection algorithm

ommit -m "add text" && > + q_to_tab <<-\EOF >text.txt && > + Qa long line to exceed per-line minimum > + Qanother long line to exceed per-line minimum > + new file > + EOF > + > + # Make sure we get a different diff using -w > + git diff --color --color-moved -w | > + grep -v "index" | > + test_decode_color >actual && > + q_to_tab <<-\EOF >expected && > + diff --git a/text.txt b/text.txt > + --- a/text.txt > + +++ b/text.txt > + @@ -1,3 +1,3 @@ > + Qa long line to exceed per-line minimum > + Qanother long line to exceed per-line minimum > + -original file > + +new file > + EOF > + test_cmp expected actual && > + > + # And now ignoring white space only in the move detection > + git diff --color --color-moved \ > + > --color-moved-ws=ignore-all-space,ignore-space-change,ignore-space-at-eol | > + grep -v "index" | > + test_decode_color >actual && > + q_to_tab <<-\EOF >expected && > + diff --git a/text.txt b/text.txt > + --- a/text.txt > + +++ b/text.txt > + @@ -1,3 +1,3 @@ > + -a long line to exceed per-line minimum > + -another long line to exceed per-line minimum > + -original file > + +Qa long line to exceed per-line > minimum > + +Qanother long line to exceed per-line > minimum > + +new file > + EOF > + test_cmp expected actual > ' > > test_done > -- > 2.18.0.399.gad0ab374a1-goog > -- Brandon Williams

Re: [PATCH v4 9/9] diff.c: add white space mode to move detection that allows indent changes

is is used to fill a ws_delta struct by calculating the whitespace delta between two lines. If that is the case then why doesn't this function verify that the first 'd' characters in the longer line are indeed whitespace? Also, maybe this is just because I'm not as familiar with the move detection code, but how would the whitespace detection handle a line being moved from being indented with tabs to spaces or vice versa? Is this something already handled and not an issue? -- Brandon Williams

Re: [PATCH 07/17] commit: increase commit message buffer size

re we do not write bogus reflog > entries., 2007-01-26) > and it appears to me that 2*40 + 5 ought to be sufficient, but no > comments or commit > messages are found as to why we rather choose 100. Whats the reason for not using a strbuf here so that we don't have to play with magic numbers? -- Brandon Williams

Re: [PATCH 07/17] commit: increase commit message buffer size

On 07/09, Junio C Hamano wrote: > Brandon Williams writes: > > >> > > diff --git a/refs/files-backend.c b/refs/files-backend.c > >> > > index a9a066dcfb..252f835bae 100644 > >> > > --- a/refs/files-backend.c > >> > > ++

Re: [PATCH v2 2/2] fetch: send "refs/tags/" prefix upon CLI refspecs

ut for now we've had enough people comment on this lacking functionality that we should fix it. [1] Thankfully it doesn't completely undo what protocol v2 did, as we still are able to eliminate refs/changes or refs/pull or other various refs which significantly add to the number of refs advertised during fetch. -- Brandon Williams

Re: [PATCH v2 2/2] fetch: send "refs/tags/" prefix upon CLI refspecs

On 07/09, Junio C Hamano wrote: > Brandon Williams writes: > > > I agree with this observation, though I'm a bit sad about it. I think > > that having tag auto-following the default is a little confusing (and > > hurts perf[1] when using proto v2) but since

[RFC] push: add documentation on push v2

Signed-off-by: Brandon Williams --- Since introducing protocol v2 and enabling fetch I've been thinking about what its inverse 'push' would look like. After talking with a number of people I have a longish list of things that could be done to improve push and I think I've

Re: [RFC] push: add documentation on push v2

On 07/17, Stefan Beller wrote: > On Tue, Jul 17, 2018 at 2:09 PM Brandon Williams wrote: > > > > Signed-off-by: Brandon Williams > > --- > > > > Since introducing protocol v2 and enabling fetch I've been thinking > > about what its inverse 'push&#

Re: [RFC] push: add documentation on push v2

On 07/18, Derrick Stolee wrote: > On 7/17/2018 7:25 PM, Stefan Beller wrote: > > On Tue, Jul 17, 2018 at 2:09 PM Brandon Williams wrote: > > > Signed-off-by: Brandon Williams > > > --- > > > > > > Since introducing protocol v2 and enabling fetch I&

Re: [RFC] push: add documentation on push v2

On 07/18, Stefan Beller wrote: > On Wed, Jul 18, 2018 at 6:31 AM Derrick Stolee wrote: > > > > On 7/17/2018 7:25 PM, Stefan Beller wrote: > > > On Tue, Jul 17, 2018 at 2:09 PM Brandon Williams > > > wrote: > > >> Signed-off-by: Brandon William

Re: [RFC] push: add documentation on push v2

On 07/18, Duy Nguyen wrote: > On Wed, Jul 18, 2018 at 7:13 PM Brandon Williams wrote: > > > > > What I've got now is a rough design for a more flexible push, more > > > > > flexible because it allows for the server to do what it wants with the > > >

Re: [RFC] push: add documentation on push v2

> > SP new_oid > > > > +update-error = PKT-LINE(txn_id SP "error" SP error-msg LF) > > > > > > Can we unify "ERR" and "error" ? > > > > No, these are very different. You could have one ref update succeed > > while another doesn't for some reason, unless you want everything to be > > atomic. > > I did not mean to unify them on the semantic level, but on the > representation level, i.e. have both of them spelled the same, > as they can still be differentiated by the leading txn id? Oh I misunderstood again :) yeas we could standardize on ERR. > > > Thanks, > Stefan > > P.S.: another feature that just came to mind is localisation of error > messages. > But that is also easy to do with capabilities (the client sends a capability > such as "preferred-i18n=DE" and the server may translate all its errors > if it can. > > That brings me to another point: do we assume all errors to be read > by humans? or do we want some markup things in there, too, similar to > EAGAIN? This sort of thing could be added as a protocol-level capability where the client sends LANG= so that those sorts of msgs could be translated server side before sending them. -- Brandon Williams

[PATCH] clone: send ref-prefixes when using protocol v2

Signed-off-by: Brandon Williams --- Noticed we miss out on server side filtering of refs when cloning using protocol v2, this will enable that. builtin/clone.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index

[PATCH v2] clone: send ref-prefixes when using protocol v2

Teach clone to send a list of ref-prefixes, when using protocol v2, to allow the server to filter out irrelevant references from the ref-advertisement. This reduces wasted time and bandwidth when cloning repositories with a larger number of references. Signed-off-by: Brandon Williams

<    2   3   4   5   6   7   8   9   10   11   >