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

[Todd Zullinger]

I wrote:
> Hi Jonathan,
> 
> Jonathan Nieder wrote:
>> Todd Zullinger wrote:
> [...]
>>> +++ 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_FALLBACKS if you do not want to install fallbacks for
>>> +# perl CPAN modules.
>> 
>> nit: Looking at this as a naive user, it's not obvious what kind of
>> fallbacks are meant. How about:
>> 
>>  Define NO_PERL_CPAN_FALLBACKS if you do not want to install bundled
>>  copies of CPAN modules that serve as a fallback in case the modules are
>>  not available on the system.
>> 
>> Or perhaps:
>> 
>>  Define HAVE_CPAN_MODULES if you have Error.pm and Mail::Address 
>> installed
>>  and do not want to install the fallback copies from perl/FromCPAN.
> 
> Hmm, a positive variable like HAVE_CPAN_MODULES is
> appealing.
> 
> I don't know about listing the modules, as those seem likely
> to change and then the comment becomes stale.  It's nice to
> have a shorter name.  I could easily go back and forth.
> Hopefully some other folks will chime in with preferences.
> 
>> Would this patch need to update the loader to expect the modules in
>> the new location?
> 
> That's a good catch.  In checking how this ends up when not
> setting NO_PERL_CPAN_FALLBACKS, we end up installing
> FromCPAN at the root of $perllibdir rather than under the
> Git dir.
> 
> While we could probably fix Git::LoadCPAN, I doubt we want
> to pollute the namespace. ;) So we'll want to ensure the
> files get put in the right place from the start.  I'll try
> to fix that up.

I ended up adding a second variable to hold the FromCPAN
wildcard match, as I couldn't come up with any clean way to
do it in the same LIB_PERL{,_GEN} vars.

I tested it with and without NO_PERL_CPAN_FALLBACKS set and
with and without the system Error and Mail::Address modules
installed.  There's nothing which automatically removes the
perl/build/lib/Git/FromCPAN tree if make was run without
NO_PERL_CPAN_FALLBACKS set and then re-run with it.  I don't
know if that's something we'd care to support or not.  I
suspect it's simpler to require 'make clean' between such
runs.

(I had to restore the FromCPAN copy of Address.pm, as noted
in <87tvuif10e@evledraar.gmail.com>, of course.)

Here's what it looks like now (still subject to changing the
NO_PERL_CPAN_FALLBACKS variable).

 8> 
From: Todd Zullinger 
Subject: [PATCH] Makefile: add NO_PERL_CPAN_FALLBACKS knob

We include some perl modules which are not part of the core perl
install, as a convenience.  This allows us to rely on those modules in
our perl-based tools and scripts without requiring users to install the
modules from CPAN or their operating system packages.

Users whose operating system provides these modules and packagers of Git
often don't want to ship or use these bundled modules.  Allow these
users to set NO_PERL_CPAN_FALLBACKS to avoid installing the bundled
modules.

Signed-off-by: Todd Zullinger 
---
 Makefile| 14 ++
 perl/{Git => }/FromCPAN/.gitattributes  |  0
 perl/{Git => }/FromCPAN/Error.pm|  0
 perl/{Git => }/FromCPAN/Mail/.gitattributes |  0
 perl/{Git => }/FromCPAN/Mail/Address.pm |  0
 5 files changed, 14 insertions(+)
 rename perl/{Git => }/FromCPAN/.gitattributes (100%)
 rename perl/{Git => }/FromCPAN/Error.pm (100%)
 rename perl/{Git => }/FromCPAN/Mail/.gitattributes (100%)
 rename perl/{Git => }/FromCPAN/Mail/Address.pm (100%)

diff --git a/Makefile b/Makefile
index bdd50069af..49244fb116 100644
--- a/Makefile
+++ b/Makefile
@@ -296,6 +296,10 @@ all::
 #
 # Define NO_PERL if you do not want Perl scripts or libraries at all.
 #
+# Define NO_PERL_CPAN_FALLBACKS if you do not want to install bundled
+# copies of CPAN modules that serve as a fallback in case the modules are
+# not available on the system.
+#
 # Define PYTHON_PATH to the path of your Python binary (often /usr/bin/python
 # but /usr/bin/python2.7 on some platforms).
 #
@@ -2300,15 +2304,25 @@ po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
 
 LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm 
perl/Git/*/*/*.pm)
 LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
+ifndef NO_PERL_CPAN_FALLBACKS
+LIB_CPAN := $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
+LIB_CPAN_GEN := $(patsubst perl/%.pm,perl/build/lib/Git/%.pm,$(LIB_CPAN))
+endif
 
 ifndef NO_PERL
 all:: $(LIB_PERL_GEN)
+ifndef NO_PERL_CPAN_FALLBACKS
+all:: $(LIB_CPAN_GEN)
+endif
 endif
 
 perl/build/lib/%.pm: perl/%.pm
$(QUIET_GEN)mkdir -p $(dir $@) && \
sed -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' < $< > $@
 
+perl/build/lib/Git/FromCPAN/%.pm: perl/FromCPAN/%.pm
+   $(QUIET_GEN)mkdir -p $(dir $@) && cp $< $@
+
 perl/build/man/man3/Git.3pm: perl/Git.pm
$(QUIET_GEN)mkdir -p $(dir $@) && \
pod2man $< $@
diff --git 

[PATCH] Makefile: remove *.spec from clean target

[Todd Zullinger]

Support for generating an rpm was dropped in ab214331cf ("Makefile: stop
pretending to support rpmbuild", 2016-04-04).  We don't generate any
*.spec files so there is no need to clean them up.

Signed-off-by: Todd Zullinger 
---

I noticed this minor cruft today.  Since we don't generate a
spec file, this is at best unneeded.  At worst we could
wrongly delete a users spec file if they happened to be
working on it in their git clone and called make clean.

 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index c56fdc14ca..d135f8baa1 100644
--- a/Makefile
+++ b/Makefile
@@ -2734,7 +2734,7 @@ clean: profile-clean coverage-clean
$(RM) $(TEST_PROGRAMS) $(NO_INSTALL)
$(RM) -r bin-wrappers $(dep_dirs)
$(RM) -r po/build/
-   $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) 
tags cscope*
+   $(RM) *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) tags 
cscope*
$(RM) -r $(GIT_TARNAME) .doc-tmp-dir
$(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
$(RM) $(htmldocs).tar.gz $(manpages).tar.gz
-- 
2.16.2

-- 
Todd
~~
A thing worth having is a thing worth cheating for.
-- W. C. Fields

Re: Please test Git for Windows' latest snapshot

[Bryan Turner]

On Fri, Feb 16, 2018 at 3:30 PM, Johannes Schindelin
 wrote:
> Hi team,
>
> I am unwilling to release Git for Windows v2.16.2 on a Friday night, but I
> have something almost as good. There is a snapshot available here:
>
> 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__wingit.blob.core.windows.net_files_index.html=DwIBAg=wBUwXtM9sKhff6UeHOQgvw=uBedA6EFFVX1HiLgmpdrBrv8bIDAScKjk1yk9LOASBM=xZghHWteeNbJ2bu5ySDq9WwqnfX8X7FZ_CWsV9gAyJU=NzSYCFSWWokPP9A9FA_EmJO5yu8qtRKw5M-Ep_qooUc=
>
> That snapshot brings quite a few updated components apart from Git proper
> (such as an updated MSYS2 runtime), and I would love to ask y'all to give
> this snapshot a proper "tire kicking".

I've run Bitbucket Server's full Git test suite (~1,500 tests) against
the Portable Git snapshot (e1848984d1), no failures to report.

Bryan

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

[Todd Zullinger]

Hi Jonathan,

Jonathan Nieder wrote:
> Todd Zullinger wrote:
[...]
>> +++ 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_FALLBACKS if you do not want to install fallbacks for
>> +# perl CPAN modules.
> 
> nit: Looking at this as a naive user, it's not obvious what kind of
> fallbacks are meant. How about:
> 
>   Define NO_PERL_CPAN_FALLBACKS if you do not want to install bundled
>   copies of CPAN modules that serve as a fallback in case the modules are
>   not available on the system.
> 
> Or perhaps:
> 
>   Define HAVE_CPAN_MODULES if you have Error.pm and Mail::Address 
> installed
>   and do not want to install the fallback copies from perl/FromCPAN.

Hmm, a positive variable like HAVE_CPAN_MODULES is
appealing.

I don't know about listing the modules, as those seem likely
to change and then the comment becomes stale.  It's nice to
have a shorter name.  I could easily go back and forth.
Hopefully some other folks will chime in with preferences.

> Would this patch need to update the loader to expect the modules in
> the new location?

That's a good catch.  In checking how this ends up when not
setting NO_PERL_CPAN_FALLBACKS, we end up installing
FromCPAN at the root of $perllibdir rather than under the
Git dir.

While we could probably fix Git::LoadCPAN, I doubt we want
to pollute the namespace. ;) So we'll want to ensure the
files get put in the right place from the start.  I'll try
to fix that up.

Thanks for the careful eyes, as usual.

-- 
Todd
~~
Happiness is like peeing on yourself. Everyone can see it, but only
you can feel its warmth

Please test Git for Windows' latest snapshot

[Johannes Schindelin]

Hi team,

I am unwilling to release Git for Windows v2.16.2 on a Friday night, but I
have something almost as good. There is a snapshot available here:

https://wingit.blob.core.windows.net/files/index.html

That snapshot brings quite a few updated components apart from Git proper
(such as an updated MSYS2 runtime), and I would love to ask y'all to give
this snapshot a proper "tire kicking".

Think of it as a release candidate. A real one, this really is the
revision that I would like to release as Git for Windows v2.16.2 on Monday
(with the only change being the version number).

Thank you for your help!
Johannes

Re: [PATCHv2 00/16] Moving global state into the repository object (part 1)

[Jonathan Nieder]

Hi,

Stefan Beller wrote:

> v2:
> * duplicated the 'ignore_env' bit into the object store as well
> * the #define trick no longer works as we do not have a "the_objectstore" 
> global,
>   which means there is just one patch per function that is converted.
>   As this follows the same structure of the previous series, I am still 
> confident
>   there is no hidden dependencies to globals outside the object store in these
>   converted functions.
> * rebased on top of current master, resolving the merge conflicts.
>   I think I used the list.h APIs right, but please double check.

For what it's worth, I think I prefer v1.  I put some comments on why
on patch 0 of v1 and would be interested in your thoughts on them
(e.g. as a reply to that).  I also think that even if we want to
switch to a style that passes around object_store separately from
repository, it is easier to do the migration in two steps: first get
rid of hidden dependencies on the_repository, then do the (simpler)
automatic migration from

 f(the_repository)

to

 f(the_repository->object_store)

*afterwards*.

Thoughts?

Thanks,
Jonathan

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

[Jonathan Nieder]

Hi,

Todd Zullinger wrote:

> Subject: [PATCH] Makefile: add NO_PERL_CPAN_FALLBACKS 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_FALLBACKS to easily avoid installing these fallback perl
> CPAN modules.
>
> Signed-off-by: Todd Zullinger 
> ---
>  Makefile| 6 ++
>  perl/{Git => }/FromCPAN/.gitattributes  | 0
>  perl/{Git => }/FromCPAN/Error.pm| 0
>  perl/{Git => }/FromCPAN/Mail/.gitattributes | 0
>  perl/{Git => }/FromCPAN/Mail/Address.pm | 0
>  5 files changed, 6 insertions(+)
>  rename perl/{Git => }/FromCPAN/.gitattributes (100%)
>  rename perl/{Git => }/FromCPAN/Error.pm (100%)
>  rename perl/{Git => }/FromCPAN/Mail/.gitattributes (100%)
>  rename perl/{Git => }/FromCPAN/Mail/Address.pm (100%)

Nice!  I like it.

[...]
> +++ 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_FALLBACKS if you do not want to install fallbacks for
> +# perl CPAN modules.

nit: Looking at this as a naive user, it's not obvious what kind of
fallbacks are meant. How about:

Define NO_PERL_CPAN_FALLBACKS if you do not want to install bundled
copies of CPAN modules that serve as a fallback in case the modules are
not available on the system.

Or perhaps:

Define HAVE_CPAN_MODULES if you have Error.pm and Mail::Address 
installed
and do not want to install the fallback copies from perl/FromCPAN.

Would this patch need to update the loader to expect the modules in
the new location?

Thanks,
Jonathan

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

[Jeff King]

On Thu, Feb 15, 2018 at 01:33:25PM +0300, Оля Тележная wrote:

> 2018-02-15 8:53 GMT+03:00 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?
> 
> We discussed that earlier outside of mailing list, and I even tried to
> implement that idea and spent a couple of days to prove that it's not
> possible.
> The problem is that the list of atoms is made dynamically, and we
> can't store pointers to any values in each atom. That's why we need
> separate cat_file_info variable that is outside of main atom list.
> We also need is_cat_file because we still have some part of logic that
> is different for cat-file and for all other commands, and sometimes we
> need to know that information.

OK, right, I forgot about the pointers thing. This is definitely the
sort of design decision that should go into the commit message.

I went over our off-list discussion again to refresh my memory. Let me
try to summarize a bit (for myself, or for other readerss). Naively, one
would hope that you could do something like:

  1. While parsing the %(objectsize) atom, add a pointer like:

   the_object_info->sizep = 

  2. Later when fulfilling the request for a specific object, if
 the_object_info is not empty, then call sha1_object_info() with it
 to hold the results.

  3. Read the value out of atom.size when assembling the formatted
 string.

