Re: [PATCH 0/4] Fix prune/gc problem with multiple worktrees

[Junio C Hamano]

Michael Haggerty  writes:

> Fixing reachability via the index and detached HEADs feels relatively
> important.
> ...

I agree with the order of importance above.  But "relatively" is a
very good keyword.  Just like bisection refs, what is in the index
and the commit detached HEAD points at are expected to be tentative.
As a part of still-experimental feature, I'd rather see our
bandwidth spent on fixing it the right way first time, instead of
piling on an unproven quick-fix as a band aid, having to rip it off
and fixing it properly later.

> It's hard for me to predict when the ref-iterator stuff will be merged.
> It is a big change, but so far the feedback seems pretty good. I can
> tell you that pushing it and ref-stores forward is high on my priority list.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] Fix prune/gc problem with multiple worktrees

[Michael Haggerty]

On 06/02/2016 11:53 AM, Duy Nguyen wrote:
> (from patch 4/4 mail)
> 
> On Wed, Jun 1, 2016 at 10:51 PM, Michael Haggerty  
> wrote:
>>> + path = xstrdup(worktree_git_path(wt, "logs/refs/bisect"));
>>> + if (file_exists(path))
>>> + handle_one_reflog(path, NULL, 0, );
>>> + free(path);
>>> +}
>>
>> `refs/bisect` is not a single reference. It is a namespace that contains
>> references with names like `refs/bisect/bad` and
>> `refs/bisect/good-66106691a1b71e445fe5e4d6b8b043dffc7dfe4c`.
> 
> Yeah I missed that. I'm not going to write another directory walker to
> collect all logs/refs/bisect/*. I didn't add pending objects for
> refs/bisect/* of other worktrees either. At that point waiting for the
> new ref iterator makes more sense...
> 
> On Wed, Jun 1, 2016 at 11:06 PM, Junio C Hamano  wrote:
>> Nguyễn Thái Ngọc Duy   writes:
>>
>>> This series makes sure that objects referenced by all worktrees are
>>> marked reachable so that we don't accidentally delete objects that are
>>> being used. Previously per-worktree references in index, detached HEAD
>>> or per-worktree reflogs come from current worktree only, not all
>>> worktrees.
>>
>> I'll let this topic simmer on the list for now, instead of picking
>> it up immediately to 'pu', as Michael in $gmane/296068 makes me
>> wonder if we want to keep piling on the current "worktree ref
>> iterations are bolted on" or if we want to first clean it up, whose
>> natural fallout hopefully would eliminate the bug away.
> 
> So what should be the way forward? My intention was having something
> that can fix the problem for now, even if a bit hacky while waiting
> for ref iterator to be ready, then convert to use it and clean things
> up, because I don't how long ref-iterator would take and losing data
> is serous enough that I'd like to fix it soon. If we go with "fix soon
> then convert to ref-iterator later", then I will drop the
> logs/bisect/* check, checking logs/HEAD alone should be good enough.
> Otherwise I'll prepare a series on top of ref-iterator.

Fixing reachability via the index and detached HEADs feels relatively
important.

Fixing refs/bisect/* feels a bit less urgent, because (1) usually
bisections don't take very long, and (2) if the commits that one is
bisecting are not otherwise reachable anymore, then the bisection is
probably not interesting anymore. However, these are real references, so
if they get broken, the worktree will be seen as corrupt and recovery is
not super obvious (I guess most people would end up deleting the whole
worktree).

Fixing the reflogs for HEAD and (especially) refs/bisect/* in worktrees
feels even less important, because reflogs are not nearly as important
as current ref values, Git is relatively tolerant of broken reflog
entries, and there are easy ways to prune them if breakage occurs.

It's hard for me to predict when the ref-iterator stuff will be merged.
It is a big change, but so far the feedback seems pretty good. I can
tell you that pushing it and ref-stores forward is high on my priority list.

Michael

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] Fix prune/gc problem with multiple worktrees

[Duy Nguyen]

(from patch 4/4 mail)

On Wed, Jun 1, 2016 at 10:51 PM, Michael Haggerty  wrote:
>> + path = xstrdup(worktree_git_path(wt, "logs/refs/bisect"));
>> + if (file_exists(path))
>> + handle_one_reflog(path, NULL, 0, );
>> + free(path);
>> +}
>
> `refs/bisect` is not a single reference. It is a namespace that contains
> references with names like `refs/bisect/bad` and
> `refs/bisect/good-66106691a1b71e445fe5e4d6b8b043dffc7dfe4c`.

Yeah I missed that. I'm not going to write another directory walker to
collect all logs/refs/bisect/*. I didn't add pending objects for
refs/bisect/* of other worktrees either. At that point waiting for the
new ref iterator makes more sense...

On Wed, Jun 1, 2016 at 11:06 PM, Junio C Hamano  wrote:
> Nguyễn Thái Ngọc Duy   writes:
>
>> This series makes sure that objects referenced by all worktrees are
>> marked reachable so that we don't accidentally delete objects that are
>> being used. Previously per-worktree references in index, detached HEAD
>> or per-worktree reflogs come from current worktree only, not all
>> worktrees.
>
> I'll let this topic simmer on the list for now, instead of picking
> it up immediately to 'pu', as Michael in $gmane/296068 makes me
> wonder if we want to keep piling on the current "worktree ref
> iterations are bolted on" or if we want to first clean it up, whose
> natural fallout hopefully would eliminate the bug away.

So what should be the way forward? My intention was having something
that can fix the problem for now, even if a bit hacky while waiting
for ref iterator to be ready, then convert to use it and clean things
up, because I don't how long ref-iterator would take and losing data
is serous enough that I'd like to fix it soon. If we go with "fix soon
then convert to ref-iterator later", then I will drop the
logs/bisect/* check, checking logs/HEAD alone should be good enough.
Otherwise I'll prepare a series on top of ref-iterator.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] Fix prune/gc problem with multiple worktrees

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> This series makes sure that objects referenced by all worktrees are
> marked reachable so that we don't accidentally delete objects that are
> being used. Previously per-worktree references in index, detached HEAD
> or per-worktree reflogs come from current worktree only, not all
> worktrees.

I'll let this topic simmer on the list for now, instead of picking
it up immediately to 'pu', as Michael in $gmane/296068 makes me
wonder if we want to keep piling on the current "worktree ref
iterations are bolted on" or if we want to first clean it up, whose
natural fallout hopefully would eliminate the bug away.

Thanks.

> The series deals with git-prune and git-gc specifically. I left out
> "git rev-list". It shares the same problem because it will only
> consider current worktree's HEAD, index and per-worktree reflogs. The
> problem is I am not sure if we simply just change, say
> --indexed-objects, to cover all indexes, or should we only do that
> with "--all-worktrees --indexed-objects". I guess this is up for
> discussion.
>
> Nguyễn Thái Ngọc Duy (4):
>   revision.c: move read_cache() out of add_index_objects_to_pending()
>   reachable.c: mark reachable objects in index from all worktrees
>   reachable.c: mark reachable detached HEAD from all worktrees
>   reachable.c: make reachable reflogs for all per-worktree reflogs
>
>  reachable.c  | 47 +--
>  revision.c   | 34 +++---
>  revision.h   |  7 ++-
>  t/t5304-prune.sh | 40 
>  4 files changed, 114 insertions(+), 14 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] Fix prune/gc problem with multiple worktrees

[Jeff King]

On Wed, Jun 01, 2016 at 05:45:15PM +0700, Nguyễn Thái Ngọc Duy wrote:

> This series makes sure that objects referenced by all worktrees are
> marked reachable so that we don't accidentally delete objects that are
> being used. Previously per-worktree references in index, detached HEAD
> or per-worktree reflogs come from current worktree only, not all
> worktrees.
> 
> The series deals with git-prune and git-gc specifically. I left out
> "git rev-list". It shares the same problem because it will only
> consider current worktree's HEAD, index and per-worktree reflogs. The
> problem is I am not sure if we simply just change, say
> --indexed-objects, to cover all indexes, or should we only do that
> with "--all-worktrees --indexed-objects". I guess this is up for
> discussion.

I don't think touching reachable.c is enough. You also need to make sure
that calling "git pack-objects --indexed-objects" will get them, too
(otherwise they will be either ejected loose or dropped completely
when --unpack-unreachable=2.weeks.ago or similar is used).

That code path uses rev-list internally. So I think you need something
like "--all-worktrees --indexed-objects", and you need to pass the new
option in from git-repack to pack-objects (or you need to simply make
"--indexed-objects" cover all worktrees by default).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html