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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]