From: Andreas Ericsson <ae@op5.se>
Date: Wed, 16 Nov 2005 00:10:36 GMT
Subject: Re: [PATCH 1/3] C implementation of the 'git' program, take two.
Message-ID: <437A78FC.10608@op5.se>
In-Reply-To: <7vwtj9eaqm.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> exon@op5.se (Andreas Ericsson) writes:
> 
> 
>>This patch provides a C implementation of the 'git' program and
>>introduces support for putting the git-* commands in a directory
>>of their own.
> 
> 
> Very nice, thanks.  Two questions and a half.
> 
> 
>>+static void prepend_to_path(const char *dir, int len)
>>+{
>>+	char *path, *old_path = getenv("PATH");
>>+	int path_len = len;
>>+
>>+	if (!old_path)
>>+		old_path = "/bin:/usr/bin:.";
> 
> 
> This is to cover strange case and probably would not matter in
> practice, but perhaps without current directory?
> 

I have no preference really and since it already covers a strange case 
it probably shouldn't matter either way.

> 
>>+int main(int argc, char **argv, char **envp)
>>+{
>>+	char git_command[PATH_MAX + 1];
>>+	char wd[PATH_MAX + 1];
>>+	int i, len, show_help = 0;
>>+	char *exec_path = getenv("GIT_EXEC_PATH");
>>+
>>+	getcwd(wd, PATH_MAX);
>>+...
>>+	/* allow relative paths, but run with exact */
>>+	if (chdir(exec_path)) {
>>+		printf("git: '%s': %s\n", exec_path, strerror(errno));
>>+		exit (1);
>>+	}
>>+
>>+	getcwd(git_command, sizeof(git_command));
>>+	chdir(wd);
> 
> 
> Can we always come back from where we started?
> 

Not sure what you mean. Perhaps "Come back *to* where we started"?

If getcwd(wd, sizeof(wd)) fails then chdir(wd) will also fail (or do 
something strange, at least). wd is otherwise absolute.

> 
>>+
>>+	len = strlen(git_command);
>>+	prepend_to_path(git_command, len);
>>+
>>+	strncat(&git_command[len], "/git-", sizeof(git_command) - len);
>>+	len += 5;
>>+	strncat(&git_command[len], argv[i], sizeof(git_command) - len);
>>+
>>+	if (access(git_command, X_OK))
>>+		usage(exec_path, "'%s' is not a git-command", argv[i]);
>>+
>>+	/* execve() can only ever return if it fails */
>>+	execve(git_command, &argv[i], envp);
> 
> 
> Shell version for Cygwin seems to do ".exe" at the end --- does
> it matter?
> 

Dunno, really. I suppose it does as it bypasses the shell with the 
execve() call, unless windows or the cygwin stuff does some trickery to 
find an .exe regardless.

Is it ok if I send a separate patch for it, or would you rather have me 
redo this one?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231