Re: [PATCH v3 16/35] transport: convert transport_get_remote_refs to take a list of ref patterns

2018-02-21 Thread Jonathan Tan
On Tue, 6 Feb 2018 17:12:53 -0800 Brandon Williams wrote: > -const struct ref *transport_get_remote_refs(struct transport *transport) > +const struct ref *transport_get_remote_refs(struct transport *transport, > + const struct

Re: [PATCH v3 20/35] upload-pack: introduce fetch server command

2018-02-21 Thread Jonathan Tan
On Tue, 6 Feb 2018 17:12:57 -0800 Brandon Williams wrote: > +want > + Indicates to the server an object which the client wants to > + retrieve. Mention that the client can "want" anything even if not advertised by the server (like uploadpack.allowanysha1inwant).

Re: [PATCH v3 35/35] remote-curl: don't request v2 when pushing

2018-02-21 Thread Jonathan Tan
On Tue, 6 Feb 2018 17:13:12 -0800 Brandon Williams wrote: > +test_expect_success 'push with http:// and a config of v2 does not request > v2' ' > + # Till v2 for push is designed, make sure that if a client has > + # protocol.version configured to use v2, that the

Re: [PATCH 09/27] sha1_file: add raw_object_store argument to alt_odb_usable

2018-02-21 Thread Jonathan Tan
off-by: Stefan Beller <sbel...@google.com> I checked that alt_odb_usable no longer depends on any repository-like globals. Reviewed-by: Jonathan Tan <jonathanta...@google.com>

Re: [PATCH v3 15/35] transport: convert get_refs_list to take a list of ref patterns

2018-02-21 Thread Jonathan Tan
On Tue, 6 Feb 2018 17:12:52 -0800 Brandon Williams wrote: > @@ -21,7 +22,8 @@ struct transport_vtable { >* the ref without a huge amount of effort, it should store it >* in the ref's old_sha1 field; otherwise it should be all 0. >**/ > - struct ref

Re: [PATCH v3 30/35] remote-curl: create copy of the service name

2018-02-21 Thread Jonathan Tan
..@google.com> Probably worth mentioning in the commit message: Currently, all service names are string constants, but a subsequent patch will introduce service names from external sources. Other than that, Reviewed-by: Jonathan Tan <jonathanta...@google.com>

Re: [PATCH 19/27] sha1_file: add repository argument to map_sha1_file_1

2018-02-21 Thread Jonathan Tan
> repositories other than the_repository yet. > > As with the previous commits, use a macro to catch callers passing a > repository other than the_repository at compile time. > > Signed-off-by: Stefan Beller <sbel...@google.com> > Signed-off-by: Jonathan Nieder <j

Re: [PATCH 22/27] sha1_file: allow sha1_file_name to handle arbitrary repositories

2018-02-21 Thread Jonathan Tan
On Tue, 20 Feb 2018 17:54:25 -0800 Stefan Beller <sbel...@google.com> wrote: > Signed-off-by: Stefan Beller <sbel...@google.com> > Signed-off-by: Jonathan Nieder <jrnie...@gmail.com> Reviewed-by: Jonathan Tan <jonathanta...@google.com> > -void sha1_file_name_

Re: [PATCH v3 08/35] connect: discover protocol version outside of get_remote_heads

2018-02-21 Thread Jonathan Tan
On Tue, 6 Feb 2018 17:12:45 -0800 Brandon Williams wrote: > - get_remote_heads(fd[0], NULL, 0, , 0, NULL, ); > + > + packet_reader_init(, fd[0], NULL, 0, > +PACKET_READ_CHOMP_NEWLINE | > +PACKET_READ_GENTLE_ON_EOF); > +

Re: [PATCH v3 12/35] serve: introduce git-serve

2018-02-21 Thread Jonathan Tan
On Tue, 6 Feb 2018 17:12:49 -0800 Brandon Williams wrote: > .gitignore | 1 + > Documentation/technical/protocol-v2.txt | 114 +++ > Makefile| 2 + > builtin.h | 1 +

Re: [PATCH v3 24/35] connect: refactor git_connect to only get the protocol version once

2018-02-21 Thread Jonathan Tan
Signed-off-by: Brandon Williams <bmw...@google.com> Reviewed-by: Jonathan Tan <jonathanta...@google.com>

Re: [PATCH v3 28/35] transport-helper: introduce stateless-connect

2018-02-21 Thread Jonathan Tan
On Tue, 6 Feb 2018 17:13:05 -0800 Brandon Williams wrote: > Introduce the transport-helper capability 'stateless-connect'. This > capability indicates that the transport-helper can be requested to run > the 'stateless-connect' command which should attempt to make a >

Re: [PATCH v3 32/35] http: allow providing extra headers for http requests

2018-02-21 Thread Jonathan Tan
On Tue, 6 Feb 2018 17:13:09 -0800 Brandon Williams wrote: > @@ -172,6 +172,8 @@ struct http_get_options { >* for details. >*/ > struct strbuf *base_url; > + > + struct string_list *extra_headers; Document this? For example: If not NULL,

Re: [PATCH v3 28/35] transport-helper: introduce stateless-connect

2018-02-22 Thread Jonathan Tan
On Thu, 22 Feb 2018 10:53:53 -0800 Brandon Williams wrote: > > > @@ -612,6 +615,11 @@ static int process_connect_service(struct transport > > > *transport, > > > if (data->connect) { > > > strbuf_addf(, "connect %s\n", name); > > > ret =

Re: [PATCH v3 14/35] connect: request remote refs using v2

2018-02-22 Thread Jonathan Tan
On Thu, 22 Feb 2018 13:26:58 -0500 Jeff King <p...@peff.net> wrote: > On Thu, Feb 22, 2018 at 10:19:22AM -0800, Brandon Williams wrote: > > > On 02/21, Jonathan Tan wrote: > > > On Tue, 6 Feb 2018 17:12:51 -0800 > > > Brandon Williams <bmw...@google.com>

Re: [PATCH v3 08/35] connect: discover protocol version outside of get_remote_heads

2018-02-22 Thread Jonathan Tan
On Thu, 22 Feb 2018 10:17:39 -0800 Brandon Williams wrote: > > > diff --git a/remote.h b/remote.h > > > index 1f6611be2..2016461df 100644 > > > --- a/remote.h > > > +++ b/remote.h > > > @@ -150,10 +150,11 @@ int check_ref_type(const struct ref *ref, int > > > flags); > > >

Re: [PATCH v3 16/35] transport: convert transport_get_remote_refs to take a list of ref patterns

2018-02-22 Thread Jonathan Tan
On Thu, 22 Feb 2018 10:26:47 -0800 Brandon Williams <bmw...@google.com> wrote: > On 02/21, Jonathan Tan wrote: > > On Tue, 6 Feb 2018 17:12:53 -0800 > > Brandon Williams <bmw...@google.com> wrote: > > > > > -const struct ref *transport_get_remote_refs

Re: [PATCH v3 21/35] fetch-pack: perform a fetch using v2

2018-02-23 Thread Jonathan Tan
On Tue, 6 Feb 2018 17:12:58 -0800 Brandon Williams wrote: > + while ((oid = get_rev())) { > + packet_buf_write(req_buf, "have %s\n", oid_to_hex(oid)); > + if (++haves_added >= INITIAL_FLUSH) > + break; > + }; Unnecessary

Re: [PATCH v3 23/35] fetch-pack: support shallow requests

2018-02-23 Thread Jonathan Tan
On Tue, 6 Feb 2018 17:13:00 -0800 Brandon Williams wrote: > @@ -1090,6 +1110,10 @@ static int send_fetch_request(int fd_out, const struct > fetch_pack_args *args, > if (prefer_ofs_delta) > packet_buf_write(_buf, "ofs-delta"); > > + /* Add

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

2018-02-21 Thread Jonathan Tan
On Tue, 6 Feb 2018 17:12:41 -0800 Brandon Williams wrote: > In order to allow for code sharing with the server-side of fetch in > protocol-v2 convert upload-pack to be a builtin. > > Signed-off-by: Brandon Williams As Stefan mentioned in [1], also

Re: [PATCHv4 00/27] Moving global state into the repository object (part 1)

2018-02-26 Thread Jonathan Tan
On Fri, 23 Feb 2018 16:47:27 -0800 Stefan Beller wrote: > v4: > * addressed feedback from the single patches (mostly nits) > * rebased on latest master The patches I looked at previously (patches 7, 15, 19, 22, and 24) look good to me.

[PATCH 4/5] commit-graph: store graph in struct object_store

2018-06-21 Thread Jonathan Tan
-by: Jonathan Tan --- commit-graph.c | 34 ++ object-store.h | 3 +++ object.c | 5 + 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/commit-graph.c b/commit-graph.c index 9f4e761229..61b4fbb925 100644 --- a/commit-graph.c +++ b/commit

[PATCH 5/5] commit-graph: add repo arg to graph readers

2018-06-21 Thread Jonathan Tan
removed. Instead, the config option core.commitGraph is now read on the first time in a repository that a commit is attempted to be parsed using its commit graph. This commit includes a test that exercises the functionality on an arbitrary repository that is not the_repository. Signed-off-by: Jonathan

[PATCH 0/5] Object store refactoring: commit graph

2018-06-21 Thread Jonathan Tan
didn't modify the function that writes commit graphs - perhaps that can be done in a subsequent series. Jonathan Tan (5): object-store: add missing include commit-graph: add missing forward declaration commit-graph: add free_commit_graph commit-graph: store graph in struct object_store

[PATCH 3/5] commit-graph: add free_commit_graph

2018-06-21 Thread Jonathan Tan
Signed-off-by: Jonathan Tan --- builtin/commit-graph.c | 2 ++ commit-graph.c | 24 ++-- commit-graph.h | 2 ++ 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 37420ae0fd..9c2d55221c

[PATCH 1/5] object-store: add missing include

2018-06-21 Thread Jonathan Tan
Signed-off-by: Jonathan Tan --- object-store.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/object-store.h b/object-store.h index d683112fd7..f0b77146e4 100644 --- a/object-store.h +++ b/object-store.h @@ -2,6 +2,9 @@ #define OBJECT_STORE_H #include "oidmap.h" +#inclu

[PATCH 2/5] commit-graph: add missing forward declaration

2018-06-21 Thread Jonathan Tan
Signed-off-by: Jonathan Tan --- commit-graph.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/commit-graph.h b/commit-graph.h index 260a468e73..7004dfdca9 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -3,6 +3,8 @@ #include "git-compat-util.h" +struct commi

Re: [PATCH 2/5] commit-graph: add missing forward declaration

2018-06-21 Thread Jonathan Tan
> Both this and the previous patch look good to me; > you seem to have better (stricter) checking for > missing includes/forward declarations, am I missing > a compile option? (I have DEVELOPER=1 in config.mak) Thanks. No I don't - I discovered that these were missing as I was including these

Re: [PATCH 5/5] commit-graph: add repo arg to graph readers

2018-06-21 Thread Jonathan Tan
> > diff --git a/commit.c b/commit.c > > index 0030e79940..38c12b002f 100644 > > --- a/commit.c > > +++ b/commit.c > > @@ -317,7 +317,7 @@ struct tree *get_commit_tree(const struct commit > > *commit) > > if (commit->graph_pos == COMMIT_NOT_FROM_GRAPH) > > BUG("commit has

Re: [PATCH 2/2] refs: switch for_each_replace_ref back to use a ref_store

2018-07-30 Thread Jonathan Tan
> So let's go back to the clean API, just requiring a ref_store as an > argument. Here, you say that we want ref_store as an argument... > -int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data) > +int for_each_replace_ref(each_ref_fn fn, void *cb_data) > { > - return

[PATCH] transport: report refs only if transport does

2018-07-30 Thread Jonathan Tan
hed in "fetched_refs" - the excluded refs would be added and reported, presenting an incomplete picture to the caller. Instead, readd the excluded refs only if the transport reported fetched refs. Signed-off-by: Jonathan Tan --- Thanks for the reproduction recipe, Peff. Here's a fix. It ca

Re: [PATCH] negotiator/skipping: skip commits during fetch

2018-07-26 Thread Jonathan Tan
> Hi Jonathan, > > On Mon, 16 Jul 2018, Jonathan Tan wrote: > > > t/t5552-skipping-fetch-negotiator.sh | 179 +++ > > This test seems to be failing consistently in the recent `pu` builds: > > https://git-for-windows.visualstudio.com/git/_bu

Re: [PATCH] negotiator/skipping: skip commits during fetch

2018-07-31 Thread Jonathan Tan
> > +fetch.negotiationAlgorithm:: > > + Control how information about the commits in the local repository is > > + sent when negotiating the contents of the packfile to be sent by the > > + server. Set to "skipping" to use an algorithm that skips commits in an > > + effort to converge

[PATCH] remote: prefer exact matches when using refspecs

2018-07-31 Thread Jonathan Tan
comes after the undesired ref in alphabetical order. However, for completeness, the test in this patch also checks what happens when the desired ref comes first alphabetically. Signed-off-by: Jonathan Tan --- remote.c | 7 +-- t/t5510-fetch.sh | 28 2 file

Re: [PATCH] transport: report refs only if transport does

2018-07-31 Thread Jonathan Tan
> What leaves me even more confused is that the entire log message > does not make it clear what the end-user observable problem the > patch is trying to solve. > > Is this "we sometimes follow and sometimes fail to follow refs while > fetching"? Does it affect all protocol versions and

Re: [PATCH] remote: prefer exact matches when using refspecs

2018-07-31 Thread Jonathan Tan
> That is, something like this, perhaps. The resulting behaviour > should match how "git rev-parse X" would give precedence to tag X > over branch X by going this route. What do you think? [snip] > static const struct ref *find_ref_by_name_abbrev(const struct ref *refs, > const char *name) >

Re: [PATCH] transport: report refs only if transport does

2018-07-31 Thread Jonathan Tan
> On Mon, Jul 30, 2018 at 03:56:01PM -0700, Jonathan Tan wrote: > > > Commit 989b8c4452 ("fetch-pack: put shallow info in output parameter", > > 2018-06-28) allows transports to report the refs that they have fetched > > in a new out-param

Re: [PATCH v2] remote: make refspec follow the same disambiguation rule as local refs

2018-08-01 Thread Jonathan Tan
> +test_expect_success 'LHS of refspec follows ref disambiguation rules' ' > + mkdir lhs-ambiguous && > + ( > + cd lhs-ambiguous && > + git init server && > + test_commit -C server unwanted && > + test_commit -C server wanted && > + > +

Re: [PATCH 0/2] negotiator: improve recent behavior + docs

2018-08-01 Thread Jonathan Tan
> I think 01/02 in this patch series implements something that's better > & more future-proof. Thanks. Both patches are: Reviewed-by: Jonathan Tan A small note: > - packfile; any other value instructs Git to use the default algorithm > + packfile; The default is

[PATCH] fetch-pack: unify ref in and out param

2018-08-01 Thread Jonathan Tan
71806.ga122...@google.com/ [2] https://public-inbox.org/git/86a128c5fb710a41791e7183207c4d64889f9307.1485381677.git.jonathanta...@google.com/ Signed-off-by: Jonathan Tan --- I now think that it's better to revert the API change introducing "fetched_refs" (or as Peff describes it, "this whole 'return

[PATCH v2 2/2] repack: repack promisor objects if -a or -A is set

2018-08-08 Thread Jonathan Tan
individually given, which takes care of deduplication and allows the resulting packfiles to respect flags such as --max-pack-size. Signed-off-by: Jonathan Tan --- Documentation/git-repack.txt | 5 +++ builtin/repack.c | 83 +-- t/t0410-partial-clone.sh

[PATCH v2 0/2] Repacking of promisor packfiles

2018-08-08 Thread Jonathan Tan
objects Peff raised the possibility of making for_each_packed_object() use pack order instead of index order - I'll also take a look at that, separately from this patch series. Jonathan Tan (2): repack: refactor setup of pack-objects cmd repack: repack promisor objects if -a or -A is set

[PATCH v2 1/2] repack: refactor setup of pack-objects cmd

2018-08-08 Thread Jonathan Tan
A subsequent patch will teach repack to run pack-objects with some same and some different arguments if repacking of promisor objects is required. Refactor the setup of the pack-objects cmd so that setting up the arguments common to both is done in a function. Signed-off-by: Jonathan Tan

[RFC PATCH] packfile: iterate packed objects in pack order

2018-08-08 Thread Jonathan Tan
. Teach for_each_object_in_pack() to iterate in pack order by first creating a reverse index. This is based off work by Jeff King. Signed-off-by: Jonathan Tan --- After writing this patch and looking at it further, I'm not sure if this is a clear benefit, but here's the patch anyway. In particular

Re: [RFC PATCH 0/5] filter: support for excluding all trees and blobs

2018-08-10 Thread Jonathan Tan
> Matthew DeVore (5): > revision: invert meaning of the USER_GIVEN flag > list-objects-filter: implement filter only:commits > list-objects: store common func args in struct > list-objects: refactor to process_tree_contents > rev-list: handle missing tree objects properly Firstly, run

Re: [PATCH 1/5] revision: invert meaning of the USER_GIVEN flag

2018-08-10 Thread Jonathan Tan
> Abandon the previous approach of mutating all new objects implicitly in > add_pending_object by inverting the meaning of the bit (it is now > NOT_USER_GIVEN) and only setting the flag when we need to. > > This more accurately tracks if a tree was provided directly by the user. > Without this

[PATCH 2/2] repack: repack promisor objects if -a or -A is set

2018-08-07 Thread Jonathan Tan
individually given, which takes care of deduplication and allows the resulting packfiles to respect flags such as --max-pack-size. Signed-off-by: Jonathan Tan --- Documentation/git-repack.txt | 5 +++ builtin/repack.c | 64 ++-- t/t0410-partial-clone.sh

[PATCH 1/2] repack: refactor setup of pack-objects cmd

2018-08-07 Thread Jonathan Tan
A subsequent patch will teach repack to run pack-objects with some same and some different arguments if repacking of promisor objects is required. Refactor the setup of the pack-objects cmd so that setting up the arguments common to both is done in a function. Signed-off-by: Jonathan Tan

[PATCH 0/2] Repacking of promisor packfiles

2018-08-07 Thread Jonathan Tan
any objects. Let me know if you can think of better ways to do that. Jonathan Tan (2): repack: refactor setup of pack-objects cmd repack: repack promisor objects if -a or -A is set Documentation/git-repack.txt | 5 ++ builtin/repack.c | 163 --- t

Re: [PATCH 2/2] repack: repack promisor objects if -a or -A is set

2018-08-07 Thread Jonathan Tan
> for_each_object_in_pack() is a fine way to make sure that you list > everythning in a pack, but I suspect it is a horrible way to feed a > list of objects to pack-objects, as it goes by the .idx order, which > is by definition a way to enumerate objects in a randomly useless > order. That's

Re: [PATCH 2/2] repack: repack promisor objects if -a or -A is set

2018-08-07 Thread Jonathan Tan
> Just a note (and a request-for-sanity-check) and not meant to be a > request to update the code, but with a still-in-pu 4b757a40 ("Use > remote_odb_get_direct() and has_remote_odb()", 2018-08-02) in > flight, repository_format_partial_clone is now gone. > > I've tentatively resolved the above

Re: [PATCH 0/7] speeding up cat-file by reordering object access

2018-08-13 Thread Jonathan Tan
: test cat-file --batch-all-objects with duplicates > [6/7]: cat-file: rename batch_{loose,packed}_object callbacks > [7/7]: cat-file: support "unordered" output for --batch-all-objects Thanks for laying all the patches out so cleanly! All of them are: Reviewed-by: Jonathan Ta

[PATCH] Documentation: partial clone-related arguments

2018-08-13 Thread Jonathan Tan
Commit 88e2f9ed8e ("introduce fetch-object: fetch one promisor object", 2017-12-05) introduced new command-line arguments without documenting them. Add documentation for these arguments. Signed-off-by: Jonathan Tan --- This is a follow-up to [1] in which Junio noticed some argumen

Re: [PATCH v2 5/5] list-objects-filter: implement filter tree:none

2018-08-13 Thread Jonathan Tan
> - case LOFS_BEGIN_TREE: > - assert(obj->type == OBJ_TREE); > - /* always include all tree objects */ > - return LOFR_MARK_SEEN | LOFR_DO_SHOW; > - > case LOFS_END_TREE: > assert(obj->type == OBJ_TREE); > return LOFR_ZERO;

Re: [PATCH v2 3/5] rev-list: handle missing tree objects properly

2018-08-13 Thread Jonathan Tan
> In list-objects.c we no longer print a message to stderr if a tree > object is missing (quiet_on_missing is always true). I couldn't find > any place where this would matter, or where the caller of > traverse_commit_list would need to be fixed to show the error. However, > in the future it would

Re: [PATCH v4 4/6] rev-list: handle missing tree objects properly

2018-08-14 Thread Jonathan Tan
> Previously, we assumed only blob objects could be missing. This patch > makes rev-list handle missing trees like missing blobs. A missing tree > will cause an error if --missing indicates an error should be caused, > and the hash is printed even if the tree is missing. The last sentence is

Re: [PATCH v4 6/6] list-objects-filter: implement filter tree:0

2018-08-14 Thread Jonathan Tan
> @@ -743,6 +743,9 @@ specification contained in . > A debug option to help with future "partial clone" development. > This option specifies how missing objects are handled. > + > +The form '--filter=tree:' omits all blobs and trees deeper than > + from the root tree. Currently, only

Re: [PATCH v4 6/6] list-objects-filter: implement filter tree:0

2018-08-14 Thread Jonathan Tan
> - grep -E "tree|blob" objs >trees_and_blobs && > - test_line_count = 1 trees_and_blobs > + grep -E "tree|blob" objs \ > + | awk -f print_1.awk >trees_and_blobs && > + git -C r1 rev-parse HEAD: >expected && > + test_cmp trees_and_blobs expected Indent "| awk" (and similar lines in this patch) -

Re: [PATCH v4 4/6] rev-list: handle missing tree objects properly

2018-08-14 Thread Jonathan Tan
> > > @@ -373,6 +375,7 @@ int cmd_rev_list(int argc, const char **argv, const > > > char *prefix) > > > init_revisions(, prefix); > > > revs.abbrev = DEFAULT_ABBREV; > > > revs.commit_format = CMIT_FMT_UNSPECIFIED; > > > + revs.do_not_die_on_missing_tree = 1; > > > > Is this

Re: [PATCH v4 4/6] rev-list: handle missing tree objects properly

2018-08-14 Thread Jonathan Tan
> > So we don't want to die in list-objects.c. If we > > fail to fetch, then we will die on line 213 in rev-list.c. > > Why don't we want to die in list-objects.c? When --missing=error is > passed, fetch_if_missing retains its default value of 1, so > parse_tree_gently() will attempt to fetch it

Re: [PATCH 5/5] rev-list: handle missing tree objects properly

2018-08-09 Thread Jonathan Tan
> @@ -209,7 +210,8 @@ static inline void finish_object__ma(struct object *obj) >*/ > switch (arg_missing_action) { > case MA_ERROR: > - die("missing blob object '%s'", oid_to_hex(>oid)); > + die("missing %s object '%s'", > +

Re: [PATCH 2/5] list-objects-filter: implement filter only:commits

2018-08-09 Thread Jonathan Tan
> Teach list-objects the "only:commits" filter which allows for filtering > out all non-commit and non-annotated tag objects (unless other objects > are explicitly specified by the user). The purpose of this patch is to > allow smaller partial clones. > > The name of this filter - only:commits -

Re: [PATCH v2 2/2] repack: repack promisor objects if -a or -A is set

2018-08-09 Thread Jonathan Tan
On Thu, Aug 9, 2018 at 10:05 AM, Junio C Hamano wrote: > Hmm, it is clever to auto-start the pack-objects (and notice there > wasn't anything needing to pack). Two things that are worth noting > are: > > - The code takes advantage of the fact that cmd.in being left as -1 >is a sign that

Re: [RFC PATCH] packfile: iterate packed objects in pack order

2018-08-09 Thread Jonathan Tan
On Wed, Aug 8, 2018 at 4:25 PM, Jeff King wrote: > Even if you just use the oid to do a separate lookup in the object > database, there's still a benefit in accessing the objects in pack > order. You're probably right, but I don't immediately see what the benefit is. On a not completely

Re: [PATCH v2 3/5] rev-list: handle missing tree objects properly

2018-08-14 Thread Jonathan Tan
> I don't understand about the ">= 0". What should I replace it with? > Maybe you mean the return is never positive so I can change: > > parse_tree_gently(tree, 1) >= 0 > > to: > !parse_tree_gently(tree, 1) > > ? Sorry for the lack of clarity - that is what I meant. > > The missing mechanism

Re: [PATCH 2/2] repack: repack promisor objects if -a or -A is set

2018-08-08 Thread Jonathan Tan
> But what to delta against what else is determined by the pathname > info, which is now lost by enumerating objects without tree/history > walking. By giving phoney pathnames to objects while enumerating > them in offset order and giving similar pathnames to objects closer > to each other, I was

[PATCH] negotiator/skipping: skip commits during fetch

2018-07-16 Thread Jonathan Tan
, but the negotiation step typically ends more quickly. Usage of this algorithm is guarded behind the configuration flag fetch.negotiationAlgorithm. Signed-off-by: Jonathan Tan --- This is on jt/fetch-pack-negotiator, but also applies cleanly on jt/fetch-nego-tip. A previous version of this had

Re: [PATCH 07/16] commit-reach: move can_all_from_reach_with_flags

2018-07-16 Thread Jonathan Tan
> /* Remember to update object flag allocation in object.h */ > +#define REACHABLE (1u<<15) > #define PARENT1 (1u<<16) > #define PARENT2 (1u<<17) > #define STALE(1u<<18) Update the object flag allocation in object.h. > +int reachable(struct

Re: [PATCH 08/16] test-reach: create new test tool for ref_newer

2018-07-16 Thread Jonathan Tan
> To use the new test-tool, use 'test-tool reach ' and provide > input to stdin that describes the inputs to the method. Currently, we > only implement the ref_newer method, which requires two commits. Use > lines "A:" and "B:" for the two inputs. We will > expand this input later to accommodate

Re: [PATCH 01/16] commit-reach: move walk methods from commit.c

2018-07-16 Thread Jonathan Tan
> +/* Remember to update object flag allocation in object.h */ > +#define PARENT1 (1u<<16) > +#define PARENT2 (1u<<17) > +#define STALE(1u<<18) > +#define RESULT (1u<<19) Update object.h to point to commit-reach.c instead of commit.c also.

Re: [PATCH 11/16] test-reach: test get_merge_bases_many

2018-07-16 Thread Jonathan Tan
> @@ -71,6 +78,14 @@ int cmd__reach(int ac, const char **av) > printf("%s(A,B):%d\n", av[1], in_merge_bases(A, B)); > else if (!strcmp(av[1], "is_descendant_of")) > printf("%s(A,X):%d\n", av[1], is_descendant_of(A, X)); > + else if (!strcmp(av[1],

Re: [PATCH 13/16] test-reach: test can_all_from_reach_with_flags

2018-07-16 Thread Jonathan Tan
The subject should be can_all_from_reach_with_flag (without the "s" at the end). Likewise in the commit message. > To make this method testable, add a new can_all_from_reach method that > does the initial setup and final tear-down. Call the method from > 'test-tool reach'. This description leads

Re: [PATCH 15/16] commit-reach: make can_all_from_reach... linear

2018-07-16 Thread Jonathan Tan
> The first step includes using a depth-first-search (DFS) from each from > commit, sorted by ascending generation number. We do not walk beyond the > minimum generation number or the minimum commit date. This DFS is likely > to be faster than the existing reachable() method because we expect >

[PATCH] gc: do not warn about too many loose objects

2018-07-16 Thread Jonathan Tan
ser, as any loose objects left are not deleted yet because of safety, and "git prune" is not a command that the user is recommended to run directly anyway. Therefore, remove this warning. Signed-off-by: Jonathan Tan --- This was noticed when a daemonized gc run wrote this warning to the log fi

Re: [PATCH v2 17/18] commit-reach: make can_all_from_reach... linear

2018-07-23 Thread Jonathan Tan
> + if (parse_commit(list[i]) || > + list[i]->generation < min_generation) Here... > + if (parse_commit(parent->item) || > + parent->item->date < > min_commit_date || > +

Re: [PATCH v2 15/18] test-reach: test commit_contains

2018-07-23 Thread Jonathan Tan
> + } else if (!strcmp(av[1], "commit_contains")) { > + struct ref_filter filter; > + struct contains_cache cache; > + init_contains_cache(); > + > + if (ac > 2 && !strcmp(av[2], "--tag")) > + filter.with_commit_tag_algo = 1;

Re: What's cooking in git.git (Jul 2018, #02; Wed, 18)

2018-07-23 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. What do you think about

[PATCH v2 6/6] commit-graph: add repo arg to graph readers

2018-07-09 Thread Jonathan Tan
removed. Instead, the config option core.commitGraph is now read on the first time in a repository that a commit is attempted to be parsed using its commit graph. This commit includes a test that exercises the functionality on an arbitrary repository that is not the_repository. Signed-off-by: Jonathan

[PATCH v2 4/6] commit-graph: add free_commit_graph

2018-07-09 Thread Jonathan Tan
Signed-off-by: Jonathan Tan --- builtin/commit-graph.c | 2 ++ commit-graph.c | 24 ++-- commit-graph.h | 2 ++ 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index c7d0db5ab4..0bf0c48657

[PATCH v2 on ds/commit-graph-fsck 0/6] Object store refactoring: commit graph

2018-07-09 Thread Jonathan Tan
atches out. I've also added a patch (patch 1) that removes some duplication of implementation that Junio talked about in [1]. [1] https://public-inbox.org/git/xmqqefgtmrgi@gitster-ct.c.googlers.com/ Jonathan Tan (6): commit-graph: refactor preparing commit graph object-store: add missi

[PATCH v2 3/6] commit-graph: add missing forward declaration

2018-07-09 Thread Jonathan Tan
Signed-off-by: Jonathan Tan --- commit-graph.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/commit-graph.h b/commit-graph.h index 506cb45fb1..674052bef4 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -5,6 +5,8 @@ #include "repository.h" #include "string-list.h&qu

[PATCH v2 1/6] commit-graph: refactor preparing commit graph

2018-07-09 Thread Jonathan Tan
Two functions in the code (1) check if the repository is configured for commit graphs, (2) call prepare_commit_graph(), and (3) check if the graph exists. Move (1) and (3) into prepare_commit_graph(), reducing duplication of code. Signed-off-by: Jonathan Tan --- commit-graph.c | 28

[PATCH v2 5/6] commit-graph: store graph in struct object_store

2018-07-09 Thread Jonathan Tan
-by: Jonathan Tan --- commit-graph.c | 40 +++- object-store.h | 3 +++ object.c | 5 + 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/commit-graph.c b/commit-graph.c index 6d1bc4ff7c..af97a10603 100644 --- a/commit-graph.c +++ b

[PATCH v2 2/6] object-store: add missing include

2018-07-09 Thread Jonathan Tan
Signed-off-by: Jonathan Tan --- object-store.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/object-store.h b/object-store.h index d683112fd7..f0b77146e4 100644 --- a/object-store.h +++ b/object-store.h @@ -2,6 +2,9 @@ #define OBJECT_STORE_H #include "oidmap.h" +#inclu

Re: [PATCH 0/2] Avoiding errors when partial cloning a tagged blob

2018-07-09 Thread Jonathan Tan
> > An argument could be made that we should not merge patch 2 just yet due > > to the fact that some server implementations (such as Git and JGit) > > still exhibit the old behavior, and the resulting clones (albeit failing > > fsck) are still usable, because when attempting to load the blob, Git

Re: [PATCH v2 6/6] commit-graph: add repo arg to graph readers

2018-07-10 Thread Jonathan Tan
On Tue, Jul 10, 2018 at 6:18 AM, Johannes Schindelin wrote: >> 32-bit builds complain about this: >> >> t/helper/test-repository.c: In function 'test_parse_commit_in_graph': >> t/helper/test-repository.c:28:9: error: format '%lu' expects argument of >> type 'long unsigned int', but argument

Re: [PATCH v2 6/6] commit-graph: add repo arg to graph readers

2018-07-10 Thread Jonathan Tan
> > - if (!core_commit_graph) > > + if (repo_config_get_bool(r, "core.commitgraph", _value) || > > + !config_value) > > + /* > > +* This repository is not configured to use commit graphs, so > > +* do not load one. (But report commit_graph_attempted

[PATCH 1/2] upload-pack: send refs' objects despite "filter"

2018-07-06 Thread Jonathan Tan
uch that the missing object is a promisor object. Update both the protocol and the upload-pack implementation to include all explicitly specified "want" objects in the packfile regardless of the filter specification. Signed-off-by: Jonathan Tan --- Documentation/technical/pack-protocol

[PATCH 2/2] clone: check connectivity even if clone is partial

2018-07-06 Thread Jonathan Tan
if cloning with --filter=blob:none from a repository that has a tag pointing to a blob, and the blob is not sent in the packfile, the clone will pass, even if the blob is not referenced by any tree in the packfile. Turn on connectivity checks for partial clone. Signed-off-by: Jonathan Tan --- built

Re: [PATCH 0/2] Avoiding errors when partial cloning a tagged blob

2018-07-06 Thread Jonathan Tan
> Hmph, the approach taken by these two patches smells bad. > > When a blob is deliberately omitted with --fitler=blob:none, the > fsck that encounters an entry in a tree object that is about that > expected-to-be-and-actually-is-missing blob does *not* complain and > that is by design, right?

Re: [PATCH 0/2] Avoiding errors when partial cloning a tagged blob

2018-07-06 Thread Jonathan Tan
bit the old behavior, and the resulting clones (albeit failing > fsck) are still usable, because when attempting to load the blob, Git > will automatically fetch it. I'm on the fence about this, and have > included patch 2 in this patch set nevertheless for completeness. > > Jonatha

[PATCH 0/2] Avoiding errors when partial cloning a tagged blob

2018-07-06 Thread Jonathan Tan
) are still usable, because when attempting to load the blob, Git will automatically fetch it. I'm on the fence about this, and have included patch 2 in this patch set nevertheless for completeness. Jonathan Tan (2): upload-pack: send refs' objects despite "filter" clone: check connect

[PATCH 1/2] revision: tolerate promised targets of tags

2018-07-12 Thread Jonathan Tan
cloning from a repository with an annotated tag pointing to a blob. The test included in this patch demonstrates this case. Signed-off-by: Jonathan Tan --- --- revision.c | 3 +++ t/t5616-partial-clone.sh | 39 +++ 2 files changed, 42 insertions

[PATCH 0/2] Annotated tags pointing to missing but promised blobs

2018-07-12 Thread Jonathan Tan
to tags pointing to blobs. Here are some fixes. Jonathan Tan (2): revision: tolerate promised targets of tags tag: don't warn if target is missing but promised revision.c | 3 +++ t/t5616-partial-clone.sh | 44 tag.c| 13

[PATCH 2/2] tag: don't warn if target is missing but promised

2018-07-12 Thread Jonathan Tan
that happens after a partial clone causes some objects to be fetched - and as part of the fetch, all local refs are read. The test included in this patch demonstrates this situation. Therefore, do not print a warning in this case. Signed-off-by: Jonathan Tan --- t/t5616-partial-clone.sh | 9

Re: [PATCH v2 6/6] commit-graph: add repo arg to graph readers

2018-07-11 Thread Jonathan Tan
> >> Also, this will conflict with sb/object-store-lookup, won't it? I'm > >> guessing this is why you didn't touch the "git commit-graph > >> [write|verify]"code paths. > > It will conflict because of the change to lookup_commit(), but the only > > new code I'm writing is in

[PATCH v3 3/6] commit-graph: add missing forward declaration

2018-07-11 Thread Jonathan Tan
Signed-off-by: Jonathan Tan --- commit-graph.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/commit-graph.h b/commit-graph.h index 506cb45fb1..674052bef4 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -5,6 +5,8 @@ #include "repository.h" #include "string-list.h&qu

[PATCH v3 6/6] commit-graph: add repo arg to graph readers

2018-07-11 Thread Jonathan Tan
removed. Instead, the config option core.commitGraph is now read on the first time in a repository that a commit is attempted to be parsed using its commit graph. This commit includes a test that exercises the functionality on an arbitrary repository that is not the_repository. Signed-off-by: Jonathan

[PATCH v3 4/6] commit-graph: add free_commit_graph

2018-07-11 Thread Jonathan Tan
Signed-off-by: Jonathan Tan --- builtin/commit-graph.c | 2 ++ commit-graph.c | 24 ++-- commit-graph.h | 2 ++ 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index c7d0db5ab4..0bf0c48657

[PATCH v3 2/6] object-store: add missing include

2018-07-11 Thread Jonathan Tan
Signed-off-by: Jonathan Tan --- object-store.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/object-store.h b/object-store.h index a3db17bbf5..0e13543bab 100644 --- a/object-store.h +++ b/object-store.h @@ -2,6 +2,9 @@ #define OBJECT_STORE_H #include "oidmap.h" +#inclu

<    4   5   6   7   8   9   10   11   >