[RFC PATCH] xdiff/xpatience: support anchoring a line

2017-11-21 Thread Jonathan Tan
Teach the patience diff to support prohibiting a user-specified line from appearing as a deletion or addition in the end result. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- I'm sending this out to see if a change similar to this would be welcome. It is useful to me as a re

Re: [PATCH v5 0/6] Partial clone part 1: object filtering

2017-11-21 Thread Jonathan Tan
On Tue, 21 Nov 2017 20:58:46 + Jeff Hostetler wrote: > From: Jeff Hostetler > > Here is V5 of the list-object filtering, rev-list, and pack-objects. > > This version addresses comments on the V4 series. I removed the > questionable

Re: [PATCH v4 5/6] rev-list: add list-objects filtering support

2017-11-16 Thread Jonathan Tan
On Thu, 16 Nov 2017 18:07:42 + Jeff Hostetler wrote: > From: Jeff Hostetler > > Teach rev-list to use the filtering provided by the > traverse_commit_list_filtered() interface to omit > unwanted objects from the result. This feature is >

Re: [PATCH v4 4/6] list-objects: filter objects in traverse_commit_list

2017-11-16 Thread Jonathan Tan
On Thu, 16 Nov 2017 18:07:41 + Jeff Hostetler wrote: > +/* > + * Return 1 if the given string needs armoring because of "special" > + * characters that may cause injection problems when a command passes > + * the argument to a subordinate command (such as when

Re: [PATCH v4 00/10] Partial clone part 2: fsck and promisors

2017-11-16 Thread Jonathan Tan
I patched both this series and the first 9 patches of mine [1] on part 1 of the entire partial clone implementation [2], and then diffed them. I'll review just the differences between the two. You can see the entire diff below (minus means in my patch set but not in Jeff's, plus means the

Re: [PATCH v2 2/4] progress: fix progress meters when dealing with lots of work

2017-11-13 Thread Jonathan Tan
On Mon, 13 Nov 2017 12:15:58 -0800 Elijah Newren wrote: > The possibility of setting merge.renameLimit beyond 2^16 raises the > possibility that the values passed to progress can exceed 2^32. > Use uint64_t, because it "ought to be enough for anybody". :-) > > Signed-off-by:

Re: [PATCH v2 2/4] progress: fix progress meters when dealing with lots of work

2017-11-13 Thread Jonathan Tan
On Mon, 13 Nov 2017 12:15:58 -0800 Elijah Newren wrote: > -static int display(struct progress *progress, unsigned n, const char *done) > +static int display(struct progress *progress, uint64_t n, const char *done) > { > const char *eol, *tp; > > @@ -106,7 +106,7 @@

Re: [PATCH 1/9] extension.partialclone: introduce partial clone extension

2017-11-03 Thread Jonathan Tan
On Fri, 3 Nov 2017 09:57:18 -0400 Jeff Hostetler <g...@jeffhostetler.com> wrote: > On 11/2/2017 6:24 PM, Jonathan Tan wrote: > > On Thu, 2 Nov 2017 20:20:44 + > > Jeff Hostetler <g...@jeffhostetler.com> wrote: > > > >> From: Jeff Hostetler &l

Re: [PATCH 1/9] extension.partialclone: introduce partial clone extension

2017-11-06 Thread Jonathan Tan
On Mon, 6 Nov 2017 12:32:45 -0500 Jeff Hostetler wrote: > >> Yes, that is a point I wanted to ask about. I renamed the > >> extensions.partialclone that you created and then I moved your > >> remote..blob-max-bytes setting to be in extensions too. > >> Moving it to

Re: [PATCH v2 4/6] list-objects: filter objects in traverse_commit_list

2017-11-06 Thread Jonathan Tan
On Mon, 6 Nov 2017 12:51:52 -0500 Jeff Hostetler wrote: > Jonathan and I were talking off-list about the performance > effects of inspecting the pathnames to identify the ".git*" > special files. I added it in my first draft back in the spring, > thinking that even if you

[PATCH] Tests: document test_submodule_{,forced_}switch()

2017-11-06 Thread Jonathan Tan
test_submodule_forced_switch() do not correctly handle the situation in which a submodule is replaced with an ordinary directory. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- I find tests that use lib-submodule-update.sh difficult to understand due to the lack of clarity o

Re: [PATCH v2 4/6] list-objects: filter objects in traverse_commit_list

2017-11-02 Thread Jonathan Tan
On Thu, 2 Nov 2017 17:50:11 + Jeff Hostetler wrote: > +int parse_list_objects_filter(struct list_objects_filter_options > *filter_options, > + const char *arg) Returning void is fine, I think. It seems that all your code paths either

Re: [PATCH v2 0/6] Partial clone part 1: object filtering

2017-11-02 Thread Jonathan Tan
On Thu, 2 Nov 2017 17:50:07 + Jeff Hostetler wrote: > From: Jeff Hostetler > > Here is V2 of the list-object filtering. It replaces [1] > and reflect a refactoring and simplification of the original. Thanks, overall this looks quite good. I

Re: [PATCH 00/14] WIP Partial clone part 3: clone, fetch, fetch-pack, upload-pack, and tests

2017-11-02 Thread Jonathan Tan
On Thu, 2 Nov 2017 20:31:15 + Jeff Hostetler wrote: > From: Jeff Hostetler > > This is part 3 of 3 for partial clone. > It assumes that part 1 [1] and part 2 [2] are in place. > > Part 3 is concerned with the commands: clone, fetch,

Re: [PATCH 1/9] extension.partialclone: introduce partial clone extension

2017-11-02 Thread Jonathan Tan
On Thu, 2 Nov 2017 20:20:44 + Jeff Hostetler wrote: > From: Jeff Hostetler > > Introduce the ability to have missing objects in a repo. This > functionality is guarded by new repository extension options: >

Re: [PATCH v3 6/6] pack-objects: add list-objects filtering

2017-11-07 Thread Jonathan Tan
On Tue, 7 Nov 2017 19:35:46 + Jeff Hostetler wrote: > +--filter-ignore-missing: > + Ignore missing objects without error. This may be used with > + or without and of the above filtering. There is a discussion about this parameter (and the corresponding ones

Re: [PATCH v3 4/6] list-objects: filter objects in traverse_commit_list

2017-11-07 Thread Jonathan Tan
On Tue, 7 Nov 2017 19:35:44 + Jeff Hostetler wrote: > +/* > + * Reject the arg if it contains any characters that might > + * require quoting or escaping when handing to a sub-command. > + */ > +static int reject_injection_chars(const char *arg) > +{ [snip] > +}

