[WIP 2/2] pack-objects: support --blob-size-limit

2017-06-01 Thread Jonathan Tan
ct in to_pack which will be excluded in the final pack generation". The other method in which objects are added to "to_pack", add_object_entry_from_bitmap(), has not been modified - thus packing in the presence of bitmaps still packs all blobs regardless of size. See the documentation u

[WIP 0/2] Modifying pack-objects to support --blob-size-limit

2017-06-01 Thread Jonathan Tan
rosoft.com/ Jonathan Tan (2): pack-objects: rename want_.* to ignore_.* pack-objects: support --blob-size-limit Documentation/git-pack-objects.txt | 19 - builtin/pack-objects.c | 163 ++--- t/t5300-pack-object.sh | 71 +++

[WIP 1/2] pack-objects: rename want_.* to ignore_.*

2017-06-01 Thread Jonathan Tan
ase" instead of "excluded" in these methods. It is true that preferred bases are not included in the final packfile generation, but at this point in the code, there is no exclusion taking place - on the contrary, if something is "excluded", it is in fact guaranteed to be in

[PATCH] send-email: check for repo before invoking hook

2017-06-01 Thread Jonathan Tan
Unless --no-validate is passed, send-email will invoke $repo->repo_path() in its search for a validate hook regardless of whether a Git repo is actually present. Teach send-email to first check for repo existence. Signed-off-by: Jonathan Tan <jonathanta...@google.com> -

Re: [PATCHv3 20/20] diff.c: color moved lines differently

