Re: [PATCH v3 0/3] Introduce log.showSignature config variable

[Mehul Jain]

On Thu, Jun 23, 2016 at 2:01 AM, Junio C Hamano  wrote:
> Mehul Jain  writes:
>
>> Add a new configuratation variable "log.showSignature" for git-log
>> and related commands. "log.showSignature=true" will enable user to
>> see GPG signature by default for git-log and related commands.
>>
>> Changes compared to v2:
>>   * A preparatory patch 1/3 has been introduced so that tests
>> in patches 2/3 and 3/3 can take advantage of it.
>
> It is unclear how this change allows the remainder to "take
> advanrage" to me.  Earlier, "signed" branch was created only when
> the GPG prerequisite is met and with this change the branch is
> always created, which is the only change as far as I can see.  But
> the tests that are added in 2 and 3 are all protected with the GPG
> prerequiste.
>
> Besides, the invocation of "git commit -S" after this change is no
> longer protected by the GPG prerequisite and it may even cause the
> 'setup' step to fail on a host without GPG.

I overlooked the GPG prerequisite when I created the "setup signed
branch" test in patch 1/3. I will send a patch to  rectify it ones
everyone agree with the idea behind this patch.

In patch 2/3 and 3/3, there are many tests which requires a branch
similar to that of "signed" branch, i.e. a branch with a commit having
GPG signature. So previously in v2, I created two new branches,
"test_sign" and "no_sign", which are identical to that of "signed"
branch. And with these branches, I wrote the tests in patch 2/3
and 3/3.

As suggested by Eric [1], rather than creating new branches, I
can take advantage of "signed" branch which already exists.
So, I created a new test to separate the creation of "signed" branch
from existing test "log --graph --show-signature". This was done
because I do not want new tests to depend on this test. If in future
someone changes this test then it will affect new tests introduced
in 2/3 and 3/3.

Now the new tests and existing one ("log --graph ... ") are using a
single branch "signed" to do there work.

If changing an existing test is not well justified here, then I can create
setup test for new tests only, without affecting the existing test.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/297648

Thanks,
Mehul
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Use docker for _some_ testing?

[Duy Nguyen]

On Wed, Jun 22, 2016 at 9:59 PM, Jeff King  wrote:
> On Wed, Jun 22, 2016 at 09:01:55PM +0200, Duy Nguyen wrote:
>
>> Which makes me think, could we use something like this to make sure
>> people (on Linux) can test more obscure cases? Sometimes there are
>> featues that require some dependencies that are not always present on
>> the developer's machine (http server is a big one, locales come close
>> second, then there will be lmdb and watchman in future...). With this,
>> said developer can do a final test run in docker covering as much as
>> possible.
>
> Neat idea. This should also cover some other distro-specific issues like
> "what's your /bin/sh look like", etc. It's a lighter-weight alternative
> to testing alternate distros in a VM.

It's lighter-weight and also helps skip preparation. Everything is
scripted, if you want distro X, it's just one command away. The VM way
would require you to go through installation process with lots of
clicking and typing (unless you go full cloud solution like OpenStack,
not really suited for single dev use).

It's not just distro-specific. Not every developer has enough stuff
installed to run all tests. My machine is svn-free for example so I
never run those tests. Same story for pcre (have it but not enable it
in git...). We can try to run both gettext and no-gettext
configuration... Devs still have to install docker this way, but at
least i could keep my laptop "clean" from unwanted stuff and still be
able to run lots of tests.

> But I think most of the interesting cases are more exotic than
> distro-to-distro. Things like HFS+ normalization, or weird shell
> toolchains on proprietary Unixes (but maybe there are docker images for
> those systems?).

I don't follow docker closely, but if it's still a container
technology (i.e. sharing host kernel) then different OSes are out of
question.

> So I dunno if it would prove that useful for day to day use or not.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pathspec: warn on empty strings as pathspec

[Emily Xie]

Awesome. Thanks, Junio---this is exciting!

As for step 2: do you have a good sense on the timing? Around how long
should I wait to submit the patch for it?

Otherwise, let me know if there is anything else that I need to do!

- Emily
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pathspec: warn on empty strings as pathspec

[Junio C Hamano]

Emily Xie  writes:

> The fix for this issue requires a two-step approach. As
> there may be existing scripts that knowingly use empty
> strings in this manner, the first step simply invokes a
> warning that (1) declares using empty strings to
> match all paths will become invalid and (2) asks the user
> to use "." if their intent was to match all.
>
> For step two, a follow-up patch several release cycles
> later will remove the warnings and throw an error instead.
>
> This patch is the first step.
>
> Signed-off-by: Emily Xie 
> Reported-by: David Turner 
> Mentored-by: Michail Denchev 
> Thanks-to: Sarah Sharp  and James Sharp 
> 
> ---

Looks sensible.  I personally do not think warn-only-once matters in
practice, but now you have implemented it, it is an additional nice
touch ;-)

Will queue.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Fast-forward able commit, otherwise fail

[Junio C Hamano]

Junio C Hamano  writes:

> However, Git as a tool is not opinionated strongly enough to make it
> hard to do these two things (independently).  I do not think it is
> unreasonable to add a new mode of "merge" that rejects a resulting
> history that is not shaped the way you like.  So far the command
> rejected --ff-only and --no-ff given together, so if an updated Git
> starts taking them together and creating a needless real merge and
> failing only when the first parent does not fast-forward to the
> second parent, nobody's existing workflow would be broken.
>
> Having said that, you need to think things through.  Sample
> questions you would want to be asking yourself are (not exhaustive):
>
>  - What is your plan to _enforce_ your project participants to use
>this new mode of operation?
>
>  - Do you _require_ your project participants to always pass a new
>option to "git merge" or "git pull"?
>
>  - Do you force them to set some new configuration variables?  
>
>  - Do you trust them once you tell them what to do?  
>
>  - How will your project's trunk get their changes?
>
>  - How you prevent some participants who misunderstood your
>instructions from pushing an incorrectly shaped history to your
>project?

Another thing to consider is that the proposed workflow would not
scale if your team becomes larger.  Requiring each and every commit
on the trunk to be a merge commit, whose second parent (i.e. the tip
of the feature branch) fast-forwards to the first parent of the
merge (i.e. you require the feature to be up-to-date), would mean
that Alice and Bob collaborating on this project would end up
working like this:

 A:git pull --ff-only origin ;# starts working
 A:git checkout -b topic-a
 A:git commit; git commit; git commit
 B:git pull --ff-only origin ;# starts working
 B:git checkout -b topic-b
 B:git commit; git commit

 A:git checkout master && git merge --ff-only --no-ff topic-a
 A:git push origin ;# happy

 B:git checkout master && git merge --ff-only --no-ff topic-b
 B:git push origin ;# fails!

 B:git fetch origin ;# starts recovering
 B:git reset --hard origin/master
 B:git merge --ff-only --no-ff topic-b ;# fails!
 B:git rebase origin/master topic-b
 B:git checkout master && git merge --ff-only --no-ff topic-b
 B:git push origin ;# hopefully nobody pushed in the meantime

The first push by Bob fails because his 'master', even though it is
a merge between the once-at-the-tip-of-public-master and topic-b
which was forked from that once-at-the-tip, it no longer fast-forwards
because Alice pushed her changes to the upstream.

And it is not sufficient to redo the previous merge after fetching
the updated upstream, because your additional "feature branch must
be up-to-date" requirement is now broken for topic-b.  Bob needs to
rebuild it on top of the latest, which includes what Alice pushed,
using rebase, do that merge again, and hope that nobody else pushed
to update the upstream in the meantime.  As you have more people
working simultanously on more features, Bob will have to spend more
time doing steps between "starts recovering" and "hopefully nobody
pushed in the meantime", because the probability is higher that
somebody other than Alice has pushed while Bob is trying to recover.

The time spend on recovery is not particularly productive, and this
workflow gives him a (wrong) incentive to do that recovery procedure
quickly to beat the other participants to become the first to push.

The workflow should instead be designed to incentivise participant
to spend more time to carefully inspect the result of his rebasing
to make sure that his changes still make sense in the context of the
updated codebase that contains changes from others since he forked
topic-b from the upstream, in order to ensure quality.

This "push quickly immediately after rebasing without carefully
checking the result of rebase" pressure is shared by a workflow that
requires a completely linear history, and not unique to your
proposed workflow, but because you also require a --no-ff merge to
the updated upstream, robbing even more time from the project
participants, it aggravates the problem.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] Refactor recv_sideband()

[Nicolas Pitre]

On Wed, 22 Jun 2016, Nicolas Pitre wrote:

> On Wed, 22 Jun 2016, Lukas Fleischer wrote:
> 
> > Before this patch, we used character buffer manipulations to split
> > messages from the sideband at line breaks and insert "remote: " at the
> > beginning of each line, using the packet size to determine the end of a
> > message. However, since it is safe to assume that diagnostic messages
> > from the sideband never contain NUL characters, we can also
> > NUL-terminate the buffer, use strpbrk() for splitting lines and use
> > format strings to insert the prefix.
> > 
> > A static strbuf is used for constructing the output which is then
> > printed using a single write() call, such that the atomicity of the
> > output is preserved. See 9ac13ec (atomic write for sideband remote
> > messages, 2006-10-11) for details.
> > 
> > Helped-by: Nicolas Pitre 
> > Signed-off-by: Lukas Fleischer 
> 
> The patch is buggy.
> 
> Once patched, the code looks like this:
> 
> case 2:
> b = buf + 1;
> /*
>  * Append a suffix to each nonempty line to clear the
>  * end of the screen line.
>  */
> while ((brk = strpbrk(b, "\n\r"))) {
> int linelen = brk - b;
> if (linelen > 0) {
> strbuf_addf(, "%.*s%s%c",
> linelen, b, suffix, *brk);
> } else {
> strbuf_addf(, "%c", *brk);
> }
> xwrite(STDERR_FILENO, outbuf.buf, outbuf.len);
> strbuf_reset();
> strbuf_addf(, "%s", PREFIX);
> b = brk + 1;
> }
> if (*b) {
> xwrite(STDERR_FILENO, outbuf.buf, outbuf.len);
> /* Incomplete line, skip the next prefix. */
> strbuf_reset();
> }
> continue;
> 
> You are probably missing a strbuf_addf() before the last xwrite().

In fact, you could simply append the partial line to the strbuf and make 
it the prefix for the next packet rather than writing a partial line.  
You'd only have to write a partial line before leaving the function if 
the strbuf is not empty at that point.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] pathspec: warn on empty strings as pathspec

[Emily Xie]

Currently, passing an empty string as a pathspec results in
a match on all paths. This is because a pathspec match is a
leading substring match that honors directory boundaries.
So, just as pathspec "dir/" (or "dir") matches "dir/file",
"" matches "file".

However, this causes problems. Namely, a user's
carelessly-written script could accidentally assign an
empty string to a variable that then gets passed to a Git
command invocation, e.g.:

git rm -r "$path"
git add "$path"

which would unintentionally affect all paths in the current
directory.

The fix for this issue requires a two-step approach. As
there may be existing scripts that knowingly use empty
strings in this manner, the first step simply invokes a
warning that (1) declares using empty strings to
match all paths will become invalid and (2) asks the user
to use "." if their intent was to match all.

For step two, a follow-up patch several release cycles
later will remove the warnings and throw an error instead.

This patch is the first step.

