Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2] git-p4: Fix git-p4.mapUser on Windows
From: Junio C Hamano @ 2017-01-27 17:33 UTC (permalink / raw)
  To: Luke Diamand, Lars Schneider; +Cc: git, George Vanburgh
In-Reply-To: <01020159d4ea6861-fce45d61-8be5-49b0-ab4e-d161b9429795-000000@eu-west-1.amazonses.com>

George Vanburgh <george@vanburgh.me> writes:

> From: George Vanburgh <gvanburgh@bloomberg.net>
>
> When running git-p4 on Windows, with multiple git-p4.mapUser entries in
> git config - no user mappings are applied to the generated repository.
> ...
> Using splitlines solves this issue, by splitting config on all
> typical delimiters ('\n', '\r\n' etc.)

Luke, Lars, this version seems to be in line with the conclusion of
your earlier reviews, e.g.

<CAE5ih7_+Vc9oqKdjo8h2vgZPup4pto9wd=sBb=W6hCs4tuW2Jg@mail.gmail.com>

Even though it looks OK to my eyes, I'll wait for Acks or further
refinement suggestions from either of you two before acting on this
patch.

Thanks.

> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>
> ---
>  git-p4.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index f427bf6..b66f68b 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -656,7 +656,7 @@ def gitConfigInt(key):
>  def gitConfigList(key):
>      if not _gitConfig.has_key(key):
>          s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
> -        _gitConfig[key] = s.strip().split(os.linesep)
> +        _gitConfig[key] = s.strip().splitlines()
>          if _gitConfig[key] == ['']:
>              _gitConfig[key] = []
>      return _gitConfig[key]
>
> --
> https://github.com/git/git/pull/319

^ permalink raw reply

* Re: [PATCH v2 1/1] reset: support the --stdin option
From: Johannes Schindelin @ 2017-01-27 17:34 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, Jakub Narębski
In-Reply-To: <20170127170422.lvpghp6jdud37zxx@sigill.intra.peff.net>

Hi Peff,

On Fri, 27 Jan 2017, Jeff King wrote:

> On Fri, Jan 27, 2017 at 01:38:55PM +0100, Johannes Schindelin wrote:
> 
> A few minor suggestions:
> 
> > +--stdin::
> > +	Instead of taking list of paths from the command line,
> > +	read list of paths from the standard input.  Paths are
> > +	separated by LF (i.e. one path per line) by default.
> > +
> > +-z::
> > +	Only meaningful with `--stdin`; paths are separated with
> > +	NUL character instead of LF.
> 
> Is it worth clarifying that these are paths, not pathspecs? The word
> "paths" is used to refer to the pathspecs on the command-line elsewhere
> in the document.
> 
> It might also be worth mentioning the quoting rules for the non-z case.

I think this would be overkill. In reality, --stdin without -z does not
make much sense, anyway.

If you feel strongly about it, I encourage you to submit a follow-up
patch.

The rest of your suggestions have been implemented in v3.

Ciao,
Johannes

^ permalink raw reply

* [PATCH v3 1/1] reset: support the --stdin option
From: Johannes Schindelin @ 2017-01-27 17:30 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jakub Narębski, Jeff King
In-Reply-To: <cover.1485538197.git.johannes.schindelin@gmx.de>

Just like with other Git commands, this option makes it read the paths
from the standard input. It comes in handy when resetting many, many
paths at once and wildcards are not an option (e.g. when the paths are
generated by a tool).

Note: we first parse the entire list and perform the actual reset action
only in a second phase. Not only does this make things simpler, it also
helps performance, as do_diff_cache() traverses the index and the
(sorted) pathspecs in simultaneously to avoid unnecessary lookups.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-reset.txt | 10 ++++++++++
 builtin/reset.c             | 47 ++++++++++++++++++++++++++++++++++++++++++++-
 t/t7107-reset-stdin.sh      | 33 +++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100755 t/t7107-reset-stdin.sh

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 25432d9257..d319ed9b20 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git reset' [-q] [<tree-ish>] [--] <paths>...
 'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]
+'git reset' [-q] [--stdin [-z]] [<tree-ish>]
 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
 
 DESCRIPTION
@@ -97,6 +98,15 @@ OPTIONS
 --quiet::
 	Be quiet, only report errors.
 
+--stdin::
+	Instead of taking list of paths from the command line,
+	read list of paths from the standard input.  The paths are
+	read verbatim, i.e. not handled as pathspecs.  Paths are
+	separated by LF (i.e. one path per line) by default.
+
+-z::
+	Only meaningful with `--stdin`; paths are separated with
+	NUL character instead of LF.
 
 EXAMPLES
 --------
diff --git a/builtin/reset.c b/builtin/reset.c
index 8ab915bfcb..fe7723c179 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -21,10 +21,14 @@
 #include "parse-options.h"
 #include "unpack-trees.h"
 #include "cache-tree.h"
+#include "strbuf.h"
+#include "quote.h"
+#include "argv-array.h"
 
 static const char * const git_reset_usage[] = {
 	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
 	N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
+	N_("git reset [-q] [--stdin [-z]] [<tree-ish>]"),
 	N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
 	NULL
 };
@@ -267,7 +271,8 @@ static int reset_refs(const char *rev, const struct object_id *oid)
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int reset_type = NONE, update_ref_status = 0, quiet = 0;
-	int patch_mode = 0, unborn;
+	int patch_mode = 0, nul_term_line = 0, read_from_stdin = 0, unborn;
+	struct argv_array stdin_paths = ARGV_ARRAY_INIT;
 	const char *rev;
 	struct object_id oid;
 	struct pathspec pathspec;
@@ -286,6 +291,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
 		OPT_BOOL('N', "intent-to-add", &intent_to_add,
 				N_("record only the fact that removed paths will be added later")),
+		OPT_BOOL('z', NULL, &nul_term_line,
+			N_("paths are separated with NUL character")),
+		OPT_BOOL(0, "stdin", &read_from_stdin,
+				N_("read paths from <stdin>")),
 		OPT_END()
 	};
 
@@ -295,6 +304,40 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 						PARSE_OPT_KEEP_DASHDASH);
 	parse_args(&pathspec, argv, prefix, patch_mode, &rev);
 
+	if (read_from_stdin) {
+		strbuf_getline_fn getline_fn = nul_term_line ?
+			strbuf_getline_nul : strbuf_getline_lf;
+		int flags = PATHSPEC_PREFER_FULL |
+			PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP;
+		struct strbuf buf = STRBUF_INIT;
+		struct strbuf unquoted = STRBUF_INIT;
+
+		if (patch_mode)
+			die(_("--stdin is incompatible with --patch"));
+
+		if (pathspec.nr)
+			die(_("--stdin is incompatible with path arguments"));
+
+		while (getline_fn(&buf, stdin) != EOF) {
+			if (!nul_term_line && buf.buf[0] == '"') {
+				strbuf_reset(&unquoted);
+				if (unquote_c_style(&unquoted, buf.buf, NULL))
+					die(_("line is badly quoted"));
+				strbuf_swap(&buf, &unquoted);
+			}
+			argv_array_push(&stdin_paths, buf.buf);
+			strbuf_reset(&buf);
+		}
+		strbuf_release(&unquoted);
+		strbuf_release(&buf);
+
+		flags |= PATHSPEC_LITERAL_PATH;
+		parse_pathspec(&pathspec, 0, flags, prefix,
+			       stdin_paths.argv);
+
+	} else if (nul_term_line)
+		die(_("-z requires --stdin"));
+
 	unborn = !strcmp(rev, "HEAD") && get_sha1("HEAD", oid.hash);
 	if (unborn) {
 		/* reset on unborn branch: treat as reset to empty tree */
@@ -385,5 +428,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (!pathspec.nr)
 		remove_branch_state();
 
+	argv_array_clear(&stdin_paths);
+
 	return update_ref_status;
 }
diff --git a/t/t7107-reset-stdin.sh b/t/t7107-reset-stdin.sh
new file mode 100755
index 0000000000..997dc42dd2
--- /dev/null
+++ b/t/t7107-reset-stdin.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+
+test_description='reset --stdin'
+
+. ./test-lib.sh
+
+test_expect_success 'reset --stdin' '
+	test_commit hello &&
+	git rm hello.t &&
+	test -z "$(git ls-files hello.t)" &&
+	echo hello.t | git reset --stdin &&
+	test hello.t = "$(git ls-files hello.t)"
+'
+
+test_expect_success 'reset --stdin -z' '
+	test_commit world &&
+	git rm hello.t world.t &&
+	test -z "$(git ls-files hello.t world.t)" &&
+	printf world.tQworld.tQhello.tQ | q_to_nul | git reset --stdin -z &&
+	printf "hello.t\nworld.t\n" >expect &&
+	git ls-files >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '--stdin requires --mixed' '
+	echo hello.t >list &&
+	test_must_fail git reset --soft --stdin <list &&
+	test_must_fail git reset --hard --stdin <list &&
+	git reset --mixed --stdin <list
+'
+
+test_done
+
-- 
2.11.1.windows.prerelease.2.9.g3014b57

^ permalink raw reply related

* [PATCH v3 0/1] Support `git reset --stdin`
From: Johannes Schindelin @ 2017-01-27 17:30 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jakub Narębski, Jeff King
In-Reply-To: <cover.1485520718.git.johannes.schindelin@gmx.de>

This feature was missing, and made it cumbersome for third-party
tools to reset a lot of paths in one go.

Support for --stdin has been added, following builtin/checkout-index.c's
example.

Changes since v2:

- the documentation clarifies that --stdin does not treat the input as
  pathspecs

- the code now uses struct argv_array instead of rolling its own


Johannes Schindelin (1):
  reset: support the --stdin option

 Documentation/git-reset.txt | 10 ++++++++++
 builtin/reset.c             | 47 ++++++++++++++++++++++++++++++++++++++++++++-
 t/t7107-reset-stdin.sh      | 33 +++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100755 t/t7107-reset-stdin.sh


base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe
Published-As: https://github.com/dscho/git/releases/tag/reset-stdin-v3
Fetch-It-Via: git fetch https://github.com/dscho/git reset-stdin-v3

