Git development
 help / color / mirror / Atom feed
* Re: git-scm.com status report
From: Eric Wong @ 2017-02-02  4:36 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20170202023349.7fopb3a6pc6dkcmd@sigill.intra.peff.net>

Jeff King <peff@peff.net> wrote:
> With the caveat that I know very little about web hosting, $230/mo
> sounds like an awful lot for what is essentially a static web site.

Yes, that's a lot.

Fwiw, that covers a year of low-end VPS hosting for the main
public-inbox.org/git machine + mail host
(~1GB git objects + ~3GB Xapian index).

> The site does see a lot of hits, but most of the content is a few basic
> web pages, and copies of other static content that is updated
> only occasionally (manpage content, lists of downloads, etc).  The biggest
> dynamic component is the site search, I think.

Maybe optimize search if that's slowest, first.  public-inbox
uses per-host Xapian indexes so there's no extra network latency
and it seems to work well.  But maybe you don't get FS write
access without full VPS access on Heroku...

nginx handles static content easily, and since it looks like you
guys use unicorn[*] for running the Ruby part.  I really hope
nginx is in front of unicorn, since (AFAIK) Heroku doesn't put
nginx in front of it by default.


[*] I wrote and maintain unicorn; and have not yet recommended
    any reverse proxy besides nginx to buffer for it.
    However, having varnish or any other caching layer in
    between nginx and unicorn is great, too.  I dunno how Heroku
    (or any proprietary deployment systems) handle it, though.

> I do wonder if there's room for improvement either:
> 
>   - by measuring and optimizing the Heroku deploy. I have no idea about
>     scaling Rails or Heroku apps. Do we really need three expensive
>     dynos, or a $50/mo database plan? I'm not even sure what to measure,
>     or how. There are some analytics on the site, but I don't have
>     access to them (I could probably dig around for access if there was
>     somebody who actually knew how to do something productive with
>     them).

I track down the most expensive requests in per-request timing
logs and work on profiling + optimizations from there...
Nothing fancy and no relying on proprietary tools like NewRelic.

I also watch for queueing in listen socket backlog (with
raindrops <https://raindrops-demo.bogomips.org/> or ss(8) to
notice overloading.  Again, I don't know how much visibility
you have with Heroku.

>   - by moving to a simpler model. I wonder if we could build the site
>     once and then deploy a more static variant of it to a cheaper
>     hosting platform. I'm not really sure what our options would be, how
>     much work it would take to do the conversion, and if we'd lose any
>     functionality.

*shrug*  That'd be more work, at least.  I'd figure out what's
slow, first.

Fwiw, Varnish definitely helps public-inbox when slammed by
HN/Reddit traffic.  It's great as long as you don't have
per-user data to invalidate, which seems to be the case for
git-scm.

> If anybody is interested in tackling a project like this, let me know,
> and I can try to provide access to whatever parts are needed.

While I'm not up-to-date with modern Rails or deployment stuff,
I'm available via email if you have any lower-level
Ruby/unicorn/nginx-related questions.  I'm sure GitHub/GitLab
also has folks familiar with nginx+unicorn deployment on
bare metal or VPS who could also help.

^ permalink raw reply

* Re: git-scm.com status report
From: Samuel Lijin @ 2017-02-02  6:54 UTC (permalink / raw)
  To: Eric Wong; +Cc: Jeff King, git@vger.kernel.org
In-Reply-To: <20170202043610.GA12738@starla>

In theory, you could also dump the build artifacts to a GH Pages repo
and host it from there, although I don't know if you would run up
against any of the usage limits[0]. The immediate problem I see with
that approach, though, is that I have no idea how any of the dynamic
stuff (e.g. search) would be replaced.

A question: there's a DB schema in there. Does the site still use a DB?

[0] https://help.github.com/articles/what-is-github-pages/#usage-limits

On Wed, Feb 1, 2017 at 10:36 PM, Eric Wong <e@80x24.org> wrote:
> Jeff King <peff@peff.net> wrote:
>> With the caveat that I know very little about web hosting, $230/mo
>> sounds like an awful lot for what is essentially a static web site.
>
> Yes, that's a lot.
>
> Fwiw, that covers a year of low-end VPS hosting for the main
> public-inbox.org/git machine + mail host
> (~1GB git objects + ~3GB Xapian index).
>
>> The site does see a lot of hits, but most of the content is a few basic
>> web pages, and copies of other static content that is updated
>> only occasionally (manpage content, lists of downloads, etc).  The biggest
>> dynamic component is the site search, I think.
>
> Maybe optimize search if that's slowest, first.  public-inbox
> uses per-host Xapian indexes so there's no extra network latency
> and it seems to work well.  But maybe you don't get FS write
> access without full VPS access on Heroku...
>
> nginx handles static content easily, and since it looks like you
> guys use unicorn[*] for running the Ruby part.  I really hope
> nginx is in front of unicorn, since (AFAIK) Heroku doesn't put
> nginx in front of it by default.
>
>
> [*] I wrote and maintain unicorn; and have not yet recommended
>     any reverse proxy besides nginx to buffer for it.
>     However, having varnish or any other caching layer in
>     between nginx and unicorn is great, too.  I dunno how Heroku
>     (or any proprietary deployment systems) handle it, though.
>
>> I do wonder if there's room for improvement either:
>>
>>   - by measuring and optimizing the Heroku deploy. I have no idea about
>>     scaling Rails or Heroku apps. Do we really need three expensive
>>     dynos, or a $50/mo database plan? I'm not even sure what to measure,
>>     or how. There are some analytics on the site, but I don't have
>>     access to them (I could probably dig around for access if there was
>>     somebody who actually knew how to do something productive with
>>     them).
>
> I track down the most expensive requests in per-request timing
> logs and work on profiling + optimizations from there...
> Nothing fancy and no relying on proprietary tools like NewRelic.
>
> I also watch for queueing in listen socket backlog (with
> raindrops <https://raindrops-demo.bogomips.org/> or ss(8) to
> notice overloading.  Again, I don't know how much visibility
> you have with Heroku.
>
>>   - by moving to a simpler model. I wonder if we could build the site
>>     once and then deploy a more static variant of it to a cheaper
>>     hosting platform. I'm not really sure what our options would be, how
>>     much work it would take to do the conversion, and if we'd lose any
>>     functionality.
>
> *shrug*  That'd be more work, at least.  I'd figure out what's
> slow, first.
>
> Fwiw, Varnish definitely helps public-inbox when slammed by
> HN/Reddit traffic.  It's great as long as you don't have
> per-user data to invalidate, which seems to be the case for
> git-scm.
>
>> If anybody is interested in tackling a project like this, let me know,
>> and I can try to provide access to whatever parts are needed.
>
> While I'm not up-to-date with modern Rails or deployment stuff,
> I'm available via email if you have any lower-level
> Ruby/unicorn/nginx-related questions.  I'm sure GitHub/GitLab
> also has folks familiar with nginx+unicorn deployment on
> bare metal or VPS who could also help.

^ permalink raw reply

* [PATCH 00/11] nd/worktree-move update
From: Nguyễn Thái Ngọc Duy @ 2017-02-02  8:49 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

This squashes two changes from Johannes and Ramsay:

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 339c622e20..a1c91f1542 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -528,7 +528,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
 
 static void validate_no_submodules(const struct worktree *wt)
 {
-	struct index_state istate = {0};
+	struct index_state istate = { NULL };
 	int i, found_submodules = 0;
 
 	if (read_index_from(&istate, worktree_git_path(wt, "index")) > 0) {
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 084acc6c6d..b3105eaaed 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -71,13 +71,14 @@ test_expect_success 'move locked worktree' '
 '
 
 test_expect_success 'move worktree' '
+	toplevel="$(pwd)" &&
 	git worktree move source destination &&
 	test_path_is_missing source &&
 	git worktree list --porcelain | grep "^worktree" >actual &&
 	cat <<-EOF >expected &&
-	worktree $TRASH_DIRECTORY
-	worktree $TRASH_DIRECTORY/destination
-	worktree $TRASH_DIRECTORY/elsewhere
+	worktree $toplevel
+	worktree $toplevel/destination
+	worktree $toplevel/elsewhere
 	EOF
 	test_cmp expected actual &&
 	git -C destination log --format=%s >actual2 &&


Nguyễn Thái Ngọc Duy (11):
  worktree.c: zero new 'struct worktree' on allocation
  worktree: reorder an if statement
  get_worktrees() must return main worktree as first item even on error
  worktree.c: get_worktrees() takes a new flag argument
  worktree list: keep the list sorted
  worktree.c: add validate_worktree()
  worktree.c: add update_worktree_location()
  worktree move: new command
  worktree move: accept destination as directory
  worktree move: refuse to move worktrees with submodules
  worktree remove: new command

 Documentation/git-worktree.txt         |  28 ++++--
 branch.c                               |   2 +-
 builtin/branch.c                       |   2 +-
 builtin/worktree.c                     | 176 +++++++++++++++++++++++++++++++--
 contrib/completion/git-completion.bash |   5 +-
 t/t2027-worktree-list.sh               |  40 ++++++++
 t/t2028-worktree-move.sh               |  57 +++++++++++
 worktree.c                             | 126 +++++++++++++++++++----
 worktree.h                             |  15 ++-
 9 files changed, 410 insertions(+), 41 deletions(-)

-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH 01/11] worktree.c: zero new 'struct worktree' on allocation
From: Nguyễn Thái Ngọc Duy @ 2017-02-02  8:49 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170202085007.21418-1-pclouds@gmail.com>

This keeps things a bit simpler when we add more fields, knowing that
default values are always zero.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 worktree.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/worktree.c b/worktree.c
index f7869f8d60..f7c1b5e24d 100644
--- a/worktree.c
+++ b/worktree.c
@@ -91,16 +91,11 @@ static struct worktree *get_main_worktree(void)
 	if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
 		goto done;
 
-	worktree = xmalloc(sizeof(struct worktree));
+	worktree = xcalloc(1, sizeof(*worktree));
 	worktree->path = strbuf_detach(&worktree_path, NULL);
-	worktree->id = NULL;
 	worktree->is_bare = is_bare;
-	worktree->head_ref = NULL;
 	worktree->is_detached = is_detached;
-	worktree->is_current = 0;
 	add_head_info(&head_ref, worktree);
-	worktree->lock_reason = NULL;
-	worktree->lock_reason_valid = 0;
 
 done:
 	strbuf_release(&path);
@@ -138,16 +133,11 @@ static struct worktree *get_linked_worktree(const char *id)
 	if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
 		goto done;
 
-	worktree = xmalloc(sizeof(struct worktree));
+	worktree = xcalloc(1, sizeof(*worktree));
 	worktree->path = strbuf_detach(&worktree_path, NULL);
 	worktree->id = xstrdup(id);
-	worktree->is_bare = 0;
-	worktree->head_ref = NULL;
 	worktree->is_detached = is_detached;
-	worktree->is_current = 0;
 	add_head_info(&head_ref, worktree);
-	worktree->lock_reason = NULL;
-	worktree->lock_reason_valid = 0;
 
 done:
 	strbuf_release(&path);
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH 02/11] worktree: reorder an if statement
From: Nguyễn Thái Ngọc Duy @ 2017-02-02  8:49 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170202085007.21418-1-pclouds@gmail.com>

This is no-op. But it helps reduce diff noise in the next patch.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/worktree.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 5c4854d3e4..8a654e4ad3 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -406,10 +406,10 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)
 	else {
 		strbuf_addf(&sb, "%-*s ", abbrev_len,
 				find_unique_abbrev(wt->head_sha1, DEFAULT_ABBREV));
-		if (!wt->is_detached)
-			strbuf_addf(&sb, "[%s]", shorten_unambiguous_ref(wt->head_ref, 0));
-		else
+		if (wt->is_detached)
 			strbuf_addstr(&sb, "(detached HEAD)");
+		else
+			strbuf_addf(&sb, "[%s]", shorten_unambiguous_ref(wt->head_ref, 0));
 	}
 	printf("%s\n", sb.buf);
 
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH 03/11] get_worktrees() must return main worktree as first item even on error
From: Nguyễn Thái Ngọc Duy @ 2017-02-02  8:49 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170202085007.21418-1-pclouds@gmail.com>

This is required by git-worktree.txt, stating that the main worktree is
the first line (especially in --porcelain mode when we can't just change
behavior at will).

There's only one case when get_worktrees() may skip main worktree, when
parse_ref() fails. Update the code so that we keep first item as main
worktree and return something sensible in this case:

 - In user-friendly mode, since we're not constraint by anything,
   returning "(error)" should do the job (we already show "(detached
   HEAD)" which is not machine-friendly). Actually errors should be
   printed on stderr by parse_ref() (*)

 - In plumbing mode, we do not show neither 'bare', 'detached' or
   'branch ...', which is possible by the format description if I read
   it right.

Careful readers may realize that when the local variable "head_ref" in
get_main_worktree() is emptied, add_head_info() will do nothing to
wt->head_sha1. But that's ok because head_sha1 is zero-ized in the
previous patch.

(*) Well, it does not. But it's supposed to be a stop gap implementation
    until we can reuse refs code to parse "ref: " stuff in HEAD, from
    resolve_refs_unsafe(). Now may be the time since refs refactoring is
    mostly done.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/worktree.c       |  6 ++++--
 t/t2027-worktree-list.sh | 21 +++++++++++++++++++++
 worktree.c               | 10 +++-------
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 8a654e4ad3..b835b91f63 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -388,7 +388,7 @@ static void show_worktree_porcelain(struct worktree *wt)
 		printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
 		if (wt->is_detached)
 			printf("detached\n");
-		else
+		else if (wt->head_ref)
 			printf("branch %s\n", wt->head_ref);
 	}
 	printf("\n");
@@ -408,8 +408,10 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)
 				find_unique_abbrev(wt->head_sha1, DEFAULT_ABBREV));
 		if (wt->is_detached)
 			strbuf_addstr(&sb, "(detached HEAD)");
-		else
+		else if (wt->head_ref)
 			strbuf_addf(&sb, "[%s]", shorten_unambiguous_ref(wt->head_ref, 0));
+		else
+			strbuf_addstr(&sb, "(error)");
 	}
 	printf("%s\n", sb.buf);
 
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 1b1b65a6b0..98b5f340e5 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -96,4 +96,25 @@ test_expect_success 'bare repo cleanup' '
 	rm -rf bare1
 '
 
+test_expect_success 'broken main worktree still at the top' '
+	git init broken-main &&
+	(
+		cd broken-main &&
+		test_commit new &&
+		git worktree add linked &&
+		cat >expected <<-EOF &&
+		worktree $(pwd)
+		HEAD $_z40
+
+		EOF
+		cd linked &&
+		echo "worktree $(pwd)" >expected &&
+		echo "ref: .broken" >../.git/HEAD &&
+		git worktree list --porcelain | head -n 3 >actual &&
+		test_cmp ../expected actual &&
+		git worktree list | head -n 1 >actual.2 &&
+		grep -F "(error)" actual.2
+	)
+'
+
 test_done
