All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Schindelin <johannes.schindelin@gmx.de>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	David Aguilar <davvid@gmail.com>,
	Dennis Kaarsemaker <dennis@kaarsemaker.net>
Subject: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
Date: Thu, 24 Nov 2016 21:55:07 +0100 (CET)	[thread overview]
Message-ID: <3a10fd72cf336743891d6bc9a003cfeb5c3f0063.1480019834.git.johannes.schindelin@gmx.de> (raw)
In-Reply-To: <cover.1480019834.git.johannes.schindelin@gmx.de>

This adds a builtin difftool that still falls back to the legacy Perl
version, which has been renamed to `legacy-difftool`.

The idea is that the new, experimental, builtin difftool immediately hands
off to the legacy difftool for now, unless the config variable
difftool.useBuiltin is set to true.

This feature flag will be used in the upcoming Git for Windows v2.11.0
release, to allow early testers to opt-in to use the builtin difftool and
flesh out any bugs.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .gitignore                                    |  1 +
 Makefile                                      |  3 +-
 builtin.h                                     |  1 +
 builtin/difftool.c                            | 63 +++++++++++++++++++++++++++
 git-difftool.perl => git-legacy-difftool.perl |  0
 git.c                                         |  6 +++
 t/t7800-difftool.sh                           |  2 +
 7 files changed, 75 insertions(+), 1 deletion(-)
 create mode 100644 builtin/difftool.c
 rename git-difftool.perl => git-legacy-difftool.perl (100%)

diff --git a/.gitignore b/.gitignore
index 05cb58a..f96e50e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -76,6 +76,7 @@
 /git-init-db
 /git-interpret-trailers
 /git-instaweb
+/git-legacy-difftool
 /git-log
 /git-ls-files
 /git-ls-remote
diff --git a/Makefile b/Makefile
index f53fcc9..7863bc2 100644
--- a/Makefile
+++ b/Makefile
@@ -527,7 +527,7 @@ SCRIPT_LIB += git-sh-setup
 SCRIPT_LIB += git-sh-i18n
 
 SCRIPT_PERL += git-add--interactive.perl
-SCRIPT_PERL += git-difftool.perl
+SCRIPT_PERL += git-legacy-difftool.perl
 SCRIPT_PERL += git-archimport.perl
 SCRIPT_PERL += git-cvsexportcommit.perl
 SCRIPT_PERL += git-cvsimport.perl
@@ -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/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..67f8051 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_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/difftool.c b/builtin/difftool.c
new file mode 100644
index 0000000..53870bb
--- /dev/null
+++ b/builtin/difftool.c
@@ -0,0 +1,63 @@
+/*
+ * "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 "builtin.h"
+#include "run-command.h"
+#include "exec_cmd.h"
+
+/*
+ * NEEDSWORK: this function can go once the legacy-difftool Perl script is
+ * retired.
+ *
+ * We intentionally avoid reading the config directly here, to avoid messing up
+ * the GIT_* environment variables when we need to fall back to exec()ing the
+ * Perl script.
+ */
+static int use_builtin_difftool(void) {
+	struct child_process cp = CHILD_PROCESS_INIT;
+	struct strbuf out = STRBUF_INIT;
+	int ret;
+
+	argv_array_pushl(&cp.args,
+			 "config", "--bool", "difftool.usebuiltin", NULL);
+	cp.git_cmd = 1;
+	if (capture_command(&cp, &out, 6))
+		return 0;
+	strbuf_trim(&out);
+	ret = !strcmp("true", out.buf);
+	strbuf_release(&out);
+	return ret;
+}
+
+int cmd_difftool(int argc, const char **argv, const char *prefix)
+{
+	/*
+	 * NEEDSWORK: Once the builtin difftool has been tested enough
+	 * and git-legacy-difftool.perl is retired to contrib/, this preamble
+	 * can be removed.
+	 */
+	if (!use_builtin_difftool()) {
+		const char *path = mkpath("%s/git-legacy-difftool",
+					  git_exec_path());
+
+		if (sane_execvp(path, (char **)argv) < 0)
+			die_errno("could not exec %s", path);
+
+		return 0;
+	}
+	prefix = setup_git_directory();
+	trace_repo_setup(prefix);
+	setup_work_tree();
+
+	die("TODO");
+}
diff --git a/git-difftool.perl b/git-legacy-difftool.perl
similarity index 100%
rename from git-difftool.perl
rename to git-legacy-difftool.perl
diff --git a/git.c b/git.c
index e8b2baf..a8e6a15 100644
--- a/git.c
+++ b/git.c
@@ -424,6 +424,12 @@ 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 },
+	/*
+	 * NEEDSWORK: Once the redirection to git-legacy-difftool.perl in
+	 * builtin/difftool.c has been removed, this entry should be changed to
+	 * RUN_SETUP | NEED_WORK_TREE
+	 */
+	{ "difftool", cmd_difftool },
 	{ "fast-export", cmd_fast_export, RUN_SETUP },
 	{ "fetch", cmd_fetch, RUN_SETUP },
 	{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 70a2de4..b6a6c30 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -23,6 +23,8 @@ prompt_given ()
 	test "$prompt" = "Launch 'test-tool' [Y/n]? branch"
 }
 