Interdiff vs v2:

 diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
 index abb71bb805..d319ed9b20 100644
 --- a/Documentation/git-reset.txt
 +++ b/Documentation/git-reset.txt
 @@ -100,7 +100,8 @@ OPTIONS
  
  --stdin::
  	Instead of taking list of paths from the command line,
 -	read list of paths from the standard input.  Paths are
 +	read list of paths from the standard input.  The paths are
 +	read verbatim, i.e. not handled as pathspecs.  Paths are
  	separated by LF (i.e. one path per line) by default.
  
  -z::
 diff --git a/builtin/reset.c b/builtin/reset.c
 index 1d3075b7ee..fe7723c179 100644
 --- a/builtin/reset.c
 +++ b/builtin/reset.c
 @@ -23,6 +23,7 @@
  #include "cache-tree.h"
  #include "strbuf.h"
  #include "quote.h"
 +#include "argv-array.h"
  
  static const char * const git_reset_usage[] = {
  	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
 @@ -271,8 +272,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
  {
  	int reset_type = NONE, update_ref_status = 0, quiet = 0;
  	int patch_mode = 0, nul_term_line = 0, read_from_stdin = 0, unborn;
 -	char **stdin_paths = NULL;
 -	int stdin_nr = 0, stdin_alloc = 0;
 +	struct argv_array stdin_paths = ARGV_ARRAY_INIT;
  	const char *rev;
  	struct object_id oid;
  	struct pathspec pathspec;
 @@ -325,18 +325,15 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
  					die(_("line is badly quoted"));
  				strbuf_swap(&buf, &unquoted);
  			}
 -			ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
 -			stdin_paths[stdin_nr++] = xstrdup(buf.buf);
 +			argv_array_push(&stdin_paths, buf.buf);
  			strbuf_reset(&buf);
  		}
  		strbuf_release(&unquoted);
  		strbuf_release(&buf);
  
 -		ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
 -		stdin_paths[stdin_nr++] = NULL;
  		flags |= PATHSPEC_LITERAL_PATH;
  		parse_pathspec(&pathspec, 0, flags, prefix,
 -			       (const char **)stdin_paths);
 +			       stdin_paths.argv);
  
  	} else if (nul_term_line)
  		die(_("-z requires --stdin"));
 @@ -431,11 +428,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
  	if (!pathspec.nr)
  		remove_branch_state();
  
 -	if (stdin_paths) {
 -		while (stdin_nr)
 -			free(stdin_paths[--stdin_nr]);
 -		free(stdin_paths);
 -	}
 +	argv_array_clear(&stdin_paths);
  
  	return update_ref_status;
  }

-- 
2.11.1.windows.prerelease.2.9.g3014b57


^ permalink raw reply

* Re: SubmittingPatches: drop temporal reference for PGP signing
From: Junio C Hamano @ 2017-01-27 17:43 UTC (permalink / raw)
  To: Cornelius Weig
  Cc: Philip Oakley, Stefan Beller, Johannes Sixt,
	bitte.keine.werbung.einwerfen, git
In-Reply-To: <60e9abdf-cc37-33d8-e7eb-8a3370ffe1cc@tngtech.com>

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

> -Do not PGP sign your patch, -at least for now-. Most likely, your (...)
> +Do not PGP sign your patch. Most likely, your maintainer or other (...)

It has been quite a while since we wrote that "at least for now", so
it probably makes sense to drop it.

>> Maybe even s/by signing off/by adding your Signed-off-by:/ to be sure
>> that the reader knows that it is _their certification_ that is being
>> sought. Even if it does double up on the 'your'.
>
> I don't think doubling on the 'your' will make the heading clearer. The
> main intention of this change is to avoid mixups with pgp-signing and
> that would IMHO not improve by that.
> Besides, I find the colon in the heading a bit awkward. Is the following
> version as expressive as with the colon?
>
> -(5) Sign your work
> +(5) Certify your work by adding Signed-off-by

I am leaning towards agreeing with Philip, and my gut feeling says
the presense of the colon, would help a lot.  It would visually
click to readers what we are talking about.

At the same time, I think ending a section header with a colon would
be seen as "awkward".  I would have written it more like this to
avoid it:

    ... by adding "Signed-off-by: " line

I personally do not mind having "your" there, either, but I can see
that a section header wants to be kept shorter.

^ permalink raw reply

* Re: [PATCH v3 4/4] urlmatch: allow globbing for the URL host part
From: Junio C Hamano @ 2017-01-27 17:45 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Patrick Steinhardt, Philip Oakley
In-Reply-To: <20170127062142.GA760@pks-pc>

Patrick Steinhardt <patrick.steinhardt@elego.de> writes:

>> This is probably a useful improvement.
>> 
>> Having said that, when I mentioned "glob", I meant to also support
>> something like this:
>> 
>> 	https://www[1-4].ibm.com/
>
> The problem with additional extended syntax like proposed by you
> is that we would indeed need an escaping mechanism here.

True.  I think a true shell globbing is overkill (so is regexp) and
just a simple wildcarding with '*' would be a good first step that
is easy to explain and later extend as needed.

Thanks.

^ permalink raw reply

* Re: show all merge conflicts
From: Jeff King @ 2017-01-27 17:51 UTC (permalink / raw)
  To: Michael Spiegel; +Cc: git
In-Reply-To: <CANwu5-o=3p8QfWo9wQvok=UZESRVtF3Uxb3nEMZVv8AvkKYYGw@mail.gmail.com>

On Fri, Jan 27, 2017 at 11:56:08AM -0500, Michael Spiegel wrote:

