[PATCH] fetch-pack: grow stateless RPC windows exponentially

during stateless RPCs to avoid the issue raised and fixed in commit 44d8dc54e73e8010c4bdf57a422fc8d5ce709029. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- fetch-pack.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fetch-pack.c b/fetch-pack.c index b501d5c..3fcbda2

Re: [PATCH] fetch-pack: grow stateless RPC windows exponentially

On Mon, Jul 18, 2016 at 12:10 PM, Junio C Hamano wrote: > I'd understand if it were more like "aggressive exponential -> > conservative exponential" without linear phase when stateless_rpc is > in use, though. I just do not quite understand the justification > behind the order

[PATCH v2] fetch-pack: grow stateless RPC windows exponentially

that there is no regression in window size). This optimization is only applied during stateless RPCs to avoid the issue raised and fixed in commit 44d8dc54e73e8010c4bdf57a422fc8d5ce709029. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- fetch-pack.c | 19 --- 1 file chang

Re: [PATCH] fetch-pack: grow stateless RPC windows exponentially

On Mon, Jul 18, 2016 at 1:00 PM, Junio C Hamano wrote: > Jonathan Nieder writes: > >> You have full control of the growth function. So how about aggressive >> growth until 1024*10? >> >> That is: >> >> Current git: >> n < 1024: aggressive exponential >>

Re: [PATCH v2] fetch-pack: grow stateless RPC windows exponentially

On Tue, Jul 19, 2016 at 9:46 AM, Stefan Beller wrote: > Care to elaborate on why you choose 11/10 as growth factor? > > (As someone who has a tick in micro optimizing: > 9/8 is roughly the same exponent, but the division > by 8 is easier as it is just a shift by 3. Similar

Re: [RFC 00/14] Allow fetch-pack to send ref names (globs allowed)

Looking back at the comments I have received in reply, I think that there were two major concerns: (i) the case where a server ACKs a client "have" line and the client forever thinks that the server has it, but it may not be the case for future servers (or future invocations of the same server),

Re: [RFC 12/14] fetch-pack: do not printf after closing stdout

On 01/25/2017 04:50 PM, Stefan Beller wrote: On Wed, Jan 25, 2017 at 2:03 PM, Jonathan Tan <jonathanta...@google.com> wrote: In fetch-pack, during a stateless RPC, printf is invoked after stdout is closed. Update the code to not do this, preserving the existing behavior. This seems

Re: Deadlock between git-remote-http and git fetch-pack

On 01/27/2017 02:31 PM, tsuna wrote: Hi there, While investigating a hung job in our CI system today, I think I found a deadlock in git-remote-http Git version: 2.9.3 Linux (amd64) kernel 4.9.0 Excerpt from the process list: jenkins 27316 0.0 0.0 18508 6024 ?S19:30 0:00 |

[RFC 13/14] fetch: send want-ref and receive fetched refs

Teach fetch to send refspecs to the underlying transport, and teach all components used by the HTTP transport (remote-curl, transport-helper, fetch-pack) to understand and propagate the names and SHA-1s of the refs fetched. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- b

[RFC 00/14] Allow fetch-pack to send ref names (globs allowed)

"want-ref refs/heads/foo", "want-ref refs/tags/foo", among others, and ensure that at least one such ref has been fetched. [2] <20161024132932.i42rqn2vlpocq...@sigill.intra.peff.net> Jonathan Tan (14): upload-pack: move parsing of "want" line

[RFC 04/14] fetch: refactor the population of hashes

Populate SHA-1 ref hashes in get_ref_map instead of do_fetch. Besides tightening scopes of variables in the code, this also prepares for a future patch where get_ref_map is called multiple times within do_fetch. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- builtin/fetch.

[RFC 05/14] fetch: refactor fetch_refs into two functions

Refactor the fetch_refs function into a function that does the fetching of refs and another function that stores them. This prepares for a future patch where some processing may be done between those tasks. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- builtin/fetch.

[RFC 12/14] fetch-pack: do not printf after closing stdout

In fetch-pack, during a stateless RPC, printf is invoked after stdout is closed. Update the code to not do this, preserving the existing behavior. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- builtin/fetch-pack.c | 14 ++ 1 file changed, 10 insertions(+), 4 del

[RFC 03/14] upload-pack: test negotiation with changing repo

his effort, a mechanism to substitute strings in an HTTP response only on the first invocation is added. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- t/lib-httpd.sh | 1 + t/lib-httpd/apache.conf| 8 t/lib-httpd/one-time-sed.sh|

[RFC 14/14] DONT USE advertise_ref_in_want=1

--- t/t5500-fetch-pack.sh | 2 ++ upload-pack.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index 18fe23c97..f39dbcab8 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -551,6 +551,7 @@

[RFC 07/14] fetch-pack: put shallow info in out param

a remote-tracking branch seems to track more than one remote branch. This is the 1st of 3 patches to eliminate using input refs to communicate information obtained by the fetch mechanism. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- builtin/clone.c| 4 ++-- b

[RFC 11/14] fetch-pack: support want-ref

Teach fetch-pack to use the want-ref mechanism whenever the server advertises it. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- builtin/fetch-pack.c | 5 +- fetch-pack.c | 173 -- fetch-pack.h | 2 + t

[RFC 01/14] upload-pack: move parsing of "want" line

Refactor to parse "want" lines when the prefix is found. This makes it easier to add support for another prefix, which will be done in a subsequent commit. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- upload-pack.c | 28 ++-- 1 file change

[RFC 02/14] upload-pack: allow ref name and glob requests

quot;, among others, and ensure that at least one such ref has been fetched. [1] pack-protocol.txt Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Documentation/technical/http-protocol.txt | 20 +- Documentation/technical/pack-protocol.txt | 24 +- Documen

[RFC 10/14] fetch-pack: support partial names and globs

Teach fetch-pack to support partial ref names and ref patterns as input. This does not use "want-ref" yet - support for that will be added in a future patch. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- builtin/fetch-pack.c | 40 ---

[RFC 08/14] fetch-pack: check returned refs for matches

the nested for loop in this patch.) Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- builtin/fetch-pack.c | 7 ++- fetch-pack.c | 9 ++--- fetch-pack.h | 2 -- remote.h | 3 +-- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/buil

