Git development
 help / color / mirror / Atom feed
* Re: Allow "git shortlog" to group by committer information
From: Johannes Sixt @ 2016-12-21 21:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Linus Torvalds, Git Mailing List
In-Reply-To: <d2ac90d6-c4f4-a759-a6e2-2d7fe5bb1c1d@kdbg.org>

Am 20.12.2016 um 19:52 schrieb Johannes Sixt:
> Am 20.12.2016 um 19:35 schrieb Junio C Hamano:
>>  test_expect_success 'shortlog --committer (internal)' '
>> +    git checkout --orphan side &&
>> +    git commit --allow-empty -m one &&
>> +    git commit --allow-empty -m two &&
>> +    GIT_COMMITTER_NAME="Sin Nombre" git commit --allow-empty -m three &&
>
> Clever! Thank you. Will test in 12 hours.
>
>> +
>>      cat >expect <<-\EOF &&
>> -         3    C O Mitter
>> +         2    C O Mitter
>> +         1    Sin Nombre
>>      EOF
>>      git shortlog -nsc HEAD >actual &&
>>      test_cmp expect actual
>>
>

I confirm that t4201 now passes on Windows with this fixup.

-- Hannes


^ permalink raw reply

* Re: Bug report: Git pull hang occasionally
From: Kai Zhang @ 2016-12-21 21:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq8tr9huc0.fsf@gitster.mtv.corp.google.com>

Thank you for your insight and detailed explanation Junio.

I think what you said is what is happening in my environment. Both writing and reading are happening simultaneously. 


> On Dec 21, 2016, at 12:59 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Kai Zhang <kai@netskope.com> writes:
> 
>> 2016/12/20 20:38:10 [error] 9957#0: *687703 FastCGI sent in stderr: "fatal: 'HEAD' is a symref but it is not?" while reading response header from upstream, client: 10.1.0.11, server: server, request: "POST /git/repo_name/.git/git-upload-pack HTTP/1.1", upstream: "fastcgi://unix:/var/run/fcgiwrap.socket:", host: "server"
> 
> (Not a solution)
> 
> In order to tell the client if HEAD is a symbolic ref and to what
> underlying ref it points at if it is a symbolic ref, at the very
> beginning of upload-pack, there is a call to head_ref_namespaced()
> that uses find_symref().  find_symref() gets "HEAD" and a boolean
> that says if it is a symbolic ref, but it does not get where the
> symbolic ref points at, so it does resolve_ref_unsafe() to learn
> that information.
> 
> Between the time head_ref_namespaced() checks the refs database and
> finds that HEAD is a symbolic ref, and the time find_symref() calls
> resolve_ref_unsafe() to find out where it leads to, if somebody else
> updates HEAD, resolve_ref_unsafe() can give an unexpected result, as
> all of these read-only operations are performed without any locking.
> 
> And the unexpected discrepancy is reported by find_symref() as
> fatal.  The server side dies, and somehow that fact is lost between
> the upload-pack process and the client and somebody in the middle
> (e.g. fastcgi interface or nginx webserver on the server side, or
> the remote-curl helper on the client side) keeps the "git fetch"
> process waiting.
> 
> So there seem to be two issues.  
> 
> - Because of the unlocked read, find_symref() can observe an
>   inconsistent state.  Perhaps it should be updated not to die but
>   to retry, expecting that transient inconsistency will go away.
> 
> - A fatal error in upload-pack is not reported back to the client
>   to cause it exit is an obvious one, and even if we find a way to
>   make this fatal error in find_symref() not to trigger, fatal
>   errors in other places in the code can trigger the same symptom.
> 


^ permalink raw reply

* Re: [PATCH 0/2] Really fix the isatty() problem on Windows
From: Johannes Sixt @ 2016-12-21 21:15 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Junio C Hamano, Pranit Bauva
In-Reply-To: <cover.1482342791.git.johannes.schindelin@gmx.de>

Am 21.12.2016 um 18:53 schrieb Johannes Schindelin:
> The current patch series is based on `pu`, as that already has the
> winansi_get_osfhandle() fix. For ease of testing, I also have a branch
> based on master which you can pull via
>
> 	git pull https://github.com/dscho/git mingw-isatty-fixup-master

Will test and report back tomorrow.

-- Hannes


^ permalink raw reply

* Re: [PATCH 0/2] Really fix the isatty() problem on Windows
From: Junio C Hamano @ 2016-12-21 21:21 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Johannes Schindelin, git, Pranit Bauva
In-Reply-To: <b0541907-ee79-207b-dc0f-1e3e7d761950@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

> Am 21.12.2016 um 18:53 schrieb Johannes Schindelin:
>> The current patch series is based on `pu`, as that already has the
>> winansi_get_osfhandle() fix. For ease of testing, I also have a branch
>> based on master which you can pull via
>>
>> 	git pull https://github.com/dscho/git mingw-isatty-fixup-master
>
> Will test and report back tomorrow.

Thanks.

^ permalink raw reply

* Re: Bug report: Git pull hang occasionally
From: Junio C Hamano @ 2016-12-21 21:32 UTC (permalink / raw)
  To: Kai Zhang; +Cc: git
In-Reply-To: <xmqq8tr9huc0.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

> And the unexpected discrepancy is reported by find_symref() as
> fatal.  The server side dies, and somehow that fact is lost between
> the upload-pack process and the client and somebody in the middle
> (e.g. fastcgi interface or nginx webserver on the server side, or
> the remote-curl helper on the client side) keeps the "git fetch"
> process waiting.
>
> So there seem to be two issues.  
>
>  - Because of the unlocked read, find_symref() can observe an
>    inconsistent state.  Perhaps it should be updated not to die but
>    to retry, expecting that transient inconsistency will go away.
>
>  - A fatal error in upload-pack is not reported back to the client
>    to cause it exit is an obvious one, and even if we find a way to
>    make this fatal error in find_symref() not to trigger, fatal
>    errors in other places in the code can trigger the same symptom.

I wonder if the latter is solved by recent patch 296b847c0d
("remote-curl: don't hang when a server dies before any output",
2016-11-18) on the client side.

-- >8 --
From: David Turner <dturner@twosigma.com>
Date: Fri, 18 Nov 2016 15:30:49 -0500
Subject: [PATCH] remote-curl: don't hang when a server dies before any output

In the event that a HTTP server closes the connection after giving a
200 but before giving any packets, we don't want to hang forever
waiting for a response that will never come.  Instead, we should die
immediately.

One case where this happens is when attempting to fetch a dangling
object by its object name.  In this case, the server dies before
sending any data.  Prior to this patch, fetch-pack would wait for
data from the server, and remote-curl would wait for fetch-pack,
causing a deadlock.

Despite this patch, there is other possible malformed input that could
cause the same deadlock (e.g. a half-finished pktline, or a pktline but
no trailing flush).  There are a few possible solutions to this:

1. Allowing remote-curl to tell fetch-pack about the EOF (so that
fetch-pack could know that no more data is coming until it says
something else).  This is tricky because an out-of-band signal would
be required, or the http response would have to be re-framed inside
another layer of pkt-line or something.

2. Make remote-curl understand some of the protocol.  It turns out
that in addition to understanding pkt-line, it would need to watch for
ack/nak.  This is somewhat fragile, as information about the protocol
would end up in two places.  Also, pkt-lines which are already at the
length limit would need special handling.

Both of these solutions would require a fair amount of work, whereas
this hack is easy and solves at least some of the problem.

Still to do: it would be good to give a better error message
than "fatal: The remote end hung up unexpectedly".

Signed-off-by: David Turner <dturner@twosigma.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 remote-curl.c               |  8 ++++++++
 t/t5551-http-fetch-smart.sh | 30 ++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/remote-curl.c b/remote-curl.c
