From: Junio C Hamano <gitster@pobox•com>
To: Michael Rappazzo <rappazzo@gmail•com>
Cc: sunshine@sunshineco•com, git@vger•kernel.org,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail•com>
Subject: Re: [PATCH v3] worktree: add 'list' command
Date: Mon, 10 Aug 2015 15:10:54 -0700 [thread overview]
Message-ID: <xmqqr3nau74h.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1439239982-42826-2-git-send-email-rappazzo@gmail.com> (Michael Rappazzo's message of "Mon, 10 Aug 2015 16:53:02 -0400")
Michael Rappazzo <rappazzo@gmail•com> 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?
next prev parent reply other threads:[~2015-08-10 22:11 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-10 20:53 [PATCH v3] worktree: add 'list' command Michael Rappazzo
2015-08-10 20:53 ` Michael Rappazzo
2015-08-10 22:10 ` Junio C Hamano [this message]
2015-08-11 11:41 ` Mike Rappazzo
2015-08-11 15:46 ` Junio C Hamano
2015-08-11 2:55 ` David Turner
2015-08-11 11:42 ` Mike Rappazzo
2015-08-11 15:46 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqr3nau74h.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=pclouds@gmail$(echo .)com \
--cc=rappazzo@gmail$(echo .)com \
--cc=sunshine@sunshineco$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox