On Mon, Aug 10, 2015 at 6:10 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Michael Rappazzo <rappazzo@xxxxxxxxx> writes:
>
>> +static int list(int ac, const char **av, const char *prefix)
>> +{
>> + int main_only = 0;
>> + struct option options[] = {
>> + OPT_BOOL(0, "main-only", &main_only, N_("only list the main worktree")),
>> + OPT_END()
>> + };
>
> Hmm, main-only is still there?
Sorry, I missed that.
>
>> + int is_bare = is_bare_repository();
>
> Please do not introduce decl-after-stmt.
Since I reused this value below, I thought it would be acceptable.
>> + if (is_bare) {
>> + strbuf_addstr(&main_path, absolute_path(common_dir));
>
> Hmm, interesting.
>
> Because .git/config is shared, core.bare read from that tells us if
> the "main" one is bare, even if you start this command from one of
> its linked worktrees. So in that sense, this test of is_bare
> correctly tells if "main" one is a bare repository.
>
> But that by itself feels wrong. Doesn't the presense of a working
> tree mean that you should not get "is_bare==true" in such a case
> (i.e. your "main" one is bare, you are in a linked worktree of it
> that has the index and the working tree)?
Is is even correct for a bare repo to be included in the list? I
think that is part of what you are asking here.
>
>> + strbuf_strip_suffix(&main_path, "/.");
>
> In any case, what is that stripping of "/." about? Who is adding
> that extra trailing string?
>
> What I am getting at is (1) perhaps it shouldn't be adding that in
> the first place, and (2) if some other code is randomly adding "/."
> at the end, what guarantees you that you would need to strip it only
> once here---if the other code added that twice, don't you have to
> repeatedly remove "/." from the end?
In the case of a common dir, the value returned is just '.'. I wanted
to resolve that to the full path, so I called
`absolute_path(common_dir)`. Hence the trailing "/.". Similarly, in
the main repo, the common dir is just ".git", and I handled that by
using `get_git_work_tree()`.
--
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