Re: [PATCH] repository: pre-initialize hash algo pointer

[Eric Sunshine]

On Thu, Jan 18, 2018 at 11:18 PM, brian m. carlson
 wrote:
> There are various git subcommands (among them, clone) which don't set up
> the repository (that is, they lack RUN_SETUP or RUN_SETUP_GENTLY) but
> end up needing to have information about the hash algorithm in use.
> Because the hash algorithm is part of struct repository and it's only
> initialized in repository setup, we can end up dereferencing a NULL
> pointer in some cases if we call one of these subcommands and look up
> the empty blob or empty tree values.
>
> In the future, we can add a command line option for this or read it from
> the configuration, but until we're ready to expose that functionality to
> the user, simply initialize the repository structure to use the current
> hash algorithm, SHA-1.
>
> Signed-off-by: brian m. carlson 
> ---
> I'm still quite mystified as to why this is working on Linux and not
> macOS, but I can only guess that compilers are just very advanced and
> have somehow concluded that we would clearly never dereference a NULL
> pointer, so they picked the only non-NULL value.

Now that we know (due to Duy's excellent detective work[1]) that the
trigger is files with names differing only in case on case-insensitive
filesystems, the commit message can be updated appropriately.

> I haven't included a test because I have no way to reproduce the issue.
> This patch is the first from a series I'm working on where I do expand
> the use of the hash struct and therefore caused a segfault on clone, so
> I can imagine what's going on without having a way to prove it affects
> this particular case.
>
> If someone with access to macOS can provide a test, I'd be very
> grateful.

Done. Find the test here[2]. It fails without your fix on MacOS and
(presumably) Windows, and succeeds with the fix.

> My apologies for the error and inconvenience.

[1]: 
https://public-inbox.org/git/CACsJy8BTFm_0sv=roL1OKKW=1dyu3vqd50nkyhg3kq7g+ma...@mail.gmail.com/
[2]: https://public-inbox.org/git/20180119074001.GA55929@flurp.local/

Re: git 2.16.0 segfaults on clone of specific repo

[Eric Sunshine]

On Fri, Jan 19, 2018 at 12:31:58PM +0700, Duy Nguyen wrote:
> On Fri, Jan 19, 2018 at 10:40 AM, brian m. carlson
> > I'm still extremely puzzled as to why this doesn't fail on Linux.  If
> > it's failing in this case, it should very, very clearly fail all the
> > time we access an empty blob or an empty tree.[...]
> 
> I think it's file system related, case-insensitive one in particular.
> 
> The call trace posted at the beginning of this thread should never
> trigger for an initial clone. You only check if an _existing_ entry
> matches what you are about to checkout when you switch trees. For this
> to happen at clone time, I suppose you have to checkout entry "A",
> then re-checkout "A" again. Which can only happen on case-insensitive
> file systems and a case-sensitive repo where the second "A" might
> actually be "a".
> [...]
> On Linux, after I hacked all over the place to force ce_match_stat()
> to eventually call index_fd() which in turns must use one of the
> hashing function, I got a crash.

Nice detective work. This particular manifestation is caught by the
following test which fails without brian's patch on MacOS (and
presumably Windows) and succeeds with it. On Linux and BSD, it will of
course succeed always, so I'm not sure how much practical value it
has.

--- >8 ---
hex2oct() {
perl -ne 'printf "\\%03o", hex for /../g'
}