> I'm trying to determine whether a merge required a conflict to resolve
> after the merge has occurred. The git book has some advice
> (https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging) to use
> `git show` on the merge commit or use `git log --cc -p -1`. These
> strategies work when the merge conflict was resolved with a change
> that is different from either parent. When the conflict is resolved
> with a change that is the same as one of the parents, then these
> commands are indistinguishable from a merge that did not conflict. Is
> it possible to distinguish between a conflict-free merge and a merge
> conflict that is resolved by with the changes from one the parents?

No. You'd have to replay the merge to know if it would have had
conflicts.

There was a patch series a few years ago that added a new diff-mode to
do exactly that, and show the diff against what was resolved. It had a
few issues (I don't remember exactly what) and never got merged.

Certainly one complication is that you don't know exactly _how_ the
merge was done in the first place (e.g., which merge strategy, which
custom merge drivers were in effect, etc). But in general, replaying
with a standard merge-recursive would get you most of what you want to
know.

I've done this manually sometimes when digging into erroneous merges
(e.g., somebody accidentally runs "git reset -- <paths>" in the middle
of a merge and throws away some changes.

You should be able to do:

  git checkout $merge^1
  git merge $merge^2
  git diff -R $merge

to see what the original resolution did.

-Peff

^ permalink raw reply

* Re: [PATCH] fixup! worktree move: new command
From: Junio C Hamano @ 2017-01-27 17:51 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <alpine.DEB.2.20.1701271150200.3469@virtualbox>

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

> always been discussed on the mailing list, I would like to kindly ask you
> to please add this patch to the nd/worktree-move branch for the time being
> (i.e. until Duy responds),

The tip of 'pu' (or anything beyond the tip of 'jch') is not always
expected to pass test or even build, and unless I know the OP is
leaky or usually slow, a "fixup!" that directly addresses the OP
tends to be left for the OP to pick up [*1*] to avoid "ah, you sent
a reroll but I already had one squashed locally", which is confusing
to both myself and OP (and adds burden to me).

It seems that OP is slower to respond than his usual, so I'll add it
to the tip of the topic so that it won't be lost.


[Footnote]

*1* It still is used on my end to leave a mental note to myself that
    the topic is expected/waiting to be rerolled, but that is not
    something you can read in "git log --first-parent master..pu".

^ permalink raw reply

* Re: [PATCH 11/12] receive-pack: treat namespace .have lines like alternates
From: Lukas Fleischer @ 2017-01-27 17:45 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <xmqqa8aec40a.fsf@gitster.mtv.corp.google.com>

On Wed, 25 Jan 2017 at 20:51:17, Junio C Hamano wrote:
> [...]
> > diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> > index 8f8762e4a..c55e2f993 100644
> > --- a/builtin/receive-pack.c
> > +++ b/builtin/receive-pack.c
> > @@ -251,8 +251,9 @@ static void show_ref(const char *path, const unsigned char *sha1)
> [...]
> >       if (ref_is_hidden(path, path_full))
> [...]
> This is an unrelated tangent, but there may want to be a knob to
> make the code return here without even showing, to make the
> advertisement even smaller and also to stop miniscule information
> leakage?  If the namespaced multiple projects are totally unrelated
> (i.e. "My sysadmin gave me a write access only to this single
> directory, so I am using the namespace feature to host these three
> projects that have nothing to do with each other"), showing objects
> of other namespaces will buy us nothing and the user is better off
> without this code showing these refs as ".have".

I think this is already possible using receive.hideRefs (which causes
the ref_is_hidden() branch above to return if applicable).

Having support for suppressing .have lines corresponding to different
namespaces was actually the reason I implemented 78a766ab6 (hideRefs:
add support for matching full refs, 2015-11-03). We have been using
namespaces for hosting the package Git repositories of the Arch Linux
User Repository [1] with a shared object storage for several months now.
See [2] for *some* technical details on how things are implemented; the
last section explains how the hideRefs mechanism can be used to limit
ref advertisement to the "active" namespace.

Regards,
Lukas

[1] https://aur.archlinux.org/
[2] https://git.archlinux.org/aurweb.git/plain/doc/git-interface.txt

^ permalink raw reply

* Re: [PATCH v2 1/1] reset: support the --stdin option
From: Jeff King @ 2017-01-27 17:54 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Junio C Hamano, Jakub Narębski
In-Reply-To: <alpine.DEB.2.20.1701271832270.3469@virtualbox>

On Fri, Jan 27, 2017 at 06:34:46PM +0100, Johannes Schindelin wrote:

> > Is it worth clarifying that these are paths, not pathspecs? The word
> > "paths" is used to refer to the pathspecs on the command-line elsewhere
> > in the document.
> > 
> > It might also be worth mentioning the quoting rules for the non-z case.
> 
> I think this would be overkill. In reality, --stdin without -z does not
> make much sense, anyway.

I think with most Unix tools people tend to use the non-z forms until
they break, and then switch to the z form. :)

And in that sense, this transparently Just Works because the output will
often come from another git tool, which will quote as appropriate.

> If you feel strongly about it, I encourage you to submit a follow-up
> patch.
> 
> The rest of your suggestions have been implemented in v3.

Thanks. I think the path/pathspec thing was the more important of the
two suggestions.

-Peff

^ permalink raw reply

* Re: [PATCH v3] t/Makefile: add a rule to re-run previously-failed tests
From: Johannes Schindelin @ 2017-01-27 17:21 UTC (permalink / raw)
  To: Jeff King
  Cc: git, Junio C Hamano, Sverre Rabbelier,
	Ævar Arnfjörð Bjarmason, Matthieu Moy
In-Reply-To: <20170127170713.kn35br4xsdco7xth@sigill.intra.peff.net>

Hi Peff,

On Fri, 27 Jan 2017, Jeff King wrote:

> On Fri, Jan 27, 2017 at 03:17:36PM +0100, Johannes Schindelin wrote:
> 
> > This patch automates the process of determinig which tests failed
> > previously and re-running them.
> 
> s/determinig/determining/

Fixed in v4,
Johannes

^ permalink raw reply

* Re: [PATCH 2/2] use absolute_pathdup()
From: Junio C Hamano @ 2017-01-27 18:18 UTC (permalink / raw)
  To: René Scharfe; +Cc: Johannes Schindelin, Git List
In-Reply-To: <0555c0aa-fcb4-19fb-a533-c451e1e477e3@web.de>

René Scharfe <l.s.r@web.de> writes:

> Hi Dscho,
>
> Am 27.01.2017 um 11:21 schrieb Johannes Schindelin:
>> On Thu, 26 Jan 2017, René Scharfe wrote:
>>> Apply the symantic patch for converting callers that duplicate the
>>
>> s/symantic/semantic/
>
> thank you!  I wonder where this came from.  And where my spellchecker
> went without as much as a farewell.  Reinstalled it now..
>
> René

Thanks.  I've already locally amended this.

^ permalink raw reply

* Re: [PATCH v2 3/3] connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config
From: Junio C Hamano @ 2017-01-27 18:17 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Segev Finer, Jeff King
In-Reply-To: <xmqqo9yt4o5i.fsf@gitster.mtv.corp.google.com>

Just to save us extra round-trip.

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

>> +`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.
>
> Similarly this should mention GIT_SSH_COMMAND at least.  It is crazy
> to set something that will cause misdetection to core.sshCommand and
> use this environment variable to fix it (instead of using ssh.variant),
> so I think it is OK not to mention core.sshCommand (but it would not
> hurt to do so).
>
>> 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)
>> +{
>> ...
>> +}
> ...
>
> The string that came from the configuration must be freed, no?  That
> is what I recall in Peff's comment yesterday.

The leak needs plugging in some way.  

Because this thing is merely an escape hatch that somebody who needs
it needs to use it always (as opposed to one-shot basis), we do not
need to support the environment variable and go with only the
configuration, which may make it easier to plug the leak.

>> @@ -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) {
>
> I like the placement of this "if the user explicitly told us" much
> better.
> ...
> And that reasoning will lead to a realization "there is no reason to
> even do the split_cmdline() if the user explicitly told us".  While
> that reasoning is correct and we _should_ further refactor, I didn't
> want to spend braincycles on that myself.

This _should_ above is a lot weaker than _must_.  

IOW, I think it is acceptable to always split GIT_SSH_COMMAND into
tokens before we realize that the user used the escape hatch and the
splitting was a wasted effort.  This is exactly because this thing
is an escape hatch that is expected to be rarely used.  Of course,
if the "wasted effort" can be eliminated without sacrificing the
simplicity of the code, that is fine as well.





^ permalink raw reply

* Re: [PATCH 11/12] receive-pack: treat namespace .have lines like alternates
From: Jeff King @ 2017-01-27 17:58 UTC (permalink / raw)
  To: Lukas Fleischer; +Cc: git, Junio C Hamano
In-Reply-To: <148553912610.7898.1319453517642036857@typhoon>

On Fri, Jan 27, 2017 at 06:45:26PM +0100, Lukas Fleischer wrote:

> > This is an unrelated tangent, but there may want to be a knob to
> > make the code return here without even showing, to make the
> > advertisement even smaller and also to stop miniscule information
> > leakage?  If the namespaced multiple projects are totally unrelated
> > (i.e. "My sysadmin gave me a write access only to this single
> > directory, so I am using the namespace feature to host these three
> > projects that have nothing to do with each other"), showing objects
> > of other namespaces will buy us nothing and the user is better off
> > without this code showing these refs as ".have".
> 
> I think this is already possible using receive.hideRefs (which causes
> the ref_is_hidden() branch above to return if applicable).
> 
> Having support for suppressing .have lines corresponding to different
> namespaces was actually the reason I implemented 78a766ab6 (hideRefs:
> add support for matching full refs, 2015-11-03). We have been using
> namespaces for hosting the package Git repositories of the Arch Linux
> User Repository [1] with a shared object storage for several months now.
> See [2] for *some* technical details on how things are implemented; the
> last section explains how the hideRefs mechanism can be used to limit
> ref advertisement to the "active" namespace.

Thanks for the pointers. I think a "turn off namespace .have lines"
option would be easier for some cases, but what you've implemented is
much more flexible. So if people using namespaces are happy with it, I
don't see any need to add another way to do the same thing.

-Peff

^ permalink raw reply

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

On Fri, Jan 27, 2017 at 2:29 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Junio,
>
> On Thu, 26 Jan 2017, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>> > 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.
>
> After digging, I found your SQUASH commit. I had not known about that.
>
> In any case, I much rather prefer to have the final version of any patch
> or patch series I contribute to be identical between what you commit and
> what I sent to the mailing list. We do disagree from time to time, and I
> would like to have the opportunity of reviewing how you tweak my changes.
>
> Ciao,
> Johannes

I saw the queued up commit and that lead me to this thread here.
The commit message, while technically correct, seems a bit off. This is because
the commit message only talks about .exe extensions, but the change in code
doesn't even have the string "exe" mentioned once.

With this discussion here, it is easy for me to connect the dots, but
it would be nice
to have the full picture in the commit message. This is what I would write:

    mingw: allow hooks to be .exe files

    Executable files in Windows need to have the extension '.exe', otherwise
    they do not work. Extend the hooks to not just look at the hard coded names,
    but also at the extended names via the custom STRIP_EXTENSION, which
    is defined as '.exe' in Windows.

Stefan

^ permalink raw reply

* Re: [PATCH v2 1/1] reset: support the --stdin option
From: Junio C Hamano @ 2017-01-27 18:30 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, git, Jakub Narębski
In-Reply-To: <20170127170422.lvpghp6jdud37zxx@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> A few minor suggestions:
>
>> +--stdin::
>> +	Instead of taking list of paths from the command line,
>> +	read list of paths from the standard input.  Paths are
>> +	separated by LF (i.e. one path per line) by default.
>> +
>> +-z::
>> +	Only meaningful with `--stdin`; paths are separated with
>> +	NUL character instead of LF.
>
> Is it worth clarifying that these are paths, not pathspecs? The word
> "paths" is used to refer to the pathspecs on the command-line elsewhere
> in the document.

If the code forces literal pathspecs, then what the user feeds to
the command is no longer pathspecs from the user's point of view,
and I agree that the distinction is important.  

If the remainder of the documentation is loose in terminology and
the reason why we were able to get away with it was because we
consistently used "paths" when we meant "pathspec", these existing
mention of "paths" have to be tightened, either in this patch or a
preparatory step patch before this one, because the addition of
"this thing reads paths not pathspecs" is what makes them ambiguous.

Thanks.  I re-read the part of the code that reads and unquotes as
necessary in the patch and they looked correct.





^ permalink raw reply

* Re: [PATCH] help: correct behavior for is_executable on Windows
From: Junio C Hamano @ 2017-01-27 18:43 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Heiko Voigt
In-Reply-To: <c1c6ccae4e60608259809914e8ff3d3d5e1ead5a.1485524999.git.johannes.schindelin@gmx.de>

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

> From: Heiko Voigt <hvoigt@hvoigt.net>
>
> The previous implementation said that the filesystem information on
> Windows is not reliable to determine whether a file is executable. To
> gather this information it was peeking into the first two bytes of a
> file to see whether it looks executable.
>
> Apart from the fact that on Windows executables are defined as such by
> their extension (and Git has special code to support "executing" scripts
> when they have a she-bang line) it leads to serious performance
> problems: not only do we have to open many files now, it gets even
> slower when a virus scanner is running.

Heiko, around here (before going into the details of how severe the
problem is and how wonderful the result applying of this change is)
is the best place to summarize the solution.  E.g.

	Because the definition of "executable" on Windows is based
	on the file extension, update the function to declare that a
	file with ".exe" extension without opening and reading the
	early bytes from it.  This avoids serious performance issues.

I paraphrased the rest only so that the description of the solution
(i.e. "instead of opening and peeking, we trust .exe suffix") fits
well in the surrounding text; the important part is to say what the
change does clearly.

I agree with the reasoning and the execution of the patch, except
that 

 - "correct behaviour" in the title makes it appear that this is a
   correctness thing, but this is primarily a performance fix.

 - It is a bit strange that "MZ" is dropped in the same patch
   without any mention.

The latter may be "correctness" fix, in that earlier we treated any
file that happens to begin with MZ as an executable, regardless of
the filename, which may have misidentified a non-executable file as
an executable.  If that is what is going on, it deserves to be
described in the log message.

> diff --git a/help.c b/help.c
> index 53e2a67e00..bc6cd19cf3 100644
> --- a/help.c
> +++ b/help.c
> @@ -105,7 +105,22 @@ static int is_executable(const char *name)
>  		return 0;
>  
>  #if defined(GIT_WINDOWS_NATIVE)
> -{	/* cannot trust the executable bit, peek into the file instead */
> +	/*
> +	 * On Windows there is no executable bit. The file extension
> +	 * indicates whether it can be run as an executable, and Git
> +	 * has special-handling to detect scripts and launch them
> +	 * through the indicated script interpreter. We test for the
> +	 * file extension first because virus scanners may make
> +	 * it quite expensive to open many files.
> +	 */
> +	if (ends_with(name, ".exe"))
> +		return S_IXUSR;
> +
> +{
> +	/*
> +	 * Now that we know it does not have an executable extension,
> +	 * peek into the file instead.
> +	 */
>  	char buf[3] = { 0 };
>  	int n;
>  	int fd = open(name, O_RDONLY);
> @@ -113,8 +128,8 @@ static int is_executable(const char *name)
>  	if (fd >= 0) {
>  		n = read(fd, buf, 2);
>  		if (n == 2)
> -			/* DOS executables start with "MZ" */
> -			if (!strcmp(buf, "#!") || !strcmp(buf, "MZ"))
> +			/* look for a she-bang */
> +			if (!strcmp(buf, "#!"))
>  				st.st_mode |= S_IXUSR;
>  		close(fd);
>  	}
>
> base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe

^ permalink raw reply

* Re: [PATCH] test-lib: on FreeBSD, look for unzip(1) in /usr/local/bin/
From: Junio C Hamano @ 2017-01-27 18:44 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Eric Wong, git
In-Reply-To: <alpine.DEB.2.20.1701271502420.3469@virtualbox>

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

> Hi Junio,
>
> On Thu, 21 Jul 2016, Eric Wong wrote:
>
>> Johannes Schindelin <johannes.schindelin@gmx.de> wrote:
>> > The common work-around is to install Info-Zip on FreeBSD, into
>> > /usr/local/bin/.
>> > 
>> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> 
>> Thanks, t5003 now works out-of-the-box.
>> Tested with Info-ZIP unzip installed and uninstalled.
>> 
>> Tested-by: Eric Wong <e@80x24.org>
>
> Did you forget about this?

This fell off the radar.  

You could have resent with Eric's Tested-by: added, to make it
easier to apply.  I'll see if I can find the original but it won't
happen until later this afternoon.

^ permalink raw reply

* Re: [PATCH v4] t/Makefile: add a rule to re-run previously-failed tests
From: Junio C Hamano @ 2017-01-27 18:53 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: git, Jeff King, Sverre Rabbelier,
	Ævar Arnfjörð Bjarmason, Matthieu Moy
In-Reply-To: <0563f07117e828c072ba542c1a57441e2e8efb81.1485537593.git.johannes.schindelin@gmx.de>

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

> This patch automates the process of determining which tests failed
> previously and re-running them.
> ...
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

I stored both versions in files and compared them, and it seems the
single word change in the proposed commit log message is the only
difference.  I would have written "Automate the process...", though.

If you are resending, touching up to cover all points raised by a
reviewer and doing nothing else, having "Reviewed-by: Jeff King
<peff@peff.net>" would have been nicer.  

Will queue.  Thanks.

> ---
> Published-As: https://github.com/dscho/git/releases/tag/failing-tests-v4
> Fetch-It-Via: git fetch https://github.com/dscho/git failing-tests-v4
>
>  t/Makefile | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/t/Makefile b/t/Makefile
> index d613935f14..1bb06c36f2 100644
> --- a/t/Makefile
> +++ b/t/Makefile
> @@ -35,6 +35,12 @@ all: $(DEFAULT_TEST_TARGET)
>  test: pre-clean $(TEST_LINT)
>  	$(MAKE) aggregate-results-and-cleanup
>  
> +failed:
> +	@failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \
> +		grep -l '^failed [1-9]' *.counts | \
> +		sed -n 's/\.counts$$/.sh/p') && \
> +	test -z "$$failed" || $(MAKE) $$failed
> +
>  prove: pre-clean $(TEST_LINT)
>  	@echo "*** prove ***"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
>  	$(MAKE) clean-except-prove-cache
>
> base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe

^ permalink raw reply

* Re: [PATCH] doc: clarify distinction between sign-off and pgp-signing
From: Cornelius Weig @ 2017-01-27 20:04 UTC (permalink / raw)
  To: git@vger.kernel.org
In-Reply-To: <20170127200136.29949-1-cornelius.weig@tngtech.com>

Sorry, I forgot to mark this patch as follow-up to message
<xmqq60l01jr9.fsf@gitster.mtv.corp.google.com>

^ permalink raw reply

* [PATCH] doc: clarify distinction between sign-off and pgp-signing
From: cornelius.weig @ 2017-01-27 20:01 UTC (permalink / raw)
  To: gitster, philipoakley, sbeller; +Cc: git, Cornelius Weig

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

The documentation for submission discourages pgp-signing, but demands
a proper sign-off by contributors. However, when skimming the headings,
the wording of the section for sign-off could mistakenly be understood
as concerning pgp-signing. Thus, new contributors could oversee the
necessary sign-off.

This commit improves the wording such that the section about sign-off
cannot be misunderstood as pgp-signing. In addition, the paragraph about
pgp-signing is changed such that it avoids the impression that
pgp-signing could be relevant at later stages of the submission.

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Philip Oakley <philipoakley@iee.org>
Signed-off-by: Stefan Beller <sbeller@google.com>
---

Notes:
    This patch summarizes the suggested changes.
    
    As I don't know what is appropriate, I took the liberty to add everybody's
    sign-off who was involved in the discussion in alphabetic order.

 Documentation/SubmittingPatches | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 08352de..3faf7eb 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -216,12 +216,11 @@ 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. 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 +245,7 @@ patch.
      *2* The mailing list: git@vger.kernel.org
 
 
-(5) Sign your work
+(5) Certify your work by adding your "Signed-off-by: " line
 
 To improve tracking of who did what, we've borrowed the
 "sign-off" procedure from the Linux kernel project on patches
-- 
2.10.2


^ permalink raw reply related

* Re: [PATCH] doc: clarify distinction between sign-off and pgp-signing
From: Stefan Beller @ 2017-01-27 20:31 UTC (permalink / raw)
  To: Cornelius Weig; +Cc: Junio C Hamano, Philip Oakley, git@vger.kernel.org
In-Reply-To: <20170127200136.29949-1-cornelius.weig@tngtech.com>

On Fri, Jan 27, 2017 at 12:01 PM,  <cornelius.weig@tngtech.com> wrote:
> From: Cornelius Weig <cornelius.weig@tngtech.com>
>
> The documentation for submission discourages pgp-signing, but demands
> a proper sign-off by contributors. However, when skimming the headings,
> the wording of the section for sign-off could mistakenly be understood
> as concerning pgp-signing. Thus, new contributors could oversee the
> necessary sign-off.
>
> This commit improves the wording such that the section about sign-off
> cannot be misunderstood as pgp-signing. In addition, the paragraph about
> pgp-signing is changed such that it avoids the impression that
> pgp-signing could be relevant at later stages of the submission.
>
> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Philip Oakley <philipoakley@iee.org>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
> Notes:
>     This patch summarizes the suggested changes.
>
>     As I don't know what is appropriate, I took the liberty to add everybody's
>     sign-off who was involved in the discussion in alphabetic order.

Heh, my first though was to conclude you haven't read the
sign off part, yet apart from the changed header.
/me goes back and actually reads the DCO again.
And actually these sign offs were there in other patches in this area,
so you'd claim (a) that yours was just created partly by you and having
other patches that were also signed off (b), whose sign offs you
merely copy over to here.

And then after reading I realized I slightly confused the signing
myself as the sign offs are also used to track the flow of a patch.
These sign offs suggest that you made the patch initially, then
passed it to Junio, then to Philip and then to me.
And Junio will sign it again when applying the patch.

So maybe s/signed-off-by/helped-by/?

The patch with the aggregation looks good to me.

Thanks,
Stefan

>
>  Documentation/SubmittingPatches | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 08352de..3faf7eb 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -216,12 +216,11 @@ 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. 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 +245,7 @@ patch.
>       *2* The mailing list: git@vger.kernel.org
>
>
> -(5) Sign your work
> +(5) Certify your work by adding your "Signed-off-by: " line
>
>  To improve tracking of who did what, we've borrowed the
>  "sign-off" procedure from the Linux kernel project on patches
> --
> 2.10.2
>

^ permalink raw reply

* Re: [PATCH 11/12] receive-pack: treat namespace .have lines like alternates
From: Junio C Hamano @ 2017-01-27 20:42 UTC (permalink / raw)
  To: Jeff King; +Cc: Lukas Fleischer, git
In-Reply-To: <20170127175807.4tjxpenu2gk77dhv@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Fri, Jan 27, 2017 at 06:45:26PM +0100, Lukas Fleischer wrote:
>
>> I think this is already possible using receive.hideRefs (which causes
>> the ref_is_hidden() branch above to return if applicable).
>> ...
>
> Thanks for the pointers. I think a "turn off namespace .have lines"
> option would be easier for some cases, but what you've implemented is
> much more flexible. So if people using namespaces are happy with it, I
> don't see any need to add another way to do the same thing.

Yeah, I agree.  Thanks, both.

^ permalink raw reply

* octopus merge --no-ff claims to fast-forward even though it doesn't
From: Samuel Lijin @ 2017-01-27 20:46 UTC (permalink / raw)
  To: git@vger.kernel.org

I was doing an octopus merge earlier and noticed that it claims to
fast-forward when you specify --no-ff, even though it does actually
abide by --no-ff.

I can consistently reproduce as follows:

$ git clone https://github.com/sxlijin/merge-octopus-experiment
$ cd merge-octopus-experiment
$ git merge --no-ff origin/A origin/B --no-edit
Fast-forwarding to: origin/A
Trying simple merge with origin/B
Merge made by the 'octopus' strategy.
 anotherA | 0
 anotherB | 0
 otherA   | 0
 otherB   | 0
 4 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 anotherA
 create mode 100644 anotherB
 create mode 100644 otherA
 create mode 100644 otherB

$ git log --graph --pretty=oneline --decorate

I've reproduced the issue with 2.11.0 on both a Windows box (MSYS) and
Linux (Arch).

The issue seems to live in git-merge-octopus.sh, specifically in that
--no-ff does not affect the initial value of NON_FF_MERGE. I'm happy
to submit a patch if someone can point me in the right direction.

Sam

^ permalink raw reply

* Re: [PATCH] test-lib: on FreeBSD, look for unzip(1) in /usr/local/bin/
From: Junio C Hamano @ 2017-01-27 20:47 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Eric Wong, git
In-Reply-To: <xmqq8tpwz6jp.fsf@gitster.mtv.corp.google.com>

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

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> On Thu, 21 Jul 2016, Eric Wong wrote:
>>
>>> Thanks, t5003 now works out-of-the-box.
>>> Tested with Info-ZIP unzip installed and uninstalled.
>>> 
>>> Tested-by: Eric Wong <e@80x24.org>
>>
>> Did you forget about this?
>
> This fell off the radar.  
>
> You could have resent with Eric's Tested-by: added, to make it
> easier to apply.  I'll see if I can find the original but it won't
> happen until later this afternoon.

The errand I needed to run earlier in the day turned out to be not
so time consuming---I found the exchange and the patch is now
queued, and will be part of today's pushout.

Thanks, both.

^ 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