[RFC 09/14] transport: put ref oid in out param

by the fetch mechanism. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- builtin/clone.c | 14 -- builtin/fetch-pack.c | 4 ++-- fetch-pack.c | 26 +++--- fetch-pack.h | 2 +- transport-helper.c

[RFC 06/14] fetch: refactor to make function args narrower

Refactor find_non_local_tags and get_ref_map to only take the information they need instead of the entire transport struct. Besides improving code clarity, this also improves their flexibility, which will be needed in a future patch. Signed-off-by: Jonathan Tan <jonathanta...@google.

Re: [RFC 00/14] Allow fetch-pack to send ref names (globs allowed)

Thanks for your comments. On 01/26/2017 03:00 PM, Jeff King wrote: On Wed, Jan 25, 2017 at 02:02:53PM -0800, Jonathan Tan wrote: Negotiation currently happens by upload-pack initially sending a list of refs with names and SHA-1 hashes, and then several request/response pairs in which

Re: [RFC 02/14] upload-pack: allow ref name and glob requests

On 01/26/2017 02:23 PM, Junio C Hamano wrote: Jonathan Tan <jonathanta...@google.com> writes: Currently, while performing packfile negotiation [1], upload-pack allows clients to specify their desired objects only as SHA-1s. This causes: (a) vulnerability to failure when an object tur

Re: [RFC 03/14] upload-pack: test negotiation with changing repo

On 01/26/2017 02:33 PM, Junio C Hamano wrote: Jonathan Tan <jonathanta...@google.com> writes: diff --git a/t/lib-httpd/one-time-sed.sh b/t/lib-httpd/one-time-sed.sh new file mode 100644 index 0..060ec0300 --- /dev/null +++ b/t/lib-httpd/one-time-sed.sh @@ -0,0 +1,8 @@ +#!/

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

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 3/3] upload-pack: compute blob reachability correctly

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 1/3] revision: unify {tree,blob}_objects in rev_info

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

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 2/3] revision: exclude trees/blobs given commit

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

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

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 6/7] grep: avoid resolving revision names in --no-index case

