threads / patch / 10404

patch"git help" and "git help -a" shouldn't exit(1) unless they error

Subject: [PATCH] "git help" and "git help -a" shouldn't exit(1) unless they error

## tl;dr

4 messages between Oct 21, 2007 and Oct 22, 2007. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Scott R Parish· Oct 21, 2007, 21:47 UTC · lore

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 help.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Show changes to help.c +2 −2

diff --git a/help.c b/help.c
index 1cd33ec..b0d2dd4 100644
--- a/help.c
+++ b/help.c
@@ -204,14 +204,14 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	if (!help_cmd) {
 		printf("usage: %s\n\n", git_usage_string);
 		list_common_cmds_help();
-		exit(1);
+		exit(0);
 	}
 
 	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
 		printf("usage: %s\n\n", git_usage_string);
 		if(exec_path)
 			list_commands(exec_path, "git-*");
-		exit(1);
+		exit(0);
 	}
 
 	else

-- 
1.5.3.4.209.g5d1ce-dirty

Shawn O. Pearce· Oct 22, 2007, 05:47 UTC · re: Scott R Parish · lore

Re: [PATCH] "git help" and "git help -a" shouldn't exit(1) unless they error

Scott R Parish <srp@srparish.net> wrote:

Show 11 quoted lines

> diff --git a/help.c b/help.c
> index 1cd33ec..b0d2dd4 100644
> --- a/help.c
> +++ b/help.c
> @@ -204,14 +204,14 @@ int cmd_help(int argc, const char **argv, const char *prefix)
>  	if (!help_cmd) {
>  		printf("usage: %s\n\n", git_usage_string);
>  		list_common_cmds_help();
> -		exit(1);
> +		exit(0);
>  	}

Although it seems simple on the surface this patch breaks the test suite:

	$ make test
	make -C t/ all
	make[1]: Entering directory `/home/spearce/mygit/t'
	*** t0000-basic.sh ***
	You do not seem to have built git yet.

The issue here is t0000-basic.sh runs "../git" and tests that the exit status is 1. If it isn't (the patch above makes it 0) we just abort the test suite entirely.

I think its correct for "git help" to exit 0, and also for "git
help checkout" or "git checkout --help" to exit 0, but "git" by
itself with no subcommand should exit with an error, it requires a
subcommand to continue.  So some sort of change is needed in git.c
to handle this special no subcommand condition.

-- 
Shawn.

Scott Parish· Oct 22, 2007, 06:19 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] "git help" and "git help -a" shouldn't exit(1) unless they error

On Mon, Oct 22, 2007 at 01:47:41AM -0400, Shawn O. Pearce wrote:

> The issue here is t0000-basic.sh runs "../git" and tests that the
> exit status is 1.  If it isn't (the patch above makes it 0) we just
> abort the test suite entirely.

Shoot, i hadn't realized i had effected the "git" case. I'll look into this further.

By the way, should i expect all the tests to pass from the unmodified public HEAD? (they don't for me)

sRp

-- 
Scott Parish
http://srparish.net/

Shawn O. Pearce· Oct 22, 2007, 06:37 UTC · re: Scott Parish · lore

Re: [PATCH] "git help" and "git help -a" shouldn't exit(1) unless they error

Scott Parish <sRp@srparish.net> wrote:

Show 11 quoted lines

> On Mon, Oct 22, 2007 at 01:47:41AM -0400, Shawn O. Pearce wrote:
> 
> > The issue here is t0000-basic.sh runs "../git" and tests that the
> > exit status is 1.  If it isn't (the patch above makes it 0) we just
> > abort the test suite entirely.
> 
> Shoot, i hadn't realized i had effected the "git" case. I'll
> look into this further.
> 
> By the way, should i expect all the tests to pass from the unmodified
> public HEAD? (they don't for me)

Yes. I only push maint, master and next if they pass all tests. If something doesn't pass I rewind the branch until it does (of course I only rewind back to what I've previously published).

I do however push a broken pu. Because individual topics in there may be valid, but one or two may also be broken. The pu branch is meant to be a place to obtain a specific topic of interest from so you can work further on it, or test it. Like tonight. pu compiles but doesn't pass the tests.

So if you are seeing one or more tests fail please go run that specific test(s) with "-i -v" and either come up with a fix for the test, or at least post the output of that to the mailing list so someone else can have a chance to resolve the problem.

Since everything passes here for me on Linux/x86_64 I'm guessing its a platform specific issue. We still should fix it.

-- 
Shawn.