2017-05-19 Thread Jonathan Tan
On Fri, May 19, 2017 at 11:40 AM, Stefan Beller wrote: >>> +static unsigned get_line_hash(struct buffered_patch_line *line, unsigned >>> ignore_ws) >>> +{ >>> + static struct strbuf sb = STRBUF_INIT; >>> + >>> + if (ignore_ws) { >>> + strbuf_reset(); >>> +

Re: [PATCHv3 20/20] diff.c: color moved lines differently

2017-05-19 Thread Jonathan Tan
On Thu, 18 May 2017 12:37:46 -0700 Stefan Beller wrote: [snip] > Instead this provides a dynamic programming greedy algorithm that Not sure if this is called "dynamic programming". > finds the largest moved hunk and then switches color to the > alternative color for the

Re: [PATCHv3 04/20] diff.c: teach emit_line_0 to accept sign parameter

2017-05-18 Thread Jonathan Tan
On Thu, 18 May 2017 12:37:30 -0700 Stefan Beller wrote: > Teach emit_line_0 take a "sign" parameter specifically intended > to hold the sign of the line instead of a separate "first" parameter > representing the first character of the line to be printed. Callers > that store

Re: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to speed up detecting new or changed files.

2017-05-16 Thread Jonathan Tan
I'm not very familiar with this part of the code - here is a partial review. Firstly, if someone invokes update-index, I wonder if it's better just to do a full refresh (e.g. by deleting the last_update time from the index). Also, the change to unpack-trees.c doesn't match my mental model. I

Re: [PATCH 03/19] diff.c: drop 'nofirst' from emit_line_0

2017-05-16 Thread Jonathan Tan
On Mon, May 15, 2017 at 11:33 AM, Stefan Beller <sbel...@google.com> wrote: > On Mon, May 15, 2017 at 11:26 AM, Jonathan Tan <jonathanta...@google.com> > wrote: >> I also don't understand the meaning of this paragraph - if you mean that >> this patch teaches other c

Re: [PATCH 19/19] diff.c: color moved lines differently

2017-05-15 Thread Jonathan Tan
I expected there to be one main function that takes in a diff options and returns the appropriate output without much (if any) changes in other functions...but (as with the previous patch) maybe there are some complications that I didn't foresee. On Sat, May 13, 2017 at 9:01 PM, Stefan Beller

Re: [PATCH 18/19] diff: buffer all output if asked to

2017-05-15 Thread Jonathan Tan
Overall, this patch seems larger than it should to me, although there might be good reasons for that that I don't know. I'll remark on what I find unexpected. On Sat, May 13, 2017 at 9:01 PM, Stefan Beller wrote: > diff --git a/diff.c b/diff.c > index 08dcc56bb9..dbab7fb44e

Re: [PATCH 14/19] diff.c: convert word diffing to use emit_line_*

2017-05-15 Thread Jonathan Tan
On 05/13/2017 09:01 PM, Stefan Beller wrote: In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+

Re: [PATCH 15/19] diff.c: convert diff_flush to use emit_line_*

2017-05-15 Thread Jonathan Tan
On 05/13/2017 09:01 PM, Stefan Beller wrote: In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+

Re: [PATCH 10/19] diff.c: convert emit_rewrite_lines to use emit_line_*

2017-05-15 Thread Jonathan Tan
On 05/13/2017 09:01 PM, Stefan Beller wrote: In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines

Re: [PATCH 08/19] diff.c: convert builtin_diff to use emit_line_*

2017-05-15 Thread Jonathan Tan
On 05/13/2017 09:01 PM, Stefan Beller wrote: In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+

Re: [PATCH 05/19] diff.c: emit_line_0 can handle no color setting

2017-05-15 Thread Jonathan Tan
On 05/13/2017 09:01 PM, Stefan Beller wrote: In a later patch, I want to propose an option to detect moved lines in a diff, which cannot be done in a one-pass over the diff. Instead we need to go over the whole diff twice, because we cannot detect the first line of the two corresponding lines (+

Re: [PATCH 03/19] diff.c: drop 'nofirst' from emit_line_0

2017-05-15 Thread Jonathan Tan
On 05/13/2017 09:01 PM, Stefan Beller wrote: In 250f79930d (diff.c: split emit_line() from the first char and the rest of the line, 2009-09-14) we introduced the local variable 'nofirst' that indicates if we have no first sign character. With the given implementation we had to use an extra

Re: [PATCH v2] send-email: support validate hook

2017-05-15 Thread Jonathan Tan
On 05/14/2017 06:55 PM, Junio C Hamano wrote: Jonathan Tan <jonathanta...@google.com> writes: diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index 706091a56..b2514f4d4 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -447,6 +447,14 @@

[PATCH v7] fetch-pack: always allow fetching of literal SHA1s

2017-05-15 Thread Jonathan Tan
the received ref advertisement if a literal SHA-1 was given by the user. Helped-by: Jeff King <p...@peff.net> Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- OK, one combined function (for lazy initialization and checking containment in the oidset) with comment it is. Change

[PATCH v2] send-email: support validate hook

2017-05-12 Thread Jonathan Tan
Currently, send-email has support for rudimentary e-mail validation. Allow the user to add support for more validation by providing a sendemail-validate hook. Helped-by: Ævar Arnfjörð Bjarmason <ava...@gmail.com> Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Change from

Re: [RFC] send-email: support validate hook

2017-05-12 Thread Jonathan Tan
On 05/12/2017 12:23 AM, Ævar Arnfjörð Bjarmason wrote: I hacked this up last night, it also addresses Junio's comment about GIT_DIR: [snip] Changes there: * use catdir instead of string concat, I don't know if we run format-patch on any platform where this matters in theory (e.g. VMS I

[PATCH v6] fetch-pack: always allow fetching of literal SHA1s

2017-05-12 Thread Jonathan Tan
the received ref advertisement if a literal SHA-1 was given by the user. Helped-by: Jeff King <p...@peff.net> Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Change from v5: used "ensure_tip_oids_initialized" function instead. This removes some of the muddiness (e.g. with

[PATCH v5] fetch-pack: always allow fetching of literal SHA1s

2017-05-12 Thread Jonathan Tan
the received ref advertisement if a literal SHA-1 was given by the user. Helped-by: Jeff King <p...@peff.net> Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Change from v5: used "ensure_tip_oids_initialized" function instead. This removes some of the muddiness (e.g. with

Re: [PATCH v5] fetch-pack: always allow fetching of literal SHA1s

2017-05-12 Thread Jonathan Tan
On Fri, May 12, 2017 at 12:59 AM, Jeff King wrote: > On Fri, May 12, 2017 at 03:01:35PM +0900, Junio C Hamano wrote: >> Also, tip_oids_contain() uses unmatched and newlist only on the >> first call, but the internal API this patch establishes gives an >> illusion (confusion) that

Re: [PATCH v5] fetch-pack: always allow fetching of literal SHA1s

2017-05-12 Thread Jonathan Tan
On Fri, May 12, 2017 at 1:14 AM, Jeff King wrote: > diff --git a/fetch-pack.c b/fetch-pack.c > index afb8b0502..e167213c0 100644 > --- a/fetch-pack.c > +++ b/fetch-pack.c > @@ -15,6 +15,7 @@ > #include "version.h" > #include "prio-queue.h" > #include "sha1-array.h" > +#include

[PATCH v5] fetch-pack: always allow fetching of literal SHA1s

2017-05-11 Thread Jonathan Tan
the received ref advertisement if a literal SHA-1 was given by the user. Helped-by: Jeff King <p...@peff.net> Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Change from v4: incorporated Jonathan Nieder's suggestion about using another function. There is no oidset_is_empty, so I c

[PATCH v4] fetch-pack: always allow fetching of literal SHA1s

2017-05-11 Thread Jonathan Tan
the received ref advertisement if a literal SHA-1 was given by the user. Helped-by: Jeff King <p...@peff.net> Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Thanks, peff. I've incorporated your suggestions - I don't feel very strongly about this, but I guess it's worthwhile to avo

[RFC] send-email: support validate hook

2017-05-11 Thread Jonathan Tan
Currently, send-email has support for rudimentary e-mail validation. Allow the user to add support for more validation by providing a sendemail-validate hook. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- This is motivated by situations in which we discuss a work in progress

Re: [PATCH v3] fetch-pack: always allow fetching of literal SHA1s

2017-05-11 Thread Jonathan Tan
, Jeff King wrote: On Wed, May 10, 2017 at 03:11:57PM -0700, Jonathan Tan wrote: After looking at ways to solve jrnieder's performance concerns, if we're going to need to manage one more item of state within the function, I might as well use my earlier idea of storing unmatched refs in its ow

[PATCH v3] fetch-pack: always allow fetching of literal SHA1s

2017-05-10 Thread Jonathan Tan
the received ref advertisement if a literal SHA-1 was given by the user. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Reviewers, note that the patch has been substantially rewritten. After looking at ways to solve jrnieder's performance concerns, if we're going to need to manage one mo

[PATCH v2] fetch-pack: always allow fetching of literal SHA1s

2017-05-10 Thread Jonathan Tan
the received ref advertisement if a literal SHA-1 was given by the user. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Thanks, peff, for your suggestions. Changes from v1: - updated commit message to explain a bit of the context - use parse_oid_hex instead of get_oid_hex to avoid

Re: [PATCH] fixup! use perl instead of sed

2017-05-09 Thread Jonathan Tan
On 05/09/2017 01:43 PM, Johannes Sixt wrote: Am 09.05.2017 um 19:00 schrieb Ævar Arnfjörð Bjarmason: Finally, you can just use -i like you did with sed, no need for the tempfile: Nope. Some implementations of perl attempt to remove the file that it has just opened. That doesn't work on

[PATCH v3] fixup! don't use perl -i because it's not portable

2017-05-09 Thread Jonathan Tan
Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Ah...thanks, Johannes, for spotting this. Here's a fixup. t/t5534-push-signed.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh index 177b933a7..807267b7f

[PATCH v3 2/2] receive-pack: verify push options in cert

2017-05-09 Thread Jonathan Tan
ut I think that this is better than the alternatives. Sending push options only within the cert is backwards-incompatible with existing Git servers (which read push options only from outside the cert), and sending push options only outside the cert means that the push options are not signed for. Signed-off-by: Jo

[PATCH v3 0/2] Clarify interaction between signed pushes and push options

2017-05-09 Thread Jonathan Tan
Changes from v2: - incorporated Junio's suggestions - incorporated Ævar's suggestions Jonathan Tan (2): docs: correct receive.advertisePushOptions default receive-pack: verify push options in cert Documentation/config.txt | 5 ++- Documentation/technical/pack

[PATCH v3 1/2] docs: correct receive.advertisePushOptions default

2017-05-09 Thread Jonathan Tan
t; however, it actually does not. (In that commit, notice that advertise_push_options defaults to 0, unlike advertise_atomic_push which defaults to 1.) Update the documentation to state that it does not advertise the ability by default. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --

[PATCH] fetch-pack: always allow fetching of literal SHA1s

2017-05-09 Thread Jonathan Tan
advertised. Teach fetch-pack to also check the SHA-1s of the refs in the received ref advertisement if a literal SHA-1 was given by the user. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- fetch-pack.c | 30 -- t/t5500-fetch-pack.sh | 6

[PATCH] fixup! use perl instead of sed

2017-05-09 Thread Jonathan Tan
Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Thanks - I didn't realize the existence of the test lint. Here's a fixup. Let me know if you prefer a full reroll. I had to use perl because "push" is a binary file (the first line contains a NUL). t/t5534-push-signe

Re: [PATCH v3 00/53] object_id part 8

2017-05-08 Thread Jonathan Tan
On 05/06/2017 03:09 PM, brian m. carlson wrote: This is the eighth series of patches to convert unsigned char [20] to struct object_id. This series converts lookup_commit, lookup_blob, lookup_tree, lookup_tag, and finally parse_object to struct object_id. I patched v2 (which I have reviewed)

[PATCH v2 1/2] docs: correct receive.advertisePushOptions default

2017-05-08 Thread Jonathan Tan
t; however, it actually does not. (In that commit, notice that advertise_push_options defaults to 0, unlike advertise_atomic_push which defaults to 1.) Update the documentation to state that it does not advertise the ability by default. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --

[PATCH v2 2/2] receive-pack: verify push options in cert

2017-05-08 Thread Jonathan Tan
ut I think that this is better than the alternatives. Sending push options only within the cert is backwards-incompatible with existing Git servers (which read push options only from outside the cert), and sending push options only outside the cert means that the push options are not signed for. Signed-off-by: Jo

[PATCH v2 0/2] Clarify interaction between signed pushes and push options

2017-05-08 Thread Jonathan Tan
t to check failure (in addition to success) - modified pack-protocol.txt to describe new behavior Jonathan Tan (2): docs: correct receive.advertisePushOptions default receive-pack: verify push options in cert Documentation/config.txt | 5 ++- Documentation/technical/pack-pr

Re: [PATCH 3/3] protocol docs: explain receive-pack push options

2017-05-08 Thread Jonathan Tan
On 05/05/2017 05:26 PM, Jonathan Nieder wrote: -This list is followed by a flush-pkt. Then the push options are transmitted -one per packet followed by another flush-pkt. After that the packfile that -should contain all the objects that the server will need to complete the new -references will

[PATCH 2/3] receive-pack: verify push options in cert

2017-05-05 Thread Jonathan Tan
sh options only from outside the cert), and sending push options only outside the cert means that the push options are not signed for. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Documentation/git-receive-pack.txt | 10 builtin/receive-pack.c

[PATCH 1/3] docs: correct receive.advertisePushOptions default

2017-05-05 Thread Jonathan Tan
t; however, it actually does not. (In that commit, notice that advertise_push_options defaults to 0, unlike advertise_atomic_push which defaults to 1.) Update the documentation to state that it does not advertise the ability by default. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --

[PATCH 3/3] protocol docs: explain receive-pack push options

2017-05-05 Thread Jonathan Tan
Support for push options in the receive-pack protocol (and all Git components that speak it) have been added over a few commits, but not fully documented (especially its interaction with signed pushes). Update the protocol documentation to include the relevant details. Signed-off-by: Jonathan Tan

[PATCH 0/3] Clarify interaction between signed pushes and push options

2017-05-05 Thread Jonathan Tan
umentation) that clarify this interaction. I would appreciate a review of this - if we could make the protocol clear, we could then update JGit to use the updated protocol and be no longer incompatible with existing Git clients. Jonathan Tan (3): docs: correct receive.advertisePushOptions default re

Re: Proposal for missing blob support in Git repos

2017-05-04 Thread Jonathan Tan
On 05/03/2017 09:29 PM, Junio C Hamano wrote: Jonathan Tan <jonathanta...@google.com> writes: I see the semantics as "don't write what you already have", where "have" means what you have in local storage, but if you extend "have" to what upstream has, then

Re: [PATCH 00/10] RFC Partial Clone and Fetch

2017-05-03 Thread Jonathan Tan
allow the client to maybe choose between various servers. Since I first posted this, Jonathan Tan has started a related discussion on missing blob support. https://public-inbox.org/git/cagf8dgk05+f4ux-8+imfvqd0n2jp6yxj18ag8udaeh6qc6s...@mail.gmail.com/T/ I want to respond to both of these thread

Re: Proposal for missing blob support in Git repos

2017-05-02 Thread Jonathan Tan
On 05/02/2017 11:32 AM, Ævar Arnfjörð Bjarmason wrote: On Tue, May 2, 2017 at 7:21 PM, Jonathan Tan <jonathanta...@google.com> wrote: On Mon, May 1, 2017 at 6:41 PM, Junio C Hamano <gits...@pobox.com> wrote: Jonathan Tan <jonathanta...@google.com> writes: On 05/01/2017

Re: Proposal for missing blob support in Git repos

2017-05-01 Thread Jonathan Tan
On 05/01/2017 04:29 PM, Junio C Hamano wrote: Jonathan Tan <jonathanta...@google.com> writes: Thanks for your comments. If you're referring to the codepath involving write_sha1_file() (for example, builtin/hash-object -> index_fd or builtin/unpack-objects), that is fine because write_

Re: [PATCH v2 53/53] object: convert parse_object* to take struct object_id

2017-05-01 Thread Jonathan Tan
On 04/30/2017 07:29 PM, brian m. carlson wrote: Make parse_object, parse_object_or_die, and parse_object_buffer take a pointer to struct object_id. Remove the temporary variables inserted earlier, since they are no longer necessary. Transform all of the callers using the following semantic

Re: [PATCH v2 39/53] refs/files-backend: convert many internals to struct object_id

2017-05-01 Thread Jonathan Tan
On 04/30/2017 07:29 PM, brian m. carlson wrote: @@ -195,27 +195,18 @@ static const char PACKED_REFS_HEADER[] = * Return a pointer to the refname within the line (null-terminated), * or NULL if there was a problem. */ -static const char *parse_ref_line(struct strbuf *line, unsigned char

Re: [PATCH v2 11/53] fast-import: convert to struct object_id

2017-05-01 Thread Jonathan Tan
On 05/01/2017 03:27 PM, Jeff King wrote: On Mon, May 01, 2017 at 03:07:22PM -0700, Jonathan Tan wrote: @@ -2298,8 +2296,12 @@ static uintmax_t do_change_note_fanout( static uintmax_t change_note_fanout(struct tree_entry *root, unsigned char fanout) { - char hex_sha1[40

Re: [PATCH v2 11/53] fast-import: convert to struct object_id

2017-05-01 Thread Jonathan Tan
On 04/30/2017 07:29 PM, brian m. carlson wrote: @@ -391,10 +391,8 @@ static void write_branch_report(FILE *rpt, struct branch *b) fputc('\n', rpt); fprintf(rpt, " tip commit : %s\n", oid_to_hex(>oid)); - fprintf(rpt, " old tree: %s\n", -

Re: [PATCH v2 09/53] builtin/rev-parse: convert to struct object_id

2017-05-01 Thread Jonathan Tan
On 04/30/2017 07:29 PM, brian m. carlson wrote: @@ -340,7 +340,7 @@ static int try_parent_shorthands(const char *arg) } if (include_rev) - show_rev(NORMAL, sha1, arg); + show_rev(NORMAL, , arg); for (parents = commit->parents, parent_number =

Re: Proposal for missing blob support in Git repos

2017-05-01 Thread Jonathan Tan
On 04/30/2017 08:57 PM, Junio C Hamano wrote: One thing I wonder is what the performance impact of a change like this to the codepath that wants to see if an object does _not_ exist in the repository. When creating a new object by hashing raw data, we see if an object with the same name already

Re: Proposal for "fetch-any-blob Git protocol" and server design

2017-04-26 Thread Jonathan Tan
On 04/21/2017 09:41 AM, Kevin David wrote: Hi Jonathan, Sorry for the delayed response - other work got in the way, unfortunately! No worries! I am envisioning (1a) as described in Jeff Hostetler's e-mail [1] ("a pre-command or hook to identify needed blobs and pre-fetch them before

Proposal for missing blob support in Git repos

2017-04-26 Thread Jonathan Tan
repos that only want to exclude large blobs), and might be tolerable in the future if we have batching support for the most commonly used commands, but is not tolerable now for repos that exclude a large amount of blobs. Signed-off-by: Jonathan Tan <jonathanta...@google.com&g

[PATCH v3] sequencer: add newline before adding footers

2017-04-26 Thread Jonathan Tan
e if it does not end in one before checking the footer for conformity. Reported-by: Brian Norris <computersforpe...@gmail.com> Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- The failure in the case of non-footers not being terminated by a newline turns out to be quite eas

[PATCH v2] sequencer: require trailing NL in footers

2017-04-25 Thread Jonathan Tan
plaining this in the function's documentation. Reported-by: Brian Norris <computersforpe...@gmail.com> Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- jrnieder pointed out the existence of commit-tree to me, which I have used to write the test below. Changes from v1: - added te

Re: [PATCH] sequencer: require trailing NL in footers

2017-04-25 Thread Jonathan Tan
On 04/25/2017 02:04 PM, Stefan Beller wrote: Thanks for the fix. :) Do we want to test for this use case in the future? Thanks! I'm not sure of the value of including a test for this specific use case, because Git normally does not create commit messages with no trailing newlines. (To test

[PATCH] sequencer: require trailing NL in footers

2017-04-25 Thread Jonathan Tan
plaining this in the function's documentation. Reported-by: Brian Norris <computersforpe...@gmail.com> Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Thanks for the bug report. Here's a fix - I've verified this with the way to reproduce provided in the or

Re: [PATCH v2] fetch-pack: show clearer error message upon ERR

2017-04-17 Thread Jonathan Tan
ve changed in this new version. I have also done one of his follow-up ideas (updating the documentation) in this patch. [1] http://public-inbox.org/git/xmqqmvbfij92@gitster.mtv.corp.google.com/ On 04/12/2017 11:06 AM, Jonathan Tan wrote: Currently, fetch-pack prints a confusing err

Re: [PATCH 3/4] submodule.c: harden submodule_move_head against broken submodules

2017-04-14 Thread Jonathan Tan
I think "harden" is the wrong word to use in the subject - to me, "harden" implies that you're defending against an invalid use. But here, not only is the use valid, but this patch also takes steps to support it. On 04/11/2017 04:49 PM, Stefan Beller wrote: Early on in submodule_move_head

Re: [PATCH 1/4] entry.c: submodule recursing: respect force flag correctly

2017-04-14 Thread Jonathan Tan
On 04/11/2017 04:49 PM, Stefan Beller wrote: In case of a non-forced worktree update, the submodule movement is tested in a dry run first, such that it doesn't matter if the actual update is done via the force flag. However for correctness, we want to give the flag is specified by the user. All

Re: Proposal for "fetch-any-blob Git protocol" and server design

2017-04-13 Thread Jonathan Tan
On 04/12/2017 03:02 PM, Kevin David wrote: Hi Jonathan, I work on the network protocols for the GVFS project at Microsoft. I shared a couple thoughts and questions below. Thanks for your reply! I know we're considering server behavior here, but how large do you generally expect these

Re: [PATCH] fetch-pack: show clearer error message upon ERR

2017-04-12 Thread Jonathan Tan
On 04/11/2017 02:47 PM, Jonathan Nieder wrote: nit about the error message: since this isn't a sign of an internal error, we don't need to tell the user that it happened in git fetch-pack. How about using the same error used e.g. in run_remote_archiver and get_remote_heads?

[PATCH v2] fetch-pack: show clearer error message upon ERR

2017-04-12 Thread Jonathan Tan
pload-pack: report "not our ref" to client", 2017-02-23) whenever a "want" line references an object that it does not have. (This is uncommon, but can happen if a repository is garbage-collected during a negotiation.) Signed-off-by: Jonathan Tan <jonathanta...@go

[PATCH] fetch-pack: show clearer error message upon ERR

2017-04-10 Thread Jonathan Tan
ferences an object that it does not have. This is uncommon, but can happen if a repository is garbage-collected during a negotiation.) Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- This situation has been noticed occasionally in my company - this is a small change that would ma

[RFC 4/4] server-endpoint: serve blobs by hash

2017-04-10 Thread Jonathan Tan
quot; in the absence of bitmaps (discussed here [1]), the server repositories in tests all have bitmaps. [1] <20170309003547.6930-1-jonathanta...@google.com> Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- server-endpoint.c | 121 +

[RFC 0/4] Implementation of fetch-blobs and fetch-refs

2017-04-10 Thread Jonathan Tan
[1] <ffd92ad9-39fe-c76b-178d-6e3d6a425...@google.com> Jonathan Tan (4): server-endpoint: serve refs without advertisement fetch-pack: refactor "want" pkt-line generation fetch-pack: support new server endpoint server-endpoint: serve blobs by hash .gitignore

[RFC 2/4] fetch-pack: refactor "want" pkt-line generation

2017-04-10 Thread Jonathan Tan
In fetch-pack, refactor the generation of the initial request (containing the "want" lines) into its own function. This cleans up the code slightly in that the scopes of certain variables are reduced, but this commit mainly is in preparation for a subsequent one. Signed-off-by: Jo

[RFC 1/4] server-endpoint: serve refs without advertisement

2017-04-10 Thread Jonathan Tan
oad-pack, fetch-pack, and now server-endpoint) so I'm inclined to just forbid it (like in the current code). Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- .gitignore| 1 + Makefile | 2 + server-endpoint.c | 228 +

[RFC 3/4] fetch-pack: support new server endpoint

2017-04-10 Thread Jonathan Tan
the upload-pack code path that sends "ACK ready". Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Makefile | 1 + builtin/fetch-pack.c | 10 +- fetch-pack.c | 75 +++ fetch-pack.h | 1 + t/helper/.gitignore| 1

Re: [PATCH] Fix 'git am' in-body header continuations

2017-04-03 Thread Jonathan Tan
dle in-body header continuations") Cc: Jonathan Tan <jonathanta...@google.com> Cc: Jeff King <p...@peff.net> Signed-off-by: Linus Torvalds <torva...@linux-foundation.org> --- diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 89a5bacac..44807e218 100755 --- a/t/t4150-am.sh ++

Re: Bug in "git am" when the body starts with spaces

2017-04-03 Thread Jonathan Tan
Thanks, everyone, for looking into this. On 04/01/2017 09:18 PM, Jeff King wrote: On Sat, Apr 01, 2017 at 12:03:44PM -0700, Linus Torvalds wrote: The logic is fairly simple: if we encounter an empty line, and we have pending in-body headers, we flush the pending headers, and mark us as no

Re: [PATCH v2 2/2] sub-process: refactor the filter process code into a reusable module

2017-03-27 Thread Jonathan Tan
On 03/24/2017 08:27 AM, Ben Peart wrote: Refactor the filter..process code into a separate sub-process module that can be used to reduce the cost of starting up a sub-process for multiple commands. It does this by keeping the external process running and processing all commands by communicating

Re: Proposal for "fetch-any-blob Git protocol" and server design

2017-03-16 Thread Jonathan Tan
On 03/16/2017 02:17 PM, Junio C Hamano wrote: Yeah, the example was solely to see how the system was to be extended, as one of the selling point of the proposal was: > === Endpoint support for forward compatibility > > This "server" endpoint requires that the first line be

Re: Proposal for "fetch-any-blob Git protocol" and server design

2017-03-16 Thread Jonathan Tan
On 03/15/2017 10:59 AM, Junio C Hamano wrote: By "SHA-1s for which it wants blobs", you mean that "want" only allows one exact blob object name? I think it is necessary to support that mode of operation as a base case, and it is a good starting point. When you know - you have a "partial"

Proposal for "fetch-any-blob Git protocol" and server design

2017-03-14 Thread Jonathan Tan
As described in "Background" below, there have been at least 2 patch sets to support "partial clones" and on-demand blob fetches, where the server part that supports on-demand blob fetches was treated at least in outline. Here is a proposal treating that server part in detail. == Background

Re: What's cooking in git.git (Mar 2017, #05; Mon, 13)

2017-03-14 Thread Jonathan Tan
On 03/13/2017 03:43 PM, Junio C Hamano wrote: * jt/mark-tree-uninteresting-for-uninteresting-commit (2017-02-28) 3 commits - upload-pack: compute blob reachability correctly - revision: exclude trees/blobs given commit - revision: unify {tree,blob}_objects in rev_info The revision/object

Re: [PATCH v2 1/2] pathspec: allow querying for attributes

2017-03-10 Thread Jonathan Tan
Thanks - I don't think I have any more comments on this patch set after these. On 03/10/2017 10:59 AM, Brandon Williams wrote: diff --git a/pathspec.c b/pathspec.c index b961f00c8..7cd5f6e3d 100644 --- a/pathspec.c +++ b/pathspec.c @@ -87,6 +89,74 @@ static void prefix_magic(struct strbuf *sb,

Re: [PATCH 2/2] pathspec: allow escaped query values

2017-03-09 Thread Jonathan Tan
On 03/09/2017 01:07 PM, Brandon Williams wrote: diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh index b5e5a0607..585d17bad 100755 --- a/t/t6135-pathspec-with-attrs.sh +++ b/t/t6135-pathspec-with-attrs.sh @@ -178,4 +178,13 @@ test_expect_success 'abort on asking for

Re: [PATCH 1/2] pathspec: allow querying for attributes

2017-03-09 Thread Jonathan Tan
On 03/09/2017 01:07 PM, Brandon Williams wrote: diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index fc9320e59..5c32d1905 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -384,6 +384,26 @@ full pathname may have

Re: [PATCH 00/10] RFC Partial Clone and Fetch

2017-03-09 Thread Jonathan Tan
Overall, this fetch/clone approach seems reasonable to me, except perhaps some unanswered questions (some of which are also being discussed elsewhere): - does the server need to tell us of missing blobs? - if yes, does the server need to tell us their file sizes? - do we need to store the

Re: What's cooking in git.git (Mar 2017, #03; Wed, 8)

2017-03-08 Thread Jonathan Tan
On 03/08/2017 03:47 PM, Junio C Hamano wrote: * jt/mark-tree-uninteresting-for-uninteresting-commit (2017-02-28) 3 commits - upload-pack: compute blob reachability correctly - revision: exclude trees/blobs given commit - revision: unify {tree,blob}_objects in rev_info The revision/object

More about blob reachability (while fetching arbitrary blobs)

2017-03-08 Thread Jonathan Tan
roduced in commit fff4275 ("pack-bitmap: add support for bitmap indexes", 2013-12-21), later than the last time this part of the code was touched in commit 051e400 ("helping smart-http/stateless-rpc fetch race", 2011-08-05)). Signed-off-by: Jonathan Tan <jonathanta...@google.c

Re: [RFC 0/4] Shallow clones with on-demand fetch

2017-03-06 Thread Jonathan Tan
On 03/04/2017 11:18 AM, Mark Thomas wrote: I was inspired a bit by Microsoft's announcement of their Git VFS. I saw that people have talked in the past about making git fetch objects from remotes as they are needed, and decided to give it a try. For reference, one such conversation is [1].

Re: RFC: Another proposed hash function transition plan

2017-03-06 Thread Jonathan Tan
On 03/06/2017 12:43 AM, Jeff King wrote: Overall the basics of the conversion seem sound to me. The "nohash" things seems more complicated than I think it ought to be, which probably just means I'm missing something. I left a few related comments on the google doc, so I won't repeat them here.

[PATCH] t/perf: export variable used in other blocks

2017-03-02 Thread Jonathan Tan
invocation than what was intended. Export that variable. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Performance indeed does drop significantly with this patch. And I cannot say "at least this is more correct", because it isn't - as Peff thinks, it is indeed still

Re: [PATCH] http: attempt updating base URL only if no error

2017-02-28 Thread Jonathan Tan
On 02/28/2017 05:28 AM, Jeff King wrote: Right, your patch makes sense. A real HTTP error should take precedence over the url-update trickery. Acked-by: Jeff King Thanks! Running your included test, we get: fatal: unable to access 'http://127.0.0.1:5550/redir-to/502/':

[PATCH] http: attempt updating base URL only if no error

2017-02-27 Thread Jonathan Tan
at a Git-shaped URL (http://.../info/refs?service=git-upload-pack) works, whereas commit 6628eb4 tests that a non-Git-shaped URL (http://.../info/refs/foo?service=git-upload-pack) does not work (even though Git is processing that URL) and is an error that is fatal, not silently swallowed. Signe

[PATCH 1/3] revision: unify {tree,blob}_objects in rev_info

2017-02-24 Thread Jonathan Tan
the blobs. However, this results in a minor performance savings at best in that objects no longer need to be instantiated (causing memory allocations and hashtable insertions) - no disk reads are being done for objects whether blob_objects is set or not. Signed-off-by: Jonathan Tan <jonathanta.

[PATCH 0/3] Test fetch-pack's ability to fetch arbitrary blobs

2017-02-24 Thread Jonathan Tan
blobs on demand" [2]) because a way for Git to download missing objects natively is (I think) a prerequisite to that. [1] <20170223230358.30050-1-jonathanta...@google.com> [2] <20170113155253.1644-1-benpe...@microsoft.com> Jonathan Tan (3): revision: unify {tree,blob}_objects

[PATCH 3/3] upload-pack: compute blob reachability correctly

2017-02-24 Thread Jonathan Tan
e user may still set allowanysha1inwant instead of allowreachablesha1inwant to opt-out of the reachability check.) Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- t/t5500-fetch-pack.sh | 30 ++ upload-pack.c | 15 +++ 2 files chang

[PATCH 2/3] revision: exclude trees/blobs given commit

2017-02-24 Thread Jonathan Tan
commit" behave consistent to "^$tree". Also, formalize this behavior in unit tests. (Some of the added tests would already pass even before this commit, but are included nevertheless for completeness.) Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- revision.c

[BUG] allowtipsha1inwant serves unreachable blobs if you know its hash

2017-02-23 Thread Jonathan Tan
we want to allow downloading blobs through this interface). I don't mind taking a look at either solution, but thought of putting this out first in case people have any opinion or insight into this problem and its possible solutions. (CC-ing some people who might have an intere

[PATCH] upload-pack: report "not our ref" to client

2017-02-23 Thread Jonathan Tan
Make upload-pack report "not our ref" errors to the client as an "ERR" line. (If not, the client would be left waiting for a response when the server is already dead.) Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Thanks, here is the independent patch. u

Re: [PATCH 1/3] add git_psprintf helper function

2017-02-16 Thread Jonathan Tan
On 02/16/2017 03:28 AM, Maxim Moseychuk wrote: There are a number of places in the code where we call xsnprintf(), with the assumption that the output will fit into the buffer. If the buffer is small, then git die. In many places buffers have compile-time size, but generated string depends from

<    4   5   6   7   8   9   10   11   >