On Tue, May 04, 2010 at 08:25:22PM -0400, Jared Hance wrote:
> The environment variable GIT_PATHNAME_PREFIX passes on the
> current working directory (where the git command was called from)
> to shell aliases (aliases that begin with "!"). This allows these
> shell aliases to know the directory that the git command was called
> from.
Seems like a reasonable goal, but...
> --- a/git.c
> +++ b/git.c
> @@ -167,6 +167,9 @@ static int handle_alias(int *argcp, const char ***argv)
> free(alias_string);
> alias_string = buf.buf;
> }
> + static char current_dir[PATH_MAX+1];
> + setenv("GIT_PATHNAME_PREFIX", getcwd(current_dir, sizeof(current_dir)), 1);
> +
> trace_printf("trace: alias to shell cmd: %s => %s\n",
> alias_command, alias_string + 1);
> ret = system(alias_string + 1);
I see three problems:
1. Don't declare variables in the middle of a function. It's a C99-ism
that we avoid to retain portability with older compilers.
2. On getcwd error, we setenv the value to NULL. Is that OK on all
platforms (I am specifically thinking of our Windows *env wrappers,
which have some restrictions, but I don't remember the details)?
3. Most importantly, isn't this totally the wrong place to look at
getcwd? We're just about to run system(), which means we will
already have done our chdir() (which probably is the one happening
in setup_git_directory). On even a simple test, it seems to always
print the root of the repository for me.
I think instead you want to pass in the prefix value computed by
setup_git_directory to handle_alias.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html