Re: [PATCH] worktree: provide better prefix to go back to original cwd

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 6, 2010, 18:07 UTC
Message-ID: <20101006180727.GA2118@burratino>
In-Reply-To: <1286373578-2484-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy wrote:

Show 18 quoted lines

> This patch allows builtin commands access to original cwd even if it's
> outside worktree, via cwd_to_worktree and worktree_to_cwd fields.
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -623,6 +623,16 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>  					puts(prefix);
>  				continue;
>  			}
> +			if (!strcmp(arg, "--cwd-to-worktree")) {
> +				if (startup_info->cwd_to_worktree)
> +					puts(startup_info->cwd_to_worktree);
> +				continue;
> +			}
> +			if (!strcmp(arg, "--worktree-to-cwd")) {
> +				if (startup_info->worktree_to_cwd)
> +					puts(startup_info->worktree_to_cwd);
> +				continue;
> +			}

Nice.

I wonder if this should use something like

	else
		puts(".");

or

	else
		putchar('\n');

.  What would be most convenient for scripted callers?

What do these commands do when run from a bare repository? Is the worktree the .git dir in that case, do they fail, or does something else happen?

Are there any examples to illustrate whether teaching --show-prefix to do what your --worktree-to-cwd does would be a good or bad idea? (Just curious.)

Show 8 quoted lines

> --- a/cache.h
> +++ b/cache.h
> @@ -1110,6 +1110,8 @@ const char *split_cmdline_strerror(int cmdline_errno);
>  /* git.c */
>  struct startup_info {
>  	int have_repository;
> +	char *cwd_to_worktree; /* chdir("this"); from cwd would return to worktree */
> +	char *worktree_to_cwd; /* chdir("this"); from worktree would return to cwd */

Comment nit: would

				/* path from original cwd to worktree */
				/* path from worktree to original cwd */

be clearer? But presumably any confused people should be able to find your log message.

Show 11 quoted lines

> --- a/setup.c
> +++ b/setup.c
> @@ -313,10 +313,109 @@ const char *read_gitfile_gently(const char *path)
>  	return path;
>  }
>  
> +/*
> + * Given "foo/bar" and "hey/hello/world", return "../../hey/hello/world/"
> + * Either path1 or path2 can be NULL
> + */
> +static char *make_path_to_path(const char *path1, const char *path2)

Nice.  Do we need to worry about:

 - alternate directory separators? (hey\hello\world)
 - DOS drive prefix? (c:\foo\bar, d:\hey\hello\world)
 - relative paths with DOS drive? (c:\foo\bar, d:hello)
 - doubled-up directory separators? (hey//hello/world, //foo/bar)
 - non-canonical paths? (hey/./hello/../hello/world)

I'm guessing some of the answers are "no", depending on where these paths come from. Compare make_relative_path(). [...]

Show 5 quoted lines

>  static const char *setup_explicit_git_dir(const char *gitdirenv,
>  				const char *work_tree_env, int *nongit_ok)
>  {
> -	static char buffer[1024 + 1];
> +	static char buffer[PATH_MAX];

Why?

It might make sense to error out a little before PATH_MAX (though later than 1024), to account for subdirs (e.g., objects/). Not sure.