Re: [PATCH v3 23/23] cat-file: update of docs

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Update the docs for cat-file command. Some new formatting atoms added
> because of reusing ref-filter code.
> We do not support cat-file atoms in general formatting logic
> (there is just the support for cat-file), that is why some of the atoms
> are still explained in cat-file docs.
> We need to move these explanations when atoms will be supported
> by other commands.

OK, so I've read through the whole series now. I still think it really
needs some squashing in the middle to turn into a more comprehensible
series.

And I think by the end we need to address the atoms here that don't work
in ref-filter. We certainly want them to work in the long term, and the
fact that they don't is I think pointing to having the wrong
architecture here in the intermediate step. We should just have a single
set of placeholders, with no special "cat_file_data". If I understand
correctly, that's what's causing those atoms not to work in
for-each-ref.

-Peff

Re: [PATCH v3 21/23] for-each-ref: tests for new atoms added

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Add tests for new formatting atoms: rest, deltabase, objectsize:disk.
> rest means nothing and we expand it into empty string.
> We need this atom for cat-file command.
> Have plans to support deltabase and objectsize:disk further
> (as it is done in cat-file), now also expand it to empty string.

I'm glad that you're adding tests, but I'm not sure it's a good idea to
add tests checking for the thing we know to be wrong. If anything, you
could be adding test_expect_failure looking for the _right_ thing, and
accept that it does not yet work.

-Peff

Re: [PATCH v3 20/23] ref-filter: unifying formatting of cat-file opts

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> cat-file options are now filled by general logic.

Yay.

One puzzling thing:

> diff --git a/ref-filter.c b/ref-filter.c
> index 8d104b567eb7c..5781416cf9126 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -824,8 +824,12 @@ static void grab_common_values(struct atom_value *val, 
> int deref, struct object
>   else if (!strcmp(name, "objectsize")) {
>   v->value = sz;
>   v->s = xstrfmt("%lu", sz);
> - }
> - else if (deref)
> + } else if (!strcmp(name, "objectsize:disk")) {
> + if (cat_file_info.is_cat_file) {
> + v->value = cat_file_info.disk_size;
> + v->s = xstrfmt("%"PRIuMAX, (uintmax_t)v->value);
> + }
> + } else if (deref)

Why do we care about is_cat_file here. Shouldn't:

  git for-each-ref --format='%(objectsize:disk)'

work? I.e., shouldn't the cat_file_info.disk_size variable be held
somewhere in a used_atom struct?

-Peff

Re: [PATCH v3 17/23] ref-filter: make valid_atom general again

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Stop using valid_cat_file_atom, making the code more general.
> Further commits will contain some tests, docs and
> support of new features.

Yay.

-Peff

Re: [PATCH v3 16/23] ref-filter: make cat_file_info independent

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Remove connection between expand_data variable
> in cat-file and in ref-filter.
> It will help further to get rid of using expand_data in cat-file.

I have to admit I'm confused at this point about what is_cat_file is
for, or even why we need cat_file_data. Shouldn't these items be handled
by their matching ref-filter atoms at this point?

-Peff

Re: [PATCH v3 15/23] cat-file: move skip_object_info into ref-filter

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Move logic related to skip_object_info into ref-filter,
> so that cat-file does not use that field at all.

I think this is going the wrong way. ref-filter should always do as
little work as possible to fulfill the request. So it should skip the
object_info call whenever it can. And then any callers who want to make
sure that the object exists can do so (as long as the formatting code
tells them whether it looked up the object or not).

And then ref-filter doesn't have to know about this skip_object_info
flag at all.

-Peff

Assalamu alaikum

[Mr Ibrahim Zaki]

Assalamu alaikum


My name is Mr. Ibrahim Zaki, I am a staff member working with BCEAO BANK here 
in Ouagadougou, Burkina Faso.


I want you to help me receive the sum of twenty-seven million two hundred 
dollars ($ 27,200,000) in your bank account. This fund was deposited in the 
bank here by a foreign customer who accidentally died next to his family 
members several years ago. Nobody has asked for this fund until now. Contact me 
for more information. PLEASE REPLY ME WITH MY PRIVATE EMAIL 
(mribrahimzak...@gmail.com)


Best regard,
Mr. Ibrahim Zaki

Re: [PATCH v3 14/23] ref_filter: add is_atom_used function

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Delete all items related to split_on_whitespace from ref-filter
> and add new function for handling the logic.
> Now cat-file could invoke that function to implementing its logic.

OK, this is a good direction. I think in a more compact series we'd
avoid moving the split-on-whitespace bits over to ref-filter in the
first place, and have two commits:

 - one early in the series adding is_atom_used()

 - one late in the series switching cat-file over to is_atom_used() as
   part of the conversion to ref-filter

> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 6db57e3533806..3a49b55a1cc2e 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -382,8 +382,7 @@ static int batch_objects(struct batch_options *opt)
>  {
>   struct strbuf buf = STRBUF_INIT;
>   struct expand_data data;
> - int save_warning;
> - int retval = 0;
> + int save_warning, is_rest, retval = 0;

Try to avoid reformatting existing code that you're not otherwise
touching, as it makes the diff noisier. Just adding "int is_rest" would
make this easier to review.

I also think the variable name should probably still be
"split_on_whitespace". It's set based on whether we saw a "%(rest)"
atom, but ultimately we'll use it to decide whether to split.

-Peff

Re: [PATCH v3 13/23] ref-filter: get rid of mark_atom_in_object_info()

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Remove mark_atom_in_object_info() and create same logic
> in terms of ref-filter style.

This one is definitely a step in the right direction. In fact, this is
what I would have expected to see in the beginning. It seems like this
first half of the series really would benefit from being squashed into a
few commits. I.e., I'd have expected to see the whole series looking
something like:

  - preparatory cleanup of ref-filter

  - teach ref-filter any new atoms (or atom options) that cat-file knows
about but it doesn't

  - convert cat-file to use ref-filter

Most of what I've seen so far is basically that second step, but strung
out along a bunch of commits. Can we compact it into a few commits that
all make clear forward progress (by using "rebase -i" with "squash")?

-Peff

Re: [PATCH v3 12/23] cat-file: start reusing populate_value()

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Move logic related to getting object info from cat-file to ref-filter.
> It will help to reuse whole formatting logic from ref-filter further.

This feels like another wrong-direction step, because we'd eventually
expect populate_value() to be called under the hood. And I think in the
end we end up back there. Which is good, but it's hard to keep track of
the dead-ends here...

-Peff

Re: [PATCH v3 09/23] cat-file: start use ref_array_item struct

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Moving from using expand_data to ref_array_item structure.
> That helps us to reuse functions from ref-filter easier.

This one feels weird. The point of a ref_array_item is for the caller to
feed data into the ref-filter formatting code (usually that data comes
from an earlier call to filter_refs(), but in the new cat-file we'd
presumably feed single items).

But here we're adding a bunch of fields for items that we'd expect the
format code to compute itself.

-Peff

Re: [PATCH v3 08/23] ref-filter: reuse parse_ref_filter_atom()

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Continue migrating formatting logic from cat-file to ref-filter.
> Reuse parse_ref_filter_atom() for unifying all processes in ref-filter
> and further removing of mark_atom_in_object_info().

OK, now it looks we're moving in a good direction.

One thing that puzzles me:

> @@ -401,20 +420,14 @@ static int is_atom(const char *atom, const char *s, int 
> slen)
>  static void mark_atom_in_object_info(const char *atom, int len,
>   struct expand_data *data)
>  {
> - if (is_atom("objectname", atom, len))
> - ; /* do nothing */
> - else if (is_atom("objecttype", atom, len))
> + if (is_atom("objecttype", atom, len))
>   data->info.typep = >type;
>   else if (is_atom("objectsize", atom, len))
>   data->info.sizep = >size;
> - else if (is_atom("objectsize:disk", atom, len))
> - data->info.disk_sizep = >disk_size;
>   else if (is_atom("rest", atom, len))
>   data->split_on_whitespace = 1;
>   else if (is_atom("deltabase", atom, len))
>   data->info.delta_base_sha1 = data->delta_base_oid.hash;
> - else
> - die("unknown format element: %.*s", len, atom);
>  }

Why do some of these atoms go away and not others? It seems like we're
now relying on ref-filter to parse some of the common ones using its
existing atom-parser. But wouldn't it have objecttype and objectsize
already, then?

-Peff

Re: [PATCH v3 07/23] cat-file: start migrating formatting to ref-filter

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Move mark_atom_in_object_info() from cat-file to ref-filter and
> start using it in verify_ref_format().
> It also means that we start reusing verify_ref_format() in cat-file.
> 
> Start from simple moving of mark_atom_in_object_info(),
> it would be removed later by integrating all needed processes into
> ref-filter logic.

I see where you're going here, but it feels like there's a lot of
boilerplate to move this thing whole, when we plan to get rid of it in
the long run. After all, we _already_ support most of these placeholders
in ref-filter. I'll keep reading...

-Peff

Re: [PATCH v3 06/23] cat-file: split expand_atom() into 2 functions

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Split expand_atom() into 2 different functions,
> mark_atom_in_object_info() prepares variable for further filling,
> (new) expand_atom() creates resulting string.
> Need that for further reusing of formatting logic from ref-filter.
> Both functions will be step-by-step removed by the end of this patch.

Hmm, this is duplicating the list of atoms. One of the reasons for the
mark_query flag in the original is to have a single list, so that it
can't go out of sync. So I'd hope to see this split go away by the end
of the series...

-Peff

Re: [PATCH v3 05/23] cat-file: move struct expand_data into ref-filter

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Need that for further reusing of formatting logic in cat-file.
> Have plans to get rid of using expand_data in cat-file at all,
> and use it only in ref-filter for collecting, formatting and printing
> needed data.

This seems like another step that we're going to end up reversing later,
because ref-filter has a different representation for these kinds of
expansions. I'll keep reading...

-Peff

Re: [PATCH v3 04/23] ref-filter: make valid_atom as function parameter

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Make valid_atom as a function parameter,
> there could be another variable further.
> Need that for further reusing of formatting logic in cat-file.c.
> 
> We do not need to allow users to pass their own valid_atom variable in
> global functions like verify_ref_format() because in the end we want to
> have same set of valid atoms for all commands. But, as a first step
> of migrating, I create further another version of valid_atom
> for cat-file.

Hmm. So I see where you're going here, but I think in the end we'd want
to have a single valid_atom list again, and we wouldn't need this.

And indeed, it looks like this goes away in patch 17. Can we
reorganize/rebase the series so that we avoid dead-end directions like
this?

-Peff

Re: [PATCH v3 03/23] cat-file: reuse struct ref_format

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Start using ref_format struct instead of simple char*.
> Need that for further reusing of formatting logic from ref-filter.

Makes sense, and the patch itself looks correct.

-Peff

Re: [PATCH v3 02/23] ref-filter: add return value to some functions

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Add return flag to format_ref_array_item(), show_ref_array_item(),
> get_ref_array_info() and populate_value() for further using.
> Need it to handle situations when item is broken but we can not invoke
> die() because we are in batch mode and all items need to be processed.

OK. The source of these errors would eventually be calls in
populate_value(), but we don't flag any errors there yet (well, we do,
but they all end up in die() for now). So I'd expect to see later in the
series those die() calls converted to errors (I haven't looked further
yet; just making a note to myself).

> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1356,8 +1356,9 @@ static const char *get_refname(struct used_atom *atom, 
> struct ref_array_item *re
>  
>  /*
>   * Parse the object referred by ref, and grab needed value.
> + * Return 0 if everything was successful, -1 otherwise.
>   */

We discussed off-list the concept that the caller may want to know one
of three outcomes:

  - we completed the request, having accessed the object
  - we completed the request, but it didn't require accessing any
objects
  - an error occurred accessing the object

Since callers like "cat-file" would need to check has_sha1_file()
manually in the second case. Should this return value actually be an
enum, which would make it easier to convert later to a tri-state?

> -static void populate_value(struct ref_array_item *ref)
> +static int populate_value(struct ref_array_item *ref)
>  {
>   void *buf;
>   struct object *obj;
> @@ -1482,7 +1483,7 @@ static void populate_value(struct ref_array_item *ref)
>   }
>   }
>   if (used_atom_cnt <= i)
> - return;
> + return 0;

Most of these conversions are obviously correct, because they just turn
a void return into one with a value. But this one is trickier:

> @@ -2138,9 +2144,10 @@ void format_ref_array_item(struct ref_array_item *info,
>   ep = strchr(sp, ')');
>   if (cp < sp)
>   append_literal(cp, sp, );
> - get_ref_atom_value(info,
> -parse_ref_filter_atom(format, sp + 2, ep),
> -);
> + if (get_ref_atom_value(info,
> +parse_ref_filter_atom(format, sp + 2, 
> ep),
> +))
> + return -1;
>   atomv->handler(atomv, );
>   }

since it affects the control flow. Might we be skipping any necessary
cleanup in the function if we see an error?

It looks like we may have called push_stack_element(), but we'd never
get to the end of the function where we call pop_stack_element(),
causing us to leak.

-Peff

Re: [PATCH 3/8] perl: generalize the Git::LoadCPAN facility

[Todd Zullinger]

Hi Jonathan,

Jonathan Nieder wrote:
> Ævar Arnfjörð Bjarmason wrote:
> [...]
>> +++ b/perl/Git/LoadCPAN.pm
>> @@ -0,0 +1,74 @@
> [...]
>> +The Perl code in Git depends on some modules from the CPAN, but we
>> +don't want to make those a hard requirement for anyone building from
>> +source.
> 
> not about this patch: have we considered making it a hard requirement?
> Both Mail::Address and Error.pm are pretty widely available, and I
> wonder if we could make the instructions in the INSTALL file say that
> they are required dependencies to simplify things.
> 
> I admit part of my bias here is coming from the distro world, where we
> have to do extra work to get rid of the FromCPAN embedded copies and
> would be happier not to have to.

Heh, a similar bias is what led me to suggest a Makefile
knob to prevent installing the fallbacks.  I neglected to
add you to the Cc list on that reply.  But I was thinking of
Debian and other distributions whom I know would similarly
want to exclude Git/FromCPAN from their git packages.  I
know it's a simple rm, but it might as well be a simple rm
in one place than spread across each package.  :)

It may still be worth considering whether it's reasonable to
make Mail::Address and Error.pm hard requirements, but I'm
not sure if there are any platforms where such a requirement
would be a pain.  If there are folks here who are happy to
maintain this fallback method, then a simple Makefile knob
gives us the best of both worlds.

-- 
Todd
~~
Historian, n. A broad-gauge gossip.
-- Ambrose Bierce, "The Devil's Dictionary"

Re: [PATCH v3 01/23] ref-filter: get rid of goto

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Get rid of goto command in ref-filter for better readability.
> 
> Signed-off-by: Olga Telezhnaia 
> Mentored-by: Christian Couder 
> Mentored by: Jeff King 

OK, makes sense, and the patch looks correct.

-Peff

Re: [PATCH 3/8] perl: generalize the Git::LoadCPAN facility

[Todd Zullinger]

Hi Ævar,

Ævar Arnfjörð Bjarmason wrote:
> +Git::LoadCPAN - Wrapper for loading modules from the CPAN (OS) or Git's own 
> copy
> +
> +=head1 DESCRIPTION
> +
> +The Perl code in Git depends on some modules from the CPAN, but we
> +don't want to make those a hard requirement for anyone building from
> +source.
> +
> +Therefore the L namespace shipped with Git contains
> +wrapper modules like C that will first
> +attempt to load C from the OS, and if that doesn't work
> +will fall back on C shipped with Git
> +itself.
> +
> +Usually OS's will not ship with Git's Git::FromCPAN tree at all,
> +preferring to use their own packaging of CPAN modules instead.

This is something I wondered about.  What's the recommended
method to ensure git packaged for an OS/distribution doesn't
ever use the fallbacks?  Remove $perllibdir/Git/FromCPAN
after make install?

If so, would it be useful to add a Makefile knob to not
install the FromCPAN bits, which may be generally useful to
packagers?

Something like the following, perhaps?

(I'd feel bad suggesting this without a patch, after all the
work you've already done to simplify and improve the perl
bits.)

 8< 
From: Todd Zullinger 
Date: Wed, 14 Feb 2018 23:00:30 -0500
Subject: [PATCH] Makefile: add NO_PERL_CPAN to disable fallback module install

As noted in perl/Git/LoadCPAN.pm, operating system packages often don't
want to ship Git::FromCPAN tree at all, preferring to use their own
packaging of CPAN modules instead.  Allow such packagers to set
NO_PERL_CPAN to easily avoid installing these fallback perl CPAN
modules.

Signed-off-by: Todd Zullinger 
---
 Makefile | 4 
 1 file changed, 4 insertions(+)

diff --git a/Makefile b/Makefile
index 5bcd83ddf3..c4e035e5bf 100644
--- a/Makefile
+++ b/Makefile
@@ -296,6 +296,9 @@ all::
 #
 # Define NO_PERL if you do not want Perl scripts or libraries at all.
 #
+# Define NO_PERL_CPAN if you do not want to install fallbacks for perl CPAN
+# modules.
+#
 # Define PYTHON_PATH to the path of your Python binary (often /usr/bin/python
 # but /usr/bin/python2.7 on some platforms).
 #
@@ -2572,6 +2575,7 @@ ifndef NO_GETTEXT
 endif
 ifndef NO_PERL
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perllibdir_SQ)'
+   test -z "$(NO_PERL_CPAN)" || rm -rf perl/build/lib/Git/FromCPAN
(cd perl/build/lib && $(TAR) cf - .) | \
(cd '$(DESTDIR_SQ)$(perllibdir_SQ)' && umask 022 && $(TAR) xof -)
$(MAKE) -C gitweb install
-- 
2.16.1

I don't particularly like NO_PERL_CPAN, but I'm confident
someone else will suggest an obviously better name.

I thought about moving the 'rm -rf Git/FromCPAN' after the
tar/untar, to keep the files in place for the tests.  No
tests seem to rely on those local files, so I stuck with
removing them before.  That diff was:

--- a/Makefile
+++ b/Makefile
@@ -2574,6 +2574,7 @@
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perllibdir_SQ)'
(cd perl/build/lib && $(TAR) cf - .) | \
(cd '$(DESTDIR_SQ)$(perllibdir_SQ)' && umask 022 && $(TAR) xof -)
+   test -n "$(NO_PERL_CPAN)" && rm -rf 
'$(DESTDIR_SQ)$(perllibdir_SQ)'/Git/FromCPAN
$(MAKE) -C gitweb install
 endif
 ifndef NO_TCLTK

-- 
Todd
~~
Man has made use of his intelligence, he invented stupidity.
-- Remy De Gourmant

Re: git-rebase --undo-skip proposal

[Jacob Keller]

On Wed, Feb 14, 2018 at 5:50 PM, Psidium Guajava  wrote:
> 2018-02-14 22:53 GMT-02:00 Johannes Schindelin :
>> Now, when is the next possible time you can call `git rebase --undo-skip`?
>
> What if the scope were reduced from `--undo-skip` to `--undo-last-skip`?
> Also, further reduce the scope to only allow `--undo-last-skip` during
> a ongoing rebase, not caring about a finished one?
>
> But, this could be so niche that I have doubts if this would ever be used;

I think this is too niche to actually be useful in practice,
especially since figuring out exactly what to replay might be tricky..
I suppose it could keep track of where in the rebase the last skip was
used, and then just go back to rebase and stop there again? That
sounds like just redoing the rebase is easier.. (ie: use the reflog to
go back to before the rebase started, and then re-run it again and
don't skip this time).

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

[Sergey Organov]

Hi Johannes,
Johannes Schindelin  writes:

[...]

>> > I'd not argue this way myself. If there are out-of-git-tree non-human
>> > users that accept and tweak todo _generated_ by current "git rebase -p"
>> > _command_, I also vote for a new option.
>> >
>> 
>> To be fair, I have not seen anything that actually reads the todo list
>> and tweaks it in such a manner. The closest example is the git garden
>> shears script, which simply replaces the todo list.
>> 
>> It's certainly *possible* that such a script would exist though,
>
> We actually know of such scripts.

Please consider to explain this in the description of the change. I
believe readers deserve an explanation of why you decided to invent new
option instead of fixing the old one, even if it were only a suspicion,
more so if it is confidence.

-- Sergey

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

[Sergey Organov]

Johannes Schindelin  writes:

> Hi,
>
> On Tue, 13 Feb 2018, Sergey Organov wrote:
>
>> Johannes Schindelin  writes:
>> 
>> > The wording is poor either way, but you are also not a native speaker so
>> > we have to rely on, say, Eric to help us out here.
>> 
>> Likely, but why didn't you keep original wording from --preserve-merges?
>> Do you feel it's somehow poor either?
>
> Yes, I felt it is poor, especially when --recreate-merges is present, that
> is indeed why I changed it.

So, how about this (yeah, I noticed the option now got arguments, but
please, tweak this to the new implementation yourself):

--recreate-merges::
Recreate merge commits instead of flattening the history. Merge
conflict resolutions or manual amendments to merge commits are
not preserved. 

-p::
--preserve-merges::
(deprecated) This option is similar to --recreate-merges. It has
no proper support for interactive mode and thus is deprecated.
Use '--recreate-merges' instead.


-- Sergey

[PATCH] Makefile: generate Git(3pm) as dependency of the 'doc' and 'man' targets

[SZEDER Gábor]

Since commit 20d2a30f8f (Makefile: replace perl/Makefile.PL with
simple make rules, 2017-12-10), the Git(3pm) man page is only
generated as an indirect dependency of the 'install-doc' and
'install-man' Makefile targets.  Consequently, if someone runs 'make
man && sudo make install-man' (or their 'doc' counterparts), then
Git(3pm) will be generated as root, and the resulting root-owned files
and directories will in turn cause the next user-run 'make clean' to
fail.  This was not an issue in the past, because Git(3pm) was
generated when 'make all' descended into 'perl/', which is usually not
run as root.

List Git(3pm) as a dependency of the 'doc' and 'man' Makefile targets,
too, so it gets generated by targets that are usually built as
ordinary users.

While at it, add 'install-man-perl' to the list of .PHONY targets.

Signed-off-by: SZEDER Gábor 
---
 Makefile | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 5bcd83ddf3..8d2bf4de59 100644
--- a/Makefile
+++ b/Makefile
@@ -2214,13 +2214,15 @@ $(VCSSVN_LIB): $(VCSSVN_OBJS)
 
 export DEFAULT_EDITOR DEFAULT_PAGER
 
-.PHONY: doc man html info pdf
-doc:
+.PHONY: doc man man-perl html info pdf
+doc: man-perl
$(MAKE) -C Documentation all
 
-man:
+man: man-perl
$(MAKE) -C Documentation man
 
+man-perl: perl/build/man/man3/Git.3pm
+
 html:
$(MAKE) -C Documentation html
 
@@ -2618,7 +2620,7 @@ endif
done && \
./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
 
-.PHONY: install-gitweb install-doc install-man install-html install-info 
install-pdf
+.PHONY: install-gitweb install-doc install-man install-man-perl install-html 
install-info install-pdf
 .PHONY: quick-install-doc quick-install-man quick-install-html
 install-gitweb:
$(MAKE) -C gitweb install
@@ -2629,7 +2631,7 @@ install-doc: install-man-perl
 install-man: install-man-perl
$(MAKE) -C Documentation install-man
 
-install-man-perl: perl/build/man/man3/Git.3pm
+install-man-perl: man-perl
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mandir_SQ)/man3'
(cd perl/build/man/man3 && $(TAR) cf - .) | \
(cd '$(DESTDIR_SQ)$(mandir_SQ)/man3' && umask 022 && $(TAR) xof -)
-- 
2.16.1.354.g266d823471

Re: git-rebase --undo-skip proposal

[Psidium Guajava]

2018-02-14 22:53 GMT-02:00 Johannes Schindelin :
> Now, when is the next possible time you can call `git rebase --undo-skip`?

What if the scope were reduced from `--undo-skip` to `--undo-last-skip`?
Also, further reduce the scope to only allow `--undo-last-skip` during
a ongoing rebase, not caring about a finished one?

But, this could be so niche that I have doubts if this would ever be used;

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

[Johannes Schindelin]

Hi Sergey,

On Tue, 13 Feb 2018, Sergey Organov wrote:

