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?
> +
> + ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
> + if (ac)
> + usage_with_options(worktree_usage, options);
> +
> + struct strbuf main_path = STRBUF_INIT;
> + const char* common_dir = get_git_common_dir();
Asterisks stick to the variable names, not types.
> + int is_bare = is_bare_repository();
Please do not introduce decl-after-stmt.
> + 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)?
Duy? Eric? What do you guys think?
There are many codepaths that change their behaviour (e.g. if we
create reflogs by default) based on the return value of
is_bare_repository(). If I am reading this correctly, I _think_ a
new working area that was prepared with "git worktree add" out of a
bare repository would not work well, as these operations behave as
if we do not have a working tree. Perhaps is_bare_repository() in
such a working area _should_ say "No", even if core.bare in the
shared bare one is set to true.
> + 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?
--
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