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

[Eric Sunshine]

On Mon, Jan 29, 2018 at 3:50 PM, Johannes Schindelin
 wrote:
> On Fri, 19 Jan 2018, Eric Sunshine wrote:
>> On Thu, Jan 18, 2018 at 10:35 AM, Johannes Schindelin
>>  wrote:
>> > +static int do_reset(const char *name, int len)
>> > +{
>> > +   for (i = 0; i < len; i++)
>> > +   if (isspace(name[i]))
>> > +   len = i;
>>
>> What is the purpose of this loop? I could imagine that it's trying to
>> strip all whitespace from the end of 'name', however, to do that it
>> would iterate backward, not forward. (Or perhaps it's trying to
>> truncate at the first space, but then it would need to invert the
>> condition or use 'break'.) Am I missing something obvious?
>
> Yes, you are missing something obvious. The idea of the `reset` command is
> that it not only has a label, but also the oneline of the original commit:
>
> reset branch-point sequencer: prepare for cleanup
>
> In this instance, `branch-point` is the label. And for convenience of the
> person editing, it also has the oneline.

No, that's not what I was missing. What I was missing was that
assigning 'i' to 'len' also causes the loop to terminate. It's
embarrassing how long I had to stare at this loop to see that, and I
suspect that's what fooled a couple other reviewers, as well, since
idiomatic loops don't normally muck with the termination condition in
quite that fashion (and is why I suggested that a 'break' might be
missing).

Had the loop been a bit more idiomatic:

for (i = 0; i < len; i++)
if (isspace(name[i]))
break;
len = i;

then the question would never have arisen. Anyhow, it's a minor point
in the greater scheme of the patch series.

> In the Git garden shears, I separated the two arguments via `#`:
>
> reset branch-point # sequencer: prepare for cleanup
>
> I guess that is actually more readable, so I will introduce that into this
> patch series, too.

Given my termination-condition blindness, the extra "#" would not have
helped me understand the loop any better.

Having now played with the feature a tiny bit, I don't have a strong
opinion about the "#" other than to note that it seems inconsistent
with other commands which don't use "#" as a separator.

Re: t9128 failing randomly with svn 1.9?

[Todd Zullinger]

I wrote:
> The 'git svn' tests are not run in Travis because the perl
> subversion bindings are not installed.  I haven't made time
> to try installing them and running the tests in Travis to
> see if the failures occur there, but I suspect they would.

Before anyone spends time wondering about this, I was
apparently looking at the MacOS build logs rather than the
Linux logs.  The git svn tests are indeed run in Travis, as
long as you look at the right build logs. :/

Sadly (or amusingly), I think I looked at this before and
made the same mistake, realized it, and then did it again
today.  Hopefully if I make the same mistake again I'll
realize it before I post it to the list. ;)

Anyway, the Travis Linux container uses Ubuntu Trusty, which
has subversion 1.8.8.  I suppose that's why these random
failures haven't turned up there.

-- 
Todd
~~
I got stopped by a cop the other day.  He said, "Why'd you run that
stop sign?"  I said, "Because I don't believe everything I read."
-- Stephen Wright



signature.asc
Description: PGP signature

Re: [PATCH v2 1/1] setup: recognise extensions.objectFormat

[Jeff King]

On Sun, Jan 28, 2018 at 01:36:17AM +0100, Patryk Obara wrote:

> This extension selects which hashing algorithm from vtable should be
> used for reading and writing objects in the object store.  At the moment
> supports only single value (sha-1).
> 
> In case value of objectFormat is an unknown hashing algorithm, Git
> command will fail with following message:
> 
>   fatal: unknown repository extensions found:
> objectformat = 
> 
> To indicate, that this specific objectFormat value is not recognised.

I don't have a strong opinion on this, but it does feel a little funny
to add this extension now, before we quite know what the code that uses
it is going to look like (or maybe we're farther along there than I
realize).

What do we gain by doing this now as opposed to later? By the design of
the extension code, we should complain on older versions anyway. And by
doing it now we carry a small risk that it might not give us the
interface we want, and it will be slightly harder to paper over this
failed direction.

All that said, if people like brian, who are thinking more about this
transition than I am, are onboard, I'm OK with it.

> The objectFormat extension is not allowed in repository marked as
> version 0 to prevent any possibility of accidentally writing a NewHash
> object in the sha-1 object store. This extension behaviour is different
> than preciousObjects extension (which is allowed in repo version 0).

It wasn't intended that anyone would specify preciousObjects with repo
version 0. It's a dangerous misconfiguration (because versions which
predate the extensions mechanism won't actually respect it at all!).

So we probably ought to complain loudly on having anything in
extensions.* when the repositoryformat is less than 1.

I originally wrote it the other way out of an abundance of
backward-compatibility. After all "extension.*" doesn't mean anything in
format 0, and somebody _could_ have added such a config key for their
own purposes. But that's a pretty weak argument, and if we are going to
start marking some extensions as forbidden there, we might as well do
them all.

Something like this:

diff --git a/cache.h b/cache.h
index d8b975a571..259c4a5361 100644
--- a/cache.h
+++ b/cache.h
@@ -921,6 +921,7 @@ struct repository_format {
int is_bare;
int hash_algo;
char *work_tree;
+   int extensions_seen;
struct string_list unknown_extensions;
 };
 
