Git development
 help / color / mirror / Atom feed
* Re: [PATCH v3 5/5] stash: teach 'push' (and 'create') to honor pathspec
From: Thomas Gummerer @ 2017-02-11 16:50 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski
In-Reply-To: <xmqqmvdz3ied.fsf@gitster.mtv.corp.google.com>

On 02/05, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > @@ -72,6 +72,11 @@ create_stash () {
> >  			untracked="$1"
> >  			new_style=t
> >  			;;
> > +		--)
> > +			shift
> > +			new_style=t
> > +			break
> > +			;;
> >  		*)
> >  			if test -n "$new_style"
> >  			then
> > @@ -99,6 +104,10 @@ create_stash () {
> >  	if test -z "$new_style"
> >  	then
> >  		stash_msg="$*"
> > +		while test $# != 0
> > +		do
> > +			shift
> > +		done
> >  	fi
> 
> The intent is correct, but I would probaly empty the "$@" not with
> an iteration of shifts but with a single
> 
> 	set x && shift
> 
> ;-)

Thanks, I knew there had to be a better way, but I was unable to find
it.  I think I like Andreas suggestion of "shift $#" the best here.

> > @@ -134,7 +143,7 @@ create_stash () {
> >  		# Untracked files are stored by themselves in a parentless commit, for
> >  		# ease of unpacking later.
> >  		u_commit=$(
> > -			untracked_files | (
> > +			untracked_files "$@" | (
> 
> The above (and all other uses of "$@" was exactly what I had in mind
> when I gave you the "can we leave the '$@' the user gave us as-is?"
> comment during the review of the previous round.  
> 
> Looks much nicer.
> 
> > +test_expect_success 'stash push with $IFS character' '
> > +	>"foo	bar" &&
> 
> IIRC, a pathname with HT in it cannot be tested on MINGW.  For the
> purpose of this test, I think it is sufficient to use SP instead of
> HT here (and matching change below in the expectation, of course).

Will change.

> > +	>foo &&
> > +	>bar &&
> > +	git add foo* &&
> > +	git stash push --include-untracked -- foo* &&
> 
> While it is good that you are testing "foo*", you would also want
> another test to cover a pathspec with $IFS in it.  That is the case
> the code in the previous round had problems with.  Perhaps try
> something like
> 
> 	>foo && >bar && >"foo bar" && git add . &&
> 	echo modify foo >foo &&
> 	echo modify bar >bar &&
> 	echo modify "foo bar" >"foo bar" &&
> 	git stash push -- "foo b*"
> 
> which should result in the changes to "foo bar" in the resulting
> stash, while changes to "foo" and "bar" are not (and left in the
> working tree).  And make sure that is what happens?  I think with
> the code from the previous round, the above would instead stash
> changes to "foo" and "bar" without catching changes to "foo bar",
> because the single pathspec "foo b*" would have been mistakenly
> split into two, i.e. "foo" and "b*", and failed to match "foo bar"
> while matching the other two.

Thanks, I'll add a test for that.

^ permalink raw reply

* [RFH] Request for Git Merge 2017 impressions for Git Rev News
From: Jakub Narębski @ 2017-02-11 16:33 UTC (permalink / raw)
  To: git

Hello,

Git Rev News #24 is planned to be released on February 15. It is meant
to cover what happened during the month of January 2017 (and earely
February 2017) and the Git Merge 2017 conference that happened on
February 2nd and 3rd 2017.

A draft of a new Git Rev News edition is available here:

  https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-24.md

I would like to ask everyone who attended the conference (and the
GitMerge 2017 Contributors’s Summit day before it), or watched it live
at http://git-merge.com/watch to write his or her impressions.

You can contribute either by replying to this email, or by editing the
above page on GitHub and sending a pull request, or by commenting on
the following GitHub issue about Git Rev News 24:

  https://github.com/git/git.github.io/issues/221

If you prefer to post on your own blog (or if you have did it
already), please send an URL.


P.S. I wonder if there should be not a separate section on
https://git.github.io/ about recollection from various Git-related
events, with Git Merge 2017 as the first one.  This way we can wait
for later response, and incorporate videos and slides from events, as
they begin to be available.

P.P.S. Please distribute this information more widely.

Thanks in advance,
-- 
Jakub Narębski


^ permalink raw reply

* Re: [PATCH v3 4/5] stash: introduce new format create
From: Thomas Gummerer @ 2017-02-11 14:51 UTC (permalink / raw)
  To: Jeff King
  Cc: git, Stephan Beyer, Junio C Hamano, Marc Strapetz,
	Johannes Schindelin, Øyvind A . Holm, Jakub Narębski
In-Reply-To: <20170206155606.xgkmhg656vuc6uki@sigill.intra.peff.net>

On 02/06, Jeff King wrote:
> On Sun, Feb 05, 2017 at 08:26:41PM +0000, Thomas Gummerer wrote:
> 
> > git stash create currently supports a positional argument for adding a
> > message.  This is not quite in line with how git commands usually take
> > comments (using a -m flag).
> > 
> > Add a new syntax for adding a message to git stash create using a -m
> > flag.  This is with the goal of deprecating the old style git stash
> > create with positional arguments.
> > 
> > This also adds a -u argument, for untracked files.  This is already used
> > internally as another positional argument, but can now be used from the
> > command line.
> 
> How do we tell the difference between new-style invocations, and
> old-style ones that look new-style? IOW, I think:
> 
>   git stash create -m works
> 
> currently treats "-m works" as the full message, and it would now become
> just "works".
> 
> That may be an acceptable loss for the benefit we are getting. The
> alternative is to make yet another verb for create, as we did with
> save/push). I have a feeling that hardly anybody uses "create", though,
> and it's mostly an implementation detail. So given the obscure nature,
> maybe it's an acceptable level of regression. I dunno.

Right.  So I did a quick search on google and github for this, and
there seems one place where git stash create -m is used [1].  From a
quick look it does however not seem like the -m in the stash message
is of any significance there, but rather that the intention was to use
a flag that doesn't exist.

I *think* this regression is acceptable, but I'm happy to introduce
another verb if people think otherwise.

> But either way, it should probably be in the commit message in case
> somebody does have to track it down later.

I'll add something there, thanks!

> -Peff

[1]: https://github.com/Andersbakken/nrdp-scripts/blob/1052fc6781c36c4b227c7dd4838a501341b0862a/bin/git-rstash#L81

^ permalink raw reply

* [PATCH] cocci: detect useless free(3) calls
From: René Scharfe @ 2017-02-11 13:58 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Pranit Bauva

Add a semantic patch for removing checks that cause free(3) to only be
called with a NULL pointer, as that must be a programming mistake.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
No cases are found in master or next, but 1d263b93 (bisect--helper:
`bisect_next_check` & bisect_voc shell function in C) introduced four
of them to pu.

 contrib/coccinelle/free.cocci | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci
index e28213161a..c03ba737e5 100644
--- a/contrib/coccinelle/free.cocci
+++ b/contrib/coccinelle/free.cocci
@@ -3,3 +3,9 @@ expression E;
 @@
 - if (E)
   free(E);
+
+@@
+expression E;
+@@
+- if (!E)
+  free(E);
-- 
2.11.1


^ permalink raw reply related

* Re: [PATCH v3 3/5] stash: add test for the create command line arguments
From: Thomas Gummerer @ 2017-02-11 13:55 UTC (permalink / raw)
  To: Jeff King
  Cc: git, Stephan Beyer, Junio C Hamano, Marc Strapetz,
	Johannes Schindelin, Øyvind A . Holm, Jakub Narębski
In-Reply-To: <20170206155012.rb2vydjvlquchwk2@sigill.intra.peff.net>

On 02/06, Jeff King wrote:
> On Sun, Feb 05, 2017 at 08:26:40PM +0000, Thomas Gummerer wrote:
> 
> > +test_expect_success 'create stores correct message' '
> > +	>foo &&
> > +	git add foo &&
> > +	STASH_ID=$(git stash create "create test message") &&
> > +	echo "On master: create test message" >expect &&
> > +	git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
> > +	test_cmp expect actual
> > +'
> 
> This misses failure exit codes from "git show" because it's on the left
> side of a pipe. Perhaps "git show -s" or "git log -1" would get you the
> same output without needing "head" (and saving a process and the time
> spent generating the diff, as a bonus).
> 
> Ditto in the other tests from this patch and later ones.

Good catch, will fix.

> -Peff

^ permalink raw reply

* Re: [PATCH v3 2/5] stash: introduce push verb
From: Thomas Gummerer @ 2017-02-11 13:33 UTC (permalink / raw)
  To: Jeff King
  Cc: git, Stephan Beyer, Junio C Hamano, Marc Strapetz,
	Johannes Schindelin, Øyvind A . Holm, Jakub Narębski
In-Reply-To: <20170206154628.v27z5mqhxylz22ba@sigill.intra.peff.net>

[sorry for the late responses, life is keeping me busy]

On 02/06, Jeff King wrote:
> On Sun, Feb 05, 2017 at 08:26:39PM +0000, Thomas Gummerer wrote:
> 
> > +		-m|--message)
> > +			shift
> > +			stash_msg=${1?"-m needs an argument"}
> > +			;;
> 
> I think this is our first use of the "?" parameter expansion magic. It
> _is_ in POSIX, so it may be fine. We may get complaints from people on
> weird shell variants, though. If that's the only reason to avoid it, I'd
> be inclined to try it and see, as it is much shorter.
> 
> OTOH, most of the other usage errors call usage(), and this one doesn't.
> Nor is the error message translatable. Perhaps we should stick to the
> longer form (and add a helper function if necessary to reduce the
> boilerplate).

Yeah I do agree that calling usage is the better option here.

> > +save_stash () {
> > +	push_options=
> > +	while test $# != 0
> > +	do
> > +		case "$1" in
> > +		--help)
> > +			show_help
> > +			;;
> > +		--)
> > +			shift
> > +			break
> > +			;;
> > +		-*)
> > +			# pass all options through to push_stash
> > +			push_options="$push_options $1"
> > +			;;
> > +		*)
> > +			break
> > +			;;
> > +		esac
> > +		shift
> > +	done
> 
> I suspect you could just let "--help" get handled in the pass-through
> case (it generally takes precedence over errors found in other options,
> but I do not see any other parsing errors that could be found by this
> loop). It is not too bad to keep it, though (the important thing is that
> we're not duplicating all of the push_stash options here).

Good point, would be good to get rid of that duplication as well.

> > +	if test -z "$stash_msg"
> > +	then
> > +		push_stash $push_options
> > +	else
> > +		push_stash $push_options -m "$stash_msg"
> > +	fi
> 
> Hmm. So $push_options is subject to word-splitting here. That's
> necessary to split the options back apart. It does the wrong thing if
> any of the options had spaces in them. But I don't think there are any
> valid options which do so, and "save" would presumably not grow any new
> options (they would go straight to "push").
> 
> So there is a detectable behavior change:
> 
>   [before]
>   $ git stash "--bogus option"
>   error: unknown option for 'stash save': --bogus option
>          To provide a message, use git stash save -- '--bogus option'
>   [etc...]
> 
>   [after]
>   $ git stash "--bogus option"
>   error: unknown option for 'stash save': --bogus
>          To provide a message, use git stash save -- '--bogus'
> 
> but it's probably an acceptable casualty (the "right" way would be to
> shell-quote everything you stuff into $push_options and then eval the
> result when you invoke push_stash).
>
> Likewise, it's usually a mistake to just stick a new option (like "-m")
> after a list of unknown options. But it's OK here because we know we
> removed any "--" or non-option arguments.
> 
> -Peff