Re: [PATCH v2 0/6] Partial clone part 1: object filtering

2017-11-07 Thread Jonathan Tan
On Fri, 3 Nov 2017 14:34:39 -0400 Jeff Hostetler wrote: > > Assuming we eventually get promisor support working, would there be > > any use case where "any missing is OK" mode would be useful in a > > sense more reasonable than "because we could have such a mode" and > >

Re: [PATCH 1/9] extension.partialclone: introduce partial clone extension

2017-11-08 Thread Jonathan Tan
On Wed, 8 Nov 2017 15:32:21 -0500 Jeff Hostetler wrote: > Thanks Jonathan. > > I moved my version of part 2 on top of yesterday's part 1. > There are a few changes between my version and yours. Could > you take a quick look at them and see if they make sense? > (I'll

Re: [RFC PATCH 0/4] git-status reports relation to superproject

2017-11-08 Thread Jonathan Tan
On Wed, 8 Nov 2017 11:55:05 -0800 Stefan Beller wrote: > $ git -c status.superprojectinfo status > HEAD detached at v2.15-rc2 > superproject is 6 commits behind HEAD 7070ce2..5e6d0fb > nothing to commit, working tree clean > > How cool is that? > > This series side

Re: [RFD] Long term plan with submodule refs?

2017-11-08 Thread Jonathan Tan
On Wed, 8 Nov 2017 16:10:07 -0800 Stefan Beller wrote: I thought of a possible alternative and how it would work. > Possible data models and workflow implications > == > In the following different data models are presented, which

Re: What's cooking in git.git (Oct 2017, #07; Mon, 30)

2017-11-01 Thread Jonathan Tan
On Wed, 01 Nov 2017 10:21:20 +0900 Junio C Hamano wrote: > Jeff Hostetler writes: > > >> Yes, that, together with the expectation that I will hear from both you > >> and JTan > >> once the result of combined effort becomes ready to replace

Re: [PATCH 02/14] clone, fetch-pack, index-pack, transport: partial clone

2017-11-03 Thread Jonathan Tan
On Thu, 2 Nov 2017 20:31:17 + Jeff Hostetler wrote: > From: Jeff Hostetler > > Signed-off-by: Jeff Hostetler > --- > builtin/clone.c | 9 + > builtin/fetch-pack.c | 4 > builtin/index-pack.c | 10

Re: [PATCH 04/14] fetch: add object filtering for partial fetch

2017-11-03 Thread Jonathan Tan
I did some of my own investigation and have a working (i.e. passing tests) version of this patch here: https://github.com/jonathantanmy/git/commits/pc20171103 If you want, you can use that, or incorporate the changes therein here. I'll also remark on my findings inline. On Thu, 2 Nov 2017