On 02/14/2017 10:04 AM, Jeff King wrote: On Tue, Feb 14, 2017 at 08:53:04AM -0800, Jonathan Tan wrote: On 02/13/2017 10:07 PM, Jeff King wrote: diff --git a/builtin/grep.c b/builtin/grep.c index e83b33bda..c4c632594 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -1176,6 +1176,12 @@ int

[PATCH for NEXT] grep: do not unnecessarily query repo for "--"

ore, always interpret "--" as signaling the end of options, instead of trying to interpret it as a rev first. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- There is probably a similar bug for commands of the form: git grep --no-index pattern foo If there

Re: [PATCH 0/7] grep rev/path parsing fixes

On 02/13/2017 10:00 PM, Jeff King wrote: I've fixed that, along with a few other bugs and cleanups. The complete series is below. Patch 2 is your (untouched) patch. My suggestions for your test are in patch 3, which can either remain on its own or be squashed in. [1/7]: grep: move thread

Re: [PATCH 6/7] grep: avoid resolving revision names in --no-index case

On 02/13/2017 10:07 PM, Jeff King wrote: diff --git a/builtin/grep.c b/builtin/grep.c index e83b33bda..c4c632594 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -1176,6 +1176,12 @@ int cmd_grep(int argc, const char **argv, const char *prefix) break; }

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

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

Re: [BUG] submodule config does not apply to upper case submodules?

On 02/15/2017 10:53 AM, Junio C Hamano wrote: Lars Schneider writes: It looks like as if submodule configs ("submodule.*") for submodules with upper case names are ignored. This observation is surprising, as the second level in three-level names like ".." is

Re: [BUG] submodule config does not apply to upper case submodules?

On 02/15/2017 03:11 PM, Junio C Hamano wrote: Junio C Hamano writes: Perhaps something like this? This looks good. I was hoping to unify the processing logic between this CLI parsing and the usual stream parsing, but this approach is probably simpler. config.c | 16

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

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

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

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 2/2] connect: know that zero-ID is not a ref

-by: Jonathan Tan <jonathanta...@google.com> --- connect.c| 7 +++ t/t5512-ls-remote.sh | 22 ++ 2 files changed, 29 insertions(+) diff --git a/connect.c b/connect.c index 722dc3f..d4a58de 100644 --- a/connect.c +++ b/connect.c @@ -165,6 +165,13 @@ stru

[PATCH 1/2] tests: move test_lazy_prereq JGIT to test-lib.sh

This enables JGIT to be used as a prereq in invocations of test_expect_success (and other functions) in other test scripts. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- t/t5310-pack-bitmaps.sh | 4 t/test-lib.sh | 4 2 files changed, 4 insertions(+), 4 del

[PATCH 0/2] handle empty spec-compliant remote repos correctly

JGit do, but the C client does not handle them correctly (treating them as an actual ref and subsequently returning incorrect responses or errors). These patches fix those while maintaining backwards compatibility with existing implementations that do not send the zero ID in such a case. Jonathan

[PATCH] sequencer: support folding in rfc2822 footer

into multiple lines, especially (but not only) to deal with the line-width limitations described in the specification, referring to this as "folding". Teach sequencer.c to treat split and unsplit field bodies in the same way (that is, to not include the blank line). Signed-off-by: Jo

Re: [PATCH 2/2] connect: know that zero-ID is not a ref