Signed-off-by: Emily Xie 
Reported-by: David Turner 
Mentored-by: Michail Denchev 
Thanks-to: Sarah Sharp  and James Sharp 
---
 pathspec.c | 11 +--
 t/t3600-rm.sh  |  5 +
 t/t3700-add.sh |  5 +
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/pathspec.c b/pathspec.c
index c9e9b6c..02aa691 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -364,7 +364,7 @@ void parse_pathspec(struct pathspec *pathspec,
 {
struct pathspec_item *item;
const char *entry = argv ? *argv : NULL;
-   int i, n, prefixlen, nr_exclude = 0;
+   int i, n, prefixlen, warn_empty_string, nr_exclude = 0;
 
memset(pathspec, 0, sizeof(*pathspec));
 
@@ -402,8 +402,15 @@ void parse_pathspec(struct pathspec *pathspec,
}
 
n = 0;
-   while (argv[n])
+   warn_empty_string = 1;
+   while (argv[n]) {
+   if (*argv[n] == '\0' && warn_empty_string) {
+   warning(_("empty strings as pathspecs will be made 
invalid in upcoming releases. "
+ "please use . instead if you meant to match 
all paths"));
+   warn_empty_string = 0;
+   }
n++;
+   }
 
pathspec->nr = n;
ALLOC_ARRAY(pathspec->items, n);
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index d046d98..14f0edc 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -881,4 +881,9 @@ test_expect_success 'rm files with two different errors' '
test_i18ncmp expect actual
 '
 
+test_expect_success 'rm empty string should invoke warning' '
+   git rm -rf "" 2>output &&
+   test_i18ngrep "warning: empty strings" output
+'
+
 test_done
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index f14a665..05379d0 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -332,4 +332,9 @@ test_expect_success 'git add --dry-run --ignore-missing of 
non-existing file out
test_i18ncmp expect.err actual.err
 '
 
+test_expect_success 'git add empty string should invoke warning' '
+   git add "" 2>output &&
+   test_i18ngrep "warning: empty strings" output
+'
+
 test_done
-- 
2.8.4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: may be bug in svn fetch no-follow-parent

[Eric Wong]

Александр Овчинников  wrote:
> Unfortunately this is not open source repository. I agree that it is 
> pointless try to handle mergeinfo (because it always fails).
> Cases when it is expensive:
> 1. delete and restore mergeinfo property
> 2. merge trunk to very old branch
> 3. create, delete, create branch with --no-follow-parent. All records in 
> mergeinfo will be hadled like new.
> 
> I already patched like this and this is helpfull, works fine and fast.

Thanks for the info.  Patch + pull request below for Junio.

> I can share only mergeinfo property

Oops, looks like your zip attachment got flagged as spam for
my mailbox and swallowed by vger.kernel.org :x

-8<
Subject: [PATCH] git-svn: skip mergeinfo handling with --no-follow-parent

For repositories without parent following enabled, finding
git parents through svn:mergeinfo or svk::parents can be
expensive and pointless.

Reported-by: Александр Овчинников 
http://mid.gmane.org/4094761466408...@web24o.yandex.ru

Signed-off-by: Eric Wong 
---
  The following changes since commit ab7797dbe95fff38d9265869ea367020046db118:

Start the post-2.9 cycle (2016-06-20 11:06:49 -0700)

  are available in the git repository at:

git://bogomips.org/git-svn.git svn-nfp-mergeinfo

  for you to fetch changes up to 6d523a3ab76cfa4ed9ae0ed9da7af43efcff3f07:

git-svn: skip mergeinfo handling with --no-follow-parent (2016-06-22 
22:48:54 +)

  
  Eric Wong (1):
git-svn: skip mergeinfo handling with --no-follow-parent

 perl/Git/SVN.pm | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index d94d01c..bee1e7d 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1905,15 +1905,22 @@ sub make_log_entry {
 
my @parents = @$parents;
my $props = $ed->{dir_prop}{$self->path};
-   if ( $props->{"svk:merge"} ) {
-   $self->find_extra_svk_parents($props->{"svk:merge"}, \@parents);
-   }
-   if ( $props->{"svn:mergeinfo"} ) {
-   my $mi_changes = $self->mergeinfo_changes
-   ($parent_path, $parent_rev,
-$self->path, $rev,
-$props->{"svn:mergeinfo"});
-   $self->find_extra_svn_parents($mi_changes, \@parents);
+   if ($self->follow_parent) {
+   my $tickets = $props->{"svk:merge"};
+   if ($tickets) {
+   $self->find_extra_svk_parents($tickets, \@parents);
+   }
+
+   my $mergeinfo_prop = $props->{"svn:mergeinfo"};
+   if ($mergeinfo_prop) {
+   my $mi_changes = $self->mergeinfo_changes(
+   $parent_path,
+   $parent_rev,
+   $self->path,
+   $rev,
+   $mergeinfo_prop);
+   $self->find_extra_svn_parents($mi_changes, \@parents);
+   }
}
 
open my $un, '>>', "$self->{dir}/unhandled.log" or croak $!;
-- 
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 8/8] use smudgeToFile filter in recursive merge

[Junio C Hamano]

Junio C Hamano  writes:

>> +int smudge_to_file = can_smudge_to_file(path);
>
> This does not compile with decl-after-statement.  I suspect other
> patches in this series have the same issue but I did not check.  Do
> you say "make DEVELOPER=1"?

As a tentative workaround, I've queued a squashable compilation fix
at the tip of the topic after queuing.  Please find it in 'pu' when
it gets pushed out sometime later today.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] doc: git-htmldocs.googlecode.com is no more

[Eric Wong]

Junio C Hamano  wrote:
> On Wed, Jun 22, 2016 at 12:00 PM, Eric Wong  wrote:
> >
> > Just wondering, who updates
> > https://kernel.org/pub/software/scm/git/docs/
> > and why hasn't it been updated in a while?
> > (currently it says Last updated 2015-06-06 at the bottom)
> 
> Nobody. It is too cumbersome to use their upload tool to update many
> files (it is geared towards updating a handful of tarballs at a time).

Alright, I've setup https://git-htmldocs.bogomips.org/ for my own
usage, at least.  It should check for updates twice an hour(*),
and plain HTTP is also available in case Let's Encrypt goes away.

Can't hurt to have more mirrors:
--8<-
#!/bin/sh
set -e
DST=/path/to/server-docroot/git-htmldocs
# my mirror of git://git.kernel.org/pub/scm/git/git-htmldocs.git:
GIT_DIR=/path/to/mirrors/git-htmldocs.git
export GIT_DIR

# rsync from a temporary dir for atomicity so nobody fetches
# a partially written file
tmp="$(mktemp -t -d htmldocs.XXX)"
git archive --format=tar HEAD | tar x -C "$tmp"
chmod 755 "$tmp"
rsync -a "$tmp/" "$DST/"
rm -rf "$tmp"

# for servers which support pre-gzipped files (e.g. gzip_static in nginx)
find "$DST" -type f -name '*.html' -o -name '*.txt' | while read file
do
gz="$file.gz"
if ! test -e "$gz" || test "$gz" -ot "$file"
then
gztmp="$gz.$$.tmp"
gzip -9 <"$file" >"$gztmp"
touch -r "$file" "$gztmp"
mv "$gztmp" "$gz"
fi
done
---
(*) On a side note, It would be nice to something like IMAP IDLE
for real-time updates of mirrors.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 8/8] use smudgeToFile filter in recursive merge

[Junio C Hamano]

Joey Hess  writes:

> @@ -781,6 +773,7 @@ static void update_file_flags(struct merge_options *o,
>   }
>   if (S_ISREG(mode) || (!has_symlinks && S_ISLNK(mode))) {
>   int fd;
> + int isreg = S_ISREG(mode);

You probably want to move this isreg business up one scope
(i.e. inside "if (update_wd) {").  Then the if () condition
for this block can use it already.

>   if (mode & 0100)
>   mode = 0777;
>   else
> @@ -788,8 +781,37 @@ static void update_file_flags(struct merge_options *o,
>   fd = open(path, O_WRONLY | O_TRUNC | O_CREAT, mode);
>   if (fd < 0)
>   die_errno(_("failed to open '%s'"), path);
> - write_in_full(fd, buf, size);
> - close(fd);
> +
> + int smudge_to_file = can_smudge_to_file(path);

This does not compile with decl-after-statement.  I suspect other
patches in this series have the same issue but I did not check.  Do
you say "make DEVELOPER=1"?

> + if (smudge_to_file) {
> + close(fd);
> + fd = 
> convert_to_working_tree_filter_to_file(path, path, buf, size);
> + if (fd < 0) {
> + /* smudgeToFile filter failed;
> +  * continue with regular file
> +  * creation. */

/*
 * Style: We format our multi-line
 * comments like this.
 */

> + smudge_to_file = 0;

Ahh, I was wondering why this is not "if (smudge_to_file) ... else ...".

> + fd = open(path, O_WRONLY | O_TRUNC | 
> O_CREAT, mode);
> + if (fd < 0)
> + die_errno(_("failed to open 
> '%s'"), path);
> + }
> + else {
> + close(fd);
> + }
> + }
> +
> + if (! smudge_to_file) {

Style: if (!smudge_to_file) {

> +test_expect_success 'smudgeToFile filter is used in merge' '
> + test_config filter.rot13.smudgeToFile ./rot13-to-file.sh &&
> +
> + git commit -m "added fstest.t" fstest.t &&
> + git checkout -b old &&
> + git reset --hard HEAD^ &&
> + git merge master &&
> +
> + test -e rot13-to-file.ran &&
> + rm -f rot13-to-file.ran &&
> +
> + cmp test fstest.t &&

"test_cmp test fstest.t"?  The difference matters when running the
test with -v option.

> + git checkout master

What happens if any of the previous steps failed?  Does the next
test get confused because you would fail to go back to the master
branch?

> +'
> +
>  test_expect_success 'smudgeToFile filter is used by git am' '
>   test_config filter.rot13.smudgeToFile ./rot13-to-file.sh &&
>  
> - git commit fstest.t -m "added fstest.t" &&
>   git format-patch HEAD^ --stdout > fstest.patch &&

Style: 

git format-patch HEAD^ --stdout >fstest.patch &&

>   git reset --hard HEAD^ &&
>   git am < fstest.patch &&

Style: 

git am http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] mingw: work around t2300's assuming non-Windows paths

[Johannes Schindelin]

Hi Junio,

On Wed, 22 Jun 2016, Junio C Hamano wrote:

> Junio C Hamano  writes:
> 
> > I think I know that well enough; please sanity check.  My
> > understanding is:
> > ...
> > The patch under discussion is the only door left for that test, and
> > a similar trickery is needed for any end-user scripts used for hooks
> > and aliases that use 'git --exec-path', if they ever want to be
> > cross-platform.
> >
> > So let's take that "if Windows do this" change to t2300 as-is.
> 
> Assuming that the sanity check passes, here is a suggested rewrite
> to explain the real problem better.

Yes, the sanity check passes, and your commit message is much better than
mine.

Thanks!
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 0/8] extend smudge/clean filters with direct file access (for pu)

[Junio C Hamano]

Joey Hess  writes:

> This is the same as v3, except rebased on top of tb/convert-peek-in-index
> to fix a build failure in pu.

This is somewhat unfortunate, as tb/convert-peek-in-index probably
needs further rerolls after getting reviewed by somebody (other than
me) and this topic will have to be rebased every time.

Let's see how it goes.

Thanks.

>
> Joey Hess (8):
>   clarify %f documentation
>   add smudgeToFile and cleanFromFile filter configs
>   use cleanFromFile in git add
>   use smudgeToFile in git checkout etc
>   warn on unusable smudgeToFile/cleanFromFile config
>   better recovery from failure of smudgeToFile filter
>   use smudgeToFile filter in git am
>   use smudgeToFile filter in recursive merge
>
>  Documentation/config.txt|  18 -
>  Documentation/gitattributes.txt |  42 
>  builtin/apply.c |  16 +
>  convert.c   | 147 
> +++-
>  convert.h   |  11 +++
>  entry.c |  53 +++
>  merge-recursive.c   |  42 +---
>  sha1_file.c |  44 ++--
>  t/t0021-conversion.sh   | 115 +++
>  9 files changed, 441 insertions(+), 47 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 3/8] use cleanFromFile in git add

[Joey Hess]

Includes test cases.

Signed-off-by: Joey Hess 
---
 sha1_file.c   | 44 ++--
 t/t0021-conversion.sh | 36 
 2 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 55604b6..df62eaf 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3339,6 +3339,31 @@ static int index_stream_convert_blob(unsigned char 
*sha1, int fd,
return ret;
 }
 
+static int index_from_file_convert_blob(unsigned char *sha1,
+ const char *path, unsigned flags)
+{
+   int ret;
+   const int write_object = flags & HASH_WRITE_OBJECT;
+   const int valid_sha1 = flags & HASH_USE_SHA_NOT_PATH;
+   struct strbuf sbuf = STRBUF_INIT;
+
+   assert(path);
+   assert(can_clean_from_file(path));
+
+   convert_to_git_filter_from_file(path, ,
+write_object ? safe_crlf : SAFE_CRLF_FALSE,
+valid_sha1 ? sha1 : NULL);
+
+   if (write_object)
+   ret = write_sha1_file(sbuf.buf, sbuf.len, typename(OBJ_BLOB),
+ sha1);
+   else
+   ret = hash_sha1_file(sbuf.buf, sbuf.len, typename(OBJ_BLOB),
+sha1);
+   strbuf_release();
+   return ret;
+}
+
 static int index_pipe(unsigned char *sha1, int fd, enum object_type type,
  const char *path, unsigned flags)
 {
@@ -3433,12 +3458,19 @@ int index_path(unsigned char *sha1, const char *path, 
struct stat *st, unsigned
 
switch (st->st_mode & S_IFMT) {
case S_IFREG:
-   fd = open(path, O_RDONLY);
-   if (fd < 0)
-   return error_errno("open(\"%s\")", path);
-   if (index_fd(sha1, fd, st, OBJ_BLOB, path, flags) < 0)
-   return error("%s: failed to insert into database",
-path);
+   if (can_clean_from_file(path)) {
+   if (index_from_file_convert_blob(sha1, path, flags) < 0)
+   return error("%s: failed to insert into 
database",
+path);
+   }
+   else {
+   fd = open(path, O_RDONLY);
+   if (fd < 0)
+   return error_errno("open(\"%s\")", path);
+   if (index_fd(sha1, fd, st, OBJ_BLOB, path, flags) < 0)
+   return error("%s: failed to insert into 
database",
+path);
+   }
break;
case S_IFLNK:
if (strbuf_readlink(, path, st->st_size))
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 7bac2bc..407d5d6 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -12,6 +12,14 @@ tr \
 EOF
 chmod +x rot13.sh
 
+cat .gitattributes &&
+
+   cat test >fstest.t &&
+   git add fstest.t &&
+   test -e rot13-from-file.ran &&
+   rm -f rot13-from-file.ran &&
+
+   rm -f fstest.t &&
+   git checkout -- fstest.t &&
+   cmp test fstest.t
+'
+
+test_expect_success 'cleanFromFile filter is not used when clean filter is not 
configured' '
+   test_config filter.noclean.smudge ./rot13.sh &&
+   test_config filter.noclean.cleanFromFile ./rot13-from-file.sh &&
+
+   echo "*.no filter=noclean" >.gitattributes &&
+
+   cat test >test.no &&
+   git add test.no &&
+   test ! -e rot13-from-file.ran &&
+   git cat-file blob :test.no >actual &&
+   cmp test actual
+'
+
 test_done
-- 
2.9.0.587.ga3bedf2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 2/8] add smudgeToFile and cleanFromFile filter configs

[Joey Hess]

This adds new smudgeToFile and cleanFromFile filter commands,
which are similar to smudge and clean but allow direct access to files on
disk.

This interface can be much more efficient when operating on large files,
because the whole file content does not need to be streamed through the
filter. It even allows for things like cleanFromFile commands that avoid
reading the whole content of the file, and for smudgeToFile commands that
populate a work tree file using an efficient Copy On Write operation.

The new filter commands will not be used for all filtering. They are
efficient to use when git add is adding a file, or when the work tree is
being updated, but not a good fit when git is internally filtering blob
objects in memory for eg, a diff.

So, a user who wants to use smudgeToFile should also provide a smudge
command to be used in cases where smudgeToFile is not used. And ditto
with cleanFromFile and clean. To avoid foot-shooting configurations, the
new commands are not used unless the old commands are also configured.

That also ensures that a filter driver configuration that includes these
new commands will work, although less efficiently, when used with an older
version of git that does not support them.

Signed-off-by: Joey Hess 
---
 Documentation/config.txt|  18 ++-
 Documentation/gitattributes.txt |  37 +
 convert.c   | 114 ++--
 convert.h   |  11 
 4 files changed, 162 insertions(+), 18 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0d4095f..b76dad3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1306,15 +1306,29 @@ format.useAutoBase::
format-patch by default.
 
 filter..clean::
-   The command which is used to convert the content of a worktree
+   The command which is used as a filter to convert the content of a 
worktree
file to a blob upon checkin.  See linkgit:gitattributes[5] for
details.
 
 filter..smudge::
-   The command which is used to convert the content of a blob
+   The command which is used as a filter to convert the content of a blob
object to a worktree file upon checkout.  See
linkgit:gitattributes[5] for details.
 
+filter..cleanFromFile::
+   Similar to filter..clean but the specified command
+   directly accesses a worktree file on disk, rather than
+   receiving the file content from standard input.
+   Only used when filter..clean is also configured.
+   See linkgit:gitattributes[5] for details.
+
+filter..smudgeToFile::
+   Similar to filter..smudge but the specified command
+   writes the content of a blob directly to a worktree file,
+   rather than to standard output.
+   Only used when filter..smudge is also configured.
+   See linkgit:gitattributes[5] for details.
+
 fsck.::
Allows overriding the message type (error, warn or ignore) of a
specific message ID such as `missingEmail`.
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 197ece8..a58aafc 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -385,6 +385,43 @@ 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.
 
+There are two extra commands "cleanFromFile" and "smudgeToFile", which
+can optionally be set in a filter driver. These are similar to the "clean"
+and "smudge" commands, but avoid needing to pipe the contents of files
+through the filters, and instead read/write files in the filesystem.
+This can be more efficient when using filters with large files that are not
+directly stored in the repository.
+
+Both "cleanFromFile" and "smudgeToFile" are provided a path as an
+added parameter after the configured command line.
+
+The "cleanFromFile" command is provided the path to the file that
+it should clean. Like the "clean" command, it should output the cleaned
+version to standard output.
+
+The "smudgeToFile" command is provided a path to the file that it
+should write to. (This file will already exist, as an empty file that can
+be written to or replaced.) Like the "smudge" command, "smudgeToFile"
+is fed the blob object from its standard input.
+
+Some git operations that need to apply filters cannot use "cleanFromFile"
+and "smudgeToFile", since the files are not present to disk. So, to avoid
+inconsistent behavior, "cleanFromFile" will only be used if "clean" is
+also configured, and "smudgeToFile" will only be used if "smudge" is also
+configured.
+
+An example large file storage filter driver using cleanFromFile and
+smudgeToFile follows:
+
+
+[filter "bigfiles"]
+   clean = store-bigfile --from-stdin
+   cleanFromFile = store-bigfile --from-file
+   smudge = 

Re: What's cooking in git.git (Jun 2016, #05; Thu, 16)

[Joey Hess]

Torsten Bögershausen wrote:
> There is a conflict in pu:
> "jh/clean-smudge-annex" does not work together with "tb/convert-peek-in-index"
> 
> (And currently pu didn't compile)

I'm sending a v4 of jh/clean-smudge-annex that is rebased on top of
tb/convert-peek-in-index to fix this.

> (I will hopefully be able to do a separate review of the smudge/clean patch)

Would be appreciated. It'll be 2 weeks until I can work on this any more.

> (And jo...@joeyh.name is not reachable from web.de)

I'd like to fix whatever's broken; you could send details out of band to
joeyh...@gmail.com

-- 
see shy jo


signature.asc
Description: PGP signature

[PATCH v4 7/8] use smudgeToFile filter in git am

[Joey Hess]

git am updates the work tree and so should use the smudgeToFile filter.

This includes some refactoring into convert_to_working_tree_filter_to_file
to make it check the file after running the smudgeToFile command, and clean
up from a failing command.

Signed-off-by: Joey Hess 
---
 builtin/apply.c   | 16 
 convert.c | 19 +--
 entry.c   | 15 +++
 t/t0021-conversion.sh | 13 +
 4 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index b24c6ba..b027ab7 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4250,6 +4250,22 @@ static int try_create_file(const char *path, unsigned 
int mode, const char *buf,
if (fd < 0)
return -1;
 
+   if (can_smudge_to_file(path)) {
+   close(fd);
+   fd = convert_to_working_tree_filter_to_file(path, path, buf, 
size);
+   if (fd < 0) {
+   /* smudgeToFile filter failed; continue
+* with regular file creation. */
+   fd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode & 
0100) ? 0777 : 0666);
+   if (fd < 0)
+   return -1;
+   }
+   else {
+   close(fd);
+   return 0;
+   }
+   }
+
if (convert_to_working_tree(path, buf, size, )) {
size = nbuf.len;
buf  = nbuf.buf;
diff --git a/convert.c b/convert.c
index 378a3bd..4fe89e0 100644
--- a/convert.c
+++ b/convert.c
@@ -1048,13 +1048,28 @@ int convert_to_working_tree(const char *path, const 
char *src, size_t len, struc
return convert_to_working_tree_internal(path, NULL, src, len, dst, 0);
 }
 
+/* Returns fd open to read the worktree file on success.
+ * On failure, the worktree file will not exist. */
 int convert_to_working_tree_filter_to_file(const char *path, const char 
*destpath, const char *src, size_t len)
 {
struct strbuf output = STRBUF_INIT;
-   int ret = convert_to_working_tree_internal(path, destpath, src, len, 
, 0);
+   int ok = convert_to_working_tree_internal(path, destpath, src, len, 
, 0);
/* The smudgeToFile filter stdout is not used. */
strbuf_release();
-   return ret;
+   if (ok) {
+   /* Open the file to make sure that it's present
+* (and readable) after the command populated it. */
+   int fd = open(path, O_RDONLY);
+   if (fd < 0)
+   unlink(path);
+   return fd;
+   }
+   else {
+   /* The command could have created the file before failing,
+* so delete it. */
+   unlink(path);
+   return -1;
+   }
 }
 
 int renormalize_buffer(const char *path, const char *src, size_t len, struct 
strbuf *dst)
diff --git a/entry.c b/entry.c
index 8322127..2f7c4fd 100644
--- a/entry.c
+++ b/entry.c
@@ -190,13 +190,10 @@ static int write_entry(struct cache_entry *ce,
 
if (smudge_to_file) {
close(fd);
-   if (! convert_to_working_tree_filter_to_file(ce->name, 
path, new, size)) {
+   fd = convert_to_working_tree_filter_to_file(ce->name, 
path, new, size);
+   if (fd < 0) {
smudge_to_file = 0;
-   /* The failing smudgeToFile filter may have
-* deleted or replaced the file; delete
-* the file and re-open for recovery write.
-*/
-   unlink(path);
+   /* re-open for recovery write */
fd = open_output_fd(path, ce, to_tempfile);
if (fd < 0) {
free(new);
@@ -205,12 +202,6 @@ static int write_entry(struct cache_entry *ce,
}
else {
free(new);
-   /* The smudgeToFile filter may have replaced
-* or deleted the file; reopen it to make sure
-* that the file exists. */
-   fd = open(path, O_RDONLY);
-   if (fd < 0)
-   return error_errno("unable to create 
file %s", path);
if (!to_tempfile)
fstat_done = fstat_output(fd, state, 
);
close(fd);
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index c0b4709..fd07bd6 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -334,6 +334,19 @@ test_expect_success 'recovery from 

[PATCH v4 5/8] warn on unusable smudgeToFile/cleanFromFile config

[Joey Hess]

Let the user know when they have a smudgeToFile/cleanFromFile config
that cannot be used because the corresponding smudge/clean config
is missing.

The warning is only displayed a maximum of once per git invocation,
and only when doing an operation that would use the filter.

Signed-off-by: Joey Hess 
---
 convert.c | 34 ++
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/convert.c b/convert.c
index 9dde406..378a3bd 100644
--- a/convert.c
+++ b/convert.c
@@ -859,32 +859,50 @@ int would_convert_to_git_filter_fd(const char *path)
return apply_filter(path, NULL, NULL, 0, -1, NULL, ca.drv->clean);
 }
 
+static int can_filter_file(const char *filefilter, const char *filefiltername,
+  const char *stdiofilter, const char *stdiofiltername,
+  const struct conv_attrs *ca,
+  int *warncount)
+{
+   if (! filefilter)
+   return 0;
+
+   if (stdiofilter)
+   return 1;
+
+   if (*warncount == 0)
+   warning("Not running your configured filter.%s.%s command, 
because filter.%s.%s is not configured",
+   ca->drv->name, filefiltername,
+   ca->drv->name, stdiofiltername);
+   *warncount=*warncount+1;
+
+   return 0;
+}
+
 int can_clean_from_file(const char *path)
 {
struct conv_attrs ca;
+   static int warncount = 0;
 
convert_attrs(, path);
if (!ca.drv)
return 0;
 
-   /* Only use the cleanFromFile filter when the clean filter is also
-* configured.
-*/
-   return (ca.drv->clean_from_file && ca.drv->clean);
+   return can_filter_file(ca.drv->clean_from_file, "cleanFromFile",
+  ca.drv->clean, "clean", , );
 }
 
 int can_smudge_to_file(const char *path)
 {
struct conv_attrs ca;
+   static int warncount = 0;
 
convert_attrs(, path);
if (!ca.drv)
return 0;
 
-   /* Only use the smudgeToFile filter when the smudge filter is also
-* configured.
-*/
-   return (ca.drv->smudge_to_file && ca.drv->smudge);
+   return can_filter_file(ca.drv->smudge_to_file, "smudgeToFile",
+  ca.drv->smudge, "smudge", , );
 }
 
 const char *get_convert_attr_ascii(const char *path)
-- 
2.9.0.587.ga3bedf2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 8/8] use smudgeToFile filter in recursive merge

[Joey Hess]

Recursive merge updates the work tree and so should use the smudgeToFile
filter.

At this point, smudgeToFile is run by everything that updates work
tree files.

Signed-off-by: Joey Hess 
---
 merge-recursive.c | 42 --
 t/t0021-conversion.sh | 16 +++-
 2 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 48fe7e7..7d38cf8 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -765,14 +765,6 @@ static void update_file_flags(struct merge_options *o,
die(_("cannot read object %s '%s'"), oid_to_hex(oid), 
path);
if (type != OBJ_BLOB)
die(_("blob expected for %s '%s'"), oid_to_hex(oid), 
path);
-   if (S_ISREG(mode)) {
-   struct strbuf strbuf = STRBUF_INIT;
-   if (convert_to_working_tree(path, buf, size, )) {
-   free(buf);
-   size = strbuf.len;
-   buf = strbuf_detach(, NULL);
-   }
-   }
 
if (make_room_for_path(o, path) < 0) {
update_wd = 0;
@@ -781,6 +773,7 @@ static void update_file_flags(struct merge_options *o,
}
if (S_ISREG(mode) || (!has_symlinks && S_ISLNK(mode))) {
int fd;
+   int isreg = S_ISREG(mode);
if (mode & 0100)
mode = 0777;
else
@@ -788,8 +781,37 @@ static void update_file_flags(struct merge_options *o,
fd = open(path, O_WRONLY | O_TRUNC | O_CREAT, mode);
if (fd < 0)
die_errno(_("failed to open '%s'"), path);
-   write_in_full(fd, buf, size);
-   close(fd);
+
+   int smudge_to_file = can_smudge_to_file(path);
+   if (smudge_to_file) {
+   close(fd);
+   fd = 
convert_to_working_tree_filter_to_file(path, path, buf, size);
+   if (fd < 0) {
+   /* smudgeToFile filter failed;
+* continue with regular file
+* creation. */
+   smudge_to_file = 0;
+   fd = open(path, O_WRONLY | O_TRUNC | 
O_CREAT, mode);
+   if (fd < 0)
+   die_errno(_("failed to open 
'%s'"), path);
+   }
+   else {
+   close(fd);
+   }
+   }
+
+   if (! smudge_to_file) {
+   if (isreg) {
+   struct strbuf strbuf = STRBUF_INIT;
+   if (convert_to_working_tree(path, buf, 
size, )) {
+   free(buf);
+   size = strbuf.len;
+   buf = strbuf_detach(, 
NULL);
+   }
+   }
+   write_in_full(fd, buf, size);
+   close(fd);
+   }
} else if (S_ISLNK(mode)) {
char *lnk = xmemdupz(buf, size);
safe_create_leading_directories_const(path);
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index fd07bd6..2722013 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -334,10 +334,24 @@ test_expect_success 'recovery from failure of 
smudgeToFile filter that deletes t
cmp test fstest.t
 '
 
+test_expect_success 'smudgeToFile filter is used in merge' '
+   test_config filter.rot13.smudgeToFile ./rot13-to-file.sh &&
+
+   git commit -m "added fstest.t" fstest.t &&
+   git checkout -b old &&
+   git reset --hard HEAD^ &&
+   git merge master &&
+
+   test -e rot13-to-file.ran &&
+   rm -f rot13-to-file.ran &&
+
+   cmp test fstest.t &&
+   git checkout master
+'
+
 test_expect_success 'smudgeToFile filter is used by git am' '
test_config filter.rot13.smudgeToFile ./rot13-to-file.sh &&
 
-   git commit fstest.t -m "added fstest.t" &&
git format-patch HEAD^ --stdout > fstest.patch &&
git reset --hard HEAD^ &&
git am < fstest.patch &&
-- 
2.9.0.587.ga3bedf2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

[PATCH v4 6/8] better recovery from failure of smudgeToFile filter

[Joey Hess]

If the smudgeToFile filter fails, it can leave the worktree file with the
wrong content, or even deleted. Recover from this by falling back to
running the smudge filter.

Signed-off-by: Joey Hess 
---
 entry.c   | 55 ---
 t/t0021-conversion.sh | 24 ++
 2 files changed, 59 insertions(+), 20 deletions(-)

diff --git a/entry.c b/entry.c
index 97975e5..8322127 100644
--- a/entry.c
+++ b/entry.c
@@ -181,12 +181,6 @@ static int write_entry(struct cache_entry *ce,
int regular_file = ce_mode_s_ifmt == S_IFREG;
int smudge_to_file = regular_file
&& can_smudge_to_file(ce->name);
-   if (regular_file && ! smudge_to_file &&
-   convert_to_working_tree(ce->name, new, size, )) {
-   free(new);
-   new = strbuf_detach(, );
-   size = newsize;
-   }
 
fd = open_output_fd(path, ce, to_tempfile);
if (fd < 0) {
@@ -194,7 +188,42 @@ static int write_entry(struct cache_entry *ce,
return error_errno("unable to create file %s", path);
}
 
+   if (smudge_to_file) {
+   close(fd);
+   if (! convert_to_working_tree_filter_to_file(ce->name, 
path, new, size)) {
+   smudge_to_file = 0;
+   /* The failing smudgeToFile filter may have
+* deleted or replaced the file; delete
+* the file and re-open for recovery write.
+*/
+   unlink(path);
+   fd = open_output_fd(path, ce, to_tempfile);
+   if (fd < 0) {
+   free(new);
+   return error_errno("unable to create 
file %s", path);
+   }
+   }
+   else {
+   free(new);
+   /* The smudgeToFile filter may have replaced
+* or deleted the file; reopen it to make sure
+* that the file exists. */
+   fd = open(path, O_RDONLY);
+   if (fd < 0)
+   return error_errno("unable to create 
file %s", path);
+   if (!to_tempfile)
+   fstat_done = fstat_output(fd, state, 
);
+   close(fd);
+   }
+   }
+
if (! smudge_to_file) {
+   if (regular_file &&
+   convert_to_working_tree(ce->name, new, size, )) 
{
+   free(new);
+   new = strbuf_detach(, );
+   size = newsize;
+   }
wrote = write_in_full(fd, new, size);
if (!to_tempfile)
fstat_done = fstat_output(fd, state, );
@@ -203,20 +232,6 @@ static int write_entry(struct cache_entry *ce,
if (wrote != size)
return error("unable to write file %s", path);
}
-   else {
-   close(fd);
-   convert_to_working_tree_filter_to_file(ce->name, path, 
new, size);
-   free(new);
-   /* The smudgeToFile filter may have replaced the
-* file; open it to make sure that the file
-* exists. */
-   fd = open(path, O_RDONLY);
-   if (fd < 0)
-   return error_errno("unable to create file %s", 
path);
-   if (!to_tempfile)
-   fstat_done = fstat_output(fd, state, );
-   close(fd);
-   }
break;
case S_IFGITLINK:
if (to_tempfile)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index cba03fd..c0b4709 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -28,6 +28,14 @@ touch rot13-to-file.ran
 EOF
 chmod +x rot13-to-file.sh
 
+cat 

[PATCH v4 1/8] clarify %f documentation

[Joey Hess]

It's natural to expect %f to be an actual file on disk; help avoid that
mistake.

Signed-off-by: Joey Hess 
---
 Documentation/gitattributes.txt | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index f2afdb6..197ece8 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -379,6 +379,11 @@ substitution.  For example:
smudge = git-p4-filter --smudge %f
 
 
+Note that "%f" is the name of the path that is being worked on. Depending
+on the version that is being filtered, the corresponding file on disk may
+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.
 
 Interaction between checkin/checkout attributes
 ^^^
-- 
2.9.0.587.ga3bedf2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 0/8] extend smudge/clean filters with direct file access (for pu)

[Joey Hess]

This is the same as v3, except rebased on top of tb/convert-peek-in-index
to fix a build failure in pu.

Joey Hess (8):
  clarify %f documentation
  add smudgeToFile and cleanFromFile filter configs
  use cleanFromFile in git add
  use smudgeToFile in git checkout etc
  warn on unusable smudgeToFile/cleanFromFile config
  better recovery from failure of smudgeToFile filter
  use smudgeToFile filter in git am
  use smudgeToFile filter in recursive merge

 Documentation/config.txt|  18 -
 Documentation/gitattributes.txt |  42 
 builtin/apply.c |  16 +
 convert.c   | 147 +++-
 convert.h   |  11 +++
 entry.c |  53 +++
 merge-recursive.c   |  42 +---
 sha1_file.c |  44 ++--
 t/t0021-conversion.sh   | 115 +++
 9 files changed, 441 insertions(+), 47 deletions(-)

-- 
2.9.0.587.ga3bedf2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 4/8] use smudgeToFile in git checkout etc

[Joey Hess]

This makes git checkout, git reset, etc use smudgeToFile.

Includes test cases.

(There's a call to convert_to_working_tree in merge-recursive.c
that could also be made to use smudgeToFile as well.)

Signed-off-by: Joey Hess 
---
 entry.c   | 37 +
 t/t0021-conversion.sh | 38 +-
 2 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/entry.c b/entry.c
index 519e042..97975e5 100644
--- a/entry.c
+++ b/entry.c
@@ -175,8 +175,13 @@ static int write_entry(struct cache_entry *ce,
 
/*
 * Convert from git internal format to working tree format
+* unless the smudgeToFile filter can write to the
+* file directly.
 */
-   if (ce_mode_s_ifmt == S_IFREG &&
+   int regular_file = ce_mode_s_ifmt == S_IFREG;
+   int smudge_to_file = regular_file
+   && can_smudge_to_file(ce->name);
+   if (regular_file && ! smudge_to_file &&
convert_to_working_tree(ce->name, new, size, )) {
free(new);
new = strbuf_detach(, );
@@ -189,13 +194,29 @@ static int write_entry(struct cache_entry *ce,
return error_errno("unable to create file %s", path);
}
 
-   wrote = write_in_full(fd, new, size);
-   if (!to_tempfile)
-   fstat_done = fstat_output(fd, state, );
-   close(fd);
-   free(new);
-   if (wrote != size)
-   return error("unable to write file %s", path);
+   if (! smudge_to_file) {
+   wrote = write_in_full(fd, new, size);
+   if (!to_tempfile)
+   fstat_done = fstat_output(fd, state, );
+   close(fd);
+   free(new);
+   if (wrote != size)
+   return error("unable to write file %s", path);
+   }
+   else {
+   close(fd);
+   convert_to_working_tree_filter_to_file(ce->name, path, 
new, size);
+   free(new);
+   /* The smudgeToFile filter may have replaced the
+* file; open it to make sure that the file
+* exists. */
+   fd = open(path, O_RDONLY);
+   if (fd < 0)
+   return error_errno("unable to create file %s", 
path);
+   if (!to_tempfile)
+   fstat_done = fstat_output(fd, state, );
+   close(fd);
+   }
break;
case S_IFGITLINK:
if (to_tempfile)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 407d5d6..cba03fd 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -14,12 +14,20 @@ chmod +x rot13.sh
 
 cat  "\$destfile"
+EOF
+chmod +x rot13-to-file.sh
+
 test_expect_success setup '
git config filter.rot13.smudge ./rot13.sh &&
git config filter.rot13.clean ./rot13.sh &&
@@ -291,6 +299,17 @@ test_expect_success 'cleanFromFile filter is used when 
adding a file' '
cmp test fstest.t
 '
 
+test_expect_success 'smudgeToFile filter is used when checking out a file' '
+   test_config filter.rot13.smudgeToFile ./rot13-to-file.sh &&
+
+   rm -f fstest.t &&
+   git checkout -- fstest.t &&
+   cmp test fstest.t &&
+
+   test -e rot13-to-file.ran &&
+   rm -f rot13-to-file.ran
+'
+
 test_expect_success 'cleanFromFile filter is not used when clean filter is not 
configured' '
test_config filter.noclean.smudge ./rot13.sh &&
test_config filter.noclean.cleanFromFile ./rot13-from-file.sh &&
@@ -299,9 +318,18 @@ test_expect_success 'cleanFromFile filter is not used when 
clean filter is not c
 
cat test >test.no &&
git add test.no &&
-   test ! -e rot13-from-file.ran &&
-   git cat-file blob :test.no >actual &&
-   cmp test actual
+   test ! -e rot13-from-file.ran
+'
+
+test_expect_success 'smudgeToFile filter is not used when smudge filter is not 
configured' '
+   test_config filter.nosmudge.clean ./rot13.sh &&
+   test_config filter.nosmudge.smudgeToFile ./rot13-to-file.sh &&
+
+   echo "*.no filter=nosmudge" >.gitattributes &&
+
+   rm -f fstest.t &&
+   git checkout -- fstest.t &&
+   test ! -e rot13-to-file.ran
 '
 
 test_done
-- 
2.9.0.587.ga3bedf2

--
To unsubscribe from 

Re: [PATCH 1/2] t/perf: fix regression in testing older versions of git

[Junio C Hamano]

Jeff King  writes:

>> I can see how this works for "git -C ... rev-parse ..." or any other
>> built-in commands, but I am not sure if this is sufficient when any
>> non-built-in command is used in the perf framework.  How does it
>> interact with GIT_EXEC_PATH we set in ../test-lib.sh that is
>> dot-sourced by ./perf-lib.sh that everybody dot-sources?
>
> I didn't test it but it should work because we are pointing to
> bin-wrappers/git, which will override GIT_EXEC_PATH, and stick itself at
> the front of the PATH.

Ah, yes, bin-wrappers/git is not the real binary we just have built
but overrides GIT_EXEC_PATH to point at the matching version.  I
forgot about that.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] t/perf: fix regression in testing older versions of git

[Jeff King]

On Wed, Jun 22, 2016 at 01:46:25PM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > So let's introduce a new variable, $MODERN_GIT, that we can
> > use both in perf-lib and in the test setup to get a reliable
> > set of git features (we might change git and break some
> > tests, of course, but $MODERN_GIT is tied to the same
> > version of git as the t/perf scripts, so they can be fixed
> > or adjusted together).
> 
> I can see how this works for "git -C ... rev-parse ..." or any other
> built-in commands, but I am not sure if this is sufficient when any
> non-built-in command is used in the perf framework.  How does it
> interact with GIT_EXEC_PATH we set in ../test-lib.sh that is
> dot-sourced by ./perf-lib.sh that everybody dot-sources?

I didn't test it but it should work because we are pointing to
bin-wrappers/git, which will override GIT_EXEC_PATH, and stick itself at
the front of the PATH.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] t/perf: fix regression in testing older versions of git

[Junio C Hamano]

Jeff King  writes:

> So let's introduce a new variable, $MODERN_GIT, that we can
> use both in perf-lib and in the test setup to get a reliable
> set of git features (we might change git and break some
> tests, of course, but $MODERN_GIT is tied to the same
> version of git as the t/perf scripts, so they can be fixed
> or adjusted together).

I can see how this works for "git -C ... rev-parse ..." or any other
built-in commands, but I am not sure if this is sufficient when any
non-built-in command is used in the perf framework.  How does it
interact with GIT_EXEC_PATH we set in ../test-lib.sh that is
dot-sourced by ./perf-lib.sh that everybody dot-sources?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/3] Introduce log.showSignature config variable

[Junio C Hamano]

Mehul Jain  writes:

> Add a new configuratation variable "log.showSignature" for git-log
> and related commands. "log.showSignature=true" will enable user to
> see GPG signature by default for git-log and related commands.
>
> Changes compared to v2:
>   * A preparatory patch 1/3 has been introduced so that tests
> in patches 2/3 and 3/3 can take advantage of it.

It is unclear how this change allows the remainder to "take
advanrage" to me.  Earlier, "signed" branch was created only when
the GPG prerequisite is met and with this change the branch is
always created, which is the only change as far as I can see.  But
the tests that are added in 2 and 3 are all protected with the GPG
prerequiste.

Besides, the invocation of "git commit -S" after this change is no
longer protected by the GPG prerequisite and it may even cause the
'setup' step to fail on a host without GPG.

What am I missing?  I do not quite see any reason to take 1/2; I
only see a possible downside without any upside.

The main two steps 2&3 looked good.

Thanks.

>   * Mistake regarding branch in [patch v2 2/2] has been
> corrected.
>   * Tight coupling between the tests in [patch v2 2/2] has
> been resovled.
>
> I would like to thanks Eric Sunshine for his feedback on previous
> series [1].



>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/297648 
>
> Mehul Jain (3):
>   t4202: refactoring of test
>   log: add "--no-show-signature" command line option
>   log: add log.showSignature configuration variable
>
>  Documentation/git-log.txt |  4 
>  builtin/log.c |  6 ++
>  revision.c|  2 ++
>  t/t4202-log.sh| 32 ++--
>  t/t7510-signed-commit.sh  |  7 +++
>  5 files changed, 49 insertions(+), 2 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] t/perf: fix regression in testing older versions of git

[Johannes Schindelin]

Hi Peff,

On Wed, 22 Jun 2016, Jeff King wrote:

> diff --git a/t/perf/README b/t/perf/README
> index 8848c14..15986ca 100644
> --- a/t/perf/README
> +++ b/t/perf/README
> @@ -115,8 +115,16 @@ After that you will want to use some of the following:
>  
>  At least one of the first two is required!
>  
> -You can use test_expect_success as usual.  For actual performance
> -tests, use
> +You can use test_expect_success as usual. In both test_expect_success
> +and in test_perf, running "git" points to the version that is being
> +peft-tested. The $MODERN_GIT variable points to the git wrapper for the

s/peft/perf/

Or s/peft/peff/. :-)

The rest looks fine!
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] t/perf: fix regression in testing older versions of git

[Johannes Schindelin]

Hi Peff,

On Wed, 22 Jun 2016, Jeff King wrote:

> Commit 7501b59 (perf: make the tests work in worktrees,
> 2016-05-13) introduced the use of "git rev-parse --git-path"
> in the perf-lib setup code. Because the to-be-tested version
> of git is at the front of the $PATH when this code runs,
> this means we cannot use modern versions of t/perf to test
> versions of git older than v2.5.0 (when that option was
> introduced).
> 
> This is a symptom of a more general problem. The t/perf
> suite is essentially independent of git versions, and
> ideally we would be able to run the most modern and complete
> set of tests across many historical versions (to see how
> they compare). But any setup code they run is therefore
> required to use the lowest common denominator we expect to
> test.
> 
> So let's introduce a new variable, $MODERN_GIT, that we can
> use both in perf-lib and in the test setup to get a reliable
> set of git features (we might change git and break some
> tests, of course, but $MODERN_GIT is tied to the same
> version of git as the t/perf scripts, so they can be fixed
> or adjusted together).
> 
> This commit fixes the "--git-path" case, but does not
> mass-convert existing setup code to use $MODERN_GIT. Most
> setup code is fairly vanilla and will work with effectively
> all versions. But now the tool is there to fix any other
> issues we find going forward.

Thanks for beating me to it!

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 3/8] use cleanFromFile in git add

[Joey Hess]

Includes test cases.

Signed-off-by: Joey Hess 
---
 sha1_file.c   | 42 --
 t/t0021-conversion.sh | 36 
 2 files changed, 72 insertions(+), 6 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index d5e1121..8df86a0 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3329,6 +3329,29 @@ static int index_stream_convert_blob(unsigned char 
*sha1, int fd,
return ret;
 }
 
+static int index_from_file_convert_blob(unsigned char *sha1,
+ const char *path, unsigned flags)
+{
+   int ret;
+   const int write_object = flags & HASH_WRITE_OBJECT;
+   struct strbuf sbuf = STRBUF_INIT;
+
+   assert(path);
+   assert(can_clean_from_file(path));
+
+   convert_to_git_filter_from_file(path, ,
+write_object ? safe_crlf : SAFE_CRLF_FALSE);
+
+   if (write_object)
+   ret = write_sha1_file(sbuf.buf, sbuf.len, typename(OBJ_BLOB),
+ sha1);
+   else
+   ret = hash_sha1_file(sbuf.buf, sbuf.len, typename(OBJ_BLOB),
+sha1);
+   strbuf_release();
+   return ret;
+}
+
 static int index_pipe(unsigned char *sha1, int fd, enum object_type type,
  const char *path, unsigned flags)
 {
@@ -3421,12 +3444,19 @@ int index_path(unsigned char *sha1, const char *path, 
struct stat *st, unsigned
 
switch (st->st_mode & S_IFMT) {
case S_IFREG:
-   fd = open(path, O_RDONLY);
-   if (fd < 0)
-   return error_errno("open(\"%s\")", path);
-   if (index_fd(sha1, fd, st, OBJ_BLOB, path, flags) < 0)
-   return error("%s: failed to insert into database",
-path);
+   if (can_clean_from_file(path)) {
+   if (index_from_file_convert_blob(sha1, path, flags) < 0)
+   return error("%s: failed to insert into 
database",
+path);
+   }
+   else {
+   fd = open(path, O_RDONLY);
+   if (fd < 0)
+   return error_errno("open(\"%s\")", path);
+   if (index_fd(sha1, fd, st, OBJ_BLOB, path, flags) < 0)
+   return error("%s: failed to insert into 
database",
+path);
+   }
break;
case S_IFLNK:
if (strbuf_readlink(, path, st->st_size))
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 7bac2bc..407d5d6 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -12,6 +12,14 @@ tr \
 EOF
 chmod +x rot13.sh
 
+cat .gitattributes &&
+
+   cat test >fstest.t &&
+   git add fstest.t &&
+   test -e rot13-from-file.ran &&
+   rm -f rot13-from-file.ran &&
+
+   rm -f fstest.t &&
+   git checkout -- fstest.t &&
+   cmp test fstest.t
+'
+
+test_expect_success 'cleanFromFile filter is not used when clean filter is not 
configured' '
+   test_config filter.noclean.smudge ./rot13.sh &&
+   test_config filter.noclean.cleanFromFile ./rot13-from-file.sh &&
+
+   echo "*.no filter=noclean" >.gitattributes &&
+
+   cat test >test.no &&
+   git add test.no &&
+   test ! -e rot13-from-file.ran &&
+   git cat-file blob :test.no >actual &&
+   cmp test actual
+'
+
 test_done
-- 
2.9.0.8.g973eabb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 8/8] use smudgeToFile filter in recursive merge

[Joey Hess]

Recursive merge updates the work tree and so should use the smudgeToFile
filter.

At this point, smudgeToFile is run by everything that updates work
tree files.

Signed-off-by: Joey Hess 
---
 merge-recursive.c | 42 --
 t/t0021-conversion.sh | 16 +++-
 2 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 65cb5d6..012fe38 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -765,14 +765,6 @@ static void update_file_flags(struct merge_options *o,
die(_("cannot read object %s '%s'"), sha1_to_hex(sha), 
path);
if (type != OBJ_BLOB)
die(_("blob expected for %s '%s'"), sha1_to_hex(sha), 
path);
-   if (S_ISREG(mode)) {
-   struct strbuf strbuf = STRBUF_INIT;
-   if (convert_to_working_tree(path, buf, size, )) {
-   free(buf);
-   size = strbuf.len;
-   buf = strbuf_detach(, NULL);
-   }
-   }
 
if (make_room_for_path(o, path) < 0) {
update_wd = 0;
@@ -781,6 +773,7 @@ static void update_file_flags(struct merge_options *o,
}
if (S_ISREG(mode) || (!has_symlinks && S_ISLNK(mode))) {
int fd;
+   int isreg = S_ISREG(mode);
if (mode & 0100)
mode = 0777;
else
@@ -788,8 +781,37 @@ static void update_file_flags(struct merge_options *o,
fd = open(path, O_WRONLY | O_TRUNC | O_CREAT, mode);
if (fd < 0)
die_errno(_("failed to open '%s'"), path);
-   write_in_full(fd, buf, size);
-   close(fd);
+
+   int smudge_to_file = can_smudge_to_file(path);
+   if (smudge_to_file) {
+   close(fd);
+   fd = 
convert_to_working_tree_filter_to_file(path, path, buf, size);
+   if (fd < 0) {
+   /* smudgeToFile filter failed;
+* continue with regular file
+* creation. */
+   smudge_to_file = 0;
+   fd = open(path, O_WRONLY | O_TRUNC | 
O_CREAT, mode);
+   if (fd < 0)
+   die_errno(_("failed to open 
'%s'"), path);
+   }
+   else {
+   close(fd);
+   }
+   }
+
+   if (! smudge_to_file) {
+   if (isreg) {
+   struct strbuf strbuf = STRBUF_INIT;
+   if (convert_to_working_tree(path, buf, 
size, )) {
+   free(buf);
+   size = strbuf.len;
+   buf = strbuf_detach(, 
NULL);
+   }
+   }
+   write_in_full(fd, buf, size);
+   close(fd);
+   }
} else if (S_ISLNK(mode)) {
char *lnk = xmemdupz(buf, size);
safe_create_leading_directories_const(path);
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index fd07bd6..2722013 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -334,10 +334,24 @@ test_expect_success 'recovery from failure of 
smudgeToFile filter that deletes t
cmp test fstest.t
 '
 
+test_expect_success 'smudgeToFile filter is used in merge' '
+   test_config filter.rot13.smudgeToFile ./rot13-to-file.sh &&
+
+   git commit -m "added fstest.t" fstest.t &&
+   git checkout -b old &&
+   git reset --hard HEAD^ &&
+   git merge master &&
+
+   test -e rot13-to-file.ran &&
+   rm -f rot13-to-file.ran &&
+
+   cmp test fstest.t &&
+   git checkout master
+'
+
 test_expect_success 'smudgeToFile filter is used by git am' '
test_config filter.rot13.smudgeToFile ./rot13-to-file.sh &&
 
-   git commit fstest.t -m "added fstest.t" &&
git format-patch HEAD^ --stdout > fstest.patch &&
git reset --hard HEAD^ &&
git am < fstest.patch &&
-- 
2.9.0.8.g973eabb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

[PATCH v3 4/8] use smudgeToFile in git checkout etc

[Joey Hess]

This makes git checkout, git reset, etc use smudgeToFile.

Includes test cases.

(There's a call to convert_to_working_tree in merge-recursive.c
that could also be made to use smudgeToFile as well.)

Signed-off-by: Joey Hess 
---
 entry.c   | 37 +
 t/t0021-conversion.sh | 38 +-
 2 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/entry.c b/entry.c
index 519e042..97975e5 100644
--- a/entry.c
+++ b/entry.c
@@ -175,8 +175,13 @@ static int write_entry(struct cache_entry *ce,
 
/*
 * Convert from git internal format to working tree format
+* unless the smudgeToFile filter can write to the
+* file directly.
 */
-   if (ce_mode_s_ifmt == S_IFREG &&
+   int regular_file = ce_mode_s_ifmt == S_IFREG;
+   int smudge_to_file = regular_file
+   && can_smudge_to_file(ce->name);
+   if (regular_file && ! smudge_to_file &&
convert_to_working_tree(ce->name, new, size, )) {
free(new);
new = strbuf_detach(, );
@@ -189,13 +194,29 @@ static int write_entry(struct cache_entry *ce,
return error_errno("unable to create file %s", path);
}
 
-   wrote = write_in_full(fd, new, size);
-   if (!to_tempfile)
-   fstat_done = fstat_output(fd, state, );
-   close(fd);
-   free(new);
-   if (wrote != size)
-   return error("unable to write file %s", path);
+   if (! smudge_to_file) {
+   wrote = write_in_full(fd, new, size);
+   if (!to_tempfile)
+   fstat_done = fstat_output(fd, state, );
+   close(fd);
+   free(new);
+   if (wrote != size)
+   return error("unable to write file %s", path);
+   }
+   else {
+   close(fd);
+   convert_to_working_tree_filter_to_file(ce->name, path, 
new, size);
+   free(new);
+   /* The smudgeToFile filter may have replaced the
+* file; open it to make sure that the file
+* exists. */
+   fd = open(path, O_RDONLY);
+   if (fd < 0)
+   return error_errno("unable to create file %s", 
path);
+   if (!to_tempfile)
+   fstat_done = fstat_output(fd, state, );
+   close(fd);
+   }
break;
case S_IFGITLINK:
if (to_tempfile)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 407d5d6..cba03fd 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -14,12 +14,20 @@ chmod +x rot13.sh
 
 cat  "\$destfile"
+EOF
+chmod +x rot13-to-file.sh
+
 test_expect_success setup '
git config filter.rot13.smudge ./rot13.sh &&
git config filter.rot13.clean ./rot13.sh &&
@@ -291,6 +299,17 @@ test_expect_success 'cleanFromFile filter is used when 
adding a file' '
cmp test fstest.t
 '
 
+test_expect_success 'smudgeToFile filter is used when checking out a file' '
+   test_config filter.rot13.smudgeToFile ./rot13-to-file.sh &&
+
+   rm -f fstest.t &&
+   git checkout -- fstest.t &&
+   cmp test fstest.t &&
+
+   test -e rot13-to-file.ran &&
+   rm -f rot13-to-file.ran
+'
+
 test_expect_success 'cleanFromFile filter is not used when clean filter is not 
configured' '
test_config filter.noclean.smudge ./rot13.sh &&
test_config filter.noclean.cleanFromFile ./rot13-from-file.sh &&
@@ -299,9 +318,18 @@ test_expect_success 'cleanFromFile filter is not used when 
clean filter is not c
 
cat test >test.no &&
git add test.no &&
-   test ! -e rot13-from-file.ran &&
-   git cat-file blob :test.no >actual &&
-   cmp test actual
+   test ! -e rot13-from-file.ran
+'
+
+test_expect_success 'smudgeToFile filter is not used when smudge filter is not 
configured' '
+   test_config filter.nosmudge.clean ./rot13.sh &&
+   test_config filter.nosmudge.smudgeToFile ./rot13-to-file.sh &&
+
+   echo "*.no filter=nosmudge" >.gitattributes &&
+
+   rm -f fstest.t &&
+   git checkout -- fstest.t &&
+   test ! -e rot13-to-file.ran
 '
 
 test_done
-- 
2.9.0.8.g973eabb.dirty

--
To unsubscribe 

[PATCH v3 7/8] use smudgeToFile filter in git am

[Joey Hess]

git am updates the work tree and so should use the smudgeToFile filter.

This includes some refactoring into convert_to_working_tree_filter_to_file
to make it check the file after running the smudgeToFile command, and clean
up from a failing command.

Signed-off-by: Joey Hess 
---
 builtin/apply.c   | 16 
 convert.c | 19 +--
 entry.c   | 15 +++
 t/t0021-conversion.sh | 13 +
 4 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index c770d7d..acd7c3e 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4109,6 +4109,22 @@ static int try_create_file(const char *path, unsigned 
int mode, const char *buf,
if (fd < 0)
return -1;
 
+   if (can_smudge_to_file(path)) {
+   close(fd);
+   fd = convert_to_working_tree_filter_to_file(path, path, buf, 
size);
+   if (fd < 0) {
+   /* smudgeToFile filter failed; continue
+* with regular file creation. */
+   fd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode & 
0100) ? 0777 : 0666);
+   if (fd < 0)
+   return -1;
+   }
+   else {
+   close(fd);
+   return 0;
+   }
+   }
+
if (convert_to_working_tree(path, buf, size, )) {
size = nbuf.len;
buf  = nbuf.buf;
diff --git a/convert.c b/convert.c
index b6a76a9..948b52f 100644
--- a/convert.c
+++ b/convert.c
@@ -1031,13 +1031,28 @@ int convert_to_working_tree(const char *path, const 
char *src, size_t len, struc
return convert_to_working_tree_internal(path, NULL, src, len, dst, 0);
 }
 
+/* Returns fd open to read the worktree file on success.
+ * On failure, the worktree file will not exist. */
 int convert_to_working_tree_filter_to_file(const char *path, const char 
*destpath, const char *src, size_t len)
 {
struct strbuf output = STRBUF_INIT;
-   int ret = convert_to_working_tree_internal(path, destpath, src, len, 
, 0);
+   int ok = convert_to_working_tree_internal(path, destpath, src, len, 
, 0);
/* The smudgeToFile filter stdout is not used. */
strbuf_release();
-   return ret;
+   if (ok) {
+   /* Open the file to make sure that it's present
+* (and readable) after the command populated it. */
+   int fd = open(path, O_RDONLY);
+   if (fd < 0)
+   unlink(path);
+   return fd;
+   }
+   else {
+   /* The command could have created the file before failing,
+* so delete it. */
+   unlink(path);
+   return -1;
+   }
 }
 
 int renormalize_buffer(const char *path, const char *src, size_t len, struct 
strbuf *dst)
diff --git a/entry.c b/entry.c
index 8322127..2f7c4fd 100644
--- a/entry.c
+++ b/entry.c
@@ -190,13 +190,10 @@ static int write_entry(struct cache_entry *ce,
 
if (smudge_to_file) {
close(fd);
-   if (! convert_to_working_tree_filter_to_file(ce->name, 
path, new, size)) {
+   fd = convert_to_working_tree_filter_to_file(ce->name, 
path, new, size);
+   if (fd < 0) {
smudge_to_file = 0;
-   /* The failing smudgeToFile filter may have
-* deleted or replaced the file; delete
-* the file and re-open for recovery write.
-*/
-   unlink(path);
+   /* re-open for recovery write */
fd = open_output_fd(path, ce, to_tempfile);
if (fd < 0) {
free(new);
@@ -205,12 +202,6 @@ static int write_entry(struct cache_entry *ce,
}
else {
free(new);
-   /* The smudgeToFile filter may have replaced
-* or deleted the file; reopen it to make sure
-* that the file exists. */
-   fd = open(path, O_RDONLY);
-   if (fd < 0)
-   return error_errno("unable to create 
file %s", path);
if (!to_tempfile)
fstat_done = fstat_output(fd, state, 
);
close(fd);
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index c0b4709..fd07bd6 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -334,6 +334,19 @@ test_expect_success 'recovery from 

[PATCH v3 5/8] warn on unusable smudgeToFile/cleanFromFile config

[Joey Hess]

Let the user know when they have a smudgeToFile/cleanFromFile config
that cannot be used because the corresponding smudge/clean config
is missing.

The warning is only displayed a maximum of once per git invocation,
and only when doing an operation that would use the filter.

Signed-off-by: Joey Hess 
---
 convert.c | 34 ++
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/convert.c b/convert.c
index bf63ba0..b6a76a9 100644
--- a/convert.c
+++ b/convert.c
@@ -847,32 +847,50 @@ int would_convert_to_git_filter_fd(const char *path)
return apply_filter(path, NULL, NULL, 0, -1, NULL, ca.drv->clean);
 }
 
+static int can_filter_file(const char *filefilter, const char *filefiltername,
+  const char *stdiofilter, const char *stdiofiltername,
+  const struct conv_attrs *ca,
+  int *warncount)
+{
+   if (! filefilter)
+   return 0;
+
+   if (stdiofilter)
+   return 1;
+
+   if (*warncount == 0)
+   warning("Not running your configured filter.%s.%s command, 
because filter.%s.%s is not configured",
+   ca->drv->name, filefiltername,
+   ca->drv->name, stdiofiltername);
+   *warncount=*warncount+1;
+
+   return 0;
+}
+
 int can_clean_from_file(const char *path)
 {
struct conv_attrs ca;
+   static int warncount = 0;
 
convert_attrs(, path);
if (!ca.drv)
return 0;
 
-   /* Only use the cleanFromFile filter when the clean filter is also
-* configured.
-*/
-   return (ca.drv->clean_from_file && ca.drv->clean);
+   return can_filter_file(ca.drv->clean_from_file, "cleanFromFile",
+  ca.drv->clean, "clean", , );
 }
 
 int can_smudge_to_file(const char *path)
 {
struct conv_attrs ca;
+   static int warncount = 0;
 
convert_attrs(, path);
if (!ca.drv)
return 0;
 
-   /* Only use the smudgeToFile filter when the smudge filter is also
-* configured.
-*/
-   return (ca.drv->smudge_to_file && ca.drv->smudge);
+   return can_filter_file(ca.drv->smudge_to_file, "smudgeToFile",
+  ca.drv->smudge, "smudge", , );
 }
 
 const char *get_convert_attr_ascii(const char *path)
-- 
2.9.0.8.g973eabb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 2/8] add smudgeToFile and cleanFromFile filter configs

[Joey Hess]

This adds new smudgeToFile and cleanFromFile filter commands,
which are similar to smudge and clean but allow direct access to files on
disk.

This interface can be much more efficient when operating on large files,
because the whole file content does not need to be streamed through the
filter. It even allows for things like cleanFromFile commands that avoid
reading the whole content of the file, and for smudgeToFile commands that
populate a work tree file using an efficient Copy On Write operation.

The new filter commands will not be used for all filtering. They are
efficient to use when git add is adding a file, or when the work tree is
being updated, but not a good fit when git is internally filtering blob
objects in memory for eg, a diff.

So, a user who wants to use smudgeToFile should also provide a smudge
command to be used in cases where smudgeToFile is not used. And ditto
with cleanFromFile and clean. To avoid foot-shooting configurations, the
new commands are not used unless the old commands are also configured.

That also ensures that a filter driver configuration that includes these
new commands will work, although less efficiently, when used with an older
version of git that does not support them.

Signed-off-by: Joey Hess 
---
 Documentation/config.txt|  18 ++-
 Documentation/gitattributes.txt |  37 ++
 convert.c   | 108 ++--
 convert.h   |  10 
 4 files changed, 157 insertions(+), 16 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2e1b2e4..c300efe 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1299,15 +1299,29 @@ format.useAutoBase::
format-patch by default.
 
 filter..clean::
-   The command which is used to convert the content of a worktree
+   The command which is used as a filter to convert the content of a 
worktree
file to a blob upon checkin.  See linkgit:gitattributes[5] for
details.
 
 filter..smudge::
-   The command which is used to convert the content of a blob
+   The command which is used as a filter to convert the content of a blob
object to a worktree file upon checkout.  See
linkgit:gitattributes[5] for details.
 
+filter..cleanFromFile::
+   Similar to filter..clean but the specified command
+   directly accesses a worktree file on disk, rather than
+   receiving the file content from standard input.
+   Only used when filter..clean is also configured.
+   See linkgit:gitattributes[5] for details.
+
+filter..smudgeToFile::
+   Similar to filter..smudge but the specified command
+   writes the content of a blob directly to a worktree file,
+   rather than to standard output.
+   Only used when filter..smudge is also configured.
+   See linkgit:gitattributes[5] for details.
+
 fsck.::
Allows overriding the message type (error, warn or ignore) of a
specific message ID such as `missingEmail`.
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 145dd10..5ae0783 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -380,6 +380,43 @@ 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.
 
+There are two extra commands "cleanFromFile" and "smudgeToFile", which
+can optionally be set in a filter driver. These are similar to the "clean"
+and "smudge" commands, but avoid needing to pipe the contents of files
+through the filters, and instead read/write files in the filesystem.
+This can be more efficient when using filters with large files that are not
+directly stored in the repository.
+
+Both "cleanFromFile" and "smudgeToFile" are provided a path as an
+added parameter after the configured command line.
+
+The "cleanFromFile" command is provided the path to the file that
+it should clean. Like the "clean" command, it should output the cleaned
+version to standard output.
+
+The "smudgeToFile" command is provided a path to the file that it
+should write to. (This file will already exist, as an empty file that can
+be written to or replaced.) Like the "smudge" command, "smudgeToFile"
+is fed the blob object from its standard input.
+
+Some git operations that need to apply filters cannot use "cleanFromFile"
+and "smudgeToFile", since the files are not present to disk. So, to avoid
+inconsistent behavior, "cleanFromFile" will only be used if "clean" is
+also configured, and "smudgeToFile" will only be used if "smudge" is also
+configured.
+
+An example large file storage filter driver using cleanFromFile and
+smudgeToFile follows:
+
+
+[filter "bigfiles"]
+   clean = store-bigfile --from-stdin
+   cleanFromFile = store-bigfile --from-file
+   smudge = 

[PATCH v3 6/8] better recovery from failure of smudgeToFile filter

[Joey Hess]

If the smudgeToFile filter fails, it can leave the worktree file with the
wrong content, or even deleted. Recover from this by falling back to
running the smudge filter.

Signed-off-by: Joey Hess 
---
 entry.c   | 55 ---
 t/t0021-conversion.sh | 24 ++
 2 files changed, 59 insertions(+), 20 deletions(-)

diff --git a/entry.c b/entry.c
index 97975e5..8322127 100644
--- a/entry.c
+++ b/entry.c
@@ -181,12 +181,6 @@ static int write_entry(struct cache_entry *ce,
int regular_file = ce_mode_s_ifmt == S_IFREG;
int smudge_to_file = regular_file
&& can_smudge_to_file(ce->name);
-   if (regular_file && ! smudge_to_file &&
-   convert_to_working_tree(ce->name, new, size, )) {
-   free(new);
-   new = strbuf_detach(, );
-   size = newsize;
-   }
 
fd = open_output_fd(path, ce, to_tempfile);
if (fd < 0) {
@@ -194,7 +188,42 @@ static int write_entry(struct cache_entry *ce,
return error_errno("unable to create file %s", path);
}
 
+   if (smudge_to_file) {
+   close(fd);
+   if (! convert_to_working_tree_filter_to_file(ce->name, 
path, new, size)) {
+   smudge_to_file = 0;
+   /* The failing smudgeToFile filter may have
+* deleted or replaced the file; delete
+* the file and re-open for recovery write.
+*/
+   unlink(path);
+   fd = open_output_fd(path, ce, to_tempfile);
+   if (fd < 0) {
+   free(new);
+   return error_errno("unable to create 
file %s", path);
+   }
+   }
+   else {
+   free(new);
+   /* The smudgeToFile filter may have replaced
+* or deleted the file; reopen it to make sure
+* that the file exists. */
+   fd = open(path, O_RDONLY);
+   if (fd < 0)
+   return error_errno("unable to create 
file %s", path);
+   if (!to_tempfile)
+   fstat_done = fstat_output(fd, state, 
);
+   close(fd);
+   }
+   }
+
if (! smudge_to_file) {
+   if (regular_file &&
+   convert_to_working_tree(ce->name, new, size, )) 
{
+   free(new);
+   new = strbuf_detach(, );
+   size = newsize;
+   }
wrote = write_in_full(fd, new, size);
if (!to_tempfile)
fstat_done = fstat_output(fd, state, );
@@ -203,20 +232,6 @@ static int write_entry(struct cache_entry *ce,
if (wrote != size)
return error("unable to write file %s", path);
}
-   else {
-   close(fd);
-   convert_to_working_tree_filter_to_file(ce->name, path, 
new, size);
-   free(new);
-   /* The smudgeToFile filter may have replaced the
-* file; open it to make sure that the file
-* exists. */
-   fd = open(path, O_RDONLY);
-   if (fd < 0)
-   return error_errno("unable to create file %s", 
path);
-   if (!to_tempfile)
-   fstat_done = fstat_output(fd, state, );
-   close(fd);
-   }
break;
case S_IFGITLINK:
if (to_tempfile)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index cba03fd..c0b4709 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -28,6 +28,14 @@ touch rot13-to-file.ran
 EOF
 chmod +x rot13-to-file.sh
 
+cat 

[PATCH v3 0/8] extend smudge/clean filters with direct file access

[Joey Hess]

Reroll of this patch set with changes:

* Added additional smudgeToFile calls in git am and recursive merge.
* Improved behavior when smudgeToFile filter fails.
* Some small improvements to the test cases.

I hope this will be the final version.

Joey Hess (8):
  clarify %f documentation
  add smudgeToFile and cleanFromFile filter configs
  use cleanFromFile in git add
  use smudgeToFile in git checkout etc
  warn on unusable smudgeToFile/cleanFromFile config
  better recovery from failure of smudgeToFile filter
  use smudgeToFile filter in git am
  use smudgeToFile filter in recursive merge

 Documentation/config.txt|  18 -
 Documentation/gitattributes.txt |  42 
 builtin/apply.c |  16 +
 convert.c   | 141 
 convert.h   |  10 +++
 entry.c |  53 +++
 merge-recursive.c   |  42 +---
 sha1_file.c |  42 ++--
 t/t0021-conversion.sh   | 115 
 9 files changed, 434 insertions(+), 45 deletions(-)

-- 
2.9.0.8.g973eabb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/8] clarify %f documentation

[Joey Hess]

It's natural to expect %f to be an actual file on disk; help avoid that
mistake.

Signed-off-by: Joey Hess 
---
 Documentation/gitattributes.txt | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index e3b1de8..145dd10 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -374,6 +374,11 @@ substitution.  For example:
smudge = git-p4-filter --smudge %f
 
 
+Note that "%f" is the name of the path that is being worked on. Depending
+on the version that is being filtered, the corresponding file on disk may
+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.
 
 Interaction between checkin/checkout attributes
 ^^^
-- 
2.9.0.8.g973eabb.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/2] Make find_commit_subject() consistent with --format=%s

[Johannes Schindelin]

Hi Junio,

On Wed, 22 Jun 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > In an intermediate iteration of my rebase--helper patches, I
> > accidentally generated commits with more than one empty line
> > between the header and the commit message. When using
> > find_commit_subject() to show the oneline, it turned up empty, even
> > if the output of `git show --format=%s` looked fine.
> 
> Much easier to read with s/this developer/I/g ;-)

:-P

> > Turned out that the pretty-printing machinery helpfully skipped any
> > blank lines before the commit message.
> >
> > In the first iteration of this patch, I failed to notice that
> > the skip_empty_lines() function used by the pretty printing (which is
> > declared static, and therefore I originally did not use it in order to
> > keep the patch as minimal as possible) skips also blank lines.
> 
> By the way, I think skip_empty_lines() is misnamed, and I think your
> use of "blank lines" in the previous paragraph indicates that you
> agree ;-) It probably was OK back when it was a file-local static
> helper in pretty.c, but it becomes a part of the global API with
> 1/2, renaming it to skip_blank_lines() may be a good thing to do
> there at the same time.
> 
> I could do the tweaking while queuing if you too think it should
> happen; that way we'd save one roundtrip ;-).

I just sent another iteration.

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 1/2] Make the skip_blank_lines() function public

[Johannes Schindelin]

This function will be used also in the find_commit_subject() function.

While at it, rename the function to reflect that it skips not only
empty lines, but any lines consisting of only whitespace, too.

Signed-off-by: Johannes Schindelin 
---
 commit.h |  1 +
 pretty.c | 16 
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/commit.h b/commit.h
index b06db4d..5b78f83 100644
--- a/commit.h
+++ b/commit.h
@@ -177,6 +177,7 @@ extern const char *format_subject(struct strbuf *sb, const 
char *msg,
  const char *line_separator);
 extern void userformat_find_requirements(const char *fmt, struct 
userformat_want *w);
 extern int commit_format_is_empty(enum cmit_fmt);
+extern const char *skip_blank_lines(const char *msg);
 extern void format_commit_message(const struct commit *commit,
  const char *format, struct strbuf *sb,
  const struct pretty_print_context *context);
diff --git a/pretty.c b/pretty.c
index c3ec430..3b6bff7 100644
--- a/pretty.c
+++ b/pretty.c
@@ -507,7 +507,7 @@ void pp_user_info(struct pretty_print_context *pp,
}
 }
 
-static int is_empty_line(const char *line, int *len_p)
+static int is_blank_line(const char *line, int *len_p)
 {
int len = *len_p;
while (len && isspace(line[len - 1]))
@@ -516,14 +516,14 @@ static int is_empty_line(const char *line, int *len_p)
return !len;
 }
 
-static const char *skip_empty_lines(const char *msg)
+const char *skip_blank_lines(const char *msg)
 {
for (;;) {
int linelen = get_one_line(msg);
int ll = linelen;
if (!linelen)
break;
-   if (!is_empty_line(msg, ))
+   if (!is_blank_line(msg, ))
break;
msg += linelen;
}
@@ -875,7 +875,7 @@ const char *format_subject(struct strbuf *sb, const char 
*msg,
int linelen = get_one_line(line);
 
msg += linelen;
-   if (!linelen || is_empty_line(line, ))
+   if (!linelen || is_blank_line(line, ))
break;
 
if (!sb)
@@ -894,11 +894,11 @@ static void parse_commit_message(struct 
format_commit_context *c)
const char *msg = c->message + c->message_off;
const char *start = c->message;
 
-   msg = skip_empty_lines(msg);
+   msg = skip_blank_lines(msg);
c->subject_off = msg - start;
 
msg = format_subject(NULL, msg, NULL);
-   msg = skip_empty_lines(msg);
+   msg = skip_blank_lines(msg);
c->body_off = msg - start;
 
c->commit_message_parsed = 1;
@@ -1711,7 +1711,7 @@ void pp_remainder(struct pretty_print_context *pp,
if (!linelen)
break;
 
-   if (is_empty_line(line, )) {
+   if (is_blank_line(line, )) {
if (first)
continue;
if (pp->fmt == CMIT_FMT_SHORT)
@@ -1782,7 +1782,7 @@ void pretty_print_commit(struct pretty_print_context *pp,
}
 
/* Skip excess blank lines at the beginning of body, if any... */
-   msg = skip_empty_lines(msg);
+   msg = skip_blank_lines(msg);
 
/* These formats treat the title line specially. */
if (pp->fmt == CMIT_FMT_ONELINE || pp->fmt == CMIT_FMT_EMAIL)
-- 
2.9.0.118.g0e1a633


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 0/2] Make find_commit_subject() consistent with --format=%s

[Johannes Schindelin]

In an intermediate iteration of my rebase--helper patches, I
accidentally generated commits with more than one empty line
between the header and the commit message. When using
find_commit_subject() to show the oneline, it turned up empty, even
if the output of `git show --format=%s` looked fine.

Turned out that the pretty-printing machinery helpfully skipped any
blank lines before the commit message.

I simply make pretty.c's skip_empty_lines() public (now appropriately
named skip_blank_lines()) to make things consistent.


Johannes Schindelin (2):
  Make the skip_blank_lines() function public
  Make find_commit_subject() more robust

 commit.c |  2 +-
 commit.h |  1 +
 pretty.c | 16 
 t/t8008-blame-formats.sh | 17 +
 4 files changed, 27 insertions(+), 9 deletions(-)

Published-As: https://github.com/dscho/git/releases/tag/leading-empty-lines-v4
Interdiff vs v3:

 diff --git a/commit.c b/commit.c
 index 0bf868f..24d4715 100644
 --- a/commit.c
 +++ b/commit.c
 @@ -414,7 +414,7 @@ int find_commit_subject(const char *commit_buffer, const 
char **subject)
while (*p && (*p != '\n' || p[1] != '\n'))
p++;
if (*p) {
 -  p = skip_empty_lines(p + 2);
 +  p = skip_blank_lines(p + 2);
for (eol = p; *eol && *eol != '\n'; eol++)
; /* do nothing */
} else
 diff --git a/commit.h b/commit.h
 index fbdd18d..5b78f83 100644
 --- a/commit.h
 +++ b/commit.h
 @@ -177,7 +177,7 @@ extern const char *format_subject(struct strbuf *sb, const 
char *msg,
  const char *line_separator);
  extern void userformat_find_requirements(const char *fmt, struct 
userformat_want *w);
  extern int commit_format_is_empty(enum cmit_fmt);
 -extern const char *skip_empty_lines(const char *msg);
 +extern const char *skip_blank_lines(const char *msg);
  extern void format_commit_message(const struct commit *commit,
  const char *format, struct strbuf *sb,
  const struct pretty_print_context *context);
 diff --git a/pretty.c b/pretty.c
 index 1b807b4..3b6bff7 100644
 --- a/pretty.c
 +++ b/pretty.c
 @@ -507,7 +507,7 @@ void pp_user_info(struct pretty_print_context *pp,
}
  }
  
 -static int is_empty_line(const char *line, int *len_p)
 +static int is_blank_line(const char *line, int *len_p)
  {
int len = *len_p;
while (len && isspace(line[len - 1]))
 @@ -516,14 +516,14 @@ static int is_empty_line(const char *line, int *len_p)
return !len;
  }
  
 -const char *skip_empty_lines(const char *msg)
 +const char *skip_blank_lines(const char *msg)
  {
for (;;) {
int linelen = get_one_line(msg);
int ll = linelen;
if (!linelen)
break;
 -  if (!is_empty_line(msg, ))
 +  if (!is_blank_line(msg, ))
break;
msg += linelen;
}
 @@ -875,7 +875,7 @@ const char *format_subject(struct strbuf *sb, const char 
*msg,
int linelen = get_one_line(line);
  
msg += linelen;
 -  if (!linelen || is_empty_line(line, ))
 +  if (!linelen || is_blank_line(line, ))
break;
  
if (!sb)
 @@ -894,11 +894,11 @@ static void parse_commit_message(struct 
format_commit_context *c)
const char *msg = c->message + c->message_off;
const char *start = c->message;
  
 -  msg = skip_empty_lines(msg);
 +  msg = skip_blank_lines(msg);
c->subject_off = msg - start;
  
msg = format_subject(NULL, msg, NULL);
 -  msg = skip_empty_lines(msg);
 +  msg = skip_blank_lines(msg);
c->body_off = msg - start;
  
c->commit_message_parsed = 1;
 @@ -1711,7 +1711,7 @@ void pp_remainder(struct pretty_print_context *pp,
if (!linelen)
break;
  
 -  if (is_empty_line(line, )) {
 +  if (is_blank_line(line, )) {
if (first)
continue;
if (pp->fmt == CMIT_FMT_SHORT)
 @@ -1782,7 +1782,7 @@ void pretty_print_commit(struct pretty_print_context *pp,
}
  
/* Skip excess blank lines at the beginning of body, if any... */
 -  msg = skip_empty_lines(msg);
 +  msg = skip_blank_lines(msg);
  
/* These formats treat the title line specially. */
if (pp->fmt == CMIT_FMT_ONELINE || pp->fmt == CMIT_FMT_EMAIL)
 diff --git a/t/t8008-blame-formats.sh b/t/t8008-blame-formats.sh
 index b98f9a4..92c8e79 100755
 --- a/t/t8008-blame-formats.sh
 +++ b/t/t8008-blame-formats.sh
 @@ -87,7 +87,7 @@ test_expect_success 'blame --line-porcelain output' '
test_cmp expect actual
  '
  
 -test_expect_success '--porcelain detects first non-empty line as subject' '
 

[PATCH v4 2/2] Make find_commit_subject() more robust

[Johannes Schindelin]

Just like the pretty printing machinery, we should simply ignore blank
lines at the beginning of the commit messages.

This discrepancy was noticed when an early version of the rebase--helper
produced commit objects with more than one empty line between the header
and the commit message.

Signed-off-by: Johannes Schindelin 
---
 commit.c |  2 +-
 t/t8008-blame-formats.sh | 17 +
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/commit.c b/commit.c
index 3f4f371..24d4715 100644
--- a/commit.c
+++ b/commit.c
@@ -414,7 +414,7 @@ int find_commit_subject(const char *commit_buffer, const 
char **subject)
while (*p && (*p != '\n' || p[1] != '\n'))
p++;
if (*p) {
-   p += 2;
+   p = skip_blank_lines(p + 2);
for (eol = p; *eol && *eol != '\n'; eol++)
; /* do nothing */
} else
diff --git a/t/t8008-blame-formats.sh b/t/t8008-blame-formats.sh
index 29f84a6..92c8e79 100755
--- a/t/t8008-blame-formats.sh
+++ b/t/t8008-blame-formats.sh
@@ -87,4 +87,21 @@ test_expect_success 'blame --line-porcelain output' '
test_cmp expect actual
 '
 
+test_expect_success '--porcelain detects first non-blank line as subject' '
+   (
+   GIT_INDEX_FILE=.git/tmp-index &&
+   export GIT_INDEX_FILE &&
+   echo "This is it" >single-file &&
+   git add single-file &&
+   tree=$(git write-tree) &&
+   commit=$(printf "%s\n%s\n%s\n\n\n  \noneline\n\nbody\n" \
+   "tree $tree" \
+   "author A  123456789 +" \
+   "committer C  123456789 +" |
+   git hash-object -w -t commit --stdin) &&
+   git blame --porcelain $commit -- single-file >output &&
+   grep "^summary oneline$" output
+   )
+'
+
 test_done
-- 
2.9.0.118.g0e1a633
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] mingw: work around t2300's assuming non-Windows paths

[Junio C Hamano]

Junio C Hamano  writes:

> I think I know that well enough; please sanity check.  My
> understanding is:
> ...
> The patch under discussion is the only door left for that test, and
> a similar trickery is needed for any end-user scripts used for hooks
> and aliases that use 'git --exec-path', if they ever want to be
> cross-platform.
>
> So let's take that "if Windows do this" change to t2300 as-is.

Assuming that the sanity check passes, here is a suggested rewrite
to explain the real problem better.

-- >8 --
From: Johannes Schindelin 
Date: Sat, 18 Jun 2016 12:49:11 +0200
Subject: [PATCH] t2300: "git --exec-path" is not usable in $PATH on Windows 
as-is

The "git" command itself has an internal logic to prepend the
exec-path to the PATH environment variable for processes it spawns.
That is how ". git-sh-setup" in our scripted Porcelains can find the
dot-sourced file in $GIT_EXEC_PATH that is not usually on user's
PATH.

When t2300 runs, because it is not spawned by the "git" command, the
scriptlet being tested did not run with a realistic setting of PATH
environment.  It lacked the exec-path on the PATH, and failed to
find the dot-sourced file.  A recent update to t2300 attempted to do
so, with "PATH=$(git --exec-path):$PATH", which was the recommended
way around v1.6.0 days (a script whose original was written before
that release that survives to this day is likely to have such a line).

However, the "git --exec-path" command outputs C:\path\to\exec\dir
(not /c/path/to/exec/dir) on Windows; the recent update failed to
consider the problem that comes from it.

Even though Git itself, when doing the equivalent internally, does
so in a platform native way (i.e. on Windows, C:\path\to\exec\dir is
prepended to the existing value of %PATH% using ';' as a component
separator), the result is further massaged by bash and gets turned
into $PATH that uses /c/path/to/exec/dir with ':' separating the
components, which is the form understood by bash, so scripted
Porcelains finds commands from PATH correctly even on Windows.

An end user script written in shell, however, cannot prepend
"C:\path\to\exec\dir:" to the existing value of $PATH and expect
bash to magically turn it into the form it understands.  In other
words, "PATH=$(git --exec-path):$PATH" does not work as an emulation
of what "Git" internally does to the PATH on Windows.

To correctly emulate how exec-path is prepended to the PATH
environment internally on Windows, we'd need to convert
C:\git-sdk-64\usr\src\git to at least /c\git-sdk-64\usr\src\git
ourselves before prepending it to PATH.

Signed-off-by: Johannes Schindelin 
Signed-off-by: Junio C Hamano 
---
 t/t2300-cd-to-toplevel.sh | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh
index cccd7d9..c8de6d8 100755
--- a/t/t2300-cd-to-toplevel.sh
+++ b/t/t2300-cd-to-toplevel.sh
@@ -4,11 +4,19 @@ test_description='cd_to_toplevel'
 
 . ./test-lib.sh
 
+EXEC_PATH="$(git --exec-path)"
+test_have_prereq !MINGW ||
+case "$EXEC_PATH" in
+[A-Za-z]:/*)
+   EXEC_PATH="/${EXEC_PATH%%:*}${EXEC_PATH#?:}"
+   ;;
+esac
+
 test_cd_to_toplevel () {
test_expect_success $3 "$2" '
(
cd '"'$1'"' &&
-   PATH="$(git --exec-path):$PATH" &&
+   PATH="$EXEC_PATH:$PATH" &&
. git-sh-setup &&
cd_to_toplevel &&
[ "$(pwd -P)" = "$TOPLEVEL" ]
-- 
2.9.0-346-g30ec1fd

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] doc: git-htmldocs.googlecode.com is no more

[Jeff King]

On Wed, Jun 22, 2016 at 10:00:56AM -0700, Junio C Hamano wrote:

> Peff suggested to use the github pages that are connected to the
> "git" org, and helped its initial set-up.  When I update the
> prebuilt docs, in addition to the listed locations, I also push to
> the gh-pages branch of https://github.com/git/htmldocs repository
> and the result appears in https://git.github.io/htmldocs/git.html
> 
> Even though we do have index.html -> git.html, gh-pages does not
> seem to let you follow it, so you need to start from git.html, and I
> suspect that it is why the below says "I wasn't able to find it" for
> (5).

Yeah, I looked into this a little back then, and I think it is simply
that the Pages builder does not handle symlinks at all. You could push
out index.html as a true copy rather than a symlink, but I think just
pointing to "git.html" as the entry page of the site is reasonable, too
(we could even make index.html a true overview page later if we want).

> So perhaps list both?  I do not know how beefy repo.or.cz is, or how
> well connected it is to the rest of the world, but if I have to pick
> only one, I'd feel safer if we didn't have to exploit the "blob_plain/:"
> backdoor.

Beefiness aside, the GitHub version will be served out of a CDN, which
means it should perform way better for the user, as it will often get
served from a geographically local cache.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] mingw: work around t2300's assuming non-Windows paths

[Johannes Schindelin]

Hi Junio,

On Wed, 22 Jun 2016, Junio C Hamano wrote:

> So let's take that "if Windows do this" change to t2300 as-is.

Thanks!
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Use docker for _some_ testing?

[Jeff King]

On Wed, Jun 22, 2016 at 09:01:55PM +0200, Duy Nguyen wrote:

> Which makes me think, could we use something like this to make sure
> people (on Linux) can test more obscure cases? Sometimes there are
> featues that require some dependencies that are not always present on
> the developer's machine (http server is a big one, locales come close
> second, then there will be lmdb and watchman in future...). With this,
> said developer can do a final test run in docker covering as much as
> possible.

Neat idea. This should also cover some other distro-specific issues like
"what's your /bin/sh look like", etc. It's a lighter-weight alternative
to testing alternate distros in a VM.

But I think most of the interesting cases are more exotic than
distro-to-distro. Things like HFS+ normalization, or weird shell
toolchains on proprietary Unixes (but maybe there are docker images for
those systems?).

So I dunno if it would prove that useful for day to day use or not.

> +ROOT="$(realpath $(git rev-parse --show-cdup))"

I tried running this as contrib/docker/run.sh, which complained here,
because --show-cdup is empty.

> +test "$(docker images --format='{{.Repository}}' $IMAGE)" = $IMAGE || \
> + build_$DISTRO

My docker complained that it doesn't know --format. I guess I might just
have an old one (it's whatever is in Debian unstable, which is 1.8.3).

Not things you need to fix, but just comments for anybody else fiddling
with it.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] p4211: explicitly disable renames in no-rename test

[Jeff King]

p4211 tests line-log performance both with and without "-M".
In v2.9.0, the case without "-M" appears to have regressed
badly, but that is only because we flipped on renames by
default.

Let's have the test explicitly disable renames to get
consistent timings (and to match the presumed intent of the
test, which is to see the effects with and without renames).

Signed-off-by: Jeff King 
---
 t/perf/p4211-line-log.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/perf/p4211-line-log.sh b/t/perf/p4211-line-log.sh
index 3d074b0..b7ff68d 100755
--- a/t/perf/p4211-line-log.sh
+++ b/t/perf/p4211-line-log.sh
@@ -23,11 +23,11 @@ test_perf 'git log --follow (baseline for -M)' '
git log --oneline --follow -- "$file" >/dev/null
 '
 
-test_perf 'git log -L' '
-   git log -L 1:"$file" >/dev/null
+test_perf 'git log -L (renames off)' '
+   git log --no-renames -L 1:"$file" >/dev/null
 '
 
-test_perf 'git log -M -L' '
+test_perf 'git log -L (renames on)' '
git log -M -L 1:"$file" >/dev/null
 '
 
-- 
2.9.0.204.g1499a7b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] t/perf: fix regression in testing older versions of git

[Jeff King]

Commit 7501b59 (perf: make the tests work in worktrees,
2016-05-13) introduced the use of "git rev-parse --git-path"
in the perf-lib setup code. Because the to-be-tested version
of git is at the front of the $PATH when this code runs,
this means we cannot use modern versions of t/perf to test
versions of git older than v2.5.0 (when that option was
introduced).

This is a symptom of a more general problem. The t/perf
suite is essentially independent of git versions, and
ideally we would be able to run the most modern and complete
set of tests across many historical versions (to see how
they compare). But any setup code they run is therefore
required to use the lowest common denominator we expect to
test.

So let's introduce a new variable, $MODERN_GIT, that we can
use both in perf-lib and in the test setup to get a reliable
set of git features (we might change git and break some
tests, of course, but $MODERN_GIT is tied to the same
version of git as the t/perf scripts, so they can be fixed
or adjusted together).

This commit fixes the "--git-path" case, but does not
mass-convert existing setup code to use $MODERN_GIT. Most
setup code is fairly vanilla and will work with effectively
all versions. But now the tool is there to fix any other
issues we find going forward.

Signed-off-by: Jeff King 
---
 t/perf/README  | 12 ++--
 t/perf/perf-lib.sh |  5 -
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/t/perf/README b/t/perf/README
index 8848c14..15986ca 100644
--- a/t/perf/README
+++ b/t/perf/README
@@ -115,8 +115,16 @@ After that you will want to use some of the following:
 
 At least one of the first two is required!
 
-You can use test_expect_success as usual.  For actual performance
-tests, use
+You can use test_expect_success as usual. In both test_expect_success
+and in test_perf, running "git" points to the version that is being
+peft-tested. The $MODERN_GIT variable points to the git wrapper for the
+currently checked-out version (i.e., the one that matches the t/perf
+scripts you are running).  This is useful if your setup uses commands
+that only work with newer versions of git than what you might want to
+test (but obviously your new commands must still create a state that can
+be used by the older version of git you are testing).
+
+For actual performance tests, use
 
test_perf 'descriptive string' '
command1 &&
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 18c363e..4eb2536 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -52,6 +52,9 @@ TEST_NO_MALLOC_CHECK=t
 # need to export them for test_perf subshells
 export TEST_DIRECTORY TRASH_DIRECTORY GIT_BUILD_DIR GIT_TEST_CMP
 
+MODERN_GIT=$GIT_BUILD_DIR/bin-wrappers/git
+export MODERN_GIT
+
 perf_results_dir=$TEST_OUTPUT_DIRECTORY/test-results
 mkdir -p "$perf_results_dir"
 rm -f "$perf_results_dir"/$(basename "$0" .sh).subtests
@@ -81,7 +84,7 @@ test_perf_create_repo_from () {
repo="$1"
source="$2"
source_git="$(git -C "$source" rev-parse --git-dir)"
-   objects_dir="$(git -C "$source" rev-parse --git-path objects)"
+   objects_dir="$("$MODERN_GIT" -C "$source" rev-parse --git-path objects)"
mkdir -p "$repo/.git"
(
cd "$source" &&
-- 
2.9.0.204.g1499a7b

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2] t/perf tests against older versions

[Jeff King]

One of the points of the t/perf suite is to be able to detect
performance regressions between versions. But I don't think anybody
really runs it systematically; we mostly just use it to show off our
shiny new improvements. :)

So I decided to run the suite against v2.0.0 and v2.9.0, to catch any
regressions that have crept in the past few years. The good news is that
there aren't any. But I did need a few patches to show that:

  [1/2]: t/perf: fix regression in testing older versions of git
  [2/2]: p4211: explicitly disable renames in no-rename test

The first one fixes the issue I reported in [1], which let me run the
suite against v2.0.0 at all. And the second fixes something that looks
like a regression in the results, but really isn't.

-Peff

[1] http://article.gmane.org/gmane.comp.version-control.git/297875


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Use docker for _some_ testing?

[Junio C Hamano]

Duy Nguyen  writes:

> The story started with my problem on Debian, which I didn't have and
> didn't want to install a Debian VM just for that problem. So I made a
> docker image with the following script.
>
> Which makes me think, could we use something like this to make sure
> people (on Linux) can test more obscure cases? Sometimes there are
> featues that require some dependencies that are not always present on
> the developer's machine (http server is a big one, locales come close
> second, then there will be lmdb and watchman in future...). With this,
> said developer can do a final test run in docker covering as much as
> possible.
>
> Of course it can't cover everything. Different compiler versions are
> out. OS-specific changes are out (but wine would be still good to test
> some aspect of Windows port, or at least make sure it builds)
>
> Comments?

Nice.

> -- 8< --
> diff --git a/contrib/docker/locale.gen b/contrib/docker/locale.gen
> new file mode 100644
> index 000..ef08e00
> --- /dev/null
> +++ b/contrib/docker/locale.gen
> @@ -0,0 +1,2 @@
> +is_IS.UTF-8 UTF-8
> +is_IS ISO-8859-1
> \ No newline at end of file
> diff --git a/contrib/docker/run.sh b/contrib/docker/run.sh
> new file mode 100755
> index 000..83e5679
> --- /dev/null
> +++ b/contrib/docker/run.sh
> @@ -0,0 +1,30 @@
> +#!/bin/sh
> +
> +die() {
> + echo "$@" >&2
> + exit 1
> +}
> +
> +build_debian() {
> + cat >Dockerfile <<-EOF
> + FROM debian:latest
> + RUN apt-get update && \
> + apt-get install -y libcurl4-gnutls-dev libexpat1-dev \
> + gettext libz-dev libssl-dev build-essential
> + RUN apt-get install -y locales
> + COPY locale.gen /etc/locale.gen
> + RUN locale-gen
> + RUN groupadd -r $(id -gn) -g $(id -g) && \
> + useradd -u $(id -u) -r -d "$HOME" -g $(id -g) -s /sbin/nologin 
> $(id -un)
> + USER $(id -un)
> + EOF
> + docker build -t $IMAGE .  || die "failed to build docker image"
> +}
> +
> +DISTRO=debian
> +IMAGE=git-$DISTRO-$(id -un)
> +ROOT="$(realpath $(git rev-parse --show-cdup))"
> +
> +test "$(docker images --format='{{.Repository}}' $IMAGE)" = $IMAGE || \
> + build_$DISTRO
> +docker run -it --rm -v "$ROOT":"$ROOT" -w "$(pwd)" $IMAGE bash
> -- 8< --
> --
> Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 00/11] Fix icase grep on non-ascii

[Junio C Hamano]

Junio C Hamano  writes:

> I think somebody's implementation of "echo" is not POSIX kosher.

Oops, I misspoke.  s/POSIX/XSI/; obviously.

But the conclusion is the same.  echo '\\\tA' may say \\\tA or
backslash HT A, depending on the system, so we cannot rely on that
in tests that are meant to be portable.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] grep: fix grepping for "intent to add" files

[Junio C Hamano]

Duy Nguyen  writes:

>>> If cached is false and ce_ita() is true and either CE_VALID or
>>> CE_SKIP_WORKTREE is set, we would continue to grep an _empty_ SHA-1.
>>> But I think we should grep_file() instead, at least for CE_VALID.
>>
>> Yes, that is the breakage I noticed in the patch under discussion
>> and that I wanted to fix in the "I wonder if a better change would
>> be..." version.
>
> Heh.. I did guess that. Since neither solution is complete, I'm in
> favor of Charles's and assume that i-t-a forces to ignore CE_SKIP and
> CE_SKIP_WORKTREE. I could wait for people to come back complaining,
> then we know there are real users in very obscure cases and will fix
> it then.

I said something that can be misunderstood.  I meant "I wonder if ..."
version is correct.  Charles's has the bugs you mentioned and I
wanted to fix them by sending the "I wonder if..." version out.

But you seem to have misread my statement as "A bug is in my version
and I want to fix that bug in my version".  That is not what I
meant.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] doc: git-htmldocs.googlecode.com is no more

[Junio C Hamano]

On Wed, Jun 22, 2016 at 12:00 PM, Eric Wong  wrote:
>
> Just wondering, who updates
> https://kernel.org/pub/software/scm/git/docs/
> and why hasn't it been updated in a while?
> (currently it says Last updated 2015-06-06 at the bottom)

Nobody. It is too cumbersome to use their upload tool to update many
files (it is geared towards updating a handful of tarballs at a time).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Use docker for _some_ testing?

[Duy Nguyen]

The story started with my problem on Debian, which I didn't have and
didn't want to install a Debian VM just for that problem. So I made a
docker image with the following script.

Which makes me think, could we use something like this to make sure
people (on Linux) can test more obscure cases? Sometimes there are
featues that require some dependencies that are not always present on
the developer's machine (http server is a big one, locales come close
second, then there will be lmdb and watchman in future...). With this,
said developer can do a final test run in docker covering as much as
possible.

Of course it can't cover everything. Different compiler versions are
out. OS-specific changes are out (but wine would be still good to test
some aspect of Windows port, or at least make sure it builds)

Comments?

-- 8< --
diff --git a/contrib/docker/locale.gen b/contrib/docker/locale.gen
new file mode 100644
index 000..ef08e00
--- /dev/null
+++ b/contrib/docker/locale.gen
@@ -0,0 +1,2 @@
+is_IS.UTF-8 UTF-8
+is_IS ISO-8859-1
\ No newline at end of file
diff --git a/contrib/docker/run.sh b/contrib/docker/run.sh
new file mode 100755
index 000..83e5679
--- /dev/null
+++ b/contrib/docker/run.sh
@@ -0,0 +1,30 @@
+#!/bin/sh
+
+die() {
+   echo "$@" >&2
+   exit 1
+}
+
+build_debian() {
+   cat >Dockerfile <<-EOF
+   FROM debian:latest
+   RUN apt-get update && \
+   apt-get install -y libcurl4-gnutls-dev libexpat1-dev \
+   gettext libz-dev libssl-dev build-essential
+   RUN apt-get install -y locales
+   COPY locale.gen /etc/locale.gen
+   RUN locale-gen
+   RUN groupadd -r $(id -gn) -g $(id -g) && \
+   useradd -u $(id -u) -r -d "$HOME" -g $(id -g) -s /sbin/nologin 
$(id -un)
+   USER $(id -un)
+   EOF
+   docker build -t $IMAGE .  || die "failed to build docker image"
+}
+
+DISTRO=debian
+IMAGE=git-$DISTRO-$(id -un)
+ROOT="$(realpath $(git rev-parse --show-cdup))"
+
+test "$(docker images --format='{{.Repository}}' $IMAGE)" = $IMAGE || \
+   build_$DISTRO
+docker run -it --rm -v "$ROOT":"$ROOT" -w "$(pwd)" $IMAGE bash
-- 8< --
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] doc: git-htmldocs.googlecode.com is no more

[Eric Wong]

Jonathan Nieder  wrote:
> I'd rather use an up-to-date https link for the rendered
> documentation, but I wasn't able to find one.  According to the 'todo'
> branch, prebuilt documentation is pushed to
> 
>  1. https://kernel.googlesource.com/pub/scm/git/git-htmldocs
>  2. git://repo.or.cz/git-htmldocs
>  3. somewhere on git.sourceforge.jp and git.sourceforge.net?
>  4. https://github.com/gitster/git-htmldocs
>  5. https://github.com/git/htmldocs

Just wondering, who updates
https://kernel.org/pub/software/scm/git/docs/
and why hasn't it been updated in a while?
(currently it says Last updated 2015-06-06 at the bottom)

I normally link people there since I would rather not advertise
for a commercial service.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 00/11] Fix icase grep on non-ascii

[Junio C Hamano]

On Wed, Jun 22, 2016 at 11:41 AM, Duy Nguyen  wrote:
> On Wed, Jun 22, 2016 at 8:36 PM, Junio C Hamano  wrote:
>> On Wed, Jun 22, 2016 at 11:29 AM, Duy Nguyen  wrote:
>>>
>>> Can any shell wizards explain this to me? With this code
>>>
>>> BS=\\
>>> echo ${BS}${BS}
>>>
>>> Debian's dash returns a single backslash while bash returns two
>>> backslashes. Section 2.2.1 [1] does not say anything about one
>>> backslash (hidden behind a variable!) after escaping the following one
>>> and still eats the one after that..
>>>
>>> [1] http://pubs.opengroup.org/onlinepubs/009604499/utilities/xcu_chap02.html
>>
>> I am not a wizard, but is the difference between the shell syntax, or just 
>> their
>> implementation of builtin-echo?  IOW, how do these three compare?
>>
>> printf "%s\n" "${BS}${BS}"
>> echo "${BS}${BS}"
>> echo ${BS}$BS}
>
> Great! printf shows two backslashes while both echo'es show one.
> printf "" behaves like echo though. Doesn't matter, at least I
> should be able to make the tests work on Debian dash.

I think somebody's implementation of "echo" is not POSIX kosher. According to

http://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html

you should expect a single backslash. If a script depends on seeing two, the
script is buggy.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/8] object_id part 4

[brian m. carlson]

On Tue, Jun 21, 2016 at 02:22:50PM -0700, Junio C Hamano wrote:
> "brian m. carlson"  writes:
> 
> > This series is part 4 in a series of conversions to replace instances of
> > unsigned char [20] with struct object_id.  Most of this series touches
> > the merge-recursive code.
> 
> I've queued them in 'pu', but please read contrib/examples/README
> ;-) Tentatively, I used contrib/coccinelle instead, but something
> even shorter (e.g. contrib/cocci) might be sufficient.

That seems fine with me.  I did read contrib/examples/README, but
figured it might be okay nevertheless.  I think contrib/coccinelle is
better, though.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH v6 00/11] Fix icase grep on non-ascii

[Duy Nguyen]

On Wed, Jun 22, 2016 at 8:36 PM, Junio C Hamano  wrote:
> On Wed, Jun 22, 2016 at 11:29 AM, Duy Nguyen  wrote:
>>
>> Can any shell wizards explain this to me? With this code
>>
>> BS=\\
>> echo ${BS}${BS}
>>
>> Debian's dash returns a single backslash while bash returns two
>> backslashes. Section 2.2.1 [1] does not say anything about one
>> backslash (hidden behind a variable!) after escaping the following one
>> and still eats the one after that..
>>
>> [1] http://pubs.opengroup.org/onlinepubs/009604499/utilities/xcu_chap02.html
>
> I am not a wizard, but is the difference between the shell syntax, or just 
> their
> implementation of builtin-echo?  IOW, how do these three compare?
>
> printf "%s\n" "${BS}${BS}"
> echo "${BS}${BS}"
> echo ${BS}$BS}

Great! printf shows two backslashes while both echo'es show one.
printf "" behaves like echo though. Doesn't matter, at least I
should be able to make the tests work on Debian dash.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] git-p4: correct hasBranchPrefix verbose output

[Junio C Hamano]

Andrew Oakley  writes:

> The logic here was inverted, you got a message saying the file is
> ignored for each file that is not ignored.
>
> Signed-off-by: Andrew Oakley 
> ---

Thanks.

>  git-p4.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index b6593cf..b123aa2 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -2674,7 +2674,7 @@ class P4Sync(Command, P4UserMap):
>  return True
>  hasPrefix = [p for p in self.branchPrefixes
>  if p4PathStartsWith(path, p)]
> -if hasPrefix and self.verbose:
> +if not hasPrefix and self.verbose:
>  print('Ignoring file outside of prefix: {0}'.format(path))
>  return hasPrefix
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 00/11] Fix icase grep on non-ascii

[Junio C Hamano]

On Wed, Jun 22, 2016 at 11:29 AM, Duy Nguyen  wrote:
>
> Can any shell wizards explain this to me? With this code
>
> BS=\\
> echo ${BS}${BS}
>
> Debian's dash returns a single backslash while bash returns two
> backslashes. Section 2.2.1 [1] does not say anything about one
> backslash (hidden behind a variable!) after escaping the following one
> and still eats the one after that..
>
> [1] http://pubs.opengroup.org/onlinepubs/009604499/utilities/xcu_chap02.html

I am not a wizard, but is the difference between the shell syntax, or just their
implementation of builtin-echo?  IOW, how do these three compare?

printf "%s\n" "${BS}${BS}"
echo "${BS}${BS}"
echo ${BS}$BS}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] grep: fix grepping for "intent to add" files

[Duy Nguyen]

On Wed, Jun 22, 2016 at 8:00 PM, Junio C Hamano  wrote:
> Duy Nguyen  writes:
>
>>> So I wonder if a better change would be more like
>>>
>>> for (...) {
>>> if (!S_ISREG(ce->ce_mode))
>>> continue; /* not a regular file */
>>> if (!ce_path_match(ce, pathspec, NULL)
>>> continue; /* uninteresting */
>>> +   if (cached && ce_intent_to_add(ce))
>>> +   continue; /* path not yet in the index */
>>>
>>> if (cached || ...)
>>> UNCHANGED FROM THE ORIGINAL
>>>
>>> perhaps?
>>
>> I did wonder a bit about these cases. But, can i-t-a really be
>> combined with CE_VALID or CE_SKIP_WORKTREE? CE_SKIP_... is
>> automatically set and should not cover i-t-a entries imo (I didn't
>> check the implementation). CE_VALID is about real entries, yes you
>> could do "git update-index --assume-unchanged " but it does
>> not feel right to me.
>
> Yeah but we know people are stupid^W^Wdo unexpected things ;-)
>
>> If cached is false and ce_ita() is true and either CE_VALID or
>> CE_SKIP_WORKTREE is set, we would continue to grep an _empty_ SHA-1.
>> But I think we should grep_file() instead, at least for CE_VALID.
>
> Yes, that is the breakage I noticed in the patch under discussion
> and that I wanted to fix in the "I wonder if a better change would
> be..." version.

Heh.. I did guess that. Since neither solution is complete, I'm in
favor of Charles's and assume that i-t-a forces to ignore CE_SKIP and
CE_SKIP_WORKTREE. I could wait for people to come back complaining,
then we know there are real users in very obscure cases and will fix
it then.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 00/11] Fix icase grep on non-ascii

[Duy Nguyen]

On Sat, Jun 18, 2016 at 2:26 AM, Duy Nguyen  wrote:
> On Sat, Jun 18, 2016 at 6:17 AM, Junio C Hamano  wrote:
>> Nguyễn Thái Ngọc Duy   writes:
>>
>>> v6 fixes comments from Ramsay and Eric. Interdiff below.
>>
>> Another thing I noticed with this is that the non-ascii test breaks
>> when run under dash (but passes under bash).  You need to have is_IS
>> locale on the system to see the breakage, it seems (which is why I
>> didn't see it so far).
>
> Is it a special version, maybe from debian? It works for me on gentoo.
>
>> ~/w/git/temp/t $ equery  --quiet list dash
> app-shells/dash-0.5.8.2
>> ~/w/git/temp/t $ dash ./t7812-grep-icase-non-ascii.sh
> # lib-gettext: Found 'is_IS.utf8' as an is_IS UTF-8 locale
> # lib-gettext: Found 'is_IS.iso88591' as an is_IS ISO-8859-1 locale
> ok 1 - setup
> ok 2 - grep literal string, no -F
> ok 3 - grep pcre utf-8 icase
> ok 4 - grep pcre utf-8 string with "+"
> ok 5 - grep literal string, with -F
> ok 6 - grep string with regex, with -F
> ok 7 - pickaxe -i on non-ascii
> # passed all 7 test(s)
> 1..7

Can any shell wizards explain this to me? With this code

BS=\\
echo ${BS}${BS}

Debian's dash returns a single backslash while bash returns two
backslashes. Section 2.2.1 [1] does not say anything about one
backslash (hidden behind a variable!) after escaping the following one
and still eats the one after that..

[1] http://pubs.opengroup.org/onlinepubs/009604499/utilities/xcu_chap02.html
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: How to find commits unique to a branch

[Junio C Hamano]

Nikolaus Rath  writes:

> On Jun 21 2016, Junio C Hamano  wrote:
>>
>> I find that the first sentence of the description is fuzzy
>> ("Determine whether" would imply that you would get "Yes/No" but
>> what we want is "here are the commits that do not have counterpart
>> in 2fix"),...
>
> This works, thanks! I don't quite understand why though. I started by
> saying that I want to know which commits in master are have been cherry
> picked after 3start was branched to 2fix, so .. must be
> 3start..2fix, which only leaves "master" as .  What's wrong
> with that thought?

The only thing that is wrong in that is it does not match what
happens, but that is not the fault of the "thought"; as I said, I
think the description is fuzzy and has room for improvement to avoid
being read in such a way that leads to a wrong conclusion.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] grep: fix grepping for "intent to add" files

[Junio C Hamano]

Duy Nguyen  writes:

>> So I wonder if a better change would be more like
>>
>> for (...) {
>> if (!S_ISREG(ce->ce_mode))
>> continue; /* not a regular file */
>> if (!ce_path_match(ce, pathspec, NULL)
>> continue; /* uninteresting */
>> +   if (cached && ce_intent_to_add(ce))
>> +   continue; /* path not yet in the index */
>>
>> if (cached || ...)
>> UNCHANGED FROM THE ORIGINAL
>>
>> perhaps?
>
> I did wonder a bit about these cases. But, can i-t-a really be
> combined with CE_VALID or CE_SKIP_WORKTREE? CE_SKIP_... is
> automatically set and should not cover i-t-a entries imo (I didn't
> check the implementation). CE_VALID is about real entries, yes you
> could do "git update-index --assume-unchanged " but it does
> not feel right to me.

Yeah but we know people are stupid^W^Wdo unexpected things ;-)

> If cached is false and ce_ita() is true and either CE_VALID or
> CE_SKIP_WORKTREE is set, we would continue to grep an _empty_ SHA-1.
> But I think we should grep_file() instead, at least for CE_VALID.

Yes, that is the breakage I noticed in the patch under discussion
and that I wanted to fix in the "I wonder if a better change would
be..." version.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] format-patch: avoid freopen()

[Junio C Hamano]

Johannes Schindelin  writes:

> Well, if I change `rev.diffopt.use_color != GIT_COLOR_ALWAYS` to
> `rev.diffopt.use_color == GIT_COLOR_AUTO`, then the files will contain
> ugly ANSI color sequences if I run `git format-patch -o . -3`.
>
> The reason is, as I tried to explain, that the use_color field is *not*
> initialized to GIT_COLOR_AUTO (which is equivalent to 2), but to -1.

OK.  I thought forcing no-color only when it is set to COLOR_AUTO or
it is set to -1 (the default) would be safer, but I changed my mind.

"when we add a new --color=,
overriding that end-user wish with the unconditional no-color is
likely to be seen as bug." was the implicit bias behind that
suggestion, but that is not substanticated and substatiatable.

If we write

if (rev.diffopt.use_color != GIT_COLOR_ALWAYS)
rev.diffopt.use_color = 0;

and if a user of --color= wonders why
her output is not colored, it is clear in the code above that we
disable unless it is set with --color=always, so it won't make
fixing such a future breakage harder.  In fact, if we did

if (rev.diffopt.use_color == GIT_COLOR_AUTO ||
rev.diffopt.use_color < 0)
rev.diffopt.use_color = 0;

it would make it _harder_ to spot where use_color is turned off when
the person who debugs such an issue.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] doc: git-htmldocs.googlecode.com is no more

[Jonathan Nieder]

http://git-htmldocs.googlecode.com/git/git.html says

 There was no service found for the uri requested.

Link to the rendered documentation on Jekyll instead.

Reported-by: Andrea Stacchiotti 
Signed-off-by: Jonathan Nieder 
---
Junio C Hamano wrote:

> Even though we do have index.html -> git.html, gh-pages does not
> seem to let you follow it, so you need to start from git.html, and I
> suspect that it is why the below says "I wasn't able to find it" for
> (5).

Yup, that explains it.

> So perhaps list both?  I do not know how beefy repo.or.cz is, or how
> well connected it is to the rest of the world, but if I have to pick
> only one, I'd feel safer if we didn't have to exploit the "blob_plain/:"
> backdoor.

There's only one option that uses https, so let's just use that one.

Thanks for the pointers.

Jonathan

 Documentation/git.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 5490d3c..0fb14f1 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -31,8 +31,8 @@ page to learn what commands Git offers.  You can learn more 
about
 individual Git commands with "git help command".  linkgit:gitcli[7]
 manual page gives you an overview of the command-line command syntax.
 
-Formatted and hyperlinked version of the latest Git documentation
-can be viewed at `http://git-htmldocs.googlecode.com/git/git.html`.
+A formatted and hyperlinked copy of the latest Git documentation
+can be viewed at `https://git.github.io/htmldocs/git.html`.
 
 ifdef::stalenotes[]
 [NOTE]
-- 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] format-patch: avoid freopen()

[Junio C Hamano]

Johannes Schindelin  writes:

> Well, if I change `rev.diffopt.use_color != GIT_COLOR_ALWAYS` to
> `rev.diffopt.use_color == GIT_COLOR_AUTO`, then the files will contain
> ugly ANSI color sequences if I run `git format-patch -o . -3`.
>
> The reason is, as I tried to explain, that the use_color field is *not*
> initialized to GIT_COLOR_AUTO (which is equivalent to 2), but to -1.

Ah, OK.

It still feels safer to force no-color only when it is auto
(i.e. the user said --color=auto) or -1 (i.e. the default), rather
than when it is anything other than always, though.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/2] Make find_commit_subject() consistent with --format=%s

[Junio C Hamano]

Johannes Schindelin  writes:

> In an intermediate iteration of my rebase--helper patches, I
> accidentally generated commits with more than one empty line
> between the header and the commit message. When using
> find_commit_subject() to show the oneline, it turned up empty, even
> if the output of `git show --format=%s` looked fine.

Much easier to read with s/this developer/I/g ;-)

> Turned out that the pretty-printing machinery helpfully skipped any
> blank lines before the commit message.
>
> In the first iteration of this patch, I failed to notice that
> the skip_empty_lines() function used by the pretty printing (which is
> declared static, and therefore I originally did not use it in order to
> keep the patch as minimal as possible) skips also blank lines.

By the way, I think skip_empty_lines() is misnamed, and I think your
use of "blank lines" in the previous paragraph indicates that you
agree ;-) It probably was OK back when it was a file-local static
helper in pretty.c, but it becomes a part of the global API with
1/2, renaming it to skip_blank_lines() may be a good thing to do
there at the same time.

I could do the tweaking while queuing if you too think it should
happen; that way we'd save one roundtrip ;-).

> To make things truly consistent, I now just make the skip_empty_lines()
> function public, and then use it.

Thanks.  Interdiff looks sensible.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Problem Following file history through sub-tree import

[Crabtree, Andrew]

Having difficulty understanding how to invoke 'git log' to track the history of 
a file that was imported into a different location through a subtree merge.
I had thought just '--follow' was needed, but I don't seem to be getting any 
results with that. 

Example below.

Thanks!
~Andrew

git --version
   git version 2.9.0
cd /tmp
mkdir subtree_test
cd subtree_test
git init 
touch foo
git add foo
git commit -m "initial"
git remote add git_src https://github.com/git/git
git fetch git_src
git merge --allow-unrelated-histories -s ours --no-commit git_src/master 
git read-tree --prefix=git_src -u git_src/master
git commit -m "import git as subtree"
cd git_src
git log pager.c
commit 22e7b2600b54f25314c399d45b1ea45d2427c754
Merge: 8df5087 ab7797d
Author: Andrew Crabtree 
Date:   Wed Jun 22 08:58:04 2016 -0700

import git as subtree
git log --follow pager.c
# nothing 

gitk pager.c *
# only shows merge commit 22e7b 

git gui blame pager.c & 
# shows history as expected 


git log HEAD^2 -- pager.c 
commit c3b1e8d85133e2a19d372b7c166d5b49fcbbfef2
Merge: 595bfef 708b8cc
Author: Junio C Hamano 
Date:   Wed Feb 24 13:26:01 2016 -0800

Merge branch 'jc/am-i-v-fix'

The "v(iew)" subcommand of the interactive "git am -i" command was
broken in 2.6.0 timeframe when the command was rewritten in C.

* jc/am-i-v-fix:
  am -i: fix "v"iew
  pager: factor out a helper to prepare a child process to run the pager
  pager: lose a separate argv[]

commit 3e3a4a41b0dac564c0302ced4ccc423d0d39bc21
Author: Junio C Hamano 
Date:   Tue Feb 16 14:34:44 2016 -0800
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: t7610-mergetool.sh test failure

[Jeff King]

On Wed, Jun 22, 2016 at 06:53:40PM +0200, Armin Kunaschik wrote:

> Another thread I'm trying to revive... the discussion went away quite a bit
> from the suggested patch to conditionally run this one test only when mktemp
> is available.
> 
> I'll create a patch when there are chances it is accepted.
> 
> I could think of a way to replace mktemp with a perl one-liner (or
> small script)...
> conditionally, when mktemp is not available... maybe in the build process?
> As far as I can see, perl is absolutely necessary and can therefore be used to
> "solve" the mktemp problem...
> 
> ...or maybe I should stop bringing this up again :-)

I think perl is necessary for the test suite, but not for git-mergetool
itself. And this is a problem in the script itself, IIRC; so it really
is broken on your system (albeit in a really tiny way), and not just a
test portability thing.

So the viable solutions to me are one of:

  1. Accept that this little part of mergetool doesn't work on systems
 without "mktemp", make sure we fail gracefully (we seem to), and make
 sure the test suite can handle this case (which was the earlier
 patch, I think).

  2. Implement our own git-mktemp (or similar abstraction) in C. We
 already have all the code, so it really is just a thin wrapper.

I don't mind (2), but given the lack of people clamoring for a fix to
mergetool itself, I'm perfectly happy with (1).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] doc: git-htmldocs.googlecode.com is no more

[Junio C Hamano]

Jonathan Nieder  writes:

> http://git-htmldocs.googlecode.com/git/git.html says
>
>  There was no service found for the uri requested.
>
> Link to the HTML documentation on repo.or.cz instead.

http://thread.gmane.org/gmane.comp.version-control.git/296483/focus=296575

Peff suggested to use the github pages that are connected to the
"git" org, and helped its initial set-up.  When I update the
prebuilt docs, in addition to the listed locations, I also push to
the gh-pages branch of https://github.com/git/htmldocs repository
and the result appears in https://git.github.io/htmldocs/git.html

Even though we do have index.html -> git.html, gh-pages does not
seem to let you follow it, so you need to start from git.html, and I
suspect that it is why the below says "I wasn't able to find it" for
(5).

So perhaps list both?  I do not know how beefy repo.or.cz is, or how
well connected it is to the rest of the world, but if I have to pick
only one, I'd feel safer if we didn't have to exploit the "blob_plain/:"
backdoor.

Thanks.

> I'd rather use an up-to-date https link for the rendered
> documentation, but I wasn't able to find one.  According to the 'todo'
> branch, prebuilt documentation is pushed to
>
>  1. https://kernel.googlesource.com/pub/scm/git/git-htmldocs
>  2. git://repo.or.cz/git-htmldocs
>  3. somewhere on git.sourceforge.jp and git.sourceforge.net?
>  4. https://github.com/gitster/git-htmldocs
>  5. https://github.com/git/htmldocs
>
> Of these, (1) and (4) don't provide a raw view with content-type
> text/html.  (5) might be available as HTML through Jekyll, but I
> wasn't able to find it --- e.g., https://git.github.io/htmldocs does
> not show those pages.  I wasn't able to find (3) at all.  That leaves
> (2).
>
> Reported-by: Andrea Stacchiotti 
> Signed-off-by: Jonathan Nieder 
> ---
> Hi Andrea,
>
> Andrea Stacchiotti wrote[1]:
>
>> In the git manual (`man git`), the documentation link:
>> > http://git-htmldocs.googlecode.com/git/git.html
>> is broken.
>
> Thanks for reporting.  How about this patch?
>
> Thanks,
> Jonathan
>
> [1] http://bugs.debian.org/827844
>
> -- >8 --
> Subject: doc: git-htmldocs.googlecode.com is no more
>
>  Documentation/git.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 5490d3c..de923db 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -31,8 +31,8 @@ page to learn what commands Git offers.  You can learn more 
> about
>  individual Git commands with "git help command".  linkgit:gitcli[7]
>  manual page gives you an overview of the command-line command syntax.
>  
> -Formatted and hyperlinked version of the latest Git documentation
> -can be viewed at `http://git-htmldocs.googlecode.com/git/git.html`.
> +A formatted and hyperlinked copy of the latest Git documentation
> +can be viewed at `http://repo.or.cz/git-htmldocs.git/blob_plain/:/git.html`.
>  
>  ifdef::stalenotes[]
>  [NOTE]
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 2/3] log: add "--no-show-signature" command line option

[Mehul Jain]

If an user creates an alias with "--show-signature" early in command
line, e.g.
[alias] logss = log --show-signature

then there is no way to countermand it through command line.

Teach git-log and related commands about "--no-show-signature" command
line option. This will make "git logss --no-show-signature" run
without showing GPG signature.

Signed-off-by: Mehul Jain 
---
 revision.c | 2 ++
 t/t4202-log.sh | 5 +
 2 files changed, 7 insertions(+)

diff --git a/revision.c b/revision.c
index d30d1c4..3546ff9 100644
--- a/revision.c
+++ b/revision.c
@@ -1871,6 +1871,8 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
revs->notes_opt.use_default_notes = 1;
} else if (!strcmp(arg, "--show-signature")) {
revs->show_signature = 1;
+   } else if (!strcmp(arg, "--no-show-signature")) {
+   revs->show_signature = 0;
} else if (!strcmp(arg, "--show-linear-break") ||
   starts_with(arg, "--show-linear-break=")) {
if (starts_with(arg, "--show-linear-break="))
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index ab66ee0..93a82e9 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -893,6 +893,11 @@ test_expect_success GPG 'log --graph --show-signature for 
merged tag' '
grep "^| | gpg: Good signature" actual
 '
 
+test_expect_success GPG '--no-show-signature overrides --show-signature' '
+   git log -1 --show-signature --no-show-signature signed >actual &&
+   ! grep "^gpg:" actual
+'
+
 test_expect_success 'log --graph --no-walk is forbidden' '
test_must_fail git log --graph --no-walk
 '
-- 
2.9.0.rc0.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 3/3] log: add log.showSignature configuration variable

[Mehul Jain]

Users may want to always use "--show-signature" while using git-log and
related commands.

When log.showSignature is set to true, git-log and related commands will
behave as if "--show-signature" was given to them.

Note that this config variable is meant to affect git-log, git-show,
git-whatchanged and git-reflog. Other commands like git-format-patch,
git-rev-list are not to be affected by this config variable.

Signed-off-by: Mehul Jain 
---
 Documentation/git-log.txt |  4 
 builtin/log.c |  6 ++
 t/t4202-log.sh| 20 
 t/t7510-signed-commit.sh  |  7 +++
 4 files changed, 37 insertions(+)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 03f9580..bbb5adc 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -196,6 +196,10 @@ log.showRoot::
`git log -p` output would be shown without a diff attached.
The default is `true`.
 
+log.showSignature::
+   If `true`, `git log` and related commands will act as if the
+   `--show-signature` option was passed to them.
+
 mailmap.*::
See linkgit:git-shortlog[1].
 
diff --git a/builtin/log.c b/builtin/log.c
index 099f4f7..7103217 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -33,6 +33,7 @@ static const char *default_date_mode = NULL;
 static int default_abbrev_commit;
 static int default_show_root = 1;
 static int default_follow;
+static int default_show_signature;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config;
@@ -119,6 +120,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)
rev->abbrev_commit = default_abbrev_commit;
rev->show_root_diff = default_show_root;
rev->subject_prefix = fmt_patch_subject_prefix;
+   rev->show_signature = default_show_signature;
DIFF_OPT_SET(>diffopt, ALLOW_TEXTCONV);
 
if (default_date_mode)
@@ -409,6 +411,10 @@ static int git_log_config(const char *var, const char 
*value, void *cb)
use_mailmap_config = git_config_bool(var, value);
return 0;
}
+   if (!strcmp(var, "log.showsignature")) {
+   default_show_signature = git_config_bool(var, value);
+   return 0;
+   }
 
if (grep_config(var, value, cb) < 0)
return -1;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 93a82e9..ecac186 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -898,6 +898,26 @@ test_expect_success GPG '--no-show-signature overrides 
--show-signature' '
! grep "^gpg:" actual
 '
 
+test_expect_success GPG 'log.showsignature=true behaves like --show-signature' 
'
+   test_config log.showsignature true &&
+   git log -1 signed >actual &&
+   grep "gpg: Signature made" actual &&
+   grep "gpg: Good signature" actual
+'
+
+test_expect_success GPG '--no-show-signature overrides log.showsignature=true' 
'
+   test_config log.showsignature true &&
+   git log -1 --no-show-signature signed >actual &&
+   ! grep "^gpg:" actual
+'
+
+test_expect_success GPG '--show-signature overrides log.showsignature=false' '
+   test_config log.showsignature false &&
+   git log -1 --show-signature signed >actual &&
+   grep "gpg: Signature made" actual &&
+   grep "gpg: Good signature" actual
+'
+
 test_expect_success 'log --graph --no-walk is forbidden' '
test_must_fail git log --graph --no-walk
 '
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 4177a86..6e839f5 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -210,4 +210,11 @@ test_expect_success GPG 'show lack of signature with 
custom format' '
test_cmp expect actual
 '
 
+test_expect_success GPG 'log.showsignature behaves like --show-signature' '
+   test_config log.showsignature true &&
+   git show initial >actual &&
+   grep "gpg: Signature made" actual &&
+   grep "gpg: Good signature" actual
+'
+
 test_done
-- 
2.9.0.rc0.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: t7610-mergetool.sh test failure

[Armin Kunaschik]

Another thread I'm trying to revive... the discussion went away quite a bit
from the suggested patch to conditionally run this one test only when mktemp
is available.

I'll create a patch when there are chances it is accepted.

I could think of a way to replace mktemp with a perl one-liner (or
small script)...
conditionally, when mktemp is not available... maybe in the build process?
As far as I can see, perl is absolutely necessary and can therefore be used to
"solve" the mktemp problem...

...or maybe I should stop bringing this up again :-)

Armin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/3] t4202: refactor test

[Mehul Jain]

Separate the creation of 'signed' branch so that other tests can take
advantage of this branch.

Signed-off-by: Mehul Jain 
---
 t/t4202-log.sh | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 128ba93..ab66ee0 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -860,12 +860,15 @@ test_expect_success 'dotdot is a parent directory' '
test_cmp expect actual
 '
 
-test_expect_success GPG 'log --graph --show-signature' '
+test_expect_success 'setup signed branch' '
test_when_finished "git reset --hard && git checkout master" &&
git checkout -b signed master &&
echo foo >foo &&
git add foo &&
-   git commit -S -m signed_commit &&
+   git commit -S -m signed_commit
+'
+
+test_expect_success GPG 'log --graph --show-signature' '
git log --graph --show-signature -n1 signed >actual &&
grep "^| gpg: Signature made" actual &&
grep "^| gpg: Good signature" actual
-- 
2.9.0.rc0.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 0/3] Introduce log.showSignature config variable

[Mehul Jain]

Add a new configuratation variable "log.showSignature" for git-log
and related commands. "log.showSignature=true" will enable user to
see GPG signature by default for git-log and related commands.

Changes compared to v2:
* A preparatory patch 1/3 has been introduced so that tests
  in patches 2/3 and 3/3 can take advantage of it.
* Mistake regarding branch in [patch v2 2/2] has been
  corrected.
* Tight coupling between the tests in [patch v2 2/2] has
  been resovled.

I would like to thanks Eric Sunshine for his feedback on previous
series [1].

[1]: http://thread.gmane.org/gmane.comp.version-control.git/297648 

Mehul Jain (3):
  t4202: refactoring of test
  log: add "--no-show-signature" command line option
  log: add log.showSignature configuration variable

 Documentation/git-log.txt |  4 
 builtin/log.c |  6 ++
 revision.c|  2 ++
 t/t4202-log.sh| 32 ++--
 t/t7510-signed-commit.sh  |  7 +++
 5 files changed, 49 insertions(+), 2 deletions(-)

-- 
2.9.0.rc0.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] mingw: work around t2300's assuming non-Windows paths

[Junio C Hamano]

Johannes Schindelin  writes:

> On Tue, 21 Jun 2016, Junio C Hamano wrote:
>
>> I said $PATH because --exec-path does not care what you do with
>> %PATH% but it deeply cares that its output is usable in $PATH.
>
> The really, really, really important part to keep in mind is that there is
> no $PATH on Windows.

I think I know that well enough; please sanity check.  My
understanding is:

 * Your (emulated) getenv(3) reads %PATH% which may look like
   "c:\a\b;c:\c\d", i.e. Windows style.

 * Your argv_exec_path also looks like "c:\e\f", i.e. Windows
   style.

 * Your setup_path() would yield "c:\e\f;c:\a\b;c:\c\d" because it
   concatenates the above two with PATH_SEP, i.e. Windows style, and
   your setenv(3) will set that to %PATH%.

 * After all that happens, your run_command(), execv_git_cmd(),
   etc. would honor that %PATH%.

 * In all of the above, a back-slash can be (and may indeed be) a
   forward-slash, as library functions and system calls are prepared
   to take both since the good old DOS days.  I.e. "c:\a\b" can be
   "c:/a/b".  It cannot be "/c/a/b", however.

 * When bash gets spawned (perhaps because a hook is written in that
   language, perhaps because child_process.use_shell is set when
   executing an alias "!cmd", running a pager, or running an
   editor), the value of %PATH% derived by the above sequence is not
   exposed as $PATH.  There is the "rewrite leading C: with /C/"
   outside us (i.e. in bash).

And that is why I suggested to keep that "internal paths are in
platform native format" and apply the same conversion as what bash
does immediately before the returned value from git_exec_path() is
fed to puts().  That way, "$PATH" (not %PATH%) can be modified with
"$(git --exec-path)" in scripts the same way on all the platforms
and you do not have to break people's hooks.

Having said that, I realize that I missed one huge thing to take
into consideration.  I assume that you have been shipping Git for
Windows with this "'git --exec-path' gives c:\a\b, not /c/a/b"
feature for a long time, so existing Windows users will be broken if
we "fix" this (which would allow them to use shell scripts their
friends use on other platforms without modification).  So from that
point alone, "PATH=$(git --exec-path):$PATH in shell should work on
any platform and "git --exec-path" should be fixed" would not fly.
This simply is too late to fix.

The patch under discussion is the only door left for that test, and
a similar trickery is needed for any end-user scripts used for hooks
and aliases that use 'git --exec-path', if they ever want to be
cross-platform.

So let's take that "if Windows do this" change to t2300 as-is.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: How to find commits unique to a branch

[Nikolaus Rath]

On Jun 21 2016, Michael J Gruber  wrote:
> Nikolaus Rath venit, vidit, dixit 21.06.2016 01:21:
>> On Jun 20 2016, Junio C Hamano  wrote:
>>> Nikolaus Rath  writes:
>>>
 What's the best way to find all commits in a branch A that have not been
 cherry-picked from (or to) another branch B?

 I think I could format-patch all commits in every branch into separate
 files, hash the Author and Date of each files, and then compare the two
 lists. But I'm hoping there's a way to instead have git do the
 heavy-lifting?
>>>
>>> "git cherry" perhaps?
>> 
>> That seems to work only the "wrong way around". I have a tag
>> fuse_3_0_start, which is the common ancestor to "master" and
>> "fuse_2_9_bugfix". I'd like to find all the commits from fuse_3_0_start
>> to master that have not been cherry-picked into fuse_2_9_bugfix.
>> 
>> However:
>> 
>> * "git cherry fuse_3_0_start master release2.9" tells me nothing has
>>   been cherry-picked at all (only lines with +)
>> 
>> * "git cherry fuse_3_0_start release2.9 master" also tells me nothing
>>   has been cherry picked, but somehow shows a smaller total number of
>>   commits.
>> 
>> * "git cherry master release2.9 fuse_3_0_start" gives me the commits
>>   from fuse_2_9_bugfix that have not been cherry-picked into master
>>   (which seems to be in contradiction to the two earlier commands).
>> 
>> 
>> Am I missing something obvious?
>
> There is always
>
> git log --left-right --cherry-mark A...B
>
> to give you a good overview of the situation.

This worked nicely too, thanks!

Best,
-Nikolaus

-- 
GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F
Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

 »Time flies like an arrow, fruit flies like a Banana.«
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: How to find commits unique to a branch

[Nikolaus Rath]

On Jun 21 2016, Junio C Hamano  wrote:
> Nikolaus Rath  writes:
>
>> On Jun 20 2016, Nikolaus Rath  wrote:
>>> On Jun 20 2016, Junio C Hamano  wrote:
 Nikolaus Rath  writes:

> What's the best way to find all commits in a branch A that have not been
> cherry-picked from (or to) another branch B?
>
> I think I could format-patch all commits in every branch into separate
> files, hash the Author and Date of each files, and then compare the two
> lists. But I'm hoping there's a way to instead have git do the
> heavy-lifting?

 "git cherry" perhaps?
>>>
>>> That seems to work only the "wrong way around". I have a tag
>>> fuse_3_0_start, which is the common ancestor to "master" and
>>> "fuse_2_9_bugfix". I'd like to find all the commits from fuse_3_0_start
>>> to master that have not been cherry-picked into fuse_2_9_bugfix.
>
> Hmm, so the topology roughly would look like:
>
> A'--B'--D' 2fix
>/
>   o---A---B---C---D---E---F master
>   3start
>
> And you want to find commits in 3start..master that do not have
> equivalent in 3start..2fix
>
> "git cherry --help" starts like this:
>
> NAME
>git-cherry - Find commits yet to be applied to upstream
>
> SYNOPSIS
>git cherry [-v] [ [ []]]
>
> DESCRIPTION
>Determine whether there are commits in ..
>that are equivalent to those in the range ...
>
> Applying that to our picture, we want to find commits yet to be
> applied to 2fix, and do so by comparing the commits between
> 3start..master and 3start..2fix.
>
> I find that the first sentence of the description is fuzzy
> ("Determine whether" would imply that you would get "Yes/No" but
> what we want is "here are the commits that do not have counterpart
> in 2fix"), but we already know  corresponds to 2fix
> (i.e. we are finding ones yet to be applied to there, which can be
> inferred from the NAME line), so  must be 'master' That means
> that  corresponds to 3start, and we will be comparing commits
> in two ranges:
>
> master..2fix (i e. .., which is the same thing as 
> 3start..2fix)
> 3start..master (i.e. ..)
>
> So perhaps "git cherry -v 2fix master 3start"?

This works, thanks! I don't quite understand why though. I started by
saying that I want to know which commits in master are have been cherry
picked after 3start was branched to 2fix, so .. must be
3start..2fix, which only leaves "master" as .  What's wrong
with that thought?


Best,
-Nikolaus

-- 
GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F
Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

 »Time flies like an arrow, fruit flies like a Banana.«
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] format-patch: avoid freopen()

[Johannes Schindelin]

Hi Junio,

On Wed, 22 Jun 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > But there's a rub... If you specify --color *explicitly*, use_color is set
> > to GIT_COLOR_ALWAYS and the file indeed contains ANSI sequences (i.e. my
> > analysis above left out the command-line part).
> 
> Heh, the command-line is the _ONLY_ thing I raised, as we knew
> ui.color is not an issue in this codepath, in $gmane/297757.
> 
> Going back to that and reading again, I suggested to check with
> GIT_COLOR_AUTO (i.e. if it is left to "auto", disable it) because I
> think the former is a much more future-proof way (imagine that we
> may add --color= in the future) than checking with
> GIT_COLOR_ALWAYS (i.e. if it is not explicitly set to "always",
> disable it).

Well, if I change `rev.diffopt.use_color != GIT_COLOR_ALWAYS` to
`rev.diffopt.use_color == GIT_COLOR_AUTO`, then the files will contain
ugly ANSI color sequences if I run `git format-patch -o . -3`.

The reason is, as I tried to explain, that the use_color field is *not*
initialized to GIT_COLOR_AUTO (which is equivalent to 2), but to -1.

So the difference between your version and mine is that mine works ;-)

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] grep: fix grepping for "intent to add" files

[Duy Nguyen]

On Wed, Jun 22, 2016 at 12:49 AM, Junio C Hamano  wrote:
>> @@ -396,7 +396,7 @@ static int grep_cache(struct grep_opt *opt, const struct 
>> pathspec *pathspec, int
>>* cache version instead
>>*/
>>   if (cached || (ce->ce_flags & CE_VALID) || 
>> ce_skip_worktree(ce)) {
>> - if (ce_stage(ce))
>> + if (ce_stage(ce) || ce_intent_to_add(ce))
>>   continue;
>>   hit |= grep_sha1(opt, ce->sha1, ce->name, 0, ce->name);
>>   }
>
> OK, so this function handles searching in either the index or the
> working tree.
>
> The first hunk used to unconditionally discard paths marked as
> i-t-a, even when we are looking at the working tree, which is
> clearly useless, and we stop rejecting i-t-a paths too early, which
> is good.
>
> The second hunk is for "grep --cached" but also covers two other
> cases.  What are these?
>
> CE_VALID is used by "Assume unchanged".  Because the user promised
> that s/he will take responsibility of keeping the working tree
> contents in sync with what is in the index by not modifying it, even
> when we are not doing "grep --cached", we pick up the contents from
> the index and look for the string in there, instead of going to the
> working tree.  In other words, even though at the mechanical level
> we are looking into the index, logically we are searching in the
> working tree.  Is it sensible to skip i-t-a entries in that case?
>
> I think the same discussion would apply to CE_SKIP_WORKTREE (see
> "Skip-worktree bit" in Documentation/git-update-index.txt).
>
> So I wonder if a better change would be more like
>
> for (...) {
> if (!S_ISREG(ce->ce_mode))
> continue; /* not a regular file */
> if (!ce_path_match(ce, pathspec, NULL)
> continue; /* uninteresting */
> +   if (cached && ce_intent_to_add(ce))
> +   continue; /* path not yet in the index */
>
> if (cached || ...)
> UNCHANGED FROM THE ORIGINAL
>
> perhaps?

I did wonder a bit about these cases. But, can i-t-a really be
combined with CE_VALID or CE_SKIP_WORKTREE? CE_SKIP_... is
automatically set and should not cover i-t-a entries imo (I didn't
check the implementation). CE_VALID is about real entries, yes you
could do "git update-index --assume-unchanged " but it does
not feel right to me.

If cached is false and ce_ita() is true and either CE_VALID or
CE_SKIP_WORKTREE is set, we would continue to grep an _empty_ SHA-1.
But I think we should grep_file() instead, at least for CE_VALID.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] grep: fix grepping for "intent to add" files

[Duy Nguyen]

On Wed, Jun 22, 2016 at 3:13 AM, Eric Sunshine  wrote:
> On Tue, Jun 21, 2016 at 5:14 PM, Charles Bailey  wrote:
>> From: Charles Bailey 
>>
>> This reverts commit 4d552005323034c1d6311796ac1074e9a4b4b57e and adds an
>> alternative fix to maintain the -L --cached behavior.
>
> It is common to provide some context along with the (shortened) commit
> ID. For instance:
>
> This reverts 4d55200 (grep: make it clear i-t-a entries are
> ignored, 2015-12-27) and adds ...

And that could be produced with some git alias like

git config alias.one 'show -s --date=short --pretty='format:%h (%s - %ad)'

No point in manually copy/pasting the context.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] format-patch: avoid freopen()

[Junio C Hamano]

Johannes Schindelin  writes:

> But there's a rub... If you specify --color *explicitly*, use_color is set
> to GIT_COLOR_ALWAYS and the file indeed contains ANSI sequences (i.e. my
> analysis above left out the command-line part).

Heh, the command-line is the _ONLY_ thing I raised, as we knew
ui.color is not an issue in this codepath, in $gmane/297757.

Going back to that and reading again, I suggested to check with
GIT_COLOR_AUTO (i.e. if it is left to "auto", disable it) because I
think the former is a much more future-proof way (imagine that we
may add --color= in the future) than checking with
GIT_COLOR_ALWAYS (i.e. if it is not explicitly set to "always",
disable it).

> In short, I think you're right, I have to guard the assignment, with the
> minor adjustment to test use_color != GIT_COLOR_ALWAYS.

So I am not sure if you said "use_color != GIT_COLOR_ALWAYS" only to
be different from what I suggested (you seem to have a tendency to
do so whenever you can), or there was some other reason.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Problem with --shallow-submodules option

[Fredrik Gustafsson]

On Mon, Jun 20, 2016 at 01:06:39PM +, Istvan Zakar wrote:
> I'm working on a relatively big project with many submodules. During 
> cloning for testing I tried to decrease the amount of data need to be 
> fetched from the server by using --shallow-submodules option in the clone 
> command. It seems to check out the tip of the remote repo, and if it's not 
> the commit registered in the superproject the submodule update fails 
> (obviously). Can I somehow tell to fetch that exact commit I need for my 
> superproject?

Maybe. http://stackoverflow.com/questions/2144406/git-shallow-submodules
gives a good overview of this problem.

git fetches a branch and is shallow from that branch, which might be an
other sha1 than the one the submodule points to, (as you say). This
is/was one of the drawbacks with this method. However the since git 2.8,
git will try to fetch the sha1 direct (and not the branch). So then it
will work, if(!), the server supports direct access to sha1. This was
previously not allowed due to security concerns (if I recall correctly).

So the answer is, yes this will work if you've a recent version of git
and support on the server side for doing this. Unfortunately I'm not
sure which git version is needed on the server side for this to work.

-- 
Fredrik Gustafsson

phone: +46 733-608274
e-mail: iv...@iveqy.com
website: http://www.iveqy.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/9] Disallow diffopt.close_file when using the log_tree machinery

[Johannes Schindelin]

Hi Junio,

On Tue, 21 Jun 2016, Junio C Hamano wrote:

> Junio C Hamano  writes:
> 
> > Junio C Hamano  writes:
> >
> >> Johannes Schindelin  writes:
> >>
> >>> We are about to teach the log_tree machinery to reuse the diffopt.file
> >>> setting to output to a file stream different from stdout.
> >>>
> >>> This means that builtin am can no longer ask the diff machinery to
> >>> close the file when actually calling the log_tree machinery (which
> >>> wants to flush the very same file stream that would then already be
> >>> closed).
> >>
> >> Sorry for being slow, but I am not sure why the first paragraph has
> >> to mean the second paragraph.  This existing caller opens a new
> >> stream, sets .fp to it, and expects that the log_tree_commit() to
> >> close it if told by setting .close_file to true, all of which sounds
> >> sensible.
> >>
> >> If a codepath wants to use the same stream for two or more calls to
> >> log_tree by pointing the stream with .fp, it would be of course a
> >> problem for the caller to set .close_file to true in its first call,
> >> as .fp will be closed and no longer usable for second and subsequent
> >> call, and that would be a bug, but for a single-shot call it feels
> >> entirely a sensible request to make, no?
> >>
> >> Obviously you have looked at the codepaths involved a lot longer
> >> than I did, and I do not doubt your conclusion, but I cannot quite
> >> convince myself with the above explanation.
> >>
> >> The option parser of "git diff" family sets ->close_file to true
> >> when the --output option is given.
> >>
> >> Wouldn't this patch break "git log --output=foo -3"?
> >
> > I wonder if the right approach is to stop using .close_file
> > everywhere.
> >
> > With this "do not set .close_file if you use log_tree_commit()",
> > "git log --output=/dev/stdout -3" gets broken, but removing that
> > check is not sufficient to correct the same command with "-p", as
> > letting .close_file to close the output file after finishing a
> > single diff would mean that subsequent write to the same file
> > descriptor will trigger a failure.
> 
> We could say "git log --output=foo -3 [-p]" without any of your
> patches is already broken, and it is a valid excuse to take this
> change that we are not making things worse with it.
> 
> It is just 3/9 is a logical first step to correct that exact
> problem, i.e. some codepaths, even though there is a place that
> holds the output stream and command line parser does prepare one for
> "foo" when --output=foo is given, ignore it and send thigns to the
> standard output stream.  You might not have written 3/9 in order to
> fix that "git log --output=foo" problem, but a fix for it should
> look exactly like your 3/9, I would think.
> 
> And it is sad that this step makes that fix impossible.

Okay, I ended up seeing that there is no way I can avoid Doing The Right
Thing.

It is not quite as pretty as I hoped it would be (callers of
log_tree_commit() that want to call it in a loop still have to override
close_file manually), but it does produce a nicer story.

Please see the fixes in the latest iteration I just sent out.

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 07/10] format-patch: avoid freopen()

[Johannes Schindelin]

We just taught the relevant functions to respect the diffopt.file field,
to allow writing somewhere else than stdout. Let's make use of it.

Technically, we do not need to avoid that call in a builtin: we assume
that builtins (as opposed to library functions) are stand-alone programs
that may do with their (global) state. Yet, we want to be able to reuse
that code in properly lib-ified code, e.g. when converting scripts into
builtins.

Further, while we did not *have* to touch the cmd_show() and cmd_cherry()
code paths (because they do not want to write anywhere but stdout as of
yet), it just makes sense to be consistent, making it easier and safer to
move the code later.

Signed-off-by: Johannes Schindelin 
---
 builtin/log.c | 64 ++-
 1 file changed, 33 insertions(+), 31 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 5683a42..869c23b 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -236,7 +236,7 @@ static void show_early_header(struct rev_info *rev, const 
char *stage, int nr)
if (rev->commit_format != CMIT_FMT_ONELINE)
putchar(rev->diffopt.line_termination);
}
-   printf(_("Final output: %d %s\n"), nr, stage);
+   fprintf(rev->diffopt.file, _("Final output: %d %s\n"), nr, stage);
 }
 
 static struct itimerval early_output_timer;
@@ -454,7 +454,7 @@ static void show_tagger(char *buf, int len, struct rev_info 
*rev)
pp.fmt = rev->commit_format;
pp.date_mode = rev->date_mode;
pp_user_info(, "Tagger", , buf, get_log_output_encoding());
-   printf("%s", out.buf);
+   fprintf(rev->diffopt.file, "%s", out.buf);
strbuf_release();
 }
 
@@ -465,7 +465,7 @@ static int show_blob_object(const unsigned char *sha1, 
struct rev_info *rev, con
char *buf;
unsigned long size;
 
-   fflush(stdout);
+   fflush(rev->diffopt.file);
if (!DIFF_OPT_TOUCHED(>diffopt, ALLOW_TEXTCONV) ||
!DIFF_OPT_TST(>diffopt, ALLOW_TEXTCONV))
return stream_blob_to_fd(1, sha1, NULL, 0);
@@ -505,7 +505,7 @@ static int show_tag_object(const unsigned char *sha1, 
struct rev_info *rev)
}
 
if (offset < size)
-   fwrite(buf + offset, size - offset, 1, stdout);
+   fwrite(buf + offset, size - offset, 1, rev->diffopt.file);
free(buf);
return 0;
 }
@@ -514,7 +514,8 @@ static int show_tree_object(const unsigned char *sha1,
struct strbuf *base,
const char *pathname, unsigned mode, int stage, void *context)
 {
-   printf("%s%s\n", pathname, S_ISDIR(mode) ? "/" : "");
+   FILE *file = context;
+   fprintf(file, "%s%s\n", pathname, S_ISDIR(mode) ? "/" : "");
return 0;
 }
 
@@ -574,7 +575,7 @@ int cmd_show(int argc, const char **argv, const char 
*prefix)
 
if (rev.shown_one)
putchar('\n');
-   printf("%stag %s%s\n",
+   fprintf(rev.diffopt.file, "%stag %s%s\n",
diff_get_color_opt(, 
DIFF_COMMIT),
t->tag,
diff_get_color_opt(, 
DIFF_RESET));
@@ -593,12 +594,12 @@ int cmd_show(int argc, const char **argv, const char 
*prefix)
case OBJ_TREE:
if (rev.shown_one)
putchar('\n');
-   printf("%stree %s%s\n\n",
+   fprintf(rev.diffopt.file, "%stree %s%s\n\n",
diff_get_color_opt(, 
DIFF_COMMIT),
name,
diff_get_color_opt(, 
DIFF_RESET));
read_tree_recursive((struct tree *)o, "", 0, 0, 
_all,
-   show_tree_object, NULL);
+   show_tree_object, rev.diffopt.file);
rev.shown_one = 1;
break;
case OBJ_COMMIT:
@@ -808,7 +809,7 @@ static FILE *realstdout = NULL;
 static const char *output_directory = NULL;
 static int outdir_offset;
 
-static int reopen_stdout(struct commit *commit, const char *subject,
+static int open_next_file(struct commit *commit, const char *subject,
 struct rev_info *rev, int quiet)
 {
struct strbuf filename = STRBUF_INIT;
@@ -832,7 +833,7 @@ static int reopen_stdout(struct commit *commit, const char 
*subject,
if (!quiet)
fprintf(realstdout, "%s\n", filename.buf + outdir_offset);
 
-   if (freopen(filename.buf, "w", stdout) == NULL)
+   if ((rev->diffopt.file = fopen(filename.buf, "w")) == NULL)
return error(_("Cannot open patch file %s"), filename.buf);
 
strbuf_release();
@@ -891,15 +892,15 @@ static void 

[PATCH v4 08/10] format-patch: use stdout directly

[Johannes Schindelin]

Earlier, we freopen()ed stdout in order to write patches to files.
That forced us to duplicate stdout (naming it "realstdout") because we
*still* wanted to be able to report the file names.

As we do not abuse stdout that way anymore, we no longer need to
duplicate stdout, either.

Signed-off-by: Johannes Schindelin 
---
 builtin/log.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 869c23b..2a42216 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -805,7 +805,6 @@ static int git_format_config(const char *var, const char 
*value, void *cb)
return git_log_config(var, value, cb);
 }
 
-static FILE *realstdout = NULL;
 static const char *output_directory = NULL;
 static int outdir_offset;
 
@@ -831,7 +830,7 @@ static int open_next_file(struct commit *commit, const char 
*subject,
fmt_output_subject(, subject, rev);
 
if (!quiet)
-   fprintf(realstdout, "%s\n", filename.buf + outdir_offset);
+   printf("%s\n", filename.buf + outdir_offset);
 
if ((rev->diffopt.file = fopen(filename.buf, "w")) == NULL)
return error(_("Cannot open patch file %s"), filename.buf);
@@ -1639,9 +1638,6 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
get_patch_ids(, );
}
 
-   if (!use_stdout)
-   realstdout = xfdopen(xdup(1), "w");
-
if (prepare_revision_walk())
die(_("revision walk setup failed"));
rev.boundary = 1;
-- 
2.9.0.118.g0e1a633


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 09/10] shortlog: respect the --output= setting

[Johannes Schindelin]

Thanks to the diff option parsing, we already know about this option.
We just have to make use of it.

Signed-off-by: Johannes Schindelin 
---
 builtin/shortlog.c  | 4 +++-
 t/t4201-shortlog.sh | 6 ++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 39d74fe..be80547 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -229,7 +229,6 @@ void shortlog_init(struct shortlog *log)
log->wrap = DEFAULT_WRAPLEN;
log->in1 = DEFAULT_INDENT1;
log->in2 = DEFAULT_INDENT2;
-   log->file = stdout;
 }
 
 int cmd_shortlog(int argc, const char **argv, const char *prefix)
@@ -277,6 +276,7 @@ parse_done:
 
log.user_format = rev.commit_format == CMIT_FMT_USERFORMAT;
log.abbrev = rev.abbrev;
+   log.file = rev.diffopt.file;
 
/* assume HEAD if from a tty */
if (!nongit && !rev.pending.nr && isatty(0))
@@ -290,6 +290,8 @@ parse_done:
get_from_rev(, );
 
shortlog_output();
+   if (log.file != stdout)
+   fclose(log.file);
return 0;
 }
 
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index a977365..bd699e1 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -184,4 +184,10 @@ test_expect_success 'shortlog with revision pseudo 
options' '
git shortlog --exclude=refs/heads/m* --all
 '
 
+test_expect_success 'shortlog with --output=' '
+   git shortlog --output=shortlog master >output &&
+   test ! -s output &&
+   test_line_count = 7 shortlog
+'
+
 test_done
-- 
2.9.0.118.g0e1a633


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 03/10] line-log: respect diffopt's configured output file stream

[Johannes Schindelin]

The diff machinery can optionally output to a file stream other than
stdout, by overriding diffopt.file. In such a case, the rest of the
log tree machinery should also write to that stream.

Currently, there is no user of the line level log that wants to
redirect output to a file. Therefore, one might argue that it is
superfluous to support that now. However, it is better to be
consistent now, rather than to face hard-to-debug problems later.

Signed-off-by: Johannes Schindelin 
---
 line-log.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/line-log.c b/line-log.c
index bbe31ed..e62a7f4 100644
--- a/line-log.c
+++ b/line-log.c
@@ -841,7 +841,7 @@ static char *get_nth_line(long line, unsigned long *ends, 
void *data)
 
 static void print_line(const char *prefix, char first,
   long line, unsigned long *ends, void *data,
-  const char *color, const char *reset)
+  const char *color, const char *reset, FILE *file)
 {
char *begin = get_nth_line(line, ends, data);
char *end = get_nth_line(line+1, ends, data);
@@ -852,14 +852,14 @@ static void print_line(const char *prefix, char first,
had_nl = 1;
}
 
-   fputs(prefix, stdout);
-   fputs(color, stdout);
-   putchar(first);
-   fwrite(begin, 1, end-begin, stdout);
-   fputs(reset, stdout);
-   putchar('\n');
+   fputs(prefix, file);
+   fputs(color, file);
+   putc(first, file);
+   fwrite(begin, 1, end-begin, file);
+   fputs(reset, file);
+   putc('\n', file);
if (!had_nl)
-   fputs("\\ No newline at end of file\n", stdout);
+   fputs("\\ No newline at end of file\n", file);
 }
 
 static char *output_prefix(struct diff_options *opt)
@@ -898,12 +898,12 @@ static void dump_diff_hacky_one(struct rev_info *rev, 
struct line_log_data *rang
fill_line_ends(pair->one, _lines, _ends);
fill_line_ends(pair->two, _lines, _ends);
 
-   printf("%s%sdiff --git a/%s b/%s%s\n", prefix, c_meta, pair->one->path, 
pair->two->path, c_reset);
-   printf("%s%s--- %s%s%s\n", prefix, c_meta,
+   fprintf(opt->file, "%s%sdiff --git a/%s b/%s%s\n", prefix, c_meta, 
pair->one->path, pair->two->path, c_reset);
+   fprintf(opt->file, "%s%s--- %s%s%s\n", prefix, c_meta,
   pair->one->sha1_valid ? "a/" : "",
   pair->one->sha1_valid ? pair->one->path : "/dev/null",
   c_reset);
-   printf("%s%s+++ b/%s%s\n", prefix, c_meta, pair->two->path, c_reset);
+   fprintf(opt->file, "%s%s+++ b/%s%s\n", prefix, c_meta, pair->two->path, 
c_reset);
for (i = 0; i < range->ranges.nr; i++) {
long p_start, p_end;
long t_start = range->ranges.ranges[i].start;
@@ -945,7 +945,7 @@ static void dump_diff_hacky_one(struct rev_info *rev, 
struct line_log_data *rang
}
 
/* Now output a diff hunk for this range */
-   printf("%s%s@@ -%ld,%ld +%ld,%ld @@%s\n",
+   fprintf(opt->file, "%s%s@@ -%ld,%ld +%ld,%ld @@%s\n",
   prefix, c_frag,
   p_start+1, p_end-p_start, t_start+1, t_end-t_start,
   c_reset);
@@ -953,18 +953,18 @@ static void dump_diff_hacky_one(struct rev_info *rev, 
struct line_log_data *rang
int k;
for (; t_cur < diff->target.ranges[j].start; t_cur++)
print_line(prefix, ' ', t_cur, t_ends, 
pair->two->data,
-  c_context, c_reset);
+  c_context, c_reset, opt->file);
for (k = diff->parent.ranges[j].start; k < 
diff->parent.ranges[j].end; k++)
print_line(prefix, '-', k, p_ends, 
pair->one->data,
-  c_old, c_reset);
+  c_old, c_reset, opt->file);
for (; t_cur < diff->target.ranges[j].end && t_cur < 
t_end; t_cur++)
print_line(prefix, '+', t_cur, t_ends, 
pair->two->data,
-  c_new, c_reset);
+  c_new, c_reset, opt->file);
j++;
}
for (; t_cur < t_end; t_cur++)
print_line(prefix, ' ', t_cur, t_ends, pair->two->data,
-  c_context, c_reset);
+  c_context, c_reset, opt->file);
}
 
free(p_ends);
@@ -977,7 +977,7 @@ static void dump_diff_hacky_one(struct rev_info *rev, 
struct line_log_data *rang
  */
 static void dump_diff_hacky(struct rev_info *rev, struct line_log_data *range)
 {
-   puts(output_prefix(>diffopt));
+   

[PATCH v4 05/10] shortlog: support outputting to streams other than stdout

[Johannes Schindelin]

This will be needed to avoid freopen() in `git format-patch`.

Signed-off-by: Johannes Schindelin 
---
 builtin/shortlog.c | 13 -
 shortlog.h |  1 +
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index bfc082e..39d74fe 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -229,6 +229,7 @@ void shortlog_init(struct shortlog *log)
log->wrap = DEFAULT_WRAPLEN;
log->in1 = DEFAULT_INDENT1;
log->in2 = DEFAULT_INDENT2;
+   log->file = stdout;
 }
 
 int cmd_shortlog(int argc, const char **argv, const char *prefix)
@@ -310,22 +311,24 @@ void shortlog_output(struct shortlog *log)
for (i = 0; i < log->list.nr; i++) {
const struct string_list_item *item = >list.items[i];
if (log->summary) {
-   printf("%6d\t%s\n", (int)UTIL_TO_INT(item), 
item->string);
+   fprintf(log->file, "%6d\t%s\n",
+   (int)UTIL_TO_INT(item), item->string);
} else {
struct string_list *onelines = item->util;
-   printf("%s (%d):\n", item->string, onelines->nr);
+   fprintf(log->file, "%s (%d):\n",
+   item->string, onelines->nr);
for (j = onelines->nr - 1; j >= 0; j--) {
const char *msg = onelines->items[j].string;
 
if (log->wrap_lines) {
strbuf_reset();
add_wrapped_shortlog_msg(, msg, log);
-   fwrite(sb.buf, sb.len, 1, stdout);
+   fwrite(sb.buf, sb.len, 1, log->file);
}
else
-   printf("  %s\n", msg);
+   fprintf(log->file, "  %s\n", msg);
}
-   putchar('\n');
+   putc('\n', log->file);
onelines->strdup_strings = 1;
string_list_clear(onelines, 0);
free(onelines);
diff --git a/shortlog.h b/shortlog.h
index de4f86f..5a326c6 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -17,6 +17,7 @@ struct shortlog {
char *common_repo_prefix;
int email;
struct string_list mailmap;
+   FILE *file;
 };
 
 void shortlog_init(struct shortlog *log);
-- 
2.9.0.118.g0e1a633


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 02/10] log-tree: respect diffopt's configured output file stream

[Johannes Schindelin]

The diff options already know how to print the output anywhere else
than stdout. The same is needed for log output in general, e.g.
when writing patches to files in `git format-patch`. Let's allow
users to use log_tree_commit() *without* changing global state via
freopen().

Signed-off-by: Johannes Schindelin 
---
 log-tree.c | 64 +++---
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 456d7e3..cf24027 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -159,12 +159,12 @@ void load_ref_decorations(int flags)
}
 }
 
-static void show_parents(struct commit *commit, int abbrev)
+static void show_parents(struct commit *commit, int abbrev, FILE *file)
 {
struct commit_list *p;
for (p = commit->parents; p ; p = p->next) {
struct commit *parent = p->item;
-   printf(" %s", find_unique_abbrev(parent->object.oid.hash, 
abbrev));
+   fprintf(file, " %s", 
find_unique_abbrev(parent->object.oid.hash, abbrev));
}
 }
 
@@ -172,7 +172,7 @@ static void show_children(struct rev_info *opt, struct 
commit *commit, int abbre
 {
struct commit_list *p = lookup_decoration(>children, 
>object);
for ( ; p; p = p->next) {
-   printf(" %s", find_unique_abbrev(p->item->object.oid.hash, 
abbrev));
+   fprintf(opt->diffopt.file, " %s", 
find_unique_abbrev(p->item->object.oid.hash, abbrev));
}
 }
 
@@ -286,11 +286,11 @@ void show_decorations(struct rev_info *opt, struct commit 
*commit)
struct strbuf sb = STRBUF_INIT;
 
if (opt->show_source && commit->util)
-   printf("\t%s", (char *) commit->util);
+   fprintf(opt->diffopt.file, "\t%s", (char *) commit->util);
if (!opt->show_decorations)
return;
format_decorations(, commit, opt->diffopt.use_color);
-   fputs(sb.buf, stdout);
+   fputs(sb.buf, opt->diffopt.file);
strbuf_release();
 }
 
@@ -364,18 +364,18 @@ void log_write_email_headers(struct rev_info *opt, struct 
commit *commit,
subject = "Subject: ";
}
 
-   printf("From %s Mon Sep 17 00:00:00 2001\n", name);
+   fprintf(opt->diffopt.file, "From %s Mon Sep 17 00:00:00 2001\n", name);
graph_show_oneline(opt->graph);
if (opt->message_id) {
-   printf("Message-Id: <%s>\n", opt->message_id);
+   fprintf(opt->diffopt.file, "Message-Id: <%s>\n", 
opt->message_id);
graph_show_oneline(opt->graph);
}
if (opt->ref_message_ids && opt->ref_message_ids->nr > 0) {
int i, n;
n = opt->ref_message_ids->nr;
-   printf("In-Reply-To: <%s>\n", 
opt->ref_message_ids->items[n-1].string);
+   fprintf(opt->diffopt.file, "In-Reply-To: <%s>\n", 
opt->ref_message_ids->items[n-1].string);
for (i = 0; i < n; i++)
-   printf("%s<%s>\n", (i > 0 ? "\t" : "References: "),
+   fprintf(opt->diffopt.file, "%s<%s>\n", (i > 0 ? "\t" : 
"References: "),
   opt->ref_message_ids->items[i].string);
graph_show_oneline(opt->graph);
}
@@ -432,7 +432,7 @@ static void show_sig_lines(struct rev_info *opt, int 
status, const char *bol)
reset = diff_get_color_opt(>diffopt, DIFF_RESET);
while (*bol) {
eol = strchrnul(bol, '\n');
-   printf("%s%.*s%s%s", color, (int)(eol - bol), bol, reset,
+   fprintf(opt->diffopt.file, "%s%.*s%s%s", color, (int)(eol - 
bol), bol, reset,
   *eol ? "\n" : "");
graph_show_oneline(opt->graph);
bol = (*eol) ? (eol + 1) : eol;
@@ -553,17 +553,17 @@ void show_log(struct rev_info *opt)
 
if (!opt->graph)
put_revision_mark(opt, commit);
-   fputs(find_unique_abbrev(commit->object.oid.hash, 
abbrev_commit), stdout);
+   fputs(find_unique_abbrev(commit->object.oid.hash, 
abbrev_commit), opt->diffopt.file);
if (opt->print_parents)
-   show_parents(commit, abbrev_commit);
+   show_parents(commit, abbrev_commit, opt->diffopt.file);
if (opt->children.name)
show_children(opt, commit, abbrev_commit);
show_decorations(opt, commit);
if (opt->graph && !graph_is_commit_finished(opt->graph)) {
-   putchar('\n');
+   putc('\n', opt->diffopt.file);
graph_show_remainder(opt->graph);
}
-   putchar(opt->diffopt.line_termination);
+   putc(opt->diffopt.line_termination, opt->diffopt.file);
return;
}
 
@@ -589,7 +589,7 @@ void show_log(struct rev_info *opt)
   

[PATCH v4 04/10] graph: respect the diffopt.file setting

[Johannes Schindelin]

When the caller overrides diffopt.file (which defaults to stdout),
the diff machinery already redirects its output, and the graph display
should also write to that file.

Signed-off-by: Johannes Schindelin 
---
 graph.c | 30 +-
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/graph.c b/graph.c
index 1350bdd..8ad8ba3 100644
--- a/graph.c
+++ b/graph.c
@@ -17,8 +17,8 @@
 static void graph_padding_line(struct git_graph *graph, struct strbuf *sb);
 
 /*
- * Print a strbuf to stdout.  If the graph is non-NULL, all lines but the
- * first will be prefixed with the graph output.
+ * Print a strbuf.  If the graph is non-NULL, all lines but the first will be
+ * prefixed with the graph output.
  *
  * If the strbuf ends with a newline, the output will end after this
  * newline.  A new graph line will not be printed after the final newline.
@@ -1193,9 +1193,10 @@ void graph_show_commit(struct git_graph *graph)
 
while (!shown_commit_line && !graph_is_commit_finished(graph)) {
shown_commit_line = graph_next_line(graph, );
-   fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);
+   fwrite(msgbuf.buf, sizeof(char), msgbuf.len,
+   graph->revs->diffopt.file);
if (!shown_commit_line)
-   putchar('\n');
+   putc('\n', graph->revs->diffopt.file);
strbuf_setlen(, 0);
}
 
@@ -1210,7 +1211,7 @@ void graph_show_oneline(struct git_graph *graph)
return;
 
graph_next_line(graph, );
-   fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);
+   fwrite(msgbuf.buf, sizeof(char), msgbuf.len, graph->revs->diffopt.file);
strbuf_release();
 }
 
@@ -1222,7 +1223,7 @@ void graph_show_padding(struct git_graph *graph)
return;
 
graph_padding_line(graph, );
-   fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);
+   fwrite(msgbuf.buf, sizeof(char), msgbuf.len, graph->revs->diffopt.file);
strbuf_release();
 }
 
@@ -1239,12 +1240,13 @@ int graph_show_remainder(struct git_graph *graph)
 
for (;;) {
graph_next_line(graph, );
-   fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);
+   fwrite(msgbuf.buf, sizeof(char), msgbuf.len,
+   graph->revs->diffopt.file);
strbuf_setlen(, 0);
shown = 1;
 
if (!graph_is_commit_finished(graph))
-   putchar('\n');
+   putc('\n', graph->revs->diffopt.file);
else
break;
}
@@ -1259,7 +1261,8 @@ static void graph_show_strbuf(struct git_graph *graph, 
struct strbuf const *sb)
char *p;
 
if (!graph) {
-   fwrite(sb->buf, sizeof(char), sb->len, stdout);
+   fwrite(sb->buf, sizeof(char), sb->len,
+   graph->revs->diffopt.file);
return;
}
 
@@ -1277,7 +1280,7 @@ static void graph_show_strbuf(struct git_graph *graph, 
struct strbuf const *sb)
} else {
len = (sb->buf + sb->len) - p;
}
-   fwrite(p, sizeof(char), len, stdout);
+   fwrite(p, sizeof(char), len, graph->revs->diffopt.file);
if (next_p && *next_p != '\0')
graph_show_oneline(graph);
p = next_p;
@@ -1297,7 +1300,8 @@ void graph_show_commit_msg(struct git_graph *graph,
 * CMIT_FMT_USERFORMAT are already missing a terminating
 * newline.  All of the other formats should have it.
 */
-   fwrite(sb->buf, sizeof(char), sb->len, stdout);
+   fwrite(sb->buf, sizeof(char), sb->len,
+   graph->revs->diffopt.file);
return;
}
 
@@ -1318,7 +1322,7 @@ void graph_show_commit_msg(struct git_graph *graph,
 * new line.
 */
if (!newline_terminated)
-   putchar('\n');
+   putc('\n', graph->revs->diffopt.file);
 
graph_show_remainder(graph);
 
@@ -1326,6 +1330,6 @@ void graph_show_commit_msg(struct git_graph *graph,
 * If sb ends with a newline, our output should too.
 */
if (newline_terminated)
-   putchar('\n');
+   putc('\n', graph->revs->diffopt.file);
}
 }
-- 
2.9.0.118.g0e1a633


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 00/10] Let log-tree and friends respect diffopt's `file` field

[Johannes Schindelin]

The idea is to allow callers to redirect log-tree's output to a file
without having to freopen() stdout (which would modify global state,
a big no-no-no for library functions).

I reviewed log-tree.c, graph.c, line-log.c, builtin/shortlog.c and
builtin/log.c line by line to ensure that all calls that assumed stdout
previously now use the `file` field instead, of course. I would
welcome additional eyes to go over the code to confirm that I did not
miss anything.

This patch series ends up removing the freopen() call in the
format-patch command, but that is only a by-product. The ulterior motive
behind this series is to allow the sequencer to write a `patch` file as
part of my endeavor to move large chunks of rebase -i into a builtin.

In contrast to the previous iteration of this patch series,

- the use_color = 0 setting was made contingent on use_color != ALWAYS

- close_file = 1 was made to work in more circumstances, most notably
  when calling log_commit_tree() (and in builtin/log.c, where this
  function is called in a loop)

- the changes to builtin/am.c were backed out completely (this is a can
  of worms I am not prepared to open for now)

- I also taught shortlog to respect the --output= option, because
  it was so easy to do

- I added a test case to ensure that `log --output=` works


Johannes Schindelin (10):
  Prepare log/log-tree to reuse the diffopt.close_file attribute
  log-tree: respect diffopt's configured output file stream
  line-log: respect diffopt's configured output file stream
  graph: respect the diffopt.file setting
  shortlog: support outputting to streams other than stdout
  format-patch: explicitly switch off color when writing to files
  format-patch: avoid freopen()
  format-patch: use stdout directly
  shortlog: respect the --output= setting
  Ensure that log respects --output=

 builtin/log.c   | 87 +
 builtin/shortlog.c  | 15 ++---
 graph.c | 30 ++
 line-log.c  | 34 ++---
 log-tree.c  | 69 ++
 shortlog.h  |  1 +
 t/t4201-shortlog.sh |  6 
 t/t4211-line-log.sh |  7 +
 8 files changed, 142 insertions(+), 107 deletions(-)

Published-As: https://github.com/dscho/git/releases/tag/diffopt.file-v4
Interdiff vs v3:

 diff --git a/builtin/am.c b/builtin/am.c
 index 47d78aa..3dfe70b 100644
 --- a/builtin/am.c
 +++ b/builtin/am.c
 @@ -1433,16 +1433,12 @@ static void get_commit_info(struct am_state *state, 
struct commit *commit)
  /**
   * Writes `commit` as a patch to the state directory's "patch" file.
   */
 -static int write_commit_patch(const struct am_state *state, struct commit 
*commit)
 +static void write_commit_patch(const struct am_state *state, struct commit 
*commit)
  {
struct rev_info rev_info;
FILE *fp;
 -  int res;
  
 -  fp = fopen(am_path(state, "patch"), "w");
 -  if (!fp)
 -  return error(_("Could not open %s for writing"),
 -  am_path(state, "patch"));
 +  fp = xfopen(am_path(state, "patch"), "w");
init_revisions(_info, NULL);
rev_info.diff = 1;
rev_info.abbrev = 0;
 @@ -1454,11 +1450,10 @@ static int write_commit_patch(const struct am_state 
*state, struct commit *commi
DIFF_OPT_SET(_info.diffopt, FULL_INDEX);
rev_info.diffopt.use_color = 0;
rev_info.diffopt.file = fp;
 +  rev_info.diffopt.close_file = 1;
add_pending_object(_info, >object, "");
diff_setup_done(_info.diffopt);
 -  res = log_tree_commit(_info, commit);
 -  fclose(fp);
 -  return res;
 +  log_tree_commit(_info, commit);
  }
  
  /**
 @@ -1506,14 +1501,13 @@ static int parse_mail_rebase(struct am_state *state, 
const char *mail)
unsigned char commit_sha1[GIT_SHA1_RAWSZ];
  
if (get_mail_commit_sha1(commit_sha1, mail) < 0)
 -  return error(_("could not parse %s"), mail);
 +  die(_("could not parse %s"), mail);
  
commit = lookup_commit_or_die(commit_sha1, mail);
  
get_commit_info(state, commit);
  
 -  if (write_commit_patch(state, commit) < 0)
 -  return -1;
 +  write_commit_patch(state, commit);
  
hashcpy(state->orig_commit, commit_sha1);
write_state_text(state, "original-commit", sha1_to_hex(commit_sha1));
 diff --git a/builtin/log.c b/builtin/log.c
 index 2bfcc43..2a42216 100644
 --- a/builtin/log.c
 +++ b/builtin/log.c
 @@ -243,9 +243,10 @@ static struct itimerval early_output_timer;
  
  static void log_show_early(struct rev_info *revs, struct commit_list *list)
  {
 -  int i = revs->early_output;
 +  int i = revs->early_output, close_file = revs->diffopt.close_file;
int show_header = 1;
  
 +  revs->diffopt.close_file = 0;
sort_in_topological_order(, revs->sort_order);
while (list && i) {
struct commit *commit = list->item;
 @@ 

[PATCH v4 10/10] Ensure that log respects --output=

[Johannes Schindelin]

The test script t4202-log.sh is already pretty long, and it is a good
idea to test --output with a more obscure option, anyway. So let's
test it in conjunction with line-log.

The most important part of this test, of course, is to ensure that the
file is not closed after writing the diff, but only at the very end
of the log output. That is the entire reason why the test tries to
generate a log that covers more than one commit.

Signed-off-by: Johannes Schindelin 
---
 t/t4211-line-log.sh | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index 4451127..9d8 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -99,4 +99,11 @@ test_expect_success '-L with --first-parent and a merge' '
git log --first-parent -L 1,1:b.c
 '
 
+test_expect_success '-L with --output' '
+   git checkout parallel-change &&
+   git log --output=log -L :main:b.c >output &&
+   test ! -s output &&
+   test_line_count = 70 log
+'
+
 test_done
-- 
2.9.0.118.g0e1a633
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 06/10] format-patch: explicitly switch off color when writing to files

[Johannes Schindelin]

We rely on the auto-detection ("is stdout a terminal?") to determine
whether to use color in the output of format-patch or not. That happens
to work because we freopen() stdout when redirecting the output to files.

However, we are about to fix that work-around, in which case the
auto-detection has no chance to guess whether to use color or not.

But then, we do not need to guess to begin with. As argued in the commit
message of 7787570c (format-patch: ignore ui.color, 2011-09-13), we do not
allow the ui.color setting to affect format-patch's output. The only time,
therefore, that we allow color sequences to be written to the output files
is when the user specified the --color command-line option explicitly.

Signed-off-by: Johannes Schindelin 
---
 builtin/log.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/log.c b/builtin/log.c
index 27bc88d..5683a42 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1578,6 +1578,8 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
setup_pager();
 
if (output_directory) {
+   if (rev.diffopt.use_color != GIT_COLOR_ALWAYS)
+   rev.diffopt.use_color = 0;
if (use_stdout)
die(_("standard output, or directory, which one?"));
if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
-- 
2.9.0.118.g0e1a633


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] Refactor recv_sideband()

[Nicolas Pitre]

On Wed, 22 Jun 2016, Lukas Fleischer wrote:

> Before this patch, we used character buffer manipulations to split
> messages from the sideband at line breaks and insert "remote: " at the
> beginning of each line, using the packet size to determine the end of a
> message. However, since it is safe to assume that diagnostic messages
> from the sideband never contain NUL characters, we can also
> NUL-terminate the buffer, use strpbrk() for splitting lines and use
> format strings to insert the prefix.
> 
> A static strbuf is used for constructing the output which is then
> printed using a single write() call, such that the atomicity of the
> output is preserved. See 9ac13ec (atomic write for sideband remote
> messages, 2006-10-11) for details.
> 
> Helped-by: Nicolas Pitre 
> Signed-off-by: Lukas Fleischer 

The patch is buggy.

Once patched, the code looks like this:

case 2:
b = buf + 1;
/*
 * Append a suffix to each nonempty line to clear the
 * end of the screen line.
 */
while ((brk = strpbrk(b, "\n\r"))) {
int linelen = brk - b;
if (linelen > 0) {
strbuf_addf(, "%.*s%s%c",
linelen, b, suffix, *brk);
} else {
strbuf_addf(, "%c", *brk);
}
xwrite(STDERR_FILENO, outbuf.buf, outbuf.len);
strbuf_reset();
strbuf_addf(, "%s", PREFIX);
b = brk + 1;
}
if (*b) {
xwrite(STDERR_FILENO, outbuf.buf, outbuf.len);
/* Incomplete line, skip the next prefix. */
strbuf_reset();
}
continue;

You are probably missing a strbuf_addf() before the last xwrite().


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 01/10] Prepare log/log-tree to reuse the diffopt.close_file attribute

[Johannes Schindelin]

We are about to teach the log-tree machinery to reuse the diffopt.file
field to output to a file stream other than stdout, in line with the
diff machinery already writing to diffopt.file.

However, we might want to write something after the diff in
log_tree_commit() (e.g. with the --show-linear-break option), therefore
we must not let the diff machinery close the file (as per
diffopt.close_file.

This means that log_tree_commit() itself must override the
diffopt.close_file flag and close the file, and if log_tree_commit() is
called in a loop, the caller is responsible to do the same.

Note: format-patch has an `--output-directory` option. Due to the fact
that format-patch's options are parsed first, and that the parse-options
machinery accepts uniquely abbreviated options, the diff options
`--output` (and `-o`) are shadowed. Therefore close_file is not set to 1
so that cmd_format_patch() does *not* need to handle the close_file flag
differently, even if it calls log_tree_commit() in a loop.

Signed-off-by: Johannes Schindelin 
---
 builtin/log.c | 15 ---
 log-tree.c|  5 -
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 099f4f7..27bc88d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -243,9 +243,10 @@ static struct itimerval early_output_timer;
 
 static void log_show_early(struct rev_info *revs, struct commit_list *list)
 {
-   int i = revs->early_output;
+   int i = revs->early_output, close_file = revs->diffopt.close_file;
int show_header = 1;
 
+   revs->diffopt.close_file = 0;
sort_in_topological_order(, revs->sort_order);
while (list && i) {
struct commit *commit = list->item;
@@ -262,14 +263,19 @@ static void log_show_early(struct rev_info *revs, struct 
commit_list *list)
case commit_ignore:
break;
case commit_error:
+   if (close_file)
+   fclose(revs->diffopt.file);
return;
}
list = list->next;
}
 
/* Did we already get enough commits for the early output? */
-   if (!i)
+   if (!i) {
+   if (close_file)
+   fclose(revs->diffopt.file);
return;
+   }
 
/*
 * ..if no, then repeat it twice a second until we
@@ -331,7 +337,7 @@ static int cmd_log_walk(struct rev_info *rev)
 {
struct commit *commit;
int saved_nrl = 0;
-   int saved_dcctc = 0;
+   int saved_dcctc = 0, close_file = rev->diffopt.close_file;
 
if (rev->early_output)
setup_early_output(rev);
@@ -347,6 +353,7 @@ static int cmd_log_walk(struct rev_info *rev)
 * and HAS_CHANGES being accumulated in rev->diffopt, so be careful to
 * retain that state information if replacing rev->diffopt in this loop
 */
+   rev->diffopt.close_file = 0;
while ((commit = get_revision(rev)) != NULL) {
if (!log_tree_commit(rev, commit) && rev->max_count >= 0)
/*
@@ -367,6 +374,8 @@ static int cmd_log_walk(struct rev_info *rev)
}
rev->diffopt.degraded_cc_to_c = saved_dcctc;
rev->diffopt.needed_rename_limit = saved_nrl;
+   if (close_file)
+   fclose(rev->diffopt.file);
 
if (rev->diffopt.output_format & DIFF_FORMAT_CHECKDIFF &&
DIFF_OPT_TST(>diffopt, CHECK_FAILED)) {
diff --git a/log-tree.c b/log-tree.c
index 78a5381..456d7e3 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -862,11 +862,12 @@ static int log_tree_diff(struct rev_info *opt, struct 
commit *commit, struct log
 int log_tree_commit(struct rev_info *opt, struct commit *commit)
 {
struct log_info log;
-   int shown;
+   int shown, close_file = opt->diffopt.close_file;
 
log.commit = commit;
log.parent = NULL;
opt->loginfo = 
+   opt->diffopt.close_file = 0;
 
if (opt->line_level_traverse)
return line_log_print(opt, commit);
@@ -883,5 +884,7 @@ int log_tree_commit(struct rev_info *opt, struct commit 
*commit)
printf("\n%s\n", opt->break_bar);
opt->loginfo = NULL;
maybe_flush_or_die(stdout, "stdout");
+   if (close_file)
+   fclose(opt->diffopt.file);
return shown;
 }
-- 
2.9.0.118.g0e1a633


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] diff: let --output= default to --no-color

[Johannes Schindelin]

It is highly unlikely that any user would want to see ANSI color
sequences in a file. So let's stop doing that by default.

This is a backwards-incompatible change.

The reason this was not caught earlier is most likely that either
--output= is not used, or only when stdout is redirected
anyway.

Technically, we do not default to --no-color here. Instead, we try to
override the GIT_COLOR_AUTO default because it would let want_color()
test whether stdout (instead of the specified file) is connected to a
terminal. Practically, we require the user to require color "always"
to force writing ANSI color sequences to the output file.

Signed-off-by: Johannes Schindelin 
---
Published-As: https://github.com/dscho/git/releases/tag/diff-o-v1

Just something I noted while working on a bit more consistency
with the diffopt.file patches.

This is a backwards-incompatible change, though. So I extracted it
from the patch series.

 diff.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/diff.c b/diff.c
index fa78fc1..b66b9be 100644
--- a/diff.c
+++ b/diff.c
@@ -3977,6 +3977,8 @@ int diff_opt_parse(struct diff_options *options,
if (!options->file)
die_errno("Could not open '%s'", path);
options->close_file = 1;
+   if (options->use_color != GIT_COLOR_ALWAYS)
+   options->use_color = GIT_COLOR_NEVER;
return argcount;
} else
return 0;
-- 
2.9.0.118.g0e1a633

base-commit: ab7797dbe95fff38d9265869ea367020046db118
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/9] am: stop ignoring errors reported by log_tree_diff()

[Johannes Schindelin]

Hi Junio,

On Tue, 21 Jun 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > Note: there are more places in the builtin am code that ignore
> > errors returned from library functions. Fixing those is outside the
> > purview of the current patch series, though.
> 
> The caller of parse_mail() and parse_mail_rebase() is not prepared
> to see an error code in the returned value from these function,
> which are to return a boolean telling the caller to skip or use the
> patch file.  At least the caller needs to notice negative return and
> made to die/exit(128), instead of silently skipping a corrupt or
> unopenable patch, no?  Otherwise this will change the behaviour.

Yeah, that is another rabbit hole I really do not want to dive in.

Will leave it alone,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/11] i18n: difftool: mark warnings for translation

[Vasco Almeida]

A Terça, 21 de Junho de 2016 21:33:29 David Aguilar escreveu:
> I'm assuming that the i18n infrastructure is prepared to deal
> with perl's . dot syntax for string concatenation, though, and
> I don't know whether that's true.  Does that work?

Yes, dot syntax does work. xgettext is able to extract those string to pot 
file.
There is an example using it in gettext manual [1].
> 
> What do you think?

I think that is a good suggestion. It is easier on the eyes. Thank you.
I'll include this in the next re-roll.

[1] 
https://www.gnu.org/software/gettext/manual/html_node/Long-Lines.html#Long-Lines
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html