[PATCH v2] Tests: clean up and document submodule helpers

2017-11-07 Thread Jonathan Tan
test_submodule_forced_switch() do not correctly handle the situation in which a submodule is replaced with an ordinary directory. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Thanks for the review. Change from v1: - changed commit message title - moved a test to the common functi

[PATCH] decorate: clean up and document API

2017-12-07 Thread Jonathan Tan
Improve the names of the identifiers in decorate.h, document them, and add an example of how to use these functions. The example is compiled and run as part of the test suite. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- This patch contains some example code in a test

Re: [PATCH v6 00/12] Partial clone part 2: fsck and promisors

2017-12-05 Thread Jonathan Tan
er effect than merely suppressing "have" lines. This setting is described in patch 7 ("introduce fetch-object: fetch one promisor object"). > Part 2 is concerned with fsck, gc, initial support for dynamic > object fetching, and tracking promisor objects. Jonathan Tan > or

Re: [PATCH v6 03/12] fsck: support refs pointing to promisor objects

2017-12-07 Thread Jonathan Tan
On Thu, 7 Dec 2017 11:18:52 -0800 Brandon Williams wrote: > Instead of requiring that every test first removes 'repo', maybe you > want to have each test do its own cleanup by adding in > 'test_when_finished' lines to do the removals? Just a thought. If "test_when_finished"

Re: [PATCH] decorate: clean up and document API

2017-12-11 Thread Jonathan Tan
On Fri, 8 Dec 2017 04:55:11 -0500 Jeff King wrote: > I have mixed feelings. On the one hand, compiling and running the code > ensures that those things actually work. On the other hand, I expect you > can make a much clearer example if instead of having running code, you > show

Re: [PATCH v7 00/16] Parial clone part 3: clone, fetch, fetch-pack, upload-pack, and tests

2017-12-11 Thread Jonathan Tan
On Fri, 8 Dec 2017 14:30:10 -0800 Brandon Williams wrote: > I just finished reading through parts 1-3. Overall I like the series. > There are a few point's that I'm not a big fan of but i wasn't able to > come up with a better alternative. One of these being the need for a >

[WIP 0/2] Submodule ref backend that mirrors superproject

2017-12-01 Thread Jonathan Tan
also describes what happens when the submodule attempts to write to any "refs/..." ref. For those interested, here's what such an implementation might look like, and a test to demonstrate such functionality. I have partial read-only functionality - a lot of it still remains to be done. [1] h

[WIP 1/2] submodule: refactor acquisition of superproject info

2017-12-01 Thread Jonathan Tan
Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- submodule.c | 76 + submodule.h | 3 +++ 2 files changed, 49 insertions(+), 30 deletions(-) diff --git a/submodule.c b/submodule.c index bb531e0e5..ce511180e

[WIP 2/2] submodule: read-only super-backed ref backend

2017-12-01 Thread Jonathan Tan
Note that a few major parts are still missing: - special handling of the current branch of the superproject - writing (whether "refs/..." to the superproject as an index change or a commit, or non-"refs/..." directly to the subproject like usual) Signed-off-by: Jona

Re: [PATCH 03/13] list-objects: filter objects in traverse_commit_list