+# NEEDSWORK: lose all the PERL prereqs once legacy-difftool is retired.
+
 # Create a file on master and change it on branch
 test_expect_success PERL 'setup' '
 	echo master >file &&
-- 
2.10.1.583.g721a9e0



  reply	other threads:[~2016-11-24 20:55 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-22 17:01 [PATCH 0/2] Show Git Mailing List: a builtin difftool Johannes Schindelin
2016-11-22 17:01 ` [PATCH 1/2] difftool: add the builtin Johannes Schindelin
2016-11-23  8:08   ` David Aguilar
2016-11-23 11:34     ` Johannes Schindelin
2016-11-22 17:01 ` [PATCH 2/2] difftool: add a feature flag for the builtin vs scripted version Johannes Schindelin
2016-11-23 14:51   ` Dennis Kaarsemaker
2016-11-23 17:29     ` Johannes Schindelin
2016-11-23 17:40       ` Junio C Hamano
2016-11-23 18:18         ` Junio C Hamano
2016-11-23 19:55           ` Johannes Schindelin
2016-11-23 20:04             ` Junio C Hamano
2016-11-23 22:01       ` Johannes Schindelin
2016-11-23 22:03 ` [PATCH v2 0/1] Show Git Mailing List: a builtin difftool Johannes Schindelin
2016-11-23 22:03   ` [PATCH v2 1/1] difftool: add the builtin Johannes Schindelin
2016-11-23 22:25     ` Junio C Hamano
2016-11-23 22:30       ` Junio C Hamano
2016-11-24 10:38         ` Johannes Schindelin
2016-11-24 20:55   ` [PATCH v3 0/2] Show Git Mailing List: a builtin difftool Johannes Schindelin
2016-11-24 20:55     ` Johannes Schindelin [this message]
2016-11-24 21:08       ` [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin Jeff King
2016-11-24 21:56         ` Johannes Schindelin
2016-11-25  3:18           ` Jeff King
2016-11-25 11:05             ` Johannes Schindelin
2016-11-25 17:19               ` Jeff King
2016-11-25 17:41                 ` Johannes Schindelin
2016-11-25 17:47                   ` Jeff King
2016-11-26 12:22                     ` Johannes Schindelin
2016-11-26 16:19                       ` Jeff King
2016-11-26 13:01                         ` Johannes Schindelin
2016-11-27 16:50                           ` Jeff King
2016-11-28 17:06                             ` Junio C Hamano
2016-11-28 17:34                               ` Johannes Schindelin
2016-11-28 19:27                                 ` Junio C Hamano
2016-11-29 20:36                                   ` Johannes Schindelin
2016-11-29 20:49                                     ` Jeff King
2016-11-30 12:30                                       ` Johannes Schindelin
2016-11-30 12:35                                         ` Jeff King
2016-11-29 20:55                                     ` Junio C Hamano
2016-11-30 12:30                                       ` Johannes Schindelin
2016-12-01 23:33                                         ` Junio C Hamano
2016-12-05 10:36                                           ` Johannes Schindelin
2016-12-05 18:37                                             ` Junio C Hamano
2016-12-06 13:16                                               ` Johannes Schindelin
2016-12-06 13:36                                                 ` Jeff King
2016-12-06 14:48                                                   ` Johannes Schindelin
2016-12-06 15:09                                                     ` Jeff King
2016-12-06 18:22                                                       ` Stefan Beller
2016-12-06 18:35                                                         ` Jeff King
2017-01-18 22:38                                                           ` Brandon Williams
2016-11-30 16:02                                 ` Jakub Narębski
2016-11-30 18:39                                   ` Junio C Hamano
2016-11-24 20:55     ` [PATCH v3 2/2] difftool: implement the functionality in the builtin Johannes Schindelin
2016-11-25 21:24       ` Jakub Narębski
2016-11-27 11:10         ` Johannes Schindelin
2016-11-27 11:20           ` Jakub Narębski
2017-01-02 16:16     ` [PATCH v4 0/4] Show Git Mailing List: a builtin difftool Johannes Schindelin
2017-01-02 16:22       ` [PATCH v4 1/4] Avoid Coverity warning about unfree()d git_exec_path() Johannes Schindelin
2017-01-03 20:11         ` Stefan Beller
2017-01-03 21:33           ` Johannes Schindelin
2017-01-04 18:09             ` Stefan Beller
2017-01-04  1:13           ` Jeff King
2017-01-09  1:25           ` Junio C Hamano
2017-01-09  6:00             ` Jeff King
2017-01-09  7:49             ` Johannes Schindelin
2017-01-09 19:21             ` Stefan Beller
2017-01-02 16:22       ` [PATCH v4 2/4] difftool: add a skeleton for the upcoming builtin Johannes Schindelin
2017-01-02 16:22       ` [PATCH v4 3/4] difftool: implement the functionality in the builtin Johannes Schindelin
2017-01-02 16:24       ` [PATCH v4 4/4] t7800: run both builtin and scripted difftool, for now Johannes Schindelin
2017-01-09  1:38         ` Junio C Hamano
2017-01-09  7:56           ` Johannes Schindelin
2017-01-09  9:46             ` Junio C Hamano
2017-01-17 15:54       ` [PATCH v5 0/3] Turn the difftool into a builtin Johannes Schindelin
2017-01-17 15:54         ` [PATCH v5 1/3] difftool: add a skeleton for the upcoming builtin Johannes Schindelin
2017-01-17 15:55         ` [PATCH v5 2/3] difftool: implement the functionality in the builtin Johannes Schindelin
2017-01-17 15:55         ` [PATCH v5 3/3] Retire the scripted difftool Johannes Schindelin
2017-01-17 21:46           ` Junio C Hamano
2017-01-18 12:33             ` Johannes Schindelin
2017-01-18 19:15               ` Junio C Hamano
2017-01-19 16:30                 ` Johannes Schindelin
2017-01-19 17:56                   ` Junio C Hamano
2017-01-19 20:32                     ` Johannes Schindelin
2017-01-17 21:31         ` [PATCH v5 0/3] Turn the difftool into a builtin Junio C Hamano
2017-01-19 20:30         ` [PATCH v6 " Johannes Schindelin
2017-01-19 20:30           ` [PATCH v6 1/3] difftool: add a skeleton for the upcoming builtin Johannes Schindelin
2017-01-19 20:30           ` [PATCH v6 2/3] difftool: implement the functionality in the builtin Johannes Schindelin
2017-01-19 20:30           ` [PATCH v6 3/3] Retire the scripted difftool Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3a10fd72cf336743891d6bc9a003cfeb5c3f0063.1480019834.git.johannes.schindelin@gmx.de \
    --to=johannes.schindelin@gmx.de \
    --cc=davvid@gmail.com \
    --cc=dennis@kaarsemaker.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.