Git development
 help / color / mirror / Atom feed
* 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: 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: 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: 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: [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: 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: 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: 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: [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: [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: 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 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: [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 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

* [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 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

* [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 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

* [RFC PATCH] show decorations at the end of the line
From: Linus Torvalds @ 2017-02-11 18:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List


So I use "--show-decorations" all the time because I find it very useful 
to see where the origin branch is, where tags are etc. In fact, my global 
git config file has

    [log]
        decorate = auto

in it, so that I don't have to type it out all the time when I just do my 
usual 'git log". It's lovely.

However, it does make one particular case uglier: with commit decorations, 
the "oneline" commit format ends up being not very pretty:

    [torvalds@i7 git]$ git log --oneline -10
    3f07dac29 (HEAD -> master) pathspec: don't error out on  all-exclusionary pathspec patterns
    ca4a562f2 pathspec magic: add '^' as alias for '!'
    02555c1b2 ls-remote: add "--diff" option to show only refs that differ
    6e3a7b339 (tag: v2.12.0-rc0, origin/master, origin/HEAD) Git 2.12-rc0
    fafca0f72 Merge branch 'cw/log-updates-for-all-refs-really'
    74dee5cfa Merge branch 'pl/complete-diff-submodule-diff'
    36acf4123 Merge branch 'rs/object-id'
    ecc486b1f Merge branch 'js/re-running-failed-tests'
    4ba6bb2d1 Merge branch 'sb/submodule-update-initial-runs-custom-script'
    5348021c6 Merge branch 'sb/submodule-recursive-absorb'

and note how the decoration comes right after the shortened commit hash, 
breaking up the alignment of the messages. 

The above doesn't show it with the colorization: I also have

    [color]
        ui=auto

so on my terminal the decoration is also nicely colorized which makes it 
much more obvious, it's not as obvious in this message.

The oneline message handling is already pretty special, this makes it even 
more special by putting the decorations at the end of the line:

    3f07dac29 pathspec: don't error out on all-exclusionary pathspec patterns (HEAD -> master)
    ca4a562f2 pathspec magic: add '^' as alias for '!'
    02555c1b2 ls-remote: add "--diff" option to show only refs that differ
    6e3a7b339 Git 2.12-rc0 (tag: v2.12.0-rc0, origin/master, origin/HEAD)
    fafca0f72 Merge branch 'cw/log-updates-for-all-refs-really'
    74dee5cfa Merge branch 'pl/complete-diff-submodule-diff'
    36acf4123 Merge branch 'rs/object-id'
    ecc486b1f Merge branch 'js/re-running-failed-tests'
    4ba6bb2d1 Merge branch 'sb/submodule-update-initial-runs-custom-script'
    5348021c6 Merge branch 'sb/submodule-recursive-absorb'

which looks a lot better (again, this is all particularly noticeable with 
colorization).

NOTE! There's a very special case for "git log --oneline -g" that shows 
the reflogs as oneliners, and this does *not* fix that special case. It's 
a lot more involved and relies on the exact show_reflog_message() 
implementation, so I left the format for that alone, along with a comment 
about how it's not at the end of line.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

I've signed off on this, because I think it's an "obvious" improvement, 
but I'm putting the "RFC" in the subject line because this is clearly a 
subjective thing.

"oneline" really is special: the other commit formats will just put the 
commit SHA1 at the end of the line anyway. And with manual formats, the 
placement of decorations is also manual, so this doesn't affect that 
case.

Comments?

 log-tree.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/log-tree.c b/log-tree.c
index 8c2415747..3bf88182e 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -622,10 +622,13 @@ void show_log(struct rev_info *opt)
 			       find_unique_abbrev(parent->object.oid.hash,
 						  abbrev_commit));
 		fputs(diff_get_color_opt(&opt->diffopt, DIFF_RESET), opt->diffopt.file);
-		show_decorations(opt, commit);
 		if (opt->commit_format == CMIT_FMT_ONELINE) {
+			/* Not at end of line, but.. */
+			if (opt->reflog_info)
+				show_decorations(opt, commit);
 			putc(' ', opt->diffopt.file);
 		} else {
+			show_decorations(opt, commit);
 			putc('\n', opt->diffopt.file);
 			graph_show_oneline(opt->graph);
 		}
@@ -716,6 +719,8 @@ void show_log(struct rev_info *opt)
 		opt->missing_newline = 0;
 
 	graph_show_commit_msg(opt->graph, opt->diffopt.file, &msgbuf);
+	if (ctx.fmt == CMIT_FMT_ONELINE)
+		show_decorations(opt, commit);
 	if (opt->use_terminator && !commit_format_is_empty(opt->commit_format)) {
 		if (!opt->missing_newline)
 			graph_show_padding(opt->graph);

^ permalink raw reply related

* Re: [RFC PATCH] show decorations at the end of the line
From: Linus Torvalds @ 2017-02-11 18:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.20.1702110943460.31350@i7.lan>

On Sat, Feb 11, 2017 at 10:02 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I've signed off on this, because I think it's an "obvious" improvement,
> but I'm putting the "RFC" in the subject line because this is clearly a
> subjective thing.

Side note: the one downside of showing the decorations at the end of
the line is that now they are obviously at the end of the line - and
thus likely to be more hidden by things like line truncation.

The default git settings (LESS=FRX) no longer truncate log output
lines, but if you use "-S" or "--chop-long-lines", you will obviously
be missing the end of long lines.

So moving the decorations to the end does obviously have a real UI
impact, apart from just being "prettier".

I just wanted to point that out. I still prefer decorations at ends of
lines (and yes, that's despite the fact that I actually personally use
the traditional git setting of "LESS=FRSX"), but it is perhaps
something that people should be aware of if this patch causes
discussion.

               Linus

^ permalink raw reply

* Re: [PATCH] mingw: use OpenSSL's SHA-1 routines
From: Junio C Hamano @ 2017-02-11 18:51 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Johannes Schindelin, git, Jeff Hostetler
In-Reply-To: <31bb0b9f-d498-24b3-57d5-9f34cb8e3914@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

> Am 10.02.2017 um 00:41 schrieb Junio C Hamano:
>> ...
>> 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.

OK.  Will mark it as "Will cook in 'next'" for now.

^ permalink raw reply

* Re: [PATCH] cocci: detect useless free(3) calls
From: Junio C Hamano @ 2017-02-11 19:01 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git List, Pranit Bauva
In-Reply-To: <7e10f934-f084-ceb4-00eb-b75cdb01886b@web.de>

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

> 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.

Thanks.  This should have been caught by code inspection, but
having automated way to do so is always good.

>
>  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);

^ permalink raw reply

* Re: [PATCH] cocci: detect useless free(3) calls
From: Lars Schneider @ 2017-02-11 19:31 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git List, Junio C Hamano, Pranit Bauva
In-Reply-To: <7e10f934-f084-ceb4-00eb-b75cdb01886b@web.de>


> On 11 Feb 2017, at 14:58, René Scharfe <l.s.r@web.de> wrote:
> 
> 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.

Hi Rene,

how do you run these checks on the entire Git source?
Do you run each semantic patch file on the source like this?

spatch --sp-file contrib/coccinelle/qsort.cocci --dir /path/to/git/git
...
spatch --sp-file contrib/coccinelle/free.cocci --dir /path/to/git/git

How stable do you consider these checks? Would it make sense to run them
as part of the Travis-CI build [1]? 

Thanks,
Lars

[1] https://travis-ci.org/git/git/branches

^ permalink raw reply

* Re: [PATCH] cocci: detect useless free(3) calls
From: René Scharfe @ 2017-02-11 19:50 UTC (permalink / raw)
  To: Lars Schneider; +Cc: Git List, Junio C Hamano, Pranit Bauva
In-Reply-To: <5DF3AA3E-90C6-443C-A22C-489CE2C89A51@gmail.com>

Am 11.02.2017 um 20:31 schrieb Lars Schneider:
> how do you run these checks on the entire Git source?
> Do you run each semantic patch file on the source like this?
> 
> spatch --sp-file contrib/coccinelle/qsort.cocci --dir /path/to/git/git
> ...
> spatch --sp-file contrib/coccinelle/free.cocci --dir /path/to/git/git

With "make coccicheck", which runs spatch against the items in the make
variable C_SOURCES, for all .cocci files.

> How stable do you consider these checks? Would it make sense to run them
> as part of the Travis-CI build [1]? 

There seem to have been problems with older versions[2], but I'm not
aware of other issues.

Having these checks run automatically would be nice because they
require a special tool which I guess not everybody is willing (or able)
to install and because they take multiple minutes.

René


[2] https://public-inbox.org/git/014ef44e-9dd8-40b3-a3ec-b483f938ee02@web.de/

^ permalink raw reply

* [PATCH] rm: reuse strbuf for all remove_dir_recursively() calls, again
From: René Scharfe @ 2017-02-11 19:51 UTC (permalink / raw)
  To: Git List, Stefan Beller; +Cc: Junio C Hamano

Don't throw the memory allocated for remove_dir_recursively() away after
a single call, use it for the other entries as well instead.

This change was done before in deb8e15a (rm: reuse strbuf for all
remove_dir_recursively() calls), but was reverted as a side-effect of
55856a35 (rm: absorb a submodules git dir before deletion). Reinstate
the optimization.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
Was deb8e15a a rebase victim?

 builtin/rm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index 452170a3ab..fb79dcab18 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -360,15 +360,14 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	 */
 	if (!index_only) {
 		int removed = 0, gitmodules_modified = 0;
+		struct strbuf buf = STRBUF_INIT;
 		for (i = 0; i < list.nr; i++) {
 			const char *path = list.entry[i].name;
 			if (list.entry[i].is_submodule) {
-				struct strbuf buf = STRBUF_INIT;
-
+				strbuf_reset(&buf);
 				strbuf_addstr(&buf, path);
 				if (remove_dir_recursively(&buf, 0))
 					die(_("could not remove '%s'"), path);
-				strbuf_release(&buf);
 
 				removed = 1;
 				if (!remove_path_from_gitmodules(path))
@@ -382,6 +381,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 			if (!removed)
 				die_errno("git rm: '%s'", path);
 		}
+		strbuf_release(&buf);
 		if (gitmodules_modified)
 			stage_updated_gitmodules();
 	}
-- 
2.11.1


^ permalink raw reply related


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