2017-10-24 Thread Jonathan Tan
On Tue, Oct 24, 2017 at 11:53 AM, Jeff Hostetler wrote: > +enum list_objects_filter_result { > + LOFR_ZERO = 0, > + LOFR_MARK_SEEN = 1<<0, Probably worth documenting, something like /* Mark this object so that it is skipped for the rest of the traversal.

Re: [PATCH 07/13] list-objects-filter-options: common argument parsing

2017-10-24 Thread Jonathan Tan
On Tue, Oct 24, 2017 at 11:53 AM, Jeff Hostetler wrote: > + * ::= blob:none > + * blob:limit:[kmg] > + * sparse:oid: > + * sparse:path: I notice in the code below that there are some usages of "=" instead of ":" - could you clarify which one

Re: [PATCH 08/13] list-objects: add traverse_commit_list_filtered method

2017-10-24 Thread Jonathan Tan
On Tue, Oct 24, 2017 at 11:53 AM, Jeff Hostetler wrote: > +void traverse_commit_list_filtered( > + struct list_objects_filter_options *filter_options, > + struct rev_info *revs, > + show_commit_fn show_commit, > + show_object_fn show_object, > +

Re: [PATCH 00/13] WIP Partial clone part 1: object filtering

2017-10-24 Thread Jonathan Tan
On Tue, Oct 24, 2017 at 11:53 AM, Jeff Hostetler <g...@jeffhostetler.com> wrote: > From: Jeff Hostetler <jeffh...@microsoft.com> > > I've been working with Jonathan Tan to combine our partial clone > proposals. This patch series represents a first step in that effort &g

Re: [PATCH 10/13] rev-list: add list-objects filtering support

2017-10-24 Thread Jonathan Tan
On Tue, Oct 24, 2017 at 11:53 AM, Jeff Hostetler wrote: > static void finish_object(struct object *obj, const char *name, void > *cb_data) > { > struct rev_list_info *info = cb_data; > - if (obj->type == OBJ_BLOB && !has_object_file(>oid)) > + if

Re: [PATCH 00/13] WIP Partial clone part 1: object filtering

2017-10-25 Thread Jonathan Tan
On Tue, Oct 24, 2017 at 10:00 PM, Junio C Hamano wrote: > OK, thanks for working well together. So does this (1) build on > Jonathan's fsck-squelching series, or (2) ignores that and builds > filtering first, potentially leaving the codebase to a broken state > where it can

Re: [PATCH v2 1/3] upload-pack: fix error message typo

2018-05-04 Thread Jonathan Tan
On Fri, 04 May 2018 11:24:39 +0900 Junio C Hamano wrote: > Hmm, when somebody breaks "git server serve", we probably would not > want to see the binary spewed to the output while debugging it. So > I'd probably keep the redirection---it may be an improvement to use > ">out"

Re: [PATCH 1/5] submodule foreach: correct '$path' in nested submodules from a subdirectory

2018-05-04 Thread Jonathan Tan
On Thu, 3 May 2018 11:12:27 -0700 Stefan Beller wrote: > >> There are three different possible solutions that have more value: > >> (a) The path value is documented as the path from the toplevel of the > >> superproject to the mount point of the submodule. If 'the' refers

Re: [PATCH v3 13/13] alloc: allow arbitrary repositories for alloc functions

2018-05-08 Thread Jonathan Tan
On Tue, 8 May 2018 12:37:36 -0700 Stefan Beller wrote: > +void clear_alloc_state(struct alloc_state *s) > +{ > + while (s->slab_nr > 0) { > + s->slab_nr--; > + free(s->slabs[s->slab_nr]); > + } I should have caught this earlier, but you need

Re: [PATCHv2 0/4] Rebooting pc/submodule-helper-foreach

2018-05-09 Thread Jonathan Tan
On Tue, 8 May 2018 17:29:48 -0700 Stefan Beller wrote: > v2: > * rebased onto origin/master > * dropped leftover "toplevel" variable from experimentation > * reworded the commit message for the first patch extensively > * dropped the third patch > * see "branch-diff" below.

Re: [PATCH v4 00/13] object store: alloc

2018-05-10 Thread Jonathan Tan
On Wed, 9 May 2018 17:40:11 -0700 Stefan Beller wrote: > if (obj->type == OBJ_TREE) > - release_tree_node((struct tree*)obj); > + free_tree_buffer((struct tree*)obj); > else if (obj->type == OBJ_COMMIT) > -

Re: [PATCH v4 00/13] object store: alloc

2018-05-10 Thread Jonathan Tan
On Thu, 10 May 2018 10:32:09 -0700 Stefan Beller wrote: > > - I would call them release_commit() and release_tag(), to match > >strbuf_release(). > > Why not commit_release and tag_release to also have the same order > of words as in strbuf_release ? At this point in

Re: [PATCH v2 01/13] repository: introduce parsed objects field

2018-05-08 Thread Jonathan Tan
On Mon, 7 May 2018 15:59:04 -0700 Stefan Beller wrote: > /* > - * Holds any information related to accessing the raw object content. > + * Holds any information needed to retrieve the raw content > + * of objects. The object_parser uses this to get

Re: [PATCH v2 13/13] alloc: allow arbitrary repositories for alloc functions

2018-05-08 Thread Jonathan Tan
On Mon, 7 May 2018 15:59:16 -0700 Stefan Beller wrote: > + for (i = 0; i < o->obj_hash_size; i++) { > + struct object *obj = o->obj_hash[i]; > + > + if (!obj) > + continue; > + > + if (obj->type == OBJ_TREE) { > +

[RFC PATCH] fetch-pack: space out sent "haves" in negotiation

2018-05-21 Thread Jonathan Tan
e corresponding remote-tracking tips. This can be done simultaneously with the approach in this patch, but if we were to evaluate only one first, the ancestor-or-descendant-of-remote-tracking-tip approach might be the better one to do first. Signed-off-by: Jonathan Tan <jonathanta

[PATCH] submodule: do not pass null OID to setup_revisions

2018-05-24 Thread Jonathan Tan
te submodule changes only)", 2017-06-23), which also introduced this feature. This is because cmd_pull() in builtin/pull.c thus invokes submodule_touches_in_range() with a null OID as the first parameter. Ensure that this case works, and document what happens in this case. Signed-of

Re: [PATCH] submodule: do not pass null OID to setup_revisions

2018-05-24 Thread Jonathan Tan
On Thu, 24 May 2018 16:07:49 -0700 Stefan Beller <sbel...@google.com> wrote: > Hi Jonathan, > > On Thu, May 24, 2018 at 1:47 PM, Jonathan Tan <jonathanta...@google.com> > wrote: > > If "git pull --recurse-submodules --rebase" is invoked when the current

Re: [PATCH 0/8] Reroll of sb/diff-color-move-more

2018-05-17 Thread Jonathan Tan
On Thu, 17 May 2018 12:46:45 -0700 Stefan Beller wrote: > Stefan Beller (8): > xdiff/xdiff.h: remove unused flags > xdiff/xdiffi.c: remove unneeded function declarations > diff.c: do not pass diff options as keydata to hashmap > diff.c: adjust hash function signature

Re: [RFC PATCH] fetch-pack: space out sent "haves" in negotiation

2018-05-22 Thread Jonathan Tan
On Mon, 21 May 2018 15:57:18 -0700 Stefan Beller wrote: > In an ideal world, the server and client would both estimate the potential > reduction of the packfile to send, and base the decision if to continue > negotiating on the trade off if the packfile reduction savings are

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

2018-06-07 Thread Jonathan Tan
reduced. Jonathan Tan (2): pack-bitmap: remove bitmap_git global variable pack-bitmap: add free function builtin/pack-objects.c | 7 +- builtin/rev-list.c | 13 +- pack-bitmap-write.c| 10 +- pack-bitmap.c | 344 - pack-bitmap.h

[PATCH 1/2] pack-bitmap: remove bitmap_git global variable

2018-06-07 Thread Jonathan Tan
if an unnecessarily long-lived "pack" field in struct bitmap_index still points to it. The bitmap API is also clearer in that we need to first obtain a struct bitmap_index, then we use it. Helped-by: Stefan Beller Signed-off-by: Jonathan Tan --- builtin/pack-objects.c | 6 +- builtin/rev-list.c

[PATCH 2/2] pack-bitmap: add free function

2018-06-07 Thread Jonathan Tan
to another field within the same struct. The documentation for that field has been updated to clarify that. Signed-off-by: Jonathan Tan --- builtin/pack-objects.c | 1 + builtin/rev-list.c | 2 ++ pack-bitmap-write.c| 4 pack-bitmap.c | 35

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

2018-06-18 Thread Jonathan Tan
> okay. Thinking long term, we may want to introduce a capability that > can filter the tag space, e.g. "want-refs-since refs/tags/*" > as a client directive as then the client only asks for new (newly > created/appearing) tags instead of all tags. I don't think refs normally have a

Re: [PATCH v2 1/2] t5702: test fetch with multiple refspecs at a time

2018-06-18 Thread Jonathan Tan
On Mon, Jun 18, 2018 at 11:30 AM, Stefan Beller wrote: >> +test_expect_success 'fetch supports various ways of have lines' ' >> + rm -rf server client trace && > > Can we move these deletions to test_when_finished of the previous(?) test > as well as have them here in a test_when_finished

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

2018-06-18 Thread Jonathan Tan
> Jonathan Tan writes: > > > When performing tag following, in addition to using the server's > > "include-tag" capability to send tag objects (and emulating it if the > > server does not support that capability), "git fetch" relies upon the > >

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

2018-06-18 Thread Jonathan Tan
> Jonathan Tan writes: > > >> Wouldn't that allow us not having to advertise the whole tags > >> namespace only to implement the tag following? > > > > Yes, it would, but as far as I can tell, it would add an extra burden on > > the server to walk all re

[PATCH v3 5/7] fetch-pack: make negotiation-related vars local

2018-06-14 Thread Jonathan Tan
when using a transport that does not support tag following), in that different priority queues will now be used in each invocation, instead of reusing the possibly non-empty one. Signed-off-by: Jonathan Tan --- fetch-pack.c | 116 ++- 1 file changed

[PATCH v3 2/7] fetch-pack: clear marks before re-marking

2018-06-14 Thread Jonathan Tan
one before any marking (whether by rev_list_insert_ref_oid() or mark_complete_and_common_ref()). Signed-off-by: Jonathan Tan --- fetch-pack.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fetch-pack.c b/fetch-pack.c index 5c87bb8bb..2812499a5 100644 --- a/fetch-pa

[PATCH v3 3/7] fetch-pack: directly end negotiation if ACK ready

2018-06-14 Thread Jonathan Tan
unconditionally. [1] The rationale is further described in the originating commit f2cba9299b ("fetch-pack: Finish negotation if remote replies "ACK %s ready"", 2011-03-14). Signed-off-by: Jonathan Tan --- fetch-pack.c | 9 + 1 file changed, 5 insertions(+), 4 deletio

[PATCH v3 7/7] fetch-pack: introduce negotiator API

2018-06-14 Thread Jonathan Tan
to negotiator/default.c, and it can be seen that the lines replaced by negotiator->X() calls are present in the X() functions respectively. Signed-off-by: Jonathan Tan --- Makefile | 2 + fetch-negotiator.c | 8 ++ fetch-negotiator.h | 57 fetch-pack.c |

[PATCH v3 4/7] fetch-pack: use ref adv. to prune "have" sent

2018-06-14 Thread Jonathan Tan
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 --- fetch-

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

2018-06-14 Thread Jonathan Tan
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 Jonathan Tan (7): fetch-pack: split up everything_local() fetch-pack: clear marks before re-marking fetch-pack: directly en

[PATCH v3 1/7] fetch-pack: split up everything_local()

2018-06-14 Thread Jonathan Tan
was introduced in a1c6d7c1a7 ("fetch-pack: restore save_commit_buffer after use", 2017-12-08), is a concern of the parse_object() call in mark_complete_and_common_ref(), so it has been moved from the end of everything_local() to the end of mark_complete_and_common_ref(). Signed-off-by: Jonathan Tan

[PATCH v3 6/7] fetch-pack: move common check and marking together

2018-06-14 Thread Jonathan Tan
-by: Jonathan Tan --- fetch-pack.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fetch-pack.c b/fetch-pack.c index 473e415c5..fb76d4017 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -505,11 +505,14 @@ static int find_common(struct negotiation_state *ns

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

2018-06-14 Thread Jonathan Tan
> @@ -1122,6 +1124,7 @@ static int do_fetch(struct transport *transport, > int autotags = (transport->remote->fetch_tags == 1); > int retcode = 0; > const struct ref *remote_refs; > + struct ref *new_remote_refs = NULL; Above, you use the name "updated_remote_refs" - it's

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

2018-06-15 Thread Jonathan Tan
(replying to the original since my e-mail is about design) > This version of ref-in-want is a bit more restrictive than what Jonathan > originally proposed (only full ref names are allowed instead of globs > and OIDs), but it is meant to accomplish the same goal (solve the issues > of refs

Re: [PATCH 1/2] pack-bitmap: remove bitmap_git global variable

2018-06-11 Thread Jonathan Tan
On Sat, 9 Jun 2018 02:04:38 -0400 Jeff King wrote: > This function used to be idempotent, so any code which wanted to use the > global bitmap_git could call it "just in case". After your patch, it's > not. I think it's probably OK, since such functions would generally now > take a bitmap_git

[PATCH] list-objects: check if filter is NULL before using

2018-06-11 Thread Jonathan Tan
In partial_clone_get_default_filter_spec(), the core_partial_clone_filter_default variable may be NULL; ensure that it is not NULL before using it. Signed-off-by: Jonathan Tan --- This was noticed by someone else at $DAY_JOB when trying to use a partial clone with no core.partialclonefilter set

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

2018-06-19 Thread Jonathan Tan
[snip] > > in which we have rarely-updated branches that we still want to fetch > > (e.g. an annotated tag when we fetch refs/tags/* or a Gerrit > > refs/changes/* branch), having the ref advertisement first means that we > > can omit them from our "want" or "want-ref" list. But not having them >

Re: What's cooking in git.git (Jun 2018, #05; Mon, 18)

2018-06-19 Thread Jonathan Tan
> Here are the topics that have been cooking. Commits prefixed with > '-' are only in 'pu' (proposed updates) while commits prefixed with > '+' are in 'next'. The ones marked with '.' do not appear in any of > the integration branches, but I am still holding onto them. Would it be possible to

Re: The state of the object store series

2018-06-19 Thread Jonathan Tan
> Floating on the mailing list, not cooking yet: One more is my bitmap one here: https://public-inbox.org/git/cover.1528397984.git.jonathanta...@google.com/ It's not in any branch yet, as far as I can tell, so I've just sent out an e-mail letting Junio know [1]. [1]

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

2018-06-19 Thread Jonathan Tan
> On 06/15, Jonathan Tan wrote: > > > > Supporting patterns would mean that we would possibly be able to > > eliminate the ls-refs step, thus saving at least a RTT. (Originally I > > thought that supporting patterns would also allow us to tolerate refs > > being

[PATCH v2 0/2] Fix protocol v2 tag following with CLI refspec

2018-06-05 Thread Jonathan Tan
ith ref-in-want (for example, in your latest series [1]) we might be able to restore performance, because the server can send refs/tags/X with the packfile instead of sending all refs/tags/X refs initially to the client. [1] https://public-inbox.org/git/20180605175144.4225-1-bmw...@google.com/

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

2018-06-05 Thread Jonathan Tan
uations. This also necessitates a change another test which tested ref advertisement filtering using tag refs - since tag refs are sent by default now, the test has been switched to using branch refs instead. Signed-off-by: Jonathan Tan --- builtin/fetch.c| 2 +- t/t5702-protocol-v2.sh |

[PATCH v2 1/2] t5702: test fetch with multiple refspecs at a time

2018-06-05 Thread Jonathan Tan
Extend the protocol v2 tests to also test fetches with multiple refspecs specified. This also covers the previously uncovered cases of fetching with prefix matching and fetching by SHA-1. Signed-off-by: Jonathan Tan --- t/t5702-protocol-v2.sh | 47 ++ 1

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

2018-06-05 Thread Jonathan Tan
e ref-prefixes when using a configured refspec", 2018-05-18) ensured that "refs/tags/" is always sent as a ref prefix when "git fetch" is invoked with no refspecs, but not when "git fetch" is invoked with refspecs. Extend that functionality to make it work in both si

[PATCH 0/2] Fix protocol v2 tag following with CLI refspec

2018-06-05 Thread Jonathan Tan
] https://public-inbox.org/git/20180531072339.ga43...@aiede.svl.corp.google.com/ Jonathan Tan (2): t5702: test fetch with multiple refspecs at a time fetch: send "refs/tags/" prefix upon CLI refspecs builtin/fetch.c| 2 +- t/t5702-protocol-

[PATCH 1/2] t5702: test fetch with multiple refspecs at a time

2018-06-05 Thread Jonathan Tan
Extend the protocol v2 tests to also test fetches with multiple refspecs specified. This also covers the previously uncovered cases of fetching with prefix matching and fetching by SHA-1. Signed-off-by: Jonathan Tan --- t/t5702-protocol-v2.sh | 47 ++ 1

Re: [PATCH 1/6] fetch-pack: clear marks before everything_local()

2018-06-05 Thread Jonathan Tan
On Tue, 5 Jun 2018 16:08:21 -0700 Jonathan Nieder wrote: > Hi, > > Jonathan Tan wrote: > > > If tag following is required when using a transport that does not > > support tag following, fetch_pack() will be invoked twice in the same > > process, necessitating

Re: [PATCH 4/6] fetch-pack: make negotiation-related vars local

2018-06-05 Thread Jonathan Tan
On Tue, Jun 5, 2018 at 4:35 PM, Jonathan Nieder wrote: > Jonathan Tan wrote: > >> Reduce the number of global variables by making the priority queue and >> the count of non-common commits in it local, passing them as a struct to >> various functions where necessary. >

Re: [PATCH 5/6] fetch-pack: move common check and marking together

2018-06-05 Thread Jonathan Tan
On Tue, Jun 5, 2018 at 5:01 PM, Jonathan Nieder wrote: > I like it. I think it should be possible to describe the benefit of > this patch without reference to the specifics of the subsequent one. > Maybe something like: > > When receiving 'ACK continue' for a common commit, >

Re: [PATCH 6/6] fetch-pack: introduce negotiator API

2018-06-05 Thread Jonathan Tan
On Tue, Jun 5, 2018 at 5:37 PM, Jonathan Nieder wrote: >> This patch is written to be more easily reviewed: static functions are >> moved verbatim from fetch-pack.c to negotiator/default.c, and it can be >> seen that the lines replaced by negotiator->X() calls are present in the >> X() functions

Re: [PATCH 3/6] fetch-pack: in protocol v2, enqueue commons first

2018-06-05 Thread Jonathan Tan
On Tue, Jun 5, 2018 at 4:30 PM, Jonathan Nieder wrote: > I get lost in the above description. I suspect it's doing a good job > of describing the code, instead of answering the question I really > have about what is broken and what behavior we want instead. > > E.g. are there some commands that

Re: [PATCH 2/6] fetch-pack: truly stop negotiation upon ACK ready

2018-06-05 Thread Jonathan Tan
On Tue, 5 Jun 2018 16:16:34 -0700 Jonathan Nieder wrote: > Hi, > > 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]. This appears > &

[PATCH v2 2/8] fetch-pack: clear marks before re-marking

2018-06-06 Thread Jonathan Tan
one before any marking (whether by rev_list_insert_ref_oid() or mark_complete_and_common_ref()). Signed-off-by: Jonathan Tan --- fetch-pack.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fetch-pack.c b/fetch-pack.c index 5c87bb8bb..2812499a5 100644 --- a/fetch-pa

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

2018-06-06 Thread Jonathan Tan
Signed-off-by: Jonathan Tan --- negotiator/default.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/negotiator/default.c b/negotiator/default.c index b8f45cf78..a9e52c943 100644 --- a/negotiator/default.c +++ b/negotiator/default.c @@ -46,11 +46,10 @@ static

[PATCH v2 0/8] Refactor fetch negotiation into its own API

2018-06-06 Thread Jonathan Tan
ge: comments should have ' *' at the start of each > > line (could do in a preparatory patch or a followup). > > I'll add a followup. I'm now not sure of the value of making a change just to update formatting, but I added the followup commit anyway - it can be easily dropped if we deci

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

2018-06-06 Thread Jonathan Tan
rationale is further described in the originating commit f2cba9299b ("fetch-pack: Finish negotation if remote replies "ACK %s ready"", 2011-03-14). Signed-off-by: Jonathan Tan --- fetch-pack.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/fetch-pack.

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

2018-06-06 Thread Jonathan Tan
was introduced in a1c6d7c1a7 ("fetch-pack: restore save_commit_buffer after use", 2017-12-08), is a concern of the parse_object() call in mark_complete_and_common_ref(), so it has been moved from the end of everything_local() to the end of mark_complete_and_common_ref(). Signed-off-by: Jonathan Tan

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

2018-06-06 Thread Jonathan Tan
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 --- fetch-pack.c | 6 +++--- t/t550

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

2018-06-06 Thread Jonathan Tan
that patch, this struct definition and several functions will be moved to a negotiation-specific file, and this allows the move to be verbatim. Signed-off-by: Jonathan Tan --- fetch-pack.c | 104 +-- 1 file changed, 59 insertions(+), 45 deletions(-)

[PATCH v2 7/8] fetch-pack: introduce negotiator API

2018-06-06 Thread Jonathan Tan
to negotiator/default.c, and it can be seen that the lines replaced by negotiator->X() calls are present in the X() functions respectively. Signed-off-by: Jonathan Tan --- Makefile | 2 + fetch-negotiator.c | 8 ++ fetch-negotiator.h | 57 fetch-pack.c |

[PATCH v2 6/8] fetch-pack: move common check and marking together

2018-06-06 Thread Jonathan Tan
-by: Jonathan Tan --- fetch-pack.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fetch-pack.c b/fetch-pack.c index c31644bb9..4a4ec4da3 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -499,11 +499,14 @@ static int find_common(struct data *data, struct fetch_pack_args *args

Re: [RFC PATCH] fetch-pack: space out sent "haves" in negotiation

2018-05-29 Thread Jonathan Tan
On Wed, 23 May 2018 12:42:10 +0900 Junio C Hamano wrote: > Somehow this feels more like a WIP than RFC, primarily for two > reasons. It was unclear what "edge" computation is trying to do; it > seems way under-explained, especially the part that takes min-max > while. merging two candidates.

[PATCH 4/6] fetch-pack: make negotiation-related vars local

2018-06-04 Thread Jonathan Tan
that patch, this struct definition and several functions will be moved to a negotiation-specific file, and this allows the move to be verbatim. Signed-off-by: Jonathan Tan --- fetch-pack.c | 104 +-- 1 file changed, 59 insertions(+), 45 deletions(-)

[PATCH 6/6] fetch-pack: introduce negotiator API

2018-06-04 Thread Jonathan Tan
to negotiator/default.c, and it can be seen that the lines replaced by negotiator->X() calls are present in the X() functions respectively. Signed-off-by: Jonathan Tan --- Makefile | 2 + fetch-negotiator.c | 7 ++ fetch-negotiator.h | 45 ++ fetch-pack.c |

[PATCH 5/6] fetch-pack: move common check and marking together

2018-06-04 Thread Jonathan Tan
This enables the calculation of was_common and the invocation to mark_common() to be abstracted into a single call to the negotiator API (to be introduced in a subsequent patch). Signed-off-by: Jonathan Tan --- fetch-pack.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git

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