-- 
Thomas

^ permalink raw reply

* Re: [PATCH v3 0/4] git gui: allow for a long recentrepo list
From: Philip Oakley @ 2017-02-11 12:48 UTC (permalink / raw)
  To: GitList
  Cc: Junio C Hamano, Pat Thoyts, Eric Sunshine, Alexey Astakhov,
	Johannes Schindelin
In-Reply-To: <20170122195301.1784-1-philipoakley@iee.org>

Ping

Any comments anyone?
(https://public-inbox.org/git/20170122195301.1784-1-philipoakley@iee.org/)

I understand that the Git-for-Windows team is planning to include this in 
their next release, so additional eyes are welcomed.

Philip

From: "Philip Oakley" <philipoakley@iee.org> date: Sunday, January 22, 2017 
7:52 PM
> Way back in December 2015 I made a couple of attempts to patch up
> the git-gui's recentrepo list in the face of duplicate entries in
> the .gitconfig
>
> The series end at 
> http://public-inbox.org/git/9731888BD4C348F5BFC82FA96D978034@PhilipOakley/
>
> A similar problem was reported recently on the Git for Windows list
> https://github.com/git-for-windows/git/issues/1014 were a full
> recentrepo list also stopped the gui working.
>
> This series applies to Pat Thoyt's upstream Github repo as a merge
> ready Pull Request https://github.com/patthoyts/git-gui/pull/10.
> Hence if applied here it should be into the sub-tree git/git-gui.
>
> I believe I've covered the points raised by Junio in the last review
> and allowed for cases where the recentrepo list is now longer than the
> maximum (which can be configured, both up and down).
>
> I've cc'd just the cover letter to those involved previously, rather
> than the series to avoid spamming.
>
> There are various other low hanging fruit that the eager could look at
> which are detailed at the end of the GfW/issues/104 referenced above.
>
> Philip Oakley (4):
>  git-gui: remove duplicate entries from .gitconfig's gui.recentrepo
>  git gui: cope with duplicates in _get_recentrepo
>  git gui: de-dup selected repo from recentrepo history
>  git gui: allow for a long recentrepo list
>
> lib/choose_repository.tcl | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> -- 
> 2.9.0.windows.1.323.g0305acf
> ---
> cc: Junio C Hamano <gitster@pobox.com>
> cc: Pat Thoyts <patthoyts@users.sourceforge.net>,
> cc: Eric Sunshine <sunshine@sunshineco.com>,
> cc: Alexey Astakhov <asstv7@gmail.com>
> cc: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> 


^ permalink raw reply

* Re: Non-zero exit code without error
From: Christian Couder @ 2017-02-11 12:28 UTC (permalink / raw)
  To: Serdar Sahin; +Cc: git
In-Reply-To: <CAL7ZE5yXaJQFci+9aF4+cxeycnf71FMyLTV14t_TGDR3cnnfVA@mail.gmail.com>

On Wed, Feb 8, 2017 at 11:26 AM, Serdar Sahin <serdar@peakgames.net> wrote:
> Hi Christian,
>
>
> We are using a private repo (Github Enterprise).

Maybe you could try 'git fast-export --anonymize ...' on it.

> Let me give you the
> details you requested.
>
>
> On Git Server: git version 2.6.5.1574.g5e6a493
>
> On my client: git version 2.10.1 (Apple Git-78)
>
>
> I’ve tried to reproduce it with public repos, but couldn’t do so.

You might try using the latest released version (2.11.1).

For example you could install the last version on the client and then
clone from the server with --bare and use this bare repo as if it was
the server.

You could also try `git fsck` to see if there are problems on your repo.

Are there submodules or something a bit special?

In the end it's difficult for us to help if we cannot reproduce, so
your best bet might be to debug yourself using gdb for example.

> If I
> could get an error/log output, that would be sufficient.
>
>
> I am also including the full output below. (also git gc)
>
>
> MacOSX:test serdar$ git clone --mirror --depth 50 --no-single-branch
> git@git.privateserver.com:Casual/code_repository.git

You could also try with different options above...

> Cloning into bare repository 'code_repository.git'...
>
> remote: Counting objects: 3362, done.
>
> remote: Compressing objects: 100% (1214/1214), done.
>
> remote: Total 3362 (delta 2335), reused 2968 (delta 2094), pack-reused 0
>
> Receiving objects: 100% (3362/3362), 56.77 MiB | 1.83 MiB/s, done.
>
> Resolving deltas: 100% (2335/2335), done.
>
> MacOSX:test serdar$ cd code_repository.git/
>
> MacOSX:code_repository.git serdar$ GIT_CURL_VERBOSE=1 GIT_TRACE=1  git
> fetch --depth 50 origin cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee

... and above.

Also it looks like you use ssh so something like GIT_SSH_COMMAND="ssh
-vv" might help more than GIT_CURL_VERBOSE=1

> 13:23:15.648337 git.c:350               trace: built-in: git 'fetch'
> '--depth' '50' 'origin' 'cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee'
>
> 13:23:15.651127 run-command.c:336       trace: run_command: 'ssh'
> 'git@git.privateserver.com' 'git-upload-pack
> '\''Casual/code_repository.git'\'''
>
> 13:23:17.750015 run-command.c:336       trace: run_command: 'gc' '--auto'
>
> 13:23:17.750829 exec_cmd.c:189          trace: exec: 'git' 'gc' '--auto'
>
> 13:23:17.753983 git.c:350               trace: built-in: git 'gc' '--auto'
>
> MacOSX:code_repository.git serdar$ echo $?
>
> 1
>
> MacOSX:code_repository.git serdar$ GIT_CURL_VERBOSE=1 GIT_TRACE=1 git gc --auto
>
> 13:23:45.899038 git.c:350               trace: built-in: git 'gc' '--auto'
>
> MacOSX:code_repository.git serdar$ echo $?
>
> 0

^ permalink raw reply

* Re: [PATCH] mingw: use OpenSSL's SHA-1 routines
From: Johannes Sixt @ 2017-02-11  8:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git, Jeff Hostetler
In-Reply-To: <xmqqbmublyo0.fsf@gitster.mtv.corp.google.com>

Am 10.02.2017 um 00:41 schrieb Junio C Hamano:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
>> From: Jeff Hostetler <jeffhost@microsoft.com>
>>
>> Use OpenSSL's SHA-1 routines rather than builtin block-sha1 routines.
>> This improves performance on SHA1 operations on Intel processors.
>> ...
>>
>> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> ---
>
> Nice.  Will queue as jh/mingw-openssl-sha1 topic; it is a bit too
> late for today's integration cycle to be merged to 'next', but let's
> have this by the end of the week in 'master'.

Please don't rush this through. I didn't have a chance to cross-check 
the patch; it will have to wait for Monday. I would like to address 
Peff's concerns about additional runtime dependencies.

-- Hannes


^ permalink raw reply

* Re: [PATCH 1/2 v3] revision.c: args starting with "-" might be a revision
From: Siddharth Kannan @ 2017-02-11  7:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Matthieu.Moy, pranit.bauva, peff, pclouds, sandals
In-Reply-To: <xmqqh941ippo.fsf@gitster.mtv.corp.google.com>

Hey Junio,
On Fri, Feb 10, 2017 at 03:35:47PM -0800, Junio C Hamano wrote:
> So the difference is just "--left" (by the way, our codebase seem to
> prefer "left--" when there is no difference between pre- or post-
> decrement/increment) that adjusts the slot in argv[] where the next
> unknown argument is stuffed to.

Understood, I will use post decrement.

> I am wondering if writing it like the following is easier to
> understand.  I had a hard time figuring out what you are trying to
> do, partly because "args" is quite a misnomer---implying "how many
> arguments did we see" that is similar to opts that does mean "how
> many options did handle_revision_opts() see?"  

Um, okay, I see that "args" is very confusing. Would it help if this variable
was called "arg_not_rev"? Because the value that is returned from
handle_revision_arg is 1 when it is not a revision, and 0 when it is a
revision. The changed block of code would look like this:

---
 revision.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/revision.c b/revision.c
index b37dbec..4131ad5 100644
--- a/revision.c
+++ b/revision.c
@@ -2205,6 +2205,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	read_from_stdin = 0;
 	for (left = i = 1; i < argc; i++) {
 		const char *arg = argv[i];
+		int handle_rev_arg_called = 0, arg_not_rev;
 		if (*arg == '-') {
 			int opts;
@@ -2234,11 +2235,18 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
                    }
                    if (opts < 0)
                            exit(128);
-                   continue;
+
+                   arg_not_rev = handle_revision_arg(arg, revs, flags, revarg_opt);
+                   handle_rev_arg_called = 1;
+                   if (arg_not_rev)
+                           continue; /* arg is neither an option nor a revision */
+                   else
+                           left--; /* arg is a revision! */
            }
 
 
-           if (handle_revision_arg(arg, revs, flags, revarg_opt)) {
+           if ((handle_rev_arg_called && arg_not_rev) ||
+                           handle_revision_arg(arg, revs, flags, revarg_opt)) {

> The rewrite below may avoid such a confusion.  I dunno.

Um, I am sorry, but I feel that decrementing left, and incrementing it again is
also confusing. I think that with a better name for the return value from
handle_revision_arg, the earlier confusion should be resolved.

I base this on my previous experience following the codepath. It was easy for
me to understand with the previous code that "continue" will be executed from
within the first if block whenever arg begins with a "-" and it is determined
that it is not an option. 

going by that, now, "continue" will be executed whenever it's not an option and
_also_ not an argument. Otherwise, the further parts of the code will execute
as before, and there are no continue statements there. I hope this argument
makes sense.

Also worth noting, The two `if` lines look better now:

1. If arg is not a revision, go to the next arg (because we have already
determined that it is not an option)

2. If handle_rev_arg was called AND the argument was not a revision, OR
if handle_revision_arg says that arg is not a rev, execute the following block.

Perhaps, someone else could please have a look at the changes in the block
above and the block below and give some feedback on which one is easier to
understand and the reason that they feel so. Thanks a lot!

> 
>  revision.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/revision.c b/revision.c
> index b37dbec378..e238430948 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2204,6 +2204,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>               revarg_opt |= REVARG_CANNOT_BE_FILENAME;
>       read_from_stdin = 0;
>       for (left = i = 1; i < argc; i++) {
> +             int maybe_rev = 0;
>               const char *arg = argv[i];
>               if (*arg == '-') {
>                       int opts;
> @@ -2234,11 +2235,16 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>                       }
>                       if (opts < 0)
>                               exit(128);
> -                     continue;
> +                     maybe_rev = 1;
> +                     left--; /* tentatively cancel "unknown opt" */
>               }
>  
> -
> -             if (handle_revision_arg(arg, revs, flags, revarg_opt)) {
> +             if (!handle_revision_arg(arg, revs, flags, revarg_opt)) {
> +                     got_rev_arg = 1;
> +             } else if (maybe_rev) {
> +                     left++; /* it turns out that it was "unknown opt" */
> +                     continue;
> +             } else {
>                       int j;
>                       if (seen_dashdash || *arg == '^')
>                               die("bad revision '%s'", arg);
> @@ -2255,8 +2261,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>                       append_prune_data(&prune_data, argv + i);
>                       break;
>               }
> -             else
> -                     got_rev_arg = 1;
>       }
>  
>       if (prune_data.nr) {

Thanks Junio, for the time you spent analysing and writing the above version of
the patch!

Regards,

- Siddharth Kannan

^ permalink raw reply related

* Re: Google Doc about the Contributors' Summit
From: Junio C Hamano @ 2017-02-11  3:26 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1702101658010.3496@virtualbox>

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

> Technically, it is not a write-up, and I never meant it to be that. I
> intended this document to help me remember what had been discussed, and I
> doubt it is useful at all to anybody who has not been there.
>
> I abused the Git mailing list to share that link, what I really should
> have done is to use an URL shortener and jot the result down on the
> whiteboard.
>
> Very sorry for that,

Heh, no need to apologize.

I saw your <alpine.DEB.2.20.1702071248430.3496@virtualbox> that was
sent to the list long after the event, which obviously no longer
meant for collaborative note taking and thought that you are
inviting others to read the result of that note taking, and that is
why I commented on that.  I've hopefully touched some "ask Junio
what he thinks of this" items and the whole thing was not wasted ;-)


^ permalink raw reply

* Re: What's cooking in git.git (Feb 2017, #03; Fri, 10)
From: Junio C Hamano @ 2017-02-11  3:12 UTC (permalink / raw)
  To: René Scharfe; +Cc: git, Vegard Nossum
In-Reply-To: <77af28f3-7a8e-fc6a-40ae-c4203d1a3a67@web.de>

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

> Am 10.02.2017 um 23:24 schrieb Junio C Hamano:
>> * vn/xdiff-func-context (2017-01-15) 1 commit
>>  - xdiff -W: relax end-of-file function detection
>>
>>  "git diff -W" has been taught to handle the case where a new
>>  function is added at the end of the file better.
>>
>>  Will hold.
>>  Discussion on an follow-up change to go back from the line that
>>  matches the funcline to show comments before the function
>>  definition has not resulted in an actionable conclusion.
>
> This one is a bug fix and can be merged already IMHO.

Absolutely.  I was just waiting if the follow-up discussion would
easily and quickly lead to another patch, forgot about what exactly
I was waiting for (i.e. the gravity of not having the follow-up),
and have left it in "Will hold" status forever.

Let's merge it to 'next' and then decide if we want to also merge it
to 'master' before the final.  The above step alone is a lot less
contriversial and tricky bugfix.

Thanks.

^ permalink raw reply

* Re: fuzzy patch application
From: Junio C Hamano @ 2017-02-11  3:09 UTC (permalink / raw)
  To: Nick Desaulniers; +Cc: Jeff King, Stefan Beller, git@vger.kernel.org
In-Reply-To: <CAKwvOdmvYiu4ApxL7jVh_0WZUMVeC2jJ4Q_Zs5Z5-J6GNH8DdQ@mail.gmail.com>

Nick Desaulniers <ndesaulniers@google.com> writes:

> For the dangers related to fuzzing, is there more info?  I found
> and old post on this from Linus calling fuzzing dangerous but
> from what I could tell about my patch that wouldn't apply
> without fuzzing, the only difference in bad hunks was
> whitespace that had diverged somehow.

If the "old post" is the one he explains why he chose not to allow
fuzz by default, I think you got all what you need.  Basically, he
wanted you and his users to make sure that the patch they are having
trouble with applying can be due to only insignificant difference
and it is safe to apply with reduced context, instead of blindly
accepting a fuzzy patch application.

^ permalink raw reply

* Re: [PATCH 1/2 v3] revision.c: args starting with "-" might be a revision
From: Junio C Hamano @ 2017-02-10 23:35 UTC (permalink / raw)
  To: Siddharth Kannan; +Cc: git, Matthieu.Moy, pranit.bauva, peff, pclouds, sandals
In-Reply-To: <1486752926-12020-2-git-send-email-kannan.siddharth12@gmail.com>

Siddharth Kannan <kannan.siddharth12@gmail.com> writes:

> @@ -2234,11 +2235,18 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>  			}
>  			if (opts < 0)
>  				exit(128);
> -			continue;
> +
> +			args = handle_revision_arg(arg, revs, flags, revarg_opt);
> +			handle_rev_arg_called = 1;
> +			if (args)
> +				continue;
> +			else
> +				--left;
>  		}
>  
>  
> -		if (handle_revision_arg(arg, revs, flags, revarg_opt)) {
> +		if ((handle_rev_arg_called && args) ||
> +				handle_revision_arg(arg, revs, flags, revarg_opt)) {

Naively I would have expected that removing the "continue" at the
end and letting the control go to the existing

	if (handle_revision_arg(arg, revs, flags, revarg_opt)) {

would be all that is needed.  The latter half of the patch is an
artifact of having ane xtra "handle_revision_arg() calls inside the
"if it begins with dash" block to avoid calling it twice.

So the difference is just "--left" (by the way, our codebase seem to
prefer "left--" when there is no difference between pre- or post-
decrement/increment) that adjusts the slot in argv[] where the next
unknown argument is stuffed to.

The adjustment is needed as the call to handle_revision_opt() that
is before the pre-context of this hunk stuffed the unknown thing
that begins with "-" into argv[left++]; if that thing turns out to
be a valid rev, then you would need to take it back, because after
all, that is not an unknown command line argument.

I am wondering if writing it like the following is easier to
understand.  I had a hard time figuring out what you are trying to
do, partly because "args" is quite a misnomer---implying "how many
arguments did we see" that is similar to opts that does mean "how
many options did handle_revision_opts() see?"  The variable means
means "yes we saw a valid rev" when it is zero.  The rewrite
below may avoid such a confusion.  I dunno.

 revision.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/revision.c b/revision.c
index b37dbec378..e238430948 100644
--- a/revision.c
+++ b/revision.c
@@ -2204,6 +2204,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 		revarg_opt |= REVARG_CANNOT_BE_FILENAME;
 	read_from_stdin = 0;
 	for (left = i = 1; i < argc; i++) {
+		int maybe_rev = 0;
 		const char *arg = argv[i];
 		if (*arg == '-') {
 			int opts;
@@ -2234,11 +2235,16 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 			}
 			if (opts < 0)
 				exit(128);
-			continue;
+			maybe_rev = 1;
+			left--; /* tentatively cancel "unknown opt" */
 		}
 
-
-		if (handle_revision_arg(arg, revs, flags, revarg_opt)) {
+		if (!handle_revision_arg(arg, revs, flags, revarg_opt)) {
+			got_rev_arg = 1;
+		} else if (maybe_rev) {
+			left++; /* it turns out that it was "unknown opt" */
+			continue;
+		} else {
 			int j;
 			if (seen_dashdash || *arg == '^')
 				die("bad revision '%s'", arg);
@@ -2255,8 +2261,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 			append_prune_data(&prune_data, argv + i);
 			break;
 		}
-		else
-			got_rev_arg = 1;
 	}
 
 	if (prune_data.nr) {

^ permalink raw reply related

* Re: What's cooking in git.git (Feb 2017, #03; Fri, 10)
From: René Scharfe @ 2017-02-10 23:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Vegard Nossum
In-Reply-To: <xmqq37flk7l4.fsf@gitster.mtv.corp.google.com>

Am 10.02.2017 um 23:24 schrieb Junio C Hamano:
> * vn/xdiff-func-context (2017-01-15) 1 commit
>  - xdiff -W: relax end-of-file function detection
>
>  "git diff -W" has been taught to handle the case where a new
>  function is added at the end of the file better.
>
>  Will hold.
>  Discussion on an follow-up change to go back from the line that
>  matches the funcline to show comments before the function
>  definition has not resulted in an actionable conclusion.

This one is a bug fix and can be merged already IMHO.


I have raw patches for showing comments before functions with -W, but 
they don't handle the case of a change being within such a comment. 
We'd want to show the trailing function which it is referring to in such 
a case, right?

And that's a bit tricky because it requires a more complicated model: 
Currently we distinguish only between function lines and the rest, while 
the new way would require identifying leading comment lines and probably 
also trailing function body lines in addition to that, and neither can 
be classified simply by looking at the line in question -- their type 
depends on that of neighboring lines.

René

^ permalink raw reply

* Re: fuzzy patch application
From: Nick Desaulniers @ 2017-02-10 22:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Stefan Beller, git@vger.kernel.org
In-Reply-To: <xmqqlgtdiroz.fsf@gitster.mtv.corp.google.com>

It's not my call about the defaults, but users can be surprised by
such changes.

For the dangers related to fuzzing, is there more info?  I found
and old post on this from Linus calling fuzzing dangerous but
from what I could tell about my patch that wouldn't apply
without fuzzing, the only difference in bad hunks was
whitespace that had diverged somehow.

On Fri, Feb 10, 2017 at 2:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Making "am -3" default may also scare users who are not exactly
>> comfortable with reading "git diff" output during a conflicted merge
>> and resolving a conflict, but other than that there shouldn't be any
>> downside.
>
> Another obvious downside is that there are those of us who prefer to
> run "am" without "-3" first to notice that the patch we expect to
> apply cleanly does apply cleanly, which gives us a chance to catch
> mistakes.  I personally feel that as long as there is a configuration
> that makes -3 a personal default (with --no-3way override from the
> command line), it is a better design not to enable "-3" by default
> for everybody.  New people can first learn using both forms and then
> after they got comfortable with resolving merges, they can choose to
> flip the default for themselves.



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply

* Re: fuzzy patch application
From: Junio C Hamano @ 2017-02-10 22:53 UTC (permalink / raw)
  To: Jeff King; +Cc: Stefan Beller, Nick Desaulniers, git@vger.kernel.org
In-Reply-To: <xmqqy3xdisos.fsf@gitster.mtv.corp.google.com>

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

> Making "am -3" default may also scare users who are not exactly
> comfortable with reading "git diff" output during a conflicted merge
> and resolving a conflict, but other than that there shouldn't be any
> downside.

Another obvious downside is that there are those of us who prefer to
run "am" without "-3" first to notice that the patch we expect to
apply cleanly does apply cleanly, which gives us a chance to catch
mistakes.  I personally feel that as long as there is a configuration
that makes -3 a personal default (with --no-3way override from the
command line), it is a better design not to enable "-3" by default
for everybody.  New people can first learn using both forms and then
after they got comfortable with resolving merges, they can choose to
flip the default for themselves.

^ permalink raw reply

* Re: [PATCH 2/2] ls-files: move only kept cache entries in prune_cache()
From: Brandon Williams @ 2017-02-10 22:51 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git List, Duy Nguyen, Junio C Hamano
In-Reply-To: <de84d0f4-cfce-1e73-6d5d-ad18df507d32@web.de>

On 02/10, René Scharfe wrote:
> prune_cache() first identifies those entries at the start of the sorted
> array that can be discarded.  Then it moves the rest of the entries up.
> Last it identifies the unwanted trailing entries among the moved ones
> and cuts them off.
> 
> Change the order: Identify both start *and* end of the range to keep
> first and then move only those entries to the top.  The resulting code
> is slightly shorter and a bit more efficient.
> 
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
> The performance impact is probably only measurable with a *really* big
> index.

Well there's been a lot of talk recently about *really* big indexes, so
I'm sure someone out there will be happy :)

> 
>  builtin/ls-files.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> index 18105ec7ea..1c0f057d02 100644
> --- a/builtin/ls-files.c
> +++ b/builtin/ls-files.c
> @@ -379,10 +379,7 @@ static void prune_cache(const char *prefix, size_t prefixlen)
>  	pos = cache_name_pos(prefix, prefixlen);
>  	if (pos < 0)
>  		pos = -pos-1;
> -	memmove(active_cache, active_cache + pos,
> -		(active_nr - pos) * sizeof(struct cache_entry *));
> -	active_nr -= pos;
> -	first = 0;
> +	first = pos;
>  	last = active_nr;
>  	while (last > first) {
>  		int next = (last + first) >> 1;
> @@ -393,7 +390,9 @@ static void prune_cache(const char *prefix, size_t prefixlen)
>  		}
>  		last = next;
>  	}
> -	active_nr = last;
> +	memmove(active_cache, active_cache + pos,
> +		(last - pos) * sizeof(struct cache_entry *));
> +	active_nr = last - pos;
>  }
>  
>  /*
> -- 
> 2.11.1
> 

Both these patches look good to me.

-- 
Brandon Williams

^ permalink raw reply

* Re: [PATCH] preload-index: avoid lstat for skip-worktree items
From: Junio C Hamano @ 2017-02-10 22:43 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jeff Hostetler
In-Reply-To: <a1297b9426e7980f41e9e662fc0f30717c576c3e.1486739428.git.johannes.schindelin@gmx.de>

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

> Teach preload-index to avoid lstat() calls for index-entries
> with skip-worktree bit set.  This is a performance optimization.
> ...
> diff --git a/preload-index.c b/preload-index.c
> index c1fe3a3ef9c..70a4c808783 100644
> --- a/preload-index.c
> +++ b/preload-index.c
> @@ -53,6 +53,8 @@ static void *preload_thread(void *_data)
>  			continue;
>  		if (ce_uptodate(ce))
>  			continue;
> +		if (ce_skip_worktree(ce))
> +			continue;
>  		if (!ce_path_match(ce, &p->pathspec, NULL))
>  			continue;
>  		if (threaded_has_symlink_leading_path(&cache, ce->name, ce_namelen(ce)))

Because we are only interested in marking the ones that match
between the index and the working tree as "up-to-date", and we are
not doing the opposite (i.e. toggle "up-to-date" bit off by noticing
that things are now different) in this codepath, this change does
make sense.  The ones marked as "skip", even if there were an
unrelated file or directory at the path where the index expects a
regular file, can be safely ignored.

Thanks.


^ permalink raw reply

* Re: [PATCH v2] git-p4: fix git-p4.pathEncoding for removed files
From: Junio C Hamano @ 2017-02-10 22:32 UTC (permalink / raw)
  To: Luke Diamand; +Cc: Lars Schneider, Git Users
In-Reply-To: <CAE5ih78xRAOFb+MC53qzNJyZ_d7j+87UYEt5Ku6JrkFhgHdD7w@mail.gmail.com>

Luke Diamand <luke@diamand.org> writes:

> On 9 February 2017 at 23:39, Junio C Hamano <gitster@pobox.com> wrote:
>> Lars Schneider <larsxschneider@gmail.com> writes:
>>
>>> unfortunately, I missed to send this v2. I agree with Luke's review and
>>> I moved the re-encode of the path name to the `streamOneP4File` and
>>> `streamOneP4Deletion` explicitly.
>>>
>>> Discussion:
>>> http://public-inbox.org/git/CAE5ih7-=bD_ZoL5pFYfD2Qvy-XE24V_cgge0XoAvuoTK02EDfg@mail.gmail.com/
>>>
>>> Thanks,
>>> Lars
>>
>> Thanks.  Will replace but will not immediately merge to 'next' yet,
>> just in case Luke wants to tell me add his "Reviewed-by:".
>
> Yes, this looks good to me now.

Thanks.

^ permalink raw reply

* Re: fuzzy patch application
From: Junio C Hamano @ 2017-02-10 22:31 UTC (permalink / raw)
  To: Jeff King; +Cc: Stefan Beller, Nick Desaulniers, git@vger.kernel.org
In-Reply-To: <20170210214850.2ok62xdmemgotwnt@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I dunno. I always use it, but I'm not sure if there are any downsides,
> aside from a little extra processing time. It does have some
> incompatibilities with other options. And I think it kicks in rename
> detection (but I might be mis-remembering another feature). That could
> be surprising, I guess.
>
> The original dates all the way back to 47f0b6d5d (Fall back to three-way
> merge when applying a patch., 2005-10-06), but I don't see any rationale
> for not making it the default. Junio probably could give a better
> answer.

Nothing deep.  Just being cautious by not to enable extra frills by
default, making it strictly o-p-t i-n.  As that was a strict fall
back (i.e. there was no "if we are going to fall back, we need to
spend extra cycles to do this extra preparation before we attempt
the regular application"), extra-processing cost was not a concern,
at least back I wrote it the first time.  I do not offhand know if
that still holds in the current one that was rewritten in C.

Making "am -3" default may also scare users who are not exactly
comfortable with reading "git diff" output during a conflicted merge
and resolving a conflict, but other than that there shouldn't be any
downside.


^ permalink raw reply

* What's cooking in git.git (Feb 2017, #03; Fri, 10)
From: Junio C Hamano @ 2017-02-10 22:24 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Pat Thoyts

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

2.12-rc1 has been tagged and most of the big things are behind us (I
am reluctant to merge anything large-ish after -rc0 and that was why
-rc0 preview had topics that cooked in 'next' only for a few days
in---usually my rule is to keep any non-trivial topic for about a
week in 'next').  There still are interesting and/or exciting things
coming into 'next', hopefully they will be the "killer" additions
for the release after the upcoming 2.12.  In the meantime, lets make
sure that we catch regressions in -rc1 and the tip of 'master'.

Oh, also I'd like to get pull requests for gitk and git-gui updates
soonish, if we are to have one during this cycle.

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

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

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

* bw/push-submodule-only (2017-02-01) 2 commits
  (merged to 'next' on 2017-02-06 at 851edafb14)
 + completion: add completion for --recurse-submodules=only
 + doc: add doc for git-push --recurse-submodules=only

 Add missing documentation update to a recent topic.


* da/t7800-cleanup (2017-02-08) 2 commits
  (merged to 'next' on 2017-02-10 at c983b65d33)
 + t7800: replace "wc -l" with test_line_count
 + Merge branch 'da/difftool-dir-diff-fix' into da/t7800-cleanup
 (this branch uses js/difftool-builtin.)

 Test updates.


* dl/difftool-doc-no-gui-option (2017-02-08) 1 commit
  (merged to 'next' on 2017-02-10 at 3a3496a740)
 + Document the --no-gui option in difftool

 Doc update.


* ew/complete-svn-authorship-options (2017-02-06) 1 commit
  (merged to 'next' on 2017-02-06 at dca324db7c)
 + completion: fix git svn authorship switches

 Correct command line completion (in contrib/) on "git svn"


* jk/log-graph-name-only (2017-02-08) 1 commit
  (merged to 'next' on 2017-02-10 at 2ab7ed99f4)
 + diff: print line prefix for --name-only output

 "git log --graph" did not work well with "--name-only", even though
 other forms of "diff" output were handled correctly.


* jk/reset-to-break-a-commit-doc (2017-02-03) 1 commit
  (merged to 'next' on 2017-02-06 at 7f571e62e9)
 + reset: add an example of how to split a commit into two

 A minor doc update.


* js/difftool-builtin (2017-02-08) 2 commits
  (merged to 'next' on 2017-02-10 at 30b3f383e8)
 + t7800: simplify basic usage test
  (merged to 'next' on 2017-02-06 at 6a90549f38)
 + difftool: fix bug when printing usage
 (this branch is used by da/t7800-cleanup.)

 A few hot-fixes to C-rewrite of "git difftool".


* nd/rev-list-all-includes-HEAD-doc (2017-02-08) 1 commit
  (merged to 'next' on 2017-02-10 at 5df8b81b57)
 + rev-list-options.txt: update --all about HEAD

 Doc update.


* ps/worktree-prune-help-fix (2017-02-06) 1 commit
  (merged to 'next' on 2017-02-06 at eeb96f1677)
 + worktree: fix option descriptions for `prune`

 Incorrect usage help message for "git worktree prune" has been fixed.


* rs/fill-directory-optim (2017-02-08) 1 commit
  (merged to 'next' on 2017-02-10 at 71047464df)
 + dir: avoid allocation in fill_directory()

 Code clean-up.


* rs/p5302-create-repositories-before-tests (2017-02-06) 1 commit
  (merged to 'next' on 2017-02-06 at f93bd2ed47)
 + p5302: create repositories for index-pack results explicitly

 Adjust a perf test to new world order where commands that do
 require a repository are really strict about having a repository.

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

* jk/alternate-ref-optim (2017-02-08) 11 commits
  (merged to 'next' on 2017-02-10 at f26f32cff6)
 + receive-pack: avoid duplicates between our refs and alternates
 + receive-pack: treat namespace .have lines like alternates
 + receive-pack: fix misleading namespace/.have comment
 + receive-pack: use oidset to de-duplicate .have lines
 + add oidset API
 + fetch-pack: cache results of for_each_alternate_ref
 + for_each_alternate_ref: replace transport code with for-each-ref
 + for_each_alternate_ref: pass name/oid instead of ref struct
 + for_each_alternate_ref: use strbuf for path allocation
 + for_each_alternate_ref: stop trimming trailing slashes
 + for_each_alternate_ref: handle failure from real_pathdup()

 Optimizes resource usage while enumerating refs from alternate
 object store, to help receiving end of "push" that hosts a
 repository with many "forks".

 Will cook in 'next'.


* lt/pathspec-negative (2017-02-10) 2 commits
  (merged to 'next' on 2017-02-10 at 8ea7874076)
 + pathspec: don't error out on all-exclusionary pathspec patterns
 + pathspec magic: add '^' as alias for '!'

 The "negative" pathspec feature was somewhat more cumbersome to use
 than necessary in that its short-hand used "!" which needed to be
 escaped from shells, and it required "exclude from what?" specified.

 Will cook in 'next'.


* nd/worktree-gc-protection (2017-02-08) 2 commits
 - worktree.c: use submodule interface to access refs from another worktree
 - refs.c: add resolve_ref_submodule()

 (hopefully) a beginning of safer "git worktree" that is resistant
 to "gc".

 Michael had a suggestion for a (hopefully) better alternative way
 to do this.  Expecting a reroll (either a tenative but quicker way,
 or a better but slower way).
 cf. <CACsJy8Diy92CNbJ1OBn893VFFrSsxBFWSyQHjt_Dzq9x7jfibQ@mail.gmail.com>


* sb/push-options-via-transport (2017-02-08) 1 commit
  (merged to 'next' on 2017-02-10 at 3e2d08e1fa)
 + push options: pass push options to the transport helper

 The push-options given via the "--push-options" option were not
 passed through to external remote helpers such as "smart HTTP" that
 are invoked via the transport helper.

 Will merge to 'master'.


* js/rebase-helper (2017-02-09) 2 commits
 - rebase -i: use the rebase--helper builtin
 - rebase--helper: add a builtin helper for interactive rebases

 "git rebase -i" starts using the recently updated "sequencer" code.

 Will merge to and then cook in 'next'.
 The change itself is small, but what it enables is rather a large
 body of new code.  We are getting there ;-)


* mh/submodule-hash (2017-02-10) 9 commits
 - read_loose_refs(): read refs using resolve_ref_recursively()
 - files_ref_store::submodule: use NULL for the main repository
 - base_ref_store_init(): remove submodule argument
 - refs: push the submodule attribute down
 - refs: store submodule ref stores in a hashmap
 - register_ref_store(): new function
 - refs: remove some unnecessary handling of submodule == ""
 - refs: make some ref_store lookup functions private
 - refs: reorder some function definitions

 Code and design clean-up for the refs API.

 Will merge to and then cook in 'next'.


* jh/mingw-openssl-sha1 (2017-02-09) 1 commit
  (merged to 'next' on 2017-02-10 at 084b3d8503)
 + mingw: use OpenSSL's SHA-1 routines

 Windows port wants to use OpenSSL's implementation of SHA-1
 routines, so let them.

 Will merge to 'master'.


* sb/doc-unify-bottom (2017-02-09) 1 commit
  (merged to 'next' on 2017-02-10 at 7229c4c1f7)
 + Documentation: unify bottom "part of git suite" lines

 Doc clean-up.

 Will merge to 'master'.


* dt/gc-ignore-old-gc-logs (2017-02-10) 2 commits
 - SQUASH???
 - gc: ignore old gc.log files

 A "gc.log" file left by a backgrounded "gc --auto" disables further
 automatic gc; it has been taught to run at least once a day (by
 default) by ignoring a stale "gc.log" file that is too old.

 Waiting for the discussion and reroll to quiet down (this one is v3
 or so and I saw v5 posted).


* js/git-path-in-subdir (2017-02-10) 2 commits
 - rev-parse: fix several options when running in a subdirectory
 - rev-parse tests: add tests executed from a subdirectory

 The "--git-path", "--git-common-dir", and "--shared-index-path"
 options of "git rev-parse" did not produce usable output.  They are
 now updated to show the path to the correct file, relative to where
 the caller is.

 Waiting for Review/Ack.
 cf. <cover.1486740772.git.johannes.schindelin@gmx.de>

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

* vn/xdiff-func-context (2017-01-15) 1 commit
 - xdiff -W: relax end-of-file function detection

 "git diff -W" has been taught to handle the case where a new
 function is added at the end of the file better.

 Will hold.
 Discussion on an follow-up change to go back from the line that
 matches the funcline to show comments before the function
 definition has not resulted in an actionable conclusion.


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

 Move more parts of "git bisect" to C.

 Expecting a reroll.
 cf. <CAFZEwPPXPPHi8KiEGS9ggzNHDCGhuqMgH9Z8-Pf9GLshg8+LPA@mail.gmail.com>
 cf. <CAFZEwPM9RSTGN54dzaw9gO9iZmsYjJ_d1SjUD4EzSDDbmh-XuA@mail.gmail.com>


* ls/filter-process-delayed (2017-01-08) 1 commit
 . convert: add "status=delayed" to filter process protocol

 Ejected, as does not build when merged to 'pu'.


* sh/grep-tree-obj-tweak-output (2017-01-20) 2 commits
 - grep: use '/' delimiter for paths
 - grep: only add delimiter if there isn't one already

 "git grep", when fed a tree-ish as an input, shows each hit
 prefixed with "<tree-ish>:<path>:<lineno>:".  As <tree-ish> is
 almost always either a commit or a tag that points at a commit, the
 early part of the output "<tree-ish>:<path>" can be used as the
 name of the blob and given to "git show".  When <tree-ish> is a
 tree given in the extended SHA-1 syntax (e.g. "<commit>:", or
 "<commit>:<dir>"), however, this results in a string that does not
 name a blob (e.g. "<commit>::<path>" or "<commit>:<dir>:<path>").
 "git grep" has been taught to be a bit more intelligent about these
 cases and omit a colon (in the former case) or use slash (in the
 latter case) to produce "<commit>:<path>" and
 "<commit>:<dir>/<path>" that can be used as the name of a blob.

 Waiting for the review discussion to settle, followed by a reroll.


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

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

 The fix in this patch is broken, unfortunately.

 Will discard.

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

* ls/p4-path-encoding (2017-02-09) 1 commit
 - git-p4: fix git-p4.pathEncoding for removed files

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

 Waiting for Review/Ack.
 cf. <20170209150656.9070-1-larsxschneider@gmail.com>


* mh/ref-remove-empty-directory (2017-01-07) 23 commits
  (merged to 'next' on 2017-02-10 at bcfd359e95)
 + files_transaction_commit(): clean up empty directories
 + try_remove_empty_parents(): teach to remove parents of reflogs, too
 + try_remove_empty_parents(): don't trash argument contents
 + try_remove_empty_parents(): rename parameter "name" -> "refname"
 + delete_ref_loose(): inline function
 + delete_ref_loose(): derive loose reference path from lock
 + log_ref_write_1(): inline function
 + log_ref_setup(): manage the name of the reflog file internally
 + log_ref_write_1(): don't depend on logfile argument
 + log_ref_setup(): pass the open file descriptor back to the caller
 + log_ref_setup(): improve robustness against races
 + log_ref_setup(): separate code for create vs non-create
 + log_ref_write(): inline function
 + rename_tmp_log(): improve error reporting
 + rename_tmp_log(): use raceproof_create_file()
 + lock_ref_sha1_basic(): use raceproof_create_file()
 + lock_ref_sha1_basic(): inline constant
 + raceproof_create_file(): new function
 + safe_create_leading_directories(): set errno on SCLD_EXISTS
 + safe_create_leading_directories_const(): preserve errno
 + t5505: use "for-each-ref" to test for the non-existence of references
 + refname_is_safe(): correct docstring
 + files_rename_ref(): tidy up whitespace

 Deletion of a branch "foo/bar" could remove .git/refs/heads/foo
 once there no longer is any other branch whose name begins with
 "foo/", but we didn't do so so far.  Now we do.

 Will cook in 'next'.


* cw/completion (2017-02-03) 7 commits
  (merged to 'next' on 2017-02-10 at b3a5cbf39c)
 + completion: recognize more long-options
 + completion: teach remote subcommands to complete options
 + completion: teach replace to complete options
 + completion: teach ls-remote to complete options
 + completion: improve bash completion for git-add
 + completion: add subcommand completion for rerere
 + completion: teach submodule subcommands to complete options

 More command line completion (in contrib/) for recent additions.

 Will merge to 'master'.


* cw/tag-reflog-message (2017-02-08) 1 commit
  (merged to 'next' on 2017-02-10 at 3968b3a58b)
 + tag: generate useful reflog message

 "git tag", because refs/tags/* doesn't keep reflog by default, did
 not leave useful message when adding a new entry to reflog.

 Will cook in 'next'.


* sg/completion (2017-02-03) 21 commits
  (merged to 'next' on 2017-02-10 at 55b2785d89)
 + completion: cache the path to the repository
 + completion: extract repository discovery from __gitdir()
 + completion: don't guard git executions with __gitdir()
 + completion: consolidate silencing errors from git commands
 + completion: don't use __gitdir() for git commands
 + completion: respect 'git -C <path>'
 + rev-parse: add '--absolute-git-dir' option
 + completion: fix completion after 'git -C <path>'
 + completion: don't offer commands when 'git --opt' needs an argument
 + completion: list short refs from a remote given as a URL
 + completion: don't list 'HEAD' when trying refs completion outside of a repo
 + completion: list refs from remote when remote's name matches a directory
 + completion: respect 'git --git-dir=<path>' when listing remote refs
 + completion: fix most spots not respecting 'git --git-dir=<path>'
 + completion: ensure that the repository path given on the command line exists
 + completion tests: add tests for the __git_refs() helper function
 + completion tests: check __gitdir()'s output in the error cases
 + completion tests: consolidate getting path of current working directory
 + completion tests: make the $cur variable local to the test helper functions
 + completion tests: don't add test cruft to the test repository
 + completion: improve __git_refs()'s in-code documentation
 (this branch is used by sg/completion-refs-speedup.)

 Clean-up and updates to command line completion (in contrib/).

 Will merge to 'master'.


* sg/completion-refs-speedup (2017-02-06) 13 commits
 - squash! completion: fill COMPREPLY directly when completing refs
 - completion: fill COMPREPLY directly when completing refs
 - completion: list only matching symbolic and pseudorefs when completing refs
 - completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMery
 - completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery
 - completion: let 'for-each-ref' strip the remote name from remote branches
 - completion: let 'for-each-ref' and 'ls-remote' filter matching refs
 - completion: don't disambiguate short refs
 - completion: don't disambiguate tags and branches
 - completion: support excluding full refs
 - completion: support completing full refs after '--option=refs/<TAB>'
 - completion: wrap __git_refs() for better option parsing
 - completion: remove redundant __gitcomp_nl() options from _git_commit()
 (this branch uses sg/completion.)

 The refs completion for large number of refs has been sped up,
 partly by giving up disambiguating ambiguous refs and partly by
 eliminating most of the shell processing between 'git for-each-ref'
 and 'ls-remote' and Bash's completion facility.

 Will hold.


* sk/parse-remote-cleanup (2017-02-06) 1 commit
  (merged to 'next' on 2017-02-06 at 6ec89f72d5)
 + parse-remote: remove reference to unused op_prep

 Code clean-up.

 Undecided.  There may be third-party scripts that are dot-sourcing
 this one.


* jk/delta-chain-limit (2017-01-27) 2 commits
  (merged to 'next' on 2017-02-06 at 9ff36ae9b2)
 + pack-objects: convert recursion to iteration in break_delta_chain()
 + pack-objects: enforce --depth limit in reused deltas

 "git repack --depth=<n>" for a long time busted the specified depth
 when reusing delta from existing packs.  This has been corrected.

 Will cook in 'next'.


* mm/merge-rename-delete-message (2017-01-30) 1 commit
  (merged to 'next' on 2017-02-10 at 8bf8146029)
 + merge-recursive: make "CONFLICT (rename/delete)" message show both paths

 When "git merge" detects a path that is renamed in one history
 while the other history deleted (or modified) it, it now reports
 both paths to help the user understand what is going on in the two
 histories being merged.

 Will cook in 'next'.


* rs/swap (2017-01-30) 5 commits
  (merged to 'next' on 2017-02-10 at 5253797d0a)
 + graph: use SWAP macro
 + diff: use SWAP macro
 + use SWAP macro
 + apply: use SWAP macro
 + add SWAP macro

 Code clean-up.

 Will merge to 'master'.


* ps/urlmatch-wildcard (2017-02-01) 5 commits
  (merged to 'next' on 2017-02-10 at 2ed9ea48ee)
 + urlmatch: allow globbing for the URL host part
 + urlmatch: include host in urlmatch ranking
 + urlmatch: split host and port fields in `struct url_info`
 + urlmatch: enable normalization of URLs with globs
 + mailmap: add Patrick Steinhardt's work address

 The <url> part in "http.<url>.<variable>" configuration variable
 can now be spelled with '*' that serves as wildcard.
 E.g. "http.https://*.example.com.proxy" can be used to specify the
 proxy used for https://a.example.com, https://b.example.com, etc.,
 i.e. any host in the example.com domain.

 Will cook in 'next'.


* sf/putty-w-args (2017-02-10) 5 commits
 - connect.c: stop conflating ssh command names and overrides
 - connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config
 - git_connect(): factor out SSH variant handling
 - connect: rename tortoiseplink and putty variables
 - connect: handle putty/plink also in GIT_SSH_COMMAND

 The command line options for ssh invocation needs to be tweaked for
 some implementations of SSH (e.g. PuTTY plink wants "-P <port>"
 while OpenSSH wants "-p <port>" to specify port to connect to), and
 the variant was guessed when GIT_SSH environment variable is used
 to specify it.  Extend the guess to the command specified by the
 newer GIT_SSH_COMMAND and also core.sshcommand configuration
 variable, and give an escape hatch for users to deal with
 misdetected cases.

 Will merge to 'next' and then to 'master'.


* jk/describe-omit-some-refs (2017-01-23) 5 commits
  (merged to 'next' on 2017-01-23 at f8a14b4996)
 + describe: teach describe negative pattern matches
 + describe: teach --match to accept multiple patterns
 + name-rev: add support to exclude refs by pattern match
 + name-rev: extend --refs to accept multiple patterns
 + doc: add documentation for OPT_STRING_LIST

 "git describe" and "git name-rev" have been taught to take more
 than one refname patterns to restrict the set of refs to base their
 naming output on, and also learned to take negative patterns to
 name refs not to be used for naming via their "--exclude" option.

 Will cook in 'next'.


* sb/submodule-doc (2017-01-12) 2 commits
  (merged to 'next' on 2017-02-10 at 5bfad5f30e)
 + submodule update documentation: don't repeat ourselves
 + submodule documentation: add options to the subcommand

 Doc updates.

 Will merge to 'master'.


* bw/attr (2017-02-01) 27 commits
 - attr: reformat git_attr_set_direction() function
 - attr: push the bare repo check into read_attr()
 - attr: store attribute stack in attr_check structure
 - attr: tighten const correctness with git_attr and match_attr
 - attr: remove maybe-real, maybe-macro from git_attr
 - attr: eliminate global check_all_attr array
 - attr: use hashmap for attribute dictionary
 - attr: change validity check for attribute names to use positive logic
 - attr: pass struct attr_check to collect_some_attrs
 - attr: retire git_check_attrs() API
 - attr: convert git_check_attrs() callers to use the new API
 - attr: convert git_all_attrs() to use "struct attr_check"
 - attr: (re)introduce git_check_attr() and struct attr_check
 - attr: rename function and struct related to checking attributes
 - attr.c: outline the future plans by heavily commenting
 - Documentation: fix a typo
 - attr.c: add push_stack() helper
 - attr: support quoting pathname patterns in C style
 - attr.c: plug small leak in parse_attr_line()
 - attr.c: tighten constness around "git_attr" structure
 - attr.c: simplify macroexpand_one()
 - attr.c: mark where #if DEBUG ends more clearly
 - attr.c: complete a sentence in a comment
 - attr.c: explain the lack of attr-name syntax check in parse_attr()
 - attr.c: update a stale comment on "struct match_attr"
 - attr.c: use strchrnul() to scan for one line
 - commit.c: use strchrnul() to scan for one line

 The gitattributes machinery is being taught to work better in a
 multi-threaded environment.

 Will merge to and then cook in 'next'.


* nd/worktree-move (2017-01-27) 7 commits
 . fixup! worktree move: new command
 . worktree remove: new command
 . worktree move: refuse to move worktrees with submodules
 . worktree move: accept destination as directory
 . worktree move: new command
 . worktree.c: add update_worktree_location()
 . worktree.c: add validate_worktree()

 "git worktree" learned move and remove subcommands.

 Tentatively ejected as it seems to break 'pu' when merged.


* cc/split-index-config (2016-12-26) 21 commits
 - Documentation/git-update-index: explain splitIndex.*
 - Documentation/config: add splitIndex.sharedIndexExpire
 - read-cache: use freshen_shared_index() in read_index_from()
 - read-cache: refactor read_index_from()
 - t1700: test shared index file expiration
 - read-cache: unlink old sharedindex files
 - config: add git_config_get_expiry() from gc.c
 - read-cache: touch shared index files when used
 - sha1_file: make check_and_freshen_file() non static
 - Documentation/config: add splitIndex.maxPercentChange
 - t1700: add tests for splitIndex.maxPercentChange
 - read-cache: regenerate shared index if necessary
 - config: add git_config_get_max_percent_split_change()
 - Documentation/git-update-index: talk about core.splitIndex config var
 - Documentation/config: add information for core.splitIndex
 - t1700: add tests for core.splitIndex
 - update-index: warn in case of split-index incoherency
 - read-cache: add and then use tweak_split_index()
 - split-index: add {add,remove}_split_index() functions
 - config: add git_config_get_split_index()
 - config: mark an error message up for translation

 The experimental "split index" feature has gained a few
 configuration variables to make it easier to use.

 Waiting for review comments to be addressed.
 cf. <20161226102222.17150-1-chriscool@tuxfamily.org>
 cf. <a1a44640-ff6c-2294-72ac-46322eff8505@ramsayjones.plus.com>


* kn/ref-filter-branch-list (2017-02-07) 21 commits
  (merged to 'next' on 2017-02-10 at 794bb8284d)
 + ref-filter: resurrect "strip" as a synonym to "lstrip"
  (merged to 'next' on 2017-01-31 at e7592a5461)
 + branch: implement '--format' option
 + branch: use ref-filter printing APIs
 + branch, tag: use porcelain output
 + ref-filter: allow porcelain to translate messages in the output
 + ref-filter: add an 'rstrip=<N>' option to atoms which deal with refnames
 + ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'
 + ref-filter: Do not abruptly die when using the 'lstrip=<N>' option
 + ref-filter: rename the 'strip' option to 'lstrip'
 + ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
 + ref-filter: introduce refname_atom_parser()
 + ref-filter: introduce refname_atom_parser_internal()
 + ref-filter: make "%(symref)" atom work with the ':short' modifier
 + ref-filter: add support for %(upstream:track,nobracket)
 + ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
 + ref-filter: introduce format_ref_array_item()
 + ref-filter: move get_head_description() from branch.c
 + ref-filter: modify "%(objectname:short)" to take length
 + ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
 + ref-filter: include reference to 'used_atom' within 'atom_value'
 + ref-filter: implement %(if), %(then), and %(else) atoms

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

 Will cook in 'next'.


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

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

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

 Will cook in 'next'.


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

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

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

 Will cook in 'next'.


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

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

--------------------------------------------------
[Discarded]

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

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

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


* sb/push-make-submodule-check-the-default (2017-01-26) 2 commits
  (merged to 'next' on 2017-01-26 at 5f4715cea6)
 + Revert "push: change submodule default to check when submodules exist"
  (merged to 'next' on 2016-12-12 at 1863e05af5)
 + push: change submodule default to check when submodules exist

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

 Retracted.

^ permalink raw reply

* Re: [PATCH v2] git-p4: fix git-p4.pathEncoding for removed files
From: Luke Diamand @ 2017-02-10 22:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Lars Schneider, Git Users
In-Reply-To: <xmqqfujnlyru.fsf@gitster.mtv.corp.google.com>

On 9 February 2017 at 23:39, Junio C Hamano <gitster@pobox.com> wrote:
> Lars Schneider <larsxschneider@gmail.com> writes:
>
>> unfortunately, I missed to send this v2. I agree with Luke's review and
>> I moved the re-encode of the path name to the `streamOneP4File` and
>> `streamOneP4Deletion` explicitly.
>>
>> Discussion:
>> http://public-inbox.org/git/CAE5ih7-=bD_ZoL5pFYfD2Qvy-XE24V_cgge0XoAvuoTK02EDfg@mail.gmail.com/
>>
>> Thanks,
>> Lars
>
> Thanks.  Will replace but will not immediately merge to 'next' yet,
> just in case Luke wants to tell me add his "Reviewed-by:".

Yes, this looks good to me now.

Luke

^ permalink raw reply

* Re: fuzzy patch application
From: Jeff King @ 2017-02-10 21:48 UTC (permalink / raw)
  To: Stefan Beller; +Cc: Junio C Hamano, Nick Desaulniers, git@vger.kernel.org
In-Reply-To: <CAGZ79kaG=oqDM=1+rz_zk6Qn-7wAszxPnBtqrkAJS29_qT7SoA@mail.gmail.com>

On Fri, Feb 10, 2017 at 01:37:12PM -0800, Stefan Beller wrote:

> > This is not exactly an answer to your question, but "git am -3" is often
> > a better solution than trying to fuzz patches. It assumes the patches
> > are Git patches (and record their origin blobs), and that you have that
> > blob (which should be true if the patches are based on the normal kernel
> > history, and you just fetch that history into your repository).
> >
> > I've found that this often manages to apply patches that "git apply"
> > will not by itself. And I also find the resulting conflicts to be much
> > easier to deal with than patch's ".rej" files.
> 
> I have been told this a couple of times before; do we want to make -3
> the default (in 2.13 then) ?

I dunno. I always use it, but I'm not sure if there are any downsides,
aside from a little extra processing time. It does have some
incompatibilities with other options. And I think it kicks in rename
detection (but I might be mis-remembering another feature). That could
be surprising, I guess.

The original dates all the way back to 47f0b6d5d (Fall back to three-way
merge when applying a patch., 2005-10-06), but I don't see any rationale
for not making it the default. Junio probably could give a better
answer.

-Peff

^ permalink raw reply

* Re: [PATCH] squash! completion: fill COMPREPLY directly when completing refs
From: Junio C Hamano @ 2017-02-10 21:44 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: git
In-Reply-To: <20170206181545.12869-1-szeder.dev@gmail.com>

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

> Care should be taken, though, because that prefix might contain
> 'for-each-ref' format specifiers as part of the left hand side of a
> '..' range or '...' symmetric difference notation or fetch/push/etc.
> refspec, e.g. 'git log "evil-%(refname)..br<TAB>'.  Doubling every '%'
> in the prefix will prevent 'git for-each-ref' from interpolating any
> of those contained format specifiers.
> ---
>
> This is really pathological, and I'm sure this has nothing to do
> with whatever breakage Jacob experienced.  The shell
> metacharacters '(' and ')' still cause us trouble in various ways,
> but that's nothing new and has been the case for quite a while
> (always?).
>
> It's already incorporated into (the rewritten)
>
>   https://github.com/szeder/git completion-refs-speedup

Should I expect a reroll to come, or is this the only fix-up to the
series that begins at <20170203025405.8242-1-szeder.dev@gmail.com>?

No hurries.

^ 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