# [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs 21 messages from 2017-05-17 to 2017-05-25. Participants: Manish Goregaokar, Jonathan Nieder, manish.earth@gmail.com, Brandon Williams, manishearth@gmail.com, Simon Ruderich, Samuel Lijin, Junio C Hamano, Duy Nguyen. Thread: https://gitlist.dev/t/45988 ## Manish Goregaokar, 2017-05-17 00:07 Subject: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: URL: https://gitlist.dev/e/CACpkpxn4zSdUMT-i6XWBR77sQTpr5-vudbLVTCs1_hiv7a2t_w%40mail.gmail.com ``` Git prune will happily delete commits checked out in other worktrees. This is probably not desired. (Tabs have been converted to spaces in this email sadly, because GMail garbles these. This should suffice for review, and I'll send the patch as an attachment or in some other form when done so that it can be cleanly applied. Let me know if this won't work.) Thanks! Patch 1/2 follows (based on maint) ----- From c3657cd0bb61921053fad4dd669589780881c574 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 16 May 2017 16:46:00 -0700 Subject: refs: Add for_each_worktree_ref for iterating over all worktree HEADs To ensure that `git prune` does not remove refs checked out in other worktrees, we need to include these HEADs in the set of roots. This adds the iteration function necessary to do this. Signed-off-by: Manish Goregaokar --- refs.c | 16 ++++++++++++++++ refs.h | 1 + 2 files changed, 17 insertions(+) diff --git a/refs.c b/refs.c index 2d71774..27e0b60 100644 --- a/refs.c +++ b/refs.c @@ -3,6 +3,7 @@ */ #include "cache.h" +#include "commit.h" #include "lockfile.h" #include "refs.h" #include "refs/refs-internal.h" @@ -1157,6 +1158,21 @@ int head_ref(each_ref_fn fn, void *cb_data) return head_ref_submodule(NULL, fn, cb_data); } +int for_each_worktree_ref(each_ref_fn fn, void *cb_data) +{ + int i, flag, retval; + struct object_id oid; + struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED); + for (i = 0; worktrees[i]; i++) { + struct commit* commit = lookup_commit_reference(worktrees[i]->head_sha1); + oid = commit->object.oid; + if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag)) { + if (retval = fn("HEAD", &oid, flag, cb_data)) + return retval; + } + } +} + /* * Call fn for each reference in the specified submodule for which the * refname begins with prefix. If trim is non-zero, then trim that diff --git a/refs.h b/refs.h index 9fbff90..425a853 100644 --- a/refs.h +++ b/refs.h @@ -192,6 +192,7 @@ typedef int each_ref_fn(const char *refname, * stop the iteration. */ int head_ref(each_ref_fn fn, void *cb_data); +int for_each_worktree_ref(each_ref_fn fn, void *cb_data); int for_each_ref(each_ref_fn fn, void *cb_data); int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data); int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, -- 2.10.1 ``` ## Manish Goregaokar, 2017-05-17 00:08 Subject: Re: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: URL: https://gitlist.dev/e/CACpkpx%3DO4UxrELRO3ojbbRL2LShcw0MLq%3D_4Dyck8csbwca9CQ%40mail.gmail.com In-Reply-To: ``` From 3f8015efdcc122e0d345baeb5f1f0485a9f0fcd8 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 16 May 2017 16:46:36 -0700 Subject: [PATCH 2/2] reachable: Add HEADs of all worktrees to reachability analysis * reachable.c: mark_reachable_objects: Include other worktrees Signed-off-by: Manish Goregaokar --- reachable.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/reachable.c b/reachable.c index d0199ca..439708e 100644 --- a/reachable.c +++ b/reachable.c @@ -178,6 +178,9 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog, /* detached HEAD is not included in the list above */ head_ref(add_one_ref, revs); + /* worktrees are not included in either */ + for_each_worktree_ref(add_one_ref, revs); + /* Add all reflog info */ if (mark_reflog) add_reflogs_to_pending(revs, 0); -- 2.10.1 -Manish Goregaokar On Tue, May 16, 2017 at 5:07 PM, Manish Goregaokar wrote: > Git prune will happily delete commits checked out in other worktrees. > This is probably not desired. > > (Tabs have been converted to spaces in this email sadly, because GMail > garbles these. This should suffice for review, and I'll send the patch > as an attachment or in some other form when done so that it can be > cleanly applied. Let me know if this won't work.) > > > Thanks! > > Patch 1/2 follows (based on maint) > > ----- > > From c3657cd0bb61921053fad4dd669589780881c574 Mon Sep 17 00:00:00 2001 > From: Manish Goregaokar > Date: Tue, 16 May 2017 16:46:00 -0700 > Subject: refs: Add for_each_worktree_ref for iterating over all worktree HEADs > > To ensure that `git prune` does not remove refs checked out > in other worktrees, we need to include these HEADs in the > set of roots. This adds the iteration function necessary > to do this. > > Signed-off-by: Manish Goregaokar > --- > refs.c | 16 ++++++++++++++++ > refs.h | 1 + > 2 files changed, 17 insertions(+) > > diff --git a/refs.c b/refs.c > index 2d71774..27e0b60 100644 > --- a/refs.c > +++ b/refs.c > @@ -3,6 +3,7 @@ > */ > > #include "cache.h" > +#include "commit.h" > #include "lockfile.h" > #include "refs.h" > #include "refs/refs-internal.h" > @@ -1157,6 +1158,21 @@ int head_ref(each_ref_fn fn, void *cb_data) > return head_ref_submodule(NULL, fn, cb_data); > } > > +int for_each_worktree_ref(each_ref_fn fn, void *cb_data) > +{ > + int i, flag, retval; > + struct object_id oid; > + struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED); > + for (i = 0; worktrees[i]; i++) { > + struct commit* commit = > lookup_commit_reference(worktrees[i]->head_sha1); > + oid = commit->object.oid; > + if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag)) { > + if (retval = fn("HEAD", &oid, flag, cb_data)) > + return retval; > + } > + } > +} > + > /* > * Call fn for each reference in the specified submodule for which the > * refname begins with prefix. If trim is non-zero, then trim that > diff --git a/refs.h b/refs.h > index 9fbff90..425a853 100644 > --- a/refs.h > +++ b/refs.h > @@ -192,6 +192,7 @@ typedef int each_ref_fn(const char *refname, > * stop the iteration. > */ > int head_ref(each_ref_fn fn, void *cb_data); > +int for_each_worktree_ref(each_ref_fn fn, void *cb_data); > int for_each_ref(each_ref_fn fn, void *cb_data); > int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data); > int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, > -- > 2.10.1 ``` ## Jonathan Nieder, 2017-05-17 00:28 Subject: Re: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: <20170517002825.GR27400@aiede.svl.corp.google.com> URL: https://gitlist.dev/e/20170517002825.GR27400%40aiede.svl.corp.google.com In-Reply-To: ``` (+cc: Duy, worktree expert) Hi Manish, Manish Goregaokar wrote: > Git prune will happily delete commits checked out in other worktrees. > This is probably not desired. Yikes. Thanks for working on it. > (Tabs have been converted to spaces in this email sadly, because GMail > garbles these. This should suffice for review, and I'll send the patch > as an attachment or in some other form when done so that it can be > cleanly applied. Let me know if this won't work.) I don't think this will work well --- many reviewers (e.g. I am one of them) rely on being able to apply patches and inspect them locally. See the Discussion and MUA-Specific Hints sections of git-format-patch(1) for some hints about how to accomplish that. Patch left unsnipped for reference. Thanks, Jonathan > To ensure that `git prune` does not remove refs checked out > in other worktrees, we need to include these HEADs in the > set of roots. This adds the iteration function necessary > to do this. > > Signed-off-by: Manish Goregaokar > --- > refs.c | 16 ++++++++++++++++ > refs.h | 1 + > 2 files changed, 17 insertions(+) > > diff --git a/refs.c b/refs.c > index 2d71774..27e0b60 100644 > --- a/refs.c > +++ b/refs.c > @@ -3,6 +3,7 @@ > */ > > #include "cache.h" > +#include "commit.h" > #include "lockfile.h" > #include "refs.h" > #include "refs/refs-internal.h" > @@ -1157,6 +1158,21 @@ int head_ref(each_ref_fn fn, void *cb_data) > return head_ref_submodule(NULL, fn, cb_data); > } > > +int for_each_worktree_ref(each_ref_fn fn, void *cb_data) > +{ > + int i, flag, retval; > + struct object_id oid; > + struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED); > + for (i = 0; worktrees[i]; i++) { > + struct commit* commit = > lookup_commit_reference(worktrees[i]->head_sha1); > + oid = commit->object.oid; > + if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag)) { > + if (retval = fn("HEAD", &oid, flag, cb_data)) > + return retval; > + } > + } > +} > + > /* > * Call fn for each reference in the specified submodule for which the > * refname begins with prefix. If trim is non-zero, then trim that > diff --git a/refs.h b/refs.h > index 9fbff90..425a853 100644 > --- a/refs.h > +++ b/refs.h > @@ -192,6 +192,7 @@ typedef int each_ref_fn(const char *refname, > * stop the iteration. > */ > int head_ref(each_ref_fn fn, void *cb_data); > +int for_each_worktree_ref(each_ref_fn fn, void *cb_data); > int for_each_ref(each_ref_fn fn, void *cb_data); > int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data); > int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, > -- > 2.10.1 ``` ## manish.earth@gmail.com, 2017-05-17 00:50 Subject: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: <20170517005041.46310-1-manishearth@gmail.com> URL: https://gitlist.dev/e/20170517005041.46310-1-manishearth%40gmail.com In-Reply-To: <20170517002825.GR27400@aiede.svl.corp.google.com> ``` From: Manish Goregaokar To ensure that `git prune` does not remove refs checked out in other worktrees, we need to include these HEADs in the set of roots. This adds the iteration function necessary to do this. Signed-off-by: Manish Goregaokar --- refs.c | 16 ++++++++++++++++ refs.h | 1 + 2 files changed, 17 insertions(+) diff --git a/refs.c b/refs.c index 2d71774..27e0b60 100644 --- a/refs.c +++ b/refs.c @@ -3,6 +3,7 @@ */ #include "cache.h" +#include "commit.h" #include "lockfile.h" #include "refs.h" #include "refs/refs-internal.h" @@ -1157,6 +1158,21 @@ int head_ref(each_ref_fn fn, void *cb_data) return head_ref_submodule(NULL, fn, cb_data); } +int for_each_worktree_ref(each_ref_fn fn, void *cb_data) +{ + int i, flag, retval; + struct object_id oid; + struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED); + for (i = 0; worktrees[i]; i++) { + struct commit* commit = lookup_commit_reference(worktrees[i]->head_sha1); + oid = commit->object.oid; + if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag)) { + if (retval = fn("HEAD", &oid, flag, cb_data)) + return retval; + } + } +} + /* * Call fn for each reference in the specified submodule for which the * refname begins with prefix. If trim is non-zero, then trim that diff --git a/refs.h b/refs.h index 9fbff90..425a853 100644 --- a/refs.h +++ b/refs.h @@ -192,6 +192,7 @@ typedef int each_ref_fn(const char *refname, * stop the iteration. */ int head_ref(each_ref_fn fn, void *cb_data); +int for_each_worktree_ref(each_ref_fn fn, void *cb_data); int for_each_ref(each_ref_fn fn, void *cb_data); int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data); int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, -- 2.10.1 ``` ## manish.earth@gmail.com, 2017-05-17 00:50 Subject: [PATCH 2/2] reachable: Add HEADs of all worktrees to reachability analysis Message-ID: <20170517005041.46310-2-manishearth@gmail.com> URL: https://gitlist.dev/e/20170517005041.46310-2-manishearth%40gmail.com In-Reply-To: <20170517005041.46310-1-manishearth@gmail.com> ``` From: Manish Goregaokar * reachable.c: mark_reachable_objects: Include other worktrees Signed-off-by: Manish Goregaokar --- reachable.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/reachable.c b/reachable.c index d0199ca..439708e 100644 --- a/reachable.c +++ b/reachable.c @@ -178,6 +178,9 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog, /* detached HEAD is not included in the list above */ head_ref(add_one_ref, revs); + /* worktrees are not included in either */ + for_each_worktree_ref(add_one_ref, revs); + /* Add all reflog info */ if (mark_reflog) add_reflogs_to_pending(revs, 0); -- 2.10.1 ``` ## Manish Goregaokar, 2017-05-17 00:55 Subject: Re: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: URL: https://gitlist.dev/e/CAM2h-yfRc69W8f%3DinRRjtyBh4EZWJk6W2gCTM%3DS%3DifJn3g8auA%40mail.gmail.com In-Reply-To: <20170517005041.46310-1-manishearth@gmail.com> ``` I *think* I got send-email to work but I can't be certain :) The tabs seem to be back! It didn't thread correctly, unsure why. Sorry about the patch problems, On Tue, May 16, 2017 at 5:50 PM, wrote: > From: Manish Goregaokar > > To ensure that `git prune` does not remove refs checked out > in other worktrees, we need to include these HEADs in the > set of roots. This adds the iteration function necessary > to do this. > > Signed-off-by: Manish Goregaokar > --- > refs.c | 16 ++++++++++++++++ > refs.h | 1 + > 2 files changed, 17 insertions(+) > > diff --git a/refs.c b/refs.c > index 2d71774..27e0b60 100644 > --- a/refs.c > +++ b/refs.c > @@ -3,6 +3,7 @@ > */ > > #include "cache.h" > +#include "commit.h" > #include "lockfile.h" > #include "refs.h" > #include "refs/refs-internal.h" > @@ -1157,6 +1158,21 @@ int head_ref(each_ref_fn fn, void *cb_data) > return head_ref_submodule(NULL, fn, cb_data); > } > > +int for_each_worktree_ref(each_ref_fn fn, void *cb_data) > +{ > + int i, flag, retval; > + struct object_id oid; > + struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED); > + for (i = 0; worktrees[i]; i++) { > + struct commit* commit = lookup_commit_reference(worktrees[i]->head_sha1); > + oid = commit->object.oid; > + if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag)) { > + if (retval = fn("HEAD", &oid, flag, cb_data)) > + return retval; > + } > + } > +} > + > /* > * Call fn for each reference in the specified submodule for which the > * refname begins with prefix. If trim is non-zero, then trim that > diff --git a/refs.h b/refs.h > index 9fbff90..425a853 100644 > --- a/refs.h > +++ b/refs.h > @@ -192,6 +192,7 @@ typedef int each_ref_fn(const char *refname, > * stop the iteration. > */ > int head_ref(each_ref_fn fn, void *cb_data); > +int for_each_worktree_ref(each_ref_fn fn, void *cb_data); > int for_each_ref(each_ref_fn fn, void *cb_data); > int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data); > int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, > -- > 2.10.1 > ``` ## Manish Goregaokar, 2017-05-17 21:48 Subject: Re: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: URL: https://gitlist.dev/e/CACpkpx%3DusSnEBPPPppdeaM%2BHUNc06ZuN3mrfN4Nemspn96K9dQ%40mail.gmail.com In-Reply-To: ``` Oh, btw, refs.c needs an #include "worktree.h" to work; I didn't get a chance to test this after rebasing onto the maint branch. (There's also another fix it needs to have no warnings, but that's not going to affect building). I have this fixed locally, but I'll wait for the rest of the review before pushing them up. -Manish Goregaokar On Tue, May 16, 2017 at 5:55 PM, Manish Goregaokar wrote: > I *think* I got send-email to work but I can't be certain :) The tabs > seem to be back! > > It didn't thread correctly, unsure why. > > Sorry about the patch problems, > > On Tue, May 16, 2017 at 5:50 PM, wrote: >> From: Manish Goregaokar >> >> To ensure that `git prune` does not remove refs checked out >> in other worktrees, we need to include these HEADs in the >> set of roots. This adds the iteration function necessary >> to do this. >> >> Signed-off-by: Manish Goregaokar >> --- >> refs.c | 16 ++++++++++++++++ >> refs.h | 1 + >> 2 files changed, 17 insertions(+) >> >> diff --git a/refs.c b/refs.c >> index 2d71774..27e0b60 100644 >> --- a/refs.c >> +++ b/refs.c >> @@ -3,6 +3,7 @@ >> */ >> >> #include "cache.h" >> +#include "commit.h" >> #include "lockfile.h" >> #include "refs.h" >> #include "refs/refs-internal.h" >> @@ -1157,6 +1158,21 @@ int head_ref(each_ref_fn fn, void *cb_data) >> return head_ref_submodule(NULL, fn, cb_data); >> } >> >> +int for_each_worktree_ref(each_ref_fn fn, void *cb_data) >> +{ >> + int i, flag, retval; >> + struct object_id oid; >> + struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED); >> + for (i = 0; worktrees[i]; i++) { >> + struct commit* commit = lookup_commit_reference(worktrees[i]->head_sha1); >> + oid = commit->object.oid; >> + if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag)) { >> + if (retval = fn("HEAD", &oid, flag, cb_data)) >> + return retval; >> + } >> + } >> +} >> + >> /* >> * Call fn for each reference in the specified submodule for which the >> * refname begins with prefix. If trim is non-zero, then trim that >> diff --git a/refs.h b/refs.h >> index 9fbff90..425a853 100644 >> --- a/refs.h >> +++ b/refs.h >> @@ -192,6 +192,7 @@ typedef int each_ref_fn(const char *refname, >> * stop the iteration. >> */ >> int head_ref(each_ref_fn fn, void *cb_data); >> +int for_each_worktree_ref(each_ref_fn fn, void *cb_data); >> int for_each_ref(each_ref_fn fn, void *cb_data); >> int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data); >> int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, >> -- >> 2.10.1 >> ``` ## Brandon Williams, 2017-05-18 00:07 Subject: Re: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: <20170518000735.GD185461@google.com> URL: https://gitlist.dev/e/20170518000735.GD185461%40google.com In-Reply-To: ``` On 05/17, Manish Goregaokar wrote: > Oh, btw, refs.c needs an #include "worktree.h" to work; I didn't get a > chance to test this after rebasing onto the maint branch. > > (There's also another fix it needs to have no warnings, but that's not > going to affect building). I have this fixed locally, but I'll wait > for the rest of the review before pushing them up. > -Manish Goregaokar Just as an fyi, its usually fine to send out a path RFC (request for comments) or WIP (Work in Progress) which compiles with warnings (or maybe not at all) and which doesn't pass all tests. If you do that just make sure to indicate as such. Though if you are sending out a patch which you want to be seriously reviewed and ultimately merged then the best practice is to ensure that it compiles without warnings and that all tests pass. I'm definitely guilty of this occasionally (no one's perfect!) but I'm just hoping to provide you with some of the expectations we have. I'm assuming you're newer to the community, so Welcome! Take a load off and stay a while :) -- Brandon Williams ``` ## Manish Goregaokar, 2017-05-18 01:16 Subject: Re: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: URL: https://gitlist.dev/e/CACpkpxkGWhcALQZ2%2B2nOCRKgzAa7U7EjZg--S71zocdGY8NYag%40mail.gmail.com In-Reply-To: <20170518000735.GD185461@google.com> ``` > Just as an fyi, its usually fine to send out a path RFC Ah, this is helpful! Yes, I was still trying to get the tests to run, so consider this WIP. I have since gotten them to run and found one failure which I fixed (didn't null-check `commit`). Waiting for them to finish again, will send new patches when done. Sorry about that. > but I'm just hoping to provide you with some of the expectations we have. Thank you! I thought I'd submit the patch early so that I could get the basic design through review; I wasn't sure if I was using the right APIs for this task. I should probably write a test for this too. Looks straightforward enough. > Welcome! Take a load off and stay a while :) :) -Manish Goregaokar On Wed, May 17, 2017 at 5:07 PM, Brandon Williams wrote: > On 05/17, Manish Goregaokar wrote: >> Oh, btw, refs.c needs an #include "worktree.h" to work; I didn't get a >> chance to test this after rebasing onto the maint branch. >> >> (There's also another fix it needs to have no warnings, but that's not >> going to affect building). I have this fixed locally, but I'll wait >> for the rest of the review before pushing them up. >> -Manish Goregaokar > > Just as an fyi, its usually fine to send out a path RFC (request for > comments) or WIP (Work in Progress) which compiles with warnings (or > maybe not at all) and which doesn't pass all tests. If you do that just > make sure to indicate as such. > > Though if you are sending out a patch which you want to be seriously > reviewed and ultimately merged then the best practice is to ensure that > it compiles without warnings and that all tests pass. I'm definitely > guilty of this occasionally (no one's perfect!) but I'm just hoping to > provide you with some of the expectations we have. > > I'm assuming you're newer to the community, so Welcome! Take a load off > and stay a while :) > > -- > Brandon Williams ``` ## manishearth@gmail.com, 2017-05-18 01:42 Subject: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: <20170518014210.94189-1-manishearth@gmail.com> URL: https://gitlist.dev/e/20170518014210.94189-1-manishearth%40gmail.com In-Reply-To: ``` From: Manish Goregaokar To ensure that `git prune` does not remove refs checked out in other worktrees, we need to include these HEADs in the set of roots. This adds the iteration function necessary to do this. Signed-off-by: Manish Goregaokar --- refs.c | 20 ++++++++++++++++++++ refs.h | 1 + 2 files changed, 21 insertions(+) diff --git a/refs.c b/refs.c index 2d71774..7dc82ba 100644 --- a/refs.c +++ b/refs.c @@ -3,10 +3,12 @@ */ #include "cache.h" +#include "commit.h" #include "lockfile.h" #include "refs.h" #include "refs/refs-internal.h" #include "object.h" +#include "worktree.h" #include "tag.h" /* @@ -1157,6 +1159,24 @@ int head_ref(each_ref_fn fn, void *cb_data) return head_ref_submodule(NULL, fn, cb_data); } +int for_each_worktree_ref(each_ref_fn fn, void *cb_data) +{ + int i, flag, retval = 0; + struct object_id oid; + struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED); + struct commit* commit; + for (i = 0; worktrees[i]; i++) { + if ((commit = lookup_commit_reference(worktrees[i]->head_sha1))) { + oid = commit->object.oid; + if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag)) { + if ((retval = fn("HEAD", &oid, flag, cb_data))) + return retval; + } + } + } + return retval; +} + /* * Call fn for each reference in the specified submodule for which the * refname begins with prefix. If trim is non-zero, then trim that diff --git a/refs.h b/refs.h index 9fbff90..425a853 100644 --- a/refs.h +++ b/refs.h @@ -192,6 +192,7 @@ typedef int each_ref_fn(const char *refname, * stop the iteration. */ int head_ref(each_ref_fn fn, void *cb_data); +int for_each_worktree_ref(each_ref_fn fn, void *cb_data); int for_each_ref(each_ref_fn fn, void *cb_data); int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data); int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, -- 2.10.1 ``` ## manishearth@gmail.com, 2017-05-18 01:42 Subject: [PATCH v2 2/2] reachable: Add HEADs of all worktrees to reachability analysis Message-ID: <20170518014210.94189-2-manishearth@gmail.com> URL: https://gitlist.dev/e/20170518014210.94189-2-manishearth%40gmail.com In-Reply-To: <20170518014210.94189-1-manishearth@gmail.com> ``` From: Manish Goregaokar * reachable.c: mark_reachable_objects: Include other worktrees Signed-off-by: Manish Goregaokar --- reachable.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/reachable.c b/reachable.c index d0199ca..439708e 100644 --- a/reachable.c +++ b/reachable.c @@ -178,6 +178,9 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog, /* detached HEAD is not included in the list above */ head_ref(add_one_ref, revs); + /* worktrees are not included in either */ + for_each_worktree_ref(add_one_ref, revs); + /* Add all reflog info */ if (mark_reflog) add_reflogs_to_pending(revs, 0); -- 2.10.1 ``` ## Manish Goregaokar, 2017-05-18 01:45 Subject: Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: URL: https://gitlist.dev/e/CACpkpxn1D_g%3Dim0um4ncLiq3A18HRPpfu-sb1FLoY__tet0--Q%40mail.gmail.com In-Reply-To: <20170518014210.94189-1-manishearth@gmail.com> ``` Hm, my invocation of git-send-email keeps getting the threading wrong. Is there a recommended set of arguments to the command? Thanks, -Manish Goregaokar On Wed, May 17, 2017 at 6:42 PM, wrote: > From: Manish Goregaokar > > To ensure that `git prune` does not remove refs checked out > in other worktrees, we need to include these HEADs in the > set of roots. This adds the iteration function necessary > to do this. > > Signed-off-by: Manish Goregaokar > --- > refs.c | 20 ++++++++++++++++++++ > refs.h | 1 + > 2 files changed, 21 insertions(+) > > diff --git a/refs.c b/refs.c > index 2d71774..7dc82ba 100644 > --- a/refs.c > +++ b/refs.c > @@ -3,10 +3,12 @@ > */ > > #include "cache.h" > +#include "commit.h" > #include "lockfile.h" > #include "refs.h" > #include "refs/refs-internal.h" > #include "object.h" > +#include "worktree.h" > #include "tag.h" > > /* > @@ -1157,6 +1159,24 @@ int head_ref(each_ref_fn fn, void *cb_data) > return head_ref_submodule(NULL, fn, cb_data); > } > > +int for_each_worktree_ref(each_ref_fn fn, void *cb_data) > +{ > + int i, flag, retval = 0; > + struct object_id oid; > + struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED); > + struct commit* commit; > + for (i = 0; worktrees[i]; i++) { > + if ((commit = lookup_commit_reference(worktrees[i]->head_sha1))) { > + oid = commit->object.oid; > + if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag)) { > + if ((retval = fn("HEAD", &oid, flag, cb_data))) > + return retval; > + } > + } > + } > + return retval; > +} > + > /* > * Call fn for each reference in the specified submodule for which the > * refname begins with prefix. If trim is non-zero, then trim that > diff --git a/refs.h b/refs.h > index 9fbff90..425a853 100644 > --- a/refs.h > +++ b/refs.h > @@ -192,6 +192,7 @@ typedef int each_ref_fn(const char *refname, > * stop the iteration. > */ > int head_ref(each_ref_fn fn, void *cb_data); > +int for_each_worktree_ref(each_ref_fn fn, void *cb_data); > int for_each_ref(each_ref_fn fn, void *cb_data); > int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data); > int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, > -- > 2.10.1 > ``` ## Simon Ruderich, 2017-05-18 09:40 Subject: Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: <20170518094038.x3xvpsfte3zxpuqd@ruderich.org> URL: https://gitlist.dev/e/20170518094038.x3xvpsfte3zxpuqd%40ruderich.org In-Reply-To: ``` On Wed, May 17, 2017 at 06:45:31PM -0700, Manish Goregaokar wrote: > Hm, my invocation of git-send-email keeps getting the threading wrong. > Is there a recommended set of arguments to the command? The threading looks fine here (for both cases where you mentioned it being wrong). Why do you think it's wrong? How does it look on your end? Regards Simon -- + privacy is necessary + using gnupg http://gnupg.org + public key id: 0x92FEFDB7E44C32F9 ``` ## Samuel Lijin, 2017-05-18 20:29 Subject: Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: URL: https://gitlist.dev/e/CAJZjrdUXXEU9g%2Bn5Xhq3eq4pW8zdaev9gVcwdcNeiDvPpgDzgQ%40mail.gmail.com In-Reply-To: <20170518094038.x3xvpsfte3zxpuqd@ruderich.org> ``` On Thu, May 18, 2017 at 5:40 AM, Simon Ruderich wrote: > On Wed, May 17, 2017 at 06:45:31PM -0700, Manish Goregaokar wrote: >> Hm, my invocation of git-send-email keeps getting the threading wrong. >> Is there a recommended set of arguments to the command? > > The threading looks fine here (for both cases where you mentioned > it being wrong). Why do you think it's wrong? How does it look on > your end? If you're on gmail (as Manish and I both are) patches in a subsequent version will be threaded (wrongly) against "earlier" versions of the patch. So if you have patch series A0, A1, A2, A3 and new version B0, B1, B2, if you thread them as A0 - A1 - A2 - A3 - B0 - B1 - B2 gmail will show them in your inbox as A0 - B0 A1 - B1 A2 A3 - B2 Depending on whatever heuristics they use to match up "threads", presumably because most emails aren't threaded correctly. ``` ## Brandon Williams, 2017-05-18 21:06 Subject: Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: <20170518210624.GF112091@google.com> URL: https://gitlist.dev/e/20170518210624.GF112091%40google.com In-Reply-To: ``` On 05/18, Samuel Lijin wrote: > On Thu, May 18, 2017 at 5:40 AM, Simon Ruderich wrote: > > On Wed, May 17, 2017 at 06:45:31PM -0700, Manish Goregaokar wrote: > >> Hm, my invocation of git-send-email keeps getting the threading wrong. > >> Is there a recommended set of arguments to the command? > > > > The threading looks fine here (for both cases where you mentioned > > it being wrong). Why do you think it's wrong? How does it look on > > your end? > > If you're on gmail (as Manish and I both are) patches in a subsequent Gmail does not do threading, well not true threading. It does grouping of emails by subject line, that's it. If you want to view threading (based on message-id) properly then you'll probably need to use a different email client. > version will be threaded (wrongly) against "earlier" versions of the > patch. So if you have patch series A0, A1, A2, A3 and new version B0, > B1, B2, if you thread them as > > A0 > - A1 > - A2 > - A3 > - B0 > - B1 > - B2 > > gmail will show them in your inbox as > > A0 > - B0 > > A1 > - B1 > > A2 > > A3 > - B2 > > Depending on whatever heuristics they use to match up "threads", > presumably because most emails aren't threaded correctly. -- Brandon Williams ``` ## Junio C Hamano, 2017-05-20 10:30 Subject: Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: URL: https://gitlist.dev/e/xmqqy3trstup.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20170518014210.94189-1-manishearth@gmail.com> ``` manishearth@gmail.com writes: > +int for_each_worktree_ref(each_ref_fn fn, void *cb_data) > +{ > + int i, flag, retval = 0; > + struct object_id oid; > + struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED); > + struct commit* commit; > + for (i = 0; worktrees[i]; i++) { > + if ((commit = lookup_commit_reference(worktrees[i]->head_sha1))) { > + oid = commit->object.oid; > + if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag)) { > + if ((retval = fn("HEAD", &oid, flag, cb_data))) > + return retval; > + } > + } > + } > + return retval; > +} I would have expected for-each-worktree-ref to iterate over all the refs in a given worktree, but that is not what this does. This instead iterates over worktrees and shows only their HEAD ref, no other refs. This helper is somewhat misnamed. By the way, doesn't nd/prune-in-worktree topic that has been cooking in 'pu' supersede this change? It not just protects the commit at the tip of HEAD in each worktree, it also makes sure the ones in HEAD's reflog are not prematurely pruned. Thanks. ``` ## Manish Goregaokar, 2017-05-20 17:33 Subject: Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: URL: https://gitlist.dev/e/CACpkpxmGzz0hUsypX5ORX7FLB3rh_xbyPOKz4SUt-zrjSjCq_g%40mail.gmail.com In-Reply-To: ``` Yes, you are right (on both counts). One thing which I think hasn't been covered yet is the rebase ORIG_HEAD. I'll see if that's still a problem on `pu` and make a patch for it if so. (I recall `git prune` during a rebase messing up repo state, though it's really my fault for trying that in the first place. Would be nice if it worked, though) -Manish Goregaokar On Sat, May 20, 2017 at 3:30 AM, Junio C Hamano wrote: > manishearth@gmail.com writes: > >> +int for_each_worktree_ref(each_ref_fn fn, void *cb_data) >> +{ >> + int i, flag, retval = 0; >> + struct object_id oid; >> + struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED); >> + struct commit* commit; >> + for (i = 0; worktrees[i]; i++) { >> + if ((commit = lookup_commit_reference(worktrees[i]->head_sha1))) { >> + oid = commit->object.oid; >> + if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag)) { >> + if ((retval = fn("HEAD", &oid, flag, cb_data))) >> + return retval; >> + } >> + } >> + } >> + return retval; >> +} > > I would have expected for-each-worktree-ref to iterate over all the > refs in a given worktree, but that is not what this does. This > instead iterates over worktrees and shows only their HEAD ref, no > other refs. This helper is somewhat misnamed. > > By the way, doesn't nd/prune-in-worktree topic that has been cooking > in 'pu' supersede this change? It not just protects the commit at > the tip of HEAD in each worktree, it also makes sure the ones in > HEAD's reflog are not prematurely pruned. > > Thanks. > ``` ## Junio C Hamano, 2017-05-20 23:49 Subject: Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: URL: https://gitlist.dev/e/xmqqzie7qeal.fsf%40gitster.mtv.corp.google.com In-Reply-To: ``` Manish Goregaokar writes: > One thing which I think hasn't been covered yet is the rebase > ORIG_HEAD. I'll see if that's still a problem on `pu` and make a patch > for it if so. IIRC, ORIG_HEAD, FETCH_HEAD, MERGE_HEAD and others are be transitory and never served as the starting points of reachability traversal, even in the primary worktree (also when you are not using any extra worktrees). The commits listed in the todo list of "rebase -i" and "git cherry-pick A..B" are the same way. Because ORIG_HEAD is expected to be in a reflog of some ref (at least, the reflog of HEAD), I do not see much benefit (or "more safety") in adding it to the starting points. Among the ones that appear in .git/ and we never considered as starting points, the commits in FETCH_HEAD might be the ones we may want to give extra protection over what we currently have, simply because the ones that do not have remote-tracking branches have NO other refs pointing at them (compared to these transitory artifacts resulting from a local operation, i.e. ORIG_HEAD and ones in the todo list). But they by definition are "new" objects, and the reason why the user does *not* use remote-tracking branches for them is because the user does not want to keep record of them unless the user decides to merge them somewhere in the repository's history, so even for them the benefit is dubious... ``` ## Duy Nguyen, 2017-05-22 11:17 Subject: Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: URL: https://gitlist.dev/e/CACsJy8CN0ZvWD6v2vkK3Lbt7xgrMwCjFKT6Nx7HtwZiEzmowpw%40mail.gmail.com In-Reply-To: ``` On Sat, May 20, 2017 at 5:30 PM, Junio C Hamano wrote: > By the way, doesn't nd/prune-in-worktree topic that has been cooking > in 'pu' supersede this change? It not just protects the commit at > the tip of HEAD in each worktree, it also makes sure the ones in > HEAD's reflog are not prematurely pruned. You have probably noticed I have stayed silent for a long time (and probably will continue). But yes nd/prune-in-worktree should fix this. Unfortunately I will not be able to fix it up (I think Michael responded on my last question about the proper way to fix files_for_each_ref). So anyone feel free to pick that series up and fix it (or drop it). For now, pretend that I'm kidnapped by aliens. -- Duy ``` ## Manish Goregaokar, 2017-05-22 22:52 Subject: Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: URL: https://gitlist.dev/e/CACpkpxk4X%2BhvmWR_ohHaKpJiiJe5pdZYcdRYjZqk%3DAe0416w2g%40mail.gmail.com In-Reply-To: ``` What work is remaining for prune-in-worktree? Link to the relevant discussions? I might be able to take it over the finish line. (No guarantees) -Manish Goregaokar On Mon, May 22, 2017 at 4:17 AM, Duy Nguyen wrote: > On Sat, May 20, 2017 at 5:30 PM, Junio C Hamano wrote: >> By the way, doesn't nd/prune-in-worktree topic that has been cooking >> in 'pu' supersede this change? It not just protects the commit at >> the tip of HEAD in each worktree, it also makes sure the ones in >> HEAD's reflog are not prematurely pruned. > > You have probably noticed I have stayed silent for a long time (and > probably will continue). But yes nd/prune-in-worktree should fix this. > Unfortunately I will not be able to fix it up (I think Michael > responded on my last question about the proper way to fix > files_for_each_ref). So anyone feel free to pick that series up and > fix it (or drop it). For now, pretend that I'm kidnapped by aliens. > -- > Duy ``` ## Duy Nguyen, 2017-05-25 12:51 Subject: Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs Message-ID: URL: https://gitlist.dev/e/CACsJy8AgAVjCTcvYf_YSiVcZVBr%2BNNJOomVGy%3D2Ej4X22%3Dv6eA%40mail.gmail.com In-Reply-To: ``` On Tue, May 23, 2017 at 5:52 AM, Manish Goregaokar wrote: > What work is remaining for prune-in-worktree? Link to the relevant discussions? > > I might be able to take it over the finish line. (No guarantees) The finish line should be pretty close. I've addressed Michael's other comments except [1]. I pushed what I have done here [2]. The "wip" commit was what I proposed to Michael and I believe he had a better way of doing it I think I had a look at the merge iterator once before writing mine, but maybe I didn't look close enough to see it as reusable in this use case. [1] http://public-inbox.org/git/%3C00720e90-ed85-e8d8-a2e4-f42f93a33d33@alum.mit.edu%3E/#r [2] https://github.com/pclouds/git/commits/prune-in-worktrees-2 -- Duy ```