diff --git a/worktree.c b/worktree.c
index f7c1b5e24d..3145522536 100644
--- a/worktree.c
+++ b/worktree.c
@@ -88,16 +88,13 @@ static struct worktree *get_main_worktree(void)
 
 	strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
 
-	if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
-		goto done;
-
 	worktree = xcalloc(1, sizeof(*worktree));
 	worktree->path = strbuf_detach(&worktree_path, NULL);
 	worktree->is_bare = is_bare;
 	worktree->is_detached = is_detached;
-	add_head_info(&head_ref, worktree);
+	if (!parse_ref(path.buf, &head_ref, &is_detached))
+		add_head_info(&head_ref, worktree);
 
-done:
 	strbuf_release(&path);
 	strbuf_release(&worktree_path);
 	strbuf_release(&head_ref);
@@ -173,8 +170,7 @@ struct worktree **get_worktrees(void)
 
 	list = xmalloc(alloc * sizeof(struct worktree *));
 
-	if ((list[counter] = get_main_worktree()))
-		counter++;
+	list[counter++] = get_main_worktree();
 
 	strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
 	dir = opendir(path.buf);
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH 05/11] worktree list: keep the list sorted
From: Nguyễn Thái Ngọc Duy @ 2017-02-02  8:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170202085007.21418-1-pclouds@gmail.com>

It makes it easier to write tests for. But it should also be good for
the user since locating a worktree by eye would be easier once they
notice this.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/worktree.c       |  2 +-
 t/t2027-worktree-list.sh | 19 +++++++++++++++++++
 worktree.c               | 14 ++++++++++++++
 worktree.h               |  2 ++
 4 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index d7d195cd95..9a97e37a3f 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -447,7 +447,7 @@ static int list(int ac, const char **av, const char *prefix)
 	if (ac)
 		usage_with_options(worktree_usage, options);
 	else {
-		struct worktree **worktrees = get_worktrees(0);
+		struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED);
 		int path_maxlen = 0, abbrev = DEFAULT_ABBREV, i;
 
 		if (!porcelain)
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 98b5f340e5..465eeeacd3 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -117,4 +117,23 @@ test_expect_success 'broken main worktree still at the top' '
 	)
 '
 
+test_expect_success 'linked worktrees are sorted' '
+	mkdir sorted &&
+	git init sorted/main &&
+	(
+		cd sorted/main &&
+		test_tick &&
+		test_commit new &&
+		git worktree add ../first &&
+		git worktree add ../second &&
+		git worktree list --porcelain | grep ^worktree >actual
+	) &&
+	cat >expected <<-EOF &&
+	worktree $(pwd)/sorted/main
+	worktree $(pwd)/sorted/first
+	worktree $(pwd)/sorted/second
+	EOF
+	test_cmp expected sorted/main/actual
+'
+
 test_done
diff --git a/worktree.c b/worktree.c
index ead088e43c..eb6121263b 100644
--- a/worktree.c
+++ b/worktree.c
@@ -160,6 +160,13 @@ static void mark_current_worktree(struct worktree **worktrees)
 	free(git_dir);
 }
 
+static int compare_worktree(const void *a_, const void *b_)
+{
+	const struct worktree *const *a = a_;
+	const struct worktree *const *b = b_;
+	return fspathcmp((*a)->path, (*b)->path);
+}
+
 struct worktree **get_worktrees(unsigned flags)
 {
 	struct worktree **list = NULL;
@@ -191,6 +198,13 @@ struct worktree **get_worktrees(unsigned flags)
 	ALLOC_GROW(list, counter + 1, alloc);
 	list[counter] = NULL;
 
+	if (flags & GWT_SORT_LINKED)
+		/*
+		 * don't sort the first item (main worktree), which will
+		 * always be the first
+		 */
+		QSORT(list + 1, counter - 1, compare_worktree);
+
 	mark_current_worktree(list);
 	return list;
 }