> Johannes Schindelin  writes:
> >
> > On Mon, 12 Feb 2018, Sergey Organov wrote:
> >
> >> Johannes Schindelin  writes:
> >> >
> >> > On Fri, 9 Feb 2018, Sergey Organov wrote:
> >> >
> >> >> Johannes Schindelin  writes:
> >> >> 
> >> >> [...]
> >> >> 
> >> >> > 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`.
> >> >> 
> >> >> This doesn't explain why you introduced this new --recreate-merges. Why
> >> >> didn't you rather fix --preserve-merges to generate and use new todo
> >> >> list format?
> >> >
> >> > Because that would of course break existing users of
> >> > --preserve-merges.
> >> 
> >> How exactly?
> >
> > Power users of interactive rebase use scripting to augment Git's
> > functionality. One particularly powerful trick is to override
> > GIT_SEQUENCER_EDITOR with an invocation of such a script, to perform
> > automated edits. Such a script breaks when we change the format of the
> > content to edit. If we change the format of the todo list generated in
> > --preserve-merges mode, that is exactly what happens. We break existing
> > users.
> 
> I didn't say a word against "--preserve-merges mode", whatever it is,
> only about re-using "--preserve-merges" command-line option to "git
> rebase", the git user interface.

*I* said something against --preserve-merges. You did not even need to. I
know fully well its limitations.

I also said something agains the suggestion to replace the functionality
of a previously well-defined (although misdesigned) feature.

I do not know how often I have to repeat that your suggestion would break
backwards-compatibility?

> I'm sure you see the difference?

Yes, of course I do, and you do not even have to suggest otherwise by
asking such a question.

I already demonstrated plenty of times that I do understand what you wish
for, and that I see serious problems with it.

> Unless there are out-of-git scripts that do use "git rebase
> --preserve-merges" and simultaneously do rely on the todo list format
> this exact command generates, there should be no breakage of existing
> users caused by changing todo list format generated by  "git rebase
> --preserve-merges".

So. Just because you cannot imagine that anybody uses rebase in such a
powerful way means you are willing to break their setups?

Git is used by millions of users. Many of them are power users. It would
be quite naive to assume that nobody uses rebase -p in a scripted manner
that modifies the todo list.

Changing the behavior of --preserve-merges would be simply irresponsible,
and that's why we won't do it.

Even if that was not so, there is yet another really good reason not to
reuse the name --preserve-merges: The name itself suggests that this mode
is about preserving all merges in the specified commit range. That was its
original intention, too, as I never designed it to be user with rebase -i.
If the todo list of rebase -p is not modified (preserving the entire
commit topology as well as possible), it works quite well.

The new mode is not so much about preserving, though. It is about
interactively modifying the todo list, to change the order of the commits,
even to change the branch topology. That means that we do not necessarily
preserve the merges. We recreate them. So you see, I did try to be careful
about the naming, too. I thought about this.

> Old broken "--preserve-merges mode" could be then kept in the
> implementation for ages, unused by the new fixed "git rebase
> --preserve-merge", for the sake of compatibility.

This sentence contradicts itself. Either you keep the code unused, or you
keep it used for backwards-compatibility.

> > BTW it seems that you did not really read my previous reply carefully
> > because I referenced such a use case: the Git garden shears.
> 
> I thought I did. You confirm below that this script doesn't use "git
> rebase --preserve-merges" in the first place, nor will it break if "git
> rebase --preserve-merges" starts to generate new todo format, yet you
> expected I'd readily see how it's relevant? No, I'm not that clever, nor
> am I a mind-reader.

You caught me. I am not a user of --preserve-merges. Not anymore.

Does that mean that by extension nobody is a user of that feature?

Certainly not.

And does my example of (ab-)using interactive rebase by scripting on top
of it maybe suggest that others do the same? Maybe even with
--preserve-merges? Most 

Re: git-rebase --undo-skip proposal

[Jacob Keller]

On Wed, Feb 14, 2018 at 4:53 PM, Johannes Schindelin
 wrote:
> Hi,
>
> On Wed, 14 Feb 2018, Psidium Guajava wrote:
>
>> On 2018-02-13 18:42 GMT-02:00 Stefan Beller  wrote:
>> > On Tue, Feb 13, 2018 at 12:22 PM, Psidium Guajava  
>> > wrote:
>> > I think this could also be done with "git rebase --edit-todo", which brings
>> > up the right file in your editor.
>>
>> Yeah that'd would only work if one started a rebase as a interactive
>> one, not am or merge.
>
> I agree that the original proposal was clearly for the non-interactive
> rebase (it talked about .git/rebase-apply/).
>
> The biggest problem with the feature request is not how useful it would
> be: I agree it would be useful. The biggest problem is that it is a little
> bit of an ill-defined problem.
>
> Imagine that you are rebasing 30 patches. Now, let's assume that patch #7
> causes a merge conflict, and you mistakenly call `git rebase --skip`.
>
> Now, when is the next possible time you can call `git rebase --undo-skip`?
> It could be after a merge conflict in #8. Or in interactive rebase, after
> a `pick` that was turned into an `edit`. Or, and this is also entirely
> possible, after the rebase finished with #30 without problems and the
> rebase is actually no longer in progress.
>
> So I do not think that there is a way, in general, to implement this
> feature. Even if you try to remember the state where a `--skip` was
> called, you would remember it in the .git/rebase-apply/ (or
> .git/rebase-merge/) directory, which is cleaned up after the rebase
> concluded successfully. So even then the information required to implement
> the feature would not necessarily be there, still, when it would be needed.
>
> Ciao,
> Johannes

Instead of an "--undo-skip", what if we ask the question of what the
user actually wants?

Generally I'd assume that the user wishes to go back to the rebase and
"pick" the commit back in.

So what if we just make "git rebase --skip" more verbose so that it
more clearly spells out which commit is being skipped? Possibly even
as extra lines of "the following patches were skipped during the
rebase" after it completes?

Then it's up to the user to determine what to do with those commits,
and there are many tools they could use to solve it, "git rebase -i,
git cherry-pick, git reflog to restore to a previous and re-run the
rebase, etc".

Thanks,
Jake

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

[Johannes Schindelin]

Hi Jake,

On Tue, 13 Feb 2018, Jacob Keller wrote:

> On Mon, Feb 12, 2018 at 11:15 PM, Sergey Organov  wrote:
> >
> > Jacob Keller  writes:
> >
> >> On Mon, Feb 12, 2018 at 12:39 PM, Johannes Schindelin
> >>  wrote:
> >>>
> >>> On Mon, 12 Feb 2018, Sergey Organov wrote:
>  > Have a look at https://github.com/git/git/pull/447, especially the
>  > latest commit in there which is an early version of the deprecation I
>  > intend to bring about.
> 
>  You shouldn't want a deprecation at all should you have re-used
>  --preserve-merges in the first place, and I still don't see why you
>  haven't.
> >>>
> >>> Keep repeating it, and it won't become truer.
> >>>
> >>> If you break formats, you break scripts. Git has *so* many users, there
> >>> are very likely some who script *every* part of it.
> >>>
> >>> We simply cannot do that.
> >>>
> >>> What we can is deprecate designs which we learned on the way were not only
> >>> incomplete from the get-go, but bad overall and hard (or impossible) to
> >>> fix. Like --preserve-merges.
> >>>
> >>> Or for that matter like the design you proposed, to use --first-parent for
> >>> --recreate-merges. Or to use --first-parent for some --recreate-merges,
> >>> surprising users in very bad ways when it is not used (or when it is
> >>> used). I get the impression that you still think it would be a good idea,
> >>> even if it should be obvious that it is not.
> >>
> >> If we consider the addition of new todo list elements as "user
> >> breaking", then yes this change would be user-script breaking.
> >
> > It _is_ user script breaking, provided such script exists. Has anybody
> > actually seen one? Not that it's wrong to be extra-cautious about it,
> > just curios. Note that to be actually affected, such a script must
> > invoke "git rebase -p" _command_ and then tweak its todo output to
> > produce outcome.
> >
> >> Since we did not originally spell out that todo-list items are subject
> >> to enhancement by addition of operations in the future, scripts are
> >> likely not designed to allow addition of new elements.
> >
> > Out of curiosity, are you going to spell it now, for the new todo
> > format?
> >
> >> Thus, adding recreate-merges, and deprecating preserve-merges, seems
> >> to me to be the correct action to take here.
> >
> > Yes, sure, provided there is actual breakage, or at least informed
> > suspicion there is one.
> >
> >> One could argue that users should have expected new todo list elements
> >> to be added in the future and thus design their scripts to cope with
> >> such a thing. If you can convincingly argue this, then I don't
> >> necessarily see it as a complete user breaking change to fix
> >> preserve-merges in order to allow it to handle re-ordering properly..
> >
> > I'd not argue this way myself. If there are out-of-git-tree non-human
> > users that accept and tweak todo _generated_ by current "git rebase -p"
> > _command_, I also vote for a new option.
> >
> 
> To be fair, I have not seen anything that actually reads the todo list
> and tweaks it in such a manner. The closest example is the git garden
> shears script, which simply replaces the todo list.
> 
> It's certainly *possible* that such a script would exist though,

We actually know of such scripts.

Remember how rewriting parts of rebase -i in C broke somebody's script
because the todo list was not re-read after a successful `exec`?

Guess three times why that script was broken? Precisely: it modified the
todo list!

To see the fix (and the explanation) in all its glory, just have a look at
54fd3243dae (rebase -i: reread the todo list if `exec` touched it,
2017-04-26).

And even if we did not know about any user. What does that mean? Does it
mean that there is no such user? Or does it not rather mean that our
imagination is rather limited, but we *still* should practice safe
software development and use the totally appropriate vehicle of
deprecating, rather than replacing, functionality?

Obviously, the latter option is what I favor, that's why I suggested it in
the first place.

Ciao,
Dscho

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

[Johannes Schindelin]

Hi,

On Tue, 13 Feb 2018, Sergey Organov wrote:

> Johannes Schindelin  writes:
> 
> > The wording is poor either way, but you are also not a native speaker so
> > we have to rely on, say, Eric to help us out here.
> 
> Likely, but why didn't you keep original wording from --preserve-merges?
> Do you feel it's somehow poor either?

Yes, I felt it is poor, especially when --recreate-merges is present, that
is indeed why I changed it.

Ciao,
Johannes

Re: git-rebase --undo-skip proposal

[Johannes Schindelin]

Hi,

On Wed, 14 Feb 2018, Psidium Guajava wrote:

> On 2018-02-13 18:42 GMT-02:00 Stefan Beller  wrote:
> > On Tue, Feb 13, 2018 at 12:22 PM, Psidium Guajava  
> > wrote:
> > I think this could also be done with "git rebase --edit-todo", which brings
> > up the right file in your editor.
> 
> Yeah that'd would only work if one started a rebase as a interactive
> one, not am or merge.

I agree that the original proposal was clearly for the non-interactive
rebase (it talked about .git/rebase-apply/).

The biggest problem with the feature request is not how useful it would
be: I agree it would be useful. The biggest problem is that it is a little
bit of an ill-defined problem.

Imagine that you are rebasing 30 patches. Now, let's assume that patch #7
causes a merge conflict, and you mistakenly call `git rebase --skip`.

Now, when is the next possible time you can call `git rebase --undo-skip`?
It could be after a merge conflict in #8. Or in interactive rebase, after
a `pick` that was turned into an `edit`. Or, and this is also entirely
possible, after the rebase finished with #30 without problems and the
rebase is actually no longer in progress.

So I do not think that there is a way, in general, to implement this
feature. Even if you try to remember the state where a `--skip` was
called, you would remember it in the .git/rebase-apply/ (or
.git/rebase-merge/) directory, which is cleaned up after the rebase
concluded successfully. So even then the information required to implement
the feature would not necessarily be there, still, when it would be needed.

Ciao,
Johannes

Re: git-rebase --undo-skip proposal

[Psidium Guajava]

On 2018-02-13 18:42 GMT-02:00 Stefan Beller  wrote:
> On Tue, Feb 13, 2018 at 12:22 PM, Psidium Guajava  wrote:
> I think this could also be done with "git rebase --edit-todo", which brings
> up the right file in your editor.

Yeah that'd would only work if one started a rebase as a interactive
one, not am or merge.

> cc'd Paul Tan, maybe he recalls the situation.

>From what I've found on the archive, he didn't recently answer mails
that come from the mail list, I could be wrong tho.

[PATCH 1/2] apply: demonstrate a problem applying svn diffs

[Johannes Schindelin]

Subversion generates diffs that contain funny ---/+++ lines containing
more than just the file names. Example:

--- a/trunk/README  (revision 4711)
+++ /dev/null   (nonexistent)

Let's add a test case demonstrating that apply cannot handle the
/dev/null line (although it can handle the trunk/README line just fine).

Reported in https://github.com/git-for-windows/git/issues/1489

Signed-off-by: Johannes Schindelin 
---
 t/t4135-apply-weird-filenames.sh | 17 +
 1 file changed, 17 insertions(+)

diff --git a/t/t4135-apply-weird-filenames.sh b/t/t4135-apply-weird-filenames.sh
index 27cb0009fb1..b14b8085786 100755
--- a/t/t4135-apply-weird-filenames.sh
+++ b/t/t4135-apply-weird-filenames.sh
@@ -89,4 +89,21 @@ test_expect_success 'traditional, whitespace-damaged, colon 
in timezone' '
test_cmp expected "post image.txt"
 '
 
+cat >diff-from-svn <<\EOF
+Index: Makefile
+===
+diff --git a/branches/Makefile
+deleted file mode 100644
+--- a/branches/Makefile(revision 13)
 /dev/null  (nonexistent)
+@@ +1 0,0 @@
+-
+EOF
+
+test_expect_failure 'apply handles a diff generated by Subversion' '
+   >Makefile &&
+   git apply -p2 diff-from-svn &&
+   test_path_is_missing Makefile
+'
+
 test_done
-- 
2.16.1.windows.1

[PATCH 2/2] apply: handle Subversion diffs with /dev/null gracefully

[Johannes Schindelin]

From: Tatyana Krasnukha 

Subversion generates diffs that can contain lines like this one:

--- /dev/null  (nonexistent)

Let's teach Git's apply machinery to handle such a line gracefully.

This fixes https://github.com/git-for-windows/git/isues/1489

Signed-off-by: Tatyana Krasnukha 
Signed-off-by: Johannes Schindelin 
---
 apply.c  | 2 +-
 t/t4135-apply-weird-filenames.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/apply.c b/apply.c
index f8b67bfee2c..107aa4c216e 100644
--- a/apply.c
+++ b/apply.c
@@ -950,7 +950,7 @@ static int gitdiff_verify_name(struct apply_state *state,
}
free(another);
} else {
-   if (!starts_with(line, "/dev/null\n"))
+   if (!is_dev_null(line))
return error(_("git apply: bad git-diff - expected 
/dev/null on line %d"), state->linenr);
}
 
diff --git a/t/t4135-apply-weird-filenames.sh b/t/t4135-apply-weird-filenames.sh
index b14b8085786..c7c688fcc4b 100755
--- a/t/t4135-apply-weird-filenames.sh
+++ b/t/t4135-apply-weird-filenames.sh
@@ -100,7 +100,7 @@ deleted file mode 100644
 -
 EOF
 
-test_expect_failure 'apply handles a diff generated by Subversion' '
+test_expect_success 'apply handles a diff generated by Subversion' '
>Makefile &&
git apply -p2 diff-from-svn &&
test_path_is_missing Makefile
-- 
2.16.1.windows.1

[PATCH 0/2] Teach `git apply` to accept Subversion-generated diffs

[Johannes Schindelin]

They are accepted already, almost. With one exception: the ---/+++ lines
contain not only the file names, but continue with a TAB and then the
revision (or "working copy" or "nonexistent") in parenthesis.

We already handle these lines, except in the case of /dev/null. Let's handle
the case gracefully where the diff contains a "--- /dev/null" line with a
trailing comment.

This contribution came in via Pull Request to Git for Windows, from a first
time contributor! Yes!


Johannes Schindelin (1):
  apply: demonstrate a problem applying svn diffs

Tatyana Krasnukha (1):
  apply: handle Subversion diffs with /dev/null gracefully

 apply.c  |  2 +-
 t/t4135-apply-weird-filenames.sh | 17 +
 2 files changed, 18 insertions(+), 1 deletion(-)


base-commit: b2e45c695d09f6a31ce09347ae0a5d2cdfe9dd4e
Published-As: https://github.com/dscho/git/releases/tag/apply-svn-diff-v1
Fetch-It-Via: git fetch https://github.com/dscho/git apply-svn-diff-v1
-- 
2.16.1.windows.1

[PATCH 0/1] git status docs.

[Stefan Beller]

A user complained about the inaccurate documentation of `git-status 
--porcelain`.
Fix the table showing the states.

I found e92e9cd3c3 (Documentation improvements for the description
of short format., 2010-04-23) which also linked to 
https://public-inbox.org/git/20100410040959.ga11...@coredump.intra.peff.net/

Stefan Beller (1):
  Documentation/git-status: clarify status table for porcelain mode

 Documentation/git-status.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.16.1.291.g4437f3f132-goog

[PATCH 1/1] Documentation/git-status: clarify status table for porcelain mode

[Stefan Beller]

It is possible to have the output ' A' from 'git status --porcelain'
by adding a file using the '--intend-to-add' flag.  Make this clear by
adding the pattern in the table of the documentation.

However the mode 'DM' (deleted in the index, modified in the working tree)
is not possible in the non-merge case in which the file only shows
as 'D ' (and adding it back to the worktree would show an additional line
of an '??' untracked file). It is also not possible in the merge case as
then the mode involves a 'U' on one side of the merge.
Remove that pattern.

Reported-by: Ross Light 
Signed-off-by: Stefan Beller 
---
 Documentation/git-status.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 72bfb87f66..de69035cca 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -184,10 +184,10 @@ in which case `XY` are `!!`.
 
 X  Y Meaning
 -
-  [MD]   not updated
+ [AMD]   not updated
 M[ MD]   updated in index
 A[ MD]   added to index
-D [ M]   deleted from index
+Ddeleted from index
 R[ MD]   renamed in index
 C[ MD]   copied in index
 [MARC]   index and work tree matches
-- 
2.16.1.291.g4437f3f132-goog

Re: [PATCH 8/8] perl: hard-depend on the File::{Temp,Spec} modules

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -1324,8 +1324,9 @@ sub _temp_cache {
>  }
>  
>  sub _verify_require {
> - eval { require File::Temp; require File::Spec; };
> - $@ and throw Error::Simple($@);
> + require File::Temp;
> + require File::Spec;
> + return;

Same question as in the other patches: any reason not to simplify by
using 'use' at the top of the file instead?

Thanks,
Jonathan

Re: [PATCH 7/8] gitweb: hard-depend on the Digest::MD5 5.8 module

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
>  gitweb/INSTALL |  3 +--
>  gitweb/gitweb.perl | 17 +
>  2 files changed, 6 insertions(+), 14 deletions(-)

Makes sense, and I like the diffstat.

[...]
> +++ b/gitweb/gitweb.perl
[...]
> @@ -1166,18 +1165,11 @@ sub configure_gitweb_features {
>   our @snapshot_fmts = gitweb_get_feature('snapshot');
>   @snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);
>  
> - # check that the avatar feature is set to a known provider name,
> - # and for each provider check if the dependencies are satisfied.
> - # if the provider name is invalid or the dependencies are not met,
> - # reset $git_avatar to the empty string.
> + # check that the avatar feature is set to a known provider
> + # name, if the provider name is invalid, reset $git_avatar to
> + # the empty string.

Comma splice.  Formatting as sentences would make this easier to read,
anyway:

# Check that the avatar feature is set to a known provider name.
# If the provider name is invalid, reset $git_avatar to the empty
# string.

Even better would be to remove the comment altogether.  The code is
clear enough on its own and the comment should not be needed now that
it is a one-liner.

[...]
> @@ -2165,6 +2157,7 @@ sub picon_url {
>  sub gravatar_url {
>   my $email = lc shift;
>   my $size = shift;
> + require Digest::MD5;

Same question as the previous patch: how do we decide whether to 'use'
or to 'require' in cases like this?

Thanks,
Jonathan

Re: [PATCH 6/8] git-send-email: unconditionally use Net::{SMTP,Domain}

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> The Net::SMTP and Net::Domain were both first released with perl
> v5.7.3, since my d48b284183 ("perl: bump the required Perl version to
> 5.8 from 5.6.[21]", 2010-09-24) we've depended on 5.8, so there's no
> reason to conditionally require this anymore.
>
> This conditional loading was initially added in
> 87840620fd ("send-email: only 'require' instead of 'use' Net::SMTP",
> 2006-06-01) for Net::SMTP and 134550fe21 ("git-send-email.perl - try
> to give real name of the calling host to HELO/EHLO", 2010-03-14) for
> Net::Domain, both of which predate the hard dependency on 5.8.
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
>  git-send-email.perl | 24 +++-
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 85bb6482f2..69bd443245 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1143,10 +1143,9 @@ sub valid_fqdn {
>  sub maildomain_net {
>   my $maildomain;
>  
> - if (eval { require Net::Domain; 1 }) {
> - my $domain = Net::Domain::domainname();
> - $maildomain = $domain if valid_fqdn($domain);
> - }
> + require Net::Domain;
> + my $domain = Net::Domain::domainname();
> + $maildomain = $domain if valid_fqdn($domain);

Now that we indeed require the module, any reason not to 'use' it?
E.g. is it particularly expensive to load?

I haven't checked the assertions above about minimal perl versions
including these modules, but I assume they're true. :)  So this looks
like a good change.

Thanks,
Jonathan

Re: [PATCH 5/8] perl: update our copy of Mail::Address

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> Update our copy of Mail::Address from 2.19 (Aug 22, 2017) to 2.20 (Jan
> 23, 2018). This should be a trivial update[1] but it seems the version
> Matthieu Moy imported in bd869f67b9 ("send-email: add and use a local
> copy of Mail::Address", 2018-01-05) doesn't correspond to any 2.19
> version found on the CPAN. From the comment at the top of the file it
> looks like some OS version with the POD stripped, and with different
> indentation.

Were there changes other than the POD stripping?

> Let's instead use the upstream version as-is, and without copyright
> notices stripped. Like Error.pm this doesn't cleanly pass --check, so
> add a .gitattributes file to ignore the errors.
>
> 1. 
> https://metacpan.org/diff/file?target=MARKOV/MailTools-2.20/lib%2FMail%2FAddress.pod=MARKOV%2FMailTools-2.19%2Flib%2FMail%2FAddress.pod
>
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
>  perl/Git/FromCPAN/Mail/.gitattributes |   1 +
>  perl/Git/FromCPAN/Mail/Address.pm | 436 
> +-
>  2 files changed, 163 insertions(+), 274 deletions(-)
>  create mode 100644 perl/Git/FromCPAN/Mail/.gitattributes

Yikes re the stripped POD with license notice. Thanks for fixing it.

Reviewed-by: Jonathan Nieder 

Re: [PATCH 4/8] perl: update our ancient copy of Error.pm

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> The Error.pm shipped with Git as a fallback if there was no Error.pm
> on the system was released in April 2006, there's been dozens of
> releases since then, the latest at August 7, 2017, let's update to
> that.

Comma splices:
 s/, there's/. There's/
 s/, let's/. Let's/

The one piece of information I was curious about that this (quite clear,
thank you) commit message is missing is what changed in the intervening
time.  Is this just about keeping up with upstream to make it easy to
keep up later, or has upstream made any useful changes?  E.g. did any
API or behaviors get better?

Related: do we have to worry about in-tree users taking advantage of
improved API and packagers forgetting to add a dependency on the new
version?  Do we declare the minimal required Error.pm version somewhere
(e.g. in the INSTALL file)?

[...]
>  perl/Git/FromCPAN/.gitattributes |   1 +
>  perl/Git/FromCPAN/Error.pm   | 296 
> +--
>  2 files changed, 256 insertions(+), 41 deletions(-)
>  create mode 100644 perl/Git/FromCPAN/.gitattributes

Most of the added lines are documentation, so this diffstat doesn't look
half-bad.

Thanks for writing it.

Jonathan

Re: [PATCH 3/8] perl: generalize the Git::LoadCPAN facility

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> Change the two wrappers to load from CPAN (local OS) or our own copy
> to do so via the same codepath.

nit: I think with s/to load/that load/ this will be easier to read.

> I added the Error.pm wrapper in 20d2a30f8f ("Makefile: replace
> perl/Makefile.PL with simple make rules", 2017-12-10), and shortly
> afterwards Matthieu Moy added a wrapper for Mail::Address in
> bd869f67b9 ("send-email: add and use a local copy of Mail::Address",
> 2018-01-05).
>
> His was simpler since Mail::Address doesn't have an "import" method,
> but didn't do the same sanity checking, e.g. a missing FromCPAN
> directory (which OS packages are likely not to have) wouldn't be
> explicitly warned about.

I'm having trouble parsing this.  Mail::Address didn't do the same
sanity checking or his didn't?

The comma before e.g. should be a period or semicolon, since it's
starting a new independent clause.

> Now both use a modification of the previously Error.pm-specific
> codepath, which has been amended to take the module to load as
> parameter, as well as whether or not that module has an import method.

Does "now" mean before this patch or after this patch?  Usually
commit messages describe the status quo without the patch in the
present tense and the change the patch will make in the imperative.
So this could say:

Update both to use a common implementation based on the previous
Error.pm loader.

[...]
> +++ b/perl/Git/LoadCPAN.pm
> @@ -0,0 +1,74 @@
[...]
> +The Perl code in Git depends on some modules from the CPAN, but we
> +don't want to make those a hard requirement for anyone building from
> +source.

not about this patch: have we considered making it a hard requirement?
Both Mail::Address and Error.pm are pretty widely available, and I
wonder if we could make the instructions in the INSTALL file say that
they are required dependencies to simplify things.

I admit part of my bias here is coming from the distro world, where we
have to do extra work to get rid of the FromCPAN embedded copies and
would be happier not to have to.

[...]
> +Usually OS's will not ship with Git's Git::FromCPAN tree at all,
> +preferring to use their own packaging of CPAN modules instead.

nit: I think the plural of OS is OSes, or something like
"distributors" or "operating systems".

[...]
> +eval {
> + require $package_pm;
> + 1;
> +} or do {

also not about this patch: this mixed tabs/spacing formatting feels a
bit unusual.  I don't know if it's idiomatic for perl, and if it is
then no complaints; it just stood out a little.  Can
Documentation/CodingGuidelines say something about the preferred
indentation in Perl to avoid having to think about such questions?

> --- a/perl/Git/LoadCPAN/Error.pm
> +++ b/perl/Git/LoadCPAN/Error.pm
> @@ -2,45 +2,9 @@ package Git::LoadCPAN::Error;
>  use 5.008;
>  use strict;
>  use warnings;
> +use Git::LoadCPAN (
> +module => 'Error',
> +import => 1,
> +);

Nice!

Thanks and hope that helps,
Jonathan

Re: [PATCH 1/2] parse-options: expand $HOME on filename options

[Ævar Arnfjörð Bjarmason]

On Wed, Feb 14 2018, Nguyễn Thái Ngọc Duy jotted:

> When you specify "--path ~/foo", the shell will automatically expand
> ~/foo to $HOME/foo before it's passed to git. The expansion is not done
> on "--path=~/foo". An experienced user sees the difference but it could
> still be confusing for others (especially when tab-completion still
> works on --path=~/foo).
>
> Support $HOME expansion for all filename options. There are about seven
> of them.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  parse-options.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index d265a756b5..c33f14c74e 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -38,10 +38,13 @@ static int get_arg(struct parse_opt_ctx_t *p, const 
> struct option *opt,
>
>  static void fix_filename(const char *prefix, const char **file)
>  {
> - if (!file || !*file || !prefix || is_absolute_path(*file)
> - || !strcmp("-", *file))
> + if (!file || !*file || is_absolute_path(*file) ||
> + !strcmp("-", *file))
>   return;
> - *file = prefix_filename(prefix, *file);
> + if (**file == '~')
> + *file = expand_user_path(*file, 0);
> + else if (prefix)
> + *file = prefix_filename(prefix, *file);
>  }
>
>  static int opt_command_mode_error(const struct option *opt,

On current versions of git:

(
mkdir '/tmp/~' &&
cd /tmp &&
touch '~/foo' &&
git init --template=~
)

Will create a /tmp/.git with a 'foo' file, whereas now it'll expand ~ to
$HOME. Since this changes the behavior of this and presumably some other
options it seems like something we should have a test for.

In general I'm mildly negative on adding this, for every user like Doron
who'll be less confused by a hack like this, you'll have other users
who'll be confused about git inexplicably working with ~ in the middle
of strings, even though;

$ echo git init --template ~/path
git init --template /home/avar/path
$ echo git init --template=~/path
git init --template=~/path

I think it makes more sense to just leave such expansion to the shell,
and not try to magically expand it after the fact, since it's both
confusing (user: why does this work with git and not this other
program?), and as shown above changes existing semantics.

We'll also be setting ourselves up for more disappointed users who'll
notice that e.g. `git clone file://~/path` doesn't work, but `git clone
file://$HOME/path` does, requiring more hacks to expand ~ in more
codepaths. Will they also expact `git log -G~` to find references to
their homedir in their dotfiles.git?

