git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
* [PATCH 0/8] various wt-status/worktree cleanups
@ 2020-09-10 19:03 Martin Ågren
  2020-09-10 19:03 ` [PATCH 1/8] wt-status: replace sha1 mentions with oid Martin Ågren
                   ` (8 more replies)
  0 siblings, 9 replies; 21+ messages in thread
From: Martin Ågren @ 2020-09-10 19:03 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Junio noticed that wt-status.c is a bit inconsistent about printing to
`s->fp` vs `stdout`. That's patch two. The third patch tries to
robustify some freeing of memory. While working on that one, I ended up
in the sibling file worktree.c.

This merges pretty well with `seen` (`jc/quote-path-cleanup`) (and the
tests still pass). These patches are obviously low priority, nothing
revolutionary here.

Martin Ågren (8):
  wt-status: replace sha1 mentions with oid
  wt-status: print to s->fp, not stdout
  wt-status: introduce wt_status_state_free_buffers()
  worktree: drop useless call to strbuf_reset
  worktree: update renamed variable in comment
  worktree: rename copy-pasted variable
  worktree: use skip_prefix to parse target
  worktree: simplify search for unique worktree

 wt-status.h  |  7 +++++
 ref-filter.c |  4 +--
 worktree.c   | 39 +++++++++++++--------------
 wt-status.c  | 76 ++++++++++++++++++++++++++++++----------------------
 4 files changed, 71 insertions(+), 55 deletions(-)

-- 
2.28.0.277.g9b3c35fffd


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/8] wt-status: replace sha1 mentions with oid
  2020-09-10 19:03 [PATCH 0/8] various wt-status/worktree cleanups Martin Ågren
@ 2020-09-10 19:03 ` Martin Ågren
  2020-09-10 19:03 ` [PATCH 2/8] wt-status: print to s->fp, not stdout Martin Ågren
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Martin Ågren @ 2020-09-10 19:03 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

`abbrev_sha1_in_line()` uses a `struct object_id oid` and should be
fully prepared to handle non-SHA1 object ids. Rename it to
`abbrev_oid_in_line()`.

A few comments in `wt_status_get_detached_from()` mention "sha1". The
variable they refer to was renamed in e86ab2c1cd ("wt-status: convert to
struct object_id", 2017-02-21). Update the comments to reference "oid"
instead.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 wt-status.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index c560cbe860..59be457015 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1232,7 +1232,7 @@ static int split_commit_in_progress(struct wt_status *s)
  * The function assumes that the line does not contain useless spaces
  * before or after the command.
  */
-static void abbrev_sha1_in_line(struct strbuf *line)
+static void abbrev_oid_in_line(struct strbuf *line)
 {
 	struct strbuf **split;
 	int i;
@@ -1282,7 +1282,7 @@ static int read_rebase_todolist(const char *fname, struct string_list *lines)
 		strbuf_trim(&line);
 		if (!line.len)
 			continue;
-		abbrev_sha1_in_line(&line);
+		abbrev_oid_in_line(&line);
 		string_list_append(lines, line.buf);
 	}
 	fclose(f);
@@ -1575,9 +1575,9 @@ static void wt_status_get_detached_from(struct repository *r,
 	}
 
 	if (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&
-	    /* sha1 is a commit? match without further lookup */
+	    /* oid is a commit? match without further lookup */
 	    (oideq(&cb.noid, &oid) ||
-	     /* perhaps sha1 is a tag, try to dereference to a commit */
+	     /* perhaps oid is a tag, try to dereference to a commit */
 	     ((commit = lookup_commit_reference_gently(r, &oid, 1)) != NULL &&
 	      oideq(&cb.noid, &commit->object.oid)))) {
 		const char *from = ref;
-- 
2.28.0.277.g9b3c35fffd


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/8] wt-status: print to s->fp, not stdout
  2020-09-10 19:03 [PATCH 0/8] various wt-status/worktree cleanups Martin Ågren
  2020-09-10 19:03 ` [PATCH 1/8] wt-status: replace sha1 mentions with oid Martin Ågren
@ 2020-09-10 19:03 ` Martin Ågren
  2020-09-10 19:03 ` [PATCH 3/8] wt-status: introduce wt_status_state_free_buffers() Martin Ågren
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Martin Ågren @ 2020-09-10 19:03 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

We pass around a `FILE *` in the `struct wt_status` and almost always
print to it. But in a few places, we write to `stdout` instead, either
explicitly through `fprintf(stdout, ...)` or implicitly with
`printf(...)` (and a few `putchar(...)`).

Always be explicit about writing to `s->fp`. To the best of my
understanding, this never mattered in practice because these spots are
involved in various forms of `git status` which always end up at
standard output anyway. When we do write to another file, it's because
we're creating a commit message template, and these code paths aren't
involved.

But let's be consistent to help future readers and avoid future bugs.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 wt-status.c | 57 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 59be457015..3e0b5d8017 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1806,29 +1806,36 @@ static void wt_longstatus_print(struct wt_status *s)
 			; /* nothing */
 		else if (s->workdir_dirty) {
 			if (s->hints)
-				printf(_("no changes added to commit "
-					 "(use \"git add\" and/or \"git commit -a\")\n"));
+				fprintf(s->fp, _("no changes added to commit "
+						 "(use \"git add\" and/or "
+						 "\"git commit -a\")\n"));
 			else
-				printf(_("no changes added to commit\n"));
+				fprintf(s->fp, _("no changes added to "
+						 "commit\n"));
 		} else if (s->untracked.nr) {
 			if (s->hints)
-				printf(_("nothing added to commit but untracked files "
-					 "present (use \"git add\" to track)\n"));
+				fprintf(s->fp, _("nothing added to commit but "
+						 "untracked files present (use "
+						 "\"git add\" to track)\n"));
 			else
-				printf(_("nothing added to commit but untracked files present\n"));
+				fprintf(s->fp, _("nothing added to commit but "
+						 "untracked files present\n"));
 		} else if (s->is_initial) {
 			if (s->hints)
-				printf(_("nothing to commit (create/copy files "
-					 "and use \"git add\" to track)\n"));
+				fprintf(s->fp, _("nothing to commit (create/"
+						 "copy files and use \"git "
+						 "add\" to track)\n"));
 			else
-				printf(_("nothing to commit\n"));
+				fprintf(s->fp, _("nothing to commit\n"));
 		} else if (!s->show_untracked_files) {
 			if (s->hints)
-				printf(_("nothing to commit (use -u to show untracked files)\n"));
+				fprintf(s->fp, _("nothing to commit (use -u to "
+						 "show untracked files)\n"));
 			else
-				printf(_("nothing to commit\n"));
+				fprintf(s->fp, _("nothing to commit\n"));
 		} else
-			printf(_("nothing to commit, working tree clean\n"));
+			fprintf(s->fp, _("nothing to commit, working tree "
+					 "clean\n"));
 	}
 	if(s->show_stash)
 		wt_longstatus_print_stash_summary(s);
@@ -1851,12 +1858,12 @@ static void wt_shortstatus_unmerged(struct string_list_item *it,
 	}
 	color_fprintf(s->fp, color(WT_STATUS_UNMERGED, s), "%s", how);
 	if (s->null_termination) {
-		fprintf(stdout, " %s%c", it->string, 0);
+		fprintf(s->fp, " %s%c", it->string, 0);
 	} else {
 		struct strbuf onebuf = STRBUF_INIT;
 		const char *one;
 		one = quote_path(it->string, s->prefix, &onebuf);
-		printf(" %s\n", one);
+		fprintf(s->fp, " %s\n", one);
 		strbuf_release(&onebuf);
 	}
 }
@@ -1869,16 +1876,16 @@ static void wt_shortstatus_status(struct string_list_item *it,
 	if (d->index_status)
 		color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%c", d->index_status);
 	else
-		putchar(' ');
+		fputc(' ', s->fp);
 	if (d->worktree_status)
 		color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%c", d->worktree_status);
 	else
-		putchar(' ');
-	putchar(' ');
+		fputc(' ', s->fp);
+	fputc(' ', s->fp);
 	if (s->null_termination) {
-		fprintf(stdout, "%s%c", it->string, 0);
+		fprintf(s->fp, "%s%c", it->string, 0);
 		if (d->rename_source)
-			fprintf(stdout, "%s%c", d->rename_source, 0);
+			fprintf(s->fp, "%s%c", d->rename_source, 0);
 	} else {
 		struct strbuf onebuf = STRBUF_INIT;
 		const char *one;
@@ -1886,20 +1893,20 @@ static void wt_shortstatus_status(struct string_list_item *it,
 		if (d->rename_source) {
 			one = quote_path(d->rename_source, s->prefix, &onebuf);
 			if (*one != '"' && strchr(one, ' ') != NULL) {
-				putchar('"');
+				fputc('"', s->fp);
 				strbuf_addch(&onebuf, '"');
 				one = onebuf.buf;
 			}
-			printf("%s -> ", one);
+			fprintf(s->fp, "%s -> ", one);
 			strbuf_release(&onebuf);
 		}
 		one = quote_path(it->string, s->prefix, &onebuf);
 		if (*one != '"' && strchr(one, ' ') != NULL) {
-			putchar('"');
+			fputc('"', s->fp);
 			strbuf_addch(&onebuf, '"');
 			one = onebuf.buf;
 		}
-		printf("%s\n", one);
+		fprintf(s->fp, "%s\n", one);
 		strbuf_release(&onebuf);
 	}
 }
@@ -1908,13 +1915,13 @@ static void wt_shortstatus_other(struct string_list_item *it,
 				 struct wt_status *s, const char *sign)
 {
 	if (s->null_termination) {
-		fprintf(stdout, "%s %s%c", sign, it->string, 0);
+		fprintf(s->fp, "%s %s%c", sign, it->string, 0);
 	} else {
 		struct strbuf onebuf = STRBUF_INIT;
 		const char *one;
 		one = quote_path(it->string, s->prefix, &onebuf);
 		color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", sign);
-		printf(" %s\n", one);
+		fprintf(s->fp, " %s\n", one);
 		strbuf_release(&onebuf);
 	}
 }
-- 
2.28.0.277.g9b3c35fffd


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 3/8] wt-status: introduce wt_status_state_free_buffers()
  2020-09-10 19:03 [PATCH 0/8] various wt-status/worktree cleanups Martin Ågren
  2020-09-10 19:03 ` [PATCH 1/8] wt-status: replace sha1 mentions with oid Martin Ågren
  2020-09-10 19:03 ` [PATCH 2/8] wt-status: print to s->fp, not stdout Martin Ågren
@ 2020-09-10 19:03 ` Martin Ågren
  2020-09-10 19:03 ` [PATCH 4/8] worktree: drop useless call to strbuf_reset Martin Ågren
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Martin Ågren @ 2020-09-10 19:03 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

When we have a `struct wt_status_state`, we manually free its `branch`,
`onto` and `detached_from`, or sometimes just one or two of them.
Provide a function `wt_status_state_free_buffers()` which does the
freeing.

The callers are still aware of these fields, e.g., they check whether
`branch` was populated or not. But this way, they don't need to know
about *all* of them, and if `struct wt_status_state` gets more fields,
they will not need to learn to free them.

Users of `struct wt_status` (which contains a `wt_status_state`) already
have `wt_status_collect_free_buffers()` (corresponding to
`wt_status_collect()`) which we can also teach to use this new helper.

Finally, note that we're currently leaving dangling pointers behind.
Some callers work on a stack-allocated struct, where this is obviously
ok. But for the users of `run_status()` in builtin/commit.c, there are
ample opportunities for someone to mistakenly use those dangling
pointers. We seem to be ok for now, but it's a use-after-free waiting to
happen. Let's leave NULL-pointers behind instead.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 wt-status.h  |  7 +++++++
 ref-filter.c |  4 +---
 worktree.c   |  5 ++---
 wt-status.c  | 11 ++++++++---
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/wt-status.h b/wt-status.h
index f1fa0ec1a7..35b44c388e 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -151,7 +151,14 @@ void wt_status_add_cut_line(FILE *fp);
 void wt_status_prepare(struct repository *r, struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
+/*
+ * Frees the buffers allocated by wt_status_collect.
+ */
 void wt_status_collect_free_buffers(struct wt_status *s);
+/*
+ * Frees the buffers of the wt_status_state.
+ */
+void wt_status_state_free_buffers(struct wt_status_state *s);
 void wt_status_get_state(struct repository *repo,
 			 struct wt_status_state *state,
 			 int get_detached_from);
diff --git a/ref-filter.c b/ref-filter.c
index 8447cb09be..3f63bb01de 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1502,9 +1502,7 @@ char *get_head_description(void)
 		strbuf_addstr(&desc, _("no branch"));
 	strbuf_addch(&desc, ')');
 
-	free(state.branch);
-	free(state.onto);
-	free(state.detached_from);
+	wt_status_state_free_buffers(&state);
 	return strbuf_detach(&desc, NULL);
 }
 
diff --git a/worktree.c b/worktree.c
index cba2e54598..23dd547e44 100644
--- a/worktree.c
+++ b/worktree.c
@@ -369,8 +369,7 @@ int is_worktree_being_rebased(const struct worktree *wt,
 		 state.branch &&
 		 starts_with(target, "refs/heads/") &&
 		 !strcmp(state.branch, target + strlen("refs/heads/")));
-	free(state.branch);
-	free(state.onto);
+	wt_status_state_free_buffers(&state);
 	return found_rebase;
 }
 
@@ -385,7 +384,7 @@ int is_worktree_being_bisected(const struct worktree *wt,
 		state.branch &&
 		starts_with(target, "refs/heads/") &&
 		!strcmp(state.branch, target + strlen("refs/heads/"));
-	free(state.branch);
+	wt_status_state_free_buffers(&state);
 	return found_rebase;
 }
 
diff --git a/wt-status.c b/wt-status.c
index 3e0b5d8017..5c4cc4805f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -782,9 +782,14 @@ void wt_status_collect(struct wt_status *s)
 
 void wt_status_collect_free_buffers(struct wt_status *s)
 {
-	free(s->state.branch);
-	free(s->state.onto);
-	free(s->state.detached_from);
+	wt_status_state_free_buffers(&s->state);
+}
+
+void wt_status_state_free_buffers(struct wt_status_state *state)
+{
+	FREE_AND_NULL(state->branch);
+	FREE_AND_NULL(state->onto);
+	FREE_AND_NULL(state->detached_from);
 }
 
 static void wt_longstatus_print_unmerged(struct wt_status *s)
-- 
2.28.0.277.g9b3c35fffd


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 4/8] worktree: drop useless call to strbuf_reset
  2020-09-10 19:03 [PATCH 0/8] various wt-status/worktree cleanups Martin Ågren
                   ` (2 preceding siblings ...)
  2020-09-10 19:03 ` [PATCH 3/8] wt-status: introduce wt_status_state_free_buffers() Martin Ågren
@ 2020-09-10 19:03 ` Martin Ågren
  2020-09-10 19:15   ` Eric Sunshine
  2020-09-10 19:03 ` [PATCH 5/8] worktree: update renamed variable in comment Martin Ågren
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Martin Ågren @ 2020-09-10 19:03 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

There's no need to reset the strbuf immediately after initializing it.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 worktree.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/worktree.c b/worktree.c
index 23dd547e44..64a9e78997 100644
--- a/worktree.c
+++ b/worktree.c
@@ -552,7 +552,6 @@ const char *worktree_ref(const struct worktree *wt, const char *refname)
 {
 	static struct strbuf sb = STRBUF_INIT;
 
-	strbuf_reset(&sb);
 	strbuf_worktree_ref(wt, &sb, refname);
 	return sb.buf;
 }
-- 
2.28.0.277.g9b3c35fffd


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 5/8] worktree: update renamed variable in comment
  2020-09-10 19:03 [PATCH 0/8] various wt-status/worktree cleanups Martin Ågren
                   ` (3 preceding siblings ...)
  2020-09-10 19:03 ` [PATCH 4/8] worktree: drop useless call to strbuf_reset Martin Ågren
@ 2020-09-10 19:03 ` Martin Ågren
  2020-09-10 19:03 ` [PATCH 6/8] worktree: rename copy-pasted variable Martin Ågren
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Martin Ågren @ 2020-09-10 19:03 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

The comment above `add_head_info()` mentions "head_sha1", but it was
renamed to "head_oid" in 0f05154c70 ("worktree: convert struct worktree
to object_id", 2017-10-15). Update the comment.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 worktree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/worktree.c b/worktree.c
index 64a9e78997..050f22dd65 100644
--- a/worktree.c
+++ b/worktree.c
@@ -21,7 +21,7 @@ void free_worktrees(struct worktree **worktrees)
 }
 
 /**
- * Update head_sha1, head_ref and is_detached of the given worktree
+ * Update head_oid, head_ref and is_detached of the given worktree
  */
 static void add_head_info(struct worktree *wt)
 {
-- 
2.28.0.277.g9b3c35fffd


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 6/8] worktree: rename copy-pasted variable
  2020-09-10 19:03 [PATCH 0/8] various wt-status/worktree cleanups Martin Ågren
                   ` (4 preceding siblings ...)
  2020-09-10 19:03 ` [PATCH 5/8] worktree: update renamed variable in comment Martin Ågren
@ 2020-09-10 19:03 ` Martin Ågren
  2020-09-10 20:29   ` Junio C Hamano
  2020-09-10 19:03 ` [PATCH 7/8] worktree: use skip_prefix to parse target Martin Ågren
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Martin Ågren @ 2020-09-10 19:03 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

As the commit message of 04a3dfb8b5 ("worktree.c: check whether branch
is bisected in another worktree", 2016-04-22) indicates, the function
`is_worktree_being_bisected()` is based on the older function
`is_worktree_being_rebased()`. This heritage can also be seen in the
name of the variable where we store our return value: It was never
adapted while copy-editing and remains as `found_rebase`.

Rename the variable to make clear that we're looking for a bisect(ion),
nothing else.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 worktree.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/worktree.c b/worktree.c
index 050f22dd65..a2d0d20564 100644
--- a/worktree.c
+++ b/worktree.c
@@ -377,15 +377,15 @@ int is_worktree_being_bisected(const struct worktree *wt,
 			       const char *target)
 {
 	struct wt_status_state state;
-	int found_rebase;
+	int found_bisect;
 
 	memset(&state, 0, sizeof(state));
-	found_rebase = wt_status_check_bisect(wt, &state) &&
-		state.branch &&
-		starts_with(target, "refs/heads/") &&
-		!strcmp(state.branch, target + strlen("refs/heads/"));
+	found_bisect = wt_status_check_bisect(wt, &state) &&
+		       state.branch &&
+		       starts_with(target, "refs/heads/") &&
+		       !strcmp(state.branch, target + strlen("refs/heads/"));
 	wt_status_state_free_buffers(&state);
-	return found_rebase;
+	return found_bisect;
 }
 
 /*
-- 
2.28.0.277.g9b3c35fffd


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 7/8] worktree: use skip_prefix to parse target
  2020-09-10 19:03 [PATCH 0/8] various wt-status/worktree cleanups Martin Ågren
                   ` (5 preceding siblings ...)
  2020-09-10 19:03 ` [PATCH 6/8] worktree: rename copy-pasted variable Martin Ågren
@ 2020-09-10 19:03 ` Martin Ågren
  2020-09-10 19:03 ` [PATCH 8/8] worktree: simplify search for unique worktree Martin Ågren
  2020-09-12  3:49 ` [PATCH 0/8] various wt-status/worktree cleanups Taylor Blau
  8 siblings, 0 replies; 21+ messages in thread
