[PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Manish Goregaokar]

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 <manishearth@gmail.com>
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 <manishearth@gmail.com>
---
 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

Re: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Manish Goregaokar]

From 3f8015efdcc122e0d345baeb5f1f0485a9f0fcd8 Mon Sep 17 00:00:00 2001
From: Manish Goregaokar <manishearth@gmail.com>
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 <manishearth@gmail.com>
---
 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
<manishearth@gmail.com> 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 <manishearth@gmail.com>
> 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 <manishearth@gmail.com>
> ---
>  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

Re: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Jonathan Nieder]

(+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 <manishearth@gmail.com>
> ---
>  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

[PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[manish.earth]

From: Manish Goregaokar <manishearth@gmail.com>

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 <manishearth@gmail.com>
---
 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

[PATCH 2/2] reachable: Add HEADs of all worktrees to reachability analysis

[manish.earth]

From: Manish Goregaokar <manishearth@gmail.com>

* reachable.c:
  mark_reachable_objects: Include other worktrees

Signed-off-by: Manish Goregaokar <manishearth@gmail.com>
---
 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

Re: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Manish Goregaokar]

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,  <manish.earth@gmail.com> wrote:
> From: Manish Goregaokar <manishearth@gmail.com>
>
> 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 <manishearth@gmail.com>
> ---
>  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
>

Re: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Manish Goregaokar]

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
<manish.earth@gmail.com> 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,  <manish.earth@gmail.com> wrote:
>> From: Manish Goregaokar <manishearth@gmail.com>
>>
>> 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 <manishearth@gmail.com>
>> ---
>>  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
>>

Re: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Brandon Williams]

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

Re: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Manish Goregaokar]

> 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 <bmwill@google.com> 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

[PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[manishearth]

From: Manish Goregaokar <manishearth@gmail.com>

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 <manishearth@gmail.com>
---
 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

[PATCH v2 2/2] reachable: Add HEADs of all worktrees to reachability analysis

[manishearth]

From: Manish Goregaokar <manishearth@gmail.com>

* reachable.c:
  mark_reachable_objects: Include other worktrees

Signed-off-by: Manish Goregaokar <manishearth@gmail.com>
---
 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

Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Manish Goregaokar]

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,  <manishearth@gmail.com> wrote:
> From: Manish Goregaokar <manishearth@gmail.com>
>
> 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 <manishearth@gmail.com>
> ---
>  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
>

Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Simon Ruderich]

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

Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Samuel Lijin]

On Thu, May 18, 2017 at 5:40 AM, Simon Ruderich <simon@ruderich.org> 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.

Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Brandon Williams]

On 05/18, Samuel Lijin wrote:
> On Thu, May 18, 2017 at 5:40 AM, Simon Ruderich <simon@ruderich.org> 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

Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Junio C Hamano]

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.

Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Manish Goregaokar]

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 <gitster@pobox.com> 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.
>

Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Junio C Hamano]

Manish Goregaokar <manishearth@gmail.com> 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...

Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Duy Nguyen]

On Sat, May 20, 2017 at 5:30 PM, Junio C Hamano <gitster@pobox.com> 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

Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Manish Goregaokar]

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 <pclouds@gmail.com> wrote:
> On Sat, May 20, 2017 at 5:30 PM, Junio C Hamano <gitster@pobox.com> 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

Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

[Duy Nguyen]

On Tue, May 23, 2017 at 5:52 AM, Manish Goregaokar
<manishearth@gmail.com> 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