I will reroll this series with adjustments as you suggested, and I
will add the extra tests for bare repos.
On Thu, Aug 13, 2015 at 3:23 PM, David Turner <dturner@xxxxxxxxxxxxxxxx> wrote:
> The scope of d can be smaller; move it down to the place I've marked
> below
I have adjusted the scoping. I misunderstood the meaning of some
comments from the v3 patch, but your statements have helped me
understand.
>
> strbuf_strip_suffix returns 1 if the suffix was stripped and 0
> otherwise, so there is no need for this strcmp.
Done.
>
> I'm a little worried about this path manipulation; it looks like the
> config setting core.bare is the actual thing to check? (Along with
> maybe the GIT_WORK_TREE environment var; not sure how that interacts
> with worktrees).
As Junio pointed out in a previous version of this patch, the
core.bare will always be 'true' since they share the config. He also
suggested that this could be the cause for concern. Therefore, I can
use core.bare to check if the main is a bare repo. I guess that with
the inclusion of tests using a bare repo, that will catch it if things
change in the future.
>
>> + }
>> +
>> + if (!main_is_bare) {
>> + strbuf_addstr(&worktree_path, main_path.buf);
>> +
>> + strbuf_addstr(&main_path, "/.git");
>> + strbuf_addstr(&worktree_git, main_path.buf);
>> +
>> + ret = fn(worktree_path.buf, worktree_git.buf, cb_data);
>> + if (ret)
>> + goto done;
>> + }
>> + strbuf_addstr(&main_path, "/worktrees");
>
> Earlier, you said:
> strbuf_addstr(&main_path, common_dir);
> strbuf_strip_suffix(&main_path, "/.git");
>
> Now you are adding /worktrees. Doesn't this mean, in the non-bare case,
> that you'll be looking in $common_dir/worktrees instead of
> $common_dir/.git/worktrees ?
I read the gitdir file from the common dir.
>> + while ((d = readdir(dir)) != NULL) {
>> + if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
>> + continue;
>> +
>> + strbuf_reset(&worktree_git);
>> + strbuf_addf(&worktree_git, "%s/%s/gitdir", main_path.buf, d->d_name);
>
> tioli: main_path never changes, so no need to keep chopping it off and
> adding it back on; just truncate worktree_git to main_path.len + 1 here
> and then add d->name.
I will try to clean it up a bit. I am not very experienced with C,
but I will do my best.
--
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