From: Martin Ågren @ 2020-09-10 19:03 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Instead of checking for "refs/heads/" using `starts_with()`, then
skipping past "refs/heads/" using `strlen()`, just use `skip_prefix()`.

In `is_worktree_being_rebased()`, we can adjust the indentation while
we're here and lose a pair of parentheses which isn't needed and which
might even make the reader wonder what they're missing and why that
grouping is there.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 worktree.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/worktree.c b/worktree.c
index a2d0d20564..faac87422c 100644
--- a/worktree.c
+++ b/worktree.c
@@ -364,11 +364,11 @@ int is_worktree_being_rebased(const struct worktree *wt,
 
 	memset(&state, 0, sizeof(state));
 	found_rebase = wt_status_check_rebase(wt, &state) &&
-		((state.rebase_in_progress ||
-		  state.rebase_interactive_in_progress) &&
-		 state.branch &&
-		 starts_with(target, "refs/heads/") &&
-		 !strcmp(state.branch, target + strlen("refs/heads/")));
+		       (state.rebase_in_progress ||
+			state.rebase_interactive_in_progress) &&
+		       state.branch &&
+		       skip_prefix(target, "refs/heads/", &target) &&
+		       !strcmp(state.branch, target);
 	wt_status_state_free_buffers(&state);
 	return found_rebase;
 }
@@ -382,8 +382,8 @@ int is_worktree_being_bisected(const struct worktree *wt,
 	memset(&state, 0, sizeof(state));
 	found_bisect = wt_status_check_bisect(wt, &state) &&
 		       state.branch &&
-		       starts_with(target, "refs/heads/") &&
-		       !strcmp(state.branch, target + strlen("refs/heads/"));
+		       skip_prefix(target, "refs/heads/", &target) &&
+		       !strcmp(state.branch, target);
 	wt_status_state_free_buffers(&state);
 	return found_bisect;
 }
-- 
2.28.0.277.g9b3c35fffd


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 8/8] worktree: simplify search for unique worktree
  2020-09-10 19:03 [PATCH 0/8] various wt-status/worktree cleanups Martin Ågren
                   ` (6 preceding siblings ...)
  2020-09-10 19:03 ` [PATCH 7/8] worktree: use skip_prefix to parse target Martin Ågren
@ 2020-09-10 19:03 ` Martin Ågren
  2020-09-10 19:28   ` Eric Sunshine
  2020-09-12  3:49 ` [PATCH 0/8] various wt-status/worktree cleanups Taylor Blau
  8 siblings, 1 reply; 21+ messages in thread
From: Martin Ågren @ 2020-09-10 19:03 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

We track the number of worktrees we've found and break out of the loop
early once we hit 2. This is because we're not really interested in the
number of matches -- we just want to make sure that we don't find more
than one worktree that matches the suffix. This can be done just as well
by checking the NULL-ness of the pointer where we collect our
answer-to-be. Drop the redundant `nr_found` variable.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 worktree.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/worktree.c b/worktree.c
index faac87422c..ac754b88ff 100644
--- a/worktree.c
+++ b/worktree.c
@@ -172,13 +172,13 @@ static struct worktree *find_worktree_by_suffix(struct worktree **list,
 						const char *suffix)
 {
 	struct worktree *found = NULL;
-	int nr_found = 0, suffixlen;
+	int suffixlen;
 
 	suffixlen = strlen(suffix);
 	if (!suffixlen)
 		return NULL;
 
-	for (; *list && nr_found < 2; list++) {
+	for (; *list; list++) {
 		const char	*path	 = (*list)->path;
 		int		 pathlen = strlen(path);
 		int		 start	 = pathlen - suffixlen;
@@ -186,11 +186,12 @@ static struct worktree *find_worktree_by_suffix(struct worktree **list,
 		/* suffix must start at directory boundary */
 		if ((!start || (start > 0 && is_dir_sep(path[start - 1]))) &&
 		    !fspathcmp(suffix, path + start)) {
+			if (found)
+				return NULL;
 			found = *list;
-			nr_found++;
 		}
 	}
-	return nr_found == 1 ? found : NULL;
+	return found;
 }
 
 struct worktree *find_worktree(struct worktree **list,
-- 
2.28.0.277.g9b3c35fffd


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/8] worktree: drop useless call to strbuf_reset
  2020-09-10 19:03 ` [PATCH 4/8] worktree: drop useless call to strbuf_reset Martin Ågren
@ 2020-09-10 19:15   ` Eric Sunshine
  2020-09-10 19:39     ` Martin Ågren
  0 siblings, 1 reply; 21+ messages in thread
From: Eric Sunshine @ 2020-09-10 19:15 UTC (permalink / raw)
  To: Martin Ågren; +Cc: Git List, Junio C Hamano

On Thu, Sep 10, 2020 at 3:08 PM Martin Ågren <martin.agren@gmail.com> wrote:
> There's no need to reset the strbuf immediately after initializing it.
>
> Signed-off-by: Martin Ågren <martin.agren@gmail.com>
> ---
> diff --git a/worktree.c b/worktree.c
> @@ -552,7 +552,6 @@ const char *worktree_ref(const struct worktree *wt, const char *refname)
>  {
>         static struct strbuf sb = STRBUF_INIT;
>
> -       strbuf_reset(&sb);
>         strbuf_worktree_ref(wt, &sb, refname);
>         return sb.buf;
>  }

I think this patch is wrong and should be dropped. That strbuf is
static, and the called strbuf_worktree_ref() does not reset it, so
each call to worktree_ref() will now merely append to the existing
content (which is undesirable) following this change.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 8/8] worktree: simplify search for unique worktree
  2020-09-10 19:03 ` [PATCH 8/8] worktree: simplify search for unique worktree Martin Ågren
@ 2020-09-10 19:28   ` Eric Sunshine
  2020-09-10 19:48     ` Martin Ågren
  0 siblings, 1 reply; 21+ messages in thread
From: Eric Sunshine @ 2020-09-10 19:28 UTC (permalink / raw)
  To: Martin Ågren; +Cc: Git List, Junio C Hamano

On Thu, Sep 10, 2020 at 3:08 PM Martin Ågren <martin.agren@gmail.com> wrote:
> We track the number of worktrees we've found and break out of the loop
> early once we hit 2. This is because we're not really interested in the
> number of matches -- we just want to make sure that we don't find more
> than one worktree that matches the suffix. This can be done just as well
> by checking the NULL-ness of the pointer where we collect our
> answer-to-be. Drop the redundant `nr_found` variable.
>
> Signed-off-by: Martin Ågren <martin.agren@gmail.com>
> ---
> diff --git a/worktree.c b/worktree.c
> @@ -172,13 +172,13 @@ static struct worktree *find_worktree_by_suffix(struct worktree **list,
>                                                 const char *suffix)
>  {
> -       for (; *list && nr_found < 2; list++) {
> +       for (; *list; list++) {
> @@ -186,11 +186,12 @@ static struct worktree *find_worktree_by_suffix(struct worktree **list,
>                 if ((!start || (start > 0 && is_dir_sep(path[start - 1]))) &&
>                     !fspathcmp(suffix, path + start)) {
> +                       if (found)
> +                               return NULL;
>                         found = *list;
> -                       nr_found++;
>                 }
>         }
> -       return nr_found == 1 ? found : NULL;
> +       return found;

Although this change appears to be correct and does simplify the code,
I think it also makes it a bit more opaque. With the explicit
`nr_found == 1`, it is quite obvious that the function considers
"success" to be when one and only one entry is found and any other
number is failure. But with the revised code, it is harder to work out
precisely what the conditions are. Having said that, I think a simple
comment before the function would suffice to clear up the opaqueness.
Perhaps something like:

    /* If exactly one worktree matches 'target', return it, else NULL. */

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/8] worktree: drop useless call to strbuf_reset
  2020-09-10 19:15   ` Eric Sunshine
@ 2020-09-10 19:39     ` Martin Ågren
  2020-09-10 19:49       ` Eric Sunshine
  0 siblings, 1 reply; 21+ messages in thread
From: Martin Ågren @ 2020-09-10 19:39 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Git List, Junio C Hamano

On Thu, 10 Sep 2020 at 21:15, Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Thu, Sep 10, 2020 at 3:08 PM Martin Ågren <martin.agren@gmail.com> wrote:
> > There's no need to reset the strbuf immediately after initializing it.
> >
> > Signed-off-by: Martin Ågren <martin.agren@gmail.com>
> > ---
> > diff --git a/worktree.c b/worktree.c
> > @@ -552,7 +552,6 @@ const char *worktree_ref(const struct worktree *wt, const char *refname)
> >  {
> >         static struct strbuf sb = STRBUF_INIT;
> >
> > -       strbuf_reset(&sb);
> >         strbuf_worktree_ref(wt, &sb, refname);
> >         return sb.buf;
> >  }
>
> I think this patch is wrong and should be dropped. That strbuf is
> static, and the called strbuf_worktree_ref() does not reset it, so
> each call to worktree_ref() will now merely append to the existing
> content (which is undesirable) following this change.

Oh wow, that's embarrassing. Thank you so much for spotting.

I wonder how many worktrees you need before this optimization to avoid
continuous allocation starts paying off. I guess our test runs never
actually hit this. Unless the tests are loose enough that my bug manages
to go unnoticed even if it actually triggers during a test run.

That's not to say this optimization won't ever be useful, of course. I
also begin to hope that no caller keeps their returned pointer around
for long. It only seems to be used from `other_ref_heads()` and that
looks ok. If we do want this strbuf reuse, maybe that function could
just keep its own strbuf and reuse it (not necessarily having it be
static) and learn not to call `worktree_ref(wt, "HEAD")` twice.

But anyway, this patch should definitely be dropped.

Thanks
Martin

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 8/8] worktree: simplify search for unique worktree
  2020-09-10 19:28   ` Eric Sunshine
@ 2020-09-10 19:48     ` Martin Ågren
  2020-09-10 20:01       ` Eric Sunshine
  0 siblings, 1 reply; 21+ messages in thread
From: Martin Ågren @ 2020-09-10 19:48 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Git List, Junio C Hamano

On Thu, 10 Sep 2020 at 21:28, Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Thu, Sep 10, 2020 at 3:08 PM Martin Ågren <martin.agren@gmail.com> wrote:
> > We track the number of worktrees we've found and break out of the loop
> > early once we hit 2. This is because we're not really interested in the
> > number of matches -- we just want to make sure that we don't find more
> > than one worktree that matches the suffix. This can be done just as well
> > by checking the NULL-ness of the pointer where we collect our
> > answer-to-be. Drop the redundant `nr_found` variable.
> >
> > Signed-off-by: Martin Ågren <martin.agren@gmail.com>
> > ---
> > diff --git a/worktree.c b/worktree.c
> > @@ -172,13 +172,13 @@ static struct worktree *find_worktree_by_suffix(struct worktree **list,
> >                                                 const char *suffix)
> >  {
> > -       for (; *list && nr_found < 2; list++) {
> > +       for (; *list; list++) {
> > @@ -186,11 +186,12 @@ static struct worktree *find_worktree_by_suffix(struct worktree **list,
> >                 if ((!start || (start > 0 && is_dir_sep(path[start - 1]))) &&
> >                     !fspathcmp(suffix, path + start)) {
> > +                       if (found)
> > +                               return NULL;
> >                         found = *list;
> > -                       nr_found++;
> >                 }
> >         }
> > -       return nr_found == 1 ? found : NULL;
> > +       return found;
>
> Although this change appears to be correct and does simplify the code,
> I think it also makes it a bit more opaque. With the explicit
> `nr_found == 1`, it is quite obvious that the function considers
> "success" to be when one and only one entry is found and any other
> number is failure. But with the revised code, it is harder to work out
> precisely what the conditions are.

Thanks for commenting. I found the original trickier than it had to be.
It spreads out the logic in several places and is careful to short-cut
the loop. My first thought was "why doesn't this just use the standard
form?". But I'm open to the idea that it might be a fairly personal
standard form... If there's any risk that someone else comes along and
simplifies this to use a `nr_found` variable, then maybe file this under
code churning?

> Having said that, I think a simple
> comment before the function would suffice to clear up the opaqueness.
> Perhaps something like:
>
>     /* If exactly one worktree matches 'target', return it, else NULL. */

That's a good suggestion regardless.

Thanks
Martin

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/8] worktree: drop useless call to strbuf_reset
  2020-09-10 19:39     ` Martin Ågren
@ 2020-09-10 19:49       ` Eric Sunshine
  2020-09-12 14:02         ` Martin Ågren
  0 siblings, 1 reply; 21+ messages in thread
From: Eric Sunshine @ 2020-09-10 19:49 UTC (permalink / raw)
  To: Martin Ågren; +Cc: Git List, Junio C Hamano

On Thu, Sep 10, 2020 at 3:40 PM Martin Ågren <martin.agren@gmail.com> wrote:
> On Thu, 10 Sep 2020 at 21:15, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > I think this patch is wrong and should be dropped. That strbuf is
> > static, and the called strbuf_worktree_ref() does not reset it, so
> > each call to worktree_ref() will now merely append to the existing
> > content (which is undesirable) following this change.
>
> That's not to say this optimization won't ever be useful, of course. I
> also begin to hope that no caller keeps their returned pointer around
> for long. It only seems to be used from `other_ref_heads()` and that
> looks ok. If we do want this strbuf reuse, maybe that function could
> just keep its own strbuf and reuse it (not necessarily having it be
> static) and learn not to call `worktree_ref(wt, "HEAD")` twice.

Yep, I wouldn't be unhappy to see worktree_ref() disappear altogether.
There are no external callers and it would be easy enough to retrofit
the lone internal caller to use the safer strbuf_worktree_ref()
anyhow. Plus, both calls to worktree_ref() in other_head_refs() invoke
it with the exact same arguments, `worktree_ref(wt, "HEAD")`, which
makes one wonder if it need be called twice at all in that particular
scenario.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 8/8] worktree: simplify search for unique worktree
  2020-09-10 19:48     ` Martin Ågren
@ 2020-09-10 20:01       ` Eric Sunshine
  2020-09-10 21:08         ` Junio C Hamano
  0 siblings, 1 reply; 21+ messages in thread
From: Eric Sunshine @ 2020-09-10 20:01 UTC (permalink / raw)
  To: Martin Ågren; +Cc: Git List, Junio C Hamano

On Thu, Sep 10, 2020 at 3:48 PM Martin Ågren <martin.agren@gmail.com> wrote:
> On Thu, 10 Sep 2020 at 21:28, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > Although this change appears to be correct and does simplify the code,
> > I think it also makes it a bit more opaque. With the explicit
> > `nr_found == 1`, it is quite obvious that the function considers
> > "success" to be when one and only one entry is found and any other
> > number is failure. But with the revised code, it is harder to work out
> > precisely what the conditions are.
>
> Thanks for commenting. I found the original trickier than it had to be.
> It spreads out the logic in several places and is careful to short-cut
> the loop. My first thought was "why doesn't this just use the standard
> form?". But I'm open to the idea that it might be a fairly personal
> standard form... If there's any risk that someone else comes along and
> simplifies this to use a `nr_found` variable, then maybe file this under
> code churning?

Maybe. Dunno. Even with the suggested function comment, I still find
that the revised code has a higher cognitive load because the reader
has to remember or figure out mentally what `found` contains by the
`return found;` at the end of the function. Compare that with the old
code, in which the reader doesn't have to remember or figure out
anything. The final `return nr_found == 1 ? found : NULL;` condition
spells out everything the reader needs to know -- even if the reader
didn't pay close attention to the meat of the function. So, we each
have a different take on the apparent complexity.

> > Having said that, I think a simple
> > comment before the function would suffice to clear up the opaqueness.
> > Perhaps something like:
> >
> >     /* If exactly one worktree matches 'target', return it, else NULL. */
>
> That's a good suggestion regardless.

The function is so small that the increased cognitive load (for me) in
the rewrite probably doesn't matter at all, so I don't feel strongly
one way or the other. Keeping the patch (amended with the suggested
comment) or dropping it are both suitable options.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] worktree: rename copy-pasted variable
  2020-09-10 19:03 ` [PATCH 6/8] worktree: rename copy-pasted variable Martin Ågren
@ 2020-09-10 20:29   ` Junio C Hamano
  2020-09-12 14:01     ` Martin Ågren
  0 siblings, 1 reply; 21+ messages in thread
From: Junio C Hamano @ 2020-09-10 20:29 UTC (permalink / raw)
  To: Martin Ågren; +Cc: git

Martin Ågren <martin.agren@gmail.com> writes:

> As the commit message of 04a3dfb8b5 ("worktree.c: check whether branch
> is bisected in another worktree", 2016-04-22) indicates, the function
> `is_worktree_being_bisected()` is based on the older function
> `is_worktree_being_rebased()`. This heritage can also be seen in the
> name of the variable where we store our return value: It was never
> adapted while copy-editing and remains as `found_rebase`.
>
> Rename the variable to make clear that we're looking for a bisect(ion),
> nothing else.

How bad is this copy and paste?  Is it a possibility to make a
single helper function and these existing two a thin wrapper around
the helper that passes customization between bisect and rebase?


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 8/8] worktree: simplify search for unique worktree
  2020-09-10 20:01       ` Eric Sunshine
@ 2020-09-10 21:08         ` Junio C Hamano
  0 siblings, 0 replies; 21+ messages in thread
From: Junio C Hamano @ 2020-09-10 21:08 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Martin Ågren, Git List

Eric Sunshine <sunshine@sunshineco.com> writes:

>> Thanks for commenting. I found the original trickier than it had to be.
>> It spreads out the logic in several places and is careful to short-cut
>> the loop. My first thought was "why doesn't this just use the standard
>> form?". But I'm open to the idea that it might be a fairly personal
>> standard form... If there's any risk that someone else comes along and
>> simplifies this to use a `nr_found` variable, then maybe file this under
>> code churning?
>
> Maybe. Dunno. Even with the suggested function comment, I still find
> that the revised code has a higher cognitive load because the reader
> has to remember or figure out mentally what `found` contains by the
> `return found;` at the end of the function.

Judging from the phrase "standard form", I have a feeling that
Martin thinks that the updated code is more intuitive (i.e. "we see
another one while keeping the one we already found, so we know there
is no unique answer").  I do not claim that would be the standard way
and using a counter clamped to 2 is substandard, but I find the code
more readable with the patch than the original.

Even though it helps somewhat that the counter is named "nr_found"
and not "nr_match", I found it a bit awkward that the counter in the
original pretends to count all the answers, yet does not really
count all of them and stops at 2.

I agree with Martin that this would probably be subjective.

Thanks.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/8] various wt-status/worktree cleanups
  2020-09-10 19:03 [PATCH 0/8] various wt-status/worktree cleanups Martin Ågren
                   ` (7 preceding siblings ...)
  2020-09-10 19:03 ` [PATCH 8/8] worktree: simplify search for unique worktree Martin Ågren
@ 2020-09-12  3:49 ` Taylor Blau
  2020-09-12 14:03   ` Martin Ågren
  8 siblings, 1 reply; 21+ messages in thread
From: Taylor Blau @ 2020-09-12  3:49 UTC (permalink / raw)
  To: Martin Ågren; +Cc: git, Junio C Hamano

On Thu, Sep 10, 2020 at 09:03:34PM +0200, Martin Ågren wrote:
> This merges pretty well with `seen` (`jc/quote-path-cleanup`) (and the
> tests still pass). These patches are obviously low priority, nothing
> revolutionary here.

What revision is this series based on, again? Applying the first patch
on top of 'seen' (bf3e2864f3, at the time of writing) produces a
conflict due to the dropped parameter in 'dwim_ref()'.

But, applying in directly on top of Junio's 'jc/quote-path-cleanup' from
his repository handles the first patch just fine, but fails in the
second patch. The conflicts here are in 'wt_shortstatus_unmerged' and
'wt_shortstatus_status'.