But that has two problems:

  a. During parsing, we're resizing the atom array, so the 
 pointer is subject to change.  This is the issue you mentioned
 above. I think we could solve it by taking a pass over the atoms
 after parsing and filling in the pointers then. But...

  b. Another problem is that the same atom may appear multiple times. So
 parsing "%(objectsize) %(objectsize)", we can't point
 the_object_info at _both_ of them. We need our call to
 sha1_object_info() to store the result in a single location, and
 then as we format each atom they can both use that (which works
 even if they might format it differently, say if you had
 "%(deltabase) %(deltabase:abbrev)".

So we do need some kind of global storage to hold these results. This is
sort of the same problem we have with parsing the objects at all. There
we get by without a global because all of the logic is crammed into
populate_value(). In theory we could do the same thing here, but there
are just a lot of different items we might be asking for. So instead of
"need_tagged", you'd have a multitude of need_objectsize_disk,
need_deltabase, etc, which gets unwieldy. We may as well fill in a
"struct object_info".

So OK, I agree that something like "struct expand_data" is needed.

But what seems non-ideal to me is the way that ref-filter depends on it
in this cat-file-specific way. If it needs those values it should be
calling sha1_object_info(). And if it doesn't, then it can skip the
call. It shouldn't need to know about cat-file at all.

And likewise cat-file should not know about expand_data. And I think
that's true by the end of the series, though the steps in the middle
left me a little confused.

-Peff

Re: [PATCH 0/2] fix minor git-worktree.txt botches

[Junio C Hamano]

Eric Sunshine  writes:

> This patch series fixes a couple git-worktree.txt botches from
> 4e85333197 (worktree: make add   dwim, 2017-11-26).
>
> Eric Sunshine (2):
>   git-worktree.txt: fix missing ")" typo
>   git-worktree.txt: fix indentation of example and text of 'add' command
>
>  Documentation/git-worktree.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Both patches looked trivially correct; thanks.

[PATCH v2] merge: allow fast-forward when merging a tracked tag

[Junio C Hamano]

Long time ago at fab47d05 ("merge: force edit and no-ff mode when
merging a tag object", 2011-11-07), "git merge" was made to always
create a merge commit when merging a tag, even when the side branch
being merged is a descendant of the current branch.

This default is good for merges made by upstream maintainers to
integrate work signed by downstream contributors, but will leave
pointless no-ff merges when downstream contributors pull a newer
release tag to make their long-running topic branches catch up with
the upstream.  When there is no local work left on the topic, such a
merge should simply fast-forward to the commit pointed at by the
release tag.

Update the default (again) for "git merge" that merges a tag object
to (1) --no-ff (i.e. create a merge commit even when side branch
fast forwards) if the tag being merged is not at its expected place
in refs/tags/ hierarchy and (2) --ff (i.e. allow fast-forward update
when able) otherwise.

Signed-off-by: Junio C Hamano 
---

 * As "git pull linus v4.16-rc1" does not create refs/tags/v4.16-rc1
   during fetch, and there is no way to mechanically tell the
   invocation from "git pull davem for-linus" which does not create
   refs/tags/for-linus, the new default that fast-forwards would
   kick in only when "git fetch linus" followed by possibly other
   other work and concluded with "git merge v4.16-rc1".  So we still
   need the "education" part to ensure that downstream does not make
   pointless merges with "git pull --ff-only".  

   With or without this patch, the other condition that disables the
   "when merging a tag, do not allow fast-forward" default is
   "unless --ff-only is given".  We may want to rethink it.  For
   example, shouldn't "git pull --ff linus v4.16-rc1" enough clue
   that the user _knows_ that the signature in that release tag will
   be lost if the current branch has no original development and
   explicitly _accepts_ fast-forwarding behaviour?  When the current
   branch may or may not have original development, having to say
   "pull --ff-only" while catching up to the upstream and accepting
   50% chance of seeing it fail (when we do have some original
   development hence the update does not fast-forward) smells like a
   clunky interface.

 Documentation/merge-options.txt |  3 ++-
 builtin/merge.c | 43 +
 t/t6200-fmt-merge-msg.sh|  2 +-
 t/t7600-merge.sh| 38 +++-
 4 files changed, 79 insertions(+), 7 deletions(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 3888c3ff85..63a3fc0954 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -35,7 +35,8 @@ set to `no` at the beginning of them.
 --no-ff::
Create a merge commit even when the merge resolves as a
fast-forward.  This is the default behaviour when merging an
-   annotated (and possibly signed) tag.
+   annotated (and possibly signed) tag that is not stored in
+   its natural place in 'refs/tags/' hierarchy.
 
 --ff-only::
Refuse to merge and exit with a non-zero status unless the
diff --git a/builtin/merge.c b/builtin/merge.c
index 30264cfd7c..532522a854 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -33,6 +33,7 @@
 #include "sequencer.h"
 #include "string-list.h"
 #include "packfile.h"
+#include "tag.h"
 
 #define DEFAULT_TWOHEAD (1<<0)
 #define DEFAULT_OCTOPUS (1<<1)
@@ -1125,6 +1126,43 @@ static struct commit_list *collect_parents(struct commit 
*head_commit,
return remoteheads;
 }
 
+static int merging_a_throwaway_tag(struct commit *commit)
+{
+   char *tag_ref;
+   struct object_id oid;
+   int is_throwaway_tag = 0;
+
+   /* Are we merging a tag? */
+   if (!merge_remote_util(commit) ||
+   !merge_remote_util(commit)->obj ||
+   merge_remote_util(commit)->obj->type != OBJ_TAG)
+   return is_throwaway_tag;
+
+   /*
+* Now we know we are merging a tag object.  Are we downstream
+* and following the tags from upstream?  If so, we must have
+* the tag object pointed at by "refs/tags/$T" where $T is the
+* tagname recorded in the tag object.  We want to allow such
+* a "just to catch up" merge to fast-forward.
+*
+* Otherwise, we are playing an integrator's role, making a
+* merge with a throw-away tag from a contributor with
+* something like "git pull $contributor $signed_tag".
+* We want to forbid such a merge from fast-forwarding
+* by default; otherwise we would not keep the signature
+* anywhere.
+*/
+   tag_ref = xstrfmt("refs/tags/%s",
+ ((struct tag *)merge_remote_util(commit)->obj)->tag);
+   if (!read_ref(tag_ref, ) &&
+   !oidcmp(, _remote_util(commit)->obj->oid))
+   is_throwaway_tag = 0;
+   else

[no subject]

[Brigadier General Dawn M. Dunlop]

-- 
Dear friend, i am General Brigadier Dawn M Dunlop from U S A, can we
talk?? Very important and urgent please.

[PATCH 1/2] git-worktree.txt: fix missing ")" typo

[Eric Sunshine]

Add the closing ")" to a parenthetical phrase introduced by 4e85333197
(worktree: make add   dwim, 2017-11-26).

While at it, add a missing ":" at the end of the same sentence since
it precedes an example literal command block.

Reported-by: Mike Nordell 
Signed-off-by: Eric Sunshine 
---
 Documentation/git-worktree.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 41585f535d..c941c48827 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -52,10 +52,10 @@ is linked to the current repository, sharing everything 
except working
 directory specific files such as HEAD, index, etc. `-` may also be
 specified as ``; it is synonymous with `@{-1}`.
 +
-If  is a branch name (call it `` and is not found,
+If  is a branch name (call it ``) and is not found,
 and neither `-b` nor `-B` nor `--detach` are used, but there does
 exist a tracking branch in exactly one remote (call it ``)
-with a matching name, treat as equivalent to
+with a matching name, treat as equivalent to:
 
 $ git worktree add --track -b   /
 
-- 
2.16.1.374.g7648891022

[PATCH 2/2] git-worktree.txt: fix indentation of example and text of 'add' command

[Eric Sunshine]

When 4e85333197 (worktree: make add   dwim, 2017-11-26)
added an example command in a literal code block, it neglected to
insert a mandatory "+" line before the block. This omission resulted
in both the literal code block and the (existing) paragraph following
the block to be outdented, even though they should be indented under
the 'add' sub-command along with the rest of the text pertaining to
that command. Furthermore, the mandatory "+" line separating the code
block from the following text got rendered as a leading character on
the line ("+ If ...") rather than being treated as a
formatting directive.

Fix these problems by adding the missing "+" line before the example
code block.

Signed-off-by: Eric Sunshine 
---
 Documentation/git-worktree.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index c941c48827..5ac3f68ab5 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -56,6 +56,7 @@ If  is a branch name (call it ``) and is 
not found,
 and neither `-b` nor `-B` nor `--detach` are used, but there does
 exist a tracking branch in exactly one remote (call it ``)
 with a matching name, treat as equivalent to:
++
 
 $ git worktree add --track -b   /
 
-- 
2.16.1.374.g7648891022

[PATCH 0/2] fix minor git-worktree.txt botches

[Eric Sunshine]

This patch series fixes a couple git-worktree.txt botches from
4e85333197 (worktree: make add   dwim, 2017-11-26).

Eric Sunshine (2):
  git-worktree.txt: fix missing ")" typo
  git-worktree.txt: fix indentation of example and text of 'add' command

 Documentation/git-worktree.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.16.1.374.g7648891022

Re: [PATCH 04/16] object-store: move packed_git and packed_git_mru to object store

[Junio C Hamano]

Stefan Beller  writes:

>   @@ @@
>   - packed_git_mru
>   + the_repository->objects.packed_git_mru

Regarding this...

> diff --git a/object-store.h b/object-store.h
> index a3f0d6ac15..024ccc91e9 100644
> --- a/object-store.h
> +++ b/object-store.h
> @@ -2,6 +2,7 @@
>  #define OBJECT_STORE_H
>  
>  #include "cache.h"
> +#include "list.h"
>  
>  struct raw_object_store {
> ... 
> + struct packed_git *packed_git;
> + /*
> +  * A most-recently-used ordered version of the packed_git list, which 
> can
> +  * be iterated instead of packed_git (and marked via mru_mark).
> +  */
> + struct list_head packed_git_mru;
> +
>   struct alternate_object_database *alt_odb_list;
>   struct alternate_object_database **alt_odb_tail;
>  
>   unsigned ignore_env : 1;
>  };
> -#define RAW_OBJECT_STORE_INIT { NULL, NULL, NULL, 0 }
> +
> +#define MRU_LIST_INIT {NULL, NULL}
> +#define RAW_OBJECT_STORE_INIT { NULL, NULL, MRU_LIST_INIT, NULL, NULL, 0 }
> ...
> diff --git a/packfile.c b/packfile.c
> index 216ea836ee..d41e4c83d0 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -7,6 +7,7 @@
> -...
> -LIST_HEAD(packed_git_mru);

Given that the definition of LIST_HEAD() is

/* Define a variable with the head and tail of the list. */
#define LIST_HEAD(name) \
struct list_head name = { &(name), &(name) }

doesn't the updated definition of RAW_OBJECT_STORE_INIT look fishy?

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

[Junio C Hamano]

Jeff King  writes:

> In which case yeah, I could see choosing an in-repo encoding to possibly
> be useful (but it also seems like a feature that could easily be tacked
> on later if somebody cares).

Yes, I think we are on the same page---in-repo-encoding that is a
natural counterpart to w-t-e attribute can be added later if/when
somebody finds it useful, and it is perfectly OK to declare that we
cater only to UTF-8 users until that happens.

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

[Junio C Hamano]

Jeff King  writes:

>> (although the calloc() case is slightly different from mallocz()),
>> see: https://public-inbox.org/git/871shum182@evledraar.gmail.com/
>
> Hmm. I do think xmallocz is a bit more explicit instruction of "please
> NUL-terminate this for me" than xcalloc is. So I don't think it's
> inconsistent to say this one is OK, but the trailing-NULL one that you
> linked is not.

Yeah, I too thought "slightly different" was an understatement of
the week ;-).

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

[Jeff King]

On Fri, Feb 16, 2018 at 02:25:41PM -0500, Jeff King wrote:

> On Fri, Feb 16, 2018 at 10:55:58AM -0800, Junio C Hamano wrote:
> 
> > Jeff King  writes:
> > 
> > > So a full proposal would support both cases: "check this out in the
> > > local platform's preferred encoding" and "always check this out in
> > > _this_ encoding". And Lars's proposal is just the second half of that.
> > 
> > Actually, what you seem to take as a whole is just half of the
> > story.  The other half that is an ability to say "what is in the
> > repository for this path is stored in this encoding".  I agree that
> > "check it out in this encoding" is a useful thing to have, and using
> > the in-tree .gitattributes as a place to state the project-wide
> > preference may be OK (and .git/info/attributes should be able to
> > override it if needed -- this probably deserves to be added to a
> > test somewhere by this series).
> 
> If we are just talking about a check-out feature, I'm not sure that the
> in-repository encoding is all that interesting. As with CRLFs, we would
> be declaring UTF-8 as the "canonical" in-repo encoding for such
> conversions. Is there a reason you'd want something else?

Maybe answering my own question: because your encoding of choice does
not round-trip to UTF-8?

In which case yeah, I could see choosing an in-repo encoding to possibly
be useful (but it also seems like a feature that could easily be tacked
on later if somebody cares).

-Peff

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

[Jeff King]

On Fri, Feb 16, 2018 at 10:55:58AM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > So a full proposal would support both cases: "check this out in the
> > local platform's preferred encoding" and "always check this out in
> > _this_ encoding". And Lars's proposal is just the second half of that.
> 
> Actually, what you seem to take as a whole is just half of the
> story.  The other half that is an ability to say "what is in the
> repository for this path is stored in this encoding".  I agree that
> "check it out in this encoding" is a useful thing to have, and using
> the in-tree .gitattributes as a place to state the project-wide
> preference may be OK (and .git/info/attributes should be able to
> override it if needed -- this probably deserves to be added to a
> test somewhere by this series).

If we are just talking about a check-out feature, I'm not sure that the
in-repository encoding is all that interesting. As with CRLFs, we would
be declaring UTF-8 as the "canonical" in-repo encoding for such
conversions. Is there a reason you'd want something else?

If the feature were "the in-repo encoding is X, and I want you to show
me a diff using encoding Y", then I could see the use of that (and I
think for most people's purposes that would be an equally valid solution
to their problem).

-Peff

Re: [PATCH 1/3] grep: move grep_source_init outside critical section

[Junio C Hamano]

Jeff King  writes:

> I think this makes sense. It does blur the memory ownership lines of the
> grep_source, though. Can we make that more clear with a comment here:
>
>> +grep_source_init(, GREP_SOURCE_OID, pathbuf.buf, path, oid);
>> +
>>  #ifndef NO_PTHREADS
>>  if (num_threads) {
>> -add_work(opt, GREP_SOURCE_OID, pathbuf.buf, path, oid);
>> +add_work(opt, );
>>  strbuf_release();
>>  return 0;
>>  } else
>
> like:
>
>   /* leak grep_source, whose fields are now owned by add_work() */
>
> or something? We could even memset() it back to all-zeroes to avoid an
> accidental call to grep_source_clear(), but that's probably unnecessary
> if we have a comment.

I share the same uneasiness about the fuzzy memory ownership this
change brings in.  Thanks for suggesting improvements.

Re: [PATCH 0/1] Colorize some errors on stderr

[Junio C Hamano]

Johannes Schindelin  writes:

> Now, what would be possible solutions for this?
>
> - introduce `int fd` in `want_color()` (and callees) so that we can make
>   a distinction whether we want to detect whether stdout or stderr is 
> connected
>   to a tty
>
> - introduce a separate `want_color_stderr()` (we still would need to decide
>   whether we want a config setting for this)

Between the above two, there probably aren't so big a difference, but
in order to avoid disrupting existing callers of want_color() while
possibly sharing as much code between the old and new callers,
perhaps:

extern int want_color_fd(int fd, int colorbool);
#define want_color(colorbool) want_color_fd(1, (colorbool))
#define want_color_stderr(colorbool) want_color_fd(2, (colorbool))

We should honor configuration at two levels, just like the colors on
stdout, i.e. color in which individual items are painted (e.g.
color.diff.filename, color.advice.hint) and whether we use colors in
UI at all (e.g. color.ui).  I do not think it is necessary or even
at the right granularity to allow settings like "do color stdout but
do not color errors".

> - not color stderr, ever

This is my personal preference, but that does not and should not
carry too much weight ;-)

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

[Junio C Hamano]

Lars Schneider  writes:

>> One thing I find more problematic is that the above places *too*
>> much stress on the UTF-8 centric worldview.  It is perfectly valid
>> to store your text contents encoded in ShiftJIS and check them out
>> as-is, with or without this patch.  It is grossly misleading to say
>> that older versions of Git will check them out in UTF-8.  "will
>> checkout these files as-is without encoding conversion" is a better
>> way to say it, probably.
>
> True. But that's not what I wanted to say in the "pitfalls" section.
> If my Git client supports w-t-e and I add the ShiftJIS encoded
> file "foo.bar" to my repository, then Git will store the file as
> UTF-8 _internally_. That means if you clone my repository and your 
> Git client does _not_ support w-t-e, then you will see "foo.bar" as 
> UTF-8 encoded.

What you wrote implies *more* than that, which is what I had trouble
with.

If you say "what you have is checked out as-is", then it is still
clear that those who use w-t-e to convert non UTF-8 into UTF-8 when
checking in will get UTF-8 out when they use an older version of
Git.  If you say "what you have will be checked out in UTF-8", it
makes it sound as if pre w-t-e Git will somehow reject non UTF-8
in-tree contents, or magically convert anything to UTF-8 while
checking out, which is *not* what you want to imply.

>> Also notice that even in the world with w-t-e, such a project won't
>> benefit from w-t-e at all.  After all, they have been happy using
>> ShiftJIS in repository and using the same encoding on the working
>> tree, and because w-t-e assumes that everybody should be using UTF-8
>> internally, such a project cannot take advantage of the new
>> mechanism.
>
> Agreed. However, people using ShiftJIS are not my target audience.

Be aware that you are writing *not* *solely* for your target
audience.  You write document for everybody, and make sure the
description of a feature makes it clear who the feature primarily
targets and how using (or not using) the feature affects users.

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

[Junio C Hamano]

Jeff King  writes:

> So a full proposal would support both cases: "check this out in the
> local platform's preferred encoding" and "always check this out in
> _this_ encoding". And Lars's proposal is just the second half of that.

Actually, what you seem to take as a whole is just half of the
story.  The other half that is an ability to say "what is in the
repository for this path is stored in this encoding".  I agree that
"check it out in this encoding" is a useful thing to have, and using
the in-tree .gitattributes as a place to state the project-wide
preference may be OK (and .git/info/attributes should be able to
override it if needed -- this probably deserves to be added to a
test somewhere by this series).

Luckily, lack of 'in-repository-encoding' attribute is not a show
stopper for this series.  A later topic could start with "earlier,
in order to make use of w-t-e attribute, you had to have your
contents in UTF-8.  Teach the codepath to honor a new attribute that
tells in what encoding the blob contents are stored." without having
to be a part of this topic.

**Gute Nachrichten**

[Euro Million Promo]

Herzlichen Glückwunsch, Sie haben am 14. Februar 2018 in den monatlichen 
Ziehungen für Euro Millions / Google Promo € 650.000 gewonnen.

Kontaktieren Sie unseren Schadenregulierungsbeauftragten mit den folgenden 
Informationen

Vollständiger Name
Heimatadresse
Geschlecht
Älter
Besetzung
Telefon

Mr.Pianese Germano
Online-Koordinator

Git Stash Creates sh.exe.stackdump (STATUS_STACK_OVERFLOW)

[Derek Foulk]

Hi,

An issue has arisen sometime between 2.16.1.windows.1 and 2.16.1.windows.4 in 
Git.

When you execute `git stash` commands (stash/pop/apply?), a sh.exe.stackdump 
file is generated that contains the following:

Exception: STATUS_STACK_OVERFLOW at rip=7FFB2C324B97
rax=0010 rbx= rcx=0010
rdx=000C rsi=AAE0 rdi=7FFB23B8D4F0
r8 =AA18 r9 =AA50 r10=A000
r11=FFE03E70 r12=B500 r13=0015C000
r14=014C r15=
rbp=AA50 rsp=AA08
program=C:\Program Files\Git\usr\bin\sh.exe, pid 41552, thread unknown (0x949C)
cs=0033 ds=002B es=002B fs=0053 gs=002B ss=002B
Stack trace:
FrameFunctionArgs
000AA50  7FFB2C324B97 (104, 000AAC8, 7FFB2C302178, 000)
000AA50  7FFB2C302277 (7FFB23B8D4F0, 7FFB23B8D4F0, 001, 0230022)
000AAC8  7FFB28D1D1F4 (7FFB2C28, 7FFB2C3021F0, 7FFB2C2EFAE0, 
7FFE)
000AC90  7FFB28D1D071 (9E0900060001, 365AE1E0F447, 023, 
000ADB8)
000AC90  7FFB28D1CA44 (7FFB23B8D700, 000, 000, 7FFB23B8D700)
000AC90  7FFB23AF03A0 (000AC90, 000ADB8, 00F3DF0, 0091380)
000AC90  7FFB23AC37DB (000B5C0, 000, 000B970, 00F3DF0)
000B3F9  7FFB23A9E4D0 (002, 000B7E8, 000, 00180010018)
00180010018  7FFB23A9CFDD (000, 000B720, 000, 000B7C0)
000B720  7FFB23A95543 (000B890, 1C8, 000, 00180271780)
420  7FFB28D1D7F6 (001, 000, 000B970, 0010001)
420  7FFB29C4E4E3 (020, 000, 000BAF0, 001)
420  001800AB022 (000BA90, 000, 000, 00180300748)
000BB10  001800ABBD5 (00100410FBB, 000, 0060004DF10, 001004E9740)
000BD10  0018011C93B (00100410FBB, 000, 0060004DF10, 001004E9740)
000BD10  2DEE458 (00100410FBB, 000, 0060004DF10, 001004E9740)
End of stack trace (more stack frames may be present)

The end result is you cannot pop your stashed changes (the .stackdump file 
injects itself into the stashed changes, then one is created before the pop 
occurs):

I've confirmed updating to the latest Git causes this to occur on three 
different workstations (and repositories).

I've tried downgrading to 2.15, but the issue has persisted (it did not exist 
prior to updating to 2.16).

Thanks,
-Derek

Environment:
Windows 10 (Fall Creators Update)
Git v2.16.1.windows.4 (Default installation options w/ Added context menu 
options)
Git Bash

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

[Jeff King]

On Fri, Feb 16, 2018 at 01:55:02PM +0100, Ævar Arnfjörð Bjarmason wrote:

> On Thu, Feb 15, 2018 at 4:27 PM,   wrote:
> > Since 3733e69464 (use xmallocz to avoid size arithmetic, 2016-02-22) we
> > allocate the buffer for the lower case string with xmallocz(). This
> > already ensures a NUL at the end of the allocated buffer.
> >
> > Remove the unnecessary assignment.
> > [...]
> > for (i = 0; i < len; i++)
> > result[i] = tolower(string[i]);
> > -   result[i] = '\0';
> > return result;
> >  }
> 
> I agree with this approach, but it's worth noting for other reviewers
> that there's been some disagreement here on-list (between Eric & I)
> about whether these sorts of patterns should be removed or kept
> (although the calloc() case is slightly different from mallocz()),
> see: https://public-inbox.org/git/871shum182@evledraar.gmail.com/

Hmm. I do think xmallocz is a bit more explicit instruction of "please
NUL-terminate this for me" than xcalloc is. So I don't think it's
inconsistent to say this one is OK, but the trailing-NULL one that you
linked is not.

I'm not sure that I have a strong opinion on either case. But in general
I'd probably err on the side of leaving such lines in, for the sake of
being explicit.

Of course this particular case could just be:

  char *result = xstrdup(string);
  for (i = 0; result[i]; i++)
result[i] = tolower(result[i]);

I picked the current implementation in 88d5a6f6cd (daemon/config: factor
out duplicate xstrdup_tolower, 2014-05-22) because it might be more
efficient (it avoids an extra copy), but I doubt it matters much in
practice.

-Peff

Re: [PATCH] test-lib.sh: unset XDG_CACHE_HOME

[Jeff King]

On Thu, Feb 15, 2018 at 09:46:04PM -0500, Genki Sky wrote:

> git respects XDG_CACHE_HOME for the credential cache. So, we should
> unset XDG_CACHE_HOME for the test environment, lest a user's custom one
> cause failure in the test.
> 
> For example, t/t0301-credential-cache.sh expects a default directory
> to be used if it hasn't explicitly set XDG_CACHE_HOME.

Yep, this makes perfect sense. I can't believe it took almost a year for
somebody to trigger this. I guess most people don't set XDG_CACHE_HOME.

Thanks.

-Peff

Re: [PATCH v3] worktree: add: fix 'post-checkout' not knowing new worktree location

[Eric Sunshine]

On Fri, Feb 16, 2018 at 11:55 AM, Lars Schneider
 wrote:
>> On 16 Feb 2018, at 00:09, Eric Sunshine  wrote:
>> The hook is run manually, rather than via run_hook_le(), since it needs
>> to change the working directory to that of the worktree, and
>> run_hook_le() does not provide such functionality. As this is a one-off
>> case, adding 'run_hook' overloads which allow the directory to be set
>> does not seem warranted at this time.
>
> Although this is an one-off case, I would still prefer it if all hook
> invocations would happen in a central place to avoid future surprises.

A number of other places in the codebase run hooks manually, so this
is not unprecedented. Rather than adding 'run_hook' overload(s)
specific to this particular case, it would make sense to review all
such places and design the API of the new overloads to handle _all_
those cases (with the hope of avoiding adding new ad-hoc overloads
each time). But, that's outside the scope of this bug fix.

>> post_checkout_hook () {
>> + gitdir=${1:-.git}
>> + test_when_finished "rm -f $gitdir/hooks/post-checkout" &&
>> + mkdir -p $gitdir/hooks &&
>> + write_script $gitdir/hooks/post-checkout <<-\EOF
>> + {
>> + echo $*
>> + git rev-parse --git-dir --show-toplevel
>
> I also checked `pwd` here in my suggested test case.
> I assume you think this check is not necessary?

I do think it's a good idea, and it is still being tested but not in
quite the same way. I removed the explicit 'pwd' from the output
because I didn't want to deal with potential fallout on Windows. In
particular, your test used raw 'pwd' for the "actual" file but
'$(pwd)' for "expect", which I think would have run afoul on Windows
since '$(pwd)' is meant only to compare output of _Git_ commands,
whereas raw 'pwd' is not a Git command. So, I think the test would
have needed to use raw 'pwd' for the "expect" file, as well. But,
since I don't have Windows on which to test, I decided to avoid that
potential mess by checking 'pwd' in a different way. Details below.

>> + } >hook.actual
>>   EOF
>> }
>>
>> test_expect_success '"add" invokes post-checkout hook (branch)' '
>>   post_checkout_hook &&
>> - printf "%s %s 1\n" $_z40 $(git rev-parse HEAD) >hook.expect &&
>> + {
>> + echo $_z40 $(git rev-parse HEAD) 1 &&
>> + echo $(pwd)/.git/worktrees/gumby &&
>> + echo $(pwd)/gumby
>> + } >hook.expect &&
>>   git worktree add gumby &&
>> - test_cmp hook.expect hook.actual
>> + test_cmp hook.expect gumby/hook.actual
>> '

The explicit 'pwd' check from your test is still here, but is now
implicit, so more subtle. Specifically, the hook now emits "actual"
within the current working directory (the location 'pwd' would
report), and 'test_cmp' looks for the "actual" file at that location.
The net result is 'pwd' is effectively, though implicitly, recorded by
the location of the "actual" file itself. If 'pwd' is wrong (that is,
if the chdir() was wrong or missing), then "actual" would not end up
at the correct location and the 'test_cmp' would fail.

Re: [PATCH] merge: allow fast-forward when merging a tracked tag

[Junio C Hamano]

Eric Sunshine  writes:

>> +   tag_ref = xstrfmt("refs/tags/%s",
>> + ((struct tag 
>> *)merge_remote_util(commit)->obj)->tag);
>
> xstrfmt() allocates a new string...
>
>> +   if (!read_ref(tag_ref, ) &&
>> +   !oidcmp(, _remote_util(commit)->obj->oid))
>> +   return 0;
>
> ...which is leaked here...
>
>> +
>> +   /*
>> +* Otherwise, we are playing an integrator's role, making a
>> +* merge with a throw-away tag from a contributor with
>> +* something like "git pull $contributor $signed_tag".
>> +* We want to forbid such a merge from fast-forwarding
>> +* by default; otherwise we would not keep the signature
>> +* anywhere.
>> +*/
>> +   return 1;
>
> ...and here.

OK.  Thanks.

Re: [PATCH v3] worktree: add: fix 'post-checkout' not knowing new worktree location

[Junio C Hamano]

Eric Sunshine  writes:

> ...
> The hook is run manually, rather than via run_hook_le(), since it needs
> to change the working directory to that of the worktree, and
> run_hook_le() does not provide such functionality. As this is a one-off
> case, adding 'run_hook' overloads which allow the directory to be set
> does not seem warranted at this time.
>
> Reported-by: Lars Schneider 
> Signed-off-by: Eric Sunshine 
> ---
>
> This is a re-roll of [1] which fixes "git worktree add" to provide
> proper context to the 'post-checkout' hook so that the hook knows the
> location of the newly-created worktree.
>
> Changes since v2:
>
> * Fix crash due to missing NULL-terminator on 'env' list passed to
>   run_command().

Thanks.  This matches what I had (v2 plus manual fixup while
queueing) and looks good.

As long as the "hand-rolled" implementation uses the same
find_hook() helper used in run_hook[lv]e, I do not think a one-off
invocation of the hook is not too bad, at least for now.

[ANNOUNCE] Git v2.16.2

[Junio C Hamano]

The latest maintenance release Git v2.16.2 is now available at
the usual places, with small fixes that are already in the 
'master' front.

The tarballs are found at:

https://www.kernel.org/pub/software/scm/git/

The following public repositories all have a copy of the 'v2.16.2'
tag and the 'maint' branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://github.com/gitster/git



Git v2.16.2 Release Notes
=

Fixes since v2.16.1
---

 * An old regression in "git describe --all $annotated_tag^0" has been
   fixed.

 * "git svn dcommit" did not take into account the fact that a
   svn+ssh:// URL with a username@ (typically used for pushing) refers
   to the same SVN repository without the username@ and failed when
   svn.pushmergeinfo option is set.

 * "git merge -Xours/-Xtheirs" learned to use our/their version when
   resolving a conflicting updates to a symbolic link.

 * "git clone $there $here" is allowed even when here directory exists
   as long as it is an empty directory, but the command incorrectly
   removed it upon a failure of the operation.

 * "git stash -- " incorrectly blew away untracked files in
   the directory that matched the pathspec, which has been corrected.

 * "git add -p" was taught to ignore local changes to submodules as
   they do not interfere with the partial addition of regular changes
   anyway.


Also contains various documentation updates and code clean-ups.



Changes since v2.16.1 are as follows:

Andreas G. Schacker (1):
  doc/read-tree: remove obsolete remark

Daniel Knittl-Frank (1):
  describe: prepend "tags/" when describing tags with embedded name

Jason Merrill (1):
  git-svn: fix svn.pushmergeinfo handling of svn+ssh usernames.

Jeff King (4):
  t5600: fix outdated comment about unborn HEAD
  t5600: modernize style
  clone: factor out dir_exists() helper
  clone: do not clean up directories we didn't create

Junio C Hamano (2):
  merge: teach -Xours/-Xtheirs to symbolic link merge
  Git 2.16.2

Nguyễn Thái Ngọc Duy (1):
  add--interactive: ignore submodule changes except HEAD

René Scharfe (9):
  commit: avoid allocation in clear_commit_marks_many()
  commit: use clear_commit_marks_many() in remove_redundant()
  ref-filter: use clear_commit_marks_many() in do_merge_filter()
  object: add clear_commit_marks_all()
  bisect: avoid using the rev_info flag leak_pending
  bundle: avoid using the rev_info flag leak_pending
  checkout: avoid using the rev_info flag leak_pending
  revision: remove the unused flag leak_pending
  commit: remove unused function clear_commit_marks_for_object_array()

Thomas Gummerer (1):
  stash: don't delete untracked files that match pathspec

Ævar Arnfjörð Bjarmason (2):
  perf: amend the grep tests to test grep.threads
  cat-file doc: document that -e will return some output

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

[Todd Zullinger]

Ævar Arnfjörð Bjarmason wrote:
> On Thu, Feb 15 2018, Todd Zullinger jotted:
>> What about moving perl/Git/FromCPAN to perl/FromCPAN and
>> then including perl/FromCPAN in LIB_PERL{,_GEN} only if
>> NO_PERL_CPAN_FALLBACKS is unset?
>>
>>  LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm 
>> perl/Git/*/*/*.pm)
>> +ifndef NO_PERL_CPAN_FALLBACKS
>> +LIB_PERL += $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
>> +endif
>>  LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
>>
>> I haven't tested that at all, so it could be broken in many
>> ways.
> 
> Yes that's a much better idea, it evades the whole problem of conflating
> the perl/Git* glob.

I did test this yesterday and it seems to work well.  Here
it is in patch form, in case it's helpful to you for the
next version.  It might well be better as part of a commit
teaching Git::LoadCPAN to respect NO_PERL_CPAN_FALLBACKS.

 8< 
From: Todd Zullinger 
Subject: [PATCH] Makefile: add NO_PERL_CPAN_FALLBACKS 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_FALLBACKS to easily avoid installing these fallback perl
CPAN modules.

Signed-off-by: Todd Zullinger 
---
 Makefile| 6 ++
 perl/{Git => }/FromCPAN/.gitattributes  | 0
 perl/{Git => }/FromCPAN/Error.pm| 0
 perl/{Git => }/FromCPAN/Mail/.gitattributes | 0
 perl/{Git => }/FromCPAN/Mail/Address.pm | 0
 5 files changed, 6 insertions(+)
 rename perl/{Git => }/FromCPAN/.gitattributes (100%)
 rename perl/{Git => }/FromCPAN/Error.pm (100%)
 rename perl/{Git => }/FromCPAN/Mail/.gitattributes (100%)
 rename perl/{Git => }/FromCPAN/Mail/Address.pm (100%)

diff --git a/Makefile b/Makefile
index 5bcd83ddf3..838b3c6393 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_FALLBACKS 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).
 #
@@ -2297,6 +2300,9 @@ po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
$(QUIET_MSGFMT)mkdir -p $(dir $@) && $(MSGFMT) -o $@ $<
 
 LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm 
perl/Git/*/*/*.pm)
+ifndef NO_PERL_CPAN_FALLBACKS
+LIB_PERL += $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
+endif
 LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
 
 ifndef NO_PERL
diff --git a/perl/Git/FromCPAN/.gitattributes b/perl/FromCPAN/.gitattributes
similarity index 100%
rename from perl/Git/FromCPAN/.gitattributes
rename to perl/FromCPAN/.gitattributes
diff --git a/perl/Git/FromCPAN/Error.pm b/perl/FromCPAN/Error.pm
similarity index 100%
rename from perl/Git/FromCPAN/Error.pm
rename to perl/FromCPAN/Error.pm
diff --git a/perl/Git/FromCPAN/Mail/.gitattributes 
b/perl/FromCPAN/Mail/.gitattributes
similarity index 100%
rename from perl/Git/FromCPAN/Mail/.gitattributes
rename to perl/FromCPAN/Mail/.gitattributes
diff --git a/perl/Git/FromCPAN/Mail/Address.pm b/perl/FromCPAN/Mail/Address.pm
similarity index 100%
rename from perl/Git/FromCPAN/Mail/Address.pm
rename to perl/FromCPAN/Mail/Address.pm
-- 
2.16.1

-- 
Todd
~~
Damn you and your estrogenical treachery!
-- Stewie Griffin

[PATCH 05/16] object-store: close all packs upon clearing the object store

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 builtin/am.c   | 2 +-
 builtin/clone.c| 2 +-
 builtin/fetch.c| 2 +-
 builtin/merge.c| 2 +-
 builtin/receive-pack.c | 2 +-
 object.c   | 6 ++
 packfile.c | 4 ++--
 packfile.h | 2 +-
 8 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 5bdd2d7578..4762a702e3 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1859,7 +1859,7 @@ static void am_run(struct am_state *state, int resume)
 */
if (!state->rebasing) {
am_destroy(state);
-   close_all_packs();
+   close_all_packs(_repository->objects);
run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
}
 }
diff --git a/builtin/clone.c b/builtin/clone.c
index 101c27a593..13cfaa6488 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1217,7 +1217,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
transport_disconnect(transport);
 
if (option_dissociate) {
-   close_all_packs();
+   close_all_packs(_repository->objects);
dissociate_from_references();
}
 
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8ee998ea2e..4d72efca78 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1478,7 +1478,7 @@ int cmd_fetch(int argc, const char **argv, const char 
*prefix)
 
string_list_clear(, 0);
 
-   close_all_packs();
+   close_all_packs(_repository->objects);
 
argv_array_pushl(_gc_auto, "gc", "--auto", NULL);
if (verbosity < 0)
diff --git a/builtin/merge.c b/builtin/merge.c
index 30264cfd7c..907ae44ab5 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -411,7 +411,7 @@ static void finish(struct commit *head_commit,
 * We ignore errors in 'gc --auto', since the
 * user should see them.
 */
-   close_all_packs();
+   close_all_packs(_repository->objects);
run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
}
}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index b2eac79a6e..954fc72c7c 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2027,7 +2027,7 @@ int cmd_receive_pack(int argc, const char **argv, const 
char *prefix)
proc.git_cmd = 1;
proc.argv = argv_gc_auto;
 
-   close_all_packs();
+   close_all_packs(_repository->objects);
if (!start_command()) {
if (use_sideband)
copy_to_sideband(proc.err, -1, NULL);
diff --git a/object.c b/object.c
index c76b62572a..34daaf37b3 100644
--- a/object.c
+++ b/object.c
@@ -4,6 +4,7 @@
 #include "tree.h"
 #include "commit.h"
 #include "tag.h"
+#include "packfile.h"
 
 static struct object **obj_hash;
 static int nr_objs, obj_hash_size;
@@ -469,8 +470,5 @@ void raw_object_store_clear(struct raw_object_store *o)
 
while (!list_empty(>packed_git_mru))
list_del(>packed_git_mru);
-   /*
-* TODO: call close_all_packs once migrated to
-* take an object store argument
-*/
+   close_all_packs(o);
 }