I think this way lies madness, and it's better to just avoid it.

But I think that if we're going to keep it it needs some tests & docs to
point confused users to.

Re: [PATCH 2/8] perl: move CPAN loader wrappers to another namespace

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> Move the Git::Error and Git::Mail::Address wrappers to the
> Git::LoadCPAN::Loader::* namespace, e.g. Git::LoadCPAN::Error, that
> module will then either load Error from CPAN (if installed on the OS),
> or use Git::FromCPAN::Error.
[...]
> This also makes things a bit less confusing since there was already a
> Git::Error namespace ever since 8b9150e3e3 ("Git.pm: Handle failed
> commands' output", 2006-06-24).
>
> Signed-off-by: Ævar Arnfjörð Bjarmason 

Makes sense.

Reviewed-by: Jonathan Nieder 

Re: [PATCH 1/8] perl: *.pm files should not have the executable bit

[Jonathan Nieder]

Hi,

Ævar Arnfjörð Bjarmason wrote:

> The Git::Mail::Address file added in bd869f67b9 ("send-email: add and
> use a local copy of Mail::Address", 2018-01-05) had the executable bit
> set, this should not be the case with *.pm files, it breaks nothing,
> but is redundant and confusing as none of the other files have it, and
> it's never executed as a stand-alone program.

Needs a period somewhere to break up the long sentence with comma
splices.  How about:

The Git::Mail::Address file added in bd869f67b9 ("send-email: add and
use a local copy of Mail::Address", 2018-01-05) had the executable bit
set. That bit should not be set for *.pm files. It breaks nothing but
but it is redundant and confusing as none of the other files have it
and these files are never executed as stand-alone programs.

> Signed-off-by: Ævar Arnfjörð Bjarmason 

With or without such a tweak,
Reviewed-by: Jonathan Nieder 

Thanks.

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

[Stefan Beller]

On Wed, Feb 14, 2018 at 10:59 AM, Brandon Williams  wrote:
> One person was interested enough for me to go back through and also
> rename all the paired 'old' variables to match the new names for the
> variables which were named 'new'.

The patches are:
Reviewed-by: Stefan Beller 
(apart from the 'this' nit Junio had)

[PATCH 8/8] perl: hard-depend on the File::{Temp,Spec} modules

[Ævar Arnfjörð Bjarmason]

Since my d48b284183 ("perl: bump the required Perl version to 5.8 from
5.6.[21]", 2010-09-24), we've depended on 5.8, so there's no reason to
conditionally require File::Temp and File::Spec anymore. They were
first released with perl versions v5.6.1 and 5.00405, respectively.

This code was originally added in c14c8ceb13 ("Git.pm: Make File::Spec
and File::Temp requirement lazy", 2008-08-15), presumably to make
Git.pm work on 5.6.0.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 perl/Git.pm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 8e02ee2cca..221e827e83 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1324,8 +1324,9 @@ sub _temp_cache {
 }
 
 sub _verify_require {
-   eval { require File::Temp; require File::Spec; };
-   $@ and throw Error::Simple($@);
+   require File::Temp;
+   require File::Spec;
+   return;
 }
 
 =item temp_reset ( FILEHANDLE )
-- 
2.15.1.424.g9478a66081

[PATCH 7/8] gitweb: hard-depend on the Digest::MD5 5.8 module

[Ævar Arnfjörð Bjarmason]

Since my d48b284183 ("perl: bump the required Perl version to 5.8 from
5.6.[21]", 2010-09-24), we've depended on 5.8, so there's no reason to
conditionally require Digest::MD5 anymore. It was released with perl
v5.7.3.

The initial introduction of the dependency in
e9fdd74e53 ("gitweb: (gr)avatar support", 2009-06-30) says as much,
this also undoes part of the later 2e9c8789b7 ("gitweb: Mention
optional Perl modules in INSTALL", 2011-02-04) since gitweb will
always be run on at least 5.8, so there's no need to mention
Digest::MD5 as a required module in the documentation, let's instead
say that we require perl 5.8.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 gitweb/INSTALL |  3 +--
 gitweb/gitweb.perl | 17 +
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/gitweb/INSTALL b/gitweb/INSTALL
index 408f2859d3..a58e6b3c44 100644
--- a/gitweb/INSTALL
+++ b/gitweb/INSTALL
@@ -29,12 +29,11 @@ Requirements
 
 
  - Core git tools
- - Perl
+ - Perl 5.8
  - Perl modules: CGI, Encode, Fcntl, File::Find, File::Basename.
  - web server
 
 The following optional Perl modules are required for extra features
- - Digest::MD5 - for gravatar support
  - CGI::Fast and FCGI - for running gitweb as FastCGI script
  - HTML::TagCloud - for fancy tag cloud in project list view
  - HTTP::Date or Time::ParseDate - to support If-Modified-Since for feeds
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 2417057f2b..8f48e3c02e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -490,7 +490,6 @@ our %feature = (
# Currently available providers are gravatar and picon.
# If an unknown provider is specified, the feature is disabled.
 
-   # Gravatar depends on Digest::MD5.
# Picon currently relies on the indiana.edu database.
 
# To enable system wide have in $GITWEB_CONFIG
@@ -1166,18 +1165,11 @@ sub configure_gitweb_features {
our @snapshot_fmts = gitweb_get_feature('snapshot');
@snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);
 
-   # check that the avatar feature is set to a known provider name,
-   # and for each provider check if the dependencies are satisfied.
-   # if the provider name is invalid or the dependencies are not met,
-   # reset $git_avatar to the empty string.
+   # check that the avatar feature is set to a known provider
+   # name, if the provider name is invalid, reset $git_avatar to
+   # the empty string.
our ($git_avatar) = gitweb_get_feature('avatar');
-   if ($git_avatar eq 'gravatar') {
-   $git_avatar = '' unless (eval { require Digest::MD5; 1; });
-   } elsif ($git_avatar eq 'picon') {
-   # no dependencies
-   } else {
-   $git_avatar = '';
-   }
+   $git_avatar = '' unless $git_avatar =~ /^(?:gravatar|picon)$/s;
 
our @extra_branch_refs = gitweb_get_feature('extra-branch-refs');
@extra_branch_refs = filter_and_validate_refs (@extra_branch_refs);
@@ -2165,6 +2157,7 @@ sub picon_url {
 sub gravatar_url {
my $email = lc shift;
my $size = shift;
+   require Digest::MD5;
$avatar_cache{$email} ||=
"//www.gravatar.com/avatar/" .
Digest::MD5::md5_hex($email) . "?s=";
-- 
2.15.1.424.g9478a66081

[PATCH 6/8] git-send-email: unconditionally use Net::{SMTP,Domain}

[Ævar Arnfjörð Bjarmason]

The Net::SMTP and Net::Domain were both first released with perl
v5.7.3, since my d48b284183 ("perl: bump the required Perl version to
5.8 from 5.6.[21]", 2010-09-24) we've depended on 5.8, so there's no
reason to conditionally require this anymore.

This conditional loading was initially added in
87840620fd ("send-email: only 'require' instead of 'use' Net::SMTP",
2006-06-01) for Net::SMTP and 134550fe21 ("git-send-email.perl - try
to give real name of the calling host to HELO/EHLO", 2010-03-14) for
Net::Domain, both of which predate the hard dependency on 5.8.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 git-send-email.perl | 24 +++-
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 85bb6482f2..69bd443245 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1143,10 +1143,9 @@ sub valid_fqdn {
 sub maildomain_net {
my $maildomain;
 
-   if (eval { require Net::Domain; 1 }) {
-   my $domain = Net::Domain::domainname();
-   $maildomain = $domain if valid_fqdn($domain);
-   }
+   require Net::Domain;
+   my $domain = Net::Domain::domainname();
+   $maildomain = $domain if valid_fqdn($domain);
 
return $maildomain;
 }
@@ -1154,17 +1153,16 @@ sub maildomain_net {
 sub maildomain_mta {
my $maildomain;
 
-   if (eval { require Net::SMTP; 1 }) {
-   for my $host (qw(mailhost localhost)) {
-   my $smtp = Net::SMTP->new($host);
-   if (defined $smtp) {
-   my $domain = $smtp->domain;
-   $smtp->quit;
+   require Net::SMTP;
+   for my $host (qw(mailhost localhost)) {
+   my $smtp = Net::SMTP->new($host);
+   if (defined $smtp) {
+   my $domain = $smtp->domain;
+   $smtp->quit;
 
-   $maildomain = $domain if valid_fqdn($domain);
+   $maildomain = $domain if valid_fqdn($domain);
 
-   last if $maildomain;
-   }
+   last if $maildomain;
}
}
 
-- 
2.15.1.424.g9478a66081

[PATCH 5/8] perl: update our copy of Mail::Address

[Ævar Arnfjörð Bjarmason]

Update our copy of Mail::Address from 2.19 (Aug 22, 2017) to 2.20 (Jan
23, 2018). This should be a trivial update[1] but it seems the version
Matthieu Moy imported in bd869f67b9 ("send-email: add and use a local
copy of Mail::Address", 2018-01-05) doesn't correspond to any 2.19
version found on the CPAN. From the comment at the top of the file it
looks like some OS version with the POD stripped, and with different
indentation.

Let's instead use the upstream version as-is, and without copyright
notices stripped. Like Error.pm this doesn't cleanly pass --check, so
add a .gitattributes file to ignore the errors.

1. 
https://metacpan.org/diff/file?target=MARKOV/MailTools-2.20/lib%2FMail%2FAddress.pod=MARKOV%2FMailTools-2.19%2Flib%2FMail%2FAddress.pod

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 perl/Git/FromCPAN/Mail/.gitattributes |   1 +
 perl/Git/FromCPAN/Mail/Address.pm | 436 +-
 2 files changed, 163 insertions(+), 274 deletions(-)
 create mode 100644 perl/Git/FromCPAN/Mail/.gitattributes

diff --git a/perl/Git/FromCPAN/Mail/.gitattributes 
b/perl/Git/FromCPAN/Mail/.gitattributes
new file mode 100644
index 00..94f3e5bb86
--- /dev/null
+++ b/perl/Git/FromCPAN/Mail/.gitattributes
@@ -0,0 +1 @@
+/Address.pm whitespace=-trailing-space
diff --git a/perl/Git/FromCPAN/Mail/Address.pm 
b/perl/Git/FromCPAN/Mail/Address.pm
index 13b2ff7d05..ee333e0f5a 100644
--- a/perl/Git/FromCPAN/Mail/Address.pm
+++ b/perl/Git/FromCPAN/Mail/Address.pm
@@ -1,276 +1,164 @@
-# Copyrights 1995-2017 by [Mark Overmeer ].
-#  For other contributors see ChangeLog.
-# See the manual pages for details on the licensing terms.
-# Pod stripped from pm file by OODoc 2.02.
-package Mail::Address;
-use vars '$VERSION';
-$VERSION = '2.19';
-
-use strict;
-
-use Carp;
-
-# use locale;   removed in version 1.78, because it causes taint problems
-
-sub Version { our $VERSION }
-
-
-
-# given a comment, attempt to extract a person's name
-sub _extract_name
-{   # This function can be called as method as well
-my $self = @_ && ref $_[0] ? shift : undef;
-
-local $_ = shift
-or return '';
-
-# Using encodings, too hard. See Mail::Message::Field::Full.
-return '' if m/\=\?.*?\?\=/;
-
-# trim whitespace
-s/^\s+//;
-s/\s+$//;
-s/\s+/ /;
-
-# Disregard numeric names (e.g. 123456.1...@compuserve.com)
-return "" if /^[\d ]+$/;
-
-s/^\((.*)\)$/$1/; # remove outermost parenthesis
-s/^"(.*)"$/$1/;   # remove outer quotation marks
-s/\(.*?\)//g; # remove minimal embedded comments
-s/\\//g;  # remove all escapes
-s/^"(.*)"$/$1/;   # remove internal quotation marks
-s/^([^\s]+) ?, ?(.*)$/$2 $1/; # reverse "Last, First M." if applicable
-s/,.*//;
-
-# Change casing only when the name contains only upper or only
-# lower cased characters.
-unless( m/[A-Z]/ && m/[a-z]/ )
-{   # Set the case of the name to first char upper rest lower
-s/\b(\w+)/\L\u$1/igo;  # Upcase first letter on name
-s/\bMc(\w)/Mc\u$1/igo; # Scottish names such as 'McLeod'
-s/\bo'(\w)/O'\u$1/igo; # Irish names such as 'O'Malley, O'Reilly'
-s/\b(x*(ix)?v*(iv)?i*)\b/\U$1/igo; # Roman numerals, eg 'Level III 
Support'
-}
+=encoding utf8
 
-# some cleanup
-s/\[[^\]]*\]//g;
-s/(^[\s'"]+|[\s'"]+$)//g;
-s/\s{2,}/ /g;
-
-$_;
-}
-
-sub _tokenise
-{   local $_ = join ',', @_;
-my (@words,$snippet,$field);
-
-s/\A\s+//;
-s/[\r\n]+/ /g;
-
-while ($_ ne '')
-{   $field = '';
-if(s/^\s*\(/(/ )# (...)
-{   my $depth = 0;
-
- PAREN: while(s/^(\(([^\(\)\\]|\\.)*)//)
-{   $field .= $1;
-$depth++;
-while(s/^(([^\(\)\\]|\\.)*\)\s*)//)
-{   $field .= $1;
-last PAREN unless --$depth;
-   $field .= $1 if s/^(([^\(\)\\]|\\.)+)//;
-}
-}
-
-carp "Unmatched () '$field' '$_'"
-if $depth;
-
-$field =~ s/\s+\Z//;
-push @words, $field;
-
-next;
-}
-
-if( s/^("(?:[^"\\]+|\\.)*")\s*//   # "..."
- || s/^(\[(?:[^\]\\]+|\\.)*\])\s*//# [...]
- || s/^([^\s()<>\@,;:\\".[\]]+)\s*//
- || s/^([()<>\@,;:\\".[\]])\s*//
-  )
-{   push @words, $1;
-next;
-}
-
-croak "Unrecognised line: $_";
-}
-
-push @words, ",";
-\@words;
-}
-
-sub _find_next
-{   my ($idx, $tokens, $len) = @_;
-
-while($idx < $len)
-{   my $c = $tokens->[$idx];
-return $c if $c eq ',' || $c eq ';' || $c eq '<';
-$idx++;
-}
-
-"";
-}
-
-sub _complete
-{   my ($class, $phrase, $address, $comment) = @_;
-
-@$phrase || @$comment || @$address
-   or return undef;
-
-my $o = $class->new(join(" ",@$phrase), join("",@$address), join(" 
",@$comment));
-@$phrase = 

[PATCH 4/8] perl: update our ancient copy of Error.pm

[Ævar Arnfjörð Bjarmason]

The Error.pm shipped with Git as a fallback if there was no Error.pm
on the system was released in April 2006, there's been dozens of
releases since then, the latest at August 7, 2017, let's update to
that.

This undoes a local hack we'd accumulated in 96bc4de85c ("Eliminate
Scalar::Util usage from private-Error.pm", 2006-07-26), it's been
redundant since my d48b284183 ("perl: bump the required Perl version
to 5.8 from 5.6.[21]", 2010-09-24).

This also undoes 3a51467b94 ("Typo fix: replacing it's -> its",
2013-04-13). This is the Nth time I find that some upstream code of
ours (in contrib/, in sha1dc/ and now in perl/ ...) has diverged from
upstream because of some tree-wide typo fixing. Let's not do those
fixes against upstream projects, it's more valuable that we have a 1=1
mapping to upstream than to fix typos in docs we never even generate
from this code. If someone wants to fix typos in them fine, but they
should do it with a patch to upstream which git.git can then
incorporate.

The upstream code doesn't cleanly pass a --check, so I'm adding a
.gitattributes file for similar reasons as done for sha1dc in
5d184f468e ("sha1dc: ignore indent-with-non-tab whitespace
violations", 2017-06-06).

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 perl/Git/FromCPAN/.gitattributes |   1 +
 perl/Git/FromCPAN/Error.pm   | 296 +--
 2 files changed, 256 insertions(+), 41 deletions(-)
 create mode 100644 perl/Git/FromCPAN/.gitattributes

diff --git a/perl/Git/FromCPAN/.gitattributes b/perl/Git/FromCPAN/.gitattributes
new file mode 100644
index 00..8b64fc5e22
--- /dev/null
+++ b/perl/Git/FromCPAN/.gitattributes
@@ -0,0 +1 @@
+/Error.pm whitespace=-blank-at-eof
diff --git a/perl/Git/FromCPAN/Error.pm b/perl/Git/FromCPAN/Error.pm
index 6098135ae2..f9c36e9e98 100644
--- a/perl/Git/FromCPAN/Error.pm
+++ b/perl/Git/FromCPAN/Error.pm
@@ -12,10 +12,12 @@
 package Error;
 
 use strict;
+use warnings;
+
 use vars qw($VERSION);
 use 5.004;
 
-$VERSION = "0.15009";
+$VERSION = "0.17025";
 
 use overload (
'""'   =>   'stringify',
@@ -32,21 +34,35 @@ $Error::THROWN = undef; # last error thrown, a 
workaround until die $ref works
 my $LAST;  # Last error created
 my %ERROR; # Last error associated with package
 
-sub throw_Error_Simple
+sub _throw_Error_Simple
 {
 my $args = shift;
 return Error::Simple->new($args->{'text'});
 }
 
-$Error::ObjectifyCallback = \_Error_Simple;
+$Error::ObjectifyCallback = \&_throw_Error_Simple;
 
 
 # Exported subs are defined in Error::subs
 
+use Scalar::Util ();
+
 sub import {
 shift;
+my @tags = @_;
 local $Exporter::ExportLevel = $Exporter::ExportLevel + 1;
-Error::subs->import(@_);
+
+@tags = grep {
+   if( $_ eq ':warndie' ) {
+  Error::WarnDie->import();
+  0;
+   }
+   else {
+  1;
+   }
+} @tags;
+
+Error::subs->import(@tags);
 }
 
 # I really want to use last for the name of this method, but it is a keyword
@@ -107,10 +123,6 @@ sub stacktrace {
 $text;
 }
 
-# Allow error propagation, ie
-#
-# $ber->encode(...) or
-#return Error->prior($ber)->associate($ldap);
 
 sub associate {
 my $err = shift;
@@ -130,6 +142,7 @@ sub associate {
 return;
 }
 
+
 sub new {
 my $self = shift;
 my($pkg,$file,$line) = caller($Error::Depth);
@@ -246,6 +259,10 @@ sub value {
 
 package Error::Simple;
 
+use vars qw($VERSION);
+
+$VERSION = "0.17025";
+
 @Error::Simple::ISA = qw(Error);
 
 sub new {
@@ -288,14 +305,6 @@ use vars qw(@EXPORT_OK @ISA %EXPORT_TAGS);
 
 @ISA = qw(Exporter);
 
-
-sub blessed {
-   my $item = shift;
-   local $@; # don't kill an outer $@
-   ref $item and eval { $item->can('can') };
-}
-
-
 sub run_clauses ($$$\@) {
 my($clauses,$err,$wantarray,$result) = @_;
 my $code = undef;
@@ -314,16 +323,17 @@ sub run_clauses ($$$\@) {
my $pkg = $catch->[$i];
unless(defined $pkg) {
#except
-   splice(@$catch,$i,2,$catch->[$i+1]->());
+   splice(@$catch,$i,2,$catch->[$i+1]->($err));
$i -= 2;
next CATCHLOOP;
}
-   elsif(blessed($err) && $err->isa($pkg)) {
+   elsif(Scalar::Util::blessed($err) && $err->isa($pkg)) {
$code = $catch->[$i+1];
while(1) {
my $more = 0;
-   local($Error::THROWN);
+   local($Error::THROWN, $@);
my $ok = eval {
+   $@ = $err;
if($wantarray) {
@{$result} = $code->($err,\$more);
}
@@ -341,10 +351,9 @@ sub run_clauses ($$$\@) {
undef $err;
}
else {
-   $err = 

[PATCH 1/8] perl: *.pm files should not have the executable bit

[Ævar Arnfjörð Bjarmason]

The Git::Mail::Address file added in bd869f67b9 ("send-email: add and
use a local copy of Mail::Address", 2018-01-05) had the executable bit
set, this should not be the case with *.pm files, it breaks nothing,
but is redundant and confusing as none of the other files have it, and
it's never executed as a stand-alone program.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 perl/Git/Mail/Address.pm | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 mode change 100755 => 100644 perl/Git/Mail/Address.pm

diff --git a/perl/Git/Mail/Address.pm b/perl/Git/Mail/Address.pm
old mode 100755
new mode 100644
-- 
2.15.1.424.g9478a66081

[PATCH 3/8] perl: generalize the Git::LoadCPAN facility

[Ævar Arnfjörð Bjarmason]

Change the two wrappers to load from CPAN (local OS) or our own copy
to do so via the same codepath.

I added the Error.pm wrapper in 20d2a30f8f ("Makefile: replace
perl/Makefile.PL with simple make rules", 2017-12-10), and shortly
afterwards Matthieu Moy added a wrapper for Mail::Address in
bd869f67b9 ("send-email: add and use a local copy of Mail::Address",
2018-01-05).

His was simpler since Mail::Address doesn't have an "import" method,
but didn't do the same sanity checking, e.g. a missing FromCPAN
directory (which OS packages are likely not to have) wouldn't be
explicitly warned about.

Now both use a modification of the previously Error.pm-specific
codepath, which has been amended to take the module to load as
parameter, as well as whether or not that module has an import method.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 perl/Git/LoadCPAN.pm  | 74 +++
 perl/Git/LoadCPAN/Error.pm| 44 +++
 perl/Git/LoadCPAN/Mail/Address.pm | 22 +++-
 3 files changed, 82 insertions(+), 58 deletions(-)
 create mode 100644 perl/Git/LoadCPAN.pm

diff --git a/perl/Git/LoadCPAN.pm b/perl/Git/LoadCPAN.pm
new file mode 100644
index 00..2262812654
--- /dev/null
+++ b/perl/Git/LoadCPAN.pm
@@ -0,0 +1,74 @@
+package Git::LoadCPAN;
+use 5.008;
+use strict;
+use warnings;
+
+=head1 NAME
+
+Git::LoadCPAN - Wrapper for loading modules from the CPAN (OS) or Git's own 
copy
+
+=head1 DESCRIPTION
+
+The Perl code in Git depends on some modules from the CPAN, but we
+don't want to make those a hard requirement for anyone building from
+source.
+
+Therefore the L namespace shipped with Git contains
+wrapper modules like C that will first
+attempt to load C from the OS, and if that doesn't work
+will fall back on C shipped with Git
+itself.
+
+Usually OS's will not ship with Git's Git::FromCPAN tree at all,
+preferring to use their own packaging of CPAN modules instead.
+
+This module is only intended to be used for code shipping in the
+C repository. Use it for anything else at your peril!
+
+=cut
+
+sub import {
+shift;
+my $caller = caller;
+my %args = @_;
+my $module = exists $args{module} ? delete $args{module} : die "BUG: 
Expected 'module' parameter!";
+my $import = exists $args{import} ? delete $args{import} : die "BUG: 
Expected 'import' parameter!";
+die "BUG: Too many arguments!" if keys %args;
+
+# Foo::Bar to Foo/Bar.pm
+my $package_pm = $module;
+$package_pm =~ s[::][/]g;
+$package_pm .= '.pm';
+
+eval {
+   require $package_pm;
+   1;
+} or do {
+   my $error = $@ || "Zombie Error";
+
+   my $Git_LoadCPAN_pm_path = $INC{"Git/LoadCPAN.pm"} || die "BUG: Should 
have our own path from %INC!";
+
+   require File::Basename;
+   my $Git_LoadCPAN_pm_root = 
File::Basename::dirname($Git_LoadCPAN_pm_path) || die "BUG: Can't figure out 
lib/Git dirname from '$Git_LoadCPAN_pm_path'!";
+
+   require File::Spec;
+   my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_LoadCPAN_pm_root, 
'FromCPAN');
+   die "BUG: '$Git_pm_FromCPAN_root' should be a directory!" unless -d 
$Git_pm_FromCPAN_root;
+
+   local @INC = ($Git_pm_FromCPAN_root, @INC);
+   require $package_pm;
+};
+
+if ($import) {
+   no strict 'refs';
+   *{"${caller}::import"}= sub {
+   shift;
+   use strict 'refs';
+   unshift @_, $module;
+   goto &{"${module}::import"};
+   };
+   use strict 'refs';
+}
+}
+
+1;
diff --git a/perl/Git/LoadCPAN/Error.pm b/perl/Git/LoadCPAN/Error.pm
index 3513fe745b..9ba53cccf2 100644
--- a/perl/Git/LoadCPAN/Error.pm
+++ b/perl/Git/LoadCPAN/Error.pm
@@ -2,45 +2,9 @@ package Git::LoadCPAN::Error;
 use 5.008;
 use strict;
 use warnings;
-
-=head1 NAME
-
-Git::LoadCPAN::Error - Wrapper for the L module, in case it's not 
installed
-
-=head1 DESCRIPTION
-
-Wraps the import function for the L module.
-
-This module is only intended to be used for code shipping in the
-C repository. Use it for anything else at your peril!
-
-=cut
-
-sub import {
-shift;
-my $caller = caller;
-
-eval {
-   require Error;
-   1;
-} or do {
-   my $error = $@ || "Zombie Error";
-
-   my $Git_Error_pm_path = $INC{"Git/LoadCPAN/Error.pm"} || die "BUG: 
Should have our own path from %INC!";
-
-   require File::Basename;
-   my $Git_Error_pm_root = File::Basename::dirname($Git_Error_pm_path) || 
die "BUG: Can't figure out lib/Git dirname from '$Git_Error_pm_path'!";
-
-   require File::Spec;
-   my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_Error_pm_root, '..', 
'FromCPAN');
-   die "BUG: '$Git_pm_FromCPAN_root' should be a directory!" unless -d 
$Git_pm_FromCPAN_root;
-
-   local @INC = ($Git_pm_FromCPAN_root, @INC);
-   require Error;
-};
-
-unshift @_, $caller;
-goto ::import;
-}
+use Git::LoadCPAN (
+module => 'Error',
+import 

[PATCH 2/8] perl: move CPAN loader wrappers to another namespace

[Ævar Arnfjörð Bjarmason]

Move the Git::Error and Git::Mail::Address wrappers to the
Git::LoadCPAN::Loader::* namespace, e.g. Git::LoadCPAN::Error, that
module will then either load Error from CPAN (if installed on the OS),
or use Git::FromCPAN::Error.

When I added the Error wrapper in 20d2a30f8f ("Makefile: replace
perl/Makefile.PL with simple make rules", 2017-12-10) I didn't think
about how confusing it would be to have these modules sitting in the
same tree as our normal modules. Let's put these all into
Git::{Load,From}CPAN::* to clearly distinguish them from the rest.

This also makes things a bit less confusing since there was already a
Git::Error namespace ever since 8b9150e3e3 ("Git.pm: Handle failed
commands' output", 2006-06-24).

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 contrib/examples/git-difftool.perl  | 2 +-
 git-send-email.perl | 4 ++--
 perl/Git.pm | 2 +-
 perl/Git/{ => LoadCPAN}/Error.pm| 8 
 perl/Git/{ => LoadCPAN}/Mail/Address.pm | 4 ++--
 5 files changed, 10 insertions(+), 10 deletions(-)
 rename perl/Git/{ => LoadCPAN}/Error.pm (78%)
 rename perl/Git/{ => LoadCPAN}/Mail/Address.pm (69%)

diff --git a/contrib/examples/git-difftool.perl 
b/contrib/examples/git-difftool.perl
index fb0fd0b84b..b2ea80f9ed 100755
--- a/contrib/examples/git-difftool.perl
+++ b/contrib/examples/git-difftool.perl
@@ -13,7 +13,7 @@
 use 5.008;
 use strict;
 use warnings;
-use Git::Error qw(:try);
+use Git::LoadCPAN::Error qw(:try);
 use File::Basename qw(dirname);
 use File::Copy;
 use File::Find;
diff --git a/git-send-email.perl b/git-send-email.perl
index bbf4deaa0d..85bb6482f2 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -26,11 +26,11 @@ use Text::ParseWords;
 use Term::ANSIColor;
 use File::Temp qw/ tempdir tempfile /;
 use File::Spec::Functions qw(catdir catfile);
-use Git::Error qw(:try);
+use Git::LoadCPAN::Error qw(:try);
 use Cwd qw(abs_path cwd);
 use Git;
 use Git::I18N;
-use Git::Mail::Address;
+use Git::LoadCPAN::Mail::Address;
 
 Getopt::Long::Configure qw/ pass_through /;
 
diff --git a/perl/Git.pm b/perl/Git.pm
index 9d60d7948b..8e02ee2cca 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -101,7 +101,7 @@ increase notwithstanding).
 
 
 use Carp qw(carp croak); # but croak is bad - throw instead
-use Git::Error qw(:try);
+use Git::LoadCPAN::Error qw(:try);
 use Cwd qw(abs_path cwd);
 use IPC::Open2 qw(open2);
 use Fcntl qw(SEEK_SET SEEK_CUR);
diff --git a/perl/Git/Error.pm b/perl/Git/LoadCPAN/Error.pm
similarity index 78%
rename from perl/Git/Error.pm
rename to perl/Git/LoadCPAN/Error.pm
index 09bbc97390..3513fe745b 100644
--- a/perl/Git/Error.pm
+++ b/perl/Git/LoadCPAN/Error.pm
@@ -1,11 +1,11 @@
-package Git::Error;
+package Git::LoadCPAN::Error;
 use 5.008;
 use strict;
 use warnings;
 
 =head1 NAME
 
-Git::Error - Wrapper for the L module, in case it's not installed
+Git::LoadCPAN::Error - Wrapper for the L module, in case it's not 
installed
 
 =head1 DESCRIPTION
 
@@ -26,13 +26,13 @@ sub import {
 } or do {
my $error = $@ || "Zombie Error";
 
-   my $Git_Error_pm_path = $INC{"Git/Error.pm"} || die "BUG: Should have 
our own path from %INC!";
+   my $Git_Error_pm_path = $INC{"Git/LoadCPAN/Error.pm"} || die "BUG: 
Should have our own path from %INC!";
 
require File::Basename;
my $Git_Error_pm_root = File::Basename::dirname($Git_Error_pm_path) || 
die "BUG: Can't figure out lib/Git dirname from '$Git_Error_pm_path'!";
 
require File::Spec;
-   my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_Error_pm_root, 
'FromCPAN');
+   my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_Error_pm_root, '..', 
'FromCPAN');
die "BUG: '$Git_pm_FromCPAN_root' should be a directory!" unless -d 
$Git_pm_FromCPAN_root;
 
local @INC = ($Git_pm_FromCPAN_root, @INC);
diff --git a/perl/Git/Mail/Address.pm b/perl/Git/LoadCPAN/Mail/Address.pm
similarity index 69%
rename from perl/Git/Mail/Address.pm
rename to perl/Git/LoadCPAN/Mail/Address.pm
index 2ce3e84670..879c2f5cd1 100644
--- a/perl/Git/Mail/Address.pm
+++ b/perl/Git/LoadCPAN/Mail/Address.pm
@@ -1,11 +1,11 @@
-package Git::Mail::Address;
+package Git::LoadCPAN::Mail::Address;
 use 5.008;
 use strict;
 use warnings;
 
 =head1 NAME
 
-Git::Mail::Address - Wrapper for the L module, in case it's not 
installed
+Git::LoadCPAN::Mail::Address - Wrapper for the L module, in 
case it's not installed
 
 =head1 DESCRIPTION
 
-- 
2.15.1.424.g9478a66081

[PATCH 0/8] various perl fixes

[Ævar Arnfjörð Bjarmason]

I'd been meaning to submit 3/* once my Makefile.PL removal landed in
master, but noticed a few more things along the way, including the
issue fixed in 1/* which I just noted in
,
and while I was at it resolved some of my long-standing TODOs noted in
<87d13jd4fd@evledraar.gmail.com>, and more.

If you're CC'd on this series it's because one of the commit messages
mentions a commit you authored.

[45]/* do not cleanly pass the default --check, so they have
.gitattributes files to make them pass, as noted in 4/* it's much
easier if we can just use this upstream code as-is, and not accumulate
our own typo/whitespace etc. fixes along the way, which just makes
subsequent updates harder.

Ævar Arnfjörð Bjarmason (8):
  perl: *.pm files should not have the executable bit
  perl: move CPAN loader wrappers to another namespace
  perl: generalize the Git::LoadCPAN facility
  perl: update our ancient copy of Error.pm
  perl: update our copy of Mail::Address
  git-send-email: unconditionally use Net::{SMTP,Domain}
  gitweb: hard-depend on the Digest::MD5 5.8 module
  perl: hard-depend on the File::{Temp,Spec} modules

 contrib/examples/git-difftool.perl|   2 +-
 git-send-email.perl   |  28 +--
 gitweb/INSTALL|   3 +-
 gitweb/gitweb.perl|  17 +-
 perl/Git.pm   |   7 +-
 perl/Git/Error.pm |  46 
 perl/Git/FromCPAN/.gitattributes  |   1 +
 perl/Git/FromCPAN/Error.pm| 296 +++
 perl/Git/FromCPAN/Mail/.gitattributes |   1 +
 perl/Git/FromCPAN/Mail/Address.pm | 436 +-
 perl/Git/LoadCPAN.pm  |  74 ++
 perl/Git/LoadCPAN/Error.pm|  10 +
 perl/Git/LoadCPAN/Mail/Address.pm |  10 +
 perl/Git/Mail/Address.pm  |  24 --
 14 files changed, 537 insertions(+), 418 deletions(-)
 delete mode 100644 perl/Git/Error.pm
 create mode 100644 perl/Git/FromCPAN/.gitattributes
 create mode 100644 perl/Git/FromCPAN/Mail/.gitattributes
 create mode 100644 perl/Git/LoadCPAN.pm
 create mode 100644 perl/Git/LoadCPAN/Error.pm
 create mode 100644 perl/Git/LoadCPAN/Mail/Address.pm
 delete mode 100755 perl/Git/Mail/Address.pm

-- 
2.15.1.424.g9478a66081

[PATCH] t/known-leaky: add list of known-leaky test scripts

[Martin Ågren]

Here's what a list of known leaks might look like. It feels a bit
awkward to post a known-incomplete list (I don't run all tests). Duy
offered to pick up the ball if I gave up, maybe you could complete and
post this as your own? :-? Even if I (or others) can't reproduce the
complete list locally, regressions will be trivial to find, and newly
leak-free tests fairly easy to notice.

I'm not sure if the shamelessly stolen shell snippets warrant their own
scripts, or how make targets overriding various variables would be
received. At least they're in the commit message.

-- >8 --
We have quite a lot of leaks in the code base. Using SANITIZE=leak, it
is easy to find them, and every now and then we simply stumble upon one.
Still, we can expect it to take some time to get to the point where
`make SANITIZE=leak test` succeeds.

Until that happens, it would be nice if we could at least try not to
regress in the sense that a test t which was at one point leak-free
turns leaky. Such a regression would indicate that leak-free code
turned leaky, that a new feature is leaky, or that we simply happen to
trigger an existing leak as part of a newly added/modified test.

To that end, introduce a list of known-leaky tests, i.e., a list of
t-values. Now this will be able to find such regressions:

make SANITIZE=leak test GIT_SKIP_TESTS="$(cat t/known-leaky)"

The list was generated, and can be updated, as follows:

# Assume we're using prove, which will keep running after failure,
# and will record the results for us to parse (using "--state=").
# Otherwise use "make -k" and grep in t/test-results.
GIT_TEST_OPTS=-i make SANITIZE=leak test
cd t
prove --dry --state=failed |
perl -lne '/^(t[0-9]{4})-.*\.sh$/ and print $1' |
sort >known-leaky

The list added in this commit might be incomplete, since I do not run
all tests (I'm missing svn, cvs, p4, Windows-only and
filesystem-dependent tests, as well as "writeable /"). The majority of
these do not primarily test our C code, but all of them might trigger
leaks, in which case they would belong in this list.

Suggested-by: Nguyễn Thái Ngọc Duy 
Helped-by: Jeff King 
Signed-off-by: Martin Ågren 
---
 t/known-leaky | 539 ++
 1 file changed, 539 insertions(+)
 create mode 100644 t/known-leaky

diff --git a/t/known-leaky b/t/known-leaky
new file mode 100644
index 00..80a0af4d09
--- /dev/null
+++ b/t/known-leaky
@@ -0,0 +1,539 @@
+t0002
+t0003
+t0006
+t0008
+t0009
+t0012
+t0020
+t0021
+t0023
+t0040
+t0050
+t0056
+t0060
+t0062
+t0064
+t0070
+t0090
+t0100
+t0101
+t0110
+t0203
+t0300
+t0301
+t0302
+t1001
+t1002
+t1004
+t1005
+t1006
+t1007
+t1008
+t1011
+t1012
+t1013
+t1020
+t1021
+t1050
+t1051
+t1090
+t1301
+t1302
+t1304
+t1306
+t1308
+t1350
+t1400
+t1401
+t1402
+t1403
+t1404
+t1405
+t1406
+t1407
+t1408
+t1409
+t1410
+t1411
+t1412
+t1413
+t1414
+t1430
+t1450
+t1500
+t1502
+t1505
+t1507
+t1508
+t1510
+t1511
+t1512
+t1514
+t1700
+t2007
+t2008
+t2009
+t2010
+t2011
+t2012
+t2013
+t2014
+t2015
+t2016
+t2017
+t2018
+t2019
+t2020
+t2021
+t2022
+t2023
+t2024
+t2025
+t2026
+t2027
+t2030
+t2103
+t2106
+t2200
+t2203
+t2204
+t3001
+t3004
+t3005
+t3007
+t3010
+t3020
+t3030
+t3031
+t3032
+t3033
+t3034
+t3040
+t3050
+t3060
+t3200
+t3201
+t3202
+t3203
+t3204
+t3205
+t3210
+t3301
+t3302
+t3303
+t3304
+t3306
+t3307
+t3308
+t3309
+t3310
+t3311
+t3400
+t3402
+t3403
+t3404
+t3405
+t3406
+t3407
+t3408
+t3409
+t3410
+t3411
+t3412
+t3413
+t3414
+t3415
+t3416
+t3417
+t3418
+t3419
+t3420
+t3421
+t3425
+t3426
+t3427
+t3428
+t3429
+t3500
+t3501
+t3502
+t3503
+t3504
+t3505
+t3506
+t3507
+t3508
+t3509
+t3510
+t3511
+t3512
+t3513
+t3600
+t3700
+t3701
+t3800
+t3900
+t3901
+t3903
+t3904
+t3905
+t3906
+t4001
+t4008
+t4010
+t4013
+t4014
+t4015
+t4016
+t4017
+t4018
+t4020
+t4021
+t4022
+t4023
+t4026
+t4027
+t4028
+t4030
+t4031
+t4036
+t4038
+t4039
+t4041
+t4042
+t4043
+t4044
+t4045
+t4047
+t4048
+t4049
+t4051
+t4052
+t4053
+t4055
+t4056
+t4057
+t4059
+t4060
+t4061
+t4064
+t4065
+t4103
+t4107
+t4108
+t4111
+t4114
+t4115
+t4117
+t4118
+t4120
+t4121
+t4122
+t4124
+t4125
+t4127
+t4131
+t4135
+t4137
+t4138
+t4150
+t4151
+t4152
+t4153
+t4200
+t4201
+t4202
+t4203
+t4204
+t4205
+t4206
+t4207
+t4208
+t4209
+t4210
+t4211
+t4212
+t4213
+t4252
+t4253
+t4254
+t4255
+t4300
+t5000
+t5001
+t5002
+t5003
+t5004
+t5100
+t5150
+t5300
+t5301
+t5302
+t5303
+t5304
+t5305
+t5306
+t5310
+t5311
+t5312
+t5313
+t5314
+t5315
+t5316
+t5317
+t5400
+t5401
+t5402
+t5403
+t5404
+t5405
+t5406
+t5407
+t5408
+t5500
+t5501
+t5502
+t5503
+t5504
+t5505
+t5506
+t5509
+t5510
+t5512
+t5513
+t5514
+t5515
+t5516
+t5517
+t5518
+t5519
+t5520
+t5521
+t5522
+t5523
+t5524
+t5525
+t5526
+t5527
+t5528
+t5531
+t5532
+t5533
+t5534
+t5535
+t5536
+t5537
+t5538
+t5539
+t5540
+t5541
+t5542
+t5543
+t5544
+t5545
+t5546
+t5547
+t5550
+t5551
+t5560
+t5561
+t5570
+t5571
+t5572
+t5573
+t5600
+t5601
+t5603
+t5604
+t5605

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

[Stefan Beller]

On Wed, Feb 14, 2018 at 10:59 AM, Brandon Williams  wrote:
> 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 | 54 +++---
>  1 file changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/apply.c b/apply.c
> index 071f653c6..e9f34dceb 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_buf, *fixed;

*_buf makes it sound like it could be a strbuf at first,
but it is just a buffer. That is ok, as we take over a buffer from
'postimage->buf'. I was suggesting a new name, but given that
reuse of "buf", I guess this is fine.

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

[Brandon Williams]

On 02/14, Junio C Hamano wrote:
> Brandon Williams  writes:
> 
> > 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 
> > ---
> 
> The patch is not as bad as renaming "this" and leaving "that" behind
> but in the original, "this" and "this_dir" were treated as a pair.
> "this" was a score for a single item in the directory, "this_dir"
> was the sum of these scores for entries in the directory.
> 
> So renaming "this" to "sum" and leaving "this_dir" as-is looks like
> readability regression.  Perhaps replace "this_dir" with "sum_changes"
> and "this" with "changes" instead, or something like that?

100% agree.  I tried to spend time on each of these changes to come up
with a meaningful name, but sometimes wasn't able to understand the
section of code (with the limited context I had with some areas) and
wasn't able to come up with the best name.  I'll change this.

> 
> >  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;
> > }
> >  
> > /*

-- 
Brandon Williams

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

[Junio C Hamano]

Brandon Williams  writes:

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

The patch is not as bad as renaming "this" and leaving "that" behind
but in the original, "this" and "this_dir" were treated as a pair.
"this" was a score for a single item in the directory, "this_dir"
was the sum of these scores for entries in the directory.

So renaming "this" to "sum" and leaving "this_dir" as-is looks like
readability regression.  Perhaps replace "this_dir" with "sum_changes"
and "this" with "changes" instead, or something like that?

>  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;
>   }
>  
>   /*

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

[Junio C Hamano]

Elijah Newren  writes:

>>  Rename detection logic in "diff" family that is used in "merge" has
>>  learned to guess when all of x/a, x/b and x/c have moved to z/a,
>>  z/b and z/c, it is likely that x/d added in the meantime would also
>>  want to move to z/d by taking the hint that the entire directory
>>  'x' moved to 'z'.
>
> When you write release notes for this series, you may want to consider
> also calling out one or more of the bugs that were fixed as a side
> effect:
>   * a bug causing dirty files involved in a rename to be overwritten
> during merge
>   * a few memory leaks
> I added a reminder about these two fixes in the cover letter for my
> latest (and possibly last?) roll of the series that I just sent out.

Thanks.

Re: [PATCH v8 00/29] Add directory rename detection to git

[Stefan Beller]

On Wed, Feb 14, 2018 at 10:51 AM, Elijah Newren  wrote:
> This patchset introduces directory rename detection to merge-recursive.  See
>   https://public-inbox.org/git/20171110190550.27059-1-new...@gmail.com/
> for the first series (including design considerations, etc.)  This series
> continues to depend on en/merge-recursive-fixes in next, at least
> contextually.  For the curious, follow-up series and comments can also be
> found at
>   https://public-inbox.org/git/20171120220209.15111-1-new...@gmail.com/
>   https://public-inbox.org/git/20171121080059.32304-1-new...@gmail.com/
>   https://public-inbox.org/git/20171129014237.32570-1-new...@gmail.com/
>   https://public-inbox.org/git/20171228041352.27880-1-new...@gmail.com/
>   https://public-inbox.org/git/20180105202711.24311-1-new...@gmail.com/
>   https://public-inbox.org/git/20180130232533.25846-1-new...@gmail.com/
>
> Also, as a reminder, this series fixes a few bugs somewhat as a side effect:
>   * a bug causing dirty files involved in a rename to be overwritten
>   * a few memory leaks
>
> Changes since v7 (full tbdiff follows below):
>   * Added Stefan's Reviewed-by.
>   * Squashed commits introducing new hash structs and associated functions
> into the commit that used them to avoid unused function
> warnings/errors.
>   * Added or clarified a number of comments where things were unclear
>   * Minor stuff:
> * Style (and typo) fixes for commit message and comments
> * Avoiding casting with hash initialization function
> * s/malloc/xmalloc/
> * struct assignment
> * s/20/GIT_MAX_RAWSZ/

Even the interdiff has Stefan's Reviewed-by.

Thanks for being persistent,
Stefan

Re: [PATCH v2] Correct mispellings of ".gitmodule" to ".gitmodules"

[Junio C Hamano]

"Robert P. J. Day"  writes:

> There are a small number of misspellings, ".gitmodule", scattered
> throughout the code base, correct them ... no apparent functional
> changes.
>
>  Documentation/technical/api-submodule-config.txt | 2 +-
>  contrib/subtree/git-subtree.txt  | 2 +-
>  submodule-config.c   | 4 ++--
>  t/t5526-fetch-submodules.sh  | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
>
> Signed-off-by: Robert P. J. Day 
>
> ---
>
>   can't believe i forgot to sign my own patch. le *sigh* ...
>
> diff --git a/Documentation/technical/api-submodule-config.txt 
> b/Documentation/technical/api-submodule-config.txt

Just for future reference, the diffstat and summary comes _after_
the three-dash line, before which is your sign-off, and after which
you can have additional comment like "can't believe...".

No need to resend, as I can remove the cruft from the log message
while queuing.

Thanks.  The patch looks good.  Will queue.

Re: "git add" with several pathspecS and one doesn't match

[Junio C Hamano]

Goffredo Baroncelli  writes:

> I am facing this issue: I am ADDing some file with several
> pathspec, and one of these fails. The results is that no file is
> added at all.

This is a good example of a pretty conscious design decision to keep
operations atomic/a-o-n when appropriate.

Re: [PATCH] am: support --quit

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> Among the "in progress" commands, only git-am and git-merge do not
> support --quit. Support --quit in git-am too.

That's a strange way to phrase it, when the number of commands that
know and do not know it are about the same.

>  --abort::
>   Restore the original branch and abort the patching operation.
>  
> +--quit::
> + Abort the patching operation but keep HEAD and the index
> + untouched.

OK, I see from the documentation of rebase that the above pair is
consistent with how --abort/--quit are done over there.

> @@ -2359,6 +2363,10 @@ int cmd_am(int argc, const char **argv, const char 
> *prefix)
>   case RESUME_ABORT:
>   am_abort();
>   break;
> + case RESUME_QUIT:
> + am_rerere_clear();
> + am_destroy();
> + break;

The internal implementation detail of am_abort() is leaking out
here, by saying "rerere-clear" is the only special thing other than
recovering the HEAD and working tree state when abort happens.  It
makes readers wonder if am_rerere_clear() should become part of
am_destroy().  I dunno.

Re: Can we make git clone --recurse-submodules --shallow-submodules work for commits that are not on tags or branches

[Stefan Beller]

On Tue, Feb 13, 2018 at 9:06 PM, Ciro Santilli  wrote:
> I have a git repo with large submodules, notably the Linux kernel, and
> shallow clone saves a lot of time.
>
> However, QEMU is also a submodule, which I don't control, and QEMU has
> submodules which don't point to commits that are not on any tag or
> branch, so:
>
>  git clone --recurse-submodules --shallow-submodules 
> git://git.qemu.org/qemu.git
>
> currently fails with:
>
> error: Server does not allow request for unadvertised object
> 22ead3e0bfdb87516656453336160e0a37b066bf
> Fetched in submodule path 'capstone', but it did not contain
> 22ead3e0bfdb87516656453336160e0a37b066bf. Direct fetching of that
> commit failed.
>
> on git 2.16.1.

In theory this should be easy. :)

In practice not so much, unfortunately. This is because cloning will just obtain
the latest tip of a branch (usually master). There is no mechanism in clone
to specify the exact sha1 that is wanted.

The wire protocol supports for asking exact sha1s, so that should be covered.
(Caveat: it only works if the server operator enables
uploadpack.allowReachableSHA1InWant which github has not AFAICT)

git-fetch allows to fetch arbitrary sha1, so as a workaround you can run a fetch
after the recursive clone by using "git submodule update" as that will use
fetches after the initial clone.


> Furthermore, I reproduce this locally with direct filesystem clones:
> https://github.com/cirosantilli/test-git-web-interface/blob/15335d3002a3e64fc5756b69fb832a733aa63fb9/shallow-submodule.sh#L158
> and on GitHub, so I'm guessing it is not just the settings for a
> specific server?
>
> Would it be possible to make that work, or are there fundamental
> reasons why it is not possible?

The wire protocol allows for it, so it should be possible fundamentally.
There is a redesign of the wire protocol going on currently, which
could allow for a new fetch mode that allows having just one
connection, which would only negotiate on the superproject and
infer the submodule parts from the superproject.


> Here is my use case repo, at the point of the ugly workaround I'm
> having to do: 
> https://github.com/cirosantilli/linux-kernel-module-cheat/blob/a14c95346cfd9d2e7b2e475b0981aa71d819c20b/configure#L23
>
> Some more context:
> https://stackoverflow.com/questions/2144406/git-shallow-submodules/47374702#47374702
>
> This would make some embedded people happy.

Yes, a lot of submodule users would be happy if that
issue is solved.

Thanks for reporting,
Stefan

Re: should "--recurse-submodule" be "--recurse-submodules"?

[Stefan Beller]

On Tue, Feb 13, 2018 at 4:30 PM, Robert P. J. Day  wrote:
>
>   also just noticed the following:
>
> Documentation/RelNotes/2.13.0.txt:489: * A few commands that recently learned 
> the "--recurse-submodule"
> Documentation/RelNotes/2.12.0.txt:226: * "git push --dry-run 
> --recurse-submodule=on-demand" wasn't
> Documentation/RelNotes/2.11.1.txt:27: * "git push --dry-run 
> --recurse-submodule=on-demand" wasn't
> t/t5531-deep-submodule-push.sh:366: git push 
> --recurse-submodule=check origin master
> t/t5572-pull-submodule.sh:45:test_expect_success 'pull --recurse-submodule 
> setup' '
>
>   should some of those be corrected?

I get the same list via
  git grep -- --recurse-submodule |grep -v -- --recurse-submodules
so there is no missing piece left.

As you can see, the first 3 are Documentation, and the other 2 are tests,
the actual code is always --recurse-submodules (with an s!).

Fixing the docs as well as t5572 has the impact of "just fixing typos"
(though that is specifically valuable for command line options where
exact spelling matters)

The fix in 5531 is an actual bugfix in the test suite, which
went unnoticed as the test itself did not fail.

The test did not fail because git autocompleted the command
line option silently. But that is not what we want to test there.

Stefan

Re: [PATCH v3 00/14] Serialized Git Commit Graph

[Derrick Stolee]

On 2/14/2018 1:27 PM, Stefan Beller wrote:

On Wed, Feb 14, 2018 at 10:15 AM, Derrick Stolee  wrote:

There has been a lot of interesting discussion on this topic. Some of that
involves some decently significant changes from v3, so I wanted to summarize
my understanding of the feedback and seek out more feedback from reviewers
before rolling v4.

If we have consensus on these topics, then I'll re-roll on Friday, Feb 16th.
Please let me know if you are planning on reviewing v3 and need more time
than that.


* Graph Storage:

 - Move the graph files to a different directory than the "pack"
directory. Currently considering ".git/objects/info"

In my copy of git there is already a file

   $ cat .git/objects/info/packs
   P pack-8fdfd126aa8c2a868baf1f89788b07b79a4d365b.pack

which seems to be in line with the information provided in
'man gitrepository-layout':
 objects/info
Additional information about the object store is
recorded in this directory.

The commit graph files are not exactly "additional info about the
object store" but rather "about the objects". Close enough IMO.

Stefan


Thanks for the tip [1]. I was unfamiliar with it because it doesn't 
exist in repos that don't repack.


[1] 
https://git-scm.com/docs/gitrepository-layout/2.12.0#gitrepository-layout-objectsinfopacks

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

[Elijah Newren]

On Tue, Feb 13, 2018 at 5:51 PM, Junio C Hamano  wrote:

> * en/rename-directory-detection (2018-01-31) 31 commits
>  - merge-recursive: ensure we write updates for directory-renamed file
>  - merge-recursive: avoid spurious rename/rename conflict from dir renames
>  - directory rename detection: new testcases showcasing a pair of bugs
>  - merge-recursive: fix remaining directory rename + dirty overwrite cases
>  - merge-recursive: fix overwriting dirty files involved in renames
>  - merge-recursive: avoid clobbering untracked files with directory renames
>  - merge-recursive: apply necessary modifications for directory renames
>  - merge-recursive: when comparing files, don't include trees
>  - merge-recursive: check for file level conflicts then get new name
>  - merge-recursive: add computation of collisions due to dir rename & merging
>  - merge-recursive: add a new hashmap for storing file collisions
>  - merge-recursive: check for directory level conflicts
>  - merge-recursive: add get_directory_renames()
>  - merge-recursive: make a helper function for cleanup for handle_renames
>  - merge-recursive: add a new hashmap for storing directory renames
>  - merge-recursive: split out code for determining diff_filepairs
>  - merge-recursive: make !o->detect_rename codepath more obvious
>  - merge-recursive: fix leaks of allocated renames and diff_filepairs
>  - merge-recursive: introduce new functions to handle rename logic
>  - merge-recursive: move the get_renames() function
>  - directory rename detection: tests for handling overwriting dirty files
>  - directory rename detection: tests for handling overwriting untracked files
>  - directory rename detection: miscellaneous testcases to complete coverage
>  - directory rename detection: testcases exploring possibly suboptimal merges
>  - directory rename detection: more involved edge/corner testcases
>  - directory rename detection: testcases checking which side did the rename
>  - directory rename detection: files/directories in the way of some renames
>  - directory rename detection: partially renamed directory testcase/discussion
>  - directory rename detection: testcases to avoid taking detection too far
>  - directory rename detection: directory splitting testcases
>  - directory rename detection: basic testcases
>  (this branch uses en/merge-recursive-fixes.)
>
>  Rename detection logic in "diff" family that is used in "merge" has
>  learned to guess when all of x/a, x/b and x/c have moved to z/a,
>  z/b and z/c, it is likely that x/d added in the meantime would also
>  want to move to z/d by taking the hint that the entire directory
>  'x' moved to 'z'.

When you write release notes for this series, you may want to consider
also calling out one or more of the bugs that were fixed as a side
effect:
  * a bug causing dirty files involved in a rename to be overwritten
during merge
  * a few memory leaks
I added a reminder about these two fixes in the cover letter for my
latest (and possibly last?) roll of the series that I just sent out.

[PATCH v2 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 v2 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 | 66 
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index d95bf904c..6487d92ab 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);
 
@@ -546,8 +546,8 @@ static int add_branch_for_removal(const char *refname,
 }
 
 struct rename_info {
-   const char *old;
-   const char *new;
+   const char *old_name;
+   const char *new_name;
struct string_list *remote_branches;
 };
 
@@ -560,7 +560,7 @@ static int read_remote_branches(const char *refname,
int flag;
const char *symref;
 
-   strbuf_addf(, "refs/remotes/%s/", rename->old);
+   strbuf_addf(, "refs/remotes/%s/", rename->old_name);
if (starts_with(refname, buf.buf)) {
item = string_list_append(rename->remote_branches, 
xstrdup(refname));
symref = resolve_ref_unsafe(refname, RESOLVE_REF_READING,
@@ -615,36 +615,36 @@ static int mv(int argc, const char **argv)
if (argc != 3)
usage_with_options(builtin_remote_rename_usage, options);
 
-   rename.old = argv[1];
-   rename.new = argv[2];
+   rename.old_name = argv[1];
+   rename.new_name = argv[2];
rename.remote_branches = _branches;
 
-   oldremote = remote_get(rename.old);
+   oldremote = remote_get(rename.old_name);
if (!remote_is_configured(oldremote, 1))
-   die(_("No such remote: %s"), rename.old);
+   die(_("No such remote: %s"), rename.old_name);
 
-   if (!strcmp(rename.old, rename.new) && oldremote->origin != 
REMOTE_CONFIG)
+   if (!strcmp(rename.old_name, 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.old_name);
+   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);
+   strbuf_addf(_remote_context, ":refs/remotes/%s/", rename.old_name);
for (i = 0; i < oldremote->fetch_refspec_nr; i++) {
char *ptr;
 
@@ -655,8 +655,8 @@ static int mv(int argc, const char **argv)
refspec_updated = 1;
strbuf_splice(,

[PATCH v2 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 | 54 +++---
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/apply.c b/apply.c
index 071f653c6..e9f34dceb 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_buf, *fixed;
struct image fixed_preimage;
 
/*
@@ -2327,25 +2327,25 @@ static void update_pre_post_images(struct image 
*preimage,
 * We trust the caller to tell us if the update can be done
 * in place (postlen==0) or not.
 */
-   old = postimage->buf;
+   old_buf = postimage->buf;
if (postlen)
-   new = postimage->buf = xmalloc(postlen);
+   new_buf = postimage->buf = xmalloc(postlen);
else
-   new = old;
+   new_buf = old_buf;
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);
-   old += l_len;
-   new += l_len;
+   memmove(new_buf, old_buf, l_len);
+   old_buf += l_len;
+   new_buf += l_len;
continue;
}
 
/* a common context -- skip it in the original postimage */
-   old += l_len;
+   old_buf += l_len;
 
/* and find the corresponding one in the fixed preimage */
while (ctx < preimage->nr &&
@@ -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_name, *new_name;
 
/* Find common prefix */
-   old = p->old_name;
-   new = p->new_name;
+   old_name = p->old_name;
+   new_name = p->new_name;
while (1) {
const char *slash_old, *slash_new;
-   slash_old = strchr(old, '/');
-   slash_new = strchr(new, '/');
+   slash_old = strchr(old_name, '/');
+   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_name != slash_new - new_name ||
+   memcmp(old_name, new_name, slash_new - new_name))
break;
-   old = slash_old + 1;
-   new = slash_new + 1;
+   old_name = slash_old + 1;
+   new_name = slash_new + 1;
}
-   /* p->old_name thru old is the common prefix, and old and new
+   /* p->old_name thru old_name is the common prefix, and old_name and 
new_name
 * through the end of names are renames
 */
-   if (old != p->old_name)
+   if (old_name != p->old_name)
printf(" %s %.*s{%s => %s} (%d%%)\n", renamecopy,
-  (int)(old - p->old_name), p->old_name,
-  old, new, p->score);
+  (int)(old_name - p->old_name), p->old_name,
+  old_name, new_name, p->score);
else

[PATCH v2 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.1.291.g4437f3f132-goog

[PATCH v2 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 | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 42cf4f62a..303ca134d 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -284,30 +284,30 @@ 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_oid, new_oid, prev;
struct strbuf ref = STRBUF_INIT;
 
-   if (get_oid(object_ref, ) < 0)
+   if (get_oid(object_ref, _oid) < 0)
die("Not a valid object name: '%s'", object_ref);
 
-   type = sha1_object_info(old.hash, NULL);
+   type = sha1_object_info(old_oid.hash, NULL);
if (type < 0)
-   die("unable to get object type for %s", oid_to_hex());
+   die("unable to get object type for %s", oid_to_hex(_oid));
 
-   check_ref_valid(, , , force);
+   check_ref_valid(_oid, , , force);
strbuf_release();
 
-   export_object(, type, raw, tmpfile);
+   export_object(_oid, 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(, ))
-   return error("new object is the same as the old one: '%s'", 
oid_to_hex());
+   if (!oidcmp(_oid, _oid))
+   return error("new object is the same as the old one: '%s'", 
oid_to_hex(_oid));
 
-   return replace_object_oid(object_ref, , "replacement", , force);
+   return replace_object_oid(object_ref, _oid, "replacement", 
_oid, force);
 }
 
 static void replace_parents(struct strbuf *buf, int argc, const char **argv)
@@ -386,16 +386,16 @@ 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_oid, new_oid;
const char *old_ref = argv[0];
struct commit *commit;
struct strbuf buf = STRBUF_INIT;
const char *buffer;
unsigned long size;
 
-   if (get_oid(old_ref, ) < 0)
+   if (get_oid(old_ref, _oid) < 0)
die(_("Not a valid object name: '%s'"), old_ref);
-   commit = lookup_commit_or_die(, old_ref);
+   commit = lookup_commit_or_die(_oid, old_ref);
 
buffer = get_commit_buffer(commit, );
strbuf_add(, buffer, size);
@@ -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(, ))
-   return error("new commit is the same as the old one: '%s'", 
oid_to_hex());
+   if (!oidcmp(_oid, _oid))
+   return error("new commit is the same as the old one: '%s'", 
oid_to_hex(_oid));
 
-   return replace_object_oid(old_ref, , "replacement", , force);
+   return replace_object_oid(old_ref, _oid, "replacement", _oid, 
force);
 }
 
 int cmd_replace(int argc, const char **argv, const char *prefix)
-- 
2.16.1.291.g4437f3f132-goog

[PATCH v2 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 v2 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.1.291.g4437f3f132-goog

[PATCH v2 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 | 44 ++--
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/diff.c b/diff.c
index d682d0d1f..2de9c5ed2 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_word, ctx;
const char *newline;
 };
 
@@ -1655,12 +1655,12 @@ static void fn_out_diff_words_aux(void *priv, char 
*line, unsigned long len)
}
if (minus_begin != minus_end) {
fn_out_diff_words_write_helper(diff_words->opt,
-   >old, style->newline,
+   >old_word, style->newline,
minus_end - minus_begin, minus_begin);
}
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);
}
 
@@ -1758,7 +1758,7 @@ static void diff_words_show(struct diff_words_data 
*diff_words)
emit_diff_symbol(diff_words->opt, DIFF_SYMBOL_WORD_DIFF,
 line_prefix, strlen(line_prefix), 0);
fn_out_diff_words_write_helper(diff_words->opt,
-   >old, style->newline,
+   >old_word, style->newline,
diff_words->minus.text.size,
diff_words->minus.text.ptr);
diff_words->minus.text.size = 0;
@@ -1883,8 +1883,8 @@ 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->old_word.color = diff_get_color_opt(o, DIFF_FILE_OLD);
+   st->new_word.color = diff_get_color_opt(o, DIFF_FILE_NEW);
st->ctx.color = diff_get_color_opt(o, DIFF_CONTEXT);
}
 }
@@ -2047,8 +2047,8 @@ 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 *old_name = a;
+   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) {
-   if (*old == '/')
-   pfx_length = old - a + 1;
-   old++;
-   new++;
+   while (*old_name && *new_name && *old_name == *new_name) {
+   if (*old_name == '/')
+   pfx_length = old_name - a + 1;
+   old_name++;
+   new_name++;
}
 
/* Find common suffix */
-   old = a + len_a;
-   new = b + len_b;
+   old_name = a + len_a;
+   new_name = b + len_b;
sfx_length = 0;
/*
 * If there is a common prefix, it must end in a slash.  In
@@ -2087,13 +2087,13 @@ static char *pprint_rename(const char *a, const char *b)
 * underrun the input strings.
 */
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) {
-   if (*old == '/')
-   sfx_length = len_a - (old - a);
-   old--;
-   new--;
+   while (a + pfx_length - pfx_adjust_for_slash <= old_name &&
+  b + pfx_length - pfx_adjust_for_slash <= new_name &&
+  *old_name == *new_name) {
+   if (*old_name == '/')
+   sfx_length = len_a - (old_name - a);
+   old_name--;
+   new_name--;
}
 
/*
-- 
2.16.1.291.g4437f3f132-goog

[PATCH v2 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 2de9c5ed2..5e46502cd 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.1.291.g4437f3f132-goog

[PATCH v2 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.1.291.g4437f3f132-goog

[PATCH v2 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 | 38 +++---
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 8104603a3..261ce13d6 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 *old_entry,
+const struct cache_entry *new_entry,
 int report_missing,
 int cached, int match_missing)
 {
@@ -330,47 +330,47 @@ 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,
->oid, 1, old->ce_mode,
+   diff_index_show_file(revs, "-", old_entry,
+_entry->oid, 1, 
old_entry->ce_mode,
 0);
return -1;
}
 
if (revs->combine_merges && !cached &&
-   (oidcmp(oid, >oid) || oidcmp(>oid, >oid))) {
+   (oidcmp(oid, _entry->oid) || oidcmp(_entry->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);
+   p->parent[1].mode = old_entry->ce_mode;
+   oidcpy(>parent[1].oid, _entry->oid);
show_combined_diff(p, 2, revs->dense_combined_merges, revs);
free(p);
return 0;
}
 
-   oldmode = old->ce_mode;
-   if (mode == oldmode && !oidcmp(oid, >oid) && !dirty_submodule &&
+   oldmode = old_entry->ce_mode;
+   if (mode == oldmode && !oidcmp(oid, _entry->oid) && 
!dirty_submodule &&
!revs->diffopt.flags.find_copies_harder)
return 0;
 
diff_change(>diffopt, oldmode, mode,
-   >oid, oid, 1, !is_null_oid(oid),
-   old->name, 0, dirty_submodule);
+   _entry->oid, oid, 1, !is_null_oid(oid),
+   old_entry->name, 0, dirty_submodule);
return 0;
 }
 
-- 
2.16.1.291.g4437f3f132-goog

[PATCH v2 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.1.291.g4437f3f132-goog

[PATCH v2 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.1.291.g4437f3f132-goog

[PATCH v2 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 

Re: [PATCH] t/: correct obvious typo "detahced"

[Stefan Beller]

On Wed, Feb 14, 2018 at 1:08 AM, Robert P. J. Day  wrote:
>
> Signed-off-by: Robert P. J. Day 

Thanks,
Stefan

>
> ---
>
> diff --git a/t/t7409-submodule-detached-work-tree.sh 
> b/t/t7409-submodule-detached-work-tree.sh
> index c20717181..fc018e363 100755
> --- a/t/t7409-submodule-detached-work-tree.sh
> +++ b/t/t7409-submodule-detached-work-tree.sh
> @@ -6,7 +6,7 @@
>  test_description='Test submodules on detached working tree
>
>  This test verifies that "git submodule" initialization, update and addition 
> works
> -on detahced working trees
> +on detached working trees
>  '
>
>  TEST_NO_CREATE_REPO=1
>
> --
>
> 
> Robert P. J. Day Ottawa, Ontario, CANADA
> http://crashcourse.ca
>
> Twitter:   http://twitter.com/rpjday
> LinkedIn:   http://ca.linkedin.com/in/rpjday
> 

[PATCH v2 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.1.291.g4437f3f132-goog

[PATCH v2 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.1.291.g4437f3f132-goog

[PATCH v2 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 | 16 
 split-index.h |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/split-index.c b/split-index.c
index 83e39ec8d..7be5f748a 100644
--- a/split-index.c
+++ b/split-index.c
@@ -303,17 +303,17 @@ 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 *old_entry,
+struct cache_entry *new_entry)
 {
-   if (old->index &&
+   if (old_entry->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;
+   old_entry->index <= istate->split_index->base->cache_nr) {
+   new_entry->index = old_entry->index;
+   if (old_entry != 
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.1.291.g4437f3f132-goog

[PATCH v2 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.1.291.g4437f3f132-goog

[PATCH v2 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 | 30 +++---
 submodule.h |  2 +-
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/submodule.c b/submodule.c
index cd18400e8..12a2503fd 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_oid = 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;
@@ -605,9 +605,9 @@ void show_submodule_inline_diff(struct diff_options *o, 
const char *path,
goto done;
 
if (left)
-   old = one;
+   old_oid = one;
if (right)
-   new = two;
+   new_oid = two;
 
cp.git_cmd = 1;
cp.dir = path;
@@ -630,7 +630,7 @@ void show_submodule_inline_diff(struct diff_options *o, 
const char *path,
argv_array_pushf(, "--dst-prefix=%s%s/",
 o->b_prefix, path);
}
-   argv_array_push(, oid_to_hex(old));
+   argv_array_push(, oid_to_hex(old_oid));
/*
 * If the submodule has modified content, we will diff against the
 * work tree, under the assumption that the user has asked for the
@@ -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())
@@ -1578,8 +1578,8 @@ static void submodule_reset_index(const char *path)
  * pass NULL for old or new respectively.
  */
 int submodule_move_head(const char *path,
-const char *old,
-const char *new,
+const char *old_head,
+const char *new_head,
 unsigned flags)
 {
int ret = 0;
@@ -1600,7 +1600,7 @@ int submodule_move_head(const char *path,
else
error_code_ptr = NULL;
 
-   if (old && !is_submodule_populated_gently(path, error_code_ptr))
+   if (old_head && !is_submodule_populated_gently(path, error_code_ptr))
return 0;
 
sub = submodule_from_path(_oid, path);
@@ -1608,14 +1608,14 @@ int submodule_move_head(const char *path,
if (!sub)
die("BUG: could not get submodule information for '%s'", path);
 
-   if (old && !(flags & SUBMODULE_MOVE_HEAD_FORCE)) {
+   if (old_head && !(flags & SUBMODULE_MOVE_HEAD_FORCE)) {
/* Check if the submodule has a dirty index. */
if (submodule_has_dirty_index(sub))
return error(_("submodule '%s' has dirty index"), path);
}
 
if (!(flags & SUBMODULE_MOVE_HEAD_DRY_RUN)) {
-   if (old) {
+   if (old_head) {
if (!submodule_uses_gitfile(path))
absorb_git_dir_into_superproject("", path,
ABSORB_GITDIR_RECURSE_SUBMODULES);
@@ -1629,7 +1629,7 @@ int submodule_move_head(const char *path,
submodule_reset_index(path);
}
 
-   if (old && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {
+   if (old_head && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {
char *gitdir = xstrfmt("%s/modules/%s",
get_git_common_dir(), sub->name);
connect_work_tree_and_git_dir(path, gitdir);
@@ -1658,9 +1658,9 @@ int submodule_move_head(const char *path,
argv_array_push(, "-m");
 
if (!(flags & SUBMODULE_MOVE_HEAD_FORCE))
-   argv_array_push(, old ? old : EMPTY_TREE_SHA1_HEX);
+   argv_array_push(, old_head ? old_head : 
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 */

[PATCH v2 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.1.291.g4437f3f132-goog

[PATCH v2 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 | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/remote.c b/remote.c
index 4e93753e1..2922dca07 100644
--- a/remote.c
+++ b/remote.c
@@ -1970,33 +1970,33 @@ 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_commit, *new_commit;
struct commit_list *list, *used;
int found = 0;
 
/*
-* Both new and old must be commit-ish and new is descendant of
-* old.  Otherwise we require --force.
+* Both new_commit and old_commit must be commit-ish and new_commit is 
descendant of
+* old_commit.  Otherwise we require --force.
 */
o = deref_tag(parse_object(old_oid), NULL, 0);
if (!o || o->type != OBJ_COMMIT)
return 0;
-   old = (struct commit *) o;
+   old_commit = (struct commit *) o;
 
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_commit) {
found = 1;
break;
}
-- 
2.16.1.291.g4437f3f132-goog

[PATCH v2 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.1.291.g4437f3f132-goog

[PATCH v2 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.1.291.g4437f3f132-goog

[PATCH v2 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 | 56 +++---
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/line-log.c b/line-log.c
index 545ad0f28..cdc2257db 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;
}
}
 }
@@ -696,18 +696,18 @@ static struct line_log_data *line_log_data_merge(struct 
line_log_data *a,
 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 *old_line = 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);
-   free_line_log_data(old);
+   old_line = lookup_decoration(>line_log_data, >object);
+   if (old_line && range) {
+   new_line = line_log_data_merge(old_line, range);
+   free_line_log_data(old_line);
} 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.1.291.g4437f3f132-goog

[PATCH v2 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 | 40 
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 2eb81a66b..b22668bfa 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_entry = 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_entry);
+   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);
+   cache_tree_invalidate_path(istate, old_entry->name);
+   untracked_cache_remove_from_index(istate, old_entry->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.1.291.g4437f3f132-goog

[PATCH v2 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.1.291.g4437f3f132-goog

Re: [PATCH 5/6] test-hashmap: simplify alloc_test_entry

[Junio C Hamano]

Jeff King  writes:

> This function takes two ptr/len pairs, which implies that
> they can be arbitrary buffers. But internally, it assumes
> that each "ptr" is NUL-terminated at "len" (because we
> memcpy an extra byte to pick up the NUL terminator).

Makes quite a lot of sense.  In addition, get_value() assumes that
the key cannot have NUL in it, so the use of counted string in
alloc_test_entry() serves no purpose other than to confuse the
readers ;-)

> Note that we can get rid of the "l1" and "l2" variables from
> cmd_main() as a further cleanup, since they are now mostly

Made me wonder if this "we can" is "we could if we wanted to in the
future although we do not go that far in this patch", but it turns
out that this is also done, and I think it is a good clean-up.

[PATCH v2 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.1.291.g4437f3f132-goog

[PATCH v2 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 | 196 ++---
 1 file changed, 98 insertions(+), 98 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index c54c78df5..ed3d655f2 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_commit, 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(old_commit ? _commit->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. */
 
 }
@@ -472,8 +472,8 @@ 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 *old_branch_info,
+ 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 {
@@ -511,7 +511,7 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
topts.initial_checkout = is_cache_unborn();
topts.update = 1;
topts.merge = 1;
-   topts.gently = opts->merge && old->commit;
+   topts.gently = opts->merge && old_branch_info->commit;
topts.verbose_update = opts->show_progress;
topts.fn = twoway_merge;
if (opts->overwrite_ignore) {
@@ -519,11 +519,11 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
topts.dir->flags |= DIR_SHOW_IGNORED;
setup_standard_excludes(topts.dir);
}
-   tree = parse_tree_indirect(old->commit ?
-  >commit->object.oid :
+   tree = parse_tree_indirect(old_branch_info->commit ?
+  _branch_info->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, );
@@ -540,10 +540,10 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
return 1;
 
/*
-* Without old->commit, the below is the same as
+* Without old_branch_info->commit, the below is the 
same as
 * the two-tree unpack we already tried and failed.
 */
-   if (!old->commit)
+   if (!old_branch_info->commit)
return 1;
 
/* Do more real merge */
@@ -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.ancestor = old_branch_info->name;
+   o.branch1 = new_branch_info->name;
o.branch2 = "local";
-   ret = merge_trees(, new->commit->tree, work,
-   old->commit->tree, );
+  

[PATCH v2 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.1.291.g4437f3f132-goog