* Re: [PATCH 3/3] submodule--helper: add intern-git-dir function
From: Junio C Hamano @ 2016-11-22 17:53 UTC (permalink / raw)
To: Stefan Beller
Cc: Brandon Williams, Jonathan Nieder, git@vger.kernel.org,
Jens Lehmann, Heiko Voigt
In-Reply-To: <CAGZ79kb7062abd6Cbf-ey3u0L6PXUcRf7m-og5EyCRZencOR6g@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Mon, Nov 21, 2016 at 11:07 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> So I guess we should test a bit more extensively, maybe
>>>
>>> git status >expect
>>> git submodule embedgitdirs
>>> git status >actual
>>> test_cmp expect actual
>>> # further testing via
>>> test -f ..
>>> test -d ..
>>
>> Something along that line. "status should succeed" does not tell
>> the readers what kind of breakage the test is expecting to protect
>> us from. If we are expecting a breakage in embed-git-dirs would
>> somehow corrupt an existing submodule, which would lead to "status"
>> that is run in the superproject report the submodule differently,
>> then comparing output before and after the operation may be a
>> reasonable test. Going there to the submodule working tree and
>> checking the health of the repository (of the submodule) may be
>> another sensible test.
>
> and by checking the health you mean just a status in there, or rather a
> more nuanced thing like `git rev-parse HEAD` ? I'll go with that for now.
Would fsck have caught the breakages you caused while developing it,
for example?
As the core of the "embed" operation is an non-atomic "move the .git
directory to .git/modules/$name in the superproject and then make a
gitfile pointing at it", verifying that there is no change in the
contents of .git/modules before and after the failed operation, and
verifying that the submodule working tree has the embedded .git/
directory would be good enough, I would think.
^ permalink raw reply
* [PATCH] git-gui: pass the branch name to git merge
From: Johannes Sixt @ 2016-11-22 17:52 UTC (permalink / raw)
To: Pat Thoyts; +Cc: René Scharfe, Junio C Hamano, Git Mailing List
The recent rewrite of the 'git merge' invocation in b5f325cb
(git-gui: stop using deprecated merge syntax) replaced the
subsidiary call of 'git fmt-merge-msg' to take advantage of
the capability of 'git merge' that can construct a merge log
message when the rev being merged is FETCH_HEAD.
A disadvantage of this method is, though, that the conflict
markers are augmented with a raw SHA1 instead of a symbolic
branch name. Revert to the former invocation style so that
we get both a useful commit message and a symbolic name in the
conflict markers.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
A minor regression of our effort to get rid of
deprecated merge syntax. Even though I had done a number of
merges since then, I noticed this only today because I
actively looked for the branch name.
lib/merge.tcl | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/lib/merge.tcl b/lib/merge.tcl
index 9f253db..39e3fb4 100644
--- a/lib/merge.tcl
+++ b/lib/merge.tcl
@@ -112,16 +112,9 @@ method _start {} {
close $fh
set _last_merged_branch $branch
- if {[git-version >= "2.5.0"]} {
- set cmd [list git merge --strategy=recursive FETCH_HEAD]
- } else {
- set cmd [list git]
- lappend cmd merge
- lappend cmd --strategy=recursive
- lappend cmd [git fmt-merge-msg <[gitdir FETCH_HEAD]]
- lappend cmd HEAD
- lappend cmd $name
- }
+ set cmd [list git merge --strategy=recursive --no-log -m]
+ lappend cmd [git fmt-merge-msg <[gitdir FETCH_HEAD]]
+ lappend cmd $name
ui_status [mc "Merging %s and %s..." $current_branch $stitle]
set cons [console::new [mc "Merge"] "merge $stitle"]
--
2.11.0.rc1.52.g65ffb51
^ permalink raw reply related
* Re: v2.11 new diff heuristic?
From: Stefan Beller @ 2016-11-22 17:51 UTC (permalink / raw)
To: Robert Dailey; +Cc: Git
In-Reply-To: <CAHd499AjXh1YnVgBj_8j0fgvOgOn53y+sPBBy6y7mSM-+dCyVw@mail.gmail.com>
On Tue, Nov 22, 2016 at 6:42 AM, Robert Dailey <rcdailey.lists@gmail.com> wrote:
> The release notes mention a new heuristic for diff:
>
> * Output from "git diff" can be made easier to read by selecting
> which lines are common and which lines are added/deleted
> intelligently when the lines before and after the changed section
> are the same. A command line option is added to help with the
> experiment to find a good heuristics.
>
> However, it lacks information on exactly how to use this new feature.
> I dug into the git diff documentation here:
>
> https://git-scm.com/docs/git-diff
>
> It mentions a "--compaction-heuristic" option. Is this the new
> heuristic outlined by the release notes?
yes.
> Is the
> compaction heuristic compatible with the histogram diff algorithm?
yes as the compaction heuristic is applied after the actual diff is performed.
> Is
> there a config option to turn this on all the time? For that matter,
> is this something I can keep on all the time or is it only useful in
> certain situations?
I think you can set diff.compactionHeuristic and it will use it by default.
>
> There's still so much more about this feature I would like to know.
The background story (and what this new compaction heuristic is doing)
is found at https://github.com/mhagger/diff-slider-tools
^ permalink raw reply
* Re: [PATCH] merge-recursive.c: use QSORT macro
From: Jeff King @ 2016-11-22 17:49 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, René Scharfe
In-Reply-To: <20161122123019.7169-1-pclouds@gmail.com>
On Tue, Nov 22, 2016 at 07:30:19PM +0700, Nguyễn Thái Ngọc Duy wrote:
> This is the follow up of rs/qsort series, merged in b8688ad (Merge
> branch 'rs/qsort' - 2016-10-10), where coccinelle was used to do
> automatic transformation.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> coccinelle missed this place, understandably, because it can't know
> that
>
> sizeof(*entries->items)
>
> is the same as
>
> sizeof(*df_name_compare.items)
>
> without some semantic analysis.
That made me wonder why "entries" is used at all. Does it point to the
same struct? But no, df_name_compare is a string list we create with the
same list of strings.
Which is why...
> - qsort(df_sorted_entries.items, entries->nr, sizeof(*entries->items),
> + QSORT(df_sorted_entries.items, entries->nr,
> string_list_df_name_compare);
...it's OK to use entries->nr here, and not df_sorted_entries.nr. It
still seems a bit odd, though. Maybe it's worth making this:
QSORT(df_sorted_entries.items, df_sorted_entries.nr,
string_list_df_name_compare);
while we're at it. Another possibility is:
df_sorted_entries.cmp = string_list_df_name_compare;
string_list_sort(&df_sorted_entries);
It's not any shorter, but maybe it's conceptually simpler.
-Peff
^ permalink raw reply
* Re: [PATCH v2 2/2] push: fix --dry-run to not push submodules
From: Junio C Hamano @ 2016-11-22 17:43 UTC (permalink / raw)
To: Brandon Williams
Cc: Stefan Beller, git@vger.kernel.org, Heiko Voigt, Johannes Sixt
In-Reply-To: <20161117190255.GK66382@google.com>
Brandon Williams <bmwill@google.com> writes:
> On 11/17, Stefan Beller wrote:
>> On Thu, Nov 17, 2016 at 10:46 AM, Brandon Williams <bmwill@google.com> wrote:
>>
>> > sha1_array_clear(&commits);
>> > - die("Failed to push all needed submodules!");
>> > + die ("Failed to push all needed submodules!");
>>
>> huh? Is this a whitespace change?
>
> That's odd...I didn't mean to add that lone space.
Is that the only glitch in this round? IOW, is the series OK to be
picked up as long as I treak this out while queuing?
^ permalink raw reply
* Re: [PATCH v6 01/16] Git.pm: add subroutines for commenting lines
From: Junio C Hamano @ 2016-11-22 17:42 UTC (permalink / raw)
To: Vasco Almeida
Cc: git, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Jakub Narębski, David Aguilar
In-Reply-To: <1479823833.1956.7.camel@sapo.pt>
Vasco Almeida <vascomalmeida@sapo.pt> writes:
> A Sex, 11-11-2016 às 11:45 -0100, Vasco Almeida escreveu:
>> +=item comment_lines ( STRING [, STRING... ])
>> +
>> +Comments lines following core.commentchar configuration.
>> +
>> +=cut
>> +
>> +sub comment_lines {
>> + my $comment_line_char = config("core.commentchar") || '#';
>> + return prefix_lines("$comment_line_char ", @_);
>> +}
>> +
>
> In light of the recent "Fix problems with rebase -i when
> core.commentchar is defined" [1], I realized that this patch does not
> handle the 'auto' value of core.commentchat configuration variable.
>
> I propose to do the patch below in the next re-roll.
>
> [1] http://www.mail-archive.com/git@vger.kernel.org/msg107818.html
The incremental update below looks sensible. We'd also want to
protect this codepath from a misconfigured two-or-more byte sequence
in core.commentchar, I would suspect, to be consistent.
> -- >8 --
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 3a6d846..8d33634 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -1073,6 +1073,7 @@ sub edit_hunk_manually {
> my $is_reverse = $patch_mode_flavour{IS_REVERSE};
> my ($remove_plus, $remove_minus) = $is_reverse ? ('-', '+') : ('+', '-');
> my $comment_line_char = Git::config("core.commentchar") || '#';
> + $comment_line_char = '#' if ($comment_line_char eq 'auto');
> print $fh Git::comment_lines sprintf(__ <<EOF, $remove_minus, $remove_plus, $comment_line_char),
> ---
> To remove '%s' lines, make them ' ' lines (context).
> diff --git a/perl/Git.pm b/perl/Git.pm
> index 69cd1dd..47b5899 100644
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -1459,6 +1459,7 @@ Comments lines following core.commentchar configuration.
>
> sub comment_lines {
> my $comment_line_char = config("core.commentchar") || '#';
> + $comment_line_char = '#' if ($comment_line_char eq 'auto');
> return prefix_lines("$comment_line_char ", @_);
> }
^ permalink raw reply
* Re: [PATCH 31/35] pathspec: allow querying for attributes
From: Stefan Beller @ 2016-11-22 17:26 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Junio C Hamano, Brandon Williams, Git Mailing List
In-Reply-To: <CACsJy8BXjYOza_1mPCJJw+Mk1zksLLJMBNKvbAk8+1-bdAGJMw@mail.gmail.com>
On Tue, Nov 22, 2016 at 2:41 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Fri, Nov 11, 2016 at 3:34 AM, Stefan Beller <sbeller@google.com> wrote:
>> @@ -139,7 +140,8 @@ static size_t common_prefix_len(const struct pathspec *pathspec)
>> PATHSPEC_LITERAL |
>> PATHSPEC_GLOB |
>> PATHSPEC_ICASE |
>> - PATHSPEC_EXCLUDE);
>> + PATHSPEC_EXCLUDE |
>> + PATHSPEC_ATTR);
>
> Hmm.. common_prefix_len() has always been a bit relaxing and can cover
> more than needed. It's for early pruning. Exact pathspec matching
> _will_ be done later anyway.
>
> Is that obvious?
Yes it is.
Not sure what your concern is, though.
Given the pathspec ":(attr:plumbing)Documentation/", the common_prefix_len
is still able to figure out that any match has a prefix of
strlen("Documentation/"),
no matter what attr stuff is involved, because the attr stuff is also
just reducing the
matching set.
Now if we have such a pathspec it is easier to claim common_prefix_len supports
attr as it is a correct thing to ignore the attrs completely, than to
rewrite the call to
common_prefix_len to be without attrs involved.
You *may* be able to improve it with knowledge of the attrs:
":(attr:internal-technical-api)Documentation/" may restrict the results to match
only "Documentation/technical/", but as you said, we don't have to be
exact here.
> I'm wondering if we need to add a line or two in the
> big comment code before this statement. I'm thinking it is and we
> probably don't need more comments...
Do I misunderstand the code completely here?
Thanks,
Stefan
> --
> Duy
^ permalink raw reply
* Re: v2.11 new diff heuristic?
From: Jacob Keller @ 2016-11-22 17:18 UTC (permalink / raw)
To: Robert Dailey, Git
In-Reply-To: <CAHd499AjXh1YnVgBj_8j0fgvOgOn53y+sPBBy6y7mSM-+dCyVw@mail.gmail.com>
On November 22, 2016 6:42:36 AM PST, Robert Dailey <rcdailey.lists@gmail.com> wrote:
>The release notes mention a new heuristic for diff:
>
>* Output from "git diff" can be made easier to read by selecting
>which lines are common and which lines are added/deleted
>intelligently when the lines before and after the changed section
>are the same. A command line option is added to help with the
>experiment to find a good heuristics.
>
>However, it lacks information on exactly how to use this new feature.
>I dug into the git diff documentation here:
>
>https://git-scm.com/docs/git-diff
>
>It mentions a "--compaction-heuristic" option. Is this the new
>heuristic outlined by the release notes? If not, which is it? Is the
>compaction heuristic compatible with the histogram diff algorithm? Is
>there a config option to turn this on all the time? For that matter,
>is this something I can keep on all the time or is it only useful in
>certain situations?
>
>There's still so much more about this feature I would like to know.
Hi,
Yes for now the compaction heuristic option has an undocumented config. (I forget the exact name off the top of my head). Currently it is being evaluated and likely we want to make it default in the near future once we are certain that it helps and doesn't make any difference worse.
So long term you will not need any special knobs to benefit.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH 3/3] submodule--helper: add intern-git-dir function
From: Stefan Beller @ 2016-11-22 17:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Brandon Williams, Jonathan Nieder, git@vger.kernel.org,
Jens Lehmann, Heiko Voigt
In-Reply-To: <xmqqmvgsf0wo.fsf@gitster.mtv.corp.google.com>
On Mon, Nov 21, 2016 at 11:07 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> So I guess we should test a bit more extensively, maybe
>>
>> git status >expect
>> git submodule embedgitdirs
>> git status >actual
>> test_cmp expect actual
>> # further testing via
>> test -f ..
>> test -d ..
>
> Something along that line. "status should succeed" does not tell
> the readers what kind of breakage the test is expecting to protect
> us from. If we are expecting a breakage in embed-git-dirs would
> somehow corrupt an existing submodule, which would lead to "status"
> that is run in the superproject report the submodule differently,
> then comparing output before and after the operation may be a
> reasonable test. Going there to the submodule working tree and
> checking the health of the repository (of the submodule) may be
> another sensible test.
and by checking the health you mean just a status in there, or rather a
more nuanced thing like `git rev-parse HEAD` ? I'll go with that for now.
>
>>> In the
>>> extreme, if the failed "git submodule" command did
>>>
>>> rm -fr .git ?* && git init
>>>
>>> wouldn't "git status" still succeed?
>>
>> In that particular case you'd get
>> $ git status
>> fatal: Not a git repository (or any parent up to mount point ....)
>
> Even with "&& git init"? Or you forgot that part?
yes I did, because why would you run init after an accidental rm -rf ... ?
^ permalink raw reply
* Re: [PATCH 1/3] rebase -i: highlight problems with core.commentchar
From: Junio C Hamano @ 2016-11-22 17:05 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1611221709180.3746@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Mon, 21 Nov 2016, Junio C Hamano wrote:
>
>> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
>> index 29e91d861c..c1f6411eb2 100755
>> --- a/t/t0030-stripspace.sh
>> +++ b/t/t0030-stripspace.sh
>> @@ -432,6 +432,15 @@ test_expect_success '-c with changed comment char' '
>> test_cmp expect actual
>> '
>>
>> +test_expect_failure '-c with comment char defined in .git/config' '
>> + test_config core.commentchar = &&
>> + printf "= foo\n" >expect &&
>> + printf "foo" | (
>
> Could I ask you to sneak in a \n here?
The one before that _is_ about fixing incomplete line, but this and
the one before this one are not, so I think we should fix them at
the same time.
But does it break anything to leave it as-is? If not, I'd prefer to
leave this (and one before this one) for a later clean-up patch post
release.
Thanks
^ permalink raw reply
* Re: [PATCH 2/3] stripspace: respect repository config
From: Junio C Hamano @ 2016-11-22 17:10 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Duy Nguyen, Git Mailing List, Ralf Thielow, Taufiq Hoven
In-Reply-To: <alpine.DEB.2.20.1611221712480.3746@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> This conditional config file reading is a trap for similar bugs to
>> happen again. Is there any reason we should not just mark the command
>> RUN_SETUP_GENTLY in git.c and call git_config() here unconditionally?
>
> As I plan to slip these patches into Git for Windows v2.11.0, i.e. making
> this a last-minute hot fix, I want to err on the side of caution.
So do I. As a hot-fix, I'd prefer the patch I queued yesterday.
I think we want to audit the ones without RUN_SETUP* in the command
table in order to hunt for regression aka "a fix revealed a bug that
was covered by .git/config accidentally getting read when run from
the top-level of the working tree", though. We may find unreported
breakages that we may have to fix.
^ permalink raw reply
* [PATCH 2/2] difftool: add a feature flag for the builtin vs scripted version
From: Johannes Schindelin @ 2016-11-22 17:01 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1479834051.git.johannes.schindelin@gmx.de>
The popular difftool command was just converted into a builtin, for
better performance on Windows as well as to reduce the number of Perl
scripts (so that we may, in the very long run, be able to ship Git for
Windows without any Perl interpreter at all).
However, it would be sloppy practice to simply switch over from the
tried-and-tested (albeit slow) scripted version to the spanking new
builtin version that has not seen a whole lot of real-world testing.
So let's add a feature flag.
If the file `use-builtin-difftool` exists in Git's exec path, Git will
now automagically use the builtin version of the difftool, without
requiring the user to call `git builtin-difftool <args>`. This comes in
particularly handy when the difftool command is used from within
scripts.
If the file `use-builtin-difftool` is absent from Git's exec path, which
is the default, Git will use the scripted version as before.
The original idea was to use an environment variable
GIT_USE_BUILTIN_DIFFTOOL, but the test suite resets those variables, and
we do want to use that feature flag to run the tests with, and without,
the feature flag.
Besides, the plan is to add an opt-in flag in Git for Windows'
installer. If we implemented the feature flag as an environment
variable, we would have to modify the user's environment, in order to
make the builtin difftool the default when called from Git Bash, Git CMD
or third-party tools.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.gitignore | 1 +
git-difftool.perl | 7 +++++++
git.c | 20 ++++++++++++++++++++
3 files changed, 28 insertions(+)
diff --git a/.gitignore b/.gitignore
index 4f54531..91bfd09 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,4 @@
+/use-builtin-difftool
/GIT-BUILD-OPTIONS
/GIT-CFLAGS
/GIT-LDFLAGS
diff --git a/git-difftool.perl b/git-difftool.perl
index a5790d0..28e47d8 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -23,6 +23,13 @@ use File::Temp qw(tempdir);
use Getopt::Long qw(:config pass_through);
use Git;
+if (-e Git::exec_path() . '/use-builtin-difftool') {
+ unshift(@ARGV, "builtin-difftool");
+ unshift(@ARGV, "git");
+ exec(@ARGV);
+ die("Could not execute builtin difftool");
+}
+
sub usage
{
my $exitcode = shift;
diff --git a/git.c b/git.c
index eaa0f67..7a0df7a 100644
--- a/git.c
+++ b/git.c
@@ -2,6 +2,7 @@
#include "exec_cmd.h"
#include "help.h"
#include "run-command.h"
+#include "dir.h"
const char git_usage_string[] =
"git [--version] [--help] [-C <path>] [-c name=value]\n"
@@ -542,6 +543,22 @@ static void strip_extension(const char **argv)
#define strip_extension(cmd)
#endif
+static int use_builtin_difftool(void)
+{
+ static int initialized, use;
+
+ if (!initialized) {
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(&buf, "%s/%s", git_exec_path(),
+ "use-builtin-difftool");
+ use = file_exists(buf.buf);
+ strbuf_release(&buf);
+ initialized = 1;
+ }
+
+ return use;
+}
+
static void handle_builtin(int argc, const char **argv)
{
struct argv_array args = ARGV_ARRAY_INIT;
@@ -551,6 +568,9 @@ static void handle_builtin(int argc, const char **argv)
strip_extension(argv);
cmd = argv[0];
+ if (!strcmp("difftool", cmd) && use_builtin_difftool())
+ cmd = "builtin-difftool";
+
/* Turn "git cmd --help" into "git help --exclude-guides cmd" */
if (argc > 1 && !strcmp(argv[1], "--help")) {
int i;
--
2.10.1.583.g721a9e0
^ permalink raw reply related
* [PATCH 1/2] difftool: add the builtin
From: Johannes Schindelin @ 2016-11-22 17:01 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1479834051.git.johannes.schindelin@gmx.de>
This adds a builtin difftool that represents a conversion of the current
Perl script version of the difftool.
The motivation is that Perl scripts are not at all native on Windows,
and that `git difftool` therefore is pretty slow on that platform, when
there is no good reason for it to be slow.
In addition, Perl does not really have access to Git's internals. That
means that any script will always have to jump through unnecessary
hoops.
The current version of the builtin difftool does not, however, make full
use of the internals but instead chooses to spawn a couple of Git
processes, still, to make for an easier conversion. There remains a lot
of room for improvement, left for a later date.
Note: the original difftool is still called by `git difftool`. To get the
new, experimental version, call `git builtin-difftool`. The reason: this
new, experimental, builtin difftool will be shipped as part of Git for
Windows v2.11.0, to allow for easier large-scale testing.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.gitignore | 1 +
Makefile | 1 +
builtin.h | 1 +
builtin/builtin-difftool.c | 680 +++++++++++++++++++++++++++++++++++++++++++++
git.c | 1 +
5 files changed, 684 insertions(+)
create mode 100644 builtin/builtin-difftool.c
diff --git a/.gitignore b/.gitignore
index 05cb58a..4f54531 100644
--- a/.gitignore
+++ b/.gitignore
@@ -51,6 +51,7 @@
/git-diff-tree
/git-difftool
/git-difftool--helper
+/git-builtin-difftool
/git-describe
/git-fast-export
/git-fast-import
diff --git a/Makefile b/Makefile
index f53fcc9..f764174 100644
--- a/Makefile
+++ b/Makefile
@@ -888,6 +888,7 @@ BUILTIN_OBJS += builtin/diff-files.o
BUILTIN_OBJS += builtin/diff-index.o
BUILTIN_OBJS += builtin/diff-tree.o
BUILTIN_OBJS += builtin/diff.o
+BUILTIN_OBJS += builtin/builtin-difftool.o
BUILTIN_OBJS += builtin/fast-export.o
BUILTIN_OBJS += builtin/fetch-pack.o
BUILTIN_OBJS += builtin/fetch.o
diff --git a/builtin.h b/builtin.h
index b9122bc..409a61e 100644
--- a/builtin.h
+++ b/builtin.h
@@ -60,6 +60,7 @@ extern int cmd_diff_files(int argc, const char **argv, const char *prefix);
extern int cmd_diff_index(int argc, const char **argv, const char *prefix);
extern int cmd_diff(int argc, const char **argv, const char *prefix);
extern int cmd_diff_tree(int argc, const char **argv, const char *prefix);
+extern int cmd_builtin_difftool(int argc, const char **argv, const char *prefix);
extern int cmd_fast_export(int argc, const char **argv, const char *prefix);
extern int cmd_fetch(int argc, const char **argv, const char *prefix);
extern int cmd_fetch_pack(int argc, const char **argv, const char *prefix);
diff --git a/builtin/builtin-difftool.c b/builtin/builtin-difftool.c
new file mode 100644
index 0000000..9feefcd
--- /dev/null
+++ b/builtin/builtin-difftool.c
@@ -0,0 +1,680 @@
+/*
+ * "git difftool" builtin command
+ *
+ * This is a wrapper around the GIT_EXTERNAL_DIFF-compatible
+ * git-difftool--helper script.
+ *
+ * This script exports GIT_EXTERNAL_DIFF and GIT_PAGER for use by git.
+ * The GIT_DIFF* variables are exported for use by git-difftool--helper.
+ *
+ * Any arguments that are unknown to this script are forwarded to 'git diff'.
+ *
+ * Copyright (C) 2016 Johannes Schindelin
+ */
+#include "cache.h"
+#include "builtin.h"
+#include "parse-options.h"
+#include "run-command.h"
+#include "argv-array.h"
+#include "strbuf.h"
+#include "lockfile.h"
+
+static char *diff_gui_tool;
+static int trust_exit_code;
+
+static const char * const builtin_difftool_usage[] = {
+ N_("git add [<options>] [--] <pathspec>..."),
+ NULL
+};
+
+static int difftool_config(const char *var, const char *value, void *cb)
+{
+ if (!strcmp(var, "diff.guitool")) {
+ diff_gui_tool = xstrdup(value);
+ return 0;
+ }
+
+ if (!strcmp(var, "difftool.trustexitcode")) {
+ trust_exit_code = git_config_bool(var, value);
+ return 0;
+ }
+
+ return git_default_config(var, value, cb);
+}
+
+static int print_tool_help(void)
+{
+ const char *argv[] = { "mergetool", "--tool-help=diff", NULL };
+ return run_command_v_opt(argv, RUN_GIT_CMD);
+}
+
+static int parse_index_info(char *p, int *mode1, int *mode2,
+ struct object_id *oid1, struct object_id *oid2,
+ char *status)
+{
+ if (*p != ':')
+ return error("expected ':', got '%c'", *p);
+ *mode1 = (int)strtol(p + 1, &p, 8);
+ if (*p != ' ')
+ return error("expected ' ', got '%c'", *p);
+ *mode2 = (int)strtol(p + 1, &p, 8);
+ if (*p != ' ')
+ return error("expected ' ', got '%c'", *p);
+ if (get_oid_hex(++p, oid1))
+ return error("expected object ID, got '%s'", p + 1);
+ p += GIT_SHA1_HEXSZ;
+ if (*p != ' ')
+ return error("expected ' ', got '%c'", *p);
+ if (get_oid_hex(++p, oid2))
+ return error("expected object ID, got '%s'", p + 1);
+ p += GIT_SHA1_HEXSZ;
+ if (*p != ' ')
+ return error("expected ' ', got '%c'", *p);
+ *status = *++p;
+ if (!status || p[1])
+ return error("unexpected trailer: '%s'", p);
+ return 0;
+}
+
+/*
+ * Remove any trailing slash from $workdir
+ * before starting to avoid double slashes in symlink targets.
+ */
+static void add_path(struct strbuf *buf, size_t base_len, const char *path)
+{
+ strbuf_setlen(buf, base_len);
+ if (buf->len && buf->buf[buf->len - 1] != '/')
+ strbuf_addch(buf, '/');
+ strbuf_addstr(buf, path);
+}
+
+/*
+ * Determine whether we can simply reuse the file in the worktree.
+ */
+static int use_wt_file(const char *workdir, const char *name,
+ struct object_id *oid)
+{
+ struct strbuf buf = STRBUF_INIT;
+ struct stat st;
+ int use = 0;
+
+ strbuf_addstr(&buf, workdir);
+ add_path(&buf, buf.len, name);
+
+ if (!lstat(buf.buf, &st) && !S_ISLNK(st.st_mode)) {
+ struct object_id wt_oid;
+ int fd = open(buf.buf, O_RDONLY);
+
+ if (!index_fd(wt_oid.hash, fd, &st, OBJ_BLOB, name, 0)) {
+ if (is_null_oid(oid)) {
+ oidcpy(oid, &wt_oid);
+ use = 1;
+ } else if (!oidcmp(oid, &wt_oid))
+ use = 1;
+ }
+ }
+
+ strbuf_release(&buf);
+
+ return use;
+}
+
+struct working_tree_entry {
+ struct hashmap_entry entry;
+ char path[FLEX_ARRAY];
+};
+
+static int working_tree_entry_cmp(struct working_tree_entry *a,
+ struct working_tree_entry *b, void *keydata)
+{
+ return strcmp(a->path, b->path);
+}
+
+/*
+ * The `left` and `right` entries hold paths for the symlinks hashmap,
+ * and a SHA-1 surrounded by brief text for submodules.
+ */
+struct pair_entry {
+ struct hashmap_entry entry;
+ char left[PATH_MAX], right[PATH_MAX];
+ const char path[FLEX_ARRAY];
+};
+
+static int pair_cmp(struct pair_entry *a, struct pair_entry *b, void *keydata)
+{
+ return strcmp(a->path, b->path);
+}
+
+static void add_left_or_right(struct hashmap *map, const char *path,
+ const char *content, int is_right)
+{
+ struct pair_entry *e, *existing;
+
+ FLEX_ALLOC_STR(e, path, path);
+ hashmap_entry_init(e, strhash(path));
+ existing = hashmap_get(map, e, NULL);
+ if (existing) {
+ free(e);
+ e = existing;
+ } else {
+ e->left[0] = e->right[0] = '\0';
+ hashmap_add(map, e);
+ }
+ strcpy(is_right ? e->right : e->left, content);
+}
+
+struct path_entry {
+ struct hashmap_entry entry;
+ char path[FLEX_ARRAY];
+};
+
+int path_entry_cmp(struct path_entry *a, struct path_entry *b, void *key)
+{
+ return strcmp(a->path, key ? key : b->path);
+}
+
+static void changed_files(struct hashmap *result, const char *index_path,
+ const char *workdir)
+{
+ struct child_process update_index = CHILD_PROCESS_INIT;
+ struct child_process diff_files = CHILD_PROCESS_INIT;
+ struct strbuf index_env = STRBUF_INIT, buf = STRBUF_INIT;
+ const char *git_dir = absolute_path(get_git_dir()), *env[] = {
+ NULL, NULL
+ };
+ FILE *fp;
+
+ strbuf_addf(&index_env, "GIT_INDEX_FILE=%s", index_path);
+ env[0] = index_env.buf;
+
+ argv_array_pushl(&update_index.args,
+ "--git-dir", git_dir, "--work-tree", workdir,
+ "update-index", "--really-refresh", "-q",
+ "--unmerged", NULL);
+ update_index.no_stdin = 1;
+ update_index.no_stdout = 1;
+ update_index.no_stderr = 1;
+ update_index.git_cmd = 1;
+ update_index.use_shell = 0;
+ update_index.clean_on_exit = 1;
+ update_index.dir = workdir;
+ update_index.env = env;
+ /* Ignore any errors of update-index */
+ run_command(&update_index);
+
+ argv_array_pushl(&diff_files.args,
+ "--git-dir", git_dir, "--work-tree", workdir,
+ "diff-files", "--name-only", "-z", NULL);
+ diff_files.no_stdin = 1;
+ diff_files.git_cmd = 1;
+ diff_files.use_shell = 0;
+ diff_files.clean_on_exit = 1;
+ diff_files.out = -1;
+ diff_files.dir = workdir;
+ diff_files.env = env;
+ if (start_command(&diff_files))
+ die("could not obtain raw diff");
+ fp = xfdopen(diff_files.out, "r");
+ while (!strbuf_getline_nul(&buf, fp)) {
+ struct path_entry *entry;
+ FLEX_ALLOC_STR(entry, path, buf.buf);
+ hashmap_entry_init(entry, strhash(buf.buf));
+ hashmap_add(result, entry);
+ }
+ if (finish_command(&diff_files))
+ die("diff-files did not exit properly");
+ strbuf_release(&index_env);
+ strbuf_release(&buf);
+}
+
+#include "dir.h"
+
+static NORETURN void exit_cleanup(const char *tmpdir, int exit_code)
+{
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addstr(&buf, tmpdir);
+ remove_dir_recursively(&buf, 0);
+ if (exit_code)
+ warning(_("failed: %d"), exit_code);
+ exit(exit_code);
+}
+
+static int ensure_leading_directories(char *path)
+{
+ switch (safe_create_leading_directories(path)) {
+ case SCLD_OK:
+ case SCLD_EXISTS:
+ return 0;
+ default:
+ return error(_("could not create leading directories "
+ "of '%s'"), path);
+ }
+}
+
+static int run_dir_diff(const char *extcmd, int symlinks,
+ int argc, const char **argv)
+{
+ char tmpdir[PATH_MAX];
+ struct strbuf info = STRBUF_INIT, lpath = STRBUF_INIT;
+ struct strbuf rpath = STRBUF_INIT, buf = STRBUF_INIT;
+ struct strbuf ldir = STRBUF_INIT, rdir = STRBUF_INIT;
+ struct strbuf wtdir = STRBUF_INIT;
+ size_t ldir_len, rdir_len, wtdir_len;
+ struct cache_entry *ce = xcalloc(1, sizeof(ce) + PATH_MAX + 1);
+ const char *workdir, *tmp;
+ int ret = 0, i;
+ FILE *fp;
+ struct hashmap working_tree_dups, submodules, symlinks2;
+ struct hashmap_iter iter;
+ struct pair_entry *entry;
+ enum object_type type;
+ unsigned long size;
+ struct index_state wtindex;
+ struct checkout lstate, rstate;
+ int rc, flags = RUN_GIT_CMD, err = 0;
+ struct child_process child = CHILD_PROCESS_INIT;
+ const char *helper_argv[] = { "difftool--helper", NULL, NULL, NULL };
+ struct hashmap wt_modified, tmp_modified;
+ int indices_loaded = 0;
+
+ setup_work_tree();
+ workdir = get_git_work_tree();
+
+ /* Setup temp directories */
+ tmp = getenv("TMPDIR");
+ sprintf(tmpdir, "%s/git-difftool.XXXXXX", tmp ? tmp : "/tmp");
+ if (!mkdtemp(tmpdir))
+ return error("could not create temporary directory");
+ strbuf_addf(&ldir, "%s/left/", tmpdir);
+ strbuf_addf(&rdir, "%s/right/", tmpdir);
+ strbuf_addstr(&wtdir, workdir);
+ if (!wtdir.len || !is_dir_sep(wtdir.buf[wtdir.len - 1]))
+ strbuf_addch(&wtdir, '/');
+ mkdir(ldir.buf, 0777);
+ mkdir(rdir.buf, 0777);
+
+ memset(&wtindex, 0, sizeof(wtindex));
+
+ memset(&lstate, 0, sizeof(lstate));
+ lstate.base_dir = ldir.buf;
+ lstate.base_dir_len = ldir.len;
+ lstate.force = 1;
+ memset(&rstate, 0, sizeof(rstate));
+ rstate.base_dir = rdir.buf;
+ rstate.base_dir_len = rdir.len;
+ rstate.force = 1;
+
+ ldir_len = ldir.len;
+ rdir_len = rdir.len;
+ wtdir_len = wtdir.len;
+
+ hashmap_init(&working_tree_dups,
+ (hashmap_cmp_fn)working_tree_entry_cmp, 0);
+ hashmap_init(&submodules, (hashmap_cmp_fn)pair_cmp, 0);
+ hashmap_init(&symlinks2, (hashmap_cmp_fn)pair_cmp, 0);
+
+ child.no_stdin = 1;
+ child.git_cmd = 1;
+ child.use_shell = 0;
+ child.clean_on_exit = 1;
+ child.out = -1;
+ argv_array_pushl(&child.args, "diff", "--raw", "--no-abbrev", "-z",
+ NULL);
+ for (i = 0; i < argc; i++)
+ argv_array_push(&child.args, argv[i]);
+ if (start_command(&child))
+ die("could not obtain raw diff");
+ fp = xfdopen(child.out, "r");
+
+ /* Build index info for left and right sides of the diff */
+ while (!strbuf_getline_nul(&info, fp)) {
+ int lmode, rmode;
+ struct object_id loid, roid;
+ char status;
+ const char *src_path, *dst_path;
+ size_t src_path_len, dst_path_len;
+
+ if (starts_with(info.buf, "::"))
+ die(N_("combined diff formats('-c' and '--cc') are "
+ "not supported in\n"
+ "directory diff mode('-d' and '--dir-diff')."));
+
+ if (parse_index_info(info.buf, &lmode, &rmode, &loid, &roid,
+ &status))
+ break;
+ if (strbuf_getline_nul(&lpath, fp))
+ break;
+ src_path = lpath.buf;
+ src_path_len = lpath.len;
+
+ if (status != 'C' && status != 'R') {
+ dst_path = src_path;
+ dst_path_len = src_path_len;
+ } else {
+ if (strbuf_getline_nul(&rpath, fp))
+ break;
+ dst_path = rpath.buf;
+ dst_path_len = rpath.len;
+ }
+
+ if (S_ISGITLINK(lmode) || S_ISGITLINK(rmode)) {
+ strbuf_reset(&buf);
+ strbuf_addf(&buf, "Subproject commit %s",
+ oid_to_hex(&loid));
+ add_left_or_right(&submodules, src_path, buf.buf, 0);
+ strbuf_reset(&buf);
+ strbuf_addf(&buf, "Subproject commit %s",
+ oid_to_hex(&roid));
+ if (!oidcmp(&loid, &roid))
+ strbuf_addstr(&buf, "-dirty");
+ add_left_or_right(&submodules, dst_path, buf.buf, 1);
+ continue;
+ }
+
+ if (S_ISLNK(lmode)) {
+ char *content = read_sha1_file(loid.hash, &type, &size);
+ add_left_or_right(&symlinks2, src_path, content, 0);
+ free(content);
+ }
+
+ if (S_ISLNK(rmode)) {
+ char *content = read_sha1_file(roid.hash, &type, &size);
+ add_left_or_right(&symlinks2, dst_path, content, 1);
+ free(content);
+ }
+
+ if (lmode && status != 'C') {
+ ce->ce_mode = lmode;
+ oidcpy(&ce->oid, &loid);
+ strcpy(ce->name, src_path);
+ ce->ce_namelen = src_path_len;
+ if (checkout_entry(ce, &lstate, NULL))
+ return error("could not write '%s'", src_path);
+ }
+
+ if (rmode) {
+ struct working_tree_entry *entry;
+
+ /* Avoid duplicate working_tree entries */
+ FLEX_ALLOC_STR(entry, path, dst_path);
+ hashmap_entry_init(entry, strhash(dst_path));
+ if (hashmap_get(&working_tree_dups, entry, NULL)) {
+ free(entry);
+ continue;
+ }
+ hashmap_add(&working_tree_dups, entry);
+
+ if (!use_wt_file(workdir, dst_path, &roid)) {
+ ce->ce_mode = rmode;
+ oidcpy(&ce->oid, &roid);
+ strcpy(ce->name, dst_path);
+ ce->ce_namelen = dst_path_len;
+ if (checkout_entry(ce, &rstate, NULL))
+ return error("could not write '%s'",
+ dst_path);
+ } else if (!is_null_oid(&roid)) {
+ /*
+ * Changes in the working tree need special
+ * treatment since they are not part of the
+ * index.
+ */
+ struct cache_entry *ce2 =
+ make_cache_entry(rmode, roid.hash,
+ dst_path, 0, 0);
+ ce_mode_from_stat(ce2, rmode);
+
+ add_index_entry(&wtindex, ce2,
+ ADD_CACHE_JUST_APPEND);
+
+ add_path(&wtdir, wtdir_len, dst_path);
+ add_path(&rdir, rdir_len, dst_path);
+ if (ensure_leading_directories(rdir.buf))
+ return error("could not create "
+ "directory for '%s'",
+ dst_path);
+ if (symlinks) {
+ if (symlink(wtdir.buf, rdir.buf)) {
+ ret = error_errno("could not symlink '%s' to '%s'", wtdir.buf, rdir.buf);
+ goto finish;
+ }
+ } else {
+ struct stat st;
+ if (stat(wtdir.buf, &st))
+ st.st_mode = 0644;
+ if (copy_file(rdir.buf, wtdir.buf,
+ st.st_mode)) {
+ ret = error("could not copy '%s' to '%s'", wtdir.buf, rdir.buf);
+ goto finish;
+ }
+ }
+ }
+ }
+ }
+ if (finish_command(&child)) {
+ ret = error("error occurred running diff --raw");
+ goto finish;
+ }
+
+ /*
+ * Changes to submodules require special treatment.This loop writes a
+ * temporary file to both the left and right directories to show the
+ * change in the recorded SHA1 for the submodule.
+ */
+ hashmap_iter_init(&submodules, &iter);
+ while ((entry = hashmap_iter_next(&iter))) {
+ if (*entry->left) {
+ add_path(&ldir, ldir_len, entry->path);
+ ensure_leading_directories(ldir.buf);
+ write_file(ldir.buf, "%s", entry->left);
+ }
+ if (*entry->right) {
+ add_path(&rdir, rdir_len, entry->path);
+ ensure_leading_directories(rdir.buf);
+ write_file(rdir.buf, "%s", entry->right);
+ }
+ }
+
+ /*
+ * Symbolic links require special treatment.The standard "git diff"
+ * shows only the link itself, not the contents of the link target.
+ * This loop replicates that behavior.
+ */
+ hashmap_iter_init(&symlinks2, &iter);
+ while ((entry = hashmap_iter_next(&iter))) {
+ if (*entry->left) {
+ add_path(&ldir, ldir_len, entry->path);
+ ensure_leading_directories(ldir.buf);
+ write_file(ldir.buf, "%s", entry->left);
+ }
+ if (*entry->right) {
+ add_path(&rdir, rdir_len, entry->path);
+ ensure_leading_directories(rdir.buf);
+ write_file(rdir.buf, "%s", entry->right);
+ }
+ }
+
+ strbuf_release(&buf);
+
+ strbuf_setlen(&ldir, ldir_len);
+ helper_argv[1] = ldir.buf;
+ strbuf_setlen(&rdir, rdir_len);
+ helper_argv[2] = rdir.buf;
+
+ if (extcmd) {
+ helper_argv[0] = extcmd;
+ flags = 0;
+ } else
+ setenv("GIT_DIFFTOOL_DIRDIFF", "true", 1);
+ rc = run_command_v_opt(helper_argv, flags);
+
+ /*
+ * If the diff includes working copy files and those
+ * files were modified during the diff, then the changes
+ * should be copied back to the working tree.
+ * Do not copy back files when symlinks are used and the
+ * external tool did not replace the original link with a file.
+ *
+ * These hashes are loaded lazily since they aren't needed
+ * in the common case of --symlinks and the difftool updating
+ * files through the symlink.
+ */
+ hashmap_init(&wt_modified, (hashmap_cmp_fn)path_entry_cmp,
+ wtindex.cache_nr);
+ hashmap_init(&tmp_modified, (hashmap_cmp_fn)path_entry_cmp,
+ wtindex.cache_nr);
+
+ for (i = 0; i < wtindex.cache_nr; i++) {
+ struct hashmap_entry dummy;
+ const char *name = wtindex.cache[i]->name;
+ struct stat st;
+
+ add_path(&rdir, rdir_len, name);
+ if (lstat(rdir.buf, &st))
+ continue;
+
+ if ((symlinks && S_ISLNK(st.st_mode)) || !S_ISREG(st.st_mode))
+ continue;
+
+ if (!indices_loaded) {
+ static struct lock_file lock;
+ strbuf_reset(&buf);
+ strbuf_addf(&buf, "%s/wtindex", tmpdir);
+ if (hold_lock_file_for_update(&lock, buf.buf, 0) < 0 ||
+ write_locked_index(&wtindex, &lock, COMMIT_LOCK)) {
+ ret = error("could not write %s", buf.buf);
+ rollback_lock_file(&lock);
+ goto finish;
+ }
+ changed_files(&wt_modified, buf.buf, workdir);
+ strbuf_setlen(&rdir, rdir_len);
+ changed_files(&tmp_modified, buf.buf, rdir.buf);
+ add_path(&rdir, rdir_len, name);
+ indices_loaded = 1;
+ }
+
+ hashmap_entry_init(&dummy, strhash(name));
+ if (hashmap_get(&tmp_modified, &dummy, name)) {
+ add_path(&wtdir, wtdir_len, name);
+ if (hashmap_get(&wt_modified, &dummy, name)) {
+ warning(_("both files modified: '%s' and '%s'."),
+ wtdir.buf, rdir.buf);
+ warning(_("working tree file has been left."));
+ warning("");
+ err = 1;
+ } else if (unlink(wtdir.buf) ||
+ copy_file(wtdir.buf, rdir.buf, st.st_mode))
+ warning_errno(_("could not copy '%s' to '%s'"),
+ rdir.buf, wtdir.buf);
+ }
+ }
+
+ if (err) {
+ warning(_("temporary files exist in '%s'."), tmpdir);
+ warning(_("you may want to cleanup or recover these."));
+ exit(1);
+ } else
+ exit_cleanup(tmpdir, rc);
+
+finish:
+ free(ce);
+ strbuf_release(&ldir);
+ strbuf_release(&rdir);
+ strbuf_release(&wtdir);
+ strbuf_release(&buf);
+
+ return ret;
+}
+
+static int run_file_diff(int prompt, int argc, const char **argv)
+{
+ struct argv_array args = ARGV_ARRAY_INIT;
+ const char *env[] = {
+ "GIT_PAGER=", "GIT_EXTERNAL_DIFF=git-difftool--helper", NULL,
+ NULL
+ };
+ int ret = 0, i;
+
+ if (prompt > 0)
+ env[2] = "GIT_DIFFTOOL_PROMPT=true";
+ else if (!prompt)
+ env[2] = "GIT_DIFFTOOL_NO_PROMPT=true";
+
+ argv_array_push(&args, "diff");
+ for (i = 0; i < argc; i++)
+ argv_array_push(&args, argv[i]);
+ ret = run_command_v_opt_cd_env(args.argv, RUN_GIT_CMD, NULL, env);
+ exit(ret);
+}
+
+int cmd_builtin_difftool(int argc, const char ** argv, const char * prefix)
+{
+ int use_gui_tool = 0, dir_diff = 0, prompt = -1, symlinks = 0,
+ tool_help = 0;
+ static char *difftool_cmd = NULL, *extcmd = NULL;
+
+ struct option builtin_difftool_options[] = {
+ OPT_BOOL('g', "gui", &use_gui_tool,
+ N_("use `diff.guitool` instead of `diff.tool`")),
+ OPT_BOOL('d', "dir-diff", &dir_diff,
+ N_("perform a full-directory diff")),
+ { OPTION_SET_INT, 'y', "no-prompt", &prompt, NULL,
+ N_("do not prompt before launching a diff tool"),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0},
+ { OPTION_SET_INT, 0, "prompt", &prompt, NULL, NULL,
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN,
+ NULL, 1 },
+ OPT_BOOL(0, "symlinks", &symlinks,
+ N_("use symlinks in dir-diff mode")),
+ OPT_STRING('t', "tool", &difftool_cmd, N_("<tool>"),
+ N_("use the specified diff tool")),
+ OPT_BOOL(0, "tool-help", &tool_help,
+ N_("print a list of diff tools that may be used with "
+ "`--tool`")),
+ OPT_BOOL(0, "trust-exit-code", &trust_exit_code,
+ N_("make 'git-difftool' exit when an invoked diff "
+ "tool returns a non - zero exit code")),
+ OPT_STRING('x', "extcmd", &extcmd, N_("<command>"),
+ N_("specify a custom command for viewing diffs")),
+ OPT_END()
+ };
+
+ symlinks = has_symlinks;
+
+ git_config(difftool_config, NULL);
+
+ argc = parse_options(argc, argv, prefix, builtin_difftool_options,
+ builtin_difftool_usage, PARSE_OPT_KEEP_UNKNOWN |
+ PARSE_OPT_KEEP_DASHDASH);
+
+ if (tool_help)
+ return print_tool_help();
+
+ if (use_gui_tool && diff_gui_tool && *diff_gui_tool)
+ setenv("GIT_DIFF_TOOL", diff_gui_tool, 1);
+ else if (difftool_cmd) {
+ if (*difftool_cmd)
+ setenv("GIT_DIFF_TOOL", difftool_cmd, 1);
+ else
+ die(_("no <tool> given for --tool=<tool>"));
+ }
+
+ if (extcmd) {
+ if (*extcmd)
+ setenv("GIT_DIFFTOOL_EXTCMD", extcmd, 1);
+ else
+ die(_("no <cmd> given for --extcmd=<cmd>"));
+ }
+
+ setenv("GIT_DIFFTOOL_TRUST_EXIT_CODE",
+ trust_exit_code ? "true" : "false", 1);
+
+ /*
+ * In directory diff mode, 'git-difftool--helper' is called once
+ * to compare the a / b directories.In file diff mode, 'git diff'
+ * will invoke a separate instance of 'git-difftool--helper' for
+ * each file that changed.
+ */
+ if (dir_diff)
+ return run_dir_diff(extcmd, symlinks, argc, argv);
+ return run_file_diff(prompt, argc, argv);
+}
diff --git a/git.c b/git.c
index efa1059..eaa0f67 100644
--- a/git.c
+++ b/git.c
@@ -424,6 +424,7 @@ static struct cmd_struct commands[] = {
{ "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE },
{ "diff-index", cmd_diff_index, RUN_SETUP },
{ "diff-tree", cmd_diff_tree, RUN_SETUP },
+ { "builtin-difftool", cmd_builtin_difftool, RUN_SETUP | NEED_WORK_TREE },
{ "fast-export", cmd_fast_export, RUN_SETUP },
{ "fetch", cmd_fetch, RUN_SETUP },
{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
--
2.10.1.583.g721a9e0
^ permalink raw reply related
* [PATCH 0/2] Show Git Mailing List: a builtin difftool
From: Johannes Schindelin @ 2016-11-22 17:01 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
I have been working on the builtin difftool for a little over a week,
for two reasons:
1. Perl is really not native on Windows. Not only is there a performance
penalty to be paid just for running Perl scripts, we also have to deal
with the fact that users may have different Perl installations, with
different options, and some other Perl installation may decide to set
PERL5LIB globally, wreaking havoc with Git for Windows' Perl (which we
have to use because almost all other Perl distributions lack the
Subversion bindings we need for `git svn`).
2. Perl makes for a rather large reason that Git for Windows' installer
weighs in with >30MB. While one Perl script less does not relieve us
of that burden, it is one step in the right direction.
This pair of patches serves two purposes: to ask for reviews, and to
show what I plan to release as part of Git for Windows v2.11.0 (which is
due this Thursday, if Git v2.11.0 is released tomorrow, as planned).
The second patch really only explains how I will make sure that the
builtin difftool will only affect users who want to opt in to testing.
Johannes Schindelin (2):
difftool: add the builtin
difftool: add a feature flag for the builtin vs scripted version
.gitignore | 2 +
Makefile | 1 +
builtin.h | 1 +
builtin/builtin-difftool.c | 680 +++++++++++++++++++++++++++++++++++++++++++++
git-difftool.perl | 7 +
git.c | 21 ++
6 files changed, 712 insertions(+)
create mode 100644 builtin/builtin-difftool.c
base-commit: 1310affe024fba407bff55dbe65cd6d670c8a32d
Published-As: https://github.com/dscho/git/releases/tag/builtin-difftool-v1
Fetch-It-Via: git fetch https://github.com/dscho/git builtin-difftool-v1
--
2.10.1.583.g721a9e0
^ permalink raw reply
* Re: [PATCH 2/3] stripspace: respect repository config
From: Johannes Schindelin @ 2016-11-22 16:13 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Junio C Hamano, Ralf Thielow, Taufiq Hoven
In-Reply-To: <CACsJy8D5oBR+vo2B+Ro2Q4SX0CG3jME4Gfs1_6AohccmpNvD0A@mail.gmail.com>
Hi Duy,
On Tue, 22 Nov 2016, Duy Nguyen wrote:
> On Mon, Nov 21, 2016 at 9:18 PM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
> > When eff80a9 (Allow custom "comment char", 2013-01-16) taught the
> > `stripspace` command to respect the config setting `core.commentChar`,
> > it forgot that this variable may be defined in .git/config.
> >
> > So when rebasing interactively with a commentChar defined in the current
> > repository's config, the help text at the bottom of the edit script
> > potentially used an incorrect comment character. This was not only
> > funny-looking, but also resulted in tons of warnings like this one:
> >
> > Warning: the command isn't recognized in the following line
> > - #
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > builtin/stripspace.c | 4 +++-
> > t/t0030-stripspace.sh | 2 +-
> > 2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/builtin/stripspace.c b/builtin/stripspace.c
> > index 15e716e..1e62a00 100644
> > --- a/builtin/stripspace.c
> > +++ b/builtin/stripspace.c
> > @@ -44,8 +44,10 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)
> > if (argc)
> > usage_with_options(stripspace_usage, options);
> >
> > - if (mode == STRIP_COMMENTS || mode == COMMENT_LINES)
> > + if (mode == STRIP_COMMENTS || mode == COMMENT_LINES) {
> > + setup_git_directory_gently(NULL);
> > git_config(git_default_config, NULL);
> > + }
>
> This conditional config file reading is a trap for similar bugs to
> happen again. Is there any reason we should not just mark the command
> RUN_SETUP_GENTLY in git.c and call git_config() here unconditionally?
As I plan to slip these patches into Git for Windows v2.11.0, i.e. making
this a last-minute hot fix, I want to err on the side of caution. So I'd
rather keep this conditional (it might regress on the performance front,
or something).
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/3] stripspace: respect repository config
From: Johannes Schindelin @ 2016-11-22 16:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20161121190514.18574-2-gitster@pobox.com>
Hi Junio,
On Mon, 21 Nov 2016, Junio C Hamano wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The way "git stripspace" reads the configuration was not quite
> correct, in that it forgot to probe for a possibly existing
> repository (note: stripspace is designed to be usable outside the
> repository as well) before doing so. Due to this, .git/config was
> read only when the command was run from the top-level of the working
> tree.
>
> A recent change b9605bc4f2 ("config: only read .git/config from
> configured repos", 2016-09-12) stopped reading the repository-local
> configuration file ".git/config" unless the repository discovery
> process is done, and ".git/config" is no longer read even when run
> from the top-level, which exposed the bug even more.
>
> When rebasing interactively with a commentChar defined in the
> current repository's config, the help text at the bottom of the edit
> script potentially used an incorrect comment character. This was not
> only funny-looking, but also resulted in tons of warnings like this
> one:
>
> Warning: the command isn't recognized in the following line
> - #
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Thanks for the corrected commit message!
Dscho
^ permalink raw reply
* Re: [PATCH 1/3] rebase -i: highlight problems with core.commentchar
From: Johannes Schindelin @ 2016-11-22 16:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20161121190514.18574-1-gitster@pobox.com>
Hi Junio,
On Mon, 21 Nov 2016, Junio C Hamano wrote:
> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> index 29e91d861c..c1f6411eb2 100755
> --- a/t/t0030-stripspace.sh
> +++ b/t/t0030-stripspace.sh
> @@ -432,6 +432,15 @@ test_expect_success '-c with changed comment char' '
> test_cmp expect actual
> '
>
> +test_expect_failure '-c with comment char defined in .git/config' '
> + test_config core.commentchar = &&
> + printf "= foo\n" >expect &&
> + printf "foo" | (
Could I ask you to sneak in a \n here?
Thanks,
Dscho
^ permalink raw reply
* Re: [PATCH 1/3] rebase -i: identify problems with core.commentchar
From: Johannes Schindelin @ 2016-11-22 16:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, git, Ralf Thielow,
Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <xmqqbmx8k8c0.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Mon, 21 Nov 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> > index 29e91d8..202ac07 100755
> > --- a/t/t0030-stripspace.sh
> > +++ b/t/t0030-stripspace.sh
> > @@ -432,6 +432,13 @@ test_expect_success '-c with changed comment char' '
> > test_cmp expect actual
> > '
> >
> > +test_expect_failure '-c with comment char defined in .git/config' '
> > + test_config core.commentchar = &&
> > + printf "= foo\n" >expect &&
> > + printf "foo" | git stripspace -c >actual &&
>
> We'd want "\n" on this printf to match the one before as well,
True.
> The analysis of the log message in [2/3] is wrong and needs
> updating, though.
Thanks for following up on that, and for fixing the commit message.
> > +test_expect_failure 'rebase -i respects core.commentchar=auto' '
> > + test_config core.commentchar auto &&
> > + write_script copy-edit-script.sh <<-\EOF &&
> > + cp "$1" edit-script
> > + EOF
> > + test_set_editor "$(pwd)/copy-edit-script.sh" &&
> > + test_when_finished "git rebase --abort || :" &&
> > + git rebase -i HEAD^ &&
> > + grep "^#" edit-script &&
>
> This was added for debugging that was forgotten?
No, this was me trying to ensure that the comment character '#' *is* used.
As opposed to somehow testing any edit script that contains no commented
lines whatsoever.
But if you don't care, I won't, either.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 3/3] rebase -i: handle core.commentChar=auto
From: Johannes Schindelin @ 2016-11-22 16:04 UTC (permalink / raw)
To: Johannes Sixt
Cc: git, Junio C Hamano, Ralf Thielow,
Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <975c0002-8afe-efa6-d967-72f2a0d21169@kdbg.org>
Hi Hannes,
On Mon, 21 Nov 2016, Johannes Sixt wrote:
> Am 21.11.2016 um 15:18 schrieb Johannes Schindelin:
> > -comment_char=$(git config --get core.commentchar 2>/dev/null | cut -c1)
> > -: ${comment_char:=#}
> > +comment_char=$(git config --get core.commentchar 2>/dev/null)
> > +case "$comment_char" in
> > +''|auto)
> > + comment_char=#
> > + ;;
> > +?)
> > + ;;
> > +*)
> > + comment_char=$(comment_char | cut -c1)
>
> comment_char is a command? Did you mean
>
> comment_char=$(echo "$comment_char" | cut -c1)
D'oh.
> It could be written without forking a process:
>
> comment_char=${comment_char%${comment_char#?}}
>
> (aka "remove from the end what remains after removing the first character")
I was considering this, actually, but it is rather unreadable. Better
rewrite it in C, to begin with.
Thanks,
Dscho
^ permalink raw reply
* Re: [GIT PULL] l10n updates for 2.11.0 round 2
From: Jiang Xin @ 2016-11-22 15:11 UTC (permalink / raw)
To: Junio C Hamano
Cc: Alexander Shopov, Alex Henrie, Ralf Thielow, Jean-Noël Avila,
Marco Paolone, Changwoo Ryu, Vasco Almeida, Dimitriy Ryazantcev,
Peter Krefting, Trần Ngọc Quân, Jiang Xin,
Git List
In-Reply-To: <CANYiYbGtfZKshmfYxHM0sL2Z67=3KTF0kixP1OjumRw87e9XkA@mail.gmail.com>
Git 2.11.0-rc2 introduced a very small l10n update. Because the
update windows will be
closed tomorrow, I made a batch updates. See commit:
* https://github.com/git-l10n/git-po/commit/275588f93
I have a reduced diff for this commit using a custom diff driver, see:
* https://gist.github.com/jiangxin/6384b1e865249228e00385fab84ef3f3
2016-11-22 22:59 GMT+08:00 Jiang Xin <worldhello.net@gmail.com>:
> Hi Junio,
>
> The following changes since commit 1310affe024fba407bff55dbe65cd6d670c8a32d:
>
> Git 2.11-rc2 (2016-11-17 13:47:36 -0800)
>
> are available in the git repository at:
>
> git://github.com/git-l10n/git-po tags/l10n-2.11.0-rnd2
--
Jiang Xin
^ permalink raw reply
* [GIT PULL] l10n updates for 2.11.0 round 2
From: Jiang Xin @ 2016-11-22 14:59 UTC (permalink / raw)
To: Junio C Hamano
Cc: Alexander Shopov, Alex Henrie, Ralf Thielow, Jean-Noël Avila,
Marco Paolone, Changwoo Ryu, Vasco Almeida, Dimitriy Ryazantcev,
Peter Krefting, Trần Ngọc Quân, Jiang Xin,
Git List
Hi Junio,
The following changes since commit 1310affe024fba407bff55dbe65cd6d670c8a32d:
Git 2.11-rc2 (2016-11-17 13:47:36 -0800)
are available in the git repository at:
git://github.com/git-l10n/git-po tags/l10n-2.11.0-rnd2
for you to fetch changes up to 275588f93eedd8d7a556c38b75944b858e704dce:
l10n: Fixed typo of git fetch-pack command (2016-11-22 22:24:59 +0800)
----------------------------------------------------------------
l10n-2.11.0-rnd2
----------------------------------------------------------------
Changwoo Ryu (1):
l10n: ko.po: Update Korean translation
Dimitriy Ryazantcev (1):
l10n: ru.po: update Russian translation
Jean-Noel Avila (1):
l10n: fr.po v2.11.0_rnd1
Jiang Xin (8):
l10n: git.pot: v2.11.0 round 1 (209 new, 53 removed)
Merge branch 'russian-l10n' of https://github.com/DJm00n/git-po-ru
Merge branch 'ko/merge-l10n' of https://github.com/changwoo/git-l10n-ko
Merge branch 'fr_v2.11.0_rnd1' of git://github.com/jnavila/git
l10n: zh_CN: for git v2.11.0 l10n round 1
Merge branch 'master' of git://github.com/git-l10n/git-po
l10n: git.pot: v2.11.0 round 2 (1 new, 1 removed)
l10n: Fixed typo of git fetch-pack command
Peter Krefting (1):
l10n: sv.po: Update Swedish translation (2913t0f0u)
Trần Ngọc Quân (1):
l10n: vi.po: Updated translation to v2.11.0 (2913t)
Vasco Almeida (1):
l10n: pt_PT: update Portuguese translation
jfbu (1):
l10n: fr.po fix grammar mistakes
po/fr.po | 8916 +++++++++++++++++++++++++++++++++--------------------------
po/git.pot | 6664 ++++++++++++++++++++++++--------------------
po/ko.po | 8729 +++++++++++++++++++++++++++++++--------------------------
po/pt_PT.po | 8794 ++++++++++++++++++++++++++++++++--------------------------
po/ru.po | 4429 +++++++++++++++++------------
po/sv.po | 8709 +++++++++++++++++++++++++++++++--------------------------
po/vi.po | 8697 +++++++++++++++++++++++++++++++--------------------------
po/zh_CN.po | 8689 +++++++++++++++++++++++++++++++--------------------------
8 files changed, 35318 insertions(+), 28309 deletions(-)
--
Jiang Xin
^ permalink raw reply
* v2.11 new diff heuristic?
From: Robert Dailey @ 2016-11-22 14:42 UTC (permalink / raw)
To: Git
The release notes mention a new heuristic for diff:
* Output from "git diff" can be made easier to read by selecting
which lines are common and which lines are added/deleted
intelligently when the lines before and after the changed section
are the same. A command line option is added to help with the
experiment to find a good heuristics.
However, it lacks information on exactly how to use this new feature.
I dug into the git diff documentation here:
https://git-scm.com/docs/git-diff
It mentions a "--compaction-heuristic" option. Is this the new
heuristic outlined by the release notes? If not, which is it? Is the
compaction heuristic compatible with the histogram diff algorithm? Is
there a config option to turn this on all the time? For that matter,
is this something I can keep on all the time or is it only useful in
certain situations?
There's still so much more about this feature I would like to know.
^ permalink raw reply
* Re: [PATCH v6 01/16] Git.pm: add subroutines for commenting lines
From: Vasco Almeida @ 2016-11-22 14:10 UTC (permalink / raw)
To: git
Cc: Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Jakub Narębski, David Aguilar,
Junio C Hamano
In-Reply-To: <20161111124541.8216-2-vascomalmeida@sapo.pt>
A Sex, 11-11-2016 às 11:45 -0100, Vasco Almeida escreveu:
> +=item comment_lines ( STRING [, STRING... ])
> +
> +Comments lines following core.commentchar configuration.
> +
> +=cut
> +
> +sub comment_lines {
> + my $comment_line_char = config("core.commentchar") || '#';
> + return prefix_lines("$comment_line_char ", @_);
> +}
> +
In light of the recent "Fix problems with rebase -i when
core.commentchar is defined" [1], I realized that this patch does not
handle the 'auto' value of core.commentchat configuration variable.
I propose to do the patch below in the next re-roll.
[1] http://www.mail-archive.com/git@vger.kernel.org/msg107818.html
-- >8 --
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 3a6d846..8d33634 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1073,6 +1073,7 @@ sub edit_hunk_manually {
my $is_reverse = $patch_mode_flavour{IS_REVERSE};
my ($remove_plus, $remove_minus) = $is_reverse ? ('-', '+') : ('+', '-');
my $comment_line_char = Git::config("core.commentchar") || '#';
+ $comment_line_char = '#' if ($comment_line_char eq 'auto');
print $fh Git::comment_lines sprintf(__ <<EOF, $remove_minus, $remove_plus, $comment_line_char),
---
To remove '%s' lines, make them ' ' lines (context).
diff --git a/perl/Git.pm b/perl/Git.pm
index 69cd1dd..47b5899 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1459,6 +1459,7 @@ Comments lines following core.commentchar configuration.
sub comment_lines {
my $comment_line_char = config("core.commentchar") || '#';
+ $comment_line_char = '#' if ($comment_line_char eq 'auto');
return prefix_lines("$comment_line_char ", @_);
}
^ permalink raw reply related
* Re: [PATCH v1 12/19] Documentation/config: add splitIndex.maxPercentChange
From: Duy Nguyen @ 2016-11-22 13:20 UTC (permalink / raw)
To: Christian Couder
Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason, git,
Christian Couder
In-Reply-To: <CAP8UFD2VFQs2X14omWEy-iJzEeTEeEOa+fxMEq3HTQK3rFyuCg@mail.gmail.com>
On Tue, Nov 22, 2016 at 8:13 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> So if we now mix things up just to avoid one more configuration
> option, we could very well make things harder to develop, to
> configure, to parse and to understand later, so it is not a trade off
> worth making.
OK since we're still in the early phase of determining how to
re-split, I guess going with elaborate, precise configuration is
better, even if it's more config options. Once we know better, maybe
we can decide a good default cover-all "auto" then (or maybe not
because we find out no shoe fits all).
--
Duy
^ permalink raw reply
* Re: [PATCH v1 12/19] Documentation/config: add splitIndex.maxPercentChange
From: Christian Couder @ 2016-11-22 13:13 UTC (permalink / raw)
To: Duy Nguyen
Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason, git,
Christian Couder
In-Reply-To: <CACsJy8BazeR=4tz3q2f35x=fCfp-Ld9LJz0mQh_CZoR_iXKEEQ@mail.gmail.com>
On Tue, Nov 22, 2016 at 11:35 AM, Duy Nguyen <pclouds@gmail.com> wrote:
[...]
>>> In my opinion, "true" _is_ auto, which is a way to say "I trust you to
>>> do the right thing, just re-split the index when it makes sense", "no"
>>> is disabled of course. If the user wants to be specific, just write
>>> "10" or some other percentage.(and either 0 or 100 would mean enable
>>> split-index but do not re-split automatically, let _me_ do it when I
>>> want it)
>>
>> The meaning of a future "auto" option for "core.splitIndex" could be
>> "use the split-index feature only if the number of entries in whole
>> index is greater than 10000 (by default)".
>
> Well.. with the "just re-split the index when it makes sense" part,
> the user entrusts git to do something sensible in all cases,
That's an interpretation of what "core.splitIndex=true" could mean,
but there could be users who trust Git to re-split when it makes
sense, but who do want to use the split-index on all theirs repos even
the small ones or who just don't trust Git to choose when it might be
better to use it or not.
Yeah, a typical git user would most of the time just trust Git for all
those things, but on the other hand there are companies out there that
are willing to tweak many configuration options to get the better
possible behavior for them.
In fact I am working on this for Booking.com, and if we find out later
that we would gain something significant, like performance
improvements or configuration simplification, by adding "auto" and/or
other configuration variables to tweak more split-index related
things, we might very well post patch series to do that.
So if we now mix things up just to avoid one more configuration
option, we could very well make things harder to develop, to
configure, to parse and to understand later, so it is not a trade off
worth making.
> and going
> with absolute numbers might not be the best way, I think. It's big
> responsibility :)
About going with absolute number, yeah I am not sure at all it is the
best way, but this was just part of an example to try to explain what
I am saying above.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox