[PATCH] Introduce git-supported-features for porcelain use

[PATCH] Introduce git-supported-features for porcelain use

[Shawn O. Pearce]

As a porcelain author I'm finding it difficult to keep track of
what features I can use in git-gui.  Newer versions of Git have
newer capabilities but they don't always immediately get newer
version numbers that I can easily test for.

This is a simple plumbing command that lets a porcelain ask the
plumbing for its capabilities, at which point the porcelain can
work around anything missing, or recommend to the user that they
upgrade their plumbing layer.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .gitignore                               |    1 +
 Documentation/git-supported-features.txt |   37 +++++++++++++++++
 Makefile                                 |    1 +
 builtin-supported-features.c             |   66 ++++++++++++++++++++++++++++++
 builtin.h                                |    1 +
 git.c                                    |    1 +
 t/t0000-basic.sh                         |   10 +++++
 7 files changed, 117 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/git-supported-features.txt
 create mode 100644 builtin-supported-features.c

diff --git a/.gitignore b/.gitignore
index 4dc0c39..6ff18d4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -126,6 +126,7 @@ git-ssh-push
 git-ssh-upload
 git-status
 git-stripspace
+git-supported-features
 git-svn
 git-svnimport
 git-symbolic-ref
diff --git a/Documentation/git-supported-features.txt b/Documentation/git-supported-features.txt
new file mode 100644
index 0000000..28e2c21
--- /dev/null
+++ b/Documentation/git-supported-features.txt
@@ -0,0 +1,37 @@
+git-supported-features(1)
+===================
+
+NAME
+----
+git-supported-features - Report capabilities of the plumbing
+
+SYNOPSIS
+--------
+'git-supported-features' [<name>]
+
+DESCRIPTION
+-----------
+Given no arguments this plumbing utility displays a list of feature
+strings, one per line, that this particular version of the Git
+plumbing supports.  These strings can be matched by higher level
+porcelain to determine what capabilities are available to its use.
+
+Given one argument (the name of the feature to test) the program
+exits with 0 if the feature is supported, and 1 if it is not.
+Other exit codes are undefined at this time.
+
+NOTES
+-----
+The exact list of feature strings is undefined by this manual page.
+
+Feature strings use a restricted character set, relying only on
+`a`-`z` (lowercase), `0`-`9` and `-` (hypen).  This restriction is
+intentional, as it makes parsing output in shell quite simple.
+
+Author
+------
+Written by Shawn O. Pearce <spearce@spearce.org>.
+
+GIT
+---
+Part of the gitlink:git[7] suite
diff --git a/Makefile b/Makefile
index 7527734..03ab471 100644
--- a/Makefile
+++ b/Makefile
@@ -372,6 +372,7 @@ BUILTIN_OBJS = \
 	builtin-shortlog.o \
 	builtin-show-branch.o \
 	builtin-stripspace.o \
+	builtin-supported-features.o \
 	builtin-symbolic-ref.o \
 	builtin-tar-tree.o \
 	builtin-unpack-objects.o \
diff --git a/builtin-supported-features.c b/builtin-supported-features.c
new file mode 100644
index 0000000..79007f4
--- /dev/null
+++ b/builtin-supported-features.c
@@ -0,0 +1,66 @@
+#include "builtin.h"
+
+static const char *features[] = {
+	"git-k",
+	"redirect-stderr",
+	"supported-features",
+};
+
+static int is_sane(const char *a)
+{
+	if (!*a || *a == '-')
+		return 0;
+	for (; *a; a++) {
+		if (! ((*a >= 'a' && *a <= 'z')
+		    || (*a >= '0' && *a <= '9')
+		    || *a == '-'))
+			return 0;
+	}
+	return 1;
+}
+
+static int sort_feature(const void *a_, const void *b_)
+{
+	const char *a = *((const char **)a_);
+	const char *b = *((const char **)b_);
+	return strcmp(a, b);
+}
+
+static void list_features()
+{
+	unsigned cnt = ARRAY_SIZE(features);
+	unsigned i;
+
+	qsort(features, cnt, sizeof(features[0]), sort_feature);
+	for (i = 0; i < cnt; i++) {
+		if (!is_sane(features[i]))
+			die("feature name \"%s\" is bogus", features[i]);
+		printf("%s\n", features[i]);
+	}
+}
+
+static int test_feature(const char *the_feature)
+{
+	unsigned cnt = ARRAY_SIZE(features);
+	unsigned i;
+
+	for (i = 0; i < cnt; i++) {
+		if (!strcmp(features[i], the_feature))
+			return 0;
+	}
+	return 1;
+}
+
+static const char supported_features_usage[] =
+"git-supported-features [<feature>]";
+
+int cmd_supported_features(int argc, const char **argv, const char *prefix)
+{
+	if (argc == 1)
+		list_features();
+	else if (argc == 2)
+		return test_feature(argv[1]);
+	else
+		usage(supported_features_usage);
+	return 0;
+}
diff --git a/builtin.h b/builtin.h
index 39290d1..f33432c 100644
--- a/builtin.h
+++ b/builtin.h
@@ -71,6 +71,7 @@ extern int cmd_shortlog(int argc, const char **argv, const char *prefix);
 extern int cmd_show(int argc, const char **argv, const char *prefix);
 extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
 extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
+extern int cmd_supported_features(int argc, const char **argv, const char *prefix);
 extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_tar_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_unpack_objects(int argc, const char **argv, const char *prefix);
diff --git a/git.c b/git.c
index 29b55a1..377fae9 100644
--- a/git.c
+++ b/git.c
@@ -284,6 +284,7 @@ static void handle_internal_command(int argc, const char **argv, char **envp)
 		{ "show-branch", cmd_show_branch, RUN_SETUP },
 		{ "show", cmd_show, RUN_SETUP | USE_PAGER },
 		{ "stripspace", cmd_stripspace },
+		{ "supported-features", cmd_supported_features },
 		{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
 		{ "tar-tree", cmd_tar_tree },
 		{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 186de70..7d0d9c9 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -31,6 +31,16 @@ fi
 . ./test-lib.sh
 
 ################################################################
+# git-supported-features should always work.
+
+test_expect_success \
+	'supported-features is functional' \
+	'git-supported-features'
+test_expect_success \
+	'supported-features recognizes itself' \
+	'git-supported-features supported-features'
+
+################################################################
 # git-init has been done in an empty repository.
 # make sure it is empty.
 
-- 
1.5.2.869.g6b3ba

Re: [PATCH] Introduce git-supported-features for porcelain use

[Alex Riesen]

On 5/30/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> As a porcelain author I'm finding it difficult to keep track of
> what features I can use in git-gui.  Newer versions of Git have
> newer capabilities but they don't always immediately get newer
> version numbers that I can easily test for.

git-version --features?

Re: [PATCH] Introduce git-supported-features for porcelain use

[Johannes Schindelin]

Hi,

On Wed, 30 May 2007, Alex Riesen wrote:

> On 5/30/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> > As a porcelain author I'm finding it difficult to keep track of
> > what features I can use in git-gui.  Newer versions of Git have
> > newer capabilities but they don't always immediately get newer
> > version numbers that I can easily test for.
> 
> git-version --features?

Melikes. (Even if it is not strictly separating between plumbing and 
porcelain; I consider git-version porcelain...)

Ciao,
Dscho

[PATCH] Introduce git version --list-features for porcelain use

[Shawn O. Pearce]

As a porcelain author I'm finding it difficult to keep track of
what features I can use in git-gui.  Newer versions of Git have
newer capabilities but they don't always immediately get newer
version numbers that I can easily test for.

This is a simple plumbing option that lets a porcelain ask the
plumbing for its capabilities, at which point the porcelain can
work around anything missing, or recommend to the user that they
upgrade their plumbing layer.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

	Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
	>
	> On Wed, 30 May 2007, Alex Riesen wrote:
	>
	> > git-version --features?
	>
	> Melikes.

	Good?

 help.c           |   78 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 t/t0000-basic.sh |   16 +++++++++++
 2 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/help.c b/help.c
index 6a9af4d..9d9cccd 100644
--- a/help.c
+++ b/help.c
@@ -9,6 +9,12 @@
 #include "common-cmds.h"
 #include <sys/ioctl.h>
 
+static const char *supported_features[] = {
+	"git-k",
+	"list-features",
+	"redirect-stderr",
+};
+
 /* most GUI terminals set COLUMNS (although some don't export it) */
 static int term_columns(void)
 {
@@ -190,10 +196,78 @@ void help_unknown_cmd(const char *cmd)
 	exit(1);
 }
 
+static int is_feature_name_sane(const char *a)
+{
+	if (!*a || *a == '-')
+		return 0;
+	for (; *a; a++) {
+		if (! ((*a >= 'a' && *a <= 'z')
+		    || (*a >= '0' && *a <= '9')
+		    || *a == '-'))
+			return 0;
+	}
+	return 1;
+}
+
+static int cmp_feature(const void *a_, const void *b_)
+{
+	const char *a = *((const char **)a_);
+	const char *b = *((const char **)b_);
+	return strcmp(a, b);
+}
+
+static void list_features()
+{
+	unsigned cnt = ARRAY_SIZE(supported_features);
+	unsigned i;
+
+	qsort(supported_features, cnt,
+		sizeof(supported_features[0]), cmp_feature);
+	for (i = 0; i < cnt; i++) {
+		const char *f = supported_features[i];
+		if (!is_feature_name_sane(f))
+			die("feature name \"%s\" is bogus", f);
+		printf("%s\n", f);
+	}
+}
+
+static int supports_feature(const char *the_feature)
+{
+	unsigned cnt = ARRAY_SIZE(supported_features);
+	unsigned i;
+
+	for (i = 0; i < cnt; i++) {
+		if (!strcmp(supported_features[i], the_feature))
+			return 0;
+	}
+	return 1;
+}
+
+static const char version_usage[] =
+"git-version [(--list-features | --supports-feature=<name>*)]";
+
 int cmd_version(int argc, const char **argv, const char *prefix)
 {
-	printf("git version %s\n", git_version_string);
-	return 0;
+	int i, ret = 0, list = 0, test = 0;
+
+	for (i = 1; i < argc; i++) {
+		const char *arg = argv[i];
+		if (!strcmp(arg, "--list-features"))
+			list = 1;
+		else if (!prefixcmp(arg, "--supports-feature=")) {
+			test = 1;
+			ret |= supports_feature(arg + 19);
+		} else
+			usage(version_usage);
+	}
+
+	if (list && test)
+		die("cannot use both --list-features and --supports-feature");
+	if (list)
+		list_features();
+	if (!list && !test)
+		printf("git version %s\n", git_version_string);
+	return ret;
 }
 
 int cmd_help(int argc, const char **argv, const char *prefix)
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 186de70..874bb04 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -31,6 +31,22 @@ fi
 . ./test-lib.sh
 
 ################################################################
+# git version
+
+test_expect_success \
+	'git version is functional' \
+	'git version'
+test_expect_success \
+	'git version --list-features' \
+	'git version --list-features'
+test_expect_success \
+	'feature "list-features" is supported' \
+	'git version --supports-feature=list-features'
+test_expect_failure \
+	'feature "THISNEVERWILLBEAGITFEATURE" is not supported' \
+	'git version --supports-feature=THISNEVERWILLBEAGITFEATURE'
+
+################################################################
 # git-init has been done in an empty repository.
 # make sure it is empty.
 
-- 
1.5.2.869.g6b3ba

Re: [PATCH] Introduce git version --list-features for porcelain use

[Johannes Schindelin]

Hi,

On Wed, 30 May 2007, Shawn O. Pearce wrote:

> 	Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 	>
> 	> On Wed, 30 May 2007, Alex Riesen wrote:
> 	>
> 	> > git-version --features?
> 	>
> 	> Melikes.
> 
> 	Good?

Yep. No more complaints from my side.

Ciao,
Dscho

Re: [PATCH] Introduce git version --list-features for porcelain use

[Alex Riesen]

On 5/31/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Wed, 30 May 2007, Alex Riesen wrote:
> >
> > > git-version --features?
> >
> > Melikes.
>
> Good?
>

Was just a suggestion. Good.

Re: [PATCH] Introduce git version --list-features for porcelain use

[Shawn O. Pearce]

Alex Riesen <raa.lkml@gmail.com> wrote:
> On 5/31/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> >Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> >>
> >> On Wed, 30 May 2007, Alex Riesen wrote:
> >>
> >> > git-version --features?
> >>
> >> Melikes.
> >
> >Good?
> >
> 
> Was just a suggestion. Good.

Well, a quick poll on IRC seemed to get like 5 additional votes for
this being a switch on git-version.  That sealed the deal.  I wasn't
committed one way or the other.  I just wanted the functionality...

Also votes from you and Dscho carry a little weight around here.  :-)

-- 
Shawn.

Re: [PATCH] Introduce git version --list-features for porcelain use

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> As a porcelain author I'm finding it difficult to keep track of
> what features I can use in git-gui.  Newer versions of Git have
> newer capabilities but they don't always immediately get newer
> version numbers that I can easily test for.
>
> This is a simple plumbing option that lets a porcelain ask the
> plumbing for its capabilities, at which point the porcelain can
> work around anything missing, or recommend to the user that they
> upgrade their plumbing layer.
>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
>
> 	Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 	>
> 	> On Wed, 30 May 2007, Alex Riesen wrote:
> 	>
> 	> > git-version --features?
> 	>
> 	> Melikes.
>
> 	Good?

Hmmm.  I am not sure if you want list-features in the features
list -- how are you going to test for it?

Also I still do not understand why you want redirect-stderr.
Are you writing for a shell-less environment?

Re: [PATCH] Introduce git version --list-features for porcelain use

[Shawn O. Pearce]

Junio C Hamano <junkio@cox.net> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > As a porcelain author I'm finding it difficult to keep track of
> > what features I can use in git-gui.  Newer versions of Git have
> > newer capabilities but they don't always immediately get newer
> > version numbers that I can easily test for.
> >
> > This is a simple plumbing option that lets a porcelain ask the
> > plumbing for its capabilities, at which point the porcelain can
> > work around anything missing, or recommend to the user that they
> > upgrade their plumbing layer.
> >
> > Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> > ---
> >
> > 	Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > 	>
> > 	> On Wed, 30 May 2007, Alex Riesen wrote:
> > 	>
> > 	> > git-version --features?
> > 	>
> > 	> Melikes.
> >
> > 	Good?
> 
> Hmmm.  I am not sure if you want list-features in the features
> list -- how are you going to test for it?

Yes, its a recursive definition.  But its also a feature that won't
ever be removed; if list-features is here then the list-features
feature is also here.  Makes the test in t0000 a lot easier, and
its not a huge deal to say "yes, the feature that i implement is
here too".  ;-)
 
> Also I still do not understand why you want redirect-stderr.
> Are you writing for a shell-less environment?

The redirect-stderr thing grew out of the MinGW port camp.
Apparently they cannot use (or its hard to use) an important little
tool called `cat` over there.

Why cat?  Tcl is so horribly broken that to get data for both stdout
and stderr through a pipe I have to do something sick like:

	git fetch 2>&1 | cat

because in Tcl its actually:

	set rdr [open "| git fetch |& cat" r]

The |& means 2>&1| in normal shell.  But that means I have to have
a process after it to receive the data.  Normally that's cat.
But MinGW doesn't have cat.  (Nor do they have dog, but neither
does Linux...).  So I need a way to redirect output.

Dscho's patch to git.c to give me `git --redirect-stderr` is quite
simple, and makes my life in git-gui easier.  I can just require
that to use git-gui on MinGW you must have the 'redirect-stderr'
feature supported in your plumbing layer.  On non-MinGW systems I
can fallback to "&| cat" if its not supported.

-- 
Shawn.

Re: [PATCH] Introduce git version --list-features for porcelain use

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Why cat?  Tcl is so horribly broken that to get data for both stdout
> and stderr through a pipe I have to do something sick like:
>
> 	git fetch 2>&1 | cat
>
> because in Tcl its actually:
>
> 	set rdr [open "| git fetch |& cat" r]
>
> The |& means 2>&1| in normal shell.  But that means I have to have
> a process after it to receive the data.  Normally that's cat.
> But MinGW doesn't have cat.  (Nor do they have dog, but neither
> does Linux...).  So I need a way to redirect output.

Wait a minute.  Who interprets |& in the above?  Isn't it a
shell?

That's why I asked if you are writing for shell-less
environment.  If you are _not_, can't you do something like this
instead?

	set rdr [open "| sh -c 'git fetch 2>&1'" r]

that is, instead of running a command called "git", you invoke a
command called "sh" with two parameters, the second parameter
being a tad long string that happens to have three letter
sequence '2>&1' in it.

Re: [PATCH] Introduce git version --list-features for porcelain use

[Shawn O. Pearce]

Junio C Hamano <junkio@cox.net> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > Why cat?  Tcl is so horribly broken that to get data for both stdout
> > and stderr through a pipe I have to do something sick like:
> >
> > 	git fetch 2>&1 | cat
> >
> > because in Tcl its actually:
> >
> > 	set rdr [open "| git fetch |& cat" r]
> >
> > The |& means 2>&1| in normal shell.  But that means I have to have
> > a process after it to receive the data.  Normally that's cat.
> > But MinGW doesn't have cat.  (Nor do they have dog, but neither
> > does Linux...).  So I need a way to redirect output.
> 
> Wait a minute.  Who interprets |& in the above?  Isn't it a
> shell?

My understanding was it is Tcl itself.  My $SHELL doesn't understand
it:

  $ echo hi |& cat
  -bash: syntax error near unexpected token `&'

> That's why I asked if you are writing for shell-less
> environment.  If you are _not_, can't you do something like this
> instead?

Ideally with MinGW we wouldn't need a UNIX shell to get things
working in git-gui.  But we have to have one for git-merge for
example, as git-gui doesn't have a builtin Grand Unified Merge
Driver.  Also for git-fetch, which is one of the prime uses of this
cat redirect thing.  ;-)

> 	set rdr [open "| sh -c 'git fetch 2>&1'" r]

Yeah, I'm already doing that sh -c trick for a different reason in
another context.  I may have to do just that here too.

-- 
Shawn.

Re: [PATCH] Introduce git version --list-features for porcelain use

[Johannes Sixt]

"Shawn O. Pearce" wrote:
> Why cat?  Tcl is so horribly broken that to get data for both stdout
> and stderr through a pipe I have to do something sick like:
> 
>         git fetch 2>&1 | cat
> 
> because in Tcl its actually:
> 
>         set rdr [open "| git fetch |& cat" r]
> 
> The |& means 2>&1| in normal shell.  But that means I have to have
> a process after it to receive the data.  Normally that's cat.
> But MinGW doesn't have cat.  (Nor do they have dog, but neither
> does Linux...).  So I need a way to redirect output.

Are you planning for the future? Currently, the MinGW port works only
with MSYS installed and $MSYS/bin in your PATH because the shell scripts
need an assorted set of POSIX tools. MSYS does ship 'cat', of course.

-- Hannes

[PATCH] Introduce git version --list-features for porcelain use

[Shawn O. Pearce]

As a porcelain author I'm finding it difficult to keep track of
what features I can use in git-gui.  Newer versions of Git have
newer capabilities but they don't always immediately get newer
version numbers that I can easily test for.

This is a simple plumbing option that lets a porcelain ask the
plumbing for its capabilities, at which point the porcelain can
work around anything missing, or recommend to the user that they
upgrade their plumbing layer.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 Because 'maint' does not support `git-blame -w` I can't apply your
 recent (and insanely good idea) 'git-gui: use "blame -w -C -C" for
 "where did it come from, originally?"' without something like the
 following, so I know that `git blame -w -C -C` will be accepted
 by the underlying plumbing.  ;-)

 help.c           |   77 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 t/t0000-basic.sh |   16 +++++++++++
 2 files changed, 91 insertions(+), 2 deletions(-)

diff --git a/help.c b/help.c
index 1cd33ec..9f24a0f 100644
--- a/help.c
+++ b/help.c
@@ -9,6 +9,11 @@
 #include "common-cmds.h"
 #include <sys/ioctl.h>
 
+static const char *supported_features[] = {
+	"blame-ignore-whitespace",
+	"list-features",
+};
+
 /* most GUI terminals set COLUMNS (although some don't export it) */
 static int term_columns(void)
 {
@@ -190,10 +195,78 @@ void help_unknown_cmd(const char *cmd)
 	exit(1);
 }
 
+static int is_feature_name_sane(const char *a)
+{
+	if (!*a || *a == '-')
+		return 0;
+	for (; *a; a++) {
+		if (! ((*a >= 'a' && *a <= 'z')
+		    || (*a >= '0' && *a <= '9')
+		    || *a == '-'))
+			return 0;
+	}
+	return 1;
+}
+
+static int cmp_feature(const void *a_, const void *b_)
+{
+	const char *a = *((const char **)a_);
+	const char *b = *((const char **)b_);
+	return strcmp(a, b);
+}
+
+static void list_features()
+{
+	unsigned cnt = ARRAY_SIZE(supported_features);
+	unsigned i;
+
+	qsort(supported_features, cnt,
+		sizeof(supported_features[0]), cmp_feature);
+	for (i = 0; i < cnt; i++) {
+		const char *f = supported_features[i];
+		if (!is_feature_name_sane(f))
+			die("feature name \"%s\" is bogus", f);
+		printf("%s\n", f);
+	}
+}
+
+static int supports_feature(const char *the_feature)
+{
+	unsigned cnt = ARRAY_SIZE(supported_features);
+	unsigned i;
+
+	for (i = 0; i < cnt; i++) {
+		if (!strcmp(supported_features[i], the_feature))
+			return 0;
+	}
+	return 1;
+}
+
+static const char version_usage[] =
+"git-version [(--list-features | --supports-feature=<name>*)]";
+
 int cmd_version(int argc, const char **argv, const char *prefix)
 {
-	printf("git version %s\n", git_version_string);
-	return 0;
+	int i, ret = 0, list = 0, test = 0;
+
+	for (i = 1; i < argc; i++) {
+		const char *arg = argv[i];
+		if (!strcmp(arg, "--list-features"))
+			list = 1;
+		else if (!prefixcmp(arg, "--supports-feature=")) {
+			test = 1;
+			ret |= supports_feature(arg + 19);
+		} else
+			usage(version_usage);
+	}
+
+	if (list && test)
+		die("cannot use both --list-features and --supports-feature");
+	if (list)
+		list_features();
+	if (!list && !test)
+		printf("git version %s\n", git_version_string);
+	return ret;
 }
 
 int cmd_help(int argc, const char **argv, const char *prefix)
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 8bfe832..9fc5824 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -31,6 +31,22 @@ fi
 . ./test-lib.sh
 
 ################################################################
+# git version
+
+test_expect_success \
+	'git version is functional' \
+	'git version'
+test_expect_success \
+	'git version --list-features' \
+	'git version --list-features'
+test_expect_success \
+	'feature "list-features" is supported' \
+	'git version --supports-feature=list-features'
+test_expect_failure \
+	'feature "THISNEVERWILLBEAGITFEATURE" is not supported' \
+	'git version --supports-feature=THISNEVERWILLBEAGITFEATURE'
+
+################################################################
 # git-init has been done in an empty repository.
 # make sure it is empty.
 
-- 
1.5.2.2.1050.g51a8b

Re: [PATCH] Introduce git version --list-features for porcelain use

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> As a porcelain author I'm finding it difficult to keep track of
> what features I can use in git-gui.  Newer versions of Git have
> newer capabilities but they don't always immediately get newer
> version numbers that I can easily test for.

Two and half comments, and a discussion.

> +static const char *supported_features[] = {
> +	"blame-ignore-whitespace",
> +	"list-features",
> +};
> +
>  /* most GUI terminals set COLUMNS (although some don't export it) */
>  static int term_columns(void)
>  {
> @@ -190,10 +195,78 @@ void help_unknown_cmd(const char *cmd)
>  	exit(1);
>  }
>  
> +static int is_feature_name_sane(const char *a)
> +{
> +	if (!*a || *a == '-')
> +		return 0;
> +	for (; *a; a++) {
> +		if (! ((*a >= 'a' && *a <= 'z')
> +		    || (*a >= '0' && *a <= '9')
> +		    || *a == '-'))
> +			return 0;
> +	}
> +	return 1;
> +}
> +

> +static int cmp_feature(const void *a_, const void *b_)
> +{
> +	const char *a = *((const char **)a_);
> +	const char *b = *((const char **)b_);
> +	return strcmp(a, b);
> +}
> +
> +static void list_features()
> +{
> +	unsigned cnt = ARRAY_SIZE(supported_features);
> +	unsigned i;
> +
> +	qsort(supported_features, cnt,
> +		sizeof(supported_features[0]), cmp_feature);
> ...
> +}

Unless we are talking about dynamically extensible feature list
(eh, dll, anybody?), it might be easier to keep (1) the list
sorted, and (2) free of insane feature name in
supported_features[] array at the source level.  Then you can
lose that is_feature_name_sane() function.

> +static int supports_feature(const char *the_feature)
> +{
> +	unsigned cnt = ARRAY_SIZE(supported_features);
> +	unsigned i;
> +
> +	for (i = 0; i < cnt; i++) {
> +		if (!strcmp(supported_features[i], the_feature))
> +			return 0;
> +	}
> +	return 1;
> +}

And you can  perform a bsearch here. instead of linear.

> +test_expect_failure \
> +	'feature "THISNEVERWILLBEAGITFEATURE" is not supported' \
> +	'git version --supports-feature=THISNEVERWILLBEAGITFEATURE'

I would expect that THISNEW... will get complaint saying "That
is not a valid feature name, as it has uppercase", from a
version that has is_feature_name_sane() function.

I suspect that this patch is meant for my 'maint' (and
1.5.2.3).  Or is it for my 'master'?  What's your plan to handle
transition?

For example, if this appears on 1.5.2.3, then
supported_features[] should not have blame-ignore-whitespace,
unless we are talking about cherry-picking, and I honestly do
not think "blame -w" deserves to go to the maintenance only
series.  On the other hand, --list-features could go to 'maint'
under 'future prooofing' category, I guess.

If this is meant to be only for 1.5.3 and later, then you know
that "blame -w" is available as well, so the fact you can do
"git version --list-features" alone tells you that you can use
"blame -w", among other many things, such as "diff -C -C"
instead of --find-copies-harder.

Where does the above discussion lead us?  It essentially means,
in either case, "blame-ignore-whitespace" should not be in that
supported_features[] array.

Re: [PATCH] Introduce git version --list-features for porcelain use

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> Unless we are talking about dynamically extensible feature list
> (eh, dll, anybody?), it might be easier to keep (1) the list
> sorted, and (2) free of insane feature name in
> supported_features[] array at the source level.  Then you can
> lose that is_feature_name_sane() function.

Yes, this is true.  But I'm being overly defensive here.  If the
list does get out of order we force it back in --list-features just
to be consistent in output, and t0000 will fail if any item in the
list is insane.

If you want to be less defensive, OK, it would also reduce the
patch size a bit, but may allow someone to add an insane item to
the list.

> > +static int supports_feature(const char *the_feature)
> 
> And you can  perform a bsearch here. instead of linear.

Sure.  But I'm actually expecting more for Porcelain that cares
to run `git version --list-features` and store that output into
its own internal table, then consult that table rather than the
--supports-feature option.  I figured the bsearch would take more
code than the linear, and probably wasn't worth it in this dark
little corner of Git.

> > +test_expect_failure \
> > +	'feature "THISNEVERWILLBEAGITFEATURE" is not supported' \
> > +	'git version --supports-feature=THISNEVERWILLBEAGITFEATURE'
> 
> I would expect that THISNEW... will get complaint saying "That
> is not a valid feature name, as it has uppercase", from a
> version that has is_feature_name_sane() function.

Eh, probably true.  I guess I should make that lowercase so it is
actually a sane name, just one so unlikely that we will never use it.
 
> I suspect that this patch is meant for my 'maint' (and
> 1.5.2.3).  Or is it for my 'master'?  What's your plan to handle
> transition?

Eh, sorry, I should have mentioned that.  I meant for you to apply
this to your master, where `git blame -w` is already present.

Currently with next I get:

  $ git version --list-features
  git version 1.5.2.2.1050.g51a8b

So what I was planning on doing in git-gui was running that, and if
I just get one line with `git version` on it (which is an insane
feature name btw) then I know --list-features is not supported,
and neither is any other feature that I might be looking for from
the --list-features command.

On the other hand if --list-features is actually supported instead
I'll get back at least a line with "list-features" on it, and I
won't get a line with "git version" on it (as that is an insane
feature name).

I guess it is good that our cmd_version() routine never actually
checked to see if its argv[] matched what it expects to receive
(nothing up until now).  :)

Now I'm fine with you applying this back into a 1.5.2.x maint
release, but because of the cmd_version() behavior I don't think
that is really required.  Nor am I looking for you to cherry-pick
back the `git blame -w` feature.
 
> If this is meant to be only for 1.5.3 and later, then you know
> that "blame -w" is available as well, so the fact you can do
> "git version --list-features" alone tells you that you can use
> "blame -w", among other many things, such as "diff -C -C"
> instead of --find-copies-harder.

Yes, that is true.
 
> Where does the above discussion lead us?  It essentially means,
> in either case, "blame-ignore-whitespace" should not be in that
> supported_features[] array.

Hmm.  So assuming that is true, under what rule do you propose we
add new features to that array in the future?  And what name(s)
do we give to them?

For a recent hypothetical example, assume this patch was already
applied in your master by the time Linus submitted his useful hack
`git log --follow`.  How would we signal in the supported_features[]
that log would understand --follow for a single file pathname?

-- 
Shawn.

Re: [PATCH] Introduce git version --list-features for porcelain use

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> If you want to be less defensive, OK, it would also reduce the
> patch size a bit, but may allow someone to add an insane item to
> the list.

If we were to go this route (which I am not convinced is the
right direction yet---using "git version" to check the version
and say "1.5.3 has blame -w" might be easier to manage, and that
needs to be maintained only by the tools that care about the
particular feature, "blame -w"), we will potentially be talking
about hundreds of features in the long run.  I would rather want
to automate this even further, as keeping the feature list and
features together would become huge maintenance headache.

So, first of all, I would want the list of features in
supported_features[] to come from a separate file, whose sole
purpose is to have that list.  I would even suggest making it an
generated file, which depends on the source files (not
necessarily limited to C files), and built from within the build
procedure.

Whenever you add a new command line option, you would add a
greppable comment near it, something like...

 		else if (!strcmp("-s", arg))
 			output_option |= OUTPUT_NO_AUTHOR;
+		/* FEATURE[[[blame-ignore-whitespace]]] */
+		else if (!strcmp("-w", arg))
+			xdl_opts |= XDF_IGNORE_WHITESPACE;
 		else if (!strcmp("-S", arg) && ++i < argc)
 			revs_file = argv[i];

and have Makefile extract them, sort them and build the meat of
supported_features[] array automatically.

Unless we have something like that, I have to worry about merge
conflicts every time a new topic graduates to 'master'.

>> > +static int supports_feature(const char *the_feature)
>> 
>> And you can  perform a bsearch here. instead of linear.
>
> Sure.  But I'm actually expecting more for Porcelain that cares
> to run `git version --list-features` and store that output into
> its own internal table,

Ok, that is probably a sane assumption.

> ....  I figured the bsearch would take more
> code than the linear, and probably wasn't worth it in this dark
> little corner of Git.

Huh?  Don't you already have compar() function that bsearch(3)
can take?

>> I suspect that this patch is meant for my 'maint' (and
>> 1.5.2.3).  Or is it for my 'master'?  What's your plan to handle
>> transition?
>
> Eh, sorry, I should have mentioned that.  I meant for you to apply
> this to your master, where `git blame -w` is already present.
>
> Currently with next I get:
>
>   $ git version --list-features
>   git version 1.5.2.2.1050.g51a8b
>
> So what I was planning on doing in git-gui was running that, and if
> I just get one line with `git version` on it (which is an insane
> feature name btw) then I know --list-features is not supported,
> and neither is any other feature that I might be looking for from
> the --list-features command.
>
> On the other hand if --list-features is actually supported instead
> I'll get back at least a line with "list-features" on it, and I
> won't get a line with "git version" on it (as that is an insane
> feature name).

The explaination why "futureproofing" is unnecessary for 'maint'
is good, but that does not necessarily mean --list-featues would
need to list _all_ the features in the version that supports
that.  "blame -w" is in 'master' already, for example.  Is
"have-mergetool" a feature?  Yes, it is.  But you already know
the command exists if "git version" knows --list-features.

> Hmm.  So assuming that is true, under what rule do you propose we
> add new features to that array in the future?  And what name(s)
> do we give to them?

Assuming if this --list-features is a viable approach in the
long run, my gut feeling of relative order of things is:

 (1) We prepare rock-solid runtime with an _EMPTY_ feature array
     in 'master'.

 (2) We add automated feature array management in 'master', as
     outlined above.

 (3) Merge the above two to 'next'.

 (4) Update all existing topics, including the ones that are
     already in 'next', with the feature mark-up in comment
     (i.e. "FEATURE[[[feature-name]]]").

After (3) happens, any new topics that introduce a new feature
should be accompanied with the feature mark-up.

When an update appears on 'master' _after_ (1) and (2) happens
_but_ Porcelain writers like yourself do not realize that the
feature would be _very_ helpful for their Porcelains, and would
want to be able to tell if it exists via --list-features, long
after we start to support the feature, we might retroactively
have to add the feature mark-up.  There will be gap between the
commit that actually introduce and make the feature usable, and
the commit that teaches "git version" to talk about it in such a
case.  But I think that should be Ok.

I do not think we want to keep track of every little "features"
if no Porcelain cares about it.

Here is a list of potential "feature names", from the latest
"What's cooking" message.

 log-unlimited-message
 log-follow
 oneline-first-paragraph
 svn-merge-local
 work-tree-environ
 filter-branch

Re: [PATCH] Introduce git version --list-features for porcelain use

[Johannes Schindelin]

Hi,

On Thu, 21 Jun 2007, Junio C Hamano wrote:

> [...] using "git version" to check the version and say "1.5.3 has blame 
> -w" > might be easier to manage [ than list-features maintenance ...]

We do something similar already with curl features. So this sounds 
sensible to me. Shawn?

Ciao,
Dscho