I didn't look deeply to figure out what exactly was going on here, but
it would be good to know so that I can play with these patches a bit
myself.

Thanks,
Taylor

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] worktree: rename copy-pasted variable
  2020-09-10 20:29   ` Junio C Hamano
@ 2020-09-12 14:01     ` Martin Ågren
  0 siblings, 0 replies; 21+ messages in thread
From: Martin Ågren @ 2020-09-12 14:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

On Thu, 10 Sep 2020 at 22:29, Junio C Hamano <gitster@pobox.com> wrote:
>
> Martin Ågren <martin.agren@gmail.com> writes:
>
> > As the commit message of 04a3dfb8b5 ("worktree.c: check whether branch
> > is bisected in another worktree", 2016-04-22) indicates, the function
> > `is_worktree_being_bisected()` is based on the older function
> > `is_worktree_being_rebased()`. This heritage can also be seen in the
> > name of the variable where we store our return value: It was never
> > adapted while copy-editing and remains as `found_rebase`.
> >
> > Rename the variable to make clear that we're looking for a bisect(ion),
> > nothing else.
>
> How bad is this copy and paste?  Is it a possibility to make a
> single helper function and these existing two a thin wrapper around
> the helper that passes customization between bisect and rebase?

That's a good point. I'll look into it.

Thanks
Martin

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/8] worktree: drop useless call to strbuf_reset
  2020-09-10 19:49       ` Eric Sunshine
@ 2020-09-12 14:02         ` Martin Ågren
  0 siblings, 0 replies; 21+ messages in thread
From: Martin Ågren @ 2020-09-12 14:02 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Git List, Junio C Hamano

On Thu, 10 Sep 2020 at 21:49, Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Thu, Sep 10, 2020 at 3:40 PM Martin Ågren <martin.agren@gmail.com> wrote:
> > That's not to say this optimization won't ever be useful, of course. I
> > also begin to hope that no caller keeps their returned pointer around
> > for long. It only seems to be used from `other_ref_heads()` and that
> > looks ok. If we do want this strbuf reuse, maybe that function could
> > just keep its own strbuf and reuse it (not necessarily having it be
> > static) and learn not to call `worktree_ref(wt, "HEAD")` twice.
>
> Yep, I wouldn't be unhappy to see worktree_ref() disappear altogether.
> There are no external callers and it would be easy enough to retrofit
> the lone internal caller to use the safer strbuf_worktree_ref()
> anyhow. Plus, both calls to worktree_ref() in other_head_refs() invoke
> it with the exact same arguments, `worktree_ref(wt, "HEAD")`, which
> makes one wonder if it need be called twice at all in that particular
> scenario.

I'll look at this hopefully tomorrow. Thanks for all your comments.


Martin

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/8] various wt-status/worktree cleanups
  2020-09-12  3:49 ` [PATCH 0/8] various wt-status/worktree cleanups Taylor Blau
@ 2020-09-12 14:03   ` Martin Ågren
  0 siblings, 0 replies; 21+ messages in thread
From: Martin Ågren @ 2020-09-12 14:03 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Git Mailing List, Junio C Hamano

On Sat, 12 Sep 2020 at 05:49, Taylor Blau <me@ttaylorr.com> wrote:
>
> On Thu, Sep 10, 2020 at 09:03:34PM +0200, Martin Ågren wrote:
> > This merges pretty well with `seen` (`jc/quote-path-cleanup`) (and the
> > tests still pass). These patches are obviously low priority, nothing
> > revolutionary here.
>
> What revision is this series based on, again? [ ... ]

This is on top of current maint: 47ae905ffb ("Git 2.28", 2020-07-26)

> I didn't look deeply to figure out what exactly was going on here, but
> it would be good to know so that I can play with these patches a bit
> myself.

Ok, beware that there's at least one bug, so you're better off dropping
"worktree: drop useless call to strbuf_reset". I'll probably replace it
with something to remove that function and basically inline it into
its only caller.


Martin

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2020-09-12 14:04 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-10 19:03 [PATCH 0/8] various wt-status/worktree cleanups Martin Ågren
2020-09-10 19:03 ` [PATCH 1/8] wt-status: replace sha1 mentions with oid Martin Ågren
2020-09-10 19:03 ` [PATCH 2/8] wt-status: print to s->fp, not stdout Martin Ågren
2020-09-10 19:03 ` [PATCH 3/8] wt-status: introduce wt_status_state_free_buffers() Martin Ågren
2020-09-10 19:03 ` [PATCH 4/8] worktree: drop useless call to strbuf_reset Martin Ågren
2020-09-10 19:15   ` Eric Sunshine
2020-09-10 19:39     ` Martin Ågren
2020-09-10 19:49       ` Eric Sunshine
2020-09-12 14:02         ` Martin Ågren
2020-09-10 19:03 ` [PATCH 5/8] worktree: update renamed variable in comment Martin Ågren
2020-09-10 19:03 ` [PATCH 6/8] worktree: rename copy-pasted variable Martin Ågren
2020-09-10 20:29   ` Junio C Hamano
2020-09-12 14:01     ` Martin Ågren
2020-09-10 19:03 ` [PATCH 7/8] worktree: use skip_prefix to parse target Martin Ågren
2020-09-10 19:03 ` [PATCH 8/8] worktree: simplify search for unique worktree Martin Ågren
2020-09-10 19:28   ` Eric Sunshine
2020-09-10 19:48     ` Martin Ågren
2020-09-10 20:01       ` Eric Sunshine
2020-09-10 21:08         ` Junio C Hamano
2020-09-12  3:49 ` [PATCH 0/8] various wt-status/worktree cleanups Taylor Blau
2020-09-12 14:03   ` Martin Ågren

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git