2010/10/7 Jonathan Nieder <jrnieder@xxxxxxxxx>: > Nguyán ThÃi Ngác Duy wrote: > >> 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? I followed the convention in rev-parse.c, specifically --show-prefix code. Don't know if it's good or bad for scripts though. > 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? These two new fields are only set when both GIT_DIR and GIT_WORK_TREE are set, which means non-bare repository. I'll need to handle other setup cases as well, but for subproject-aware grep, this should be enough. > 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.) I think it's a bad idea. --show-prefix traditionally never returns "../(something)" and scripts (even builtin commands) depend on that. It's better to slowly migrate over the new prefix(es) when it makes sense. >> --- 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(). The two last ones are OK. Paths passed to this function come from either getcwd or make_absolute_path(). I'm sure they are canonicalized. But I'll add a comment to note that. I'll look into backslashes and DOS drives (hmm and UNC path too). > [...] > >> Â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. I'm not really POSIX-fluent to tell. But I'm sure in this function there won't be any subdirs. -- Duy -- 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