* Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
From: Johannes Schindelin @ 2016-09-20 21:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqd1jzcvs5.fsf@gitster.mtv.corp.google.com>
Hi,
On Mon, 19 Sep 2016, Junio C Hamano wrote:
> * jk/rebase-i-drop-ident-check (2016-07-29) 1 commit
> (merged to 'next' on 2016-08-14 at 6891bcd)
> + rebase-interactive: drop early check for valid ident
>
> Even when "git pull --rebase=preserve" (and the underlying "git
> rebase --preserve") can complete without creating any new commit
> (i.e. fast-forwards), it still insisted on having a usable ident
> information (read: user.email is set correctly), which was less
> than nice. As the underlying commands used inside "git rebase"
> would fail with a more meaningful error message and advice text
> when the bogus ident matters, this extra check was removed.
>
> Will hold to see if people scream.
> cf. <20160729224944.GA23242@sigill.intra.peff.net>
Let's do this.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v2] ls-files: add pathspec matching for submodules
From: Brandon Williams @ 2016-09-20 21:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <CAKoko1oU+QR61Vy0eSxaRe_w8u4q_bC9gx9H7oMqH=CwNzBVCA@mail.gmail.com>
>>> +
>>> + if (item->flags & PATHSPEC_ONESTAR) {
>>> + return WM_MATCH;
>>> + } else if (item->magic & PATHSPEC_GLOB) {
>>> + return wildmatch(pattern, string,
>>> + WM_PATHNAME |
>>> + (item->magic & PATHSPEC_ICASE ?
>>> + WM_CASEFOLD : 0),
>>> + NULL);
>>
>> Isn't this last one overly tight? I am wondering about a scenario
>> where you have a submodule at "sub/" in the superproject, and "sub/"
>> has a "file" at the top of its working tree. And you do:
>>
>> git ls-files --recurse-submodules ':(glob)??b/fi?e'
>>
>> at the top of the superproject. The "pattern" would be '??b/fi?e"
>> while string would be 'sub', and wildmatch() would not like it, but
>> there is no way for this caller to append anything to 'sub' before
>> making this call, as it hasn't looked into what paths appear in the
>> submodule repository (and it should not want to). And I think we
>> would want it to recurse to find sub/file. IOW, this looks like a
>> false negative we must avoid in this function. As we cannot afford
>> to check if anything that matches 'fi?e' is in the index file of the
>> submodule repository, we shouldn't try to match 'fi?e' portion of
>> the given pathspec pattern.
>
> good point. Let me think about this some more.
On a similar but slightly different note. In general do we want the
pathspec '??b' to
match against the sib/ directory and subsequently have ls-files print
all entries inside
of the sib/ directory? (this is in the non-recursive case)
^ permalink raw reply
* Re: [PATCH] gitweb: use highlight's shebang detection
From: Jakub Narębski @ 2016-09-20 20:22 UTC (permalink / raw)
To: Ian Kelling, git
In-Reply-To: <20160906190037.12442-1-ian@iankelling.org>
W dniu 06.09.2016 o 21:00, Ian Kelling pisze:
> The highlight binary can detect language by shebang when we can't tell
> the syntax type by the name of the file.
Was it something always present among highlight[1] binary capabilities,
or is it something present only in new enough highlight app? Or only
in some specific fork / specific binary? I couldn't find language
detection in highlight[1] documentation...
[1]: http://www.andre-simon.de/doku/highlight/en/highlight.php
If this feature is available only for some version, or for some
highlighters, gitweb would have to provide an option to configure
it. It might be an additional configuration variable, it might
be a special value in the %highlight_basename or %highlight_ext.
> To use highlight's shebang
> detection, add highlight to the pipeline whenever highlight is enabled.
This describes what this patch does, but the sentence feels
a bit convoluted, as it is stated.
>
> Document the shebang detection and add a test which exercises it in
> t/t9500-gitweb-standalone-no-errors.sh.
Nice!
>
> Signed-off-by: Ian Kelling <ian@iankelling.org>
> ---
>
> Notes:
> I wondered if adding highlight to the pipeline would make viewing a blob
> with no highlighting take longer but it did not on my computer. I found
> no noticeable impact on small files and strangely, on a 159k file, it
> took 7% less time averaged over several requests.
Strange. I would guess that invoking separate binary and perl would always
add to the time (especially on operation systems where forking / running
command is expensive... though those are not often used with web servers,
isn't it).
>
> Documentation/gitweb.conf.txt | 21 ++++++++++++++-------
> gitweb/gitweb.perl | 10 +++++-----
> t/t9500-gitweb-standalone-no-errors.sh | 18 +++++++++++++-----
> 3 files changed, 32 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
> index a79e350..e632089 100644
> --- a/Documentation/gitweb.conf.txt
> +++ b/Documentation/gitweb.conf.txt
> @@ -246,13 +246,20 @@ $highlight_bin::
> Note that 'highlight' feature must be set for gitweb to actually
> use syntax highlighting.
> +
> -*NOTE*: if you want to add support for new file type (supported by
> -"highlight" but not used by gitweb), you need to modify `%highlight_ext`
> -or `%highlight_basename`, depending on whether you detect type of file
> -based on extension (for example "sh") or on its basename (for example
> -"Makefile"). The keys of these hashes are extension and basename,
> -respectively, and value for given key is name of syntax to be passed via
> -`--syntax <syntax>` to highlighter.
> +*NOTE*: for a file to be highlighted, its syntax type must be detected
> +and that syntax must be supported by "highlight". The default syntax
> +detection is minimal, and there are many supported syntax types with no
> +detection by default. There are three options for adding syntax
> +detection. The first and second priority are `%highlight_basename` and
> +`%highlight_ext`, which detect based on basename (the full filename, for
> +example "Makefile") and extension (for example "sh"). The keys of these
> +hashes are the basename and extension, respectively, and the value for a
> +given key is the name of the syntax to be passed via `--syntax <syntax>`
> +to "highlight". The last priority is the "highlight" configuration of
> +`Shebang` regular expressions to detect the language based on the first
> +line in the file, (for example, matching the line "#!/bin/bash"). See
> +the highlight documentation and the default config at
> +/etc/highlight/filetypes.conf for more details.
All right; in addition to expanding the docs, it also improves them.
> +
> For example if repositories you are hosting use "phtml" extension for
> PHP files, and you want to have correct syntax-highlighting for those
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 33d701d..a672181 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3931,15 +3931,16 @@ sub guess_file_syntax {
> # or return original FD if no highlighting
> sub run_highlighter {
> my ($fd, $highlight, $syntax) = @_;
> - return $fd unless ($highlight && defined $syntax);
> + return $fd unless ($highlight);
Here we would have check if we want / can invoke "highlight".
>
> close $fd;
> + my $syntax_arg = (defined $syntax) ? "--syntax $syntax" : "--force";
> open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
> quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',
> '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',
> '--', "-fe=$fallback_encoding")." | ".
> quote_command($highlight_bin).
> - " --replace-tabs=8 --fragment --syntax $syntax |"
> + " --replace-tabs=8 --fragment $syntax_arg |"
> or die_error(500, "Couldn't open file or run syntax highlighter");
> return $fd;
> }
All right (well, except for the question asked at the beginning).
> @@ -7063,8 +7064,7 @@ sub git_blob {
>
> my $highlight = gitweb_check_feature('highlight');
> my $syntax = guess_file_syntax($highlight, $mimetype, $file_name);
> - $fd = run_highlighter($fd, $highlight, $syntax)
> - if $syntax;
Hmmm... it looks like the old code checked if there was $syntax defined
twice: once for truthy value in caller, once for definedness in run_highlighter().
> + $fd = run_highlighter($fd, $highlight, $syntax);
All right.
>
> git_header_html(undef, $expires);
> my $formats_nav = '';
> @@ -7117,7 +7117,7 @@ sub git_blob {
> $line = untabify($line);
> printf qq!<div class="pre"><a id="l%i" href="%s#l%i" class="linenr">%4i</a> %s</div>\n!,
> $nr, esc_attr(href(-replay => 1)), $nr, $nr,
> - $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);
> + $highlight ? sanitize($line) : esc_html($line, -nbsp=>1);
Oh, well. It looks like checking if highlighter could be run in
run_highlight() is wrong, as the caller (that is, git_blob()) needs
to know if it is using "highlight" output (which is HTML) or raw blob
contents (which needs to be HTML-escaped).
> }
> }
> close $fd
> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
> index e94b2f1..9e5fcfe 100755
> --- a/t/t9500-gitweb-standalone-no-errors.sh
> +++ b/t/t9500-gitweb-standalone-no-errors.sh
> @@ -702,12 +702,20 @@ test_expect_success HIGHLIGHT \
> gitweb_run "p=.git;a=blob;f=file"'
>
> test_expect_success HIGHLIGHT \
> - 'syntax highlighting (highlighted, shell script)' \
> + 'syntax highlighting (highlighted, shell script shebang)' \
It would be nice to have in test name that it checks if highlighter
autodetection works, or at least doesn't crash gitweb.
> 'git config gitweb.highlight yes &&
> - echo "#!/usr/bin/sh" > test.sh &&
> - git add test.sh &&
> - git commit -m "Add test.sh" &&
> - gitweb_run "p=.git;a=blob;f=test.sh"'
> + echo "#!/usr/bin/sh" > test &&
> + git add test &&
> + git commit -m "Add test" &&
> + gitweb_run "p=.git;a=blob;f=test"'
> +
> +test_expect_success HIGHLIGHT \
> + 'syntax highlighting (highlighted, header file)' \
Do we check explicit syntax knowledge (based on the extension),
or autodetect again?
> + 'git config gitweb.highlight yes &&
> + echo "#define ANSWER 42" > test.h &&
> + git add test.h &&
> + git commit -m "Add test.h" &&
> + gitweb_run "p=.git;a=blob;f=test.h"'
>
> # ----------------------------------------------------------------------
> # forks of projects
>
Thank you for your work on this patch,
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH tg/add-chmod+x-fix 2/2] t3700-add: protect one --chmod=+x test with POSIXPERM
From: Thomas Gummerer @ 2016-09-20 19:34 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git Mailing List
In-Reply-To: <5effaf36-d017-3ff1-1de1-d9b303c04f23@kdbg.org>
Hi Johannes,
On 09/20, Johannes Sixt wrote:
> A recently introduced test checks the result of 'git status' after
> setting the executable bit on a file. This check does not yield the
> expected result when the filesystem does not support the executable bit
> (and core.filemode is false). Skip the test case.
Thanks for cleaning up my mess. The patch looks correct to me.
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
> I am surprised that add --chmod=+x changes only the index, but not
> the file on disk!?!
I *think* --chmod is mainly thought of as a convenience for git users
on a filesystem that doesn't have an executable flag. So it was
introduced this way as the permissions on the file system don't matter
in that case. A change of that behaviour may make sense for this
though.
> t/t3700-add.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t3700-add.sh b/t/t3700-add.sh
> index 16ab2da..13e0dd2 100755
> --- a/t/t3700-add.sh
> +++ b/t/t3700-add.sh
> @@ -361,7 +361,7 @@ test_expect_success 'git add --chmod=[+-]x changes index with already added file
> test_mode_in_index 100644 xfoo3
> '
>
> -test_expect_success 'file status is changed after git add --chmod=+x' '
> +test_expect_success POSIXPERM 'file status is changed after git add --chmod=+x' '
> echo "AM foo4" >expected &&
> echo foo >foo4 &&
> git add foo4 &&
> --
> 2.10.0.85.gea34e30
>
--
Thomas
^ permalink raw reply
* Re: v2.9.3 and v2.10.0: `name-ref' HEAD gives wrong branch name
From: Steffen Nurpmeso @ 2016-09-20 19:29 UTC (permalink / raw)
To: Bryan Turner; +Cc: Git Users
In-Reply-To: <CAGyf7-FR11Ludt3vNZX7Ek4n_JLSON5+Y9By83GvNgpg94TWcg@mail.gmail.com>
Hey.
Bryan Turner <bturner@atlassian.com> wrote:
|On Tue, Sep 20, 2016 at 9:23 AM, Steffen Nurpmeso <steffen@sdaoden.eu> \
|wrote:
|> yah, sorry, i'm back again..
|> I try to find a way to find the name of the current branch in an
|> automated way, because i need to ensure that a commit happens on
|> it and no other branch. Now the problem arises that the commit
|> ref at the time of that commit maybe shared in between several
|> different branches, but no more thereafter, of course:
|>
|> ?0[steffen@wales ]$ git branch|grep '^*'
|> * stable/v14.9
|> ?0[steffen@wales ]$ git name-rev --name-only HEAD
|> stable/v14.8
|>
|> Is there another way except looking into .git/HEAD or using sed(1)
|> on the output of `branch' to find the right name?
|
|Have you tried "git symbolic-ref HEAD"?
Not until now.
|$ git symbolic-ref HEAD
|refs/heads/master
Works.
|If you don't want the fully-qualified ref, you can add --short:
|
|$ git symbolic-ref --short HEAD
|master
Yep, works even better. Fantastic. Thank you. And that command
was already existing when i have learned to use git(1), it is even
in the progit-09.markdown as of 2011-09-22 that i have laying
around. Five years, and anything forgotten. That is what you get
from working with blinkers on. Shame!
Thank you Bryan.
--steffen
^ permalink raw reply
* Re: [PATCH tg/add-chmod+x-fix 1/2] t3700-add: create subdirectory gently
From: Thomas Gummerer @ 2016-09-20 19:28 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git Mailing List
In-Reply-To: <cee3c784-8f03-c524-2f67-d35cb3755a41@kdbg.org>
Hi Johannes,
On 09/20, Johannes Sixt wrote:
> The subdirectory 'sub' is created early in the test file. Later, a test
> case removes it during its clean-up actions. However, this test case is
> protected by POSIXPERM. Consequently, 'sub' remains when the POSIXPERM
> prerequisite is not satisfied. Later, a recently introduced test case
> creates 'sub' again. Use -p with mkdir so that it does not fail if 'sub'
> already exists.
Thanks for catching and fixing this. Having a look at the tests shows
that this happens through a git reset --hard at the start of the tests
that require POSIXPERM, which makes me wonder whether we should do
such a cleanup somewhere unconditionally, or move the tests that do
require POSIXPERM to the end of the test file to possibly prevent
similar breakages in the future?
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
> t/t3700-add.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t3700-add.sh b/t/t3700-add.sh
> index 0a962a6..16ab2da 100755
> --- a/t/t3700-add.sh
> +++ b/t/t3700-add.sh
> @@ -380,7 +380,7 @@ test_expect_success 'no file status change if no pathspec is given' '
> '
>
> test_expect_success 'no file status change if no pathspec is given in subdir' '
> - mkdir sub &&
> + mkdir -p sub &&
> (
> cd sub &&
> >sub-foo1 &&
> --
> 2.10.0.85.gea34e30
>
--
Thomas
^ permalink raw reply
* Re: Re: Homebrew and Git
From: John Keeping @ 2016-09-20 19:15 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Jonas Thiel, git
In-Reply-To: <20160920110700.GB64315@book.hvoigt.net>
On Tue, Sep 20, 2016 at 01:07:00PM +0200, Heiko Voigt wrote:
> On Tue, Sep 20, 2016 at 01:02:28PM +0200, Heiko Voigt wrote:
> > Hi,
> >
> > On Sun, Sep 18, 2016 at 05:50:28PM +0200, Jonas Thiel wrote:
> > > A while ago I have described my problem with Homebrew at the following
> > > GitHub channel
> > > (https://github.com/Homebrew/homebrew-core/issues/2970). In the
> > > meanwhile, I believe that I my problem with Homebrew is based on an
> > > issues with my Git. I have found the attached Git Crash reports on my
> > > Mac and because I am not familiar with reading/analysing Crash
> > > Reports, it would be great if someone could give me some feedback on
> > > it.
> > >
> > > If you have any question, please do not hesitate to contact me.
> >
> > From your crash reports I see that git is apparently crashing in a
> > strchr() call from within ident_default_email() which is a function that
> > tries to assemble a name and email to put into your commits.
>
> BTW, here is the callstack inlined from the crashreport:
>
> bsystem_platform.dylib 0x00007fff840db41c _platform_strchr$VARIANT$Haswell + 28
> 1 git 0x000000010ba1d3f4 ident_default_email + 801
> 2 git 0x000000010ba1d68f fmt_ident + 66
> 3 git 0x000000010ba4b495 files_log_ref_write + 175
> 4 git 0x000000010ba4b0a6 commit_ref_update + 106
> 5 git 0x000000010ba4c3a8 ref_transaction_commit + 468
> 6 git 0x000000010b994dd8 s_update_ref + 271
> 7 git 0x000000010b994556 fetch_refs + 1969
> 8 git 0x000000010b9935f2 fetch_one + 1913
> 9 git 0x000000010b992bc4 cmd_fetch + 549
> 10 git 0x000000010b9666c4 handle_builtin + 478
> 11 git 0x000000010b96602f main + 376
> 12 libdyld.dylib 0x00007fff834ef5ad start + 1
>
> Maybe someone else has an idea what might be causing this...
The only strchr I can see that could be called here is in
canonical_name(), where it's called with addrinfo::ai_canonname.
Searching for OS X and ai_canonname, leads me straight back to this
list, although 7 years ago! I think ident.c needs a fix similar to
commit 3e8a00a (daemon.c: fix segfault on OS X, 2009-04-27); from the
commit message there:
On OS X (and maybe other unices), getaddrinfo(3) returns NULL
in the ai_canonname field if it's called with an IP address for
the hostname.
^ permalink raw reply
* Re: [PATCH 1/6] i18n: commit: mark message for translation
From: Jean-Noël AVILA @ 2016-09-20 19:04 UTC (permalink / raw)
To: Junio C Hamano
Cc: Vasco Almeida, git, Jiang Xin,
Ævar Arnfjörð Bjarmason
In-Reply-To: <xmqq4m5begxj.fsf@gitster.mtv.corp.google.com>
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
---
Instead of distillating change requests, I'd better do it by myself. Here is the reworked version of the patch.
diff.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/diff.c b/diff.c
index c6da383..494f723 100644
--- a/diff.c
+++ b/diff.c
@@ -55,6 +55,11 @@ static char diff_colors[][COLOR_MAXLEN] = {
GIT_COLOR_NORMAL, /* FUNCINFO */
};
+static NORETURN void die_want_option(const char *option_name)
+{
+ die(_("option '%s' requires a value"), option_name);
+}
+
static int parse_diff_color_slot(const char *var)
{
if (!strcasecmp(var, "context") || !strcasecmp(var, "plain"))
@@ -3325,7 +3330,7 @@ void diff_setup_done(struct diff_options *options)
if (options->output_format & DIFF_FORMAT_NO_OUTPUT)
count++;
if (count > 1)
- die("--name-only, --name-status, --check and -s are mutually exclusive");
+ die(_("--name-only, --name-status, --check and -s are mutually exclusive"));
/*
* Most of the time we can say "there are changes"
@@ -3521,7 +3526,7 @@ static int stat_opt(struct diff_options *options, const char **av)
if (*arg == '=')
width = strtoul(arg + 1, &end, 10);
else if (!*arg && !av[1])
- die("Option '--stat-width' requires a value");
+ die_want_option("--stat-width");
else if (!*arg) {
width = strtoul(av[1], &end, 10);
argcount = 2;
@@ -3530,7 +3535,7 @@ static int stat_opt(struct diff_options *options, const char **av)
if (*arg == '=')
name_width = strtoul(arg + 1, &end, 10);
else if (!*arg && !av[1])
- die("Option '--stat-name-width' requires a value");
+ die_want_option("--stat-name-width");
else if (!*arg) {
name_width = strtoul(av[1], &end, 10);
argcount = 2;
@@ -3539,7 +3544,7 @@ static int stat_opt(struct diff_options *options, const char **av)
if (*arg == '=')
graph_width = strtoul(arg + 1, &end, 10);
else if (!*arg && !av[1])
- die("Option '--stat-graph-width' requires a value");
+ die_want_option("--stat-graph-width");
else if (!*arg) {
graph_width = strtoul(av[1], &end, 10);
argcount = 2;
@@ -3548,7 +3553,7 @@ static int stat_opt(struct diff_options *options, const char **av)
if (*arg == '=')
count = strtoul(arg + 1, &end, 10);
else if (!*arg && !av[1])
- die("Option '--stat-count' requires a value");
+ die_want_option("--stat-count");
else if (!*arg) {
count = strtoul(av[1], &end, 10);
argcount = 2;
--
2.10.0
^ permalink raw reply related
* [PATCH v8 11/11] convert: add filter.<driver>.process option
From: larsxschneider @ 2016-09-20 19:02 UTC (permalink / raw)
To: git
Cc: peff, gitster, sbeller, jnareb, mlbright, tboegi, ramsay,
Lars Schneider
In-Reply-To: <20160920190247.82189-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
Git's clean/smudge mechanism invokes an external filter process for
every single blob that is affected by a filter. If Git filters a lot of
blobs then the startup time of the external filter processes can become
a significant part of the overall Git execution time.
In a preliminary performance test this developer used a clean/smudge
filter written in golang to filter 12,000 files. This process took 364s
with the existing filter mechanism and 5s with the new mechanism. See
details here: https://github.com/github/git-lfs/pull/1382
This patch adds the `filter.<driver>.process` string option which, if
used, keeps the external filter process running and processes all blobs
with the packet format (pkt-line) based protocol over standard input and
standard output. The full protocol is explained in detail in
`Documentation/gitattributes.txt`.
A few key decisions:
* The long running filter process is referred to as filter protocol
version 2 because the existing single shot filter invocation is
considered version 1.
* Git sends a welcome message and expects a response right after the
external filter process has started. This ensures that Git will not
hang if a version 1 filter is incorrectly used with the
filter.<driver>.process option for version 2 filters. In addition,
Git can detect this kind of error and warn the user.
* The status of a filter operation (e.g. "success" or "error) is set
before the actual response and (if necessary!) re-set after the
response. The advantage of this two step status response is that if
the filter detects an error early, then the filter can communicate
this and Git does not even need to create structures to read the
response.
* All status responses are pkt-line lists terminated with a flush
packet. This allows us to send other status fields with the same
protocol in the future.
Helped-by: Martin-Louis Bright <mlbright@gmail.com>
Reviewed-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
Documentation/gitattributes.txt | 156 +++++++++++++-
contrib/long-running-filter/example.pl | 123 +++++++++++
convert.c | 348 ++++++++++++++++++++++++++++---
pkt-line.h | 1 +
t/t0021-conversion.sh | 365 ++++++++++++++++++++++++++++++++-
t/t0021/rot13-filter.pl | 191 +++++++++++++++++
6 files changed, 1153 insertions(+), 31 deletions(-)
create mode 100755 contrib/long-running-filter/example.pl
create mode 100755 t/t0021/rot13-filter.pl
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 7aff940..946dcad 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -293,7 +293,13 @@ checkout, when the `smudge` command is specified, the command is
fed the blob object from its standard input, and its standard
output is used to update the worktree file. Similarly, the
`clean` command is used to convert the contents of worktree file
-upon checkin.
+upon checkin. By default these commands process only a single
+blob and terminate. If a long running `process` filter is used
+in place of `clean` and/or `smudge` filters, then Git can process
+all blobs with a single filter command invocation for the entire
+life of a single Git command, for example `git add --all`. See
+section below for the description of the protocol used to
+communicate with a `process` filter.
One use of the content filtering is to massage the content into a shape
that is more convenient for the platform, filesystem, and the user to use.
@@ -373,6 +379,154 @@ not exist, or may have different contents. So, smudge and clean commands
should not try to access the file on disk, but only act as filters on the
content provided to them on standard input.
+Long Running Filter Process
+^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+If the filter command (a string value) is defined via
+`filter.<driver>.process` then Git can process all blobs with a
+single filter invocation for the entire life of a single Git
+command. This is achieved by using a packet format (pkt-line,
+see technical/protocol-common.txt) based protocol over standard
+input and standard output as follows. All packets are considered
+text and therefore are terminated by an LF. Exceptions are the
+"*CONTENT" packets and the flush packet.
+
+Git starts the filter when it encounters the first file
+that needs to be cleaned or smudged. After the filter started
+Git sends a welcome message ("git-filter-client"), a list of
+supported protocol version numbers, and a flush packet. Git expects
+to read a welcome response message ("git-filter-server") and exactly
+one protocol version number from the previously sent list. All further
+communication will be based on the selected version. The remaining
+protocol description below documents "version=2". Please note that
+"version=42" in the example below does not exist and is only there
+to illustrate how the protocol would look like with more than one
+version.
+
+After the version negotiation Git sends a list of supported capabilities
+and a flush packet. Git expects to read a list of desired capabilities,
+which must be a subset of the supported capabilities list, and a flush
+packet as response:
+------------------------
+packet: git> git-filter-client
+packet: git> version=2
+packet: git> version=42
+packet: git> 0000
+packet: git< git-filter-server
+packet: git< version=2
+packet: git> clean=true
+packet: git> smudge=true
+packet: git> not-yet-invented=true
+packet: git> 0000
+packet: git< clean=true
+packet: git< smudge=true
+packet: git< 0000
+------------------------
+Supported filter capabilities in version 2 are "clean" and
+"smudge".
+
+Afterwards Git sends a list of "key=value" pairs terminated with
+a flush packet. The list will contain at least the filter command
+(based on the supported capabilities) and the pathname of the file
+to filter relative to the repository root. Right after these packets
+Git sends the content split in zero or more pkt-line packets and a
+flush packet to terminate content.
+------------------------
+packet: git> command=smudge
+packet: git> pathname=path/testfile.dat
+packet: git> 0000
+packet: git> CONTENT
+packet: git> 0000
+------------------------
+
+The filter is expected to respond with a list of "key=value" pairs
+terminated with a flush packet. If the filter does not experience
+problems then the list must contain a "success" status. Right after
+these packets the filter is expected to send the content in zero
+or more pkt-line packets and a flush packet at the end. Finally, a
+second list of "key=value" pairs terminated with a flush packet
+is expected. The filter can change the status in the second list.
+------------------------
+packet: git< status=success
+packet: git< 0000
+packet: git< SMUDGED_CONTENT
+packet: git< 0000
+packet: git< 0000 # empty list!
+------------------------
+
+If the result content is empty then the filter is expected to respond
+with a success status and an empty list.
+------------------------
+packet: git< status=success
+packet: git< 0000
+packet: git< 0000 # empty content!
+packet: git< 0000 # empty list!
+------------------------
+
+In case the filter cannot or does not want to process the content,
+it is expected to respond with an "error" status. Depending on the
+`filter.<driver>.required` flag Git will interpret that as error
+but it will not stop or restart the filter process.
+------------------------
+packet: git< status=error
+packet: git< 0000
+------------------------
+
+If the filter experiences an error during processing, then it can
+send the status "error" after the content was (partially or
+completely) sent. Depending on the `filter.<driver>.required` flag
+Git will interpret that as error but it will not stop or restart the
+filter process.
+------------------------
+packet: git< status=success
+packet: git< 0000
+packet: git< HALF_WRITTEN_ERRONEOUS_CONTENT
+packet: git< 0000
+packet: git< status=error
+packet: git< 0000
+------------------------
+
+If the filter dies during the communication or does not adhere to
+the protocol then Git will stop the filter process and restart it
+with the next file that needs to be processed. Depending on the
+`filter.<driver>.required` flag Git will interpret that as error.
+
+The error handling for all cases above mimic the behavior of
+the `filter.<driver>.clean` / `filter.<driver>.smudge` error
+handling.
+
+In case the filter cannot or does not want to process the content
+as well as any future content for the lifetime of the Git process,
+it is expected to respond with an "abort" status. Depending on
+the `filter.<driver>.required` flag Git will interpret that as error
+for the content as well as any future content for the lifetime of the
+Git process but it will not stop or restart the filter process.
+------------------------
+packet: git< status=abort
+packet: git< 0000
+------------------------
+
+After the filter has processed a blob it is expected to wait for
+the next "key=value" list containing a command. Git will close
+the command pipe on exit. The filter is expected to detect EOF
+and exit gracefully on its own.
+
+A long running filter demo implementation can be found in
+`contrib/long-running-filter/example.pl` located in the Git
+core repository. If you develop your own long running filter
+process then the `GIT_TRACE_PACKET` environment variables can be
+very helpful for debugging (see linkgit:git[1]).
+
+If a `filter.<driver>.process` command is configured then it
+always takes precedence over a configured `filter.<driver>.clean`
+or `filter.<driver>.smudge` command.
+
+Please note that you cannot use an existing `filter.<driver>.clean`
+or `filter.<driver>.smudge` command with `filter.<driver>.process`
+because the former two use a different inter process communication
+protocol than the latter one.
+
+
Interaction between checkin/checkout attributes
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/contrib/long-running-filter/example.pl b/contrib/long-running-filter/example.pl
new file mode 100755
index 0000000..c13a631
--- /dev/null
+++ b/contrib/long-running-filter/example.pl
@@ -0,0 +1,123 @@
+#!/usr/bin/perl
+#
+# Example implementation for the Git filter protocol version 2
+# See Documentation/gitattributes.txt, section "Filter Protocol"
+#
+
+use strict;
+use warnings;
+
+my $MAX_PACKET_CONTENT_SIZE = 65516;
+
+sub packet_bin_read {
+ my $buffer;
+ my $bytes_read = read STDIN, $buffer, 4;
+ if ( $bytes_read == 0 ) {
+
+ # EOF - Git stopped talking to us!
+ exit();
+ }
+ elsif ( $bytes_read != 4 ) {
+ die "invalid packet size '$bytes_read' field";
+ }
+ my $pkt_size = hex($buffer);
+ if ( $pkt_size == 0 ) {
+ return ( 1, "" );
+ }
+ elsif ( $pkt_size > 4 ) {
+ my $content_size = $pkt_size - 4;
+ $bytes_read = read STDIN, $buffer, $content_size;
+ if ( $bytes_read != $content_size ) {
+ die "invalid packet ($content_size expected; $bytes_read read)";
+ }
+ return ( 0, $buffer );
+ }
+ else {
+ die "invalid packet size";
+ }
+}
+
+sub packet_txt_read {
+ my ( $res, $buf ) = packet_bin_read();
+ unless ( $buf =~ /\n$/ ) {
+ die "A non-binary line SHOULD BE terminated by an LF.";
+ }
+ return ( $res, substr( $buf, 0, -1 ) );
+}
+
+sub packet_bin_write {
+ my ($packet) = @_;
+ print STDOUT sprintf( "%04x", length($packet) + 4 );
+ print STDOUT $packet;
+ STDOUT->flush();
+}
+
+sub packet_txt_write {
+ packet_bin_write( $_[0] . "\n" );
+}
+
+sub packet_flush {
+ print STDOUT sprintf( "%04x", 0 );
+ STDOUT->flush();
+}
+
+( packet_txt_read() eq ( 0, "git-filter-client" ) ) || die "bad initialize";
+( packet_txt_read() eq ( 0, "version=2" ) ) || die "bad version";
+( packet_bin_read() eq ( 1, "" ) ) || die "bad version end";
+
+packet_txt_write("git-filter-server");
+packet_txt_write("version=2");
+
+( packet_txt_read() eq ( 0, "clean=true" ) ) || die "bad capability";
+( packet_txt_read() eq ( 0, "smudge=true" ) ) || die "bad capability";
+( packet_bin_read() eq ( 1, "" ) ) || die "bad capability end";
+
+packet_txt_write("clean=true");
+packet_txt_write("smudge=true");
+packet_flush();
+
+while (1) {
+ my ($command) = packet_txt_read() =~ /^command=([^=]+)$/;
+ my ($pathname) = packet_txt_read() =~ /^pathname=([^=]+)$/;
+
+ packet_bin_read();
+
+ my $input = "";
+ {
+ binmode(STDIN);
+ my $buffer;
+ my $done = 0;
+ while ( !$done ) {
+ ( $done, $buffer ) = packet_bin_read();
+ $input .= $buffer;
+ }
+ }
+
+ my $output;
+ if ( $command eq "clean" ) {
+ ### Perform clean here ###
+ $output = $input;
+ }
+ elsif ( $command eq "smudge" ) {
+ ### Perform smudge here ###
+ $output = $input;
+ }
+ else {
+ die "bad command '$command'";
+ }
+
+ packet_txt_write("status=success");
+ packet_flush();
+ while ( length($output) > 0 ) {
+ my $packet = substr( $output, 0, $MAX_PACKET_CONTENT_SIZE );
+ packet_bin_write($packet);
+ if ( length($output) > $MAX_PACKET_CONTENT_SIZE ) {
+ $output = substr( $output, $MAX_PACKET_CONTENT_SIZE );
+ }
+ else {
+ $output = "";
+ }
+ }
+ packet_flush(); # flush content!
+ packet_flush(); # empty list!
+}
diff --git a/convert.c b/convert.c
index 597f561..bd66257 100644
--- a/convert.c
+++ b/convert.c
@@ -3,6 +3,7 @@
#include "run-command.h"
#include "quote.h"
#include "sigchain.h"
+#include "pkt-line.h"
/*
* convert.c - convert a file when checking it out and checking it in.
@@ -442,7 +443,7 @@ static int filter_buffer_or_fd(int in, int out, void *data)
return (write_err || status);
}
-static int apply_filter(const char *path, const char *src, size_t len, int fd,
+static int apply_single_file_filter(const char *path, const char *src, size_t len, int fd,
struct strbuf *dst, const char *cmd)
{
/*
@@ -456,12 +457,6 @@ static int apply_filter(const char *path, const char *src, size_t len, int fd,
struct async async;
struct filter_params params;
- if (!cmd || !*cmd)
- return 0;
-
- if (!dst)
- return 1;
-
memset(&async, 0, sizeof(async));
async.proc = filter_buffer_or_fd;
async.data = ¶ms;
@@ -493,14 +488,317 @@ static int apply_filter(const char *path, const char *src, size_t len, int fd,
return !err;
}
+#define CAP_CLEAN (1u<<0)
+#define CAP_SMUDGE (1u<<1)
+
+struct cmd2process {
+ struct hashmap_entry ent; /* must be the first member! */
+ unsigned int supported_capabilities;
+ const char *cmd;
+ struct child_process process;
+};
+
+static int cmd_process_map_initialized;
+static struct hashmap cmd_process_map;
+
+static int cmd2process_cmp(const struct cmd2process *e1,
+ const struct cmd2process *e2,
+ const void *unused)
+{
+ return strcmp(e1->cmd, e2->cmd);
+}
+
+static struct cmd2process *find_multi_file_filter_entry(struct hashmap *hashmap, const char *cmd)
+{
+ struct cmd2process key;
+ hashmap_entry_init(&key, strhash(cmd));
+ key.cmd = cmd;
+ return hashmap_get(hashmap, &key, NULL);
+}
+
+static void kill_multi_file_filter(struct hashmap *hashmap, struct cmd2process *entry)
+{
+ if (!entry)
+ return;
+ sigchain_push(SIGPIPE, SIG_IGN);
+ /*
+ * We kill the filter most likely because an error happened already.
+ * That's why we are not interested in any error code here.
+ */
+ close(entry->process.in);
+ close(entry->process.out);
+ sigchain_pop(SIGPIPE);
+ finish_command(&entry->process);
+ hashmap_remove(hashmap, entry, NULL);
+ free(entry);
+}
+
+static int packet_write_list(int fd, const char *line, ...)
+{
+ va_list args;
+ int err;
+ va_start(args, line);
+ for (;;) {
+ if (!line)
+ break;
+ if (strlen(line) > PKTLINE_DATA_MAXLEN)
+ return -1;
+ err = packet_write_fmt_gently(fd, "%s\n", line);
+ if (err)
+ return err;
+ line = va_arg(args, const char*);
+ }
+ va_end(args);
+ return packet_flush_gently(fd);
+}
+
+static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, const char *cmd)
+{
+ int err;
+ struct cmd2process *entry;
+ struct child_process *process;
+ const char *argv[] = { cmd, NULL };
+ struct string_list cap_list = STRING_LIST_INIT_NODUP;
+ char *cap_buf;
+ const char *cap_name;
+
+ entry = xmalloc(sizeof(*entry));
+ hashmap_entry_init(entry, strhash(cmd));
+ entry->cmd = cmd;
+ entry->supported_capabilities = 0;
+ process = &entry->process;
+
+ child_process_init(process);
+ process->argv = argv;
+ process->use_shell = 1;
+ process->in = -1;
+ process->out = -1;
+
+ if (start_command(process)) {
+ error("cannot fork to run external filter '%s'", cmd);
+ kill_multi_file_filter(hashmap, entry);
+ return NULL;
+ }
+
+ sigchain_push(SIGPIPE, SIG_IGN);
+
+ err = packet_write_list(process->in, "git-filter-client", "version=2", NULL);
+ if (err)
+ goto done;
+
+ err = strcmp(packet_read_line(process->out, NULL), "git-filter-server");
+ if (err) {
+ error("external filter '%s' does not support long running filter protocol", cmd);
+ goto done;
+ }
+ err = strcmp(packet_read_line(process->out, NULL), "version=2");
+ if (err)
+ goto done;
+
+ err = packet_write_list(process->in, "clean=true", "smudge=true", NULL);
+
+ for (;;) {
+ cap_buf = packet_read_line(process->out, NULL);
+ if (!cap_buf)
+ break;
+ string_list_split_in_place(&cap_list, cap_buf, '=', 1);
+
+ if (cap_list.nr != 2 || strcmp(cap_list.items[1].string, "true"))
+ continue;
+
+ cap_name = cap_list.items[0].string;
+ if (!strcmp(cap_name, "clean")) {
+ entry->supported_capabilities |= CAP_CLEAN;
+ } else if (!strcmp(cap_name, "smudge")) {
+ entry->supported_capabilities |= CAP_SMUDGE;
+ } else {
+ warning(
+ "external filter '%s' requested unsupported filter capability '%s'",
+ cmd, cap_name
+ );
+ }
+
+ string_list_clear(&cap_list, 0);
+ }
+
+done:
+ sigchain_pop(SIGPIPE);
+
+ if (err || errno == EPIPE) {
+ error("initialization for external filter '%s' failed", cmd);
+ kill_multi_file_filter(hashmap, entry);
+ return NULL;
+ }
+
+ hashmap_add(hashmap, entry);
+ return entry;
+}
+
+static void read_multi_file_filter_values(int fd, struct strbuf *status) {
+ struct strbuf **pair;
+ char *line;
+ for (;;) {
+ line = packet_read_line(fd, NULL);
+ if (!line)
+ break;
+ pair = strbuf_split_str(line, '=', 2);
+ if (pair[0] && pair[0]->len && pair[1]) {
+ if (!strcmp(pair[0]->buf, "status=")) {
+ strbuf_reset(status);
+ strbuf_addbuf(status, pair[1]);
+ }
+ }
+ }
+}
+
+static int apply_multi_file_filter(const char *path, const char *src, size_t len,
+ int fd, struct strbuf *dst, const char *cmd,
+ const unsigned int wanted_capability)
+{
+ int err;
+ struct cmd2process *entry;
+ struct child_process *process;
+ struct stat file_stat;
+ struct strbuf nbuf = STRBUF_INIT;
+ struct strbuf filter_status = STRBUF_INIT;
+ char *filter_type;
+
+ if (!cmd_process_map_initialized) {
+ cmd_process_map_initialized = 1;
+ hashmap_init(&cmd_process_map, (hashmap_cmp_fn) cmd2process_cmp, 0);
+ entry = NULL;
+ } else {
+ entry = find_multi_file_filter_entry(&cmd_process_map, cmd);
+ }
+
+ fflush(NULL);
+
+ if (!entry) {
+ entry = start_multi_file_filter(&cmd_process_map, cmd);
+ if (!entry)
+ return 0;
+ }
+ process = &entry->process;
+
+ if (!(wanted_capability & entry->supported_capabilities))
+ return 0;
+
+ if (CAP_CLEAN & wanted_capability)
+ filter_type = "clean";
+ else if (CAP_SMUDGE & wanted_capability)
+ filter_type = "smudge";
+ else
+ die("unexpected filter type");
+
+ if (fd >= 0 && !src) {
+ if (fstat(fd, &file_stat) == -1)
+ return 0;
+ len = xsize_t(file_stat.st_size);
+ }
+
+ sigchain_push(SIGPIPE, SIG_IGN);
+
+ err = strlen(filter_type) > PKTLINE_DATA_MAXLEN;
+ if (err)
+ goto done;
+
+ err = packet_write_fmt_gently(process->in, "command=%s\n", filter_type);
+ if (err)
+ goto done;
+
+ err = strlen(path) > PKTLINE_DATA_MAXLEN;
+ if (err)
+ goto done;
+
+ err = packet_write_fmt_gently(process->in, "pathname=%s\n", path);
+ if (err)
+ goto done;
+
+ err = packet_flush_gently(process->in);
+ if (err)
+ goto done;
+
+ if (fd >= 0)
+ err = write_packetized_from_fd(fd, process->in);
+ else
+ err = write_packetized_from_buf(src, len, process->in);
+ if (err)
+ goto done;
+
+ read_multi_file_filter_values(process->out, &filter_status);
+ err = strcmp(filter_status.buf, "success");
+ if (err)
+ goto done;
+
+ err = read_packetized_to_buf(process->out, &nbuf) < 0;
+ if (err)
+ goto done;
+
+ read_multi_file_filter_values(process->out, &filter_status);
+ err = strcmp(filter_status.buf, "success");
+
+done:
+ sigchain_pop(SIGPIPE);
+
+ if (err || errno == EPIPE) {
+ if (!strcmp(filter_status.buf, "error")) {
+ /* The filter signaled a problem with the file. */
+ } else if (!strcmp(filter_status.buf, "abort")) {
+ /*
+ * The filter signaled a permanent problem. Don't try to filter
+ * files with the same command for the lifetime of the current
+ * Git process.
+ */
+ entry->supported_capabilities &= ~wanted_capability;
+ } else {
+ /*
+ * Something went wrong with the protocol filter.
+ * Force shutdown and restart if another blob requires filtering!
+ */
+ error("external filter '%s' failed", cmd);
+ kill_multi_file_filter(&cmd_process_map, entry);
+ }
+ } else {
+ strbuf_swap(dst, &nbuf);
+ }
+ strbuf_release(&nbuf);
+ return !err;
+}
+
static struct convert_driver {
const char *name;
struct convert_driver *next;
const char *smudge;
const char *clean;
+ const char *process;
int required;
} *user_convert, **user_convert_tail;
+static int apply_filter(const char *path, const char *src, size_t len,
+ int fd, struct strbuf *dst, struct convert_driver *drv,
+ const unsigned int wanted_capability)
+{
+ const char *cmd = NULL;
+
+ if (!drv)
+ return 0;
+
+ if (!dst)
+ return 1;
+
+ if (!drv->process && (CAP_CLEAN & wanted_capability) && drv->clean)
+ cmd = drv->clean;
+ else if (!drv->process && (CAP_SMUDGE & wanted_capability) && drv->smudge)
+ cmd = drv->smudge;
+
+ if (cmd && *cmd)
+ return apply_single_file_filter(path, src, len, fd, dst, cmd);
+ else if (drv->process && *drv->process)
+ return apply_multi_file_filter(path, src, len, fd, dst, drv->process, wanted_capability);
+
+ return 0;
+}
+
static int read_convert_config(const char *var, const char *value, void *cb)
{
const char *key, *name;
@@ -538,6 +836,9 @@ static int read_convert_config(const char *var, const char *value, void *cb)
if (!strcmp("clean", key))
return git_config_string(&drv->clean, var, value);
+ if (!strcmp("process", key))
+ return git_config_string(&drv->process, var, value);
+
if (!strcmp("required", key)) {
drv->required = git_config_bool(var, value);
return 0;
@@ -839,7 +1140,7 @@ int would_convert_to_git_filter_fd(const char *path)
if (!ca.drv->required)
return 0;
- return apply_filter(path, NULL, 0, -1, NULL, ca.drv->clean);
+ return apply_filter(path, NULL, 0, -1, NULL, ca.drv, CAP_CLEAN);
}
const char *get_convert_attr_ascii(const char *path)
@@ -872,18 +1173,12 @@ int convert_to_git(const char *path, const char *src, size_t len,
struct strbuf *dst, enum safe_crlf checksafe)
{
int ret = 0;
- const char *filter = NULL;
- int required = 0;
struct conv_attrs ca;
convert_attrs(&ca, path);
- if (ca.drv) {
- filter = ca.drv->clean;
- required = ca.drv->required;
- }
- ret |= apply_filter(path, src, len, -1, dst, filter);
- if (!ret && required)
+ ret |= apply_filter(path, src, len, -1, dst, ca.drv, CAP_CLEAN);
+ if (!ret && ca.drv && ca.drv->required)
die("%s: clean filter '%s' failed", path, ca.drv->name);
if (ret && dst) {
@@ -905,9 +1200,9 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,
convert_attrs(&ca, path);
assert(ca.drv);
- assert(ca.drv->clean);
+ assert(ca.drv->clean || ca.drv->process);
- if (!apply_filter(path, NULL, 0, fd, dst, ca.drv->clean))
+ if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN))
die("%s: clean filter '%s' failed", path, ca.drv->name);
crlf_to_git(path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);
@@ -919,15 +1214,9 @@ static int convert_to_working_tree_internal(const char *path, const char *src,
int normalizing)
{
int ret = 0, ret_filter = 0;
- const char *filter = NULL;
- int required = 0;
struct conv_attrs ca;
convert_attrs(&ca, path);
- if (ca.drv) {
- filter = ca.drv->smudge;
- required = ca.drv->required;
- }
ret |= ident_to_worktree(path, src, len, dst, ca.ident);
if (ret) {
@@ -936,9 +1225,10 @@ static int convert_to_working_tree_internal(const char *path, const char *src,
}
/*
* CRLF conversion can be skipped if normalizing, unless there
- * is a smudge filter. The filter might expect CRLFs.
+ * is a smudge or process filter (even if the process filter doesn't
+ * support smudge). The filters might expect CRLFs.
*/
- if (filter || !normalizing) {
+ if ((ca.drv && (ca.drv->smudge || ca.drv->process)) || !normalizing) {
ret |= crlf_to_worktree(path, src, len, dst, ca.crlf_action);
if (ret) {
src = dst->buf;
@@ -946,8 +1236,8 @@ static int convert_to_working_tree_internal(const char *path, const char *src,
}
}
- ret_filter = apply_filter(path, src, len, -1, dst, filter);
- if (!ret_filter && required)
+ ret_filter = apply_filter(path, src, len, -1, dst, ca.drv, CAP_SMUDGE);
+ if (!ret_filter && ca.drv && ca.drv->required)
die("%s: smudge filter %s failed", path, ca.drv->name);
return ret | ret_filter;
@@ -1399,7 +1689,7 @@ struct stream_filter *get_stream_filter(const char *path, const unsigned char *s
struct stream_filter *filter = NULL;
convert_attrs(&ca, path);
- if (ca.drv && (ca.drv->smudge || ca.drv->clean))
+ if (ca.drv && (ca.drv->process || ca.drv->smudge || ca.drv->clean))
return NULL;
if (ca.crlf_action == CRLF_AUTO || ca.crlf_action == CRLF_AUTO_CRLF)
diff --git a/pkt-line.h b/pkt-line.h
index 6df8449..3d873f3 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -86,6 +86,7 @@ ssize_t read_packetized_to_buf(int fd_in, struct strbuf *sb_out);
#define DEFAULT_PACKET_MAX 1000
#define LARGE_PACKET_MAX 65520
+#define PKTLINE_DATA_MAXLEN (LARGE_PACKET_MAX - 4)
extern char packet_buffer[LARGE_PACKET_MAX];
#endif
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index dc50938..210c4f6 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -31,7 +31,10 @@ test_expect_success setup '
cat test >test.i &&
git add test test.t test.i &&
rm -f test test.t test.i &&
- git checkout -- test test.t test.i
+ git checkout -- test test.t test.i &&
+
+ echo "content-test2" >test2.o &&
+ echo "content-test3 - subdir" >"test3 - subdir.o"
'
script='s/^\$Id: \([0-9a-f]*\) \$/\1/p'
@@ -279,4 +282,364 @@ test_expect_success 'diff does not reuse worktree files that need cleaning' '
test_line_count = 0 count
'
+check_filter () {
+ rm -f rot13-filter.log actual.log &&
+ "$@" 2> git_stderr.log &&
+ test_must_be_empty git_stderr.log &&
+ cat >expected.log &&
+ sort rot13-filter.log | uniq -c | sed "s/^[ ]*//" >actual.log &&
+ test_cmp expected.log actual.log
+}
+
+check_filter_count_clean () {
+ rm -f rot13-filter.log actual.log &&
+ "$@" 2> git_stderr.log &&
+ test_must_be_empty git_stderr.log &&
+ cat >expected.log &&
+ sort rot13-filter.log | uniq -c | sed "s/^[ ]*//" |
+ sed "s/^\([0-9]\) IN: clean/x IN: clean/" >actual.log &&
+ test_cmp expected.log actual.log
+}
+
+check_filter_ignore_clean () {
+ rm -f rot13-filter.log actual.log &&
+ "$@" &&
+ cat >expected.log &&
+ grep -v "IN: clean" rot13-filter.log >actual.log &&
+ test_cmp expected.log actual.log
+}
+
+check_filter_no_call () {
+ rm -f rot13-filter.log &&
+ "$@" 2> git_stderr.log &&
+ test_must_be_empty git_stderr.log &&
+ test_must_be_empty rot13-filter.log
+}
+
+check_rot13 () {
+ test_cmp "$1" "$2" &&
+ ./../rot13.sh <"$1" >expected &&
+ git cat-file blob :"$2" >actual &&
+ test_cmp expected actual
+}
+
+test_expect_success PERL 'required process filter should filter data' '
+ test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge" &&
+ test_config_global filter.protocol.required true &&
+ rm -rf repo &&
+ mkdir repo &&
+ (
+ cd repo &&
+ git init &&
+
+ echo "*.r filter=protocol" >.gitattributes &&
+ git add . &&
+ git commit . -m "test commit" &&
+ git branch empty &&
+
+ cp ../test.o test.r &&
+ cp ../test2.o test2.r &&
+ mkdir testsubdir &&
+ cp "../test3 - subdir.o" "testsubdir/test3 - subdir.r" &&
+ >test4-empty.r &&
+
+ check_filter \
+ git add . \
+ <<-\EOF &&
+ 1 IN: clean test.r 57 [OK] -- OUT: 57 . [OK]
+ 1 IN: clean test2.r 14 [OK] -- OUT: 14 . [OK]
+ 1 IN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]
+ 1 IN: clean testsubdir/test3 - subdir.r 23 [OK] -- OUT: 23 . [OK]
+ 1 START
+ 1 STOP
+ 1 wrote filter header
+ EOF
+
+ check_filter_count_clean \
+ git commit . -m "test commit" \
+ <<-\EOF &&
+ x IN: clean test.r 57 [OK] -- OUT: 57 . [OK]
+ x IN: clean test2.r 14 [OK] -- OUT: 14 . [OK]
+ x IN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]
+ x IN: clean testsubdir/test3 - subdir.r 23 [OK] -- OUT: 23 . [OK]
+ 1 START
+ 1 STOP
+ 1 wrote filter header
+ EOF
+
+ rm -f test?.r "testsubdir/test3 - subdir.r" &&
+
+ check_filter_ignore_clean \
+ git checkout . \
+ <<-\EOF &&
+ START
+ wrote filter header
+ IN: smudge test2.r 14 [OK] -- OUT: 14 . [OK]
+ IN: smudge testsubdir/test3 - subdir.r 23 [OK] -- OUT: 23 . [OK]
+ STOP
+ EOF
+
+ check_filter_ignore_clean \
+ git checkout empty \
+ <<-\EOF &&
+ START
+ wrote filter header
+ STOP
+ EOF
+
+ check_filter_ignore_clean \
+ git checkout master \
+ <<-\EOF &&
+ START
+ wrote filter header
+ IN: smudge test.r 57 [OK] -- OUT: 57 . [OK]
+ IN: smudge test2.r 14 [OK] -- OUT: 14 . [OK]
+ IN: smudge test4-empty.r 0 [OK] -- OUT: 0 [OK]
+ IN: smudge testsubdir/test3 - subdir.r 23 [OK] -- OUT: 23 . [OK]
+ STOP
+ EOF
+
+ check_rot13 ../test.o test.r &&
+ check_rot13 ../test2.o test2.r &&
+ check_rot13 "../test3 - subdir.o" "testsubdir/test3 - subdir.r"
+ )
+'
+
+test_expect_success PERL 'required process filter should clean only and take precedence' '
+ test_config_global filter.protocol.clean ./../rot13.sh &&
+ test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean" &&
+ test_config_global filter.protocol.required true &&
+ rm -rf repo &&
+ mkdir repo &&
+ (
+ cd repo &&
+ git init &&
+
+ echo "*.r filter=protocol" >.gitattributes &&
+ git add . &&
+ git commit . -m "test commit" &&
+ git branch empty &&
+
+ cp ../test.o test.r &&
+
+ check_filter \
+ git add . \
+ <<-\EOF &&
+ 1 IN: clean test.r 57 [OK] -- OUT: 57 . [OK]
+ 1 START
+ 1 STOP
+ 1 wrote filter header
+ EOF
+
+ check_filter_count_clean \
+ git commit . -m "test commit" \
+ <<-\EOF
+ x IN: clean test.r 57 [OK] -- OUT: 57 . [OK]
+ 1 START
+ 1 STOP
+ 1 wrote filter header
+ EOF
+ )
+'
+
+generate_test_data () {
+ LEN=$1
+ NAME=$2
+ test-genrandom end $LEN |
+ perl -pe "s/./chr((ord($&) % 26) + 97)/sge" >../$NAME.file &&
+ cp ../$NAME.file . &&
+ ./../rot13.sh <../$NAME.file >../$NAME.file.rot13
+}
+
+test_expect_success PERL 'required process filter should process multiple packets' '
+ test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge" &&
+ test_config_global filter.protocol.required true &&
+
+ rm -rf repo &&
+ mkdir repo &&
+ (
+ cd repo &&
+ git init &&
+
+ # Generate data that requires 3 packets
+ PKTLINE_DATA_MAXLEN=65516 &&
+
+ generate_test_data $(($PKTLINE_DATA_MAXLEN )) 1pkt_1__ &&
+ generate_test_data $(($PKTLINE_DATA_MAXLEN + 1)) 2pkt_1+1 &&
+ generate_test_data $(($PKTLINE_DATA_MAXLEN * 2 - 1)) 2pkt_2-1 &&
+ generate_test_data $(($PKTLINE_DATA_MAXLEN * 2 )) 2pkt_2__ &&
+ generate_test_data $(($PKTLINE_DATA_MAXLEN * 2 + 1)) 3pkt_2+1 &&
+
+ echo "*.file filter=protocol" >.gitattributes &&
+ check_filter \
+ git add *.file .gitattributes \
+ <<-\EOF &&
+ 1 IN: clean 1pkt_1__.file 65516 [OK] -- OUT: 65516 . [OK]
+ 1 IN: clean 2pkt_1+1.file 65517 [OK] -- OUT: 65517 .. [OK]
+ 1 IN: clean 2pkt_2-1.file 131031 [OK] -- OUT: 131031 .. [OK]
+ 1 IN: clean 2pkt_2__.file 131032 [OK] -- OUT: 131032 .. [OK]
+ 1 IN: clean 3pkt_2+1.file 131033 [OK] -- OUT: 131033 ... [OK]
+ 1 START
+ 1 STOP
+ 1 wrote filter header
+ EOF
+ git commit . -m "test commit" &&
+
+ rm -f *.file &&
+ git checkout -- *.file &&
+
+ for f in *.file
+ do
+ git cat-file blob :$f >actual &&
+ test_cmp ../$f.rot13 actual
+ done
+ )
+'
+
+test_expect_success PERL 'required process filter should with clean error should fail' '
+ test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge" &&
+ test_config_global filter.protocol.required true &&
+ rm -rf repo &&
+ mkdir repo &&
+ (
+ cd repo &&
+ git init &&
+
+ echo "*.r filter=protocol" >.gitattributes &&
+
+ cp ../test.o test.r &&
+ echo "this is going to fail" >clean-write-fail.r &&
+ echo "content-test3-subdir" >test3.r &&
+
+ # Note: There are three clean paths in convert.c we just test one here.
+ test_must_fail git add .
+ )
+'
+
+test_expect_success PERL 'process filter should restart after unexpected write failure' '
+ test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge" &&
+ rm -rf repo &&
+ mkdir repo &&
+ (
+ cd repo &&
+ git init &&
+
+ echo "*.r filter=protocol" >.gitattributes &&
+
+ cp ../test.o test.r &&
+ cp ../test2.o test2.r &&
+ echo "this is going to fail" >smudge-write-fail.o &&
+ cat smudge-write-fail.o >smudge-write-fail.r &&
+ git add . &&
+ git commit . -m "test commit" &&
+ rm -f *.r &&
+
+ check_filter_ignore_clean \
+ git checkout . \
+ <<-\EOF &&
+ START
+ wrote filter header
+ IN: smudge smudge-write-fail.r 22 [OK] -- OUT: 22 [WRITE FAIL]
+ START
+ wrote filter header
+ IN: smudge test.r 57 [OK] -- OUT: 57 . [OK]
+ IN: smudge test2.r 14 [OK] -- OUT: 14 . [OK]
+ STOP
+ EOF
+
+ check_rot13 ../test.o test.r &&
+ check_rot13 ../test2.o test2.r &&
+
+ ! test_cmp smudge-write-fail.o smudge-write-fail.r && # Smudge failed!
+ ./../rot13.sh <smudge-write-fail.o >expected &&
+ git cat-file blob :smudge-write-fail.r >actual &&
+ test_cmp expected actual # Clean worked!
+ )
+'
+
+test_expect_success PERL 'process filter should not restart in case of an error' '
+ test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge" &&
+ rm -rf repo &&
+ mkdir repo &&
+ (
+ cd repo &&
+ git init &&
+
+ echo "*.r filter=protocol" >.gitattributes &&
+
+ cp ../test.o test.r &&
+ cp ../test2.o test2.r &&
+ echo "this will cause an error" >error.o &&
+ cp error.o error.r &&
+ git add . &&
+ git commit . -m "test commit" &&
+ rm -f *.r &&
+
+ check_filter_ignore_clean \
+ git checkout . \
+ <<-\EOF &&
+ START
+ wrote filter header
+ IN: smudge error.r 25 [OK] -- OUT: 0 [ERROR]
+ IN: smudge test.r 57 [OK] -- OUT: 57 . [OK]
+ IN: smudge test2.r 14 [OK] -- OUT: 14 . [OK]
+ STOP
+ EOF
+
+ check_rot13 ../test.o test.r &&
+ check_rot13 ../test2.o test2.r &&
+ test_cmp error.o error.r
+ )
+'
+
+test_expect_success PERL 'process filter should be able to signal an error for all future files' '
+ test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge" &&
+ rm -rf repo &&
+ mkdir repo &&
+ (
+ cd repo &&
+ git init &&
+
+ echo "*.r filter=protocol" >.gitattributes &&
+
+ cp ../test.o test.r &&
+ cp ../test2.o test2.r &&
+ echo "error this blob and all future blobs" >abort.o &&
+ cp abort.o abort.r &&
+ git add . &&
+ git commit . -m "test commit" &&
+ rm -f *.r &&
+
+ check_filter_ignore_clean \
+ git checkout . \
+ <<-\EOF &&
+ START
+ wrote filter header
+ IN: smudge abort.r 37 [OK] -- OUT: 0 [ABORT]
+ STOP
+ EOF
+
+ test_cmp ../test.o test.r &&
+ test_cmp ../test2.o test2.r &&
+ test_cmp abort.o abort.r
+ )
+'
+
+test_expect_success PERL 'invalid process filter must fail (and not hang!)' '
+ test_config_global filter.protocol.process cat &&
+ test_config_global filter.protocol.required true &&
+ rm -rf repo &&
+ mkdir repo &&
+ (
+ cd repo &&
+ git init &&
+
+ echo "*.r filter=protocol" >.gitattributes &&
+
+ cp ../test.o test.r &&
+ test_must_fail git add . 2> git_stderr.log &&
+ grep "not support long running filter protocol" git_stderr.log
+ )
+'
+
test_done
diff --git a/t/t0021/rot13-filter.pl b/t/t0021/rot13-filter.pl
new file mode 100755
index 0000000..8958f71
--- /dev/null
+++ b/t/t0021/rot13-filter.pl
@@ -0,0 +1,191 @@
+#!/usr/bin/perl
+#
+# Example implementation for the Git filter protocol version 2
+# See Documentation/gitattributes.txt, section "Filter Protocol"
+#
+# The script takes the list of supported protocol capabilities as
+# arguments ("clean", "smudge", etc).
+#
+# This implementation supports special test cases:
+# (1) If data with the pathname "clean-write-fail.r" is processed with
+# a "clean" operation then the write operation will die.
+# (2) If data with the pathname "smudge-write-fail.r" is processed with
+# a "smudge" operation then the write operation will die.
+# (3) If data with the pathname "error.r" is processed with any
+# operation then the filter signals that it cannot or does not want
+# to process the file.
+# (4) If data with the pathname "abort.r" is processed with any
+# operation then the filter signals that it cannot or does not want
+# to process the file and any file after that is processed with the
+# same command.
+#
+
+use strict;
+use warnings;
+
+my $MAX_PACKET_CONTENT_SIZE = 65516;
+my @capabilities = @ARGV;
+
+open my $debug, ">>", "rot13-filter.log";
+
+sub rot13 {
+ my ($str) = @_;
+ $str =~ y/A-Za-z/N-ZA-Mn-za-m/;
+ return $str;
+}
+
+sub packet_bin_read {
+ my $buffer;
+ my $bytes_read = read STDIN, $buffer, 4;
+ if ( $bytes_read == 0 ) {
+
+ # EOF - Git stopped talking to us!
+ print $debug "STOP\n";
+ exit();
+ }
+ elsif ( $bytes_read != 4 ) {
+ die "invalid packet size '$bytes_read' field";
+ }
+ my $pkt_size = hex($buffer);
+ if ( $pkt_size == 0 ) {
+ return ( 1, "" );
+ }
+ elsif ( $pkt_size > 4 ) {
+ my $content_size = $pkt_size - 4;
+ $bytes_read = read STDIN, $buffer, $content_size;
+ if ( $bytes_read != $content_size ) {
+ die "invalid packet ($content_size expected; $bytes_read read)";
+ }
+ return ( 0, $buffer );
+ }
+ else {
+ die "invalid packet size";
+ }
+}
+
+sub packet_txt_read {
+ my ( $res, $buf ) = packet_bin_read();
+ unless ( $buf =~ /\n$/ ) {
+ die "A non-binary line SHOULD BE terminated by an LF.";
+ }
+ return ( $res, substr( $buf, 0, -1 ) );
+}
+
+sub packet_bin_write {
+ my ($packet) = @_;
+ print STDOUT sprintf( "%04x", length($packet) + 4 );
+ print STDOUT $packet;
+ STDOUT->flush();
+}
+
+sub packet_txt_write {
+ packet_bin_write( $_[0] . "\n" );
+}
+
+sub packet_flush {
+ print STDOUT sprintf( "%04x", 0 );
+ STDOUT->flush();
+}
+
+print $debug "START\n";
+$debug->flush();
+
+( packet_txt_read() eq ( 0, "git-filter-client" ) ) || die "bad initialize";
+( packet_txt_read() eq ( 0, "version=2" ) ) || die "bad version";
+( packet_bin_read() eq ( 1, "" ) ) || die "bad version end";
+
+packet_txt_write("git-filter-server");
+packet_txt_write("version=2");
+
+( packet_txt_read() eq ( 0, "clean=true" ) ) || die "bad capability";
+( packet_txt_read() eq ( 0, "smudge=true" ) ) || die "bad capability";
+( packet_bin_read() eq ( 1, "" ) ) || die "bad capability end";
+
+foreach (@capabilities) {
+ packet_txt_write( $_ . "=true" );
+}
+packet_flush();
+print $debug "wrote filter header\n";
+$debug->flush();
+
+while (1) {
+ my ($command) = packet_txt_read() =~ /^command=([^=]+)$/;
+ print $debug "IN: $command";
+ $debug->flush();
+
+ my ($pathname) = packet_txt_read() =~ /^pathname=([^=]+)$/;
+ print $debug " $pathname";
+ $debug->flush();
+
+ # Flush
+ packet_bin_read();
+
+ my $input = "";
+ {
+ binmode(STDIN);
+ my $buffer;
+ my $done = 0;
+ while ( !$done ) {
+ ( $done, $buffer ) = packet_bin_read();
+ $input .= $buffer;
+ }
+ print $debug " " . length($input) . " [OK] -- ";
+ $debug->flush();
+ }
+
+ my $output;
+ if ( $pathname eq "error.r" or $pathname eq "abort.r" ) {
+ $output = "";
+ }
+ elsif ( $command eq "clean" and grep( /^clean$/, @capabilities ) ) {
+ $output = rot13($input);
+ }
+ elsif ( $command eq "smudge" and grep( /^smudge$/, @capabilities ) ) {
+ $output = rot13($input);
+ }
+ else {
+ die "bad command '$command'";
+ }
+
+ print $debug "OUT: " . length($output) . " ";
+ $debug->flush();
+
+ if ( $pathname eq "error.r" ) {
+ print $debug "[ERROR]\n";
+ $debug->flush();
+ packet_txt_write("status=error");
+ packet_flush();
+ }
+ elsif ( $pathname eq "abort.r" ) {
+ print $debug "[ABORT]\n";
+ $debug->flush();
+ packet_txt_write("status=abort");
+ packet_flush();
+ }
+ else {
+ packet_txt_write("status=success");
+ packet_flush();
+
+ if ( $pathname eq "${command}-write-fail.r" ) {
+ print $debug "[WRITE FAIL]\n";
+ $debug->flush();
+ die "${command} write error";
+ }
+
+ while ( length($output) > 0 ) {
+ my $packet = substr( $output, 0, $MAX_PACKET_CONTENT_SIZE );
+ packet_bin_write($packet);
+ print $debug ".";
+ if ( length($output) > $MAX_PACKET_CONTENT_SIZE ) {
+ $output = substr( $output, $MAX_PACKET_CONTENT_SIZE );
+ }
+ else {
+ $output = "";
+ }
+ }
+ packet_flush();
+ print $debug " [OK]\n";
+ $debug->flush();
+ packet_flush();
+ }
+}
--
2.10.0
^ permalink raw reply related
* [PATCH v8 07/11] pkt-line: add functions to read/write flush terminated packet streams
From: larsxschneider @ 2016-09-20 19:02 UTC (permalink / raw)
To: git
Cc: peff, gitster, sbeller, jnareb, mlbright, tboegi, ramsay,
Lars Schneider
In-Reply-To: <20160920190247.82189-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
write_packetized_from_fd() and write_packetized_from_buf() write a
stream of packets. All content packets use the maximal packet size
except for the last one. After the last content packet a `flush` control
packet is written.
read_packetized_to_buf() reads arbitrary sized packets until it detects
a `flush` packet.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
pkt-line.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
pkt-line.h | 7 +++++++
2 files changed, 75 insertions(+)
diff --git a/pkt-line.c b/pkt-line.c
index fc0ac12..a0a8543 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -196,6 +196,47 @@ void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
va_end(args);
}
+int write_packetized_from_fd(int fd_in, int fd_out)
+{
+ static char buf[PKTLINE_DATA_MAXLEN];
+ int err = 0;
+ ssize_t bytes_to_write;
+
+ while (!err) {
+ bytes_to_write = xread(fd_in, buf, sizeof(buf));
+ if (bytes_to_write < 0)
+ return COPY_READ_ERROR;
+ if (bytes_to_write == 0)
+ break;
+ err = packet_write_gently(fd_out, buf, bytes_to_write);
+ }
+ if (!err)
+ err = packet_flush_gently(fd_out);
+ return err;
+}
+
+int write_packetized_from_buf(const char *src_in, size_t len, int fd_out)
+{
+ static char buf[PKTLINE_DATA_MAXLEN];
+ int err = 0;
+ size_t bytes_written = 0;
+ size_t bytes_to_write;
+
+ while (!err) {
+ if ((len - bytes_written) > sizeof(buf))
+ bytes_to_write = sizeof(buf);
+ else
+ bytes_to_write = len - bytes_written;
+ if (bytes_to_write == 0)
+ break;
+ err = packet_write_gently(fd_out, src_in + bytes_written, bytes_to_write);
+ bytes_written += bytes_to_write;
+ }
+ if (!err)
+ err = packet_flush_gently(fd_out);
+ return err;
+}
+
static int get_packet_data(int fd, char **src_buf, size_t *src_size,
void *dst, unsigned size, int options)
{
@@ -305,3 +346,30 @@ char *packet_read_line_buf(char **src, size_t *src_len, int *dst_len)
{
return packet_read_line_generic(-1, src, src_len, dst_len);
}
+
+ssize_t read_packetized_to_buf(int fd_in, struct strbuf *sb_out)
+{
+ int paket_len;
+ int options = PACKET_READ_GENTLE_ON_EOF;
+
+ size_t oldlen = sb_out->len;
+ size_t oldalloc = sb_out->alloc;
+
+ for (;;) {
+ strbuf_grow(sb_out, PKTLINE_DATA_MAXLEN+1);
+ paket_len = packet_read(fd_in, NULL, NULL,
+ sb_out->buf + sb_out->len, PKTLINE_DATA_MAXLEN+1, options);
+ if (paket_len <= 0)
+ break;
+ sb_out->len += paket_len;
+ }
+
+ if (paket_len < 0) {
+ if (oldalloc == 0)
+ strbuf_release(sb_out);
+ else
+ strbuf_setlen(sb_out, oldlen);
+ return paket_len;
+ }
+ return sb_out->len - oldlen;
+}
diff --git a/pkt-line.h b/pkt-line.h
index 3fa0899..6df8449 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -25,6 +25,8 @@ void packet_buf_flush(struct strbuf *buf);
void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
int packet_flush_gently(int fd);
int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+int write_packetized_from_fd(int fd_in, int fd_out);
+int write_packetized_from_buf(const char *src_in, size_t len, int fd_out);
/*
* Read a packetized line into the buffer, which must be at least size bytes
@@ -77,6 +79,11 @@ char *packet_read_line(int fd, int *size);
*/
char *packet_read_line_buf(char **src_buf, size_t *src_len, int *size);
+/*
+ * Reads a stream of variable sized packets until a flush packet is detected.
+ */
+ssize_t read_packetized_to_buf(int fd_in, struct strbuf *sb_out);
+
#define DEFAULT_PACKET_MAX 1000
#define LARGE_PACKET_MAX 65520
extern char packet_buffer[LARGE_PACKET_MAX];
--
2.10.0
^ permalink raw reply related
* [PATCH v8 08/11] convert: quote filter names in error messages
From: larsxschneider @ 2016-09-20 19:02 UTC (permalink / raw)
To: git
Cc: peff, gitster, sbeller, jnareb, mlbright, tboegi, ramsay,
Lars Schneider
In-Reply-To: <20160920190247.82189-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
Git filter driver commands with spaces (e.g. `filter.sh foo`) are hard
to read in error messages. Quote them to improve the readability.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
convert.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/convert.c b/convert.c
index 077f5e6..986c239 100644
--- a/convert.c
+++ b/convert.c
@@ -412,7 +412,7 @@ static int filter_buffer_or_fd(int in, int out, void *data)
child_process.out = out;
if (start_command(&child_process))
- return error("cannot fork to run external filter %s", params->cmd);
+ return error("cannot fork to run external filter '%s'", params->cmd);
sigchain_push(SIGPIPE, SIG_IGN);
@@ -430,13 +430,13 @@ static int filter_buffer_or_fd(int in, int out, void *data)
if (close(child_process.in))
write_err = 1;
if (write_err)
- error("cannot feed the input to external filter %s", params->cmd);
+ error("cannot feed the input to external filter '%s'", params->cmd);
sigchain_pop(SIGPIPE);
status = finish_command(&child_process);
if (status)
- error("external filter %s failed %d", params->cmd, status);
+ error("external filter '%s' failed %d", params->cmd, status);
strbuf_release(&cmd);
return (write_err || status);
@@ -477,15 +477,15 @@ static int apply_filter(const char *path, const char *src, size_t len, int fd,
return 0; /* error was already reported */
if (strbuf_read(&nbuf, async.out, len) < 0) {
- error("read from external filter %s failed", cmd);
+ error("read from external filter '%s' failed", cmd);
ret = 0;
}
if (close(async.out)) {
- error("read from external filter %s failed", cmd);
+ error("read from external filter '%s' failed", cmd);
ret = 0;
}
if (finish_async(&async)) {
- error("external filter %s failed", cmd);
+ error("external filter '%s' failed", cmd);
ret = 0;
}
--
2.10.0
^ permalink raw reply related
* [PATCH v8 09/11] convert: modernize tests
From: larsxschneider @ 2016-09-20 19:02 UTC (permalink / raw)
To: git
Cc: peff, gitster, sbeller, jnareb, mlbright, tboegi, ramsay,
Lars Schneider
In-Reply-To: <20160920190247.82189-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
Use `test_config` to set the config, check that files are empty with
`test_must_be_empty`, compare files with `test_cmp`, and remove spaces
after ">" and "<".
Please note that the "rot13" filter configured in "setup" keeps using
`git config` instead of `test_config` because subsequent tests might
depend on it.
Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
t/t0021-conversion.sh | 58 +++++++++++++++++++++++++--------------------------
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index e799e59..dc50938 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -38,8 +38,8 @@ script='s/^\$Id: \([0-9a-f]*\) \$/\1/p'
test_expect_success check '
- cmp test.o test &&
- cmp test.o test.t &&
+ test_cmp test.o test &&
+ test_cmp test.o test.t &&
# ident should be stripped in the repository
git diff --raw --exit-code :test :test.i &&
@@ -47,10 +47,10 @@ test_expect_success check '
embedded=$(sed -ne "$script" test.i) &&
test "z$id" = "z$embedded" &&
- git cat-file blob :test.t > test.r &&
+ git cat-file blob :test.t >test.r &&
- ./rot13.sh < test.o > test.t &&
- cmp test.r test.t
+ ./rot13.sh <test.o >test.t &&
+ test_cmp test.r test.t
'
# If an expanded ident ever gets into the repository, we want to make sure that
@@ -130,7 +130,7 @@ test_expect_success 'filter shell-escaped filenames' '
# delete the files and check them out again, using a smudge filter
# that will count the args and echo the command-line back to us
- git config filter.argc.smudge "sh ./argc.sh %f" &&
+ test_config filter.argc.smudge "sh ./argc.sh %f" &&
rm "$normal" "$special" &&
git checkout -- "$normal" "$special" &&
@@ -141,7 +141,7 @@ test_expect_success 'filter shell-escaped filenames' '
test_cmp expect "$special" &&
# do the same thing, but with more args in the filter expression
- git config filter.argc.smudge "sh ./argc.sh %f --my-extra-arg" &&
+ test_config filter.argc.smudge "sh ./argc.sh %f --my-extra-arg" &&
rm "$normal" "$special" &&
git checkout -- "$normal" "$special" &&
@@ -154,9 +154,9 @@ test_expect_success 'filter shell-escaped filenames' '
'
test_expect_success 'required filter should filter data' '
- git config filter.required.smudge ./rot13.sh &&
- git config filter.required.clean ./rot13.sh &&
- git config filter.required.required true &&
+ test_config filter.required.smudge ./rot13.sh &&
+ test_config filter.required.clean ./rot13.sh &&
+ test_config filter.required.required true &&
echo "*.r filter=required" >.gitattributes &&
@@ -165,17 +165,17 @@ test_expect_success 'required filter should filter data' '
rm -f test.r &&
git checkout -- test.r &&
- cmp test.o test.r &&
+ test_cmp test.o test.r &&
./rot13.sh <test.o >expected &&
git cat-file blob :test.r >actual &&
- cmp expected actual
+ test_cmp expected actual
'
test_expect_success 'required filter smudge failure' '
- git config filter.failsmudge.smudge false &&
- git config filter.failsmudge.clean cat &&
- git config filter.failsmudge.required true &&
+ test_config filter.failsmudge.smudge false &&
+ test_config filter.failsmudge.clean cat &&
+ test_config filter.failsmudge.required true &&
echo "*.fs filter=failsmudge" >.gitattributes &&
@@ -186,9 +186,9 @@ test_expect_success 'required filter smudge failure' '
'
test_expect_success 'required filter clean failure' '
- git config filter.failclean.smudge cat &&
- git config filter.failclean.clean false &&
- git config filter.failclean.required true &&
+ test_config filter.failclean.smudge cat &&
+ test_config filter.failclean.clean false &&
+ test_config filter.failclean.required true &&
echo "*.fc filter=failclean" >.gitattributes &&
@@ -197,8 +197,8 @@ test_expect_success 'required filter clean failure' '
'
test_expect_success 'filtering large input to small output should use little memory' '
- git config filter.devnull.clean "cat >/dev/null" &&
- git config filter.devnull.required true &&
+ test_config filter.devnull.clean "cat >/dev/null" &&
+ test_config filter.devnull.required true &&
for i in $(test_seq 1 30); do printf "%1048576d" 1; done >30MB &&
echo "30MB filter=devnull" >.gitattributes &&
GIT_MMAP_LIMIT=1m GIT_ALLOC_LIMIT=1m git add 30MB
@@ -207,7 +207,7 @@ test_expect_success 'filtering large input to small output should use little mem
test_expect_success 'filter that does not read is fine' '
test-genrandom foo $((128 * 1024 + 1)) >big &&
echo "big filter=epipe" >.gitattributes &&
- git config filter.epipe.clean "echo xyzzy" &&
+ test_config filter.epipe.clean "echo xyzzy" &&
git add big &&
git cat-file blob :big >actual &&
echo xyzzy >expect &&
@@ -215,20 +215,20 @@ test_expect_success 'filter that does not read is fine' '
'
test_expect_success EXPENSIVE 'filter large file' '
- git config filter.largefile.smudge cat &&
- git config filter.largefile.clean cat &&
+ test_config filter.largefile.smudge cat &&
+ test_config filter.largefile.clean cat &&
for i in $(test_seq 1 2048); do printf "%1048576d" 1; done >2GB &&
echo "2GB filter=largefile" >.gitattributes &&
git add 2GB 2>err &&
- ! test -s err &&
+ test_must_be_empty err &&
rm -f 2GB &&
git checkout -- 2GB 2>err &&
- ! test -s err
+ test_must_be_empty err
'
test_expect_success "filter: clean empty file" '
- git config filter.in-repo-header.clean "echo cleaned && cat" &&
- git config filter.in-repo-header.smudge "sed 1d" &&
+ test_config filter.in-repo-header.clean "echo cleaned && cat" &&
+ test_config filter.in-repo-header.smudge "sed 1d" &&
echo "empty-in-worktree filter=in-repo-header" >>.gitattributes &&
>empty-in-worktree &&
@@ -240,8 +240,8 @@ test_expect_success "filter: clean empty file" '
'
test_expect_success "filter: smudge empty file" '
- git config filter.empty-in-repo.clean "cat >/dev/null" &&
- git config filter.empty-in-repo.smudge "echo smudged && cat" &&
+ test_config filter.empty-in-repo.clean "cat >/dev/null" &&
+ test_config filter.empty-in-repo.smudge "echo smudged && cat" &&
echo "empty-in-repo filter=empty-in-repo" >>.gitattributes &&
echo dead data walking >empty-in-repo &&
--
2.10.0
^ permalink raw reply related
* [PATCH v8 06/11] pkt-line: add packet_write_gently()
From: larsxschneider @ 2016-09-20 19:02 UTC (permalink / raw)
To: git
Cc: peff, gitster, sbeller, jnareb, mlbright, tboegi, ramsay,
Lars Schneider
In-Reply-To: <20160920190247.82189-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
packet_write_fmt_gently() uses format_packet() which lets the caller
only send string data via "%s". That means it cannot be used for
arbitrary data that may contain NULs.
Add packet_write_gently() which writes arbitrary data and does not die
in case of an error. The function is used by other pkt-line functions in
a subsequent patch.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
pkt-line.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/pkt-line.c b/pkt-line.c
index 19f0271..fc0ac12 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -171,6 +171,22 @@ int packet_write_fmt_gently(int fd, const char *fmt, ...)
return status;
}
+static int packet_write_gently(const int fd_out, const char *buf, size_t size)
+{
+ static char packet_write_buffer[LARGE_PACKET_MAX];
+
+ if (size > sizeof(packet_write_buffer) - 4) {
+ return error("packet write failed - data exceeds max packet size");
+ }
+ packet_trace(buf, size, 1);
+ size += 4;
+ set_packet_header(packet_write_buffer, size);
+ memcpy(packet_write_buffer + 4, buf, size - 4);
+ if (write_in_full(fd_out, packet_write_buffer, size) == size)
+ return 0;
+ return error("packet write failed");
+}
+
void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
{
va_list args;
--
2.10.0
^ permalink raw reply related
* [PATCH v8 10/11] convert: make apply_filter() adhere to standard Git error handling
From: larsxschneider @ 2016-09-20 19:02 UTC (permalink / raw)
To: git
Cc: peff, gitster, sbeller, jnareb, mlbright, tboegi, ramsay,
Lars Schneider
In-Reply-To: <20160920190247.82189-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
apply_filter() returns a boolean that tells the caller if it
"did convert or did not convert". The variable `ret` was used throughout
the function to track errors whereas `1` denoted success and `0`
failure. This is unusual for the Git source where `0` denotes success.
Rename the variable and flip its value to make the function easier
readable for Git developers.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
convert.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/convert.c b/convert.c
index 986c239..597f561 100644
--- a/convert.c
+++ b/convert.c
@@ -451,7 +451,7 @@ static int apply_filter(const char *path, const char *src, size_t len, int fd,
*
* (child --> cmd) --> us
*/
- int ret = 1;
+ int err = 0;
struct strbuf nbuf = STRBUF_INIT;
struct async async;
struct filter_params params;
@@ -477,23 +477,20 @@ static int apply_filter(const char *path, const char *src, size_t len, int fd,
return 0; /* error was already reported */
if (strbuf_read(&nbuf, async.out, len) < 0) {
- error("read from external filter '%s' failed", cmd);
- ret = 0;
+ err = error("read from external filter '%s' failed", cmd);
}
if (close(async.out)) {
- error("read from external filter '%s' failed", cmd);
- ret = 0;
+ err = error("read from external filter '%s' failed", cmd);
}
if (finish_async(&async)) {
- error("external filter '%s' failed", cmd);
- ret = 0;
+ err = error("external filter '%s' failed", cmd);
}
- if (ret) {
+ if (!err) {
strbuf_swap(dst, &nbuf);
}
strbuf_release(&nbuf);
- return ret;
+ return !err;
}
static struct convert_driver {
--
2.10.0
^ permalink raw reply related
* [PATCH v8 04/11] pkt-line: add packet_write_fmt_gently()
From: larsxschneider @ 2016-09-20 19:02 UTC (permalink / raw)
To: git
Cc: peff, gitster, sbeller, jnareb, mlbright, tboegi, ramsay,
Lars Schneider
In-Reply-To: <20160920190247.82189-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
packet_write_fmt() would die in case of a write error even though for
some callers an error would be acceptable. Add packet_write_fmt_gently()
which writes a formatted pkt-line like packet_write_fmt() but does not
die in case of an error. The function is used in a subsequent patch.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
pkt-line.c | 34 ++++++++++++++++++++++++++++++----
pkt-line.h | 1 +
2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/pkt-line.c b/pkt-line.c
index e8adc0f..3b465fd 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -125,16 +125,42 @@ static void format_packet(struct strbuf *out, const char *fmt, va_list args)
packet_trace(out->buf + orig_len + 4, n - 4, 1);
}
+static int packet_write_fmt_1(int fd, int gently,
+ const char *fmt, va_list args)
+{
+ struct strbuf buf = STRBUF_INIT;
+ size_t count;
+
+ format_packet(&buf, fmt, args);
+ count = write_in_full(fd, buf.buf, buf.len);
+ if (count == buf.len)
+ return 0;
+
+ if (!gently) {
+ check_pipe(errno);
+ die_errno("packet write with format failed");
+ }
+ return error("packet write with format failed");
+}
+
void packet_write_fmt(int fd, const char *fmt, ...)
{
- static struct strbuf buf = STRBUF_INIT;
va_list args;
- strbuf_reset(&buf);
va_start(args, fmt);
- format_packet(&buf, fmt, args);
+ packet_write_fmt_1(fd, 0, fmt, args);
+ va_end(args);
+}
+
+int packet_write_fmt_gently(int fd, const char *fmt, ...)
+{
+ int status;
+ va_list args;
+
+ va_start(args, fmt);
+ status = packet_write_fmt_1(fd, 1, fmt, args);
va_end(args);
- write_or_die(fd, buf.buf, buf.len);
+ return status;
}
void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
diff --git a/pkt-line.h b/pkt-line.h
index 1902fb3..3caea77 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -23,6 +23,7 @@ void packet_flush(int fd);
void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
void packet_buf_flush(struct strbuf *buf);
void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
/*
* Read a packetized line into the buffer, which must be at least size bytes
--
2.10.0
^ permalink raw reply related
* [PATCH v8 05/11] pkt-line: add packet_flush_gently()
From: larsxschneider @ 2016-09-20 19:02 UTC (permalink / raw)
To: git
Cc: peff, gitster, sbeller, jnareb, mlbright, tboegi, ramsay,
Lars Schneider
In-Reply-To: <20160920190247.82189-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
packet_flush() would die in case of a write error even though for some
callers an error would be acceptable. Add packet_flush_gently() which
writes a pkt-line flush packet like packet_flush() but does not die in
case of an error. The function is used in a subsequent patch.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
pkt-line.c | 8 ++++++++
pkt-line.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/pkt-line.c b/pkt-line.c
index 3b465fd..19f0271 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -91,6 +91,14 @@ void packet_flush(int fd)
write_or_die(fd, "0000", 4);
}
+int packet_flush_gently(int fd)
+{
+ packet_trace("0000", 4, 1);
+ if (write_in_full(fd, "0000", 4) == 4)
+ return 0;
+ return error("flush packet write failed");
+}
+
void packet_buf_flush(struct strbuf *buf)
{
packet_trace("0000", 4, 1);
diff --git a/pkt-line.h b/pkt-line.h
index 3caea77..3fa0899 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -23,6 +23,7 @@ void packet_flush(int fd);
void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
void packet_buf_flush(struct strbuf *buf);
void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+int packet_flush_gently(int fd);
int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
/*
--
2.10.0
^ permalink raw reply related
* [PATCH v8 02/11] pkt-line: extract set_packet_header()
From: larsxschneider @ 2016-09-20 19:02 UTC (permalink / raw)
To: git
Cc: peff, gitster, sbeller, jnareb, mlbright, tboegi, ramsay,
Lars Schneider
In-Reply-To: <20160920190247.82189-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
set_packet_header() converts an integer to a 4 byte hex string. Make
this function locally available so that other pkt-line functions can
use it.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
pkt-line.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/pkt-line.c b/pkt-line.c
index 0a9b61c..e8adc0f 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -97,10 +97,20 @@ void packet_buf_flush(struct strbuf *buf)
strbuf_add(buf, "0000", 4);
}
-#define hex(a) (hexchar[(a) & 15])
-static void format_packet(struct strbuf *out, const char *fmt, va_list args)
+static void set_packet_header(char *buf, const int size)
{
static char hexchar[] = "0123456789abcdef";
+
+ #define hex(a) (hexchar[(a) & 15])
+ buf[0] = hex(size >> 12);
+ buf[1] = hex(size >> 8);
+ buf[2] = hex(size >> 4);
+ buf[3] = hex(size);
+ #undef hex
+}
+
+static void format_packet(struct strbuf *out, const char *fmt, va_list args)
+{
size_t orig_len, n;
orig_len = out->len;
@@ -111,10 +121,7 @@ static void format_packet(struct strbuf *out, const char *fmt, va_list args)
if (n > LARGE_PACKET_MAX)
die("protocol error: impossibly long line");
- out->buf[orig_len + 0] = hex(n >> 12);
- out->buf[orig_len + 1] = hex(n >> 8);
- out->buf[orig_len + 2] = hex(n >> 4);
- out->buf[orig_len + 3] = hex(n);
+ set_packet_header(&out->buf[orig_len], n);
packet_trace(out->buf + orig_len + 4, n - 4, 1);
}
--
2.10.0
^ permalink raw reply related
* [PATCH v8 03/11] run-command: move check_pipe() from write_or_die to run_command
From: larsxschneider @ 2016-09-20 19:02 UTC (permalink / raw)
To: git
Cc: peff, gitster, sbeller, jnareb, mlbright, tboegi, ramsay,
Lars Schneider
In-Reply-To: <20160920190247.82189-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
Move check_pipe() to run_command and make it public. This is necessary
to call the function from pkt-line in a subsequent patch.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
run-command.c | 13 +++++++++++++
run-command.h | 2 ++
write_or_die.c | 13 -------------
3 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/run-command.c b/run-command.c
index 5a4dbb6..b72f6d1 100644
--- a/run-command.c
+++ b/run-command.c
@@ -6,6 +6,19 @@
#include "thread-utils.h"
#include "strbuf.h"
+void check_pipe(int err)
+{
+ if (err == EPIPE) {
+ if (in_async())
+ async_exit(141);
+
+ signal(SIGPIPE, SIG_DFL);
+ raise(SIGPIPE);
+ /* Should never happen, but just in case... */
+ exit(141);
+ }
+}
+
void child_process_init(struct child_process *child)
{
memset(child, 0, sizeof(*child));
diff --git a/run-command.h b/run-command.h
index 5066649..e7c5f71 100644
--- a/run-command.h
+++ b/run-command.h
@@ -54,6 +54,8 @@ int finish_command(struct child_process *);
int finish_command_in_signal(struct child_process *);
int run_command(struct child_process *);
+void check_pipe(int err);
+
/*
* Returns the path to the hook file, or NULL if the hook is missing
* or disabled. Note that this points to static storage that will be
diff --git a/write_or_die.c b/write_or_die.c
index 0734432..eab8c8d 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -1,19 +1,6 @@
#include "cache.h"
#include "run-command.h"
-static void check_pipe(int err)
-{
- if (err == EPIPE) {
- if (in_async())
- async_exit(141);
-
- signal(SIGPIPE, SIG_DFL);
- raise(SIGPIPE);
- /* Should never happen, but just in case... */
- exit(141);
- }
-}
-
/*
* Some cases use stdio, but want to flush after the write
* to get error handling (and to get better interactive
--
2.10.0
^ permalink raw reply related
* [PATCH v8 01/11] pkt-line: rename packet_write() to packet_write_fmt()
From: larsxschneider @ 2016-09-20 19:02 UTC (permalink / raw)
To: git
Cc: peff, gitster, sbeller, jnareb, mlbright, tboegi, ramsay,
Lars Schneider
In-Reply-To: <20160920190247.82189-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
packet_write() should be called packet_write_fmt() as the string
parameter can be formatted.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
builtin/archive.c | 4 ++--
builtin/receive-pack.c | 4 ++--
builtin/remote-ext.c | 4 ++--
builtin/upload-archive.c | 4 ++--
connect.c | 2 +-
daemon.c | 2 +-
http-backend.c | 2 +-
pkt-line.c | 2 +-
pkt-line.h | 2 +-
shallow.c | 2 +-
upload-pack.c | 30 +++++++++++++++---------------
11 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/builtin/archive.c b/builtin/archive.c
index a1e3b94..49f4914 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -47,10 +47,10 @@ static int run_remote_archiver(int argc, const char **argv,
if (name_hint) {
const char *format = archive_format_from_filename(name_hint);
if (format)
- packet_write(fd[1], "argument --format=%s\n", format);
+ packet_write_fmt(fd[1], "argument --format=%s\n", format);
}
for (i = 1; i < argc; i++)
- packet_write(fd[1], "argument %s\n", argv[i]);
+ packet_write_fmt(fd[1], "argument %s\n", argv[i]);
packet_flush(fd[1]);
buf = packet_read_line(fd[0], NULL);
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 011db00..1ce7682 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -218,7 +218,7 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
static void show_ref(const char *path, const unsigned char *sha1)
{
if (sent_capabilities) {
- packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
+ packet_write_fmt(1, "%s %s\n", sha1_to_hex(sha1), path);
} else {
struct strbuf cap = STRBUF_INIT;
@@ -233,7 +233,7 @@ static void show_ref(const char *path, const unsigned char *sha1)
if (advertise_push_options)
strbuf_addstr(&cap, " push-options");
strbuf_addf(&cap, " agent=%s", git_user_agent_sanitized());
- packet_write(1, "%s %s%c%s\n",
+ packet_write_fmt(1, "%s %s%c%s\n",
sha1_to_hex(sha1), path, 0, cap.buf);
strbuf_release(&cap);
sent_capabilities = 1;
diff --git a/builtin/remote-ext.c b/builtin/remote-ext.c
index 88eb8f9..11b48bf 100644
--- a/builtin/remote-ext.c
+++ b/builtin/remote-ext.c
@@ -128,9 +128,9 @@ static void send_git_request(int stdin_fd, const char *serv, const char *repo,
const char *vhost)
{
if (!vhost)
- packet_write(stdin_fd, "%s %s%c", serv, repo, 0);
+ packet_write_fmt(stdin_fd, "%s %s%c", serv, repo, 0);
else
- packet_write(stdin_fd, "%s %s%chost=%s%c", serv, repo, 0,
+ packet_write_fmt(stdin_fd, "%s %s%chost=%s%c", serv, repo, 0,
vhost, 0);
}
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index 2caedf1..dc872f6 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -88,11 +88,11 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
writer.git_cmd = 1;
if (start_command(&writer)) {
int err = errno;
- packet_write(1, "NACK unable to spawn subprocess\n");
+ packet_write_fmt(1, "NACK unable to spawn subprocess\n");
die("upload-archive: %s", strerror(err));
}
- packet_write(1, "ACK\n");
+ packet_write_fmt(1, "ACK\n");
packet_flush(1);
while (1) {
diff --git a/connect.c b/connect.c
index 722dc3f..5330d9c 100644
--- a/connect.c
+++ b/connect.c
@@ -730,7 +730,7 @@ struct child_process *git_connect(int fd[2], const char *url,
* Note: Do not add any other headers here! Doing so
* will cause older git-daemon servers to crash.
*/
- packet_write(fd[1],
+ packet_write_fmt(fd[1],
"%s %s%chost=%s%c",
prog, path, 0,
target_host, 0);
diff --git a/daemon.c b/daemon.c
index 425aad0..afce1b9 100644
--- a/daemon.c
+++ b/daemon.c
@@ -281,7 +281,7 @@ static int daemon_error(const char *dir, const char *msg)
{
if (!informative_errors)
msg = "access denied or repository not exported";
- packet_write(1, "ERR %s: %s", msg, dir);
+ packet_write_fmt(1, "ERR %s: %s", msg, dir);
return -1;
}
diff --git a/http-backend.c b/http-backend.c
index adc8c8c..eef0a36 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -464,7 +464,7 @@ static void get_info_refs(struct strbuf *hdr, char *arg)
hdr_str(hdr, content_type, buf.buf);
end_headers(hdr);
- packet_write(1, "# service=git-%s\n", svc->name);
+ packet_write_fmt(1, "# service=git-%s\n", svc->name);
packet_flush(1);
argv[0] = svc->name;
diff --git a/pkt-line.c b/pkt-line.c
index 62fdb37..0a9b61c 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -118,7 +118,7 @@ static void format_packet(struct strbuf *out, const char *fmt, va_list args)
packet_trace(out->buf + orig_len + 4, n - 4, 1);
}
-void packet_write(int fd, const char *fmt, ...)
+void packet_write_fmt(int fd, const char *fmt, ...)
{
static struct strbuf buf = STRBUF_INIT;
va_list args;
diff --git a/pkt-line.h b/pkt-line.h
index 3cb9d91..1902fb3 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -20,7 +20,7 @@
* side can't, we stay with pure read/write interfaces.
*/
void packet_flush(int fd);
-void packet_write(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
void packet_buf_flush(struct strbuf *buf);
void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
diff --git a/shallow.c b/shallow.c
index 54e2db7..d666e24 100644
--- a/shallow.c
+++ b/shallow.c
@@ -260,7 +260,7 @@ static int advertise_shallow_grafts_cb(const struct commit_graft *graft, void *c
{
int fd = *(int *)cb;
if (graft->nr_parent == -1)
- packet_write(fd, "shallow %s\n", oid_to_hex(&graft->oid));
+ packet_write_fmt(fd, "shallow %s\n", oid_to_hex(&graft->oid));
return 0;
}
diff --git a/upload-pack.c b/upload-pack.c
index ca7f941..cd47de6 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -393,13 +393,13 @@ static int get_common_commits(void)
if (multi_ack == 2 && got_common
&& !got_other && ok_to_give_up()) {
sent_ready = 1;
- packet_write(1, "ACK %s ready\n", last_hex);
+ packet_write_fmt(1, "ACK %s ready\n", last_hex);
}
if (have_obj.nr == 0 || multi_ack)
- packet_write(1, "NAK\n");
+ packet_write_fmt(1, "NAK\n");
if (no_done && sent_ready) {
- packet_write(1, "ACK %s\n", last_hex);
+ packet_write_fmt(1, "ACK %s\n", last_hex);
return 0;
}
if (stateless_rpc)
@@ -416,20 +416,20 @@ static int get_common_commits(void)
const char *hex = sha1_to_hex(sha1);
if (multi_ack == 2) {
sent_ready = 1;
- packet_write(1, "ACK %s ready\n", hex);
+ packet_write_fmt(1, "ACK %s ready\n", hex);
} else
- packet_write(1, "ACK %s continue\n", hex);
+ packet_write_fmt(1, "ACK %s continue\n", hex);
}
break;
default:
got_common = 1;
memcpy(last_hex, sha1_to_hex(sha1), 41);
if (multi_ack == 2)
- packet_write(1, "ACK %s common\n", last_hex);
+ packet_write_fmt(1, "ACK %s common\n", last_hex);
else if (multi_ack)
- packet_write(1, "ACK %s continue\n", last_hex);
+ packet_write_fmt(1, "ACK %s continue\n", last_hex);
else if (have_obj.nr == 1)
- packet_write(1, "ACK %s\n", last_hex);
+ packet_write_fmt(1, "ACK %s\n", last_hex);
break;
}
continue;
@@ -437,10 +437,10 @@ static int get_common_commits(void)
if (!strcmp(line, "done")) {
if (have_obj.nr > 0) {
if (multi_ack)
- packet_write(1, "ACK %s\n", last_hex);
+ packet_write_fmt(1, "ACK %s\n", last_hex);
return 0;
}
- packet_write(1, "NAK\n");
+ packet_write_fmt(1, "NAK\n");
return -1;
}
die("git upload-pack: expected SHA1 list, got '%s'", line);
@@ -650,7 +650,7 @@ static void receive_needs(void)
while (result) {
struct object *object = &result->item->object;
if (!(object->flags & (CLIENT_SHALLOW|NOT_SHALLOW))) {
- packet_write(1, "shallow %s",
+ packet_write_fmt(1, "shallow %s",
oid_to_hex(&object->oid));
register_shallow(object->oid.hash);
shallow_nr++;
@@ -662,7 +662,7 @@ static void receive_needs(void)
struct object *object = shallows.objects[i].item;
if (object->flags & NOT_SHALLOW) {
struct commit_list *parents;
- packet_write(1, "unshallow %s",
+ packet_write_fmt(1, "unshallow %s",
oid_to_hex(&object->oid));
object->flags &= ~CLIENT_SHALLOW;
/* make sure the real parents are parsed */
@@ -741,7 +741,7 @@ static int send_ref(const char *refname, const struct object_id *oid,
struct strbuf symref_info = STRBUF_INIT;
format_symref_info(&symref_info, cb_data);
- packet_write(1, "%s %s%c%s%s%s%s%s agent=%s\n",
+ packet_write_fmt(1, "%s %s%c%s%s%s%s%s agent=%s\n",
oid_to_hex(oid), refname_nons,
0, capabilities,
(allow_unadvertised_object_request & ALLOW_TIP_SHA1) ?
@@ -753,11 +753,11 @@ static int send_ref(const char *refname, const struct object_id *oid,
git_user_agent_sanitized());
strbuf_release(&symref_info);
} else {
- packet_write(1, "%s %s\n", oid_to_hex(oid), refname_nons);
+ packet_write_fmt(1, "%s %s\n", oid_to_hex(oid), refname_nons);
}
capabilities = NULL;
if (!peel_ref(refname, peeled.hash))
- packet_write(1, "%s %s^{}\n", oid_to_hex(&peeled), refname_nons);
+ packet_write_fmt(1, "%s %s^{}\n", oid_to_hex(&peeled), refname_nons);
return 0;
}
--
2.10.0
^ permalink raw reply related
* [PATCH v8 00/11] Git filter protocol
From: larsxschneider @ 2016-09-20 19:02 UTC (permalink / raw)
To: git
Cc: peff, gitster, sbeller, jnareb, mlbright, tboegi, ramsay,
Lars Schneider
From: Lars Schneider <larsxschneider@gmail.com>
The goal of this series is to avoid launching a new clean/smudge filter
process for each file that is filtered.
A short summary about v1 to v5 can be found here:
https://git.github.io/rev_news/2016/08/17/edition-18/
This series is also published on web:
https://github.com/larsxschneider/git/pull/12
Thanks a lot to
Stefan, Torsten, Junio, Jeff, and Ramsay
for very helpful reviews,
Lars
## Major changes since v7
* explicitly define all packets as text packets terminated by an LF (except CONTENT and flush)
* move check_pipe() from write_or_die to run_command and reuse it
## All changes since v7
### Stefan
* http://public-inbox.org/git/CAGZ79kY0GaWuuh_MzKL6FZ7KWF2Kwhfh9qnEYd-qX8VDQWNmCQ@mail.gmail.com/
* move check_pipe() from write_or_die to run_command and reuse it
* use error() (== -1) as return value
* http://public-inbox.org/git/CAGZ79kZdroDdD5SHP+-9svSTYbJfn2vsFXAwC4aen3hMVEOOPA@mail.gmail.com/
* remove verbose return value explanation in commit messages
* on "packet_flush_gently" introduction, mention that the function is used later
### Torsten
* http://public-inbox.org/git/20160910164056.GA14646@tb-raspi/
* remove unnecessary parenthesis
* http://public-inbox.org/git/20160910062919.GB11001@tb-raspi/
* explicitly define all packets as text packets terminated by an LF (except CONTENT and flush)
### Junio
* http://public-inbox.org/git/xmqq8tuvx1sz.fsf@gitster.mtv.corp.google.com/
* fix SP in Perl script
* use `unsigned int` for CAP_CLEAN and CAP_SMUDGE
* fix pointer notation
* remove invalid "convert.h" include
### Ramsay
* http://public-inbox.org/git/6373d68b-574d-59f3-7b8d-60dd3a673806@ramsayjones.plus.com
* declare packet_write_gently() static
### Lars
* add SP in paths for test case
* fix "{" code formatting
## Interdiff (v7..v8)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index ac000ea..946dcad 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -385,9 +385,11 @@ Long Running Filter Process
If the filter command (a string value) is defined via
`filter.<driver>.process` then Git can process all blobs with a
single filter invocation for the entire life of a single Git
-command. This is achieved by using the following packet format
-(pkt-line, see technical/protocol-common.txt) based protocol over
-standard input and standard output.
+command. This is achieved by using a packet format (pkt-line,
+see technical/protocol-common.txt) based protocol over standard
+input and standard output as follows. All packets are considered
+text and therefore are terminated by an LF. Exceptions are the
+"*CONTENT" packets and the flush packet.
Git starts the filter when it encounters the first file
that needs to be cleaned or smudged. After the filter started
@@ -430,8 +432,8 @@ to filter relative to the repository root. Right after these packets
Git sends the content split in zero or more pkt-line packets and a
flush packet to terminate content.
------------------------
-packet: git> command=smudge\n
-packet: git> pathname=path/testfile.dat\n
+packet: git> command=smudge
+packet: git> pathname=path/testfile.dat
packet: git> 0000
packet: git> CONTENT
packet: git> 0000
@@ -445,7 +447,7 @@ or more pkt-line packets and a flush packet at the end. Finally, a
second list of "key=value" pairs terminated with a flush packet
is expected. The filter can change the status in the second list.
------------------------
-packet: git< status=success\n
+packet: git< status=success
packet: git< 0000
packet: git< SMUDGED_CONTENT
packet: git< 0000
@@ -455,7 +457,7 @@ packet: git< 0000 # empty list!
If the result content is empty then the filter is expected to respond
with a success status and an empty list.
------------------------
-packet: git< status=success\n
+packet: git< status=success
packet: git< 0000
packet: git< 0000 # empty content!
packet: git< 0000 # empty list!
@@ -466,7 +468,7 @@ it is expected to respond with an "error" status. Depending on the
`filter.<driver>.required` flag Git will interpret that as error
but it will not stop or restart the filter process.
------------------------
-packet: git< status=error\n
+packet: git< status=error
packet: git< 0000
------------------------
@@ -476,11 +478,11 @@ completely) sent. Depending on the `filter.<driver>.required` flag
Git will interpret that as error but it will not stop or restart the
filter process.
------------------------
-packet: git< status=success\n
+packet: git< status=success
packet: git< 0000
packet: git< HALF_WRITTEN_ERRONEOUS_CONTENT
packet: git< 0000
-packet: git< status=error\n
+packet: git< status=error
packet: git< 0000
------------------------
@@ -500,7 +502,7 @@ the `filter.<driver>.required` flag Git will interpret that as error
for the content as well as any future content for the lifetime of the
Git process but it will not stop or restart the filter process.
------------------------
-packet: git< status=abort\n
+packet: git< status=abort
packet: git< 0000
------------------------
@@ -510,8 +512,8 @@ the command pipe on exit. The filter is expected to detect EOF
and exit gracefully on its own.
A long running filter demo implementation can be found in
-`contrib/long-running-filter/example.pl` located in the Git
-core repository. If you develop your own long running filter
+`contrib/long-running-filter/example.pl` located in the Git
+core repository. If you develop your own long running filter
process then the `GIT_TRACE_PACKET` environment variables can be
very helpful for debugging (see linkgit:git[1]).
diff --git a/contrib/long-running-filter/example.pl b/contrib/long-running-filter/example.pl
index 279fbfb..c13a631 100755
--- a/contrib/long-running-filter/example.pl
+++ b/contrib/long-running-filter/example.pl
@@ -9,7 +9,7 @@ use warnings;
my $MAX_PACKET_CONTENT_SIZE = 65516;
-sub packet_read {
+sub packet_bin_read {
my $buffer;
my $bytes_read = read STDIN, $buffer, 4;
if ( $bytes_read == 0 ) {
@@ -37,38 +37,50 @@ sub packet_read {
}
}
-sub packet_write {
+sub packet_txt_read {
+ my ( $res, $buf ) = packet_bin_read();
+ unless ( $buf =~ /\n$/ ) {
+ die "A non-binary line SHOULD BE terminated by an LF.";
+ }
+ return ( $res, substr( $buf, 0, -1 ) );
+}
+
+sub packet_bin_write {
my ($packet) = @_;
print STDOUT sprintf( "%04x", length($packet) + 4 );
print STDOUT $packet;
STDOUT->flush();
}
+sub packet_txt_write {
+ packet_bin_write( $_[0] . "\n" );
+}
+
sub packet_flush {
print STDOUT sprintf( "%04x", 0 );
STDOUT->flush();
}
-( packet_read() eq ( 0, "git-filter-client" ) ) || die "bad initialization";
-( packet_read() eq ( 0, "version=2" ) ) || die "bad version";
-( packet_read() eq ( 1, "" ) ) || die "bad version end";
+( packet_txt_read() eq ( 0, "git-filter-client" ) ) || die "bad initialize";
+( packet_txt_read() eq ( 0, "version=2" ) ) || die "bad version";
+( packet_bin_read() eq ( 1, "" ) ) || die "bad version end";
-packet_write("git-filter-server\n");
-packet_write("version=2\n");
+packet_txt_write("git-filter-server");
+packet_txt_write("version=2");
-( packet_read() eq ( 0, "clean=true" ) ) || die "bad capability";
-( packet_read() eq ( 0, "smudge=true" ) ) || die "bad capability";
-( packet_read() eq ( 1, "" ) ) || die "bad capability end";
+( packet_txt_read() eq ( 0, "clean=true" ) ) || die "bad capability";
+( packet_txt_read() eq ( 0, "smudge=true" ) ) || die "bad capability";
+( packet_bin_read() eq ( 1, "" ) ) || die "bad capability end";
-packet_write( "clean=true\n" );
-packet_write( "smudge=true\n" );
+packet_txt_write("clean=true");
+packet_txt_write("smudge=true");
packet_flush();
while (1) {
- my ($command) = packet_read() =~ /^command=([^=]+)\n$/;
- my ($pathname) = packet_read() =~ /^pathname=([^=]+)\n$/;
+ my ($command) = packet_txt_read() =~ /^command=([^=]+)$/;
+ my ($pathname) = packet_txt_read() =~ /^pathname=([^=]+)$/;
- packet_read();
+ packet_bin_read();
my $input = "";
{
@@ -76,7 +88,7 @@ while (1) {
my $buffer;
my $done = 0;
while ( !$done ) {
- ( $done, $buffer ) = packet_read();
+ ( $done, $buffer ) = packet_bin_read();
$input .= $buffer;
}
}
@@ -94,11 +106,11 @@ while (1) {
die "bad command '$command'";
}
- packet_write("status=success\n");
+ packet_txt_write("status=success");
packet_flush();
while ( length($output) > 0 ) {
my $packet = substr( $output, 0, $MAX_PACKET_CONTENT_SIZE );
- packet_write($packet);
+ packet_bin_write($packet);
if ( length($output) > $MAX_PACKET_CONTENT_SIZE ) {
$output = substr( $output, $MAX_PACKET_CONTENT_SIZE );
}
@@ -106,6 +118,6 @@ while (1) {
$output = "";
}
}
- packet_flush(); # flush content!
- packet_flush(); # empty list!
+ packet_flush(); # flush content!
+ packet_flush(); # empty list!
}
diff --git a/convert.c b/convert.c
index 0ed48ed..bd66257 100644
--- a/convert.c
+++ b/convert.c
@@ -472,16 +472,13 @@ static int apply_single_file_filter(const char *path, const char *src, size_t le
return 0; /* error was already reported */
if (strbuf_read(&nbuf, async.out, len) < 0) {
- error("read from external filter '%s' failed", cmd);
- err = -1;
+ err = error("read from external filter '%s' failed", cmd);
}
if (close(async.out)) {
- error("read from external filter '%s' failed", cmd);
- err = -1;
+ err = error("read from external filter '%s' failed", cmd);
}
if (finish_async(&async)) {
- error("external filter '%s' failed", cmd);
- err = -1;
+ err = error("external filter '%s' failed", cmd);
}
if (!err) {
@@ -496,7 +493,7 @@ static int apply_single_file_filter(const char *path, const char *src, size_t le
struct cmd2process {
struct hashmap_entry ent; /* must be the first member! */
- int supported_capabilities;
+ unsigned int supported_capabilities;
const char *cmd;
struct child_process process;
};
@@ -541,13 +538,12 @@ static int packet_write_list(int fd, const char *line, ...)
va_list args;
int err;
va_start(args, line);
- for (;;)
- {
+ for (;;) {
if (!line)
break;
if (strlen(line) > PKTLINE_DATA_MAXLEN)
return -1;
- err = packet_write_fmt_gently(fd, "%s", line);
+ err = packet_write_fmt_gently(fd, "%s\n", line);
if (err)
return err;
line = va_arg(args, const char*);
@@ -601,8 +597,7 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons
err = packet_write_list(process->in, "clean=true", "smudge=true", NULL);
- for (;;)
- {
+ for (;;) {
cap_buf = packet_read_line(process->out, NULL);
if (!cap_buf)
break;
@@ -658,7 +653,7 @@ static void read_multi_file_filter_values(int fd, struct strbuf *status) {
static int apply_multi_file_filter(const char *path, const char *src, size_t len,
int fd, struct strbuf *dst, const char *cmd,
- const int wanted_capability)
+ const unsigned int wanted_capability)
{
int err;
struct cmd2process *entry;
@@ -703,17 +698,18 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len
sigchain_push(SIGPIPE, SIG_IGN);
-
- err = (strlen(filter_type) > PKTLINE_DATA_MAXLEN);
+ err = strlen(filter_type) > PKTLINE_DATA_MAXLEN;
if (err)
goto done;
+
err = packet_write_fmt_gently(process->in, "command=%s\n", filter_type);
if (err)
goto done;
- err = (strlen(path) > PKTLINE_DATA_MAXLEN);
+ err = strlen(path) > PKTLINE_DATA_MAXLEN;
if (err)
goto done;
+
err = packet_write_fmt_gently(process->in, "pathname=%s\n", path);
if (err)
goto done;
@@ -780,9 +776,9 @@ static struct convert_driver {
static int apply_filter(const char *path, const char *src, size_t len,
int fd, struct strbuf *dst, struct convert_driver *drv,
- const int wanted_capability)
+ const unsigned int wanted_capability)
{
- const char* cmd = NULL;
+ const char *cmd = NULL;
if (!drv)
return 0;
diff --git a/pkt-line.c b/pkt-line.c
index 5001a07..a0a8543 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -96,8 +96,7 @@ int packet_flush_gently(int fd)
packet_trace("0000", 4, 1);
if (write_in_full(fd, "0000", 4) == 4)
return 0;
- error("flush packet write failed");
- return -1;
+ return error("flush packet write failed");
}
void packet_buf_flush(struct strbuf *buf)
@@ -146,19 +145,10 @@ static int packet_write_fmt_1(int fd, int gently,
return 0;
if (!gently) {
- if (errno == EPIPE) {
- if (in_async())
- async_exit(141);
-
- signal(SIGPIPE, SIG_DFL);
- raise(SIGPIPE);
- /* Should never happen, but just in case... */
- exit(141);
- }
- die_errno("packet write error");
+ check_pipe(errno);
+ die_errno("packet write with format failed");
}
- error("packet write failed");
- return -1;
+ return error("packet write with format failed");
}
void packet_write_fmt(int fd, const char *fmt, ...)
@@ -181,13 +171,12 @@ int packet_write_fmt_gently(int fd, const char *fmt, ...)
return status;
}
-int packet_write_gently(const int fd_out, const char *buf, size_t size)
+static int packet_write_gently(const int fd_out, const char *buf, size_t size)
{
static char packet_write_buffer[LARGE_PACKET_MAX];
if (size > sizeof(packet_write_buffer) - 4) {
- error("packet write failed");
- return -1;
+ return error("packet write failed - data exceeds max packet size");
}
packet_trace(buf, size, 1);
size += 4;
@@ -195,9 +184,7 @@ int packet_write_gently(const int fd_out, const char *buf, size_t size)
memcpy(packet_write_buffer + 4, buf, size - 4);
if (write_in_full(fd_out, packet_write_buffer, size) == size)
return 0;
-
- error("packet write failed");
- return -1;
+ return error("packet write failed");
}
void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
diff --git a/run-command.c b/run-command.c
index 5a4dbb6..b72f6d1 100644
--- a/run-command.c
+++ b/run-command.c
@@ -6,6 +6,19 @@
#include "thread-utils.h"
#include "strbuf.h"
+void check_pipe(int err)
+{
+ if (err == EPIPE) {
+ if (in_async())
+ async_exit(141);
+
+ signal(SIGPIPE, SIG_DFL);
+ raise(SIGPIPE);
+ /* Should never happen, but just in case... */
+ exit(141);
+ }
+}
+
void child_process_init(struct child_process *child)
{
memset(child, 0, sizeof(*child));
diff --git a/run-command.h b/run-command.h
index 5066649..e7c5f71 100644
--- a/run-command.h
+++ b/run-command.h
@@ -54,6 +54,8 @@ int finish_command(struct child_process *);
int finish_command_in_signal(struct child_process *);
int run_command(struct child_process *);
+void check_pipe(int err);
+
/*
* Returns the path to the hook file, or NULL if the hook is missing
* or disabled. Note that this points to static storage that will be
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 1c98ac3..210c4f6 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -34,7 +34,7 @@ test_expect_success setup '
git checkout -- test test.t test.i &&
echo "content-test2" >test2.o &&
- echo "content-test3-subdir" >test3-subdir.o
+ echo "content-test3 - subdir" >"test3 - subdir.o"
'
script='s/^\$Id: \([0-9a-f]*\) \$/\1/p'
@@ -317,9 +317,9 @@ check_filter_no_call () {
}
check_rot13 () {
- test_cmp $1 $2 &&
- ./../rot13.sh <$1 >expected &&
- git cat-file blob :$2 >actual &&
+ test_cmp "$1" "$2" &&
+ ./../rot13.sh <"$1" >expected &&
+ git cat-file blob :"$2" >actual &&
test_cmp expected actual
}
@@ -340,7 +340,7 @@ test_expect_success PERL 'required process filter should filter data' '
cp ../test.o test.r &&
cp ../test2.o test2.r &&
mkdir testsubdir &&
- cp ../test3-subdir.o testsubdir/test3-subdir.r &&
+ cp "../test3 - subdir.o" "testsubdir/test3 - subdir.r" &&
>test4-empty.r &&
check_filter \
@@ -349,7 +349,7 @@ test_expect_success PERL 'required process filter should filter data' '
1 IN: clean test.r 57 [OK] -- OUT: 57 . [OK]
1 IN: clean test2.r 14 [OK] -- OUT: 14 . [OK]
1 IN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]
- 1 IN: clean testsubdir/test3-subdir.r 21 [OK] -- OUT: 21 . [OK]
+ 1 IN: clean testsubdir/test3 - subdir.r 23 [OK] -- OUT: 23 . [OK]
1 START
1 STOP
1 wrote filter header
@@ -361,13 +361,13 @@ test_expect_success PERL 'required process filter should filter data' '
x IN: clean test.r 57 [OK] -- OUT: 57 . [OK]
x IN: clean test2.r 14 [OK] -- OUT: 14 . [OK]
x IN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]
- x IN: clean testsubdir/test3-subdir.r 21 [OK] -- OUT: 21 . [OK]
+ x IN: clean testsubdir/test3 - subdir.r 23 [OK] -- OUT: 23 . [OK]
1 START
1 STOP
1 wrote filter header
EOF
- rm -f test?.r testsubdir/test3-subdir.r &&
+ rm -f test?.r "testsubdir/test3 - subdir.r" &&
check_filter_ignore_clean \
git checkout . \
@@ -375,7 +375,7 @@ test_expect_success PERL 'required process filter should filter data' '
START
wrote filter header
IN: smudge test2.r 14 [OK] -- OUT: 14 . [OK]
- IN: smudge testsubdir/test3-subdir.r 21 [OK] -- OUT: 21 . [OK]
+ IN: smudge testsubdir/test3 - subdir.r 23 [OK] -- OUT: 23 . [OK]
STOP
EOF
@@ -395,13 +395,13 @@ test_expect_success PERL 'required process filter should filter data' '
IN: smudge test.r 57 [OK] -- OUT: 57 . [OK]
IN: smudge test2.r 14 [OK] -- OUT: 14 . [OK]
IN: smudge test4-empty.r 0 [OK] -- OUT: 0 [OK]
- IN: smudge testsubdir/test3-subdir.r 21 [OK] -- OUT: 21 . [OK]
+ IN: smudge testsubdir/test3 - subdir.r 23 [OK] -- OUT: 23 . [OK]
STOP
EOF
check_rot13 ../test.o test.r &&
check_rot13 ../test2.o test2.r &&
- check_rot13 ../test3-subdir.o testsubdir/test3-subdir.r
+ check_rot13 "../test3 - subdir.o" "testsubdir/test3 - subdir.r"
)
'
diff --git a/t/t0021/rot13-filter.pl b/t/t0021/rot13-filter.pl
index 8e27877..8958f71 100755
--- a/t/t0021/rot13-filter.pl
+++ b/t/t0021/rot13-filter.pl
@@ -34,7 +34,7 @@ sub rot13 {
return $str;
}
-sub packet_read {
+sub packet_bin_read {
my $buffer;
my $bytes_read = read STDIN, $buffer, 4;
if ( $bytes_read == 0 ) {
@@ -63,13 +63,25 @@ sub packet_read {
}
}
-sub packet_write {
+sub packet_txt_read {
+ my ( $res, $buf ) = packet_bin_read();
+ unless ( $buf =~ /\n$/ ) {
+ die "A non-binary line SHOULD BE terminated by an LF.";
+ }
+ return ( $res, substr( $buf, 0, -1 ) );
+}
+
+sub packet_bin_write {
my ($packet) = @_;
print STDOUT sprintf( "%04x", length($packet) + 4 );
print STDOUT $packet;
STDOUT->flush();
}
+sub packet_txt_write {
+ packet_bin_write( $_[0] . "\n" );
+}
+
sub packet_flush {
print STDOUT sprintf( "%04x", 0 );
STDOUT->flush();
@@ -78,35 +90,35 @@ sub packet_flush {
print $debug "START\n";
$debug->flush();
-( packet_read() eq ( 0, "git-filter-client" ) ) || die "bad initialization";
-( packet_read() eq ( 0, "version=2" ) ) || die "bad version";
-( packet_read() eq ( 1, "" ) ) || die "bad version end";
+( packet_txt_read() eq ( 0, "git-filter-client" ) ) || die "bad initialize";
+( packet_txt_read() eq ( 0, "version=2" ) ) || die "bad version";
+( packet_bin_read() eq ( 1, "" ) ) || die "bad version end";
-packet_write("git-filter-server\n");
-packet_write("version=2\n");
+packet_txt_write("git-filter-server");
+packet_txt_write("version=2");
-( packet_read() eq ( 0, "clean=true" ) ) || die "bad capability";
-( packet_read() eq ( 0, "smudge=true" ) ) || die "bad capability";
-( packet_read() eq ( 1, "" ) ) || die "bad capability end";
+( packet_txt_read() eq ( 0, "clean=true" ) ) || die "bad capability";
+( packet_txt_read() eq ( 0, "smudge=true" ) ) || die "bad capability";
+( packet_bin_read() eq ( 1, "" ) ) || die "bad capability end";
foreach (@capabilities) {
- packet_write( $_ . "=true\n" );
+ packet_txt_write( $_ . "=true" );
}
packet_flush();
print $debug "wrote filter header\n";
$debug->flush();
while (1) {
- my ($command) = packet_read() =~ /^command=([^=]+)\n$/;
+ my ($command) = packet_txt_read() =~ /^command=([^=]+)$/;
print $debug "IN: $command";
$debug->flush();
- my ($pathname) = packet_read() =~ /^pathname=([^=]+)\n$/;
+ my ($pathname) = packet_txt_read() =~ /^pathname=([^=]+)$/;
print $debug " $pathname";
$debug->flush();
# Flush
- packet_read();
+ packet_bin_read();
my $input = "";
{
@@ -114,7 +126,7 @@ while (1) {
my $buffer;
my $done = 0;
while ( !$done ) {
- ( $done, $buffer ) = packet_read();
+ ( $done, $buffer ) = packet_bin_read();
$input .= $buffer;
}
print $debug " " . length($input) . " [OK] -- ";
@@ -141,17 +153,17 @@ while (1) {
if ( $pathname eq "error.r" ) {
print $debug "[ERROR]\n";
$debug->flush();
- packet_write("status=error\n");
+ packet_txt_write("status=error");
packet_flush();
}
elsif ( $pathname eq "abort.r" ) {
print $debug "[ABORT]\n";
$debug->flush();
- packet_write("status=abort\n");
+ packet_txt_write("status=abort");
packet_flush();
}
else {
- packet_write("status=success\n");
+ packet_txt_write("status=success");
packet_flush();
if ( $pathname eq "${command}-write-fail.r" ) {
@@ -162,7 +174,7 @@ while (1) {
while ( length($output) > 0 ) {
my $packet = substr( $output, 0, $MAX_PACKET_CONTENT_SIZE );
- packet_write($packet);
+ packet_bin_write($packet);
print $debug ".";
if ( length($output) > $MAX_PACKET_CONTENT_SIZE ) {
$output = substr( $output, $MAX_PACKET_CONTENT_SIZE );
diff --git a/unpack-trees.c b/unpack-trees.c
index f6798f8..11c37fb 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -10,7 +10,6 @@
#include "attr.h"
#include "split-index.h"
#include "dir.h"
-#include "convert.h"
/*
* Error messages expected by scripts out of plumbing commands such as
diff --git a/write_or_die.c b/write_or_die.c
index 0734432..eab8c8d 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -1,19 +1,6 @@
#include "cache.h"
#include "run-command.h"
-static void check_pipe(int err)
-{
- if (err == EPIPE) {
- if (in_async())
- async_exit(141);
-
- signal(SIGPIPE, SIG_DFL);
- raise(SIGPIPE);
- /* Should never happen, but just in case... */
- exit(141);
- }
-}
-
/*
* Some cases use stdio, but want to flush after the write
* to get error handling (and to get better interactive
Lars Schneider (11):
pkt-line: rename packet_write() to packet_write_fmt()
pkt-line: extract set_packet_header()
run-command: move check_pipe() from write_or_die to run_command
pkt-line: add packet_write_fmt_gently()
pkt-line: add packet_flush_gently()
pkt-line: add packet_write_gently()
pkt-line: add functions to read/write flush terminated packet streams
convert: quote filter names in error messages
convert: modernize tests
convert: make apply_filter() adhere to standard Git error handling
convert: add filter.<driver>.process option
Documentation/gitattributes.txt | 156 +++++++++++-
builtin/archive.c | 4 +-
builtin/receive-pack.c | 4 +-
builtin/remote-ext.c | 4 +-
builtin/upload-archive.c | 4 +-
connect.c | 2 +-
contrib/long-running-filter/example.pl | 123 ++++++++++
convert.c | 369 ++++++++++++++++++++++++----
daemon.c | 2 +-
http-backend.c | 2 +-
pkt-line.c | 147 +++++++++++-
pkt-line.h | 12 +-
run-command.c | 13 +
run-command.h | 2 +
shallow.c | 2 +-
t/t0021-conversion.sh | 423 ++++++++++++++++++++++++++++++---
t/t0021/rot13-filter.pl | 191 +++++++++++++++
upload-pack.c | 30 +--
write_or_die.c | 13 -
19 files changed, 1379 insertions(+), 124 deletions(-)
create mode 100755 contrib/long-running-filter/example.pl
create mode 100755 t/t0021/rot13-filter.pl
--
2.10.0
^ permalink raw reply related
* Re: v2.9.3 and v2.10.0: `name-ref' HEAD gives wrong branch name
From: Bryan Turner @ 2016-09-20 18:54 UTC (permalink / raw)
To: Steffen Nurpmeso; +Cc: Git Users
In-Reply-To: <20160920162309.l0neYMQ3l%steffen@sdaoden.eu>
On Tue, Sep 20, 2016 at 9:23 AM, Steffen Nurpmeso <steffen@sdaoden.eu> wrote:
> Hello again,
>
> yah, sorry, i'm back again..
> I try to find a way to find the name of the current branch in an
> automated way, because i need to ensure that a commit happens on
> it and no other branch. Now the problem arises that the commit
> ref at the time of that commit maybe shared in between several
> different branches, but no more thereafter, of course:
>
> ?0[steffen@wales ]$ git branch|grep '^*'
> * stable/v14.9
> ?0[steffen@wales ]$ git name-rev --name-only HEAD
> stable/v14.8
>
> Is there another way except looking into .git/HEAD or using sed(1)
> on the output of `branch' to find the right name?
Have you tried "git symbolic-ref HEAD"?
$ git symbolic-ref HEAD
refs/heads/master
If you don't want the fully-qualified ref, you can add --short:
$ git symbolic-ref --short HEAD
master
> Thank you.
> Ciao!
>
> --steffen
^ permalink raw reply
* Aw: Re: Homebrew and Git
From: Jonas Thiel @ 2016-09-20 18:45 UTC (permalink / raw)
To: Heiko Voigt; +Cc: git
In-Reply-To: <20160920110228.GA64315@book.hvoigt.net>
Hi Heiko,
thanks for your reply on my issue.
Here are the following outputs you asked for:
git version
git version 2.7.4 (Apple Git-66)
hostname -f
0x6A6E73
whoami
jns
I have tried the Homebrew version of git, but no change in case of the issue.
I just customized my system with the Mathias Bynens dotfiles (https://github.com/mathiasbynens/dotfiles) as well as with drduhs OS X Security Guide (https://github.com/drduh/OS-X-Security-and-Privacy-Guide). Because I reinstall my Mac on a regularly basis, wherefore I can say that one year ago these tweaks have not caused any issue.
Thanks a lot. I really appreciate your help.
Cheers,
Jonas
> Gesendet: Dienstag, 20. September 2016 um 13:02 Uhr
> Von: "Heiko Voigt" <hvoigt@hvoigt.net>
> An: "Jonas Thiel" <jonas.lierschied@gmx.de>
> Cc: git@vger.kernel.org
> Betreff: Re: Homebrew and Git
>
> Hi,
>
> On Sun, Sep 18, 2016 at 05:50:28PM +0200, Jonas Thiel wrote:
> > A while ago I have described my problem with Homebrew at the following
> > GitHub channel
> > (https://github.com/Homebrew/homebrew-core/issues/2970). In the
> > meanwhile, I believe that I my problem with Homebrew is based on an
> > issues with my Git. I have found the attached Git Crash reports on my
> > Mac and because I am not familiar with reading/analysing Crash
> > Reports, it would be great if someone could give me some feedback on
> > it.
> >
> > If you have any question, please do not hesitate to contact me.
>
> From your crash reports I see that git is apparently crashing in a
> strchr() call from within ident_default_email() which is a function that
> tries to assemble a name and email to put into your commits.
>
> Can you post us the output of
>
> hostname -f
>
> and
>
> whoami
>
> ?
>
> Since it seems you are using an Apple git can you also give us the
> output of
>
> git version
>
> Since it seems that Apple is compiling its own git (and not publishing
> the changes they made conveniently via git). Have you tried
> installing a vanilla git via homebrew and seeing whether that also
> produces the issue?
>
> In your bugreport you are talking about modifications you do to your
> system after which the issue occurred. I would suggest to exactly find
> out which step lead to git crashing (if it actually is the issue). First
> to identify an issue we need something that is reproduceable.
>
> Cheers Heiko
>
^ permalink raw reply
* [PATCH v3 3/3] mailinfo: handle in-body header continuations
From: Jonathan Tan @ 2016-09-20 17:17 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster
In-Reply-To: <cover.1474391594.git.jonathantanmy@google.com>
Mailinfo currently handles multi-line headers, but it does not handle
multi-line in-body headers. Teach it to handle such headers, for
example, for this input:
From: author <author@example.com>
Date: Fri, 9 Jun 2006 00:44:16 -0700
Subject: a very long
broken line
Subject: another 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.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
mailinfo.c | 50 +++++++++++++++++++++++++++++++++++-
mailinfo.h | 1 +
t/t4150-am.sh | 23 +++++++++++++++++
t/t5100-mailinfo.sh | 2 +-
t/t5100/info0018 | 5 ++++
t/t5100/info0018--no-inbody-headers | 5 ++++
t/t5100/msg0015 | 2 --
t/t5100/msg0018 | 2 ++
t/t5100/msg0018--no-inbody-headers | 8 ++++++
t/t5100/patch0018 | 6 +++++
t/t5100/patch0018--no-inbody-headers | 6 +++++
t/t5100/sample.mbox | 19 ++++++++++++++
12 files changed, 125 insertions(+), 4 deletions(-)
create mode 100644 t/t5100/info0018
create mode 100644 t/t5100/info0018--no-inbody-headers
create mode 100644 t/t5100/msg0018
create mode 100644 t/t5100/msg0018--no-inbody-headers
create mode 100644 t/t5100/patch0018
create mode 100644 t/t5100/patch0018--no-inbody-headers
diff --git a/mailinfo.c b/mailinfo.c
index 69391aa..2275b28 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -500,6 +500,21 @@ static int check_header(struct mailinfo *mi,
return ret;
}
+/*
+ * Returns 1 if the given line or any line beginning with the given line is an
+ * in-body header (that is, check_header will succeed when passed
+ * mi->s_hdr_data).
+ */
+static int is_inbody_header(const struct mailinfo *mi,
+ const struct strbuf *line)
+{
+ int i;
+ for (i = 0; header[i]; i++)
+ if (!mi->s_hdr_data[i] && cmp_header(line, header[i]))
+ return 1;
+ return 0;
+}
+
static void decode_transfer_encoding(struct mailinfo *mi, struct strbuf *line)
{
struct strbuf *ret;
@@ -609,8 +624,33 @@ static int is_scissors_line(const char *line)
gap * 2 < perforation);
}
+static void flush_inbody_header_accum(struct mailinfo *mi)
+{
+ if (!mi->inbody_header_accum.len)
+ return;
+ assert(check_header(mi, &mi->inbody_header_accum, mi->s_hdr_data, 0));
+ strbuf_reset(&mi->inbody_header_accum);
+}
+
static int check_inbody_header(struct mailinfo *mi, const struct strbuf *line)
{
+ if (mi->inbody_header_accum.len &&
+ (line->buf[0] == ' ' || line->buf[0] == '\t')) {
+ if (mi->use_scissors && is_scissors_line(line->buf)) {
+ /*
+ * This is a scissors line; do not consider this line
+ * as a header continuation line.
+ */
+ flush_inbody_header_accum(mi);
+ return 0;
+ }
+ strbuf_strip_suffix(&mi->inbody_header_accum, "\n");
+ strbuf_addbuf(&mi->inbody_header_accum, line);
+ return 1;
+ }
+
+ flush_inbody_header_accum(mi);
+
if (starts_with(line->buf, ">From") && isspace(line->buf[5]))
return is_format_patch_separator(line->buf + 1, line->len - 1);
if (starts_with(line->buf, "[PATCH]") && isspace(line->buf[7])) {
@@ -622,7 +662,11 @@ static int check_inbody_header(struct mailinfo *mi, const struct strbuf *line)
}
return 0;
}
- return check_header(mi, line, mi->s_hdr_data, 0);
+ if (is_inbody_header(mi, line)) {
+ strbuf_addbuf(&mi->inbody_header_accum, line);
+ return 1;
+ }
+ return 0;
}
static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
@@ -888,6 +932,8 @@ static void handle_body(struct mailinfo *mi, struct strbuf *line)
break;
} while (!strbuf_getwholeline(line, mi->input, '\n'));
+ flush_inbody_header_accum(mi);
+
handle_body_out:
strbuf_release(&prev);
}
@@ -1003,6 +1049,7 @@ void setup_mailinfo(struct mailinfo *mi)
strbuf_init(&mi->email, 0);
strbuf_init(&mi->charset, 0);
strbuf_init(&mi->log_message, 0);
+ strbuf_init(&mi->inbody_header_accum, 0);
mi->header_stage = 1;
mi->use_inbody_headers = 1;
mi->content_top = mi->content;
@@ -1016,6 +1063,7 @@ void clear_mailinfo(struct mailinfo *mi)
strbuf_release(&mi->name);
strbuf_release(&mi->email);
strbuf_release(&mi->charset);
+ strbuf_release(&mi->inbody_header_accum);
free(mi->message_id);
for (i = 0; mi->p_hdr_data[i]; i++)
diff --git a/mailinfo.h b/mailinfo.h
index 93776a7..04a2535 100644
--- a/mailinfo.h
+++ b/mailinfo.h
@@ -27,6 +27,7 @@ struct mailinfo {
int patch_lines;
int filter_stage; /* still reading log or are we copying patch? */
int header_stage; /* still checking in-body headers? */
+ struct strbuf inbody_header_accum;
struct strbuf **p_hdr_data;
struct strbuf **s_hdr_data;
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 9ce9424..89a5bac 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -977,4 +977,27 @@ test_expect_success 'am --patch-format=mboxrd handles mboxrd' '
test_cmp msg out
'
+test_expect_success 'am works with multi-line in-body headers' '
+ FORTY="String that has a length of more than forty characters" &&
+ LONG="$FORTY $FORTY" &&
+ rm -fr .git/rebase-apply &&
+ git checkout -f first &&
+ echo one >> file &&
+ git commit -am "$LONG" --author="$LONG <long@example.com>" &&
+ git format-patch --stdout -1 >patch &&
+ # bump from, date, and subject down to in-body header
+ perl -lpe "
+ if (/^From:/) {
+ print \"From: x <x\@example.com>\";
+ print \"Date: Sat, 1 Jan 2000 00:00:00 +0000\";
+ print \"Subject: x\n\";
+ }
+ " patch >msg &&
+ git checkout HEAD^ &&
+ git am msg &&
+ # Ensure that the author and full message are present
+ git cat-file commit HEAD | grep "^author.*long@example.com" &&
+ git cat-file commit HEAD | grep "^$LONG"
+'
+
test_done
diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index 1a5a546..e173c33 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -11,7 +11,7 @@ test_expect_success 'split sample box' \
'git mailsplit -o. "$TEST_DIRECTORY"/t5100/sample.mbox >last &&
last=$(cat last) &&
echo total is $last &&
- test $(cat last) = 17'
+ test $(cat last) = 18'
check_mailinfo () {
mail=$1 opt=$2
diff --git a/t/t5100/info0018 b/t/t5100/info0018
new file mode 100644
index 0000000..d53e749
--- /dev/null
+++ b/t/t5100/info0018
@@ -0,0 +1,5 @@
+Author: Another Thor
+Email: a.thor@example.com
+Subject: This one contains a tab and a space
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/info0018--no-inbody-headers b/t/t5100/info0018--no-inbody-headers
new file mode 100644
index 0000000..30b17bd
--- /dev/null
+++ b/t/t5100/info0018--no-inbody-headers
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@example.com
+Subject: check multiline inbody headers
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/msg0015 b/t/t5100/msg0015
index 4abb3d5..e69de29 100644
--- a/t/t5100/msg0015
+++ b/t/t5100/msg0015
@@ -1,2 +0,0 @@
- - a list
- - of stuff
diff --git a/t/t5100/msg0018 b/t/t5100/msg0018
new file mode 100644
index 0000000..56de83d
--- /dev/null
+++ b/t/t5100/msg0018
@@ -0,0 +1,2 @@
+a commit message
+
diff --git a/t/t5100/msg0018--no-inbody-headers b/t/t5100/msg0018--no-inbody-headers
new file mode 100644
index 0000000..b1e05d3
--- /dev/null
+++ b/t/t5100/msg0018--no-inbody-headers
@@ -0,0 +1,8 @@
+From: Another Thor
+ <a.thor@example.com>
+Subject: This one contains
+ a tab
+ and a space
+
+a commit message
+
diff --git a/t/t5100/patch0018 b/t/t5100/patch0018
new file mode 100644
index 0000000..789df6d
--- /dev/null
+++ b/t/t5100/patch0018
@@ -0,0 +1,6 @@
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
diff --git a/t/t5100/patch0018--no-inbody-headers b/t/t5100/patch0018--no-inbody-headers
new file mode 100644
index 0000000..789df6d
--- /dev/null
+++ b/t/t5100/patch0018--no-inbody-headers
@@ -0,0 +1,6 @@
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
diff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox
index 8b2ae06..6d4d0e4 100644
--- a/t/t5100/sample.mbox
+++ b/t/t5100/sample.mbox
@@ -699,3 +699,22 @@ index e69de29..d95f3ad 100644
+++ b/foo
@@ -0,0 +1 @@
+New content
+From nobody Mon Sep 17 00:00:00 2001
+From: A U Thor <a.u.thor@example.com>
+Subject: check multiline inbody headers
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
+From: Another Thor
+ <a.thor@example.com>
+Subject: This one contains
+ a tab
+ and a space
+
+a commit message
+
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
--
2.10.0.rc2.20.g5b18e70
^ permalink raw reply related
* [PATCH v3 2/3] mailinfo: make is_scissors_line take plain char *
From: Jonathan Tan @ 2016-09-20 17:17 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster
In-Reply-To: <cover.1474391594.git.jonathantanmy@google.com>
The is_scissors_line takes a struct strbuf * when a char * would
suffice. Make it take char *.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
mailinfo.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/mailinfo.c b/mailinfo.c
index 0c4738a..69391aa 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -557,37 +557,35 @@ static inline int patchbreak(const struct strbuf *line)
return 0;
}
-static int is_scissors_line(const struct strbuf *line)
+static int is_scissors_line(const char *line)
{
- size_t i, len = line->len;
+ const char *c;
int scissors = 0, gap = 0;
- int first_nonblank = -1;
- int last_nonblank = 0, visible, perforation = 0, in_perforation = 0;
- const char *buf = line->buf;
+ const char *first_nonblank = NULL, *last_nonblank = NULL;
+ int visible, perforation = 0, in_perforation = 0;
- for (i = 0; i < len; i++) {
- if (isspace(buf[i])) {
+ for (c = line; *c; c++) {
+ if (isspace(*c)) {
if (in_perforation) {
perforation++;
gap++;
}
continue;
}
- last_nonblank = i;
- if (first_nonblank < 0)
- first_nonblank = i;
- if (buf[i] == '-') {
+ last_nonblank = c;
+ if (first_nonblank == NULL)
+ first_nonblank = c;
+ if (*c == '-') {
in_perforation = 1;
perforation++;
continue;
}
- if (i + 1 < len &&
- (!memcmp(buf + i, ">8", 2) || !memcmp(buf + i, "8<", 2) ||
- !memcmp(buf + i, ">%", 2) || !memcmp(buf + i, "%<", 2))) {
+ if ((!memcmp(c, ">8", 2) || !memcmp(c, "8<", 2) ||
+ !memcmp(c, ">%", 2) || !memcmp(c, "%<", 2))) {
in_perforation = 1;
perforation += 2;
scissors += 2;
- i++;
+ c++;
continue;
}
in_perforation = 0;
@@ -602,7 +600,10 @@ static int is_scissors_line(const struct strbuf *line)
* than half of the perforation.
*/
- visible = last_nonblank - first_nonblank + 1;
+ if (first_nonblank && last_nonblank)
+ visible = last_nonblank - first_nonblank + 1;
+ else
+ visible = 0;
return (scissors && 8 <= visible &&
visible < perforation * 3 &&
gap * 2 < perforation);
@@ -647,7 +648,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
if (convert_to_utf8(mi, line, mi->charset.buf))
return 0; /* mi->input_error already set */
- if (mi->use_scissors && is_scissors_line(line)) {
+ if (mi->use_scissors && is_scissors_line(line->buf)) {
int i;
strbuf_setlen(&mi->log_message, 0);
--
2.10.0.rc2.20.g5b18e70
^ permalink raw reply related
* [PATCH v3 1/3] mailinfo: separate in-body header processing
From: Jonathan Tan @ 2016-09-20 17:17 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster
In-Reply-To: <cover.1474391594.git.jonathantanmy@google.com>
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 <jonathantanmy@google.com>
---
mailinfo.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/mailinfo.c b/mailinfo.c
index e19abe3..0c4738a 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -495,21 +495,6 @@ static int check_header(struct mailinfo *mi,
goto check_header_out;
}
- /* for inbody stuff */
- if (starts_with(line->buf, ">From") && isspace(line->buf[5])) {
- ret = is_format_patch_separator(line->buf + 1, line->len - 1);
- goto check_header_out;
- }
- if (starts_with(line->buf, "[PATCH]") && isspace(line->buf[7])) {
- for (i = 0; header[i]; i++) {
- if (!strcmp("Subject", header[i])) {
- handle_header(&hdr_data[i], line);
- ret = 1;
- goto check_header_out;
- }
- }
- }
-
check_header_out:
strbuf_release(&sb);
return ret;
@@ -623,6 +608,22 @@ static int is_scissors_line(const struct strbuf *line)
gap * 2 < perforation);
}
+static int check_inbody_header(struct mailinfo *mi, const struct strbuf *line)
+{
+ if (starts_with(line->buf, ">From") && isspace(line->buf[5]))
+ return is_format_patch_separator(line->buf + 1, line->len - 1);
+ if (starts_with(line->buf, "[PATCH]") && isspace(line->buf[7])) {
+ int i;
+ for (i = 0; header[i]; i++)
+ if (!strcmp("Subject", header[i])) {
+ handle_header(&mi->s_hdr_data[i], line);
+ return 1;
+ }
+ return 0;
+ }
+ return check_header(mi, line, mi->s_hdr_data, 0);
+}
+
static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
{
assert(!mi->filter_stage);
@@ -633,7 +634,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
}
if (mi->use_inbody_headers && mi->header_stage) {
- mi->header_stage = check_header(mi, line, mi->s_hdr_data, 0);
+ mi->header_stage = check_inbody_header(mi, line);
if (mi->header_stage)
return 0;
} else
--
2.10.0.rc2.20.g5b18e70
^ 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