diff --git a/setup.c b/setup.c
index 8cc34186ce..85dfcf330b 100644
--- a/setup.c
+++ b/setup.c
@@ -413,6 +413,7 @@ static int check_repo_format(const char *var, const char 
*value, void *vdata)
if (strcmp(var, "core.repositoryformatversion") == 0)
data->version = git_config_int(var, value);
else if (skip_prefix(var, "extensions.", )) {
+   data->extensions_seen = 1;
/*
 * record any known extensions here; otherwise,
 * we fall through to recording it as unknown, and
@@ -503,6 +504,11 @@ int verify_repository_format(const struct 
repository_format *format,
return -1;
}
 
+   if (format->version < 1 && format->extensions_seen) {
+   strbuf_addstr(err, _("extensions found but repo version is < 
1"));
+   return -1;
+   }
+
if (format->version >= 1 && format->unknown_extensions.nr) {
int i;
 
diff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh
index ce4cff13bb..9e9f67d756 100755
--- a/t/t1302-repo-version.sh
+++ b/t/t1302-repo-version.sh
@@ -80,9 +80,10 @@ while read outcome version extensions; do
 done <<\EOF
 allow 0
 allow 1
+abort 0 noop
 allow 1 noop
+abort 0 no-such-extension
 abort 1 no-such-extension
-allow 0 no-such-extension
 EOF
 
 test_expect_success 'precious-objects allowed' '

Re: Some rough edges of core.fsmonitor

[Ben Peart]

On 1/28/2018 5:28 PM, Ævar Arnfjörð Bjarmason wrote:

On Sun, Jan 28, 2018 at 9:44 PM, Johannes Schindelin
 wrote:

Hi,

On Sat, 27 Jan 2018, Ævar Arnfjörð Bjarmason wrote:


I just got around to testing this since it landed, for context some
previous poking of mine in [1].

Issues / stuff I've noticed:

1) We end up invalidating the untracked cache because stuff in .git/
changed. For example:

 01:09:24.975524 fsmonitor.c:173 fsmonitor process 
'.git/hooks/fsmonitor-watchman' returned success
 01:09:24.975548 fsmonitor.c:138 fsmonitor_refresh_callback '.git'
 01:09:24.975556 fsmonitor.c:138 fsmonitor_refresh_callback 
'.git/config'
 01:09:24.975568 fsmonitor.c:138 fsmonitor_refresh_callback 
'.git/index'
 01:09:25.122726 fsmonitor.c:91  write fsmonitor extension 
successful

Am I missing something or should we do something like:

 diff --git a/fsmonitor.c b/fsmonitor.c
 index 0af7c4edba..5067b89bda 100644
 --- a/fsmonitor.c
 +++ b/fsmonitor.c
 @@ -118,7 +118,12 @@ static int query_fsmonitor(int version, uint64_t 
last_update, struct strbuf *que

  static void fsmonitor_refresh_callback(struct index_state *istate, const 
char *name)
  {
 -   int pos = index_name_pos(istate, name, strlen(name));
 +   int pos;
 +
 +   if (!strcmp(name, ".git") || starts_with(name, ".git/"))
 +   return;
 +
 +   pos = index_name_pos(istate, name, strlen(name));


I would much rather have the fsmonitor hook already exclude those.


As documented the fsmonitor-watchman hook we ship doesn't work as
described in githooks(5), unless "files in the working directory" is
taken to include .git/, but I haven't seen that ever used.

On the other hand relying on arbitrary user-provided hooks to do the
right thing at the cost of silent performance degradation is bad. If
we're going to expect the hook to remove these we should probably
warn/die here if it does send us .git/* files.



I'm not sure how often something is modified in the git directory when 
nothing was modified in the working directory but this seems like a nice 
optimization.


We can't just blindly ignore changes under ".git" as the git directory 
may have been moved somewhere else.  Instead we'd need to use get_git_dir().


Rather than assuming the hook will optimize for this particular case, I 
think a better solution would be to update 
untracked_cache_invalidate_path() so that it doesn't invalidate the 
untracked cache and mark the index as dirty when it was asked to 
invalidate a path under GIT_DIR.  I can't think of a case when that 
would be the desired behavior.


Somewhat off topic but related to the overall performance discussion: 
I've also thought the untracked cache shouldn't mark the index as dirty 
except in the case where the extension is being added or removed.  We've 
observed that it causes unnecessary index writes that actually slows 
down overall performance.


Since it is a cache, it does not require the index to be written out for 
correctness, it can simply update the cache again the next time it is 
needed. This is typically faster than the cost of the index write so 
makes things faster overall.  I adopted this same model with the 
fsmonitor extension.



If you *must* add these comparisons, you have to use fspathcmp() and
fspathncmp() instead (because case-insensitivity).


Thanks.

Re: t9128 failing randomly with svn 1.9?

[brian m. carlson]

On Mon, Jan 29, 2018 at 12:06:27PM +, Eric Wong wrote:
> Todd Zullinger  wrote:
> diff --git a/git-svn.perl b/git-svn.perl
> index 76a75d0b3d..2ba14269bb 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -1200,6 +1200,11 @@ sub cmd_branch {
>   $ctx->copy($src, $rev, $dst)
>   unless $_dry_run;
>  
> + # Release resources held by ctx before creating another SVN::Ra
> + # so destruction is orderly.  This seems necessary Subversion 1.9.5
> + # to avoid segfaults.
> + $ctx = undef;
> +

This may be the right thing to do.  I've seen a decent number of cases
in Perl where global destruction randomly causes segfaults.

>   $gs->fetch_all;
>  }
>  
> 
> I'll be looping t9128, t9141 and t9167 with that for a few
> hours or day.  Will report back sooner if it fails.
> I'm on an ancient 32-bit system, I guess you guys encountered
> it on 64-bit machines?

Yes, both systems are 64-bit Debian systems, one stable, and the other
unstable.
-- 
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: How juggle branches?

[Patryk Obara]

On 29/01/2018 22:24, Andrzej wrote:


I am in master branch and am changing to hbase:
git checkout -b hbase
git push origin hbase


These two commands create new branch called "hbase" in your local repo,
and then in remote repo - so probably not what you wanted to do.


now worse:
I am in branch before_hbase and need change to master
git checkout -b master  - not works because master exists


"git checkout -b name" creates new branch called "name", starting in 
your latest commit and switches you to this new branch.


"git checkout name" switches your working tree to branch "name".

So just drop "-b". You can read more in manual for git-checkout:
https://git-scm.com/docs/git-checkout

(in polish) Jeśli masz jakieś konkretne pytania, to możesz napisać do
mnie po polsku :).

--
| ← Ceci n'est pas une pipe
Patryk Obara

Re: [PATCH 00/37] removal of some c++ keywords

[Duy Nguyen]

On Tue, Jan 30, 2018 at 5:36 AM, Brandon Williams  wrote:
> A while back there was some discussion of getting our codebase into a state
> where we could use a c++ compiler if we wanted to (for various reason like
> leveraging c++ only analysis tools, etc.).  Johannes Sixt had a very large

I would be more convinced if I knew exactly what leverage we could
have here (e.g. what tool, how many problems it caught...).

> patch that achieved this but it wasn't in a state where it could be 
> upstreamed.
> I took that idea and did some removals of c++ keywords (new, template, try,
> this, etc) but broke it up into several (well maybe more than several) 
> patches.
> I don't believe I've captured all of them in this series but this is at least
> moving a step closer to being able to compile using a c++ compiler.

Is it simpler (though hacky) to just  do

#ifdef __cplusplus
#define new not_new
#define try really_try
...

somewhere in git-compat-util.h?

Do we use any C features that are incompatible with C++? (or do we not
need to care?)

> I don't know if this is something the community still wants to move towards,
> but if this is something people are still interested in, and this series is
> wanted, then we can keep doing these sort of conversions in chunks slowly.

You're going to need to setup C++ build job on Travis or something to
catch new C++ keywords from entering the code base as well if you move
this to the end.
-- 
Duy

Re: Bug/comment

[Andrew Ardill]

Hi Ilija,

On 30 January 2018 at 10:21, Ilija Pecelj  wrote:
> Though it might not be considered a bug 'per se' it is definitely wired.
> Namely, when you type 'yes' word and hit enter in git bash for widnows, the
> process enters infinite loop and just prints 'y' letter in new line.

What you are seeing is a program called `yes`, the job of which is to
print the letter 'y' (or something else if requested) on a new line as
often as it can.

It is used, for example, when you want to answer 'yes' to all the
prompts a program may ask. See more at
https://en.wikipedia.org/wiki/Yes_(Unix)

I agree it's a little weird if you have no idea what it's doing, but
it is very useful and very old, used by many many different scripts
etc, and so unlikely to change.

Regards,

Andrew Ardill

Bug/comment

[Ilija Pecelj]

Though it might not be considered a bug 'per se' it is definitely wired. 
Namely, when you type 'yes' word and hit enter in git bash for widnows, 
the process enters infinite loop and just prints 'y' letter in new line. 
It can be interrupted with CTRL+C. I'm not sure if it has any other 
consequences other than printing letter 'y' in infinite loop.



Cheers,

Ilija

[PATCH] git-svn: control destruction order to avoid segfault

[Eric Wong]

Todd Zullinger  wrote:
> I'm running the tests with and without your patch as well.
> So far I've run t9128 300 times with the patch and no
> failures.  Without it, it's failed 3 times in only a few
> dozen runs.  That's promising.

Thanks for confirming it works on other systems.
Pull request and patch below:

The following changes since commit 5be1f00a9a701532232f57958efab4be8c959a29:

  First batch after 2.16 (2018-01-23 13:21:10 -0800)

are available in the Git repository at:

  git://bogomips.org/git-svn.git svn-branch-segfault

for you to fetch changes up to 2784b8d68faca823489949cbc69ead2f296cfc07:

  git-svn: control destruction order to avoid segfault (2018-01-29 23:12:00 
+)


Eric Wong (1):
  git-svn: control destruction order to avoid segfault

 git-svn.perl | 5 +
 1 file changed, 5 insertions(+)

-8<-
Subject: [PATCH] git-svn: control destruction order to avoid segfault

It seems necessary to control destruction ordering to avoid a
segfault with SVN 1.9.5 when using "git svn branch".
I've also reported the problem against libsvn-perl to Debian
[Bug #888791], but releasing the SVN::Client instance can be
beneficial anyways to save memory.

ref: https://bugs.debian.org/888791
Tested-by: Todd Zullinger 
Reported-by: brian m. carlson 
Signed-off-by: Eric Wong 
---
 git-svn.perl | 5 +
 1 file changed, 5 insertions(+)

diff --git a/git-svn.perl b/git-svn.perl
index 76a75d0b3d..a6b6c3e40c 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1200,6 +1200,11 @@ sub cmd_branch {
$ctx->copy($src, $rev, $dst)
unless $_dry_run;
 
+   # Release resources held by ctx before creating another SVN::Ra
+   # so destruction is orderly.  This seems necessary with SVN 1.9.5
+   # to avoid segfaults.
+   $ctx = undef;
+
$gs->fetch_all;
 }
 
-- 
EW

Re: Some rough edges of core.fsmonitor

[Ben Peart]

On 1/29/2018 4:40 AM, Duy Nguyen wrote:

On Sat, Jan 27, 2018 at 12:43:41PM +0100, Ævar Arnfjörð Bjarmason wrote:

b) with fsmonitor

 $ time GIT_TRACE_PERFORMANCE=1 ~/g/git/git-status
 12:34:23.833625 read-cache.c:1890   performance: 0.049485685 s: read 
cache .git/index


This is sort of off topic but may be interesting for big repo guys. It
looks like read cache's time is partially dominated by malloc().



That is correct.  We have tried a few different ways to address this. 
First was my patch series [1] that would parallelize all of the read 
cache code.


We quickly found that malloc() was the biggest culprit and by speeding 
that up, we got most of the wins.  At Peff's recommendation [2], we 
looked into using tcmalloc but found that 1) it has bugs on Windows and 
2) it isn't being actively maintained so it didn't seem those bugs would 
ever get fixed.


We are currently working on a patch that will use a refactored version 
of the mem_pool in fast-import.c to do block allocations of the cache 
entries which is giving us about a 22% improvement in "git status" 
times.  One challenge has been ensuring that cache entries are not 
passed from one index/mem_pool to another which could cause access after 
free bugs.


[1] 
https://public-inbox.org/git/20171109141737.47976-1-benpe...@microsoft.com/
[2] 
https://public-inbox.org/git/20171120153846.v5b7ho42yzrzn...@sigill.intra.peff.net/




This is the performance breakdown of do_read_index()

$ GIT_TRACE_PERFORMANCE=2 ~/w/git/t/helper/test-read-cache
0.78489 s: open/mmap/close
0.038915239 s: main entries
0.018983150 s: ext TREE
0.012667080 s: ext UNTR
0.05372 s: ext FSMN
0.001473470 s: munmap
0.072386911 s: read cache .git/index

Reading main index entries takes like half of the time (0.038 vs
0.072). With the below patch to take out hundred thousands of malloc()
we have this, loading main entries now only takes 0.012s:

$ GIT_TRACE_PERFORMANCE=2 ~/w/git/t/helper/test-read-cache
0.46587 s: open/mmap/close
0.012077300 s: main entries
0.020477683 s: ext TREE
0.010259998 s: ext UNTR
0.10250 s: ext FSMN
0.000753854 s: munmap
0.043906473 s: read cache .git/index

We used to do less malloc until debed2a629 (read-cache.c: allocate
index entries individually - 2011-10-24) but I don't think we can
simply revert that (not worth the extra complexity of the old way).

Now "TREE" and "UNTR" extensions become a bigger problem.

-- 8< --
diff --git a/read-cache.c b/read-cache.c
index d60e0a8480..88f4213c99 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1622,7 +1622,12 @@ static struct cache_entry 
*cache_entry_from_ondisk(struct ondisk_cache_entry *on
   const char *name,
   size_t len)
  {
+#if 0
struct cache_entry *ce = xmalloc(cache_entry_size(len));
+#else
+   static char buf[1024];
+   struct cache_entry *ce = (struct cache_entry *)buf;
+#endif
  
  	ce->ce_stat_data.sd_ctime.sec = get_be32(>ctime.sec);

ce->ce_stat_data.sd_mtime.sec = get_be32(>mtime.sec);
diff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c
index 48255eef31..e1d21d17a3 100644
--- a/t/helper/test-read-cache.c
+++ b/t/helper/test-read-cache.c
@@ -8,7 +8,9 @@ int cmd_main(int argc, const char **argv)
setup_git_directory();
for (i = 0; i < cnt; i++) {
read_cache();
+#if 0
discard_cache();
+#endif
}
return 0;
  }
-- 8< --

Hello Dear,

[Patricia Long,]

Did you receive and understand the previous email i sent you? Please let me
Know.

Re: [PATCH v3 3/3] sequencer: run 'prepare-commit-msg' hook

[Johannes Schindelin]

Hi Phillip,

On Wed, 24 Jan 2018, Phillip Wood wrote:

> diff --git a/sequencer.h b/sequencer.h
> index 
> 24401b07d57b7ca875dea939f465f3e6cf1162a5..e45b178dfc41d723bf186f20674c4515d7c7fa00
>  100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -1,6 +1,7 @@
>  #ifndef SEQUENCER_H
>  #define SEQUENCER_H
>  
> +const char *git_path_commit_editmsg(void);
>  const char *git_path_seq_dir(void);
>  
>  #define APPEND_SIGNOFF_DEDUP (1u << 0)

I would rather have stuck this into `commit.h` and `commit.c`, but it does
not really matter all that much. The rest looks good (if a little verbose
on the test front, I think testing just the cherry-pick would have
exercised the code path enough).

All three patches are Reviewed-by: me.

Ciao,
Dscho

[PATCH v2 10/10] 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`. So far, the new branch structure would be:

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

This is unintuitive behavior. Even worse, when recreating branch
structure, most use cases would appear to want cousins *not* to be
rebased onto the new base commit. For example, Git for Windows (the
heaviest user of the Git garden shears, which served as the blueprint
for --recreate-merges) frequently merges branches from `next` early, and
these branches certainly do *not* want to be rebased. 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 let's not rebase them by default, introducing the new
"rebase-cousins" mode for use cases where they should be rebased.

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   |  6 ++
 t/t3430-rebase-recreate-merges.sh | 23 +++
 7 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index e9da7e26329..0e6d020d924 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -368,11 +368,16 @@ 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 recreated automatically, but have to be recreated
manually.
++
+By default, or when `no-rebase-cousins` was specified, commits which do not
+have `` as direct ancestor keep their original branch point.
+If the `rebase-cousins` mode is turned on, such commits are rebased onto
+`` (or ``, if specified).
 
 -p::
 --preserve-merges::
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index a34ab5c0655..cea99cb3235 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, 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, "rebase-cousins", _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 |= rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;
flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
 
+   if (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 97b7954f7d3..5e21e4cf269 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -903,6 +903,7 @@ if test t != 

Re: Shawn Pearce has died

[Christian Couder]

On Mon, Jan 29, 2018 at 6:21 PM, Jeff King  wrote:
> On Mon, Jan 29, 2018 at 10:33:08AM +0100, Johannes Schindelin wrote:
>
>> I found these sad news in my timeline today:
>>
>> https://twitter.com/cdibona/status/957822400518696960
>
> Thanks for posting this.

Yeah, thanks.

> I know Shawn has not been all that active on this list in the past few
> years, so many may not know how active he has been behind the scenes:
>
>  - serving on the Git project leadership committee
>
>  - managing many of the contributors whose names you see here daily
>
>  - working on other Git ecosystem projects that aren't on this list
>(like JGit!)

also:

-  organizing most of the first in real life meetings of Git
developers (https://git.wiki.kernel.org/index.php/GitTogether)

[PATCH v2 06/10] 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   |   9 +-
 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, 162 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..e9da7e26329 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -368,6 +368,12 @@ 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 recreated automatically, but have to be recreated
+   manually.
+
 -p::
 --preserve-merges::
Recreate merge commits instead of flattening the history by replaying
@@ -770,7 +776,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
 

[PATCH v2 09/10] pull: accept --rebase=recreate to recreate the branch topology

[Johannes Schindelin]

Similar to the `preserve` mode simply passing the `--preserve-merges`
option to the `rebase` command, the `recreate` mode simply passes the
`--recreate-merges` option.

This will allow users to conveniently rebase non-trivial commit
topologies when pulling new commits, without flattening them.

Signed-off-by: Johannes Schindelin 
---
 Documentation/config.txt   |  8 
 Documentation/git-pull.txt |  5 -
 builtin/pull.c | 14 ++
 builtin/remote.c   |  2 ++
 contrib/completion/git-completion.bash |  2 +-
 5 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0e25b2c92b3..da41ab246dc 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1058,6 +1058,10 @@ branch..rebase::
"git pull" is run. See "pull.rebase" for doing this in a non
branch-specific manner.
 +
+When recreate, also pass `--recreate-merges` along to 'git rebase'
+so that locally committed merge commits will not be flattened
+by running 'git pull'.
++
 When preserve, also pass `--preserve-merges` along to 'git rebase'
 so that locally committed merge commits will not be flattened
 by running 'git pull'.
@@ -2607,6 +2611,10 @@ pull.rebase::
pull" is run. See "branch..rebase" for setting this on a
per-branch basis.
 +
+When recreate, also pass `--recreate-merges` along to 'git rebase'
+so that locally committed merge commits will not be flattened
+by running 'git pull'.
++
 When preserve, also pass `--preserve-merges` along to 'git rebase'
 so that locally committed merge commits will not be flattened
 by running 'git pull'.
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index ce05b7a5b13..b4f9f057ea9 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -101,13 +101,16 @@ Options related to merging
 include::merge-options.txt[]
 
 -r::
---rebase[=false|true|preserve|interactive]::
+--rebase[=false|true|recreate|preserve|interactive]::
When true, rebase the current branch on top of the upstream
branch after fetching. If there is a remote-tracking branch
corresponding to the upstream branch and the upstream branch
was rebased since last fetched, the rebase uses that information
to avoid rebasing non-local changes.
 +
+When set to recreate, rebase with the `--recreate-merges` option passed
+to `git rebase` so that locally created merge commits will not be flattened.
++
 When set to preserve, rebase with the `--preserve-merges` option passed
 to `git rebase` so that locally created merge commits will not be flattened.
 +
diff --git a/builtin/pull.c b/builtin/pull.c
index 511dbbe0f6e..e33c84e0345 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -27,14 +27,16 @@ enum rebase_type {
REBASE_FALSE = 0,
REBASE_TRUE,
REBASE_PRESERVE,
+   REBASE_RECREATE,
REBASE_INTERACTIVE
 };
 
 /**
  * Parses the value of --rebase. If value is a false value, returns
  * REBASE_FALSE. If value is a true value, returns REBASE_TRUE. If value is
- * "preserve", returns REBASE_PRESERVE. If value is a invalid value, dies with
- * a fatal error if fatal is true, otherwise returns REBASE_INVALID.
+ * "recreate", returns REBASE_RECREATE. If value is "preserve", returns
+ * REBASE_PRESERVE. If value is a invalid value, dies with a fatal error if
+ * fatal is true, otherwise returns REBASE_INVALID.
  */
 static enum rebase_type parse_config_rebase(const char *key, const char *value,
int fatal)
@@ -47,6 +49,8 @@ static enum rebase_type parse_config_rebase(const char *key, 
const char *value,
return REBASE_TRUE;
else if (!strcmp(value, "preserve"))
return REBASE_PRESERVE;
+   else if (!strcmp(value, "recreate"))
+   return REBASE_RECREATE;
else if (!strcmp(value, "interactive"))
return REBASE_INTERACTIVE;
 
@@ -130,7 +134,7 @@ static struct option pull_options[] = {
/* Options passed to git-merge or git-rebase */
OPT_GROUP(N_("Options related to merging")),
{ OPTION_CALLBACK, 'r', "rebase", _rebase,
- "false|true|preserve|interactive",
+ "false|true|recreate|preserve|interactive",
  N_("incorporate changes by rebasing rather than merging"),
  PARSE_OPT_OPTARG, parse_opt_rebase },
OPT_PASSTHRU('n', NULL, _diffstat, NULL,
@@ -798,7 +802,9 @@ static int run_rebase(const struct object_id *curr_head,
argv_push_verbosity();
 
/* Options passed to git-rebase */
-   if (opt_rebase == REBASE_PRESERVE)
+   if (opt_rebase == REBASE_RECREATE)
+   argv_array_push(, "--recreate-merges");
+   else if (opt_rebase == REBASE_PRESERVE)
argv_array_push(, "--preserve-merges");
else if (opt_rebase == REBASE_INTERACTIVE)

[PATCH v2 07/10] sequencer: make refs generated by the `label` command worktree-local

[Johannes Schindelin]

This allows for rebases to be run in parallel in separate worktrees
(think: interrupted in the middle of one rebase, being asked to perform
a different rebase, adding a separate worktree just for that job).

Signed-off-by: Johannes Schindelin 
---
 refs.c|  3 ++-
 t/t3430-rebase-recreate-merges.sh | 14 ++
 2 files changed, 16 insertions(+), 1 deletion(-)

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/t/t3430-rebase-recreate-merges.sh 
b/t/t3430-rebase-recreate-merges.sh
index b5ea4130bb5..5295bb03dc0 100755
--- a/t/t3430-rebase-recreate-merges.sh
+++ b/t/t3430-rebase-recreate-merges.sh
@@ -143,4 +143,18 @@ 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_done
-- 
2.16.1.windows.1

[PATCH v2 08/10] 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` and `reset`) do not create new commits, therefore
post-rewrite & autosquash do not need to handle them.

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

diff --git a/sequencer.c b/sequencer.c
index d5af315a440..4cc73775394 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2414,10 +2414,13 @@ static int pick_commits(struct todo_list *todo_list, 
struct replay_opts *opts)
res = do_label(item->arg, item->arg_len);
else if (item->command == TODO_RESET)
res = do_reset(item->arg, item->arg_len);
-   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++;
@@ -3560,7 +3563,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 5295bb03dc0..2eeda0c512b 100755
--- a/t/t3430-rebase-recreate-merges.sh
+++ b/t/t3430-rebase-recreate-merges.sh
@@ -157,4 +157,29 @@ test_expect_success 'refs/rewritten/* is worktree-local' '
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.16.1.windows.1

[PATCH v2 00/10] 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.

Changes since v1:

- reintroduced "sequencer: make refs generated by the `label` command
  worktree-local" (which was squashed into "sequencer: handle autosquash
  and post-rewrite for merge commands" by accident)

- got rid of the universally-hated `bud` command

- as per Stefan's suggestion, the help blurb at the end of the todo list
  now lists the syntax

- the no-rebase-cousins mode was made the default; This not only reflects
  the experience won from those years of using the Git garden shears, but
  was also deemed the better default in the discussion on the PR at
  https://github.com/git/git/pull/447

- I tried to clarify the role of the `onto` label in the commit message of
  `rebase-helper --make-script: introduce a flag to recreate merges`

- fixed punctuation at the end of error(...) messages, and incorrect
  upper-case at the start

- changed the generated todo lists to separate the label and the oneline in
  the `reset` command with a `#`, for readability

- dropped redundant paragraph in the commit message that talked about
  support for octopus merges

- avoided empty error message when HEAD could not be read during do_label()

- merge commits are fast-forwarded only unless --force-rebase was passed

- do_merge() now errors out a lot earlier when HEAD could not be parsed

- the one-letter variables to hold either abbreviated or full todo list
  instructions in make_script_recreating_merges() were renamed to clearer
  names

- The description of rebase's --recreate-merge option has been reworded;
  Hopefully it is a lot more clear now.


Johannes Schindelin (9):
  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: make refs generated by the `label` command worktree-local
  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

Stefan Beller (1):
  git-rebase--interactive: clarify arguments

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


base-commit: 5be1f00a9a701532232f57958efab4be8c959a29
Published-As: https://github.com/dscho/git/releases/tag/recreate-merges-v2
Fetch-It-Via: git fetch https://github.com/dscho/git recreate-merges-v2

Interdiff vs v1:
 diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
 index ac07a5c3fc9..0e6d020d924 100644
 --- a/Documentation/git-rebase.txt
 +++ b/Documentation/git-rebase.txt
 @@ -371,12 +371,13 @@ have the long commit hash prepended to the format.
  

[PATCH v2 05/10] rebase-helper --make-script: introduce a flag to recreate merges

[Johannes Schindelin]

The sequencer just learned 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 (where the HEAD points to C):

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

As a special, hard-coded label, all merge-recreating todo lists start with
the command `label onto` so that we can later always refer to the revision
onto which everything is rebased.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase--helper.c |   4 +-
 sequencer.c  | 346 ++-
 sequencer.h  |   1 +
 3 files changed, 348 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 df61e7883b3..d5af315a440 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"
 
@@ -2786,6 +2788,338 @@ 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);
+   

[PATCH v2 04/10] 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 dfc9f9e13cd..df61e7883b3 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2088,7 +2088,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++)
@@ -2154,6 +2154,14 @@ static int do_merge(struct commit *commit, const char 
*arg, int arg_len,
strbuf_release();
}
 
+   /*
+* If HEAD is not identical to the parent of the original merge commit,
+* we cannot fast-forward.
+*/
+   can_fast_forward = opts->allow_ff && 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) {
@@ -2167,6 +2175,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.16.1.windows.1

[PATCH v2 03/10] 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` 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
reset onto
pick deadbeef Hello, world!
label abc

reset onto
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 a separate
patch series, support for merges using strategies other than the
recursive merge is left for the future.

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

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 7e5281e74aa..d6fd30f6c09 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -164,6 +164,10 @@ 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
+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.
 
 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 92ca8d2adee..dfc9f9e13cd 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -778,6 +778,7 @@ enum todo_command {
TODO_EXEC,
TODO_LABEL,
TODO_RESET,
+   TODO_MERGE,
/* commands that do nothing but are counted for reporting progress */
TODO_NOOP,
TODO_DROP,
@@ -798,6 +799,7 @@ static struct {
{ 'x', "exec" },
{ 'l', "label" },
{ 't', "reset" },
+   { 'm', "merge" },
{ 0,   "noop" },
{ 'd', "drop" },
{ 0,   NULL }
@@ -1302,14 +1304,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);
+
+   if (item->command == TODO_MERGE && *bol == '-' &&
+   bol + 1 == end_of_object_name) {
+   item->commit = NULL;
+   return 0;
+   }
+
saved = *end_of_object_name;
*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;
 
@@ -2072,6 +2080,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;
+
+   head_commit = lookup_commit_reference_by_name("HEAD");
+   if (!head_commit) {
+   rollback_lock_file();
+   return error(_("cannot merge without a current revision"));
+   }
+
+   if (commit) {
+   const char *message = get_commit_buffer(commit, NULL);
+   const char *body;
+   int len;
+
+   if 

[PATCH v2 02/10] 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 

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

Later in this patch series, we will mark the `refs/rewritten/` refs as
worktree-local, to allow for interactive rebases to be run in parallel in
worktrees linked to the same repository.

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

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index fcedece1860..7e5281e74aa 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -162,6 +162,8 @@ 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
 
 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..92ca8d2adee 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,8 @@ enum todo_command {
TODO_SQUASH,
/* commands that do something else than handling a single commit */
TODO_EXEC,
+   TODO_LABEL,
+   TODO_RESET,
/* commands that do nothing but are counted for reporting progress */
TODO_NOOP,
TODO_DROP,
@@ -785,6 +796,8 @@ static struct {
{ 'f', "fixup" },
{ 's', "squash" },
{ 'x', "exec" },
+   { 'l', "label" },
+   { 't', "reset" },
{ 0,   "noop" },
{ 'd', "drop" },
{ 0,   NULL }
@@ -1253,7 +1266,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;
@@ -1279,7 +1293,8 @@ static int parse_insn_line(struct todo_item *item, const 
char *bol, char *eol)
return error(_("missing arguments for %s"),
 command_to_string(item->command));
 
-   if (item->command == TODO_EXEC) {
+   if (item->command == TODO_EXEC || item->command == TODO_LABEL ||
+   item->command == TODO_RESET) {
item->commit = NULL;
item->arg = bol;
item->arg_len = (int)(eol - bol);
@@ -1919,6 +1934,144 @@ static int do_exec(const char *command_line)
return status;
 }
 
+static int safe_append(const char *filename, const char *fmt, ...)
+{
+   va_list ap;
+   struct lock_file lock = LOCK_INIT;
+   int fd = hold_lock_file_for_update(, 

[PATCH v2 01/10] git-rebase--interactive: clarify arguments

[Johannes Schindelin]

From: 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 are about to introduce commands that take different
arguments, clarify each command by giving the argument list.

Signed-off-by: Stefan Beller 
Signed-off-by: Johannes Schindelin 
---
 git-rebase--interactive.sh | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d47bd29593a..fcedece1860 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -155,13 +155,13 @@ 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
+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
 
 These lines can be re-ordered; they are executed from top to bottom.
 " | git stripspace --comment-lines >>"$todo"
-- 
2.16.1.windows.1

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

[Johannes Schindelin]

Hi Junio,

On Tue, 23 Jan 2018, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > My original attempt was --preserve-merges, but that design was so
> > limited that I did not even enable it in interactive mode.
> > ...
> > 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 left comments on a handful of them, but I do not think any of them
> spotted a grave design issue to be a show stopper.  Overall, the
> series was quite a pleasant read, even with those minor nits and
> rooms for improvements.

I objected to a couple obviously problematic suggestions, and I
implemented all others (even those to which I did not respond
specifically).

Ciao,
Dscho

[PATCH 13/37] remote: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 builtin/remote.c | 44 ++--
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index d95bf904c..4f4783e70 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -322,7 +322,7 @@ static void read_branches(void)
 
 struct ref_states {
struct remote *remote;
-   struct string_list new, stale, tracked, heads, push;
+   struct string_list new_refs, stale, tracked, heads, push;
int queried;
 };
 
@@ -337,12 +337,12 @@ static int get_ref_states(const struct ref *remote_refs, 
struct ref_states *stat
die(_("Could not get fetch map for refspec %s"),
states->remote->fetch_refspec[i]);
 
-   states->new.strdup_strings = 1;
+   states->new_refs.strdup_strings = 1;
states->tracked.strdup_strings = 1;
states->stale.strdup_strings = 1;
for (ref = fetch_map; ref; ref = ref->next) {
if (!ref->peer_ref || !ref_exists(ref->peer_ref->name))
-   string_list_append(>new, 
abbrev_branch(ref->name));
+   string_list_append(>new_refs, 
abbrev_branch(ref->name));
else
string_list_append(>tracked, 
abbrev_branch(ref->name));
}
@@ -356,7 +356,7 @@ static int get_ref_states(const struct ref *remote_refs, 
struct ref_states *stat
free_refs(stale_refs);
free_refs(fetch_map);
 
-   string_list_sort(>new);
+   string_list_sort(>new_refs);
string_list_sort(>tracked);
string_list_sort(>stale);
 
@@ -547,7 +547,7 @@ static int add_branch_for_removal(const char *refname,
 
 struct rename_info {
const char *old;
-   const char *new;
+   const char *new_name;
struct string_list *remote_branches;
 };
 
@@ -616,33 +616,33 @@ static int mv(int argc, const char **argv)
usage_with_options(builtin_remote_rename_usage, options);
 
rename.old = argv[1];
-   rename.new = argv[2];
+   rename.new_name = argv[2];
rename.remote_branches = _branches;
 
oldremote = remote_get(rename.old);
if (!remote_is_configured(oldremote, 1))
die(_("No such remote: %s"), rename.old);
 
-   if (!strcmp(rename.old, rename.new) && oldremote->origin != 
REMOTE_CONFIG)
+   if (!strcmp(rename.old, rename.new_name) && oldremote->origin != 
REMOTE_CONFIG)
return migrate_file(oldremote);
 
-   newremote = remote_get(rename.new);
+   newremote = remote_get(rename.new_name);
if (remote_is_configured(newremote, 1))
-   die(_("remote %s already exists."), rename.new);
+   die(_("remote %s already exists."), rename.new_name);
 
-   strbuf_addf(, "refs/heads/test:refs/remotes/%s/test", rename.new);
+   strbuf_addf(, "refs/heads/test:refs/remotes/%s/test", 
rename.new_name);
if (!valid_fetch_refspec(buf.buf))
-   die(_("'%s' is not a valid remote name"), rename.new);
+   die(_("'%s' is not a valid remote name"), rename.new_name);
 
strbuf_reset();
strbuf_addf(, "remote.%s", rename.old);
-   strbuf_addf(, "remote.%s", rename.new);
+   strbuf_addf(, "remote.%s", rename.new_name);
if (git_config_rename_section(buf.buf, buf2.buf) < 1)
return error(_("Could not rename config section '%s' to '%s'"),
buf.buf, buf2.buf);
 
strbuf_reset();
-   strbuf_addf(, "remote.%s.fetch", rename.new);
+   strbuf_addf(, "remote.%s.fetch", rename.new_name);
git_config_set_multivar(buf.buf, NULL, NULL, 1);
strbuf_addf(_remote_context, ":refs/remotes/%s/", rename.old);
for (i = 0; i < oldremote->fetch_refspec_nr; i++) {
@@ -655,8 +655,8 @@ static int mv(int argc, const char **argv)
refspec_updated = 1;
strbuf_splice(,
  ptr-buf2.buf + strlen(":refs/remotes/"),
- strlen(rename.old), rename.new,
- strlen(rename.new));
+ strlen(rename.old), rename.new_name,
+ strlen(rename.new_name));
} else
warning(_("Not updating non-default fetch refspec\n"
  "\t%s\n"
@@ -673,7 +673,7 @@ static int mv(int argc, const char **argv)
if (info->remote_name && !strcmp(info->remote_name, 
rename.old)) {
strbuf_reset();
strbuf_addf(, "branch.%s.remote", item->string);
-   git_config_set(buf.buf, rename.new);
+   

[PATCH 12/37] reflog: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 builtin/reflog.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 223372531..c1bcab5c5 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -289,20 +289,20 @@ static int should_expire_reflog_ent(struct object_id 
*ooid, struct object_id *no
const char *message, void *cb_data)
 {
struct expire_reflog_policy_cb *cb = cb_data;
-   struct commit *old, *new;
+   struct commit *old, *new_commit;
 
if (timestamp < cb->cmd.expire_total)
return 1;
 
-   old = new = NULL;
+   old = new_commit = NULL;
if (cb->cmd.stalefix &&
-   (!keep_entry(, ooid) || !keep_entry(, noid)))
+   (!keep_entry(, ooid) || !keep_entry(_commit, noid)))
return 1;
 
if (timestamp < cb->cmd.expire_unreachable) {
if (cb->unreachable_expire_kind == UE_ALWAYS)
return 1;
-   if (unreachable(cb, old, ooid) || unreachable(cb, new, noid))
+   if (unreachable(cb, old, ooid) || unreachable(cb, new_commit, 
noid))
return 1;
}
 
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 11/37] pack-redundant: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 builtin/pack-redundant.c | 48 
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index aaa813632..e18b53df8 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -48,17 +48,17 @@ static inline void llist_item_put(struct llist_item *item)
 
 static inline struct llist_item *llist_item_get(void)
 {
-   struct llist_item *new;
+   struct llist_item *new_item;
if ( free_nodes ) {
-   new = free_nodes;
+   new_item = free_nodes;
free_nodes = free_nodes->next;
} else {
int i = 1;
-   ALLOC_ARRAY(new, BLKSIZE);
+   ALLOC_ARRAY(new_item, BLKSIZE);
for (; i < BLKSIZE; i++)
-   llist_item_put([i]);
+   llist_item_put(_item[i]);
}
-   return new;
+   return new_item;
 }
 
 static void llist_free(struct llist *list)
@@ -80,26 +80,26 @@ static inline void llist_init(struct llist **list)
 static struct llist * llist_copy(struct llist *list)
 {
struct llist *ret;
-   struct llist_item *new, *old, *prev;
+   struct llist_item *new_item, *old, *prev;
 
llist_init();
 
if ((ret->size = list->size) == 0)
return ret;
 
-   new = ret->front = llist_item_get();
-   new->sha1 = list->front->sha1;
+   new_item = ret->front = llist_item_get();
+   new_item->sha1 = list->front->sha1;
 
old = list->front->next;
while (old) {
-   prev = new;
-   new = llist_item_get();
-   prev->next = new;
-   new->sha1 = old->sha1;
+   prev = new_item;
+   new_item = llist_item_get();
+   prev->next = new_item;
+   new_item->sha1 = old->sha1;
old = old->next;
}
-   new->next = NULL;
-   ret->back = new;
+   new_item->next = NULL;
+   ret->back = new_item;
 
return ret;
 }
@@ -108,24 +108,24 @@ static inline struct llist_item *llist_insert(struct 
llist *list,
  struct llist_item *after,
   const unsigned char *sha1)
 {
-   struct llist_item *new = llist_item_get();
-   new->sha1 = sha1;
-   new->next = NULL;
+   struct llist_item *new_item = llist_item_get();
+   new_item->sha1 = sha1;
+   new_item->next = NULL;
 
if (after != NULL) {
-   new->next = after->next;
-   after->next = new;
+   new_item->next = after->next;
+   after->next = new_item;
if (after == list->back)
-   list->back = new;
+   list->back = new_item;
} else {/* insert in front */
if (list->size == 0)
-   list->back = new;
+   list->back = new_item;
else
-   new->next = list->front;
-   list->front = new;
+   new_item->next = list->front;
+   list->front = new_item;
}
list->size++;
-   return new;
+   return new_item;
 }
 
 static inline struct llist_item *llist_insert_back(struct llist *list,
-- 
2.16.0.rc1.238.g530d649a79-goog

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

[Johannes Schindelin]

Hi Junio,

On Tue, 23 Jan 2018, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > structure (similar in spirit to --preserve-merges, but with a
> > substantially less-broken design).
> > ...
> > @@ -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);
> > +}
> 
> label_oid() accesses state->labels hash using strihash() as the hash
> function, but the final comparison between the entries in the same
> hash buckets are done with case sensitivity.  It is unclear to me if
> that is what was intended, and why.

Heh... you were almost there with your analysis. strihash() is needed for
case-insensitive comparisons, and labels are... ref names.

So the idea (which I implemented only partially) was to make the labels
case-insensitive based on `ignore_case`.

I think the best way forward will be to copy
merge-recursive.c:path_hash().

> > +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';
> > +   if (!hashmap_get_from_hash(>labels,
> > +  strihash(p), p))
> > +   break;
> > +   p[i] = save;
> > +   }
> > +   }
> 
> If oid->hash required full 40-hex to disambiguate, then
> find-unique-abbrev would give 40-hex and we'd want the same "-"
> suffix technique employed below to make it consistently unique.  I
> wonder if organizing the function this way ...
> 
>   if (!label)
>   label = oid-to-hex(oid);
> 
>   if (label already exists or full oid) {
>   make it unambiguous;
>   }

It was hard to miss, I agree. The first arm of the if() is not trying to
make a label unambiguous by adding `-`, but by extending the
abbreviated 40-hex until it is unique. (And we guarantee that there is a
solution, as we do not allow valid 40-hex strings to be used as label.)

The reason: if no `label` is provided, we want a valid raw (possibly
abbreviated) commit name. If `label` is provided, we want a valid ref name
(possibly made unique by appending `-`).

Different beasts. Cannot be put into the same if() arm.

> A related tangent.  Does an auto-label given to "uninteresting"
> commit need to be visible to end users?

Yes. The reason for this is the `merge`/`reset` commands when *not*
rebasing cousins. Because in that case, we have to refer to commits that
we could not possibly have `label`ed, because they were never the current
revision. Therefore we cannot assign - labels, but 

[PATCH 10/37] help: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 builtin/help.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/help.c b/builtin/help.c
index d3c8fc408..598867cfe 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -194,11 +194,11 @@ static void do_add_man_viewer_info(const char *name,
   size_t len,
   const char *value)
 {
-   struct man_viewer_info_list *new;
-   FLEX_ALLOC_MEM(new, name, name, len);
-   new->info = xstrdup(value);
-   new->next = man_viewer_info_list;
-   man_viewer_info_list = new;
+   struct man_viewer_info_list *new_man_viewer;
+   FLEX_ALLOC_MEM(new_man_viewer, name, name, len);
+   new_man_viewer->info = xstrdup(value);
+   new_man_viewer->next = man_viewer_info_list;
+   man_viewer_info_list = new_man_viewer;
 }
 
 static int add_man_viewer_path(const char *name,
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 16/37] diff-lib: rename 'new' variable

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 diff-lib.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 8104603a3..46375abb4 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -302,7 +302,7 @@ static int get_stat_data(const struct cache_entry *ce,
 }
 
 static void show_new_file(struct rev_info *revs,
- const struct cache_entry *new,
+ const struct cache_entry *new_file,
  int cached, int match_missing)
 {
const struct object_id *oid;
@@ -313,16 +313,16 @@ static void show_new_file(struct rev_info *revs,
 * New file in the index: it might actually be different in
 * the working tree.
 */
-   if (get_stat_data(new, , , cached, match_missing,
+   if (get_stat_data(new_file, , , cached, match_missing,
_submodule, >diffopt) < 0)
return;
 
-   diff_index_show_file(revs, "+", new, oid, !is_null_oid(oid), mode, 
dirty_submodule);
+   diff_index_show_file(revs, "+", new_file, oid, !is_null_oid(oid), mode, 
dirty_submodule);
 }
 
 static int show_modified(struct rev_info *revs,
 const struct cache_entry *old,
-const struct cache_entry *new,
+const struct cache_entry *new_entry,
 int report_missing,
 int cached, int match_missing)
 {
@@ -330,7 +330,7 @@ static int show_modified(struct rev_info *revs,
const struct object_id *oid;
unsigned dirty_submodule = 0;
 
-   if (get_stat_data(new, , , cached, match_missing,
+   if (get_stat_data(new_entry, , , cached, match_missing,
  _submodule, >diffopt) < 0) {
if (report_missing)
diff_index_show_file(revs, "-", old,
@@ -340,21 +340,21 @@ static int show_modified(struct rev_info *revs,
}
 
if (revs->combine_merges && !cached &&
-   (oidcmp(oid, >oid) || oidcmp(>oid, >oid))) {
+   (oidcmp(oid, >oid) || oidcmp(>oid, _entry->oid))) {
struct combine_diff_path *p;
-   int pathlen = ce_namelen(new);
+   int pathlen = ce_namelen(new_entry);
 
p = xmalloc(combine_diff_path_size(2, pathlen));
p->path = (char *) >parent[2];
p->next = NULL;
-   memcpy(p->path, new->name, pathlen);
+   memcpy(p->path, new_entry->name, pathlen);
p->path[pathlen] = 0;
p->mode = mode;
oidclr(>oid);
memset(p->parent, 0, 2 * sizeof(struct combine_diff_parent));
p->parent[0].status = DIFF_STATUS_MODIFIED;
-   p->parent[0].mode = new->ce_mode;
-   oidcpy(>parent[0].oid, >oid);
+   p->parent[0].mode = new_entry->ce_mode;
+   oidcpy(>parent[0].oid, _entry->oid);
p->parent[1].status = DIFF_STATUS_MODIFIED;
p->parent[1].mode = old->ce_mode;
oidcpy(>parent[1].oid, >oid);
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 15/37] commit: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 commit.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/commit.c b/commit.c
index cd9ace105..874b6e510 100644
--- a/commit.c
+++ b/commit.c
@@ -861,19 +861,19 @@ struct commit_list *get_octopus_merge_bases(struct 
commit_list *in)
commit_list_insert(in->item, );
 
for (i = in->next; i; i = i->next) {
-   struct commit_list *new = NULL, *end = NULL;
+   struct commit_list *new_commits = NULL, *end = NULL;
 
for (j = ret; j; j = j->next) {
struct commit_list *bases;
bases = get_merge_bases(i->item, j->item);
-   if (!new)
-   new = bases;
+   if (!new_commits)
+   new_commits = bases;
else
end->next = bases;
for (k = bases; k; k = k->next)
end = k;
}
-   ret = new;
+   ret = new_commits;
}
return ret;
 }
@@ -1617,11 +1617,11 @@ struct commit *get_merge_parent(const char *name)
 struct commit_list **commit_list_append(struct commit *commit,
struct commit_list **next)
 {
-   struct commit_list *new = xmalloc(sizeof(struct commit_list));
-   new->item = commit;
-   *next = new;
-   new->next = NULL;
-   return >next;
+   struct commit_list *new_commit = xmalloc(sizeof(struct commit_list));
+   new_commit->item = commit;
+   *next = new_commit;
+   new_commit->next = NULL;
+   return _commit->next;
 }
 
 const char *find_commit_header(const char *msg, const char *key, size_t 
*out_len)
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 20/37] http: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 http.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/http.c b/http.c
index 597771271..41cfa41a9 100644
--- a/http.c
+++ b/http.c
@@ -1194,14 +1194,14 @@ static struct fill_chain *fill_cfg;
 
 void add_fill_function(void *data, int (*fill)(void *))
 {
-   struct fill_chain *new = xmalloc(sizeof(*new));
+   struct fill_chain *new_fill = xmalloc(sizeof(*new_fill));
struct fill_chain **linkp = _cfg;
-   new->data = data;
-   new->fill = fill;
-   new->next = NULL;
+   new_fill->data = data;
+   new_fill->fill = fill;
+   new_fill->next = NULL;
while (*linkp)
linkp = &(*linkp)->next;
-   *linkp = new;
+   *linkp = new_fill;
 }
 
 void fill_active_slots(void)
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 29/37] unpack-trees: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 unpack-trees.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 96c3327f1..bdedabcd5 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -194,10 +194,10 @@ static int do_add_entry(struct unpack_trees_options *o, 
struct cache_entry *ce,
 static struct cache_entry *dup_entry(const struct cache_entry *ce)
 {
unsigned int size = ce_size(ce);
-   struct cache_entry *new = xmalloc(size);
+   struct cache_entry *new_entry = xmalloc(size);
 
-   memcpy(new, ce, size);
-   return new;
+   memcpy(new_entry, ce, size);
+   return new_entry;
 }
 
 static void add_entry(struct unpack_trees_options *o,
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 21/37] imap-send: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 imap-send.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 36c7c1b4f..ffb0a6eca 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1189,11 +1189,11 @@ static struct imap_store *imap_open_store(struct 
imap_server_conf *srvc, char *f
  */
 static void lf_to_crlf(struct strbuf *msg)
 {
-   char *new;
+   char *new_msg;
size_t i, j;
char lastc;
 
-   /* First pass: tally, in j, the size of the new string: */
+   /* First pass: tally, in j, the size of the new_msg string: */
for (i = j = 0, lastc = '\0'; i < msg->len; i++) {
if (msg->buf[i] == '\n' && lastc != '\r')
j++; /* a CR will need to be added here */
@@ -1201,18 +1201,18 @@ static void lf_to_crlf(struct strbuf *msg)
j++;
}
 
-   new = xmallocz(j);
+   new_msg = xmallocz(j);
 
/*
-* Second pass: write the new string.  Note that this loop is
+* Second pass: write the new_msg string.  Note that this loop is
 * otherwise identical to the first pass.
 */
for (i = j = 0, lastc = '\0'; i < msg->len; i++) {
if (msg->buf[i] == '\n' && lastc != '\r')
-   new[j++] = '\r';
-   lastc = new[j++] = msg->buf[i];
+   new_msg[j++] = '\r';
+   lastc = new_msg[j++] = msg->buf[i];
}
-   strbuf_attach(msg, new, j, j + 1);
+   strbuf_attach(msg, new_msg, j, j + 1);
 }
 
 /*
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 28/37] trailer: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 trailer.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/trailer.c b/trailer.c
index 3ba157ed0..5a4a2ecf9 100644
--- a/trailer.c
+++ b/trailer.c
@@ -174,12 +174,12 @@ static void print_all(FILE *outfile, struct list_head 
*head,
 
 static struct trailer_item *trailer_from_arg(struct arg_item *arg_tok)
 {
-   struct trailer_item *new = xcalloc(sizeof(*new), 1);
-   new->token = arg_tok->token;
-   new->value = arg_tok->value;
+   struct trailer_item *new_item = xcalloc(sizeof(*new_item), 1);
+   new_item->token = arg_tok->token;
+   new_item->value = arg_tok->value;
arg_tok->token = arg_tok->value = NULL;
free_arg_item(arg_tok);
-   return new;
+   return new_item;
 }
 
 static void add_arg_to_input_list(struct trailer_item *on_tok,
@@ -666,30 +666,30 @@ static void parse_trailer(struct strbuf *tok, struct 
strbuf *val,
 static struct trailer_item *add_trailer_item(struct list_head *head, char *tok,
 char *val)
 {
-   struct trailer_item *new = xcalloc(sizeof(*new), 1);
-   new->token = tok;
-   new->value = val;
-   list_add_tail(>list, head);
-   return new;
+   struct trailer_item *new_item = xcalloc(sizeof(*new_item), 1);
+   new_item->token = tok;
+   new_item->value = val;
+   list_add_tail(_item->list, head);
+   return new_item;
 }
 
 static void add_arg_item(struct list_head *arg_head, char *tok, char *val,
 const struct conf_info *conf,
 const struct new_trailer_item *new_trailer_item)
 {
-   struct arg_item *new = xcalloc(sizeof(*new), 1);
-   new->token = tok;
-   new->value = val;
-   duplicate_conf(>conf, conf);
+   struct arg_item *new_item = xcalloc(sizeof(*new_item), 1);
+   new_item->token = tok;
+   new_item->value = val;
+   duplicate_conf(_item->conf, conf);
if (new_trailer_item) {
if (new_trailer_item->where != WHERE_DEFAULT)
-   new->conf.where = new_trailer_item->where;
+   new_item->conf.where = new_trailer_item->where;
if (new_trailer_item->if_exists != EXISTS_DEFAULT)
-   new->conf.if_exists = new_trailer_item->if_exists;
+   new_item->conf.if_exists = new_trailer_item->if_exists;
if (new_trailer_item->if_missing != MISSING_DEFAULT)
-   new->conf.if_missing = new_trailer_item->if_missing;
+   new_item->conf.if_missing = 
new_trailer_item->if_missing;
}
-   list_add_tail(>list, arg_head);
+   list_add_tail(_item->list, arg_head);
 }
 
 static void process_command_line_args(struct list_head *arg_head,
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 33/37] environment: rename 'namespace' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 environment.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/environment.c b/environment.c
index 98f77ea95..270ba98b5 100644
--- a/environment.c
+++ b/environment.c
@@ -98,7 +98,7 @@ int ignore_untracked_cache_config;
 /* This is set by setup_git_dir_gently() and/or git_default_config() */
 char *git_work_tree_cfg;
 
-static char *namespace;
+static char *git_namespace;
 
 static const char *super_prefix;
 
@@ -156,8 +156,8 @@ void setup_git_env(void)
free(git_replace_ref_base);
git_replace_ref_base = xstrdup(replace_ref_base ? replace_ref_base
  : "refs/replace/");
-   free(namespace);
-   namespace = expand_namespace(getenv(GIT_NAMESPACE_ENVIRONMENT));
+   free(git_namespace);
+   git_namespace = expand_namespace(getenv(GIT_NAMESPACE_ENVIRONMENT));
shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
if (shallow_file)
set_alternate_shallow_file(shallow_file, 0);
@@ -191,9 +191,9 @@ const char *get_git_common_dir(void)
 
 const char *get_git_namespace(void)
 {
-   if (!namespace)
+   if (!git_namespace)
BUG("git environment hasn't been setup");
-   return namespace;
+   return git_namespace;
 }
 
 const char *strip_namespace(const char *namespaced_ref)
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 27/37] submodule: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 submodule.c | 14 +++---
 submodule.h |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/submodule.c b/submodule.c
index cd18400e8..244c3c21b 100644
--- a/submodule.c
+++ b/submodule.c
@@ -590,7 +590,7 @@ void show_submodule_inline_diff(struct diff_options *o, 
const char *path,
struct object_id *one, struct object_id *two,
unsigned dirty_submodule)
 {
-   const struct object_id *old = the_hash_algo->empty_tree, *new = 
the_hash_algo->empty_tree;
+   const struct object_id *old = the_hash_algo->empty_tree, *new_oid = 
the_hash_algo->empty_tree;
struct commit *left = NULL, *right = NULL;
struct commit_list *merge_bases = NULL;
struct child_process cp = CHILD_PROCESS_INIT;
@@ -607,7 +607,7 @@ void show_submodule_inline_diff(struct diff_options *o, 
const char *path,
if (left)
old = one;
if (right)
-   new = two;
+   new_oid = two;
 
cp.git_cmd = 1;
cp.dir = path;
@@ -638,7 +638,7 @@ void show_submodule_inline_diff(struct diff_options *o, 
const char *path,
 * haven't yet been committed to the submodule yet.
 */
if (!(dirty_submodule & DIRTY_SUBMODULE_MODIFIED))
-   argv_array_push(, oid_to_hex(new));
+   argv_array_push(, oid_to_hex(new_oid));
 
prepare_submodule_repo_env(_array);
if (start_command())
@@ -1579,7 +1579,7 @@ static void submodule_reset_index(const char *path)
  */
 int submodule_move_head(const char *path,
 const char *old,
-const char *new,
+const char *new_head,
 unsigned flags)
 {
int ret = 0;
@@ -1660,7 +1660,7 @@ int submodule_move_head(const char *path,
if (!(flags & SUBMODULE_MOVE_HEAD_FORCE))
argv_array_push(, old ? old : EMPTY_TREE_SHA1_HEX);
 
-   argv_array_push(, new ? new : EMPTY_TREE_SHA1_HEX);
+   argv_array_push(, new_head ? new_head : EMPTY_TREE_SHA1_HEX);
 
if (run_command()) {
ret = -1;
@@ -1668,7 +1668,7 @@ int submodule_move_head(const char *path,
}
 
if (!(flags & SUBMODULE_MOVE_HEAD_DRY_RUN)) {
-   if (new) {
+   if (new_head) {
child_process_init();
/* also set the HEAD accordingly */
cp.git_cmd = 1;
@@ -1677,7 +1677,7 @@ int submodule_move_head(const char *path,
 
prepare_submodule_repo_env(_array);
argv_array_pushl(, "update-ref", "HEAD",
-"--no-deref", new, NULL);
+"--no-deref", new_head, NULL);
 
if (run_command()) {
ret = -1;
diff --git a/submodule.h b/submodule.h
index b9b7ef003..9589f1312 100644
--- a/submodule.h
+++ b/submodule.h
@@ -117,7 +117,7 @@ int submodule_to_gitdir(struct strbuf *buf, const char 
*submodule);
 #define SUBMODULE_MOVE_HEAD_FORCE   (1<<1)
 extern int submodule_move_head(const char *path,
   const char *old,
-  const char *new,
+  const char *new_head,
   unsigned flags);
 
 /*
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 37/37] replace: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 builtin/replace.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 42cf4f62a..e48835b54 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -284,7 +284,7 @@ static int edit_and_replace(const char *object_ref, int 
force, int raw)
 {
char *tmpfile = git_pathdup("REPLACE_EDITOBJ");
enum object_type type;
-   struct object_id old, new, prev;
+   struct object_id old, new_oid, prev;
struct strbuf ref = STRBUF_INIT;
 
if (get_oid(object_ref, ) < 0)
@@ -300,14 +300,14 @@ static int edit_and_replace(const char *object_ref, int 
force, int raw)
export_object(, type, raw, tmpfile);
if (launch_editor(tmpfile, NULL, NULL) < 0)
die("editing object file failed");
-   import_object(, type, raw, tmpfile);
+   import_object(_oid, type, raw, tmpfile);
 
free(tmpfile);
 
-   if (!oidcmp(, ))
+   if (!oidcmp(, _oid))
return error("new object is the same as the old one: '%s'", 
oid_to_hex());
 
-   return replace_object_oid(object_ref, , "replacement", , force);
+   return replace_object_oid(object_ref, , "replacement", _oid, 
force);
 }
 
 static void replace_parents(struct strbuf *buf, int argc, const char **argv)
@@ -386,7 +386,7 @@ static void check_mergetags(struct commit *commit, int 
argc, const char **argv)
 
 static int create_graft(int argc, const char **argv, int force)
 {
-   struct object_id old, new;
+   struct object_id old, new_oid;
const char *old_ref = argv[0];
struct commit *commit;
struct strbuf buf = STRBUF_INIT;
@@ -410,15 +410,15 @@ static int create_graft(int argc, const char **argv, int 
force)
 
check_mergetags(commit, argc, argv);
 
-   if (write_sha1_file(buf.buf, buf.len, commit_type, new.hash))
+   if (write_sha1_file(buf.buf, buf.len, commit_type, new_oid.hash))
die(_("could not write replacement commit for: '%s'"), old_ref);
 
strbuf_release();
 
-   if (!oidcmp(, ))
+   if (!oidcmp(, _oid))
return error("new commit is the same as the old one: '%s'", 
oid_to_hex());
 
-   return replace_object_oid(old_ref, , "replacement", , force);
+   return replace_object_oid(old_ref, , "replacement", _oid, 
force);
 }
 
 int cmd_replace(int argc, const char **argv, const char *prefix)
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 36/37] trailer: rename 'template' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 trailer.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/trailer.c b/trailer.c
index 5a4a2ecf9..c508c9b75 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1000,7 +1000,7 @@ static struct tempfile *trailers_tempfile;
 static FILE *create_in_place_tempfile(const char *file)
 {
struct stat st;
-   struct strbuf template = STRBUF_INIT;
+   struct strbuf filename_template = STRBUF_INIT;
const char *tail;
FILE *outfile;
 
@@ -1014,11 +1014,11 @@ static FILE *create_in_place_tempfile(const char *file)
/* Create temporary file in the same directory as the original */
tail = strrchr(file, '/');
if (tail != NULL)
-   strbuf_add(, file, tail - file + 1);
-   strbuf_addstr(, "git-interpret-trailers-XX");
+   strbuf_add(_template, file, tail - file + 1);
+   strbuf_addstr(_template, "git-interpret-trailers-XX");
 
-   trailers_tempfile = xmks_tempfile_m(template.buf, st.st_mode);
-   strbuf_release();
+   trailers_tempfile = xmks_tempfile_m(filename_template.buf, st.st_mode);
+   strbuf_release(_template);
outfile = fdopen_tempfile(trailers_tempfile, "w");
if (!outfile)
die_errno(_("could not open temporary file"));
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 32/37] diff: rename 'template' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 diff.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/diff.c b/diff.c
index d49732b3b..142a633e1 100644
--- a/diff.c
+++ b/diff.c
@@ -3660,15 +3660,15 @@ static void prep_temp_blob(const char *path, struct 
diff_tempfile *temp,
   int mode)
 {
struct strbuf buf = STRBUF_INIT;
-   struct strbuf template = STRBUF_INIT;
+   struct strbuf tempfile = STRBUF_INIT;
char *path_dup = xstrdup(path);
const char *base = basename(path_dup);
 
/* Generate "XX_basename.ext" */
-   strbuf_addstr(, "XX_");
-   strbuf_addstr(, base);
+   strbuf_addstr(, "XX_");
+   strbuf_addstr(, base);
 
-   temp->tempfile = mks_tempfile_ts(template.buf, strlen(base) + 1);
+   temp->tempfile = mks_tempfile_ts(tempfile.buf, strlen(base) + 1);
if (!temp->tempfile)
die_errno("unable to create temp-file");
if (convert_to_working_tree(path,
@@ -3683,7 +3683,7 @@ static void prep_temp_blob(const char *path, struct 
diff_tempfile *temp,
oid_to_hex_r(temp->hex, oid);
xsnprintf(temp->mode, sizeof(temp->mode), "%06o", mode);
strbuf_release();
-   strbuf_release();
+   strbuf_release();
free(path_dup);
 }
 
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 35/37] tempfile: rename 'template' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 tempfile.c | 12 ++--
 tempfile.h | 34 +-
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/tempfile.c b/tempfile.c
index 5fdafdd2d..139ecd97f 100644
--- a/tempfile.c
+++ b/tempfile.c
@@ -165,11 +165,11 @@ struct tempfile *register_tempfile(const char *path)
return tempfile;
 }
 
-struct tempfile *mks_tempfile_sm(const char *template, int suffixlen, int mode)
+struct tempfile *mks_tempfile_sm(const char *filename_template, int suffixlen, 
int mode)
 {
struct tempfile *tempfile = new_tempfile();
 
-   strbuf_add_absolute_path(>filename, template);
+   strbuf_add_absolute_path(>filename, filename_template);
tempfile->fd = git_mkstemps_mode(tempfile->filename.buf, suffixlen, 
mode);
if (tempfile->fd < 0) {
deactivate_tempfile(tempfile);
@@ -179,7 +179,7 @@ struct tempfile *mks_tempfile_sm(const char *template, int 
suffixlen, int mode)
return tempfile;
 }
 
-struct tempfile *mks_tempfile_tsm(const char *template, int suffixlen, int 
mode)
+struct tempfile *mks_tempfile_tsm(const char *filename_template, int 
suffixlen, int mode)
 {
struct tempfile *tempfile = new_tempfile();
const char *tmpdir;
@@ -188,7 +188,7 @@ struct tempfile *mks_tempfile_tsm(const char *template, int 
suffixlen, int mode)
if (!tmpdir)
tmpdir = "/tmp";
 
-   strbuf_addf(>filename, "%s/%s", tmpdir, template);
+   strbuf_addf(>filename, "%s/%s", tmpdir, filename_template);
tempfile->fd = git_mkstemps_mode(tempfile->filename.buf, suffixlen, 
mode);
if (tempfile->fd < 0) {
deactivate_tempfile(tempfile);
@@ -198,12 +198,12 @@ struct tempfile *mks_tempfile_tsm(const char *template, 
int suffixlen, int mode)
return tempfile;
 }
 
-struct tempfile *xmks_tempfile_m(const char *template, int mode)
+struct tempfile *xmks_tempfile_m(const char *filename_template, int mode)
 {
struct tempfile *tempfile;
struct strbuf full_template = STRBUF_INIT;
 
-   strbuf_add_absolute_path(_template, template);
+   strbuf_add_absolute_path(_template, filename_template);
tempfile = mks_tempfile_m(full_template.buf, mode);
if (!tempfile)
die_errno("Unable to create temporary file '%s'",
diff --git a/tempfile.h b/tempfile.h
index 450908b2e..8959c5f1b 100644
--- a/tempfile.h
+++ b/tempfile.h
@@ -135,58 +135,58 @@ extern struct tempfile *register_tempfile(const char 
*path);
  */
 
 /* See "mks_tempfile functions" above. */
-extern struct tempfile *mks_tempfile_sm(const char *template,
+extern struct tempfile *mks_tempfile_sm(const char *filename_template,
int suffixlen, int mode);
 
 /* See "mks_tempfile functions" above. */
-static inline struct tempfile *mks_tempfile_s(const char *template,
+static inline struct tempfile *mks_tempfile_s(const char *filename_template,
  int suffixlen)
 {
-   return mks_tempfile_sm(template, suffixlen, 0600);
+   return mks_tempfile_sm(filename_template, suffixlen, 0600);
 }
 
 /* See "mks_tempfile functions" above. */
-static inline struct tempfile *mks_tempfile_m(const char *template, int mode)
+static inline struct tempfile *mks_tempfile_m(const char *filename_template, 
int mode)
 {
-   return mks_tempfile_sm(template, 0, mode);
+   return mks_tempfile_sm(filename_template, 0, mode);
 }
 
 /* See "mks_tempfile functions" above. */
-static inline struct tempfile *mks_tempfile(const char *template)
+static inline struct tempfile *mks_tempfile(const char *filename_template)
 {
-   return mks_tempfile_sm(template, 0, 0600);
+   return mks_tempfile_sm(filename_template, 0, 0600);
 }
 
 /* See "mks_tempfile functions" above. */
-extern struct tempfile *mks_tempfile_tsm(const char *template,
+extern struct tempfile *mks_tempfile_tsm(const char *filename_template,
 int suffixlen, int mode);
 
 /* See "mks_tempfile functions" above. */
-static inline struct tempfile *mks_tempfile_ts(const char *template,
+static inline struct tempfile *mks_tempfile_ts(const char *filename_template,
   int suffixlen)
 {
-   return mks_tempfile_tsm(template, suffixlen, 0600);
+   return mks_tempfile_tsm(filename_template, suffixlen, 0600);
 }
 
 /* See "mks_tempfile functions" above. */
-static inline struct tempfile *mks_tempfile_tm(const char *template, int mode)
+static inline struct tempfile *mks_tempfile_tm(const char *filename_template, 
int mode)
 {
-   return mks_tempfile_tsm(template, 0, mode);
+   return mks_tempfile_tsm(filename_template, 0, mode);
 }
 
 /* See "mks_tempfile functions" above. */
-static inline struct 

[PATCH 34/37] wrapper: rename 'template' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 git-compat-util.h |  4 ++--
 wrapper.c | 40 
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 68b2ad531..07e383257 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -826,8 +826,8 @@ extern ssize_t xpread(int fd, void *buf, size_t len, off_t 
offset);
 extern int xdup(int fd);
 extern FILE *xfopen(const char *path, const char *mode);
 extern FILE *xfdopen(int fd, const char *mode);
-extern int xmkstemp(char *template);
-extern int xmkstemp_mode(char *template, int mode);
+extern int xmkstemp(char *temp_filename);
+extern int xmkstemp_mode(char *temp_filename, int mode);
 extern char *xgetcwd(void);
 extern FILE *fopen_for_writing(const char *path);
 extern FILE *fopen_or_warn(const char *path, const char *mode);
diff --git a/wrapper.c b/wrapper.c
index d20356a77..1fd5e33ea 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -445,21 +445,21 @@ FILE *fopen_or_warn(const char *path, const char *mode)
return NULL;
 }
 
-int xmkstemp(char *template)
+int xmkstemp(char *filename_template)
 {
int fd;
char origtemplate[PATH_MAX];
-   strlcpy(origtemplate, template, sizeof(origtemplate));
+   strlcpy(origtemplate, filename_template, sizeof(origtemplate));
 
-   fd = mkstemp(template);
+   fd = mkstemp(filename_template);
if (fd < 0) {
int saved_errno = errno;
const char *nonrelative_template;
 
-   if (strlen(template) != strlen(origtemplate))
-   template = origtemplate;
+   if (strlen(filename_template) != strlen(origtemplate))
+   filename_template = origtemplate;
 
-   nonrelative_template = absolute_path(template);
+   nonrelative_template = absolute_path(filename_template);
errno = saved_errno;
die_errno("Unable to create temporary file '%s'",
nonrelative_template);
@@ -481,7 +481,7 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int 
mode)
static const int num_letters = 62;
uint64_t value;
struct timeval tv;
-   char *template;
+   char *filename_template;
size_t len;
int fd, count;
 
@@ -503,16 +503,16 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int 
mode)
 */
gettimeofday(, NULL);
value = ((size_t)(tv.tv_usec << 16)) ^ tv.tv_sec ^ getpid();
-   template = [len - 6 - suffix_len];
+   filename_template = [len - 6 - suffix_len];
for (count = 0; count < TMP_MAX; ++count) {
uint64_t v = value;
/* Fill in the random bits. */
-   template[0] = letters[v % num_letters]; v /= num_letters;
-   template[1] = letters[v % num_letters]; v /= num_letters;
-   template[2] = letters[v % num_letters]; v /= num_letters;
-   template[3] = letters[v % num_letters]; v /= num_letters;
-   template[4] = letters[v % num_letters]; v /= num_letters;
-   template[5] = letters[v % num_letters]; v /= num_letters;
+   filename_template[0] = letters[v % num_letters]; v /= 
num_letters;
+   filename_template[1] = letters[v % num_letters]; v /= 
num_letters;
+   filename_template[2] = letters[v % num_letters]; v /= 
num_letters;
+   filename_template[3] = letters[v % num_letters]; v /= 
num_letters;
+   filename_template[4] = letters[v % num_letters]; v /= 
num_letters;
+   filename_template[5] = letters[v % num_letters]; v /= 
num_letters;
 
fd = open(pattern, O_CREAT | O_EXCL | O_RDWR, mode);
if (fd >= 0)
@@ -541,21 +541,21 @@ int git_mkstemp_mode(char *pattern, int mode)
return git_mkstemps_mode(pattern, 0, mode);
 }
 
-int xmkstemp_mode(char *template, int mode)
+int xmkstemp_mode(char *filename_template, int mode)
 {
int fd;
char origtemplate[PATH_MAX];
-   strlcpy(origtemplate, template, sizeof(origtemplate));
+   strlcpy(origtemplate, filename_template, sizeof(origtemplate));
 
-   fd = git_mkstemp_mode(template, mode);
+   fd = git_mkstemp_mode(filename_template, mode);
if (fd < 0) {
int saved_errno = errno;
const char *nonrelative_template;
 
-   if (!template[0])
-   template = origtemplate;
+   if (!filename_template[0])
+   filename_template = origtemplate;
 
-   nonrelative_template = absolute_path(template);
+   nonrelative_template = absolute_path(filename_template);
errno = saved_errno;
die_errno("Unable to create temporary file 

[PATCH 31/37] environment: rename 'template' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 cache.h   |  2 +-
 environment.c | 14 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/cache.h b/cache.h
index 69b5a3bf8..5b8d49377 100644
--- a/cache.h
+++ b/cache.h
@@ -1673,7 +1673,7 @@ struct pack_entry {
  * usual "XX" trailer, and the resulting filename is written into the
  * "template" buffer. Returns the open descriptor.
  */
-extern int odb_mkstemp(struct strbuf *template, const char *pattern);
+extern int odb_mkstemp(struct strbuf *temp_filename, const char *pattern);
 
 /*
  * Create a pack .keep file named "name" (which should generally be the output
diff --git a/environment.c b/environment.c
index 63ac38a46..98f77ea95 100644
--- a/environment.c
+++ b/environment.c
@@ -247,7 +247,7 @@ char *get_object_directory(void)
return the_repository->objectdir;
 }
 
-int odb_mkstemp(struct strbuf *template, const char *pattern)
+int odb_mkstemp(struct strbuf *temp_filename, const char *pattern)
 {
int fd;
/*
@@ -255,16 +255,16 @@ int odb_mkstemp(struct strbuf *template, const char 
*pattern)
 * restrictive except to remove write permission.
 */
int mode = 0444;
-   git_path_buf(template, "objects/%s", pattern);
-   fd = git_mkstemp_mode(template->buf, mode);
+   git_path_buf(temp_filename, "objects/%s", pattern);
+   fd = git_mkstemp_mode(temp_filename->buf, mode);
if (0 <= fd)
return fd;
 
/* slow path */
-   /* some mkstemp implementations erase template on failure */
-   git_path_buf(template, "objects/%s", pattern);
-   safe_create_leading_directories(template->buf);
-   return xmkstemp_mode(template->buf, mode);
+   /* some mkstemp implementations erase temp_filename on failure */
+   git_path_buf(temp_filename, "objects/%s", pattern);
+   safe_create_leading_directories(temp_filename->buf);
+   return xmkstemp_mode(temp_filename->buf, mode);
 }
 
 int odb_pack_keep(const char *name)
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 23/37] read-cache: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 read-cache.c | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 2eb81a66b..21b859087 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -70,20 +70,20 @@ static void replace_index_entry(struct index_state *istate, 
int nr, struct cache
 
 void rename_index_entry_at(struct index_state *istate, int nr, const char 
*new_name)
 {
-   struct cache_entry *old = istate->cache[nr], *new;
+   struct cache_entry *old = istate->cache[nr], *new_entry;
int namelen = strlen(new_name);
 
-   new = xmalloc(cache_entry_size(namelen));
-   copy_cache_entry(new, old);
-   new->ce_flags &= ~CE_HASHED;
-   new->ce_namelen = namelen;
-   new->index = 0;
-   memcpy(new->name, new_name, namelen + 1);
+   new_entry = xmalloc(cache_entry_size(namelen));
+   copy_cache_entry(new_entry, old);
+   new_entry->ce_flags &= ~CE_HASHED;
+   new_entry->ce_namelen = namelen;
+   new_entry->index = 0;
+   memcpy(new_entry->name, new_name, namelen + 1);
 
cache_tree_invalidate_path(istate, old->name);
untracked_cache_remove_from_index(istate, old->name);
remove_index_entry_at(istate, nr);
-   add_index_entry(istate, new, 
ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
+   add_index_entry(istate, new_entry, 
ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
 }
 
 void fill_stat_data(struct stat_data *sd, struct stat *st)
@@ -615,18 +615,18 @@ static struct cache_entry *create_alias_ce(struct 
index_state *istate,
   struct cache_entry *alias)
 {
int len;
-   struct cache_entry *new;
+   struct cache_entry *new_entry;
 
if (alias->ce_flags & CE_ADDED)
die("Will not add file alias '%s' ('%s' already exists in 
index)", ce->name, alias->name);
 
/* Ok, create the new entry using the name of the existing alias */
len = ce_namelen(alias);
-   new = xcalloc(1, cache_entry_size(len));
-   memcpy(new->name, alias->name, len);
-   copy_cache_entry(new, ce);
+   new_entry = xcalloc(1, cache_entry_size(len));
+   memcpy(new_entry->name, alias->name, len);
+   copy_cache_entry(new_entry, ce);
save_or_free_index_entry(istate, ce);
-   return new;
+   return new_entry;
 }
 
 void set_object_name_for_intent_to_add_entry(struct cache_entry *ce)
@@ -1379,7 +1379,7 @@ int refresh_index(struct index_state *istate, unsigned 
int flags,
added_fmt = (in_porcelain ? "A\t%s\n" : "%s needs update\n");
unmerged_fmt = (in_porcelain ? "U\t%s\n" : "%s: needs merge\n");
for (i = 0; i < istate->cache_nr; i++) {
-   struct cache_entry *ce, *new;
+   struct cache_entry *ce, *new_entry;
int cache_errno = 0;
int changed = 0;
int filtered = 0;
@@ -1408,10 +1408,10 @@ int refresh_index(struct index_state *istate, unsigned 
int flags,
if (filtered)
continue;
 
-   new = refresh_cache_ent(istate, ce, options, _errno, 
);
-   if (new == ce)
+   new_entry = refresh_cache_ent(istate, ce, options, 
_errno, );
+   if (new_entry == ce)
continue;
-   if (!new) {
+   if (!new_entry) {
const char *fmt;
 
if (really && cache_errno == EINVAL) {
@@ -1440,7 +1440,7 @@ int refresh_index(struct index_state *istate, unsigned 
int flags,
continue;
}
 
-   replace_index_entry(istate, i, new);
+   replace_index_entry(istate, i, new_entry);
}
return has_errors;
 }
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 25/37] remote: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 remote.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/remote.c b/remote.c
index 4e93753e1..6f79881f6 100644
--- a/remote.c
+++ b/remote.c
@@ -1970,12 +1970,12 @@ static void unmark_and_free(struct commit_list *list, 
unsigned int mark)
 int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid)
 {
struct object *o;
-   struct commit *old, *new;
+   struct commit *old, *new_commit;
struct commit_list *list, *used;
int found = 0;
 
/*
-* Both new and old must be commit-ish and new is descendant of
+* Both new_commit and old must be commit-ish and new_commit is 
descendant of
 * old.  Otherwise we require --force.
 */
o = deref_tag(parse_object(old_oid), NULL, 0);
@@ -1986,17 +1986,17 @@ int ref_newer(const struct object_id *new_oid, const 
struct object_id *old_oid)
o = deref_tag(parse_object(new_oid), NULL, 0);
if (!o || o->type != OBJ_COMMIT)
return 0;
-   new = (struct commit *) o;
+   new_commit = (struct commit *) o;
 
-   if (parse_commit(new) < 0)
+   if (parse_commit(new_commit) < 0)
return 0;
 
used = list = NULL;
-   commit_list_insert(new, );
+   commit_list_insert(new_commit, );
while (list) {
-   new = pop_most_recent_commit(, TMP_MARK);
-   commit_list_insert(new, );
-   if (new == old) {
+   new_commit = pop_most_recent_commit(, TMP_MARK);
+   commit_list_insert(new_commit, );
+   if (new_commit == old) {
found = 1;
break;
}
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 30/37] init-db: rename 'template' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 builtin/init-db.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index c9b7946ba..68ff4ad75 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -24,11 +24,11 @@ static int init_is_bare_repository = 0;
 static int init_shared_repository = -1;
 static const char *init_db_template_dir;
 
-static void copy_templates_1(struct strbuf *path, struct strbuf *template,
+static void copy_templates_1(struct strbuf *path, struct strbuf *template_path,
 DIR *dir)
 {
size_t path_baselen = path->len;
-   size_t template_baselen = template->len;
+   size_t template_baselen = template_path->len;
struct dirent *de;
 
/* Note: if ".git/hooks" file exists in the repository being
@@ -44,12 +44,12 @@ static void copy_templates_1(struct strbuf *path, struct 
strbuf *template,
int exists = 0;
 
strbuf_setlen(path, path_baselen);
-   strbuf_setlen(template, template_baselen);
+   strbuf_setlen(template_path, template_baselen);
 
if (de->d_name[0] == '.')
continue;
strbuf_addstr(path, de->d_name);
-   strbuf_addstr(template, de->d_name);
+   strbuf_addstr(template_path, de->d_name);
if (lstat(path->buf, _git)) {
if (errno != ENOENT)
die_errno(_("cannot stat '%s'"), path->buf);
@@ -57,36 +57,36 @@ static void copy_templates_1(struct strbuf *path, struct 
strbuf *template,
else
exists = 1;
 
-   if (lstat(template->buf, _template))
-   die_errno(_("cannot stat template '%s'"), 
template->buf);
+   if (lstat(template_path->buf, _template))
+   die_errno(_("cannot stat template '%s'"), 
template_path->buf);
 
if (S_ISDIR(st_template.st_mode)) {
-   DIR *subdir = opendir(template->buf);
+   DIR *subdir = opendir(template_path->buf);
if (!subdir)
-   die_errno(_("cannot opendir '%s'"), 
template->buf);
+   die_errno(_("cannot opendir '%s'"), 
template_path->buf);
strbuf_addch(path, '/');
-   strbuf_addch(template, '/');
-   copy_templates_1(path, template, subdir);
+   strbuf_addch(template_path, '/');
+   copy_templates_1(path, template_path, subdir);
closedir(subdir);
}
else if (exists)
continue;
else if (S_ISLNK(st_template.st_mode)) {
struct strbuf lnk = STRBUF_INIT;
-   if (strbuf_readlink(, template->buf, 0) < 0)
-   die_errno(_("cannot readlink '%s'"), 
template->buf);
+   if (strbuf_readlink(, template_path->buf, 0) < 0)
+   die_errno(_("cannot readlink '%s'"), 
template_path->buf);
if (symlink(lnk.buf, path->buf))
die_errno(_("cannot symlink '%s' '%s'"),
  lnk.buf, path->buf);
strbuf_release();
}
else if (S_ISREG(st_template.st_mode)) {
-   if (copy_file(path->buf, template->buf, 
st_template.st_mode))
+   if (copy_file(path->buf, template_path->buf, 
st_template.st_mode))
die_errno(_("cannot copy '%s' to '%s'"),
- template->buf, path->buf);
+ template_path->buf, path->buf);
}
else
-   error(_("ignoring template %s"), template->buf);
+   error(_("ignoring template %s"), template_path->buf);
}
 }
 
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 26/37] split-index: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 split-index.c | 10 +-
 split-index.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/split-index.c b/split-index.c
index 83e39ec8d..8ae4bee89 100644
--- a/split-index.c
+++ b/split-index.c
@@ -304,16 +304,16 @@ void save_or_free_index_entry(struct index_state *istate, 
struct cache_entry *ce
 
 void replace_index_entry_in_base(struct index_state *istate,
 struct cache_entry *old,
-struct cache_entry *new)
+struct cache_entry *new_entry)
 {
if (old->index &&
istate->split_index &&
istate->split_index->base &&
old->index <= istate->split_index->base->cache_nr) {
-   new->index = old->index;
-   if (old != istate->split_index->base->cache[new->index - 1])
-   free(istate->split_index->base->cache[new->index - 1]);
-   istate->split_index->base->cache[new->index - 1] = new;
+   new_entry->index = old->index;
+   if (old != istate->split_index->base->cache[new_entry->index - 
1])
+   free(istate->split_index->base->cache[new_entry->index 
- 1]);
+   istate->split_index->base->cache[new_entry->index - 1] = 
new_entry;
}
 }
 
diff --git a/split-index.h b/split-index.h
index df91c1bda..43d66826e 100644
--- a/split-index.h
+++ b/split-index.h
@@ -21,7 +21,7 @@ struct split_index *init_split_index(struct index_state 
*istate);
 void save_or_free_index_entry(struct index_state *istate, struct cache_entry 
*ce);
 void replace_index_entry_in_base(struct index_state *istate,
 struct cache_entry *old,
-struct cache_entry *new);
+struct cache_entry *new_entry);
 int read_link_extension(struct index_state *istate,
const void *data, unsigned long sz);
 int write_link_extension(struct strbuf *sb,
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 24/37] ref-filter: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 ref-filter.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 9dae6cfe3..99a45beb1 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -529,12 +529,12 @@ static void end_align_handler(struct ref_formatting_stack 
**stack)
 
 static void align_atom_handler(struct atom_value *atomv, struct 
ref_formatting_state *state)
 {
-   struct ref_formatting_stack *new;
+   struct ref_formatting_stack *new_stack;
 
push_stack_element(>stack);
-   new = state->stack;
-   new->at_end = end_align_handler;
-   new->at_end_data = >atom->u.align;
+   new_stack = state->stack;
+   new_stack->at_end = end_align_handler;
+   new_stack->at_end_data = >atom->u.align;
 }
 
 static void if_then_else_handler(struct ref_formatting_stack **stack)
@@ -574,16 +574,16 @@ static void if_then_else_handler(struct 
ref_formatting_stack **stack)
 
 static void if_atom_handler(struct atom_value *atomv, struct 
ref_formatting_state *state)
 {
-   struct ref_formatting_stack *new;
+   struct ref_formatting_stack *new_stack;
struct if_then_else *if_then_else = xcalloc(sizeof(struct 
if_then_else), 1);
 
if_then_else->str = atomv->atom->u.if_then_else.str;
if_then_else->cmp_status = atomv->atom->u.if_then_else.cmp_status;
 
push_stack_element(>stack);
-   new = state->stack;
-   new->at_end = if_then_else_handler;
-   new->at_end_data = if_then_else;
+   new_stack = state->stack;
+   new_stack->at_end = if_then_else_handler;
+   new_stack->at_end_data = if_then_else;
 }
 
 static int is_empty(const char *s)
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 22/37] line-log: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 line-log.c | 48 
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/line-log.c b/line-log.c
index 545ad0f28..346664c3d 100644
--- a/line-log.c
+++ b/line-log.c
@@ -151,29 +151,29 @@ static void range_set_union(struct range_set *out,
 
assert(out->nr == 0);
while (i < a->nr || j < b->nr) {
-   struct range *new;
+   struct range *new_range;
if (i < a->nr && j < b->nr) {
if (ra[i].start < rb[j].start)
-   new = [i++];
+   new_range = [i++];
else if (ra[i].start > rb[j].start)
-   new = [j++];
+   new_range = [j++];
else if (ra[i].end < rb[j].end)
-   new = [i++];
+   new_range = [i++];
else
-   new = [j++];
+   new_range = [j++];
} else if (i < a->nr)  /* b exhausted */
-   new = [i++];
+   new_range = [i++];
else   /* a exhausted */
-   new = [j++];
-   if (new->start == new->end)
+   new_range = [j++];
+   if (new_range->start == new_range->end)
; /* empty range */
-   else if (!out->nr || out->ranges[out->nr-1].end < new->start) {
+   else if (!out->nr || out->ranges[out->nr-1].end < 
new_range->start) {
range_set_grow(out, 1);
-   out->ranges[out->nr].start = new->start;
-   out->ranges[out->nr].end = new->end;
+   out->ranges[out->nr].start = new_range->start;
+   out->ranges[out->nr].end = new_range->end;
out->nr++;
-   } else if (out->ranges[out->nr-1].end < new->end) {
-   out->ranges[out->nr-1].end = new->end;
+   } else if (out->ranges[out->nr-1].end < new_range->end) {
+   out->ranges[out->nr-1].end = new_range->end;
}
}
 }
@@ -697,17 +697,17 @@ static void add_line_range(struct rev_info *revs, struct 
commit *commit,
   struct line_log_data *range)
 {
struct line_log_data *old = NULL;
-   struct line_log_data *new = NULL;
+   struct line_log_data *new_line = NULL;
 
old = lookup_decoration(>line_log_data, >object);
if (old && range) {
-   new = line_log_data_merge(old, range);
+   new_line = line_log_data_merge(old, range);
free_line_log_data(old);
} else if (range)
-   new = line_log_data_copy(range);
+   new_line = line_log_data_copy(range);
 
-   if (new)
-   add_decoration(>line_log_data, >object, new);
+   if (new_line)
+   add_decoration(>line_log_data, >object, new_line);
 }
 
 static void clear_commit_line_range(struct rev_info *revs, struct commit 
*commit)
@@ -1042,12 +1042,12 @@ static int process_diff_filepair(struct rev_info *rev,
 
 static struct diff_filepair *diff_filepair_dup(struct diff_filepair *pair)
 {
-   struct diff_filepair *new = xmalloc(sizeof(struct diff_filepair));
-   new->one = pair->one;
-   new->two = pair->two;
-   new->one->count++;
-   new->two->count++;
-   return new;
+   struct diff_filepair *new_filepair = xmalloc(sizeof(struct 
diff_filepair));
+   new_filepair->one = pair->one;
+   new_filepair->two = pair->two;
+   new_filepair->one->count++;
+   new_filepair->two->count++;
+   return new_filepair;
 }
 
 static void free_diffqueues(int n, struct diff_queue_struct *dq)
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 08/37] apply: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 apply.c | 38 +++---
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/apply.c b/apply.c
index 071f653c6..657c73930 100644
--- a/apply.c
+++ b/apply.c
@@ -2301,7 +2301,7 @@ static void update_pre_post_images(struct image *preimage,
   size_t len, size_t postlen)
 {
int i, ctx, reduced;
-   char *new, *old, *fixed;
+   char *new_buf, *old, *fixed;
struct image fixed_preimage;
 
/*
@@ -2329,18 +2329,18 @@ static void update_pre_post_images(struct image 
*preimage,
 */
old = postimage->buf;
if (postlen)
-   new = postimage->buf = xmalloc(postlen);
+   new_buf = postimage->buf = xmalloc(postlen);
else
-   new = old;
+   new_buf = old;
fixed = preimage->buf;
 
for (i = reduced = ctx = 0; i < postimage->nr; i++) {
size_t l_len = postimage->line[i].len;
if (!(postimage->line[i].flag & LINE_COMMON)) {
/* an added line -- no counterparts in preimage */
-   memmove(new, old, l_len);
+   memmove(new_buf, old, l_len);
old += l_len;
-   new += l_len;
+   new_buf += l_len;
continue;
}
 
@@ -2365,21 +2365,21 @@ static void update_pre_post_images(struct image 
*preimage,
 
/* and copy it in, while fixing the line length */
l_len = preimage->line[ctx].len;
-   memcpy(new, fixed, l_len);
-   new += l_len;
+   memcpy(new_buf, fixed, l_len);
+   new_buf += l_len;
fixed += l_len;
postimage->line[i].len = l_len;
ctx++;
}
 
if (postlen
-   ? postlen < new - postimage->buf
-   : postimage->len < new - postimage->buf)
+   ? postlen < new_buf - postimage->buf
+   : postimage->len < new_buf - postimage->buf)
die("BUG: caller miscounted postlen: asked %d, orig = %d, used 
= %d",
-   (int)postlen, (int) postimage->len, (int)(new - 
postimage->buf));
+   (int)postlen, (int) postimage->len, (int)(new_buf - 
postimage->buf));
 
/* Fix the length of the whole thing */
-   postimage->len = new - postimage->buf;
+   postimage->len = new_buf - postimage->buf;
postimage->nr -= reduced;
 }
 
@@ -4163,30 +4163,30 @@ static void show_mode_change(struct patch *p, int 
show_name)
 static void show_rename_copy(struct patch *p)
 {
const char *renamecopy = p->is_rename ? "rename" : "copy";
-   const char *old, *new;
+   const char *old, *new_name;
 
/* Find common prefix */
old = p->old_name;
-   new = p->new_name;
+   new_name = p->new_name;
while (1) {
const char *slash_old, *slash_new;
slash_old = strchr(old, '/');
-   slash_new = strchr(new, '/');
+   slash_new = strchr(new_name, '/');
if (!slash_old ||
!slash_new ||
-   slash_old - old != slash_new - new ||
-   memcmp(old, new, slash_new - new))
+   slash_old - old != slash_new - new_name ||
+   memcmp(old, new_name, slash_new - new_name))
break;
old = slash_old + 1;
-   new = slash_new + 1;
+   new_name = slash_new + 1;
}
-   /* p->old_name thru old is the common prefix, and old and new
+   /* p->old_name thru old is the common prefix, and old and new_name
 * through the end of names are renames
 */
if (old != p->old_name)
printf(" %s %.*s{%s => %s} (%d%%)\n", renamecopy,
   (int)(old - p->old_name), p->old_name,
-  old, new, p->score);
+  old, new_name, p->score);
else
printf(" %s %s => %s (%d%%)\n", renamecopy,
   p->old_name, p->new_name, p->score);
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 09/37] checkout: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 builtin/checkout.c | 138 ++---
 1 file changed, 69 insertions(+), 69 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index c54c78df5..30f09ec50 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -54,14 +54,14 @@ struct checkout_opts {
struct tree *source_tree;
 };
 
-static int post_checkout_hook(struct commit *old, struct commit *new,
+static int post_checkout_hook(struct commit *old, struct commit *new_commit,
  int changed)
 {
return run_hook_le(NULL, "post-checkout",
   oid_to_hex(old ? >object.oid : _oid),
-  oid_to_hex(new ? >object.oid : _oid),
+  oid_to_hex(new_commit ? _commit->object.oid : 
_oid),
   changed ? "1" : "0", NULL);
-   /* "new" can be NULL when checking out from the index before
+   /* "new_commit" can be NULL when checking out from the index before
   a commit exists. */
 
 }
@@ -473,7 +473,7 @@ static void setup_branch_path(struct branch_info *branch)
 
 static int merge_working_tree(const struct checkout_opts *opts,
  struct branch_info *old,
- struct branch_info *new,
+ struct branch_info *new_branch_info,
  int *writeout_error)
 {
int ret;
@@ -485,7 +485,7 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
 
resolve_undo_clear();
if (opts->force) {
-   ret = reset_tree(new->commit->tree, opts, 1, writeout_error);
+   ret = reset_tree(new_branch_info->commit->tree, opts, 1, 
writeout_error);
if (ret)
return ret;
} else {
@@ -523,7 +523,7 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
   >commit->object.oid :
   the_hash_algo->empty_tree);
init_tree_desc([0], tree->buffer, tree->size);
-   tree = parse_tree_indirect(>commit->object.oid);
+   tree = 
parse_tree_indirect(_branch_info->commit->object.oid);
init_tree_desc([1], tree->buffer, tree->size);
 
ret = unpack_trees(2, trees, );
@@ -571,18 +571,18 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
o.verbosity = 0;
work = write_tree_from_memory();
 
-   ret = reset_tree(new->commit->tree, opts, 1,
+   ret = reset_tree(new_branch_info->commit->tree, opts, 1,
 writeout_error);
if (ret)
return ret;
o.ancestor = old->name;
-   o.branch1 = new->name;
+   o.branch1 = new_branch_info->name;
o.branch2 = "local";
-   ret = merge_trees(, new->commit->tree, work,
+   ret = merge_trees(, new_branch_info->commit->tree, 
work,
old->commit->tree, );
if (ret < 0)
exit(128);
-   ret = reset_tree(new->commit->tree, opts, 0,
+   ret = reset_tree(new_branch_info->commit->tree, opts, 0,
 writeout_error);
strbuf_release();
if (ret)
@@ -600,15 +600,15 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
die(_("unable to write new index file"));
 
if (!opts->force && !opts->quiet)
-   show_local_changes(>commit->object, >diff_options);
+   show_local_changes(_branch_info->commit->object, 
>diff_options);
 
return 0;
 }
 
-static void report_tracking(struct branch_info *new)
+static void report_tracking(struct branch_info *new_branch_info)
 {
struct strbuf sb = STRBUF_INIT;
-   struct branch *branch = branch_get(new->name);
+   struct branch *branch = branch_get(new_branch_info->name);
 
if (!format_tracking_info(branch, ))
return;
@@ -618,7 +618,7 @@ static void report_tracking(struct branch_info *new)
 
 static void update_refs_for_switch(const struct checkout_opts *opts,
   struct branch_info *old,
-  struct branch_info *new)
+  struct branch_info *new_branch_info)
 {
struct strbuf msg = STRBUF_INIT;
const char *old_desc, *reflog_msg;
@@ -645,14 +645,14 @@ static void update_refs_for_switch(const struct 

[PATCH 06/37] diff: rename 'this' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 diff.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index 0a9a0cdf1..d682d0d1f 100644
--- a/diff.c
+++ b/diff.c
@@ -2601,7 +2601,7 @@ static long gather_dirstat(struct diff_options *opt, 
struct dirstat_dir *dir,
while (dir->nr) {
struct dirstat_file *f = dir->files;
int namelen = strlen(f->name);
-   unsigned long this;
+   unsigned long sum;
char *slash;
 
if (namelen < baselen)
@@ -2611,15 +2611,15 @@ static long gather_dirstat(struct diff_options *opt, 
struct dirstat_dir *dir,
slash = strchr(f->name + baselen, '/');
if (slash) {
int newbaselen = slash + 1 - f->name;
-   this = gather_dirstat(opt, dir, changed, f->name, 
newbaselen);
+   sum = gather_dirstat(opt, dir, changed, f->name, 
newbaselen);
sources++;
} else {
-   this = f->changed;
+   sum = f->changed;
dir->files++;
dir->nr--;
sources += 2;
}
-   this_dir += this;
+   this_dir += sum;
}
 
/*
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 07/37] apply: rename 'try' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 apply.c | 68 -
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/apply.c b/apply.c
index 321a9fa68..071f653c6 100644
--- a/apply.c
+++ b/apply.c
@@ -2386,8 +2386,8 @@ static void update_pre_post_images(struct image *preimage,
 static int line_by_line_fuzzy_match(struct image *img,
struct image *preimage,
struct image *postimage,
-   unsigned long try,
-   int try_lno,
+   unsigned long current,
+   int current_lno,
int preimage_limit)
 {
int i;
@@ -2404,9 +2404,9 @@ static int line_by_line_fuzzy_match(struct image *img,
 
for (i = 0; i < preimage_limit; i++) {
size_t prelen = preimage->line[i].len;
-   size_t imglen = img->line[try_lno+i].len;
+   size_t imglen = img->line[current_lno+i].len;
 
-   if (!fuzzy_matchlines(img->buf + try + imgoff, imglen,
+   if (!fuzzy_matchlines(img->buf + current + imgoff, imglen,
  preimage->buf + preoff, prelen))
return 0;
if (preimage->line[i].flag & LINE_COMMON)
@@ -2443,7 +2443,7 @@ static int line_by_line_fuzzy_match(struct image *img,
 */
extra_chars = preimage_end - preimage_eof;
strbuf_init(, imgoff + extra_chars);
-   strbuf_add(, img->buf + try, imgoff);
+   strbuf_add(, img->buf + current, imgoff);
strbuf_add(, preimage_eof, extra_chars);
fixed_buf = strbuf_detach(, _len);
update_pre_post_images(preimage, postimage,
@@ -2455,8 +2455,8 @@ static int match_fragment(struct apply_state *state,
  struct image *img,
  struct image *preimage,
  struct image *postimage,
- unsigned long try,
- int try_lno,
+ unsigned long current,
+ int current_lno,
  unsigned ws_rule,
  int match_beginning, int match_end)
 {
@@ -2466,12 +2466,12 @@ static int match_fragment(struct apply_state *state,
size_t fixed_len, postlen;
int preimage_limit;
 
-   if (preimage->nr + try_lno <= img->nr) {
+   if (preimage->nr + current_lno <= img->nr) {
/*
 * The hunk falls within the boundaries of img.
 */
preimage_limit = preimage->nr;
-   if (match_end && (preimage->nr + try_lno != img->nr))
+   if (match_end && (preimage->nr + current_lno != img->nr))
return 0;
} else if (state->ws_error_action == correct_ws_error &&
   (ws_rule & WS_BLANK_AT_EOF)) {
@@ -2482,7 +2482,7 @@ static int match_fragment(struct apply_state *state,
 * match with img, and the remainder of the preimage
 * must be blank.
 */
-   preimage_limit = img->nr - try_lno;
+   preimage_limit = img->nr - current_lno;
} else {
/*
 * The hunk extends beyond the end of the img and
@@ -2492,27 +2492,27 @@ static int match_fragment(struct apply_state *state,
return 0;
}
 
-   if (match_beginning && try_lno)
+   if (match_beginning && current_lno)
return 0;
 
/* Quick hash check */
for (i = 0; i < preimage_limit; i++)
-   if ((img->line[try_lno + i].flag & LINE_PATCHED) ||
-   (preimage->line[i].hash != img->line[try_lno + i].hash))
+   if ((img->line[current_lno + i].flag & LINE_PATCHED) ||
+   (preimage->line[i].hash != img->line[current_lno + i].hash))
return 0;
 
if (preimage_limit == preimage->nr) {
/*
 * Do we have an exact match?  If we were told to match
-* at the end, size must be exactly at try+fragsize,
-* otherwise try+fragsize must be still within the preimage,
+* at the end, size must be exactly at current+fragsize,
+* otherwise current+fragsize must be still within the preimage,
 * and either case, the old piece should match the preimage
 * exactly.
 */
if ((match_end
-? (try + preimage->len == img->len)
-: (try + preimage->len <= img->len)) &&
-   !memcmp(img->buf + try, preimage->buf, preimage->len))
+ 

[PATCH 04/37] pack-objects: rename 'this' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 builtin/pack-objects.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 6b9cfc289..bfda8602c 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1376,10 +1376,10 @@ static void cleanup_preferred_base(void)
it = pbase_tree;
pbase_tree = NULL;
while (it) {
-   struct pbase_tree *this = it;
-   it = this->next;
-   free(this->pcache.tree_data);
-   free(this);
+   struct pbase_tree *tmp = it;
+   it = tmp->next;
+   free(tmp->pcache.tree_data);
+   free(tmp);
}
 
for (i = 0; i < ARRAY_SIZE(pbase_tree_cache); i++) {
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 05/37] rev-parse: rename 'this' variable

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 builtin/rev-parse.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 74aa644cb..171c7a2b4 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -243,28 +243,28 @@ static int show_file(const char *arg, int output_prefix)
 static int try_difference(const char *arg)
 {
char *dotdot;
-   struct object_id oid;
-   struct object_id end;
-   const char *next;
-   const char *this;
+   struct object_id start_oid;
+   struct object_id end_oid;
+   const char *end;
+   const char *start;
int symmetric;
static const char head_by_default[] = "HEAD";
 
if (!(dotdot = strstr(arg, "..")))
return 0;
-   next = dotdot + 2;
-   this = arg;
-   symmetric = (*next == '.');
+   end = dotdot + 2;
+   start = arg;
+   symmetric = (*end == '.');
 
*dotdot = 0;
-   next += symmetric;
+   end += symmetric;
 
-   if (!*next)
-   next = head_by_default;
+   if (!*end)
+   end = head_by_default;
if (dotdot == arg)
-   this = head_by_default;
+   start = head_by_default;
 
-   if (this == head_by_default && next == head_by_default &&
+   if (start == head_by_default && end == head_by_default &&
!symmetric) {
/*
 * Just ".."?  That is not a range but the
@@ -274,14 +274,14 @@ static int try_difference(const char *arg)
return 0;
}
 
-   if (!get_oid_committish(this, ) && !get_oid_committish(next, )) 
{
-   show_rev(NORMAL, , next);
-   show_rev(symmetric ? NORMAL : REVERSED, , this);
+   if (!get_oid_committish(start, _oid) && !get_oid_committish(end, 
_oid)) {
+   show_rev(NORMAL, _oid, end);
+   show_rev(symmetric ? NORMAL : REVERSED, _oid, start);
if (symmetric) {
struct commit_list *exclude;
struct commit *a, *b;
-   a = lookup_commit_reference();
-   b = lookup_commit_reference();
+   a = lookup_commit_reference(_oid);
+   b = lookup_commit_reference(_oid);
exclude = get_merge_bases(a, b);
while (exclude) {
struct commit *commit = pop_commit();
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 18/37] diffcore-delta: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 diffcore-delta.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/diffcore-delta.c b/diffcore-delta.c
index ebe70fb06..c83d45a04 100644
--- a/diffcore-delta.c
+++ b/diffcore-delta.c
@@ -48,16 +48,16 @@ struct spanhash_top {
 
 static struct spanhash_top *spanhash_rehash(struct spanhash_top *orig)
 {
-   struct spanhash_top *new;
+   struct spanhash_top *new_spanhash;
int i;
int osz = 1 << orig->alloc_log2;
int sz = osz << 1;
 
-   new = xmalloc(st_add(sizeof(*orig),
+   new_spanhash = xmalloc(st_add(sizeof(*orig),
 st_mult(sizeof(struct spanhash), sz)));
-   new->alloc_log2 = orig->alloc_log2 + 1;
-   new->free = INITIAL_FREE(new->alloc_log2);
-   memset(new->data, 0, sizeof(struct spanhash) * sz);
+   new_spanhash->alloc_log2 = orig->alloc_log2 + 1;
+   new_spanhash->free = INITIAL_FREE(new_spanhash->alloc_log2);
+   memset(new_spanhash->data, 0, sizeof(struct spanhash) * sz);
for (i = 0; i < osz; i++) {
struct spanhash *o = &(orig->data[i]);
int bucket;
@@ -65,11 +65,11 @@ static struct spanhash_top *spanhash_rehash(struct 
spanhash_top *orig)
continue;
bucket = o->hashval & (sz - 1);
while (1) {
-   struct spanhash *h = &(new->data[bucket++]);
+   struct spanhash *h = &(new_spanhash->data[bucket++]);
if (!h->cnt) {
h->hashval = o->hashval;
h->cnt = o->cnt;
-   new->free--;
+   new_spanhash->free--;
break;
}
if (sz <= bucket)
@@ -77,7 +77,7 @@ static struct spanhash_top *spanhash_rehash(struct 
spanhash_top *orig)
}
}
free(orig);
-   return new;
+   return new_spanhash;
 }
 
 static struct spanhash_top *add_spanhash(struct spanhash_top *top,
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 03/37] blame: rename 'this' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 blame.c | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/blame.c b/blame.c
index 2893f3c10..21c867664 100644
--- a/blame.c
+++ b/blame.c
@@ -998,28 +998,29 @@ unsigned blame_entry_score(struct blame_scoreboard *sb, 
struct blame_entry *e)
 }
 
 /*
- * best_so_far[] and this[] are both a split of an existing blame_entry
- * that passes blame to the parent.  Maintain best_so_far the best split
- * so far, by comparing this and best_so_far and copying this into
+ * best_so_far[] and potential[] are both a split of an existing blame_entry
+ * that passes blame to the parent.  Maintain best_so_far the best split so
+ * far, by comparing potential and best_so_far and copying potential into
  * bst_so_far as needed.
  */
 static void copy_split_if_better(struct blame_scoreboard *sb,
 struct blame_entry *best_so_far,
-struct blame_entry *this)
+struct blame_entry *potential)
 {
int i;
 
-   if (!this[1].suspect)
+   if (!potential[1].suspect)
return;
if (best_so_far[1].suspect) {
-   if (blame_entry_score(sb, [1]) < blame_entry_score(sb, 
_so_far[1]))
+   if (blame_entry_score(sb, [1]) <
+   blame_entry_score(sb, _so_far[1]))
return;
}
 
for (i = 0; i < 3; i++)
-   blame_origin_incref(this[i].suspect);
+   blame_origin_incref(potential[i].suspect);
decref_split(best_so_far);
-   memcpy(best_so_far, this, sizeof(struct blame_entry [3]));
+   memcpy(best_so_far, potential, sizeof(struct blame_entry[3]));
 }
 
 /*
@@ -1046,12 +1047,12 @@ static void handle_split(struct blame_scoreboard *sb,
if (ent->num_lines <= tlno)
return;
if (tlno < same) {
-   struct blame_entry this[3];
+   struct blame_entry potential[3];
tlno += ent->s_lno;
same += ent->s_lno;
-   split_overlap(this, ent, tlno, plno, same, parent);
-   copy_split_if_better(sb, split, this);
-   decref_split(this);
+   split_overlap(potential, ent, tlno, plno, same, parent);
+   copy_split_if_better(sb, split, potential);
+   decref_split(potential);
}
 }
 
@@ -1273,7 +1274,7 @@ static void find_copy_in_parent(struct blame_scoreboard 
*sb,
struct diff_filepair *p = diff_queued_diff.queue[i];
struct blame_origin *norigin;
mmfile_t file_p;
-   struct blame_entry this[3];
+   struct blame_entry potential[3];
 
if (!DIFF_FILE_VALID(p->one))
continue; /* does not exist in parent */
@@ -1292,10 +1293,10 @@ static void find_copy_in_parent(struct blame_scoreboard 
*sb,
 
for (j = 0; j < num_ents; j++) {
find_copy_in_blob(sb, blame_list[j].ent,
- norigin, this, _p);
+ norigin, potential, _p);
copy_split_if_better(sb, blame_list[j].split,
-this);
-   decref_split(this);
+potential);
+   decref_split(potential);
}
blame_origin_decref(norigin);
}
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 01/37] object_info: change member name from 'typename' to 'type_name'

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 builtin/cat-file.c |  2 +-
 cache.h|  2 +-
 packfile.c |  6 +++---
 sha1_file.c| 10 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index f5fa4fd75..d06c66c77 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -76,7 +76,7 @@ static int cat_one_file(int opt, const char *exp_type, const 
char *obj_name,
buf = NULL;
switch (opt) {
case 't':
-   oi.typename = 
+   oi.type_name = 
if (sha1_object_info_extended(oid.hash, , flags) < 0)
die("git cat-file: could not get object info");
if (sb.len) {
diff --git a/cache.h b/cache.h
index d8b975a57..69b5a3bf8 100644
--- a/cache.h
+++ b/cache.h
@@ -1744,7 +1744,7 @@ struct object_info {
unsigned long *sizep;
off_t *disk_sizep;
unsigned char *delta_base_sha1;
-   struct strbuf *typename;
+   struct strbuf *type_name;
void **contentp;
 
/* Response */
diff --git a/packfile.c b/packfile.c
index 4a5fe7ab1..6657a0a49 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1350,16 +1350,16 @@ int packed_object_info(struct packed_git *p, off_t 
obj_offset,
*oi->disk_sizep = revidx[1].offset - obj_offset;
}
 
-   if (oi->typep || oi->typename) {
+   if (oi->typep || oi->type_name) {
enum object_type ptot;
ptot = packed_to_object_type(p, obj_offset, type, _curs,
 curpos);
if (oi->typep)
*oi->typep = ptot;
-   if (oi->typename) {
+   if (oi->type_name) {
const char *tn = typename(ptot);
if (tn)
-   strbuf_addstr(oi->typename, tn);
+   strbuf_addstr(oi->type_name, tn);
}
if (ptot < 0) {
type = OBJ_BAD;
diff --git a/sha1_file.c b/sha1_file.c
index 3da70ac65..2c03458ea 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1087,8 +1087,8 @@ static int parse_sha1_header_extended(const char *hdr, 
struct object_info *oi,
}
 
type = type_from_string_gently(type_buf, type_len, 1);
-   if (oi->typename)
-   strbuf_add(oi->typename, type_buf, type_len);
+   if (oi->type_name)
+   strbuf_add(oi->type_name, type_buf, type_len);
/*
 * Set type to 0 if its an unknown object and
 * we're obtaining the type using '--allow-unknown-type'
@@ -1158,7 +1158,7 @@ static int sha1_loose_object_info(const unsigned char 
*sha1,
 * return value implicitly indicates whether the
 * object even exists.
 */
-   if (!oi->typep && !oi->typename && !oi->sizep && !oi->contentp) {
+   if (!oi->typep && !oi->type_name && !oi->sizep && !oi->contentp) {
const char *path;
struct stat st;
if (stat_sha1_file(sha1, , ) < 0)
@@ -1239,8 +1239,8 @@ int sha1_object_info_extended(const unsigned char *sha1, 
struct object_info *oi,
*(oi->disk_sizep) = 0;
if (oi->delta_base_sha1)
hashclr(oi->delta_base_sha1);
-   if (oi->typename)
-   strbuf_addstr(oi->typename, typename(co->type));
+   if (oi->type_name)
+   strbuf_addstr(oi->type_name, 
typename(co->type));
if (oi->contentp)
*oi->contentp = xmemdupz(co->buf, co->size);
oi->whence = OI_CACHED;
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 02/37] object: rename function 'typename' to 'type_name'

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 builtin/cat-file.c |  2 +-
 builtin/diff-tree.c|  2 +-
 builtin/fast-export.c  |  8 
 builtin/fsck.c |  4 ++--
 builtin/grep.c |  2 +-
 builtin/index-pack.c   | 12 ++--
 builtin/merge.c|  2 +-
 builtin/mktree.c   |  4 ++--
 builtin/prune.c|  2 +-
 builtin/replace.c  | 12 ++--
 builtin/tag.c  |  2 +-
 builtin/unpack-objects.c   | 10 +-
 builtin/verify-commit.c|  2 +-
 bulk-checkin.c |  2 +-
 commit.c   |  2 +-
 contrib/examples/builtin-fetch--tool.c |  2 +-
 fast-import.c  | 16 
 fsck.c |  2 +-
 http-push.c|  2 +-
 log-tree.c |  2 +-
 object.c   |  6 +++---
 object.h   |  2 +-
 pack-check.c   |  2 +-
 packfile.c |  2 +-
 reachable.c|  2 +-
 ref-filter.c   |  4 ++--
 sequencer.c|  2 +-
 sha1_file.c| 20 ++--
 sha1_name.c|  6 +++---
 submodule.c|  2 +-
 tag.c  |  2 +-
 walker.c   |  4 ++--
 32 files changed, 73 insertions(+), 73 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index d06c66c77..c6b3b1bfb 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -229,7 +229,7 @@ static void expand_atom(struct strbuf *sb, const char 
*atom, int len,
if (data->mark_query)
data->info.typep = >type;
else
-   strbuf_addstr(sb, typename(data->type));
+   strbuf_addstr(sb, type_name(data->type));
} else if (is_atom("objectsize", atom, len)) {
if (data->mark_query)
data->info.sizep = >size;
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index b775a7564..473615117 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -76,7 +76,7 @@ static int diff_tree_stdin(char *line)
if (obj->type == OBJ_TREE)
return stdin_diff_trees((struct tree *)obj, p);
error("Object %s is a %s, not a commit or tree",
- oid_to_hex(), typename(obj->type));
+ oid_to_hex(), type_name(obj->type));
return -1;
 }
 
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 796d0cd66..27b2cc138 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -240,7 +240,7 @@ static void export_blob(const struct object_id *oid)
buf = read_sha1_file(oid->hash, , );
if (!buf)
die ("Could not read blob %s", oid_to_hex(oid));
-   if (check_sha1_signature(oid->hash, buf, size, typename(type)) 
< 0)
+   if (check_sha1_signature(oid->hash, buf, size, type_name(type)) 
< 0)
die("sha1 mismatch in blob %s", oid_to_hex(oid));
object = parse_object_buffer(oid, type, size, buf, );
}
@@ -757,7 +757,7 @@ static void handle_tag(const char *name, struct tag *tag)
if (tagged->type != OBJ_COMMIT) {
die ("Tag %s tags unexported %s!",
 oid_to_hex(>object.oid),
-typename(tagged->type));
+type_name(tagged->type));
}
p = (struct commit *)tagged;
for (;;) {
@@ -839,7 +839,7 @@ static void get_tags_and_duplicates(struct rev_cmdline_info 
*info)
if (!commit) {
warning("%s: Unexpected object of type %s, skipping.",
e->name,
-   typename(e->item->type));
+   type_name(e->item->type));
continue;
}
 
@@ -851,7 +851,7 @@ static void get_tags_and_duplicates(struct rev_cmdline_info 
*info)
continue;
default: /* OBJ_TAG (nested tags) is already handled */
warning("Tag points to object of unexpected type %s, 
skipping.",
-   typename(commit->object.type));
+   type_name(commit->object.type));
continue;
 

[PATCH 19/37] entry: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 entry.c | 40 
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/entry.c b/entry.c
index 30211447a..6c33112ae 100644
--- a/entry.c
+++ b/entry.c
@@ -85,12 +85,12 @@ static int create_file(const char *path, unsigned int mode)
 static void *read_blob_entry(const struct cache_entry *ce, unsigned long *size)
 {
enum object_type type;
-   void *new = read_sha1_file(ce->oid.hash, , size);
+   void *blob_data = read_sha1_file(ce->oid.hash, , size);
 
-   if (new) {
+   if (blob_data) {
if (type == OBJ_BLOB)
-   return new;
-   free(new);
+   return blob_data;
+   free(blob_data);
}
return NULL;
 }
@@ -256,7 +256,7 @@ static int write_entry(struct cache_entry *ce,
unsigned int ce_mode_s_ifmt = ce->ce_mode & S_IFMT;
struct delayed_checkout *dco = state->delayed_checkout;
int fd, ret, fstat_done = 0;
-   char *new;
+   char *new_blob;
struct strbuf buf = STRBUF_INIT;
unsigned long size;
ssize_t wrote;
@@ -276,8 +276,8 @@ static int write_entry(struct cache_entry *ce,
 
switch (ce_mode_s_ifmt) {
case S_IFLNK:
-   new = read_blob_entry(ce, );
-   if (!new)
+   new_blob = read_blob_entry(ce, );
+   if (!new_blob)
return error("unable to read sha1 file of %s (%s)",
 path, oid_to_hex(>oid));
 
@@ -288,8 +288,8 @@ static int write_entry(struct cache_entry *ce,
if (!has_symlinks || to_tempfile)
goto write_file_entry;
 
-   ret = symlink(new, path);
-   free(new);
+   ret = symlink(new_blob, path);
+   free(new_blob);
if (ret)
return error_errno("unable to create symlink %s", path);
break;
@@ -300,11 +300,11 @@ static int write_entry(struct cache_entry *ce,
 * bother reading it at all.
 */
if (dco && dco->state == CE_RETRY) {
-   new = NULL;
+   new_blob = NULL;
size = 0;
} else {
-   new = read_blob_entry(ce, );
-   if (!new)
+   new_blob = read_blob_entry(ce, );
+   if (!new_blob)
return error("unable to read sha1 file of %s 
(%s)",
 path, oid_to_hex(>oid));
}
@@ -313,18 +313,18 @@ static int write_entry(struct cache_entry *ce,
 * Convert from git internal format to working tree format
 */
if (dco && dco->state != CE_NO_DELAY) {
-   ret = async_convert_to_working_tree(ce->name, new,
+   ret = async_convert_to_working_tree(ce->name, new_blob,
size, , dco);
if (ret && string_list_has_string(>paths, 
ce->name)) {
-   free(new);
+   free(new_blob);
goto delayed;
}
} else
-   ret = convert_to_working_tree(ce->name, new, size, 
);
+   ret = convert_to_working_tree(ce->name, new_blob, size, 
);
 
if (ret) {
-   free(new);
-   new = strbuf_detach(, );
+   free(new_blob);
+   new_blob = strbuf_detach(, );
size = newsize;
}
/*
@@ -336,15 +336,15 @@ static int write_entry(struct cache_entry *ce,
write_file_entry:
fd = open_output_fd(path, ce, to_tempfile);
if (fd < 0) {
-   free(new);
+   free(new_blob);
return error_errno("unable to create file %s", path);
}
 
-   wrote = write_in_full(fd, new, size);
+   wrote = write_in_full(fd, new_blob, size);
if (!to_tempfile)
fstat_done = fstat_output(fd, state, );
close(fd);
-   free(new);
+   free(new_blob);
if (wrote < 0)
return error("unable to write file %s", path);
break;
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 14/37] combine-diff: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 combine-diff.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index bc08c4c5b..14db48966 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -162,7 +162,7 @@ enum coalesce_direction { MATCH, BASE, NEW };
 
 /* Coalesce new lines into base by finding LCS */
 static struct lline *coalesce_lines(struct lline *base, int *lenbase,
-   struct lline *new, int lennew,
+   struct lline *newline, int lennew,
unsigned long parent, long flags)
 {
int **lcs;
@@ -170,12 +170,12 @@ static struct lline *coalesce_lines(struct lline *base, 
int *lenbase,
struct lline *baseend, *newend = NULL;
int i, j, origbaselen = *lenbase;
 
-   if (new == NULL)
+   if (newline == NULL)
return base;
 
if (base == NULL) {
*lenbase = lennew;
-   return new;
+   return newline;
}
 
/*
@@ -200,7 +200,7 @@ static struct lline *coalesce_lines(struct lline *base, int 
*lenbase,
directions[0][j] = NEW;
 
for (i = 1, baseend = base; i < origbaselen + 1; i++) {
-   for (j = 1, newend = new; j < lennew + 1; j++) {
+   for (j = 1, newend = newline; j < lennew + 1; j++) {
if (match_string_spaces(baseend->line, baseend->len,
newend->line, newend->len, 
flags)) {
lcs[i][j] = lcs[i - 1][j - 1] + 1;
@@ -241,7 +241,7 @@ static struct lline *coalesce_lines(struct lline *base, int 
*lenbase,
if (lline->prev)
lline->prev->next = lline->next;
else
-   new = lline->next;
+   newline = lline->next;
if (lline->next)
lline->next->prev = lline->prev;
 
@@ -270,7 +270,7 @@ static struct lline *coalesce_lines(struct lline *base, int 
*lenbase,
}
}
 
-   newend = new;
+   newend = newline;
while (newend) {
struct lline *lline = newend;
newend = newend->next;
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 17/37] diff: rename 'new' variables

[Brandon Williams]

Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams 
---
 diff.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/diff.c b/diff.c
index d682d0d1f..d49732b3b 100644
--- a/diff.c
+++ b/diff.c
@@ -1504,7 +1504,7 @@ struct diff_words_style_elem {
 
 struct diff_words_style {
enum diff_words_type type;
-   struct diff_words_style_elem new, old, ctx;
+   struct diff_words_style_elem new_word, old, ctx;
const char *newline;
 };
 
@@ -1660,7 +1660,7 @@ static void fn_out_diff_words_aux(void *priv, char *line, 
unsigned long len)
}
if (plus_begin != plus_end) {
fn_out_diff_words_write_helper(diff_words->opt,
-   >new, style->newline,
+   >new_word, style->newline,
plus_end - plus_begin, plus_begin);
}
 
@@ -1884,7 +1884,7 @@ static void init_diff_words_data(struct emit_callback 
*ecbdata,
if (want_color(o->use_color)) {
struct diff_words_style *st = ecbdata->diff_words->style;
st->old.color = diff_get_color_opt(o, DIFF_FILE_OLD);
-   st->new.color = diff_get_color_opt(o, DIFF_FILE_NEW);
+   st->new_word.color = diff_get_color_opt(o, DIFF_FILE_NEW);
st->ctx.color = diff_get_color_opt(o, DIFF_CONTEXT);
}
 }
@@ -2048,7 +2048,7 @@ static void fn_out_consume(void *priv, char *line, 
unsigned long len)
 static char *pprint_rename(const char *a, const char *b)
 {
const char *old = a;
-   const char *new = b;
+   const char *new_name = b;
struct strbuf name = STRBUF_INIT;
int pfx_length, sfx_length;
int pfx_adjust_for_slash;
@@ -2067,16 +2067,16 @@ static char *pprint_rename(const char *a, const char *b)
 
/* Find common prefix */
pfx_length = 0;
-   while (*old && *new && *old == *new) {
+   while (*old && *new_name && *old == *new_name) {
if (*old == '/')
pfx_length = old - a + 1;
old++;
-   new++;
+   new_name++;
}
 
/* Find common suffix */
old = a + len_a;
-   new = b + len_b;
+   new_name = b + len_b;
sfx_length = 0;
/*
 * If there is a common prefix, it must end in a slash.  In
@@ -2088,12 +2088,12 @@ static char *pprint_rename(const char *a, const char *b)
 */
pfx_adjust_for_slash = (pfx_length ? 1 : 0);
while (a + pfx_length - pfx_adjust_for_slash <= old &&
-  b + pfx_length - pfx_adjust_for_slash <= new &&
-  *old == *new) {
+  b + pfx_length - pfx_adjust_for_slash <= new_name &&
+  *old == *new_name) {
if (*old == '/')
sfx_length = len_a - (old - a);
old--;
-   new--;
+   new_name--;
}
 
/*
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH 00/37] removal of some c++ keywords

[Brandon Williams]

A while back there was some discussion of getting our codebase into a state
where we could use a c++ compiler if we wanted to (for various reason like
leveraging c++ only analysis tools, etc.).  Johannes Sixt had a very large
patch that achieved this but it wasn't in a state where it could be upstreamed.
I took that idea and did some removals of c++ keywords (new, template, try,
this, etc) but broke it up into several (well maybe more than several) patches.
I don't believe I've captured all of them in this series but this is at least
moving a step closer to being able to compile using a c++ compiler.

I don't know if this is something the community still wants to move towards,
but if this is something people are still interested in, and this series is
wanted, then we can keep doing these sort of conversions in chunks slowly.

Brandon Williams (37):
  object_info: change member name from 'typename' to 'type_name'
  object: rename function 'typename' to 'type_name'
  blame: rename 'this' variables
  pack-objects: rename 'this' variables
  rev-parse: rename 'this' variable
  diff: rename 'this' variables
  apply: rename 'try' variables
  apply: rename 'new' variables
  checkout: rename 'new' variables
  help: rename 'new' variables
  pack-redundant: rename 'new' variables
  reflog: rename 'new' variables
  remote: rename 'new' variables
  combine-diff: rename 'new' variables
  commit: rename 'new' variables
  diff-lib: rename 'new' variable
  diff: rename 'new' variables
  diffcore-delta: rename 'new' variables
  entry: rename 'new' variables
  http: rename 'new' variables
  imap-send: rename 'new' variables
  line-log: rename 'new' variables
  read-cache: rename 'new' variables
  ref-filter: rename 'new' variables
  remote: rename 'new' variables
  split-index: rename 'new' variables
  submodule: rename 'new' variables
  trailer: rename 'new' variables
  unpack-trees: rename 'new' variables
  init-db: rename 'template' variables
  environment: rename 'template' variables
  diff: rename 'template' variables
  environment: rename 'namespace' variables
  wrapper: rename 'template' variables
  tempfile: rename 'template' variables
  trailer: rename 'template' variables
  replace: rename 'new' variables

 apply.c| 106 -
 blame.c|  33 
 builtin/cat-file.c |   4 +-
 builtin/checkout.c | 138 -
 builtin/diff-tree.c|   2 +-
 builtin/fast-export.c  |   8 +-
 builtin/fsck.c |   4 +-
 builtin/grep.c |   2 +-
 builtin/help.c |  10 +--
 builtin/index-pack.c   |  12 +--
 builtin/init-db.c  |  30 +++
 builtin/merge.c|   2 +-
 builtin/mktree.c   |   4 +-
 builtin/pack-objects.c |   8 +-
 builtin/pack-redundant.c   |  48 ++--
 builtin/prune.c|   2 +-
 builtin/reflog.c   |   8 +-
 builtin/remote.c   |  44 +--
 builtin/replace.c  |  28 +++
 builtin/rev-parse.c|  34 
 builtin/tag.c  |   2 +-
 builtin/unpack-objects.c   |  10 +--
 builtin/verify-commit.c|   2 +-
 bulk-checkin.c |   2 +-
 cache.h|   4 +-
 combine-diff.c |  12 +--
 commit.c   |  20 ++---
 contrib/examples/builtin-fetch--tool.c |   2 +-
 diff-lib.c |  20 ++---
 diff.c |  38 -
 diffcore-delta.c   |  16 ++--
 entry.c|  40 +-
 environment.c  |  24 +++---
 fast-import.c  |  16 ++--
 fsck.c |   2 +-
 git-compat-util.h  |   4 +-
 http-push.c|   2 +-
 http.c |  10 +--
 imap-send.c|  14 ++--
 line-log.c |  48 ++--
 log-tree.c |   2 +-
 object.c   |   6 +-
 object.h   |   2 +-
 pack-check.c   |   2 +-
 packfile.c |   8 +-
 reachable.c|   2 +-
 read-cache.c   |  36 -
 ref-filter.c   |  20 ++---
 remote.c   |  16 ++--
 sequencer.c|   2 +-
 sha1_file.c|  28 +++
 sha1_name.c|   6 +-
 split-index.c  |  10 +--
 

Donation of £1,500,000.00 GBP!!

[Chris Colin & Weir]

My wife and I have awarded you with a donation 
of £1,500,000.00 GBP from part of our Jackpot 
Lottery of £161,653,000 Million Pounds, send 
your name,address, phone for claims.

We await your earliest response and God Bless you. 

Best of luck. 
Chris Colin & Weir

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

[Johannes Schindelin]

Hi Junio,

On Tue, 23 Jan 2018, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > +   /*
> > +* 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);
> > +
> 
> I think this expression and assignment should better be done much
> later.  Are you going to update commit, commit->parents, etc. that
> are involved in the computation in the meantime???

No, it is in the exact right spot.

What you are missing is that there will be some more code here, to support
octopus merges.

That's why it is exactly the right spot.

> > @@ -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)) {
> 
> ... Namely, here.

... which the octopus merge code will never reach. That's why this is the
wrong spot for that initial can_fast_forward assignment.

> Because the earlier one is computing "are we
> replaying exactly the same commit on top of exactly the same
> state?", which is merely one half of "can we fast-forward", and
> storing it in a variable whose name is over-promising way before it
> becomes necessary.  The other half of "can we fast-forward?" logic
> is the remainder of the if() condition we see above.  IOW, when
> fully spelled, this code can fast-forward when we are replaying a
> commit on top of exactly the same first-parent and the commit being
> replayed is a single parent merge.
> 
> We may even want to get rid of can_fast_forward variable.

Absolutely not.

Ciao,
Dscho

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

[Johannes Schindelin]

Hi Junio,

On Mon, 22 Jan 2018, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > 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);
> > -
> 
> Assigning to "saved" before the added "if we are doing merge and see
> '-', do this special thing" is not only unnecessary, but makes the
> logic in the non-special case harder to read.  The four things
> "saved = *eol; *eol = 0; do_thing_using(bol); *eol = saved;" is a
> single logical unit; keep them together.

True. This was a sloppily resolved merge conflict in one of the many
rewrites, I guess.

> > +   if (*p)
> > +   len = strlen(p);
> > +   else {
> > +   strbuf_addf(, "Merge branch '%.*s'",
> > +   merge_arg_len, arg);
> > +   p = buf.buf;
> > +   len = buf.len;
> > +   }
> 
> So... "arg" received by this function can be a single non-whitespace
> token, which is taken as the name of the branch being merged (in
> this else clause).  Or it can also be followed by a single liner
> message for the merge commit.  Presumably, this is for creating a
> new merge (i.e. "commit==NULL" case), and preparing a proper log
> message in the todo list is unrealistic, so this would be a
> reasonable compromise.  Those users who want to write proper log
> message could presumably follow such "merge" insn with a "x git
> commit --amend" or something, I presume, if they really wanted to.

Precisely.

> > +   if (write_message(p, len, git_path_merge_msg(), 0) < 0) {
> > +   error_errno(_("Could not write '%s'"),
> > +   git_path_merge_msg());
> > +   strbuf_release();
> > +   rollback_lock_file();
> > +   return -1;
> > +   }
> > +   strbuf_release();
> > +   }
> 
> OK.  Now we have prepared the MERGE_MSG file and are ready to commit.
> 
> > +   head_commit = lookup_commit_reference_by_name("HEAD");
> > +   if (!head_commit) {
> > +   rollback_lock_file();
> > +   return error(_("Cannot merge without a current revision"));
> > +   }
> 
> Hmph, I would have expected to see this a lot earlier, before
> dealing with the log message.  Leftover MERGE_MSG file after an
> error will cause unexpected fallout to the end-user experience
> (including what is shown by the shell prompt scripts), but if we do
> this before the MERGE_MSG thing, we do not have to worry about
> error codepath having to remove it.

Fixed.

> > +   strbuf_addf(_name, "refs/rewritten/%.*s", merge_arg_len, arg);
> > +   merge_commit = lookup_commit_reference_by_name(ref_name.buf);
> > +   if (!merge_commit) {
> > +   /* fall back to non-rewritten ref or commit */
> > +   strbuf_splice(_name, 0, strlen("refs/rewritten/"), "", 0);
> > +   merge_commit = lookup_commit_reference_by_name(ref_name.buf);
> > +   }
> 
> OK, so "abc" in the example in the log message is looked up first as
> a label and then we take a fallback to interpret as an object name.

Yes. And auto-generated labels are guaranteed not to be full hex hashes
for that reason.

> Hopefully allowed names in "label" would be documented clearly in
> later steps (I am guessing that "a name that can be used as a branch
> name can be used as a label name and vice versa" or something like
> that).

Well, I thought that it would suffice to say that these labels are
available as refs/rewritten/. It kind of goes without saying that
those need to be valid ref names, then?

> > +   if (!merge_commit) {
> > +   error(_("could not resolve '%s'"), ref_name.buf);
> > +   strbuf_release(_name);
> > +   rollback_lock_file();
> > +   return -1;
> > +   }
> > +   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);
> 
> These two calls gave me a "Huh?" moment; write_message() sounds like
> it is allowed to be later updated with frills suitable for *_MSG
> files we place in .git/ directory (iow, it is in principle OK if
> commented out instructions common to these files are added to the
> output by the function), but these want exact bytes passed in the
> result, for which wrapper.c::write_file() is more appropriate.

I agree that 

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

[Johannes Schindelin]

Hi Junio,

On Mon, 22 Jan 2018, Junio C Hamano wrote:

> Jacob Keller  writes:
> 
> > 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.
> 
> I do not think we would mind "bud" too much in the end result, but
> the change in 1/8 is made harder to read than necessary with it.  It
> is the only thing that needs "a single-letter command name may now
> not have any argument after it" change to the parser among the three
> things being added here, and it also needs to be added to the list
> of special commands without arguments.
> 
> It would have been easier to reason about if addition of "bud" was
> in its own patch done after label and reset are added.  And if done
> as a separate step, perhaps it would have been easier to realize
> that it would be a more future-proof solution for handling the
> "special" ness of BUD to add a new "unsigned flags" word to
> todo_command_info[] structure and using a bit that says "this does
> not take an arg" than to hardcode "noop and bud are the commands
> without args" in the code.  That hardcode was good enough when there
> was only one thing in that special case.  Now it has two.

I dropped the `bud` command. It did come in handy when I truly recreated
branch structure from a way-too-long topic branch, but that is probably a
rare use case, and not worth spending so much air time on.

> In a similar way, the code to special case label and reset just like
> exec may also want to become more table driven, perhaps using
> another bit in the same new flags word to say "this does not refer
> to commit".  I think that can become [v2 1/N] while addition of "bud"
> can be [v2 2/N] (after all, "bud" just does the same do_reset() with
> hardcoded argument, so "label/reset" must come first).

The downside of such a table-driven approach is readability, of course. It
becomes *less* readable because all of a sudden you have to jump back and
forth between the parsing code and the table (and then you also have to
keep the table header in sight).

I have had enough problems with such table-driven approaches in the past,
even in Git's own source code. Exhibit A: t0027. I do not wish upon my
worst enemies having to investigate problems in that script, for in those
tables despair awaits ye who dare enter.

So I respectfully decline to go into that direction in the sequencer.

Ciao,
Dscho

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

[Johannes Schindelin]

Hi Junio,

On Fri, 19 Jan 2018, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > 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
> 
> Yup.  I've seen this design talked about on list in the past, and
> I've always felt that this is "sequencer done right".
> 
> At the first glance, it may feel somewhat unsatisfying that "merge"
> has to say effects of which commits should be reflected in the
> result and which commot to take the log message from, i.e.
> (recreated)D is merged to form the resulting tree, and 3456=C is
> used for the log, to recreate C in the above example, while "pick"
> always uses the same commit for both, i.e. recreated B inherits both
> the changes and log message from the original B=2345 (or depending
> on the readers' point of view, "merge" is allowed to use two
> different commits, while "pick" is always limited to the same one).
> 
> But I think this distinction is probably fundamental and I am not
> opposed to it at all.  The result of "pick" has only one parent, and
> the parent is determined only by the previous actions and not by
> anything on the "pick" line in the todo list.  But the result of
> "merge" has to record all the other parents, and only the first
> parent is determined implicitly by the previous actions.  We need to
> tell the "merge" command about "3456=C" in order to recreate the
> effect of original merge commit (i.e. changes between B and C) as
> well as its log message, and we also need to tell it about label "D"
> that it is the "other parent" that need to be recorded.

Yes, this was the hard lesson of the failed preserve-merges design.

> Obviously "merge" command syntax should allow recreating an octopus,
> so whenever I said "two" in the above, I meant "N".  The original
> merge commit is needed so that the effect to replay (roughly: a
> patch going to the original merge result from its first parent) can
> be learned from the existing history, and all the other "N-1"
> parents needs to be given (and they must have been already created
> in the todo list) so that the resulting recreated merge can be
> recorded with them as parents (in addition to the first parent that
> is implicitly given as the result of all the previous steps).

I have two more patch series lined up after this one, the first one
implements --root via the sequencer, and the second one indeed extends
`merge` to handle octopus commits.

> One interesting (and probably useful) thing to notice is that if A
> were not rebased in the above sample picture, and only B were the
> one that was tweaked, then a recreated C may use the same original D
> as its side parent, and the mechanism outlined above naturally can
> support it by allowing an un-rewritten commit to be given as a side
> parent when "merge" is redoing C.

I think that you will get a kick out of reading the commit message of the
last commit, as it does talk about the problematic C: it *would* be
rebased by default.

In the next iteration I will actually switch around the default from
rebase-cousins to no-rebase-cousins for that reason.

Ciao,
Dscho

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

[Johannes Schindelin]

Hi Phillip,

On Fri, 19 Jan 2018, Phillip Wood wrote:

> On 18/01/18 15:35, Johannes Schindelin wrote:
> > 
> > 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.
> 
> I might have missed something but shouldn't this be checking
> opts->allow_ff?

Good point. This is the type of review for which I was hoping.

> Another possible optimization is that if the parent branches have only
> reworded commits or some commits that have been squashed but no other
> changes then their trees will be the same as in the original merge
> commit and so could be reused without calling merge_recursive().

True. It is also a bit involved to check this condition, and I am not sure
that it is worth the effort for my use case.

So I would invite you to work on this after this patch series settles, if
you are interested.

Ciao,
Dscho

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

[Johannes Schindelin]

Hi Jake & Phillip,

On Sat, 20 Jan 2018, Jacob Keller wrote:

> On Fri, Jan 19, 2018 at 6:45 AM, Phillip Wood  
> wrote:
> > On 18/01/18 15:35, 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   
> >
> > I'm concerned that this will be confusing for users. All of the other
> > rebase commands replay the changes in the commit hash immediately
> > following the command name. This command instead uses the first commit
> > to specify the message which is different to both 'git merge' and the
> > existing rebase commands. I wonder if it would be clearer to have 'merge
> > -C   ...' instead so it's clear which argument specifies
> > the message and which the remote head to merge. It would also allow for
> > 'merge -c   ...' in the future for rewording an existing
> > merge message and also avoid the slightly odd 'merge -  ...'. Where
> > it's creating new merges I'm not sure it's a good idea to encourage
> > people to only have oneline commit messages by making it harder to edit
> > them, perhaps it could take another argument to mean open the editor or
> > not, though as Jake said I guess it's not that common.
> 
> I actually like the idea of re-using commit message options like -C,
> -c,  and -m, so we could do:
> 
> merge -C  ... to take message from commit

That is exactly how the Git garden shears do it.

I found it not very readable. That is why I wanted to get away from it in
--recreate-merges.

> merge -c  ...  to take the message from commit and open editor to edit
> merge -m "" ... to take the message from the quoted test
> merge ... to merge and open commit editor with default message
> 
> This also, I think, allows us to not need to put the oneline on the
> end, meaning we wouldn't have to quote the parent commit arguments
> since we could use option semantics?

The oneline is there primarily to give you, the reader, a clue when
reading and editing the todo list.

Reusing it for the `merge -` command was only an afterthought.

> > One thought that just struck me - if a merge or reset command specifies
> > an invalid label is it rescheduled so that it's still in the to-do list
> > when the user edits it after rebase stops?

It is not rescheduled, because the command already failed, so we know it
is bad.

You have to go edit the todo list, possibly after copying the faulty
command from `git status`' output.

> > In the future it might be nice if the label, reset and merge commands
> > were validated when the to-do list is parsed so that the user gets
> > immediate feedback if they try to create a label that is not a valid
> > ref name or that they have a typo in a name given to reset or merge
> > rather than the rebase stopping later.

There are too many possible errors to make this fool-proof. What if the
ref name is valid, but there was no `label` command yet?  What if there
*has* been a `label` command but it is now stuck in the `done` file (which
we do not parse, ever)? What if the user specified two label commands with
the same label?

It sounds like an exercise in futility to try to catch these things in the
parser.

And keep in mind that the parser is used

- when shortening the commit names
- when checking the todo list for accidentally dropped picks
- when skipping unnecessary picks
- when rearranging fixup!/squash! commands
- when adding `exec` commands specified via `-x`

I am not sure that I would come out in favor of trying to catch ref name
errors during parsing time if I wanted to balance bang vs buck.

Ciao,
Dscho

How juggle branches?

[Andrzej]

First, I develop program which uses mysql in branch master.
Next i change name this branch to before_hbase, and began develop HBase 
in branch master. I also develop before_hbase.
Now, instead HBase will be MapR-DB which will before_hbase , not master 
succesor.

How do:
- change before_hbase to master
- old master to hbase
- develop MapR in mapr branch
?
I am in master branch and am changing to hbase:
git checkout -b hbase
git push origin hbase

now worse:
I am in branch before_hbase and need change to master
git checkout -b master  - not works because master exists

I must use rebase? I hear, that rebase can be used only in private 
(.git), not github repositories.
If I change to master and copy files and next commit - change will not 
atomic.

Re: [PATCH 1/8] sequencer: introduce new commands to resettherevision

[Johannes Schindelin]

Hi,

On Mon, 29 Jan 2018, Johannes Schindelin wrote:

> On Fri, 19 Jan 2018, Jacob Keller wrote:
> 
> > On Fri, Jan 19, 2018 at 10:55 AM, Phillip Wood
> >  wrote:
> > > On 19/01/18 12:24, Phillip Wood wrote:
> > >>
> > >> On 18/01/18 15:35, Johannes Schindelin wrote:
> > >>>
> > >>> 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`).
> > >>
> > >> If a user has two work trees and runs a rebase in each with the same
> > >> label name, they'll clobber each other. I'd suggest storing them under
> > >> refs/rewritten/ instead. If the user
> > >> tries to rebase a second worktree with the same detached HEAD as an
> > >> existing rebase then refuse to start.
> > >>
> > >
> > > Ah this isn't a concern after all as patch 5 makes refs/rewritten local
> > > to the worktree. Perhaps you could move that part of patch 5 here or add
> > > a note to the commit message that it will become worktree local later in
> > > the series
> > >
> > > Best Wishes
> > >
> > > Phillip
> > 
> > I'd rather it be included here as well.
> 
> But it would have been really easy to overlook in here. I really want this
> to be a separate commit, also to have a chance to get this done
> *differently* if somebody comes up with a splendid idea how to do that
> (because hard-coding feels quite dirty).

BTW there is an additional good reason why the patch to make
refs/rewritten/* worktree-local is so far away: that is the first time in
the patch series when we can test this really effectively; at that stage
we can easily just add to t3430 because all the building blocks for
`rebase -i --recreate-merges` are in place.

Ciao,
Dscho

Re: [PATCH 1/8] sequencer: introduce new commands to resettherevision

[Johannes Schindelin]

Hi Jake,

On Fri, 19 Jan 2018, Jacob Keller wrote:

> On Fri, Jan 19, 2018 at 10:55 AM, Phillip Wood
>  wrote:
> > On 19/01/18 12:24, Phillip Wood wrote:
> >>
> >> On 18/01/18 15:35, Johannes Schindelin wrote:
> >>>
> >>> 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`).
> >>
> >> If a user has two work trees and runs a rebase in each with the same
> >> label name, they'll clobber each other. I'd suggest storing them under
> >> refs/rewritten/ instead. If the user
> >> tries to rebase a second worktree with the same detached HEAD as an
> >> existing rebase then refuse to start.
> >>
> >
> > Ah this isn't a concern after all as patch 5 makes refs/rewritten local
> > to the worktree. Perhaps you could move that part of patch 5 here or add
> > a note to the commit message that it will become worktree local later in
> > the series
> >
> > Best Wishes
> >
> > Phillip
> 
> I'd rather it be included here as well.

But it would have been really easy to overlook in here. I really want this
to be a separate commit, also to have a chance to get this done
*differently* if somebody comes up with a splendid idea how to do that
(because hard-coding feels quite dirty).

Ciao,
Dscho

Re: [PATCH 1/8] sequencer: introduce new commands to resettherevision

[Johannes Schindelin]

Hi Phillip,

On Fri, 19 Jan 2018, Phillip Wood wrote:

> 
> On 18/01/18 15:35, Johannes Schindelin wrote:
> 
> > 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 
> 
> If I've understood the code below correctly then reset will clobber
> untracked files, this is the opposite behaviour to what happens when
> tries to checkout  at the start of a rebase - then it will fail if
> untracked files would be overwritten.

This would be completely unintentional, I will verify that untracked files
are not clobbered.

However, in practice this should not happen because the intended use case
is for revisions to be labeled *before* checking them out at a later
stage. Therefore, the files that would be clobbered would already have
been tracked in the revision when it was labeled, and I do not quite see
how those files could become untracked without playing sloppy exec games
in between.

> > 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`).
> 
> If a user has two work trees and runs a rebase in each with the same
> label name, they'll clobber each other. I'd suggest storing them under
> refs/rewritten/ instead. If the user
> tries to rebase a second worktree with the same detached HEAD as an
> existing rebase then refuse to start.

That is why a later patch marks those refs/rewritten/ refs as
worktree-local.

> > +static int do_label(const char *name, int len)
> > +{
> > +   struct ref_store *refs = get_main_ref_store();
> > +   struct ref_transaction *transaction;
> > +   struct strbuf ref_name = STRBUF_INIT, err = STRBUF_INIT;
> > +   struct strbuf msg = STRBUF_INIT;
> > +   int ret = 0;
> > +   struct object_id head_oid;
> > +
> > +   strbuf_addf(_name, "refs/rewritten/%.*s", len, name);
> > +   strbuf_addf(, "label '%.*s'", len, name);
> 
> The other reflog messages below have a (rebase -i) prefix

Good point. I changed it to "rebase -i (label)".

> > +   transaction = ref_store_transaction_begin(refs, );
> > +   if (!transaction ||
> > +   get_oid("HEAD", _oid) ||
> > +   ref_transaction_update(transaction, ref_name.buf, _oid, NULL,
> > +  0, msg.buf, ) < 0 ||
> > +   ref_transaction_commit(transaction, )) {
> > +   error("%s", err.buf);
> 
> if get_oid() fails then err is empty so there wont be an message after
> the 'error: '

Yep, that would be nasty. Fixed.

> > +static int do_reset(const char *name, int len)
> > +{
> > +   struct strbuf ref_name = STRBUF_INIT;
> > +   struct object_id oid;
> > +   struct lock_file lock = LOCK_INIT;
> > +   struct tree_desc desc;
> > +   struct tree *tree;
> > +   struct unpack_trees_options opts;
> > +   int ret = 0, i;
> > +
> > +   if (hold_locked_index(, LOCK_REPORT_ON_ERROR) < 0)
> > +   return -1;
> > +
> > +   for (i = 0; i < len; i++)
> > +   if (isspace(name[i]))
> > +   len = i;
> 
> If name starts with any white space then I think this effectively
> truncates name to a bunch of white space which doesn't sound right. I'm
> not sure how this is being called, but it might be better to clean up
> name when the to-do list is parsed instead.

The left-trimming of the name was already performed as part of the todo
list parsing.

And we are not really right-trimming here. We are splitting a line of the
form

reset  

In fact, after reflecting about it, I changed the code so that it would
now even read:

reset  # 

So the code really is doing the intended thing here.

Ciao,
Dscho

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

[Johannes Schindelin]

Hi Eric,

On Fri, 19 Jan 2018, Eric Sunshine wrote:

> On Thu, Jan 18, 2018 at 10:35 AM, Johannes Schindelin
>  wrote:
> > [...]
> > 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`.
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> > @@ -900,6 +900,7 @@ fi
> >  if test t != "$preserve_merges"
> >  then
> > git rebase--helper --make-script ${keep_empty:+--keep-empty} \
> > +   ${recreate_merges:+--recreate-merges} \
> 
> If the user specifies both --preserve-merges and --recreate-merges, it
> looks like --preserve-merges will take precedence.
> 
> Should git-rebase.sh have a mutual-exclusion check and error out if
> both are specified?

Maybe. I welcome you to contribute such a patch once recreate-merges made
it into the code base.

In other words: this would be premature optimization. We're not at that
stage yet.

Ciao,
Dscho

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

[Johannes Schindelin]

Hi Junio,

On Tue, 23 Jan 2018, Junio C Hamano wrote:

> Eric Sunshine  writes:
> 
> >> +   is_octopus = to_merge && to_merge->next;
> >> +
> >> +   if (is_octopus)
> >> +   BUG("Octopus merges not yet supported");
> >
> > Is this a situation which the end-user can trigger by specifying a
> > merge with more than two parents? If so, shouldn't this be just a
> > normal error message rather than a (developer) bug message? Or, am I
> > misunderstanding?
> 
> BUG() is "we wrote code carefully so that this should not trigger;
> we do not _expect_ the code to reach here".  This one is expected to
> trigger, and I agree with you that it should be die(), if the series
> is meant to be released to the general public in the current form
> (i.e. until the limitation is lifted so that it can handle an
> octopus).
> 
> If the callers are made more careful to check if there is an octopus
> involved and reject the request early, then seeing an octopus in
> this location in a loop will become a BUG().

This has occupied both of you for way too long.

It is *not interesting*. What *is* interesting is for example the
discussion about the "cousin commits". And maybe both of you gentle
persons can spend your brain cycles splendidly by trying to come up with a
better term. Or by trying to beat out obvious or not-so-obvious bugs in
the code.

Seriously, I am not interested in a discussion about BUG() vs die() as
long as there may be real bugs hiding.

Ciao,
Dscho

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

[Johannes Schindelin]

Hi Eric,

On Fri, 19 Jan 2018, Eric Sunshine wrote:

> On Thu, Jan 18, 2018 at 10:35 AM, Johannes Schindelin
>  wrote:
> 
> > structure (similar in spirit to --preserve-merges, but with a
> > substantially less-broken design).
> > [...]
> > Signed-off-by: Johannes Schindelin 
> > ---
> > diff --git a/sequencer.c b/sequencer.c
> > @@ -2785,6 +2787,335 @@ void append_signoff(struct strbuf *msgbuf, int 
> > ignore_footer, unsigned flag)
> > +static const char *label_oid(struct object_id *oid, const char *label,
> > +struct label_state *state)
> > +{
> > +   [...]
> > +   } else if (((len = strlen(label)) == GIT_SHA1_RAWSZ &&
> > +   !get_oid_hex(label, )) ||
> > +  hashmap_get_from_hash(>labels,
> > +strihash(label), label)) {
> > +   /*
> > +* If the label already exists, or if the label is a valid 
> > full
> > +* OID, we append a dash and a number to make it unique.
> > +*/
> > +   [...]
> > +   for (i = 2; ; i++) {
> 
> Why '2'? Is there some non-obvious significance to this value?

I personally found it irritating to have labels "sequencer",
"sequencer-1". It sounds *wrong* to have a "-1". Because it is the second
label referring to the term "sequencer". So if there are two labels that
both want to be named "sequencer", the first one wins, and the second one
will be called "sequencer-2".

Hence the 2.

> > +static int make_script_with_merges(struct pretty_print_context *pp,
> > +  struct rev_info *revs, FILE *out,
> > +  unsigned flags)
> > +{
> > +   [...]
> > +   is_octopus = to_merge && to_merge->next;
> > +
> > +   if (is_octopus)
> > +   BUG("Octopus merges not yet supported");
> 
> Is this a situation which the end-user can trigger by specifying a
> merge with more than two parents? If so, shouldn't this be just a
> normal error message rather than a (developer) bug message? Or, am I
> misunderstanding?

You are misunderstanding.

This is just a place-holder here. The patches to introduce support for
octopus merges are already written. They are lined up after this here
patch series, is all.

As such, please do not occupy your mind on the specifics or even the
upper-case of the "Octopus". This line is here only as a hint for the
reviewer that this is not yet implemented. And BUG(...) was chosen because
that way, we are not even tempted to waste the time of translators.

Speaking of wasting time... let's move on to further interesting code
reviews.

Ciao,
Dscho

Re: Location limits on development, staging and production environments

[Bryan Turner]

On Mon, Jan 29, 2018 at 11:08 AM, H  wrote:
> I am a newcomer to git looking to set up a web development environment where 
> individual computers are used for development, the development.git, 
> staging.git and production.git repositories are stored on an external server 
> reachable by password-less ssh and the staging and production websites are on 
> yet another server, also reachable by password-less ssh from the git-server 
> (and the development machines).
>
> Locating the three git repositories on an external server works fine but I 
> have not been able to have the staging and production deployment files on 
> another server. I believe this is what is referred by GIT_WORK_TREE and based 
> on what I found on the web I created a post-receive hook of staging.git with 
> the two lines:
>
> #!/bin/sh
> GIT_WORK_TREE=user@1.2.3.4:/var/www/html/dev.whatever git checkout -f master
>
> I believe this should deploy the files from the development work tree.
>
> The above, however, fails. Should it work? I am running git 1.7.1 on CentOS 6.

No, I wouldn't expect that to work. GIT_WORK_TREE is not remote-aware
in that way. It's expected to be a normal-ish filesystem path.

Based on your description, and the hook you've written, it seems like
your intention is for the source to automatically be fetched and
checked out on the staging environment after each push. (This is
dangerous, and likely _not_ what you actually want, but I'll get to
that in a moment.)

One option would be to setup something like NFS, so the git-server can
mount the filesystems from the staging and production nodes.

A different, likely better, option would be to have the post-receive
script on the git-server use straight ssh to trigger a checkout script
on the staging server, e.g.:
#!/bin/sh
ssh example@staging-server -C /opt/deploy-staging.sh

Your deploy-staging script would then do something like:
#!/bin/sh
GIT_WORK_TREE=/var/www/html/dev.whatever git pull origin

That said, though, having such a simple script is dangerous because
Git is fully capable of having receiving multiple pushes concurrently,
and they can all succeed as long as they're updating different
branches. Since your script isn't considering what branches were
changed by the push, it could end up triggering simultaneous git
processes on the staging server all attempting to deploy concurrently.

The stdin for the post-receive hook receives details about which refs
were changed, and you'll likely want to update your script to parse
stdin and only try to deploy staging if a specific, relevant branch
(master in your example) has changed.

Lastly, I'll note that using post-receive will make the pushing
(remote) user wait while the staging server is deployed. If that
process is likely to take very long, you might want to decouple the
two somehow.

Hope this helps!

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

[Johannes Schindelin]

Hi Junio,

On Wed, 24 Jan 2018, Junio C Hamano wrote:

> Eric Sunshine  writes:
> 
> >> +static int do_reset(const char *name, int len)
> >> +{
> >> +   [...]
> >> +   if (hold_locked_index(, LOCK_REPORT_ON_ERROR) < 0)
> >> +   return -1;
> >> +
> >> +   for (i = 0; i < len; i++)
> >> +   if (isspace(name[i]))
> >> +   len = i;
> >
> > What is the purpose of this loop? I could imagine that it's trying to
> > strip all whitespace from the end of 'name', however, to do that it
> > would iterate backward, not forward. (Or perhaps it's trying to
> > truncate at the first space, but then it would need to invert the
> > condition or use 'break'.) Am I missing something obvious?
> 
> I must be missing the same thing.  Given that the callers of
> do_reset(), other than the "bug" thing that passes the hard coded
> "onto", uses item->arg/item->arg_len which includes everything after
> the insn word on the line in the todo list, I do suspect that the
> intention is to stop at the first whitespace char to avoid creating
> a ref with whitespace in it, i.e. it is a bug that can be fixed with
> s/len = i/break/.
> 
> The code probably should further check the resulting string with
> check_ref_format() to detect strange chars and char sequences that
> make the resulting refname invalid.  For example, you would not want
> to allow a label with two consecutive periods in it.

The code already checks that by creating a ref.

No need to go crazy and validate ref names every time we parse the todo
list (which is quite often), eh?

Ciao,
Dscho

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

[Johannes Schindelin]

Hi Eric,

On Fri, 19 Jan 2018, Eric Sunshine wrote:

> On Thu, Jan 18, 2018 at 10:35 AM, Johannes Schindelin
>  wrote:
> > [...]
> > +static int do_reset(const char *name, int len)
> > +{
> > +   [...]
> > +   if (hold_locked_index(, LOCK_REPORT_ON_ERROR) < 0)
> > +   return -1;
> > +
> > +   for (i = 0; i < len; i++)
> > +   if (isspace(name[i]))
> > +   len = i;
> 
> What is the purpose of this loop? I could imagine that it's trying to
> strip all whitespace from the end of 'name', however, to do that it
> would iterate backward, not forward. (Or perhaps it's trying to
> truncate at the first space, but then it would need to invert the
> condition or use 'break'.) Am I missing something obvious?

Yes, you are missing something obvious. The idea of the `reset` command is
that it not only has a label, but also the oneline of the original commit:

reset branch-point sequencer: prepare for cleanup

In this instance, `branch-point` is the label. And for convenience of the
person editing, it also has the oneline. This came in *extremely* handy
when editing the commit topology in Git for Windows, i.e. when introducing
topic branches or flattening them.

In the Git garden shears, I separated the two arguments via `#`:

reset branch-point # sequencer: prepare for cleanup

I guess that is actually more readable, so I will introduce that into this
patch series, too.

Ciao,
Dscho

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

[Johannes Schindelin]

Hi Philip,

On Thu, 18 Jan 2018, Philip Oakley wrote:

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

It is not necessarily unchanged, because this is an *interactive* rebase.
If you mark `D` for `reword`, for example, it may be changed.

I use the label D' in the mathematical sense, to indicate that D' is
derived from D. It may even be identical to D, but the point is that it is
in the todo list of the interactive rebase, so it can be changed. As
opposed to, say, A and B. Those cannot be changed in this interactive
rebase.

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

I don't find "ancestry path" any more intuitive a term than the
mathematically correct "uncomparable".

If you have a better way to explain this (without devolving into
mathematical terminology), please let's hear it.

Don't get me wrong, as a mathematician I am comfortable with very precise
descriptions involving plenty of Greek symbols.

But this documentation, and these commit messages do not target myself. I
know perfectly well what I am talking about here. The target audience are
software developers who may not have a background in mathematics, who do
not even want to fully understand what the heck constitutes a Directed
Acyclic Graph.

So what we need here is plain English. And I had thought that the analogy
with the family tree would be intuitive enough for even math haters to
understand easily and quickly...

Ciao,
Dscho

Re: "git fast-import" crashes parsing output from "fossil export --git"

[Rowan Thorpe]

On 29 January 2018 at 19:11, SZEDER Gábor  wrote:
> > ..[snip]..
> > Just commit some weird filenames, even one with a newline in it, to
> > test the code.
> > from :26779
> > M 100644 :427 :abc
> > M 100644 :10049 abc
> > def.txt
>
> A path like this must be quoted.  Quoting from 'git fast-import'
> manpage:
>
>   A  string must use UNIX-style directory separators (forward
>   slash /), may contain any byte other than LF, and must not start
>   with double quote (").
>
>   A path can use C-style string quoting; this is accepted in all
>   cases and mandatory if the filename starts with double quote or
>   contains LF. In C-style quoting, the complete name should be
>   surrounded with double quotes, and any LF, backslash, or double
>   quote characters must be escaped by preceding them with a backslash
>   (e.g., "path/with\n, \\ and \" in it").

Ah, thanks. I had skimmed that manpage quickly but obviously too
quickly and somehow missed that paragraph. I will post the bug-report
at Fossil, where it belongs. Sorry for the noise.

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

[Johannes Schindelin]

Hi Philip,

On Thu, 18 Jan 2018, 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 hoped that the example todo lists would clarify that 'onto' is just the
name of the first label:

label onto

is *literally* how all of those todo lists start. And that is why there
are no possible concerns about any missing `--onto` argument: that
argument is irrelevant for that label.

Maybe it is just a bad name. Maybe `START` would be a better label.

What do you think?

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

I dropped the `bud` command.

Ciao,
Dscho

[PATCH v5 1/7] strbuf: remove unnecessary NUL assignment in xstrdup_tolower()

[tboegi]

From: Lars Schneider 

Since 3733e69464 (use xmallocz to avoid size arithmetic, 2016-02-22) we
allocate the buffer for the lower case string with xmallocz(). This
already ensures a NUL at the end of the allocated buffer.

Remove the unnecessary assignment.

Signed-off-by: Lars Schneider 
Signed-off-by: Torsten Bögershausen 
---
 strbuf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/strbuf.c b/strbuf.c
index 8007be8fb..490f7850e 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -781,7 +781,6 @@ char *xstrdup_tolower(const char *string)
result = xmallocz(len);
for (i = 0; i < len; i++)
result[i] = tolower(string[i]);
-   result[i] = '\0';
return result;
 }
 
-- 
2.16.0.rc0.2.g64d3e4d0cc.dirty

[PATCH v5 5/7] convert: add 'working-tree-encoding' attribute

[tboegi]

From: Lars Schneider 

Git recognizes files encoded with ASCII or one of its supersets (e.g.
UTF-8 or ISO-8859-1) as text files. All other encodings are usually
interpreted as binary and consequently built-in Git text processing
tools (e.g. 'git diff') as well as most Git web front ends do not
visualize the content.

Add an attribute to tell Git what encoding the user has defined for a
given file. If the content is added to the index, then Git converts the
content to a canonical UTF-8 representation. On checkout Git will
reverse the conversion.

Signed-off-by: Lars Schneider 
Signed-off-by: Torsten Bögershausen 
---
 Documentation/gitattributes.txt  |  60 
 convert.c| 190 -
 convert.h|   1 +
 sha1_file.c  |   2 +-
 t/t0028-working-tree-encoding.sh | 196 +++
 5 files changed, 447 insertions(+), 2 deletions(-)
 create mode 100755 t/t0028-working-tree-encoding.sh

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 30687de81..a8dbf4be3 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -272,6 +272,66 @@ few exceptions.  Even though...
   catch potential problems early, safety triggers.
 
 
+`working-tree-encoding`
+^^^
+
+Git recognizes files encoded with ASCII or one of its supersets (e.g.
+UTF-8 or ISO-8859-1) as text files.  All other encodings are usually
+interpreted as binary and consequently built-in Git text processing
+tools (e.g. 'git diff') as well as most Git web front ends do not
+visualize the content.
+
+In these cases you can tell Git the encoding of a file in the working
+directory with the `working-tree-encoding` attribute. If a file with this
+attributes is added to Git, then Git reencodes the content from the
+specified encoding to UTF-8 and stores the result in its internal data
+structure (called "the index"). On checkout the content is encoded
+back to the specified encoding.
+
+Please note that using the `working-tree-encoding` attribute may have a
+number of pitfalls:
+
+- Git clients that do not support the `working-tree-encoding` attribute
+  will checkout the respective files UTF-8 encoded and not in the
+  expected encoding. Consequently, these files will appear different
+  which typically causes trouble. This is in particular the case for
+  older Git versions and alternative Git implementations such as JGit
+  or libgit2 (as of January 2018).
+
+- Reencoding content to non-UTF encodings (e.g. SHIFT-JIS) can cause
+  errors as the conversion might not be round trip safe.
+
+- Reencoding content requires resources that might slow down certain
+  Git operations (e.g 'git checkout' or 'git add').
+
+Use the `working-tree-encoding` attribute only if you cannot store a file in
+UTF-8 encoding and if you want Git to be able to process the content as
+text.
+
+Use the following attributes if your '*.txt' files are UTF-16 encoded
+with byte order mark (BOM) and you want Git to perform automatic line
+ending conversion based on your platform.
+
+
+*.txt  text working-tree-encoding=UTF-16
+
+
+Use the following attributes if your '*.txt' files are UTF-16 little
+endian encoded without BOM and you want Git to use Windows line endings
+in the working directory.
+
+
+*.txt  working-tree-encoding=UTF-16LE text eol=CRLF
+
+
+You can get a list of all available encodings on your platform with the
+following command:
+
+
+iconv --list
+
+
+
 `ident`
 ^^^
 
diff --git a/convert.c b/convert.c
index b976eb968..0c372069b 100644
--- a/convert.c
+++ b/convert.c
@@ -7,6 +7,7 @@
 #include "sigchain.h"
 #include "pkt-line.h"
 #include "sub-process.h"
+#include "utf8.h"
 
 /*
  * convert.c - convert a file when checking it out and checking it in.
@@ -265,6 +266,147 @@ static int will_convert_lf_to_crlf(size_t len, struct 
text_stat *stats,
 
 }
 
+static struct encoding {
+   const char *name;
+   struct encoding *next;
+} *encoding, **encoding_tail;
+static const char *default_encoding = "UTF-8";
+
+static int encode_to_git(const char *path, const char *src, size_t src_len,
+struct strbuf *buf, struct encoding *enc, int 
conv_flags)
+{
+   char *dst;
+   int dst_len;
+
+   /*
+* No encoding is specified or there is nothing to encode.
+* Tell the caller that the content was not modified.
+*/
+   if (!enc || (src && !src_len))
+   return 0;
+
+   /*
+* Looks like we got called from "would_convert_to_git()".
+* This means Git wants to know if it would encode (= modify!)
+* the content. Let's answer with "yes", since an encoding was
+  

[PATCH/RFC v5 7/7] Careful with CRLF when using e.g. UTF-16 for working-tree-encoding

[tboegi]

From: Torsten Bögershausen 

UTF-16 encoded files are treated as "binary" by Git, and no CRLF
conversion is done.
When the UTF-16 encoded files are converted into UF-8 using the new
"working-tree-encoding", the CRLF are converted if core.autocrlf is true.

This may lead to confusion:
A tool writes an UTF-16 encoded file with CRLF.
The file is commited with core.autocrlf=true, the CLRF are converted into LF.
The repo is pushed somewhere and cloned by a different user, who has
decided to use core.autocrlf=false.
He uses the same tool, and now the CRLF are not there as expected, but LF,
make the file useless for the tool.

Avoid this (possible) confusion by ignoring core.autocrlf for all files
which have "working-tree-encoding" defined.

The user can still use a .gitattributes file and specify the line endings
like "text=auto", "text", or "text eol=crlf" and let that .gitattribute
file travel together with push and clone.

Change convert.c to e more careful, simplify the initialization when
attributes are retrived (and none are specified) and update the documentation.

Signed-off-by: Torsten Bögershausen 
---
 Documentation/gitattributes.txt |  9 ++---
 convert.c   | 15 ---
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index a8dbf4be3..3665c4677 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -308,12 +308,15 @@ Use the `working-tree-encoding` attribute only if you 
cannot store a file in
 UTF-8 encoding and if you want Git to be able to process the content as
 text.
 
+Note that when `working-tree-encoding` is defined, core.autocrlf is ignored.
+Set the `text` attribute (or `text=auto`) to enable CRLF conversions.
+
 Use the following attributes if your '*.txt' files are UTF-16 encoded
-with byte order mark (BOM) and you want Git to perform automatic line
-ending conversion based on your platform.
+with byte order mark (BOM) and you want Git to perform line
+ending conversion based on core.eol.
 
 
-*.txt  text working-tree-encoding=UTF-16
+*.txt  working-tree-encoding=UTF-16 text
 
 
 Use the following attributes if your '*.txt' files are UTF-16 little
diff --git a/convert.c b/convert.c
index 13fad490c..e7f11d1db 100644
--- a/convert.c
+++ b/convert.c
@@ -1264,15 +1264,24 @@ static void convert_attrs(struct conv_attrs *ca, const 
char *path)
}
ca->checkout_encoding = git_path_check_encoding(ccheck + 5);
} else {
-   ca->drv = NULL;
-   ca->crlf_action = CRLF_UNDEFINED;
-   ca->ident = 0;
+   memset(ca, 0, sizeof(*ca));
}
 
/* Save attr and make a decision for action */
ca->attr_action = ca->crlf_action;
if (ca->crlf_action == CRLF_TEXT)
ca->crlf_action = text_eol_is_crlf() ? CRLF_TEXT_CRLF : 
CRLF_TEXT_INPUT;
+   /*
+* Often UTF-16 encoded files are read and written by programs which
+* really need CRLF, and it is important to keep the CRLF "as is" when
+* files are committed with core.autocrlf=true and the repo is pushed.
+* The CRLF would be converted into LF when the repo is cloned to
+* a machine with core.autocrlf=false.
+* Obey the "text" and "eol" attributes and be independent on the
+* local core.autocrlf for all "encoded" files.
+*/
+   if ((ca->crlf_action == CRLF_UNDEFINED) && ca->checkout_encoding)
+   ca->crlf_action = CRLF_BINARY;
if (ca->crlf_action == CRLF_UNDEFINED && auto_crlf == AUTO_CRLF_FALSE)
ca->crlf_action = CRLF_BINARY;
if (ca->crlf_action == CRLF_UNDEFINED && auto_crlf == AUTO_CRLF_TRUE)
-- 
2.16.0.rc0.2.g64d3e4d0cc.dirty

[PATCH v5 2/7] strbuf: add xstrdup_toupper()

[tboegi]

From: Lars Schneider 

Create a copy of an existing string and make all characters upper case.
Similar xstrdup_tolower().

This function is used in a subsequent commit.

Signed-off-by: Lars Schneider 
Signed-off-by: Torsten Bögershausen 
---
 strbuf.c | 12 
 strbuf.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/strbuf.c b/strbuf.c
index 490f7850e..a20af696b 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -784,6 +784,18 @@ char *xstrdup_tolower(const char *string)
return result;
 }
 
+char *xstrdup_toupper(const char *string)
+{
+   char *result;
+   size_t len, i;
+
+   len = strlen(string);
+   result = xmallocz(len);
+   for (i = 0; i < len; i++)
+   result[i] = toupper(string[i]);
+   return result;
+}
+
 char *xstrvfmt(const char *fmt, va_list ap)
 {
struct strbuf buf = STRBUF_INIT;
diff --git a/strbuf.h b/strbuf.h
index 14c8c10d6..df7ced53e 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -607,6 +607,7 @@ __attribute__((format (printf,2,3)))
 extern int fprintf_ln(FILE *fp, const char *fmt, ...);
 
 char *xstrdup_tolower(const char *);
+char *xstrdup_toupper(const char *);
 
 /**
  * Create a newly allocated string using printf format. You can do this easily
-- 
2.16.0.rc0.2.g64d3e4d0cc.dirty

[PATCH v5 6/7] convert: add tracing for 'working-tree-encoding' attribute

[tboegi]

From: Lars Schneider 

Add the GIT_TRACE_CHECKOUT_ENCODING environment variable to enable
tracing for content that is reencoded with the 'working-tree-encoding'
attribute. This is useful to debug encoding issues.

Signed-off-by: Lars Schneider 
Signed-off-by: Torsten Bögershausen 
---
 convert.c| 28 
 t/t0028-working-tree-encoding.sh |  2 ++
 2 files changed, 30 insertions(+)

diff --git a/convert.c b/convert.c
index 0c372069b..13fad490c 100644
--- a/convert.c
+++ b/convert.c
@@ -266,6 +266,29 @@ static int will_convert_lf_to_crlf(size_t len, struct 
text_stat *stats,
 
 }
 
+static void trace_encoding(const char *context, const char *path,
+  const char *encoding, const char *buf, size_t len)
+{
+   static struct trace_key coe = TRACE_KEY_INIT(CHECKOUT_ENCODING);
+   struct strbuf trace = STRBUF_INIT;
+   int i;
+
+   strbuf_addf(, "%s (%s, considered %s):\n", context, path, 
encoding);
+   for (i = 0; i < len && buf; ++i) {
+   strbuf_addf(
+   ,"| \e[2m%2i:\e[0m %2x \e[2m%c\e[0m%c",
+   i,
+   (unsigned char) buf[i],
+   (buf[i] > 32 && buf[i] < 127 ? buf[i] : ' '),
+   ((i+1) % 8 && (i+1) < len ? ' ' : '\n')
+   );
+   }
+   strbuf_addchars(, '\n', 1);
+
+   trace_strbuf(, );
+   strbuf_release();
+}
+
 static struct encoding {
const char *name;
struct encoding *next;
@@ -325,6 +348,7 @@ static int encode_to_git(const char *path, const char *src, 
size_t src_len,
error(error_msg, path, enc->name);
}
 
+   trace_encoding("source", path, enc->name, src, src_len);
dst = reencode_string_len(src, src_len, default_encoding, enc->name,
  _len);
if (!dst) {
@@ -340,6 +364,7 @@ static int encode_to_git(const char *path, const char *src, 
size_t src_len,
else
error(msg, path, enc->name, default_encoding);
}
+   trace_encoding("destination", path, default_encoding, dst, dst_len);
 
/*
 * UTF supports lossless round tripping [1]. UTF to other encoding are
@@ -365,6 +390,9 @@ static int encode_to_git(const char *path, const char *src, 
size_t src_len,
 enc->name, default_encoding,
 _src_len);
 
+   trace_encoding("reencoded source", path, enc->name,
+  re_src, re_src_len);
+
if (!re_src || src_len != re_src_len ||
memcmp(src, re_src, src_len)) {
const char* msg = _("encoding '%s' from %s to %s and "
diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh
index 4d85b4277..0f36d4990 100755
--- a/t/t0028-working-tree-encoding.sh
+++ b/t/t0028-working-tree-encoding.sh
@@ -4,6 +4,8 @@ test_description='working-tree-encoding conversion via 
gitattributes'
 
 . ./test-lib.sh
 
+GIT_TRACE_CHECKOUT_ENCODING=1 && export GIT_TRACE_CHECKOUT_ENCODING
+
 test_expect_success 'setup test repo' '
git config core.eol lf &&
 
-- 
2.16.0.rc0.2.g64d3e4d0cc.dirty

[PATCH v5 3/7] utf8: add function to detect prohibited UTF-16/32 BOM

[tboegi]

From: Lars Schneider 

Whenever a data stream is declared to be UTF-16BE, UTF-16LE, UTF-32BE
or UTF-32LE a BOM must not be used [1]. The function returns true if
this is the case.

This function is used in a subsequent commit.

[1] http://unicode.org/faq/utf_bom.html#bom10

Signed-off-by: Lars Schneider 
Signed-off-by: Torsten Bögershausen 
---
 utf8.c | 24 
 utf8.h |  9 +
 2 files changed, 33 insertions(+)

diff --git a/utf8.c b/utf8.c
index 2c27ce013..914881cd1 100644
--- a/utf8.c
+++ b/utf8.c
@@ -538,6 +538,30 @@ char *reencode_string_len(const char *in, int insz,
 }
 #endif
 
+static int has_bom_prefix(const char *data, size_t len,
+ const char *bom, size_t bom_len)
+{
+   return (len >= bom_len) && !memcmp(data, bom, bom_len);
+}
+
+static const char utf16_be_bom[] = {0xFE, 0xFF};
+static const char utf16_le_bom[] = {0xFF, 0xFE};
+static const char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};
+static const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};
+
+int has_prohibited_utf_bom(const char *enc, const char *data, size_t len)
+{
+   return (
+ (!strcmp(enc, "UTF-16BE") || !strcmp(enc, "UTF-16LE")) &&
+ (has_bom_prefix(data, len, utf16_be_bom, sizeof(utf16_be_bom)) ||
+  has_bom_prefix(data, len, utf16_le_bom, sizeof(utf16_le_bom)))
+   ) || (
+ (!strcmp(enc, "UTF-32BE") || !strcmp(enc, "UTF-32LE")) &&
+ (has_bom_prefix(data, len, utf32_be_bom, sizeof(utf32_be_bom)) ||
+  has_bom_prefix(data, len, utf32_le_bom, sizeof(utf32_le_bom)))
+   );
+}
+
 /*
  * Returns first character length in bytes for multi-byte `text` according to
  * `encoding`.
diff --git a/utf8.h b/utf8.h
index 6bbcf31a8..4711429af 100644
--- a/utf8.h
+++ b/utf8.h
@@ -70,4 +70,13 @@ typedef enum {
 void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int 
width,
   const char *s);
 
+/*
+ * Whenever a data stream is declared to be UTF-16BE, UTF-16LE, UTF-32BE
+ * or UTF-32LE a BOM must not be used [1]. The function returns true if
+ * this is the case.
+ *
+ * [1] http://unicode.org/faq/utf_bom.html#bom10
+ */
+int has_prohibited_utf_bom(const char *enc, const char *data, size_t len);
+
 #endif
-- 
2.16.0.rc0.2.g64d3e4d0cc.dirty

[PATCH v5 4/7] utf8: add function to detect a missing UTF-16/32 BOM

[tboegi]

From: Lars Schneider 

If the endianness is not defined in the encoding name, then let's
be strict and require a BOM to avoid any encoding confusion. The
has_missing_utf_bom() function returns true if a required BOM is
missing.

The Unicode standard instructs to assume big-endian if there in no BOM
for UTF-16/32 [1][2]. However, the W3C/WHATWG encoding standard used
in HTML5 recommends to assume little-endian to "deal with deployed
content" [3]. Strictly requiring a BOM seems to be the safest option
for content in Git.

This function is used in a subsequent commit.

[1] http://unicode.org/faq/utf_bom.html#gen6
[2] http://www.unicode.org/versions/Unicode10.0.0/ch03.pdf
 Section 3.10, D98, page 132
[3] https://encoding.spec.whatwg.org/#utf-16le

Signed-off-by: Lars Schneider 
Signed-off-by: Torsten Bögershausen 
---
 utf8.c | 13 +
 utf8.h | 16 
 2 files changed, 29 insertions(+)

diff --git a/utf8.c b/utf8.c
index 914881cd1..f033fec1c 100644
--- a/utf8.c
+++ b/utf8.c
@@ -562,6 +562,19 @@ int has_prohibited_utf_bom(const char *enc, const char 
*data, size_t len)
);
 }
 
+int has_missing_utf_bom(const char *enc, const char *data, size_t len)
+{
+   return (
+  !strcmp(enc, "UTF-16") &&
+  !(has_bom_prefix(data, len, utf16_be_bom, sizeof(utf16_be_bom)) ||
+has_bom_prefix(data, len, utf16_le_bom, sizeof(utf16_le_bom)))
+   ) || (
+  !strcmp(enc, "UTF-32") &&
+  !(has_bom_prefix(data, len, utf32_be_bom, sizeof(utf32_be_bom)) ||
+has_bom_prefix(data, len, utf32_le_bom, sizeof(utf32_le_bom)))
+   );
+}
+
 /*
  * Returns first character length in bytes for multi-byte `text` according to
  * `encoding`.
diff --git a/utf8.h b/utf8.h
index 4711429af..26b5e9185 100644
--- a/utf8.h
+++ b/utf8.h
@@ -79,4 +79,20 @@ void strbuf_utf8_align(struct strbuf *buf, align_type 
position, unsigned int wid
  */
 int has_prohibited_utf_bom(const char *enc, const char *data, size_t len);
 
+/*
+ * If the endianness is not defined in the encoding name, then we
+ * require a BOM. The function returns true if a required BOM is missing.
+ *
+ * The Unicode standard instructs to assume big-endian if there
+ * in no BOM for UTF-16/32 [1][2]. However, the W3C/WHATWG
+ * encoding standard used in HTML5 recommends to assume
+ * little-endian to "deal with deployed content" [3].
+ *
+ * [1] http://unicode.org/faq/utf_bom.html#gen6
+ * [2] http://www.unicode.org/versions/Unicode10.0.0/ch03.pdf
+ * Section 3.10, D98, page 132
+ * [3] https://encoding.spec.whatwg.org/#utf-16le
+ */
+int has_missing_utf_bom(const char *enc, const char *data, size_t len);
+
 #endif
-- 
2.16.0.rc0.2.g64d3e4d0cc.dirty

[PATCH v5 0/7] convert: add support for different encodings

[tboegi]

From: Torsten Bögershausen 

Take V4 from Lars, manually integrated the V2 squash patch,
so a review would be good.

Add my "comments" as a patch, see 7/7 (and this is more like an RFC)

This needs to go on top of tb/crlf-conv-flags


Lars Schneider (6):
  strbuf: remove unnecessary NUL assignment in xstrdup_tolower()
  strbuf: add xstrdup_toupper()
  utf8: add function to detect prohibited UTF-16/32 BOM
  utf8: add function to detect a missing UTF-16/32 BOM
  convert: add 'working-tree-encoding' attribute
  convert: add tracing for 'working-tree-encoding' attribute

Torsten Bögershausen (1):
  Careful with CRLF when using e.g. UTF-16 for working-tree-encoding

 Documentation/gitattributes.txt  |  63 +++
 convert.c| 233 ++-
 convert.h|   1 +
 sha1_file.c  |   2 +-
 strbuf.c |  13 ++-
 strbuf.h |   1 +
 t/t0028-working-tree-encoding.sh | 198 +
 utf8.c   |  37 +++
 utf8.h   |  25 +
 9 files changed, 567 insertions(+), 6 deletions(-)
 create mode 100755 t/t0028-working-tree-encoding.sh

-- 
2.16.0.rc0.2.g64d3e4d0cc.dirty

Re: Shawn Pearce has died

[Ævar Arnfjörð Bjarmason]

On Mon, Jan 29 2018, Jeff King jotted:

> On Mon, Jan 29, 2018 at 04:17:32PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> They don't want any flowers sent over to them, but I wonder if we
>> couldn't make some sort of tribute to Shawn at the upcoming developer
>> meeting in Barcelona (and find some way to have remote people contribute
>> to it).
>>
>> E.g. a short video/audio of different people in the dev community
>> sharing some story about Shawn, or a written list of memories
>> contributed by and signed by various people.
>>
>> I don't know what that would look like exactly, but I think it would be
>> a good thing for his family and especially for his children when they're
>> grown to remember him by, to know that their father contributed to these
>> software projects with people all over the world, and that all these
>> people appreciated his work and him personally.
>
> I like this direction (though like you, I'm not sure exactly what it
> should look like). I'm not sure what kind of video presence GitHub will
> have at the contrib summit, but I'll see about getting some interview
> footage there.
>
> Doing something written, though, may make it easier for remote people to
> collaborate.

Since I suspect many people will want to participate in text form that
wouldn't via video, here's an idea (not mutually exclusive with the
video).

We gather text from people involved in git about Shawn. This gets added
to a git repo, we fast-export it, have it printed as hardcover in
fixed-width font, in "a format your dad made".

[PATCH v1 0/5] Incremental rewrite of git-submodules: git-foreach

[Prathamesh Chavan]

Following series of patches focuses on porting submodule subcommand
git-foreach from shell to C.
An initial attempt for porting was introduced about 9 months back,
and since then then patches have undergone many changes. Some of the 
notable discussion thread which I would like to point out is: [1] 
The previous version of this patch series which was floated is
available at: [2].

The following changes were made to that:
* As it was observed in other submodule subcommand's ported function
  that the number of params increased a lot, the variables quiet and 
  recursive, were replaced in the cb_foreach struct with a single
  unsigned integer variable called flags.

* To accomodate the possiblity of a direct call to the functions
  runcommand_in_submodule(), callback function
  runcommand_in_submodule_cb() was introduced.

[1]: https://public-inbox.org/git/20170419170513.16475-1-pc44...@gmail.com/T/#u
[2]: https://public-inbox.org/git/20170807211900.15001-14-pc44...@gmail.com/

As before you can find this series at: 
https://github.com/pratham-pc/git/commits/patch-series-3

And its build report is available at: 
https://travis-ci.org/pratham-pc/git/builds/
Branch: patch-series-3
Build #202

Prathamesh Chavan (5):
  submodule foreach: correct '$path' in nested submodules from a
subdirectory
  submodule foreach: document '$sm_path' instead of '$path'
  submodule foreach: clarify the '$toplevel' variable documentation
  submodule foreach: document variable '$displaypath'
  submodule: port submodule subcommand 'foreach' from shell to C

 Documentation/git-submodule.txt |  15 ++--
 builtin/submodule--helper.c | 151 
 git-submodule.sh|  40 +--
 t/t7407-submodule-foreach.sh|  38 +-
 4 files changed, 197 insertions(+), 47 deletions(-)

-- 
2.15.1

[PATCH v1 4/5] submodule foreach: document variable '$displaypath'

[Prathamesh Chavan]

It was observed that the variable '$displaypath' was accessible but
undocumented. Hence, document it.

Discussed-with: Ramsay Jones 
Signed-off-by: Stefan Beller 
Signed-off-by: Prathamesh Chavan 
---
 Documentation/git-submodule.txt |  6 --
 t/t7407-submodule-foreach.sh| 22 +++---
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8e7930ebc..0cca702cb 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -183,10 +183,12 @@ information too.
 
 foreach [--recursive] ::
Evaluates an arbitrary shell command in each checked out submodule.
-   The command has access to the variables $name, $sm_path, $sha1 and
-   $toplevel:
+   The command has access to the variables $name, $sm_path, $displaypath,
+   $sha1 and $toplevel:
$name is the name of the relevant submodule section in `.gitmodules`,
$sm_path is the path of the submodule as recorded in the superproject,
+   $displaypath contains the relative path from the current working
+   directory to the submodules root directory,
$sha1 is the commit as recorded in the superproject, and
$toplevel is the absolute path to its superproject, such that
$toplevel/$sm_path is the absolute path of the submodule.
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 0663622a4..6ad57e061 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -82,16 +82,16 @@ test_expect_success 'test basic "submodule foreach" usage' '
 
 cat >expect <../../actual
+   git submodule foreach "echo 
\$toplevel-\$name-\$sm_path-\$displaypath-\$sha1" >../../actual
) &&
test_i18ncmp expect actual
 '
@@ -206,25 +206,25 @@ submodulesha1=$(cd 
clone2/nested1/nested2/nested3/submodule && git rev-parse HEA
 
 cat >expect <../../actual
+   git submodule foreach --recursive "echo 
\$toplevel-\$name-\$sm_path-\$displaypath-\$sha1" >../../actual
) &&
test_i18ncmp expect actual
 '
-- 
2.15.1

[PATCH v1 5/5] submodule: port submodule subcommand 'foreach' from shell to C

[Prathamesh Chavan]

This aims to make git-submodule foreach a builtin. This is the very
first step taken in this direction. Hence, 'foreach' is ported to
submodule--helper, and submodule--helper is called from git-submodule.sh.
The code is split up to have one function to obtain all the list of
submodules. This function acts as the front-end of git-submodule foreach
subcommand. It calls the function for_each_listed_submodule(), which basically
loops through the list and calls function fn, which in this case is
runcommand_in_submodule_cb(). This third function is a callback function that
calls runcommand_in_submodule() with the appropriate parameters and then
takes care of running the command in that submodule, and recursively
performing the same when --recursive is flagged.

The first function module_foreach first parses the options present in
argv, and then with the help of module_list_compute(), generates the list of
submodules present in the current working tree.

The second function for_each_listed_submodule() traverses through the
list, and calls function fn (which in case of submodule subcommand
foreach is runcommand_in_submodule_cb()) is called for each entry.

The third function runcommand_in_submodule_cb() calls the function
runcommand_in_submodule() after passing appropraite parameters.

The fourth function runcommand_in_submodule(), generates a submodule struct sub
for $name, value and then later prepends name=sub->name; and other
value assignment to the env argv_array structure of a child_process.
Also the  of submodule-foreach is push to args argv_array
structure and finally, using run_command the commands are executed
using a shell.

The fourth function also takes care of the recursive flag, by creating
a separate child_process structure and prepending "--super-prefix displaypath",
to the args argv_array structure. Other required arguments and the
input  of submodule-foreach is also appended to this argv_array.

Helped-by: Brandon Williams 
Mentored-by: Christian Couder 
Mentored-by: Stefan Beller 
Signed-off-by: Prathamesh Chavan 
---
 builtin/submodule--helper.c | 151 
 git-submodule.sh|  39 +---
 2 files changed, 152 insertions(+), 38 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index a5c4a8a69..46dee6bf5 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -718,6 +718,156 @@ static int module_name(int argc, const char **argv, const 
char *prefix)
return 0;
 }
 
+struct cb_foreach {
+   int argc;
+   const char **argv;
+   const char *prefix;
+   unsigned int flags;
+};
+#define CB_FOREACH_INIT { 0, NULL, NULL, 0 }
+
+static void runcommand_in_submodule(const char *path, const struct object_id 
*ce_oid,
+   int argc, const char **argv, const char 
*prefix,
+   unsigned int flags)
+{
+   const struct submodule *sub;
+   struct child_process cp = CHILD_PROCESS_INIT;
+   char *displaypath;
+
+   displaypath = get_submodule_displaypath(path, prefix);
+
+   sub = submodule_from_path(_oid, path);
+
+   if (!sub)
+   die(_("No url found for submodule path '%s' in .gitmodules"),
+ displaypath);
+
+   if (!is_submodule_populated_gently(path, NULL))
+   goto cleanup;
+
+   prepare_submodule_repo_env(_array);
+
+   /*
+* For the purpose of executing  in the submodule,
+* separate shell is used for the purpose of running the
+* child process.
+*/
+   cp.use_shell = 1;
+   cp.dir = path;
+
+   /*
+* NEEDSWORK: the command currently has access to the variables $name,
+* $sm_path, $displaypath, $sha1 and $toplevel only when the command
+* contains a single argument. This is done for maintianing a faithful
+* translation from shell script.
+*/
+   if (argc == 1) {
+   char *toplevel = xgetcwd();
+
+   argv_array_pushf(_array, "name=%s", sub->name);
+   argv_array_pushf(_array, "sm_path=%s", path);
+   argv_array_pushf(_array, "displaypath=%s", displaypath);
+   argv_array_pushf(_array, "sha1=%s",
+oid_to_hex(ce_oid));
+   argv_array_pushf(_array, "toplevel=%s", toplevel);
+
+   /*
+* Since the path variable was accessible from the script
+* before porting, it is also made available after porting.
+* The environment variable "PATH" has a very special purpose
+* on windows. And since environment variables are
+* case-insensitive in windows, it interferes with the
+* existing PATH variable. Hence, to avoid that, we expose
+* path via the 

[PATCH v1 3/5] submodule foreach: clarify the '$toplevel' variable documentation

[Prathamesh Chavan]

It does not contain the topmost superproject as the author assumed,
but the direct superproject, such that $toplevel/$sm_path is the
actual absolute path of the submodule.

Discussed-with: Ramsay Jones 
Signed-off-by: Stefan Beller 
Signed-off-by: Prathamesh Chavan 
---
 Documentation/git-submodule.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index a23baef62..8e7930ebc 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -188,7 +188,8 @@ foreach [--recursive] ::
$name is the name of the relevant submodule section in `.gitmodules`,
$sm_path is the path of the submodule as recorded in the superproject,
$sha1 is the commit as recorded in the superproject, and
-   $toplevel is the absolute path to the top-level of the superproject.
+   $toplevel is the absolute path to its superproject, such that
+   $toplevel/$sm_path is the absolute path of the submodule.
Note that to avoid conflicts with '$PATH' on Windows, the '$path'
variable is now a deprecated synonym of '$sm_path' variable.
Any submodules defined in the superproject but not checked out are
-- 
2.15.1

[PATCH v1 1/5] submodule foreach: correct '$path' in nested submodules from a subdirectory

[Prathamesh Chavan]

When running 'git submodule foreach' from a subdirectory of your
repository, nested submodules get a bogus value for $sm_path:
For a submodule 'sub' that contains a nested submodule 'nested',
running 'git -C dir submodule foreach echo $path' would report
path='../nested' for the nested submodule. The first part '../' is
derived from the logic computing the relative path from $pwd to the
root of the superproject. The second part is the submodule path inside
the submodule. This value is of little use and is hard to document.

There are two different possible solutions that have more value:
(a) The path value is documented as the path from the toplevel of the
superproject to the mount point of the submodule.
In this case we would want to have path='sub/nested'.

(b) As Ramsay noticed the documented value is wrong. For the non-nested
case the path is equal to the relative path from $pwd to the
submodules working directory. When following this model,
the expected value would be path='../sub/nested'.

The behavior for (b) was introduced in 091a6eb0fe (submodule: drop the
top-level requirement, 2013-06-16) the intent for $path seemed to be
relative to $cwd to the submodule worktree, but that did not work for
nested submodules, as the intermittent submodules were not included in
the path.

If we were to fix the meaning of the $path using (a) such that "path"
is "the path from the toplevel of the superproject to the mount point
of the submodule", we would break any existing submodule user that runs
foreach from non-root of the superproject as the non-nested submodule
'../sub' would change its path to 'sub'.

If we would fix the meaning of the $path using (b), such that "path"
is "the relative path from $pwd to the submodule", then we would break
any user that uses nested submodules (even from the root directory) as
the 'nested' would become 'sub/nested'.

Both groups can be found in the wild.  The author has no data if one group
outweighs the other by large margin, and offending each one seems equally
bad at first.  However in the authors imagination it is better to go with
(a) as running from a sub directory sounds like it is carried out
by a human rather than by some automation task.  With a human on
the keyboard the feedback loop is short and the changed behavior can be
adapted to quickly unlike some automation that can break silently.

Discussed-with: Ramsay Jones 
Signed-off-by: Prathamesh Chavan 
Signed-off-by: Stefan Beller 
---
 git-submodule.sh |  1 -
 t/t7407-submodule-foreach.sh | 36 ++--
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 156255a9e..7305ee25f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -345,7 +345,6 @@ cmd_foreach()
prefix="$prefix$sm_path/"
sanitize_submodule_env
cd "$sm_path" &&
-   sm_path=$(git submodule--helper relative-path 
"$sm_path" "$wt_prefix") &&
# we make $path available to scripts ...
path=$sm_path &&
if test $# -eq 1
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 6ba5daf42..0663622a4 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -82,9 +82,9 @@ test_expect_success 'test basic "submodule foreach" usage' '
 
 cat >expect  expect <

[PATCH v1 2/5] submodule foreach: document '$sm_path' instead of '$path'

[Prathamesh Chavan]

As using a variable '$path' may be harmful to users due to
capitalization issues, see 64394e3ae9 (git-submodule.sh: Don't
use $path variable in eval_gettext string, 2012-04-17). Adjust
the documentation to advocate for using $sm_path,  which contains
the same value. We still make the 'path' variable available and
document it as a deprecated synonym of 'sm_path'.

Discussed-with: Ramsay Jones 
Signed-off-by: Stefan Beller 
Signed-off-by: Prathamesh Chavan 
---
 Documentation/git-submodule.txt | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index ff612001d..a23baef62 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -183,12 +183,14 @@ information too.
 
 foreach [--recursive] ::
Evaluates an arbitrary shell command in each checked out submodule.
-   The command has access to the variables $name, $path, $sha1 and
+   The command has access to the variables $name, $sm_path, $sha1 and
$toplevel:
$name is the name of the relevant submodule section in `.gitmodules`,
-   $path is the name of the submodule directory relative to the
-   superproject, $sha1 is the commit as recorded in the superproject,
-   and $toplevel is the absolute path to the top-level of the superproject.
+   $sm_path is the path of the submodule as recorded in the superproject,
+   $sha1 is the commit as recorded in the superproject, and
+   $toplevel is the absolute path to the top-level of the superproject.
+   Note that to avoid conflicts with '$PATH' on Windows, the '$path'
+   variable is now a deprecated synonym of '$sm_path' variable.
Any submodules defined in the superproject but not checked out are
ignored by this command. Unless given `--quiet`, foreach prints the name
of each submodule before evaluating the command.
-- 
2.15.1