diff --git a/packfile.c b/packfile.c
index d41e4c83d0..511a2b0cdf 100644
--- a/packfile.c
+++ b/packfile.c
@@ -311,11 +311,11 @@ static void close_pack(struct packed_git *p)
close_pack_index(p);
 }
 
-void close_all_packs(void)
+void close_all_packs(struct raw_object_store *o)
 {
struct packed_git *p;
 
-   for (p = the_repository->objects.packed_git; p; p = p->next)
+   for (p = o->packed_git; p; p = p->next)
if (p->do_not_close)
die("BUG: want to close pack marked 'do-not-close'");
else
diff --git a/packfile.h b/packfile.h
index a7fca598d6..6a2c57045c 100644
--- a/packfile.h
+++ b/packfile.h
@@ -63,7 +63,7 @@ extern void close_pack_index(struct packed_git *);
 
 extern unsigned char *use_pack(struct packed_git *, struct pack_window **, 
off_t, unsigned long *);
 extern void close_pack_windows(struct packed_git *);
-extern void close_all_packs(void);
+extern void close_all_packs(struct raw_object_store *o);
 extern void unuse_pack(struct pack_window **);
 extern void clear_delta_base_cache(void);
 extern struct packed_git *add_packed_git(const char *path, size_t path_len, 
int local);
-- 
2.16.1.291.g4437f3f132-goog

[PATCH 04/16] object-store: move packed_git and packed_git_mru to object store

[Stefan Beller]

In a process with multiple repositories open, packfile accessors
should be associated to a single repository and not shared globally.
Move packed_git and packed_git_mru into the_repository and adjust
callers to reflect this.

Patch generated by

 1. Moving the struct packed_git declaration to object-store.h
and packed_git, packed_git_mru globals to struct object_store.

 2. Applying the semantic patch to adjust callers.
  @@ @@
  - packed_git
  + the_repository->objects.packed_git

  @@ @@
  - packed_git_mru
  + the_repository->objects.packed_git_mru

 3. Applying line wrapping fixes from "make style" to break the
resulting long lines.

 4. Adding missing #includes of repository.h and object-store.h
where needed.

 5. As the packfiles are now owned by an objectstore/repository, which
is ephemeral unlike globals, we introduce memory leaks. So address
them in raw_object_store_clear().

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 builtin/count-objects.c  |  6 --
 builtin/fsck.c   |  7 +--
 builtin/gc.c |  4 +++-
 builtin/index-pack.c |  1 +
 builtin/pack-objects.c   | 19 +++
 builtin/pack-redundant.c |  6 --
 builtin/receive-pack.c   |  1 +
 cache.h  | 29 -
 fast-import.c|  6 --
 http-backend.c   |  6 --
 http-push.c  |  1 +
 http-walker.c|  1 +
 http.c   |  1 +
 object-store.h   | 36 +++-
 object.c |  7 +++
 pack-bitmap.c|  4 +++-
 pack-check.c |  1 +
 pack-revindex.c  |  1 +
 packfile.c   | 39 ---
 reachable.c  |  1 +
 repository.c |  3 +++
 server-info.c|  6 --
 sha1_name.c  |  5 +++--
 23 files changed, 118 insertions(+), 73 deletions(-)

diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 33343818c8..9334648dcc 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -7,6 +7,8 @@
 #include "cache.h"
 #include "config.h"
 #include "dir.h"
+#include "repository.h"
+#include "object-store.h"
 #include "builtin.h"
 #include "parse-options.h"
 #include "quote.h"
@@ -120,9 +122,9 @@ int cmd_count_objects(int argc, const char **argv, const 
char *prefix)
struct strbuf loose_buf = STRBUF_INIT;
struct strbuf pack_buf = STRBUF_INIT;
struct strbuf garbage_buf = STRBUF_INIT;
-   if (!packed_git)
+   if (!the_repository->objects.packed_git)
prepare_packed_git();
-   for (p = packed_git; p; p = p->next) {
+   for (p = the_repository->objects.packed_git; p; p = p->next) {
if (!p->pack_local)
continue;
if (open_pack_index(p))
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 908e4f092a..2e99e02128 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -2,6 +2,7 @@
 #include "cache.h"
 #include "repository.h"
 #include "config.h"
+#include "object-store.h"
 #include "commit.h"
 #include "tree.h"
 #include "blob.h"
@@ -729,7 +730,8 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
prepare_packed_git();
 
if (show_progress) {
-   for (p = packed_git; p; p = p->next) {
+   for (p = the_repository->objects.packed_git; p;
+p = p->next) {
if (open_pack_index(p))
continue;
total += p->num_objects;
@@ -737,7 +739,8 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
 
progress = start_progress(_("Checking 
objects"), total);
}
-   for (p = packed_git; p; p = p->next) {
+   for (p = the_repository->objects.packed_git; p;
+p = p->next) {
/* verify gives error messages itself */
if (verify_pack(p, fsck_obj_buffer,
progress, count))
diff --git a/builtin/gc.c b/builtin/gc.c
index 77fa720bd0..96ff790867 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -11,6 +11,7 @@
  */
 
 #include "builtin.h"
+#include "repository.h"
 #include "config.h"
 #include "tempfile.h"
 #include "lockfile.h"
@@ -18,6 +19,7 @@
 #include "run-command.h"
 #include "sigchain.h"
 #include "argv-array.h"
+#include "object-store.h"
 #include "commit.h"
 #include "packfile.h"
 
@@ -173,7 +175,7 @@ static int too_many_packs(void)
return 0;
 
prepare_packed_git();
-  

[PATCH 08/16] sha1_file: add raw_object_store argument to alt_odb_usable

[Stefan Beller]

Add a raw_object_store to alt_odb_usable to be more specific about which
repository to act on. The choice of the repository is delegated to its
only caller link_alt_odb_entry.

Signed-off-by: Stefan Beller 
---
 sha1_file.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 1348dce68f..a8e23bd2f8 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -346,7 +346,9 @@ static const char *alt_sha1_path(struct 
alternate_object_database *alt,
 /*
  * Return non-zero iff the path is usable as an alternate object database.
  */
-static int alt_odb_usable(struct strbuf *path, const char *normalized_objdir)
+static int alt_odb_usable(struct raw_object_store *o,
+ struct strbuf *path,
+ const char *normalized_objdir)
 {
struct alternate_object_database *alt;
 
@@ -362,7 +364,7 @@ static int alt_odb_usable(struct strbuf *path, const char 
*normalized_objdir)
 * Prevent the common mistake of listing the same
 * thing twice, or object directory itself.
 */
-   for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next) {
+   for (alt = o->alt_odb_list; alt; alt = alt->next) {
if (!fspathcmp(path->buf, alt->path))
return 0;
}
@@ -414,7 +416,7 @@ static int link_alt_odb_entry(const char *entry, const char 
*relative_base,
while (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')
strbuf_setlen(, pathbuf.len - 1);
 
-   if (!alt_odb_usable(, normalized_objdir)) {
+   if (!alt_odb_usable(_repository->objects, , 
normalized_objdir)) {
strbuf_release();
return -1;
}
-- 
2.16.1.291.g4437f3f132-goog

[PATCH 16/16] sha1_file: allow sha1_loose_object_info to handle arbitrary object stores

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 sha1_file.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 4cbff471a2..31be57249f 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1154,7 +1154,8 @@ int parse_sha1_header(const char *hdr, unsigned long 
*sizep)
return parse_sha1_header_extended(hdr, , 0);
 }
 
-static int sha1_loose_object_info(const unsigned char *sha1,
+static int sha1_loose_object_info(struct raw_object_store *o,
+ const unsigned char *sha1,
  struct object_info *oi,
  int flags)
 {
@@ -1180,14 +1181,14 @@ static int sha1_loose_object_info(const unsigned char 
*sha1,
if (!oi->typep && !oi->typename && !oi->sizep && !oi->contentp) {
const char *path;
struct stat st;
-   if (stat_sha1_file(_repository->objects, sha1, , ) 
< 0)
+   if (stat_sha1_file(o, sha1, , ) < 0)
return -1;
if (oi->disk_sizep)
*oi->disk_sizep = st.st_size;
return 0;
}
 
-   map = map_sha1_file(_repository->objects, sha1, );
+   map = map_sha1_file(o, sha1, );
if (!map)
return -1;
 
@@ -1275,7 +1276,7 @@ int sha1_object_info_extended(const unsigned char *sha1, 
struct object_info *oi,
break;
 
/* Most likely it's a loose object. */
-   if (!sha1_loose_object_info(real, oi, flags))
+   if (!sha1_loose_object_info(_repository->objects, real, oi, 
flags))
return 0;
 
/* Not a loose object; someone else may have just packed it. */
-- 
2.16.1.291.g4437f3f132-goog

[PATCH 11/16] sha1_file: allow sha1_file_name to handle arbitrary object stores

[Stefan Beller]

While at it, move the declaration to object-store.h, where it should
be easier to find.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 cache.h|  6 --
 http-walker.c  |  3 ++-
 http.c |  5 ++---
 object-store.h |  6 ++
 sha1_file.c| 12 ++--
 5 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/cache.h b/cache.h
index d0c1a533c5..2495c7081d 100644
--- a/cache.h
+++ b/cache.h
@@ -961,12 +961,6 @@ extern void check_repository_format(void);
 #define DATA_CHANGED0x0020
 #define TYPE_CHANGED0x0040
 
-/*
- * Put in `buf` the name of the file in the local object database that
- * would be used to store a loose object with the specified sha1.
- */
-extern void sha1_file_name(struct strbuf *buf, const unsigned char *sha1);
-
 /*
  * Return an abbreviated sha1 unique within this repository's object database.
  * The result will be at least `len` characters long, and will be NUL
diff --git a/http-walker.c b/http-walker.c
index 8bb5d991bb..59bc0417d2 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "repository.h"
 #include "commit.h"
 #include "walker.h"
 #include "http.h"
@@ -546,7 +547,7 @@ static int fetch_object(struct walker *walker, unsigned 
char *sha1)
ret = error("File %s has bad hash", hex);
} else if (req->rename < 0) {
struct strbuf buf = STRBUF_INIT;
-   sha1_file_name(, req->sha1);
+   sha1_file_name(_repository->objects, , req->sha1);
ret = error("unable to write sha1 filename %s", buf.buf);
strbuf_release();
}
diff --git a/http.c b/http.c
index a4a9e583c7..058436d9d3 100644
--- a/http.c
+++ b/http.c
@@ -2247,7 +2247,7 @@ struct http_object_request *new_http_object_request(const 
char *base_url,
hashcpy(freq->sha1, sha1);
freq->localfile = -1;
 
-   sha1_file_name(, sha1);
+   sha1_file_name(_repository->objects, , sha1);
snprintf(freq->tmpfile, sizeof(freq->tmpfile),
 "%s.temp", filename.buf);
 
@@ -2396,8 +2396,7 @@ int finish_http_object_request(struct http_object_request 
*freq)
unlink_or_warn(freq->tmpfile);
return -1;
}
-
-   sha1_file_name(, freq->sha1);
+   sha1_file_name(_repository->objects, , freq->sha1);
freq->rename = finalize_object_file(freq->tmpfile, filename.buf);
strbuf_release();
 
diff --git a/object-store.h b/object-store.h
index 280e8e16ad..5f6beb9d91 100644
--- a/object-store.h
+++ b/object-store.h
@@ -67,6 +67,12 @@ extern struct packed_git {
char pack_name[FLEX_ARRAY]; /* more */
 } *packed_git;
 
+/*
+ * Put in `buf` the name of the file in the local object database that
+ * would be used to store a loose object with the specified sha1.
+ */
+void sha1_file_name(struct raw_object_store *o, struct strbuf *buf, const 
unsigned char *sha1);
+
 void prepare_alt_odb(struct raw_object_store *o);
 
 #endif /* OBJECT_STORE_H */
diff --git a/sha1_file.c b/sha1_file.c
index 04a81e29f2..d64cdbb494 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -323,9 +323,9 @@ static void fill_sha1_path(struct strbuf *buf, const 
unsigned char *sha1)
}
 }
 
-void sha1_file_name(struct strbuf *buf, const unsigned char *sha1)
+void sha1_file_name(struct raw_object_store *o, struct strbuf *buf, const 
unsigned char *sha1)
 {
-   strbuf_addstr(buf, get_object_directory());
+   strbuf_addstr(buf, o->objectdir);
strbuf_addch(buf, '/');
fill_sha1_path(buf, sha1);
 }
@@ -717,7 +717,7 @@ static int check_and_freshen_local(const unsigned char 
*sha1, int freshen)
static struct strbuf buf = STRBUF_INIT;
 
strbuf_reset();
-   sha1_file_name(, sha1);
+   sha1_file_name(_repository->objects, , sha1);
 
return check_and_freshen_file(buf.buf, freshen);
 }
@@ -878,7 +878,7 @@ static int stat_sha1_file(const unsigned char *sha1, struct 
stat *st,
static struct strbuf buf = STRBUF_INIT;
 
strbuf_reset();
-   sha1_file_name(, sha1);
+   sha1_file_name(_repository->objects, , sha1);
*path = buf.buf;
 
if (!lstat(*path, st))
@@ -907,7 +907,7 @@ static int open_sha1_file(const unsigned char *sha1, const 
char **path)
static struct strbuf buf = STRBUF_INIT;
 
strbuf_reset();
-   sha1_file_name(, sha1);
+   sha1_file_name(_repository->objects, , sha1);
*path = buf.buf;
 
fd = git_open(*path);
@@ -1592,7 +1592,7 @@ static int write_loose_object(const unsigned char *sha1, 
char *hdr, int hdrlen,
static struct strbuf filename = STRBUF_INIT;
 
strbuf_reset();
-   sha1_file_name(, sha1);
+   sha1_file_name(_repository->objects, , sha1);
 
fd = create_tmpfile(_file, filename.buf);
if (fd < 0) {
-- 
2.16.1.291.g4437f3f132-goog

[PATCH 09/16] sha1_file: allow link_alt_odb_entries to handle arbitrary object stores

[Stefan Beller]

Actually this also allows read_info_alternates and link_alt_odb_entry to
handle arbitrary object stores, but link_alt_odb_entries is the most
interesting function in this set of functions, hence the commit subject.

These functions span a strongly connected component in the function
graph, i.e. the recursive call chain might look like

  -> link_alt_odb_entries
-> link_alt_odb_entry
  -> read_info_alternates
-> link_alt_odb_entries

That is why we need to convert all these functions at the same time.

Signed-off-by: Jonathan Nieder 
Signed-off-by: Stefan Beller 
---
 cache.h |  4 
 sha1_file.c | 42 +-
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/cache.h b/cache.h
index 018a297e10..12bb42f951 100644
--- a/cache.h
+++ b/cache.h
@@ -1593,6 +1593,10 @@ struct alternate_object_database {
char loose_objects_subdir_seen[256];
struct oid_array loose_objects_cache;
 
+   /*
+* Path to the alternative object store. If this is a relative path,
+* it is relative to the current working directory.
+*/
char path[FLEX_ARRAY];
 };
 extern void prepare_alt_odb(void);
diff --git a/sha1_file.c b/sha1_file.c
index a8e23bd2f8..c4255a2da6 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -389,9 +389,11 @@ static int alt_odb_usable(struct raw_object_store *o,
  * SHA1, an extra slash for the first level indirection, and the
  * terminating NUL.
  */
-static void read_info_alternates(const char * relative_base, int depth);
-static int link_alt_odb_entry(const char *entry, const char *relative_base,
-   int depth, const char *normalized_objdir)
+static void read_info_alternates(struct raw_object_store *o,
+const char *relative_base,
+int depth);
+static int link_alt_odb_entry(struct raw_object_store *o, const char *entry,
+   const char *relative_base, int depth, const char *normalized_objdir)
 {
struct alternate_object_database *ent;
struct strbuf pathbuf = STRBUF_INIT;
@@ -416,7 +418,7 @@ static int link_alt_odb_entry(const char *entry, const char 
*relative_base,
while (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')
strbuf_setlen(, pathbuf.len - 1);
 
-   if (!alt_odb_usable(_repository->objects, , 
normalized_objdir)) {
+   if (!alt_odb_usable(o, , normalized_objdir)) {
strbuf_release();
return -1;
}
@@ -424,12 +426,12 @@ static int link_alt_odb_entry(const char *entry, const 
char *relative_base,
ent = alloc_alt_odb(pathbuf.buf);
 
/* add the alternate entry */
-   *the_repository->objects.alt_odb_tail = ent;
-   the_repository->objects.alt_odb_tail = &(ent->next);
+   *o->alt_odb_tail = ent;
+   o->alt_odb_tail = &(ent->next);
ent->next = NULL;
 
/* recursively add alternates */
-   read_info_alternates(pathbuf.buf, depth + 1);
+   read_info_alternates(o, pathbuf.buf, depth + 1);
 
strbuf_release();
return 0;
@@ -464,8 +466,8 @@ static const char *parse_alt_odb_entry(const char *string,
return end;
 }
 
-static void link_alt_odb_entries(const char *alt, int sep,
-const char *relative_base, int depth)
+static void link_alt_odb_entries(struct raw_object_store *o, const char *alt,
+int sep, const char *relative_base, int depth)
 {
struct strbuf objdirbuf = STRBUF_INIT;
struct strbuf entry = STRBUF_INIT;
@@ -479,7 +481,7 @@ static void link_alt_odb_entries(const char *alt, int sep,
return;
}
 
-   strbuf_add_absolute_path(, get_object_directory());
+   strbuf_add_absolute_path(, o->objectdir);
if (strbuf_normalize_path() < 0)
die("unable to normalize object directory: %s",
objdirbuf.buf);
@@ -488,13 +490,16 @@ static void link_alt_odb_entries(const char *alt, int sep,
alt = parse_alt_odb_entry(alt, sep, );
if (!entry.len)
continue;
-   link_alt_odb_entry(entry.buf, relative_base, depth, 
objdirbuf.buf);
+   link_alt_odb_entry(o, entry.buf,
+  relative_base, depth, objdirbuf.buf);
}
strbuf_release();
strbuf_release();
 }
 
-static void read_info_alternates(const char * relative_base, int depth)
+static void read_info_alternates(struct raw_object_store *o,
+const char *relative_base,
+int depth)
 {
char *path;
struct strbuf buf = STRBUF_INIT;
@@ -506,7 +511,7 @@ static void read_info_alternates(const char * 
relative_base, int depth)
return;
}
 
-   link_alt_odb_entries(buf.buf, '\n', relative_base, depth);
+   

[PATCH 07/16] pack: move approximate object count to object store

[Stefan Beller]

The approximate_object_count() function maintains a rough count of
objects in a repository to estimate how long object name abbreviates
should be.  Object names are scoped to a repository and the
appropriate length may differ by repository, so the object count
should not be global.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 object-store.h | 10 +-
 packfile.c | 11 +--
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/object-store.h b/object-store.h
index 794cb0af59..80a2ca6f75 100644
--- a/object-store.h
+++ b/object-store.h
@@ -23,6 +23,14 @@ struct raw_object_store {
 
unsigned ignore_env : 1;
 
+   /*
+* A fast, rough count of the number of objects in the repository.
+* These two fields are not meant for direct access. Use
+* approximate_object_count() instead.
+*/
+   unsigned long approximate_object_count;
+   unsigned approximate_object_count_valid : 1;
+
/*
 * Whether packed_git has already been populated with this repository's
 * packs.
@@ -31,7 +39,7 @@ struct raw_object_store {
 };
 
 #define MRU_LIST_INIT {NULL, NULL}
-#define RAW_OBJECT_STORE_INIT { NULL, NULL, MRU_LIST_INIT, NULL, NULL, 0, 0 }
+#define RAW_OBJECT_STORE_INIT { NULL, NULL, MRU_LIST_INIT, NULL, NULL, 0, 0, 
0, 0 }
 
 void raw_object_store_clear(struct raw_object_store *o);
 
diff --git a/packfile.c b/packfile.c
index a8a2e7fe43..693bafbc98 100644
--- a/packfile.c
+++ b/packfile.c
@@ -803,8 +803,6 @@ static void prepare_packed_git_one(char *objdir, int local)
strbuf_release();
 }
 
-static int approximate_object_count_valid;
-
 /*
  * Give a fast, rough count of the number of objects in the repository. This
  * ignores loose objects completely. If you have a lot of them, then either
@@ -814,8 +812,8 @@ static int approximate_object_count_valid;
  */
 unsigned long approximate_object_count(void)
 {
-   static unsigned long count;
-   if (!approximate_object_count_valid) {
+   if (!the_repository->objects.approximate_object_count_valid) {
+   unsigned long count;
struct packed_git *p;
 
prepare_packed_git();
@@ -825,8 +823,9 @@ unsigned long approximate_object_count(void)
continue;
count += p->num_objects;
}
+   the_repository->objects.approximate_object_count = count;
}
-   return count;
+   return the_repository->objects.approximate_object_count;
 }
 
 static void *get_next_packed_git(const void *p)
@@ -901,7 +900,7 @@ void prepare_packed_git(void)
 
 void reprepare_packed_git(void)
 {
-   approximate_object_count_valid = 0;
+   the_repository->objects.approximate_object_count_valid = 0;
the_repository->objects.packed_git_initialized = 0;
prepare_packed_git();
 }
-- 
2.16.1.291.g4437f3f132-goog

[PATCH 06/16] pack: move prepare_packed_git_run_once to object store

[Stefan Beller]

Each repository's object store can be initialized independently, so
they must not share a run_once variable.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 object-store.h | 8 +++-
 packfile.c | 7 +++
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/object-store.h b/object-store.h
index 024ccc91e9..794cb0af59 100644
--- a/object-store.h
+++ b/object-store.h
@@ -22,10 +22,16 @@ struct raw_object_store {
struct alternate_object_database **alt_odb_tail;
 
unsigned ignore_env : 1;
+
+   /*
+* Whether packed_git has already been populated with this repository's
+* packs.
+*/
+   unsigned packed_git_initialized : 1;
 };
 
 #define MRU_LIST_INIT {NULL, NULL}
-#define RAW_OBJECT_STORE_INIT { NULL, NULL, MRU_LIST_INIT, NULL, NULL, 0 }
+#define RAW_OBJECT_STORE_INIT { NULL, NULL, MRU_LIST_INIT, NULL, NULL, 0, 0 }
 
 void raw_object_store_clear(struct raw_object_store *o);
 
diff --git a/packfile.c b/packfile.c
index 511a2b0cdf..a8a2e7fe43 100644
--- a/packfile.c
+++ b/packfile.c
@@ -884,12 +884,11 @@ static void prepare_packed_git_mru(void)
list_add_tail(>mru, _repository->objects.packed_git_mru);
 }
 
-static int prepare_packed_git_run_once = 0;
 void prepare_packed_git(void)
 {
struct alternate_object_database *alt;
 
-   if (prepare_packed_git_run_once)
+   if (the_repository->objects.packed_git_initialized)
return;
prepare_packed_git_one(get_object_directory(), 1);
prepare_alt_odb();
@@ -897,13 +896,13 @@ void prepare_packed_git(void)
prepare_packed_git_one(alt->path, 0);
rearrange_packed_git();
prepare_packed_git_mru();
-   prepare_packed_git_run_once = 1;
+   the_repository->objects.packed_git_initialized = 1;
 }
 
 void reprepare_packed_git(void)
 {
approximate_object_count_valid = 0;
-   prepare_packed_git_run_once = 0;
+   the_repository->objects.packed_git_initialized = 0;
prepare_packed_git();
 }
 
-- 
2.16.1.291.g4437f3f132-goog

[PATCH 02/16] object-store: move alt_odb_list and alt_odb_tail to object store

[Stefan Beller]

In a process with multiple repositories open, alternates should be
associated to a single repository and not shared globally. Move
alt_odb_list and alt_odb_tail into the_repository and adjust callers
to reflect this.

Now that the alternative object data base is per repository, we're
leaking its memory upon freeing a repository. The next patch plugs
this hole.

No functional change intended.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 builtin/fsck.c |  4 +++-
 cache.h|  4 ++--
 object-store.h |  7 ++-
 packfile.c |  3 ++-
 sha1_file.c| 25 -
 sha1_name.c|  3 ++-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 7a8a679d4f..908e4f092a 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "cache.h"
+#include "repository.h"
 #include "config.h"
 #include "commit.h"
 #include "tree.h"
@@ -716,7 +717,8 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
fsck_object_dir(get_object_directory());
 
prepare_alt_odb();
-   for (alt = alt_odb_list; alt; alt = alt->next)
+   for (alt = the_repository->objects.alt_odb_list;
+   alt; alt = alt->next)
fsck_object_dir(alt->path);
 
if (check_full) {
diff --git a/cache.h b/cache.h
index 9cac7bb518..f373c30e25 100644
--- a/cache.h
+++ b/cache.h
@@ -1576,7 +1576,7 @@ extern int has_dirs_only_path(const char *name, int len, 
int prefix_len);
 extern void schedule_dir_for_removal(const char *name, int len);
 extern void remove_scheduled_dirs(void);
 
-extern struct alternate_object_database {
+struct alternate_object_database {
struct alternate_object_database *next;
 
/* see alt_scratch_buf() */
@@ -1594,7 +1594,7 @@ extern struct alternate_object_database {
struct oid_array loose_objects_cache;
 
char path[FLEX_ARRAY];
-} *alt_odb_list;
+};
 extern void prepare_alt_odb(void);
 extern char *compute_alternate_path(const char *path, struct strbuf *err);
 typedef int alt_odb_fn(struct alternate_object_database *, void *);
diff --git a/object-store.h b/object-store.h
index 5959d990fc..a3f0d6ac15 100644
--- a/object-store.h
+++ b/object-store.h
@@ -1,6 +1,8 @@
 #ifndef OBJECT_STORE_H
 #define OBJECT_STORE_H
 
+#include "cache.h"
+
 struct raw_object_store {
/*
 * Path to the repository's object store.
@@ -8,9 +10,12 @@ struct raw_object_store {
 */
char *objectdir;
 
+   struct alternate_object_database *alt_odb_list;
+   struct alternate_object_database **alt_odb_tail;
+
unsigned ignore_env : 1;
 };
-#define RAW_OBJECT_STORE_INIT { NULL, 0 }
+#define RAW_OBJECT_STORE_INIT { NULL, NULL, NULL, 0 }
 
 void raw_object_store_clear(struct raw_object_store *o);
 
diff --git a/packfile.c b/packfile.c
index 7dbe8739d1..216ea836ee 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "list.h"
 #include "pack.h"
+#include "repository.h"
 #include "dir.h"
 #include "mergesort.h"
 #include "packfile.h"
@@ -891,7 +892,7 @@ void prepare_packed_git(void)
return;
prepare_packed_git_one(get_object_directory(), 1);
prepare_alt_odb();
-   for (alt = alt_odb_list; alt; alt = alt->next)
+   for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next)
prepare_packed_git_one(alt->path, 0);
rearrange_packed_git();
prepare_packed_git_mru();
diff --git a/sha1_file.c b/sha1_file.c
index 831d9e7343..1348dce68f 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -22,6 +22,7 @@
 #include "pack-revindex.h"
 #include "sha1-lookup.h"
 #include "bulk-checkin.h"
+#include "repository.h"
 #include "streaming.h"
 #include "dir.h"
 #include "list.h"
@@ -342,9 +343,6 @@ static const char *alt_sha1_path(struct 
alternate_object_database *alt,
return buf->buf;
 }
 
-struct alternate_object_database *alt_odb_list;
-static struct alternate_object_database **alt_odb_tail;
-
 /*
  * Return non-zero iff the path is usable as an alternate object database.
  */
@@ -364,7 +362,7 @@ static int alt_odb_usable(struct strbuf *path, const char 
*normalized_objdir)
 * Prevent the common mistake of listing the same
 * thing twice, or object directory itself.
 */
-   for (alt = alt_odb_list; alt; alt = alt->next) {
+   for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next) {
if (!fspathcmp(path->buf, alt->path))
return 0;
}
@@ -424,8 +422,8 @@ static int link_alt_odb_entry(const char *entry, const char 
*relative_base,
ent = alloc_alt_odb(pathbuf.buf);
 
/* add the alternate entry */
-   *alt_odb_tail = ent;
-   alt_odb_tail = &(ent->next);
+   *the_repository->objects.alt_odb_tail 

[PATCH 15/16] sha1_file: allow map_sha1_file to handle arbitrary object stores

[Stefan Beller]

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 cache.h| 1 -
 object-store.h | 2 ++
 sha1_file.c| 7 ---
 streaming.c| 5 -
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index 2495c7081d..df29ee72e9 100644
--- a/cache.h
+++ b/cache.h
@@ -1242,7 +1242,6 @@ extern int pretend_sha1_file(void *, unsigned long, enum 
object_type, unsigned c
 extern int force_object_loose(const unsigned char *sha1, time_t mtime);
 extern int git_open_cloexec(const char *name, int flags);
 #define git_open(name) git_open_cloexec(name, O_RDONLY)
-extern void *map_sha1_file(const unsigned char *sha1, unsigned long *size);
 extern int unpack_sha1_header(git_zstream *stream, unsigned char *map, 
unsigned long mapsize, void *buffer, unsigned long bufsiz);
 extern int parse_sha1_header(const char *hdr, unsigned long *sizep);
 
diff --git a/object-store.h b/object-store.h
index 5f6beb9d91..30b4cfdbf5 100644
--- a/object-store.h
+++ b/object-store.h
@@ -73,6 +73,8 @@ extern struct packed_git {
  */
 void sha1_file_name(struct raw_object_store *o, struct strbuf *buf, const 
unsigned char *sha1);
 
+void *map_sha1_file(struct raw_object_store *o, const unsigned char *sha1, 
unsigned long *size);
+
 void prepare_alt_odb(struct raw_object_store *o);
 
 #endif /* OBJECT_STORE_H */
diff --git a/sha1_file.c b/sha1_file.c
index aeb925a895..4cbff471a2 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -962,9 +962,10 @@ static void *map_sha1_file_1(struct raw_object_store *o, 
const char *path,
return map;
 }
 
-void *map_sha1_file(const unsigned char *sha1, unsigned long *size)
+void *map_sha1_file(struct raw_object_store *o,
+   const unsigned char *sha1, unsigned long *size)
 {
-   return map_sha1_file_1(_repository->objects, NULL, sha1, size);
+   return map_sha1_file_1(o, NULL, sha1, size);
 }
 
 static int unpack_sha1_short_header(git_zstream *stream,
@@ -1186,7 +1187,7 @@ static int sha1_loose_object_info(const unsigned char 
*sha1,
return 0;
}
 
-   map = map_sha1_file(sha1, );
+   map = map_sha1_file(_repository->objects, sha1, );
if (!map)
return -1;
 
diff --git a/streaming.c b/streaming.c
index 5892b50bd8..6888448b12 100644
--- a/streaming.c
+++ b/streaming.c
@@ -3,6 +3,8 @@
  */
 #include "cache.h"
 #include "streaming.h"
+#include "repository.h"
+#include "object-store.h"
 #include "packfile.h"
 
 enum input_source {
@@ -335,7 +337,8 @@ static struct stream_vtbl loose_vtbl = {
 
 static open_method_decl(loose)
 {
-   st->u.loose.mapped = map_sha1_file(sha1, >u.loose.mapsize);
+   st->u.loose.mapped = map_sha1_file(_repository->objects,
+  sha1, >u.loose.mapsize);
if (!st->u.loose.mapped)
return -1;
if ((unpack_sha1_header(>z,
-- 
2.16.1.291.g4437f3f132-goog

[PATCH 13/16] sha1_file: allow open_sha1_file to handle arbitrary object stores

[Stefan Beller]

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 sha1_file.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index bd8b0331f0..3356f70dd2 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -900,7 +900,8 @@ static int stat_sha1_file(struct raw_object_store *o, const 
unsigned char *sha1,
  * Like stat_sha1_file(), but actually open the object and return the
  * descriptor. See the caveats on the "path" parameter above.
  */
-static int open_sha1_file(const unsigned char *sha1, const char **path)
+static int open_sha1_file(struct raw_object_store *o,
+ const unsigned char *sha1, const char **path)
 {
int fd;
struct alternate_object_database *alt;
@@ -908,7 +909,7 @@ static int open_sha1_file(const unsigned char *sha1, const 
char **path)
static struct strbuf buf = STRBUF_INIT;
 
strbuf_reset();
-   sha1_file_name(_repository->objects, , sha1);
+   sha1_file_name(o, , sha1);
*path = buf.buf;
 
fd = git_open(*path);
@@ -916,8 +917,8 @@ static int open_sha1_file(const unsigned char *sha1, const 
char **path)
return fd;
most_interesting_errno = errno;
 
-   prepare_alt_odb(_repository->objects);
-   for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next) {
+   prepare_alt_odb(o);
+   for (alt = o->alt_odb_list; alt; alt = alt->next) {
*path = alt_sha1_path(alt, sha1);
fd = git_open(*path);
if (fd >= 0)
@@ -943,7 +944,7 @@ static void *map_sha1_file_1(const char *path,
if (path)
fd = git_open(path);
else
-   fd = open_sha1_file(sha1, );
+   fd = open_sha1_file(_repository->objects, sha1, );
map = NULL;
if (fd >= 0) {
struct stat st;
-- 
2.16.1.291.g4437f3f132-goog

[PATCH 03/16] object-store: free alt_odb_list

[Stefan Beller]

Free the memory and reset alt_odb_{list, tail} to NULL.

Signed-off-by: Stefan Beller 
---
 object.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/object.c b/object.c
index 11d904c033..17b1da6d6b 100644
--- a/object.c
+++ b/object.c
@@ -446,7 +446,24 @@ void clear_commit_marks_all(unsigned int flags)
}
 }
 
+static void free_alt_odb(struct alternate_object_database *alt)
+{
+   strbuf_release(>scratch);
+   oid_array_clear(>loose_objects_cache);
+}
+
+static void free_alt_odbs(struct raw_object_store *o)
+{
+   while (o->alt_odb_list) {
+   free_alt_odb(o->alt_odb_list);
+   o->alt_odb_list = o->alt_odb_list->next;
+   }
+}
+
 void raw_object_store_clear(struct raw_object_store *o)
 {
free(o->objectdir);
+
+   free_alt_odbs(o);
+   o->alt_odb_tail = NULL;
 }
-- 
2.16.1.291.g4437f3f132-goog

[PATCH 10/16] sha1_file: allow prepare_alt_odb to handle arbitrary object stores

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 builtin/fsck.c |  2 +-
 cache.h|  1 -
 object-store.h |  2 ++
 packfile.c |  2 +-
 sha1_file.c| 23 +++
 sha1_name.c|  3 ++-
 6 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 2e99e02128..c0d967d6d7 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -717,7 +717,7 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
} else {
fsck_object_dir(get_object_directory());
 
-   prepare_alt_odb();
+   prepare_alt_odb(_repository->objects);
for (alt = the_repository->objects.alt_odb_list;
alt; alt = alt->next)
fsck_object_dir(alt->path);
diff --git a/cache.h b/cache.h
index 12bb42f951..d0c1a533c5 100644
--- a/cache.h
+++ b/cache.h
@@ -1599,7 +1599,6 @@ struct alternate_object_database {
 */
char path[FLEX_ARRAY];
 };
-extern void prepare_alt_odb(void);
 extern char *compute_alternate_path(const char *path, struct strbuf *err);
 typedef int alt_odb_fn(struct alternate_object_database *, void *);
 extern int foreach_alt_odb(alt_odb_fn, void*);
diff --git a/object-store.h b/object-store.h
index 80a2ca6f75..280e8e16ad 100644
--- a/object-store.h
+++ b/object-store.h
@@ -67,4 +67,6 @@ extern struct packed_git {
char pack_name[FLEX_ARRAY]; /* more */
 } *packed_git;
 
+void prepare_alt_odb(struct raw_object_store *o);
+
 #endif /* OBJECT_STORE_H */
diff --git a/packfile.c b/packfile.c
index 693bafbc98..83b10e55c0 100644
--- a/packfile.c
+++ b/packfile.c
@@ -890,7 +890,7 @@ void prepare_packed_git(void)
if (the_repository->objects.packed_git_initialized)
return;
prepare_packed_git_one(get_object_directory(), 1);
-   prepare_alt_odb();
+   prepare_alt_odb(_repository->objects);
for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next)
prepare_packed_git_one(alt->path, 0);
rearrange_packed_git();
diff --git a/sha1_file.c b/sha1_file.c
index c4255a2da6..04a81e29f2 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -23,6 +23,7 @@
 #include "sha1-lookup.h"
 #include "bulk-checkin.h"
 #include "repository.h"
+#include "object-store.h"
 #include "streaming.h"
 #include "dir.h"
 #include "list.h"
@@ -577,7 +578,7 @@ void add_to_alternates_memory(const char *reference)
 * Make sure alternates are initialized, or else our entry may be
 * overwritten when they are.
 */
-   prepare_alt_odb();
+   prepare_alt_odb(_repository->objects);
 
link_alt_odb_entries(_repository->objects, reference,
 '\n', NULL, 0);
@@ -663,7 +664,7 @@ int foreach_alt_odb(alt_odb_fn fn, void *cb)
struct alternate_object_database *ent;
int r = 0;
 
-   prepare_alt_odb();
+   prepare_alt_odb(_repository->objects);
for (ent = the_repository->objects.alt_odb_list; ent; ent = ent->next) {
r = fn(ent, cb);
if (r)
@@ -672,21 +673,19 @@ int foreach_alt_odb(alt_odb_fn fn, void *cb)
return r;
 }
 
-void prepare_alt_odb(void)
+void prepare_alt_odb(struct raw_object_store *o)
 {
const char *alt;
 
-   if (the_repository->objects.alt_odb_tail)
+   if (o->alt_odb_tail)
return;
 
alt = getenv(ALTERNATE_DB_ENVIRONMENT);
 
-   the_repository->objects.alt_odb_tail =
-   _repository->objects.alt_odb_list;
-   link_alt_odb_entries(_repository->objects, alt,
-PATH_SEP, NULL, 0);
+   o->alt_odb_tail = >alt_odb_list;
+   link_alt_odb_entries(o, alt, PATH_SEP, NULL, 0);
 
-   read_info_alternates(_repository->objects, get_object_directory(), 
0);
+   read_info_alternates(o, o->objectdir, 0);
 }
 
 /* Returns 1 if we have successfully freshened the file, 0 otherwise. */
@@ -726,7 +725,7 @@ static int check_and_freshen_local(const unsigned char 
*sha1, int freshen)
 static int check_and_freshen_nonlocal(const unsigned char *sha1, int freshen)
 {
struct alternate_object_database *alt;
-   prepare_alt_odb();
+   prepare_alt_odb(_repository->objects);
for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next) {
const char *path = alt_sha1_path(alt, sha1);
if (check_and_freshen_file(path, freshen))
@@ -885,7 +884,7 @@ static int stat_sha1_file(const unsigned char *sha1, struct 
stat *st,
if (!lstat(*path, st))
return 0;
 
-   prepare_alt_odb();
+   prepare_alt_odb(_repository->objects);
errno = ENOENT;
for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next) {
*path = alt_sha1_path(alt, sha1);
@@ -916,7 +915,7 @@ static int open_sha1_file(const unsigned char *sha1, const 
char **path)
return fd;

[PATCH 12/16] sha1_file: allow stat_sha1_file to handle arbitrary object stores

[Stefan Beller]

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 sha1_file.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index d64cdbb494..bd8b0331f0 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -871,22 +871,23 @@ int git_open_cloexec(const char *name, int flags)
  * Note that it may point to static storage and is only valid until another
  * call to sha1_file_name(), etc.
  */
-static int stat_sha1_file(const unsigned char *sha1, struct stat *st,
- const char **path)
+static int stat_sha1_file(struct raw_object_store *o, const unsigned char 
*sha1,
+ struct stat *st, const char **path)
 {
struct alternate_object_database *alt;
static struct strbuf buf = STRBUF_INIT;
 
strbuf_reset();
-   sha1_file_name(_repository->objects, , sha1);
+   sha1_file_name(o, , sha1);
*path = buf.buf;
 
if (!lstat(*path, st))
return 0;
 
-   prepare_alt_odb(_repository->objects);
+   prepare_alt_odb(o);
+
errno = ENOENT;
-   for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next) {
+   for (alt = o->alt_odb_list; alt; alt = alt->next) {
*path = alt_sha1_path(alt, sha1);
if (!lstat(*path, st))
return 0;
@@ -1178,7 +1179,7 @@ static int sha1_loose_object_info(const unsigned char 
*sha1,
if (!oi->typep && !oi->typename && !oi->sizep && !oi->contentp) {
const char *path;
struct stat st;
-   if (stat_sha1_file(sha1, , ) < 0)
+   if (stat_sha1_file(_repository->objects, sha1, , ) 
< 0)
return -1;
if (oi->disk_sizep)
*oi->disk_sizep = st.st_size;
@@ -1392,7 +1393,7 @@ void *read_sha1_file_extended(const unsigned char *sha1,
die("replacement %s not found for %s",
sha1_to_hex(repl), sha1_to_hex(sha1));
 
-   if (!stat_sha1_file(repl, , ))
+   if (!stat_sha1_file(_repository->objects, repl, , ))
die("loose object %s (stored in %s) is corrupt",
sha1_to_hex(repl), path);
 
-- 
2.16.1.291.g4437f3f132-goog

[PATCH 14/16] sha1_file: allow map_sha1_file_1 to handle arbitrary object stores

[Stefan Beller]

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 sha1_file.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 3356f70dd2..aeb925a895 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -934,9 +934,8 @@ static int open_sha1_file(struct raw_object_store *o,
  * Map the loose object at "path" if it is not NULL, or the path found by
  * searching for a loose object named "sha1".
  */
-static void *map_sha1_file_1(const char *path,
-const unsigned char *sha1,
-unsigned long *size)
+static void *map_sha1_file_1(struct raw_object_store *o, const char *path,
+const unsigned char *sha1, unsigned long *size)
 {
void *map;
int fd;
@@ -944,7 +943,7 @@ static void *map_sha1_file_1(const char *path,
if (path)
fd = git_open(path);
else
-   fd = open_sha1_file(_repository->objects, sha1, );
+   fd = open_sha1_file(o, sha1, );
map = NULL;
if (fd >= 0) {
struct stat st;
@@ -965,7 +964,7 @@ static void *map_sha1_file_1(const char *path,
 
 void *map_sha1_file(const unsigned char *sha1, unsigned long *size)
 {
-   return map_sha1_file_1(NULL, sha1, size);
+   return map_sha1_file_1(_repository->objects, NULL, sha1, size);
 }
 
 static int unpack_sha1_short_header(git_zstream *stream,
@@ -2195,7 +2194,7 @@ int read_loose_object(const char *path,
 
*contents = NULL;
 
-   map = map_sha1_file_1(path, NULL, );
+   map = map_sha1_file_1(_repository->objects, path, NULL, );
if (!map) {
error_errno("unable to mmap %s", path);
goto out;
-- 
2.16.1.291.g4437f3f132-goog

[PATCH 01/16] repository: introduce raw object store field

[Stefan Beller]

The raw object store field will contain any objects needed for
access to objects in a given repository.

This patch introduces the raw object store and populates it with the
`objectdir`, which used to be part of the repository struct as well as
'ignore_env' which is duplicated from the repository. A later refactoring
(not in this series) will hopefully get rid of that one bit, once all
the construction of repo objects goes through repo_init(). The reason for
duplication now is that the refactoring done in this series allows for
smaller scoped objects in the functions refactored, nameny passing around
the object store instead of the repository object.

As the struct gains members, we'll also populate the function to clear
the memory for these members.

In a later we'll introduce a struct object_parser, that will complement
the object parsing in a repository struct: The raw object parser is the
layer that will provide access to raw object content, while the higher
level object parser code will parse raw objects and keeps track of
parenthood and other object relationships using 'struct object'.
For now only add the lower level to the repository struct.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 builtin/grep.c |  2 +-
 environment.c  |  5 +++--
 object-store.h | 17 +
 object.c   |  5 +
 path.c |  2 +-
 repository.c   | 19 +++
 repository.h   |  7 ---
 7 files changed, 46 insertions(+), 11 deletions(-)
 create mode 100644 object-store.h

diff --git a/builtin/grep.c b/builtin/grep.c
index 3ca4ac80d8..0f0c195705 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -432,7 +432,7 @@ static int grep_submodule(struct grep_opt *opt, struct 
repository *superproject,
 * object.
 */
grep_read_lock();
-   add_to_alternates_memory(submodule.objectdir);
+   add_to_alternates_memory(submodule.objects.objectdir);
grep_read_unlock();
 
if (oid) {
diff --git a/environment.c b/environment.c
index de8431e01e..ec10b062e6 100644
--- a/environment.c
+++ b/environment.c
@@ -13,6 +13,7 @@
 #include "refs.h"
 #include "fmt-merge-msg.h"
 #include "commit.h"
+#include "object-store.h"
 
 int trust_executable_bit = 1;
 int trust_ctime = 1;
@@ -244,9 +245,9 @@ const char *get_git_work_tree(void)
 
 char *get_object_directory(void)
 {
-   if (!the_repository->objectdir)
+   if (!the_repository->objects.objectdir)
BUG("git environment hasn't been setup");
-   return the_repository->objectdir;
+   return the_repository->objects.objectdir;
 }
 
 int odb_mkstemp(struct strbuf *template, const char *pattern)
diff --git a/object-store.h b/object-store.h
new file mode 100644
index 00..5959d990fc
--- /dev/null
+++ b/object-store.h
@@ -0,0 +1,17 @@
+#ifndef OBJECT_STORE_H
+#define OBJECT_STORE_H
+
+struct raw_object_store {
+   /*
+* Path to the repository's object store.
+* Cannot be NULL after initialization.
+*/
+   char *objectdir;
+
+   unsigned ignore_env : 1;
+};
+#define RAW_OBJECT_STORE_INIT { NULL, 0 }
+
+void raw_object_store_clear(struct raw_object_store *o);
+
+#endif /* OBJECT_STORE_H */
diff --git a/object.c b/object.c
index 9e6f9ff20b..11d904c033 100644
--- a/object.c
+++ b/object.c
@@ -445,3 +445,8 @@ void clear_commit_marks_all(unsigned int flags)
obj->flags &= ~flags;
}
 }
+
+void raw_object_store_clear(struct raw_object_store *o)
+{
+   free(o->objectdir);
+}
diff --git a/path.c b/path.c
index da8b655730..81a42d9115 100644
--- a/path.c
+++ b/path.c
@@ -382,7 +382,7 @@ static void adjust_git_path(const struct repository *repo,
strbuf_splice(buf, 0, buf->len,
  repo->index_file, strlen(repo->index_file));
else if (dir_prefix(base, "objects"))
-   replace_dir(buf, git_dir_len + 7, repo->objectdir);
+   replace_dir(buf, git_dir_len + 7, repo->objects.objectdir);
else if (git_hooks_path && dir_prefix(base, "hooks"))
replace_dir(buf, git_dir_len + 5, git_hooks_path);
else if (repo->different_commondir)
diff --git a/repository.c b/repository.c
index 4ffbe9bc94..3b8f1b91be 100644
--- a/repository.c
+++ b/repository.c
@@ -1,11 +1,18 @@
 #include "cache.h"
 #include "repository.h"
+#include "object-store.h"
 #include "config.h"
 #include "submodule-config.h"
 
 /* The main repository */
 static struct repository the_repo = {
-   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, 
_algos[GIT_HASH_SHA1], 0, 0
+   NULL, NULL,
+   RAW_OBJECT_STORE_INIT,
+   NULL, NULL, NULL,
+   NULL, NULL, NULL,
+   _index,
+   _algos[GIT_HASH_SHA1],
+   0, 0
 };
 struct repository *the_repository = _repo;
 
@@ -42,9 +49,11 @@ static void repo_setup_env(struct repository *repo)

[PATCHv2 00/16] Moving global state into the repository object (part 1)

[Stefan Beller]

v2:
* duplicated the 'ignore_env' bit into the object store as well
* the #define trick no longer works as we do not have a "the_objectstore" 
global,
  which means there is just one patch per function that is converted.
  As this follows the same structure of the previous series, I am still 
confident
  there is no hidden dependencies to globals outside the object store in these
  converted functions.
* rebased on top of current master, resolving the merge conflicts.
  I think I used the list.h APIs right, but please double check.
  
Thanks,
Stefan

v1:
This is a real take on the first part of the recent RFC[1].

Jonathan Tan suggested[2] that "sha1_loose_object_info to handle arbitrary 
repositories"
might be a good breaking point for a first part at that RFC at patch 38.
This series is smaller and contains only 26 patches as the patches in the big
RFC were slightly out of order.

I developed this series partly by writing patches, but mostly by cherrypicking
from that RFC on top of current master. I noticed no external conflicts apart
from one addition to the repositories _INIT macro, which was easy to resolve.

Comments in the early range of that RFC were on 003 where Junio pointed out
that the coccinelle patch ought to be not in contrib/coccinelle, so I put it
in a sub directory there, as 'make coccicheck' doesn't traverse subdirs.

brian had a questoin on patch 25 in the RFC, but that seemed to resolve itself
without any suggestion to include into this series[3].

Duy suggested that we shall not use the repository blindly, but should carefully
examine whether to pass on an object store or the refstore or such[4], which
I agree with if it makes sense. This series unfortunately has an issue with that
as I would not want to pass down the `ignore_env` flag separately from the 
object
store, so I made all functions that only take the object store to have the raw
object store as the first parameter, and others using the full repository.

Eric Sunshine brought up memory leaks with the RFC, and I would think to
have plugged all holes.

[1] https://public-inbox.org/git/20180205235508.216277-1-sbel...@google.com/
[2] 
https://public-inbox.org/git/20180207143300.ce1c39ca07f6a0d64fe0e...@google.com/
[3] 
https://public-inbox.org/git/20180206011940.gd7...@genre.crustytoothpaste.net/
[4] 
https://public-inbox.org/git/cacsjy8cggekpx4czkyytsprj87uqvkzsol7fyt__p2dh_1l...@mail.gmail.com/

Thanks,
Stefan

Stefan Beller (16):
  repository: introduce raw object store field
  object-store: move alt_odb_list and alt_odb_tail to object store
  object-store: free alt_odb_list
  object-store: move packed_git and packed_git_mru to object store
  object-store: close all packs upon clearing the object store
  pack: move prepare_packed_git_run_once to object store
  pack: move approximate object count to object store
  sha1_file: add raw_object_store argument to alt_odb_usable
  sha1_file: allow link_alt_odb_entries to handle arbitrary object
stores
  sha1_file: allow prepare_alt_odb to handle arbitrary object stores
  sha1_file: allow sha1_file_name to handle arbitrary object stores
  sha1_file: allow stat_sha1_file to handle arbitrary object stores
  sha1_file: allow open_sha1_file to handle arbitrary object stores
  sha1_file: allow map_sha1_file_1 to handle arbitrary object stores
  sha1_file: allow map_sha1_file to handle arbitrary object stores
  sha1_file: allow sha1_loose_object_info to handle arbitrary object
stores

 builtin/am.c |   2 +-
 builtin/clone.c  |   2 +-
 builtin/count-objects.c  |   6 +-
 builtin/fetch.c  |   2 +-
 builtin/fsck.c   |  13 +++--
 builtin/gc.c |   4 +-
 builtin/grep.c   |   2 +-
 builtin/index-pack.c |   1 +
 builtin/merge.c  |   2 +-
 builtin/pack-objects.c   |  19 +++---
 builtin/pack-redundant.c |   6 +-
 builtin/receive-pack.c   |   3 +-
 cache.h  |  45 ++
 environment.c|   5 +-
 fast-import.c|   6 +-
 http-backend.c   |   6 +-
 http-push.c  |   1 +
 http-walker.c|   4 +-
 http.c   |   6 +-
 object-store.h   |  80 +
 object.c |  27 +
 pack-bitmap.c|   4 +-
 pack-check.c |   1 +
 pack-revindex.c  |   1 +
 packfile.c   |  64 ++--
 packfile.h   |   2 +-
 path.c   |   2 +-
 reachable.c  |   1 +
 repository.c |  22 +--
 repository.h |   7 ++-
 server-info.c|   6 +-
 sha1_file.c  | 123 +--
 sha1_name.c  |  11 ++--
 streaming.c  |   5 +-
 34 files changed, 314 insertions(+), 177 deletions(-)
 create mode 100644 object-store.h

-- 
2.16.1.291.g4437f3f132-goog

Re: Line ending normalization doesn't work as expected

[Robert Dailey]

On Fri, Feb 16, 2018 at 10:34 AM, Torsten Bögershausen  wrote:
> On Thu, Feb 15, 2018 at 09:24:40AM -0600, Robert Dailey wrote:
>> On Tue, Oct 3, 2017 at 9:00 PM, Junio C Hamano  wrote:
>
> []
>>
>> Sorry to bring this old thread back to life, but I did notice that
>> this causes file modes to reset back to 644 (from 755) on Windows
>> version of Git. Is there a way to `$ git read-tree --empty && git add
>> .` without mucking with file permissions?
>
> No problem with the delay, under the time we had the chance to improve Git:
>
>>Git 2.16 Release Notes
>>==
>>[]
>>* "git add --renormalize ." is a new and safer way to record the fact
>>   that you are correcting the end-of-line convention and other
>>   "convert_to_git()" glitches in the in-repository data.
>
> Could you upgrade to Git 2.16.1 (or higher, just take the latest)
> and try with
> git add --renormalize .
> ?

Thanks for the response. Unfortunately I've deliberately been stuck on
v2.13 because of [a bug in Git for Windows][1] that hasn't yet been
resolved (it's a bug *somewhere*, not sure if it's git related or
not). Curly braces in aliases are being stripped which makes newer
releases unusable for me. I'll try upgrading on a different machine
and see if renormalize works for the case of binary files with file
modes set to 755.

[1]: https://github.com/git-for-windows/git/issues/1220

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

[Torsten Bögershausen]

On Fri, Feb 16, 2018 at 03:42:35PM +0100, Lars Schneider wrote:
[]
> 
> Agreed. However, people using ShiftJIS are not my target audience.
> My target audience are:
> 
> (1) People that have to encode their text files in UTF-16 (for 
> whatever reason - usually because of legacy processes or tools).
> 
> (2) People that want to see textual diffs of their UTF-16 encoded 
> files in their Git tools without special adjustments (on the 
> command line, on the web, etc).
> 
> That was my primary motivation. The fact that w-t-e supports any
> other encoding too is just a nice side effect. I don't foresee people
> using other w-t-encodings other than UTF-16 in my organization.
> 
> I have the suspicion that the feature could be useful for the Git
> community at large. Consider this Stack Overflow question:
> https://stackoverflow.com/questions/777949/can-i-make-git-recognize-a-utf-16-file-as-text
> 
> This question was viewed 42k times and there is no good solution.
> I believe w-t-e could be a good solution.
> 

If it was only about a diff of UTF-16 files, I may suggest a patch.
I simply copy-paste it here for review, if someone thinks that it may
be useful, I can send it as a real patch/RFC.

git show HEAD


commit 9f7d43f29eaf6017b7b16261ce91d8ef182cf415
Author: Torsten Bögershausen 
Date:   Fri Feb 2 15:35:23 2018 +0100

Auto diff of UTF-16 files in UTF-8

When an UTF-16 file is commited and later changed, `git diff` shows
"Binary files XX and YY differ".

When the user wants a diff in UTF-8, a textconv needs to be specified
in .gitattributes and the textconv must be configured.

A more user-friendly diff can be produced for UTF-16 if
- the user did not use `git diff --binary`
- the blob is identified as binary
- the blob has an UTF-16 BOM
- the blob can be converted into UTF-8

Enhance the diff machinery to auto-detect UTF-16 blobs and show them
as UTF-8, unless the user specifies `git diff --binary` which creates
a binary diff.

diff --git a/diff.c b/diff.c
index fb22b19f09..51831ee94d 100644
--- a/diff.c
+++ b/diff.c
@@ -3192,6 +3192,10 @@ static void builtin_diff(const char *name_a,
strbuf_reset();
}
 
+   if (one && one->reencoded_from_utf16)
+   strbuf_addf(, "a is converted to UTF-8 from 
UTF-16\n");
+   if (two && two->reencoded_from_utf16)
+   strbuf_addf(, "b is converted to UTF-8 from 
UTF-16\n");
mf1.size = fill_textconv(textconv_one, one, );
mf2.size = fill_textconv(textconv_two, two, );
 
@@ -3611,8 +3615,25 @@ int diff_populate_filespec(struct diff_filespec *s, 
unsigned int flags)
s->size = size;
s->should_free = 1;
}
-   }
-   else {
+   if (!s->binary && buffer_is_binary(s->data, s->size) &&
+   buffer_has_utf16_bom(s->data, s->size)) {
+   int outsz = 0;
+   char *outbuf;
+   outbuf = reencode_string_len(s->data, (int)s->size,
+"UTF-8", "UTF-16", );
+   if (outbuf) {
+   if (s->should_free)
+   free(s->data);
+   if (s->should_munmap)
+   munmap(s->data, s->size);
+   s->should_munmap = 0;
+   s->data = outbuf;
+   s->size = outsz;
+   s->reencoded_from_utf16 = 1;
+   s->should_free = 1;
+   }
+   }
+   } else {
enum object_type type;
if (size_only || (flags & CHECK_BINARY)) {
type = sha1_object_info(s->oid.hash, >size);
@@ -3629,6 +3650,19 @@ int diff_populate_filespec(struct diff_filespec *s, 
unsigned int flags)
s->data = read_sha1_file(s->oid.hash, , >size);
if (!s->data)
die("unable to read %s", oid_to_hex(>oid));
+   if (!s->binary && buffer_is_binary(s->data, s->size) &&
+   buffer_has_utf16_bom(s->data, s->size)) {
+   int outsz = 0;
+   char *buf;
+   buf = reencode_string_len(s->data, (int)s->size,
+ "UTF-8", "UTF-16", );
+   if (buf) {
+   free(s->data);
+   s->data = buf;
+   s->size = outsz;
+   s->reencoded_from_utf16 = 1;
+   }
+   }
s->should_free = 1;
}
return 0;
@@ -5695,6 +5729,10 @@ 

Re: [PATCH v3] worktree: add: fix 'post-checkout' not knowing new worktree location

[Lars Schneider]

> On 16 Feb 2018, at 00:09, Eric Sunshine  wrote:
> 
> Although "git worktree add" learned to run the 'post-checkout' hook in
> ade546be47 (worktree: invoke post-checkout hook, 2017-12-07), it
> neglected to change to the directory of the newly-created worktree
> before running the hook. Instead, the hook runs within the directory
> from which the "git worktree add" command itself was invoked, which
> effectively neuters the hook since it knows nothing about the new
> worktree directory.
> 
> Further, ade546be47 failed to sanitize the environment before running
> the hook, which means that user-assigned values of GIT_DIR and
> GIT_WORK_TREE could mislead the hook about the location of the new
> worktree. In the case of "git worktree add" being run from a bare
> repository, the GIT_DIR="." assigned by Git itself leaks into the hook's
> environment and breaks Git commands; this is so even when the working
> directory is correctly changed to the new worktree before the hook runs
> since ".", relative to the new worktree directory, does not point at the
> bare repository.
> 
> Fix these problems by (1) changing to the new worktree's directory
> before running the hook, and (2) sanitizing the environment of GIT_DIR
> and GIT_WORK_TREE so hooks can't be confused by misleading values.
> 
> Enhance the t2025 'post-checkout' tests to verify that the hook is
> indeed run within the correct directory and that Git commands invoked by
> the hook compute Git-dir and top-level worktree locations correctly.
> 
> While at it, also add two new tests: (1) verify that the hook is run
> within the correct directory even when the new worktree is created from
> a sibling worktree (as opposed to the main worktree); (2) verify that
> the hook is provided with correct context when the new worktree is
> created from a bare repository (test provided by Lars Schneider).

Thanks! This patch works great and fixes the problem!
More comments below.


> Implementation Notes:
> 
> Rather than sanitizing the environment of GIT_DIR and GIT_WORK_TREE, an
> alternative would be to set them explicitly, as is already done for
> other Git commands run internally by "git worktree add". This patch opts
> instead to sanitize the environment in order to clearly document that
> the worktree is fully functional by the time the hook is run, thus does
> not require special environmental overrides.
> 
> The hook is run manually, rather than via run_hook_le(), since it needs
> to change the working directory to that of the worktree, and
> run_hook_le() does not provide such functionality. As this is a one-off
> case, adding 'run_hook' overloads which allow the directory to be set
> does not seem warranted at this time.

Although this is an one-off case, I would still prefer it if all hook 
invocations would happen in a central place to avoid future surprises.


> Reported-by: Lars Schneider 
> Signed-off-by: Eric Sunshine 
> ---
> 
> This is a re-roll of [1] which fixes "git worktree add" to provide
> proper context to the 'post-checkout' hook so that the hook knows the
> location of the newly-created worktree.
> 
> Changes since v2:
> 
> * Fix crash due to missing NULL-terminator on 'env' list passed to
>  run_command().
> 
> [1]: 
> https://public-inbox.org/git/20180215191841.40848-1-sunsh...@sunshineco.com/
> 
> builtin/worktree.c  | 20 ---
> t/t2025-worktree-add.sh | 54 ++---
> 2 files changed, 62 insertions(+), 12 deletions(-)
> 
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 7cef5b120b..f69f862947 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -345,9 +345,23 @@ static int add_worktree(const char *path, const char 
> *refname,
>* Hook failure does not warrant worktree deletion, so run hook after
>* is_junk is cleared, but do return appropriate code when hook fails.
>*/
> - if (!ret && opts->checkout)
> - ret = run_hook_le(NULL, "post-checkout", oid_to_hex(_oid),
> -   oid_to_hex(>object.oid), "1", NULL);
> + if (!ret && opts->checkout) {
> + const char *hook = find_hook("post-checkout");
> + if (hook) {
> + const char *env[] = { "GIT_DIR", "GIT_WORK_TREE", NULL 
> };
> + cp.git_cmd = 0;
> + cp.no_stdin = 1;
> + cp.stdout_to_stderr = 1;
> + cp.dir = path;
> + cp.env = env;
> + cp.argv = NULL;
> + argv_array_pushl(, absolute_path(hook),
> +  oid_to_hex(_oid),
> +  oid_to_hex(>object.oid),
> +  "1", NULL);
> + ret = run_command();
> + }
> + }
> 
>   argv_array_clear(_env);
>   

Re: Line ending normalization doesn't work as expected

[Torsten Bögershausen]

On Thu, Feb 15, 2018 at 09:24:40AM -0600, Robert Dailey wrote:
> On Tue, Oct 3, 2017 at 9:00 PM, Junio C Hamano  wrote:

[]
> 
> Sorry to bring this old thread back to life, but I did notice that
> this causes file modes to reset back to 644 (from 755) on Windows
> version of Git. Is there a way to `$ git read-tree --empty && git add
> .` without mucking with file permissions?

No problem with the delay, under the time we had the chance to improve Git:

>Git 2.16 Release Notes
>==
>[]
>* "git add --renormalize ." is a new and safer way to record the fact
>   that you are correcting the end-of-line convention and other
>   "convert_to_git()" glitches in the in-repository data.

Could you upgrade to Git 2.16.1 (or higher, just take the latest)
and try with
git add --renormalize .
?

Re: [PATCH v3 22/23] cat-file: tests for new atoms added

[Adam Dinwoodie]

On 12 February 2018 at 08:08, Olga Telezhnaya wrote:
> Add some tests for new formatting atoms from ref-filter.
> Some of new atoms are supported automatically,
> some of them are expanded into empty string
> (because they are useless for some types of objects),
> some of them could be supported later in other patches.

This commit appears to be introducing failures in t1006 on Cygwin.
Specifically, tests 15, 36, 58 and 89, all titled "Check %(refname)
gives empty output", are failing on the pu branch at 21937aad4, and
git bisect identifies this commit, 3c1571744 ("cat-file: tests for new
atoms added", 2018-02-12), as the culprit.

I'm afraid I'm not going to have the time to investigate the failure
any further in the immediate future, but I wanted to report it
promptly in case you / someone else can see what's going wrong.

Adam

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

[Lars Schneider]

> On 15 Feb 2018, at 21:03, Junio C Hamano  wrote:
> 
> lars.schnei...@autodesk.com writes:
> 
>> -- Git clients that do not support the `working-tree-encoding` attribute
>> -  will checkout the respective files UTF-8 encoded and not in the
>> -  expected encoding. Consequently, these files will appear different
>> -  which typically causes trouble. This is in particular the case for
>> -  older Git versions and alternative Git implementations such as JGit
>> -  or libgit2 (as of February 2018).
>> +- Third party Git implementations that do not support the
>> +  `working-tree-encoding` attribute will checkout the respective files
>> +  UTF-8 encoded and not in the expected encoding. Consequently, these
>> +  files will appear different which typically causes trouble. This is
>> +  in particular the case for older Git versions and alternative Git
>> +  implementations such as JGit or libgit2 (as of February 2018).
> 
> I know somebody found "clients" misleading in the original, but the
> ones that do not understand w-t-e do not have to be third party
> reimplementations and imitations.  All existing Git implementations,
> including ours, don't.

Agreed!


> One thing I find more problematic is that the above places *too*
> much stress on the UTF-8 centric worldview.  It is perfectly valid
> to store your text contents encoded in ShiftJIS and check them out
> as-is, with or without this patch.  It is grossly misleading to say
> that older versions of Git will check them out in UTF-8.  "will
> checkout these files as-is without encoding conversion" is a better
> way to say it, probably.

True. But that's not what I wanted to say in the "pitfalls" section.
If my Git client supports w-t-e and I add the ShiftJIS encoded
file "foo.bar" to my repository, then Git will store the file as
UTF-8 _internally_. That means if you clone my repository and your 
Git client does _not_ support w-t-e, then you will see "foo.bar" as 
UTF-8 encoded.


> Also notice that even in the world with w-t-e, such a project won't
> benefit from w-t-e at all.  After all, they have been happy using
> ShiftJIS in repository and using the same encoding on the working
> tree, and because w-t-e assumes that everybody should be using UTF-8
> internally, such a project cannot take advantage of the new
> mechanism.

Agreed. However, people using ShiftJIS are not my target audience.
My target audience are:

(1) People that have to encode their text files in UTF-16 (for 
whatever reason - usually because of legacy processes or tools).

(2) People that want to see textual diffs of their UTF-16 encoded 
files in their Git tools without special adjustments (on the 
command line, on the web, etc).

That was my primary motivation. The fact that w-t-e supports any
other encoding too is just a nice side effect. I don't foresee people
using other w-t-encodings other than UTF-16 in my organization.

I have the suspicion that the feature could be useful for the Git
community at large. Consider this Stack Overflow question:
https://stackoverflow.com/questions/777949/can-i-make-git-recognize-a-utf-16-file-as-text

This question was viewed 42k times and there is no good solution.
I believe w-t-e could be a good solution.



With your comments in mind, I tried to improve the text like this:

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

...

Please note that using the `working-tree-encoding` attribute may have a
number of pitfalls:

- Alternative Git implementations (e.g. JGit or libgit2) and older Git 
  versions (as of February 2018) do not support the `working-tree-encoding`
  attribute. If you decide to use the `working-tree-encoding` attribute
  in your repository, then it is strongly recommended to ensure that all
  clients working with the repository support it.

  If you declare `*.proj` files as UTF-16 and you add `foo.proj` with an
  `working-tree-encoding` enabled Git client, then `foo.proj` will be
  stored as UTF-8 internally. A client without `working-tree-encoding`
  support will checkout `foo.proj` as UTF-8 encoded file. This will
  typically cause trouble for the users of this file.

  If a Git client, that does not support the `working-tree-encoding`
  attribute, adds a new file `bar.proj`, then `bar.proj` will be
  stored "as-is" internally (in this example probably as UTF-16). 
  A client with `working-tree-encoding` support will interpret the 
  internal contents as UTF-8 and try to convert it to UTF-16 on checkout.
  That operation will fail and cause an error.

...



> And from that point of view, perhaps 

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

[Ævar Arnfjörð Bjarmason]

On Thu, Feb 15 2018, Todd Zullinger jotted:

> [I dropped bbour...@slb.com from the Cc: list, as it bounced
> on my previous reply.]
>
> Ævar Arnfjörð Bjarmason wrote:
>> That makes sense, I'll incorporate that in a re-roll. I like
>> NO_PERL_CPAN_FALLBACKS or just NO_CPAN_FALLBACKS better.
>
> Either is an improvement.  Starting with NO_PERL_ seems
> like a slightly better bikeshed color. :)
>
>> I'd really like to find some solution that works differently though,
>> because with this approach we'll run the full test suite against a
>> system where our fallbacks will be in place (although if the OS
>> distributor has done as promised we won't use them), and then just
>> remove this at 'make install' time, also meaning we'll re-gen it before
>> running 'make install' again, only to rm it again.
>>
>> The former issue we could deal with by munging the Git::LoadCPAN file so
>> it knows about NO_PERL_CPAN_FALLBACKS, and will always refuse to use the
>> fallbacks if that's set. That's a good idea anyway, because right now if
>> you e.g. uninstall Error.pm on Debian (which strips the CPAN fallbacks)
>> you get a cryptic "BUG: ..." message, it should instead say "we couldn't
>> get this module the OS promised we'd have" or something to that effect.
>
> Teaching Git::LoadCPAN to never fallback sounds like a good
> idea.  At least then if the packager intended to avoid the
> fallbacks and didn't get it right the error message could be
> more useful.
>
> Hopefully that's not a common problem for packagers though.
> (And adding the Makefile knob was intended to help make it
> easier for packagers to achieve this common goal.)
>
>> The latter is trickier, I don't see an easy way to coerce the Makefile
>> into not copying the FromCPAN directory without going back to a
>> hardcoded list again, the easiest thing is probably to turn that:
>>
>> $(TAR) cf - .)
>>
>> Into:
>>
>> $(TAR) cf - $(find ... -not )
>>
>> Or something like that to get all the stuff that isn't the Git/FromCPAN
>> directory.
>>
>> Other suggestions most welcome.
>
> What about moving perl/Git/FromCPAN to perl/FromCPAN and
> then including perl/FromCPAN in LIB_PERL{,_GEN} only if
> NO_PERL_CPAN_FALLBACKS is unset?
>
>  LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm 
> perl/Git/*/*/*.pm)
> +ifndef NO_PERL_CPAN_FALLBACKS
> +LIB_PERL += $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
> +endif
>  LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
>
> I haven't tested that at all, so it could be broken in many
> ways.

Yes that's a much better idea, it evades the whole problem of conflating
the perl/Git* glob.

[RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

[Sergey Organov]

Hi,

By accepting the challenges raised in recent discussion of advanced
support for history rebasing and editing in Git, I hopefully figured out
a clean and elegant method of rebasing merges that I think is "The Right
Way (TM)" to perform this so far troublesome operation. ["(TM)" here has
second meaning: a "Trivial Merge (TM)", see below.]

Let me begin by outlining the method in git terms, and special thanks
here must go to "Johannes Sixt"  for his original bright
idea to use "cherry-pick -m1" to rebase merge commits.

End of preface -- here we go.

Given 2 original branches, b1 and b2, and a merge commit M that joins
them, suppose we've already rebased b1 to b1', and b2 to b2'. Suppose
also that B1' and B2' happen to be the tip commits on b1' and b2',
respectively.

To produce merge commit M' that joins b1' and b2', the following
operations will suffice:

1. Checkout b2' and cherry-pick -m2 M, to produce U2' (and new b2').
2. Checkout b1' and cherry-pick -m1 M, to produce U1' (and new b1').
3. Merge --no-ff new b2' to current new b1', to produce UM'.
4. Get rid of U1' and U2' by re-writing parent references of UM' from
   U1' and U2' to  B1' and B2', respectively, to produce M'.
5. Mission complete.

Let's now see why and how the method actually works.

Firs off, let me introduce you to my new friend, the Trivial Merge, or
(TM) for short. By definition, (TM) is a merge that introduces
absolutely no differences to the sides of the merge. (I also like to
sometimes call him "Angel Merge", both as the most beautiful of all
merges, and as direct antithesis to "evil merge".)

One very nice thing about (TM) is that to safely rebase it, it suffices
to merge its (rebased) parents. It is safe in this case, as (TM) itself
doesn't posses any content changes, and thus none could be missed by
replacing it with another merge commit.

I bet most of us have never seen (TM) in practice though, so let's see
how (TM) can help us handle general case of some random merge. What I'm
going to do is to create a virtual (TM) and see how it goes from there.

Let's start with this history:

  M
 / \
B1  B2

And let's transform it to the following one, contextually equivalent to
the original, by introducing 2 simple utility commits U1 and U2, and a
new utility merge commit UM:

  UM
 /  \
U1   U2
||
B1   B2

Here content of any of the created UM, U1, and U2 is the same, and is
exact copy of original content of M. I.e., provided [A] denotes
"content of commit A", we have:

[UM] = [U1] = [U2] = [M]

Stress again how these changes to the history preserve the exact content
of the original merge ([UM] = [M]), and how U1 an U2 represent content
changes due to merge on either side[*], and how neither preceding nor
subsequent commits content would be affected by the change of
representation.

Now observe that as [U1] = [UM], and [U2] = [UM], the UM happens to be
exactly our new friend -- the "Trivial Merge (TM)" his true self,
introducing zero changes to content.

Next we rebase our new representation of the history and we get:

  UM'
 /  \
U1'  U2'
||
B1'  B2'

Here UM' is bare merge of U1' and U2', in exact accordance with the
method of rebasing a (TM) we've already discussed above, and U1' and U2'
are rebased versions of U1 and U2, obtained by usual rebasing methods
for non-merge commits.

(Note, however, that at this point UM' is not necessarily a (TM)
anymore, so in real implementation it may make sense to check if UM' is
not a (TM) and stop for possible user amendment.)

Finally, to get to our required merge commit M', we get the content of
UM' and record two actual parents of the merge:

  M'
 / \
B1' B2'

Where [M'] = [UM'].

That's it. Mission complete.

I expect the method to have the following nice features:

- it carefully preserves user changes by rebasing the merge commit
itself, in a way that is semantically similar to rebasing simple
(non-merge) commits, yet it allows changes made to branches during
history editing to propagate over corresponding merge commit that joins
the branches, even automatically when the changes don't conflict, as
expected.

- it has provision for detection of even slightest chances of ending up
with surprising merge (just check if UM' is still (TM)), so that
implementation could stop for user inspection and amendment when
appropriate, yet it is capable of handling trivial cases smoothly and
automatically.

- it never falls back to simple invocation of merge operation on rebased
original branches themselves, thus avoiding the problem of lack of
knowledge of how the merge at hand has been performed in the first
place. It doesn't prevent implementation from letting user to manually
perform whatever merge she wishes when suspect result is automatically
detected though.

- it extends trivially to octopus merges.

- it appears shiny to the point that it will likely be able to handle
even darkest evil merges nicely, no special treatment required.

Footnote:

[*] We may as well 

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

[Ævar Arnfjörð Bjarmason]

On Thu, Feb 15, 2018 at 4:27 PM,   wrote:
> Since 3733e69464 (use xmallocz to avoid size arithmetic, 2016-02-22) we
> allocate the buffer for the lower case string with xmallocz(). This
> already ensures a NUL at the end of the allocated buffer.
>
> Remove the unnecessary assignment.
> [...]
> for (i = 0; i < len; i++)
> result[i] = tolower(string[i]);
> -   result[i] = '\0';
> return result;
>  }

I agree with this approach, but it's worth noting for other reviewers
that there's been some disagreement here on-list (between Eric & I)
about whether these sorts of patterns should be removed or kept
(although the calloc() case is slightly different from mallocz()),
see: https://public-inbox.org/git/871shum182@evledraar.gmail.com/

[PATCH 1/1] Colorize push errors

[Johannes Schindelin]

From: Ryan Dammrose 

This is an attempt to resolve an issue I experience with people that are
new to Git -- especially colleagues in a team setting -- where they miss
that their push to a remote location failed because the failure and
success both return a block of white text.

An example is if I push something to a remote repository and then a
colleague attempts to push to the same remote repository and the push
fails because it requires them to pull first, but they don't notice
because a success and failure both return a block of white text. They
then continue about their business, thinking it has been successfully
pushed.

My solution was to try to change the stderr and hint colors (red and
yellow, respectively) so whenever there is a failure when pushing to a
remote repository that fails, it is more noticeable.

The challenge was that it seemed that stderr has never been colored and
I attempted to utilize what was already established; but this meant
using functions like want_color() even if it targets stdout while I
wanted to target stderr.

Additionally, to check for all rejection types, I did a strstr check in
transport.c, but this code could be problematic if there is need for
translation.

Signed-off-by: Ryan Dammrose ryandammr...@gmail.com
Signed-off-by: Johannes Schindelin 
---
 advice.c   | 42 +++--
 builtin/push.c | 38 +
 transport.c| 59 +-
 3 files changed, 136 insertions(+), 3 deletions(-)

diff --git a/advice.c b/advice.c
index 406efc183ba..42460877ef6 100644
--- a/advice.c
+++ b/advice.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "config.h"
+#include "color.h"
 
 int advice_push_update_rejected = 1;
 int advice_push_non_ff_current = 1;
@@ -20,6 +21,33 @@ int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
 int advice_waiting_for_editor = 1;
 
+static int advice_use_color = -1;
+static char advice_colors[][COLOR_MAXLEN] = {
+   GIT_COLOR_RESET,
+   GIT_COLOR_YELLOW,   /* HINT */
+};
+
+enum color_advice {
+   ADVICE_COLOR_RESET = 0,
+   ADVICE_COLOR_HINT = 1,
+};
+
+static int parse_advise_color_slot(const char *slot)
+{
+   if (!strcasecmp(slot, "reset"))
+   return ADVICE_COLOR_RESET;
+   if (!strcasecmp(slot, "advice"))
+   return ADVICE_COLOR_HINT;
+   return -1;
+}
+
+static const char *advise_get_color(enum color_advice ix)
+{
+   if (want_color(advice_use_color))
+   return advice_colors[ix];
+   return "";
+}
+
 static struct {
const char *name;
int *preference;
@@ -59,7 +87,8 @@ void advise(const char *advice, ...)
 
for (cp = buf.buf; *cp; cp = np) {
np = strchrnul(cp, '\n');
-   fprintf(stderr, _("hint: %.*s\n"), (int)(np - cp), cp);
+   fprintf(stderr, _("%shint: %.*s%s\n"), 
advise_get_color(ADVICE_COLOR_HINT),
+   (int)(np - cp), cp, 
advise_get_color(ADVICE_COLOR_RESET));
if (*np)
np++;
}
@@ -68,9 +97,18 @@ void advise(const char *advice, ...)
 
 int git_default_advice_config(const char *var, const char *value)
 {
-   const char *k;
+   const char *k, *slot_name;
int i;
 
+   if (skip_prefix(var, "color.advice.", _name)) {
+   int slot = parse_advise_color_slot(slot_name);
+   if (slot < 0)
+   return 0;
+   if (!value)
+   return config_error_nonbool(var);
+   return color_parse(value, advice_colors[slot]);
+   }
+
if (!skip_prefix(var, "advice.", ))
return 0;
 
diff --git a/builtin/push.c b/builtin/push.c
index 1c28427d82e..997ccab52ac 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -12,12 +12,40 @@
 #include "submodule.h"
 #include "submodule-config.h"
 #include "send-pack.h"
+#include "color.h"
 
 static const char * const push_usage[] = {
N_("git push [] [ [...]]"),
NULL,
 };
 
+static int push_use_color = -1;
+static char push_colors[][COLOR_MAXLEN] = {
+   GIT_COLOR_RESET,
+   GIT_COLOR_RED,  /* ERROR */
+};
+
+enum color_push {
+   PUSH_COLOR_RESET = 0,
+   PUSH_COLOR_ERROR = 1
+};
+
+static int parse_push_color_slot(const char *slot)
+{
+   if (!strcasecmp(slot, "reset"))
+   return PUSH_COLOR_RESET;
+   if (!strcasecmp(slot, "error"))
+   return PUSH_COLOR_ERROR;
+   return -1;
+}
+
+static const char *push_get_color(enum color_push ix)
+{
+   if (want_color(push_use_color))
+   return push_colors[ix];
+   return "";
+}
+
 static int thin = 1;
 static int deleterefs;
 static const char *receivepack;
@@ -338,7 +366,9 @@ static int push_with_options(struct transport *transport, 
int flags)
err = transport_push(transport, refspec_nr, 

[PATCH 0/1] Colorize some errors on stderr

[Johannes Schindelin]

This is an RFC because it tries to introduce a fundamental new color feature:
Coloring messages *on stderr*.

So far, pretty much everything in color.[ch] assumed that you want to color
only stuff on stdout.

However, in this case, a user (who became a contributor!) wanted some messages
that are printed to stderr and were missed by his colleagues to be colored.

The contribution comes via Pull Request from the Git for Windows project:

https://github.com/git-for-windows/git/pull/1429

Now, what would be possible solutions for this?

- introduce `int fd` in `want_color()` (and callees) so that we can make
  a distinction whether we want to detect whether stdout or stderr is connected
  to a tty

- introduce a separate `want_color_stderr()` (we still would need to decide
  whether we want a config setting for this)

- not color stderr, ever

Also, I did not have too much time to dig into the question how to test this in
Git's test suite. Do we already have tests that generate fake server-side errors
onto which I could piggy-back a new test case?

Thoughts? Suggestions? Help?


Ryan Dammrose (1):
  Colorize push errors

 advice.c   | 42 +++--
 builtin/push.c | 38 +
 transport.c| 59 +-
 3 files changed, 136 insertions(+), 3 deletions(-)


base-commit: b2e45c695d09f6a31ce09347ae0a5d2cdfe9dd4e
Published-As: https://github.com/dscho/git/releases/tag/colorize-push-errors-v1
Fetch-It-Via: git fetch https://github.com/dscho/git colorize-push-errors-v1
-- 
2.16.1.windows.4

My Seasons of greetings to you and your family.

[Mohammed ZONGO]

Dear Friend,
My Seasons of greetings to you and your family.

First, I am Mr. Mohammed ZONGO, a banker working with Bank of Africa
here in my country Burkina Faso West Africa, I have a business
transaction that will benefit both of us, and want to know if you can
handle it and claim the fund to your country's account. The amount is
($10.5Million). After the transfer, we have to share it, 50% for me,
and 50% for you. Please let me know if you can assist me, for more
details information regarding the transfer. I hope you will work with
me honestly?

This is legal, risk free and will be 100% successful, because I work
in the same bank and have arranged for it properly before contacting
you.

If yes, update me with your personal information such as;

1. Your name in Full:
2. Your House Address:
3. Your Occupation:
4. Your Age:
5. Your direct phone number;

Regards.
Mr. Mohammed ZONGO.

[PATCH] git-worktree.txt (single char)

[Mike Nordell]

Missing ')' after the closing '`'.

"If  is a branch name (call it `` and"

Re: Is the -w option for git blame bugged?

[Konstantin Khomoutov]

On Fri, Feb 16, 2018 at 05:47:41PM +0800, hgfds jhgfds wrote:

> I recently asked a question on stackoverflow regarding what seemed to
> be erroneous output from git blame when the -w option is specified.
> However, no one answered my question, so I decided to ask here
> instead.
> 
> The question is available at
> https://stackoverflow.com/questions/48808281/git-blame-ignore-whitespace-option-bugged
> 
> Hope someone can help to shed light on this issue soon.

This is indeed the correct place to ask such questions.
Still, the next time please literally ask the question - inclusing its
full text - rather than dropping a link: few people bother do to that,
and even fewer of them have an SO account to actually answer it.
So it's both just basic netiquette and increasing the circle of the
potentional helpers ;-)

In order to remove the churn, I've copied and pasted your question here;
hope you have no issues with me doing this.

8<
Based on my understanding, the command `git blame` is supposed to show, for
each line in a file, the author and the commit in which the line was last
modified.  So for example, if I run

git blame -- ""

and get the following output for line 5:

106b77db (Walrus 2016-03-24 10:01:36 +0800   5) .root {

it means the line

.root {

originated from the author Walrus in the commit 106b77db.
In other words, if I inspect the patch produced by 106b77db using
`git show -p 106b77db`, I would expect the line

+.root {

to show up in the diff. Indeed, this is the case.

Snippet from 106b77db's diff for :

 /* JavaFX CSS - Leave this comment until you have at least create one rule 
  which uses -fx-Property */
+.root {
+   -fx-background-color: transparent;
+}
+

Now, when I run

git blame -w -- ""

(the -w option ignores whitespace changes, i.e. traces each line backwards in
time to find the last author which introduced non-whitespace changes to that
line), I now get the following output for line 5:

b6a6e8a2 (Walrus 2016-03-31 23:32:50 +0800   5) .root {

However, when I inspect the patch for b6a6e8a2 using

git show -p b6a6e8a2

the diff shows

.root {

rather than

+.root {

as expected.

Snippet from b6a6e8a2's diff for :

+
+/* setting window to be transparent */
 .root {
-fx-background-color: transparent;
-}
-

Has Git given me erroneous output, because according to the diff, the line
.root { was not modified at all in the commit b6a6e8a2?

I am using Git 2.13.3.windows.1.

EDIT: the repository is https://github.com/cs2103jan2016-f14-2j/main,
and the file is JimplePlanner/src/application.css.
After upgrading to Git 2.16.1.windows.4, the issue still persists.
8<

Is the -w option for git blame bugged?

[hgfds jhgfds]

Hi,

I recently asked a question on stackoverflow regarding what seemed to
be erroneous output from git blame when the -w option is specified.
However, no one answered my question, so I decided to ask here
instead.

The question is available at
https://stackoverflow.com/questions/48808281/git-blame-ignore-whitespace-option-bugged

Hope someone can help to shed light on this issue soon.

Thanks!