diff --git a/worktree.h b/worktree.h
index 2e68d4ad86..d59ce1fee8 100644
--- a/worktree.h
+++ b/worktree.h
@@ -15,6 +15,8 @@ struct worktree {
 
 /* Functions for acting on the information about worktrees. */
 
+#define GWT_SORT_LINKED (1 << 0) /* keeps linked worktrees sorted */
+
 /*
  * Get the worktrees.  The primary worktree will always be the first returned,
  * and linked worktrees will be pointed to by 'next' in each subsequent
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH 04/11] worktree.c: get_worktrees() takes a new flag argument
From: Nguyễn Thái Ngọc Duy @ 2017-02-02  8:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170202085007.21418-1-pclouds@gmail.com>

This is another no-op patch, in preparation for get_worktrees() to do
optional things, like sorting.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 branch.c           | 2 +-
 builtin/branch.c   | 2 +-
 builtin/worktree.c | 6 +++---
 worktree.c         | 4 ++--
 worktree.h         | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/branch.c b/branch.c
index 0d459b3cfe..c431cbf6a9 100644
--- a/branch.c
+++ b/branch.c
@@ -348,7 +348,7 @@ void die_if_checked_out(const char *branch, int ignore_current_worktree)
 int replace_each_worktree_head_symref(const char *oldref, const char *newref)
 {
 	int ret = 0;
-	struct worktree **worktrees = get_worktrees();
+	struct worktree **worktrees = get_worktrees(0);
 	int i;
 
 	for (i = 0; worktrees[i]; i++) {
diff --git a/builtin/branch.c b/builtin/branch.c
index 60cc5c8e8d..475707528a 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -531,7 +531,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
 
 static void reject_rebase_or_bisect_branch(const char *target)
 {
-	struct worktree **worktrees = get_worktrees();
+	struct worktree **worktrees = get_worktrees(0);
 	int i;
 
 	for (i = 0; worktrees[i]; i++) {
diff --git a/builtin/worktree.c b/builtin/worktree.c
index b835b91f63..d7d195cd95 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -447,7 +447,7 @@ static int list(int ac, const char **av, const char *prefix)
 	if (ac)
 		usage_with_options(worktree_usage, options);
 	else {
-		struct worktree **worktrees = get_worktrees();
+		struct worktree **worktrees = get_worktrees(0);
 		int path_maxlen = 0, abbrev = DEFAULT_ABBREV, i;
 
 		if (!porcelain)
@@ -478,7 +478,7 @@ static int lock_worktree(int ac, const char **av, const char *prefix)
 	if (ac != 1)
 		usage_with_options(worktree_usage, options);
 
-	worktrees = get_worktrees();
+	worktrees = get_worktrees(0);
 	wt = find_worktree(worktrees, prefix, av[0]);
 	if (!wt)
 		die(_("'%s' is not a working tree"), av[0]);
@@ -511,7 +511,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
 	if (ac != 1)
 		usage_with_options(worktree_usage, options);
 
-	worktrees = get_worktrees();
+	worktrees = get_worktrees(0);
 	wt = find_worktree(worktrees, prefix, av[0]);
 	if (!wt)
 		die(_("'%s' is not a working tree"), av[0]);
diff --git a/worktree.c b/worktree.c
index 3145522536..ead088e43c 100644
--- a/worktree.c
+++ b/worktree.c
@@ -160,7 +160,7 @@ static void mark_current_worktree(struct worktree **worktrees)
 	free(git_dir);
 }
 
-struct worktree **get_worktrees(void)
+struct worktree **get_worktrees(unsigned flags)
 {
 	struct worktree **list = NULL;
 	struct strbuf path = STRBUF_INIT;
@@ -327,7 +327,7 @@ const struct worktree *find_shared_symref(const char *symref,
 
 	if (worktrees)
 		free_worktrees(worktrees);
-	worktrees = get_worktrees();
+	worktrees = get_worktrees(0);
 
 	for (i = 0; worktrees[i]; i++) {
 		struct worktree *wt = worktrees[i];
diff --git a/worktree.h b/worktree.h
index 90e1311fa7..2e68d4ad86 100644
--- a/worktree.h
+++ b/worktree.h
@@ -23,7 +23,7 @@ struct worktree {
  * The caller is responsible for freeing the memory from the returned
  * worktree(s).
  */
-extern struct worktree **get_worktrees(void);
+extern struct worktree **get_worktrees(unsigned flags);
 
 /*
  * Return git dir of the worktree. Note that the path may be relative.
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH 06/11] worktree.c: add validate_worktree()
From: Nguyễn Thái Ngọc Duy @ 2017-02-02  8:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170202085007.21418-1-pclouds@gmail.com>

This function is later used by "worktree move" and "worktree remove"
to ensure that we have a good connection between the repository and
the worktree. For example, if a worktree is moved manually, the
worktree location recorded in $GIT_DIR/worktrees/.../gitdir is
incorrect and we should not move that one.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 worktree.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 worktree.h |  5 +++++
 2 files changed, 68 insertions(+)

diff --git a/worktree.c b/worktree.c
index eb6121263b..929072ad89 100644
--- a/worktree.c
+++ b/worktree.c
@@ -291,6 +291,69 @@ const char *is_worktree_locked(struct worktree *wt)
 	return wt->lock_reason;
 }
 
+static int report(int quiet, const char *fmt, ...)
+{
+	va_list params;
+
+	if (quiet)
+		return -1;
+
+	va_start(params, fmt);
+	vfprintf(stderr, fmt, params);
+	fputc('\n', stderr);
+	va_end(params);
+	return -1;
+}
+
+int validate_worktree(const struct worktree *wt, int quiet)
+{
+	struct strbuf sb = STRBUF_INIT;
+	const char *path;
+	int err;
+
+	if (is_main_worktree(wt)) {
+		/*
+		 * Main worktree using .git file to point to the
+		 * repository would make it impossible to know where
+		 * the actual worktree is if this function is executed
+		 * from another worktree. No .git file support for now.
+		 */
+		strbuf_addf(&sb, "%s/.git", wt->path);
+		if (!is_directory(sb.buf)) {
+			strbuf_release(&sb);
+			return report(quiet, _("'%s/.git' at main worktree is not the repository directory"),
+				      wt->path);
+		}
+		return 0;
+	}
+
+	/*
+	 * Make sure "gitdir" file points to a real .git file and that
+	 * file points back here.
+	 */
+	if (!is_absolute_path(wt->path))
+		return report(quiet, _("'%s' file does not contain absolute path to the worktree location"),
+			      git_common_path("worktrees/%s/gitdir", wt->id));
+
+	strbuf_addf(&sb, "%s/.git", wt->path);
+	if (!file_exists(sb.buf)) {
+		strbuf_release(&sb);
+		return report(quiet, _("'%s/.git' does not exist"), wt->path);
+	}
+
+	path = read_gitfile_gently(sb.buf, &err);
+	strbuf_release(&sb);
+	if (!path)
+		return report(quiet, _("'%s/.git' is not a .git file, error code %d"),
+			      wt->path, err);
+
+	if (fspathcmp(path, real_path(git_common_path("worktrees/%s", wt->id))))
+		return report(quiet, _("'%s' does not point back to"),
+			      wt->path, git_common_path("worktrees/%s", wt->id));
+
+	return 0;
+}
+
 int is_worktree_being_rebased(const struct worktree *wt,
 			      const char *target)
 {
diff --git a/worktree.h b/worktree.h
index d59ce1fee8..4433db2eb3 100644
--- a/worktree.h
+++ b/worktree.h
@@ -53,6 +53,11 @@ extern int is_main_worktree(const struct worktree *wt);
 extern const char *is_worktree_locked(struct worktree *wt);
 
 /*
+ * Return zero if the worktree is in good condition.
+ */
+extern int validate_worktree(const struct worktree *wt, int quiet);
+
+/*
  * Free up the memory for worktree(s)
  */
 extern void free_worktrees(struct worktree **);
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH 08/11] worktree move: new command
From: Nguyễn Thái Ngọc Duy @ 2017-02-02  8:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170202085007.21418-1-pclouds@gmail.com>

There are two options to move the main worktree, but both have
complications, so it's not implemented yet. Anyway the options are:

 - convert the main worktree to a linked one and move it away, leave the
   git repository where it is. The repo essentially becomes bare after
   this move.

 - move the repository with the main worktree. The tricky part is make
   sure all file descriptors to the repository are closed, or it may
   fail on Windows.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-worktree.txt         |  7 +++++-
 builtin/worktree.c                     | 43 ++++++++++++++++++++++++++++++++++
 contrib/completion/git-completion.bash |  2 +-
 t/t2028-worktree-move.sh               | 31 ++++++++++++++++++++++++
 4 files changed, 81 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e257c19ebe..13105138a7 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -12,6 +12,7 @@ SYNOPSIS
 'git worktree add' [-f] [--detach] [--checkout] [-b <new-branch>] <path> [<branch>]
 'git worktree list' [--porcelain]
 'git worktree lock' [--reason <string>] <worktree>
+'git worktree move' <worktree> <new-path>
 'git worktree prune' [-n] [-v] [--expire <expire>]
 'git worktree unlock' <worktree>
 
@@ -71,6 +72,11 @@ files from being pruned automatically. This also prevents it from
 being moved or deleted. Optionally, specify a reason for the lock
 with `--reason`.
 
+move::
+
+Move a working tree to a new location. Note that the main working tree
+cannot be moved yet.
+
 prune::
 
 Prune working tree information in $GIT_DIR/worktrees.
@@ -252,7 +258,6 @@ performed manually, such as:
 
 - `remove` to remove a linked working tree and its administrative files (and
   warn if the working tree is dirty)
-- `mv` to move or rename a working tree and update its administrative files
 
 GIT
 ---
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 9a97e37a3f..0d8b57ceb3 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -15,6 +15,7 @@ static const char * const worktree_usage[] = {
 	N_("git worktree add [<options>] <path> [<branch>]"),
 	N_("git worktree list [<options>]"),
 	N_("git worktree lock [<options>] <path>"),
+	N_("git worktree move <worktree> <new-path>"),
 	N_("git worktree prune [<options>]"),
 	N_("git worktree unlock <path>"),
 	NULL
@@ -524,6 +525,46 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
 	return ret;
 }
 