test_expect_success 'clone on case-insensitive fs' '
o=$(git hash-object -w --stdin 8 ---

(hex2oct lifted from t1007/t1450)

Re: git 2.16.0 segfaults on clone of specific repo

[Duy Nguyen]

On Fri, Jan 19, 2018 at 10:40 AM, brian m. carlson
 wrote:
> On Thu, Jan 18, 2018 at 10:06:10PM -0500, Eric Sunshine wrote:
>> > I have a guess about what the problem might be.  Can you try this patch
>> > and see if it fixes things?
>>
>> That does fix the crash. Thanks for the quick diagnosis.
>>
>> Can the commit message go into more detail as to why this was crashing
>> (or your speculation about why)? Perhaps give more detail about what
>> 'clone' is doing that led to the crash.
>
> Sure.  I ran into this as I was expanding the hash structure abstraction
> into my next series.  I'll send a follow-up patch with a more
> descriptive answer.
>
> I'm still extremely puzzled as to why this doesn't fail on Linux.  If
> it's failing in this case, it should very, very clearly fail all the
> time we access an empty blob or an empty tree.  I've tried with gcc and
> two versions of clang, using -fno-lto, with address sanitizer, with -O0,
> and so on.  I'd really like to catch this kind of issue sooner in the
> future if I can figure out how to reproduce it.

I think it's file system related, case-insensitive one in particular.

The call trace posted at the beginning of this thread should never
trigger for an initial clone. You only check if an _existing_ entry
matches what you are about to checkout when you switch trees. For this
to happen at clone time, I suppose you have to checkout entry "A",
then re-checkout "A" again. Which can only happen on case-insensitive
file systems and a case-sensitive repo where the second "A" might
actually be "a".

vim-colorschemes.git has these two entries, which meets one of the
conditions, I suppose macos meets the other

colors/darkblue.vim
colors/darkBlue.vim

On Linux, after I hacked all over the place to force ce_match_stat()
to eventually call index_fd() which in turns must use one of the
hashing function, I got a crash.
-- 
Duy

It's all about 14

[Facebook Int'l]

Hello ,


facebook is given out  14,000,000.USD (Fourteen Million Dollars) its all about 
14 Please, respond with your Unique Code (FB/BF14-13M5250UD) 
using your registration email, to the Verification Department at; 
dustinmoskovitz.faceb...@gmail.com


Dustin Moskovitz
Facebook Team
Copyright © 2018 Facebook Int'l

[PATCH] repository: pre-initialize hash algo pointer

[brian m. carlson]

There are various git subcommands (among them, clone) which don't set up
the repository (that is, they lack RUN_SETUP or RUN_SETUP_GENTLY) but
end up needing to have information about the hash algorithm in use.
Because the hash algorithm is part of struct repository and it's only
initialized in repository setup, we can end up dereferencing a NULL
pointer in some cases if we call one of these subcommands and look up
the empty blob or empty tree values.

In the future, we can add a command line option for this or read it from
the configuration, but until we're ready to expose that functionality to
the user, simply initialize the repository structure to use the current
hash algorithm, SHA-1.

Signed-off-by: brian m. carlson 
---
 repository.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

I'm still quite mystified as to why this is working on Linux and not
macOS, but I can only guess that compilers are just very advanced and
have somehow concluded that we would clearly never dereference a NULL
pointer, so they picked the only non-NULL value.

I haven't included a test because I have no way to reproduce the issue.
This patch is the first from a series I'm working on where I do expand
the use of the hash struct and therefore caused a segfault on clone, so
I can imagine what's going on without having a way to prove it affects
this particular case.

If someone with access to macOS can provide a test, I'd be very
grateful.

My apologies for the error and inconvenience.

diff --git a/repository.c b/repository.c
index 998413b8bb..f66fcb1342 100644
--- a/repository.c
+++ b/repository.c
@@ -5,7 +5,7 @@
 
 /* The main repository */
 static struct repository the_repo = {
-   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, NULL, 
0, 0
+   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, 
_algos[GIT_HASH_SHA1], 0, 0
 };
 struct repository *the_repository = _repo;
 

Re: git 2.16.0 segfaults on clone of specific repo

[brian m. carlson]

On Thu, Jan 18, 2018 at 10:06:10PM -0500, Eric Sunshine wrote:
> > I have a guess about what the problem might be.  Can you try this patch
> > and see if it fixes things?
> 
> That does fix the crash. Thanks for the quick diagnosis.
> 
> Can the commit message go into more detail as to why this was crashing
> (or your speculation about why)? Perhaps give more detail about what
> 'clone' is doing that led to the crash.

Sure.  I ran into this as I was expanding the hash structure abstraction
into my next series.  I'll send a follow-up patch with a more
descriptive answer.

I'm still extremely puzzled as to why this doesn't fail on Linux.  If
it's failing in this case, it should very, very clearly fail all the
time we access an empty blob or an empty tree.  I've tried with gcc and
two versions of clang, using -fno-lto, with address sanitizer, with -O0,
and so on.  I'd really like to catch this kind of issue sooner in the
future if I can figure out how to reproduce it.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

RE: git 2.16.0 segfaults on clone of specific repo

[Randall S. Becker]

On January 18, 2018 10:06 PM, Eric Sunshine wrote:
> On Thu, Jan 18, 2018 at 9:47 PM, brian m. carlson
>  wrote:
> > On Thu, Jan 18, 2018 at 07:15:56PM -0500, Eric Sunshine wrote:
> >> On Thu, Jan 18, 2018 at 3:55 PM, Александр Булаев
> >>  wrote:
> >> > I found that git 2.16.0 segfaults on clone of vim-colorschemes repo.
> >>
> >> I can confirm that this crashes on MacOS; it does not crash on Linux or
> BSD.
> >>
> >> git-bisect places blame on eb0ccfd7f5 (Switch empty tree and blob
> >> lookups to use hash abstraction, 2017-11-12).
> >
> > I unfortunately don't have a macOS system to test with, and I've
> > compiled with both gcc and clang on my Debian system and, as you
> > mentioned, it doesn't fail there.
> >
> > I have a guess about what the problem might be.  Can you try this
> > patch and see if it fixes things?
> 
> That does fix the crash. Thanks for the quick diagnosis.
> 
> Can the commit message go into more detail as to why this was crashing (or
> your speculation about why)? Perhaps give more detail about what 'clone' is
> doing that led to the crash.

I'm curious as to why this worked on my platform, given how it tends to get 
annoyed with NULL in the wrong place.

> 
> > -- >8 --
> > From 10b690241619a452634b31fbc5ccd054a4f6e5ec Mon Sep 17 00:00:00
> 2001
> > From: "brian m. carlson" 
> > Date: Sun, 14 Jan 2018 18:26:29 +
> > Subject: [PATCH] repository: pre-initialize hash algo pointer
> >
> > There are various git subcommands (among them, clone) which don't set
> > up the repository but end up needing to have information about the
> > hash algorithm in use.  In the future, we can add a command line
> > option for this or read it from the configuration, but until we're
> > ready to expose that functionality to the user, simply initialize the
> > repository structure to use the current hash algorithm, SHA-1.
> >
> > Signed-off-by: brian m. carlson 
> > ---
> >  repository.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/repository.c b/repository.c index 998413b8bb..f66fcb1342
> > 100644
> > --- a/repository.c
> > +++ b/repository.c
> > @@ -5,7 +5,7 @@
> >
> >  /* The main repository */
> >  static struct repository the_repo = {
> > -   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index,
> NULL, 0, 0
> > +   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> > + _index, _algos[GIT_HASH_SHA1], 0, 0
> >  };
> >  struct repository *the_repository = _repo;
> >
> > --

Re: git 2.16.0 segfaults on clone of specific repo

[Eric Sunshine]

On Thu, Jan 18, 2018 at 9:47 PM, brian m. carlson
 wrote:
> On Thu, Jan 18, 2018 at 07:15:56PM -0500, Eric Sunshine wrote:
>> On Thu, Jan 18, 2018 at 3:55 PM, Александр Булаев
>>  wrote:
>> > I found that git 2.16.0 segfaults on clone of vim-colorschemes repo.
>>
>> I can confirm that this crashes on MacOS; it does not crash on Linux or BSD.
>>
>> git-bisect places blame on eb0ccfd7f5 (Switch empty tree and blob
>> lookups to use hash abstraction, 2017-11-12).
>
> I unfortunately don't have a macOS system to test with, and I've
> compiled with both gcc and clang on my Debian system and, as you
> mentioned, it doesn't fail there.
>
> I have a guess about what the problem might be.  Can you try this patch
> and see if it fixes things?

That does fix the crash. Thanks for the quick diagnosis.

Can the commit message go into more detail as to why this was crashing
(or your speculation about why)? Perhaps give more detail about what
'clone' is doing that led to the crash.

> -- >8 --
> From 10b690241619a452634b31fbc5ccd054a4f6e5ec Mon Sep 17 00:00:00 2001
> From: "brian m. carlson" 
> Date: Sun, 14 Jan 2018 18:26:29 +
> Subject: [PATCH] repository: pre-initialize hash algo pointer
>
> There are various git subcommands (among them, clone) which don't set up
> the repository but end up needing to have information about the hash
> algorithm in use.  In the future, we can add a command line option for
> this or read it from the configuration, but until we're ready to expose
> that functionality to the user, simply initialize the repository
> structure to use the current hash algorithm, SHA-1.
>
> Signed-off-by: brian m. carlson 
> ---
>  repository.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/repository.c b/repository.c
> index 998413b8bb..f66fcb1342 100644
> --- a/repository.c
> +++ b/repository.c
> @@ -5,7 +5,7 @@
>
>  /* The main repository */
>  static struct repository the_repo = {
> -   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, 
> NULL, 0, 0
> +   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, 
> _algos[GIT_HASH_SHA1], 0, 0
>  };
>  struct repository *the_repository = _repo;
>
> --

Re: git 2.16.0 segfaults on clone of specific repo

[brian m. carlson]

On Thu, Jan 18, 2018 at 07:15:56PM -0500, Eric Sunshine wrote:
> [+cc:brian]
> 
> On Thu, Jan 18, 2018 at 3:55 PM, Александр Булаев
>  wrote:
> > I found that git 2.16.0 segfaults on clone of vim-colorschemes repo.
> >
> > (lldb) run
> > Process 25643 launched: '/usr/local/bin/git' (x86_64)
> > Cloning into 'vim-colorschemes'...
> > remote: Counting objects: 1457, done.
> > remote: Total 1457 (delta 0), reused 0 (delta 0), pack-reused 1457
> > Receiving objects: 100% (1457/1457), 1.43 MiB | 289.00 KiB/s, done.
> > Resolving deltas: 100% (424/424), done.
> > Process 25643 stopped
> > * thread #1, queue = 'com.apple.main-thread', stop reason =
> > EXC_BAD_ACCESS (code=1, address=0x48)
> 
> I can confirm that this crashes on MacOS; it does not crash on Linux or BSD.
> 
> git-bisect places blame on eb0ccfd7f5 (Switch empty tree and blob
> lookups to use hash abstraction, 2017-11-12).

I unfortunately don't have a macOS system to test with, and I've
compiled with both gcc and clang on my Debian system and, as you
mentioned, it doesn't fail there.

I have a guess about what the problem might be.  Can you try this patch
and see if it fixes things?

-- >8 --
From 10b690241619a452634b31fbc5ccd054a4f6e5ec Mon Sep 17 00:00:00 2001
From: "brian m. carlson" 
Date: Sun, 14 Jan 2018 18:26:29 +
Subject: [PATCH] repository: pre-initialize hash algo pointer

There are various git subcommands (among them, clone) which don't set up
the repository but end up needing to have information about the hash
algorithm in use.  In the future, we can add a command line option for
this or read it from the configuration, but until we're ready to expose
that functionality to the user, simply initialize the repository
structure to use the current hash algorithm, SHA-1.

Signed-off-by: brian m. carlson 
---
 repository.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/repository.c b/repository.c
index 998413b8bb..f66fcb1342 100644
--- a/repository.c
+++ b/repository.c
@@ -5,7 +5,7 @@
 
 /* The main repository */
 static struct repository the_repo = {
-   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, NULL, 
0, 0
+   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, 
_algos[GIT_HASH_SHA1], 0, 0
 };
 struct repository *the_repository = _repo;
 
-- 
2.16.0.rc2.280.g09355b536d
-- >8 --
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

RE: [RFC PATCH] NonStop port changes for git 2.16.0.

[Randall S. Becker]

On January 18, 2018 7:11 PM, Ævar Arnfjörð Bjarmason wrote:
> On Thu, Jan 18 2018, Stefan Beller jotted:
> > On Thu, Jan 18, 2018 at 2:42 PM,   wrote:
> >> Further: there are 6 known breakages that have been reported. The
> >> perl issues relating to completion codes are being examined at
> >> present by the platform support teams so are not addressed by this
> patch.
> >
> > For perl I'd suggest cc'ing Ævar (cc'd just now) as that seems his
> > field of expertise.
> 
> Just to make sure I'm keeping up, this refers to some breakage not detailed
> in the patch above, right?
> 
> The only perl-related change I see is undoing part of 6c109904bc ("Port to HP
> NonStop", 2012-09-19) having to do with how we find perl/python (which,
> not being at all familiar with NonStop, makes sense to me).
> 
> But sure, if there's some details about those 6 issues I might have time to
> take a look, but it sounds like it's being looked at by NonStop support...

The issue I found is that NonStop perl is reporting completion code 162/169 
when 'die "stuff";' is run from stdin, while reporting 255 when a real file is 
used. This isn't covered by the patch. The values are in the range 128+sig#, 
but no such signals are value on the platform (SIGABEND is 31 and SIGGUARDIAN 
is 99 with a gap between those). I had tried a few experiments mapping the 
wonky completion codes in run-command.c to something sane but that caused more 
breakages (60) in the test suite than leaving well enough alone (6). From what 
I can determine, git is behaving reasonably properly in the conditions tested, 
but the test suite structure in its use of perl is triggering artifacts that 
appear to be breakages when really not. This is currently being investigated by 
the HPE support team, so I'm leaving perl completion matters in their hands for 
now.

Cheers,
Randall

-- Brief whoami:
  NonStop developer since approximately NonStop(2112884442)
  UNIX developer since approximately 421664400
-- In my real life, I talk too much.

Re: [PATCH 06/11] git fetch doc: add a new section to explain the ins & outs of pruning

[Eric Sunshine]

On Thu, Jan 18, 2018 at 7:00 PM, Ævar Arnfjörð Bjarmason
 wrote:
> Add a new section to canonically explain how remote reference pruning
> works, and how users should be careful about using it in conjunction
> with tag refspecs in particular.
>
> A subsequent commit will update the git-remote documentation to refer
> to this section, and details the motivation for writing this in the
> first place.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
> diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
> @@ -99,6 +99,55 @@ The latter use of the `remote..fetch` values 
> can be
> +PRUNING
> +---
> +
> +Git has a default disposition to keeping data unless it's explicitly

s/keeping/keep/ or s/to keeping/of keeping/

> +thrown away, this extends to keeping a hold of local references to

s/away,/away;/
s/to keeping a hold of/holding onto/

> +branches on remotes that have themselves deleted those branches.
> +
> +If left to accumulate these stale references might make performance

s/accumulate/&,/

> +worse on big and busy repos that have a lot of branch churn, and
> +e.g. make the output of commands like `git branch -a --contains
> +` needlessly verbose, as well as impacting anything else
> +that'll work with the complete set of known references.
> +
> +These remote tracking references can be deleted as a one-off with
> +either of:
> +
> +
> +# While fetching
> +$ git fetch --prune 
> +
> +# Only prune, don't fetch
> +$ git remote 
> +

Did you mean "git remote prune "?

> +To prune references on a remote as part of your normal workflow

This reads as if it's possible to prune something on the remote
machine itself. Maybe just say:

To prune references as part of your normal workflow...

> +without needing to remember to run that set `fetch.prune` globally, or
> +`remote..prune` per-remote in the config. See
> +linkgit:git-config[1].

Re: [PATCH 3/3] read-cache: don't write index twice if we can't write shared index

[Duy Nguyen]

On Fri, Jan 19, 2018 at 5:32 AM, SZEDER Gábor  wrote:
> On Thu, Jan 18, 2018 at 10:37 PM, Jeff King  wrote:
>> On Thu, Jan 18, 2018 at 10:00:14PM +0700, Duy Nguyen wrote:
>>
>>> The test suite was run as root, no wonder why my removing write access
>>> has no effect. I got the test to pass with this, but then it fails
>>> with
>>>
>>> Can't write .prove (Permission denied) at 
>>> /usr/share/perl/5.22/App/Prove.pm line 542.
>>>
>>> Some more chown'ing or chmod'ing is required
>
> This is the fallout of running the tests as root in the past.  With your
> patch 'prove' is run as a non-root user, but the prove state is loaded
> from Travis CI's cache, where it has been written as root the last time
> around, so now we don't have permissions to (over)write it.
>
> I have patches in the works to address this as well.

Great. I'll leave it to you then. I will update the test with SANITY
(and a small fix in the test name) and think more about the "-i" Jeff
mentioned.
-- 
Duy

[PATCH v2 0/2] Cookie redaction during GIT_TRACE_CURL

[Jonathan Tan]

Thanks, Eric. Changes in v2:
 - documented all environment variables introduced
 - made test more clear by ensuring that no cookie keys are suffixes or
   prefixes of others
 - tested empty value

As far as I can tell, it does not seem possible that Git generates a
cookie with no equals sign (like "Secure" or "HttpOnly" described in RFC
6265). When I try to craft a cookie file containing that (using
"Set-Cookie: Foo=; HttpOnly", for example), the no-equals-sign cookie
just disappears.

Jonathan Tan (2):
  http: support cookie redaction when tracing
  http: support omitting data from traces

 Documentation/git.txt   | 10 ++
 http.c  | 82 -
 t/t5551-http-fetch-smart.sh | 33 ++
 3 files changed, 117 insertions(+), 8 deletions(-)

-- 
2.16.0.rc2.37.ge0d575025.dirty

[PATCH v2 2/2] http: support omitting data from traces

[Jonathan Tan]

GIT_TRACE_CURL provides a way to debug what is being sent and received
over HTTP, with automatic redaction of sensitive information. But it
also logs data transmissions, which significantly increases the log file
size, sometimes unnecessarily. Add an option "GIT_TRACE_CURL_NO_DATA" to
allow the user to omit such data transmissions.

Signed-off-by: Jonathan Tan 
---
 Documentation/git.txt   |  4 
 http.c  | 27 +++
 t/t5551-http-fetch-smart.sh | 12 
 3 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 5446d2143..8163b5796 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -646,6 +646,10 @@ of clones and fetches.
variable.
See `GIT_TRACE` for available trace output options.
 
+`GIT_TRACE_CURL_NO_DATA`::
+   When a curl trace is enabled (see `GIT_TRACE_CURL` above), do not dump
+   data (that is, only dump info lines and headers).
+
 `GIT_REDACT_COOKIES`::
This can be set to a comma-separated list of strings. When a curl trace
is enabled (see `GIT_TRACE_CURL` above), whenever a "Cookies:" header
diff --git a/http.c b/http.c
index 088cf70bf..32a823895 100644
--- a/http.c
+++ b/http.c
@@ -16,6 +16,7 @@
 #include "string-list.h"
 
 static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
+static int trace_curl_data = 1;
 static struct string_list cookies_to_redact = STRING_LIST_INIT_DUP;
 #if LIBCURL_VERSION_NUM >= 0x070a08
 long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
@@ -695,24 +696,32 @@ static int curl_trace(CURL *handle, curl_infotype type, 
char *data, size_t size,
curl_dump_header(text, (unsigned char *)data, size, DO_FILTER);
break;
case CURLINFO_DATA_OUT:
-   text = "=> Send data";
-   curl_dump_data(text, (unsigned char *)data, size);
+   if (trace_curl_data) {
+   text = "=> Send data";
+   curl_dump_data(text, (unsigned char *)data, size);
+   }
break;
case CURLINFO_SSL_DATA_OUT:
-   text = "=> Send SSL data";
-   curl_dump_data(text, (unsigned char *)data, size);
+   if (trace_curl_data) {
+   text = "=> Send SSL data";
+   curl_dump_data(text, (unsigned char *)data, size);
+   }
break;
case CURLINFO_HEADER_IN:
text = "<= Recv header";
curl_dump_header(text, (unsigned char *)data, size, NO_FILTER);
break;
case CURLINFO_DATA_IN:
-   text = "<= Recv data";
-   curl_dump_data(text, (unsigned char *)data, size);
+   if (trace_curl_data) {
+   text = "<= Recv data";
+   curl_dump_data(text, (unsigned char *)data, size);
+   }
break;
case CURLINFO_SSL_DATA_IN:
-   text = "<= Recv SSL data";
-   curl_dump_data(text, (unsigned char *)data, size);
+   if (trace_curl_data) {
+   text = "<= Recv SSL data";
+   curl_dump_data(text, (unsigned char *)data, size);
+   }
break;
 
default:/* we ignore unknown types by default */
@@ -857,6 +866,8 @@ static CURL *get_curl_handle(void)
if (getenv("GIT_CURL_VERBOSE"))
curl_easy_setopt(result, CURLOPT_VERBOSE, 1L);
setup_curl_trace(result);
+   if (getenv("GIT_TRACE_CURL_NO_DATA"))
+   trace_curl_data = 0;
if (getenv("GIT_REDACT_COOKIES")) {
string_list_split(_to_redact,
  getenv("GIT_REDACT_COOKIES"), ',', -1);
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index 21a5ce860..f5721b4a5 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -385,5 +385,17 @@ test_expect_success 'GIT_REDACT_COOKIES handles empty 
values' '
grep "Cookie:.*Foo=" err
 '
 
+test_expect_success 'GIT_TRACE_CURL_NO_DATA prevents data from being traced' '
+   rm -rf clone &&
+   GIT_TRACE_CURL=true \
+   git clone $HTTPD_URL/smart/repo.git clone 2>err &&
+   grep "=> Send data" err &&
+
+   rm -rf clone &&
+   GIT_TRACE_CURL=true GIT_TRACE_CURL_NO_DATA=1 \
+   git clone $HTTPD_URL/smart/repo.git clone 2>err &&
+   ! grep "=> Send data" err
+'
+
 stop_httpd
 test_done
-- 
2.16.0.rc2.37.ge0d575025.dirty

[PATCH v2 1/2] http: support cookie redaction when tracing

[Jonathan Tan]

When using GIT_TRACE_CURL, Git already redacts the "Authorization:" and
"Proxy-Authorization:" HTTP headers. Extend this redaction to a
user-specified list of cookies, specified through the
"GIT_REDACT_COOKIES" environment variable.

Signed-off-by: Jonathan Tan 
---
 Documentation/git.txt   |  6 +
 http.c  | 55 +
 t/t5551-http-fetch-smart.sh | 21 +
 3 files changed, 82 insertions(+)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 3f4161a79..5446d2143 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -646,6 +646,12 @@ of clones and fetches.
variable.
See `GIT_TRACE` for available trace output options.
 
+`GIT_REDACT_COOKIES`::
+   This can be set to a comma-separated list of strings. When a curl trace
+   is enabled (see `GIT_TRACE_CURL` above), whenever a "Cookies:" header
+   sent by the client is dumped, values of cookies whose key is in that
+   list (case-sensitive) are redacted.
+
 `GIT_LITERAL_PATHSPECS`::
Setting this variable to `1` will cause Git to treat all
pathspecs literally, rather than as glob patterns. For example,
diff --git a/http.c b/http.c
index 597771271..088cf70bf 100644
--- a/http.c
+++ b/http.c
@@ -13,8 +13,10 @@
 #include "transport.h"
 #include "packfile.h"
 #include "protocol.h"
+#include "string-list.h"
 
 static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
+static struct string_list cookies_to_redact = STRING_LIST_INIT_DUP;
 #if LIBCURL_VERSION_NUM >= 0x070a08
 long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
 #else
@@ -575,6 +577,54 @@ static void redact_sensitive_header(struct strbuf *header)
/* Everything else is opaque and possibly sensitive */
strbuf_setlen(header,  sensitive_header - header->buf);
strbuf_addstr(header, " ");
+   } else if (cookies_to_redact.nr &&
+  skip_prefix(header->buf, "Cookie:", _header)) {
+   struct strbuf redacted_header = STRBUF_INIT;
+   char *cookie;
+
+   while (isspace(*sensitive_header))
+   sensitive_header++;
+
+   /*
+* The contents of header starting from sensitive_header will
+* subsequently be overridden, so it is fine to mutate this
+* string (hence the assignment to "char *").
+*/
+   cookie = (char *) sensitive_header;
+
+   while (cookie) {
+   char *equals;
+   char *semicolon = strstr(cookie, "; ");
+   if (semicolon)
+   *semicolon = 0;
+   equals = strchrnul(cookie, '=');
+   if (!equals) {
+   /* invalid cookie, just append and continue */
+   strbuf_addstr(_header, cookie);
+   continue;
+   }
+   *equals = 0; /* temporarily set to NUL for lookup */
+   if (string_list_lookup(_to_redact, cookie)) {
+   strbuf_addstr(_header, cookie);
+   strbuf_addstr(_header, "=");
+   } else {
+   *equals = '=';
+   strbuf_addstr(_header, cookie);
+   }
+   if (semicolon) {
+   /*
+* There are more cookies. (Or, for some
+* reason, the input string ends in "; ".)
+*/
+   strbuf_addstr(_header, "; ");
+   cookie = semicolon + strlen("; ");
+   } else {
+   cookie = NULL;
+   }
+   }
+
+   strbuf_setlen(header, sensitive_header - header->buf);
+   strbuf_addbuf(header, _header);
}
 }
 
@@ -807,6 +857,11 @@ static CURL *get_curl_handle(void)
if (getenv("GIT_CURL_VERBOSE"))
curl_easy_setopt(result, CURLOPT_VERBOSE, 1L);
setup_curl_trace(result);
+   if (getenv("GIT_REDACT_COOKIES")) {
+   string_list_split(_to_redact,
+ getenv("GIT_REDACT_COOKIES"), ',', -1);
+   string_list_sort(_to_redact);
+   }
 
curl_easy_setopt(result, CURLOPT_USERAGENT,
user_agent ? user_agent : git_user_agent());
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index a51b7e20d..21a5ce860 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -364,5 +364,26 @@ test_expect_success 'custom http headers' '
submodule update sub
 '
 

Re: [PATCH v2] diff: add --compact-summary option to complement --stat

[Duy Nguyen]

On Fri, Jan 19, 2018 at 5:48 AM, Jeff King  wrote:
> On Thu, Jan 18, 2018 at 05:05:46PM +0700, Nguyễn Thái Ngọc Duy wrote:
>
>> This is partly inspired by gerrit web interface which shows diffstat
>> like this, e.g. with commit 0433d533f1 (notice the "A" column on the
>> third line):
>>
>>  Documentation/merge-config.txt |  4 +
>>  builtin/merge.c|  2 +
>>A t/t5573-pull-verify-signatures.sh  | 81 ++
>>  t/t7612-merge-verify-signatures.sh | 45 ++
>>4 files changed, 132 insertions(+)
>
> I like the general concept. Perusing "git log" output, though, it felt
> like the summary column was very close to the filenames. What do you
> think of putting it after the "|", where it is only close to a number?
>
> Something like the patch below (on top of yours, but it probably needs
> tweaked further for graph_width), which gives:
>
>t/t5573-pull-verify-signatures.sh | A+x  78 
>
> (I know this is a bikeshed, so I'm perfectly willing to take "yuck, I
> don't like that as well" as a response).

The position of A+x column is exactly where gerrit put it. Though web
pages have more flexibility than our terminal console so its position
does not have to be the same. I'm just throwing options out there

For many years I have this instead

 t/t5573-pull-verify-signatures.sh (new +x) | 81 

Another option is just wrap the code in [] to make it look like check
boxes. But that wastes two more columns

   builtin/merge.c|  2 +
 [A+x] t/t5573-pull-verify-signatures.sh  | 81 
   t/t7612-merge-verify-signatures.sh | 45 +

Back to your suggestion, I kinda like the closeness between the +/-
count and "|" though. The output on 10c78a162f is like this, which
makes "A" looks a bit umm.. disconnected from the path name?

  Documentation/RelNotes/2.14.0.txt | A  97 +++
  GIT-VERSION-GEN   | 2 +-
  RelNotes  | 2 +-

Another way is just separate the status code from file name

 A | Documentation/RelNotes/2.14.0.txt | 97 +++
   | GIT-VERSION-GEN   |  2 +-
   | RelNotes  |  2 +-

Last note. With colored diffstat, we should be able to use a separate
color (or something in the +/- part) to denote new/deleted files. I
didn't think about this...

>> The new option --compact-summary implements this with a tweak to support
>> mode change, which is shown in --summary too.
>
> One thing I noticed is that --compact-summary by itself does nothing.
> Should it imply --stat?

It might go with --numstat or --dirstat in future too. Didn't really
think hard about this yet. But I probably will go with Eric suggestion
and keep this in --stat= unless it really makes sense to have
something like this in --numstat or --dirstat.
-- 
Duy

Re: git 2.16.0 segfaults on clone of specific repo

[Eric Sunshine]

[+cc:brian]

On Thu, Jan 18, 2018 at 3:55 PM, Александр Булаев
 wrote:
> I found that git 2.16.0 segfaults on clone of vim-colorschemes repo.
>
> (lldb) run
> Process 25643 launched: '/usr/local/bin/git' (x86_64)
> Cloning into 'vim-colorschemes'...
> remote: Counting objects: 1457, done.
> remote: Total 1457 (delta 0), reused 0 (delta 0), pack-reused 1457
> Receiving objects: 100% (1457/1457), 1.43 MiB | 289.00 KiB/s, done.
> Resolving deltas: 100% (424/424), done.
> Process 25643 stopped
> * thread #1, queue = 'com.apple.main-thread', stop reason =
> EXC_BAD_ACCESS (code=1, address=0x48)

I can confirm that this crashes on MacOS; it does not crash on Linux or BSD.

git-bisect places blame on eb0ccfd7f5 (Switch empty tree and blob
lookups to use hash abstraction, 2017-11-12).

Re: [RFC PATCH] NonStop port changes for git 2.16.0.

[Ævar Arnfjörð Bjarmason]

On Thu, Jan 18 2018, Stefan Beller jotted:

> On Thu, Jan 18, 2018 at 2:42 PM,   wrote:
>> Further: there are 6 known breakages that have been reported. The perl
>> issues relating to completion codes are being examined at present by the
>> platform support teams so are not addressed by this patch.
>
> For perl I'd suggest cc'ing Ævar (cc'd just now) as that seems his
> field of expertise.

Just to make sure I'm keeping up, this refers to some breakage not
detailed in the patch above, right?

The only perl-related change I see is undoing part of 6c109904bc ("Port
to HP NonStop", 2012-09-19) having to do with how we find perl/python
(which, not being at all familiar with NonStop, makes sense to me).

But sure, if there's some details about those 6 issues I might have time
to take a look, but it sounds like it's being looked at by NonStop
support...

[ANNOUNCE] Git Merge Contributor's Summit Mar 7, 2018, Barcelona

[Jeff King]

Git Merge 2018 is happening on March 8th; there will be a Contributor's
Summit the day before. Here are the details:

  When: Wednesday, March 7, 2018. 10am-5pm.
  Where: Convent Dels Àngels[1], Barcelona, Spain
  What: Round-table discussion about Git
  Who: All contributors to Git or related projects in the Git ecosystem
   are invited; if you're not sure if you qualify, just ask!

In order to attend, you'll need to register ahead of time. There's a
super-secret link to do so; email me and I will provide it.
Registration is free, and comes with a ticket to the main conference on
the 8th (which I encourage you to attend, but you don't have to).

As with past years, the agenda is whatever we choose. We'll have room
for about 25 people with "boardroom-style seating" and a projector.
Come prepared with topics to present or discuss.

If you're interested in financial aid for traveling to the conference,
please send an email to g...@sfconservancy.org.  And please do so soon
(let's say by the end of next week, Jan 26th), so that we have an idea
of the number and size of requests before making any grants.

-Peff

[1] 
https://www.google.com/maps/place/Convent+Dels+Angels/@41.3827189,2.1652982,17z/data=!3m1!4b1!4m5!3m4!1s0x12a4a310123f3dc1:0x4588a81b66dce9dc!8m2!3d41.3827189!4d2.1674869

Re: [PATCH v2] diff: add --compact-summary option to complement --stat

[Duy Nguyen]

On Fri, Jan 19, 2018 at 4:23 AM, Ævar Arnfjörð Bjarmason
 wrote:
> Wait, isn't there a bug here in the existing --summary code, its
> documentation says it'll show information "such as creations, renames
> and mode changes".
>
> But even though your --compact-summary shows that the file is being
> added and its mode changed:
>
> $ diff -ru <(./git-show --stat 0433d533f1) <(./git-show --stat 
> --compact-summary 0433d533f1)
> --- /dev/fd/63  2018-01-18 21:11:51.186570555 +
> +++ /dev/fd/62  2018-01-18 21:11:51.186570555 +
> @@ -14,8 +14,8 @@
>t: add tests for pull --verify-signatures
>merge: add config option for verifySignatures
>
> - Documentation/merge-config.txt |  4 ++
> - builtin/merge.c|  2 +
> - t/t5573-pull-verify-signatures.sh  | 81 
> ++
> - t/t7612-merge-verify-signatures.sh | 45 +
> + Documentation/merge-config.txt |  4 ++
> + builtin/merge.c|  2 +
> + A+x t/t5573-pull-verify-signatures.sh  | 81 
> ++
> + t/t7612-merge-verify-signatures.sh | 45 +++
>   4 files changed, 132 insertions(+)
>
> There is no difference between --stat with and without --summary on the
> same commit, shouldn't it show "create mode [...]" ?
>
> E.g. 95450bbbaa will do the trick for both:
>
> $ diff -ru <(./git-show --stat 95450bbbaa) <(./git-show --stat --summary 
> 95450bbbaa)
> --- /dev/fd/63  2018-01-18 21:14:20.770050599 +
> +++ /dev/fd/62  2018-01-18 21:14:20.770050599 +
> @@ -14,3 +14,4 @@
>   git-svn.perl|  1 +
>   t/t9169-git-svn-dcommit-crlf.sh | 27 +++
>   2 files changed, 28 insertions(+)
> + create mode 100755 t/t9169-git-svn-dcommit-crlf.sh
>
> $ diff -ru <(./git-show --stat --summary 95450bbbaa) <(./git-show --stat 
> --compact-summary 95450bbbaa)
> --- /dev/fd/63  2018-01-18 21:14:30.646016210 +
> +++ /dev/fd/62  2018-01-18 21:14:30.646016210 +
> @@ -11,7 +11,6 @@
>  Reported-by: Brian Bennett 
>  Signed-off-by: Eric Wong 
>
> - git-svn.perl|  1 +
> - t/t9169-git-svn-dcommit-crlf.sh | 27 +++
> + git-svn.perl|  1 +
> + A+x t/t9169-git-svn-dcommit-crlf.sh | 27 +++
>   2 files changed, 28 insertions(+)
> - create mode 100755 t/t9169-git-svn-dcommit-crlf.sh

Interesting. 0433d533f1 is a merge commit, perhaps that has something
to do with this. Adding --first-parent does show "create mode" line.
I'll check this later.
-- 
Duy

[PATCH 02/11] fetch tests: arrange arguments for future readability

[Ævar Arnfjörð Bjarmason]

Re-arrange the arguments to the test_configured_prune() function used
in this test to pass the arguments to --fetch last. A subsequent
change will test for more elaborate fetch arguments, including long
refspecs. It'll be more readable to be able to wrap those on a new
line of their own.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t5510-fetch.sh | 82 ++--
 1 file changed, 44 insertions(+), 38 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 11da97f9b7..ab8b25344d 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -551,10 +551,10 @@ set_config_tristate () {
 test_configured_prune () {
fetch_prune=$1
remote_origin_prune=$2
-   cmdline=$3
-   expected_branch=$4
+   expected_branch=$3
+   cmdline=$4
 
-   test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${3:+ 
$3}; branch:$4" '
+   test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${4:+ 
$4}; branch:$3" '
# make sure a newbranch is there in . and also in one
git branch -f newbranch &&
(
@@ -587,41 +587,47 @@ test_configured_prune () {
'
 }
 
-test_configured_prune unset unset ""   kept
-test_configured_prune unset unset "--no-prune" kept
-test_configured_prune unset unset "--prune"pruned
-
-test_configured_prune false unset ""   kept
-test_configured_prune false unset "--no-prune" kept
-test_configured_prune false unset "--prune"pruned
-
-test_configured_prune true  unset ""   pruned
-test_configured_prune true  unset "--prune"pruned
-test_configured_prune true  unset "--no-prune" kept
-
-test_configured_prune unset false ""   kept
-test_configured_prune unset false "--no-prune" kept
-test_configured_prune unset false "--prune"pruned
-
-test_configured_prune false false ""   kept
-test_configured_prune false false "--no-prune" kept
-test_configured_prune false false "--prune"pruned
-
-test_configured_prune true  false ""   kept
-test_configured_prune true  false "--prune"pruned
-test_configured_prune true  false "--no-prune" kept
-
-test_configured_prune unset true  ""   pruned
-test_configured_prune unset true  "--no-prune" kept
-test_configured_prune unset true  "--prune"pruned
-
-test_configured_prune false true  ""   pruned
-test_configured_prune false true  "--no-prune" kept
-test_configured_prune false true  "--prune"pruned
-
-test_configured_prune true  true  ""   pruned
-test_configured_prune true  true  "--prune"pruned
-test_configured_prune true  true  "--no-prune" kept
+# $1 config: fetch.prune
+# $2 config: remote..prune
+# $3 expect: branch to be pruned?
+# $4 git-fetch $cmdline:
+#
+# $1$2$3 $4
+test_configured_prune unset unset kept   ""
+test_configured_prune unset unset kept   "--no-prune"
+test_configured_prune unset unset pruned "--prune"
+
+test_configured_prune false unset kept   ""
+test_configured_prune false unset kept   "--no-prune"
+test_configured_prune false unset pruned "--prune"
+
+test_configured_prune true  unset pruned ""
+test_configured_prune true  unset pruned "--prune"
+test_configured_prune true  unset kept   "--no-prune"
+
+test_configured_prune unset false kept   ""
+test_configured_prune unset false kept   "--no-prune"
+test_configured_prune unset false pruned "--prune"
+
+test_configured_prune false false kept   ""
+test_configured_prune false false kept   "--no-prune"
+test_configured_prune false false pruned "--prune"
+
+test_configured_prune true  false kept   ""
+test_configured_prune true  false pruned "--prune"
+test_configured_prune true  false kept   "--no-prune"
+
+test_configured_prune unset true  pruned ""
+test_configured_prune unset true  kept   "--no-prune"
+test_configured_prune unset true  pruned "--prune"
+
+test_configured_prune false true  pruned ""
+test_configured_prune false true  kept   "--no-prune"
+test_configured_prune false true  pruned "--prune"
+
+test_configured_prune true  true  pruned ""
+test_configured_prune true  true  pruned "--prune"
+test_configured_prune true  true  kept   "--no-prune"
 
 test_expect_success 'all boundary commits are excluded' '
test_commit base &&
-- 
2.15.1.424.g9478a66081

[PATCH 06/11] git fetch doc: add a new section to explain the ins & outs of pruning

[Ævar Arnfjörð Bjarmason]

Add a new section to canonically explain how remote reference pruning
works, and how users should be careful about using it in conjunction
with tag refspecs in particular.

A subsequent commit will update the git-remote documentation to refer
to this section, and details the motivation for writing this in the
first place.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/git-fetch.txt | 49 +
 1 file changed, 49 insertions(+)

diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index b153aefa68..b07b669a1f 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -99,6 +99,55 @@ The latter use of the `remote..fetch` values can 
be
 overridden by giving the `--refmap=` parameter(s) on the
 command line.
 
+PRUNING
+---
+
+Git has a default disposition to keeping data unless it's explicitly
+thrown away, this extends to keeping a hold of local references to
+branches on remotes that have themselves deleted those branches.
+
+If left to accumulate these stale references might make performance
+worse on big and busy repos that have a lot of branch churn, and
+e.g. make the output of commands like `git branch -a --contains
+` needlessly verbose, as well as impacting anything else
+that'll work with the complete set of known references.
+
+These remote tracking references can be deleted as a one-off with
+either of:
+
+
+# While fetching
+$ git fetch --prune 
+
+# Only prune, don't fetch
+$ git remote 
+
+
+To prune references on a remote as part of your normal workflow
+without needing to remember to run that set `fetch.prune` globally, or
+`remote..prune` per-remote in the config. See
+linkgit:git-config[1].
+
+Here's where things get tricky and more specific. The pruning feature
+doesn't actually care about branches, instead it'll prune local <->
+remote references as a function of the refspec of the remote (see
+`` and <> above).
+
+Therefore if the refspec for the remote includes
+e.g. `refs/tags/*:refs/tags/*`, or you manually run e.g. `git fetch
+--prune  "refs/tags/*:refs/tags/*"` it won't be stale remote
+tracking branches that are deleted, but any local tag that doesn't
+exist on the remote.
+
+This might not be what you expect, i.e. you want to prune remote
+``, but also explicitly fetch tags from it, so when you fetch
+from it you delete all your local tags, most of which may not have
+come from the `` remote in the first place.
+
+So be careful when using this with a refspec like
+`refs/tags/*:refs/tags/*`, or any other refspec which might map
+references from multiple remotes to the same local namespace.
+
 OUTPUT
 --
 
-- 
2.15.1.424.g9478a66081

[RFC/PATCH 11/11] WIP fetch: add a --fetch-prune option and fetch.pruneTags config

[Ævar Arnfjörð Bjarmason]

[WIP: This doesn't (yet) work as advertised, see further WIP note
below]

Add a --fetch-prune option to git-fetch along with fetch.pruneTags
config option. This allows for doing:

git fetch origin -p -P

Or simply:

git config fetch.prune true &&
git config fetch.pruneTags true &&
git fetch origin

Instead of the much more verbose:

git fetch --prune origin 'refs/tags/*:refs/tags/*' 
'+refs/heads/*:refs/remotes/origin/*'

A use-case of git which is a pain to support now is frequent pulling
from a repo which is having both its branches *and* tags deleted
regularly. At work we create deployment tags in the repo for each
rollout, and there's *lots* of those, so they're archived within weeks
for performance reasons.

Without this change it's hard to centrally configure such repos in
/etc/gitconfig (on servers that are only used for working with
them). You need to set fetch.prune=true globally, and then for each
repo:

git -C {} config --replace-all remote.origin.fetch 
"refs/tags/*:refs/tags/*" "^refs/tags/*:refs/tags/*$"

Now I can simply set fetch.pruneTags=true in /etc/gitconfig as well,
and users running "git pull" will automatically get the pruning
semantics we want.

See my "Re: [BUG] git remote prune removes local tags, depending on
fetch config" (87po6ahx87@evledraar.gmail.com;
https://public-inbox.org/git/87po6ahx87@evledraar.gmail.com/) for
more details.

[WIP: This doesn't actually work, since the implementation is to just
pretend that 'refs/tags/*:refs/tags/*' was supplied on the
command-line, we'll consequently stop fetching any branches, since we
don't use the default refspec found in .git/config anymore

I'm not familiar enough with this code and the refspec machinery to
know if this argument injection is crazy, and how to tell it to *also*
do whatever it's doing by default]

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/config.txt| 15 +++
 Documentation/fetch-options.txt | 15 ++-
 builtin/fetch.c | 37 ++---
 remote.c|  2 ++
 remote.h|  1 +
 t/t5510-fetch.sh|  1 +
 6 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0f27af5760..476175ef8d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1401,6 +1401,14 @@ fetch.prune::
option was given on the command line.  See also `remote..prune`
and the PRUNING section of linkgit:git-fetch[1].
 
+fetch.pruneTags::
+   If true, fetch will automatically behave as if the
+   `refs/tags/*:refs/tags/*` refspec was provided when pruning,
+   if not set already. This allows for setting both this option
+   and `fetch.prune` to maintain a 1=1 mapping to upstrem
+   refs. See also `remote..pruneTags` and the PRUNING
+   section of linkgit:git-fetch[1].
+
 fetch.output::
Control how ref update status is printed. Valid values are
`full` and `compact`. Default value is `full`. See section
@@ -2945,6 +2953,13 @@ remote..prune::
remove any remote-tracking references that no longer exist on the
remote (as if the `--prune` option was given on the command line).
Overrides `fetch.prune` settings, if any.
+
+remote..pruneTags::
+   When set to true, fetching from this remote by default will also
+   remove any local tags that no longer exist on the remote (as
+   if the `--prune` option was given on the command line in
+   conjunction with the `refs/tags/*:refs/tags/*` refspec).
+   Overrides `fetch.prune` settings, if any.
 +
 See also `remote..prune` and the PRUNING section of
 linkgit:git-fetch[1].
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 9f5c85ad96..dc13bed741 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -73,7 +73,20 @@ ifndef::git-pull[]
are fetched due to an explicit refspec (either on the command
line or in the remote configuration, for example if the remote
was cloned with the --mirror option), then they are also
-   subject to pruning.
+   subject to pruning. Supplying `--prune-tags` is a shorthand for
+   providing the tag refspec.
++
+See the PRUNING section below for more details.
+
+-P::
+--prune-tags::
+   Before fetching, remove any local tags that no longer exist on
+   the remote if `--prune` is enabled. This option should be used
+   more carefully, unlike `--prune` it will remove any local
+   references (local tags) that have been created. This option is
+   merely a shorthand for providing the explicit tag refspec
+   along with `--prune`, see the discussion about that in its
+   documentation.
 +
 See the PRUNING section below for more details.
 
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 

[PATCH 00/11] document & test fetch pruning + WIP fetch.pruneTags

[Ævar Arnfjörð Bjarmason]

Michael Giuffrida noted that the git-remote docs were very confusing,
and upthread I said I wanted this shiny related thing in 11/11.

Along the way I fixed up fetch tests & documentation to hopefully be a
lot less confusing.

I think 1-10/11 of this makes sense for inclusion as-is (pending
review etc.), 11/11 is broken currently, but review / comments on it
welcome, particularly the CLI / config interface / docs etc.

The bug causing it not to work is less of a "I can't figure this out"
and more of a "I won't have time again for hacking in the next couple
of days, and wanted to see what people thought", but if someone wants
to see what I'm screwing up there and do my homework for me that's
also most welcome.

Ævar Arnfjörð Bjarmason (11):
  fetch tests: refactor in preparation for testing tag pruning
  fetch tests: arrange arguments for future readability
  fetch tests: add a tag to be deleted to the pruning tests
  fetch tests: double quote a variable for interpolation
  fetch tests: test --prune and refspec interaction
  git fetch doc: add a new section to explain the ins & outs of pruning
  git remote doc: correct dangerous lies about what prune does
  git-fetch & config doc: link to the new PRUNING section
  fetch: don't redundantly null something calloc() gave us
  fetch tests: add scaffolding for the new fetch.pruneTags
  WIP fetch: add a --fetch-prune option and fetch.pruneTags config

 Documentation/config.txt|  21 ++-
 Documentation/fetch-options.txt |  18 +-
 Documentation/git-fetch.txt |  49 
 Documentation/git-remote.txt|  14 +++--
 builtin/fetch.c |  38 ++--
 remote.c|   2 +
 remote.h|   1 +
 t/t5510-fetch.sh| 125 +++-
 8 files changed, 216 insertions(+), 52 deletions(-)

-- 
2.15.1.424.g9478a66081

[PATCH 01/11] fetch tests: refactor in preparation for testing tag pruning

[Ævar Arnfjörð Bjarmason]

In a subsequent commit this function will learn to test for tag
pruning, prepare for that by making space for more variables, and
making it clear that "expected" here refers to branches.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t5510-fetch.sh | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 668c54be41..11da97f9b7 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -549,9 +549,12 @@ set_config_tristate () {
 }
 
 test_configured_prune () {
-   fetch_prune=$1 remote_origin_prune=$2 cmdline=$3 expected=$4
+   fetch_prune=$1
+   remote_origin_prune=$2
+   cmdline=$3
+   expected_branch=$4
 
-   test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${3:+ 
$3}; $4" '
+   test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${3:+ 
$3}; branch:$4" '
# make sure a newbranch is there in . and also in one
git branch -f newbranch &&
(
@@ -572,7 +575,7 @@ test_configured_prune () {
set_config_tristate remote.origin.prune 
$remote_origin_prune &&
 
git fetch $cmdline &&
-   case "$expected" in
+   case "$expected_branch" in
pruned)
test_must_fail git rev-parse --verify 
refs/remotes/origin/newbranch
;;
-- 
2.15.1.424.g9478a66081

[PATCH 08/11] git-fetch & config doc: link to the new PRUNING section

[Ævar Arnfjörð Bjarmason]

Amend the documentation for fetch.prune, fetch..prune and
--prune to link to the recently added PRUNING section.

I'd have liked to link directly to it with "<>" from
fetch-options.txt, since it's included in git-fetch.txt (git-pull.txt
also includes it, but doesn't include that option). However making a
reference across files yields this error:

[...]/Documentation/git-fetch.xml:226: element xref: validity
error : IDREF attribute linkend references an unknown ID "PRUNING"

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/config.txt| 6 +-
 Documentation/fetch-options.txt | 3 +++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0e25b2c92b..0f27af5760 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1398,7 +1398,8 @@ fetch.unpackLimit::
 
 fetch.prune::
If true, fetch will automatically behave as if the `--prune`
-   option was given on the command line.  See also `remote..prune`.
+   option was given on the command line.  See also `remote..prune`
+   and the PRUNING section of linkgit:git-fetch[1].
 
 fetch.output::
Control how ref update status is printed. Valid values are
@@ -2944,6 +2945,9 @@ remote..prune::
remove any remote-tracking references that no longer exist on the
remote (as if the `--prune` option was given on the command line).
Overrides `fetch.prune` settings, if any.
++
+See also `remote..prune` and the PRUNING section of
+linkgit:git-fetch[1].
 
 remotes.::
The list of remotes which are fetched by "git remote update
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index fb6bebbc61..9f5c85ad96 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -74,6 +74,9 @@ ifndef::git-pull[]
line or in the remote configuration, for example if the remote
was cloned with the --mirror option), then they are also
subject to pruning.
++
+See the PRUNING section below for more details.
+
 endif::git-pull[]
 
 ifndef::git-pull[]
-- 
2.15.1.424.g9478a66081

Re: [PATCH v2] diff: add --compact-summary option to complement --stat

[Duy Nguyen]

On Fri, Jan 19, 2018 at 1:57 AM, Eric Sunshine  wrote:
>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
>> @@ -188,6 +188,17 @@ and accumulating child directory counts in the parent 
>> directories:
>> +--compact-summary::
>> +   Output a condensed summary of extended header information in
>> +   front of the file name part of diffstat. This option is
>> +   ignored if --stat is not specified.
>
> Rather than ignoring this option if --stat is not specified, a
> different approach would be for --compact-summary to imply --stat.
>
> Also, per documentation:
>
> --stat[=[,[,]]]::
>
> These parameters can also be set individually with `--stat-width=`,
> `--stat-name-width=` and `--stat-count=`.
>
> One wonders if "compact" could be another modifier recognized by --stat.
>
> (Genuine questions/observations; I haven't formed strong opinions either way.)

I left open an option to combine this with other --*stat like numstat
(or unlikely, dirstat). I haven't really thought about this. Yeah
perhaps putting this in --stat would be a better move.
--
Duy

[PATCH 07/11] git remote doc: correct dangerous lies about what prune does

[Ævar Arnfjörð Bjarmason]

The "git remote prune " command uses the same machinery as "git
fetch  --prune", and shares all the same caveats, but its
documentation has suggested that it'll just "delete stale
remote-tracking branches under ".

This isn't true, and hasn't been true since at least v1.8.5.6 (the
oldest version I could be bothered to test).

E.g. if "+refs/tags/*:refs/tags/*" is explicitly set in the refspec of
the remote it'll delete all local tags  doesn't know about.

Instead, briefly give the reader just enough of a hint that this
option might constitute a shotgun aimed at their foot, and point them
to the new PRUNING section in the git-fetch documentation which
explains all the nuances of what this facility does.

See "[BUG] git remote prune removes local tags, depending on fetch
config" (caci5s_39wnrbfjlfn0xhcy+uewtfn2ymnacrc86z6pjutjw...@mail.gmail.com)
by Michael Giuffrida for the initial report.

Reported-by: Michael Giuffrida 
Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/git-remote.txt | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 577b969c1b..7183a72a23 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -172,10 +172,14 @@ With `-n` option, the remote heads are not queried first 
with
 
 'prune'::
 
-Deletes all stale remote-tracking branches under .
-These stale branches have already been removed from the remote repository
-referenced by , but are still locally available in
-"remotes/".
+Deletes stale references associated with . By default stale
+remote-tracking branches under , but depending on global
+configuration and the configuration of the remote we might even prune
+local tags. Equivalent to `git fetch  --prune`, except that no
+new references will be fetched.
++
+See the PRUNING section of linkgit:git-fetch[1] for what it'll prune
+depending on various configuration.
 +
 With `--dry-run` option, report what branches will be pruned, but do not
 actually prune them.
@@ -189,7 +193,7 @@ remotes.default is not defined, all remotes which do not 
have the
 configuration parameter remote..skipDefaultUpdate set to true will
 be updated.  (See linkgit:git-config[1]).
 +
-With `--prune` option, prune all the remotes that are updated.
+With `--prune` option, run pruning against all the remotes that are updated.
 
 
 DISCUSSION
-- 
2.15.1.424.g9478a66081

[PATCH 05/11] fetch tests: test --prune and refspec interaction

[Ævar Arnfjörð Bjarmason]

Add a test for the interaction between explicitly provided refspecs
and fetch.prune.

There's no point in adding this boilerplate to every combination of
unset/false/true, it's instructive and sufficient to show that no
matter if the variable is unset, false or true the refspec on the
command-line overrides any configuration variable.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t5510-fetch.sh | 12 
 1 file changed, 12 insertions(+)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 542eb53a99..576c2598c9 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -609,6 +609,10 @@ test_configured_prune () {
 test_configured_prune unset unset kept   kept   ""
 test_configured_prune unset unset kept   kept   "--no-prune"
 test_configured_prune unset unset pruned kept   "--prune"
+test_configured_prune unset unset kept   pruned \
+   "--prune origin 'refs/tags/*:refs/tags/*'"
+test_configured_prune unset unset pruned pruned \
+   "--prune origin 'refs/tags/*:refs/tags/*' 
'+refs/heads/*:refs/remotes/origin/*'"
 
 test_configured_prune false unset kept   kept   ""
 test_configured_prune false unset kept   kept   "--no-prune"
@@ -625,6 +629,10 @@ test_configured_prune unset false pruned kept   "--prune"
 test_configured_prune false false kept   kept   ""
 test_configured_prune false false kept   kept   "--no-prune"
 test_configured_prune false false pruned kept   "--prune"
+test_configured_prune false false kept   pruned \
+   "--prune origin 'refs/tags/*:refs/tags/*'"
+test_configured_prune false false pruned pruned \
+   "--prune origin 'refs/tags/*:refs/tags/*' 
'+refs/heads/*:refs/remotes/origin/*'"
 
 test_configured_prune true  false kept   kept   ""
 test_configured_prune true  false pruned kept   "--prune"
@@ -641,6 +649,10 @@ test_configured_prune false true  pruned kept   "--prune"
 test_configured_prune true  true  pruned kept   ""
 test_configured_prune true  true  pruned kept   "--prune"
 test_configured_prune true  true  kept   kept   "--no-prune"
+test_configured_prune true  true  kept   pruned \
+   "--prune origin 'refs/tags/*:refs/tags/*'"
+test_configured_prune true  true  pruned pruned \
+   "--prune origin 'refs/tags/*:refs/tags/*' 
'+refs/heads/*:refs/remotes/origin/*'"
 
 test_expect_success 'all boundary commits are excluded' '
test_commit base &&
-- 
2.15.1.424.g9478a66081

[PATCH 09/11] fetch: don't redundantly null something calloc() gave us

[Ævar Arnfjörð Bjarmason]

Stop redundantly NULL-ing the last element of the refs structure,
which was retrieved via calloc() and is thus guaranteed to be
pre-NULL'd.

This code dates back to b888d61c83 ("Make fetch a builtin",
2007-09-10), where wasn't any reason to do this back then either, it's
just something left over from when git-fetch was initially introduced.

The initial motivation for this change was to make a subsequent change
which'll also modify the refs variable smaller, since it won't have to
copy this redundant "NULL the last + 1 item" pattern.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 builtin/fetch.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7bbcd26faf..b34665db9e 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1302,7 +1302,6 @@ static int fetch_one(struct remote *remote, int argc, 
const char **argv)
} else
refs[j++] = argv[i];
}
-   refs[j] = NULL;
ref_nr = j;
}
 
-- 
2.15.1.424.g9478a66081

[PATCH 04/11] fetch tests: double quote a variable for interpolation

[Ævar Arnfjörð Bjarmason]

If the $cmdline variable contains multiple arguments they won't be
interpolated correctly since the body of the test is single quoted. I
don't know what part of test-lib.sh is expanding variables within
single-quoted strings, but interpolating this inline is the desired
behavior here.

This will be used in a subsequent commit to pass more than one
variable to git-fetch.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t5510-fetch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index fad65bd885..542eb53a99 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -578,7 +578,7 @@ test_configured_prune () {
set_config_tristate fetch.prune $fetch_prune &&
set_config_tristate remote.origin.prune 
$remote_origin_prune &&
 
-   git fetch $cmdline &&
+   git fetch '"$cmdline"' &&
case "$expected_branch" in
pruned)
test_must_fail git rev-parse --verify 
refs/remotes/origin/newbranch
-- 
2.15.1.424.g9478a66081

[PATCH 10/11] fetch tests: add scaffolding for the new fetch.pruneTags

[Ævar Arnfjörð Bjarmason]

The fetch.pruneTags configuration doesn't exist yet, but will be added
in a subsequent commit. Since testing for it requires adding new
parameters to the test_configured_prune function it's easier to review
this patch first to assert that no functional changes are introduced
yet.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t5510-fetch.sh | 90 ++--
 1 file changed, 49 insertions(+), 41 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 576c2598c9..18280df4fc 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -551,18 +551,22 @@ set_config_tristate () {
 test_configured_prune () {
fetch_prune=$1
remote_origin_prune=$2
-   expected_branch=$3
-   expected_tag=$4
-   cmdline=$5
+   fetch_prune_tags=$3
+   remote_origin_prune_tags=$4
+   expected_branch=$5
+   expected_tag=$6
+   cmdline=$7
 
-   test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${5:+ 
$5}; branch:$3 tag:$4" '
+   test_expect_success "prune fetch.prune=$1 fetch.pruneTags=$2 
remote.origin.prune=$3 remote.origin.prune=$4${7:+ $7}; branch:$5 tag:$6" '
# make sure a newbranch is there in . and also in one
git branch -f newbranch &&
git tag -f newtag &&
(
cd one &&
test_unconfig fetch.prune &&
+   test_unconfig fetch.pruneTags &&
test_unconfig remote.origin.prune &&
+   test_unconfig remote.origin.pruneTags &&
git fetch &&
git rev-parse --verify refs/remotes/origin/newbranch &&
git rev-parse --verify refs/tags/newtag
@@ -576,7 +580,9 @@ test_configured_prune () {
(
cd one &&
set_config_tristate fetch.prune $fetch_prune &&
+   set_config_tristate fetch.pruneTags $fetch_prune_tags &&
set_config_tristate remote.origin.prune 
$remote_origin_prune &&
+   set_config_tristate remote.origin.pruneTags 
$remote_origin_prune_tags &&
 
git fetch '"$cmdline"' &&
case "$expected_branch" in
@@ -601,57 +607,59 @@ test_configured_prune () {
 
 # $1 config: fetch.prune
 # $2 config: remote..prune
-# $3 expect: branch to be pruned?
-# $4 expect: tag to be pruned?
-# $5 git-fetch $cmdline:
+# $3 config: fetch.pruneTags
+# $4 config: remote..pruneTags
+# $5 expect: branch to be pruned?
+# $6 expect: tag to be pruned?
+# $7 git-fetch $cmdline:
 #
-# $1$2$3 $4 $5
-test_configured_prune unset unset kept   kept   ""
-test_configured_prune unset unset kept   kept   "--no-prune"
-test_configured_prune unset unset pruned kept   "--prune"
-test_configured_prune unset unset kept   pruned \
+# $1$2$3$4$5 $6 $7
+test_configured_prune unset unset unset unset kept   kept   ""
+test_configured_prune unset unset unset unset kept   kept   "--no-prune"
+test_configured_prune unset unset unset unset pruned kept   "--prune"
+test_configured_prune unset unset unset unset kept   pruned \
"--prune origin 'refs/tags/*:refs/tags/*'"
-test_configured_prune unset unset pruned pruned \
+test_configured_prune unset unset unset unset pruned pruned \
"--prune origin 'refs/tags/*:refs/tags/*' 
'+refs/heads/*:refs/remotes/origin/*'"
 
-test_configured_prune false unset kept   kept   ""
-test_configured_prune false unset kept   kept   "--no-prune"
-test_configured_prune false unset pruned kept   "--prune"
+test_configured_prune false unset unset unset kept   kept   ""
+test_configured_prune false unset unset unset kept   kept   "--no-prune"
+test_configured_prune false unset unset unset pruned kept   "--prune"
 
-test_configured_prune true  unset pruned kept   ""
-test_configured_prune true  unset pruned kept   "--prune"
-test_configured_prune true  unset kept   kept   "--no-prune"
+test_configured_prune true  unset unset unset pruned kept   ""
+test_configured_prune true  unset unset unset pruned kept   "--prune"
+test_configured_prune true  unset unset unset kept   kept   "--no-prune"
 
-test_configured_prune unset false kept   kept   ""
-test_configured_prune unset false kept   kept   "--no-prune"
-test_configured_prune unset false pruned kept   "--prune"
+test_configured_prune unset false unset unset kept   kept   ""
+test_configured_prune unset false unset unset kept   kept   "--no-prune"
+test_configured_prune unset false unset unset pruned kept   "--prune"
 
-test_configured_prune false false kept   kept   ""
-test_configured_prune false false kept   kept   "--no-prune"
-test_configured_prune false false pruned kept   "--prune"
-test_configured_prune false false kept   pruned \
+test_configured_prune false false unset 

[PATCH 03/11] fetch tests: add a tag to be deleted to the pruning tests

[Ævar Arnfjörð Bjarmason]

Add a tag to be deleted to the fetch --prune tests. The tag is always
kept for now, which is the expected behavior, but now I can add a test
for tag pruning in a later commit.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t5510-fetch.sh | 93 
 1 file changed, 53 insertions(+), 40 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index ab8b25344d..fad65bd885 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -552,21 +552,25 @@ test_configured_prune () {
fetch_prune=$1
remote_origin_prune=$2
expected_branch=$3
-   cmdline=$4
+   expected_tag=$4
+   cmdline=$5
 
-   test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${4:+ 
$4}; branch:$3" '
+   test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${5:+ 
$5}; branch:$3 tag:$4" '
# make sure a newbranch is there in . and also in one
git branch -f newbranch &&
+   git tag -f newtag &&
(
cd one &&
test_unconfig fetch.prune &&
test_unconfig remote.origin.prune &&
git fetch &&
-   git rev-parse --verify refs/remotes/origin/newbranch
+   git rev-parse --verify refs/remotes/origin/newbranch &&
+   git rev-parse --verify refs/tags/newtag
) &&
 
# now remove it
git branch -d newbranch &&
+   git tag -d newtag &&
 
# then test
(
@@ -582,6 +586,14 @@ test_configured_prune () {
kept)
git rev-parse --verify 
refs/remotes/origin/newbranch
;;
+   esac &&
+   case "$expected_tag" in
+   pruned)
+   test_must_fail git rev-parse --verify 
refs/tags/newtag
+   ;;
+   kept)
+   git rev-parse --verify refs/tags/newtag
+   ;;
esac
)
'
@@ -590,44 +602,45 @@ test_configured_prune () {
 # $1 config: fetch.prune
 # $2 config: remote..prune
 # $3 expect: branch to be pruned?
-# $4 git-fetch $cmdline:
+# $4 expect: tag to be pruned?
+# $5 git-fetch $cmdline:
 #
-# $1$2$3 $4
-test_configured_prune unset unset kept   ""
-test_configured_prune unset unset kept   "--no-prune"
-test_configured_prune unset unset pruned "--prune"
-
-test_configured_prune false unset kept   ""
-test_configured_prune false unset kept   "--no-prune"
-test_configured_prune false unset pruned "--prune"
-
-test_configured_prune true  unset pruned ""
-test_configured_prune true  unset pruned "--prune"
-test_configured_prune true  unset kept   "--no-prune"
-
-test_configured_prune unset false kept   ""
-test_configured_prune unset false kept   "--no-prune"
-test_configured_prune unset false pruned "--prune"
-
-test_configured_prune false false kept   ""
-test_configured_prune false false kept   "--no-prune"
-test_configured_prune false false pruned "--prune"
-
-test_configured_prune true  false kept   ""
-test_configured_prune true  false pruned "--prune"
-test_configured_prune true  false kept   "--no-prune"
-
-test_configured_prune unset true  pruned ""
-test_configured_prune unset true  kept   "--no-prune"
-test_configured_prune unset true  pruned "--prune"
-
-test_configured_prune false true  pruned ""
-test_configured_prune false true  kept   "--no-prune"
-test_configured_prune false true  pruned "--prune"
-
-test_configured_prune true  true  pruned ""
-test_configured_prune true  true  pruned "--prune"
-test_configured_prune true  true  kept   "--no-prune"
+# $1$2$3 $4 $5
+test_configured_prune unset unset kept   kept   ""
+test_configured_prune unset unset kept   kept   "--no-prune"
+test_configured_prune unset unset pruned kept   "--prune"
+
+test_configured_prune false unset kept   kept   ""
+test_configured_prune false unset kept   kept   "--no-prune"
+test_configured_prune false unset pruned kept   "--prune"
+
+test_configured_prune true  unset pruned kept   ""
+test_configured_prune true  unset pruned kept   "--prune"
+test_configured_prune true  unset kept   kept   "--no-prune"
+
+test_configured_prune unset false kept   kept   ""
+test_configured_prune unset false kept   kept   "--no-prune"
+test_configured_prune unset false pruned kept   "--prune"
+
+test_configured_prune false false kept   kept   ""
+test_configured_prune false false kept   kept   "--no-prune"
+test_configured_prune false false pruned kept   "--prune"
+
+test_configured_prune true  false kept   kept   ""
+test_configured_prune true  false pruned kept   "--prune"
+test_configured_prune 

Re: [PATCH] describe: use strbuf_add_unique_abbrev() for adding short hashes

[Lars Schneider]

> On 18 Jan 2018, at 23:40, SZEDER Gábor  wrote:
> 
> On Thu, Jan 18, 2018 at 10:40 PM, René Scharfe  wrote:
>> Am 16.01.2018 um 18:11 schrieb SZEDER Gábor:
>>> Unfortunately, most of the changes coming from 'strbuf.cocci' don't
>>> make any sense, they appear to be the mis-application of the "use
>>> strbuf_addstr() instead of strbuf_addf() to add a single string" rule:
>>> 
>>>   - strbuf_addf(_repo, "%d", counter);
>>>   + strbuf_addstr(_repo, counter);
>>> 
>>> It seems that those rules need some refinement, but I have no idea
>>> about Coccinelle and this is not the time for me to dig deeper.
>>> 
>>> What makes all this weird is that running 'make coccicheck' on my own
>>> machine doesn't produce any of these additional proposed changes, just
>>> like at René's.  Can it be related to differing Coccinelle versions?
>>> Travis CI installs 1.0.0~rc19.deb-3; I have 1.0.4.deb-2.
>> 
>> The version difference may explain it, but I couldn't find a matching
>> bugfix in http://coccinelle.lip6.fr/distrib/changes.html when I just
>> skimmed it.  I wonder if the following patch could make a difference:
> 
> Yes, it does, now all those nonsense suggestions are gone on Travis CI.
> 
>  https://travis-ci.org/szeder/git/jobs/330572425#L713
> 
> Those "memmove() -> MOVE_ARRAY" suggestions are still there, of course.

Nice!

Travis CI runs Ubuntu Trusty as current base image with Coccinelle
version "1.0.0~rc19.deb-3": https://packages.ubuntu.com/trusty/coccinelle

I was experimenting with a docker container that has the latest stable
version of Coccinelle installed ("1.0.4.deb-2"). We could run this
docker container inside of the Travis CI job similar to the 32-bit job.
But with René's changes that doesn't seem to be necessary anymore!

- Lars


PS: Thanks a lot Gábor for recognizing and fixing the invalid
Travis CI Coccinelle build!

Re: [RFC PATCH] NonStop port changes for git 2.16.0.

[Stefan Beller]

On Thu, Jan 18, 2018 at 2:42 PM,   wrote:
> From: "Randall S. Becker" 
>
> Explanation: I'm looking for comments on how best to handle the changes
> below that are needed for the NonStop port.

Ideally you'd send them as single patches, each of them describing why it
makes sense generally (such as the s/read/xread/ line) or specifically for
your arch (these #ifdefs and Makefile changes).

> The hashmap.h and
> transport-helper.c will not be included in the final patch as they have
> already been communicated but did not make it into 2.16.0.
> It is likely that some of the changes below are in the wrong files
> (for example: NSIG, intptr_t) and should be moved. This is just the
> current state of the port, as it grew (and fortunately shrank a lot)
> over the past few years. My objective, as the platform maintainer, is
> to clean it up, and to try to get the platform's modifications
> included so that we can just (obviously) pull directly from the standard
> repository and not have to apply these mods in future and focus on
> any platform-related breakages.

Thanks for keeping up with the latest version! The xread issue sounds
like you found a bug across all platforms, which is valued by the wider
community. So feel free to send individual patches and the list will get
back to you. :)

> Further: there are 6 known breakages that have been reported. The perl
> issues relating to completion codes are being examined at present by the
> platform support teams so are not addressed by this patch.

For perl I'd suggest cc'ing Ævar (cc'd just now) as that seems his
field of expertise.

Thanks for such a report,
Stefan

Re: [PATCH v2] diff: add --compact-summary option to complement --stat

[Jeff King]

On Thu, Jan 18, 2018 at 05:05:46PM +0700, Nguyễn Thái Ngọc Duy wrote:

> This is partly inspired by gerrit web interface which shows diffstat
> like this, e.g. with commit 0433d533f1 (notice the "A" column on the
> third line):
> 
>  Documentation/merge-config.txt |  4 +
>  builtin/merge.c|  2 +
>A t/t5573-pull-verify-signatures.sh  | 81 ++
>  t/t7612-merge-verify-signatures.sh | 45 ++
>4 files changed, 132 insertions(+)

I like the general concept. Perusing "git log" output, though, it felt
like the summary column was very close to the filenames. What do you
think of putting it after the "|", where it is only close to a number?

Something like the patch below (on top of yours, but it probably needs
tweaked further for graph_width), which gives:

   t/t5573-pull-verify-signatures.sh | A+x  78 

(I know this is a bikeshed, so I'm perfectly willing to take "yuck, I
don't like that as well" as a response).

> The new option --compact-summary implements this with a tweak to support
> mode change, which is shown in --summary too.

One thing I noticed is that --compact-summary by itself does nothing.
Should it imply --stat?

-Peff

[RFC PATCH] NonStop port changes for git 2.16.0.

[randall . s . becker]

From: "Randall S. Becker" 

Explanation: I'm looking for comments on how best to handle the changes
below that are needed for the NonStop port. The hashmap.h and
transport-helper.c will not be included in the final patch as they have 
already been communicated but did not make it into 2.16.0.
It is likely that some of the changes below are in the wrong files
(for example: NSIG, intptr_t) and should be moved. This is just the
current state of the port, as it grew (and fortunately shrank a lot)
over the past few years. My objective, as the platform maintainer, is
to clean it up, and to try to get the platform's modifications
included so that we can just (obviously) pull directly from the standard
repository and not have to apply these mods in future and focus on
any platform-related breakages.

Further: there are 6 known breakages that have been reported. The perl
issues relating to completion codes are being examined at present by the
platform support teams so are not addressed by this patch.

With Respect,
Randall

* Fixes platform issues not covered in the vanilla git code.

* Pulls previous ports forward into a single commit.

* Still has some known breaks based on platform limits and restrictions.

* Makefile: allows error codes during install. Change the options associated
  with the tar operation providing options required on platform. To be
  refactored into variables.

* regcomp.c: fixes missing intptr_t on NonStop.

* config.mak.uname: upgrades old options for current operating system
  requirements.

* git-compat-util.h: adds FLOSS definitions to allow platform support.

* remote.c: force ignoring of GCC __attribute construct not supported
  by c99.

* run_command.c: Added NSIG definition that is missing from NonStop
  signal.h.

* lib-git-daemon.sh: fixed incompatibilities with ksh traps not cleared
  automatically on platform.

* wrapper.c: added setbuf(stream,0) to force pipe flushes not enabled by
  default on platform.

Signed-off-by: Randall S. Becker 
---
 Makefile   |  4 ++--
 compat/regex/regcomp.c |  7 +++
 config.mak.uname   | 29 +
 git-compat-util.h  |  9 +
 hashmap.h  |  3 +--
 remote.c   |  4 
 run-command.c  |  5 +
 t/lib-git-daemon.sh|  3 +++
 transport-helper.c |  2 +-
 wrapper.c  |  3 +++
 10 files changed, 56 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile
index 1a9b23b67..c91602609 100644
--- a/Makefile
+++ b/Makefile
@@ -2567,9 +2567,9 @@ install: all
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
$(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
 ifndef NO_GETTEXT
-   $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'
+   -$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'
(cd po/build/locale && $(TAR) cf - .) | \
-   (cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) xof -)
+   (cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) xvof -)
 endif
 ifndef NO_PERL
$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c
index 51cd60baa..cb03fab78 100644
--- a/compat/regex/regcomp.c
+++ b/compat/regex/regcomp.c
@@ -17,6 +17,13 @@
License along with the GNU C Library; if not, see
.  */
 
+#if defined __TANDEM
+#ifdef NO_INTPTR_T
+typedef long intptr_t;
+typedef unsigned long uintptr_t;
+#endif
+#endif
+
 static reg_errcode_t re_compile_internal (regex_t *preg, const char * pattern,
  size_t length, reg_syntax_t syntax);
 static void re_compile_fastmap_iter (regex_t *bufp,
diff --git a/config.mak.uname b/config.mak.uname
index 685a80d13..d9f8d57e3 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -428,27 +428,37 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
# INLINE='' would just replace one set of warnings with another and
# still not compile in c89 mode, due to non-const array initializations.
CC = cc -c99
+   # Build down-rev compatible objects that don't use our new getopt_long.
+   ifeq ($(uname_R).$(uname_V),J06.21)
+   CC += -WRVU=J06.20
+   endif
+   ifeq ($(uname_R).$(uname_V),L17.02)
+   CC += -WRVU=L16.05
+   endif
# Disable all optimization, seems to result in bad code, with -O or -O2
# or even -O1 (default), /usr/local/libexec/git-core/git-pack-objects
# abends on "git push". Needs more investigation.
-   CFLAGS = -g -O0
+   CFLAGS = -g -O0 -Winline
# We'd want it to be here.
prefix = /usr/local
# Our's are in ${prefix}/bin (perl might also be in /usr/bin/perl).
-   PERL_PATH = ${prefix}/bin/perl
-   PYTHON_PATH = ${prefix}/bin/python
-
+   PERL_PATH = /usr/bin/perl
+   PYTHON_PATH = /usr/bin/python
+   

Re: [PATCH] describe: use strbuf_add_unique_abbrev() for adding short hashes

[SZEDER Gábor]

On Thu, Jan 18, 2018 at 10:40 PM, René Scharfe  wrote:
> Am 16.01.2018 um 18:11 schrieb SZEDER Gábor:
>> Unfortunately, most of the changes coming from 'strbuf.cocci' don't
>> make any sense, they appear to be the mis-application of the "use
>> strbuf_addstr() instead of strbuf_addf() to add a single string" rule:
>>
>>- strbuf_addf(_repo, "%d", counter);
>>+ strbuf_addstr(_repo, counter);
>>
>> It seems that those rules need some refinement, but I have no idea
>> about Coccinelle and this is not the time for me to dig deeper.
>>
>> What makes all this weird is that running 'make coccicheck' on my own
>> machine doesn't produce any of these additional proposed changes, just
>> like at René's.  Can it be related to differing Coccinelle versions?
>> Travis CI installs 1.0.0~rc19.deb-3; I have 1.0.4.deb-2.
>
> The version difference may explain it, but I couldn't find a matching
> bugfix in http://coccinelle.lip6.fr/distrib/changes.html when I just
> skimmed it.  I wonder if the following patch could make a difference:

Yes, it does, now all those nonsense suggestions are gone on Travis CI.

  https://travis-ci.org/szeder/git/jobs/330572425#L713

Those "memmove() -> MOVE_ARRAY" suggestions are still there, of course.

> ---
>  contrib/coccinelle/strbuf.cocci | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci
> index 1d580e49b0..6fe8727421 100644
> --- a/contrib/coccinelle/strbuf.cocci
> +++ b/contrib/coccinelle/strbuf.cocci
> @@ -29,8 +29,9 @@ cocci.include_match("%" not in fmt)
>
>  @@
>  expression E1, E2;
> +format F =~ "s";
>  @@
> -- strbuf_addf(E1, "%s", E2);
> +- strbuf_addf(E1, "%@F@", E2);
>  + strbuf_addstr(E1, E2);
>
>  @@
> --
> 2.16.0

RE: [Question] format-patch along a specific path

[Randall S. Becker]

On January 18, 2018 2:17 PM, I wrote:
> What I’m trying to do is to format a patch based on a single commit from
> 2.16.0 representing the NonStop port, for review and comments to the team.
> Here is a partial (somewhat familiar) tree:
> 
> *   f1a482cd8 (HEAD -> randall_2.16, ituglib_release) NonStop port changes
> for git 2.16.0.
> |\
> | | * b2a06dcfb (refs/stash) WIP on randall: 5653e94 Removed unnecessary
> void* from hashmap.h that caused compile warnings
> | |/
> | * 5653e943a (randall) Removed unnecessary void* from hashmap.h that
> | caused
> compile warnings
> | *   b318de9ca Merge branch 'ituglib_devel' into 2.16.0-rc1
> | |\
> | | * a4cdf025d (randall_2.13.5) Replaced read with xread in
> transport-helper.c to fix SSIZE_MAX overun in t5509
> | | | * b72dbd7fa (origin/pu) Merge branch 'ds/use-get-be64' into pu
> | |_|/
> |/| |
> | | | * 896df04e4 (origin/next) Sync with 2.16
> | |_|/
> |/| |
> * | | 2512f1544 (tag: v2.16.0, origin/master, origin/HEAD) Git 2.16
> |/ /
> * | c6c75c93a (tag: v2.16.0-rc2) Git 2.16-rc2
> 
> Trying the intuitive approach of git format-patch -1 f1a482cd8, I end up
with
> a patch for c6c75c93a, which I assume is a backtrack. If I use
> 2512f1544..f1a482cd8, I end up with a whole bunch of commits along the
> long path rather than the short single hop path, (I actually want the
short
> path). The --base=2512f1544 option complains that the base commit
> shouldn't be in revision list, which I also assume is an artifact of
backtracking.
> Is there a clean way in 2.16.0 to do this or should I redo the commit as a
> squash and disconnect it from the other path?

I decided to go down the squash route figuring that the eventual fix will
have to be squashed anyway. The RFC patch will follow.

I'm still curious how to do this, and if it's not possible, perhaps we need
a --via:commit-ish option to format-patch to force a path.

Cheers,
Randall

-- Brief whoami:
  NonStop developer since approximately NonStop(2112884442)
  UNIX developer since approximately 421664400
-- In my real life, I talk too much.

Re: [PATCH 3/3] read-cache: don't write index twice if we can't write shared index

[SZEDER Gábor]

On Thu, Jan 18, 2018 at 10:37 PM, Jeff King  wrote:
> On Thu, Jan 18, 2018 at 10:00:14PM +0700, Duy Nguyen wrote:
>
>> The test suite was run as root, no wonder why my removing write access
>> has no effect. I got the test to pass with this, but then it fails
>> with
>>
>> Can't write .prove (Permission denied) at 
>> /usr/share/perl/5.22/App/Prove.pm line 542.
>>
>> Some more chown'ing or chmod'ing is required

This is the fallout of running the tests as root in the past.  With your
patch 'prove' is run as a non-root user, but the prove state is loaded
from Travis CI's cache, where it has been written as root the last time
around, so now we don't have permissions to (over)write it.

I have patches in the works to address this as well.

>> Subject: [PATCH] ci: don't accidentally run the test suite as root
>>
>> This script assigns and adds a user named "ci" in a subshell so the
>> outer CI_USER is not affected. For some reason, CI_USER is actually
>> empty on Travis linux32 builds. This makes the following "su" useless
>> and the test suite is run as root.
>
> Are we sure this was the problem on Travis, and it wasn't just an issue
> with how I reproduced via docker?

Yes, we are.


Gábor

Re: What's cooking in git.git (Jan 2018, #02; Tue, 9)

[Jonathan Tan]

On Thu, 18 Jan 2018 09:56:50 +0100
Christian Couder  wrote:

> I am still not very happy with fetch_object() not returning anything.
> I wonder what happens when that function is used to fetch from a repo
> that cannot provide the requested object.

My idea was to save a verification step - the caller of fetch_object()
needs to reattempt the object load anyway (which includes a verification
that the object exists), so I didn't see the need to have fetch_object()
do it too.

> Also I think the "extensions.partialclone = " config option is
> not very future proof. If people start using partial clone, it is
> likely that at some point they will need their repo to talk to more
> than one remote that is partial clone enabled and I don't see how such
> a config option can scale in this case.

In the case that they want to talk to more than one
partial-clone-enabled repo, I think that there still needs to be one
"default" remote from which missing objects are fetched. I can think of
a few reasons for that - for example, (a) we need to support commands
that give a nonexistent-in-repo SHA-1 directly, and (b) existing Git
code relies on the ability to fetch an object given only its SHA-1. (a)
can be overcome by forbidding that (?) and (b) can be overcome by an
overhaul of the object-fetching and object-using code, but I don't think
both (a) and (b) will occur anytime soon.

[ANNOUNCE] Git for Windows 2.16.0(2)

[Johannes Schindelin]

Dear Git users,

It is my pleasure to announce that Git for Windows 2.16.0(2) is available from:

https://git-for-windows.github.io/

Changes since Git for Windows v2.15.1(2) (November 30th 2017)

Git for Windows now has a new homepage: https://gitforwindows.org/ (it
is still graciously hosted by GitHub, but now much quicker to type).

New Features

  * Comes with Git Credential Manager v1.14.0.
  * The Git for Windows installer now offers to configure Visual Studio
Code as default editor for Git.
  * Comes with OpenSSL v1.0.2n.
  * git checkout is now a lot faster when checking out a lot of files.
  * The core.excludesfile can now reference a symbolic link.
  * Comes with patch level 7 of the MSYS2 runtime (Git for Windows
flavor) based on Cygwin 2.9.0.
  * With lots of files, git reset --hard is now a lot faster when the
FSCache feature is in effect.

Bug Fixes

  * When cloning into an existing (empty) directory fails, Git no
longer removes said directory.
  * Interrupting processes (and their children) using Control+C is now
a lot more robust.

Filename | SHA-256
 | ---
Git-2.16.0.2-64-bit.exe | 
dda9c9eacdec5bb1369351b48d84c7ba947c85cf5f6285e369eebc89075f8bb7
Git-2.16.0.2-32-bit.exe | 
f595b9245f387aa6e4e1dbf7eda7c963b80d3ded05fafa16f09dbac582fa3e6a
PortableGit-2.16.0.2-64-bit.7z.exe | 
967f65c6e014e543109e9bca1a7cffbe192eae82c349ea1639eefdfe087e44e8
PortableGit-2.16.0.2-32-bit.7z.exe | 
b5d0699f9720cfba62e61d8c54bc4a4696323273a4ae9273c82829f16d5a2af9
MinGit-2.16.0.2-64-bit.zip | 
fb028d2a18c7ec18f8febecafc95e9dad0dd583ab8fe376c95a06eff62058bbd
MinGit-2.16.0.2-32-bit.zip | 
1c49443bddde815b5ef551f692a44fd9f681c1cfb8cf9457461690c924d2352a
MinGit-2.16.0.2-busybox-64-bit.zip | 
63b08dc7a583e843e796c2f92036d40cc42e51096812dd38be1433453bb49f61
MinGit-2.16.0.2-busybox-32-bit.zip | 
84bcfe30841f490cd2d20677be217854221a1ca4a7691f27b95c60034b498010
Git-2.16.0.2-64-bit.tar.bz2 | 
2685b9d2dce7be83a2f8528ef55498c5938216680d4d1b4d3c4fa5907d7eac04
Git-2.16.0.2-32-bit.tar.bz2 | 
18899b6b9738565c7d5d49e385c67bd5130e110f0f9269c3a051af481b1ce55a

Ciao,
Johannes

Re: [PATCH 10/8] [DO NOT APPLY, but improve?] rebase--interactive: introduce "stop" command

[Jacob Keller]

On Thu, Jan 18, 2018 at 2:08 PM, Philip Oakley  wrote:
> From: "Jacob Keller" 
>>
>> On Thu, Jan 18, 2018 at 10:36 AM, Stefan Beller 
>> wrote:
>>>
>>> Jake suggested using "x false" instead of "edit" for some corner cases.
>>>
>>> I do prefer using "x false" for all kinds of things such as stopping
>>> before a commit (edit only let's you stop after a commit), and the
>>> knowledge that "x false" does the least amount of actions behind my back.
>>>
>>> We should have that command as well, maybe?
>>>
>>
>>
>> I agree. I use "x false" very often, and I think stop is probably a
>> better solution since it avoids spawning an extra shell that will just
>> fail. Not sure if stop implies too much about "stop the whole thing"
>> as opposed to "stop here and let me do something manual", but I think
>> it's clear enough.
>>
> 'hold' or 'pause' maybe options (leads to
> http://www.thesaurus.com/browse/put+on+hold offering procastinate etc.)
> 'adjourn'.
>
>

I like break, as suggested by Dscho. That also works well for
abbreviation if we drop the "bud" command.

Thanks,
Jake

Re: [PATCH 10/8] [DO NOT APPLY, but improve?] rebase--interactive: introduce "stop" command

[Stefan Beller]

On Thu, Jan 18, 2018 at 2:00 PM, Johannes Schindelin
 wrote:

>> + TODO_STOP,
>
> I see that your original idea was "stop", but then you probably realized
> that there would be no good abbreviation for that, and changed your mind.
>
> Personally, I would have called it `break`...

I was looking at a synonym list of stop to find a word that contained a letter
which was not already taken. 'break' would allow for 'a', or 'k', assuming 'bud'
takes 'b' (or can that go to 'u'? Are there people out there with muscle memory
on these letters already?)

Any word (of stop, break, stay, control) sounds good to me, though 'break' might
be the clearest.

>
>> @@ -2407,6 +2415,8 @@ static int pick_commits(struct todo_list *todo_list, 
>> struct replay_opts *opts)
>>   /* `current` will be incremented below */
>>   todo_list->current = -1;
>>   }
>> + } else if (item->command == TODO_STOP) {
>> + todo_list->current = -1;
>
> That is incorrect, it will most likely write an unexpected `done` file.
>
> Did you mean `return 0` instead?

I guess. I did not compile or test the patch, I was merely writing down enough
to convey the idea, hopefully.

While talking about this idea of exploding the number of keywords,
maybe we can also have 'abort', which does the same as deleting all lines
(every time I want to abort I still get shivers if I just drop all
patches instead
of aborting, so maybe typing 'abort-and-restore' as the first thing in the file
would convey a safer feeling to users?)

Thanks for taking these additional considerations into mind while I don't
review the actual patches,

Stefan

Re: [PATCH 10/8] [DO NOT APPLY, but improve?] rebase--interactive: introduce "stop" command

[Philip Oakley]

From: "Jacob Keller" 
On Thu, Jan 18, 2018 at 10:36 AM, Stefan Beller  
wrote:

Jake suggested using "x false" instead of "edit" for some corner cases.

I do prefer using "x false" for all kinds of things such as stopping
before a commit (edit only let's you stop after a commit), and the
knowledge that "x false" does the least amount of actions behind my back.

We should have that command as well, maybe?




I agree. I use "x false" very often, and I think stop is probably a
better solution since it avoids spawning an extra shell that will just
fail. Not sure if stop implies too much about "stop the whole thing"
as opposed to "stop here and let me do something manual", but I think
it's clear enough.

'hold' or 'pause' maybe options (leads to 
http://www.thesaurus.com/browse/put+on+hold offering procastinate etc.)

'adjourn'.




Signed-off-by: Stefan Beller 
---
 git-rebase--interactive.sh |  1 +
 sequencer.c| 10 ++
 2 files changed, 11 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3cd7446d0b..9eac53f0c5 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -166,6 +166,7 @@ l, label = label current HEAD with a name
 t, reset  = reset HEAD to a label
 b, bud = reset HEAD to the revision labeled 'onto', no arguments
 m, merge []* = create a merge commit using a given 
commit's message

+y, stay = stop for  shortcut for

 These lines can be re-ordered; they are executed from top to bottom.
 " | git stripspace --comment-lines >>"$todo"
diff --git a/sequencer.c b/sequencer.c
index 2b4e6b1232..4b3b9fe59d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -782,6 +782,7 @@ enum todo_command {
TODO_RESET,
TODO_BUD,
TODO_MERGE,
+   TODO_STOP,
/* commands that do nothing but are counted for reporting 
progress */

TODO_NOOP,
TODO_DROP,
@@ -803,6 +804,7 @@ static struct {
{ 'l', "label" },
{ 't', "reset" },
{ 'b', "bud" },
+   { 'y', "stay" },
{ 'm', "merge" },
{ 0,   "noop" },
{ 'd', "drop" },
@@ -1307,6 +1309,12 @@ static int parse_insn_line(struct todo_item *item, 
const char *bol, char *eol)

return 0;
}

+   if (item->command == TODO_STOP) {
+   item->commit = NULL;
+   item->arg = "";
+   item->arg_len = 0;
+   }
+
end_of_object_name = (char *) bol + strcspn(bol, " \t\n");
item->arg = end_of_object_name + strspn(end_of_object_name, " 
\t");

item->arg_len = (int)(eol - item->arg);
@@ -2407,6 +2415,8 @@ static int pick_commits(struct todo_list 
*todo_list, struct replay_opts *opts)

/* `current` will be incremented below */
todo_list->current = -1;
}
+   } else if (item->command == TODO_STOP) {
+   todo_list->current = -1;
} else if (item->command == TODO_LABEL)
res = do_label(item->arg, item->arg_len);
else if (item->command == TODO_RESET)
--
2.16.0.rc1.238.g530d649a79-goog

Re: [PATCH 8/8] rebase -i: introduce --recreate-merges=no-rebase-cousins

[Philip Oakley]

From: "Johannes Schindelin" 

This one is a bit tricky to explain, so let's try with a diagram:

   C
 /   \
A - B - E - F
 \   /
   D

To illustrate what this new mode is all about, let's consider what
happens upon `git rebase -i --recreate-merges B`, in particular to
the commit `D`. In the default mode, the new branch structure is:

  --- C' --
 / \
A - B -- E' - F'
 \/
   D'

This is not really preserving the branch topology from before! The
reason is that the commit `D` does not have `B` as ancestor, and
therefore it gets rebased onto `B`.

However, when recreating branch structure, there are legitimate use
cases where one might want to preserve the branch points of commits that
do not descend from the  commit that was passed to the rebase
command, e.g. when a branch from core Git's `next` was merged into Git
for Windows' master we will not want to rebase those commits on top of a
Windows-specific commit. In the example above, the desired outcome would
look like this:

  --- C' --
 / \
A - B -- E' - F'
 \/
  -- D' --


I'm not understanding this. I see that D properly starts from A, but don't 
see why it is now D'. Surely it's unchanged.
Maybe it's the arc/node confusion. Maybe even spell out that the rebased 
commits from the command are B..HEAD, but that includes D, which may not be 
what folk had expected. (not even sure if the reflog comes into determining 
merge-bases here..)


I do think an exact definition is needed (e.g. via --ancestry-path or its 
equivalent?).




Let's introduce the term "cousins" for such commits ("D" in the
example), and the "no-rebase-cousins" mode of the merge-recreating
rebase, to help those use cases.

Signed-off-by: Johannes Schindelin 
---
Documentation/git-rebase.txt  |  7 ++-
builtin/rebase--helper.c  |  9 -
git-rebase--interactive.sh|  1 +
git-rebase.sh | 12 +++-
sequencer.c   |  4 
sequencer.h   |  8 
t/t3430-rebase-recreate-merges.sh | 23 +++
7 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 1d061373288..ac07a5c3fc9 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -368,10 +368,15 @@ The commit list format can be changed by setting the 
configuration option
rebase.instructionFormat.  A customized instruction format will 
automatically

have the long commit hash prepended to the format.

---recreate-merges::
+--recreate-merges[=(rebase-cousins|no-rebase-cousins)]::
 Recreate merge commits instead of flattening the history by replaying
 merges. Merge conflict resolutions or manual amendments to merge
 commits are not preserved.
++
+By default, or when `rebase-cousins` was specified, commits which do not 
have
+`` as direct ancestor are rebased onto `` (or 
``,
+if specified). If the `rebase-cousins` mode is turned off, such commits 
will

+retain their original branch point.

-p::
--preserve-merges::
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index a34ab5c0655..ef08fef4d14 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -13,7 +13,7 @@ int cmd_rebase__helper(int argc, const char **argv, 
const char *prefix)

{
 struct replay_opts opts = REPLAY_OPTS_INIT;
 unsigned flags = 0, keep_empty = 0, recreate_merges = 0;
- int abbreviate_commands = 0;
+ int abbreviate_commands = 0, no_rebase_cousins = -1;
 enum {
 CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
 CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
@@ -23,6 +23,8 @@ int cmd_rebase__helper(int argc, const char **argv, 
const char *prefix)

 OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
 OPT_BOOL(0, "keep-empty", _empty, N_("keep empty commits")),
 OPT_BOOL(0, "recreate-merges", _merges, N_("recreate merge 
commits")),

+ OPT_BOOL(0, "no-rebase-cousins", _rebase_cousins,
+ N_("keep original branch points of cousins")),
 OPT_CMDMODE(0, "continue", , N_("continue rebase"),
 CONTINUE),
 OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -57,8 +59,13 @@ int cmd_rebase__helper(int argc, const char **argv, 
const char *prefix)

 flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0;
 flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
 flags |= recreate_merges ? TODO_LIST_RECREATE_MERGES : 0;
+ flags |= no_rebase_cousins > 0 ? TODO_LIST_NO_REBASE_COUSINS : 0;
 flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;

+ if (no_rebase_cousins >= 0&& !recreate_merges)
+ warning(_("--[no-]rebase-cousins has no effect without "
+   "--recreate-merges"));
+
 if (command == CONTINUE && argc == 1)
 return !!sequencer_continue();
 if (command == ABORT && argc == 1)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3459ec5a018..23184c77e88 100644
--- 

Re: [PATCH 10/8] [DO NOT APPLY, but improve?] rebase--interactive: introduce "stop" command

[Johannes Schindelin]

Hi Stefan,

On Thu, 18 Jan 2018, Stefan Beller wrote:

> Jake suggested using "x false" instead of "edit" for some corner cases.
> 
> I do prefer using "x false" for all kinds of things such as stopping
> before a commit (edit only let's you stop after a commit), and the
> knowledge that "x false" does the least amount of actions behind my back.
> 
> We should have that command as well, maybe?
> 
> Signed-off-by: Stefan Beller 
> ---
>  git-rebase--interactive.sh |  1 +
>  sequencer.c| 10 ++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 3cd7446d0b..9eac53f0c5 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -166,6 +166,7 @@ l, label = label current HEAD with a name
>  t, reset  = reset HEAD to a label
>  b, bud = reset HEAD to the revision labeled 'onto', no arguments
>  m, merge []* = create a merge commit using a given commit's 
> message
> +y, stay = stop for  shortcut for
>  
>  These lines can be re-ordered; they are executed from top to bottom.
>  " | git stripspace --comment-lines >>"$todo"
> diff --git a/sequencer.c b/sequencer.c
> index 2b4e6b1232..4b3b9fe59d 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -782,6 +782,7 @@ enum todo_command {
>   TODO_RESET,
>   TODO_BUD,
>   TODO_MERGE,
> + TODO_STOP,

I see that your original idea was "stop", but then you probably realized
that there would be no good abbreviation for that, and changed your mind.

Personally, I would have called it `break`...

> @@ -2407,6 +2415,8 @@ static int pick_commits(struct todo_list *todo_list, 
> struct replay_opts *opts)
>   /* `current` will be incremented below */
>   todo_list->current = -1;
>   }
> + } else if (item->command == TODO_STOP) {
> + todo_list->current = -1;

That is incorrect, it will most likely write an unexpected `done` file.

Did you mean `return 0` instead?

Ciao,
Dscho

Re: [PATCH 9/8] [DO NOT APPLY, but squash?] git-rebase--interactive: clarify arguments

[Stefan Beller]

On Thu, Jan 18, 2018 at 1:36 PM, Johannes Schindelin
 wrote:

> Good idea! I would rather do it as an introductory patch (that only
> converts the existing list).

That makes sense.

>
> As to `merge`: it is a bit more complicated ;-)
>
> m, merge  (  | "..." ) 
> []
> create a merge commit using the original merge commit's
> message (or the oneline, if "-" is given). Use a quoted
> list of commits to be merged for octopus merges.

That makes sense. Thanks for being precise at the the user facing UX here.

Thanks,
Stefan

Re: [PATCH v3 0/3] fixes for split index mode

[Thomas Gummerer]

Friendly ping on this series now that 2.16 is out :) Is there anything
in this series (up to 3/3, 4/3 can be dropped now that Duy fixed it in
a nicer way) that still needs updating?  It fixes a few bugs in split
index mode with submodules/worktrees, so it would be nice to get this
reviewed/merged.

On 01/07, Thomas Gummerer wrote:
> Thanks Brandon and Lars for comments on the previous round.
> 
> Previous rounds were at <20171210212202.28231-1-t.gumme...@gmail.com>
> and <20171217225122.28941-1-t.gumme...@gmail.com>.
> 
> Changes since the previous round:
> 
> - reworked the patches to no longer try to use struct repository for
>   worktrees, but pass gitdir into read_index_from instead (Thanks
>   Brandon :)).  So the fixes that were in 1/3 and 2/3 previously are
>   now in 1/3
> - 2/3 now fixes t7009 properly.  I thought it was fixed before, but it
>   probably just passed the tests because of the GIT_TEST_SPLIT_INDEX
>   "randomness".
> - The travis job is now only running the 64-bit linux build with split
>   index mode to save even more cycles.
> - As this wasn't picked up anywhere yet, I took the freedom to rebase
>   this onto the current master, which includes sg/travis-fixes, which
>   made it a bit easier for me to test.  If this makes the life harder
>   for anyone reviewing this let me know and I can base it on the same
>   commit previous iterations were based on.
> 
> Thomas Gummerer (3):
>   read-cache: fix reading the shared index for other repos
>   split-index: don't write cache tree with null sha1 entries
>   travis: run tests with GIT_TEST_SPLIT_INDEX
> 
>  cache-tree.c|  2 +-
>  cache.h |  8 +---
>  ci/run-linux32-build.sh |  1 +
>  ci/run-tests.sh |  4 
>  read-cache.c| 25 ++---
>  repository.c|  2 +-
>  revision.c  |  3 ++-
>  split-index.c   |  2 ++
>  t/t1700-split-index.sh  | 19 +++
>  9 files changed, 49 insertions(+), 17 deletions(-)
> 
> -- 
> 2.16.0.rc1.238.g530d649a79
> 

Re: [PATCH] describe: use strbuf_add_unique_abbrev() for adding short hashes

[René Scharfe]

Am 16.01.2018 um 18:11 schrieb SZEDER Gábor:
> Unfortunately, most of the changes coming from 'strbuf.cocci' don't
> make any sense, they appear to be the mis-application of the "use
> strbuf_addstr() instead of strbuf_addf() to add a single string" rule:
> 
>- strbuf_addf(_repo, "%d", counter);
>+ strbuf_addstr(_repo, counter);
> 
> It seems that those rules need some refinement, but I have no idea
> about Coccinelle and this is not the time for me to dig deeper.
> 
> What makes all this weird is that running 'make coccicheck' on my own
> machine doesn't produce any of these additional proposed changes, just
> like at René's.  Can it be related to differing Coccinelle versions?
> Travis CI installs 1.0.0~rc19.deb-3; I have 1.0.4.deb-2.

The version difference may explain it, but I couldn't find a matching
bugfix in http://coccinelle.lip6.fr/distrib/changes.html when I just
skimmed it.  I wonder if the following patch could make a difference:

---
 contrib/coccinelle/strbuf.cocci | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci
index 1d580e49b0..6fe8727421 100644
--- a/contrib/coccinelle/strbuf.cocci
+++ b/contrib/coccinelle/strbuf.cocci
@@ -29,8 +29,9 @@ cocci.include_match("%" not in fmt)
 
 @@
 expression E1, E2;
+format F =~ "s";
 @@
-- strbuf_addf(E1, "%s", E2);
+- strbuf_addf(E1, "%@F@", E2);
 + strbuf_addstr(E1, E2);
 
 @@
-- 
2.16.0

Re: [PATCH 4/8] rebase-helper --make-script: introduce a flag to recreate merges

[Philip Oakley]

From: "Johannes Schindelin" 

The sequencer just learned a new commands intended to recreate branch
structure (similar in spirit to --preserve-merges, but with a
substantially less-broken design).

Let's allow the rebase--helper to generate todo lists making use of
these commands, triggered by the new --recreate-merges option. For a
commit topology like this:

A - B - C
  \   /
D


Could the topology include the predecessor for context. Alo it is easy for 
readers to become confused between the arcs of the graphs and the nodes of 
the graphs, such that we confuse 'commits as patches' with 'commits as 
snapshots'. It might need an 'Aa' distinction between the two types, 
especially around merges and potential evilness.




the generated todo list would look like this:

# branch D
pick 0123 A
label branch-point
pick 1234 D
label D

reset branch-point
pick 2345 B
merge 3456 D C

To keep things simple, we first only implement support for merge commits
with exactly two parents, leaving support for octopus merges to a later
patch in this patch series.

Signed-off-by: Johannes Schindelin 
---
builtin/rebase--helper.c |   4 +-
sequencer.c  | 343 
++-

sequencer.h  |   1 +
3 files changed, 345 insertions(+), 3 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 7daee544b7b..a34ab5c0655 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -12,7 +12,7 @@ static const char * const builtin_rebase_helper_usage[] 
= {

int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
{
 struct replay_opts opts = REPLAY_OPTS_INIT;
- unsigned flags = 0, keep_empty = 0;
+ unsigned flags = 0, keep_empty = 0, recreate_merges = 0;
 int abbreviate_commands = 0;
 enum {
 CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
@@ -22,6 +22,7 @@ int cmd_rebase__helper(int argc, const char **argv, 
const char *prefix)

 struct option options[] = {
 OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
 OPT_BOOL(0, "keep-empty", _empty, N_("keep empty commits")),
+ OPT_BOOL(0, "recreate-merges", _merges, N_("recreate merge 
commits")),

 OPT_CMDMODE(0, "continue", , N_("continue rebase"),
 CONTINUE),
 OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -55,6 +56,7 @@ int cmd_rebase__helper(int argc, const char **argv, 
const char *prefix)


 flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0;
 flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
+ flags |= recreate_merges ? TODO_LIST_RECREATE_MERGES : 0;
 flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;

 if (command == CONTINUE && argc == 1)
diff --git a/sequencer.c b/sequencer.c
index a96255426e7..1bef16647b4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -23,6 +23,8 @@
#include "hashmap.h"
#include "unpack-trees.h"
#include "worktree.h"
+#include "oidmap.h"
+#include "oidset.h"

#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"

@@ -2785,6 +2787,335 @@ void append_signoff(struct strbuf *msgbuf, int 
ignore_footer, unsigned flag)

 strbuf_release();
}

+struct labels_entry {
+ struct hashmap_entry entry;
+ char label[FLEX_ARRAY];
+};
+
+static int labels_cmp(const void *fndata, const struct labels_entry *a,
+   const struct labels_entry *b, const void *key)
+{
+ return key ? strcmp(a->label, key) : strcmp(a->label, b->label);
+}
+
+struct string_entry {
+ struct oidmap_entry entry;
+ char string[FLEX_ARRAY];
+};
+
+struct label_state {
+ struct oidmap commit2label;
+ struct hashmap labels;
+ struct strbuf buf;
+};
+
+static const char *label_oid(struct object_id *oid, const char *label,
+  struct label_state *state)
+{
+ struct labels_entry *labels_entry;
+ struct string_entry *string_entry;
+ struct object_id dummy;
+ size_t len;
+ int i;
+
+ string_entry = oidmap_get(>commit2label, oid);
+ if (string_entry)
+ return string_entry->string;
+
+ /*
+ * For "uninteresting" commits, i.e. commits that are not to be
+ * rebased, and which can therefore not be labeled, we use a unique
+ * abbreviation of the commit name. This is slightly more complicated
+ * than calling find_unique_abbrev() because we also need to make
+ * sure that the abbreviation does not conflict with any other
+ * label.
+ *
+ * We disallow "interesting" commits to be labeled by a string that
+ * is a valid full-length hash, to ensure that we always can find an
+ * abbreviation for any uninteresting commit's names that does not
+ * clash with any other label.
+ */
+ if (!label) {
+ char *p;
+
+ strbuf_reset(>buf);
+ strbuf_grow(>buf, GIT_SHA1_HEXSZ);
+ label = p = state->buf.buf;
+
+ find_unique_abbrev_r(p, oid->hash, default_abbrev);
+
+ /*
+ * We may need to extend the abbreviated hash so that there is
+ * no conflicting label.
+ */
+ if (hashmap_get_from_hash(>labels, strihash(p), p)) {
+ size_t i = strlen(p) + 1;
+
+ oid_to_hex_r(p, oid);
+ for (; i < GIT_SHA1_HEXSZ; i++) {
+ char save = p[i];
+ p[i] = '\0';

Re: [PATCH 3/3] read-cache: don't write index twice if we can't write shared index

[Jeff King]

On Thu, Jan 18, 2018 at 10:00:14PM +0700, Duy Nguyen wrote:

> The test suite was run as root, no wonder why my removing write access
> has no effect. I got the test to pass with this, but then it fails
> with
> 
> Can't write .prove (Permission denied) at 
> /usr/share/perl/5.22/App/Prove.pm line 542.
> 
> Some more chown'ing or chmod'ing is required

Ah, right. I agree that we probably ought to run the ci as non-root.
However, if the test requires non-root, it probably needs to be marked
with the SANITY prereq.

I also ran into one funny thing: if you run the script with "-i", then
we do not run the test_when_finished block. And therefore the "ro"
directory is left without its write bit, and the next test run fails, as
it cannot "rm -rf" the old trash directory out of the way.

I'm not sure there's a good solution, though. Skipping the
test_when_finished block on a "-i" run is intentional, to let you
inspect the broken state.

> Subject: [PATCH] ci: don't accidentally run the test suite as root
> 
> This script assigns and adds a user named "ci" in a subshell so the
> outer CI_USER is not affected. For some reason, CI_USER is actually
> empty on Travis linux32 builds. This makes the following "su" useless
> and the test suite is run as root.

Are we sure this was the problem on Travis, and it wasn't just an issue
with how I reproduced via docker?

-Peff

Re: [PATCH 9/8] [DO NOT APPLY, but squash?] git-rebase--interactive: clarify arguments

[Johannes Schindelin]

Hi Stefan,

On Thu, 18 Jan 2018, Stefan Beller wrote:

> Up to now each command took a commit as its first argument and ignored
> the rest of the line (usually the subject of the commit)
> 
> Now that we have commands that take different arguments, clarify each
> command by giving the argument list.
> 
> Signed-off-by: Stefan Beller 
> ---
>  git-rebase--interactive.sh | 22 +++---
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 23184c77e8..3cd7446d0b 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -155,17 +155,17 @@ reschedule_last_action () {
>  append_todo_help () {
>   gettext "
>  Commands:
> -p, pick = use commit
> -r, reword = use commit, but edit the commit message
> -e, edit = use commit, but stop for amending
> -s, squash = use commit, but meld into previous commit
> -f, fixup = like \"squash\", but discard this commit's log message
> -x, exec = run command (the rest of the line) using shell
> -d, drop = remove commit
> -l, label = label current HEAD with a name
> -t, reset = reset HEAD to a label
> -b, bud = reset HEAD to the revision labeled 'onto'
> -m, merge = create a merge commit using a given commit's message
> +p, pick  = use commit
> +r, reword  = use commit, but edit the commit message
> +e, edit  = use commit, but stop for amending
> +s, squash  = use commit, but meld into previous commit
> +f, fixup  = like \"squash\", but discard this commit's log message
> +x, exec  = run command (the rest of the line) using shell
> +d, drop  = remove commit
> +l, label = label current HEAD with a name
> +t, reset  = reset HEAD to a label
> +b, bud = reset HEAD to the revision labeled 'onto', no arguments
> +m, merge []* = create a merge commit using a given commit's 
> message

Good idea! I would rather do it as an introductory patch (that only
converts the existing list).

As to `merge`: it is a bit more complicated ;-)

m, merge  (  | "..." ) []
create a merge commit using the original merge commit's
message (or the oneline, if "-" is given). Use a quoted
list of commits to be merged for octopus merges.

Thanks,
Dscho

Re: [PATCH 6/8] sequencer: handle autosquash and post-rewrite for merge commands

[Jacob Keller]

>> Same for the test here, I can't figure out why this is necessary in
>> this patch as opposed to the patch which first introduced the
>> refs/rewritten/ refs.
>
> Woops. This was its own commit, and must have been accidentally squashed
> during one of my rebases. Will re-introduce it;

Yep that makes sense!

Thanks,
Jake

Re: [PATCH 6/8] sequencer: handle autosquash and post-rewrite for merge commands

[Johannes Schindelin]

Hi Jake,

On Thu, 18 Jan 2018, Jacob Keller wrote:

> On Thu, Jan 18, 2018 at 7:35 AM, Johannes Schindelin
>  wrote:
> > In the previous patches, we implemented the basic functionality of the
> > `git rebase -i --recreate-merges` command, in particular the `merge`
> > command to create merge commits in the sequencer.
> >
> > The interactive rebase is a lot more these days, though, than a simple
> > cherry-pick in a loop. For example, it calls the post-rewrite hook (if
> > any) after rebasing with a mapping of the old->new commits. And the
> > interactive rebase also supports the autosquash mode, where commits
> > whose oneline is of the form `fixup! ` or `squash! `
> > are rearranged to amend commits whose oneline they match.
> >
> > This patch implements the post-rewrite and autosquash handling for the
> > `merge` command we just introduced. The other commands that were added
> > recently (`label`, `reset` and `bud`) do not create new commits,
> > therefore post-rewrite & autosquash do not need to handle them.
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> >  refs.c|  3 ++-
> >  sequencer.c   | 10 +++---
> >  t/t3430-rebase-recreate-merges.sh | 39 
> > +++
> >  3 files changed, 48 insertions(+), 4 deletions(-)
> >
> > diff --git a/refs.c b/refs.c
> > index 20ba82b4343..e8b84c189ff 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -600,7 +600,8 @@ int dwim_log(const char *str, int len, struct object_id 
> > *oid, char **log)
> >  static int is_per_worktree_ref(const char *refname)
> >  {
> > return !strcmp(refname, "HEAD") ||
> > -   starts_with(refname, "refs/bisect/");
> > +   starts_with(refname, "refs/bisect/") ||
> > +   starts_with(refname, "refs/rewritten/");
> >  }
> 
> Would this part make more sense to move into the commit that
> introduces writing these refs, or does it only matter once you start
> this step here?
> 
> >
> >  static int is_pseudoref_syntax(const char *refname)
> > diff --git a/sequencer.c b/sequencer.c
> > index 1bef16647b4..b63bfb9a141 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -2413,10 +2413,13 @@ static int pick_commits(struct todo_list 
> > *todo_list, struct replay_opts *opts)
> > res = do_reset(item->arg, item->arg_len);
> > else if (item->command == TODO_BUD)
> > res = do_reset("onto", 4);
> > -   else if (item->command == TODO_MERGE)
> > +   else if (item->command == TODO_MERGE) {
> > res = do_merge(item->commit,
> >item->arg, item->arg_len, opts);
> > -   else if (!is_noop(item->command))
> > +   if (item->commit)
> > +   
> > record_in_rewritten(>commit->object.oid,
> > +   peek_command(todo_list, 
> > 1));
> > +   } else if (!is_noop(item->command))
> > return error(_("unknown command %d"), 
> > item->command);
> >
> > todo_list->current++;
> > @@ -3556,7 +3559,8 @@ int rearrange_squash(void)
> > struct subject2item_entry *entry;
> >
> > next[i] = tail[i] = -1;
> > -   if (item->command >= TODO_EXEC) {
> > +   if (item->command >= TODO_EXEC &&
> > +   (item->command != TODO_MERGE || !item->commit)) {
> > subjects[i] = NULL;
> > continue;
> > }
> > diff --git a/t/t3430-rebase-recreate-merges.sh 
> > b/t/t3430-rebase-recreate-merges.sh
> > index 46ae52f88b3..76e615bd7c1 100755
> > --- a/t/t3430-rebase-recreate-merges.sh
> > +++ b/t/t3430-rebase-recreate-merges.sh
> > @@ -143,4 +143,43 @@ test_expect_success 'with a branch tip that was 
> > cherry-picked already' '
> > EOF
> >  '
> >
> > +test_expect_success 'refs/rewritten/* is worktree-local' '
> > +   git worktree add wt &&
> > +   cat >wt/script-from-scratch <<-\EOF &&
> > +   label xyz
> > +   exec GIT_DIR=../.git git rev-parse --verify refs/rewritten/xyz >a 
> > || :
> > +   exec git rev-parse --verify refs/rewritten/xyz >b
> > +   EOF
> > +
> > +   test_config -C wt sequence.editor \""$PWD"/replace-editor.sh\" &&
> > +   git -C wt rebase -i HEAD &&
> > +   test_must_be_empty wt/a &&
> > +   test_cmp_rev HEAD "$(cat wt/b)"
> > +'
> > +
> 
> Same for the test here, I can't figure out why this is necessary in
> this patch as opposed to the patch which first introduced the
> refs/rewritten/ refs.

Woops. This was its own commit, and must have been accidentally squashed
during one of my rebases. Will re-introduce it; this is roughly what it
will look like:

-- snipsnap --
Author: Johannes Schindelin 

Re: [PATCH 1/8] sequencer: introduce new commands to reset the revision

[Jacob Keller]

On Thu, Jan 18, 2018 at 1:24 PM, Philip Oakley  wrote:
> From: "Jacob Keller" 
>
>> On Thu, Jan 18, 2018 at 7:35 AM, Johannes Schindelin
>>  wrote:
>>>
>>> This commit implements the commands to label, and to reset to, given
>>> revisions. The syntax is:
>>>
>>> label 
>>> reset 
>>>
>>> As a convenience shortcut, also to improve readability of the generated
>>> todo list, a third command is introduced: bud. It simply resets to the
>>> "onto" revision, i.e. the commit onto which we currently rebase.
>>>
>>
>> The code looks good, but I'm a little wary of adding bud which
>> hard-codes a specific label. I suppose it does grant a bit of
>> readability to the resulting script... ? It doesn't seem that
>> important compared to use using "reset onto"? At least when
>> documenting this it should be made clear that the "onto" label is
>> special.
>>
>> Thanks,
>> Jake.
>
>
> I'd agree.
>
> The special 'onto' label should be fully documented, and the commit message
> should indicate which patch actually defines it (and all its corner cases
> and fall backs if --onto isn't explicitly given..)

I don't think it actually relates to "--onto" but rather to simply
using "label onto" in your sequencer script allows bud to work, and
simply shortens the overall work necessary. It's equivalent to "reset
onto" if I understand.

>
> Likewise the choice of 'bud' should be explained with some nice phraseology
> indicating that we are growing the new flowering from the bud, otherwise the
> word is a bit too short and sudden for easy explanation.
>
> Philip

Re: [PATCH 2/8] sequencer: introduce the `merge` command

[Jacob Keller]

On Thu, Jan 18, 2018 at 1:22 PM, Johannes Schindelin
 wrote:
>> Would it be possible to open the editor with the supplied text when
>> there's no commit?  The text after  must be oneline only..
>
> I actually want to avoid that because my main use case is fire-and-forget,
> i.e. I want to edit only the todo list and then (barring any merge
> conflicts) I do not want to edit anything anymore.
>

Agreed, for the case where we copy a commit message, I do not want the
editor either.

> But I guess we could special-case the thing where `-` is specified as
> "merge commit message provider" and an empty oneline is provided?
>

It's for when there is a new merge, for when we are creating a new one
using "-", yes.

>> It's difficult to reword merges because of the nature of rebase
>> interactive, you can't just re-run the rebase command and use
>> "reword".
>>
>> I suppose you could cheat by putting in an "edit" command that let you
>> create an empty commit with a message...
>
> Or you could "cheat" by adding `exec git commit --amend`...
>
> Seriously again, I have no good idea how to provide an equivalent to the
> `reword` verb that would work on merge commits...
>

Given that there is a work around, and I doubt it's that common, I'm
not sure we need one, plus i have no idea what verb to use

We could allow reword on its own to simply reword the top commit?

That being said, since there's a simple-ish workaruond using "stop",
or "exec git commit --amend" I don't see this as being important
enough to worry about now.

Thanks,
Jake

Re: [PATCH 1/8] sequencer: introduce new commands to reset the revision

[Philip Oakley]

From: "Jacob Keller" 

On Thu, Jan 18, 2018 at 7:35 AM, Johannes Schindelin
 wrote:

This commit implements the commands to label, and to reset to, given
revisions. The syntax is:

label 
reset 

As a convenience shortcut, also to improve readability of the generated
todo list, a third command is introduced: bud. It simply resets to the
"onto" revision, i.e. the commit onto which we currently rebase.



The code looks good, but I'm a little wary of adding bud which
hard-codes a specific label. I suppose it does grant a bit of
readability to the resulting script... ? It doesn't seem that
important compared to use using "reset onto"? At least when
documenting this it should be made clear that the "onto" label is
special.

Thanks,
Jake.


I'd agree.

The special 'onto' label should be fully documented, and the commit message 
should indicate which patch actually defines it (and all its corner cases 
and fall backs if --onto isn't explicitly given..)


Likewise the choice of 'bud' should be explained with some nice phraseology 
indicating that we are growing the new flowering from the bud, otherwise the 
word is a bit too short and sudden for easy explanation.


Philip 

Re: [PATCH v2] diff: add --compact-summary option to complement --stat

[Ævar Arnfjörð Bjarmason]

On Thu, Jan 18 2018, Nguyễn Thái Ngọc Duy jotted:

> This is partly inspired by gerrit web interface which shows diffstat
> like this, e.g. with commit 0433d533f1 (notice the "A" column on the
> third line):
>
>
>  Documentation/merge-config.txt |  4 +
>  builtin/merge.c|  2 +
>A t/t5573-pull-verify-signatures.sh  | 81 ++
>  t/t7612-merge-verify-signatures.sh | 45 ++
>4 files changed, 132 insertions(+)

This feature is awesome. This has bothered me about --stat, but I
haven't done anything about it.

> In other words, certain information currently shown with --summary is
> embedded in the diffstat. This helps reading (all information of the
> same file in the same line instead of two) and can reduce the number of
> lines if you add/delete a lot of files.

Wait, isn't there a bug here in the existing --summary code, its
documentation says it'll show information "such as creations, renames
and mode changes".

But even though your --compact-summary shows that the file is being
added and its mode changed:

$ diff -ru <(./git-show --stat 0433d533f1) <(./git-show --stat 
--compact-summary 0433d533f1)
--- /dev/fd/63  2018-01-18 21:11:51.186570555 +
+++ /dev/fd/62  2018-01-18 21:11:51.186570555 +
@@ -14,8 +14,8 @@
   t: add tests for pull --verify-signatures
   merge: add config option for verifySignatures

- Documentation/merge-config.txt |  4 ++
- builtin/merge.c|  2 +
- t/t5573-pull-verify-signatures.sh  | 81 
++
- t/t7612-merge-verify-signatures.sh | 45 +
+ Documentation/merge-config.txt |  4 ++
+ builtin/merge.c|  2 +
+ A+x t/t5573-pull-verify-signatures.sh  | 81 
++
+ t/t7612-merge-verify-signatures.sh | 45 +++
  4 files changed, 132 insertions(+)

There is no difference between --stat with and without --summary on the
same commit, shouldn't it show "create mode [...]" ?

E.g. 95450bbbaa will do the trick for both:

$ diff -ru <(./git-show --stat 95450bbbaa) <(./git-show --stat --summary 
95450bbbaa)
--- /dev/fd/63  2018-01-18 21:14:20.770050599 +
+++ /dev/fd/62  2018-01-18 21:14:20.770050599 +
@@ -14,3 +14,4 @@
  git-svn.perl|  1 +
  t/t9169-git-svn-dcommit-crlf.sh | 27 +++
  2 files changed, 28 insertions(+)
+ create mode 100755 t/t9169-git-svn-dcommit-crlf.sh

$ diff -ru <(./git-show --stat --summary 95450bbbaa) <(./git-show --stat 
--compact-summary 95450bbbaa)
--- /dev/fd/63  2018-01-18 21:14:30.646016210 +
+++ /dev/fd/62  2018-01-18 21:14:30.646016210 +
@@ -11,7 +11,6 @@
 Reported-by: Brian Bennett 
 Signed-off-by: Eric Wong 

- git-svn.perl|  1 +
- t/t9169-git-svn-dcommit-crlf.sh | 27 +++
+ git-svn.perl|  1 +
+ A+x t/t9169-git-svn-dcommit-crlf.sh | 27 +++
  2 files changed, 28 insertions(+)
- create mode 100755 t/t9169-git-svn-dcommit-crlf.sh

> +--compact-summary::
> + Output a condensed summary of extended header information in
> + front of the file name part of diffstat. This option is
> + ignored if --stat is not specified.
> ++

If for some reason the lack of information about the commit under
--summary isn't a bug/fixable it makes sense to document the differences
here.

> +File creations or deletions are denoted wigth "A" or "D" respectively,

s/wigth/with/

> +optionally "+l" if it's a symlink, or "+x" if it's executable.
> +Mode changes are shown "M+x" or "M-x" for adding or removing

"Mode changes are shown as" is better worded.

Re: [PATCH 2/8] sequencer: introduce the `merge` command

[Johannes Schindelin]

Hi Jake,

On Thu, 18 Jan 2018, Jacob Keller wrote:

> On Thu, Jan 18, 2018 at 7:35 AM, Johannes Schindelin
>  wrote:
> > This patch is part of the effort to reimplement `--preserve-merges` with
> > a substantially improved design, a design that has been developed in the
> > Git for Windows project to maintain the dozens of Windows-specific patch
> > series on top of upstream Git.
> >
> > The previous patch implemented the `label`, `bud` and `reset` commands
> > to label commits and to reset to a labeled commits. This patch adds the
> > `merge` command, with the following syntax:
> >
> > merge   
> >
> > The  parameter in this instance is the *original* merge commit,
> > whose author and message will be used for the to-be-created merge
> > commit.
> >
> > The  parameter refers to the (possibly rewritten) revision to
> > merge. Let's see an example of a todo list:
> >
> > label onto
> >
> > # Branch abc
> > bud
> > pick deadbeef Hello, world!
> > label abc
> >
> > bud
> > pick cafecafe And now for something completely different
> > merge baaabaaa abc Merge the branch 'abc' into master
> >
> > To support creating *new* merges, i.e. without copying the commit
> > message from an existing commit, use the special value `-` as 
> > parameter (in which case the text after the  parameter is used as
> > commit message):
> >
> > merge - abc This will be the actual commit message of the merge
> >
> > This comes in handy when splitting a branch into two or more branches.
> >
> 
> Would it be possible to open the editor with the supplied text when
> there's no commit?  The text after  must be oneline only..

I actually want to avoid that because my main use case is fire-and-forget,
i.e. I want to edit only the todo list and then (barring any merge
conflicts) I do not want to edit anything anymore.

But I guess we could special-case the thing where `-` is specified as
"merge commit message provider" and an empty oneline is provided?

> It's difficult to reword merges because of the nature of rebase
> interactive, you can't just re-run the rebase command and use
> "reword".
> 
> I suppose you could cheat by putting in an "edit" command that let you
> create an empty commit with a message...

Or you could "cheat" by adding `exec git commit --amend`...

Seriously again, I have no good idea how to provide an equivalent to the
`reword` verb that would work on merge commits...

Anyone?

Ciao,
Dscho

Re: [PATCH 1/8] sequencer: introduce new commands to reset the revision

[Jacob Keller]

On Thu, Jan 18, 2018 at 1:13 PM, Johannes Schindelin
 wrote:
> Hi Jake,
>
> On Thu, 18 Jan 2018, Jacob Keller wrote:
>
>> On Thu, Jan 18, 2018 at 7:35 AM, Johannes Schindelin
>>  wrote:
>> > This commit implements the commands to label, and to reset to, given
>> > revisions. The syntax is:
>> >
>> > label 
>> > reset 
>> >
>> > As a convenience shortcut, also to improve readability of the generated
>> > todo list, a third command is introduced: bud. It simply resets to the
>> > "onto" revision, i.e. the commit onto which we currently rebase.
>> >
>>
>> The code looks good, but I'm a little wary of adding bud which
>> hard-codes a specific label. I suppose it does grant a bit of
>> readability to the resulting script... ? It doesn't seem that
>> important compared to use using "reset onto"? At least when
>> documenting this it should be made clear that the "onto" label is
>> special.
>
> Indeed, `bud` helped me more in the earlier times of the Git garden shears
> when I had to *introduce* branch structure into the long list of Git for
> Windows' patches.
>
> If there are no voices in favor of `bud` other than mine, I will remove
> support for it in the next iteration.
>
> Ciao,
> Dscho

After I saw more examples, I don't mind it. I do think it's not
strictly necessary, but it seemed to help me with the readability. My
only main concern is that it's limited to a special label.

Thanks,
Jake

Re: [PATCH 10/8] [DO NOT APPLY, but improve?] rebase--interactive: introduce "stop" command

[Jacob Keller]

On Thu, Jan 18, 2018 at 10:36 AM, Stefan Beller  wrote:
> Jake suggested using "x false" instead of "edit" for some corner cases.
>
> I do prefer using "x false" for all kinds of things such as stopping
> before a commit (edit only let's you stop after a commit), and the
> knowledge that "x false" does the least amount of actions behind my back.
>
> We should have that command as well, maybe?
>


I agree. I use "x false" very often, and I think stop is probably a
better solution since it avoids spawning an extra shell that will just
fail. Not sure if stop implies too much about "stop the whole thing"
as opposed to "stop here and let me do something manual", but I think
it's clear enough.

Thanks,
Jake

> Signed-off-by: Stefan Beller 
> ---
>  git-rebase--interactive.sh |  1 +
>  sequencer.c| 10 ++
>  2 files changed, 11 insertions(+)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 3cd7446d0b..9eac53f0c5 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -166,6 +166,7 @@ l, label = label current HEAD with a name
>  t, reset  = reset HEAD to a label
>  b, bud = reset HEAD to the revision labeled 'onto', no arguments
>  m, merge []* = create a merge commit using a given commit's 
> message
> +y, stay = stop for  shortcut for
>
>  These lines can be re-ordered; they are executed from top to bottom.
>  " | git stripspace --comment-lines >>"$todo"
> diff --git a/sequencer.c b/sequencer.c
> index 2b4e6b1232..4b3b9fe59d 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -782,6 +782,7 @@ enum todo_command {
> TODO_RESET,
> TODO_BUD,
> TODO_MERGE,
> +   TODO_STOP,
> /* commands that do nothing but are counted for reporting progress */
> TODO_NOOP,
> TODO_DROP,
> @@ -803,6 +804,7 @@ static struct {
> { 'l', "label" },
> { 't', "reset" },
> { 'b', "bud" },
> +   { 'y', "stay" },
> { 'm', "merge" },
> { 0,   "noop" },
> { 'd', "drop" },
> @@ -1307,6 +1309,12 @@ static int parse_insn_line(struct todo_item *item, 
> const char *bol, char *eol)
> return 0;
> }
>
> +   if (item->command == TODO_STOP) {
> +   item->commit = NULL;
> +   item->arg = "";
> +   item->arg_len = 0;
> +   }
> +
> end_of_object_name = (char *) bol + strcspn(bol, " \t\n");
> item->arg = end_of_object_name + strspn(end_of_object_name, " \t");
> item->arg_len = (int)(eol - item->arg);
> @@ -2407,6 +2415,8 @@ static int pick_commits(struct todo_list *todo_list, 
> struct replay_opts *opts)
> /* `current` will be incremented below */
> todo_list->current = -1;
> }
> +   } else if (item->command == TODO_STOP) {
> +   todo_list->current = -1;
> } else if (item->command == TODO_LABEL)
> res = do_label(item->arg, item->arg_len);
> else if (item->command == TODO_RESET)
> --
> 2.16.0.rc1.238.g530d649a79-goog
>

Re: [PATCH 9/8] [DO NOT APPLY, but squash?] git-rebase--interactive: clarify arguments

[Jacob Keller]

On Thu, Jan 18, 2018 at 10:36 AM, Stefan Beller  wrote:
> Up to now each command took a commit as its first argument and ignored
> the rest of the line (usually the subject of the commit)
>
> Now that we have commands that take different arguments, clarify each
> command by giving the argument list.
>
> Signed-off-by: Stefan Beller 
> ---
>  git-rebase--interactive.sh | 22 +++---
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 23184c77e8..3cd7446d0b 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -155,17 +155,17 @@ reschedule_last_action () {
>  append_todo_help () {
> gettext "
>  Commands:
> -p, pick = use commit
> -r, reword = use commit, but edit the commit message
> -e, edit = use commit, but stop for amending
> -s, squash = use commit, but meld into previous commit
> -f, fixup = like \"squash\", but discard this commit's log message
> -x, exec = run command (the rest of the line) using shell
> -d, drop = remove commit
> -l, label = label current HEAD with a name
> -t, reset = reset HEAD to a label
> -b, bud = reset HEAD to the revision labeled 'onto'
> -m, merge = create a merge commit using a given commit's message
> +p, pick  = use commit
> +r, reword  = use commit, but edit the commit message
> +e, edit  = use commit, but stop for amending
> +s, squash  = use commit, but meld into previous commit
> +f, fixup  = like \"squash\", but discard this commit's log message
> +x, exec  = run command (the rest of the line) using shell
> +d, drop  = remove commit
> +l, label = label current HEAD with a name
> +t, reset  = reset HEAD to a label
> +b, bud = reset HEAD to the revision labeled 'onto', no arguments
> +m, merge []* = create a merge commit using a given commit's 
> message

The merge arguments are complicated, i'm not sure how best to add
clarification here.. I think it might need its own paragraph, since it
takes something like:  * ?

Thanks,
Jake

>
>  These lines can be re-ordered; they are executed from top to bottom.
>  " | git stripspace --comment-lines >>"$todo"
> --
> 2.16.0.rc1.238.g530d649a79-goog
>

Re: [PATCH 1/8] sequencer: introduce new commands to reset the revision

[Johannes Schindelin]

Hi Jake,

On Thu, 18 Jan 2018, Jacob Keller wrote:

> On Thu, Jan 18, 2018 at 7:35 AM, Johannes Schindelin
>  wrote:
> > This commit implements the commands to label, and to reset to, given
> > revisions. The syntax is:
> >
> > label 
> > reset 
> >
> > As a convenience shortcut, also to improve readability of the generated
> > todo list, a third command is introduced: bud. It simply resets to the
> > "onto" revision, i.e. the commit onto which we currently rebase.
> >
> 
> The code looks good, but I'm a little wary of adding bud which
> hard-codes a specific label. I suppose it does grant a bit of
> readability to the resulting script... ? It doesn't seem that
> important compared to use using "reset onto"? At least when
> documenting this it should be made clear that the "onto" label is
> special.

Indeed, `bud` helped me more in the earlier times of the Git garden shears
when I had to *introduce* branch structure into the long list of Git for
Windows' patches.

If there are no voices in favor of `bud` other than mine, I will remove
support for it in the next iteration.

Ciao,
Dscho

RE: git 2.16.0 segfaults on clone of specific repo

[Randall S. Becker]

On January 18, 2018 3:56 PM, Aleks wrote:
> I found that git 2.16.0 segfaults on clone of vim-colorschemes repo.


Just tested on NonStop NSE and works fine here. Just an FYI now that we're on 
2.16.0.

Cheers,
Randall

-- Brief whoami:
  NonStop developer since approximately NonStop(2112884442)
  UNIX developer since approximately 421664400
-- In my real life, I talk too much.

git 2.16.0 segfaults on clone of specific repo

[Александр Булаев]

I found that git 2.16.0 segfaults on clone of vim-colorschemes repo.
See the log below.

alexbool@alexbool-osx ~/Documents> lldb -- git clone
https://github.com/flazz/vim-colorschemes.git
(lldb) target create "git"
Current executable set to 'git' (x86_64).
(lldb) settings set -- target.run-args  "clone"
"https://github.com/flazz/vim-colorschemes.git;
(lldb) run
Process 25643 launched: '/usr/local/bin/git' (x86_64)
Cloning into 'vim-colorschemes'...
remote: Counting objects: 1457, done.
remote: Total 1457 (delta 0), reused 0 (delta 0), pack-reused 1457
Receiving objects: 100% (1457/1457), 1.43 MiB | 289.00 KiB/s, done.
Resolving deltas: 100% (424/424), done.
Process 25643 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason =
EXC_BAD_ACCESS (code=1, address=0x48)
frame #0: 0x0001000f7c3d git`ce_match_stat_basic + 263
git`ce_match_stat_basic:
->  0x1000f7c3d <+263>: movq   0x48(%rax), %rsi
0x1000f7c41 <+267>: movl   $0x14, %edx
0x1000f7c46 <+272>: movq   %r14, %rdi
0x1000f7c49 <+275>: callq  0x1001659fa   ; symbol stub
for: memcmp
Target 0: (git) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason =
EXC_BAD_ACCESS (code=1, address=0x48)
  * frame #0: 0x0001000f7c3d git`ce_match_stat_basic + 263
frame #1: 0x0001000f7ae8 git`ie_match_stat + 108
frame #2: 0x0001000bbe07 git`checkout_entry + 266
frame #3: 0x000100144ff9 git`unpack_trees + 2317
frame #4: 0x000100017e13 git`cmd_clone + 5733
frame #5: 0x00011ea3 git`handle_builtin + 532
frame #6: 0x00011954 git`cmd_main + 263
frame #7: 0x000100079a28 git`main + 80
frame #8: 0x7fff64d76115 libdyld.dylib`start + 1
(lldb) ^D
alexbool@alexbool-osx ~/Documents> git --version
git version 2.16.0

OS: macOS 10.13.2
git installed from homebrew

Re: Git For Aix 6 and 7

[Peter Harris]

On 2018-01-18 09:47 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Thu, Jan 18 2018, raikrishna jotted:
> 
>> Hi Team,
>>
>> I have an urgent requirement to install Git client for Aix 6 and 7, could
>> you please help send me or navigate me to the correct url.
>> My present infrastructure comprise of Aix and Linux servers , I am
>> successfully using Git on Linux however I am struggling to find correct
>> package for AIX platform.
>>
>> Appreciate your quick response.
> 
> Hi raikrishna. The git-packagers list is a rather small list so perhaps
> someone on the general git list (CC'd) knows the answer to this.
> 
> I'm not aware of anyone providing binary git packages for AIX, but I
> don't use it so maybe they exist.

IBM provides git packages for AIX here:
https://www-03.ibm.com/systems/power/software/aix/linux/toolbox/alpha.html

It's git 2.12.0, so it's not the very latest version of git, but it's
not bad for AIX.

Peter Harris

[Question] format-patch along a specific path

[Randall S. Becker]

Hi all,

What I’m trying to do is to format a patch based on a single commit from
2.16.0 representing the NonStop port, for review and comments to the team.
Here is a partial (somewhat familiar) tree:

*   f1a482cd8 (HEAD -> randall_2.16, ituglib_release) NonStop port changes
for git 2.16.0.
|\
| | * b2a06dcfb (refs/stash) WIP on randall: 5653e94 Removed unnecessary
void* from hashmap.h that caused compile warnings
| |/
| * 5653e943a (randall) Removed unnecessary void* from hashmap.h that caused
compile warnings
| *   b318de9ca Merge branch 'ituglib_devel' into 2.16.0-rc1
| |\
| | * a4cdf025d (randall_2.13.5) Replaced read with xread in
transport-helper.c to fix SSIZE_MAX overun in t5509
| | | * b72dbd7fa (origin/pu) Merge branch 'ds/use-get-be64' into pu
| |_|/
|/| |
| | | * 896df04e4 (origin/next) Sync with 2.16
| |_|/
|/| |
* | | 2512f1544 (tag: v2.16.0, origin/master, origin/HEAD) Git 2.16
|/ /
* | c6c75c93a (tag: v2.16.0-rc2) Git 2.16-rc2

Trying the intuitive approach of git format-patch -1 f1a482cd8, I end up
with a patch for c6c75c93a, which I assume is a backtrack. If I use
2512f1544..f1a482cd8, I end up with a whole bunch of commits along the long
path rather than the short single hop path, (I actually want the short
path). The --base=2512f1544 option complains that the base commit shouldn't
be in revision list, which I also assume is an artifact of backtracking. Is
there a clean way in 2.16.0 to do this or should I redo the commit as a
squash and disconnect it from the other path?

Cheers,
Randall

-- Brief whoami:
  NonStop developer since approximately NonStop(2112884442)
 UNIX developer since approximately 421664400
-- In my real life, I talk too much.

Re: [PATCH v2] diff: add --compact-summary option to complement --stat

[Eric Sunshine]

On Thu, Jan 18, 2018 at 5:05 AM, Nguyễn Thái Ngọc Duy  wrote:
> [...]
> The new option --compact-summary implements this with a tweak to support
> mode change, which is shown in --summary too.
>
> For mode changes, executable bit is denoted as "M+x" or "M-x" when it's
> added or removed respectively. The same for when a regular file is
> replaced with a symlink "M+l" or the other way "M-l". This also applies
> to new files. New regulare files are "A", while new executable files or

s/regulare/regular/

> symlinks are "A+x" or "A+l".
> [...]
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> @@ -188,6 +188,17 @@ and accumulating child directory counts in the parent 
> directories:
> +--compact-summary::
> +   Output a condensed summary of extended header information in
> +   front of the file name part of diffstat. This option is
> +   ignored if --stat is not specified.

Rather than ignoring this option if --stat is not specified, a
different approach would be for --compact-summary to imply --stat.

Also, per documentation:

--stat[=[,[,]]]::

These parameters can also be set individually with `--stat-width=`,
`--stat-name-width=` and `--stat-count=`.

One wonders if "compact" could be another modifier recognized by --stat.

(Genuine questions/observations; I haven't formed strong opinions either way.)

> diff --git a/diff.c b/diff.c
> @@ -2287,6 +2289,18 @@ static void show_stats(struct diffstat_t *data, struct 
> diff_options *options)
> +   for (i = 0; (i < count) && (i < data->nr); i++) {

Noisy extra parentheses...

for (i = 0; i < count && i < data->nr; i++) {

perhaps? (Not at all worth a re-roll.)

> +   const struct diffstat_file *file = data->files[i];
> +   int len;
> +
> +   if (!file->status_code)
> +   continue;
> +   len = strlen(file->status_code) + 1;

The +1 is for the space following the status code? (Reading ahead,
that seems to be the case.)

len = strlen(file->status_code) + strlen(" ");

perhaps? (Probably not worth a re-roll.)

> +   if (len > max_status_len)
> +   max_status_len = len;
> +   }
> +
> @@ -2383,6 +2397,8 @@ static void show_stats(struct diffstat_t *data, struct 
> diff_options *options)
>   options->stat_name_width < max_len) ?
> options->stat_name_width : max_len;
>
> +   name_width += max_status_len;

I wonder if it would be clearer to account for the space after the the
status code here rather than above when it was not obvious what +1 was
for.

name_width += max_status_len + strlen(" ");

(and drop the earlier +1)

[PATCH 9/8] [DO NOT APPLY, but squash?] git-rebase--interactive: clarify arguments

[Stefan Beller]

Up to now each command took a commit as its first argument and ignored
the rest of the line (usually the subject of the commit)

Now that we have commands that take different arguments, clarify each
command by giving the argument list.

Signed-off-by: Stefan Beller 
---
 git-rebase--interactive.sh | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 23184c77e8..3cd7446d0b 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -155,17 +155,17 @@ reschedule_last_action () {
 append_todo_help () {
gettext "
 Commands:
-p, pick = use commit
-r, reword = use commit, but edit the commit message
-e, edit = use commit, but stop for amending
-s, squash = use commit, but meld into previous commit
-f, fixup = like \"squash\", but discard this commit's log message
-x, exec = run command (the rest of the line) using shell
-d, drop = remove commit
-l, label = label current HEAD with a name
-t, reset = reset HEAD to a label
-b, bud = reset HEAD to the revision labeled 'onto'
-m, merge = create a merge commit using a given commit's message
+p, pick  = use commit
+r, reword  = use commit, but edit the commit message
+e, edit  = use commit, but stop for amending
+s, squash  = use commit, but meld into previous commit
+f, fixup  = like \"squash\", but discard this commit's log message
+x, exec  = run command (the rest of the line) using shell
+d, drop  = remove commit
+l, label = label current HEAD with a name
+t, reset  = reset HEAD to a label
+b, bud = reset HEAD to the revision labeled 'onto', no arguments
+m, merge []* = create a merge commit using a given commit's 
message
 
 These lines can be re-ordered; they are executed from top to bottom.
 " | git stripspace --comment-lines >>"$todo"
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 9, 10/8] interactive rebase feedback

[Stefan Beller]

I thought of writing a reply, but instead I wrote it in patch form.

Stefan Beller (2):
  [DO NOT APPLY, but squash?] git-rebase--interactive: clarify arguments
  [DO NOT APPLY, but improve?] rebase--interactive: introduce "stop"
command

 git-rebase--interactive.sh | 23 ---
 sequencer.c| 10 ++
 2 files changed, 22 insertions(+), 11 deletions(-)

-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 10/8] [DO NOT APPLY, but improve?] rebase--interactive: introduce "stop" command

[Stefan Beller]

Jake suggested using "x false" instead of "edit" for some corner cases.

I do prefer using "x false" for all kinds of things such as stopping
before a commit (edit only let's you stop after a commit), and the
knowledge that "x false" does the least amount of actions behind my back.

We should have that command as well, maybe?

Signed-off-by: Stefan Beller 
---
 git-rebase--interactive.sh |  1 +
 sequencer.c| 10 ++
 2 files changed, 11 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3cd7446d0b..9eac53f0c5 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -166,6 +166,7 @@ l, label = label current HEAD with a name
 t, reset  = reset HEAD to a label
 b, bud = reset HEAD to the revision labeled 'onto', no arguments
 m, merge []* = create a merge commit using a given commit's 
message
+y, stay = stop for  shortcut for
 
 These lines can be re-ordered; they are executed from top to bottom.
 " | git stripspace --comment-lines >>"$todo"
diff --git a/sequencer.c b/sequencer.c
index 2b4e6b1232..4b3b9fe59d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -782,6 +782,7 @@ enum todo_command {
TODO_RESET,
TODO_BUD,
TODO_MERGE,
+   TODO_STOP,
/* commands that do nothing but are counted for reporting progress */
TODO_NOOP,
TODO_DROP,
@@ -803,6 +804,7 @@ static struct {
{ 'l', "label" },
{ 't', "reset" },
{ 'b', "bud" },
+   { 'y', "stay" },
{ 'm', "merge" },
{ 0,   "noop" },
{ 'd', "drop" },
@@ -1307,6 +1309,12 @@ static int parse_insn_line(struct todo_item *item, const 
char *bol, char *eol)
return 0;
}
 
+   if (item->command == TODO_STOP) {
+   item->commit = NULL;
+   item->arg = "";
+   item->arg_len = 0;
+   }
+
end_of_object_name = (char *) bol + strcspn(bol, " \t\n");
item->arg = end_of_object_name + strspn(end_of_object_name, " \t");
item->arg_len = (int)(eol - item->arg);
@@ -2407,6 +2415,8 @@ static int pick_commits(struct todo_list *todo_list, 
struct replay_opts *opts)
/* `current` will be incremented below */
todo_list->current = -1;
}
+   } else if (item->command == TODO_STOP) {
+   todo_list->current = -1;
} else if (item->command == TODO_LABEL)
res = do_label(item->arg, item->arg_len);
else if (item->command == TODO_RESET)
-- 
2.16.0.rc1.238.g530d649a79-goog

Re: [RFC PATCH 2/2] http: support omitting data from traces

[Eric Sunshine]

On Wed, Jan 17, 2018 at 7:34 PM, Jonathan Tan  wrote:
> GIT_TRACE_CURL provides a way to debug what is being sent and received
> over HTTP, with automatic redaction of sensitive information. But it
> also logs data transmissions, which significantly increases the log file
> size, sometimes unnecessarily. Add an option "GIT_TRACE_CURL_NO_DATA" to
> allow the user to omit such data transmissions.
>
> Signed-off-by: Jonathan Tan 
> ---
>  http.c  | 27 +++
>  t/t5551-http-fetch-smart.sh | 12 
>  2 files changed, 31 insertions(+), 8 deletions(-)

Please document GIT_TRACE_CURL_NO_DATA in Documentation/git.txt.

Re: [RFC PATCH 1/2] http: support cookie redaction when tracing

[Eric Sunshine]

On Wed, Jan 17, 2018 at 7:34 PM, Jonathan Tan  wrote:
> When using GIT_TRACE_CURL, Git already redacts the "Authorization:" and
> "Proxy-Authorization:" HTTP headers. Extend this redaction to a
> user-specified list of cookies, specified through the
> "GIT_REDACT_COOKIES" environment variable.
>
> Signed-off-by: Jonathan Tan 
> ---
>  http.c  | 55 
> +
>  t/t5551-http-fetch-smart.sh | 12 ++
>  2 files changed, 67 insertions(+)

Please document GIT_REDACT_COOKIES in Documentation/git.txt.

> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> @@ -364,5 +364,17 @@ test_expect_success 'custom http headers' '
> +test_expect_success 'GIT_REDACT_COOKIES redacts cookies' '
> +   rm -rf clone &&
> +   echo "Set-Cookie: NotSecret=Foo" >cookies &&
> +   echo "Set-Cookie: Secret=Bar" >>cookies &&
> +   GIT_TRACE_CURL=true GIT_REDACT_COOKIES=Secret,AnotherSecret \
> +   git -c "http.cookieFile=$(pwd)/cookies" clone \
> +   $HTTPD_URL/smart/repo.git clone 2>err &&
> +   grep "Cookie:.*NotSecret=Foo" err &&
> +   grep "Cookie:.*Secret=" err &&
> +   ! grep "Cookie:.*Secret=Bar" err
> +'

The looseness of these grep expressions (/Cookie:.*Secret/ also
matches "Cookie: NotSecret", for instance) requires extra
concentration on the part of the reader to see that you do indeed
cover all cases. I wonder, therefore, if it would be better to tighten
them to instead match the exact string.

Also, after reading the implementation, I had expected to see testing
of the "Cookie: foo=bar; cow=moo" case, as well as the handled corner
cases, such as missing missing "=" and missing value after "=".

Re: [PATCH 00/11] Some fixes and bunch of object_id conversions

[Jonathan Tan]

On Thu, 18 Jan 2018 15:50:52 +0100
Patryk Obara  wrote:

> Patch 1 is not directly related to object_id conversions but helped with
> debugging t5540, which kept failing on master for me (spoiler: it was Fedora
> fault).  It helps with debugging of failing git-push over HTTP in case of
> internal server error, so I think it might be worthwhile.

This patch seems reasonable to me - it is a little strange that,
currently, an error message is printed upon an XML error and upon
failure of start_active_slot(), but not when the slot cannot be run.
This patch fills the gap.

> Patch 2 is a small adjustment to .clang-format, which prevents unnecessary
> line breaks after function return type.

I'm not very familiar with clang-format, so no comment from me here.

> Patch 6 is a tiny fix in oidclr function.

Ah, good catch. I would write the memset line as
"memset(oid, 0, sizeof(*oid))", but the existing one is fine too.

> All other patches are progressive conversions to struct object_id with some
> formatting fixes sprinkled in. These should be somewhat uncontroversial, I 
> hope.

I've looked at those and they appear to be obviously correct.

Re: [PATCH 0/8] rebase -i: offer to recreate merge commits

[Jacob Keller]

On Thu, Jan 18, 2018 at 7:35 AM, Johannes Schindelin
 wrote:
> Once upon a time, I dreamt of an interactive rebase that would not
> flatten branch structure, but instead recreate the commit topology
> faithfully.
>
> My original attempt was --preserve-merges, but that design was so
> limited that I did not even enable it in interactive mode.
>
> Subsequently, it *was* enabled in interactive mode, with the predictable
> consequences: as the --preserve-merges design does not allow for
> specifying the parents of merge commits explicitly, all the new commits'
> parents are defined *implicitly* by the previous commit history, and
> hence it is *not possible to even reorder commits*.
>
> This design flaw cannot be fixed. Not without a complete re-design, at
> least. This patch series offers such a re-design.
>
> Think of --recreate-merges as "--preserve-merges done right". It
> introduces new verbs for the todo list, `label`, `reset` and `merge`.
> For a commit topology like this:
>
> A - B - C
>   \   /
> D
>
> the generated todo list would look like this:
>
> # branch D
> pick 0123 A
> label branch-point
> pick 1234 D
> label D
>
> reset branch-point
> pick 2345 B
> merge 3456 D C
>
> There are more patches in the pipeline, based on this patch series, but
> left for later in the interest of reviewable patch series: one mini
> series to use the sequencer even for `git rebase -i --root`, and another
> one to add support for octopus merges to --recreate-merges.
>
>

I've been looking forward to seeing this hit the list! Overall I don't
think I have any major complaints, except to make sure the special
label "onto" is documented.

I think it's possible to "reword" or "edit" the merge commit by
inserting "x false" after the merge commnd to halt the rebase and let
the user perform commands to edit, so that should work well.

Thanks for taking the time to make this a reality!

Thanks,
Jake

> Johannes Schindelin (8):
>   sequencer: introduce new commands to reset the revision
>   sequencer: introduce the `merge` command
>   sequencer: fast-forward merge commits, if possible
>   rebase-helper --make-script: introduce a flag to recreate merges
>   rebase: introduce the --recreate-merges option
>   sequencer: handle autosquash and post-rewrite for merge commands
>   pull: accept --rebase=recreate to recreate the branch topology
>   rebase -i: introduce --recreate-merges=no-rebase-cousins
>
>  Documentation/config.txt   |   8 +
>  Documentation/git-pull.txt |   5 +-
>  Documentation/git-rebase.txt   |  13 +-
>  builtin/pull.c |  14 +-
>  builtin/rebase--helper.c   |  13 +-
>  builtin/remote.c   |   2 +
>  contrib/completion/git-completion.bash |   4 +-
>  git-rebase--interactive.sh |   6 +
>  git-rebase.sh  |  16 +
>  refs.c |   3 +-
>  sequencer.c| 697 
> -
>  sequencer.h|   9 +
>  t/t3430-rebase-recreate-merges.sh  | 208 ++
>  13 files changed, 977 insertions(+), 21 deletions(-)
>  create mode 100755 t/t3430-rebase-recreate-merges.sh
>
>
> base-commit: 2512f15446149235156528dafbe75930c712b29e
> Published-As: https://github.com/dscho/git/releases/tag/recreate-merges-v1
> Fetch-It-Via: git fetch https://github.com/dscho/git recreate-merges-v1
> --
> 2.15.1.windows.2.1430.ga56c4f9e2a9
>

Re: [PATCH 6/8] sequencer: handle autosquash and post-rewrite for merge commands

[Jacob Keller]

On Thu, Jan 18, 2018 at 7:35 AM, Johannes Schindelin
 wrote:
> In the previous patches, we implemented the basic functionality of the
> `git rebase -i --recreate-merges` command, in particular the `merge`
> command to create merge commits in the sequencer.
>
> The interactive rebase is a lot more these days, though, than a simple
> cherry-pick in a loop. For example, it calls the post-rewrite hook (if
> any) after rebasing with a mapping of the old->new commits. And the
> interactive rebase also supports the autosquash mode, where commits
> whose oneline is of the form `fixup! ` or `squash! `
> are rearranged to amend commits whose oneline they match.
>
> This patch implements the post-rewrite and autosquash handling for the
> `merge` command we just introduced. The other commands that were added
> recently (`label`, `reset` and `bud`) do not create new commits,
> therefore post-rewrite & autosquash do not need to handle them.
>
> Signed-off-by: Johannes Schindelin 
> ---
>  refs.c|  3 ++-
>  sequencer.c   | 10 +++---
>  t/t3430-rebase-recreate-merges.sh | 39 
> +++
>  3 files changed, 48 insertions(+), 4 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 20ba82b4343..e8b84c189ff 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -600,7 +600,8 @@ int dwim_log(const char *str, int len, struct object_id 
> *oid, char **log)
>  static int is_per_worktree_ref(const char *refname)
>  {
> return !strcmp(refname, "HEAD") ||
> -   starts_with(refname, "refs/bisect/");
> +   starts_with(refname, "refs/bisect/") ||
> +   starts_with(refname, "refs/rewritten/");
>  }

Would this part make more sense to move into the commit that
introduces writing these refs, or does it only matter once you start
this step here?

>
>  static int is_pseudoref_syntax(const char *refname)
> diff --git a/sequencer.c b/sequencer.c
> index 1bef16647b4..b63bfb9a141 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2413,10 +2413,13 @@ static int pick_commits(struct todo_list *todo_list, 
> struct replay_opts *opts)
> res = do_reset(item->arg, item->arg_len);
> else if (item->command == TODO_BUD)
> res = do_reset("onto", 4);
> -   else if (item->command == TODO_MERGE)
> +   else if (item->command == TODO_MERGE) {
> res = do_merge(item->commit,
>item->arg, item->arg_len, opts);
> -   else if (!is_noop(item->command))
> +   if (item->commit)
> +   record_in_rewritten(>commit->object.oid,
> +   peek_command(todo_list, 
> 1));
> +   } else if (!is_noop(item->command))
> return error(_("unknown command %d"), item->command);
>
> todo_list->current++;
> @@ -3556,7 +3559,8 @@ int rearrange_squash(void)
> struct subject2item_entry *entry;
>
> next[i] = tail[i] = -1;
> -   if (item->command >= TODO_EXEC) {
> +   if (item->command >= TODO_EXEC &&
> +   (item->command != TODO_MERGE || !item->commit)) {
> subjects[i] = NULL;
> continue;
> }
> diff --git a/t/t3430-rebase-recreate-merges.sh 
> b/t/t3430-rebase-recreate-merges.sh
> index 46ae52f88b3..76e615bd7c1 100755
> --- a/t/t3430-rebase-recreate-merges.sh
> +++ b/t/t3430-rebase-recreate-merges.sh
> @@ -143,4 +143,43 @@ test_expect_success 'with a branch tip that was 
> cherry-picked already' '
> EOF
>  '
>
> +test_expect_success 'refs/rewritten/* is worktree-local' '
> +   git worktree add wt &&
> +   cat >wt/script-from-scratch <<-\EOF &&
> +   label xyz
> +   exec GIT_DIR=../.git git rev-parse --verify refs/rewritten/xyz >a || :
> +   exec git rev-parse --verify refs/rewritten/xyz >b
> +   EOF
> +
> +   test_config -C wt sequence.editor \""$PWD"/replace-editor.sh\" &&
> +   git -C wt rebase -i HEAD &&
> +   test_must_be_empty wt/a &&
> +   test_cmp_rev HEAD "$(cat wt/b)"
> +'
> +

Same for the test here, I can't figure out why this is necessary in
this patch as opposed to the patch which first introduced the
refs/rewritten/ refs.

> +test_expect_success 'post-rewrite hook and fixups work for merges' '
> +   git checkout -b post-rewrite &&
> +   test_commit same1 &&
> +   git reset --hard HEAD^ &&
> +   test_commit same2 &&
> +   git merge -m "to fix up" same1 &&
> +   echo same old same old >same2.t &&
> +   test_tick &&
> +   git commit --fixup HEAD same2.t &&
> +   fixup="$(git rev-parse HEAD)" &&
> +
> +   mkdir -p .git/hooks &&
> +   test_when_finished "rm .git/hooks/post-rewrite" &&
> 

Re: [PATCH 2/8] sequencer: introduce the `merge` command

[Jacob Keller]

On Thu, Jan 18, 2018 at 7:35 AM, Johannes Schindelin
 wrote:
> This patch is part of the effort to reimplement `--preserve-merges` with
> a substantially improved design, a design that has been developed in the
> Git for Windows project to maintain the dozens of Windows-specific patch
> series on top of upstream Git.
>
> The previous patch implemented the `label`, `bud` and `reset` commands
> to label commits and to reset to a labeled commits. This patch adds the
> `merge` command, with the following syntax:
>
> merge   
>
> The  parameter in this instance is the *original* merge commit,
> whose author and message will be used for the to-be-created merge
> commit.
>
> The  parameter refers to the (possibly rewritten) revision to
> merge. Let's see an example of a todo list:
>
> label onto
>
> # Branch abc
> bud
> pick deadbeef Hello, world!
> label abc
>
> bud
> pick cafecafe And now for something completely different
> merge baaabaaa abc Merge the branch 'abc' into master
>
> To support creating *new* merges, i.e. without copying the commit
> message from an existing commit, use the special value `-` as 
> parameter (in which case the text after the  parameter is used as
> commit message):
>
> merge - abc This will be the actual commit message of the merge
>
> This comes in handy when splitting a branch into two or more branches.
>

Would it be possible to open the editor with the supplied text when
there's no commit? The text after  must be oneline only..

It's difficult to reword merges because of the nature of rebase
interactive, you can't just re-run the rebase command and use
"reword".

I suppose you could cheat by putting in an "edit" command that let you
create an empty commit with a message...

Thanks,
Jake

Re: [PATCH] enable core.fsyncObjectFiles by default

[Christoph Hellwig]

[adding Chris to the Cc list - this is about the awful ext3 data=ordered
behavior of syncing the whole file system data and metadata on each
fsync]

On Wed, Jan 17, 2018 at 03:57:53PM -0800, Linus Torvalds wrote:
> On Wed, Jan 17, 2018 at 3:52 PM, Theodore Ts'o  wrote:
> >
> > Well, let's be fair; this is something *ext3* got wrong, and it was
> > the default file system back them.
> 
> I'm pretty sure reiserfs and btrfs did too..

I'm pretty sure btrfs never did, and reiserfs at least looks like
it currently doesn't but I'd have to dig into history to check if
it ever did.

Re: [PATCH 1/8] sequencer: introduce new commands to reset the revision

[Jacob Keller]

On Thu, Jan 18, 2018 at 7:35 AM, Johannes Schindelin
 wrote:
> This commit implements the commands to label, and to reset to, given
> revisions. The syntax is:
>
> label 
> reset 
>
> As a convenience shortcut, also to improve readability of the generated
> todo list, a third command is introduced: bud. It simply resets to the
> "onto" revision, i.e. the commit onto which we currently rebase.
>

The code looks good, but I'm a little wary of adding bud which
hard-codes a specific label. I suppose it does grant a bit of
readability to the resulting script... ? It doesn't seem that
important compared to use using "reset onto"? At least when
documenting this it should be made clear that the "onto" label is
special.

Thanks,
Jake.

Re: Bug Report: Subtrees and GPG Signed Commits

[Stephen R Guglielmo]

Hi, just following up on this bug report. I have not heard back. Is
there additional information that's needed? Is there a better place to
file bug reports?

Thanks

On Sat, Jan 6, 2018 at 5:45 PM, Stephen R Guglielmo
 wrote:
> Hi all,
>
> I've noticed an issue regarding the use of `git subtree add` and `git
> subtree pull` when the subtree repository's commit (either HEAD or
> whatever commit specified by the subtree command) is signed with GPG.
> It seems to work properly if the commit is not signed but previous
> commits are.
>
> The gist of the issue is that `git subtree add` does not add the
> subree properly and a "fatal: Not a valid object name" error is
> thrown. Running `git subtree pull` does not pull any upstream changes
> after that ("'subtree' was never added").
>
> I have not done extensive testing, however, below are instructions to
> reproduce the issue. This was tested using git version 2.15.1
> installed via Homebrew on MacOS. I did not test with the built-in
> version of git on MacOS.
>
> Thanks,
> Steve
>
> # Create a new repository
> mkdir repoA && cd repoA
> git init
> echo "Test File in Repo A" > FileA
> git add -A && git commit -m 'Initial commit in repo A'
>
> # Create a second repository
> cd .. && mkdir repoB && cd repoB
> git init
> echo "Test File in Repo B" > FileB
> git add -A && git commit -m 'Initial commit in repo B'
>
> # Create a signed commit in repo B
> echo "Signed Commit" >> FileB
> git commit -a -S  -m 'Signed commit in repo B'
>
> # Now, add repoB as a subtree of RepoA
> cd ../repoA
> git subtree add --prefix repoB_subtree/ ../repoB/ master --squash
> # Output:
> git fetch ../repoB/ master
> warning: no common commits
> remote: Counting objects: 6, done.
> remote: Compressing objects: 100% (2/2), done.
> remote: Total 6 (delta 0), reused 0 (delta 0)
> Unpacking objects: 100% (6/6), done.
> From ../repoB
>  * branchmaster -> FETCH_HEAD
> fatal: Not a valid object name gpg: Signature made Sat Jan  6 17:38:31 2018 
> EST
> gpg:using RSA key 6900E9CFDD39B6A741D601F50999759F2DCF3E7C
> gpg: Good signature from "Stephen Robert Guglielmo (Temple University
> Computer Services) " [ultimate]
> Primary key fingerprint: 6900 E9CF DD39 B6A7 41D6  01F5 0999 759F 2DCF 3E7C
> 4b700b1a4ebb9e2c1011aafd6b0f720b38f059a4
> # Note, git exits with status 128 at this point.
>
> # FileB was in fact added and staged to repoA, despite the "fatal"
> above. Commit it:
> git commit -m 'Add repoB subtree'
>
> # Ok, let's make another commit in repoB and try a `subtree pull`
> instead of `subtree add`
> cd ../repoB
> echo "Another Line" >> FileB
> git commit -a -S -m 'Another signed commit'
> cd ../repoA
> git subtree pull --prefix repoB_subtree/ ../repoB master --squash
> # Output:
> warning: no common commits
> remote: Counting objects: 9, done.
> remote: Compressing objects: 100% (3/3), done.
> remote: Total 9 (delta 0), reused 0 (delta 0)
> Unpacking objects: 100% (9/9), done.
> From ../repoB
>  * branchmaster -> FETCH_HEAD
> Can't squash-merge: 'repoB_subtree' was never added.
> # Note, git exits with status 1 at this point.
>
> # RepoB's third commit ('Another signed commit') is not pulled into
> the subree in repo A.
> # This can be verified by running a diff:
> diff -qr --exclude ".git" repoB_subtree ../repoB
> # Output:
> Files repoB_subtree/FileB and ../repoB/FileB differ

[PATCH 8/8] rebase -i: introduce --recreate-merges=no-rebase-cousins

[Johannes Schindelin]

This one is a bit tricky to explain, so let's try with a diagram:

C
  /   \
A - B - E - F
  \   /
D

To illustrate what this new mode is all about, let's consider what
happens upon `git rebase -i --recreate-merges B`, in particular to
the commit `D`. In the default mode, the new branch structure is:

   --- C' --
  / \
A - B -- E' - F'
  \/
D'

This is not really preserving the branch topology from before! The
reason is that the commit `D` does not have `B` as ancestor, and
therefore it gets rebased onto `B`.

However, when recreating branch structure, there are legitimate use
cases where one might want to preserve the branch points of commits that
do not descend from the  commit that was passed to the rebase
command, e.g. when a branch from core Git's `next` was merged into Git
for Windows' master we will not want to rebase those commits on top of a
Windows-specific commit. In the example above, the desired outcome would
look like this:

   --- C' --
  / \
A - B -- E' - F'
  \/
   -- D' --

Let's introduce the term "cousins" for such commits ("D" in the
example), and the "no-rebase-cousins" mode of the merge-recreating
rebase, to help those use cases.

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-rebase.txt  |  7 ++-
 builtin/rebase--helper.c  |  9 -
 git-rebase--interactive.sh|  1 +
 git-rebase.sh | 12 +++-
 sequencer.c   |  4 
 sequencer.h   |  8 
 t/t3430-rebase-recreate-merges.sh | 23 +++
 7 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 1d061373288..ac07a5c3fc9 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -368,10 +368,15 @@ The commit list format can be changed by setting the 
configuration option
 rebase.instructionFormat.  A customized instruction format will automatically
 have the long commit hash prepended to the format.
 
---recreate-merges::
+--recreate-merges[=(rebase-cousins|no-rebase-cousins)]::
Recreate merge commits instead of flattening the history by replaying
merges. Merge conflict resolutions or manual amendments to merge
commits are not preserved.
++
+By default, or when `rebase-cousins` was specified, commits which do not have
+`` as direct ancestor are rebased onto `` (or ``,
+if specified). If the `rebase-cousins` mode is turned off, such commits will
+retain their original branch point.
 
 -p::
 --preserve-merges::
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index a34ab5c0655..ef08fef4d14 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -13,7 +13,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
unsigned flags = 0, keep_empty = 0, recreate_merges = 0;
-   int abbreviate_commands = 0;
+   int abbreviate_commands = 0, no_rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
@@ -23,6 +23,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
OPT_BOOL(0, "keep-empty", _empty, N_("keep empty 
commits")),
OPT_BOOL(0, "recreate-merges", _merges, N_("recreate 
merge commits")),
+   OPT_BOOL(0, "no-rebase-cousins", _rebase_cousins,
+N_("keep original branch points of cousins")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -57,8 +59,13 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0;
flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
flags |= recreate_merges ? TODO_LIST_RECREATE_MERGES : 0;
+   flags |= no_rebase_cousins > 0 ? TODO_LIST_NO_REBASE_COUSINS : 0;
flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
 
+   if (no_rebase_cousins >= 0&& !recreate_merges)
+   warning(_("--[no-]rebase-cousins has no effect without "
+ "--recreate-merges"));
+
if (command == CONTINUE && argc == 1)
return !!sequencer_continue();
if (command == ABORT && argc == 1)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3459ec5a018..23184c77e88 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -901,6 +901,7 @@ if test t != "$preserve_merges"
 then
git rebase--helper --make-script ${keep_empty:+--keep-empty} \

[PATCH 6/8] sequencer: handle autosquash and post-rewrite for merge commands

[Johannes Schindelin]

In the previous patches, we implemented the basic functionality of the
`git rebase -i --recreate-merges` command, in particular the `merge`
command to create merge commits in the sequencer.

The interactive rebase is a lot more these days, though, than a simple
cherry-pick in a loop. For example, it calls the post-rewrite hook (if
any) after rebasing with a mapping of the old->new commits. And the
interactive rebase also supports the autosquash mode, where commits
whose oneline is of the form `fixup! ` or `squash! `
are rearranged to amend commits whose oneline they match.

This patch implements the post-rewrite and autosquash handling for the
`merge` command we just introduced. The other commands that were added
recently (`label`, `reset` and `bud`) do not create new commits,
therefore post-rewrite & autosquash do not need to handle them.

Signed-off-by: Johannes Schindelin 
---
 refs.c|  3 ++-
 sequencer.c   | 10 +++---
 t/t3430-rebase-recreate-merges.sh | 39 +++
 3 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/refs.c b/refs.c
index 20ba82b4343..e8b84c189ff 100644
--- a/refs.c
+++ b/refs.c
@@ -600,7 +600,8 @@ int dwim_log(const char *str, int len, struct object_id 
*oid, char **log)
 static int is_per_worktree_ref(const char *refname)
 {
return !strcmp(refname, "HEAD") ||
-   starts_with(refname, "refs/bisect/");
+   starts_with(refname, "refs/bisect/") ||
+   starts_with(refname, "refs/rewritten/");
 }
 
 static int is_pseudoref_syntax(const char *refname)
diff --git a/sequencer.c b/sequencer.c
index 1bef16647b4..b63bfb9a141 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2413,10 +2413,13 @@ static int pick_commits(struct todo_list *todo_list, 
struct replay_opts *opts)
res = do_reset(item->arg, item->arg_len);
else if (item->command == TODO_BUD)
res = do_reset("onto", 4);
-   else if (item->command == TODO_MERGE)
+   else if (item->command == TODO_MERGE) {
res = do_merge(item->commit,
   item->arg, item->arg_len, opts);
-   else if (!is_noop(item->command))
+   if (item->commit)
+   record_in_rewritten(>commit->object.oid,
+   peek_command(todo_list, 1));
+   } else if (!is_noop(item->command))
return error(_("unknown command %d"), item->command);
 
todo_list->current++;
@@ -3556,7 +3559,8 @@ int rearrange_squash(void)
struct subject2item_entry *entry;
 
next[i] = tail[i] = -1;
-   if (item->command >= TODO_EXEC) {
+   if (item->command >= TODO_EXEC &&
+   (item->command != TODO_MERGE || !item->commit)) {
subjects[i] = NULL;
continue;
}
diff --git a/t/t3430-rebase-recreate-merges.sh 
b/t/t3430-rebase-recreate-merges.sh
index 46ae52f88b3..76e615bd7c1 100755
--- a/t/t3430-rebase-recreate-merges.sh
+++ b/t/t3430-rebase-recreate-merges.sh
@@ -143,4 +143,43 @@ test_expect_success 'with a branch tip that was 
cherry-picked already' '
EOF
 '
 
+test_expect_success 'refs/rewritten/* is worktree-local' '
+   git worktree add wt &&
+   cat >wt/script-from-scratch <<-\EOF &&
+   label xyz
+   exec GIT_DIR=../.git git rev-parse --verify refs/rewritten/xyz >a || :
+   exec git rev-parse --verify refs/rewritten/xyz >b
+   EOF
+
+   test_config -C wt sequence.editor \""$PWD"/replace-editor.sh\" &&
+   git -C wt rebase -i HEAD &&
+   test_must_be_empty wt/a &&
+   test_cmp_rev HEAD "$(cat wt/b)"
+'
+
+test_expect_success 'post-rewrite hook and fixups work for merges' '
+   git checkout -b post-rewrite &&
+   test_commit same1 &&
+   git reset --hard HEAD^ &&
+   test_commit same2 &&
+   git merge -m "to fix up" same1 &&
+   echo same old same old >same2.t &&
+   test_tick &&
+   git commit --fixup HEAD same2.t &&
+   fixup="$(git rev-parse HEAD)" &&
+
+   mkdir -p .git/hooks &&
+   test_when_finished "rm .git/hooks/post-rewrite" &&
+   echo "cat >actual" | write_script .git/hooks/post-rewrite &&
+
+   test_tick &&
+   git rebase -i --autosquash --recreate-merges HEAD^^^ &&
+   printf "%s %s\n%s %s\n%s %s\n%s %s\n" >expect $(git rev-parse \
+   $fixup^^2 HEAD^2 \
+   $fixup^^ HEAD^ \
+   $fixup^ HEAD \
+   $fixup HEAD) &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.15.1.windows.2.1430.ga56c4f9e2a9

[PATCH 5/8] rebase: introduce the --recreate-merges option

[Johannes Schindelin]

Once upon a time, this here developer thought: wouldn't it be nice if,
say, Git for Windows' patches on top of core Git could be represented as
a thicket of branches, and be rebased on top of core Git in order to
maintain a cherry-pick'able set of patch series?

The original attempt at an answer was: git rebase --preserve-merges.

However, that experiment was never intended as an interactive option,
and it only piggy-backed on git rebase --interactive because that
command's implementation looked already very, very familiar: it was
designed by the same person who designed --preserve-merges: yours truly.

Some time later, some other developer (I am looking at you, Andreas!
;-)) decided that it would be a good idea to allow --preserve-merges to
be combined with --interactive (with caveats!) and the Git maintainer
(well, the interim Git maintainer during Junio's absence, that is)
agreed, and that is when the glamor of the --preserve-merges design
started to fall apart rather quickly and unglamorously.

The reason? In --preserve-merges mode, the parents of a merge commit (or
for that matter, of *any* commit) were not stated explicitly, but were
*implied* by the commit name passed to the `pick` command.

This made it impossible, for example, to reorder commits. Not to mention
to flatten the branch topology or, deity forbid, to split topic branches
into two.

Alas, these shortcomings also prevented that mode (whose original
purpose was to serve Git for Windows' needs, with the additional hope
that it may be useful to others, too) from serving Git for Windows'
needs.

Five years later, when it became really untenable to have one unwieldy,
big hodge-podge patch series of partly related, partly unrelated patches
in Git for Windows that was rebased onto core Git's tags from time to
time (earning the undeserved wrath of the developer of the ill-fated
git-remote-hg series that first obsoleted Git for Windows' competing
approach, only to be abandoned without maintainer later) was really
untenable, the "Git garden shears" were born [*1*/*2*]: a script,
piggy-backing on top of the interactive rebase, that would first
determine the branch topology of the patches to be rebased, create a
pseudo todo list for further editing, transform the result into a real
todo list (making heavy use of the `exec` command to "implement" the
missing todo list commands) and finally recreate the patch series on
top of the new base commit.

That was in 2013. And it took about three weeks to come up with the
design and implement it as an out-of-tree script. Needless to say, the
implementation needed quite a few years to stabilize, all the while the
design itself proved itself sound.

With this patch, the goodness of the Git garden shears comes to `git
rebase -i` itself. Passing the `--recreate-merges` option will generate
a todo list that can be understood readily, and where it is obvious
how to reorder commits. New branches can be introduced by inserting
`label` commands and calling `merge -  `. And once this
mode has become stable and universally accepted, we can deprecate the
design mistake that was `--preserve-merges`.

Link *1*:
https://github.com/msysgit/msysgit/blob/master/share/msysGit/shears.sh
Link *2*:
https://github.com/git-for-windows/build-extra/blob/master/shears.sh

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-rebase.txt   |   8 +-
 contrib/completion/git-completion.bash |   2 +-
 git-rebase--interactive.sh |   1 +
 git-rebase.sh  |   6 ++
 t/t3430-rebase-recreate-merges.sh  | 146 +
 5 files changed, 161 insertions(+), 2 deletions(-)
 create mode 100755 t/t3430-rebase-recreate-merges.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 8a861c1e0d6..1d061373288 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -368,6 +368,11 @@ The commit list format can be changed by setting the 
configuration option
 rebase.instructionFormat.  A customized instruction format will automatically
 have the long commit hash prepended to the format.
 
+--recreate-merges::
+   Recreate merge commits instead of flattening the history by replaying
+   merges. Merge conflict resolutions or manual amendments to merge
+   commits are not preserved.
+
 -p::
 --preserve-merges::
Recreate merge commits instead of flattening the history by replaying
@@ -770,7 +775,8 @@ BUGS
 The todo list presented by `--preserve-merges --interactive` does not
 represent the topology of the revision graph.  Editing commits and
 rewording their commit messages should work fine, but attempts to
-reorder commits tend to produce counterintuitive results.
+reorder commits tend to produce counterintuitive results. Use
+--recreate-merges for a more faithful representation.
 
 For example, an attempt to rearrange
 
diff --git a/contrib/completion/git-completion.bash 

[PATCH 3/8] sequencer: fast-forward merge commits, if possible

[Johannes Schindelin]

Just like with regular `pick` commands, if we are trying to recreate a
merge commit, we now test whether the parents of said commit match HEAD
and the commits to be merged, and fast-forward if possible.

This is not only faster, but also avoids unnecessary proliferation of
new objects.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index 567cfcbbe8b..a96255426e7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2085,7 +2085,7 @@ static int do_merge(struct commit *commit, const char 
*arg, int arg_len,
struct commit *head_commit, *merge_commit, *i;
struct commit_list *common, *j, *reversed = NULL;
struct merge_options o;
-   int ret;
+   int can_fast_forward, ret;
static struct lock_file lock;
 
for (merge_arg_len = 0; merge_arg_len < arg_len; merge_arg_len++)
@@ -2151,6 +2151,14 @@ static int do_merge(struct commit *commit, const char 
*arg, int arg_len,
return error(_("Cannot merge without a current revision"));
}
 
+   /*
+* If HEAD is not identical to the parent of the original merge commit,
+* we cannot fast-forward.
+*/
+   can_fast_forward = commit && commit->parents &&
+   !oidcmp(>parents->item->object.oid,
+   _commit->object.oid);
+
strbuf_addf(_name, "refs/rewritten/%.*s", merge_arg_len, arg);
merge_commit = lookup_commit_reference_by_name(ref_name.buf);
if (!merge_commit) {
@@ -2164,6 +2172,17 @@ static int do_merge(struct commit *commit, const char 
*arg, int arg_len,
rollback_lock_file();
return -1;
}
+
+   if (can_fast_forward && commit->parents->next &&
+   !commit->parents->next->next &&
+   !oidcmp(>parents->next->item->object.oid,
+   _commit->object.oid)) {
+   strbuf_release(_name);
+   rollback_lock_file();
+   return fast_forward_to(>object.oid,
+  _commit->object.oid, 0, opts);
+   }
+
write_message(oid_to_hex(_commit->object.oid), GIT_SHA1_HEXSZ,
  git_path_merge_head(), 0);
write_message("no-ff", 5, git_path_merge_mode(), 0);
-- 
2.15.1.windows.2.1430.ga56c4f9e2a9

[PATCH 4/8] rebase-helper --make-script: introduce a flag to recreate merges

[Johannes Schindelin]

The sequencer just learned a new commands intended to recreate branch
structure (similar in spirit to --preserve-merges, but with a
substantially less-broken design).

Let's allow the rebase--helper to generate todo lists making use of
these commands, triggered by the new --recreate-merges option. For a
commit topology like this:

A - B - C
  \   /
D

the generated todo list would look like this:

# branch D
pick 0123 A
label branch-point
pick 1234 D
label D

reset branch-point
pick 2345 B
merge 3456 D C

To keep things simple, we first only implement support for merge commits
with exactly two parents, leaving support for octopus merges to a later
patch in this patch series.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase--helper.c |   4 +-
 sequencer.c  | 343 ++-
 sequencer.h  |   1 +
 3 files changed, 345 insertions(+), 3 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 7daee544b7b..a34ab5c0655 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -12,7 +12,7 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
-   unsigned flags = 0, keep_empty = 0;
+   unsigned flags = 0, keep_empty = 0, recreate_merges = 0;
int abbreviate_commands = 0;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
@@ -22,6 +22,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
OPT_BOOL(0, "keep-empty", _empty, N_("keep empty 
commits")),
+   OPT_BOOL(0, "recreate-merges", _merges, N_("recreate 
merge commits")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -55,6 +56,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
 
flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0;
flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
+   flags |= recreate_merges ? TODO_LIST_RECREATE_MERGES : 0;
flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
 
if (command == CONTINUE && argc == 1)
diff --git a/sequencer.c b/sequencer.c
index a96255426e7..1bef16647b4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -23,6 +23,8 @@
 #include "hashmap.h"
 #include "unpack-trees.h"
 #include "worktree.h"
+#include "oidmap.h"
+#include "oidset.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
@@ -2785,6 +2787,335 @@ void append_signoff(struct strbuf *msgbuf, int 
ignore_footer, unsigned flag)
strbuf_release();
 }
 
+struct labels_entry {
+   struct hashmap_entry entry;
+   char label[FLEX_ARRAY];
+};
+
+static int labels_cmp(const void *fndata, const struct labels_entry *a,
+ const struct labels_entry *b, const void *key)
+{
+   return key ? strcmp(a->label, key) : strcmp(a->label, b->label);
+}
+
+struct string_entry {
+   struct oidmap_entry entry;
+   char string[FLEX_ARRAY];
+};
+
+struct label_state {
+   struct oidmap commit2label;
+   struct hashmap labels;
+   struct strbuf buf;
+};
+
+static const char *label_oid(struct object_id *oid, const char *label,
+struct label_state *state)
+{
+   struct labels_entry *labels_entry;
+   struct string_entry *string_entry;
+   struct object_id dummy;
+   size_t len;
+   int i;
+
+   string_entry = oidmap_get(>commit2label, oid);
+   if (string_entry)
+   return string_entry->string;
+
+   /*
+* For "uninteresting" commits, i.e. commits that are not to be
+* rebased, and which can therefore not be labeled, we use a unique
+* abbreviation of the commit name. This is slightly more complicated
+* than calling find_unique_abbrev() because we also need to make
+* sure that the abbreviation does not conflict with any other
+* label.
+*
+* We disallow "interesting" commits to be labeled by a string that
+* is a valid full-length hash, to ensure that we always can find an
+* abbreviation for any uninteresting commit's names that does not
+* clash with any other label.
+*/
+   if (!label) {
+   char *p;
+
+   strbuf_reset(>buf);
+   strbuf_grow(>buf, GIT_SHA1_HEXSZ);
+   label = p = state->buf.buf;
+
+   find_unique_abbrev_r(p, oid->hash, default_abbrev);
+
+   /*
+* We may need to extend the abbreviated hash so 

[PATCH 1/8] sequencer: introduce new commands to reset the revision

[Johannes Schindelin]

In the upcoming commits, we will teach the sequencer to recreate merges.
This will be done in a very different way from the unfortunate design of
`git rebase --preserve-merges` (which does not allow for reordering
commits, or changing the branch topology).

The main idea is to introduce new todo list commands, to support
labeling the current revision with a given name, resetting the current
revision to a previous state, merging labeled revisions.

This idea was developed in Git for Windows' Git garden shears (that are
used to maintain the "thicket of branches" on top of upstream Git), and
this patch is part of the effort to make it available to a wider
audience, as well as to make the entire process more robust (by
implementing it in a safe and portable language rather than a Unix shell
script).

This commit implements the commands to label, and to reset to, given
revisions. The syntax is:

label 
reset 

As a convenience shortcut, also to improve readability of the generated
todo list, a third command is introduced: bud. It simply resets to the
"onto" revision, i.e. the commit onto which we currently rebase.

Internally, the `label ` command creates the ref
`refs/rewritten/`. This makes it possible to work with the labeled
revisions interactively, or in a scripted fashion (e.g. via the todo
list command `exec`).

Signed-off-by: Johannes Schindelin 
---
 git-rebase--interactive.sh |   3 +
 sequencer.c| 181 -
 2 files changed, 180 insertions(+), 4 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d47bd29593a..3d2cd19d65a 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -162,6 +162,9 @@ s, squash = use commit, but meld into previous commit
 f, fixup = like \"squash\", but discard this commit's log message
 x, exec = run command (the rest of the line) using shell
 d, drop = remove commit
+l, label = label current HEAD with a name
+t, reset = reset HEAD to a label
+b, bud = reset HEAD to the revision labeled 'onto'
 
 These lines can be re-ordered; they are executed from top to bottom.
 " | git stripspace --comment-lines >>"$todo"
diff --git a/sequencer.c b/sequencer.c
index 4d3f60594cb..91cc55a002f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -21,6 +21,8 @@
 #include "log-tree.h"
 #include "wt-status.h"
 #include "hashmap.h"
+#include "unpack-trees.h"
+#include "worktree.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
@@ -116,6 +118,13 @@ static GIT_PATH_FUNC(rebase_path_stopped_sha, 
"rebase-merge/stopped-sha")
 static GIT_PATH_FUNC(rebase_path_rewritten_list, "rebase-merge/rewritten-list")
 static GIT_PATH_FUNC(rebase_path_rewritten_pending,
"rebase-merge/rewritten-pending")
+
+/*
+ * The path of the file listing refs that need to be deleted after the rebase
+ * finishes. This is used by the `merge` command.
+ */
+static GIT_PATH_FUNC(rebase_path_refs_to_delete, "rebase-merge/refs-to-delete")
+
 /*
  * The following files are written by git-rebase just after parsing the
  * command-line (and are only consumed, not modified, by the sequencer).
@@ -767,6 +776,9 @@ enum todo_command {
TODO_SQUASH,
/* commands that do something else than handling a single commit */
TODO_EXEC,
+   TODO_LABEL,
+   TODO_RESET,
+   TODO_BUD,
/* commands that do nothing but are counted for reporting progress */
TODO_NOOP,
TODO_DROP,
@@ -785,6 +797,9 @@ static struct {
{ 'f', "fixup" },
{ 's', "squash" },
{ 'x', "exec" },
+   { 'l', "label" },
+   { 't', "reset" },
+   { 'b', "bud" },
{ 0,   "noop" },
{ 'd', "drop" },
{ 0,   NULL }
@@ -1253,7 +1268,8 @@ static int parse_insn_line(struct todo_item *item, const 
char *bol, char *eol)
if (skip_prefix(bol, todo_command_info[i].str, )) {
item->command = i;
break;
-   } else if (bol[1] == ' ' && *bol == todo_command_info[i].c) {
+   } else if ((bol + 1 == eol || bol[1] == ' ') &&
+  *bol == todo_command_info[i].c) {
bol++;
item->command = i;
break;
@@ -1265,7 +1281,7 @@ static int parse_insn_line(struct todo_item *item, const 
char *bol, char *eol)
padding = strspn(bol, " \t");
bol += padding;
 
-   if (item->command == TODO_NOOP) {
+   if (item->command == TODO_NOOP || item->command == TODO_BUD) {
if (bol != eol)
return error(_("%s does not accept arguments: '%s'"),
 command_to_string(item->command), bol);
@@ -1279,7 +1295,8 @@ static int parse_insn_line(struct todo_item *item, const 
char *bol, char *eol)
return error(_("missing arguments for %s"),
 

[PATCH 2/8] sequencer: introduce the `merge` command

[Johannes Schindelin]

This patch is part of the effort to reimplement `--preserve-merges` with
a substantially improved design, a design that has been developed in the
Git for Windows project to maintain the dozens of Windows-specific patch
series on top of upstream Git.

The previous patch implemented the `label`, `bud` and `reset` commands
to label commits and to reset to a labeled commits. This patch adds the
`merge` command, with the following syntax:

merge   

The  parameter in this instance is the *original* merge commit,
whose author and message will be used for the to-be-created merge
commit.

The  parameter refers to the (possibly rewritten) revision to
merge. Let's see an example of a todo list:

label onto

# Branch abc
bud
pick deadbeef Hello, world!
label abc

bud
pick cafecafe And now for something completely different
merge baaabaaa abc Merge the branch 'abc' into master

To support creating *new* merges, i.e. without copying the commit
message from an existing commit, use the special value `-` as 
parameter (in which case the text after the  parameter is used as
commit message):

merge - abc This will be the actual commit message of the merge

This comes in handy when splitting a branch into two or more branches.

Note: this patch only adds support for recursive merges, to keep things
simple. Support for octopus merges will be added later in this patch
series, support for merges using strategies other than the recursive
merge is left for future contributions.

The design of the `merge` command as introduced by this patch only
supports creating new merge commits with exactly two parents, i.e. it
adds no support for octopus merges.

We will introduce support for octopus merges in a later commit.

Signed-off-by: Johannes Schindelin 
---
 git-rebase--interactive.sh |   1 +
 sequencer.c| 146 +++--
 2 files changed, 143 insertions(+), 4 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3d2cd19d65a..5bf1ea3781f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -165,6 +165,7 @@ d, drop = remove commit
 l, label = label current HEAD with a name
 t, reset = reset HEAD to a label
 b, bud = reset HEAD to the revision labeled 'onto'
+m, merge = create a merge commit using a given commit's message
 
 These lines can be re-ordered; they are executed from top to bottom.
 " | git stripspace --comment-lines >>"$todo"
diff --git a/sequencer.c b/sequencer.c
index 91cc55a002f..567cfcbbe8b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -779,6 +779,7 @@ enum todo_command {
TODO_LABEL,
TODO_RESET,
TODO_BUD,
+   TODO_MERGE,
/* commands that do nothing but are counted for reporting progress */
TODO_NOOP,
TODO_DROP,
@@ -800,6 +801,7 @@ static struct {
{ 'l', "label" },
{ 't', "reset" },
{ 'b', "bud" },
+   { 'm', "merge" },
{ 0,   "noop" },
{ 'd', "drop" },
{ 0,   NULL }
@@ -1304,14 +1306,20 @@ static int parse_insn_line(struct todo_item *item, 
const char *bol, char *eol)
}
 
end_of_object_name = (char *) bol + strcspn(bol, " \t\n");
+   item->arg = end_of_object_name + strspn(end_of_object_name, " \t");
+   item->arg_len = (int)(eol - item->arg);
+
saved = *end_of_object_name;
+   if (item->command == TODO_MERGE && *bol == '-' &&
+   bol + 1 == end_of_object_name) {
+   item->commit = NULL;
+   return 0;
+   }
+
*end_of_object_name = '\0';
status = get_oid(bol, _oid);
*end_of_object_name = saved;
 
-   item->arg = end_of_object_name + strspn(end_of_object_name, " \t");
-   item->arg_len = (int)(eol - item->arg);
-
if (status < 0)
return -1;
 
@@ -2069,6 +2077,132 @@ static int do_reset(const char *name, int len)
return ret;
 }
 
+static int do_merge(struct commit *commit, const char *arg, int arg_len,
+   struct replay_opts *opts)
+{
+   int merge_arg_len;
+   struct strbuf ref_name = STRBUF_INIT;
+   struct commit *head_commit, *merge_commit, *i;
+   struct commit_list *common, *j, *reversed = NULL;
+   struct merge_options o;
+   int ret;
+   static struct lock_file lock;
+
+   for (merge_arg_len = 0; merge_arg_len < arg_len; merge_arg_len++)
+   if (isspace(arg[merge_arg_len]))
+   break;
+
+   if (hold_locked_index(, LOCK_REPORT_ON_ERROR) < 0)
+   return -1;
+
+   if (commit) {
+   const char *message = get_commit_buffer(commit, NULL);
+   const char *body;
+   int len;
+
+   if (!message) {
+   rollback_lock_file();
+   return error(_("could not get commit message of '%s'"),
+   

[PATCH 0/8] rebase -i: offer to recreate merge commits

[Johannes Schindelin]

Once upon a time, I dreamt of an interactive rebase that would not
flatten branch structure, but instead recreate the commit topology
faithfully.

My original attempt was --preserve-merges, but that design was so
limited that I did not even enable it in interactive mode.

Subsequently, it *was* enabled in interactive mode, with the predictable
consequences: as the --preserve-merges design does not allow for
specifying the parents of merge commits explicitly, all the new commits'
parents are defined *implicitly* by the previous commit history, and
hence it is *not possible to even reorder commits*.

This design flaw cannot be fixed. Not without a complete re-design, at
least. This patch series offers such a re-design.

Think of --recreate-merges as "--preserve-merges done right". It
introduces new verbs for the todo list, `label`, `reset` and `merge`.
For a commit topology like this:

A - B - C
  \   /
D

the generated todo list would look like this:

# branch D
pick 0123 A
label branch-point
pick 1234 D
label D

reset branch-point
pick 2345 B
merge 3456 D C

There are more patches in the pipeline, based on this patch series, but
left for later in the interest of reviewable patch series: one mini
series to use the sequencer even for `git rebase -i --root`, and another
one to add support for octopus merges to --recreate-merges.


Johannes Schindelin (8):
  sequencer: introduce new commands to reset the revision
  sequencer: introduce the `merge` command
  sequencer: fast-forward merge commits, if possible
  rebase-helper --make-script: introduce a flag to recreate merges
  rebase: introduce the --recreate-merges option
  sequencer: handle autosquash and post-rewrite for merge commands
  pull: accept --rebase=recreate to recreate the branch topology
  rebase -i: introduce --recreate-merges=no-rebase-cousins

 Documentation/config.txt   |   8 +
 Documentation/git-pull.txt |   5 +-
 Documentation/git-rebase.txt   |  13 +-
 builtin/pull.c |  14 +-
 builtin/rebase--helper.c   |  13 +-
 builtin/remote.c   |   2 +
 contrib/completion/git-completion.bash |   4 +-
 git-rebase--interactive.sh |   6 +
 git-rebase.sh  |  16 +
 refs.c |   3 +-
 sequencer.c| 697 -
 sequencer.h|   9 +
 t/t3430-rebase-recreate-merges.sh  | 208 ++
 13 files changed, 977 insertions(+), 21 deletions(-)
 create mode 100755 t/t3430-rebase-recreate-merges.sh


base-commit: 2512f15446149235156528dafbe75930c712b29e
Published-As: https://github.com/dscho/git/releases/tag/recreate-merges-v1
Fetch-It-Via: git fetch https://github.com/dscho/git recreate-merges-v1
-- 
2.15.1.windows.2.1430.ga56c4f9e2a9

Re: [PATCH 3/3] read-cache: don't write index twice if we can't write shared index

[Duy Nguyen]

On Thu, Jan 18, 2018 at 08:36:32PM +0700, Duy Nguyen wrote:
> On Thu, Jan 18, 2018 at 8:29 PM, Jeff King  wrote:
> > On Thu, Jan 18, 2018 at 07:47:50PM +0700, Duy Nguyen wrote:
> >
> >> I may need help getting that log (or even better the trash directory
> >> of t1700). I tried -m32 build, then valgrind on amd64 (because it
> >> didn't work with my 32 build), running the tests with dash and even
> >> the linux32 docker version that comes with "ci" directory. Everything
> >> worked for me.
> >
> > It fails for me locally if I run it in the linux32 docker environment.
> > Here's the end of the "-v -x" output:
> >
> >   + GIT_INDEX_FILE=/usr/src/git/t/trash 
> > directory.t1700-split-index/new-index git -C ro update-index --split-index
> >   + chmod -R u+w ro
> >   + rm ro/.git/sharedindex.bbdf63f5a51242904dba2ea929ea4f56fcc340b6
> >   + GIT_INDEX_FILE=new-index git ls-files
> >   fatal: .git/sharedindex.bbdf63f5a51242904dba2ea929ea4f56fcc340b6: index 
> > file open failed: No such file or directory
> >   error: last command exited with $?=128
> >   not ok 23 - graceful handling splitting index is not allowed
> >
> > I don't know if the trash state will be helpful, but it's attached.
> 
> Thanks. Somehow my way of forcing mks_tempfile() to fail, well..
> failed in this environment. I see the same output if I remove "chmod
> -R u-w ro". I think I have enough to continue from here.

The test suite was run as root, no wonder why my removing write access
has no effect. I got the test to pass with this, but then it fails
with

Can't write .prove (Permission denied) at /usr/share/perl/5.22/App/Prove.pm 
line 542.

Some more chown'ing or chmod'ing is required

-- 8< --
Subject: [PATCH] ci: don't accidentally run the test suite as root

This script assigns and adds a user named "ci" in a subshell so the
outer CI_USER is not affected. For some reason, CI_USER is actually
empty on Travis linux32 builds. This makes the following "su" useless
and the test suite is run as root.

Some tests may expect file/dir permissions to be respected. But root
rules all and ignores all. That could make tests fail.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 ci/run-linux32-build.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh
index c19c50c1c9..92b488ff27 100755
--- a/ci/run-linux32-build.sh
+++ b/ci/run-linux32-build.sh
@@ -21,8 +21,8 @@ linux32 --32bit i386 sh -c '
 # If a host user id is given, then create a user "ci" with the host user
 # id to make everything accessible to the host user.
 HOST_UID=$1 &&
-CI_USER=$USER &&
-test -z $HOST_UID || (CI_USER="ci" && useradd -u $HOST_UID $CI_USER) &&
+CI_USER=${USER:-ci} &&
+test -z "$HOST_UID" || useradd -u $HOST_UID $CI_USER &&
 
 # Build and test
 linux32 --32bit i386 su -m -l $CI_USER -c '
-- 
2.16.0.47.g5ff498d35b

-- 8< --

[PATCH 03/11] sha1_file: convert pretend_sha1_file to object_id

[Patryk Obara]

Convert the declaration and definition of pretend_sha1_file to use
struct object_id and adjust all usages of this function.

Signed-off-by: Patryk Obara 
---
 blame.c | 2 +-
 cache.h | 3 ++-
 sha1_file.c | 8 
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/blame.c b/blame.c
index 2893f3c103..2ad656c1be 100644
--- a/blame.c
+++ b/blame.c
@@ -232,7 +232,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
convert_to_git(_index, path, buf.buf, buf.len, , 0);
origin->file.ptr = buf.buf;
origin->file.size = buf.len;
-   pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_oid.hash);
+   pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, >blob_oid);
 
/*
 * Read the current index, replace the path entry with
diff --git a/cache.h b/cache.h
index d8b975a571..8ed75d7260 100644
--- a/cache.h
+++ b/cache.h
@@ -1241,7 +1241,8 @@ extern int sha1_object_info(const unsigned char *, 
unsigned long *);
 extern int hash_sha1_file(const void *buf, unsigned long len, const char 
*type, unsigned char *sha1);
 extern int write_sha1_file(const void *buf, unsigned long len, const char 
*type, unsigned char *return_sha1);
 extern int hash_sha1_file_literally(const void *buf, unsigned long len, const 
char *type, struct object_id *oid, unsigned flags);
-extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned 
char *);
+extern int pretend_sha1_file(void *, unsigned long, enum object_type,
+struct object_id *oid);
 extern int force_object_loose(const unsigned char *sha1, time_t mtime);
 extern int git_open_cloexec(const char *name, int flags);
 #define git_open(name) git_open_cloexec(name, O_RDONLY)
diff --git a/sha1_file.c b/sha1_file.c
index 3da70ac650..dc8adb9d17 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1313,12 +1313,12 @@ static void *read_object(const unsigned char *sha1, 
enum object_type *type,
 }
 
 int pretend_sha1_file(void *buf, unsigned long len, enum object_type type,
- unsigned char *sha1)
+ struct object_id *oid)
 {
struct cached_object *co;
 
-   hash_sha1_file(buf, len, typename(type), sha1);
-   if (has_sha1_file(sha1) || find_cached_object(sha1))
+   hash_sha1_file(buf, len, typename(type), oid->hash);
+   if (has_sha1_file(oid->hash) || find_cached_object(oid->hash))
return 0;
ALLOC_GROW(cached_objects, cached_object_nr + 1, cached_object_alloc);
co = _objects[cached_object_nr++];
@@ -1326,7 +1326,7 @@ int pretend_sha1_file(void *buf, unsigned long len, enum 
object_type type,
co->type = type;
co->buf = xmalloc(len);
memcpy(co->buf, buf, len);
-   hashcpy(co->sha1, sha1);
+   hashcpy(co->sha1, oid->hash);
return 0;
 }
 
-- 
2.14.3

[PATCH 05/11] sha1_file: convert hash_sha1_file to object_id

[Patryk Obara]

Convert the declaration and definition of hash_sha1_file to use
struct object_id and adjust all function calls.

Signed-off-by: Patryk Obara 
---
 apply.c  |  4 ++--
 builtin/index-pack.c |  3 +--
 builtin/replace.c|  2 +-
 builtin/unpack-objects.c |  2 +-
 cache-tree.c | 11 +--
 cache.h  |  3 ++-
 convert.c|  6 +++---
 diffcore-rename.c|  2 +-
 dir.c|  2 +-
 log-tree.c   |  2 +-
 sha1_file.c  | 22 +++---
 11 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/apply.c b/apply.c
index 321a9fa68d..d6cbb4dc66 100644
--- a/apply.c
+++ b/apply.c
@@ -3154,7 +3154,7 @@ static int apply_binary(struct apply_state *state,
 * See if the old one matches what the patch
 * applies to.
 */
-   hash_sha1_file(img->buf, img->len, blob_type, oid.hash);
+   hash_sha1_file(img->buf, img->len, blob_type, );
if (strcmp(oid_to_hex(), patch->old_sha1_prefix))
return error(_("the patch applies to '%s' (%s), "
   "which does not match the "
@@ -3199,7 +3199,7 @@ static int apply_binary(struct apply_state *state,
 name);
 
/* verify that the result matches */
-   hash_sha1_file(img->buf, img->len, blob_type, oid.hash);
+   hash_sha1_file(img->buf, img->len, blob_type, );
if (strcmp(oid_to_hex(), patch->new_sha1_prefix))
return error(_("binary patch to '%s' creates incorrect 
result (expecting %s, got %s)"),
name, patch->new_sha1_prefix, oid_to_hex());
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 4c51aec81f..f7c69008f1 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -959,8 +959,7 @@ static void resolve_delta(struct object_entry *delta_obj,
if (!result->data)
bad_object(delta_obj->idx.offset, _("failed to apply delta"));
hash_sha1_file(result->data, result->size,
-  typename(delta_obj->real_type),
-  delta_obj->idx.oid.hash);
+  typename(delta_obj->real_type), _obj->idx.oid);
sha1_object(result->data, NULL, result->size, delta_obj->real_type,
_obj->idx.oid);
counter_lock();
diff --git a/builtin/replace.c b/builtin/replace.c
index 10078ae371..7267d06b17 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -355,7 +355,7 @@ static void check_one_mergetag(struct commit *commit,
struct tag *tag;
int i;
 
-   hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), 
tag_oid.hash);
+   hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), _oid);
tag = lookup_tag(_oid);
if (!tag)
die(_("bad mergetag in commit '%s'"), ref);
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 62ea264c46..2e494e2a4b 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -258,7 +258,7 @@ static void write_object(unsigned nr, enum object_type type,
} else {
struct object *obj;
int eaten;
-   hash_sha1_file(buf, size, typename(type), 
obj_list[nr].oid.hash);
+   hash_sha1_file(buf, size, typename(type), _list[nr].oid);
added_object(nr, type, buf, size);
obj = parse_object_buffer(_list[nr].oid, type, size, buf,
  );
diff --git a/cache-tree.c b/cache-tree.c
index e03e72c34a..0df09180ee 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -400,15 +400,14 @@ static int update_one(struct cache_tree *it,
}
 
if (repair) {
-   unsigned char sha1[20];
-   hash_sha1_file(buffer.buf, buffer.len, tree_type, sha1);
-   if (has_sha1_file(sha1))
-   hashcpy(it->oid.hash, sha1);
+   struct object_id oid;
+   hash_sha1_file(buffer.buf, buffer.len, tree_type, );
+   if (has_sha1_file(oid.hash))
+   oidcpy(>oid, );
else
to_invalidate = 1;
} else if (dryrun)
-   hash_sha1_file(buffer.buf, buffer.len, tree_type,
-  it->oid.hash);
+   hash_sha1_file(buffer.buf, buffer.len, tree_type, >oid);
else if (write_sha1_file(buffer.buf, buffer.len, tree_type, 
it->oid.hash)) {
strbuf_release();
return -1;
diff --git a/cache.h b/cache.h
index 04af1d69e4..b953d9f82c 100644
--- a/cache.h
+++ b/cache.h
@@ -1236,7 +1236,8 @@ static inline const unsigned char 
*lookup_replace_object(const unsigned char *sh
 
 /* Read and unpack a sha1 file into memory, write memory to a sha1 

[PATCH 08/11] commit: convert commit_tree* to object_id

[Patryk Obara]

Convert the definitions and declarations of commit_tree and
commit_tree_extended to use struct object_id and adjust all usages of
these functions.

Signed-off-by: Patryk Obara 
---
 builtin/am.c  |  4 ++--
 builtin/commit-tree.c |  4 ++--
 builtin/commit.c  |  5 +++--
 builtin/merge.c   |  8 
 commit.c  | 15 +++
 commit.h  | 11 ++-
 notes-cache.c |  4 ++--
 notes-merge.c |  9 -
 notes-utils.c |  7 ---
 notes-utils.h |  3 ++-
 10 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index acfe9d3c8c..6e6abb05cd 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1641,8 +1641,8 @@ static void do_commit(const struct am_state *state)
setenv("GIT_COMMITTER_DATE",
state->ignore_date ? "" : state->author_date, 1);
 
-   if (commit_tree(state->msg, state->msg_len, tree.hash, parents, 
commit.hash,
-   author, state->sign_commit))
+   if (commit_tree(state->msg, state->msg_len, , parents, ,
+   author, state->sign_commit))
die(_("failed to write commit object"));
 
reflog_msg = getenv("GIT_REFLOG_ACTION");
diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index 2177251e24..e5bdf57b1e 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -117,8 +117,8 @@ int cmd_commit_tree(int argc, const char **argv, const char 
*prefix)
die_errno("git commit-tree: failed to read");
}
 
-   if (commit_tree(buffer.buf, buffer.len, tree_oid.hash, parents,
-   commit_oid.hash, NULL, sign_commit)) {
+   if (commit_tree(buffer.buf, buffer.len, _oid, parents, _oid,
+   NULL, sign_commit)) {
strbuf_release();
return 1;
}
diff --git a/builtin/commit.c b/builtin/commit.c
index 8a87701414..c14878302e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1792,8 +1792,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
append_merge_tag_headers(parents, );
}
 
-   if (commit_tree_extended(sb.buf, sb.len, active_cache_tree->oid.hash,
-parents, oid.hash, author_ident.buf, sign_commit, 
extra)) {
+   if (commit_tree_extended(sb.buf, sb.len, _cache_tree->oid,
+parents, , author_ident.buf, sign_commit,
+extra)) {
rollback_index_files();
die(_("failed to write commit object"));
}
diff --git a/builtin/merge.c b/builtin/merge.c
index 30264cfd7c..92ba99a1a5 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -820,8 +820,8 @@ static int merge_trivial(struct commit *head, struct 
commit_list *remoteheads)
pptr = commit_list_append(head, pptr);
pptr = commit_list_append(remoteheads->item, pptr);
prepare_to_commit(remoteheads);
-   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree.hash, parents,
-   result_commit.hash, NULL, sign_commit))
+   if (commit_tree(merge_msg.buf, merge_msg.len, _tree, parents,
+   _commit, NULL, sign_commit))
die(_("failed to write commit object"));
finish(head, remoteheads, _commit, "In-index merge");
drop_save();
@@ -845,8 +845,8 @@ static int finish_automerge(struct commit *head,
commit_list_insert(head, );
strbuf_addch(_msg, '\n');
prepare_to_commit(remoteheads);
-   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree->hash, 
parents,
-   result_commit.hash, NULL, sign_commit))
+   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
+   _commit, NULL, sign_commit))
die(_("failed to write commit object"));
strbuf_addf(, "Merge made by the '%s' strategy.", wt_strategy);
finish(head, remoteheads, _commit, buf.buf);
diff --git a/commit.c b/commit.c
index cab8d4455b..760137e54b 100644
--- a/commit.c
+++ b/commit.c
@@ -1395,9 +1395,8 @@ void free_commit_extra_headers(struct commit_extra_header 
*extra)
}
 }
 
-int commit_tree(const char *msg, size_t msg_len,
-   const unsigned char *tree,
-   struct commit_list *parents, unsigned char *ret,
+int commit_tree(const char *msg, size_t msg_len, const struct object_id *tree,
+   struct commit_list *parents, struct object_id *ret,
const char *author, const char *sign_commit)
 {
struct commit_extra_header *extra = NULL, **tail = 
@@ -1526,8 +1525,8 @@ N_("Warning: commit message did not conform to UTF-8.\n"
"variable i18n.commitencoding to the encoding your project uses.\n");
 
 int commit_tree_extended(const char *msg, size_t msg_len,
-const unsigned 

[PATCH 00/11] Some fixes and bunch of object_id conversions

[Patryk Obara]

This patch series puts some mundane groundwork for experimentation with a new
hashing algorithm in git-hash-object.

Some time has passed since my last patches, and  I see, that work on new
hashing algorithm progressed nicely since then:

* brian m. carlson implemented vtable for hash algorithm selection and pushed
the repository format front - thanks to him it's now quite easy to
experimentally replace hashing functions and, e.g. do some performance testing.

* _a lot of people_ contributed to the transition plan, and now it's available
in text format in Documentation dir - Thank you all! It's much more readable
this way.

Patch 1 is not directly related to object_id conversions but helped with
debugging t5540, which kept failing on master for me (spoiler: it was Fedora
fault).  It helps with debugging of failing git-push over HTTP in case of
internal server error, so I think it might be worthwhile.

Patch 2 is a small adjustment to .clang-format, which prevents unnecessary
line breaks after function return type.

Patch 6 is a tiny fix in oidclr function.

All other patches are progressive conversions to struct object_id with some
formatting fixes sprinkled in. These should be somewhat uncontroversial, I hope.

Patryk Obara (11):
  http-push: improve error log
  clang-format: adjust penalty for return type line break
  sha1_file: convert pretend_sha1_file to object_id
  dir: convert struct sha1_stat to use object_id
  sha1_file: convert hash_sha1_file to object_id
  cache: clear whole hash buffer with oidclr
  match-trees: convert splice_tree to object_id
  commit: convert commit_tree* to object_id
  notes: convert combine_notes_* to object_id
  notes: convert write_notes_tree to object_id
  sha1_file: convert write_sha1_file to object_id

 .clang-format|  2 +-
 apply.c  | 12 +++
 blame.c  |  2 +-
 builtin/am.c |  4 +--
 builtin/checkout.c   |  3 +-
 builtin/commit-tree.c|  4 +--
 builtin/commit.c |  5 +--
 builtin/index-pack.c |  3 +-
 builtin/merge.c  |  8 ++---
 builtin/mktag.c  |  6 ++--
 builtin/mktree.c | 10 +++---
 builtin/notes.c  |  8 ++---
 builtin/receive-pack.c   | 11 ---
 builtin/replace.c|  4 +--
 builtin/tag.c|  2 +-
 builtin/unpack-objects.c | 11 ---
 cache-tree.c | 13 
 cache.h  | 13 
 commit.c | 13 
 commit.h | 11 ---
 convert.c|  6 ++--
 diffcore-rename.c|  2 +-
 dir.c| 31 +-
 dir.h|  2 +-
 http-push.c  |  4 +++
 log-tree.c   |  2 +-
 match-trees.c| 40 +++
 merge-recursive.c|  5 +--
 notes-cache.c|  8 ++---
 notes-merge.c|  9 +++---
 notes-utils.c|  9 +++---
 notes-utils.h|  3 +-
 notes.c  | 63 ++--
 notes.h  | 29 ++---
 read-cache.c |  6 ++--
 sha1_file.c  | 51 +++--
 t/helper/test-dump-untracked-cache.c |  4 +--
 37 files changed, 217 insertions(+), 202 deletions(-)

-- 
2.14.3

[PATCH 10/11] notes: convert write_notes_tree to object_id

[Patryk Obara]

Convert the definition and declaration of write_notes_tree to
struct object_id and adjust usage of this function.

Additionally, improve style of small part of this function, as old
formatting made it hard to understand at glance what this part of
code is doing.

Signed-off-by: Patryk Obara 
---
 notes-cache.c |  2 +-
 notes-utils.c |  2 +-
 notes.c   | 16 +---
 notes.h   |  4 ++--
 4 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/notes-cache.c b/notes-cache.c
index d2f87147cc..010ad236cb 100644
--- a/notes-cache.c
+++ b/notes-cache.c
@@ -54,7 +54,7 @@ int notes_cache_write(struct notes_cache *c)
if (!c->tree.dirty)
return 0;
 
-   if (write_notes_tree(>tree, tree_oid.hash))
+   if (write_notes_tree(>tree, _oid))
return -1;
if (commit_tree(c->validity, strlen(c->validity), _oid, NULL,
_oid, NULL, NULL) < 0)
diff --git a/notes-utils.c b/notes-utils.c
index 058c642dac..02407fe2a7 100644
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -12,7 +12,7 @@ void create_notes_commit(struct notes_tree *t, struct 
commit_list *parents,
 
assert(t->initialized);
 
-   if (write_notes_tree(t, tree_oid.hash))
+   if (write_notes_tree(t, _oid))
die("Failed to write notes tree to database");
 
if (!parents) {
diff --git a/notes.c b/notes.c
index 3f4f94507a..09ef1ce33a 100644
--- a/notes.c
+++ b/notes.c
@@ -1123,11 +1123,12 @@ int for_each_note(struct notes_tree *t, int flags, 
each_note_fn fn,
return for_each_note_helper(t, t->root, 0, 0, flags, fn, cb_data);
 }
 
-int write_notes_tree(struct notes_tree *t, unsigned char *result)
+int write_notes_tree(struct notes_tree *t, struct object_id *result)
 {
struct tree_write_stack root;
struct write_each_note_data cb_data;
int ret;
+   int flags;
 
if (!t)
t = _notes_tree;
@@ -1141,12 +1142,13 @@ int write_notes_tree(struct notes_tree *t, unsigned 
char *result)
cb_data.next_non_note = t->first_non_note;
 
/* Write tree objects representing current notes tree */
-   ret = for_each_note(t, FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
-   FOR_EACH_NOTE_YIELD_SUBTREES,
-   write_each_note, _data) ||
-   write_each_non_note_until(NULL, _data) ||
-   tree_write_stack_finish_subtree() ||
-   write_sha1_file(root.buf.buf, root.buf.len, tree_type, result);
+   flags = FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
+   FOR_EACH_NOTE_YIELD_SUBTREES;
+   ret = for_each_note(t, flags, write_each_note, _data) ||
+ write_each_non_note_until(NULL, _data) ||
+ tree_write_stack_finish_subtree() ||
+ write_sha1_file(root.buf.buf, root.buf.len, tree_type,
+ result->hash);
strbuf_release();
return ret;
 }
diff --git a/notes.h b/notes.h
index 88da38b5f4..0433f45db5 100644
--- a/notes.h
+++ b/notes.h
@@ -217,7 +217,7 @@ int for_each_note(struct notes_tree *t, int flags, 
each_note_fn fn,
  * Write the given notes_tree structure to the object database
  *
  * Creates a new tree object encapsulating the current state of the given
- * notes_tree, and stores its SHA1 into the 'result' argument.
+ * notes_tree, and stores its object id into the 'result' argument.
  *
  * Returns zero on success, non-zero on failure.
  *
@@ -225,7 +225,7 @@ int for_each_note(struct notes_tree *t, int flags, 
each_note_fn fn,
  * this function has returned zero. Please also remember to create a
  * corresponding commit object, and update the appropriate notes ref.
  */
-int write_notes_tree(struct notes_tree *t, unsigned char *result);
+int write_notes_tree(struct notes_tree *t, struct object_id *result);
 
 /* Flags controlling the operation of prune */
 #define NOTES_PRUNE_VERBOSE 1
-- 
2.14.3

[PATCH 09/11] notes: convert combine_notes_* to object_id

[Patryk Obara]

Convert the definition and declarations of combine_notes_* functions
to struct object_id and adjust usage of these functions.

Signed-off-by: Patryk Obara 
---
 notes.c | 46 +++---
 notes.h | 25 +++--
 2 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/notes.c b/notes.c
index c7f21fae44..3f4f94507a 100644
--- a/notes.c
+++ b/notes.c
@@ -270,8 +270,8 @@ static int note_tree_insert(struct notes_tree *t, struct 
int_node *tree,
if (!oidcmp(>val_oid, >val_oid))
return 0;
 
-   ret = combine_notes(l->val_oid.hash,
-   entry->val_oid.hash);
+   ret = combine_notes(>val_oid,
+   >val_oid);
if (!ret && is_null_oid(>val_oid))
note_tree_remove(t, tree, n, entry);
free(entry);
@@ -786,8 +786,8 @@ static int prune_notes_helper(const struct object_id 
*object_oid,
return 0;
 }
 
-int combine_notes_concatenate(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_concatenate(struct object_id *cur_oid,
+ const struct object_id *new_oid)
 {
char *cur_msg = NULL, *new_msg = NULL, *buf;
unsigned long cur_len, new_len, buf_len;
@@ -795,18 +795,18 @@ int combine_notes_concatenate(unsigned char *cur_sha1,
int ret;
 
/* read in both note blob objects */
-   if (!is_null_sha1(new_sha1))
-   new_msg = read_sha1_file(new_sha1, _type, _len);
+   if (!is_null_oid(new_oid))
+   new_msg = read_sha1_file(new_oid->hash, _type, _len);
if (!new_msg || !new_len || new_type != OBJ_BLOB) {
free(new_msg);
return 0;
}
-   if (!is_null_sha1(cur_sha1))
-   cur_msg = read_sha1_file(cur_sha1, _type, _len);
+   if (!is_null_oid(cur_oid))
+   cur_msg = read_sha1_file(cur_oid->hash, _type, _len);
if (!cur_msg || !cur_len || cur_type != OBJ_BLOB) {
free(cur_msg);
free(new_msg);
-   hashcpy(cur_sha1, new_sha1);
+   oidcpy(cur_oid, new_oid);
return 0;
}
 
@@ -825,20 +825,20 @@ int combine_notes_concatenate(unsigned char *cur_sha1,
free(new_msg);
 
/* create a new blob object from buf */
-   ret = write_sha1_file(buf, buf_len, blob_type, cur_sha1);
+   ret = write_sha1_file(buf, buf_len, blob_type, cur_oid->hash);
free(buf);
return ret;
 }
 
-int combine_notes_overwrite(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_overwrite(struct object_id *cur_oid,
+   const struct object_id *new_oid)
 {
-   hashcpy(cur_sha1, new_sha1);
+   oidcpy(cur_oid, new_oid);
return 0;
 }
 
-int combine_notes_ignore(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_ignore(struct object_id *cur_oid,
+const struct object_id *new_oid)
 {
return 0;
 }
@@ -848,17 +848,17 @@ int combine_notes_ignore(unsigned char *cur_sha1,
  * newlines removed.
  */
 static int string_list_add_note_lines(struct string_list *list,
- const unsigned char *sha1)
+ const struct object_id *oid)
 {
char *data;
unsigned long len;
enum object_type t;
 
-   if (is_null_sha1(sha1))
+   if (is_null_oid(oid))
return 0;
 
/* read_sha1_file NUL-terminates */
-   data = read_sha1_file(sha1, , );
+   data = read_sha1_file(oid->hash, , );
if (t != OBJ_BLOB || !data || !len) {
free(data);
return t != OBJ_BLOB || !data;
@@ -884,17 +884,17 @@ static int string_list_join_lines_helper(struct 
string_list_item *item,
return 0;
 }
 
-int combine_notes_cat_sort_uniq(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_cat_sort_uniq(struct object_id *cur_oid,
+   const struct object_id *new_oid)
 {
struct string_list sort_uniq_list = STRING_LIST_INIT_DUP;
struct strbuf buf = STRBUF_INIT;
int ret = 1;
 
/* read both note blob objects into unique_lines */
-   if (string_list_add_note_lines(_uniq_list, cur_sha1))
+   if (string_list_add_note_lines(_uniq_list, cur_oid))
goto out;
-   if (string_list_add_note_lines(_uniq_list, new_sha1))
+   if (string_list_add_note_lines(_uniq_list, new_oid))
goto out;
string_list_remove_empty_items(_uniq_list, 0);
string_list_sort(_uniq_list);

[PATCH 02/11] clang-format: adjust penalty for return type line break

[Patryk Obara]

The penalty of 5 makes clang-format very eager to put even short type
declarations (e.g. "extern int") into a separate line, even when
breaking parameters list is sufficient.

Signed-off-by: Patryk Obara 
---
 .clang-format | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.clang-format b/.clang-format
index 611ab4750b..12a89f95f9 100644
--- a/.clang-format
+++ b/.clang-format
@@ -163,7 +163,7 @@ PenaltyBreakComment: 10
 PenaltyBreakFirstLessLess: 0
 PenaltyBreakString: 10
 PenaltyExcessCharacter: 100
-PenaltyReturnTypeOnItsOwnLine: 5
+PenaltyReturnTypeOnItsOwnLine: 60
 
 # Don't sort #include's
 SortIncludes: false
-- 
2.14.3

[PATCH 11/11] sha1_file: convert write_sha1_file to object_id

[Patryk Obara]

Convert the definition and declaration of write_sha1_file to
struct object_id and adjust usage of this function.

This commit also converts static function write_sha1_file_prepare, as it
is closely related.

Replace sha1_to_hex, hashcpy and hashclr with their oid equivalents
wherever possible.

Signed-off-by: Patryk Obara 
---
 apply.c  |  8 
 builtin/checkout.c   |  3 +--
 builtin/mktag.c  |  6 +++---
 builtin/mktree.c | 10 +-
 builtin/notes.c  |  8 
 builtin/receive-pack.c   | 11 ++-
 builtin/replace.c|  2 +-
 builtin/tag.c|  2 +-
 builtin/unpack-objects.c |  9 ++---
 cache-tree.c |  2 +-
 cache.h  |  3 ++-
 commit.c |  2 +-
 match-trees.c|  2 +-
 merge-recursive.c|  5 +++--
 notes-cache.c|  2 +-
 notes.c  |  9 -
 read-cache.c |  6 +++---
 sha1_file.c  | 25 +
 18 files changed, 60 insertions(+), 55 deletions(-)

diff --git a/apply.c b/apply.c
index d6cbb4dc66..4951f8a130 100644
--- a/apply.c
+++ b/apply.c
@@ -3554,7 +3554,7 @@ static int try_threeway(struct apply_state *state,
 
/* Preimage the patch was prepared for */
if (patch->is_new)
-   write_sha1_file("", 0, blob_type, pre_oid.hash);
+   write_sha1_file("", 0, blob_type, _oid);
else if (get_oid(patch->old_sha1_prefix, _oid) ||
 read_blob_object(, _oid, patch->old_mode))
return error(_("repository lacks the necessary blob to fall 
back on 3-way merge."));
@@ -3570,7 +3570,7 @@ static int try_threeway(struct apply_state *state,
return -1;
}
/* post_oid is theirs */
-   write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, post_oid.hash);
+   write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, _oid);
clear_image(_image);
 
/* our_oid is ours */
@@ -3583,7 +3583,7 @@ static int try_threeway(struct apply_state *state,
return error(_("cannot read the current contents of 
'%s'"),
 patch->old_name);
}
-   write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, our_oid.hash);
+   write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, _oid);
clear_image(_image);
 
/* in-core three-way merge between post and our using pre as base */
@@ -4291,7 +4291,7 @@ static int add_index_file(struct apply_state *state,
}
fill_stat_cache_info(ce, );
}
-   if (write_sha1_file(buf, size, blob_type, ce->oid.hash) < 0) {
+   if (write_sha1_file(buf, size, blob_type, >oid) < 0) {
free(ce);
return error(_("unable to create backing store "
   "for newly created file %s"), path);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 8bdc927d3f..a98e88288e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -227,8 +227,7 @@ static int checkout_merged(int pos, const struct checkout 
*state)
 * (it also writes the merge result to the object database even
 * when it may contain conflicts).
 */
-   if (write_sha1_file(result_buf.ptr, result_buf.size,
-   blob_type, oid.hash))
+   if (write_sha1_file(result_buf.ptr, result_buf.size, blob_type, ))
die(_("Unable to add merge result for '%s'"), path);
free(result_buf.ptr);
ce = make_cache_entry(mode, oid.hash, path, 2, 0);
diff --git a/builtin/mktag.c b/builtin/mktag.c
index 031b750f06..d4044da3e4 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -151,7 +151,7 @@ static int verify_tag(char *buffer, unsigned long size)
 int cmd_mktag(int argc, const char **argv, const char *prefix)
 {
struct strbuf buf = STRBUF_INIT;
-   unsigned char result_sha1[20];
+   struct object_id result;
 
if (argc != 1)
usage("git mktag");
@@ -165,10 +165,10 @@ int cmd_mktag(int argc, const char **argv, const char 
*prefix)
if (verify_tag(buf.buf, buf.len) < 0)
die("invalid tag signature file");
 
-   if (write_sha1_file(buf.buf, buf.len, tag_type, result_sha1) < 0)
+   if (write_sha1_file(buf.buf, buf.len, tag_type, ) < 0)
die("unable to write tag file");
 
strbuf_release();
-   printf("%s\n", sha1_to_hex(result_sha1));
+   printf("%s\n", oid_to_hex());
return 0;
 }
diff --git a/builtin/mktree.c b/builtin/mktree.c
index da0fd8cd70..1988f5396d 100644
--- a/builtin/mktree.c
+++ b/builtin/mktree.c
@@ -40,7 +40,7 @@ static int ent_compare(const void *a_, const void *b_)
 b->name, b->len, b->mode);
 }
 
-static void write_tree(unsigned char *sha1)

[PATCH 01/11] http-push: improve error log

[Patryk Obara]

When git push fails due to server-side WebDAV error, it's not easy to
point to the main culprit.  Additional information about exact cURL
error and HTTP server response is helpful for debugging purpose.

New error log helped me pinpoint failing test t5540-http-push-webdav
to a missing Apache dependency in Fedora 27:
https://bugzilla.redhat.com/show_bug.cgi?id=1491151

Signed-off-by: Patryk Obara 
---
 http-push.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/http-push.c b/http-push.c
index 14435ab65d..0913f8ab86 100644
--- a/http-push.c
+++ b/http-push.c
@@ -915,6 +915,10 @@ static struct remote_lock *lock_remote(const char *path, 
long timeout)
lock->timeout = -1;
}
XML_ParserFree(parser);
+   } else {
+   fprintf(stderr,
+   "error: curl result=%d, HTTP code=%ld\n",
+   results.curl_result, results.http_code);
}
} else {
fprintf(stderr, "Unable to start LOCK request\n");
-- 
2.14.3

[PATCH 04/11] dir: convert struct sha1_stat to use object_id

[Patryk Obara]

Convert the declaration of struct sha1_stat. Adjust all usages of this
struct and replace hash{clr,cmp,cpy} with oid{clr,cmp,cpy} wherever
possible.

Remove macro EMPTY_BLOB_SHA1_BIN, as it's no longer used.

Signed-off-by: Patryk Obara 
---
 cache.h  |  2 --
 dir.c| 31 ---
 dir.h|  2 +-
 t/helper/test-dump-untracked-cache.c |  4 ++--
 4 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/cache.h b/cache.h
index 8ed75d7260..04af1d69e4 100644
--- a/cache.h
+++ b/cache.h
@@ -1047,8 +1047,6 @@ extern const struct object_id empty_tree_oid;
"\xe6\x9d\xe2\x9b\xb2\xd1\xd6\x43\x4b\x8b" \
"\x29\xae\x77\x5a\xd8\xc2\xe4\x8c\x53\x91"
 extern const struct object_id empty_blob_oid;
-#define EMPTY_BLOB_SHA1_BIN (empty_blob_oid.hash)
-
 
 static inline int is_empty_blob_sha1(const unsigned char *sha1)
 {
diff --git a/dir.c b/dir.c
index 7c4b45e30e..effa531d35 100644
--- a/dir.c
+++ b/dir.c
@@ -253,7 +253,7 @@ static int do_read_blob(const struct object_id *oid,
 
if (sha1_stat) {
memset(_stat->stat, 0, sizeof(sha1_stat->stat));
-   hashcpy(sha1_stat->sha1, oid->hash);
+   oidcpy(_stat->oid, oid);
}
 
if (sz == 0) {
@@ -823,7 +823,7 @@ static int add_excludes(const char *fname, const char 
*base, int baselen,
if (size == 0) {
if (sha1_stat) {
fill_stat_data(_stat->stat, );
-   hashcpy(sha1_stat->sha1, EMPTY_BLOB_SHA1_BIN);
+   oidcpy(_stat->oid, _blob_oid);
sha1_stat->valid = 1;
}
close(fd);
@@ -847,10 +847,11 @@ static int add_excludes(const char *fname, const char 
*base, int baselen,
 !ce_stage(istate->cache[pos]) &&
 ce_uptodate(istate->cache[pos]) &&
 !would_convert_to_git(istate, fname))
-   hashcpy(sha1_stat->sha1,
-   istate->cache[pos]->oid.hash);
+   oidcpy(_stat->oid,
+  >cache[pos]->oid);
else
-   hash_sha1_file(buf, size, "blob", 
sha1_stat->sha1);
+   hash_sha1_file(buf, size, "blob",
+  sha1_stat->oid.hash);
fill_stat_data(_stat->stat, );
sha1_stat->valid = 1;
}
@@ -1223,7 +1224,7 @@ static void prep_exclude(struct dir_struct *dir,
}
 
/* Try to read per-directory file */
-   hashclr(sha1_stat.sha1);
+   oidclr(_stat.oid);
sha1_stat.valid = 0;
if (dir->exclude_per_dir &&
/*
@@ -1269,9 +1270,9 @@ static void prep_exclude(struct dir_struct *dir,
 * order, though, if you do that.
 */
if (untracked &&
-   hashcmp(sha1_stat.sha1, untracked->exclude_sha1)) {
+   hashcmp(sha1_stat.oid.hash, untracked->exclude_sha1)) {
invalidate_gitignore(dir->untracked, untracked);
-   hashcpy(untracked->exclude_sha1, sha1_stat.sha1);
+   hashcpy(untracked->exclude_sha1, sha1_stat.oid.hash);
}
dir->exclude_stack = stk;
current = stk->baselen;
@@ -2228,13 +2229,13 @@ static struct untracked_cache_dir 
*validate_untracked_cache(struct dir_struct *d
 
/* Validate $GIT_DIR/info/exclude and core.excludesfile */
root = dir->untracked->root;
-   if (hashcmp(dir->ss_info_exclude.sha1,
-   dir->untracked->ss_info_exclude.sha1)) {
+   if (oidcmp(>ss_info_exclude.oid,
+  >untracked->ss_info_exclude.oid)) {
invalidate_gitignore(dir->untracked, root);
dir->untracked->ss_info_exclude = dir->ss_info_exclude;
}
-   if (hashcmp(dir->ss_excludes_file.sha1,
-   dir->untracked->ss_excludes_file.sha1)) {
+   if (oidcmp(>ss_excludes_file.oid,
+  >untracked->ss_excludes_file.oid)) {
invalidate_gitignore(dir->untracked, root);
dir->untracked->ss_excludes_file = dir->ss_excludes_file;
}
@@ -2638,8 +2639,8 @@ void write_untracked_extension(struct strbuf *out, struct 
untracked_cache *untra
FLEX_ALLOC_MEM(ouc, exclude_per_dir, untracked->exclude_per_dir, len);
stat_data_to_disk(>info_exclude_stat, 
>ss_info_exclude.stat);
stat_data_to_disk(>excludes_file_stat, 
>ss_excludes_file.stat);
-   hashcpy(ouc->info_exclude_sha1,