On 09/02/2016 01:13 PM, Jeff King wrote: On Fri, Sep 02, 2016 at 10:15:39AM -0700, Jonathan Tan wrote: (git-daemon should probably also be changed to serve zero IDs, but such a change can be considered independently from this change; even if both the client and server changes were made in one

[PATCH v2 2/2] connect: advertized capability is not a ref

llow git upload-pack to advertise capabilities when there are no refs to advertise. Leave it disabled by default since git clients can't be counted on to have this patch (1) yet. 4. After a year or so, flip the default for that server configuration variable to true. Signed-off-by: Jona

[PATCH v2 1/2] tests: move test_lazy_prereq JGIT to test-lib.sh

This enables JGIT to be used as a prereq in invocations of test_expect_success (and other functions) in other test scripts. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- t/t5310-pack-bitmaps.sh | 4 t/test-lib.sh | 4 2 files changed, 4 insertions(+), 4 del

[PATCH v2 0/2] handle empty spec-compliant remote repos correctly

daemon` (instead of sleeping) Jonathan Tan (2): tests: move test_lazy_prereq JGIT to test-lib.sh connect: advertized capability is not a ref connect.c | 3 +++ t/t5310-pack-bitmaps.sh | 4 t/t5512-ls-remote.sh| 39 +++ t/test-lib.sh

Re: [PATCH] sequencer: support folding in rfc2822 footer

On 09/02/2016 07:23 PM, Junio C Hamano wrote: Jonathan Tan <jonathanta...@google.com> writes: Sample-field: multiple-line field body that causes a blank line below I am not sure this is unconditionally good, or may cause problems to those with workflows you did not consider when you

Re: [PATCH] sequencer: support folding in rfc2822 footer

On 09/06/2016 03:08 PM, Jonathan Tan wrote: On 09/02/2016 07:23 PM, Junio C Hamano wrote: A slightly related tangent. An unconditionally good change you could make is to allow folding of in-body headers. I.e. you can have e.g. -- >8 -- Subject: [PATCH] sequencer: support in-b

[PATCH v3 2/2] connect: advertized capability is not a ref

h but since it never remembered to do so for fetch, the client forgot to handle this case. Handle it. In this aspect, JGit is compliant with the specification in pack-protocol.txt. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- connect.c| 17 +

[PATCH v3 1/2] tests: move test_lazy_prereq JGIT to test-lib.sh

This enables JGIT to be used as a prereq in invocations of test_expect_success (and other functions) in other test scripts. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- t/t5310-pack-bitmaps.sh | 4 t/test-lib.sh | 4 2 files changed, 4 insertions(+), 4 del

[PATCH v3 0/2] handle empty spec-compliant remote repos correctly

stead - I can change it if consensus is that these should be errors. Jonathan Tan (2): tests: move test_lazy_prereq JGIT to test-lib.sh connect: advertized capability is not a ref connect.c | 17 + t/t5310-pack-bitmaps.sh | 4 t/t5512-

[PATCH v4 3/3] connect: advertized capability is not a ref

h but since it never did so for fetch, the client didn't need to handle this case. Handle it. In this aspect, JGit is compliant with the specification in pack-protocol.txt. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- connect.c| 17 +

[PATCH v4 2/3] connect: tighten check for unexpected early hang up

From: Jonathan Nieder A server hanging up immediately to mark access being denied does not send any .have refs, shallow lines, or anything else before hanging up. If the server has sent anything, then the hangup is unexpected. That is, if the server hangs up after a shallow

[PATCH v4 0/3] handle empty spec-compliant remote repos correctly

continuing on when facing a recoverable error (that is, "warning"). But I agree that there are good points in favor of using fatal errors ("die") and I can switch to that if there is consensus. Jonathan Nieder (1): connect: tighten check for unexpected early hang up J

[PATCH v4 1/3] tests: move test_lazy_prereq JGIT to test-lib.sh

This enables JGIT to be used as a prereq in invocations of test_expect_success (and other functions) in other test scripts. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- t/t5310-pack-bitmaps.sh | 4 t/test-lib.sh | 4 2 files changed, 4 insertions(+), 4 del

[PATCH v5 3/3] connect: advertized capability is not a ref

capabilities^{} ref in its ref advertisement for push but since it never did so for fetch, the client didn't need to handle this case. Handle it. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- connect.c| 14 ++ t/t5512-ls-remote.sh | 40 +

[PATCH v5 1/3] tests: move test_lazy_prereq JGIT to test-lib.sh

This enables JGIT to be used as a prereq in invocations of test_expect_success (and other functions) in other test scripts. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- t/t5310-pack-bitmaps.sh | 4 t/test-lib.sh | 4 2 files changed, 4 insertions(+), 4 del

[PATCH v5 2/3] connect: tighten check for unexpected early hang up

From: Jonathan Nieder A server hanging up immediately to mark access being denied does not send any .have refs, shallow lines, or anything else before hanging up. If the server has sent anything, then the hangup is unexpected. That is, if the server hangs up after a shallow

[PATCH v5 0/3] handle empty spec-compliant remote repos correctly

nnect: tighten check for unexpected early hang up Jonathan Tan (2): tests: move test_lazy_prereq JGIT to test-lib.sh connect: advertized capability is not a ref connect.c | 32 ++-- t/t5310-pack-bitmaps.sh | 4 t/t5512-

[RFC/PATCH 1/2] sequencer: refactor message and origin appending

Move the appending of the commit message and origin information into its own function, in preparation for a subsequent patch. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- sequencer.c | 46 -- 1 file changed, 28 insertions(+), 18 del

[RFC/PATCH 2/2] sequencer: allow origin line below commit title

"cherry picked" line to be placed in a consistent manner, independent of the nature of the footer of the existing commit message. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Documentation/config.txt | 4 +++ Documentation/git-cherry-pick.txt | 15 +- bu

[RFC/PATCH 0/2] place cherry pick line below commit title

ommit message: commit title This is an explanatory paragraph. Footer: foo place the "(cherry picked from ...)" line below "commit title". Would this be better? [1] <1472846322-5592-1-git-send-email-jonathanta...@google.com> Jonathan Tan (2): sequencer: refactor message a

Re: [RFC/PATCH 0/2] place cherry pick line below commit title

On 10/04/2016 10:25 AM, Junio C Hamano wrote: So I would say it is perfectly OK if your update works only for cases we can clearly define the semantics for. For example, we can even start with something simple like: * A RFC822-header like line, together with any number of whitespace

Re: [RFC/PATCH 0/2] place cherry pick line below commit title

On 10/04/2016 11:28 AM, Junio C Hamano wrote: An addendum. We may also want to be prepared to accept an input that has some garbage lines _after_ the trailer block, if we can clearly identify them as such. For example, we could change the definition of "the last paragraph" as "the block of

[PATCH v3 3/3] mailinfo: handle in-body header continuations

r very long broken line interpret the in-body subject to be "another very long broken line" instead of "another very long". An existing test (t/t5100/msg0015) has an indented line immediately after an in-body header - it has been modified to reflect the new functionality. S

[PATCH v3 1/3] mailinfo: separate in-body header processing

The check_header function contains logic specific to in-body headers, although it is invoked during both the processing of actual headers and in-body headers. Separate out the in-body header part into its own function. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- mailinfo.

[PATCH v3 2/3] mailinfo: make is_scissors_line take plain char *

The is_scissors_line takes a struct strbuf * when a char * would suffice. Make it take char *. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- mailinfo.c | 35 ++- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/mailinfo.c b/mail

[PATCH v3 0/3] handle multiline in-body headers

no longer necessary to make is_scissors_line take plain char * (the second patch) - I think that that patch still improves the code, but let me know if you want me to remove it from this patch set. Jonathan Tan (3): mailinfo: separate in-body header processing mailinfo: make is_scissors_line take

Re: [RFC/PATCH 3/3] mailinfo: handle in-body header continuations

On 09/16/2016 01:17 PM, Junio C Hamano wrote: In other words, wouldn't something like the illustration at the end of this message sufficient? If the body consists solely of in-body header without any patch or patchbreak, we may reach EOF with something in mi->in_line_header buffer and nothing

Re: [RFC/PATCH 3/3] mailinfo: handle in-body header continuations

On 09/16/2016 01:59 PM, Junio C Hamano wrote: if (mi->in_line_header->len) { /* we have read the beginning of one in-line header */ if (line->len && isspace(*line->buf) && !(mi->use_scissors && is_scissors_line(line))) { Minor note:

Re: [RFC/PATCH 3/3] mailinfo: handle in-body header continuations

On 09/16/2016 04:04 PM, Junio C Hamano wrote: Jonathan Tan <jonathanta...@google.com> writes: I'm concerned about what happens if check_header fails - we would then have some lines which need to be treated as log messages. (At least, they are currently treated that way.) I actually th

Re: [RFC/PATCH 2/3] mailinfo: correct malformed test example

On 09/16/2016 03:55 PM, Junio C Hamano wrote: Hmph, these: t/t5100/info0008--no-inbody-headers | 5 + t/t5100/msg0008--no-inbody-headers | 6 ++ t/t5100/msg0015--no-inbody-headers | 1 + have --no-inbody-headers in their names; wouldn't that an indication that they are expected

Re: [RFC/PATCH 2/3] mailinfo: correct malformed test example

On 09/16/2016 12:19 PM, Junio C Hamano wrote: Jonathan Tan <jonathanta...@google.com> writes: An existing sample message (0015) in the tests for mailinfo contains an indented line immediately after an in-body header (without any intervening blank line). This comes from d25e5159 (&

[PATCH v2] fetch-pack: do not reset in_vain on non-novel acks

r, as new acks for the purpose of MAX_IN_VAIN, acks for objects for which the client has never received an ack before in this session. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- Thanks for your comments - I really appreciate them. Update from original: o removed redundant text from c

[PATCH] fetch-pack: do not reset in_vain on non-novel acks

se of MAX_IN_VAIN. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- fetch-pack.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/fetch-pack.c b/fetch-pack.c index 85e77af..1141e3c 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -428,10 +428,18 @@ static in

[PATCH] do not reset in_vain on non-novel acks

refs or heads instead of merely prioritizing by date in the priority queue of objects), but I thought I'd send the patch out first anyway to see what others think. Jonathan Tan (1): fetch-pack: do not reset in_vain on non-novel acks fetch-pack.c | 12 ++-- 1 file changed, 10 insertions

[RFC/PATCH 3/3] mailinfo: handle in-body header continuations

FILE pointer, but this would also require a buffer, with the same issues. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- mailinfo.c | 24 ++-- mailinfo.h | 1 + t/t4150-am.sh | 23 +++ t/t5100-mailinfo.sh | 4 ++-- t/t510

[RFC/PATCH 2/3] mailinfo: correct malformed test example

-by: Jonathan Tan <jonathanta...@google.com> --- t/t5100/info0008--no-inbody-headers | 5 + t/t5100/msg0008--no-inbody-headers | 6 ++ t/t5100/msg0015--no-inbody-headers | 1 + t/t5100/patch0008--no-inbody-headers | 0 t/t5100/sample.mbox | 1 + 5 files chang

[RFC/PATCH 1/3] mailinfo: refactor commit message processing

is line is a patchbreak line, in which case handle_patch is subsequently called (in handle_filter) on "line". In this case, "line" must have passed UTF-8 conversion both before and after this patch, so the result is still the same overall. Signed-off-by: Jonathan Tan <j

[RFC/PATCH 0/3] handle multiline in-body headers

Thanks, Peff, for the explanation and the method to reproduce the issue. The issue seems to be in mailinfo.c - this patch set addresses that, and I have also included a test for "git am" in t/t4150-am.sh to show the effect of this patch set on that command. Jonathan Tan (3): mailinfo

[PATCH v2 3/4] mailinfo: make is_scissors_line take plain char *

The is_scissors_line takes a struct strbuf * when a char * would suffice. Make it take char *. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- mailinfo.c | 35 ++- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/mailinfo.c b/mail

[PATCH v2 0/4] handle multiline in-body headers

dy_header function performing an airtight check on whether a line is an in-body header) o simpler try_convert_to_utf8 API o one file of the expected output of t/t5100/*0015 is modified (instead of the input) o t/t5100/*0018--no-inbody-headers test files added o example in commit message improved followin

[PATCH v2 4/4] mailinfo: handle in-body header continuations

r very long broken line interpret the in-body subject to be "another very long broken line" instead of "another very long". An existing test (t/t5100/msg0015) has an indented line immediately after an in-body header - it has been modified to reflect the new functionality. S

[PATCH v2 1/4] mailinfo: separate in-body header processing

The check_header function contains logic specific to in-body headers, although it is invoked during both the processing of actual headers and in-body headers. Separate out the in-body header part into its own function. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- mailinfo.

[PATCH v2 2/4] mailinfo: refactor to support utf8 decode attempts

by a subsequent patch. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- mailinfo.c | 42 +- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/mailinfo.c b/mailinfo.c index 0c4738a..aadad09 100644 --- a/mailinfo.c +++ b/mailinfo.c @@ -340,23 +

Re: [RFC/PATCH 0/2] place cherry pick line below commit title

On 09/29/2016 02:56 PM, Junio C Hamano wrote: Jonathan Tan <jonathanta...@google.com> writes: This is somewhat of a follow-up to my previous e-mail with subject "[PATCH] sequencer: support folding in rfc2822 footer" [1], in which I proposed relaxing the definition of a commi

Re: [RFC/PATCH 0/2] place cherry pick line below commit title

On 09/30/2016 01:49 PM, Junio C Hamano wrote: Junio C Hamano <gits...@pobox.com> writes: Jonathan Tan <jonathanta...@google.com> writes: I vaguely recall that there were some discussion on the definition of "what's a trailer line" with folks from the kernel land, pe

Re: [RFC/PATCH 0/2] place cherry pick line below commit title

On 10/03/2016 12:17 PM, Junio C Hamano wrote: It may be necessary for the code to analize the lines in a block identified as "likely to be a trailing block" more carefully, though. The example 59f0aa94 in the message you are responding to has "Link 1:" that consists of 3 physical lines. An

Re: [RFC/PATCH 0/2] place cherry pick line below commit title

On 10/03/2016 03:13 PM, Junio C Hamano wrote: Jonathan Tan <jonathanta...@google.com> writes: There are other options like checking for indentation or checking for balanced parentheses/brackets, but I think that these would lead to surprising behavior for the user (this woul

Re: [RFC/PATCH 0/2] place cherry pick line below commit title

On 09/30/2016 12:34 PM, Junio C Hamano wrote: 2) The Linux kernel's repository has some "commit ... upstream." lines in this position (below the commit title) - for example, in commit dacc0987fd2e. "A group of people seem to prefer it there" does not lead to "therefore let's move it there for

[PATCH] fetch: do not redundantly calculate tag refmap

ot; anyway. This was introduced in commit c5a84e9 ("fetch --tags: fetch tags *in addition to* other stuff", 2013-10-29) when modifying the effect of the --tags parameter to "git fetch". The refmap-for-tag calculation was copied instead of moved. Signed-off-by: Jonathan Tan <

Re: [PATCH 4/5] grep: optionally recurse into submodules

On Thu, Oct 27, 2016 at 3:38 PM, Brandon Williams wrote: > diff --git a/builtin/grep.c b/builtin/grep.c > index 8887b6add..f34f16df9 100644 > --- a/builtin/grep.c > +++ b/builtin/grep.c > @@ -18,12 +18,20 @@ > #include "quote.h" > #include "dir.h" > #include "pathspec.h" >

Re: [PATCH v2 1/6] submodules: add helper functions to determine presence of submodules

On 10/31/2016 03:38 PM, Brandon Williams wrote: + struct strbuf buf = STRBUF_INIT; + char *submodule_url = NULL; + + strbuf_addf(, "submodule.%s.url", module->name); + ret = !git_config_get_string(buf.buf, _url); + +

Re: [PATCH v3 5/6] trailer: allow non-trailers in trailer block

On 10/17/2016 06:42 PM, Junio C Hamano wrote: Stefan Beller <sbel...@google.com> writes: On Fri, Oct 14, 2016 at 10:38 AM, Jonathan Tan <jonathanta...@google.com> wrote: Existing trailers are extracted from the input message by looking for -a group of one or more lines that con

[PATCH v5 1/8] trailer: improve const correctness

Change "const char *" to "char *" in struct trailer_item and in the return value of apply_command (since those strings are owned strings). Change "struct conf_info *" to "const struct conf_info *" (since that struct is not modified). Signed-off-by

[PATCH v5 5/8] trailer: clarify failure modes in parse_trailer

command line arguments (which allow '=' as separator) and file input (which does not allow '=' as separator). Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- trailer.c | 75 --- 1 file changed, 53 insertions(+), 22 del

[PATCH v5 4/8] trailer: make args have their own struct

"struct trailer_item" to be further differentiated from "struct arg_item" in future patches. Signed-off-by: Jonathan Tan <jonathanta...@google.com> --- trailer.c | 135 +++--- 1 file changed, 85 insertions(+), 50 deletion

  1   2   3   4   5   6   7   8   9   10   >