+static int move_worktree(int ac, const char **av, const char *prefix)
+{
+	struct option options[] = {
+		OPT_END()
+	};
+	struct worktree **worktrees, *wt;
+	struct strbuf dst = STRBUF_INIT;
+	const char *reason;
+
+	ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+	if (ac != 2)
+		usage_with_options(worktree_usage, options);
+
+	strbuf_addstr(&dst, prefix_filename(prefix,
+					    strlen(prefix),
+					    av[1]));
+	if (file_exists(dst.buf))
+		die(_("target '%s' already exists"), av[1]);
+
+	worktrees = get_worktrees(0);
+	wt = find_worktree(worktrees, prefix, av[0]);
+	if (!wt)
+		die(_("'%s' is not a working directory"), av[0]);
+	if (is_main_worktree(wt))
+		die(_("'%s' is a main working directory"), av[0]);
+	reason = is_worktree_locked(wt);
+	if (reason) {
+		if (*reason)
+			die(_("already locked, reason: %s"), reason);
+		die(_("already locked, no reason"));
+	}
+	if (validate_worktree(wt, 0))
+		return -1;
+
+	if (rename(wt->path, dst.buf) == -1)
+		die_errno(_("failed to move '%s' to '%s'"), wt->path, dst.buf);
+
+	return update_worktree_location(wt, dst.buf);
+}
+
 int cmd_worktree(int ac, const char **av, const char *prefix)
 {
 	struct option options[] = {
@@ -546,5 +587,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix)
 		return lock_worktree(ac - 1, av + 1, prefix);
 	if (!strcmp(av[1], "unlock"))
 		return unlock_worktree(ac - 1, av + 1, prefix);
+	if (!strcmp(av[1], "move"))
+		return move_worktree(ac - 1, av + 1, prefix);
 	usage_with_options(worktree_usage, options);
 }
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 21016bf8df..613e03b879 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2715,7 +2715,7 @@ _git_whatchanged ()
 
 _git_worktree ()
 {
-	local subcommands="add list lock prune unlock"
+	local subcommands="add list lock move prune unlock"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		__gitcomp "$subcommands"
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 8298aaf97f..bef420a086 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -59,4 +59,35 @@ test_expect_success 'unlock worktree twice' '
 	test_path_is_missing .git/worktrees/source/locked
 '
 
+test_expect_success 'move non-worktree' '
+	mkdir abc &&
+	test_must_fail git worktree move abc def
+'
+
+test_expect_success 'move locked worktree' '
+	git worktree lock source &&
+	test_must_fail git worktree move source destination &&
+	git worktree unlock source
+'
+
+test_expect_success 'move worktree' '
+	toplevel="$(pwd)" &&
+	git worktree move source destination &&
+	test_path_is_missing source &&
+	git worktree list --porcelain | grep "^worktree" >actual &&
+	cat <<-EOF >expected &&
+	worktree $toplevel
+	worktree $toplevel/destination
+	worktree $toplevel/elsewhere
+	EOF
+	test_cmp expected actual &&
+	git -C destination log --format=%s >actual2 &&
+	echo init >expected2 &&
+	test_cmp expected2 actual2
+'
+
+test_expect_success 'move main worktree' '
+	test_must_fail git worktree move . def
+'
+
 test_done
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH 07/11] worktree.c: add update_worktree_location()
From: Nguyễn Thái Ngọc Duy @ 2017-02-02  8:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170202085007.21418-1-pclouds@gmail.com>

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 worktree.c | 21 +++++++++++++++++++++
 worktree.h |  6 ++++++
 2 files changed, 27 insertions(+)

diff --git a/worktree.c b/worktree.c
index 929072ad89..7684951da5 100644
--- a/worktree.c
+++ b/worktree.c
@@ -354,6 +354,27 @@ int validate_worktree(const struct worktree *wt, int quiet)
 	return 0;
 }
 
+int update_worktree_location(struct worktree *wt, const char *path_)
+{
+	struct strbuf path = STRBUF_INIT;
+	int ret = 0;
+
+	if (is_main_worktree(wt))
+		return 0;
+
+	strbuf_add_absolute_path(&path, path_);
+	if (fspathcmp(wt->path, path.buf)) {
+		write_file(git_common_path("worktrees/%s/gitdir",
+					   wt->id),
+			   "%s/.git", real_path(path.buf));
+		free(wt->path);
+		wt->path = strbuf_detach(&path, NULL);
+		ret = 0;
+	}
+	strbuf_release(&path);
+	return ret;
+}
+
 int is_worktree_being_rebased(const struct worktree *wt,
 			      const char *target)
 {
diff --git a/worktree.h b/worktree.h
index 4433db2eb3..1ee03f4d06 100644
--- a/worktree.h
+++ b/worktree.h
@@ -58,6 +58,12 @@ extern const char *is_worktree_locked(struct worktree *wt);
 extern int validate_worktree(const struct worktree *wt, int quiet);
 
 /*
+ * Update worktrees/xxx/gitdir with the new path.
+ */
+extern int update_worktree_location(struct worktree *wt,
+				    const char *path_);
+
+/*
  * Free up the memory for worktree(s)
  */
 extern void free_worktrees(struct worktree **);
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH 09/11] worktree move: accept destination as directory
From: Nguyễn Thái Ngọc Duy @ 2017-02-02  8:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170202085007.21418-1-pclouds@gmail.com>

Similar to "mv a b/", which is actually "mv a b/a", we extract basename
of source worktree and create a directory of the same name at
destination if dst path is a directory.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/worktree.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 0d8b57ceb3..900b68bb5d 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -541,7 +541,13 @@ static int move_worktree(int ac, const char **av, const char *prefix)
 	strbuf_addstr(&dst, prefix_filename(prefix,
 					    strlen(prefix),
 					    av[1]));
-	if (file_exists(dst.buf))
+	if (is_directory(dst.buf))
+		/*
+		 * keep going, dst will be appended after we get the
+		 * source's absolute path
+		 */
+		;
+	else if (file_exists(dst.buf))
 		die(_("target '%s' already exists"), av[1]);
 
 	worktrees = get_worktrees(0);
@@ -559,6 +565,17 @@ static int move_worktree(int ac, const char **av, const char *prefix)
 	if (validate_worktree(wt, 0))
 		return -1;
 
+	if (is_directory(dst.buf)) {
+		const char *sep = find_last_dir_sep(wt->path);
+
+		if (!sep)
+			die(_("could not figure out destination name from '%s'"),
+			    wt->path);
+		strbuf_addstr(&dst, sep);
+		if (file_exists(dst.buf))
+			die(_("target '%s' already exists"), dst.buf);
+	}
+
 	if (rename(wt->path, dst.buf) == -1)
 		die_errno(_("failed to move '%s' to '%s'"), wt->path, dst.buf);
 
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH 11/11] worktree remove: new command
From: Nguyễn Thái Ngọc Duy @ 2017-02-02  8:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170202085007.21418-1-pclouds@gmail.com>

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-worktree.txt         | 21 +++++----
 builtin/worktree.c                     | 79 ++++++++++++++++++++++++++++++++++
 contrib/completion/git-completion.bash |  5 ++-
 t/t2028-worktree-move.sh               | 26 +++++++++++
 4 files changed, 121 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 13105138a7..bbde6b8110 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -14,6 +14,7 @@ SYNOPSIS
 'git worktree lock' [--reason <string>] <worktree>
 'git worktree move' <worktree> <new-path>
 'git worktree prune' [-n] [-v] [--expire <expire>]
+'git worktree remove' [--force] <worktree>
 'git worktree unlock' <worktree>
 
 DESCRIPTION
@@ -81,6 +82,13 @@ prune::
 
 Prune working tree information in $GIT_DIR/worktrees.
 
+remove::
+
+Remove a working tree. Only clean working trees (no untracked files
+and no modification in tracked files) can be removed. Unclean working
+trees can be removed with `--force`. The main working tree cannot be
+removed.
+
 unlock::
 
 Unlock a working tree, allowing it to be pruned, moved or deleted.
@@ -90,9 +98,10 @@ OPTIONS
 
 -f::
 --force::
-	By default, `add` refuses to create a new working tree when `<branch>`
-	is already checked out by another working tree. This option overrides
-	that safeguard.
+	By default, `add` refuses to create a new working tree when
+	`<branch>` is already checked out by another working tree and
+	`remove` refuses to remove an unclean working tree. This option
+	overrides that safeguard.
 
 -b <new-branch>::
 -B <new-branch>::