index f14c41f4c0..ee4423659f 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -400,6 +400,7 @@ struct rpc_state {
 	size_t pos;
 	int in;
 	int out;
+	int any_written;
 	struct strbuf result;
 	unsigned gzip_request : 1;
 	unsigned initial_buffer : 1;
@@ -456,6 +457,8 @@ static size_t rpc_in(char *ptr, size_t eltsize,
 {
 	size_t size = eltsize * nmemb;
 	struct rpc_state *rpc = buffer_;
+	if (size)
+		rpc->any_written = 1;
 	write_or_die(rpc->in, ptr, size);
 	return size;
 }
@@ -659,6 +662,8 @@ static int post_rpc(struct rpc_state *rpc)
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);
 
+
+	rpc->any_written = 0;
 	err = run_slot(slot, NULL);
 	if (err == HTTP_REAUTH && !large_request) {
 		credential_fill(&http_auth);
@@ -667,6 +672,9 @@ static int post_rpc(struct rpc_state *rpc)
 	if (err != HTTP_OK)
 		err = -1;
 
+	if (!rpc->any_written)
+		err = -1;
+
 	curl_slist_free_all(headers);
 	free(gzip_body);
 	return err;
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index 1ec5b2747a..43665ab4a8 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -276,6 +276,36 @@ test_expect_success 'large fetch-pack requests can be split across POSTs' '
 	test_line_count = 2 posts
 '
 
+test_expect_success 'test allowreachablesha1inwant' '
+	test_when_finished "rm -rf test_reachable.git" &&
+	server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	master_sha=$(git -C "$server" rev-parse refs/heads/master) &&
+	git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
+
+	git init --bare test_reachable.git &&
+	git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
+	git -C test_reachable.git fetch origin "$master_sha"
+'
+
+test_expect_success 'test allowreachablesha1inwant with unreachable' '
+	test_when_finished "rm -rf test_reachable.git; git reset --hard $(git rev-parse HEAD)" &&
+
+	#create unreachable sha
+	echo content >file2 &&
+	git add file2 &&
+	git commit -m two &&
+	git push public HEAD:refs/heads/doomed &&
+	git push public :refs/heads/doomed &&
+
+	server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	master_sha=$(git -C "$server" rev-parse refs/heads/master) &&
+	git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
+
+	git init --bare test_reachable.git &&
+	git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
+	test_must_fail git -C test_reachable.git fetch origin "$(git rev-parse HEAD)"
+'
+
 test_expect_success EXPENSIVE 'http can handle enormous ref negotiation' '
 	(
 		cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-- 
2.11.0-442-g0c85c54a77


^ permalink raw reply related

* [PATCH 4/2] t5615-alternate-env: double-quotes in file names do not work on Windows
From: Johannes Sixt @ 2016-12-21 21:33 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Klaus Ethgen, git
In-Reply-To: <d9d2580c-a2e5-d9f3-1f56-6814b2b2285d@kdbg.org>

Protect a recently added test case with !MINGW.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 I don't remember why I did not notice this failure sooner.
 Perhaps I did, but then ran out of time to debug it...

 The patch should go on top of jk/quote-env-path-list-component.

 t/t5615-alternate-env.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5615-alternate-env.sh b/t/t5615-alternate-env.sh
index 69fd8f8911..26ebb0375d 100755
--- a/t/t5615-alternate-env.sh
+++ b/t/t5615-alternate-env.sh
@@ -79,7 +79,7 @@ test_expect_success 'mix of quoted and unquoted alternates' '
 	$two blob
 '
 
-test_expect_success 'broken quoting falls back to interpreting raw' '
+test_expect_success !MINGW 'broken quoting falls back to interpreting raw' '
 	mv one.git \"one.git &&
 	check_obj \"one.git/objects <<-EOF
 	$one blob
-- 
2.11.0.79.gf6b77ca


^ permalink raw reply related

* [PATCH bw/realpath-wo-chdir] real_path: canonicalize directory separators in root parts
From: Johannes Sixt @ 2016-12-21 21:51 UTC (permalink / raw)
  To: Brandon Williams
  Cc: git, sbeller, peff, jacob.keller, gitster, ramsay, tboegi,
	pclouds
In-Reply-To: <1481566615-75299-1-git-send-email-bmwill@google.com>

When an absolute path is resolved, resolution begins at the first path
component after the root part. The root part is just copied verbatim,
because it must not be inspected for symbolic links. For POSIX paths,
this is just the initial slash, but on Windows, the root part has the
forms c:\ or \\server\share. We do want to canonicalize the back-slashes
in the root part because these parts are compared to the result of
getcwd(), which does return a fully canonicalized path.

Factor out a helper that splits off the root part, and have it
canonicalize the copied part.

This change was prompted because t1504-ceiling-dirs.sh caught a breakage
in GIT_CEILING_DIRECTORIES handling on Windows.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 This introduces the second #ifdef GIT_WINDOWS_NATIVE in this file.
 It could be avoided if convert_slashes were defined as a do-nothing
 on POSIX, but that would not help the other occurrence. Therefore,
 I suggest to leave it at this.

 abspath.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/abspath.c b/abspath.c
index 79ee310867..1d56f5ed9f 100644
--- a/abspath.c
+++ b/abspath.c
@@ -48,6 +48,19 @@ static void get_next_component(struct strbuf *next, struct strbuf *remaining)
 	strbuf_remove(remaining, 0, end - remaining->buf);
 }
 
+/* copies root part from remaining to resolved, canonicalizing it on the way */
+static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)
+{
+	int offset = offset_1st_component(remaining->buf);
+
+	strbuf_reset(resolved);
+	strbuf_add(resolved, remaining->buf, offset);
+#ifdef GIT_WINDOWS_NATIVE
+	convert_slashes(resolved->buf);
+#endif
+	strbuf_remove(remaining, 0, offset);
+}
+
 /* We allow "recursive" symbolic links. Only within reason, though. */
 #define MAXSYMLINKS 5
 
@@ -80,14 +93,10 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,
 			goto error_out;
 	}
 
-	strbuf_reset(resolved);
+	strbuf_addstr(&remaining, path);
+	get_root_part(resolved, &remaining);
 
-	if (is_absolute_path(path)) {
-		/* absolute path; start with only root as being resolved */
-		int offset = offset_1st_component(path);
-		strbuf_add(resolved, path, offset);
-		strbuf_addstr(&remaining, path + offset);
-	} else {
+	if (!resolved->len) {
 		/* relative path; can use CWD as the initial resolved path */
 		if (strbuf_getcwd(resolved)) {
 			if (die_on_error)
@@ -95,7 +104,6 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,
 			else
 				goto error_out;
 		}
-		strbuf_addstr(&remaining, path);
 	}
 
 	/* Iterate over the remaining path components */
@@ -150,10 +158,7 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,
 
 			if (is_absolute_path(symlink.buf)) {
 				/* absolute symlink; set resolved to root */
-				int offset = offset_1st_component(symlink.buf);
-				strbuf_reset(resolved);
-				strbuf_add(resolved, symlink.buf, offset);
-				strbuf_remove(&symlink, 0, offset);
+				get_root_part(resolved, &symlink);
 			} else {
 				/*
 				 * relative symlink
-- 
2.11.0.79.gf6b77ca


^ permalink raw reply related

* Re: Bug report: Git pull hang occasionally
From: Kai Zhang @ 2016-12-21 22:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqshphge7o.fsf@gitster.mtv.corp.google.com>

I will verify it. Thanks.
> On Dec 21, 2016, at 1:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> And the unexpected discrepancy is reported by find_symref() as
>> fatal.  The server side dies, and somehow that fact is lost between
>> the upload-pack process and the client and somebody in the middle
>> (e.g. fastcgi interface or nginx webserver on the server side, or
>> the remote-curl helper on the client side) keeps the "git fetch"
>> process waiting.
>> 
>> So there seem to be two issues.  
>> 
>> - Because of the unlocked read, find_symref() can observe an
>>   inconsistent state.  Perhaps it should be updated not to die but
>>   to retry, expecting that transient inconsistency will go away.
>> 
>> - A fatal error in upload-pack is not reported back to the client
>>   to cause it exit is an obvious one, and even if we find a way to
>>   make this fatal error in find_symref() not to trigger, fatal
>>   errors in other places in the code can trigger the same symptom.
> 
> I wonder if the latter is solved by recent patch 296b847c0d
> ("remote-curl: don't hang when a server dies before any output",
> 2016-11-18) on the client side.
> 
> -- >8 --
> From: David Turner <dturner@twosigma.com>
> Date: Fri, 18 Nov 2016 15:30:49 -0500
> Subject: [PATCH] remote-curl: don't hang when a server dies before any output
> 
> In the event that a HTTP server closes the connection after giving a
> 200 but before giving any packets, we don't want to hang forever
> waiting for a response that will never come.  Instead, we should die
> immediately.
> 
> One case where this happens is when attempting to fetch a dangling
> object by its object name.  In this case, the server dies before
> sending any data.  Prior to this patch, fetch-pack would wait for
> data from the server, and remote-curl would wait for fetch-pack,
> causing a deadlock.
> 
> Despite this patch, there is other possible malformed input that could
> cause the same deadlock (e.g. a half-finished pktline, or a pktline but
> no trailing flush).  There are a few possible solutions to this:
> 
> 1. Allowing remote-curl to tell fetch-pack about the EOF (so that
> fetch-pack could know that no more data is coming until it says
> something else).  This is tricky because an out-of-band signal would
> be required, or the http response would have to be re-framed inside
> another layer of pkt-line or something.
> 
> 2. Make remote-curl understand some of the protocol.  It turns out
> that in addition to understanding pkt-line, it would need to watch for
> ack/nak.  This is somewhat fragile, as information about the protocol
> would end up in two places.  Also, pkt-lines which are already at the
> length limit would need special handling.
> 
> Both of these solutions would require a fair amount of work, whereas
> this hack is easy and solves at least some of the problem.
> 
> Still to do: it would be good to give a better error message
> than "fatal: The remote end hung up unexpectedly".
> 
> Signed-off-by: David Turner <dturner@twosigma.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> remote-curl.c               |  8 ++++++++
> t/t5551-http-fetch-smart.sh | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 38 insertions(+)
> 
> diff --git a/remote-curl.c b/remote-curl.c
> index f14c41f4c0..ee4423659f 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -400,6 +400,7 @@ struct rpc_state {
> 	size_t pos;
> 	int in;
> 	int out;
> +	int any_written;
> 	struct strbuf result;
> 	unsigned gzip_request : 1;
> 	unsigned initial_buffer : 1;
> @@ -456,6 +457,8 @@ static size_t rpc_in(char *ptr, size_t eltsize,
> {
> 	size_t size = eltsize * nmemb;
> 	struct rpc_state *rpc = buffer_;
> +	if (size)
> +		rpc->any_written = 1;
> 	write_or_die(rpc->in, ptr, size);
> 	return size;
> }
> @@ -659,6 +662,8 @@ static int post_rpc(struct rpc_state *rpc)
> 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);
> 	curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);
> 
> +
> +	rpc->any_written = 0;
> 	err = run_slot(slot, NULL);
> 	if (err == HTTP_REAUTH && !large_request) {
> 		credential_fill(&http_auth);
> @@ -667,6 +672,9 @@ static int post_rpc(struct rpc_state *rpc)
> 	if (err != HTTP_OK)
> 		err = -1;
> 
> +	if (!rpc->any_written)
> +		err = -1;
> +
> 	curl_slist_free_all(headers);
> 	free(gzip_body);
> 	return err;
> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index 1ec5b2747a..43665ab4a8 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -276,6 +276,36 @@ test_expect_success 'large fetch-pack requests can be split across POSTs' '
> 	test_line_count = 2 posts
> '
> 
> +test_expect_success 'test allowreachablesha1inwant' '
> +	test_when_finished "rm -rf test_reachable.git" &&
> +	server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> +	master_sha=$(git -C "$server" rev-parse refs/heads/master) &&
> +	git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
> +
> +	git init --bare test_reachable.git &&
> +	git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
> +	git -C test_reachable.git fetch origin "$master_sha"
> +'
> +
> +test_expect_success 'test allowreachablesha1inwant with unreachable' '
> +	test_when_finished "rm -rf test_reachable.git; git reset --hard $(git rev-parse HEAD)" &&
> +
> +	#create unreachable sha
> +	echo content >file2 &&
> +	git add file2 &&
> +	git commit -m two &&
> +	git push public HEAD:refs/heads/doomed &&
> +	git push public :refs/heads/doomed &&
> +
> +	server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> +	master_sha=$(git -C "$server" rev-parse refs/heads/master) &&
> +	git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
> +
> +	git init --bare test_reachable.git &&
> +	git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
> +	test_must_fail git -C test_reachable.git fetch origin "$(git rev-parse HEAD)"
> +'
> +
> test_expect_success EXPENSIVE 'http can handle enormous ref negotiation' '
> 	(
> 		cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> -- 
> 2.11.0-442-g0c85c54a77
> 


^ permalink raw reply

* Re: [PATCH bw/realpath-wo-chdir] real_path: canonicalize directory separators in root parts
From: Brandon Williams @ 2016-12-21 22:33 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: git, sbeller, peff, jacob.keller, gitster, ramsay, tboegi,
	pclouds
In-Reply-To: <097e3e2e-f46d-b0aa-be9c-68c274c5e3dc@kdbg.org>

On 12/21, Johannes Sixt wrote:
> When an absolute path is resolved, resolution begins at the first path
> component after the root part. The root part is just copied verbatim,
> because it must not be inspected for symbolic links. For POSIX paths,
> this is just the initial slash, but on Windows, the root part has the
> forms c:\ or \\server\share. We do want to canonicalize the back-slashes
> in the root part because these parts are compared to the result of
> getcwd(), which does return a fully canonicalized path.
> 
> Factor out a helper that splits off the root part, and have it
> canonicalize the copied part.
> 
> This change was prompted because t1504-ceiling-dirs.sh caught a breakage
> in GIT_CEILING_DIRECTORIES handling on Windows.
> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  This introduces the second #ifdef GIT_WINDOWS_NATIVE in this file.
>  It could be avoided if convert_slashes were defined as a do-nothing
>  on POSIX, but that would not help the other occurrence. Therefore,
>  I suggest to leave it at this.
> 
>  abspath.c | 29 +++++++++++++++++------------
>  1 file changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/abspath.c b/abspath.c
> index 79ee310867..1d56f5ed9f 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -48,6 +48,19 @@ static void get_next_component(struct strbuf *next, struct strbuf *remaining)
>  	strbuf_remove(remaining, 0, end - remaining->buf);
>  }
>  
> +/* copies root part from remaining to resolved, canonicalizing it on the way */
> +static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)
> +{
> +	int offset = offset_1st_component(remaining->buf);
> +
> +	strbuf_reset(resolved);
> +	strbuf_add(resolved, remaining->buf, offset);
> +#ifdef GIT_WINDOWS_NATIVE
> +	convert_slashes(resolved->buf);
> +#endif

So then the only extra cononicalization that is happening here is
converting '\\server\share' to '//server/share'? (or 'c:\' to 'c:/')

-- 
Brandon Williams

^ permalink raw reply

* Re: [PATCH 4/2] t5615-alternate-env: double-quotes in file names do not work on Windows
From: Jeff King @ 2016-12-21 22:42 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, Klaus Ethgen, git
In-Reply-To: <00b6235d-c1bc-30c2-6539-6c78c4ce9eb0@kdbg.org>

On Wed, Dec 21, 2016 at 10:33:43PM +0100, Johannes Sixt wrote:

> Protect a recently added test case with !MINGW.
> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  I don't remember why I did not notice this failure sooner.
>  Perhaps I did, but then ran out of time to debug it...
> 
>  The patch should go on top of jk/quote-env-path-list-component.
> [...]
> -test_expect_success 'broken quoting falls back to interpreting raw' '
> +test_expect_success !MINGW 'broken quoting falls back to interpreting raw' '
>  	mv one.git \"one.git &&
>  	check_obj \"one.git/objects <<-EOF
>  	$one blob

Hmph. I explicitly avoided a colon in the filename so that it would run
on MINGW. Is a double-quote also not allowed?

-Peff

^ permalink raw reply

* References to old messages
From: Stephen & Linda Smith @ 2016-12-21 23:12 UTC (permalink / raw)
  To: Git Mailing List

I want to pick up work  on a patch that I was working on previously.  

I had been told to reference (i.e. footnote) a gmane URL.  With that service 
no longer being being online, what is the preferred method footnoting?

sps

^ permalink raw reply

* Re: References to old messages
From: Stefan Beller @ 2016-12-21 23:15 UTC (permalink / raw)
  To: Stephen & Linda Smith; +Cc: Git Mailing List
In-Reply-To: <2284357.IWoC4bU66L@thunderbird>

On Wed, Dec 21, 2016 at 3:12 PM, Stephen & Linda Smith <ischis2@cox.net> wrote:
> I want to pick up work  on a patch that I was working on previously.
>
> I had been told to reference (i.e. footnote) a gmane URL.  With that service
> no longer being being online, what is the preferred method footnoting?
>
> sps

See https://public-inbox.org/git/

^ permalink raw reply

* Re: References to old messages
From: Junio C Hamano @ 2016-12-21 23:20 UTC (permalink / raw)
  To: Stephen & Linda Smith; +Cc: Git Mailing List
In-Reply-To: <2284357.IWoC4bU66L@thunderbird>

Stephen & Linda Smith <ischis2@cox.net> writes:

> I want to pick up work  on a patch that I was working on previously.  
>
> I had been told to reference (i.e. footnote) a gmane URL.  With that service 
> no longer being being online, what is the preferred method footnoting?
>
> sps

The NNTP interface of GMane is still working, so referring to e.g.
$gmane/286483 _could_ identify a message uniquely, but people who do
not usually talk NNTP to GMane won't be able to find the message
with 286483, so it is not very nice.

The same message is referred to like this around here these days:

https://public-inbox.org/git/1455685597-22445-1-git-send-email-ischis2@cox.net/

The point is people know its message id is <14556855...@cox.net> and
refer to other archives with it, even when public-inbox.org is not
available.


^ permalink raw reply

* What's cooking in git.git (Dec 2016, #06; Wed, 21)
From: Junio C Hamano @ 2016-12-21 23:25 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

You can find the changes described here in the integration branches
of the repositories listed at

    http://git-blame.blogspot.com/p/git-public-repositories.html

--------------------------------------------------
[Graduated to "master"]

* jk/index-pack-wo-repo-from-stdin (2016-12-16) 4 commits
  (merged to 'next' on 2016-12-19 at 9a88221347)
 + index-pack: skip collision check when not in repository
 + t: use nongit() function where applicable
 + index-pack: complain when --stdin is used outside of a repo
 + t5000: extract nongit function to test-lib-functions.sh

 "git index-pack --stdin" needs an access to an existing repository,
 but "git index-pack file.pack" to generate an .idx file that
 corresponds to a packfile does not.


* jk/parseopt-usage-msg-opt (2016-12-14) 1 commit
  (merged to 'next' on 2016-12-19 at c488c7c6e1)
 + parse-options: print "fatal:" before usage_msg_opt()

 The function usage_msg_opt() has been updated to say "fatal:"
 before the custom message programs give, when they want to die
 with a message about wrong command line options followed by the
 standard usage string.


* jk/quote-env-path-list-component (2016-12-21) 5 commits
  (merged to 'next' on 2016-12-21 at 729971d31e)
 + t5615-alternate-env: double-quotes in file names do not work on Windows
  (merged to 'next' on 2016-12-16 at d2cd6008b9)
 + t5547-push-quarantine: run the path separator test on Windows, too
 + tmp-objdir: quote paths we add to alternates
 + alternates: accept double-quoted paths
 + Merge branch 'jk/alt-odb-cleanup' into jk/quote-env-path-list-component

 A recent update to receive-pack to make it easier to drop garbage
 objects made it clear that GIT_ALTERNATE_OBJECT_DIRECTORIES cannot
 have a pathname with a colon in it (no surprise!), and this in turn
 made it impossible to push into a repository at such a path.  This
 has been fixed by introducing a quoting mechanism used when
 appending such a path to the colon-separated list.


* jt/mailinfo-fold-in-body-headers (2016-12-20) 1 commit
  (merged to 'next' on 2016-12-20 at ec9ae616aa)
 + mailinfo.c: move side-effects outside of assert

 Fix for NDEBUG builds.


* nd/shallow-fixup (2016-12-07) 6 commits
  (merged to 'next' on 2016-12-13 at 1a3edb8bce)
 + shallow.c: remove useless code
 + shallow.c: bit manipulation tweaks
 + shallow.c: avoid theoretical pointer wrap-around
 + shallow.c: make paint_alloc slightly more robust
 + shallow.c: stop abusing COMMIT_SLAB_SIZE for paint_info's memory pools
 + shallow.c: rename fields in paint_info to better express their purposes

 Code cleanup in shallow boundary computation.


* sb/sequencer-abort-safety (2016-12-14) 6 commits
  (merged to 'next' on 2016-12-16 at ec71fb1217)
 + Revert "sequencer: remove useless get_dir() function"
  (merged to 'next' on 2016-12-13 at 6107e43d65)
 + sequencer: remove useless get_dir() function
 + sequencer: make sequencer abort safer
 + t3510: test that cherry-pick --abort does not unsafely change HEAD
 + am: change safe_to_abort()'s not rewinding error into a warning
 + am: fix filename in safe_to_abort() error message

 Unlike "git am --abort", "git cherry-pick --abort" moved HEAD back
 to where cherry-pick started while picking multiple changes, when
 the cherry-pick stopped to ask for help from the user, and the user
 did "git reset --hard" to a different commit in order to re-attempt
 the operation.


* vs/submodule-clone-nested-submodules-alternates (2016-12-12) 1 commit
  (merged to 'next' on 2016-12-13 at 8a317ab745)
 + submodule--helper: set alternateLocation for cloned submodules

 "git clone --reference $there --recurse-submodules $super" has been
 taught to guess repositories usable as references for submodules of
 $super that are embedded in $there while making a clone of the
 superproject borrow objects from $there; extend the mechanism to
 also allow submodules of these submodules to borrow repositories
 embedded in these clones of the submodules embedded in the clone of
 the superproject.

--------------------------------------------------
[New Topics]

* bw/push-submodule-only (2016-12-20) 3 commits
 - push: add option to push only submodules
 - submodules: add RECURSE_SUBMODULES_ONLY value
 - transport: reformat flag #defines to be more readable

 "git submodule push" learned "--recurse-submodules=only option to
 push submodules out without pushing the top-level superproject.


* mk/mingw-winansi-ttyname-termination-fix (2016-12-20) 1 commit
  (merged to 'next' on 2016-12-21 at 1e8e994605)
 + mingw: consider that UNICODE_STRING::Length counts bytes

 A potential but unlikely buffer overflow in Windows port has been
 fixed.

 Will merge to 'master'.


* nd/config-misc-fixes (2016-12-20) 4 commits
 - SQUASH???
 - config.c: handle lock file in error case in git_config_rename_...
 - config.c: rename label unlock_and_out
 - config.c: handle error case for fstat() calls

 Leakage of lockfiles in the config subsystem has been fixed.


* va/i18n-even-more (2016-12-20) 1 commit
 - i18n: fix misconversion in shell scripts

 Will merge to 'next'.

--------------------------------------------------
[Stalled]

* jc/retire-compaction-heuristics (2016-11-02) 3 commits
 - SQUASH???
 - SQUASH???
 - diff: retire the original experimental "compaction" heuristics

 Waiting for a reroll.


* jc/abbrev-autoscale-config (2016-11-01) 1 commit
 - config.abbrev: document the new default that auto-scales

 Waiting for a reroll.


* jk/nofollow-attr-ignore (2016-11-02) 5 commits
 - exclude: do not respect symlinks for in-tree .gitignore
 - attr: do not respect symlinks for in-tree .gitattributes
 - exclude: convert "check_index" into a flags field
 - attr: convert "macro_ok" into a flags field
 - add open_nofollow() helper

 As we do not follow symbolic links when reading control files like
 .gitignore and .gitattributes from the index, match the behaviour
 and not follow symbolic links when reading them from the working
 tree.  This also tightens security a bit by not leaking contents of
 an unrelated file in the error messages when it is pointed at by
 one of these files that is a symbolic link.

 Perhaps we want to cover .gitmodules too with the same mechanism?


* nd/worktree-move (2016-11-28) 11 commits
 . worktree remove: new command
 . worktree move: refuse to move worktrees with submodules
 . worktree move: accept destination as directory
 . worktree move: new command
 . worktree.c: add update_worktree_location()
 . worktree.c: add validate_worktree()
 . copy.c: convert copy_file() to copy_dir_recursively()
 . copy.c: style fix
 . copy.c: convert bb_(p)error_msg to error(_errno)
 . copy.c: delete unused code in copy_file()
 . copy.c: import copy_file() from busybox

 "git worktree" learned move and remove subcommands.

 Reported to break builds on Windows.


* jc/bundle (2016-03-03) 6 commits
 - index-pack: --clone-bundle option
 - Merge branch 'jc/index-pack' into jc/bundle
 - bundle v3: the beginning
 - bundle: keep a copy of bundle file name in the in-core bundle header
 - bundle: plug resource leak
 - bundle doc: 'verify' is not about verifying the bundle

 The beginning of "split bundle", which could be one of the
 ingredients to allow "git clone" traffic off of the core server
 network to CDN.

 While I think it would make it easier for people to experiment and
 build on if the topic is merged to 'next', I am at the same time a
 bit reluctant to merge an unproven new topic that introduces a new
 file format, which we may end up having to support til the end of
 time.  It is likely that to support a "prime clone from CDN", it
 would need a lot more than just "these are the heads and the pack
 data is over there", so this may not be sufficient.

 Will discard.


* mh/connect (2016-06-06) 10 commits
 - connect: [host:port] is legacy for ssh
 - connect: move ssh command line preparation to a separate function
 - connect: actively reject git:// urls with a user part
 - connect: change the --diag-url output to separate user and host
 - connect: make parse_connect_url() return the user part of the url as a separate value
 - connect: group CONNECT_DIAG_URL handling code
 - connect: make parse_connect_url() return separated host and port
 - connect: re-derive a host:port string from the separate host and port variables
 - connect: call get_host_and_port() earlier
 - connect: document why we sometimes call get_port after get_host_and_port

 Rewrite Git-URL parsing routine (hopefully) without changing any
 behaviour.

 It has been two months without any support.  We may want to discard
 this.


* ec/annotate-deleted (2015-11-20) 1 commit
 - annotate: skip checking working tree if a revision is provided

 Usability fix for annotate-specific "<file> <rev>" syntax with deleted
 files.

 Has been waiting for a review for too long without seeing anything.

 Will discard.


* dk/gc-more-wo-pack (2016-01-13) 4 commits
 - gc: clean garbage .bitmap files from pack dir
 - t5304: ensure non-garbage files are not deleted
 - t5304: test .bitmap garbage files
 - prepare_packed_git(): find more garbage

 Follow-on to dk/gc-idx-wo-pack topic, to clean up stale
 .bitmap and .keep files.

 Has been waiting for a reroll for too long.
 cf. <xmqq60ypbeng.fsf@gitster.mtv.corp.google.com>

 Will discard.


* jc/diff-b-m (2015-02-23) 5 commits
 . WIPWIP
 . WIP: diff-b-m
 - diffcore-rename: allow easier debugging
 - diffcore-rename.c: add locate_rename_src()
 - diffcore-break: allow debugging

 "git diff -B -M" produced incorrect patch when the postimage of a
 completely rewritten file is similar to the preimage of a removed
 file; such a resulting file must not be expressed as a rename from
 other place.

 The fix in this patch is broken, unfortunately.

 Will discard.

--------------------------------------------------
[Cooking]

* gv/p4-multi-path-commit-fix (2016-12-19) 1 commit
  (merged to 'next' on 2016-12-21 at f7ba714387)
 + git-p4: fix multi-path changelist empty commits

 "git p4" that tracks multile p4 paths imported a single changelist
 that touches files in these multiple paths as one commit, followed
 by many empty commits.  This has been fixed.

 Will merge to 'master'.


* js/mingw-isatty (2016-12-21) 2 commits
 - mingw: replace isatty() hack
 - mingw: adjust is_console() to work with stdin

 Update the isatty() emulation for Windows by updating the previous
 hack that depended on internals of (older) MSVC runtime.

 Waiting for review to conclude.


* ls/p4-lfs (2016-12-20) 1 commit
 - git-p4: add diff/merge properties to .gitattributes for GitLFS files

 Update GitLFS integration with "git p4".

 Will merge to 'next'.


* ls/p4-path-encoding (2016-12-18) 1 commit
 - git-p4: fix git-p4.pathEncoding for removed files

 When "git p4" imports changelist that removes paths, it failed to
 convert pathnames when the p4 used encoding different from the one
 used on the Git side.  This has been corrected.

 Waiting for review to conclude.


* mh/fast-import-notes-fix-new (2016-12-20) 1 commit
 - fast-import: properly fanout notes when tree is imported

 "git fast-import" sometimes mishandled while rebalancing notes
 tree, which has been fixed.

 Will merge to 'next'.


* bw/pathspec-cleanup (2016-12-14) 16 commits
 - pathspec: rename prefix_pathspec to init_pathspec_item
 - pathspec: small readability changes
 - pathspec: create strip submodule slash helpers
 - pathspec: create parse_element_magic helper
 - pathspec: create parse_long_magic function
 - pathspec: create parse_short_magic function
 - pathspec: factor global magic into its own function
 - pathspec: simpler logic to prefix original pathspec elements
 - pathspec: always show mnemonic and name in unsupported_magic
 - pathspec: remove unused variable from unsupported_magic
 - pathspec: copy and free owned memory
 - pathspec: remove the deprecated get_pathspec function
 - ls-tree: convert show_recursive to use the pathspec struct interface
 - dir: convert fill_directory to use the pathspec struct interface
 - dir: remove struct path_simplify
 - mv: remove use of deprecated 'get_pathspec()'

 Code clean-up in the pathspec API.

 Waiting for the (hopefully) final round of review before 'next'.


* cp/merge-continue (2016-12-15) 4 commits
  (merged to 'next' on 2016-12-19 at 8ba0094f45)
 + merge: mark usage error strings for translation
 + merge: ensure '--abort' option takes no arguments
 + completion: add --continue option for merge
 + merge: add '--continue' option as a synonym for 'git commit'

 "git merge --continue" has been added as a synonym to "git commit"
 to conclude a merge that has stopped due to conflicts.

 Will merge to 'master'.


* ld/p4-compare-dir-vs-symlink (2016-12-18) 1 commit
  (merged to 'next' on 2016-12-20 at f58fed9ef8)
 + git-p4: avoid crash adding symlinked directory

 "git p4" misbehaved when swapping a directory and a symbolic link.

 Will merge to 'master'.


* js/prepare-sequencer-more (2016-12-14) 34 commits
 - sequencer (rebase -i): write out the final message
 - sequencer (rebase -i): write the progress into files
 - sequencer (rebase -i): show the progress
 - sequencer (rebase -i): suggest --edit-todo upon unknown command
 - sequencer (rebase -i): show only failed cherry-picks' output
 - sequencer (rebase -i): show only failed `git commit`'s output
 - run_command_opt(): optionally hide stderr when the command succeeds
 - sequencer (rebase -i): differentiate between comments and 'noop'
 - sequencer (rebase -i): implement the 'drop' command
 - sequencer (rebase -i): allow rescheduling commands
 - sequencer (rebase -i): respect strategy/strategy_opts settings
 - sequencer (rebase -i): respect the rebase.autostash setting
 - sequencer (rebase -i): run the post-rewrite hook, if needed
 - sequencer (rebase -i): record interrupted commits in rewritten, too
 - sequencer (rebase -i): copy commit notes at end
 - sequencer (rebase -i): set the reflog message consistently
 - sequencer (rebase -i): refactor setting the reflog message
 - sequencer (rebase -i): allow fast-forwarding for edit/reword
 - sequencer (rebase -i): implement the 'reword' command
 - sequencer (rebase -i): leave a patch upon error
 - sequencer (rebase -i): update refs after a successful rebase
 - sequencer (rebase -i): the todo can be empty when continuing
 - sequencer (rebase -i): skip some revert/cherry-pick specific code path
 - sequencer (rebase -i): remove CHERRY_PICK_HEAD when no longer needed
 - sequencer (rebase -i): allow continuing with staged changes
 - sequencer (rebase -i): write an author-script file
 - sequencer (rebase -i): implement the short commands
 - sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
 - sequencer (rebase -i): write the 'done' file
 - sequencer (rebase -i): learn about the 'verbose' mode
 - sequencer (rebase -i): implement the 'exec' command
 - sequencer (rebase -i): implement the 'edit' command
 - sequencer (rebase -i): implement the 'noop' command
 - sequencer: support a new action: 'interactive rebase'

 The sequencer has further been extended in preparation to act as a
 back-end for "rebase -i".

 Waiting for review to conclude.


* lt/shortlog-by-committer (2016-12-20) 3 commits
  (merged to 'next' on 2016-12-21 at c72e6e7f76)
 + t4201: make tests work with and without the MINGW prerequiste
  (merged to 'next' on 2016-12-19 at 555976fc0a)
 + shortlog: test and document --committer option
 + shortlog: group by committer information

 "git shortlog" learned "--committer" option to group commits by
 committer, instead of author.


* bw/realpath-wo-chdir (2016-12-12) 4 commits
 - real_path: have callers use real_pathdup and strbuf_realpath
 - real_path: create real_pathdup
 - real_path: convert real_path_internal to strbuf_realpath
 - real_path: resolve symlinks by hand
 (this branch is used by bw/grep-recurse-submodules.)

 The implementation of "real_path()" was to go there with chdir(2)
 and call getcwd(3), but this obviously wouldn't be usable in a
 threaded environment.  Rewrite it to manually resolve relative
 paths including symbolic links in path components.

 Will wait for an issue on Windows to be resolved.
 cf. <097e3e2e-f46d-b0aa-be9c-68c274c5e3dc@kdbg.org>


* jk/difftool-in-subdir (2016-12-11) 4 commits
  (merged to 'next' on 2016-12-21 at c1056e014b)
 + difftool: rename variables for consistency
 + difftool: chdir as early as possible
 + difftool: sanitize $workdir as early as possible
 + difftool: fix dir-diff index creation when in a subdirectory

 Even though an fix was attempted in Git 2.9.3 days, but running
 "git difftool --dir-diff" from a subdirectory never worked. This
 has been fixed.

 Will merge to 'master'.


* ls/filter-process (2016-12-18) 2 commits
  (merged to 'next' on 2016-12-19 at 5ed29656db)
 + t0021: fix flaky test
  (merged to 'next' on 2016-12-12 at 8ed1f9eb02)
 + docs: warn about possible '=' in clean/smudge filter process values

 Doc update.

 Will merge to 'master'.


* js/difftool-builtin (2016-11-28) 2 commits
 - difftool: implement the functionality in the builtin
 - difftool: add a skeleton for the upcoming builtin

 Rewrite a scripted porcelain "git difftool" in C.

 What's the doneness of this topic?


* sb/push-make-submodule-check-the-default (2016-11-29) 2 commits
  (merged to 'next' on 2016-12-12 at 1863e05af5)
 + push: change submodule default to check when submodules exist
 + submodule add: extend force flag to add existing repos

 Turn the default of "push.recurseSubmodules" to "check" when
 submodules seem to be in use.

 Will cook in 'next'.


* sb/submodule-embed-gitdir (2016-12-12) 6 commits
  (merged to 'next' on 2016-12-21 at e6cdbcf013)
 + submodule: add absorb-git-dir function
 + move connect_work_tree_and_git_dir to dir.h
 + worktree: check if a submodule uses worktrees
 + test-lib-functions.sh: teach test_commit -C <dir>
 + submodule helper: support super prefix
 + submodule: use absolute path for computing relative path connecting

 A new submodule helper "git submodule embedgitdirs" to make it
 easier to move embedded .git/ directory for submodules in a
 superproject to .git/modules/ (and point the latter with the former
 that is turned into a "gitdir:" file) has been added.

 Will merge to 'master'.


* bw/grep-recurse-submodules (2016-12-16) 7 commits
 - grep: search history of moved submodules
 - grep: enable recurse-submodules to work on <tree> objects
 - grep: optionally recurse into submodules
 - grep: add submodules as a grep source type
 - submodules: load gitmodules file from commit sha1
 - submodules: add helper to determine if a submodule is initialized
 - submodules: add helper to determine if a submodule is populated
 (this branch uses bw/realpath-wo-chdir.)

 "git grep" learns to optionally recurse into submodules.

 Will hold until bw/realpath-wo-chdir settles.


* dt/smart-http-detect-server-going-away (2016-11-18) 2 commits
  (merged to 'next' on 2016-12-05 at 3ea70d01af)
 + upload-pack: optionally allow fetching any sha1
 + remote-curl: don't hang when a server dies before any output

 Originally merged to 'next' on 2016-11-21

 When the http server gives an incomplete response to a smart-http
 rpc call, it could lead to client waiting for a full response that
 will never come.  Teach the client side to notice this condition
 and abort the transfer.

 An improvement counterproposal has failed.
 cf. <20161114194049.mktpsvgdhex2f4zv@sigill.intra.peff.net>

 Will cook in 'next'.


* mm/push-social-engineering-attack-doc (2016-11-14) 1 commit
  (merged to 'next' on 2016-12-05 at 9a2b5bd1a9)
 + doc: mention transfer data leaks in more places

 Originally merged to 'next' on 2016-11-16

 Doc update on fetching and pushing.

 Will cook in 'next'.


* jc/compression-config (2016-11-15) 1 commit
  (merged to 'next' on 2016-12-05 at 323769ca07)
 + compression: unify pack.compression configuration parsing

 Originally merged to 'next' on 2016-11-23

 Compression setting for producing packfiles were spread across
 three codepaths, one of which did not honor any configuration.
 Unify these so that all of them honor core.compression and
 pack.compression variables the same way.

 Will cook in 'next'.


* mm/gc-safety-doc (2016-11-16) 1 commit
  (merged to 'next' on 2016-12-05 at 031ecc1886)
 + git-gc.txt: expand discussion of races with other processes

 Originally merged to 'next' on 2016-11-17

 Doc update.

 Will cook in 'next'.


* kn/ref-filter-branch-list (2016-12-08) 20 commits
 - branch: implement '--format' option
 - branch: use ref-filter printing APIs
 - branch, tag: use porcelain output
 - ref-filter: allow porcelain to translate messages in the output
 - ref-filter: add an 'rstrip=<N>' option to atoms which deal with refnames
 - ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'
 - ref-filter: rename the 'strip' option to 'lstrip'
 - ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
 - ref-filter: introduce refname_atom_parser()
 - ref-filter: introduce refname_atom_parser_internal()
 - ref-filter: make "%(symref)" atom work with the ':short' modifier
 - ref-filter: add support for %(upstream:track,nobracket)
 - ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
 - ref-filter: introduce format_ref_array_item()
 - ref-filter: move get_head_description() from branch.c
 - ref-filter: modify "%(objectname:short)" to take length
 - ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
 - ref-filter: include reference to 'used_atom' within 'atom_value'
 - ref-filter: implement %(if), %(then), and %(else) atoms
 - for-each-ref: do not segv with %(HEAD) on an unborn branch

 The code to list branches in "git branch" has been consolidated
 with the more generic ref-filter API.

 What's the doneness of the topic?  I recall discussing die vs empty
 and also saw a "squash this in when you reroll", but I lost track.


* bw/transport-protocol-policy (2016-12-15) 6 commits
  (merged to 'next' on 2016-12-19 at 166168205c)
 + http: respect protocol.*.allow=user for http-alternates
 + transport: add from_user parameter to is_transport_allowed
 + http: create function to get curl allowed protocols
 + transport: add protocol policy config option
 + http: always warn if libcurl version is too old
 + lib-proto-disable: variable name fix

 Finer-grained control of what protocols are allowed for transports
 during clone/fetch/push have been enabled via a new configuration
 mechanism.

 Will merge to 'master'.


* jt/fetch-no-redundant-tag-fetch-map (2016-11-11) 1 commit
  (merged to 'next' on 2016-12-05 at 432f9469a7)
 + fetch: do not redundantly calculate tag refmap

 Originally merged to 'next' on 2016-11-16

 Code cleanup to avoid using redundant refspecs while fetching with
 the --tags option.

 Will cook in 'next'.


* sb/submodule-config-cleanup (2016-11-22) 3 commits
  (merged to 'next' on 2016-12-05 at 658b8764bf)
 + submodule-config: clarify parsing of null_sha1 element
 + submodule-config: rename commit_sha1 to treeish_name
 + submodule config: inline config_from_{name, path}

 Originally merged to 'next' on 2016-11-23

 Minor code clean-up.

 Will cook in 'next'.


* jc/push-default-explicit (2016-10-31) 2 commits
  (merged to 'next' on 2016-12-05 at d63f3777af)
 + push: test pushing ambiguously named branches
 + push: do not use potentially ambiguous default refspec

 Originally merged to 'next' on 2016-11-01

 A lazy "git push" without refspec did not internally use a fully
 specified refspec to perform 'current', 'simple', or 'upstream'
 push, causing unnecessary "ambiguous ref" errors.

 Will cook in 'next'.


* jc/git-open-cloexec (2016-11-02) 3 commits
 - sha1_file: stop opening files with O_NOATIME
 - git_open_cloexec(): use fcntl(2) w/ FD_CLOEXEC fallback
 - git_open(): untangle possible NOATIME and CLOEXEC interactions

 The codeflow of setting NOATIME and CLOEXEC on file descriptors Git
 opens has been simplified.

 We may want to drop the tip one.


* jk/no-looking-at-dotgit-outside-repo-final (2016-10-26) 1 commit
  (merged to 'next' on 2016-12-05 at 0c77e39cd5)
 + setup_git_env: avoid blind fall-back to ".git"

 Originally merged to 'next' on 2016-10-26

 This is the endgame of the topic to avoid blindly falling back to
 ".git" when the setup sequence said we are _not_ in Git repository.
 A corner case that happens to work right now may be broken by a
 call to die("BUG").

 Will cook in 'next'.


* jc/reset-unmerge (2016-10-24) 1 commit
 - reset: --unmerge

 After "git add" is run prematurely during a conflict resolution,
 "git diff" can no longer be used as a way to sanity check by
 looking at the combined diff.  "git reset" learned a new
 "--unmerge" option to recover from this situation.

 Will discard.
 This may not be needed, given that update-index has a similar
 option.


* jc/merge-base-fp-only (2016-10-19) 8 commits
 . merge-base: fp experiment
 - merge: allow to use only the fp-only merge bases
 - merge-base: limit the output to bases that are on first-parent chain
 - merge-base: mark bases that are on first-parent chain
 - merge-base: expose get_merge_bases_many_0() a bit more
 - merge-base: stop moving commits around in remove_redundant()
 - sha1_name: remove ONELINE_SEEN bit
 - commit: simplify fastpath of merge-base

 An experiment of merge-base that ignores common ancestors that are
 not on the first parent chain.

 Will discard.
 The whole premise feels wrong.


* tb/convert-stream-check (2016-10-27) 2 commits
 - convert.c: stream and fast search for binary
 - read-cache: factor out get_sha1_from_index() helper

 End-of-line conversion sometimes needs to see if the current blob
 in the index has NULs and CRs to base its decision.  We used to
 always get a full statistics over the blob, but in many cases we
 can return early when we have seen "enough" (e.g. if we see a
 single NUL, the blob will be handled as binary).  The codepaths
 have been optimized by using streaming interface.

 Will discard.
 Retracted.
 cf. <20161102071646.GA5094@tb-raspi>


* pb/bisect (2016-10-18) 27 commits
 - bisect--helper: remove the dequote in bisect_start()
 - bisect--helper: retire `--bisect-auto-next` subcommand
 - bisect--helper: retire `--bisect-autostart` subcommand
 - bisect--helper: retire `--bisect-write` subcommand
 - bisect--helper: `bisect_replay` shell function in C
 - bisect--helper: `bisect_log` shell function in C
 - bisect--helper: retire `--write-terms` subcommand
 - bisect--helper: retire `--check-expected-revs` subcommand
 - bisect--helper: `bisect_state` & `bisect_head` shell function in C
 - bisect--helper: `bisect_autostart` shell function in C
 - bisect--helper: retire `--next-all` subcommand
 - bisect--helper: retire `--bisect-clean-state` subcommand
 - bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
 - t6030: no cleanup with bad merge base
 - bisect--helper: `bisect_start` shell function partially in C
 - bisect--helper: `get_terms` & `bisect_terms` shell function in C
 - bisect--helper: `bisect_next_check` & bisect_voc shell function in C
 - bisect--helper: `check_and_set_terms` shell function in C
 - bisect--helper: `bisect_write` shell function in C
 - bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
 - bisect--helper: `bisect_reset` shell function in C
 - wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
 - t6030: explicitly test for bisection cleanup
 - bisect--helper: `bisect_clean_state` shell function in C
 - bisect--helper: `write_terms` shell function in C
 - bisect: rewrite `check_term_format` shell function in C
 - bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

 Move more parts of "git bisect" to C.

 Waiting for review to conclude.


* st/verify-tag (2016-10-10) 7 commits
 - t/t7004-tag: Add --format specifier tests
 - t/t7030-verify-tag: Add --format specifier tests
 - builtin/tag: add --format argument for tag -v
 - builtin/verify-tag: add --format to verify-tag
 - tag: add format specifier to gpg_verify_tag
 - ref-filter: add function to print single ref_array_item
 - gpg-interface, tag: add GPG_VERIFY_QUIET flag

 "git tag" and "git verify-tag" learned to put GPG verification
 status in their "--format=<placeholders>" output format.

 Waiting for a reroll.
 cf. <20161007210721.20437-1-santiago@nyu.edu>


* sb/attr (2016-11-11) 35 commits
 . completion: clone can initialize specific submodules
 . clone: add --init-submodule=<pathspec> switch
 . submodule update: add `--init-default-path` switch
 . pathspec: allow escaped query values
 . pathspec: allow querying for attributes
 . pathspec: move prefix check out of the inner loop
 . pathspec: move long magic parsing out of prefix_pathspec
 - Documentation: fix a typo
 - attr: keep attr stack for each check
 - attr: convert to new threadsafe API
 - attr: make git_check_attr_counted static
 - attr.c: outline the future plans by heavily commenting
 - attr.c: always pass check[] to collect_some_attrs()
 - attr.c: introduce empty_attr_check_elems()
 - attr.c: correct ugly hack for git_all_attrs()
 - attr.c: rename a local variable check
 - attr.c: pass struct git_attr_check down the callchain
 - attr.c: add push_stack() helper
 - attr: support quoting pathname patterns in C style
 - attr: expose validity check for attribute names
 - attr: add counted string version of git_check_attr()
 - attr: retire git_check_attrs() API
 - attr: convert git_check_attrs() callers to use the new API
 - attr: convert git_all_attrs() to use "struct git_attr_check"
 - attr: (re)introduce git_check_attr() and struct git_attr_check
 - attr: rename function and struct related to checking attributes
 - attr.c: plug small leak in parse_attr_line()
 - attr.c: tighten constness around "git_attr" structure
 - attr.c: simplify macroexpand_one()
 - attr.c: mark where #if DEBUG ends more clearly
 - attr.c: complete a sentence in a comment
 - attr.c: explain the lack of attr-name syntax check in parse_attr()
 - attr.c: update a stale comment on "struct match_attr"
 - attr.c: use strchrnul() to scan for one line
 - commit.c: use strchrnul() to scan for one line

 The attributes API has been updated so that it can later be
 optimized using the knowledge of which attributes are queried.
 Building on top of the updated API, the pathspec machinery learned
 to select only paths with given attributes set.

 The parts near the tip about pathspec would need to work better
 with bw/pathspec-cleanup topic and has been dropped for now.


* va/i18n-perl-scripts (2016-12-14) 16 commits
  (merged to 'next' on 2016-12-19 at ec800aba9f)
 + i18n: difftool: mark warnings for translation
 + i18n: send-email: mark composing message for translation
 + i18n: send-email: mark string with interpolation for translation
 + i18n: send-email: mark warnings and errors for translation
 + i18n: send-email: mark strings for translation
 + i18n: add--interactive: mark status words for translation
 + i18n: add--interactive: remove %patch_modes entries
 + i18n: add--interactive: mark edit_hunk_manually message for translation
 + i18n: add--interactive: i18n of help_patch_cmd
 + i18n: add--interactive: mark patch prompt for translation
 + i18n: add--interactive: mark plural strings
 + i18n: clean.c: match string with git-add--interactive.perl
 + i18n: add--interactive: mark strings with interpolation for translation
 + i18n: add--interactive: mark simple here-documents for translation
 + i18n: add--interactive: mark strings for translation
 + Git.pm: add subroutines for commenting lines

 Porcelain scripts written in Perl are getting internationalized.

 Will merge to 'master'.


* jc/latin-1 (2016-09-26) 2 commits
  (merged to 'next' on 2016-12-05 at fb549caa12)
 + utf8: accept "latin-1" as ISO-8859-1
 + utf8: refactor code to decide fallback encoding

 Originally merged to 'next' on 2016-09-28

 Some platforms no longer understand "latin-1" that is still seen in
 the wild in e-mail headers; replace them with "iso-8859-1" that is
 more widely known when conversion fails from/to it.

 Will cook in 'next'.


* sg/fix-versioncmp-with-common-suffix (2016-12-08) 8 commits
 - versioncmp: generalize version sort suffix reordering
 - squash! versioncmp: use earliest-longest contained suffix to determine sorting order
 - versioncmp: use earliest-longest contained suffix to determine sorting order
 - versioncmp: cope with common part overlapping with prerelease suffix
 - versioncmp: pass full tagnames to swap_prereleases()
 - t7004-tag: add version sort tests to show prerelease reordering issues
 - t7004-tag: use test_config helper
 - t7004-tag: delete unnecessary tags with test_when_finished

 The prereleaseSuffix feature of version comparison that is used in
 "git tag -l" did not correctly when two or more prereleases for the
 same release were present (e.g. when 2.0, 2.0-beta1, and 2.0-beta2
 are there and the code needs to compare 2.0-beta1 and 2.0-beta2).

 Waiting for review to conclude.
 cf. <20161208142401.1329-1-szeder.dev@gmail.com>


* jc/merge-drop-old-syntax (2015-04-29) 1 commit
  (merged to 'next' on 2016-12-05 at 041946dae0)
 + merge: drop 'git merge <message> HEAD <commit>' syntax

 Originally merged to 'next' on 2016-10-11

 Stop supporting "git merge <message> HEAD <commit>" syntax that has
 been deprecated since October 2007, and issues a deprecation
 warning message since v2.5.0.

 Will cook in 'next'.

^ permalink raw reply

* Re: config for `format-patch --notes`?
From: Johan Herland @ 2016-12-22  0:00 UTC (permalink / raw)
  To: Norbert Kiesel; +Cc: Git mailing list
In-Reply-To: <CAM+g_NtsSaiZdy2Pq0gR9AaO6xiNnGSUbj5yd-uW3S80roEGCg@mail.gmail.com>

On Wed, Dec 21, 2016 at 1:18 AM, Norbert Kiesel <nkiesel@gmail.com> wrote:
> Hi,
>
> I use `git format-patch master..myBranch` quite a bit to send patches
> to other developers.  I also add notes to the commits
> so that I e.g. remember which patches were emailed to whom.  `git log`
> has an option to automatically include the notes in
> the output.  However, I can't find such an option for `git
> format-patch`.  Am I missing something?

I assume you mean _config_ option here (format-patch has had a --notes
command-line option since v1.8.1). AFAICS, there's no config option
that corresponds to the format-patch --notes command-line option.

You can easily alias or script your way around this, e.g.:

  git config alias.fp 'format-patch --notes'

This creates a 'git fp' command that does what you want, I believe.

Alternatively, if you need more control/automation of the resulting
patches, you can write a script to edit the output files from
format-patch. Currently, I don't believe there is any format-patch
hook available to automatically trigger such a script, but, again,
that can be achieved with an alias.

If you believe a config option would be a useful addition for more
users than yourself, I am sure the community welcomes patches adding
a format.notes config option.

> Another nice option would to to somehow include the branch name in the
> resulting output.  Right now I use either notes
> or abuse the `--subject` option for this.

From git-config(1):

    branch.<name>.description
        Branch description, can be edited with git branch
        --edit-description. Branch description is automatically added
        in the format-patch cover letter or request-pull summary.


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* Re: References to old messages
From: Eric Wong @ 2016-12-22  1:00 UTC (permalink / raw)
  To: Stephen & Linda Smith; +Cc: git
In-Reply-To: <2284357.IWoC4bU66L@thunderbird>

Stephen & Linda Smith <ischis2@cox.net> wrote:
> I had been told to reference (i.e. footnote) a gmane URL.  With that service 
> no longer being being online, what is the preferred method footnoting?

What the others said about Message-IDs and public-inbox :)

If you only have old URLs pointing to gmane and no NNTP access,
you can also search for "gmane:$ARTICLE_NUMBER" via public-inbox
(at least up to messages around this summer):

	https://public-inbox.org/git/?q=gmane:12345

https://public-inbox.org/git/_/text/help documents other
search prefixes you can use, too.

^ permalink raw reply

* Re: [PATCH] mailinfo.c: move side-effects outside of assert
From: Kyle J. McKay @ 2016-12-22  2:21 UTC (permalink / raw)
  To: Jeff King
  Cc: Johannes Schindelin, Jonathan Tan, Junio C Hamano,
	Git mailing list
In-Reply-To: <20161221155539.aykcmkuzqvq733ri@sigill.intra.peff.net>

On Dec 21, 2016, at 07:55, Jeff King wrote:

> On Tue, Dec 20, 2016 at 09:54:15PM -0800, Kyle J. McKay wrote:
>
>>> I wasn't aware anybody actually built with NDEBUG at all. You'd  
>>> have to
>>> explicitly ask for it via CFLAGS, so I assume most people don't.
>>
>> Not a good assumption.  You know what happens when you assume[1],  
>> right? ;)
>
> Kind of. If it's a configuration that nobody[1] in the Git development
> community intended to support or test, then isn't the person  
> triggering
> it the one making assumptions?

No, I don't think so.  NDEBUG is very clearly specified in POSIX [1].

If NDEBUG is defined then "assert(...)" disappears (and in a nice way  
so as not to precipitate "unused variable" warnings).  "N" being "No"  
or "Not" or "Negated" or "bar over the top" + "DEBUG" meaning Not  
DEBUG.  So the code that goes away when NDEBUG is defined is clearly  
debug code.

Considering the wide deployment and use of Git at this point I think  
rather the opposite to be true that "Git does Not require DEBUGging  
code to be enabled for everyday use."  The alternative that it does  
suggests it's not ready for prime time and quite clearly that's not  
the case.

>> I've been defining NDEBUG whenever I make a release build for quite  
>> some
>> time (not just for Git) in order to squeeze every last possible  
>> drop of
>> performance out of it.
>
> I think here you are getting into superstition. Is there any single
> assert() in Git that will actually have an impact on performance?

You have suggested there is and that Git is enabling NDEBUG for  
exactly that reason -- to increase performance:

>> On Dec 20, 2016, at 08:45, Jeff King wrote:
>>
>>> I do notice that we set NDEBUG for nedmalloc, though if I am  
>>> reading the
>>> Makefile right, it is just for compiling those files. It looks like
>>> there are a ton of asserts there that _are_ potentially expensive


>> Perhaps Git should provide a "verify" macro.  Works like "assert"  
>> except
>> that it doesn't go away when NDEBUG is defined.  Being Git-provided  
>> it could
>> also use Git's die function.  Then Git could do a global replace of  
>> assert
>> with verify and institute a no-assert policy.
>
> What would be the advantage of that over `if(...) die("BUG: ...")`? It
> does not require you to write a reason in the die(), but I am not sure
> that is a good thing.

You have stated that you believe the current "assert" calls in Git  
(excluding nedmalloc) should not magically disappear when NDEBUG is  
defined.  So precluding a more labor intensive approach where all  
currently existing "assert(...)" calls are replaced with an "if (!...)  
die(...)" combination, providing a "verify" macro is a quick way to  
make that happen.  Consider this, was the value that Jonathan provided  
for the "die" string immediately obvious to you?  It sure wasn't to  
me.  That means that whoever does the "assert(...)" -> "if(!...)die"  
swap out may need to be intimately familiar with that particular piece  
of code or the result will be no better than using a "verify" macro.

I'm just trying to find a quick and easy way to accommodate your  
wishes without redefining the semantics of NDEBUG. ;)

--Kyle

[1] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/assert.h.html

^ permalink raw reply

* Re: [PATCH] mailinfo.c: move side-effects outside of assert
From: Jeff King @ 2016-12-22  3:34 UTC (permalink / raw)
  To: Kyle J. McKay
  Cc: Johannes Schindelin, Jonathan Tan, Junio C Hamano,
	Git mailing list
In-Reply-To: <F5001DF2-20C2-4757-997F-9D40BD48E1D9@gmail.com>

On Wed, Dec 21, 2016 at 06:21:37PM -0800, Kyle J. McKay wrote:

> > Kind of. If it's a configuration that nobody[1] in the Git development
> > community intended to support or test, then isn't the person triggering
> > it the one making assumptions?
> 
> No, I don't think so.  NDEBUG is very clearly specified in POSIX [1].

I know what NDEBUG is, and I know it is widely used in other projects.
My claim is only that I do not think that the use of NDEBUG was
generally considered when people wrote assert() in git. That is
certainly true of me. I don't know about other developers.

> > I think here you are getting into superstition. Is there any single
> > assert() in Git that will actually have an impact on performance?
> 
> You have suggested there is and that Git is enabling NDEBUG for exactly that
> reason -- to increase performance:

Sorry if I wasn't clear, but I meant in Git itself, not in the nedmalloc
compat code. I think it's worth considering them separately because the
latter is not part of most Git builds in the first place, and certainly
it is written in a different style, and may have different assumptions
about how people might build it.

I do not know the nedmalloc code well at all. I gave a brief read of the
results of "git grep 'assert('" and it looked like some of its
assertions are more involved than simple comparisons. So I guessed that
perhaps the reason that NDEBUG was added there when the code was
imported is that there is a performance difference there. But that's
just a guess. It may or may not be borne out by measurements.

I'd be surprised if any of the assertions in the rest of git have a
noticeable impact (and I could not detect one by running t/perf).

> > What would be the advantage of that over `if(...) die("BUG: ...")`? It
> > does not require you to write a reason in the die(), but I am not sure
> > that is a good thing.
> 
> You have stated that you believe the current "assert" calls in Git
> (excluding nedmalloc) should not magically disappear when NDEBUG is defined.

Well, no, I mostly just said that I do not think there is any point in
defining NDEBUG in the first place, as there is little or no benefit to
removing those asserts from the built product.

> So precluding a more labor intensive approach where all currently existing
> "assert(...)" calls are replaced with an "if (!...) die(...)" combination,
> providing a "verify" macro is a quick way to make that happen.  Consider
> this, was the value that Jonathan provided for the "die" string immediately
> obvious to you?  It sure wasn't to me.  That means that whoever does the
> "assert(...)" -> "if(!...)die" swap out may need to be intimately familiar
> with that particular piece of code or the result will be no better than
> using a "verify" macro.

Sure, if you want to mass-convert them, doing so with a macro similar to
assert is the simplest way. I don't think we are in a huge hurry to do
that conversion though. I'm not complaining that NDEBUG works as
advertised by disabling asserts. I'm just claiming that it's largely
pointless in our code base, and I'd consider die("BUG") to be our
"usual" style. If any change is to be made, it would be to suggest
people prefer die("BUG") as a style guideline for future patches. But I
haven't seen anybody agree (or disagree) with the notion, so I'd
hesitate to impose my style suggestion without more discussion in that
area.

If we _did_ accept that as a style guideline, then we could move over
existing assert() calls over time (either as janitorial projects, or as
people touch the related code). But there's not a pressing need to do it
quickly.

-Peff

^ permalink raw reply

* Re: [PATCH] mailinfo.c: move side-effects outside of assert
From: Kyle J. McKay @ 2016-12-22  3:53 UTC (permalink / raw)
  To: Jeff King
  Cc: Johannes Schindelin, Jonathan Tan, Junio C Hamano,
	Git mailing list
In-Reply-To: <F5001DF2-20C2-4757-997F-9D40BD48E1D9@gmail.com>

On Dec 21, 2016, at 18:21, Kyle J. McKay wrote:

> On Dec 21, 2016, at 07:55, Jeff King wrote:
>
>> On Tue, Dec 20, 2016 at 09:54:15PM -0800, Kyle J. McKay wrote:
>>
>>>> I wasn't aware anybody actually built with NDEBUG at all. You'd  
>>>> have to
>>>> explicitly ask for it via CFLAGS, so I assume most people don't.
>>>
>>> Not a good assumption.  You know what happens when you assume[1],  
>>> right? ;)
>>
>> Kind of. If it's a configuration that nobody[1] in the Git  
>> development
>> community intended to support or test, then isn't the person  
>> triggering
>> it the one making assumptions?
>
> No, I don't think so.  NDEBUG is very clearly specified in POSIX [1].
>
> If NDEBUG is defined then "assert(...)" disappears (and in a nice  
> way so as not to precipitate "unused variable" warnings).  "N" being  
> "No" or "Not" or "Negated" or "bar over the top" + "DEBUG" meaning  
> Not DEBUG.  So the code that goes away when NDEBUG is defined is  
> clearly debug code.

I think there is a useful distinction here that I make that's worth  
sharing.  Perhaps it's splitting hairs, but I categorize this "extra"  
code that we've been discussing ("assert(...)" or "if (!...) die(...)"  
or "verify(...)" into two groups:


1) DEBUG code

This is code that developers use when creating new features.  Or  
helpful code that's needed when stepping through a program with the  
debugger to debug a problem.  Or even code that's only used by some  
kind of external "test".  It may be expensive, it may do things that  
should never be done in a build for wider consumption (such as write  
information to special log files, write special syslog messages  
etc.).  Often this code is used in combination with a "-g" debug  
symbols build and possibly even a "-O0" or "-O1" option.

Code like this has no place in a release executable meant for general  
use by an end user.

2) DIAGNOSTIC code

This is near zero overhead code that is intended to be left in a  
release build meant for general use and normally sits there not doing  
anything and NOT leaching any performance out of the build either.   
Its sole purpose in life is to provide a trail of "bread crumbs" if  
the executable goes ***BOOM***.  These "bread crumbs" should be just  
enough when combined with feedback from the unfortunate user who  
experienced the meltdown to re-create the issue in a real DEBUG build  
and find and fix the problem.


It seems to me what you are saying is that Git's "assert" calls are  
DIAGNOSTIC and therefore belong in a release build -- well, except for  
the nedmalloc "assert" calls which do not.

What I'm saying is if they are diagnostic and not debug (and I'm not  
arguing one way or the other, but you've already indicated they are  
near zero overhead which suggests they are indeed diagnostic in  
nature), then they do not belong inside an "assert" which can be  
disabled with "NDEBUG".  I'm arguing that "assert" is not intended for  
diagnostic code, but only debug code as used by nedmalloc.  Having Git  
treat "NDEBUG" one way -- "no, no, do NOT define NDEBUG because that  
disables Git diagnostics and I promise you there's no performance  
penalty" -- versus nedmalloc -- "yes, yes please DO define NDEBUG  
unless you really need our slow debugging code to be present for  
debugging purposes" -- just creates needless unnecessary confusion.

--Kyle

^ permalink raw reply

* Re: [PATCH] mailinfo.c: move side-effects outside of assert
From: Jeff King @ 2016-12-22  3:59 UTC (permalink / raw)
  To: Kyle J. McKay
  Cc: Johannes Schindelin, Jonathan Tan, Junio C Hamano,
	Git mailing list
In-Reply-To: <99C4A905-D66B-4609-9E55-06F9BC301C74@gmail.com>

On Wed, Dec 21, 2016 at 07:53:15PM -0800, Kyle J. McKay wrote:

> It seems to me what you are saying is that Git's "assert" calls are
> DIAGNOSTIC and therefore belong in a release build -- well, except for the
> nedmalloc "assert" calls which do not.

Yes, I think that is a good way of thinking about it (modulo that I
really can't say one way or the other about nedmalloc's uses).

There _are_ some DEBUG-type things in Git that are protected by #ifdefs
that default to "off" (grep for DIFF_DEBUG, for instance). I'm actually
of the opinion that debugging code like that should be in all builds and
triggerable at run-time, provided it carries no significant performance
penalty when the run-time switch is not enabled. But I do agree that's a
totally separate question than from your DEBUG/DIAGNOSTIC distinction.

-Peff

^ permalink raw reply

* builtin difftool parsing issue
From: Paul Sbarra @ 2016-12-22  4:53 UTC (permalink / raw)
  To: johannes.schindelin; +Cc: davvid, dennis, git, gitster

Sadly, I haven't been able to figure out how to get the mbox file from
this tread into gmail, but wanted to report a parsing issue I've found
with the builtin difftool.

Original Patch:
https://public-inbox.org/git/ac91e4818cfb5c5af6b5874662dbeb61cde1f69d.1480019834.git.johannes.schindelin@gmx.de/#t

> + *status = *++p;
> + if (!status || p[1])
> + return error("unexpected trailer: '%s'", p);
> + return 0;

The p[1] null check assumes the status is only one character long, but
git-diff's raw output format shows that a numeric value can follow in
the copy-edit and rename-edit cases.

I'm looking forward to seeing the builtin difftool land.  I came across it
while investigating adding --submodule=diff (expanding on diff's
recent addition) support and this looks more promising then the perl
script.  Hopefully I will make some progress.  Any tips/pointers would
be greatly appreciated.

Thanks

^ permalink raw reply

* Re: [PATCH bw/realpath-wo-chdir] real_path: canonicalize directory separators in root parts
From: Johannes Sixt @ 2016-12-22  6:07 UTC (permalink / raw)
  To: Brandon Williams
  Cc: git, sbeller, peff, jacob.keller, gitster, ramsay, tboegi,
	pclouds
In-Reply-To: <20161221223304.GA119874@google.com>

Am 21.12.2016 um 23:33 schrieb Brandon Williams:
> On 12/21, Johannes Sixt wrote:
>> +/* copies root part from remaining to resolved, canonicalizing it on the way */
>> +static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)
>> +{
>> +	int offset = offset_1st_component(remaining->buf);
>> +
>> +	strbuf_reset(resolved);
>> +	strbuf_add(resolved, remaining->buf, offset);
>> +#ifdef GIT_WINDOWS_NATIVE
>> +	convert_slashes(resolved->buf);
>> +#endif
>
> So then the only extra cononicalization that is happening here is
> converting '\\server\share' to '//server/share'? (or 'c:\' to 'c:/')

Correct. All other directory separators are canonicalized by the primary 
function, strbuf_realpath.

-- Hannes


^ permalink raw reply

* Re: [PATCH 4/2] t5615-alternate-env: double-quotes in file names do not work on Windows
From: Johannes Sixt @ 2016-12-22  6:13 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Klaus Ethgen, git
In-Reply-To: <20161221224222.6fn6irefmd6li6oa@sigill.intra.peff.net>

Am 21.12.2016 um 23:42 schrieb Jeff King:
> On Wed, Dec 21, 2016 at 10:33:43PM +0100, Johannes Sixt wrote:
>
>> Protect a recently added test case with !MINGW.
>>
>> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
>> ---
>>  I don't remember why I did not notice this failure sooner.
>>  Perhaps I did, but then ran out of time to debug it...
>>
>>  The patch should go on top of jk/quote-env-path-list-component.
>> [...]
>> -test_expect_success 'broken quoting falls back to interpreting raw' '
>> +test_expect_success !MINGW 'broken quoting falls back to interpreting raw' '
>>  	mv one.git \"one.git &&
>>  	check_obj \"one.git/objects <<-EOF
>>  	$one blob
>
> Hmph. I explicitly avoided a colon in the filename so that it would run
> on MINGW. Is a double-quote also not allowed?

It is not allowed; that was my conclusion. But now that you ask, I'll 
double-check.

-- Hannes


^ permalink raw reply

* Re: config for `format-patch --notes`?
From: Norbert Kiesel @ 2016-12-22  7:08 UTC (permalink / raw)
  To: johan; +Cc: git, nkiesel

Thanks for the reply.  I did not knew that branch descriptions
automatically make it in the cover letter.
Learned something new!

Unfortunately this would still mean I have to manually add the branch
name to the branch description.

I will try to create a patch for adding a config option for the
--notes command line option over the upcoming
free days.

^ permalink raw reply

* Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references
From: Michael Haggerty @ 2016-12-22  8:15 UTC (permalink / raw)
  To: Marc Branchaud, Paul Mackerras; +Cc: git
In-Reply-To: <bfbd5992-da30-b1f0-59e5-a2f36d2e3062@xiplink.com>

On 12/21/2016 08:07 PM, Marc Branchaud wrote:
> On 2016-12-20 07:05 PM, Michael Haggerty wrote:
>> On 12/20/2016 04:01 PM, Marc Branchaud wrote:
>>> [...]
>>> Please don't change the remotebgcolor default.
>>>
>>> Also, perhaps the default remoterefbgcolor should be
>>>     set remoterefbgcolor $headbgcolor
>>> ?
>>>
>>> I say this because when I applied the series, without the last patch, I
>>> was miffed that the remote/ref colour had changed.
>>
>> This is a one-time inconvenience that gitk developers will experience. I
>> doubt that users jump backwards and forwards in gitk versions very often.
> 
> In what way do you mean it's restricted to gitk developers?

Maybe I misunderstood your objection.

While developing this, I realized that the very first time your run gitk
(i.e., when you don't already have a configuration file), it writes the
then-current default colors into your configuration file. If you later
switch gitk versions to a version with different default colors, the
colors from the first-run version are preserved (unless the names of the
variables used to hold the colors are changed).

So if you would run the tip version of my branch first, then the parent
of that version, you would continue to see the colors as modified by the
final commit. If you then switch to the master version, the remote
branch names go back to green (because it goes back to using
`headbgcolor` again), but the remote prefix stays light brown. I thought
you were unhappy about some form of this unexpected persistence problem.
But this problem will mostly be experienced by gitk developers who jump
back and forth between versions.

A normal user probably mostly moves forward through version numbers, and
only occasionally. Such a user, jumping from master to the tip of my
branch (assuming they haven't customized anything), would see

* local branches stay lime
* remote branch prefixes stay pale orange (they don't change to light
brown because the pale orange color from master is stored in their
configuration file)
* remote branch names change from lime to brown (because the
`remoterefbgcolor` configuration setting didn't exist before)

> Patch 12 introduces remoterefbgcolor, with a specific default value.
> Prior to that, the "ref part" of remote refs was rendered with
> headbgcolor.  Users who changed their headbgcolor are used to seeing the
> "ref part" of remote refs in the same color as their local heads.
> Applying patch 12 changes the "ref part" color of remote refs, for such
> users.
> 
> All I'm saying is that make the remoterefbgcolor default be $headbgcolor
> avoids this.

For somebody who thinks that most people will want local and
remote-tracking branch names to be rendered in the same color, your
suggestion would be an improvement.

But for somebody like me who thinks it is a good idea to render
remote-tracking branch names in a different color than local branch
names, this is a feature, not a bug. Even a user who explicitly
configured `headbgcolor` to, say, purple, wasn't necessarily expressing
a wish to have remote-tracking branch names rendered in purple. Until
now they had no choice to set these colors separately!

So even for somebody who configured this setting before, I think that
having the remote-tracking branch names change color when the user
upgrades to this version is a good thing, because it lets the user know
that these two things can now be colored independently. If they don't
like the new default brown color, such a user knows where to change it
(even to make it agree with `headbgcolor` if they want that).

But I understand that this is a matter of personal preference. I have
but one "vote" :-)

Michael


^ 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