Re: [PATCH 0/4] Fix prune/gc problem with multiple worktrees
Michael Haggertywrites: > 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
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
(from patch 4/4 mail) On Wed, Jun 1, 2016 at 10:51 PM, Michael Haggertywrote: >> + 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
Nguyễn Thái Ngọc Duywrites: > 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
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
[PATCH 0/4] Fix prune/gc problem with multiple worktrees
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. 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(-) -- 2.8.2.524.g6ff3d78 -- 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