@@ -253,12 +262,6 @@ Multiple checkout in general is still experimental, and the support
 for submodules is incomplete. It is NOT recommended to make multiple
 checkouts of a superproject.
 
-git-worktree could provide more automation for tasks currently
-performed manually, such as:
-
-- `remove` to remove a linked working tree and its administrative files (and
-  warn if the working tree is dirty)
-
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 6c58d620ce..a1c91f1542 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -17,6 +17,7 @@ static const char * const worktree_usage[] = {
 	N_("git worktree lock [<options>] <path>"),
 	N_("git worktree move <worktree> <new-path>"),
 	N_("git worktree prune [<options>]"),
+	N_("git worktree remove [<options>] <worktree>"),
 	N_("git worktree unlock <path>"),
 	NULL
 };
@@ -605,6 +606,82 @@ static int move_worktree(int ac, const char **av, const char *prefix)
 	return update_worktree_location(wt, dst.buf);
 }
 
+static int remove_worktree(int ac, const char **av, const char *prefix)
+{
+	int force = 0;
+	struct option options[] = {
+		OPT_BOOL(0, "force", &force,
+			 N_("force removing even if the worktree is dirty")),
+		OPT_END()
+	};
+	struct worktree **worktrees, *wt;
+	struct strbuf sb = STRBUF_INIT;
+	const char *reason;
+	int ret = 0;
+
+	ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+	if (ac != 1)
+		usage_with_options(worktree_usage, options);
+
+	worktrees = get_worktrees(0);
+	wt = find_worktree(worktrees, prefix, av[0]);
+	if (!wt)
+		die(_("'%s' is not a working directory"), av[0]);
+	if (is_main_worktree(wt))
+		die(_("'%s' is a main working directory"), av[0]);
+	reason = is_worktree_locked(wt);
+	if (reason) {
+		if (*reason)
+			die(_("already locked, reason: %s"), reason);
+		die(_("already locked, no reason"));
+	}
+	if (validate_worktree(wt, 0))
+		return -1;
+
+	if (!force) {
+		struct argv_array child_env = ARGV_ARRAY_INIT;
+		struct child_process cp;
+		char buf[1];
+
+		argv_array_pushf(&child_env, "%s=%s/.git",
+				 GIT_DIR_ENVIRONMENT, wt->path);
+		argv_array_pushf(&child_env, "%s=%s",
+				 GIT_WORK_TREE_ENVIRONMENT, wt->path);
+		memset(&cp, 0, sizeof(cp));
+		argv_array_pushl(&cp.args, "status", "--porcelain", NULL);
+		cp.env = child_env.argv;
+		cp.git_cmd = 1;
+		cp.dir = wt->path;
+		cp.out = -1;
+		ret = start_command(&cp);
+		if (ret)
+			die_errno(_("failed to run git-status on '%s', code %d"),
+				  av[0], ret);
+		ret = xread(cp.out, buf, sizeof(buf));
+		if (ret)
+			die(_("'%s' is dirty, use --force to delete it"), av[0]);
+		close(cp.out);
+		ret = finish_command(&cp);
+		if (ret)
+			die_errno(_("failed to run git-status on '%s', code %d"),
+				  av[0], ret);
+	}
+	strbuf_addstr(&sb, wt->path);
+	if (remove_dir_recursively(&sb, 0)) {
+		error_errno(_("failed to delete '%s'"), sb.buf);
+		ret = -1;
+	}
+	strbuf_reset(&sb);
+	strbuf_addstr(&sb, git_common_path("worktrees/%s", wt->id));
+	if (remove_dir_recursively(&sb, 0)) {
+		error_errno(_("failed to delete '%s'"), sb.buf);
+		ret = -1;
+	}
+	strbuf_release(&sb);
+	free_worktrees(worktrees);
+	return ret;
+}
+
 int cmd_worktree(int ac, const char **av, const char *prefix)
 {
 	struct option options[] = {
@@ -629,5 +706,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix)
 		return unlock_worktree(ac - 1, av + 1, prefix);
 	if (!strcmp(av[1], "move"))
 		return move_worktree(ac - 1, av + 1, prefix);
+	if (!strcmp(av[1], "remove"))
+		return remove_worktree(ac - 1, av + 1, prefix);
 	usage_with_options(worktree_usage, options);
 }
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 613e03b879..f6855af1f6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2715,7 +2715,7 @@ _git_whatchanged ()
 
 _git_worktree ()
 {
-	local subcommands="add list lock move prune unlock"
+	local subcommands="add list lock move prune remove unlock"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		__gitcomp "$subcommands"
@@ -2733,6 +2733,9 @@ _git_worktree ()
 		prune,--*)
 			__gitcomp "--dry-run --expire --verbose"
 			;;
+		remove,--*)
+			__gitcomp "--force"
+			;;
 		*)
 			;;
 		esac
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index bef420a086..b3105eaaed 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -90,4 +90,30 @@ test_expect_success 'move main worktree' '
 	test_must_fail git worktree move . def
 '
 
+test_expect_success 'remove main worktree' '
+	test_must_fail git worktree remove .
+'
+
+test_expect_success 'remove locked worktree' '
+	git worktree lock destination &&
+	test_must_fail git worktree remove destination &&
+	git worktree unlock destination
+'
+
+test_expect_success 'remove worktree with dirty tracked file' '
+	echo dirty >>destination/init.t &&
+	test_must_fail git worktree remove destination
+'
+
+test_expect_success 'remove worktree with untracked file' '
+	git -C destination checkout init.t &&
+	: >destination/untracked &&
+	test_must_fail git worktree remove destination
+'
+
+test_expect_success 'force remove worktree with untracked file' '
+	git worktree remove --force destination &&
+	test_path_is_missing destination
+'
+
 test_done
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH 10/11] worktree move: refuse to move worktrees with submodules
From: Nguyễn Thái Ngọc Duy @ 2017-02-02  8:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170202085007.21418-1-pclouds@gmail.com>

Submodules contains .git files with relative paths. After a worktree
move, these files need to be updated or they may point to nowhere.

This is a bandage patch to make sure "worktree move" don't break
people's worktrees by accident. When .git file update code is in
place, this validate_no_submodules() could be removed.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/worktree.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 900b68bb5d..6c58d620ce 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -525,6 +525,27 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
 	return ret;
 }
 
+static void validate_no_submodules(const struct worktree *wt)
+{
+	struct index_state istate = { NULL };
+	int i, found_submodules = 0;
+
+	if (read_index_from(&istate, worktree_git_path(wt, "index")) > 0) {
+		for (i = 0; i < istate.cache_nr; i++) {
+			struct cache_entry *ce = istate.cache[i];
+
+			if (S_ISGITLINK(ce->ce_mode)) {
+				found_submodules = 1;
+				break;
+			}
+		}
+	}
+	discard_index(&istate);
+
+	if (found_submodules)
+		die(_("This working tree contains submodules and cannot be moved yet"));
+}
+
 static int move_worktree(int ac, const char **av, const char *prefix)
 {
 	struct option options[] = {
@@ -565,6 +586,8 @@ static int move_worktree(int ac, const char **av, const char *prefix)
 	if (validate_worktree(wt, 0))
 		return -1;
 
+	validate_no_submodules(wt);
+
 	if (is_directory(dst.buf)) {
 		const char *sep = find_last_dir_sep(wt->path);
 
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* Google Doc about the Contributors' Summit
From: Johannes Schindelin @ 2017-02-02  9:08 UTC (permalink / raw)
  To: git

Hi team,

I just started typing stuff up in a Google Doc, and made it editable to
everyone, feel free to help and add things:

https://docs.google.com/document/d/1KDoSn4btbK5VJCVld32he29U0pUeFGhpFxyx9ZJDDB0/edit?usp=sharing

Ciao,
Johannes


^ permalink raw reply

* Re: [PATCH] color_parse_mem: allow empty color spec
From: Duy Nguyen @ 2017-02-02  9:16 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20170201002129.xb62hmxwrzwgp6vg@sigill.intra.peff.net>

On Wed, Feb 01, 2017 at 01:21:29AM +0100, Jeff King wrote:
> On Tue, Jan 31, 2017 at 02:45:40PM -0800, Junio C Hamano wrote:
> 
> > * nd/log-graph-configurable-colors (2017-01-23) 3 commits
> >   (merged to 'next' on 2017-01-23 at c369982ad8)
> >  + log --graph: customize the graph lines with config log.graphColors
> >  + color.c: trim leading spaces in color_parse_mem()
> >  + color.c: fix color_parse_mem() with value_len == 0
> > 
> >  Some people feel the default set of colors used by "git log --graph"
> >  rather limiting.  A mechanism to customize the set of colors has
> >  been introduced.
> > 
> >  Reported to break "add -p".
> >  cf. <20170128040709.tqx4u45ktgpkbfm4@sigill.intra.peff.net>
> 
> I hadn't heard anything back,

Sorry I was accidentally busy during Luna new year holiday.

> so I wrapped up my fix with a commit
> message and tests (it needs to go on top anyway, since the breakage is
> in 'next').
> 
> -- >8 --
> Subject: [PATCH] color_parse_mem: allow empty color spec
> 
> Prior to c2f41bf52 (color.c: fix color_parse_mem() with
> value_len == 0, 2017-01-19), the empty string was
> interpreted as a color "reset". This was an accidental
> outcome, and that commit turned it into an error.
> 
> However, scripts may pass the empty string as a default
> value to "git config --get-color" to disable color when the
> value is not defined. The git-add--interactive script does
> this. As a result, the script is unusable since c2f41bf52
> unless you have color.diff.plain defined (if it is defined,
> then we don't parse the empty default at all).

..

> --- a/color.c
> +++ b/color.c
> @@ -212,8 +212,10 @@ int color_parse_mem(const char *value, int value_len, char *dst)
>  		len--;
>  	}
>  
> -	if (!len)
> -		return -1;
> +	if (!len) {
> +		dst[0] = '\0';
> +		return 0;
> +	}
>  
>  	if (!strncasecmp(ptr, "reset", len)) {
>  		xsnprintf(dst, end - dst, GIT_COLOR_RESET);

I wonder if it makes more sense to do this in builtin/config.c. The
"default value" business is strictly git-config command's. The parsing
function does not need to know. Maybe something like this?

diff --git a/builtin/config.c b/builtin/config.c
index 05843a0f96..ec1f4f0cf4 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -322,8 +322,10 @@ static void get_color(const char *var, const char *def_color)
 	git_config_with_options(git_get_color_config, NULL,
 				&given_config_source, respect_includes);
 
-	if (!get_color_found && def_color) {
-		if (color_parse(def_color, parsed_color) < 0)
+	if (!get_color_found) {
+		if (!def_color)
+			strcpy(parsed_color, GIT_COLOR_RESET);
+		else if (color_parse(def_color, parsed_color) < 0)
 			die(_("unable to parse default color value"));
 	}
 
This is also a good opportunity to document this behavior in
git-config.txt, I think.
--
Duy

^ permalink raw reply related

* Re: [PATCH 2/7] completion: add subcommand completion for rerere
From: Cornelius Weig @ 2017-02-02  9:16 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: bitte.keine.werbung.einwerfen, thomas.braun, john, git
In-Reply-To: <20170202005724.24754-1-szeder.dev@gmail.com>



On 02/02/2017 01:57 AM, SZEDER Gábor wrote:
> You didn't add 'rerere' to the list of porcelain commands, i.e. it
> won't be listed after 'git <TAB><TAB>'.  I'm not saying it should be
> listed there, because I can't decide whether 'rerere' is considered
> porcelain or plumbing...  Just wanted to make sure that this omission
> was intentional.

Yes this is intentional. There is a number of plumbing commands that
have command completion, but are not listed in 'git <Tab><Tab>' (e.g.
ls-tree, ls-files, ls-remote, ...). Given that rerere will not be
frequently invoked, I would not add it to the porcelain commands.

^ permalink raw reply

* Re: [PATCH 00/11] nd/worktree-move update
From: Johannes Schindelin @ 2017-02-02  9:16 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <20170202085007.21418-1-pclouds@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 283 bytes --]

Hi Duy,

On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:

> This squashes two changes from Johannes and Ramsay: [...]

Sorry, I lost track of the worktree discussions... Could you remind me
which patch is supposed to fix my continuous reflog corruption?

Thanks,
Dscho

^ permalink raw reply

* Re: git-daemon shallow checkout fail
From: Duy Nguyen @ 2017-02-02  9:26 UTC (permalink / raw)
  To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20170130172730.x5guphyqf5fsfi7m@sigill.intra.peff.net>

On Tue, Jan 31, 2017 at 12:27 AM, Jeff King <peff@peff.net> wrote:
> On Sat, Jan 28, 2017 at 05:29:32PM -0700, Bob Proulx wrote:
>
>> However the problem driving me crazy is that this only fails this way
>> on one machine.  Unfortunately failing on the machine I need to use.
>> If I try this same setup on any other machine I try then there is no
>> failure and it works okay.  Therefore I conclude that in the failing
>> case it is trying to write a shallow_XXXXXX file in the repository but
>> in all of the passing cases it does not.  I browsed through the
>> git-daemon source but couldn't deduce the flow yet.
>>
>> Does anyone know why one system would try to create shallow_XXXXXX
>> files in the repository while another one would not?
>
> It depends on the git version on the server. The interesting code is in
> upload-pack.c, which is spawned by git-daemon to serve a fetch or clone
> request.
>
> See commit b790e0f67 (upload-pack: send shallow info over stdin to
> pack-objects, 2014-03-11), which lays out the history. Since that commit
> (in git v2.0.0), there should be no tmpfile needed.

It must be it. There's nowhere else that upload-pack can create
shallow_XXXXXX. (receive-pack and fetch-pack can).

>> Of course git-daemon running as nobody can't create a temporary file
>> shallow_XXXXXX in the /srv/git/test-project.git because it has no
>> permissions by design.  But why does this work on other systems and
>> not work on my target system?
>>
>>   git --version  # from today's git clone build
>>   git version 2.11.0.485.g4e59582
>
> This shouldn't be happening with git v2.11. Are you sure that the "git
> daemon" invocation is running that same version? I notice you set up a
> restricted PATH. Is it possible that /usr/local/bin or /usr/bin has an
> older version of git?

One easy way to verify is clone or fetch again with GIT_TRACE_PACKET=1
since we send the server's version as a capability since 1.8.0
-- 
Duy

^ permalink raw reply

* Re: [PATCH 00/11] nd/worktree-move update
From: Duy Nguyen @ 2017-02-02  9:27 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1702021015160.3496@virtualbox>

On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Duy,
>
> On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
>
>> This squashes two changes from Johannes and Ramsay: [...]
>
> Sorry, I lost track of the worktree discussions... Could you remind me
> which patch is supposed to fix my continuous reflog corruption?

The corruption caused by git-gc? It's not fixed. All the changes in
this series is shown here.
-- 
Duy

^ permalink raw reply

* Re: [PATCH 2/2] completion: add completion for --recurse-submodules=only
From: Stefan Beller @ 2017-02-02  9:30 UTC (permalink / raw)
  To: Cornelius Weig; +Cc: git@vger.kernel.org, Brandon Williams
In-Reply-To: <20170201230753.19462-2-cornelius.weig@tngtech.com>

On Wed, Feb 1, 2017 at 3:07 PM,  <cornelius.weig@tngtech.com> wrote:
> From: Cornelius Weig <cornelius.weig@tngtech.com>
>
> Command completion for 'git-push --recurse-submodules' already knows to
> complete some modes. However, the recently added mode 'only' is missing.
>
> Adding 'only' to the recognized modes completes the list of non-trivial
> modes.
>
> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
> ---

Looks good,

Thanks,
Stefan

>  contrib/completion/git-completion.bash | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index ff7072a..fe3b0f8 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1675,7 +1675,7 @@ _git_pull ()
>         __git_complete_remote_or_refspec
>  }
>
> -__git_push_recurse_submodules="check on-demand"
> +__git_push_recurse_submodules="check on-demand only"
>
>  __git_complete_force_with_lease ()
>  {
> --
> 2.10.2
>

^ permalink raw reply

* Re: init --separate-git-dir does not set core.worktree
From: Duy Nguyen @ 2017-02-02  9:31 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: Git Mailing List
In-Reply-To: <87h94d8cwi.fsf@kyleam.com>

On Thu, Feb 2, 2017 at 10:55 AM, Kyle Meyer <kyle@kyleam.com> wrote:
> Hello,
>
> As of 6311cfaf9 (init: do not set unnecessary core.worktree,
> 2016-09-25), "git init --separate-git-dir" no longer sets core.worktree
> (test below).  Based on the commit message and the corresponding thread
> [1], I don't think this change in behavior was intentional, but I wasn't
> able to understand things well enough to attempt a patch.

I'm missing some context. Why does --separate-git-dir have to set
core.worktree? What fails for you exactly?

>
> Thanks.
>
> [1] https://public-inbox.org/git/CALqjkKZO_y0DNcRJjooyZ7Eso7yBMGhvZ6fE92oO4Su7JeCeng@mail.gmail.com/T/#u
>
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index b8fc588b1..444e75865 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -309,6 +309,7 @@ test_expect_success 'init with separate gitdir' '
>         git init --separate-git-dir realgitdir newdir &&
>         echo "gitdir: $(pwd)/realgitdir" >expected &&
>         test_cmp expected newdir/.git &&
> +       check_config realgitdir false "$(pwd)/newdir" &&
>         test_path_is_dir realgitdir/refs
>  '
>



-- 
Duy

^ permalink raw reply

* Fwd: [PATCH 1/2] doc: add doc for git-push --recurse-submodules=only
From: Brandon Williams @ 2017-02-02  9:32 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <CAKoko1q=6agpGsABxy8rmm6sGFWx9gE_c1j44h4M=yJ3r4JJBQ@mail.gmail.com>

Looks good to me!  Thanks for writing the documentation.  I really
need to be better about getting documentation done at the same time
I'm adding features :)

-Brandon

On Wed, Feb 1, 2017 at 3:16 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> cornelius.weig@tngtech.com writes:
>
> > From: Cornelius Weig <cornelius.weig@tngtech.com>
> >
> > Add documentation for the `--recurse-submodules=only` option of
> > git-push. The feature was added in commit 225e8bf (add option to
> > push only submodules).
> >
> > Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
> > ---
> >
> > Notes:
> >     This feature is already in 'next' but was undocumented. Unless somebody reads
> >     the release notes, there is no way of knowing about it.
>
> Good eyes; the topic bw/push-submodule-only is already in 'master'.
>
> Looks good to me; Brandon?
>
> >
> >  Documentation/git-push.txt | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> > index 8eefabd..1624a35 100644
> > --- a/Documentation/git-push.txt
> > +++ b/Documentation/git-push.txt
> > @@ -272,7 +272,7 @@ origin +master` to force a push to the `master` branch). See the
> >       standard error stream is not directed to a terminal.
> >
> >  --no-recurse-submodules::
> > ---recurse-submodules=check|on-demand|no::
> > +--recurse-submodules=check|on-demand|only|no::
> >       May be used to make sure all submodule commits used by the
> >       revisions to be pushed are available on a remote-tracking branch.
> >       If 'check' is used Git will verify that all submodule commits that
> > @@ -280,11 +280,12 @@ origin +master` to force a push to the `master` branch). See the
> >       remote of the submodule. If any commits are missing the push will
> >       be aborted and exit with non-zero status. If 'on-demand' is used
> >       all submodules that changed in the revisions to be pushed will be
> > -     pushed. If on-demand was not able to push all necessary revisions
> > -     it will also be aborted and exit with non-zero status. A value of
> > -     'no' or using `--no-recurse-submodules` can be used to override the
> > -     push.recurseSubmodules configuration variable when no submodule
> > -     recursion is required.
> > +     pushed. If on-demand was not able to push all necessary revisions it will
> > +     also be aborted and exit with non-zero status. If 'only' is used all
> > +     submodules will be recursively pushed while the superproject is left
> > +     unpushed. A value of 'no' or using `--no-recurse-submodules` can be used
> > +     to override the push.recurseSubmodules configuration variable when no
> > +     submodule recursion is required.
> >
> >  --[no-]verify::
> >       Toggle the pre-push hook (see linkgit:githooks[5]).  The

^ permalink raw reply

* Re: [PATCH 4/7] completion: teach ls-remote to complete options
From: Cornelius Weig @ 2017-02-02  9:40 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: bitte.keine.werbung.einwerfen, thomas.braun, john, git
In-Reply-To: <20170202014014.25878-1-szeder.dev@gmail.com>



On 02/02/2017 02:40 AM, SZEDER Gábor wrote:
> 
>> ls-remote needs to complete remote names and its own options.
> 
> And refnames, too.

Yes, right. However, do you think it is reasonable to complete remote
refnames? I don't think so, because it would mean we would have to run
ls-remote during completion -- and waiting for ls-remote could be quite
lengthy.

>> In
>> addition to the existing remote name completions, do also complete
>> the options --heads, --tags, --refs, and --get-url.
> 
> Why only these four options and not the other four?
> 
> There are eight options in total here, so there is really no chance
> for cluttered terminals, and all eight are mentioned in the synopsis.

My line of thought is the following:

--quiet: does not print anything and is therefore only useful for
scripting. Thus, there is no need to have it on the command line completion.

--exit-code: has no visible effect and is only useful for scripting.

--upload-pack: is really exotic. Nobody will ever use it without digging
deep in the manuals. Therefore, I think it's unnecessary to have the
option completable.


--symref: Should probably be added, thanks.

However, if you don't find my reasoning for omitting the three options
above conclusive, I have no problem including them.

^ permalink raw reply

* Re: [PATCH 00/11] nd/worktree-move update
From: Johannes Schindelin @ 2017-02-02  9:43 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <CACsJy8B3bdokeYVt6aEyZVSzO50PiQRn+0sid9mSDTZ9q-mnww@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 701 bytes --]

Hi Duy,

On Thu, 2 Feb 2017, Duy Nguyen wrote:

> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi Duy,
> >
> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
> >
> >> This squashes two changes from Johannes and Ramsay: [...]
> >
> > Sorry, I lost track of the worktree discussions... Could you remind me
> > which patch is supposed to fix my continuous reflog corruption?
> 
> The corruption caused by git-gc? It's not fixed. All the changes in
> this series is shown here.

Oh sorry, I meant to ask "and if it is not in this patch series, would you
mind pointing me at the patch series that has that fix?"

Thanks,
Johannes

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox