Git development
 help / color / mirror / Atom feed
* [PATCH v2 3/3] connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config
From: Johannes Schindelin @ 2017-01-26 14:52 UTC (permalink / raw)
  To: git; +Cc: Segev Finer, Junio C Hamano, Jeff King
In-Reply-To: <cover.1485442231.git.johannes.schindelin@gmx.de>

From: Segev Finer <segev208@gmail.com>

This environment variable and configuration value allow to
override the autodetection of plink/tortoiseplink in case that
Git gets it wrong.

[jes: wrapped overly-long lines, changed get_ssh_variant() to
handle_ssh_variant() to accomodate the change from the
putty/tortoiseplink variables to port_option/needs_batch.]

Signed-off-by: Segev Finer <segev208@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/config.txt |  7 +++++++
 Documentation/git.txt    |  7 +++++++
 connect.c                | 24 ++++++++++++++++++++++--
 t/t5601-clone.sh         | 26 ++++++++++++++++++++++++++
 4 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index af2ae4cc02..f2c210f0a0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1949,6 +1949,13 @@ Environment variable settings always override any matches.  The URLs that are
 matched against are those given directly to Git commands.  This means any URLs
 visited as a result of a redirection do not participate in matching.
 
+ssh.variant::
+	Override the autodetection of plink/tortoiseplink in the SSH
+	command that 'git fetch' and 'git push' use. It can be set to
+	either `ssh`, `plink`, `putty` or `tortoiseplink`. Any other
+	value will be treated as normal ssh. This is useful in case
+	that Git gets this wrong.
+
 i18n.commitEncoding::
 	Character encoding the commit messages are stored in; Git itself
 	does not care per se, but this information is necessary e.g. when
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 4f208fab92..c322558aa7 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -1020,6 +1020,13 @@ Usually it is easier to configure any desired options through your
 personal `.ssh/config` file.  Please consult your ssh documentation
 for further details.
 
+`GIT_SSH_VARIANT`::
+	If this environment variable is set, it overrides the autodetection
+	of plink/tortoiseplink in the SSH command that 'git fetch' and 'git
+	push' use. It can be set to either `ssh`, `plink`, `putty` or
+	`tortoiseplink`. Any other value will be treated as normal ssh. This
+	is useful in case that Git gets this wrong.
+
 `GIT_ASKPASS`::
 	If this environment variable is set, then Git commands which need to
 	acquire passwords or passphrases (e.g. for HTTP or IMAP authentication)
diff --git a/connect.c b/connect.c
index 9f750eacb6..7b4437578b 100644
--- a/connect.c
+++ b/connect.c
@@ -691,6 +691,24 @@ static const char *get_ssh_command(void)
 	return NULL;
 }
 
+static int handle_ssh_variant(int *port_option, int *needs_batch)
+{
+	const char *variant;
+
+	if (!(variant = getenv("GIT_SSH_VARIANT")) &&
+		git_config_get_string_const("ssh.variant", &variant))
+		return 0;
+
+	if (!strcmp(variant, "plink") || !strcmp(variant, "putty"))
+		*port_option = 'P';
+	else if (!strcmp(variant, "tortoiseplink")) {
+		*port_option = 'P';
+		*needs_batch = 1;
+	}
+
+	return 1;
+}
+
 /*
  * This returns a dummy child_process if the transport protocol does not
  * need fork(2), or a struct child_process object if it does.  Once done,
@@ -817,7 +835,8 @@ struct child_process *git_connect(int fd[2], const char *url,
 				ssh_argv0 = xstrdup(ssh);
 			}
 
-			if (ssh_argv0) {
+			if (!handle_ssh_variant(&port_option, &needs_batch) &&
+			    ssh_argv0) {
 				const char *base = basename(ssh_argv0);
 
 				if (!strcasecmp(base, "tortoiseplink") ||
@@ -828,9 +847,10 @@ struct child_process *git_connect(int fd[2], const char *url,
 					   !strcasecmp(base, "plink.exe")) {
 					port_option = 'P';
 				}
-				free(ssh_argv0);
 			}
 
+			free(ssh_argv0);
+
 			argv_array_push(&conn->args, ssh);
 			if (flags & CONNECT_IPV4)
 				argv_array_push(&conn->args, "-4");
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 9335e10c2a..b52b8acf98 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -401,6 +401,32 @@ test_expect_success 'single quoted plink.exe in GIT_SSH_COMMAND' '
 	expect_ssh "-v -P 123" myhost src
 '
 
+test_expect_success 'GIT_SSH_VARIANT overrides plink detection' '
+	copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" &&
+	GIT_SSH_VARIANT=ssh \
+	git clone "[myhost:123]:src" ssh-bracket-clone-variant-1 &&
+	expect_ssh "-p 123" myhost src
+'
+
+test_expect_success 'ssh.variant overrides plink detection' '
+	copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" &&
+	git -c ssh.variant=ssh \
+		clone "[myhost:123]:src" ssh-bracket-clone-variant-2 &&
+	expect_ssh "-p 123" myhost src
+'
+
+test_expect_success 'GIT_SSH_VARIANT overrides plink detection to plink' '
+	GIT_SSH_VARIANT=plink \
+	git clone "[myhost:123]:src" ssh-bracket-clone-variant-3 &&
+	expect_ssh "-P 123" myhost src
+'
+
+test_expect_success 'GIT_SSH_VARIANT overrides plink to tortoiseplink' '
+	GIT_SSH_VARIANT=tortoiseplink \
+	git clone "[myhost:123]:src" ssh-bracket-clone-variant-4 &&
+	expect_ssh "-batch -P 123" myhost src
+'
+
 # Reset the GIT_SSH environment variable for clone tests.
 setup_ssh_wrapper
 
-- 
2.11.1.windows.prerelease.2.9.g3014b57

^ permalink raw reply related

* SoC Microprojects 2017
From: Pranit Bauva @ 2017-01-26 15:57 UTC (permalink / raw)
  To: Git List

Hey everyone,

I helped in just re-organizing the micro project list for 2017. I have
removed the ones which I remember have been done and I have added one
new.

Please add any microproject if it comes to your mind or reply here so
I will add it.

Unfortunately, I can't send the patch here (SMTP blocked by institute
proxy) but I have included it as a link[1]. And here is the PR[2].

[1]: https://patch-diff.githubusercontent.com/raw/git/git.github.io/pull/219.patch
[2]: https://github.com/git/git.github.io/pull/219

Regards,
Pranit Bauva

^ permalink raw reply

* [PATCH v2 0/1] Let `git status` handle a not-yet-started `rebase -i` gracefully
From: Johannes Schindelin @ 2017-01-26 16:08 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Stefan Beller, Matthieu Moy, Guillaume Pages
In-Reply-To: <99f6de4be107044fdf01ee796f42e124ac147891.1453480067.git.johannes.schindelin@gmx.de>

When the `done` file is missing, we die()d. This is not necessary, we
can do much better than that.

Changes since v1:

- When `done` is missing, we still read `git-rebase-todo` and report the
  next steps.

- We now report a missing git-rebase-todo.

- Added a test (thanks, Matthieu, for prodding me into working harder
  ;-)).

- As I changed so much, I took authorship of the patch.


Johannes Schindelin (1):
  status: be prepared for not-yet-started interactive rebase

 t/t7512-status-help.sh | 19 +++++++++++++++++++
 wt-status.c            | 14 ++++++++++----
 2 files changed, 29 insertions(+), 4 deletions(-)


base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe
Published-As: https://github.com/dscho/git/releases/tag/wt-status-v2
Fetch-It-Via: git fetch https://github.com/dscho/git wt-status-v2

Interdiff vs v1:

 diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
 index 5c3db656df..458608cc1e 100755
 --- a/t/t7512-status-help.sh
 +++ b/t/t7512-status-help.sh
 @@ -944,4 +944,23 @@ EOF
  	test_i18ncmp expected actual
  '
  
 +test_expect_success 'status: handle not-yet-started rebase -i gracefully' '
 +	ONTO=$(git rev-parse --short HEAD^) &&
 +	COMMIT=$(git rev-parse --short HEAD) &&
 +	EDITOR="git status --untracked-files=no >actual" git rebase -i HEAD^ &&
 +	cat >expected <<EOF &&
 +On branch several_commits
 +No commands done.
 +Next command to do (1 remaining command):
 +   pick $COMMIT four_commit
 +  (use "git rebase --edit-todo" to view and edit)
 +You are currently editing a commit while rebasing branch '\''several_commits'\'' on '\''$ONTO'\''.
 +  (use "git commit --amend" to amend the current commit)
 +  (use "git rebase --continue" once you are satisfied with your changes)
 +
 +nothing to commit (use -u to show untracked files)
 +EOF
 +	test_i18ncmp expected actual
 +'
 +
  test_done
 diff --git a/wt-status.c b/wt-status.c
 index 13afe66649..4dff0b3e21 100644
 --- a/wt-status.c
 +++ b/wt-status.c
 @@ -1169,12 +1169,12 @@ static void show_rebase_information(struct wt_status *s,
  		struct string_list have_done = STRING_LIST_INIT_DUP;
  		struct string_list yet_to_do = STRING_LIST_INIT_DUP;
  
 -		if ((read_rebase_todolist("rebase-merge/done", &have_done)) ||
 -		    (read_rebase_todolist("rebase-merge/git-rebase-todo",
 -				  &yet_to_do)))
 +		read_rebase_todolist("rebase-merge/done", &have_done);
 +		if (read_rebase_todolist("rebase-merge/git-rebase-todo",
 +					 &yet_to_do))
  			status_printf_ln(s, color,
 -				_("rebase-i not started yet."));
 -		else if (have_done.nr == 0)
 +				_("git-rebase-todo is missing."));
 +		if (have_done.nr == 0)
  			status_printf_ln(s, color, _("No commands done."));
  		else {
  			status_printf_ln(s, color,
 @@ -1192,9 +1192,7 @@ static void show_rebase_information(struct wt_status *s,
  					_("  (see more in file %s)"), git_path("rebase-merge/done"));
  		}
  
 -		if (have_done.nr == 0)
 -			; /* do nothing */
 -		else if (yet_to_do.nr == 0)
 +		if (yet_to_do.nr == 0)
  			status_printf_ln(s, color,
  					 _("No commands remaining."));
  		else {

-- 
2.11.1.windows.prerelease.2.9.g3014b57


^ permalink raw reply

* [PATCH v2 1/1] status: be prepared for not-yet-started interactive rebase
From: Johannes Schindelin @ 2017-01-26 16:08 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Stefan Beller, Matthieu Moy, Guillaume Pages
In-Reply-To: <cover.1485446899.git.johannes.schindelin@gmx.de>

Some developers might want to call `git status` in a working
directory where they just started an interactive rebase, but the
edit script is still opened in the editor.

Let's show a meaningful message in such cases.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t7512-status-help.sh | 19 +++++++++++++++++++
 wt-status.c            | 14 ++++++++++----
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index 5c3db656df..458608cc1e 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -944,4 +944,23 @@ EOF
 	test_i18ncmp expected actual
 '
 
+test_expect_success 'status: handle not-yet-started rebase -i gracefully' '
+	ONTO=$(git rev-parse --short HEAD^) &&
+	COMMIT=$(git rev-parse --short HEAD) &&
+	EDITOR="git status --untracked-files=no >actual" git rebase -i HEAD^ &&
+	cat >expected <<EOF &&
+On branch several_commits
+No commands done.
+Next command to do (1 remaining command):
+   pick $COMMIT four_commit
+  (use "git rebase --edit-todo" to view and edit)
+You are currently editing a commit while rebasing branch '\''several_commits'\'' on '\''$ONTO'\''.
+  (use "git commit --amend" to amend the current commit)
+  (use "git rebase --continue" once you are satisfied with your changes)
+
+nothing to commit (use -u to show untracked files)
+EOF
+	test_i18ncmp expected actual
+'
+
 test_done
diff --git a/wt-status.c b/wt-status.c
index a715e71906..4dff0b3e21 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1135,14 +1135,17 @@ static void abbrev_sha1_in_line(struct strbuf *line)
 	strbuf_list_free(split);
 }
 
-static void read_rebase_todolist(const char *fname, struct string_list *lines)
+static int read_rebase_todolist(const char *fname, struct string_list *lines)
 {
 	struct strbuf line = STRBUF_INIT;
 	FILE *f = fopen(git_path("%s", fname), "r");
 
-	if (!f)
+	if (!f) {
+		if (errno == ENOENT)
+			return -1;
 		die_errno("Could not open file %s for reading",
 			  git_path("%s", fname));
+	}
 	while (!strbuf_getline_lf(&line, f)) {
 		if (line.len && line.buf[0] == comment_line_char)
 			continue;
@@ -1152,6 +1155,7 @@ static void read_rebase_todolist(const char *fname, struct string_list *lines)
 		abbrev_sha1_in_line(&line);
 		string_list_append(lines, line.buf);
 	}
+	return 0;
 }
 
 static void show_rebase_information(struct wt_status *s,
@@ -1166,8 +1170,10 @@ static void show_rebase_information(struct wt_status *s,
 		struct string_list yet_to_do = STRING_LIST_INIT_DUP;
 
 		read_rebase_todolist("rebase-merge/done", &have_done);
-		read_rebase_todolist("rebase-merge/git-rebase-todo", &yet_to_do);
-
+		if (read_rebase_todolist("rebase-merge/git-rebase-todo",
+					 &yet_to_do))
+			status_printf_ln(s, color,
+				_("git-rebase-todo is missing."));
 		if (have_done.nr == 0)
 			status_printf_ln(s, color, _("No commands done."));
 		else {
-- 
2.11.1.windows.prerelease.2.9.g3014b57

^ permalink raw reply related

* PATCH 1/2] abspath: add absolute_pathdup()
From: René Scharfe @ 2017-01-26 17:47 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano

Add a function that returns a buffer containing the absolute path of its
argument and a semantic patch for its intended use.  It avoids an extra
string copy to a static buffer.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 abspath.c                                | 7 +++++++
 cache.h                                  | 1 +
 contrib/coccinelle/xstrdup_or_null.cocci | 6 ++++++
 3 files changed, 14 insertions(+)

diff --git a/abspath.c b/abspath.c
index fce40fddcc..2f0c26e0e2 100644
--- a/abspath.c
+++ b/abspath.c
@@ -239,6 +239,13 @@ const char *absolute_path(const char *path)
 	return sb.buf;
 }
 
+char *absolute_pathdup(const char *path)
+{
+	struct strbuf sb = STRBUF_INIT;
+	strbuf_add_absolute_path(&sb, path);
+	return strbuf_detach(&sb, NULL);
+}
+
 /*
  * Unlike prefix_path, this should be used if the named file does
  * not have to interact with index entry; i.e. name of a random file
diff --git a/cache.h b/cache.h
index 00a029af36..d7b7a8cd7a 100644
--- a/cache.h
+++ b/cache.h
@@ -1069,6 +1069,7 @@ const char *real_path(const char *path);
 const char *real_path_if_valid(const char *path);
 char *real_pathdup(const char *path);
 const char *absolute_path(const char *path);
+char *absolute_pathdup(const char *path);
 const char *remove_leading_path(const char *in, const char *prefix);
 const char *relative_path(const char *in, const char *prefix, struct strbuf *sb);
 int normalize_path_copy_len(char *dst, const char *src, int *prefix_len);
diff --git a/contrib/coccinelle/xstrdup_or_null.cocci b/contrib/coccinelle/xstrdup_or_null.cocci
index 3fceef132b..8e05d1ca4b 100644
--- a/contrib/coccinelle/xstrdup_or_null.cocci
+++ b/contrib/coccinelle/xstrdup_or_null.cocci
@@ -5,3 +5,9 @@ expression V;
 - if (E)
 -    V = xstrdup(E);
 + V = xstrdup_or_null(E);
+
+@@
+expression E;
+@@
+- xstrdup(absolute_path(E))
++ absolute_pathdup(E)
-- 
2.11.0


^ permalink raw reply related

* [PATCH 2/2] use absolute_pathdup()
From: René Scharfe @ 2017-01-26 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano
In-Reply-To: <d94d742d-1247-ac35-c081-7db1f2178d34@web.de>

Apply the symantic patch for converting callers that duplicate the
result of absolute_path() to call absolute_pathdup() instead, which
avoids an extra string copy to a static buffer.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 builtin/clone.c             | 4 ++--
 builtin/submodule--helper.c | 2 +-
 worktree.c                  | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 5ef81927a6..3f63edbbf9 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -170,7 +170,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)
 
 	strbuf_addstr(&path, repo);
 	raw = get_repo_path_1(&path, is_bundle);
-	canon = raw ? xstrdup(absolute_path(raw)) : NULL;
+	canon = raw ? absolute_pathdup(raw) : NULL;
 	strbuf_release(&path);
 	return canon;
 }
@@ -894,7 +894,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 	path = get_repo_path(repo_name, &is_bundle);
 	if (path)
-		repo = xstrdup(absolute_path(repo_name));
+		repo = absolute_pathdup(repo_name);
 	else if (!strchr(repo_name, ':'))
 		die(_("repository '%s' does not exist"), repo_name);
 	else
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 74614a951e..899dc334e3 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -626,7 +626,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 				   module_clone_options);
 
 	strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name);
-	sm_gitdir = xstrdup(absolute_path(sb.buf));
+	sm_gitdir = absolute_pathdup(sb.buf);
 	strbuf_reset(&sb);
 
 	if (!is_absolute_path(path)) {
diff --git a/worktree.c b/worktree.c
index 53b4771c04..d633761575 100644
--- a/worktree.c
+++ b/worktree.c
@@ -145,7 +145,7 @@ static struct worktree *get_linked_worktree(const char *id)
 
 static void mark_current_worktree(struct worktree **worktrees)
 {
-	char *git_dir = xstrdup(absolute_path(get_git_dir()));
+	char *git_dir = absolute_pathdup(get_git_dir());
 	int i;
 
 	for (i = 0; worktrees[i]; i++) {
-- 
2.11.0


^ permalink raw reply related

* Re: SubmittingPatches: drop temporal reference for PGP signing
From: Stefan Beller @ 2017-01-26 17:57 UTC (permalink / raw)
  To: Cornelius Weig
  Cc: Philip Oakley, Johannes Sixt, bitte.keine.werbung.einwerfen,
	git@vger.kernel.org, Junio C Hamano, thomas.braun, John Keeping
In-Reply-To: <baff65ba-1e98-d5a7-5b5a-a50a7fc723ee@tngtech.com>

On Thu, Jan 26, 2017 at 5:30 AM, Cornelius Weig
<cornelius.weig@tngtech.com> wrote:
>
>>
>> Yeah I agree. My patch was not the best shot by far.
>>
>
> How about something along these lines? Does the forward reference
> break the main line of thought too severly?
>
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 08352de..c2b0cbe 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -216,12 +216,12 @@ that it will be postponed.
>  Exception:  If your mailer is mangling patches then someone may ask
>  you to re-send them using MIME, that is OK.
>
> -Do not PGP sign your patch, at least for now.  Most likely, your
> -maintainer or other people on the list would not have your PGP
> -key and would not bother obtaining it anyway.  Your patch is not
> -judged by who you are; a good patch from an unknown origin has a
> -far better chance of being accepted than a patch from a known,
> -respected origin that is done poorly or does incorrect things.
> +Do not PGP sign your patch, but do sign-off your work as explained in (5).
> +Most likely, your maintainer or other people on the list would not have your
> +PGP key and would not bother obtaining it anyway. Your patch is not judged by
> +who you are; a good patch from an unknown origin has a far better chance of
> +being accepted than a patch from a known, respected origin that is done poorly
> +or does incorrect things.
>
>  If you really really really really want to do a PGP signed
>  patch, format it as "multipart/signed", not a text/plain message
> @@ -246,7 +246,7 @@ patch.
>       *2* The mailing list: git@vger.kernel.org
>
>
> -(5) Sign your work
> +(5) Certify your work by signing off
>
>  To improve tracking of who did what, we've borrowed the
>  "sign-off" procedure from the Linux kernel project on patches

I like it.

Thanks,
Stefan

^ permalink raw reply

* Re: [PATCH v2 1/1] status: be prepared for not-yet-started interactive rebase
From: Matthieu Moy @ 2017-01-26 18:04 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Junio C Hamano, Stefan Beller, Guillaume Pages
In-Reply-To: <alpine.DEB.2.20.1701261708370.3469@virtualbox>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Some developers might want to call `git status` in a working
> directory where they just started an interactive rebase, but the
> edit script is still opened in the editor.
>
> Let's show a meaningful message in such cases.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  t/t7512-status-help.sh | 19 +++++++++++++++++++
>  wt-status.c            | 14 ++++++++++----
>  2 files changed, 29 insertions(+), 4 deletions(-)

The patch looks good to me.

> @@ -1166,8 +1170,10 @@ static void show_rebase_information(struct wt_status *s,
>  		struct string_list yet_to_do = STRING_LIST_INIT_DUP;
>  
>  		read_rebase_todolist("rebase-merge/done", &have_done);
> -		read_rebase_todolist("rebase-merge/git-rebase-todo", &yet_to_do);
> -
> +		if (read_rebase_todolist("rebase-merge/git-rebase-todo",
> +					 &yet_to_do))
> +			status_printf_ln(s, color,
> +				_("git-rebase-todo is missing."));

I first was surprised not to see this "git-rebase-todo" in the output of
status, but the testcase tests a missing 'done', not a missing todo, so
it's normal.

Thanks,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2017, #04; Mon, 23)
From: Junio C Hamano @ 2017-01-26 18:05 UTC (permalink / raw)
  To: Lars Schneider; +Cc: Jeff King, Johannes Schindelin, git
In-Reply-To: <D9F0976B-9F78-44BE-B9DD-CAB6506FA3A9@gmail.com>

Lars Schneider <larsxschneider@gmail.com> writes:

> The difference between Travis and my machine is that I changed the 
> default shell to ZSH with a few plugins [1]. If I run the test with 
> plain BASH on my Mac then I can reproduce the test failure. Therefore,
> we might want to adjust the commit message if anyone else can reproduce
> the problem on a Mac. 

With "Reportedly macOS does this, at least in the Travis builds.",
even with the result from you in your follow-up message to the
message I am responding to, I think what Peff wrote in the log
message is good enough.

Thanks for digging, and thanks Peff for coming up the workaround.

^ permalink raw reply

* Re: [PATCH v2 1/1] status: be prepared for not-yet-started interactive rebase
From: Stefan Beller @ 2017-01-26 18:02 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: git@vger.kernel.org, Junio C Hamano, Matthieu Moy,
	Guillaume Pages
In-Reply-To: <alpine.DEB.2.20.1701261708370.3469@virtualbox>

On Thu, Jan 26, 2017 at 8:08 AM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> Some developers might want to call `git status` in a working
> directory where they just started an interactive rebase, but the
> edit script is still opened in the editor.
>
> Let's show a meaningful message in such cases.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  t/t7512-status-help.sh | 19 +++++++++++++++++++
>  wt-status.c            | 14 ++++++++++----
>  2 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
> index 5c3db656df..458608cc1e 100755
> --- a/t/t7512-status-help.sh
> +++ b/t/t7512-status-help.sh
> @@ -944,4 +944,23 @@ EOF
>         test_i18ncmp expected actual
>  '
>
> +test_expect_success 'status: handle not-yet-started rebase -i gracefully' '
> +       ONTO=$(git rev-parse --short HEAD^) &&
> +       COMMIT=$(git rev-parse --short HEAD) &&
> +       EDITOR="git status --untracked-files=no >actual" git rebase -i HEAD^ &&
> +       cat >expected <<EOF &&
> +On branch several_commits
> +No commands done.
> +Next command to do (1 remaining command):
> +   pick $COMMIT four_commit
> +  (use "git rebase --edit-todo" to view and edit)
> +You are currently editing a commit while rebasing branch '\''several_commits'\'' on '\''$ONTO'\''.
> +  (use "git commit --amend" to amend the current commit)
> +  (use "git rebase --continue" once you are satisfied with your changes)
> +
> +nothing to commit (use -u to show untracked files)
> +EOF
> +       test_i18ncmp expected actual
> +'
> +
>  test_done
> diff --git a/wt-status.c b/wt-status.c
> index a715e71906..4dff0b3e21 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1135,14 +1135,17 @@ static void abbrev_sha1_in_line(struct strbuf *line)
>         strbuf_list_free(split);
>  }
>
> -static void read_rebase_todolist(const char *fname, struct string_list *lines)
> +static int read_rebase_todolist(const char *fname, struct string_list *lines)
>  {
>         struct strbuf line = STRBUF_INIT;
>         FILE *f = fopen(git_path("%s", fname), "r");
>
> -       if (!f)
> +       if (!f) {
> +               if (errno == ENOENT)
> +                       return -1;
>                 die_errno("Could not open file %s for reading",
>                           git_path("%s", fname));

While at it, fix the translation with die_errno(_(..),..) ?
(The errno message is translated already by the system,
which make untranslated die_errno things awkward for the users.)

Otherwise the patch looks good to me

^ permalink raw reply

* Re: SubmittingPatches: drop temporal reference for PGP signing
From: Junio C Hamano @ 2017-01-26 18:18 UTC (permalink / raw)
  To: Cornelius Weig
  Cc: Stefan Beller, Philip Oakley, Johannes Sixt,
	bitte.keine.werbung.einwerfen, git@vger.kernel.org, thomas.braun,
	John Keeping
In-Reply-To: <baff65ba-1e98-d5a7-5b5a-a50a7fc723ee@tngtech.com>

Cornelius Weig <cornelius.weig@tngtech.com> writes:

> How about something along these lines? Does the forward reference
> break the main line of thought too severly?

I find it a bit distracting for those who know PGP signing has
nothing to do with signing off your patch, but I think that is OK
because they are not the primary target audience of this part of the
document.

I however am more worried that it may be misleading to mention these
two in the same sentence.  Those who skim these paragraphs without
knowing the difference between the two may get a false impression
that these two may somehow be related because they are mentioned in
the same sentence.

The retitling of section (5) you did, without any other change,
might be sufficient.  It may also help to be even more explicit in
the updated title, i.e. s/by signing off/by adding Signed-off-by:/

Thanks.

> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 08352de..c2b0cbe 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -216,12 +216,12 @@ that it will be postponed.
>  Exception:  If your mailer is mangling patches then someone may ask
>  you to re-send them using MIME, that is OK.
>  
> -Do not PGP sign your patch, at least for now.  Most likely, your
> -maintainer or other people on the list would not have your PGP
> -key and would not bother obtaining it anyway.  Your patch is not
> -judged by who you are; a good patch from an unknown origin has a
> -far better chance of being accepted than a patch from a known,
> -respected origin that is done poorly or does incorrect things.
> +Do not PGP sign your patch, but do sign-off your work as explained in (5).
> +Most likely, your maintainer or other people on the list would not have your
> +PGP key and would not bother obtaining it anyway. Your patch is not judged by
> +who you are; a good patch from an unknown origin has a far better chance of
> +being accepted than a patch from a known, respected origin that is done poorly
> +or does incorrect things.
>  
>  If you really really really really want to do a PGP signed
>  patch, format it as "multipart/signed", not a text/plain message
> @@ -246,7 +246,7 @@ patch.
>       *2* The mailing list: git@vger.kernel.org
>  
>  
> -(5) Sign your work
> +(5) Certify your work by signing off
>  
>  To improve tracking of who did what, we've borrowed the
>  "sign-off" procedure from the Linux kernel project on patches

^ permalink raw reply

* Re: Fixing the warning about warning(""); was: Re: [PATCH] difftool.c: mark a file-local symbol with static
From: Junio C Hamano @ 2017-01-26 18:26 UTC (permalink / raw)
  To: Jeff King
  Cc: Johannes Sixt, Johannes Schindelin, David Aguilar, Ramsay Jones,
	GIT Mailing-list
In-Reply-To: <20170126143252.ne533mcv3n2ksbai@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Thu, Jan 26, 2017 at 07:39:55AM +0100, Johannes Sixt wrote:
>
>> Am 25.01.2017 um 23:01 schrieb Jeff King:
>> > +#pragma GCC diagnostic ignored "-Wformat-zero-length"
>> 
>> Last time I used #pragma GCC in a cross-platform project, it triggered an
>> "unknown pragma" warning for MSVC. (It was the C++ compiler, I don't know if
>> the C compiler would also warn.) It would have to be spelled like this:
>> 
>> #pragma warning(disable: 4068)   /* MSVC: unknown pragma */
>> #pragma GCC diagnostic ignored "-Wformat-zero-length"
>> 
>> Dscho mentioned that he's compiling with MSVC. It would do him a favor.
>
> Bleh. The point of #pragma is to ignore ones you don't know about.

Yes.  Let's not go there; somebody else's compiler will complain
about "#pragma warning(disable: 4068)" that it does not understand.

> Anyway. I do not want to make life harder for anyone. I think there are
> several options floating around now, so I will let Junio decide which
> one he wants to pick up.

Well, I'll keep the "do nothing other than squelching this instance"
to solve one of the two problems for now.  

The other "can we make it harder to make the same issue and reduce
the need to discuss this again on the list?" can be an independent
follow-up patch, and I do have a preference (the "less horrible
version, that is static inline warning_blank_line(void)" you gave us
in <20170124230500.h3fasbvutjkkke5h@sigill.intra.peff.net>), but I
do not think we are in a hurry.

Thanks.



^ permalink raw reply

* Re: [RFC 12/14] fetch-pack: do not printf after closing stdout
From: Jonathan Tan @ 2017-01-26 18:18 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kb+VVQoimCDCxk1JPtVdDcS0vgi3NgVfo_aZ_=feed8Cw@mail.gmail.com>

On 01/25/2017 04:50 PM, Stefan Beller wrote:
> On Wed, Jan 25, 2017 at 2:03 PM, Jonathan Tan <jonathantanmy@google.com> wrote:
>> In fetch-pack, during a stateless RPC, printf is invoked after stdout is
>> closed. Update the code to not do this, preserving the existing
>> behavior.
>
> This seems to me as if it could go as an independent
> bugfix(?) or refactoring as this seems to be unclear from the code?

The subsequent patches in this patch set are dependent on this patch, 
but it's true that this could be sent out on its own first.

I'm not sure if bugfix is the right word, since the existing behavior is 
correct (except perhaps that we rely on the fact that printf after 
closing stdout does effectively nothing).

^ permalink raw reply

* Re: [PATCH] gpg-interface: Add some output from gpg when it errors out.
From: Junio C Hamano @ 2017-01-26 18:29 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git
In-Reply-To: <20170126025530.r4fesye447do5wdx@glandium.org>

Mike Hommey <mh@glandium.org> writes:

>> With that information recorded in the log (or in-code comment, or
>> both), if it turns out that some lines with the prefix are useful
>> (or some other lines without the prefix are not very useful), they
>> can tweak the filtering criteria as appropriate, with confidence
>> that they _know_ for what purpose the initial "filter lines with the
>> prefix" was trying to serve, and their update is still in the same
>> spirit as the original, only executed better.
>
> Come to think of it, and considering that mutt happily signs emails in
> the same conditions, maybe it would make sense to just ignore gpg return
> code as long as there is a SIG_CREATED message...

I do not think we want to go there.  If GPG reports failure, there
is something funny going on.

^ permalink raw reply

* [PATCH] git-bisect: allow running in a working tree subdirectory
From: marcandre.lureau @ 2017-01-26 18:30 UTC (permalink / raw)
  To: git; +Cc: chriscool, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

It looks like it can do it.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 git-bisect.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/git-bisect.sh b/git-bisect.sh
index ae3cb013e..b0bd604d4 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,5 +1,6 @@
 #!/bin/sh
 
+SUBDIRECTORY_OK=Yes
 USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'
 LONG_USAGE='git bisect help
 	print this long help message.
-- 
2.11.0.295.gd7dffce1c.dirty


^ permalink raw reply related

* Re: PATCH 1/2] abspath: add absolute_pathdup()
From: Brandon Williams @ 2017-01-26 18:32 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git List, Junio C Hamano
In-Reply-To: <d94d742d-1247-ac35-c081-7db1f2178d34@web.de>

On 01/26, René Scharfe wrote:
> Add a function that returns a buffer containing the absolute path of its
> argument and a semantic patch for its intended use.  It avoids an extra
> string copy to a static buffer.
> 
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
>  abspath.c                                | 7 +++++++
>  cache.h                                  | 1 +
>  contrib/coccinelle/xstrdup_or_null.cocci | 6 ++++++
>  3 files changed, 14 insertions(+)
> 
> diff --git a/abspath.c b/abspath.c
> index fce40fddcc..2f0c26e0e2 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -239,6 +239,13 @@ const char *absolute_path(const char *path)
>  	return sb.buf;
>  }
>  
> +char *absolute_pathdup(const char *path)
> +{
> +	struct strbuf sb = STRBUF_INIT;
> +	strbuf_add_absolute_path(&sb, path);
> +	return strbuf_detach(&sb, NULL);
> +}
> +
>  /*
>   * Unlike prefix_path, this should be used if the named file does
>   * not have to interact with index entry; i.e. name of a random file
> diff --git a/cache.h b/cache.h
> index 00a029af36..d7b7a8cd7a 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1069,6 +1069,7 @@ const char *real_path(const char *path);
>  const char *real_path_if_valid(const char *path);
>  char *real_pathdup(const char *path);
>  const char *absolute_path(const char *path);
> +char *absolute_pathdup(const char *path);
>  const char *remove_leading_path(const char *in, const char *prefix);
>  const char *relative_path(const char *in, const char *prefix, struct strbuf *sb);
>  int normalize_path_copy_len(char *dst, const char *src, int *prefix_len);
> diff --git a/contrib/coccinelle/xstrdup_or_null.cocci b/contrib/coccinelle/xstrdup_or_null.cocci
> index 3fceef132b..8e05d1ca4b 100644
> --- a/contrib/coccinelle/xstrdup_or_null.cocci
> +++ b/contrib/coccinelle/xstrdup_or_null.cocci
> @@ -5,3 +5,9 @@ expression V;
>  - if (E)
>  -    V = xstrdup(E);
>  + V = xstrdup_or_null(E);
> +
> +@@
> +expression E;
> +@@
> +- xstrdup(absolute_path(E))
> ++ absolute_pathdup(E)
> -- 
> 2.11.0
> 

These two patches look good to me.

-- 
Brandon Williams

^ permalink raw reply

* Re: [PATCH] git-bisect: allow running in a working tree subdirectory
From: Stefan Beller @ 2017-01-26 18:46 UTC (permalink / raw)
  To: marcandre.lureau, Duy Nguyen; +Cc: git@vger.kernel.org, Christian Couder
In-Reply-To: <20170126183030.28632-1-marcandre.lureau@redhat.com>

+ Duy, main author of the worktree feature.

On Thu, Jan 26, 2017 at 10:30 AM,  <marcandre.lureau@redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> It looks like it can do it.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  git-bisect.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/git-bisect.sh b/git-bisect.sh
> index ae3cb013e..b0bd604d4 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -1,5 +1,6 @@
>  #!/bin/sh
>
> +SUBDIRECTORY_OK=Yes
>  USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'
>  LONG_USAGE='git bisect help
>         print this long help message.
> --
> 2.11.0.295.gd7dffce1c.dirty
>

^ permalink raw reply

* Re: [PATCH 4/5] revision.c: refactor ref selection handler after --exclude
From: Junio C Hamano @ 2017-01-26 18:43 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Jeff King, Git Mailing List, Jacob Keller
In-Reply-To: <CACsJy8ATM_kc5SPY0dqprUefRy3vtpKW-4QEyJFK54jw0QgeJA@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

> On Thu, Jan 26, 2017 at 3:57 AM, Jeff King <peff@peff.net> wrote:
>> I don't think it means either. It means to include remotes in the
>> selected revisions, but excluding the entries mentioned by --exclude.
>>
>> IOW:
>>
>>   --exclude=foo --remotes
>>         include all remotes except refs/remotes/foo
>>
>>   --exclude=foo --unrelated --remotes
>>         same
>>
>>   --exclude=foo --decorate-reflog --remotes
>>         decorate reflogs of all remotes except "foo". Do _not_ use them
>>         as traversal tips.
>>
>>   --decorate-reflog --exclude=foo --remotes
>>         same
>>
>> IOW, the ref-selector options build up until a group option is given,
>> which acts on the built-up options (over that group) and then resets the
>> built-up options. Doing "--unrelated" as above is orthogonal (though I
>> think in practice nobody would do that, because it's hard to read).
>
> This is because it makes sense to combine --exclude and
> --decorate-reflog. But what about a new --something that conflicts
> with either --exclude or --decorate-reflog?

I would think that "--exclude=foo --something --remotes" 

 * should be diagnosed as an error if "--something" is not compatible
   with "--exclude";

 * should take effect at the concluding "--remotes" if "--something"
   is similar to "--decorate-reflog" whose effect ends at a
   concluding --remotes/--branches/etc.; and

 * should work independently if "--something" is neither.


^ permalink raw reply

* Re: [PATCH 2/2] fsck: lazily load types under --connectivity-only
From: Junio C Hamano @ 2017-01-26 18:51 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20170126041206.5qfv7r7czbwfqvaa@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> The recent fixes to "fsck --connectivity-only" load all of
> the objects with their correct types. This keeps the
> connectivity-only code path close to the regular one, but it
> also introduces some unnecessary inefficiency. While getting
> the type of an object is cheap compared to actually opening
> and parsing the object (as the non-connectivity-only case
> would do), it's still not free.
>
> For reachable non-blob objects, we end up having to parse
> them later anyway (to see what they point to), making our
> type lookup here redundant.
>
> For unreachable objects, we might never hit them at all in
> the reachability traversal, making the lookup completely
> wasted. And in some cases, we might have quite a few
> unreachable objects (e.g., when alternates are used for
> shared object storage between repositories, it's normal for
> there to be objects reachable from other repositories but
> not the one running fsck).
>
> The comment in mark_object_for_connectivity() claims two
> benefits to getting the type up front:
>
>   1. We need to know the types during fsck_walk(). (And not
>      explicitly mentioned, but we also need them when
>      printing the types of broken or dangling commits).
>
>      We can address this by lazy-loading the types as
>      necessary. Most objects never need this lazy-load at
>      all, because they fall into one of these categories:
>
>        a. Reachable from our tips, and are coerced into the
> 	  correct type as we traverse (e.g., a parent link
> 	  will call lookup_commit(), which converts OBJ_NONE
> 	  to OBJ_COMMIT).
>
>        b. Unreachable, but not at the tip of a chunk of
>           unreachable history. We only mention the tips as
> 	  "dangling", so an unreachable commit which links
> 	  to hundreds of other objects needs only report the
> 	  type of the tip commit.
>
>   2. It serves as a cross-check that the coercion in (1a) is
>      correct (i.e., we'll complain about a parent link that
>      points to a blob). But we get most of this for free
>      already, because right after coercing, we'll parse any
>      non-blob objects. So we'd notice then if we expected a
>      commit and got a blob.
>
>      The one exception is when we expect a blob, in which
>      case we never actually read the object contents.
>
>      So this is a slight weakening, but given that the whole
>      point of --connectivity-only is to sacrifice some data
>      integrity checks for speed, this seems like an
>      acceptable tradeoff.

The only weakening is that a non-blob (or a corrupt blob) object
that sits where we expect a blob (because the containing tree or the
tag says so) would not be diagnosed as an error, then?  I think that
is in line with the spirit of --connectivity-only and is a good
trade-off.

^ permalink raw reply

* Re: [PATCH] mingw: allow hooks to be .exe files
From: Junio C Hamano @ 2017-01-26 18:44 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Jeff King, git
In-Reply-To: <alpine.DEB.2.20.1701261321010.3469@virtualbox>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Peff,
>
> On Wed, 25 Jan 2017, Jeff King wrote:
>
>> On Wed, Jan 25, 2017 at 05:58:42PM +0100, Johannes Schindelin wrote:
>> 
>> > -	if (access(path.buf, X_OK) < 0)
>> > +	if (access(path.buf, X_OK) < 0) {
>> > +#ifdef STRIP_EXTENSION
>> > +		strbuf_addstr(&path, ".exe");
>> 
>> I think STRIP_EXTENSION is a string.  Should this line be:
>> 
>>   strbuf_addstr(&path, STRIP_EXTENSION);
>
> Yep.
>
> v2 coming,
> Johannes

I think I've already tweaked it out when I queued the original one.

^ permalink raw reply

* Re: [PATCH] rebase: pass --signoff option to git am
From: Giuseppe Bilotta @ 2017-01-26 18:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List
In-Reply-To: <CAOxFTcyuLkvgPOxQuzaDUVuDRu_KJg=JrYtU84pQyjLstChbLg@mail.gmail.com>

On Mon, Jan 23, 2017 at 9:03 PM, Giuseppe Bilotta
<giuseppe.bilotta@gmail.com> wrote:
> On Mon, Jan 23, 2017 at 7:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Should we plan to extend this to the interactive backend that is
>> shared between rebase -i and rebase -m, too?  Or is this patch
>> already sufficient to cover them?
>
> AFAIK this is sufficient for both, in the sense that I've used it with
> git rebase -i and it works.

Hm, something very strange is going on, I've just tested the patch on
top of current next and for some reason the signoff line does not get
added. The command-line option gets passed to git am, but I get no
signoff for some reason, so something is failing down the line, I'll
have to investigate.

-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* Re: [PATCH 2/2] ref-filter: add function to parse atoms from a nul-terminated string
From: Junio C Hamano @ 2017-01-26 18:54 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: git
In-Reply-To: <CAM0VKjntATMwDTdu1fSmjeLbwVe73iTo2NQizNXjZchBzqG44w@mail.gmail.com>

SZEDER Gábor <szeder.dev@gmail.com> writes:

> On Wed, Dec 7, 2016 at 5:09 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote:
>> ref-filter's parse_ref_filter_atom() function parses an atom between
>> the start and end pointers it gets as arguments.  This is fine for two
>> of its callers, which process '%(atom)' format specifiers and the end
>> pointer comes directly from strchr() looking for the closing ')'.
>> However, it's not quite so straightforward for its other two callers,
>> which process sort specifiers given as plain nul-terminated strings.
>> Especially not for ref_default_sorting(), which has the default
>> hard-coded as a string literal, but can't use it directly, because a
>> pointer to the end of that string literal is needed as well.
>> The next patch will add yet another caller using a string literal.
>>
>> Add a helper function around parse_ref_filter_atom() to parse an atom
>> up to the terminating nul, and call this helper in those two callers.
>>
>> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
>> ---
>>  ref-filter.c | 8 ++------
>>  ref-filter.h | 4 ++++
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> Ping?
>
> It looks like that this little two piece cleanup series fell on the floor.

I am still waiting for somebody else to comment on the changes, and
the final reroll after such a review discussion to address your own
comment in <CAM0VKjk1mnNzQX6LThq1t7keesBz_fjE9x2e0ywsBKSNKP9SCw@mail.gmail.com>




^ permalink raw reply

* Re: [PATCH 2/2] fsck: lazily load types under --connectivity-only
From: Jeff King @ 2017-01-26 19:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqziid4puz.fsf@gitster.mtv.corp.google.com>

On Thu, Jan 26, 2017 at 10:51:00AM -0800, Junio C Hamano wrote:

> >   2. It serves as a cross-check that the coercion in (1a) is
> >      correct (i.e., we'll complain about a parent link that
> >      points to a blob). But we get most of this for free
> >      already, because right after coercing, we'll parse any
> >      non-blob objects. So we'd notice then if we expected a
> >      commit and got a blob.
> >
> >      The one exception is when we expect a blob, in which
> >      case we never actually read the object contents.
> >
> >      So this is a slight weakening, but given that the whole
> >      point of --connectivity-only is to sacrifice some data
> >      integrity checks for speed, this seems like an
> >      acceptable tradeoff.
> 
> The only weakening is that a non-blob (or a corrupt blob) object
> that sits where we expect a blob (because the containing tree or the
> tag says so) would not be diagnosed as an error, then?  I think that
> is in line with the spirit of --connectivity-only and is a good
> trade-off.

Correct. The corrupt-blob case we always knew was a tradeoff (that's the
whole point of --connectivity-only). We could add back in the "we expect
a blob, is it really one?" at the moment we traverse to it, but IMHO
it's not interesting enough to even be worth the sha1_object_info()
lookup time.

-Peff

^ permalink raw reply

* Re: [PATCH] Documentation: implement linkgit macro for Asciidoctor
From: Eric Wong @ 2017-01-26 19:18 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, brian m. carlson, git, Johannes Schindelin,
	Øyvind A. Holm
In-Reply-To: <xmqq1svp7lcs.fsf@gitster.mtv.corp.google.com>

> Eric Wong <e@80x24.org> writes:
> > You can use '\' to continue long lines with any Ruby version:
> >
> >     "<citerefentry>" \
> >       "<refentrytitle>#{target}</refentrytitle>" \
> >       "<manvolnum>#{attrs[1]}</manvolnum>" \
> >     "</citerefentry>"

Junio C Hamano <gitster@pobox.com> wrote:
> +          "<citerefentry>\n"
> +            "<refentrytitle>#{target}</refentrytitle>"
> +            "<manvolnum>#{attrs[1]}</manvolnum>\n"
> +          "</citerefentry>\n"
>          end

You need the '\' at the end of those strings, it's not like C
since Ruby doesn't require semi-colons to terminate lines.
In other words, that should be:

          "<citerefentry>\n" \
            "<refentrytitle>#{target}</refentrytitle>" \
            "<manvolnum>#{attrs[1]}</manvolnum>\n" \
          "</citerefentry>\n"

^ permalink raw reply

* Re: Force Confirmation for Dropping Changed Lines
From: Hilco Wijbenga @ 2017-01-26 19:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jacob Keller, Git Users
In-Reply-To: <xmqqd1fa7dqf.fsf@gitster.mtv.corp.google.com>

On 25 January 2017 at 18:32, Junio C Hamano <gitster@pobox.com> wrote:
> I think you should be able to do something like
>
>         $ cat >$HOME/bin/fail-3way <<\EOF
>         #!/bin/sh
>         git merge-file "$@"
>         exit 1
>         EOF
>         $ chmod +x $HOME/bin/fail-3way
>         $ cat >>$HOME/.gitconfig <<\EOF
>         [merge "fail"]
>                 name = always fail 3-way merge
>                 driver = $HOME/bin/fail-3way %A %O %B
>                 recursive = text
>         EOF
>         $ echo pom.xml merge=fail >>.gitattributes
>
> to define a custom merge driver whose name is "fail", that runs the
> fail-3way program, which runs the bog standard 3-way merge we use
> (so that it will do the best-effort textual merge) but always return
> with a non-zero status to signal that the result is conflicting and
> needs manual resolution.

Thank you, that's an improvement. :-) Unfortunately, it still
completes the merge. Is there any way to simply get the
>>>>>>>>/<<<<<<<< markers?

^ 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