* Re: What's cooking in git.git (Oct 2016, #05; Thu, 20)
From: Johannes Schindelin @ 2016-10-21 13:08 UTC (permalink / raw)
To: Junio C Hamano, Lars Schneider; +Cc: git
In-Reply-To: <xmqqk2d2ein7.fsf@gitster.mtv.corp.google.com>
Hi Junio & Lars,
On Thu, 20 Oct 2016, Junio C Hamano wrote:
> * ls/filter-process (2016-10-17) 14 commits
> (merged to 'next' on 2016-10-19 at ffd0de042c)
> + contrib/long-running-filter: add long running filter example
> + convert: add filter.<driver>.process option
> + convert: prepare filter.<driver>.process option
> + convert: make apply_filter() adhere to standard Git error handling
> + pkt-line: add functions to read/write flush terminated packet streams
> + pkt-line: add packet_write_gently()
> + pkt-line: add packet_flush_gently()
> + pkt-line: add packet_write_fmt_gently()
> + pkt-line: extract set_packet_header()
> + pkt-line: rename packet_write() to packet_write_fmt()
> + run-command: add clean_on_exit_handler
> + run-command: move check_pipe() from write_or_die to run_command
> + convert: modernize tests
> + convert: quote filter names in error messages
>
> The smudge/clean filter API expect an external process is spawned
> to filter the contents for each path that has a filter defined. A
> new type of "process" filter API has been added to allow the first
> request to run the filter for a path to spawn a single process, and
> all filtering need is served by this single process for multiple
> paths, reducing the process creation overhead.
>
> Will merge to 'master'.
This breaks in Git for Windows' SDK (I only now realized that t0060 was
not the only thing breaking in `next` for a while now):
-- snip --
not ok 15 - required process filter should filter data
#
# test_config_global filter.protocol.process
# "$TEST_DIRECTORY/t002
# 1/rot13-filter.pl clean smudge" &&
# test_config_global filter.protocol.required true &&
# rm -rf repo &&
# mkdir repo &&
# (
# cd repo &&
# git init &&
#
# echo "git-stderr.log" >.gitignore &&
# echo "*.r filter=protocol" >.gitattributes &&
# git add . &&
# git commit . -m "test commit 1" &&
# git branch empty-branch &&
#
# cp "$TEST_ROOT/test.o" test.r &&
# cp "$TEST_ROOT/test2.o" test2.r &&
# mkdir testsubdir &&
# cp "$TEST_ROOT/test3 'sq',\$x.o" "testsubdir/test3
# 'sq',
# \$x.r" &&
# >test4-empty.r &&
#
# S=$(file_size test.r) &&
# S2=$(file_size test2.r) &&
# S3=$(file_size "testsubdir/test3 'sq',\$x.r") &&
#
# filter_git add . &&
# cat >expected.log <<-EOF &&
# START
# init handshake complete
# IN: clean test.r $S [OK] -- OUT: $S . [OK]
# IN: clean test2.r $S2 [OK] -- OUT: $S2 .
# [OK]
# IN: clean test4-empty.r 0 [OK] -- OUT: 0
# [OK]
# IN: clean testsubdir/test3 'sq',\$x.r $S3
# [OK] -
# - OUT: $S3 . [OK]
# STOP
# EOF
# test_cmp_count expected.log rot13-filter.log &&
#
# filter_git commit . -m "test commit 2" &&
# cat >expected.log <<-EOF &&
# START
# init handshake complete
# IN: clean test.r $S [OK] -- OUT: $S . [OK]
# IN: clean test2.r $S2 [OK] -- OUT: $S2 .
# [OK]
# IN: clean test4-empty.r 0 [OK] -- OUT: 0
# [OK]
# IN: clean testsubdir/test3 'sq',\$x.r $S3
# [OK] -
# - OUT: $S3 . [OK]
# IN: clean test.r $S [OK] -- OUT: $S . [OK]
# IN: clean test2.r $S2 [OK] -- OUT: $S2 .
# [OK]
# IN: clean test4-empty.r 0 [OK] -- OUT: 0
# [OK]
# IN: clean testsubdir/test3 'sq',\$x.r $S3
# [OK] -
# - OUT: $S3 . [OK]
# STOP
# EOF
# test_cmp_count expected.log rot13-filter.log &&
#
# rm -f test2.r "testsubdir/test3 'sq',\$x.r" &&
#
# filter_git checkout --quiet --no-progress . &&
# cat >expected.log <<-EOF &&
# START
# init handshake complete
# IN: smudge test2.r $S2 [OK] -- OUT: $S2 .
# [OK]
# IN: smudge testsubdir/test3 'sq',\$x.r $S3
# [OK]
# -- OUT: $S3 . [OK]
# STOP
# EOF
# test_cmp_exclude_clean expected.log
# rot13-filter.log &&
#
# filter_git checkout --quiet --no-progress
# empty-branch &
# &
# cat >expected.log <<-EOF &&
# START
# init handshake complete
# IN: clean test.r $S [OK] -- OUT: $S . [OK]
# STOP
# EOF
# test_cmp_exclude_clean expected.log
# rot13-filter.log &&
#
# filter_git checkout --quiet --no-progress master
# &&
# cat >expected.log <<-EOF &&
# START
# init handshake complete
# IN: smudge test.r $S [OK] -- OUT: $S .
# [OK]
# IN: smudge test2.r $S2 [OK] -- OUT: $S2 .
# [OK]
# IN: smudge test4-empty.r 0 [OK] -- OUT: 0
# [OK]
# IN: smudge testsubdir/test3 'sq',\$x.r $S3
# [OK]
# -- OUT: $S3 . [OK]
# STOP
# EOF
# test_cmp_exclude_clean expected.log
# rot13-filter.log &&
#
# test_cmp_committed_rot13 "$TEST_ROOT/test.o"
# test.r &&
# test_cmp_committed_rot13 "$TEST_ROOT/test2.o"
# test2.r &&
# test_cmp_committed_rot13 "$TEST_ROOT/test3
# 'sq',\$x.o" "
# testsubdir/test3 'sq',\$x.r"
# )
#
ok 16 - required process filter takes precedence
not ok 17 - required process filter should be used only for "clean"
operation on
ly
#
# test_config_global filter.protocol.process
# "$TEST_DIRECTORY/t002
# 1/rot13-filter.pl clean" &&
# rm -rf repo &&
# mkdir repo &&
# (
# cd repo &&
# git init &&
#
# echo "*.r filter=protocol" >.gitattributes &&
# cp "$TEST_ROOT/test.o" test.r &&
# S=$(file_size test.r) &&
#
# filter_git add . &&
# cat >expected.log <<-EOF &&
# START
# init handshake complete
# IN: clean test.r $S [OK] -- OUT: $S . [OK]
# STOP
# EOF
# test_cmp_count expected.log rot13-filter.log &&
#
# rm test.r &&
#
# filter_git checkout --quiet --no-progress . &&
# # If the filter would be used for "smudge", too,
# we woul
# d see
# # "IN: smudge test.r 57 [OK] -- OUT: 57 . [OK]"
# here
# cat >expected.log <<-EOF &&
# START
# init handshake complete
# STOP
# EOF
# test_cmp_exclude_clean expected.log
# rot13-filter.log
# )
-- snap --
Unsurprisingly, bisect identifies "convert: add filter.<driver>.process
option" as the first bad commit, although it only fails on the test case
15, but not on 17.
I am unfortunately still busy with trying to figure out what exactly makes
t6030 hang on `pu` (seems it thinks stdin is a tty and just waits for an
answer), and then trying to reduce that insane amount of time wasted on
running, and waiting for, the test suite, and for unrelated reasons I'll
have to go offline for the rest of the day, so I will most likely be
unable to assist further with this.
Sorry,
Johannes
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: Matthieu Moy @ 2016-10-21 13:10 UTC (permalink / raw)
To: René Scharfe
Cc: Duy Nguyen, Johannes Schindelin, Junio C Hamano, Git Mailing List
In-Reply-To: <e0e320ce-14a1-b4a8-aa07-6bc4f3c4a424@web.de>
René Scharfe <l.s.r@web.de> writes:
> I get this on WSL with prove -j8:
>
> Files=750, Tests=13657, 906 wallclock secs ( 8.51 usr 17.17 sys + 282.62 cusr 3731.85 csys = 4040.15 CPU)
>
> And this for a run on Debian inside a Hyper-V VM on the same system:
>
> Files=759, Tests=13895, 99 wallclock secs ( 4.81 usr 1.06 sys + 39.70 cusr 25.82 csys = 71.39 CPU)
What about the same without WSL on windows?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: Git context menu in Windows Exporer has a “git bash” option but it doesn't work
From: Stefan Monov @ 2016-10-21 13:11 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1610211437510.3264@virtualbox>
On Fri, Oct 21, 2016 at 3:40 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Are you referring to Git for Windows?
> Which version? From where did you obtain it?
I'm not sure about the answer to these questions, so to clear things
up, I'm downloading it anew now. Getting it from
https://git-scm.com/download/win .
I downloaded and installed Git for Windows, version 2.10.1, 32-bit.
Indeed the context menu option is now called "Git Bash Here". I don't
know what it was called before I installed this new version of git,
but now it works fine. I.e. no more auto-closing bash window.
Thanks for (indirectly) helping me resolve the issue. :)
On Fri, Oct 21, 2016 at 3:40 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Stefan,
>
> On Fri, 21 Oct 2016, Stefan Monov wrote:
>
>> The default git distribution for windows
>
> Are you referring to Git for Windows? Which version? From where did you
> obtain it?
>
>> contains, besides the CLI tools, a shell integration, i.e. a context
>> menu for Windows Explorer. This context menu has a "Git bash" option
>> which I suppose should open a Git bash prompt cd'd to the current
>> directory.
>
> I guess you mean either "Git Bash Here" or "Git Bash" (not "Git bash")?
>
>> But instead, the git bash window opens and immediately closes, giving me
>> no chance to even see what it says inside it. Any fix?
>
> Sorry, there is not enough information to go on here. In Git for Windows'
> bug tracker, we were careful to provide you with a template that helps
> with providing relevant information:
> https://github.com/git-for-windows/git/issues/new
>
>> Note: Opening Git Bash from the Windows Start Menu works fine, but if
>> I open it that way, then I need to manually cd to the needed dir.
>
> For me, both work fine. For the record, I use Git for Windows (64-bit)
> v2.10.1, as downloaded from https://git-for-windows.github.io/:
>
> $ git version
> git version 2.10.1.windows.1
>
> Ciao,
> Johannes
>
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2016, #05; Thu, 20)
From: Pranit Bauva @ 2016-10-21 15:28 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Lars Schneider, Git List
In-Reply-To: <alpine.DEB.2.20.1610211457030.3264@virtualbox>
Hey Johannes,
On Fri, Oct 21, 2016 at 6:38 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> I am unfortunately still busy with trying to figure out what exactly makes
> t6030 hang on `pu` (seems it thinks stdin is a tty and just waits for an
> answer), and then trying to reduce that insane amount of time wasted on
> running, and waiting for, the test suite, and for unrelated reasons I'll
> have to go offline for the rest of the day, so I will most likely be
> unable to assist further with this.
My patch series [1] is recently merged into pu which changed a lot of
things around "git bisect next" which I remember that you mention it
somewhere else too but don't exactly recollect where. There were some
*really* drastic changes and they are evident with my conversation
with Junio. Maybe you could tell a little bit more about what's
happening with t6030 so that even I could dig further.
Heads up: The changes in "git bisect next" were related to revision
walking so you might want to check it out.
Sorry for any inconvenience caused by this series.
Regards,
Pranit Bauva
^ permalink raw reply
* Re: Prove "Tests out of sequence" Error
From: Jacob Keller @ 2016-10-21 15:29 UTC (permalink / raw)
To: Jeff King; +Cc: Stefan Beller, Lars Schneider, git
In-Reply-To: <20161021084348.dp4jfpfownodl7nz@sigill.intra.peff.net>
On Fri, Oct 21, 2016 at 1:43 AM, Jeff King <peff@peff.net> wrote:
> prove t5547-push-quarantine.sh :: -v
>
> The Travis tests do exactly this (along with --tee to actually save the
> output). It seems like a minor miracle that this is the first test
> output that has actually triggered as TAP input. I'd suggest that the
> problem is not in the test, though, but that our "--verbose" option is
> unsuitable for using with a TAP harness.
>
> The obvious fix would be to send "--verbose" output to stderr, but I
> suspect that would end up annoying for people who do:
>
> ./t5547-push-quarantine.sh -v | less
>
> to read long output. Probably we need some option like "--log" which
> logs in the same way that "--tee" does, but _without_ sending the data
> to stdout. Naively, that just means replacing the "tee" invocation with
> "cat", but I suspect it will be a lot more complicated than that,
> because we still need to let the TAP output go to stdout.
>
> -Peff
Can we determine that we're running with something monitoring the TAP
output? Because then we could make verbose go to stderr instead
dynamically?
Thanks,
Jake
^ permalink raw reply
* Re: Prove "Tests out of sequence" Error
From: Jeff King @ 2016-10-21 15:35 UTC (permalink / raw)
To: Jacob Keller; +Cc: Stefan Beller, Lars Schneider, git
In-Reply-To: <CA+P7+xozpk2-WZUYtaKmCazG3Owzpa3bsDejyUuGj0YEQnJWJg@mail.gmail.com>
On Fri, Oct 21, 2016 at 08:29:49AM -0700, Jacob Keller wrote:
> > The Travis tests do exactly this (along with --tee to actually save the
> > output). It seems like a minor miracle that this is the first test
> > output that has actually triggered as TAP input. I'd suggest that the
> > problem is not in the test, though, but that our "--verbose" option is
> > unsuitable for using with a TAP harness.
> >
> > The obvious fix would be to send "--verbose" output to stderr, but I
> > suspect that would end up annoying for people who do:
> >
> > ./t5547-push-quarantine.sh -v | less
> >
> > to read long output. Probably we need some option like "--log" which
> > logs in the same way that "--tee" does, but _without_ sending the data
> > to stdout. Naively, that just means replacing the "tee" invocation with
> > "cat", but I suspect it will be a lot more complicated than that,
> > because we still need to let the TAP output go to stdout.
>
> Can we determine that we're running with something monitoring the TAP
> output? Because then we could make verbose go to stderr instead
> dynamically?
I think $HARNESS_ACTIVE could tell us that. But the hard part isn't
activating it; it's directing the verbose output to the log without
sending it to stdout.
See the patch I posted later in the thread, and my musings on
auto-activating it. I guess we could do so safely when we see
$HARNESS_ACTIVE along with "--tee" and "--verbose", though I don't know
if it's worth the trouble.
For $HARNESS_ACTIVE with _just_ "--verbose", I don't think it would be a
good idea to activate it. We should either silently ignore --verbose
then, or complain and die.
-Peff
^ permalink raw reply
* Re: [PATCH v4 20/25] sequencer: refactor write_message()
From: Junio C Hamano @ 2016-10-21 15:40 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <alpine.DEB.2.20.1610211343310.3264@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> Ah, make that four steps. The final one is:
>>
>> - add append_eol parameter that nobody uses at this step in the
>> series.
>>
>> This is a new feature to the helper. While it is OK to have it as a
>> preparatory step in this series, it is easier to understand if it
>> kept as a separate step. It is even more preferrable if it is made
>> as a preparatory step in a series that adds a caller that passes
>> true to append_eol to this helper...
>
> Done,
> Dscho
Hmm, what has been done exactly? I still see append_eol in v5 where
nobody uses it yet. Confused...
^ permalink raw reply
* Re: Prove "Tests out of sequence" Error
From: Jacob Keller @ 2016-10-21 15:42 UTC (permalink / raw)
To: Jeff King; +Cc: Stefan Beller, Lars Schneider, git
In-Reply-To: <20161021153534.mjed3sqhxjelxzwx@sigill.intra.peff.net>
On Fri, Oct 21, 2016 at 8:35 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Oct 21, 2016 at 08:29:49AM -0700, Jacob Keller wrote:
>
>> > The Travis tests do exactly this (along with --tee to actually save the
>> > output). It seems like a minor miracle that this is the first test
>> > output that has actually triggered as TAP input. I'd suggest that the
>> > problem is not in the test, though, but that our "--verbose" option is
>> > unsuitable for using with a TAP harness.
>> >
>> > The obvious fix would be to send "--verbose" output to stderr, but I
>> > suspect that would end up annoying for people who do:
>> >
>> > ./t5547-push-quarantine.sh -v | less
>> >
>> > to read long output. Probably we need some option like "--log" which
>> > logs in the same way that "--tee" does, but _without_ sending the data
>> > to stdout. Naively, that just means replacing the "tee" invocation with
>> > "cat", but I suspect it will be a lot more complicated than that,
>> > because we still need to let the TAP output go to stdout.
>>
>> Can we determine that we're running with something monitoring the TAP
>> output? Because then we could make verbose go to stderr instead
>> dynamically?
>
> I think $HARNESS_ACTIVE could tell us that. But the hard part isn't
> activating it; it's directing the verbose output to the log without
> sending it to stdout.
>
> See the patch I posted later in the thread, and my musings on
> auto-activating it. I guess we could do so safely when we see
> $HARNESS_ACTIVE along with "--tee" and "--verbose", though I don't know
> if it's worth the trouble.
>
> For $HARNESS_ACTIVE with _just_ "--verbose", I don't think it would be a
> good idea to activate it. We should either silently ignore --verbose
> then, or complain and die.
>
We should probably do that to make sure people realize what might
happen. I read your series and it has a good explanation of the
possible alternatives. I like the approach you chose.
Thanks,
Jake
> -Peff
^ permalink raw reply
* Re: Prove "Tests out of sequence" Error
From: Jeff King @ 2016-10-21 15:48 UTC (permalink / raw)
To: Jacob Keller; +Cc: Stefan Beller, Lars Schneider, git
In-Reply-To: <CA+P7+xrix3UZDEE1swG_=jBLwWHA04KoYikWBBMNx7aa=Hevmw@mail.gmail.com>
On Fri, Oct 21, 2016 at 08:42:58AM -0700, Jacob Keller wrote:
> > For $HARNESS_ACTIVE with _just_ "--verbose", I don't think it would be a
> > good idea to activate it. We should either silently ignore --verbose
> > then, or complain and die.
>
> We should probably do that to make sure people realize what might
> happen. I read your series and it has a good explanation of the
> possible alternatives. I like the approach you chose.
Thanks. Do you want to make a patch on top of my series?
-Peff
^ permalink raw reply
* Re: [PATCH] daemon, path.c: fix a bug with ~ in repo paths
From: Junio C Hamano @ 2016-10-21 15:58 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Luke Shumaker, Git Mailing List, Jeff King
In-Reply-To: <CACsJy8AzWqJ_9JQ_BGnAB3E3GrGSTS40z8umbtO6mQJukWOooQ@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> The amount of changes is unbelievable for fixing such a rare case
> though. I wonder if we can just detect this in daemon.c and pass
> "./~foo/bar" instead of "~foo/bar" to enter_repo() in non-strict mode
> to "disable" expand_user_path(). If it works, it's much simpler
> changes (even though a bit hacky)
Conceptually, it ought to be updating the code that says "Does it
begin with a tilde? Then try to do user-path expansion and fail if
that is not enabled and if it succeeds use the resulting directory"
to "Is user-path enabled and does it begin with a tilde? Then try
to do user-path expansion and if it succeeds use the resulting
directory". Compared to that mental model we have with this
codepath, I agree that the change does look quite invasive and
large.
It is OK for a change to be large, as long as the end result is
easier to read and understand than the original. I am undecided if
that is the case with this patch, though.
Also I am a bit worried about the change in the semantics. While it
is not the best way to achieve this, the server operators could use
it as a way to add directories whose contents need to be hidden to
give them names that begin with a tilde without enabling user-path
expansion. This change may be a new and useful feature for Luke,
but to these server operators this change can be a new bug---it is
probably a minor new bug as they can work it around by using other
means to hide these directories, though.
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2016, #05; Thu, 20)
From: Lars Schneider @ 2016-10-21 16:11 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.2.20.1610211457030.3264@virtualbox>
> On 21 Oct 2016, at 06:08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> Hi Junio & Lars,
>
> On Thu, 20 Oct 2016, Junio C Hamano wrote:
>
>> * ls/filter-process (2016-10-17) 14 commits
>> (merged to 'next' on 2016-10-19 at ffd0de042c)
>> + contrib/long-running-filter: add long running filter example
>> + convert: add filter.<driver>.process option
>> + convert: prepare filter.<driver>.process option
>> + convert: make apply_filter() adhere to standard Git error handling
>> + pkt-line: add functions to read/write flush terminated packet streams
>> + pkt-line: add packet_write_gently()
>> + pkt-line: add packet_flush_gently()
>> + pkt-line: add packet_write_fmt_gently()
>> + pkt-line: extract set_packet_header()
>> + pkt-line: rename packet_write() to packet_write_fmt()
>> + run-command: add clean_on_exit_handler
>> + run-command: move check_pipe() from write_or_die to run_command
>> + convert: modernize tests
>> + convert: quote filter names in error messages
>>
>> The smudge/clean filter API expect an external process is spawned
>> to filter the contents for each path that has a filter defined. A
>> new type of "process" filter API has been added to allow the first
>> request to run the filter for a path to spawn a single process, and
>> all filtering need is served by this single process for multiple
>> paths, reducing the process creation overhead.
>>
>> Will merge to 'master'.
>
> This breaks in Git for Windows' SDK (I only now realized that t0060 was
> not the only thing breaking in `next` for a while now):
> ...
> -- snap --
>
> Unsurprisingly, bisect identifies "convert: add filter.<driver>.process
> option" as the first bad commit, although it only fails on the test case
> 15, but not on 17.
>
> I am unfortunately still busy with trying to figure out what exactly makes
> t6030 hang on `pu` (seems it thinks stdin is a tty and just waits for an
> answer), and then trying to reduce that insane amount of time wasted on
> running, and waiting for, the test suite, and for unrelated reasons I'll
> have to go offline for the rest of the day, so I will most likely be
> unable to assist further with this.
Hi Johannes,
thanks for reporting this. Looks like I misunderstood your comment when
you wrote the error is already fixed in GfW:
https://github.com/git-for-windows/git/issues/770#issuecomment-251426487
I think this patch series (referenced by the link above, too) should fix
the problem:
http://public-inbox.org/git/20160905211111.72956-1-larsxschneider@gmail.com/
Don't waste any time on this. I will look into it ASAP (traveling right now).
Cheers,
Lars
^ permalink raw reply
* Re: Prove "Tests out of sequence" Error
From: Jacob Keller @ 2016-10-21 16:15 UTC (permalink / raw)
To: Jeff King; +Cc: Stefan Beller, Lars Schneider, git
In-Reply-To: <20161021154840.pdmb5c3gjharsrxe@sigill.intra.peff.net>
On Fri, Oct 21, 2016 at 8:48 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Oct 21, 2016 at 08:42:58AM -0700, Jacob Keller wrote:
>
>> > For $HARNESS_ACTIVE with _just_ "--verbose", I don't think it would be a
>> > good idea to activate it. We should either silently ignore --verbose
>> > then, or complain and die.
>>
>> We should probably do that to make sure people realize what might
>> happen. I read your series and it has a good explanation of the
>> possible alternatives. I like the approach you chose.
>
> Thanks. Do you want to make a patch on top of my series?
>
> -Peff
I am not sure I will find time to do it today, so it wouldn't be for a
few more days.
Thanks,
Jake
^ permalink raw reply
* Re: Problems with "git svn clone"
From: K Richard Pixley @ 2016-10-21 16:33 UTC (permalink / raw)
To: Eric Wong; +Cc: git
In-Reply-To: <20161020004956.GA8083@starla>
Thank you.
Running on debian stretch/testing it got past the 11 second mark and has
been running for a day now. I expect it will take days, perhaps longer,
so we'll see.
--rich
________________________________
- CONFIDENTIAL-
This email and any files transmitted with it are confidential, and may also be legally privileged. If you are not the intended recipient, you may not review, use, copy, or distribute this message. If you receive this email in error, please notify the sender immediately by reply email and then delete this email.
^ permalink raw reply
* Re: [PATCH 1/2] t9000-addresses: update expected results after fix
From: Junio C Hamano @ 2016-10-21 16:48 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, Ramsay Jones
In-Reply-To: <20161021092024.15861-1-Matthieu.Moy@imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> e3fdbcc8e1 (parse_mailboxes: accept extra text after <...> address,
> 2016-10-13) improved our in-house address parser and made it closer to
> Mail::Address. As a consequence, some tests comparing it to
> Mail::Address now pass, but e3fdbcc8e1 forgot to update the test.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
Thanks.
^ permalink raw reply
* Re: [BUG] fetch output is ugly in 'next'
From: Junio C Hamano @ 2016-10-21 16:50 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Jeff King, Git Mailing List
In-Reply-To: <CACsJy8CkK8K0ty3fv9qyD7wwB+81VPB-P1UUCbDJTJ4iM0Y+Cw@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Fri, Oct 21, 2016 at 7:11 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>> Yeah.. replacing the 4 DEFAULT_ABBREV in fetch.c with something
>> sensible should do it.
>
> Correction (if somebody will pick this up), it's
> TRANSPORT_SUMMARY_WIDTH that needs to be adjusted, not those four.
Yes, it used to be and it still is (2 * DEFAULT_ABBREV + 3) but in
the new world order where default-abbrev is often -1 the expression
does not make much sense.
Perhaps something along this line?
transport.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/transport.h b/transport.h
index 18d2cf8275..5339fabbad 100644
--- a/transport.h
+++ b/transport.h
@@ -127,7 +127,7 @@ struct transport {
#define TRANSPORT_PUSH_CERT 2048
#define TRANSPORT_PUSH_ATOMIC 4096
-#define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
+#define TRANSPORT_SUMMARY_WIDTH (2 * (DEFAULT_ABBREV < 0 ? 7 : DEFAULT_ABBREV) + 3)
#define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_width(x)), (x)
/* Returns a transport suitable for the url */
^ permalink raw reply related
* Re: [PATCH 2/3] test-lib: add --verbose-log option
From: Junio C Hamano @ 2016-10-21 17:12 UTC (permalink / raw)
To: Jeff King; +Cc: Stefan Beller, Lars Schneider, git
In-Reply-To: <20161021104759.hx6q7u66r3ll7l44@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> This patch implements option (5), which seems to work well
> in practice.
Your journey to reach this final design and implementation may have
been painful---I really like the end result. This was an accident
waiting to happen.
Thanks for fixing it.
> @@ -319,7 +332,10 @@ fi
>
> exec 5>&1
> exec 6<&0
> -if test "$verbose" = "t"
> +if test "$verbose_log" = "t"
> +then
> + exec 3>>"$GIT_TEST_TEE_OUTPUT_FILE" 4>&3
> +elif test "$verbose" = "t"
> then
> exec 4>&2 3>&1
> else
OK, unlike "verbose" case where we give 3 (saved stdout) to 1 and 4
(saved stderr) to 2, we send 3 to the output file and 4 to the same.
Makes sense.
^ permalink raw reply
* Re: [PATCH 0/3] fix travis TAP/--verbose conflict
From: Stefan Beller @ 2016-10-21 17:19 UTC (permalink / raw)
To: Jeff King; +Cc: Lars Schneider, git
In-Reply-To: <20161021104107.vh3bjx6x6pd6izat@sigill.intra.peff.net>
On Fri, Oct 21, 2016 at 3:41 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Oct 21, 2016 at 04:43:48AM -0400, Jeff King wrote:
>
>> The obvious fix would be to send "--verbose" output to stderr, but I
>> suspect that would end up annoying for people who do:
>>
>> ./t5547-push-quarantine.sh -v | less
>>
>> to read long output. Probably we need some option like "--log" which
>> logs in the same way that "--tee" does, but _without_ sending the data
>> to stdout. Naively, that just means replacing the "tee" invocation with
>> "cat", but I suspect it will be a lot more complicated than that,
>> because we still need to let the TAP output go to stdout.
>
> Yeah, it was definitely a lot more complicated. This patch series fixes
> it.
>
> [1/3]: test-lib: handle TEST_OUTPUT_DIRECTORY with spaces
> [2/3]: test-lib: add --verbose-log option
> [3/3]: travis: use --verbose-log test option
All patches look good to me
(1&3 are obvious, and 2 is very well described).
Thanks,
Stefan
^ permalink raw reply
* Re: [BUG] [PATCH]: run-command.c
From: Junio C Hamano @ 2016-10-21 17:19 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20161021090029.glr5u6gwrxluavir@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> 16:42:45$ cat /proc/2601/cmdline | xargs -0 -n1 echo
>> /bin/sh
>> -c
>> echo $PWD;sleep 600
>> echo $PWD;sleep 600
>
> This duplicated argument is expected and normal.
Well explained. Thanks.
^ permalink raw reply
* [PATCH v5 0/8] allow non-trailers and multiple-line trailers
From: Jonathan Tan @ 2016-10-21 17:54 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, sbeller, gitster
In-Reply-To: <cover.1476232683.git.jonathantanmy@google.com>
I've updated patch 5/8 to use strcspn and to pass in the list of
separators, meaning that we no longer accept '=' in file input (and also
updated its commit message accordingly).
We also discussed inlining find_separator, but after looking at the
code, I think that it is more convenient if find_separator returns -1
when there is no separator, because of the 3 times it is used (as
of 8/8), it is checked twice with '>= 1' (since both "no separator" and
"string begins with separator" are handled in the same way - treating
them as a non-trailer line). So I have left it as its own function.
No other updates.
Jonathan Tan (8):
trailer: improve const correctness
trailer: use list.h for doubly-linked list
trailer: streamline trailer item create and add
trailer: make args have their own struct
trailer: clarify failure modes in parse_trailer
trailer: allow non-trailers in trailer block
trailer: forbid leading whitespace in trailers
trailer: support values folded to multiple lines
Documentation/git-interpret-trailers.txt | 14 +-
t/t7513-interpret-trailers.sh | 299 +++++++++++++++
trailer.c | 620 +++++++++++++++++--------------
3 files changed, 654 insertions(+), 279 deletions(-)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply
* [PATCH v5 1/8] trailer: improve const correctness
From: Jonathan Tan @ 2016-10-21 17:54 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, sbeller, gitster
In-Reply-To: <cover.1477072247.git.jonathantanmy@google.com>
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: Jonathan Tan <jonathantanmy@google.com>
---
trailer.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/trailer.c b/trailer.c
index c6ea9ac..1f191b2 100644
--- a/trailer.c
+++ b/trailer.c
@@ -27,8 +27,8 @@ static struct conf_info default_conf_info;
struct trailer_item {
struct trailer_item *previous;
struct trailer_item *next;
- const char *token;
- const char *value;
+ char *token;
+ char *value;
struct conf_info conf;
};
@@ -95,8 +95,8 @@ static void free_trailer_item(struct trailer_item *item)
free(item->conf.name);
free(item->conf.key);
free(item->conf.command);
- free((char *)item->token);
- free((char *)item->value);
+ free(item->token);
+ free(item->value);
free(item);
}
@@ -215,13 +215,13 @@ static struct trailer_item *remove_first(struct trailer_item **first)
return item;
}
-static const char *apply_command(const char *command, const char *arg)
+static char *apply_command(const char *command, const char *arg)
{
struct strbuf cmd = STRBUF_INIT;
struct strbuf buf = STRBUF_INIT;
struct child_process cp = CHILD_PROCESS_INIT;
const char *argv[] = {NULL, NULL};
- const char *result;
+ char *result;
strbuf_addstr(&cmd, command);
if (arg)
@@ -425,7 +425,7 @@ static int set_if_missing(struct conf_info *item, const char *value)
return 0;
}
-static void duplicate_conf(struct conf_info *dst, struct conf_info *src)
+static void duplicate_conf(struct conf_info *dst, const struct conf_info *src)
{
*dst = *src;
if (src->name)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v5 5/8] trailer: clarify failure modes in parse_trailer
From: Jonathan Tan @ 2016-10-21 17:55 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, sbeller, gitster
In-Reply-To: <cover.1477072247.git.jonathantanmy@google.com>
The parse_trailer function has a few modes of operation, all depending
on whether the separator is present in its input, and if yes, the
separator's position. Some of these modes are failure modes, and these
failure modes are handled differently depending on whether the trailer
line was sourced from a file or from a command-line argument.
Extract a function to find the separator, allowing the invokers of
parse_trailer to determine how to handle the failure modes instead of
making parse_trailer do it. In this function, also take in the list of
separators, so that we can distinguish between command line arguments
(which allow '=' as separator) and file input (which does not allow '='
as separator).
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
trailer.c | 75 ++++++++++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 53 insertions(+), 22 deletions(-)
diff --git a/trailer.c b/trailer.c
index 99018f8..aff858b 100644
--- a/trailer.c
+++ b/trailer.c
@@ -543,29 +543,37 @@ static int token_matches_item(const char *tok, struct arg_item *item, int tok_le
return item->conf.key ? !strncasecmp(tok, item->conf.key, tok_len) : 0;
}
-static int parse_trailer(struct strbuf *tok, struct strbuf *val,
- const struct conf_info **conf, const char *trailer)
+/*
+ * Return the location of the first separator in line, or -1 if there is no
+ * separator.
+ */
+static int find_separator(const char *line, const char *separators)
+{
+ int loc = strcspn(line, separators);
+ if (!line[loc])
+ return -1;
+ return loc;
+}
+
+/*
+ * Obtain the token, value, and conf from the given trailer.
+ *
+ * separator_pos must not be 0, since the token cannot be an empty string.
+ *
+ * If separator_pos is -1, interpret the whole trailer as a token.
+ */
+static void parse_trailer(struct strbuf *tok, struct strbuf *val,
+ const struct conf_info **conf, const char *trailer,
+ int separator_pos)
{
- size_t len;
- struct strbuf seps = STRBUF_INIT;
struct arg_item *item;
int tok_len;
struct list_head *pos;
- strbuf_addstr(&seps, separators);
- strbuf_addch(&seps, '=');
- len = strcspn(trailer, seps.buf);
- strbuf_release(&seps);
- if (len == 0) {
- int l = strlen(trailer);
- while (l > 0 && isspace(trailer[l - 1]))
- l--;
- return error(_("empty trailer token in trailer '%.*s'"), l, trailer);
- }
- if (len < strlen(trailer)) {
- strbuf_add(tok, trailer, len);
+ if (separator_pos != -1) {
+ strbuf_add(tok, trailer, separator_pos);
strbuf_trim(tok);
- strbuf_addstr(val, trailer + len + 1);
+ strbuf_addstr(val, trailer + separator_pos + 1);
strbuf_trim(val);
} else {
strbuf_addstr(tok, trailer);
@@ -587,8 +595,6 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val,
break;
}
}
-
- return 0;
}
static void add_trailer_item(struct list_head *head, char *tok, char *val)
@@ -619,6 +625,12 @@ static void process_command_line_args(struct list_head *arg_head,
const struct conf_info *conf;
struct list_head *pos;
+ /*
+ * In command-line arguments, '=' is accepted (in addition to the
+ * separators that are defined).
+ */
+ char *cl_separators = xstrfmt("=%s", separators);
+
/* Add an arg item for each configured trailer with a command */
list_for_each(pos, &conf_head) {
item = list_entry(pos, struct arg_item, list);
@@ -631,12 +643,25 @@ static void process_command_line_args(struct list_head *arg_head,
/* Add an arg item for each trailer on the command line */
for_each_string_list_item(tr, trailers) {
- if (!parse_trailer(&tok, &val, &conf, tr->string))
+ int separator_pos = find_separator(tr->string, cl_separators);
+ if (separator_pos == 0) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_addstr(&sb, tr->string);
+ strbuf_trim(&sb);
+ error(_("empty trailer token in trailer '%.*s'"),
+ (int) sb.len, sb.buf);
+ strbuf_release(&sb);
+ } else {
+ parse_trailer(&tok, &val, &conf, tr->string,
+ separator_pos);
add_arg_item(arg_head,
strbuf_detach(&tok, NULL),
strbuf_detach(&val, NULL),
conf);
+ }
}
+
+ free(cl_separators);
}
static struct strbuf **read_input_file(const char *file)
@@ -775,11 +800,17 @@ static int process_input_file(FILE *outfile,
/* Parse trailer lines */
for (i = trailer_start; i < trailer_end; i++) {
- if (lines[i]->buf[0] != comment_line_char &&
- !parse_trailer(&tok, &val, NULL, lines[i]->buf))
+ int separator_pos;
+ if (lines[i]->buf[0] == comment_line_char)
+ continue;
+ separator_pos = find_separator(lines[i]->buf, separators);
+ if (separator_pos >= 1) {
+ parse_trailer(&tok, &val, NULL, lines[i]->buf,
+ separator_pos);
add_trailer_item(head,
strbuf_detach(&tok, NULL),
strbuf_detach(&val, NULL));
+ }
}
return trailer_end;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v5 4/8] trailer: make args have their own struct
From: Jonathan Tan @ 2016-10-21 17:54 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, sbeller, gitster
In-Reply-To: <cover.1477072247.git.jonathantanmy@google.com>
Improve type safety by making arguments (whether from configuration or
from the command line) have their own "struct arg_item" type, separate
from the "struct trailer_item" type used to represent the trailers in
the buffer being manipulated.
This change also prepares "struct trailer_item" to be further
differentiated from "struct arg_item" in future patches.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
trailer.c | 135 +++++++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 85 insertions(+), 50 deletions(-)
diff --git a/trailer.c b/trailer.c
index ae3972a..99018f8 100644
--- a/trailer.c
+++ b/trailer.c
@@ -29,6 +29,12 @@ struct trailer_item {
struct list_head list;
char *token;
char *value;
+};
+
+struct arg_item {
+ struct list_head list;
+ char *token;
+ char *value;
struct conf_info conf;
};
@@ -62,7 +68,7 @@ static size_t token_len_without_separator(const char *token, size_t len)
return len;
}
-static int same_token(struct trailer_item *a, struct trailer_item *b)
+static int same_token(struct trailer_item *a, struct arg_item *b)
{
size_t a_len = token_len_without_separator(a->token, strlen(a->token));
size_t b_len = token_len_without_separator(b->token, strlen(b->token));
@@ -71,12 +77,12 @@ static int same_token(struct trailer_item *a, struct trailer_item *b)
return !strncasecmp(a->token, b->token, min_len);
}
-static int same_value(struct trailer_item *a, struct trailer_item *b)
+static int same_value(struct trailer_item *a, struct arg_item *b)
{
return !strcasecmp(a->value, b->value);
}
-static int same_trailer(struct trailer_item *a, struct trailer_item *b)
+static int same_trailer(struct trailer_item *a, struct arg_item *b)
{
return same_token(a, b) && same_value(a, b);
}
@@ -98,6 +104,13 @@ static inline void strbuf_replace(struct strbuf *sb, const char *a, const char *
static void free_trailer_item(struct trailer_item *item)
{
+ free(item->token);
+ free(item->value);
+ free(item);
+}
+
+static void free_arg_item(struct arg_item *item)
+{
free(item->conf.name);
free(item->conf.key);
free(item->conf.command);
@@ -137,17 +150,29 @@ static void print_all(FILE *outfile, struct list_head *head, int trim_empty)
}
}
+static struct trailer_item *trailer_from_arg(struct arg_item *arg_tok)
+{
+ struct trailer_item *new = xcalloc(sizeof(*new), 1);
+ new->token = arg_tok->token;
+ new->value = arg_tok->value;
+ arg_tok->token = arg_tok->value = NULL;
+ free_arg_item(arg_tok);
+ return new;
+}
+
static void add_arg_to_input_list(struct trailer_item *on_tok,
- struct trailer_item *arg_tok)
+ struct arg_item *arg_tok)
{
- if (after_or_end(arg_tok->conf.where))
- list_add(&arg_tok->list, &on_tok->list);
+ int aoe = after_or_end(arg_tok->conf.where);
+ struct trailer_item *to_add = trailer_from_arg(arg_tok);
+ if (aoe)
+ list_add(&to_add->list, &on_tok->list);
else
- list_add_tail(&arg_tok->list, &on_tok->list);
+ list_add_tail(&to_add->list, &on_tok->list);
}
static int check_if_different(struct trailer_item *in_tok,
- struct trailer_item *arg_tok,
+ struct arg_item *arg_tok,
int check_all,
struct list_head *head)
{
@@ -200,7 +225,7 @@ static char *apply_command(const char *command, const char *arg)
return result;
}
-static void apply_item_command(struct trailer_item *in_tok, struct trailer_item *arg_tok)
+static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg_tok)
{
if (arg_tok->conf.command) {
const char *arg;
@@ -218,13 +243,13 @@ static void apply_item_command(struct trailer_item *in_tok, struct trailer_item
}
static void apply_arg_if_exists(struct trailer_item *in_tok,
- struct trailer_item *arg_tok,
+ struct arg_item *arg_tok,
struct trailer_item *on_tok,
struct list_head *head)
{
switch (arg_tok->conf.if_exists) {
case EXISTS_DO_NOTHING:
- free_trailer_item(arg_tok);
+ free_arg_item(arg_tok);
break;
case EXISTS_REPLACE:
apply_item_command(in_tok, arg_tok);
@@ -241,39 +266,41 @@ static void apply_arg_if_exists(struct trailer_item *in_tok,
if (check_if_different(in_tok, arg_tok, 1, head))
add_arg_to_input_list(on_tok, arg_tok);
else
- free_trailer_item(arg_tok);
+ free_arg_item(arg_tok);
break;
case EXISTS_ADD_IF_DIFFERENT_NEIGHBOR:
apply_item_command(in_tok, arg_tok);
if (check_if_different(on_tok, arg_tok, 0, head))
add_arg_to_input_list(on_tok, arg_tok);
else
- free_trailer_item(arg_tok);
+ free_arg_item(arg_tok);
break;
}
}
static void apply_arg_if_missing(struct list_head *head,
- struct trailer_item *arg_tok)
+ struct arg_item *arg_tok)
{
enum action_where where;
+ struct trailer_item *to_add;
switch (arg_tok->conf.if_missing) {
case MISSING_DO_NOTHING:
- free_trailer_item(arg_tok);
+ free_arg_item(arg_tok);
break;
case MISSING_ADD:
where = arg_tok->conf.where;
apply_item_command(NULL, arg_tok);
+ to_add = trailer_from_arg(arg_tok);
if (after_or_end(where))
- list_add_tail(&arg_tok->list, head);
+ list_add_tail(&to_add->list, head);
else
- list_add(&arg_tok->list, head);
+ list_add(&to_add->list, head);
}
}
static int find_same_and_apply_arg(struct list_head *head,
- struct trailer_item *arg_tok)
+ struct arg_item *arg_tok)
{
struct list_head *pos;
struct trailer_item *in_tok;
@@ -306,11 +333,11 @@ static void process_trailers_lists(struct list_head *head,
struct list_head *arg_head)
{
struct list_head *pos, *p;
- struct trailer_item *arg_tok;
+ struct arg_item *arg_tok;
list_for_each_safe(pos, p, arg_head) {
int applied = 0;
- arg_tok = list_entry(pos, struct trailer_item, list);
+ arg_tok = list_entry(pos, struct arg_item, list);
list_del(pos);
@@ -375,20 +402,20 @@ static void duplicate_conf(struct conf_info *dst, const struct conf_info *src)
dst->command = xstrdup(src->command);
}
-static struct trailer_item *get_conf_item(const char *name)
+static struct arg_item *get_conf_item(const char *name)
{
struct list_head *pos;
- struct trailer_item *item;
+ struct arg_item *item;
/* Look up item with same name */
list_for_each(pos, &conf_head) {
- item = list_entry(pos, struct trailer_item, list);
+ item = list_entry(pos, struct arg_item, list);
if (!strcasecmp(item->conf.name, name))
return item;
}
/* Item does not already exists, create it */
- item = xcalloc(sizeof(struct trailer_item), 1);
+ item = xcalloc(sizeof(*item), 1);
duplicate_conf(&item->conf, &default_conf_info);
item->conf.name = xstrdup(name);
@@ -442,7 +469,7 @@ static int git_trailer_default_config(const char *conf_key, const char *value, v
static int git_trailer_config(const char *conf_key, const char *value, void *cb)
{
const char *trailer_item, *variable_name;
- struct trailer_item *item;
+ struct arg_item *item;
struct conf_info *conf;
char *name = NULL;
enum trailer_info_type type;
@@ -500,7 +527,7 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb)
return 0;
}
-static const char *token_from_item(struct trailer_item *item, char *tok)
+static const char *token_from_item(struct arg_item *item, char *tok)
{
if (item->conf.key)
return item->conf.key;
@@ -509,7 +536,7 @@ static const char *token_from_item(struct trailer_item *item, char *tok)
return item->conf.name;
}
-static int token_matches_item(const char *tok, struct trailer_item *item, int tok_len)
+static int token_matches_item(const char *tok, struct arg_item *item, int tok_len)
{
if (!strncasecmp(tok, item->conf.name, tok_len))
return 1;
@@ -521,7 +548,7 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val,
{
size_t len;
struct strbuf seps = STRBUF_INIT;
- struct trailer_item *item;
+ struct arg_item *item;
int tok_len;
struct list_head *pos;
@@ -547,12 +574,14 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val,
/* Lookup if the token matches something in the config */
tok_len = token_len_without_separator(tok->buf, tok->len);
- *conf = &default_conf_info;
+ if (conf)
+ *conf = &default_conf_info;
list_for_each(pos, &conf_head) {
- item = list_entry(pos, struct trailer_item, list);
+ item = list_entry(pos, struct arg_item, list);
if (token_matches_item(tok->buf, item, tok_len)) {
char *tok_buf = strbuf_detach(tok, NULL);
- *conf = &item->conf;
+ if (conf)
+ *conf = &item->conf;
strbuf_addstr(tok, token_from_item(item, tok_buf));
free(tok_buf);
break;
@@ -562,43 +591,51 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val,
return 0;
}
-static void add_trailer_item(struct list_head *head, char *tok, char *val,
- const struct conf_info *conf)
+static void add_trailer_item(struct list_head *head, char *tok, char *val)
{
struct trailer_item *new = xcalloc(sizeof(*new), 1);
new->token = tok;
new->value = val;
- duplicate_conf(&new->conf, conf);
list_add_tail(&new->list, head);
}
+static void add_arg_item(struct list_head *arg_head, char *tok, char *val,
+ const struct conf_info *conf)
+{
+ struct arg_item *new = xcalloc(sizeof(*new), 1);
+ new->token = tok;
+ new->value = val;
+ duplicate_conf(&new->conf, conf);
+ list_add_tail(&new->list, arg_head);
+}
+
static void process_command_line_args(struct list_head *arg_head,
struct string_list *trailers)
{
struct string_list_item *tr;
- struct trailer_item *item;
+ struct arg_item *item;
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
const struct conf_info *conf;
struct list_head *pos;
- /* Add a trailer item for each configured trailer with a command */
+ /* Add an arg item for each configured trailer with a command */
list_for_each(pos, &conf_head) {
- item = list_entry(pos, struct trailer_item, list);
+ item = list_entry(pos, struct arg_item, list);
if (item->conf.command)
- add_trailer_item(arg_head,
- xstrdup(token_from_item(item, NULL)),
- xstrdup(""),
- &item->conf);
+ add_arg_item(arg_head,
+ xstrdup(token_from_item(item, NULL)),
+ xstrdup(""),
+ &item->conf);
}
- /* Add a trailer item for each trailer on the command line */
+ /* Add an arg item for each trailer on the command line */
for_each_string_list_item(tr, trailers) {
if (!parse_trailer(&tok, &val, &conf, tr->string))
- add_trailer_item(arg_head,
- strbuf_detach(&tok, NULL),
- strbuf_detach(&val, NULL),
- conf);
+ add_arg_item(arg_head,
+ strbuf_detach(&tok, NULL),
+ strbuf_detach(&val, NULL),
+ conf);
}
}
@@ -721,7 +758,6 @@ static int process_input_file(FILE *outfile,
int patch_start, trailer_start, trailer_end, i;
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
- const struct conf_info *conf;
/* Get the line count */
while (lines[count])
@@ -740,11 +776,10 @@ static int process_input_file(FILE *outfile,
/* Parse trailer lines */
for (i = trailer_start; i < trailer_end; i++) {
if (lines[i]->buf[0] != comment_line_char &&
- !parse_trailer(&tok, &val, &conf, lines[i]->buf))
+ !parse_trailer(&tok, &val, NULL, lines[i]->buf))
add_trailer_item(head,
strbuf_detach(&tok, NULL),
- strbuf_detach(&val, NULL),
- conf);
+ strbuf_detach(&val, NULL));
}
return trailer_end;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v5 7/8] trailer: forbid leading whitespace in trailers
From: Jonathan Tan @ 2016-10-21 17:55 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, sbeller, gitster
In-Reply-To: <cover.1477072247.git.jonathantanmy@google.com>
Currently, interpret-trailers allows leading whitespace in trailer
lines. This leads to false positives, especially for quoted lines or
bullet lists.
Forbid leading whitespace in trailers.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
Documentation/git-interpret-trailers.txt | 2 +-
t/t7513-interpret-trailers.sh | 15 +++++++++++++++
trailer.c | 2 +-
3 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
index cf4c5ea..4966b5b 100644
--- a/Documentation/git-interpret-trailers.txt
+++ b/Documentation/git-interpret-trailers.txt
@@ -55,7 +55,7 @@ The group must either be at the end of the message or be the last
non-whitespace lines before a line that starts with '---'. Such three
minus signs start the patch part of the message.
-When reading trailers, there can be whitespaces before and after the
+When reading trailers, there can be whitespaces after the
token, the separator and the value. There can also be whitespaces
inside the token and the value.
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 003e90f..3d94b3a 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -241,6 +241,21 @@ test_expect_success 'with non-trailer lines only' '
test_cmp expected actual
'
+test_expect_success 'line with leading whitespace is not trailer' '
+ q_to_tab >patch <<-\EOF &&
+
+ Qtoken: value
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ Qtoken: value
+
+ token: value
+ EOF
+ git interpret-trailers --trailer "token: value" patch >actual &&
+ test_cmp expected actual
+'
+
test_expect_success 'with config setup' '
git config trailer.ack.key "Acked-by: " &&
cat >expected <<-\EOF &&
diff --git a/trailer.c b/trailer.c
index 199f86a..d978437 100644
--- a/trailer.c
+++ b/trailer.c
@@ -775,7 +775,7 @@ static int find_trailer_start(struct strbuf **lines, int count)
}
separator_pos = find_separator(lines[start]->buf);
- if (separator_pos >= 1) {
+ if (separator_pos >= 1 && !isspace(lines[start]->buf[0])) {
struct list_head *pos;
trailer_lines++;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v5 6/8] trailer: allow non-trailers in trailer block
From: Jonathan Tan @ 2016-10-21 17:55 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, sbeller, gitster
In-Reply-To: <cover.1477072247.git.jonathantanmy@google.com>
Currently, interpret-trailers requires all lines of a trailer block to
be trailers (or comments) - if not it would not identify that block as a
trailer block, and thus create its own trailer block, inserting a blank
line. For example:
echo -e "\nSigned-off-by: x\nnot trailer" |
git interpret-trailers --trailer "c: d"
would result in:
Signed-off-by: x
not trailer
c: d
Relax the definition of a trailer block to require that the trailers (i)
are all trailers, or (ii) contain at least one Git-generated trailer and
consists of at least 25% trailers.
Signed-off-by: x
not trailer
c: d
(i) is the existing functionality. (ii) allows arbitrary lines to be
included in trailer blocks, like those in [1], and still allow
interpret-trailers to be used.
[1]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/stable/linux-stable/+/e7d316a02f683864a12389f8808570e37fb90aa3
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
Documentation/git-interpret-trailers.txt | 5 +-
t/t7513-interpret-trailers.sh | 115 +++++++++++++++++++++++++++++++
trailer.c | 89 ++++++++++++++++++++----
3 files changed, 194 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
index 93d1db6..cf4c5ea 100644
--- a/Documentation/git-interpret-trailers.txt
+++ b/Documentation/git-interpret-trailers.txt
@@ -48,8 +48,9 @@ with only spaces at the end of the commit message part, one blank line
will be added before the new trailer.
Existing trailers are extracted from the input message by looking for
-a group of one or more lines that contain a colon (by default), where
-the group is preceded by one or more empty (or whitespace-only) lines.
+a group of one or more lines that (i) are all trailers, or (ii) contains at
+least one Git-generated trailer and consists of at least 25% trailers.
+The group must be preceded by one or more empty (or whitespace-only) lines.
The group must either be at the end of the message or be the last
non-whitespace lines before a line that starts with '---'. Such three
minus signs start the patch part of the message.
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index aee785c..003e90f 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -126,6 +126,121 @@ test_expect_success 'with multiline title in the message' '
test_cmp expected actual
'
+test_expect_success 'with non-trailer lines mixed with Signed-off-by' '
+ cat >patch <<-\EOF &&
+
+ this is not a trailer
+ this is not a trailer
+ Signed-off-by: a <a@example.com>
+ this is not a trailer
+ EOF
+ cat >expected <<-\EOF &&
+
+ this is not a trailer
+ this is not a trailer
+ Signed-off-by: a <a@example.com>
+ this is not a trailer
+ token: value
+ EOF
+ git interpret-trailers --trailer "token: value" patch >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'with non-trailer lines mixed with cherry picked from' '
+ cat >patch <<-\EOF &&
+
+ this is not a trailer
+ this is not a trailer
+ (cherry picked from commit x)
+ this is not a trailer
+ EOF
+ cat >expected <<-\EOF &&
+
+ this is not a trailer
+ this is not a trailer
+ (cherry picked from commit x)
+ this is not a trailer
+ token: value
+ EOF
+ git interpret-trailers --trailer "token: value" patch >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'with non-trailer lines mixed with a configured trailer' '
+ cat >patch <<-\EOF &&
+
+ this is not a trailer
+ this is not a trailer
+ My-trailer: x
+ this is not a trailer
+ EOF
+ cat >expected <<-\EOF &&
+
+ this is not a trailer
+ this is not a trailer
+ My-trailer: x
+ this is not a trailer
+ token: value
+ EOF
+ test_config trailer.my.key "My-trailer: " &&
+ git interpret-trailers --trailer "token: value" patch >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'with non-trailer lines mixed with a non-configured trailer' '
+ cat >patch <<-\EOF &&
+
+ this is not a trailer
+ this is not a trailer
+ I-am-not-configured: x
+ this is not a trailer
+ EOF
+ cat >expected <<-\EOF &&
+
+ this is not a trailer
+ this is not a trailer
+ I-am-not-configured: x
+ this is not a trailer
+
+ token: value
+ EOF
+ test_config trailer.my.key "My-trailer: " &&
+ git interpret-trailers --trailer "token: value" patch >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'with all non-configured trailers' '
+ cat >patch <<-\EOF &&
+
+ I-am-not-configured: x
+ I-am-also-not-configured: x
+ EOF
+ cat >expected <<-\EOF &&
+
+ I-am-not-configured: x
+ I-am-also-not-configured: x
+ token: value
+ EOF
+ test_config trailer.my.key "My-trailer: " &&
+ git interpret-trailers --trailer "token: value" patch >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'with non-trailer lines only' '
+ cat >patch <<-\EOF &&
+
+ this is not a trailer
+ EOF
+ cat >expected <<-\EOF &&
+
+ this is not a trailer
+
+ token: value
+ EOF
+ git interpret-trailers --trailer "token: value" patch >actual &&
+ test_cmp expected actual
+'
+
test_expect_success 'with config setup' '
git config trailer.ack.key "Acked-by: " &&
cat >expected <<-\EOF &&
diff --git a/trailer.c b/trailer.c
index aff858b..199f86a 100644
--- a/trailer.c
+++ b/trailer.c
@@ -27,6 +27,10 @@ static struct conf_info default_conf_info;
struct trailer_item {
struct list_head list;
+ /*
+ * If this is not a trailer line, the line is stored in value
+ * (excluding the terminating newline) and token is NULL.
+ */
char *token;
char *value;
};
@@ -44,6 +48,12 @@ static char *separators = ":";
#define TRAILER_ARG_STRING "$ARG"
+static const char *git_generated_prefixes[] = {
+ "Signed-off-by: ",
+ "(cherry picked from commit ",
+ NULL
+};
+
/* Iterate over the elements of the list. */
#define list_for_each_dir(pos, head, is_reverse) \
for (pos = is_reverse ? (head)->prev : (head)->next; \
@@ -70,9 +80,14 @@ static size_t token_len_without_separator(const char *token, size_t len)
static int same_token(struct trailer_item *a, struct arg_item *b)
{
- size_t a_len = token_len_without_separator(a->token, strlen(a->token));
- size_t b_len = token_len_without_separator(b->token, strlen(b->token));
- size_t min_len = (a_len > b_len) ? b_len : a_len;
+ size_t a_len, b_len, min_len;
+
+ if (!a->token)
+ return 0;
+
+ a_len = token_len_without_separator(a->token, strlen(a->token));
+ b_len = token_len_without_separator(b->token, strlen(b->token));
+ min_len = (a_len > b_len) ? b_len : a_len;
return !strncasecmp(a->token, b->token, min_len);
}
@@ -130,7 +145,14 @@ static char last_non_space_char(const char *s)
static void print_tok_val(FILE *outfile, const char *tok, const char *val)
{
- char c = last_non_space_char(tok);
+ char c;
+
+ if (!tok) {
+ fprintf(outfile, "%s\n", val);
+ return;
+ }
+
+ c = last_non_space_char(tok);
if (!c)
return;
if (strchr(separators, c))
@@ -709,6 +731,7 @@ static int find_patch_start(struct strbuf **lines, int count)
static int find_trailer_start(struct strbuf **lines, int count)
{
int start, end_of_title, only_spaces = 1;
+ int recognized_prefix = 0, trailer_lines = 0, non_trailer_lines = 0;
/* The first paragraph is the title and cannot be trailers */
for (start = 0; start < count; start++) {
@@ -720,26 +743,60 @@ static int find_trailer_start(struct strbuf **lines, int count)
end_of_title = start;
/*
- * Get the start of the trailers by looking starting from the end
- * for a line with only spaces before lines with one separator.
+ * Get the start of the trailers by looking starting from the end for a
+ * blank line before a set of non-blank lines that (i) are all
+ * trailers, or (ii) contains at least one Git-generated trailer and
+ * consists of at least 25% trailers.
*/
for (start = count - 1; start >= end_of_title; start--) {
+ const char **p;
+ int separator_pos;
+
if (lines[start]->buf[0] == comment_line_char)
continue;
if (contains_only_spaces(lines[start]->buf)) {
if (only_spaces)
continue;
- return start + 1;
+ if (recognized_prefix &&
+ trailer_lines * 3 >= non_trailer_lines)
+ return start + 1;
+ if (trailer_lines && !non_trailer_lines)
+ return start + 1;
+ return count;
}
- if (strcspn(lines[start]->buf, separators) < lines[start]->len) {
- if (only_spaces)
- only_spaces = 0;
- continue;
+ only_spaces = 0;
+
+ for (p = git_generated_prefixes; *p; p++) {
+ if (starts_with(lines[start]->buf, *p)) {
+ trailer_lines++;
+ recognized_prefix = 1;
+ goto continue_outer_loop;
+ }
}
- return count;
+
+ separator_pos = find_separator(lines[start]->buf);
+ if (separator_pos >= 1) {
+ struct list_head *pos;
+
+ trailer_lines++;
+ if (recognized_prefix)
+ continue;
+ list_for_each(pos, &conf_head) {
+ struct arg_item *item;
+ item = list_entry(pos, struct arg_item, list);
+ if (token_matches_item(lines[start]->buf, item,
+ separator_pos)) {
+ recognized_prefix = 1;
+ break;
+ }
+ }
+ } else
+ non_trailer_lines++;
+continue_outer_loop:
+ ;
}
- return only_spaces ? count : 0;
+ return count;
}
/* Get the index of the end of the trailers */
@@ -810,6 +867,12 @@ static int process_input_file(FILE *outfile,
add_trailer_item(head,
strbuf_detach(&tok, NULL),
strbuf_detach(&val, NULL));
+ } else {
+ strbuf_addbuf(&val, lines[i]);
+ strbuf_strip_suffix(&val, "\n");
+ add_trailer_item(head,
+ NULL,
+ strbuf_detach(&val, NULL));
}
}
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v5 8/8] trailer: support values folded to multiple lines
From: Jonathan Tan @ 2016-10-21 17:55 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, sbeller, gitster
In-Reply-To: <cover.1477072247.git.jonathantanmy@google.com>
Currently, interpret-trailers requires that a trailer be only on 1 line.
For example:
a: first line
second line
would be interpreted as one trailer line followed by one non-trailer line.
Make interpret-trailers support RFC 822-style folding, treating those
lines as one logical trailer.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
Documentation/git-interpret-trailers.txt | 7 +-
t/t7513-interpret-trailers.sh | 169 +++++++++++++++++++++++++++++++
trailer.c | 45 ++++++--
3 files changed, 211 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
index 4966b5b..e99bda6 100644
--- a/Documentation/git-interpret-trailers.txt
+++ b/Documentation/git-interpret-trailers.txt
@@ -57,11 +57,12 @@ minus signs start the patch part of the message.
When reading trailers, there can be whitespaces after the
token, the separator and the value. There can also be whitespaces
-inside the token and the value.
+inside the token and the value. The value may be split over multiple lines with
+each subsequent line starting with whitespace, like the "folding" in RFC 822.
Note that 'trailers' do not follow and are not intended to follow many
-rules for RFC 822 headers. For example they do not follow the line
-folding rules, the encoding rules and probably many other rules.
+rules for RFC 822 headers. For example they do not follow
+the encoding rules and probably many other rules.
OPTIONS
-------
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 3d94b3a..4dd1d7c 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -256,6 +256,175 @@ test_expect_success 'line with leading whitespace is not trailer' '
test_cmp expected actual
'
+test_expect_success 'multiline field treated as one trailer for 25% check' '
+ q_to_tab >patch <<-\EOF &&
+
+ Signed-off-by: a <a@example.com>
+ name: value on
+ Qmultiple lines
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ Signed-off-by: a <a@example.com>
+ name: value on
+ Qmultiple lines
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ name: value
+ EOF
+ git interpret-trailers --trailer "name: value" patch >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'multiline field treated as atomic for placement' '
+ q_to_tab >patch <<-\EOF &&
+
+ another: trailer
+ name: value on
+ Qmultiple lines
+ another: trailer
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ another: trailer
+ name: value on
+ Qmultiple lines
+ name: value
+ another: trailer
+ EOF
+ test_config trailer.name.where after &&
+ git interpret-trailers --trailer "name: value" patch >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'multiline field treated as atomic for replacement' '
+ q_to_tab >patch <<-\EOF &&
+
+ another: trailer
+ name: value on
+ Qmultiple lines
+ another: trailer
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ another: trailer
+ another: trailer
+ name: value
+ EOF
+ test_config trailer.name.ifexists replace &&
+ git interpret-trailers --trailer "name: value" patch >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'multiline field treated as atomic for difference check' '
+ q_to_tab >patch <<-\EOF &&
+
+ another: trailer
+ name: first line
+ Qsecond line
+ another: trailer
+ EOF
+ test_config trailer.name.ifexists addIfDifferent &&
+
+ q_to_tab >trailer <<-\EOF &&
+ name: first line
+ Qsecond line
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ another: trailer
+ name: first line
+ Qsecond line
+ another: trailer
+ EOF
+ git interpret-trailers --trailer "$(cat trailer)" patch >actual &&
+ test_cmp expected actual &&
+
+ q_to_tab >trailer <<-\EOF &&
+ name: first line
+ QQQQQsecond line
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ another: trailer
+ name: first line
+ Qsecond line
+ another: trailer
+ name: first line
+ QQQQQsecond line
+ EOF
+ git interpret-trailers --trailer "$(cat trailer)" patch >actual &&
+ test_cmp expected actual &&
+
+ q_to_tab >trailer <<-\EOF &&
+ name: first line *DIFFERENT*
+ Qsecond line
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ another: trailer
+ name: first line
+ Qsecond line
+ another: trailer
+ name: first line *DIFFERENT*
+ Qsecond line
+ EOF
+ git interpret-trailers --trailer "$(cat trailer)" patch >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'multiline field treated as atomic for neighbor check' '
+ q_to_tab >patch <<-\EOF &&
+
+ another: trailer
+ name: first line
+ Qsecond line
+ another: trailer
+ EOF
+ test_config trailer.name.where after &&
+ test_config trailer.name.ifexists addIfDifferentNeighbor &&
+
+ q_to_tab >trailer <<-\EOF &&
+ name: first line
+ Qsecond line
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ another: trailer
+ name: first line
+ Qsecond line
+ another: trailer
+ EOF
+ git interpret-trailers --trailer "$(cat trailer)" patch >actual &&
+ test_cmp expected actual &&
+
+ q_to_tab >trailer <<-\EOF &&
+ name: first line
+ QQQQQsecond line
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ another: trailer
+ name: first line
+ Qsecond line
+ name: first line
+ QQQQQsecond line
+ another: trailer
+ EOF
+ git interpret-trailers --trailer "$(cat trailer)" patch >actual &&
+ test_cmp expected actual
+'
+
test_expect_success 'with config setup' '
git config trailer.ack.key "Acked-by: " &&
cat >expected <<-\EOF &&
diff --git a/trailer.c b/trailer.c
index d978437..65866d0 100644
--- a/trailer.c
+++ b/trailer.c
@@ -619,12 +619,14 @@ static void parse_trailer(struct strbuf *tok, struct strbuf *val,
}
}
-static void add_trailer_item(struct list_head *head, char *tok, char *val)
+static struct trailer_item *add_trailer_item(struct list_head *head, char *tok,
+ char *val)
{
struct trailer_item *new = xcalloc(sizeof(*new), 1);
new->token = tok;
new->value = val;
list_add_tail(&new->list, head);
+ return new;
}
static void add_arg_item(struct list_head *arg_head, char *tok, char *val,
@@ -732,6 +734,14 @@ static int find_trailer_start(struct strbuf **lines, int count)
{
int start, end_of_title, only_spaces = 1;
int recognized_prefix = 0, trailer_lines = 0, non_trailer_lines = 0;
+ /*
+ * Number of possible continuation lines encountered. This will be
+ * reset to 0 if we encounter a trailer (since those lines are to be
+ * considered continuations of that trailer), and added to
+ * non_trailer_lines if we encounter a non-trailer (since those lines
+ * are to be considered non-trailers).
+ */
+ int possible_continuation_lines = 0;
/* The first paragraph is the title and cannot be trailers */
for (start = 0; start < count; start++) {
@@ -752,11 +762,15 @@ static int find_trailer_start(struct strbuf **lines, int count)
const char **p;
int separator_pos;
- if (lines[start]->buf[0] == comment_line_char)
+ if (lines[start]->buf[0] == comment_line_char) {
+ non_trailer_lines += possible_continuation_lines;
+ possible_continuation_lines = 0;
continue;
+ }
if (contains_only_spaces(lines[start]->buf)) {
if (only_spaces)
continue;
+ non_trailer_lines += possible_continuation_lines;
if (recognized_prefix &&
trailer_lines * 3 >= non_trailer_lines)
return start + 1;
@@ -769,16 +783,18 @@ static int find_trailer_start(struct strbuf **lines, int count)
for (p = git_generated_prefixes; *p; p++) {
if (starts_with(lines[start]->buf, *p)) {
trailer_lines++;
+ possible_continuation_lines = 0;
recognized_prefix = 1;
goto continue_outer_loop;
}
}
- separator_pos = find_separator(lines[start]->buf);
+ separator_pos = find_separator(lines[start]->buf, separators);
if (separator_pos >= 1 && !isspace(lines[start]->buf[0])) {
struct list_head *pos;
trailer_lines++;
+ possible_continuation_lines = 0;
if (recognized_prefix)
continue;
list_for_each(pos, &conf_head) {
@@ -790,8 +806,13 @@ static int find_trailer_start(struct strbuf **lines, int count)
break;
}
}
- } else
+ } else if (isspace(lines[start]->buf[0]))
+ possible_continuation_lines++;
+ else {
non_trailer_lines++;
+ non_trailer_lines += possible_continuation_lines;
+ possible_continuation_lines = 0;
+ }
continue_outer_loop:
;
}
@@ -840,6 +861,7 @@ static int process_input_file(FILE *outfile,
int patch_start, trailer_start, trailer_end, i;
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
+ struct trailer_item *last = NULL;
/* Get the line count */
while (lines[count])
@@ -860,19 +882,28 @@ static int process_input_file(FILE *outfile,
int separator_pos;
if (lines[i]->buf[0] == comment_line_char)
continue;
+ if (last && isspace(lines[i]->buf[0])) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_addf(&sb, "%s\n%s", last->value, lines[i]->buf);
+ strbuf_strip_suffix(&sb, "\n");
+ free(last->value);
+ last->value = strbuf_detach(&sb, NULL);
+ continue;
+ }
separator_pos = find_separator(lines[i]->buf, separators);
if (separator_pos >= 1) {
parse_trailer(&tok, &val, NULL, lines[i]->buf,
separator_pos);
- add_trailer_item(head,
- strbuf_detach(&tok, NULL),
- strbuf_detach(&val, NULL));
+ last = add_trailer_item(head,
+ strbuf_detach(&tok, NULL),
+ strbuf_detach(&val, NULL));
} else {
strbuf_addbuf(&val, lines[i]);
strbuf_strip_suffix(&val, "\n");
add_trailer_item(head,
NULL,
strbuf_detach(&val, NULL));
+ last = NULL;
}
}
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox