* Re: RFE: Discard hunks during `git add -p`
From: Jeff King @ 2016-11-02 22:37 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: git
In-Reply-To: <20161102221113.peur2zyfs66bdchm@sigill.intra.peff.net>
On Wed, Nov 02, 2016 at 06:11:14PM -0400, Jeff King wrote:
> > Being able to discard hunks (reset working copy to index contents)
> > during add-p would alleviate the (quite broad) hard reset.
>
> As Konstantin pointed out, you can already discard interactively with
> "git checkout -p". It might be nice to be able to do both in the same
> run, and turn the "yes/no" decision into "yes/no/discard".
>
> In theory it should be easy, as the same code drives the hunk selector
> for both commands. It's just a matter of which command we feed the
> selected hunks to. I don't know if there would be corner cases around
> hunk-editing and splitting, though. The "add" phase should never touch
> the working tree file itself, so any hunks present from the initial list
> should still apply cleanly during the "discard" phase.
The patch is something like the one below, which worked for me in a very
trivial test. I won't be surprised if there are some corner cases it's
missing. At the very least, coalesce_overlapping_hunks() needs to learn
about the differences between "apply" and "discard" hunks (and not
coalesce them!).
I don't have immediate plans for this, so if somebody wants to pick it
up and run with it, be my guest.
-Peff
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index ee3d81269..43651435a 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -109,6 +109,7 @@ my %patch_modes = (
PARTICIPLE => 'staging',
FILTER => 'file-only',
IS_REVERSE => 0,
+ DISCARD => sub { apply_patch 'apply -R', @_; },
},
'stash' => {
DIFF => 'diff-index -p HEAD',
@@ -1325,6 +1326,11 @@ sub patch_update_file {
my ($prev, $next, $other, $undecided, $i);
$other = '';
+ my $discard = exists $patch_mode_flavour{DISCARD};
+ if ($discard) {
+ $other .= ',D';
+ }
+
if ($num <= $ix) {
$ix = 0;
}
@@ -1384,6 +1390,9 @@ sub patch_update_file {
elsif ($line =~ /^n/i) {
$hunk[$ix]{USE} = 0;
}
+ elsif ($discard && $line =~ /^D/) {
+ $hunk[$ix]{USE} = -1;
+ }
elsif ($line =~ /^a/i) {
while ($ix < $num) {
if (!defined $hunk[$ix]{USE}) {
@@ -1539,9 +1548,12 @@ sub patch_update_file {
my $n_lofs = 0;
my @result = ();
+ my @discard = ();
for (@hunk) {
- if ($_->{USE}) {
+ if ($_->{USE} > 0) {
push @result, @{$_->{TEXT}};
+ } elsif ($_->{USE} < 0) {
+ push @discard, @{$_->{TEXT}};
}
}
@@ -1552,6 +1564,13 @@ sub patch_update_file {
refresh();
}
+ if (@discard) {
+ my @patch = reassemble_patch($head->{TEXT}, @discard);
+ my $apply_routine = $patch_mode_flavour{DISCARD};
+ &$apply_routine(@patch);
+ refresh();
+ }
+
print "\n";
return $quit;
}
^ permalink raw reply related
* Re: send-email garbled header with trailing doublequote in email
From: Andrea Arcangeli @ 2016-11-02 22:29 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20161102220437.5ewzezxs2nasyouv@sigill.intra.peff.net>
On Wed, Nov 02, 2016 at 06:04:37PM -0400, Jeff King wrote:
> Nope, it looks exactly as --dry-run reports it.
My sendmail is postfix 3.1.0.
> To see exactly what is being sent out, try:
>
> -- >8 --
>
> cat >/tmp/foo <<\EOF
> #!/bin/sh
> echo "args: $*"
> sed 's/^/stdin: /'
> EOF
>
> chmod +x /tmp/foo
>
> git send-email --smtp-server=/tmp/foo --to=whatever
>
> -- 8< --
Right it's the same as --dry-run:
stdin: To: "what ever" " <.....>
There's not my hostname and not removed space. If I add more addresses
they also go in the second line with a leading space and they're not cut.
So this must be postfix then that out of the blue decided to garble it
in a strange way while parsing the input... The removal of all
whitespaces s/what ever/whatever/ especially I've no idea how it
decided to do so.
Can you reproduce with postfix as sendmail at least? If you can
reproduce also see what happens if you add another --to.
^ permalink raw reply
* [PATCH] transport: add core.allowProtocol config option
From: Brandon Williams @ 2016-11-02 22:20 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
Add configuration option 'core.allowProtocol' to allow users to create a
whitelist of allowed protocols for fetch/push/clone in their gitconfig.
For git-submodule.sh, fallback to default whitelist only if the user
hasn't explicitly set `GIT_ALLOW_PROTOCOL` or doesn't have a whitelist
in their gitconfig.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/config.txt | 9 +++++++++
git-submodule.sh | 3 ++-
transport.c | 2 +-
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 27069ac..7f83e40 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -455,6 +455,15 @@ core.sshCommand::
the `GIT_SSH_COMMAND` environment variable and is overridden
when the environment variable is set.
+core.allowProtocol::
+ Provide a colon-separated list of protocols which are allowed to be
+ used with fetch/push/clone. This is useful to restrict recursive
+ submodule initialization from an untrusted repository. Any protocol not
+ mentioned will be disallowed (i.e., this is a whitelist, not a
+ blacklist). If the variable is not set at all, all protocols are
+ enabled. If the `GIT_ALLOW_PROTOCOL` enviornment variable is set, it is
+ used as the protocol whitelist instead of this config option.
+
core.ignoreStat::
If true, Git will avoid using lstat() calls to detect if files have
changed by setting the "assume-unchanged" bit for those tracked files
diff --git a/git-submodule.sh b/git-submodule.sh
index a024a13..ad94c75 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -27,7 +27,8 @@ cd_to_toplevel
#
# If the user has already specified a set of allowed protocols,
# we assume they know what they're doing and use that instead.
-: ${GIT_ALLOW_PROTOCOL=file:git:http:https:ssh}
+config_whitelist=$(git config core.allowProtocol)
+: ${GIT_ALLOW_PROTOCOL=${config_whitelist:-file:git:http:https:ssh}}
export GIT_ALLOW_PROTOCOL
command=
diff --git a/transport.c b/transport.c
index d57e8de..b1098cd 100644
--- a/transport.c
+++ b/transport.c
@@ -652,7 +652,7 @@ static const struct string_list *protocol_whitelist(void)
if (enabled < 0) {
const char *v = getenv("GIT_ALLOW_PROTOCOL");
- if (v) {
+ if (v || !git_config_get_value("core.allowProtocol", &v)) {
string_list_split(&allowed, v, ':', -1);
string_list_sort(&allowed);
enabled = 1;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* Re: RFE: Discard hunks during `git add -p`
From: Jeff King @ 2016-11-02 22:11 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: git
In-Reply-To: <alpine.LSU.2.20.1611021435280.21207@nerf40.vanv.qr>
On Wed, Nov 02, 2016 at 02:46:04PM +0100, Jan Engelhardt wrote:
> Current version: 2.10.2
> Example workflow:
>
> * I would do a global substitution across a source tree, e.g. `perl -i
> -pe 's{OLD_FOO\(x\)}{NEW_BAR(x, 0)}' *.c`
> * Using `git add -p`, I would verify each of the substitutions that they
> make sense in their respective locations, and, based on that,
> answer "y" or "n" to the interactive prompting to stage good hunks.
> * When done with add-p, I would commit the so-staged hunks,
> and then use `git reset --hard` to discard all changes that were
> not acknowledged during add-p.
>
> Being able to discard hunks (reset working copy to index contents)
> during add-p would alleviate the (quite broad) hard reset.
As Konstantin pointed out, you can already discard interactively with
"git checkout -p". It might be nice to be able to do both in the same
run, and turn the "yes/no" decision into "yes/no/discard".
In theory it should be easy, as the same code drives the hunk selector
for both commands. It's just a matter of which command we feed the
selected hunks to. I don't know if there would be corner cases around
hunk-editing and splitting, though. The "add" phase should never touch
the working tree file itself, so any hunks present from the initial list
should still apply cleanly during the "discard" phase.
-Peff
^ permalink raw reply
* Re: send-email garbled header with trailing doublequote in email
From: Jeff King @ 2016-11-02 22:04 UTC (permalink / raw)
To: Andrea Arcangeli; +Cc: git
In-Reply-To: <20161102213805.GJ4611@redhat.com>
On Wed, Nov 02, 2016 at 10:38:05PM +0100, Andrea Arcangeli wrote:
> > But in either case, in my test, the actual email address is still
> > extracted correctly and fed to the MTA, so the mail is delivered.
>
> The email is delivered to the right email for me too, but to see the
> problem you need to check the email itself, and look how the To: field
> looks in the actual email in your mail client or web interface.
>
> Don't you see whatever@yourhostname without spaces and @yourhostname
> instead of the email domain?
Nope, it looks exactly as --dry-run reports it.
To see exactly what is being sent out, try:
-- >8 --
cat >/tmp/foo <<\EOF
#!/bin/sh
echo "args: $*"
sed 's/^/stdin: /'
EOF
chmod +x /tmp/foo
git send-email --smtp-server=/tmp/foo --to=whatever
-- 8< --
Mine shows the same header as --dry-run. Which makes sense, because the
code is literally just dumping the $header variable, which is the same
thing that gets sent to the sendmail binary (or the SMTP server if that
is in use).
So my guess would be that either an MTA in the routing path is garbling
it (or possibly mailing list software). or maybe even the eventual MUA,
though it sounds like you checked the raw bytes.
> If you still can't reproduce, maybe it's a different perl
> Mail::Address, I'm using dev-perl/MailTools-2.140.0
Mine is 2.13, which I would imagine is comparable.
> If --dry-run complained and failed instead of passing and then sending
> an email with garbled To/Cc list, it'd be more than enough. Either
> that or it should generate a mail header that matches the output of
> --dry-run so the review of --dry-run becomes meaningful again.
OK. I think we get that first part right. The problem is that the
garbling is such that somebody in the middle is unhappy with it (which
makes sense; it's syntactically bogus). So the tool is there to see the
problem in --dry-run, but of course it's rather subtle.
In theory we should be able to detect and complain about bogus syntax
like this, but right now we don't re-parse the addresses at all. We rely
on Mail::Address to produce valid output, and it doesn't seem to be
doing so here.
-Peff
^ permalink raw reply
* Re: [ANNOUNCE] Git v2.11.0-rc0
From: Michael Haggerty @ 2016-11-02 21:44 UTC (permalink / raw)
To: Junio C Hamano, Jeff King; +Cc: Stefan Beller, Jacob Keller, git
In-Reply-To: <xmqqzilinanp.fsf@gitster.mtv.corp.google.com>
On 11/01/2016 10:38 PM, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> If it involves renaming and swapping, however, we may be talking
>> about a change that we cannot deliver with confidence in less than a
>> week before -rc1, which sounds, eh, suboptimal.
>>
>> FWIW, here is how the removal of compaction without renaming looks
>> like.
>
> And here is _with_ renaming. I think "compaction heuristics" is a
> much better phrase to call "heuristics used during the diff hunk
> compaction process" without specifying how that heuristics work
> (like taking hints in the indentation levels). If we are to retire
> one while keeping the other, compaction-heuristics should be the
> name we give and keep for the surviving one.
Personally, I'm not a fan of the name "compaction heuristics".
The name *seems* to make sense because it affects the behavior of a
function called `xdl_change_compact()`. But that means nothing to end
users. The option doesn't affect how the diff is "compacted" in the
usual sense of the word; the slider optimization usually doesn't change
the number of lines in the diff at all [1].
Moreover, if we ever want to add another heuristic (for example, imagine
a language-aware algorithm that is based on parsing the file), we would
want a different name for that option, at least temporarily. From that
point of view, it makes a little bit of sense for the name of the option
to hint at the particular heuristic being used.
That being said, I don't think it is a big deal either way, and I can
live with either name. I rather hope that this option will become the
default soon, and hopefully after that hardly anybody will need to learn
its name.
Regarding making it the default: given that it is presumably too late in
this release cycle to make this option the default behavior, I suggest
that it be made the default early in the next release cycle so that it
gets a lot of testing, and people have enough time to voice any objections.
Regarding your patches themselves, once the old compaction heuristic is
removed (with or without renaming), you can also get rid of the
`blank_lines` local variable in `xdl_change_compact()`, and also the
function `is_blank_line()` in the same source file.
Michael
[1] The only exceptions are when it causes context lines for adjacent
diff hunks to join/split, or makes the context lines for a diff hunk
extend past the beginning/end of the file. But these are uninteresting
side-effects; it doesn't *try* to do or avoid either of these things.
^ permalink raw reply
* Re: send-email garbled header with trailing doublequote in email
From: Andrea Arcangeli @ 2016-11-02 21:38 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20161102211118.sc4j3fezfqxg23i3@sigill.intra.peff.net>
Hello,
On Wed, Nov 02, 2016 at 05:11:18PM -0400, Jeff King wrote:
> In fact, it is not even git that does this, but rather what
> Mail::Address happens to output (though git has fallback routines if
> that module isn't available that do the same thing).
If it's something in perl it should be fixed there indeed. I didn't
debug what exactly was wrong, so I sent it here first.
> But in either case, in my test, the actual email address is still
> extracted correctly and fed to the MTA, so the mail is delivered.
The email is delivered to the right email for me too, but to see the
problem you need to check the email itself, and look how the To: field
looks in the actual email in your mail client or web interface.
Don't you see whatever@yourhostname without spaces and @yourhostname
instead of the email domain?
If you still can't reproduce, maybe it's a different perl
Mail::Address, I'm using dev-perl/MailTools-2.140.0
From who receives the email it just looks garbled, but --dry-run
showed a correct To/Cc list so it was undetectable that it would end
up garbled during the real email delivery.
> So I'm not sure what you wanted to happen that didn't. Did you want
> --dry-run to complain about the bogus address? Did the message not get
> delivered for you? Something else?
If --dry-run complained and failed instead of passing and then sending
an email with garbled To/Cc list, it'd be more than enough. Either
that or it should generate a mail header that matches the output of
--dry-run so the review of --dry-run becomes meaningful again.
Thanks,
Andrea
^ permalink raw reply
* Re: send-email garbled header with trailing doublequote in email
From: Jeff King @ 2016-11-02 21:11 UTC (permalink / raw)
To: Andrea Arcangeli; +Cc: git
In-Reply-To: <20161102202709.GI4611@redhat.com>
On Wed, Nov 02, 2016 at 09:27:09PM +0100, Andrea Arcangeli wrote:
> send-email gets confused with one trailing " at the end of the
> email. Details and how to reproduce below, it breaks also with
> upstream git version 2.10.2.dirty.
I'm not quite sure what happened, and what you wanted to happen. In your
example:
> FYI: apparently I hit a git bug in this submit... reproducible with
> the below command:
>
> git send-email -1 --to '"what ever" <--your--@--email--.com>"'
The "to" address is slightly bogus here because of the extra
double-quote. That gets turned into a slightly bogus rfc2822 header:
> *snip*
> Dry-OK. Log says:
> To: "what ever" " <--your--@--email--.com>
Which is funny, but matches what we do with other addresses that are
invalid according to the rfc. E.g., there was a discussion recently on:
Stable <stable@vger.kernel.org> [4.8+]
which has historically been converted to:
"Stable [4.8+]" <stable@vger.kernel.org>
In fact, it is not even git that does this, but rather what
Mail::Address happens to output (though git has fallback routines if
that module isn't available that do the same thing).
But in either case, in my test, the actual email address is still
extracted correctly and fed to the MTA, so the mail is delivered.
So I'm not sure what you wanted to happen that didn't. Did you want
--dry-run to complain about the bogus address? Did the message not get
delivered for you? Something else?
-Peff
^ permalink raw reply
* [ANNOUNCE] Git for Windows 2.10.2
From: Johannes Schindelin @ 2016-11-02 20:58 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 2477 bytes --]
Dear Git users,
It is my pleasure to announce that Git for Windows 2.10.2 is available from:
https://git-for-windows.github.io/
Changes since Git for Windows v2.10.1(2) (October 13th 2016)
Git for windows v2.10.1(2) was a MinGit-only release (i.e. there was no
Git for windows installer for that version).
New Features
• Comes with Git v2.10.2.
• Comes with Git Credential Manager v1.8.1.
• Comes with cURL v7.51.0.
• Git for Windows can now be built easily with Visual C++ 2015.
• The installer now logs post-install errors more verbosely.
• A new option asks the installer to skip installation if Git's files
are in use.
• A new option asks the installer to quietly skip downgrading Git for
Windows, without indicating failure.
• There is now an explicit option for symbolic link support,
including a link to a more verbose explanation of the issue.
Bug Fixes
• when upgrading Git for Windows, SSH agent processes are now
auto-terminated.
• When trying to install/upgrade on a Windows version that is no
longer supported, we now refuse to do so.
Filename | SHA-256
-------- | -------
Git-2.10.2-64-bit.exe | 57238ebf86f8b51e32cab44bb91c8060e04774676b77b9fb92f78e7bc7e9a179
Git-2.10.2-32-bit.exe | 8b15054a4ea2dd5d2be0471a430d8ae7c772b94e1a1048221083a0040011011c
PortableGit-2.10.2-64-bit.7z.exe | 101314826892480043d5b11989726fc8ee448991eb7b0a1c61aca751161bc15b
PortableGit-2.10.2-32-bit.7z.exe | edc616817e96a6f15246bb0dd93c97e53d38d3b2a0b7375f26bd0bd082c41a73
MinGit-2.10.2-64-bit.zip | a10de5d5a8e71e207eff20d833ca56902a0668940e3def5f21d089e4f533ea40
MinGit-2.10.2-32-bit.zip | 2b191598bcb37565a2b80729ef8d00c03df02b75f6b9d012080c458999f89cc0
Git-2.10.2-64-bit.tar.bz2 | 7df449ac1813876b5a2e9215d94bca9458c2e0870c65e5b78bd7fc2a448a2a90
Git-2.10.2-32-bit.tar.bz2 | 3f8d0bebc53fabad863b8fe352a317fde61833efa4750f96656cf95017a621e8
Ciao,
Johannes
--
You received this message because you are subscribed to the Google Groups "git-for-windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to git-for-windows+unsubscribe@googlegroups.com.
To post to this group, send email to git-for-windows@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/git-for-windows/20161102204358.6856-1-johannes.schindelin%40gmx.de.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH 4/4] t0021: fix filehandle usage on older perl
From: Torsten Bögershausen @ 2016-11-02 20:54 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, Lars Schneider, Junio C Hamano, git
In-Reply-To: <20161102182301.hezruhtuhra6uoft@sigill.intra.peff.net>
> +use IO::File;
That did the trick (the J6T version work as well)
Thanks for the fast responses.
^ permalink raw reply
* send-email garbled header with trailing doublequote in email
From: Andrea Arcangeli @ 2016-11-02 20:27 UTC (permalink / raw)
To: git
Hello,
send-email gets confused with one trailing " at the end of the
email. Details and how to reproduce below, it breaks also with
upstream git version 2.10.2.dirty.
Feel free to CC me if you need any further info.
Thanks,
Andrea
----- Forwarded message from Andrea Arcangeli <aarcange@redhat.com> -----
Date: Wed, 2 Nov 2016 21:07:02 +0100
From: Andrea Arcangeli <aarcange@redhat.com>
To: linux-mm@kvack.org
Subject: Re: [PATCH 00/33] userfaultfd tmpfs/hugetlbfs/non-cooperative
User-Agent: Mutt/1.7.1 (2016-10-04)
FYI: apparently I hit a git bug in this submit... reproducible with
the below command:
git send-email -1 --to '"what ever" <--your--@--email--.com>"'
after replacing --your--@email--.com with your own email.
*snip*
Dry-OK. Log says:
To: "what ever" " <--your--@--email--.com>
*snip*
X-Mailer: git-send-email 2.7.3
Result: OK
It's not ok if the --dry-run outputs the above with a fine header, but
the actual header in the email data is different. Of course I tested
--dry-run twice and it was fine like the above is fine as well.
*snip*
----- End forwarded message -----
^ permalink raw reply
* Re: [PATCH 0/5] disallow symlinks for .gitignore and .gitattributes
From: Jeff King @ 2016-11-02 19:56 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kY195Bff5bUVm44Bx7+XzDDehYbRxgdii4s040pQXRRSQ@mail.gmail.com>
On Wed, Nov 02, 2016 at 12:46:28PM -0700, Stefan Beller wrote:
> On Wed, Nov 2, 2016 at 6:04 AM, Jeff King <peff@peff.net> wrote:
>
> >
> > attr.c | 58 ++++++++++++++++++++++++++++++++-------------------
>
> $ git diff --stat origin/master..origin/sb/attr |grep attr.c
> attr.c | 531 +-
>
> From a cursory read of your series this may result in a merge
> conflict, but would be
> easily fixable (changed signature of functions that clash).
Yeah, I knew you guys were doing some refactoring of the attribute code
elsewhere, but hadn't actually seen how bad the damage was. I just did
the merge with sb/attr, though, and the conflicts are quite trivial
(mostly s/1/flags/ in bootstrap_attr_stack()).
I'm happy to re-roll on a different base if sb/attr graduates first, but
I suspect Junio can just resolve the conflicts at merge time.
-Peff
^ permalink raw reply
* Re: [PATCH 0/5] disallow symlinks for .gitignore and .gitattributes
From: Stefan Beller @ 2016-11-02 19:46 UTC (permalink / raw)
To: Jeff King; +Cc: git@vger.kernel.org
In-Reply-To: <20161102130432.d3zprdul4sqgcfwu@sigill.intra.peff.net>
On Wed, Nov 2, 2016 at 6:04 AM, Jeff King <peff@peff.net> wrote:
>
> attr.c | 58 ++++++++++++++++++++++++++++++++-------------------
$ git diff --stat origin/master..origin/sb/attr |grep attr.c
attr.c | 531 +-
From a cursory read of your series this may result in a merge
conflict, but would be
easily fixable (changed signature of functions that clash).
^ permalink raw reply
* [PATCH 4/4] t0021: fix filehandle usage on older perl
From: Jeff King @ 2016-11-02 18:23 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Johannes Sixt, Lars Schneider, Junio C Hamano, git
In-Reply-To: <20161102181625.e2uprqdlzl7z2xrz@sigill.intra.peff.net>
The rot13-filter.pl script calls methods on implicitly
defined filehandles (STDOUT, and the result of an open()
call). Prior to perl 5.13, these methods are not
automatically loaded, and perl will complain with:
Can't locate object method "flush" via package "IO::Handle"
Let's explicitly load IO::File (which inherits from
IO::Handle). That's more than we need for just "flush", but
matches what perl has done since:
http://perl5.git.perl.org/perl.git/commit/15e6cdd91beb4cefae4b65e855d68cf64766965d
Signed-off-by: Jeff King <peff@peff.net>
---
I see J6t solved this a week ago using "FileHandle". These days that is
basically a compatibility synonym for IO::File. I think both should be
available pretty much everywhere, so I went with IO::File for the
reasons above. But if that doesn't work for some reason, switching to
"use FileHandle" should be OK, too.
I don't have an old enough perl easily available to test it either way.
t/t0021/rot13-filter.pl | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t0021/rot13-filter.pl b/t/t0021/rot13-filter.pl
index e3ea58e1e..4d5697ee5 100644
--- a/t/t0021/rot13-filter.pl
+++ b/t/t0021/rot13-filter.pl
@@ -21,6 +21,7 @@
use strict;
use warnings;
+use IO::File;
my $MAX_PACKET_CONTENT_SIZE = 65516;
my @capabilities = @ARGV;
--
2.11.0.rc0.258.gf434c15
^ permalink raw reply related
* [PATCH 3/4] t0021: use $PERL_PATH for rot13-filter.pl
From: Jeff King @ 2016-11-02 18:20 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Lars Schneider, Junio C Hamano, git
In-Reply-To: <20161102181625.e2uprqdlzl7z2xrz@sigill.intra.peff.net>
The rot13-filter.pl script hardcodes "#!/usr/bin/perl", and
does not respect $PERL_PATH at all. That is a problem if the
system does not have perl at that path, or if it has a perl
that is too old to run a complicated script like the
rot13-filter (but PERL_PATH points to a more modern one).
We can fix this by using write_script() to create a new copy
of the script with the correct #!-line. In theory we could
move the whole script inside t0021-conversion.sh rather than
having it as an auxiliary file, but it's long enough that
it just makes things harder to read.
As a bonus, we can stop using the full path to the script in
the filter-process config we add (because the trash
directory is in our PATH). Not only is this shorter, but it
sidesteps any shell-quoting issues. The original was broken
when $TEST_DIRECTORY contained a space, because it was
interpolated in the outer script.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0021-conversion.sh | 19 +++++++++++--------
t/t0021/rot13-filter.pl | 1 -
2 files changed, 11 insertions(+), 9 deletions(-)
mode change 100755 => 100644 t/t0021/rot13-filter.pl
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index c1ad20c61..a8fa52148 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -13,6 +13,9 @@ tr \
'nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM'
EOF
+write_script rot13-filter.pl "$PERL_PATH" \
+ <"$TEST_DIRECTORY"/t0021/rot13-filter.pl
+
generate_random_characters () {
LEN=$1
NAME=$2
@@ -341,7 +344,7 @@ test_expect_success 'diff does not reuse worktree files that need cleaning' '
'
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.process "rot13-filter.pl clean smudge" &&
test_config_global filter.protocol.required true &&
rm -rf repo &&
mkdir repo &&
@@ -434,7 +437,7 @@ test_expect_success PERL 'required process filter should filter data' '
test_expect_success PERL 'required process filter takes precedence' '
test_config_global filter.protocol.clean false &&
- test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean" &&
+ test_config_global filter.protocol.process "rot13-filter.pl clean" &&
test_config_global filter.protocol.required true &&
rm -rf repo &&
mkdir repo &&
@@ -459,7 +462,7 @@ test_expect_success PERL 'required process filter takes precedence' '
'
test_expect_success PERL 'required process filter should be used only for "clean" operation only' '
- test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean" &&
+ test_config_global filter.protocol.process "rot13-filter.pl clean" &&
rm -rf repo &&
mkdir repo &&
(
@@ -494,7 +497,7 @@ test_expect_success PERL 'required process filter should be used only for "clean
'
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.process "rot13-filter.pl clean smudge" &&
test_config_global filter.protocol.required true &&
rm -rf repo &&
@@ -554,7 +557,7 @@ test_expect_success PERL 'required process filter should process multiple packet
'
test_expect_success PERL 'required process filter with clean error should fail' '
- test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge" &&
+ test_config_global filter.protocol.process "rot13-filter.pl clean smudge" &&
test_config_global filter.protocol.required true &&
rm -rf repo &&
mkdir repo &&
@@ -573,7 +576,7 @@ test_expect_success PERL 'required process filter with clean error should fail'
'
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" &&
+ test_config_global filter.protocol.process "rot13-filter.pl clean smudge" &&
rm -rf repo &&
mkdir repo &&
(
@@ -624,7 +627,7 @@ test_expect_success PERL 'process filter should restart after unexpected write f
'
test_expect_success PERL 'process filter should not be restarted if it signals an error' '
- test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge" &&
+ test_config_global filter.protocol.process "rot13-filter.pl clean smudge" &&
rm -rf repo &&
mkdir repo &&
(
@@ -663,7 +666,7 @@ test_expect_success PERL 'process filter should not be restarted if it signals a
'
test_expect_success PERL 'process filter abort stops processing of all further files' '
- test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge" &&
+ test_config_global filter.protocol.process "rot13-filter.pl clean smudge" &&
rm -rf repo &&
mkdir repo &&
(
diff --git a/t/t0021/rot13-filter.pl b/t/t0021/rot13-filter.pl
old mode 100755
new mode 100644
index ae4c50f5c..e3ea58e1e
--- a/t/t0021/rot13-filter.pl
+++ b/t/t0021/rot13-filter.pl
@@ -1,4 +1,3 @@
-#!/usr/bin/perl
#
# Example implementation for the Git filter protocol version 2
# See Documentation/gitattributes.txt, section "Filter Protocol"
--
2.11.0.rc0.258.gf434c15
^ permalink raw reply related
* [PATCH 2/4] t0021: put $TEST_ROOT in $PATH
From: Jeff King @ 2016-11-02 18:18 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Lars Schneider, Junio C Hamano, git
In-Reply-To: <20161102181625.e2uprqdlzl7z2xrz@sigill.intra.peff.net>
We create a rot13.sh script in the trash directory, but need
to call it by its full path when we have moved our cwd to
another directory. Let's just put $TEST_ROOT in our $PATH so
that the script is always found.
This is a minor convenience for rot13.sh, but will be a
major one when we switch rot13-filter.pl to a script in the
same directory, as it means we will not have to deal with
shell quoting inside the filter-process config.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0021-conversion.sh | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index dfde22549..c1ad20c61 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -5,6 +5,7 @@ test_description='blob conversion via gitattributes'
. ./test-lib.sh
TEST_ROOT="$(pwd)"
+PATH=$TEST_ROOT:$PATH
write_script <<\EOF "$TEST_ROOT/rot13.sh"
tr \
@@ -64,7 +65,7 @@ test_cmp_exclude_clean () {
# is equal to the committed content.
test_cmp_committed_rot13 () {
test_cmp "$1" "$2" &&
- "$TEST_ROOT/rot13.sh" <"$1" >expected &&
+ rot13.sh <"$1" >expected &&
git cat-file blob :"$2" >actual &&
test_cmp expected actual
}
@@ -513,7 +514,7 @@ test_expect_success PERL 'required process filter should process multiple packet
for FILE in "$TEST_ROOT"/*.file
do
cp "$FILE" . &&
- "$TEST_ROOT/rot13.sh" <"$FILE" >"$FILE.rot13"
+ rot13.sh <"$FILE" >"$FILE.rot13"
done &&
echo "*.file filter=protocol" >.gitattributes &&
@@ -616,7 +617,7 @@ test_expect_success PERL 'process filter should restart after unexpected write f
# Smudge failed
! test_cmp smudge-write-fail.o smudge-write-fail.r &&
- "$TEST_ROOT/rot13.sh" <smudge-write-fail.o >expected &&
+ rot13.sh <smudge-write-fail.o >expected &&
git cat-file blob :smudge-write-fail.r >actual &&
test_cmp expected actual
)
--
2.11.0.rc0.258.gf434c15
^ permalink raw reply related
* [PATCH 1/4] t0021: use write_script to create rot13 shell script
From: Jeff King @ 2016-11-02 18:17 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Lars Schneider, Junio C Hamano, git
In-Reply-To: <20161102181625.e2uprqdlzl7z2xrz@sigill.intra.peff.net>
This avoids us fooling around with $SHELL_PATH and the
executable bit ourselves.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0021-conversion.sh | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index a20b9f58e..dfde22549 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -6,13 +6,11 @@ test_description='blob conversion via gitattributes'
TEST_ROOT="$(pwd)"
-cat <<EOF >"$TEST_ROOT/rot13.sh"
-#!$SHELL_PATH
+write_script <<\EOF "$TEST_ROOT/rot13.sh"
tr \
'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ' \
'nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM'
EOF
-chmod +x "$TEST_ROOT/rot13.sh"
generate_random_characters () {
LEN=$1
--
2.11.0.rc0.258.gf434c15
^ permalink raw reply related
* [PATCH 0/4] t0021 perl portability fixups
From: Jeff King @ 2016-11-02 18:16 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Lars Schneider, Junio C Hamano, git
In-Reply-To: <20161102174021.awhmbmuffbzv5b7t@sigill.intra.peff.net>
This series should fix the portability problem you saw (in the final
patch), and fixes the failure to use PERL_PATH along the way.
[1/4]: t0021: use write_script to create rot13 shell script
[2/4]: t0021: put $TEST_ROOT in $PATH
[3/4]: t0021: use $PERL_PATH for rot13-filter.pl
[4/4]: t0021: fix filehandle usage on older perl
t/t0021-conversion.sh | 30 ++++++++++++++++--------------
t/t0021/rot13-filter.pl | 2 +-
2 files changed, 17 insertions(+), 15 deletions(-)
mode change 100755 => 100644 t/t0021/rot13-filter.pl
^ permalink raw reply
* Re: [PATCH v11 13/14] convert: add filter.<driver>.process option
From: Johannes Sixt @ 2016-11-02 18:03 UTC (permalink / raw)
To: larsxschneider; +Cc: git, gitster, jnareb, peff, ramsay, tboegi
In-Reply-To: <20161016232038.84951-14-larsxschneider@gmail.com>
Am 17.10.2016 um 01:20 schrieb larsxschneider@gmail.com:
> +# Compare two files and ensure that `clean` and `smudge` respectively are
> +# called at least once if specified in the `expect` file. The actual
> +# invocation count is not relevant because their number can vary.
> +# c.f. http://public-inbox.org/git/xmqqshv18i8i.fsf@gitster.mtv.corp.google.com/
> +test_cmp_count () {
> + expect=$1
> + actual=$2
> + for FILE in "$expect" "$actual"
> + do
> + sort "$FILE" | uniq -c | sed "s/^[ ]*//" |
> + sed "s/^\([0-9]\) IN: clean/x IN: clean/" |
> + sed "s/^\([0-9]\) IN: smudge/x IN: smudge/" >"$FILE.tmp" &&
This is not sufficiently portable. Some versions of uniq write the
count left-adjusted, not right-adjusted. How about this on top:
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index a20b9f58e3..f60858c517 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -40,10 +40,9 @@ test_cmp_count () {
actual=$2
for FILE in "$expect" "$actual"
do
- sort "$FILE" | uniq -c | sed "s/^[ ]*//" |
- sed "s/^\([0-9]\) IN: clean/x IN: clean/" |
- sed "s/^\([0-9]\) IN: smudge/x IN: smudge/" >"$FILE.tmp" &&
- mv "$FILE.tmp" "$FILE"
+ sort "$FILE" | uniq -c |
+ sed -e "s/^ *[0-9][0-9]* *IN: /x IN: /" >"$FILE.tmp" &&
+ mv "$FILE.tmp" "$FILE" || return
done &&
test_cmp "$expect" "$actual"
}
-- Hannes
^ permalink raw reply related
* Re: What's cooking in git.git (Oct 2016, #09; Mon, 31)
From: Lars Schneider @ 2016-11-02 17:44 UTC (permalink / raw)
To: Torsten Bögershausen, mlbright; +Cc: Junio C Hamano, git
In-Reply-To: <20161102170415.GA6420@tb-raspi>
On 2 Nov 2016, at 17:04, Torsten Bögershausen <tboegi@web.de> wrote:
>> * ls/filter-process (2016-10-17) 14 commits
>> (merged to 'next' on 2016-10-19 at ffd0de042c)
>
> Some (late, as I recently got a new battery for the Mac OS 10.6 test system)
> comments:
> t0021 failes here:
>
>
> Can't locate object method "flush" via package "IO::Handle" at /Users/tb/projects/git/git.next/t/t0021/rot13-filter.pl line 90.
> fatal: The remote end hung up unexpectedly
>
>
> perl itself is 5.10 and we use the one shipped with Mac OS.
> Why that ?
Thanks for reporting! It surprises me that flush is not available. I don't have a 10.6 system to tests. Where you able to reproduce the problem on a newer system with an older Perl, too?
@Martin: do you have a clue?
I can't debug the issue right now (only on mobile) but I will look into it on Friday!
> t0021 uses the hard-coded path:
> t0021/rot13-filter.pl (around line 345) and the nice macro
> PERL_PATH from the Makefile is fully t
Can you check the line number? Rot13-filter.pl has only 192 lines. I'll look into the PERL_PATH.
> Commenting out the different "flush" makes the test hang, and I haven't digged further.
That would be expected because the pl file writes output to a file which is checked by the calling sh script.
Thanks,
Lars
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2016, #09; Mon, 31)
From: Johannes Sixt @ 2016-11-02 17:43 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Junio C Hamano, git
In-Reply-To: <20161102170415.GA6420@tb-raspi>
Am 02.11.2016 um 18:04 schrieb Torsten Bögershausen:
>> * ls/filter-process (2016-10-17) 14 commits
>> (merged to 'next' on 2016-10-19 at ffd0de042c)
>
> Some (late, as I recently got a new battery for the Mac OS 10.6 test system)
> comments:
> t0021 failes here:
>
>
> Can't locate object method "flush" via package "IO::Handle" at /Users/tb/projects/git/git.next/t/t0021/rot13-filter.pl line 90.
> fatal: The remote end hung up unexpectedly
>
>
> perl itself is 5.10 and we use the one shipped with Mac OS.
> Why that ?
> t0021 uses the hard-coded path:
> t0021/rot13-filter.pl (around line 345) and the nice macro
> PERL_PATH from the Makefile is fully ignored.
>
> Commenting out the different "flush" makes the test hang, and I haven't digged further.
>
https://public-inbox.org/git/e8deda5f-11a6-1463-4fc5-25454084ccb1@kdbg.org/
-- Hannes
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2016, #09; Mon, 31)
From: Jeff King @ 2016-11-02 17:40 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Lars Schneider, Junio C Hamano, git
In-Reply-To: <20161102170415.GA6420@tb-raspi>
On Wed, Nov 02, 2016 at 05:04:15PM +0000, Torsten Bögershausen wrote:
> > * ls/filter-process (2016-10-17) 14 commits
> > (merged to 'next' on 2016-10-19 at ffd0de042c)
>
> Some (late, as I recently got a new battery for the Mac OS 10.6 test system)
> comments:
> t0021 failes here:
>
>
> Can't locate object method "flush" via package "IO::Handle" at /Users/tb/projects/git/git.next/t/t0021/rot13-filter.pl line 90.
> fatal: The remote end hung up unexpectedly
>
>
> perl itself is 5.10 and we use the one shipped with Mac OS.
Wow, haven't seen that bug in a while[1]. The problem is that STDIN is a
filehandle object, but older versions of perl do not automatically load
IO::Handle to get all of the methods. This was fixed in perl 5.13.
We can work around it with:
use IO::Handle;
at the top of the script. That should work everywhere, as IO::Handle has
been part of the core system for ages. But another option would be to
just turn on autoflush, with:
$| = 1;
at the top of the script (though it looks like we flush $debug, too, so
we'd probably need to "select $debug; $| = 1" there, too). The "use"
command is preferable IMHO.
> Why that ?
> t0021 uses the hard-coded path:
> t0021/rot13-filter.pl (around line 345) and the nice macro
> PERL_PATH from the Makefile is fully ignored.
Yeah, we should be using PERL_PATH. Doing so inside the filter config
value is probably a pain due to shell quoting issues. But we could
use write_script() to get a local copy.
I'll see if I can work up a patch.
-Peff
[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=261953
^ permalink raw reply
* [PATCH v3 4/5] trailer: have function to describe trailer layout
From: Jonathan Tan @ 2016-11-02 17:29 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster
In-Reply-To: <cover.1478107666.git.jonathantanmy@google.com>
Create a function that, taking a string, describes the position of its
trailer block (if available) and the contents thereof, and make trailer
use it. This makes it easier for other Git components, in the future, to
interpret trailer blocks in the same way as trailer.
In a subsequent patch, another component will be made to use this.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
trailer.c | 118 +++++++++++++++++++++++++++++++++++++++++++-------------------
trailer.h | 25 +++++++++++++
2 files changed, 107 insertions(+), 36 deletions(-)
diff --git a/trailer.c b/trailer.c
index afbff4b..bc6893b 100644
--- a/trailer.c
+++ b/trailer.c
@@ -46,6 +46,8 @@ static LIST_HEAD(conf_head);
static char *separators = ":";
+static int configured;
+
#define TRAILER_ARG_STRING "$ARG"
static const char *git_generated_prefixes[] = {
@@ -546,6 +548,17 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb)
return 0;
}
+static void ensure_configured(void)
+{
+ if (configured)
+ return;
+
+ /* Default config must be setup first */
+ git_config(git_trailer_default_config, NULL);
+ git_config(git_trailer_config, NULL);
+ configured = 1;
+}
+
static const char *token_from_item(struct arg_item *item, char *tok)
{
if (item->conf.key)
@@ -873,59 +886,43 @@ static int process_input_file(FILE *outfile,
const char *str,
struct list_head *head)
{
- int patch_start, trailer_start, trailer_end;
+ struct trailer_info info;
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
- struct trailer_item *last = NULL;
- struct strbuf *trailer, **trailer_lines, **ptr;
+ int i;
- patch_start = find_patch_start(str);
- trailer_end = find_trailer_end(str, patch_start);
- trailer_start = find_trailer_start(str, trailer_end);
+ trailer_info_get(&info, str);
/* Print lines before the trailers as is */
- fwrite(str, 1, trailer_start, outfile);
+ fwrite(str, 1, info.trailer_start - str, outfile);
- if (!ends_with_blank_line(str, trailer_start))
+ if (!info.blank_line_before_trailer)
fprintf(outfile, "\n");
- /* Parse trailer lines */
- trailer_lines = strbuf_split_buf(str + trailer_start,
- trailer_end - trailer_start,
- '\n',
- 0);
- for (ptr = trailer_lines; *ptr; ptr++) {
+ for (i = 0; i < info.trailer_nr; i++) {
int separator_pos;
- trailer = *ptr;
- if (trailer->buf[0] == comment_line_char)
- continue;
- if (last && isspace(trailer->buf[0])) {
- struct strbuf sb = STRBUF_INIT;
- strbuf_addf(&sb, "%s\n%s", last->value, trailer->buf);
- strbuf_strip_suffix(&sb, "\n");
- free(last->value);
- last->value = strbuf_detach(&sb, NULL);
+ char *trailer = info.trailers[i];
+ if (trailer[0] == comment_line_char)
continue;
- }
- separator_pos = find_separator(trailer->buf, separators);
+ separator_pos = find_separator(trailer, separators);
if (separator_pos >= 1) {
- parse_trailer(&tok, &val, NULL, trailer->buf,
+ parse_trailer(&tok, &val, NULL, trailer,
separator_pos);
- last = add_trailer_item(head,
- strbuf_detach(&tok, NULL),
- strbuf_detach(&val, NULL));
+ add_trailer_item(head,
+ strbuf_detach(&tok, NULL),
+ strbuf_detach(&val, NULL));
} else {
- strbuf_addbuf(&val, trailer);
+ strbuf_addstr(&val, trailer);
strbuf_strip_suffix(&val, "\n");
add_trailer_item(head,
NULL,
strbuf_detach(&val, NULL));
- last = NULL;
}
}
- strbuf_list_free(trailer_lines);
- return trailer_end;
+ trailer_info_release(&info);
+
+ return info.trailer_end - str;
}
static void free_all(struct list_head *head)
@@ -976,9 +973,7 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str
int trailer_end;
FILE *outfile = stdout;
- /* Default config must be setup first */
- git_config(git_trailer_default_config, NULL);
- git_config(git_trailer_config, NULL);
+ ensure_configured();
read_input_file(&sb, file);
@@ -1005,3 +1000,54 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str
strbuf_release(&sb);
}
+
+void trailer_info_get(struct trailer_info *info, const char *str)
+{
+ int patch_start, trailer_end, trailer_start;
+ struct strbuf **trailer_lines, **ptr;
+ char **trailer_strings = NULL;
+ size_t nr = 0, alloc = 0;
+ char **last = NULL;
+
+ ensure_configured();
+
+ patch_start = find_patch_start(str);
+ trailer_end = find_trailer_end(str, patch_start);
+ trailer_start = find_trailer_start(str, trailer_end);
+
+ trailer_lines = strbuf_split_buf(str + trailer_start,
+ trailer_end - trailer_start,
+ '\n',
+ 0);
+ for (ptr = trailer_lines; *ptr; ptr++) {
+ if (last && isspace((*ptr)->buf[0])) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_attach(&sb, *last, strlen(*last), strlen(*last));
+ strbuf_addbuf(&sb, *ptr);
+ *last = strbuf_detach(&sb, NULL);
+ continue;
+ }
+ ALLOC_GROW(trailer_strings, nr + 1, alloc);
+ trailer_strings[nr] = strbuf_detach(*ptr, NULL);
+ last = find_separator(trailer_strings[nr], separators) >= 1
+ ? &trailer_strings[nr]
+ : NULL;
+ nr++;
+ }
+ strbuf_list_free(trailer_lines);
+
+ info->blank_line_before_trailer = ends_with_blank_line(str,
+ trailer_start);
+ info->trailer_start = str + trailer_start;
+ info->trailer_end = str + trailer_end;
+ info->trailers = trailer_strings;
+ info->trailer_nr = nr;
+}
+
+void trailer_info_release(struct trailer_info *info)
+{
+ int i;
+ for (i = 0; i < info->trailer_nr; i++)
+ free(info->trailers[i]);
+ free(info->trailers);
+}
diff --git a/trailer.h b/trailer.h
index 36b40b8..65cc5d7 100644
--- a/trailer.h
+++ b/trailer.h
@@ -1,7 +1,32 @@
#ifndef TRAILER_H
#define TRAILER_H
+struct trailer_info {
+ /*
+ * True if there is a blank line before the location pointed to by
+ * trailer_start.
+ */
+ int blank_line_before_trailer;
+
+ /*
+ * Pointers to the start and end of the trailer block found. If there
+ * is no trailer block found, these 2 pointers point to the end of the
+ * input string.
+ */
+ const char *trailer_start, *trailer_end;
+
+ /*
+ * Array of trailers found.
+ */
+ char **trailers;
+ size_t trailer_nr;
+};
+
void process_trailers(const char *file, int in_place, int trim_empty,
struct string_list *trailers);
+void trailer_info_get(struct trailer_info *info, const char *str);
+
+void trailer_info_release(struct trailer_info *info);
+
#endif /* TRAILER_H */
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v3 5/5] sequencer: use trailer's trailer layout
From: Jonathan Tan @ 2016-11-02 17:29 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster
In-Reply-To: <cover.1478107666.git.jonathantanmy@google.com>
Make sequencer use trailer.c's trailer layout definition, as opposed to
parsing the footer by itself. This makes "commit -s", "cherry-pick -x",
and "format-patch --signoff" consistent with trailer, allowing
non-trailer lines and multiple-line trailers in trailer blocks under
certain conditions, and therefore suppressing the extra newline in those
cases.
Consistency with trailer extends to respecting trailer configs. Tests
have been included to show that.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
sequencer.c | 75 +++++++++---------------------------------------
t/t3511-cherry-pick-x.sh | 16 +++++++++--
t/t4014-format-patch.sh | 37 ++++++++++++++++++++----
t/t7501-commit.sh | 36 +++++++++++++++++++++++
4 files changed, 95 insertions(+), 69 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 5fd75f3..d64c973 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -16,6 +16,7 @@
#include "refs.h"
#include "argv-array.h"
#include "quote.h"
+#include "trailer.h"
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -56,30 +57,6 @@ static const char *get_todo_path(const struct replay_opts *opts)
return git_path_todo_file();
}
-static int is_rfc2822_line(const char *buf, int len)
-{
- int i;
-
- for (i = 0; i < len; i++) {
- int ch = buf[i];
- if (ch == ':')
- return 1;
- if (!isalnum(ch) && ch != '-')
- break;
- }
-
- return 0;
-}
-
-static int is_cherry_picked_from_line(const char *buf, int len)
-{
- /*
- * We only care that it looks roughly like (cherry picked from ...)
- */
- return len > strlen(cherry_picked_prefix) + 1 &&
- starts_with(buf, cherry_picked_prefix) && buf[len - 1] == ')';
-}
-
/*
* Returns 0 for non-conforming footer
* Returns 1 for conforming footer
@@ -89,49 +66,25 @@ static int is_cherry_picked_from_line(const char *buf, int len)
static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
int ignore_footer)
{
- char prev;
- int i, k;
- int len = sb->len - ignore_footer;
- const char *buf = sb->buf;
- int found_sob = 0;
-
- /* footer must end with newline */
- if (!len || buf[len - 1] != '\n')
- return 0;
+ struct trailer_info info;
+ int i;
+ int found_sob = 0, found_sob_last = 0;
- prev = '\0';
- for (i = len - 1; i > 0; i--) {
- char ch = buf[i];
- if (prev == '\n' && ch == '\n') /* paragraph break */
- break;
- prev = ch;
- }
+ trailer_info_get(&info, sb->buf);
- /* require at least one blank line */
- if (prev != '\n' || buf[i] != '\n')
+ if (info.trailer_start == info.trailer_end)
return 0;
- /* advance to start of last paragraph */
- while (i < len - 1 && buf[i] == '\n')
- i++;
-
- for (; i < len; i = k) {
- int found_rfc2822;
-
- for (k = i; k < len && buf[k] != '\n'; k++)
- ; /* do nothing */
- k++;
+ for (i = 0; i < info.trailer_nr; i++)
+ if (sob && !strncmp(info.trailers[i], sob->buf, sob->len)) {
+ found_sob = 1;
+ if (i == info.trailer_nr - 1)
+ found_sob_last = 1;
+ }
- found_rfc2822 = is_rfc2822_line(buf + i, k - i - 1);
- if (found_rfc2822 && sob &&
- !strncmp(buf + i, sob->buf, sob->len))
- found_sob = k;
+ trailer_info_release(&info);
- if (!(found_rfc2822 ||
- is_cherry_picked_from_line(buf + i, k - i - 1)))
- return 0;
- }
- if (found_sob == i)
+ if (found_sob_last)
return 3;
if (found_sob)
return 2;
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index 9cce5ae..bf0a5c9 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -25,9 +25,8 @@ Signed-off-by: B.U. Thor <buthor@example.com>"
mesg_broken_footer="$mesg_no_footer
-The signed-off-by string should begin with the words Signed-off-by followed
-by a colon and space, and then the signers name and email address. e.g.
-Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+This is not recognized as a footer because Myfooter is not a recognized token.
+Myfooter: A.U. Thor <author@example.com>"
mesg_with_footer_sob="$mesg_with_footer
Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
@@ -112,6 +111,17 @@ test_expect_success 'cherry-pick -s inserts blank line after non-conforming foot
test_cmp expect actual
'
+test_expect_success 'cherry-pick -s recognizes trailer config' '
+ pristine_detach initial &&
+ git -c "trailer.Myfooter.ifexists=add" cherry-pick -s mesg-broken-footer &&
+ cat <<-EOF >expect &&
+ $mesg_broken_footer
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'cherry-pick -x inserts blank line when conforming footer not found' '
pristine_detach initial &&
sha1=$(git rev-parse mesg-no-footer^0) &&
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index ba4902d..482112c 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1294,8 +1294,7 @@ EOF
4:Subject: [PATCH] subject
8:
10:Signed-off-by: example happens to be wrapped here.
-11:
-12:Signed-off-by: C O Mitter <committer@example.com>
+11:Signed-off-by: C O Mitter <committer@example.com>
EOF
test_cmp expected actual
'
@@ -1368,7 +1367,7 @@ EOF
test_cmp expected actual
'
-test_expect_success 'signoff: detect garbage in non-conforming footer' '
+test_expect_success 'signoff: tolerate garbage in conforming footer' '
append_signoff <<\EOF >actual &&
subject
@@ -1383,8 +1382,36 @@ EOF
8:
10:
13:Signed-off-by: C O Mitter <committer@example.com>
-14:
-15:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: respect trailer config' '
+ append_signoff <<\EOF >actual &&
+subject
+
+Myfooter: x
+Some Trash
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+11:
+12:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual &&
+
+ test_config trailer.Myfooter.ifexists add &&
+ append_signoff <<\EOF >actual &&
+subject
+
+Myfooter: x
+Some Trash
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+11:Signed-off-by: C O Mitter <committer@example.com>
EOF
test_cmp expected actual
'
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index d84897a..4003a27 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -460,6 +460,42 @@ $alt" &&
test_cmp expected actual
'
+test_expect_success 'signoff respects trailer config' '
+
+ echo 5 >positive &&
+ git add positive &&
+ git commit -s -m "subject
+
+non-trailer line
+Myfooter: x" &&
+ git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
+ (
+ echo subject
+ echo
+ echo non-trailer line
+ echo Myfooter: x
+ echo
+ echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+ ) >expected &&
+ test_cmp expected actual &&
+
+ echo 6 >positive &&
+ git add positive &&
+ git -c "trailer.Myfooter.ifexists=add" commit -s -m "subject
+
+non-trailer line
+Myfooter: x" &&
+ git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
+ (
+ echo subject
+ echo
+ echo non-trailer line
+ echo Myfooter: x
+ echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+ ) >expected &&
+ test_cmp expected actual
+'
+
test_expect_success 'multiple -m' '
>negative &&
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v3 3/5] trailer: avoid unnecessary splitting on lines
From: Jonathan Tan @ 2016-11-02 17:29 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster
In-Reply-To: <cover.1478107666.git.jonathantanmy@google.com>
trailer.c currently splits lines while processing a buffer (and also
rejoins lines when needing to invoke ignore_non_trailer).
Avoid such line splitting, except when generating the strings
corresponding to trailers (for ease of use by clients - a subsequent
patch will allow other components to obtain the layout of a trailer
block in a buffer, including the trailers themselves). The main purpose
of this is to make it easy to return pointers into the original buffer
(for a subsequent patch), but this also significantly reduces the number
of memory allocations required.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
trailer.c | 194 ++++++++++++++++++++++++++++++++------------------------------
1 file changed, 100 insertions(+), 94 deletions(-)
diff --git a/trailer.c b/trailer.c
index 9d7765e..afbff4b 100644
--- a/trailer.c
+++ b/trailer.c
@@ -102,12 +102,12 @@ static int same_trailer(struct trailer_item *a, struct arg_item *b)
return same_token(a, b) && same_value(a, b);
}
-static inline int contains_only_spaces(const char *str)
+static inline int is_blank_line(const char *str)
{
const char *s = str;
- while (*s && isspace(*s))
+ while (*s && *s != '\n' && isspace(*s))
s++;
- return !*s;
+ return !*s || *s == '\n';
}
static inline void strbuf_replace(struct strbuf *sb, const char *a, const char *b)
@@ -700,51 +700,71 @@ static void process_command_line_args(struct list_head *arg_head,
free(cl_separators);
}
-static struct strbuf **read_input_file(const char *file)
+static void read_input_file(struct strbuf *sb, const char *file)
{
- struct strbuf **lines;
- struct strbuf sb = STRBUF_INIT;
-
if (file) {
- if (strbuf_read_file(&sb, file, 0) < 0)
+ if (strbuf_read_file(sb, file, 0) < 0)
die_errno(_("could not read input file '%s'"), file);
} else {
- if (strbuf_read(&sb, fileno(stdin), 0) < 0)
+ if (strbuf_read(sb, fileno(stdin), 0) < 0)
die_errno(_("could not read from stdin"));
}
+}
- lines = strbuf_split(&sb, '\n');
+static const char *next_line(const char *str)
+{
+ const char *nl = strchrnul(str, '\n');
+ return nl + !!*nl;
+}
- strbuf_release(&sb);
+/*
+ * Return the position of the start of the last line. If len is 0, return -1.
+ */
+static int last_line(const char *buf, size_t len)
+{
+ int i;
+ if (len == 0)
+ return -1;
+ if (len == 1)
+ return 0;
+ /*
+ * Skip the last character (in addition to the null terminator),
+ * because if the last character is a newline, it is considered as part
+ * of the last line anyway.
+ */
+ i = len - 2;
- return lines;
+ for (; i >= 0; i--) {
+ if (buf[i] == '\n')
+ return i + 1;
+ }
+ return 0;
}
/*
- * Return the (0 based) index of the start of the patch or the line
- * count if there is no patch in the message.
+ * Return the position of the start of the patch or the length of str if there
+ * is no patch in the message.
*/
-static int find_patch_start(struct strbuf **lines, int count)
+static int find_patch_start(const char *str)
{
- int i;
+ const char *s;
- /* Get the start of the patch part if any */
- for (i = 0; i < count; i++) {
- if (starts_with(lines[i]->buf, "---"))
- return i;
+ for (s = str; *s; s = next_line(s)) {
+ if (starts_with(s, "---"))
+ return s - str;
}
- return count;
+ return s - str;
}
/*
- * Return the (0 based) index of the first trailer line or count if
- * there are no trailers. Trailers are searched only in the lines from
- * index (count - 1) down to index 0.
+ * Return the position of the first trailer line or len if there are no
+ * trailers.
*/
-static int find_trailer_start(struct strbuf **lines, int count)
+static int find_trailer_start(const char *buf, size_t len)
{
- int start, end_of_title, only_spaces = 1;
+ const char *s;
+ int end_of_title, l, only_spaces = 1;
int recognized_prefix = 0, trailer_lines = 0, non_trailer_lines = 0;
/*
* Number of possible continuation lines encountered. This will be
@@ -756,13 +776,13 @@ static int find_trailer_start(struct strbuf **lines, int count)
int possible_continuation_lines = 0;
/* The first paragraph is the title and cannot be trailers */
- for (start = 0; start < count; start++) {
- if (lines[start]->buf[0] == comment_line_char)
+ for (s = buf; s < buf + len; s = next_line(s)) {
+ if (s[0] == comment_line_char)
continue;
- if (contains_only_spaces(lines[start]->buf))
+ if (is_blank_line(s))
break;
}
- end_of_title = start;
+ end_of_title = s - buf;
/*
* Get the start of the trailers by looking starting from the end for a
@@ -770,30 +790,33 @@ static int find_trailer_start(struct strbuf **lines, int count)
* trailers, or (ii) contains at least one Git-generated trailer and
* consists of at least 25% trailers.
*/
- for (start = count - 1; start >= end_of_title; start--) {
+ for (l = last_line(buf, len);
+ l >= end_of_title;
+ l = last_line(buf, l)) {
+ const char *bol = buf + l;
const char **p;
int separator_pos;
- if (lines[start]->buf[0] == comment_line_char) {
+ if (bol[0] == comment_line_char) {
non_trailer_lines += possible_continuation_lines;
possible_continuation_lines = 0;
continue;
}
- if (contains_only_spaces(lines[start]->buf)) {
+ if (is_blank_line(bol)) {
if (only_spaces)
continue;
non_trailer_lines += possible_continuation_lines;
if (recognized_prefix &&
trailer_lines * 3 >= non_trailer_lines)
- return start + 1;
- if (trailer_lines && !non_trailer_lines)
- return start + 1;
- return count;
+ return next_line(bol) - buf;
+ else if (trailer_lines && !non_trailer_lines)
+ return next_line(bol) - buf;
+ return len;
}
only_spaces = 0;
for (p = git_generated_prefixes; *p; p++) {
- if (starts_with(lines[start]->buf, *p)) {
+ if (starts_with(bol, *p)) {
trailer_lines++;
possible_continuation_lines = 0;
recognized_prefix = 1;
@@ -801,8 +824,8 @@ static int find_trailer_start(struct strbuf **lines, int count)
}
}
- separator_pos = find_separator(lines[start]->buf, separators);
- if (separator_pos >= 1 && !isspace(lines[start]->buf[0])) {
+ separator_pos = find_separator(bol, separators);
+ if (separator_pos >= 1 && !isspace(bol[0])) {
struct list_head *pos;
trailer_lines++;
@@ -812,13 +835,13 @@ static int find_trailer_start(struct strbuf **lines, int count)
list_for_each(pos, &conf_head) {
struct arg_item *item;
item = list_entry(pos, struct arg_item, list);
- if (token_matches_item(lines[start]->buf, item,
+ if (token_matches_item(bol, item,
separator_pos)) {
recognized_prefix = 1;
break;
}
}
- } else if (isspace(lines[start]->buf[0]))
+ } else if (isspace(bol[0]))
possible_continuation_lines++;
else {
non_trailer_lines++;
@@ -829,88 +852,70 @@ static int find_trailer_start(struct strbuf **lines, int count)
;
}
- return count;
-}
-
-/* Get the index of the end of the trailers */
-static int find_trailer_end(struct strbuf **lines, int patch_start)
-{
- struct strbuf sb = STRBUF_INIT;
- int i, ignore_bytes;
-
- for (i = 0; i < patch_start; i++)
- strbuf_addbuf(&sb, lines[i]);
- ignore_bytes = ignore_non_trailer(sb.buf, sb.len);
- strbuf_release(&sb);
- for (i = patch_start - 1; i >= 0 && ignore_bytes > 0; i--)
- ignore_bytes -= lines[i]->len;
-
- return i + 1;
+ return len;
}
-static int has_blank_line_before(struct strbuf **lines, int start)
+/* Return the position of the end of the trailers. */
+static int find_trailer_end(const char *buf, size_t len)
{
- for (;start >= 0; start--) {
- if (lines[start]->buf[0] == comment_line_char)
- continue;
- return contains_only_spaces(lines[start]->buf);
- }
- return 0;
+ return len - ignore_non_trailer(buf, len);
}
-static void print_lines(FILE *outfile, struct strbuf **lines, int start, int end)
+static int ends_with_blank_line(const char *buf, size_t len)
{
- int i;
- for (i = start; lines[i] && i < end; i++)
- fprintf(outfile, "%s", lines[i]->buf);
+ int ll = last_line(buf, len);
+ if (ll < 0)
+ return 0;
+ return is_blank_line(buf + ll);
}
static int process_input_file(FILE *outfile,
- struct strbuf **lines,
+ const char *str,
struct list_head *head)
{
- int count = 0;
- int patch_start, trailer_start, trailer_end, i;
+ int patch_start, trailer_start, trailer_end;
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
struct trailer_item *last = NULL;
+ struct strbuf *trailer, **trailer_lines, **ptr;
- /* Get the line count */
- while (lines[count])
- count++;
-
- patch_start = find_patch_start(lines, count);
- trailer_end = find_trailer_end(lines, patch_start);
- trailer_start = find_trailer_start(lines, trailer_end);
+ patch_start = find_patch_start(str);
+ trailer_end = find_trailer_end(str, patch_start);
+ trailer_start = find_trailer_start(str, trailer_end);
/* Print lines before the trailers as is */
- print_lines(outfile, lines, 0, trailer_start);
+ fwrite(str, 1, trailer_start, outfile);
- if (!has_blank_line_before(lines, trailer_start - 1))
+ if (!ends_with_blank_line(str, trailer_start))
fprintf(outfile, "\n");
/* Parse trailer lines */
- for (i = trailer_start; i < trailer_end; i++) {
+ trailer_lines = strbuf_split_buf(str + trailer_start,
+ trailer_end - trailer_start,
+ '\n',
+ 0);
+ for (ptr = trailer_lines; *ptr; ptr++) {
int separator_pos;
- if (lines[i]->buf[0] == comment_line_char)
+ trailer = *ptr;
+ if (trailer->buf[0] == comment_line_char)
continue;
- if (last && isspace(lines[i]->buf[0])) {
+ if (last && isspace(trailer->buf[0])) {
struct strbuf sb = STRBUF_INIT;
- strbuf_addf(&sb, "%s\n%s", last->value, lines[i]->buf);
+ strbuf_addf(&sb, "%s\n%s", last->value, trailer->buf);
strbuf_strip_suffix(&sb, "\n");
free(last->value);
last->value = strbuf_detach(&sb, NULL);
continue;
}
- separator_pos = find_separator(lines[i]->buf, separators);
+ separator_pos = find_separator(trailer->buf, separators);
if (separator_pos >= 1) {
- parse_trailer(&tok, &val, NULL, lines[i]->buf,
+ parse_trailer(&tok, &val, NULL, trailer->buf,
separator_pos);
last = add_trailer_item(head,
strbuf_detach(&tok, NULL),
strbuf_detach(&val, NULL));
} else {
- strbuf_addbuf(&val, lines[i]);
+ strbuf_addbuf(&val, trailer);
strbuf_strip_suffix(&val, "\n");
add_trailer_item(head,
NULL,
@@ -918,6 +923,7 @@ static int process_input_file(FILE *outfile,
last = NULL;
}
}
+ strbuf_list_free(trailer_lines);
return trailer_end;
}
@@ -966,7 +972,7 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str
{
LIST_HEAD(head);
LIST_HEAD(arg_head);
- struct strbuf **lines;
+ struct strbuf sb = STRBUF_INIT;
int trailer_end;
FILE *outfile = stdout;
@@ -974,13 +980,13 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str
git_config(git_trailer_default_config, NULL);
git_config(git_trailer_config, NULL);
- lines = read_input_file(file);
+ read_input_file(&sb, file);
if (in_place)
outfile = create_in_place_tempfile(file);
/* Print the lines before the trailers */
- trailer_end = process_input_file(outfile, lines, &head);
+ trailer_end = process_input_file(outfile, sb.buf, &head);
process_command_line_args(&arg_head, trailers);
@@ -991,11 +997,11 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str
free_all(&head);
/* Print the lines after the trailers as is */
- print_lines(outfile, lines, trailer_end, INT_MAX);
+ fwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);
if (in_place)
if (rename_tempfile(&trailers_tempfile, file))
die_errno(_("could not rename temporary file to %s"), file);
- strbuf_list_free(lines);
+ strbuf_release(&sb);
}
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox