Re: [PATCH/RFC] WIP: log: allow "-" as a short-hand for "previous branch"

[Matthieu Moy]

Siddharth Kannan  writes:

> Making a change in sha1_name.c will touch a lot of commands
> (setup_revisions is called from everywhere in the codebase), so, I am
> still trying to figure out how to do this such that the rest of the
> codepath remains unchanged.

Changing sha1_name.c is the way to go *if* we want all commands to
support this. Just like other ways to name a revision...

> I hope that you do not mind this side-effect, but rather, you intended
> for this to happen, right? More commands will start supporting this
> shorthand, suddenly.  (such as format-patch, whatchanged, diff to name
> a very few).

... but: the initial implementation of this '-' shorthand was
special-casing a single command (IIRC, "git checkout") for which the
shorthand was useful.

In a previous discussion, I made an analogy with "cd -" (which is the
source of inspiration of this shorthand AFAIK): "-" did not magically
become "the last visited directory" for all Unix commands, just for
"cd". And in this case, I'm happy with it. For example, I never need
"mkdir -", and I'm happy I can't "rm -fr -" by mistake.

So, it's debatable whether it's a good thing to have all commands
support "-". For example, forcing users to explicitly type "git branch
-d @{1}" and not providing them with a shortcut might be a good thing.

I don't have strong opinion on this: I tend to favor consistency and
supporting "-" everywhere goes in this direction, but I think the
downsides should be considered too. A large part of the exercice here is
to write a good commit message!

Another issue with this is: - is also a common way to say "use stdin
instead of a file", so before enabling - for "previous branch", we need
to make sure it does not introduce any ambiguity. Git does not seem to
use "- for stdin" much (most commands able to read from stdin have an
explicit --stdin option for that), a quick grep in the docs shows only
"git blame --contents -" which is OK because a revision wouldn't make
sense here anyway.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: GSoC 2017: application open, deadline = February 9, 2017

[Matthieu Moy]

Jeff King  writes:

> On Mon, Jan 23, 2017 at 04:02:02PM +0100, Matthieu Moy wrote:
>
>> * We need to write the application, i.e. essentially polish and update
>>   the text here: https://git.github.io/SoC-2016-Org-Application/ and
>>   update the list of project ideas and microprojects :
>>   https://git.github.io/SoC-2017-Ideas/
>>   https://git.github.io/SoC-2016-Microprojects/
>
> That can be done incrementally by people who care (especially mentors)
> over the next week or so, and doesn't require any real admin
> coordination. If it happens and the result looks good, then the
> application process is pretty straightforward.
>
> If it doesn't, then we probably ought not to participate in GSoC.

OK, it seems the last message did not raise a lot of enthousiasm (unless
I missed some off-list discussion at Git-Merge?).

The application deadline is tomorrow. I think it's time to admit that we
won't participate this year, unless someone steps in really soon.

If we don't participate, I'll add a disclaimer at the top of the
SoC-related pages on git.github.io to make sure students don't waste
time preparing an application.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 1/5] add SWAP macro

[Johannes Schindelin]

Hi Junio & René,

On Tue, 7 Feb 2017, Junio C Hamano wrote:

> René Scharfe  writes:
> 
> > Swapping between different types would then still have to be done
> > manually, but I wonder how common that is -- couldn't find such a case
> > in our tree.
> 
> I do not think it is a common thing to do, and more importantly, I doubt
> we want to hide such a swap inside a macro.  And that is why I said the
> seemingly extra "type" thing may be an improvement over your original
> SWAP() thing if it gives us more type safety.
> 
> It seems that the thread has been quite for a while. Perhaps people are
> happy enough with your patches?  If so, let's move it forward, but I'll
> wait for a while in case follow-up discussion appears soonish.  The
> changes are fairly well isolated and I do not think we are in a hurry.

I am still unhappy about choosing to complicate things and lean heavily on
the compiler to make things right again.

But it appears that I am the only one with this concern, so go ahead. I
promise not to say "I told you so" in case that it breaks things or that
certain platforms are experiencing a disadvantage.

Ciao,
Johannes

Re: GSoC 2017: application open, deadline = February 9, 2017

[Jeff King]

On Wed, Feb 08, 2017 at 03:54:25PM +0100, Matthieu Moy wrote:

> >> * We need to write the application, i.e. essentially polish and update
> >>   the text here: https://git.github.io/SoC-2016-Org-Application/ and
> >>   update the list of project ideas and microprojects :
> >>   https://git.github.io/SoC-2017-Ideas/
> >>   https://git.github.io/SoC-2016-Microprojects/
> >
> > That can be done incrementally by people who care (especially mentors)
> > over the next week or so, and doesn't require any real admin
> > coordination. If it happens and the result looks good, then the
> > application process is pretty straightforward.
> >
> > If it doesn't, then we probably ought not to participate in GSoC.
> 
> OK, it seems the last message did not raise a lot of enthousiasm (unless
> I missed some off-list discussion at Git-Merge?).

Nope, there was no discussion that I'm aware of.

> The application deadline is tomorrow. I think it's time to admit that we
> won't participate this year, unless someone steps in really soon.

Yes, I'd agree with that.

Outreachy folks asked if we were interested in participating, but I
think it has roughly the same pre-requisite lists for ideas and
microprojects.

> If we don't participate, I'll add a disclaimer at the top of the
> SoC-related pages on git.github.io to make sure students don't waste
> time preparing an application.

Good idea.

-Peff

Re: init --separate-git-dir does not set core.worktree

[Stefan Beller]

> [*] https://github.com/magit/magit/issues/460#issuecomment-36035787.

I would agree with the thinking in that issue.

Копия: Sеx & Daтіng: Тесhnology and тhе fеаr of rejесtіоn

[santehmetservis-m]

Это копия сообщения, которое вы отправили ООО "УТЭП-Московский" через 
УТЭП-Московский

Это письмо отправлено с сайта http://danfoss-utep.ru/ от:
LolaMagboidoda 

Оftеntiмеs, тhе sаyіng, “thеrе аrе рlеnty оf fish іn тhе sea,” іs gіvеn аs а 
соnsolatіоn аfтer a breакuр or аs rеassurаnсе during a реriоd of lоnelіnеss. Іf 
тhis sеа rеally еxіsтs, surеly а рооl suсh as UGА, flооdеd with тhousаnds of 
undеrgrаduate sтudеnтs, wоuld sеrvе аs a good рlаce тo сast baіт. 
 
Ноwеvеr, sомеtimеs iт seемs fеw fish are bitіng оn самрus—a nотіon тhat cоuld 
bе aтtributеd tо тhe laск of fishеrмеn саsтіng тhеir neтs in-рersоn. Insтead, 
маny studеnтs fіnd dаtеs onlіne тhrоugh apрlicатіоns. 
 
http://wooga.info/6Kts#SwothwowQuonvartdon>Sex & Dатіng

Re: init --separate-git-dir does not set core.worktree

[Stefan Beller]

On Fri, Feb 3, 2017 at 5:38 AM, Duy Nguyen  wrote:
> On Thu, Feb 2, 2017 at 7:37 PM, Kyle Meyer  wrote:
>> Duy Nguyen  writes:
>>
>>> On Thu, Feb 2, 2017 at 10:55 AM, Kyle Meyer  wrote:

 As of 6311cfaf9 (init: do not set unnecessary core.worktree,
 2016-09-25), "git init --separate-git-dir" no longer sets core.worktree
 (test below).  Based on the commit message and the corresponding thread
 [1], I don't think this change in behavior was intentional, but I wasn't
 able to understand things well enough to attempt a patch.
>>>
>>> I'm missing some context. Why does --separate-git-dir have to set
>>> core.worktree? What fails for you exactly?
>>
>> Sorry for not providing enough information.  I haven't run into a
>> failure.
>>
>> In Magit, we need to determine the top-level of the working tree from
>> COMMIT_EDITMSG.  Right now that logic [*1*] looks something like this:
>
> This is much better :)
>
>>  * COMMIT_EDITMSG in .git/modules//: set working tree to the
>>output of "git rev-parse --show-toplevel"

.git/modules// itself is a fully fledged git directory, making use of
the core.worktree variable to have the working tree at an "unusual" place.

"git rev-parse --show-toplevel" is the correct place of the working tree

>>
>>  * COMMIT_EDITMSG in .git/worktrees//: set working tree to the
>>path in .git/worktrees//gitdir, minus the trailing "/.git"

(in worktree/:) "git rev-parse --show-toplevel"s output is empty,
but the gitdir file exists unlike in the other cases.

>>
>>  * COMMIT_EDITMSG in .git: set working tree to the parent directory

(in .git:) "git rev-parse --show-toplevel"s output is empty.

>>
>> This fails for a repo set up with --separate-git-dir [*2*], where the
>> last step will go out into an unrelated repo.  If core.worktree was set
>> and "git rev-parse --show-toplevel" returned the working tree like it
>> did for submodules, things would work.

When using --separate-git-dir you cannot tell where the working tree is from
within the git dir, despite knowing it has to be *somewhere*:

[core]
bare = false

So I would hope we'll set core.worktree again in this case. Otherwise how
would we discover the working tree?

>
> OK. If I read this right, given a path of any text file somewhere
> within ".git" directory. you are tasked to find out where the
> associated worktree is? I.e. this is not an emacsclient executed as
> part of "git commit", correct?

I always assumed core.worktree is precisely that analogy, i.e.
core.worktree is the backwards pointer to the .git file (which
is true coming from a submodule background).

>
> So you need some sort of back-link to ".git" location. And
> unfortunately there's no such thing for .git file (unless it points to
> .git/worktrees/...). I'm hesitant to set core.worktree unless it's
> absolutely needed since it may have unexpected interaction with
> $GIT_WORK_TREE and others (not sure if it has any interaction with
> submodules too). I think adding a new file "gitdir" would be a safer
> option.

How would "gitdir" (should it be called worktree/workingtree instead?)
work together with core.worktree set?
Would it point at the .git file or the root level of the working tree?

>
> I'm not entirely sure if enforcing "one worktree - one repository" is
> safe though. The first two bullet points are very specific and we can
> assume that, but ".git" files alone can be used for anything. In
> theory you can always create a secondary worktree (that's not managed
> by "git worktree") by setting GIT_WORK_TREE and "git checkout -f"
> somewhere. But I guess those would be temporary and nobody would want
> magic to point back to them.
>
> As a fall-back mechanism, I think after magit has found the worktree,
> it should verify the found location is the correct worktree, with "git
> rev-parse --git-dir" or something, and alert the user otherwise. I
> think "git rev-parse --git-path COMMIT_MSG" should give back the same
> COMMIT_MSG path (and it applies for any files in .git dir, covering
> all three cases). The user could add some magit-specific files to tell
> magit where the actual worktree is when they hit corner cases.
>
> If the use case is limited to editing COMMIT_EDITMSG only (after it's
> generated by git), it may be best to add `pwd` as a comment to that
> file. You won't have to go through all the magic rules to find it out
> (*). And it helps non-magic users too.
>
> (*) well, you do, because you probably can't expect everybody to have
> latest git version.
>
>> Of course, the issue above isn't a reason that --separate-git-dir should
>> set core.worktree, but the submodule behavior is why we were wondering
>> if it should.
>
> I'm not a submodule person, so I'll pass that "why" question to Stefan.

Good question. As said above I always assumed it is the backlink to know where
the working tree of the submodule is.

Digging through history I found 

Re: [PATCH] difftool: fix bug when printing usage

[Johannes Schindelin]

Hi Junio,

On Tue, 7 Feb 2017, Junio C Hamano wrote:

> Junio C Hamano  writes:
> 
> > Johannes Schindelin  writes:
> >
> >>> > Likewise, this would become
> >>> >
> >>> > GIT_CEILING_DIRECTORIES="$PWD/not" \
> >>> > test_expect_code 129 git -C not/repo difftool -h >output &&
> >>> > grep ^usage: output
> >>> 
> >>> I agree with the intent, but the execution here is "Not quite".
> >>> test_expect_code being a shell function, it does not take the
> >>> "one-shot environment assignment for this single invocation," like
> >>> external commands do.
> >>
> >> So now that we know what is wrong, can you please enlighten me about
> >> what is right?
> >
> > David's original is just fine, isn't it?
> 
> I've also seen people use "env VAR=VAL git command" as the command to be
> tested in t/ scripts.  You can run that under test_expect_code,
> methinks.

That is exactly what David ended up sending out as follow-up patches.

I did not mean to be critical, I just found it to be more helpful to
accompany "that does not work" comments with "but this does" in the past.

Ciao,
Johannes

Dear God's Select,

[mrs louisa besson]

Dear God's Select,

I am writing this mail to you with heavy tears In my eyes and great
sorrow in my heart, My Name is Mrs.Louisa Besson, and I am contacting
you from my countryTunisia I want to tell you this because I don't
have any other option than totell you as I was touched to open up to
you, I married to Mr.samih habib. Who worked with Tunisia embassy in
Burkina Faso for nine years before he died in the year 2005.We were
married for eleven years without a child

He died after a brief illness that lasted for only five days. Since
his death I decided not to remarry, When my late husband was alive he
deposited the sum of US$ 8.5m (Eight million And Five hundred Thousand
Dollars) in a bank in Ouagadougou the capital city of Burkina Faso in
west Africa Presently this money is still in bank. He made this money
available for exportation of Gold from Burkina Faso mining.

Recently, My Doctor told me that I would not last for the period of
seven months due to cancer problem. The one that disturbs me most is
my stroke sickness.Having known my condition I decided to hand you
over this money to take care of the less-privileged people, you will
utilize this money the way I am going to instruct herein.

I want you to take 30 Percent of the total money for your personal use
While 70% of the money will go to charity, people in the street and
helping the orphanage. I grew up as an Orphan and I don't have any
body as my family member, just to endeavour that the house of God is
maintained. Am doing this so that God will forgive my sins and accept
my soul because these sicknesses have suffered me so much.

As soon as I receive your reply I shall give you the contact of the
bank in Burkina Faso and I will also instruct the Bank Manager to
issue you an authority letter that will prove you the present
beneficiary of the money in the bank that is if you assure me that you
will act accordingly as I Stated herein.

Always reply to my alternative for security purposes.

If you are interested in carrying out this task please contact my
private email at (mrslouisa...@gmail.com ) for more details on this
noble project of mine.

Hoping to receive your reply:

yours Faithfully,
>From Mrs.Louisa Benson,

Re: Fwd: Possibly nicer pathspec syntax?

[Junio C Hamano]

Duy Nguyen  writes:

> On Wed, Feb 8, 2017 at 12:12 PM, Linus Torvalds
>  wrote:
>> Two-patch series to follow.
>
> glossary-content.txt update for both patches would be nice.

I am no longer worried about it as I saw somebody actually sent
follow-up patches on this, but I want to pick your brain on one
thing that is related to this codepath.

We have PATHSPEC_PREFER_CWD and PATHSPEC_PREFER_FULL bits in flags,
added at fc12261fea ("parse_pathspec: add PATHSPEC_PREFER_{CWD,FULL}
flags", 2013-07-14), and I think the intent is some commands when
given no pathspec work on all paths in the current subdirectory
while others work on the full tree, regardless of where you are.
"grep" is in the former camp, "log" is in the latter.  And there is
a check to catch a bug in a caller that sets both.

I am wondering about this hunk (this is from the original commit
that added it):

if (!entry) {
static const char *raw[2];
 
+   if (flags & PATHSPEC_PREFER_FULL)
+   return;
+
+   if (!(flags & PATHSPEC_PREFER_CWD))
+   die("BUG: PATHSPEC_PREFER_CWD requires arguments");
+
pathspec->items = item = xmalloc(sizeof(*item));
memset(item, 0, sizeof(*item));
item->match = prefix;
... returns a single entry pathspec to cover cwd ...

The BUG message is given when 

 - The command got no pathspec from the caller; and
 - PATHSPEC_PREFER_FULL is not set; and
 - PATHSPEC_PREFER_CWD is NOT set.

but the message says that the caller must have args when it sets
prefer-cwd.  Is this a simple typo?  If so what should it say?

die("BUG: one of PATHSPEC_PREFER_FULL or _CWD must be set");

If that were the case, we are expressing only one bit of information
(do we limit to cwd, or do we work on full-tree?), but there must
have been a reason why we added two bits and made them mutually
incompatible so that we can express three possibilities.

Does this third possibility (i.e. a caller is allowed to pass
"flags" that does not prefer either) exist to support a command
where the caller MUST have at least one pathspec?  If that were the
case, this wouldn't be a BUG but an end-user error, e.g.

die("at least one pathspec element is required");

If you know offhand which callers pass neither of the two
PATHSPEC_PREFER_* bits and remember for what purpose you allowed
them to do so, please remind me.  I'll keep digging in the meantime.

Thanks.

Re: "disabling bitmap writing, as some objects are not being packed"?

[Junio C Hamano]

Duy Nguyen  writes:

> On second thought, perhaps gc.autoDetach should default to false if
> there's no tty, since its main point it to stop breaking interactive
> usage. That would make the server side happy (no tty there).

Sounds like an idea, but wouldn't that keep the end-user coming over
the network waiting after accepting a push until the GC completes, I
wonder.  If an impatient user disconnects, would that end up killing
an ongoing GC?  etc.

Re: [PATCH] push options: fail properly in the stateless case

[Junio C Hamano]

Stefan Beller  writes:

> When using non-builtin protocols relying on a transport helper
> (such as http), push options are not propagated to the helper.
>
> Fix this by propagating the push options to the transport helper and

The description up to this point is VERY readable and sensible.  But
that makes the title sound a bit strange.  I read it as if it were
saying "stateless case can never support push-options so fail if the
caller attempts to use one", but that does not seem to be what is
going on.

> adding a test that push options using http fail properly.

Sounds sensible.  What end-user visible effect does this fix have?
IOW, what feature do we use "push-option" for?

Ahh, OK, so you need to describe that there are two issues in order
to be understood by the readers:

 (1) the helper protocol does not propagate push-option
 (2) the http helper is not prepared to handle push-option

You fix (1), and you take advantage of the fact (2) to ensure that
(1) is fixed in the new test.

With such an understanding, the title makes (sort of) sense and you
wouldn't have to be asked "what end-user visible effect/benefit does
this have?"

> +'option push-option ::
> + Transmit this push option.
> +

There is no "c-string" in the current documentation used or
defined.  The closest thing I found is

... that field will be quoted in the manner of a C string ...

in git-status page, but I do not think you send the value for an
push-option after running quote_c_style(), so I am puzzled.

I'd rather see 'option push-option ' as the bullet item, and
in its description say how arbitrary values (if you allow them, that
is) can be used, e.g. "Transmit  encoded in such and such
way a the value of the push-option".

Re: Trying to use xfuncname without success.

[Jack Adrian Zappa]

Thanks Rene, but you seem to have missed the point.  NOTHING is
working.  No matter what I put there, it doesn't seem to get matched.

Just to be sure, I tested your regex and again it didn't work.

Someone on the SO site stated they could get it to work on FreeBSD and
I'm on Windows, so this might be a platform thing.  Can anyone else on
Windows please confirm?

Thanks,


A

On Tue, Feb 7, 2017 at 6:18 PM, René Scharfe  wrote:
> Am 07.02.2017 um 20:21 schrieb Jack Adrian Zappa:
>>
>> I'm trying to setup a hunk header for .natvis files. For some reason,
>> it doesn't seem to be working. I'm following their instructions from
>> here, which doesn't say much in terms of restrictions of the regex,
>> such as, is the matched item considered the hunk header or do I need a
>> group? I have tried both with no success. This is what I have:
>>
>> [diff "natvis"]
>> xfuncname = "^[\\\t ]*
>
> The extra "\\" allow backslashes to be used for indentation as well as
> between Type and Name, which is probably not what you want.  And your
> expression only matches single-char Name attributes.  Try:
>
> xfuncname = "^[\t ]*
> René

Re: [PATCH RFC 0/2] Kill manual ref parsing code in worktree.c

[Junio C Hamano]

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

> A hundred years ago I added this code because a "standalone" ref
> parsing code was not available from refs.c and the file was going through
> some heavy changes that refactoring its ref parsing code out was not
> an option. I promised to kill this parse_ref() eventually. I'm
> fulfilling it today (or soon).

;-)  

Thanks.  And thanks for looping Michael in.  I'd appreciate his
input in this area.

> I would really like to you double check the approach I'm using here
> (using submodule interface for accessing refs from another worktree)
> since that may be the way forward to fix the "gc losing objects" in
> multi worktrees. I've given it lots of thoughts in the last 24 hours.
> Still can't find any fundamental flaw...

I see that you posted a separate message outlining the idea
yesterday and I didn't see any response (and I was sick and lacked
energy to think it through); I think the basic approach to use "an
API to bring set of refs hidden from your normal view" is sensible.
Except for the unfortunate naming of the interface that makes it
sound as if it is only to access submodules, but that is where the
feature original came from, so let's not complain too loudly ;-)

> Nguyễn Thái Ngọc Duy (2):
>   refs.c: add resolve_ref_submodule()
>   worktree.c: use submodule interface to access refs from another worktree
>
>  branch.c   |  3 +-
>  refs.c | 20 +
>  refs.h |  3 ++
>  worktree.c | 99 
> +++---
>  worktree.h |  2 +-
>  5 files changed, 44 insertions(+), 83 deletions(-)

Software Freedom Conservancy donations

[Jeff King]

The recent thread about the git-scm.com website raised some discussion
about money, and a lot of people offered financial assistance. As I said
in that thread, we _don't_ have a dire need for money to keep hosting
the site.

But we do sometimes need money for other things (like conference
travel), and we rely on services (like accounting and legal advice)
provided by the Software Freedom Conservancy which are free to us, but
funded ultimately by donations.

I went into detail on our project activities and finances recently at:

  
http://public-inbox.org/git/20170202024501.57hrw4657tsqe...@sigill.intra.peff.net/

If you are interested in contributing financially, you can either donate
to the Git project (10% of which goes to Conservancy's general fund, and
the rest ends up in Git's account):

  https://git-scm.com/sfc

Or you can donate directly to Conservancy's general fund:

  https://sfconservancy.org/donate/

They also have an annual Supporter program, which includes the
all-important t-shirt:

  https://sfconservancy.org/supporter/

Through February 13th, there's an anonymous donor matching all Supporter
signups and renewals, so any donation you make before then will go twice
as far.

-Peff

Re: Trying to use xfuncname without success.

[René Scharfe]

Am 08.02.2017 um 18:11 schrieb Jack Adrian Zappa:
> Thanks Rene, but you seem to have missed the point.  NOTHING is
> working.  No matter what I put there, it doesn't seem to get matched.

I'm not so sure about that.  With your example I get this diff without
setting diff.natvis.xfuncname:

diff --git a/a.natvis b/a.natvis
index 7f9bdf5..bc3c090 100644
--- a/a.natvis
+++ b/a.natvis
@@ -19,7 +19,7 @@ 
xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010;>
 
 
   
-  added_var
+  added_vars
 
 
   var2

Note the XML namespace in the hunk header.  It's put there by the
default rule because "xmlns" starts at the beginning of the line.  Your
diff has nothing there, which means the default rule is not used, i.e.
your user-defined rule is in effect.

Come to think of it, this line break in the middle of the AutoVisualizer
tab might have been added by your email client unintentionally, so that
we use different test files, which then of course results in different
diffs.  Is that the case?

Anyway, if I run the following two commands:

$ git config diff.natvis.xfuncname "^[\t ]*.gitattributes

... then I get this, both on Linux (git version 2.11.1) and on Windows
(git version 2.11.1.windows.1):

diff --git a/a.natvis b/a.natvis
index 7f9bdf5..bc3c090 100644
--- a/a.natvis
+++ b/a.natvis
@@ -19,7 +19,7 @@ test
 
 
   
-  added_var
+  added_vars
 
 
   var2

> Just to be sure, I tested your regex and again it didn't work.

At this point I'm out of ideas, sorry. :(  The only way I was able to
break it was due to mistyping the extension as "netvis" several times
for some reason.

René

Re: [PATCH] rev-parse --git-path: fix output when running in a subdirectory

[Junio C Hamano]

Johannes Schindelin  writes:

> The problem lies deeper, much deeper...
> ... but it bought us many, many problems.

I think you are making a mountain out of molehill here.  
This looks like as an opposite problem of a bug that forgets to
prepend the prefix to relative pathnames given by the end user.

I do agree that some calling scripts may find it more convenient if
the output were relative to their current directory, and in that
sense, this is worth addressing.

However.

How long has "rev-parse --git-path" been around?  Had scripts in the
wild chance to learn to live with the "output is relative to the top
of the working tree" reality?  I think the answers are "since 2.5" and
"yes".

I do not think we can make this unconditionally without breaking
users.  We instead need something like a new "--git-path-relative"
option, similar in the spirit that output from "git diff" can be
made relative to the current directory with the "--relative" option.

Assuming that we are discussing the new behaviour that is
conditionally triggered, let's see the implementation.

> - puts(git_path("%s", argv[i + 1]));
> + path = git_path("%s", argv[i + 1]);
> + if (prefix && !is_absolute_path(path))
> + path = real_path(path);
> + puts(path);

Duy, want to help me out here?  I am wondering if using a logic
similar to what is used by "cd t && git grep -e foo :/" to emit
paths as "../Documentation/CodingGuidelines" as relative to the
current working directory is more appropriate than forcing the
absolute path output here (and if so, it may be preferrable to use
the relative_path() helper to do so), or the paths to files in
$GIT_DIR are conceptually different enough from paths to files in
the working tree and it will be more robust to have the output as an
absolute path.

I am leaning toward the latter (i.e. the above use of real_path() is
simple and good), but I haven't thought things through and since we
have an area expert here in the thread...

Re: Trying to use xfuncname without success.

[Samuel Lijin]

I just put this togther: https://github.com/sxlijin/xfuncname-test

Try cloning and then for any of config1 thru 3,

$ cp configX .git/config
$ git diff HEAD^ -- test.natvis

On Wed, Feb 8, 2017 at 2:42 PM, Jack Adrian Zappa  wrote:
> Thanks Samuel,
>
> So, the question is, what is causing this problem on my system?
>
> Anyone have an idea to help diagnose this problem?
>
> On Wed, Feb 8, 2017 at 3:24 PM, Samuel Lijin  wrote:
>> On Windows 7, it works for me in both CMD and Git Bash:
>>
>> $ git --version
>> git version 2.11.0.windows.3
>>
>> $ git diff HEAD^ --word-diff
>> diff --git a/test.natvis b/test.natvis
>> index 93396ad..1233b8c 100644
>> --- a/test.natvis
>> +++ b/test.natvis
>> @@ -18,6 +18,7 @@ test
>>
>>
>>   
>>   {+added_var+}
>>
>>
>>   var2
>>
>> On Wed, Feb 8, 2017 at 12:37 PM, René Scharfe  wrote:
>>> Am 08.02.2017 um 18:11 schrieb Jack Adrian Zappa:
 Thanks Rene, but you seem to have missed the point.  NOTHING is
 working.  No matter what I put there, it doesn't seem to get matched.
>>>
>>> I'm not so sure about that.  With your example I get this diff without
>>> setting diff.natvis.xfuncname:
>>>
>>> diff --git a/a.natvis b/a.natvis
>>> index 7f9bdf5..bc3c090 100644
>>> --- a/a.natvis
>>> +++ b/a.natvis
>>> @@ -19,7 +19,7 @@ 
>>> xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010;>
>>>
>>>
>>>
>>> -  added_var
>>> +  added_vars
>>>
>>>
>>>var2
>>>
>>> Note the XML namespace in the hunk header.  It's put there by the
>>> default rule because "xmlns" starts at the beginning of the line.  Your
>>> diff has nothing there, which means the default rule is not used, i.e.
>>> your user-defined rule is in effect.
>>>
>>> Come to think of it, this line break in the middle of the AutoVisualizer
>>> tab might have been added by your email client unintentionally, so that
>>> we use different test files, which then of course results in different
>>> diffs.  Is that the case?
>>>
>>> Anyway, if I run the following two commands:
>>>
>>> $ git config diff.natvis.xfuncname "^[\t ]*>> $ echo '*.natvis diff=natvis' >.gitattributes
>>>
>>> ... then I get this, both on Linux (git version 2.11.1) and on Windows
>>> (git version 2.11.1.windows.1):
>>>
>>> diff --git a/a.natvis b/a.natvis
>>> index 7f9bdf5..bc3c090 100644
>>> --- a/a.natvis
>>> +++ b/a.natvis
>>> @@ -19,7 +19,7 @@ test
>>>
>>>
>>>
>>> -  added_var
>>> +  added_vars
>>>
>>>
>>>var2
>>>
 Just to be sure, I tested your regex and again it didn't work.
>>>
>>> At this point I'm out of ideas, sorry. :(  The only way I was able to
>>> break it was due to mistyping the extension as "netvis" several times
>>> for some reason.
>>>
>>> René

Re: [PATCH] push options: fail properly in the stateless case

[Stefan Beller]

On Wed, Feb 8, 2017 at 10:11 AM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> When using non-builtin protocols relying on a transport helper
>> (such as http), push options are not propagated to the helper.
>>
>> Fix this by propagating the push options to the transport helper and
>
> The description up to this point is VERY readable and sensible.  But
> that makes the title sound a bit strange.

Yes, the title was there first and then I massaged the commit message
until it was readable. Originally I thought the issue is with stateless
protocols, but that is wrong. The underlying issue is just the transport
helper communication lacking.

> I read it as if it were
> saying "stateless case can never support push-options so fail if the
> caller attempts to use one", but that does not seem to be what is
> going on.

Indeed the subject is wrong.

>
>> adding a test that push options using http fail properly.
>
> Sounds sensible.  What end-user visible effect does this fix have?
> IOW, what feature do we use "push-option" for?

The Gerrit world started using push options for having a better git
experience, not needing to navigate to the web UI, e.g.:

# push for review and set me as a reviewer:
git push --push-option reviewer=sbel...@google.com  ssh://gerrit...

# same with http, it looks like it worked, but the push option
# never made it to the server
git push --push-option reviewer=sbel...@google.com  http://gerrit...

# This patch will make the second command fail, reporting
# the http helper not supporting push options.

>
> Ahh, OK, so you need to describe that there are two issues in order
> to be understood by the readers:
>
>  (1) the helper protocol does not propagate push-option
>  (2) the http helper is not prepared to handle push-option
>
> You fix (1), and you take advantage of the fact (2) to ensure that
> (1) is fixed in the new test.
>
> With such an understanding, the title makes (sort of) sense and you
> wouldn't have to be asked "what end-user visible effect/benefit does
> this have?"

Your analysis is correct.

>
>> +'option push-option ::
>> + Transmit this push option.
>> +
>
> There is no "c-string" in the current documentation used or
> defined.  The closest thing I found is
>
> ... that field will be quoted in the manner of a C string ...
>
> in git-status page, but I do not think you send the value for an
> push-option after running quote_c_style(), so I am puzzled.

When implementing push options, we discussed that and according to
Documentation/git-push:

The given string must not contain a NUL or LF character.

>
> I'd rather see 'option push-option ' as the bullet item, and
> in its description say how arbitrary values (if you allow them, that
> is) can be used, e.g. "Transmit  encoded in such and such
> way a the value of the push-option".

okay.

Re: "disabling bitmap writing, as some objects are not being packed"?

[Jeff King]

On Wed, Feb 08, 2017 at 02:05:42PM -0500, David Turner wrote:

> On Wed, 2017-02-08 at 09:44 -0800, Junio C Hamano wrote:
> > Duy Nguyen  writes:
> > 
> > > On second thought, perhaps gc.autoDetach should default to false if
> > > there's no tty, since its main point it to stop breaking interactive
> > > usage. That would make the server side happy (no tty there).
> > 
> > Sounds like an idea, but wouldn't that keep the end-user coming over
> > the network waiting after accepting a push until the GC completes, I
> > wonder.  If an impatient user disconnects, would that end up killing
> > an ongoing GC?  etc.
> 
> Regardless, it's impolite to keep the user waiting. So, I think we
> should just not write the "too many unreachable loose objects" message
> if auto-gc is on.  Does that sound OK?

I thought the point of that message was to prevent auto-gc from kicking
in over and over again due to objects that won't actually get pruned.

I wonder if you'd want to either bump the auto-gc object limit, or
possibly reduce the gc.pruneExpire limit to keep this situation from
coming up in the first place (or at least mitigating the amount of time
it's the case).

-Peff

[PATCH] diff: print line prefix for --name-only output

[Jeff King]

On Wed, Feb 08, 2017 at 01:24:34PM +0700, Duy Nguyen wrote:

> On Wed, Feb 8, 2017 at 6:11 AM, Davide Del Vento  wrote:
> > `git log --graph  --name-only` works fine, but the name is not
> > properly indented as it is for `git log --graph  --name-status`
> >
> > I tested this in git v1.9.1 the only one I have access at the moment
> 
> Confirmed still happens on master. --stat plays nicely with --graph
> though, so it's probably just some small fixes somewhere in diff*.c.

Yep. Looks like every format except --name-only handles this correctly.

-- >8 --
Subject: diff: print line prefix for --name-only output

If you run "git log --graph --name-only", the pathnames are
not indented to go along with their matching commits (unlike
all of the other diff formats). We need to output the line
prefix for each item before writing it.

The tests cover both --name-status and --name-only. The
former actually gets this right already, because it builds
on the --raw format functions. It's only --name-only which
uses its own code (and this fix mirrors the code in
diff_flush_raw()).

Note that the tests don't follow our usual style of setting
up the "expect" output inside the test block. This matches
the surrounding style, but more importantly it is easier to
read: we don't have to worry about embedded single-quotes,
and the leading indentation is more obvious.

Signed-off-by: Jeff King 
---
 diff.c |  1 +
 t/t4202-log.sh | 48 
 2 files changed, 49 insertions(+)

diff --git a/diff.c b/diff.c
index d91a34490..a79f3408d 100644
--- a/diff.c
+++ b/diff.c
@@ -4450,6 +4450,7 @@ static void flush_one_pair(struct diff_filepair *p, 
struct diff_options *opt)
name_a = p->two->path;
name_b = NULL;
strip_prefix(opt->prefix_length, _a, _b);
+   fprintf(opt->file, "%s", diff_line_prefix(opt));
write_name_quoted(name_a, opt->file, opt->line_termination);
}
 }
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 08ea725de..48b55bfd2 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1212,6 +1212,54 @@ test_expect_success 'log --line-prefix="*** " --graph 
with diff and stats' '
test_i18ncmp expect actual.sanitized
 '
 
+cat >expect <<-\EOF
+* reach
+|
+| Areach.t
+* Merge branch 'tangle'
+*   Merge branch 'side'
+|\
+| * side-2
+|
+|   A  2
+* Second
+|
+| Aone
+* sixth
+
+  Da/two
+EOF
+
+test_expect_success 'log --graph with --name-status' '
+   git log --graph --format=%s --name-status tangle..reach >actual &&
+   sanitize_output actual.sanitized &&
+   test_cmp expect actual.sanitized
+'
+
+cat >expect <<-\EOF
+* reach
+|
+| reach.t
+* Merge branch 'tangle'
+*   Merge branch 'side'
+|\
+| * side-2
+|
+|   2
+* Second
+|
+| one
+* sixth
+
+  a/two
+EOF
+
+test_expect_success 'log --graph with --name-only' '
+   git log --graph --format=%s --name-only tangle..reach >actual &&
+   sanitize_output actual.sanitized &&
+   test_cmp expect actual.sanitized
+'
+
 test_expect_success 'dotdot is a parent directory' '
mkdir -p a/b &&
( echo sixth && echo fifth ) >expect &&
-- 
2.12.0.rc0.371.ga6cf8653b

Re: Trying to use xfuncname without success.

[Jack Adrian Zappa]

> I'm not so sure about that.  With your example I get this diff without
setting diff.natvis.xfuncname:

So, to make sure we are on the same page, I removed the
diff.natvis.xfuncname from the .gitconfig and .git/config.  My output
was:

C:\Users\adrianh\Documents\tmp>git diff
diff --git a/test.natvis b/test.natvis
index 93fd5b4..351301f 100644
--- a/test.natvis
+++ b/test.natvis
@@ -18,6 +18,7 @@


  
+ added_var


   var2

So I didn't get the default output that your specified.

I've been modifying the .gitconfig file directly, but I tried your command:

git config diff.natvis.xfuncname "^[\t ]* wrote:
> Am 08.02.2017 um 18:11 schrieb Jack Adrian Zappa:
>> Thanks Rene, but you seem to have missed the point.  NOTHING is
>> working.  No matter what I put there, it doesn't seem to get matched.
>
> I'm not so sure about that.  With your example I get this diff without
> setting diff.natvis.xfuncname:
>
> diff --git a/a.natvis b/a.natvis
> index 7f9bdf5..bc3c090 100644
> --- a/a.natvis
> +++ b/a.natvis
> @@ -19,7 +19,7 @@ 
> xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010;>
>
>
>
> -  added_var
> +  added_vars
>
>
>var2
>
> Note the XML namespace in the hunk header.  It's put there by the
> default rule because "xmlns" starts at the beginning of the line.  Your
> diff has nothing there, which means the default rule is not used, i.e.
> your user-defined rule is in effect.
>
> Come to think of it, this line break in the middle of the AutoVisualizer
> tab might have been added by your email client unintentionally, so that
> we use different test files, which then of course results in different
> diffs.  Is that the case?
>
> Anyway, if I run the following two commands:
>
> $ git config diff.natvis.xfuncname "^[\t ]* $ echo '*.natvis diff=natvis' >.gitattributes
>
> ... then I get this, both on Linux (git version 2.11.1) and on Windows
> (git version 2.11.1.windows.1):
>
> diff --git a/a.natvis b/a.natvis
> index 7f9bdf5..bc3c090 100644
> --- a/a.natvis
> +++ b/a.natvis
> @@ -19,7 +19,7 @@ test
>
>
>
> -  added_var
> +  added_vars
>
>
>var2
>
>> Just to be sure, I tested your regex and again it didn't work.
>
> At this point I'm out of ideas, sorry. :(  The only way I was able to
> break it was due to mistyping the extension as "netvis" several times
> for some reason.
>
> René

Re: Trying to use xfuncname without success.

[Jack Adrian Zappa]

Thanks Samuel,

So, the question is, what is causing this problem on my system?

Anyone have an idea to help diagnose this problem?

On Wed, Feb 8, 2017 at 3:24 PM, Samuel Lijin  wrote:
> On Windows 7, it works for me in both CMD and Git Bash:
>
> $ git --version
> git version 2.11.0.windows.3
>
> $ git diff HEAD^ --word-diff
> diff --git a/test.natvis b/test.natvis
> index 93396ad..1233b8c 100644
> --- a/test.natvis
> +++ b/test.natvis
> @@ -18,6 +18,7 @@ test
>
>
>   
>   {+added_var+}
>
>
>   var2
>
> On Wed, Feb 8, 2017 at 12:37 PM, René Scharfe  wrote:
>> Am 08.02.2017 um 18:11 schrieb Jack Adrian Zappa:
>>> Thanks Rene, but you seem to have missed the point.  NOTHING is
>>> working.  No matter what I put there, it doesn't seem to get matched.
>>
>> I'm not so sure about that.  With your example I get this diff without
>> setting diff.natvis.xfuncname:
>>
>> diff --git a/a.natvis b/a.natvis
>> index 7f9bdf5..bc3c090 100644
>> --- a/a.natvis
>> +++ b/a.natvis
>> @@ -19,7 +19,7 @@ 
>> xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010;>
>>
>>
>>
>> -  added_var
>> +  added_vars
>>
>>
>>var2
>>
>> Note the XML namespace in the hunk header.  It's put there by the
>> default rule because "xmlns" starts at the beginning of the line.  Your
>> diff has nothing there, which means the default rule is not used, i.e.
>> your user-defined rule is in effect.
>>
>> Come to think of it, this line break in the middle of the AutoVisualizer
>> tab might have been added by your email client unintentionally, so that
>> we use different test files, which then of course results in different
>> diffs.  Is that the case?
>>
>> Anyway, if I run the following two commands:
>>
>> $ git config diff.natvis.xfuncname "^[\t ]*> $ echo '*.natvis diff=natvis' >.gitattributes
>>
>> ... then I get this, both on Linux (git version 2.11.1) and on Windows
>> (git version 2.11.1.windows.1):
>>
>> diff --git a/a.natvis b/a.natvis
>> index 7f9bdf5..bc3c090 100644
>> --- a/a.natvis
>> +++ b/a.natvis
>> @@ -19,7 +19,7 @@ test
>>
>>
>>
>> -  added_var
>> +  added_vars
>>
>>
>>var2
>>
>>> Just to be sure, I tested your regex and again it didn't work.
>>
>> At this point I'm out of ideas, sorry. :(  The only way I was able to
>> break it was due to mistyping the extension as "netvis" several times
>> for some reason.
>>
>> René

Re: gitconfig get out of sync with submodule entries on branch switch

[Stefan Beller]

On Mon, Feb 6, 2017 at 4:17 AM, Benjamin Schindler
 wrote:
>
>
> On 06.02.2017 11:35, Stefan Beller wrote:
>>
>> Answering the original email, as I feel we're going down the wrong rabbit
>> hole in the existing thread.
>>
>> On Mon, Jan 30, 2017 at 8:21 AM, Benjamin Schindler
>>  wrote:
>>>
>>> Hi
>>>
>>> Consider the following usecase: I have the master branch where I have a
>>> submodule A. I create a branch where I rename the submodule to be in the
>>> directory B. After doing all of this, everything looks good.
>>> Now, I switch back to master. The first oddity is, that it fails to
>>> remove
>>> the folder B because there are still files in there:
>>>
>>> bschindler@metis ~/Projects/submodule_test (testbranch) $ git checkout
>>> master
>>> warning: unable to rmdir other_submodule: Directory not empty
>>> Switched to branch 'master'
>>
>>
>> checkout currently doesn't support submodules, so it should neither
>> try to delete B nor try to repopulate A when switching back to master.
>> checkout ought to not even touch the existing submodule B.
>
>
> Well, it tried to remove the folder (the rmdir warning) but it failed so in
> some sense you are right. Is there a technical reason for this default
> though? Here, I frequently have to point out to people that they need to
> initialize/update the submodule on e.g. clone.

The reasoning is more based on historical momentum.
Back then when gitlinks/submodules were invented (git repositories
inside other git repositories! How frickin' cool is that?) nobody quite knew
how they were going to be used eventually.  So the safe play at the time was
to not touch them at all. (Also easy to implement, but that was not the point
as I learned).

And now we have a lot of people expecting just that. So we cannot change
the behavior overnight. So we'd first implement the alternative (e.g. the
--recurse-submodules flag for checkout) and then once we do a major
release we may want to flip the default.

>
>>
>>>
>>> Git submodule deinit on B fails because the submodule is not known to git
>>> anymore (after all, the folder B exists only in the other branch). I can
>>> easily just remove the folder B from disk and initialize the submodule A
>>> again, so all seems good.
>>
>>
>> by initializing you mean populating(?), i.e.
>>
>> git submodule update
>>
>> would work without the --init flag or preceding "git submodule init A".
>> That ought to not redownload A, but just put files back in the working
>> tree
>> from the submodule git directory inside the superprojects git dir.
>>
>>>
>>> However, what is not good is that the submodule b is still known in
>>> .git/config.
>>
>>
>> Oh, I see. You did not just rename the path, but also the name
>> in the .gitmodules?
>
>
> I wasn't even aware that the submodule name was something different from the
> path because the name is by default set to be the path to it. So yes, I
> didn't just relocate it, it had a different name.

Yeah the path/name is tricky and usually only differs when you move the
submodule inside the working tree. (As the name stays constant we know
where the git directory is expected: .git/modules so we can check there
instead of re-cloning to the "new" submodule-path.)

>
>>
>>> This is in particular a problem for us, because I know a number
>>> of tools which use git config to retrieve the submodule list. Is it
>>> therefore a bug that upon branch switch, the submodule gets deregistered,
>>> but its entry in .git/config remains?
>>
>>
>> The config remains as it indicates that you express(ed) interest in
>> submodule A, such that when switching branches
>>
>>   master->renamedToB->master
>>
>> then we'd still care about A. As for the tools, I'd rather see them use
>>
>> git submodule status/summary
>>
>> instead of directly looking at the config, because the config may
>> change in the future.
>
>
> That was my feeling but its good to know to have more solid reasons why that
> would be.

The reasons here are backward/forward compatibility.
When trying to change the behavior of submodule related things, there is no
clear distinction of plumbing/porcelain as we have in the rest of Git.
So even in gits own test suite we sometimes rely on things that may change
later on, and that makes it very hard to move forward, which is why I try to
have an opinion on how to do things properly.

Thanks,
Stefan

Re: Trying to use xfuncname without success.

[Samuel Lijin]

On Windows 7, it works for me in both CMD and Git Bash:

$ git --version
git version 2.11.0.windows.3

$ git diff HEAD^ --word-diff
diff --git a/test.natvis b/test.natvis
index 93396ad..1233b8c 100644
--- a/test.natvis
+++ b/test.natvis
@@ -18,6 +18,7 @@ test


  
  {+added_var+}


  var2

On Wed, Feb 8, 2017 at 12:37 PM, René Scharfe  wrote:
> Am 08.02.2017 um 18:11 schrieb Jack Adrian Zappa:
>> Thanks Rene, but you seem to have missed the point.  NOTHING is
>> working.  No matter what I put there, it doesn't seem to get matched.
>
> I'm not so sure about that.  With your example I get this diff without
> setting diff.natvis.xfuncname:
>
> diff --git a/a.natvis b/a.natvis
> index 7f9bdf5..bc3c090 100644
> --- a/a.natvis
> +++ b/a.natvis
> @@ -19,7 +19,7 @@ 
> xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010;>
>
>
>
> -  added_var
> +  added_vars
>
>
>var2
>
> Note the XML namespace in the hunk header.  It's put there by the
> default rule because "xmlns" starts at the beginning of the line.  Your
> diff has nothing there, which means the default rule is not used, i.e.
> your user-defined rule is in effect.
>
> Come to think of it, this line break in the middle of the AutoVisualizer
> tab might have been added by your email client unintentionally, so that
> we use different test files, which then of course results in different
> diffs.  Is that the case?
>
> Anyway, if I run the following two commands:
>
> $ git config diff.natvis.xfuncname "^[\t ]* $ echo '*.natvis diff=natvis' >.gitattributes
>
> ... then I get this, both on Linux (git version 2.11.1) and on Windows
> (git version 2.11.1.windows.1):
>
> diff --git a/a.natvis b/a.natvis
> index 7f9bdf5..bc3c090 100644
> --- a/a.natvis
> +++ b/a.natvis
> @@ -19,7 +19,7 @@ test
>
>
>
> -  added_var
> +  added_vars
>
>
>var2
>
>> Just to be sure, I tested your regex and again it didn't work.
>
> At this point I'm out of ideas, sorry. :(  The only way I was able to
> break it was due to mistyping the extension as "netvis" several times
> for some reason.
>
> René

Re: [RFC] mailmap.blob overrides default .mailmap

[Jeff King]

On Tue, Feb 07, 2017 at 11:45:31PM +0100, Cornelius Weig wrote:

> On the other hand, a checked-in .mailmap file and a mailmap-blob are
> both as in-history as the other to me. Now consider the following
> settings:

I think it depends how you use them. You could point mailmap.blob to
some other ref entirely (even one that you fetched from another
repository).

I'd expect normal use to point it to HEAD:.mailmap, though (and that was
certainly the use case I wrote it for). On the other hand, the point of
pointing it to that particular blob is that it works even when you
_don't_ have a checkout (and this kicks in automatically in a bare
repo).

> $ git config --unset mailmap.file
> $ git config mailmap.blob HEAD:.mailmap
> $ sed -i 's:p...@peff.com:no-valid-address:' .mailmap
> $ git log -1 --author 'Jeff King'

In case anybody wants to experiment, there are a bunch of things that
make this a non-working example (at least on git.git):

  - my address is actually peff.net :)

  - There mailmap which mentions peff.net maps p...@github.com to
peff.net, so this change would require --author=p...@github.com.

  - We don't apply mailmaps for the default output of "git log". You can
format with "%aN %aE", or just use "git shortlog -ns --author=peff"
which does map.

But that aside, yeah, you can make an argument to expect one way or the
other, depending on the situation you set up. I don't have a strong
feeling about it, but my gut feeling is that no ordering is
significantly better than the other, and that puts me in favor of
leaving it as-is purely out of inertia and backwards-compatibility.

-Peff

Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree

[Stefan Beller]

On Wed, Feb 8, 2017 at 3:31 AM, Nguyễn Thái Ngọc Duy  wrote:
> (**) At this point, we may want to rename refs *_submodule API to
> something more neutral, maybe s/_submodule/_remote/

I agree on (**), except that I am not sure if /_remote/ is a better name,
because there is already a concept of a "remote" as well as
"remote-tracking" in Git. (Usually it is not reachable on the same
FS, but resides on another machine).

So if we were to do s/_submodule/_remote/, we'd have e.g.

for_each_ref_remote

which could mean that we do networking things to obtain the
actual remote refs or just talk about remote tracking refs.

My gut reaction would be to s/submodule/alternative/ here,
but we also have a thing called alternates already.

So we're looking for a name that describes refs for both
worktrees as well as submodules. (Not sure if we can generalize
to also include alternates, too)

And the one thing they share is that they have their refs
"not at the usual place", e.g. not at .git/refs but rather at
.git/{modules,worktrees}.

  Recently we had a tangential discussion in submodule land
  about the different places, when adding the
  "git submodule absorbgitdirs" command, that moves
  the submodule/.git directory into .git/modules/.
  We chose "absorb" here as the name, because it
  was moved into the .git/ area.

So maybe one of:

s/submodule_ref/unusual_ref/
(to emphasize it is not a regular ref inside the repo, so:)
s/submodule_ref/irregular_ref/

s/resolve_ref_submodule/resolve_ref_out_of_place/
s/resolve_ref_submodule/resolve_ref_gitlink_pointed/
s/resolve_ref_submodule/resolve_ref_linked_pointer/

would be my current thinking.

Thanks,
Stefan

[PATCH v2 02/11] for_each_alternate_ref: stop trimming trailing slashes

[Jeff King]

The real_pathdup() function will have removed extra slashes
for us already (on top of the normalize_path() done when we
created the alternate_object_database struct in the first
place).

Incidentally, this also fixes the case where the path is
just "/", which would read off the start of the array.
That doesn't seem possible to trigger in practice, though,
as link_alt_odb_entry() blindly eats trailing slashes,
including a bare "/".

Signed-off-by: Jeff King 
---
 transport.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/transport.c b/transport.c
index 9ce0ee96b..6fba9e95b 100644
--- a/transport.c
+++ b/transport.c
@@ -1226,8 +1226,6 @@ static int refs_from_alternate_cb(struct 
alternate_object_database *e,
return 0;
len = strlen(other);
 
-   while (other[len-1] == '/')
-   other[--len] = '\0';
if (len < 8 || memcmp(other + len - 8, "/objects", 8))
goto out;
/* Is this a git repository with refs? */
-- 
2.12.0.rc0.371.ga6cf8653b

[PATCH v2 0/11] reducing resource usage of for_each_alternate_ref

[Jeff King]

This is a minor re-roll of the patches from:

  
http://public-inbox.org/git/20170124003729.j4ygjcgypdq7h...@sigill.intra.peff.net/

(which got some review, but I don't think was picked up for even 'pu').

I won't repeat the numbers and background from that message, but the
gist of it is that this reduces memory usage significantly when your
alternate has a lot of refs in it.

This version makes two minor changes:

  - it drops the save_commit_buffer patch to clone; it's redundant with
what fetch_pack() is doing internally, and I wasn't able to measure
any improvement

  - it adds a missing "static" to an internal function

The only other possible change from the review would be sorting the
expected output in the test of the final script. I'm on the fence
whether it is a feature that we expect a particular ordering. It's not
set in stone ,but it _is_ deterministic, and if we change the order, it
might be worth somebody actually noticing.

  [01/11]: for_each_alternate_ref: handle failure from real_pathdup()
  [02/11]: for_each_alternate_ref: stop trimming trailing slashes
  [03/11]: for_each_alternate_ref: use strbuf for path allocation
  [04/11]: for_each_alternate_ref: pass name/oid instead of ref struct
  [05/11]: for_each_alternate_ref: replace transport code with for-each-ref
  [06/11]: fetch-pack: cache results of for_each_alternate_ref
  [07/11]: add oidset API
  [08/11]: receive-pack: use oidset to de-duplicate .have lines
  [09/11]: receive-pack: fix misleading namespace/.have comment
  [10/11]: receive-pack: treat namespace .have lines like alternates
  [11/11]: receive-pack: avoid duplicates between our refs and alternates

 Makefile   |  1 +
 builtin/receive-pack.c | 41 +++-
 fetch-pack.c   | 48 -
 object.h   |  2 +-
 oidset.c   | 49 ++
 oidset.h   | 45 +++
 t/t5400-send-pack.sh   | 38 ++
 transport.c| 72 +++---
 transport.h|  2 +-
 9 files changed, 249 insertions(+), 49 deletions(-)
 create mode 100644 oidset.c
 create mode 100644 oidset.h

Re: [PATCH] dir: avoid allocation in fill_directory()

[Brandon Williams]

On 02/08, Duy Nguyen wrote:
> On Wed, Feb 8, 2017 at 5:04 AM, René Scharfe  wrote:
> > Pass the match member of the first pathspec item directly to
> > read_directory() instead of using common_prefix() to duplicate it first,
> > thus avoiding memory duplication, strlen(3) and free(3).
> 
> How about killing common_prefix()? There are two other callers in
> ls-files.c and commit.c and it looks safe to do (but I didn't look
> very hard).
> 
> > diff --git a/dir.c b/dir.c
> > index 65c3e681b8..4541f9e146 100644
> > --- a/dir.c
> > +++ b/dir.c
> > @@ -174,20 +174,19 @@ char *common_prefix(const struct pathspec *pathspec)
> >
> >  int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec)
> >  {
> > -   char *prefix;
> > +   const char *prefix;
> > size_t prefix_len;
> >
> > /*
> >  * Calculate common prefix for the pathspec, and
> >  * use that to optimize the directory walk
> >  */
> > -   prefix = common_prefix(pathspec);
> > -   prefix_len = prefix ? strlen(prefix) : 0;
> > +   prefix_len = common_prefix_len(pathspec);
> > +   prefix = prefix_len ? pathspec->items[0].match : "";
> 
> There's a subtle difference. Before the patch, prefix[prefix_len] is
> NUL. After the patch, it's not always true. If some code (incorrectly)
> depends on that, this patch exposes it. I had a look inside
> read_directory() though and it looks like no such code exists. So, all
> good.

Yeah I had the exact same thought when looking at this, but I agree
everything looks fine.  And if something does indeed depend on prefix
having a \0 at prefix_len then this will allow us to more easily find
the bug and fix it.

> 
> >
> > /* Read the directory and prune it */
> > read_directory(dir, prefix, prefix_len, pathspec);
> >
> > -   free(prefix);
> > return prefix_len;
> >  }
> -- 
> Duy

-- 
Brandon Williams

[PATCH v2 03/11] for_each_alternate_ref: use strbuf for path allocation

[Jeff King]

We have a string with ".../objects" pointing to the
alternate object store, and overwrite bits of it to look at
other paths in the (potential) git repository holding it.
This works because the only path we care about is "refs",
which is shorter than "objects".

Using a strbuf to hold the path lets us get rid of some
magic numbers, and makes it more obvious that the memory
operations are safe.

Signed-off-by: Jeff King 
---
 transport.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/transport.c b/transport.c
index 6fba9e95b..6b7847131 100644
--- a/transport.c
+++ b/transport.c
@@ -1214,34 +1214,34 @@ struct alternate_refs_data {
 static int refs_from_alternate_cb(struct alternate_object_database *e,
  void *data)
 {
-   char *other;
-   size_t len;
+   struct strbuf path = STRBUF_INIT;
+   size_t base_len;
struct remote *remote;
struct transport *transport;
const struct ref *extra;
struct alternate_refs_data *cb = data;
 
-   other = real_pathdup(e->path);
-   if (!other)
-   return 0;
-   len = strlen(other);
-
-   if (len < 8 || memcmp(other + len - 8, "/objects", 8))
+   if (!strbuf_realpath(, e->path, 0))
+   goto out;
+   if (!strbuf_strip_suffix(, "/objects"))
goto out;
+   base_len = path.len;
+
/* Is this a git repository with refs? */
-   memcpy(other + len - 8, "/refs", 6);
-   if (!is_directory(other))
+   strbuf_addstr(, "/refs");
+   if (!is_directory(path.buf))
goto out;
-   other[len - 8] = '\0';
-   remote = remote_get(other);
-   transport = transport_get(remote, other);
+   strbuf_setlen(, base_len);
+
+   remote = remote_get(path.buf);
+   transport = transport_get(remote, path.buf);
for (extra = transport_get_remote_refs(transport);
 extra;
 extra = extra->next)
cb->fn(extra, cb->data);
transport_disconnect(transport);
 out:
-   free(other);
+   strbuf_release();
return 0;
 }
 
-- 
2.12.0.rc0.371.ga6cf8653b

[PATCH v2 04/11] for_each_alternate_ref: pass name/oid instead of ref struct

[Jeff King]

Breaking down the fields in the interface makes it easier to
change the backend of for_each_alternate_ref to something
that doesn't use "struct ref" internally.

The only field that callers actually look at is the oid,
anyway. The refname is kept in the interface as a plausible
thing for future code to want.

Signed-off-by: Jeff King 
---
 builtin/receive-pack.c |  6 --
 fetch-pack.c   | 12 
 transport.c|  2 +-
 transport.h|  2 +-
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 1dbb8a069..d21332d9e 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -277,10 +277,12 @@ static int show_one_alternate_sha1(const unsigned char 
sha1[20], void *unused)
return 0;
 }
 
-static void collect_one_alternate_ref(const struct ref *ref, void *data)
+static void collect_one_alternate_ref(const char *refname,
+ const struct object_id *oid,
+ void *data)
 {
struct sha1_array *sa = data;
-   sha1_array_append(sa, ref->old_oid.hash);
+   sha1_array_append(sa, oid->hash);
 }
 
 static void write_head_info(void)
diff --git a/fetch-pack.c b/fetch-pack.c
index 601f0779a..54f84c573 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -253,9 +253,11 @@ static void send_request(struct fetch_pack_args *args,
write_or_die(fd, buf->buf, buf->len);
 }
 
-static void insert_one_alternate_ref(const struct ref *ref, void *unused)
+static void insert_one_alternate_ref(const char *refname,
+const struct object_id *oid,
+void *unused)
 {
-   rev_list_insert_ref(NULL, ref->old_oid.hash);
+   rev_list_insert_ref(NULL, oid->hash);
 }
 
 #define INITIAL_FLUSH 16
@@ -619,9 +621,11 @@ static void filter_refs(struct fetch_pack_args *args,
*refs = newlist;
 }
 
-static void mark_alternate_complete(const struct ref *ref, void *unused)
+static void mark_alternate_complete(const char *refname,
+   const struct object_id *oid,
+   void *unused)
 {
-   mark_complete(ref->old_oid.hash);
+   mark_complete(oid->hash);
 }
 
 static int everything_local(struct fetch_pack_args *args,
diff --git a/transport.c b/transport.c
index 6b7847131..c87147046 100644
--- a/transport.c
+++ b/transport.c
@@ -1238,7 +1238,7 @@ static int refs_from_alternate_cb(struct 
alternate_object_database *e,
for (extra = transport_get_remote_refs(transport);
 extra;
 extra = extra->next)
-   cb->fn(extra, cb->data);
+   cb->fn(extra->name, >old_oid, cb->data);
transport_disconnect(transport);
 out:
strbuf_release();
diff --git a/transport.h b/transport.h
index e597b31b3..bc5571574 100644
--- a/transport.h
+++ b/transport.h
@@ -255,6 +255,6 @@ int transport_refs_pushed(struct ref *ref);
 void transport_print_push_status(const char *dest, struct ref *refs,
  int verbose, int porcelain, unsigned int *reject_reasons);
 
-typedef void alternate_ref_fn(const struct ref *, void *);
+typedef void alternate_ref_fn(const char *refname, const struct object_id 
*oid, void *);
 extern void for_each_alternate_ref(alternate_ref_fn, void *);
 #endif
-- 
2.12.0.rc0.371.ga6cf8653b

[PATCH v2 01/11] for_each_alternate_ref: handle failure from real_pathdup()

[Jeff King]

In older versions of git, if real_path() failed to resolve
the alternate object store path, we would die() with an
error. However, since 4ac9006f8 (real_path: have callers use
real_pathdup and strbuf_realpath, 2016-12-12) we use the
real_pathdup() function, which may return NULL. Since we
don't check the return value, we can segfault.

This is hard to trigger in practice, since we check that the
path is accessible before creating the alternate_object_database
struct. But it could be removed racily, or we could see a
transient filesystem error.

We could restore the original behavior by switching back to
xstrdup(real_path()).  However, dying is probably not the
best option here. This whole function is best-effort
already; there might not even be a repository around the
shared objects at all. And if the alternate store has gone
away, there are no objects to show.

So let's just quietly return, as we would if we failed to
open "refs/", or if upload-pack failed to start, etc.

Signed-off-by: Jeff King 
---
 transport.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/transport.c b/transport.c
index d72e08948..9ce0ee96b 100644
--- a/transport.c
+++ b/transport.c
@@ -1222,6 +1222,8 @@ static int refs_from_alternate_cb(struct 
alternate_object_database *e,
struct alternate_refs_data *cb = data;
 
other = real_pathdup(e->path);
+   if (!other)
+   return 0;
len = strlen(other);
 
while (other[len-1] == '/')
-- 
2.12.0.rc0.371.ga6cf8653b

Re: [PATCH v4] tag: generate useful reflog message

[Junio C Hamano]

cornelius.w...@tngtech.com writes:

> From: Cornelius Weig 
>
> When tags are created with `--create-reflog` or with the option
> `core.logAllRefUpdates` set to 'always', a reflog is created for them.
> So far, the description of reflog entries for tags was empty, making the
> reflog hard to understand. For example:
> 6e3a7b3 refs/tags/test@{0}:
>
> Now, a reflog message is generated when creating a tag, following the
> pattern "tag: tagging  ()". If
> GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
> ()" instead. If the tag references a commit object, the
> description is set to the subject line of the commit, followed by its
> commit date. For example:
> 6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)
>
> If the tag points to a tree/blob/tag objects, the following static
> strings are taken as description:
>
>  - "tree object"
>  - "blob object"
>  - "other tag object"
>
> Signed-off-by: Cornelius Weig 
> Reviewed-by: Junio C Hamano 

This last line is inappropriate, as I didn't review _THIS_ version,
which is different from the previous one, and I haven't checked if
the way the comments on the previous review were addressed in this
version is agreeable.

> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 072e6c6..894959f 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -80,10 +80,24 @@ test_expect_success 'creating a tag using default HEAD 
> should succeed' '
>   test_must_fail git reflog exists refs/tags/mytag
>  '
>  
> +git log -1 > expected \
> + --format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F

We do not want to do this kind of thing outside the
test_expect_success immediately below, unless there is a good
reason, and in this case I do not see any.

Also write redirection operator and redirection target pathname
without SP in between.

>  test_expect_success 'creating a tag with --create-reflog should create 
> reflog' '
>   test_when_finished "git tag -d tag_with_reflog" &&
>   git tag --create-reflog tag_with_reflog &&
> - git reflog exists refs/tags/tag_with_reflog
> + git reflog exists refs/tags/tag_with_reflog &&
> + sed -e "s/^.*   //" .git/logs/refs/tags/tag_with_reflog > actual &&
> + test_cmp expected actual
> +'

In other words, something like:

test_expect_success 'creating a tag with --create-reflog should create reflog' '
git log -1 \
--format="format:tag: tagging %h (%s, %cd)%n" \
--date=format:%Y-%m-%d >expected &&
test_when_finished "git tag -d tag_with_reflog" &&
git tag --create-reflog tag_with_reflog &&
git reflog exists refs/tags/tag_with_reflog &&
sed -e "s/^.*   //" .git/logs/refs/tags/tag_with_reflog >actual &&
test_cmp expected actual
'   

Even though %F may be shorter, spelling it out makes what we expect
more explicit, and what is what I did in the above example.

Thanks.

Re: Bug with fixup and autosquash

[Junio C Hamano]

Ashutosh Bapat  writes:

> I have been using git rebase heavily these days and seem to have found a bug.
>
> If there are two commit messages which have same prefix e.g.
> yy This is prefix
> xx This is prefix and message
>
> xx comitted before yy
>
> Now I commit a fixup to yy using git commit --fixup yy
> zz fixup! This is prefix
>
> When I run git rebase -i --autosquash, the script it shows me looks like
> pick xx This is prefix and message
> fixup zz fixup! This is prefix
> pick yy This is prefix
>
> I think the correct order is
> pick xx This is prefix and message
> pick yy This is prefix
> fixup zz fixup! This is prefix
>
> Is that right?

Because "commit" pretends as if it took the exact commit object name
to be fixed up (after all, it accepts "yy" that is a name of the
commit object), it would be nice if the fixup is applied to that
exact commit, even if you had many commits that share exactly the
same title (i.e. not just shared prefix).

Unfortunately, "rebase -i --autosquash" reorders the entries by
identifying the commit by its title, and it goes with prefix match
so that fix-up commits created without using --fixup option but
manually records the title's prefix substring can also work.  

We could argue that the logic should notice that there is one exact
match and another non-exact prefix match and favor the former, and
certainly such a change would make your made-up example (you didn't
actually have a commit whose title is literally "This is prefix")
above work better.

But I am not sure if adding such heuristics would really help in
general.  It would not help those whose commits are mostly titled
ultra-vaguely, like "fix", "bugfix", "docfix", etc.

Another possibility is to teach "commit --fixup" to cast exact
commit object name in stone.  That certainly would solve your
immediate problem, but it has a grave negative impact when the user
rebases the same topic many times (which happens often).

For example, I may have a series of commits A and B, notice that A
could be done a bit better and have "fixup A" on top, build a new
commit C on it, and then realize that the next step (i.e. D) would
need support from a newer codebase than where I started (i.e. A^).

At that point, I would have a 4-commit series (A, B, "fixup A", and
C), and I would rebase them on top of something newer.  I am
undecided if that "fixup A" is really an improvement or unnecessary,
when I do this, but I do know that I want to build the series on top
of a different commit.  So I do rebase without --autosquash (I would
probably rebase without --interactive for this one).

Then I keep working and add a new commit D on top.  After all that,
I would have a more-or-less completed series and would be ready to
re-assess the whole series.  I perhaps decide that "fixup A" is
really an improvement.  And then I would "rebase -i" to squash the
fix-up into A.

But notice that at this point, what we are calling A has different
object name than the original A the fixup was written for, because
we rebased once on top of a newer codebase.  That commit can still
be identified by its title, but not with its original commit object
name.

I think that is why "commit --fixup " turns the commit
object name (your "yy") into a string (your "This is prefix")
and that is a reasonable design decision [*1*].

So from that point of view, if we were to address your issue, it
should happen in "rebase -i --autosquash" side, not "commit --fixup"
side.

Let's hear from some of those (Cc'ed) who were involved in an
earlier --autosquash thread.

https://public-inbox.org/git/cover.1259934977.git.mhag...@alum.mit.edu/


[Footnote]

*1* "rebase -i --autosquash" does understand "fixup! yy", so if
you are willing to accept the consequence of not being able to
rebase twice, you could instead do

$ git commit -m "fixup! yy"

I would think.

Re: Fwd: Possibly nicer pathspec syntax?

[Junio C Hamano]

Junio C Hamano  writes:

> If you know offhand which callers pass neither of the two
> PATHSPEC_PREFER_* bits and remember for what purpose you allowed
> them to do so, please remind me.  I'll keep digging in the meantime.

Answering my own questions, here are my findings so far and a
tentative conclusion.

With or without the patch in this thread, parse_pathspec() behaves
the same way for either CWD or FULL if you feed a non-empty
pathspec with at least one positive element.  IOW, if a caller feeds
a non-empty pathspec and there is no "negative" element involved, it
does not matter if we feed CWD or FULL.

There are only a handful of callers that pass neither preference
bits to parse_pathspec().  Here are my observations on them that
tells me that most of them are OK if we change them to prefer
either CWD or FULL:

 - archive.c::path_exists() feeds a pathspec with a single element
   to see if read_tree_recursive() finds any matching paths, to
   allow its caller to iterate over the original pathspec and see
   if there is a typo (i.e. an element that matches nothing).  It
   should prefer FULL to match what parse_pathspec_arg(), its
   caller, uses.

   The caller probably should refrain from passing ones with
   negative magic.  I.e. "git archive -- t :\!t/perf" errors out
   because checking each element independently in the loop means
   that ":\!t/perf" is checked alone, triggering "there is nothing
   to exclude from".

 - blame.c::find_origin() feeds a pathspec with a single element,
   which is a path in the history and does so as a literal, hence
   no room for "negative" to kick in.

 - builtin/check-ignore.c::check_ignore(), when argc==0, does not
   call parse_pathspec().  It does not take any magic other than
   FROMTOP, so "negative" won't come into the picture.

 - builtin/checkout.c::cmd_checkout(), when argc==0, does not call
   parse_pathspec().  This codepath will get affected by Linus's
   change ("cd t && git checkout :\!perf" would try to check out
   everything except t/perf, but what is a reasonable definition of
   "everything" in the context of this command).  We need to
   decide, and I am leaning towards preferring CWD for this case.

 - revision.c::setup_revisions() calls parse_pathspec() only when
   the caller gave a non-empty pathspec.  This pathspec is used for
   pruning log traversal (e.g. "only show commits that touch these
   paths") and is affected by Linus's change.  It should favor
   FULL.

 - tree-diff.c::try_to_follow_renames() feeds a pathspec with a
   single element as a literal, hence no room for "negative" to
   kick in.

So, I am tempted to suggest us doing the following:

 * Leave a NEEDSWORK comment to archive.c::path_exists() that is
   used for checking the validation of pathspec elements.  To fix it
   properly, we need to be able to skip a negative pathspec to be
   passed to this function by the caller, and to do so, we need to
   expose a helper from the pathspec API that gets a single string
   and returns what magic it has, but that is of lower priority.

 * Retire the PATHSPEC_PREFER_CWD bit and replace its use with the
   lack of the PATHSPEC_PREFER_FULL bit.

 * Keep most of the above callsites that currently do not pass
   CWD/FULL as they are, except the ones that should take FULL (see
   above).

Comments?

Re: Trying to use xfuncname without success.

[Jack Adrian Zappa]

Thanks Samuel,

That example showed that there must be something wrong in my .git
directory, because with it, I'm getting the correct output.  Moving
the same lines to my .git/config didn't work.

On Wed, Feb 8, 2017 at 3:46 PM, Samuel Lijin  wrote:
> I just put this togther: https://github.com/sxlijin/xfuncname-test
>
> Try cloning and then for any of config1 thru 3,
>
> $ cp configX .git/config
> $ git diff HEAD^ -- test.natvis
>
> On Wed, Feb 8, 2017 at 2:42 PM, Jack Adrian Zappa  
> wrote:
>> Thanks Samuel,
>>
>> So, the question is, what is causing this problem on my system?
>>
>> Anyone have an idea to help diagnose this problem?
>>
>> On Wed, Feb 8, 2017 at 3:24 PM, Samuel Lijin  wrote:
>>> On Windows 7, it works for me in both CMD and Git Bash:
>>>
>>> $ git --version
>>> git version 2.11.0.windows.3
>>>
>>> $ git diff HEAD^ --word-diff
>>> diff --git a/test.natvis b/test.natvis
>>> index 93396ad..1233b8c 100644
>>> --- a/test.natvis
>>> +++ b/test.natvis
>>> @@ -18,6 +18,7 @@ test
>>>
>>>
>>>   
>>>   {+added_var+}
>>>
>>>
>>>   var2
>>>
>>> On Wed, Feb 8, 2017 at 12:37 PM, René Scharfe  wrote:
 Am 08.02.2017 um 18:11 schrieb Jack Adrian Zappa:
> Thanks Rene, but you seem to have missed the point.  NOTHING is
> working.  No matter what I put there, it doesn't seem to get matched.

 I'm not so sure about that.  With your example I get this diff without
 setting diff.natvis.xfuncname:

 diff --git a/a.natvis b/a.natvis
 index 7f9bdf5..bc3c090 100644
 --- a/a.natvis
 +++ b/a.natvis
 @@ -19,7 +19,7 @@ 
 xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010;>



 -  added_var
 +  added_vars


var2

 Note the XML namespace in the hunk header.  It's put there by the
 default rule because "xmlns" starts at the beginning of the line.  Your
 diff has nothing there, which means the default rule is not used, i.e.
 your user-defined rule is in effect.

 Come to think of it, this line break in the middle of the AutoVisualizer
 tab might have been added by your email client unintentionally, so that
 we use different test files, which then of course results in different
 diffs.  Is that the case?

 Anyway, if I run the following two commands:

 $ git config diff.natvis.xfuncname "^[\t ]*.gitattributes

 ... then I get this, both on Linux (git version 2.11.1) and on Windows
 (git version 2.11.1.windows.1):

 diff --git a/a.natvis b/a.natvis
 index 7f9bdf5..bc3c090 100644
 --- a/a.natvis
 +++ b/a.natvis
 @@ -19,7 +19,7 @@ test



 -  added_var
 +  added_vars


var2

> Just to be sure, I tested your regex and again it didn't work.

 At this point I'm out of ideas, sorry. :(  The only way I was able to
 break it was due to mistyping the extension as "netvis" several times
 for some reason.

 René

[PATCH v2 06/11] fetch-pack: cache results of for_each_alternate_ref

[Jeff King]

We may run for_each_alternate_ref() twice, once in
find_common() and once in everything_local(). This operation
can be expensive, because it involves running a sub-process
which must freshly load all of the alternate's refs from
disk.

Let's cache and reuse the results between the two calls. We
can make some optimizations based on the particular use
pattern in fetch-pack to keep our memory usage down.

The first is that we only care about the sha1s, not the refs
themselves. So it's OK to store only the sha1s, and to
suppress duplicates. The natural fit would therefore be a
sha1_array.

However, sha1_array's de-duplication happens only after it
has read and sorted all entries. It still stores each
duplicate. For an alternate with a large number of refs
pointing to the same commits, this is a needless expense.

Instead, we'd prefer to eliminate duplicates before putting
them in the cache, which implies using a hash. We can
further note that fetch-pack will call parse_object() on
each alternate sha1. We can therefore keep our cache as a
set of pointers to "struct object". That gives us a place to
put our "already seen" bit with an optimized hash lookup.
And as a bonus, the object stores the sha1 for us, so
pointer-to-object is all we need.

There are two extra optimizations I didn't do here:

  - we actually store an array of pointer-to-object.
Technically we could just walk the obj_hash table
looking for entries with the ALTERNATE flag set (because
our use case doesn't care about the order here).

But that hash table may be mostly composed of
non-ALTERNATE entries, so we'd waste time walking over
them. So it would be a slight win in memory use, but a
loss in CPU.

  - the items we pull out of the cache are actual "struct
object"s, but then we feed "obj->sha1" to our
sub-functions, which promptly call parse_object().

This second parse is cheap, because it starts with
lookup_object() and will bail immediately when it sees
we've already parsed the object. We could save the extra
hash lookup, but it would involve refactoring the
functions we call. It may or may not be worth the
trouble.

Signed-off-by: Jeff King 
---
 fetch-pack.c | 52 ++--
 object.h |  2 +-
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 54f84c573..e0f5d5ce8 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -35,6 +35,7 @@ static const char *alternate_shallow_file;
 #define COMMON_REF (1U << 2)
 #define SEEN   (1U << 3)
 #define POPPED (1U << 4)
+#define ALTERNATE  (1U << 5)
 
 static int marked;
 
@@ -67,6 +68,41 @@ static inline void print_verbose(const struct 
fetch_pack_args *args,
fputc('\n', stderr);
 }
 
+struct alternate_object_cache {
+   struct object **items;
+   size_t nr, alloc;
+};
+
+static void cache_one_alternate(const char *refname,
+   const struct object_id *oid,
+   void *vcache)
+{
+   struct alternate_object_cache *cache = vcache;
+   struct object *obj = parse_object(oid->hash);
+
+   if (!obj || (obj->flags & ALTERNATE))
+   return;
+
+   obj->flags |= ALTERNATE;
+   ALLOC_GROW(cache->items, cache->nr + 1, cache->alloc);
+   cache->items[cache->nr++] = obj;
+}
+
+static void for_each_cached_alternate(void (*cb)(struct object *))
+{
+   static int initialized;
+   static struct alternate_object_cache cache;
+   size_t i;
+
+   if (!initialized) {
+   for_each_alternate_ref(cache_one_alternate, );
+   initialized = 1;
+   }
+
+   for (i = 0; i < cache.nr; i++)
+   cb(cache.items[i]);
+}
+
 static void rev_list_push(struct commit *commit, int mark)
 {
if (!(commit->object.flags & mark)) {
@@ -253,11 +289,9 @@ static void send_request(struct fetch_pack_args *args,
write_or_die(fd, buf->buf, buf->len);
 }
 
-static void insert_one_alternate_ref(const char *refname,
-const struct object_id *oid,
-void *unused)
+static void insert_one_alternate_object(struct object *obj)
 {
-   rev_list_insert_ref(NULL, oid->hash);
+   rev_list_insert_ref(NULL, obj->oid.hash);
 }
 
 #define INITIAL_FLUSH 16
@@ -300,7 +334,7 @@ static int find_common(struct fetch_pack_args *args,
marked = 1;
 
for_each_ref(rev_list_insert_ref_oid, NULL);
-   for_each_alternate_ref(insert_one_alternate_ref, NULL);
+   for_each_cached_alternate(insert_one_alternate_object);
 
fetching = 0;
for ( ; refs ; refs = refs->next) {
@@ -621,11 +655,9 @@ static void filter_refs(struct fetch_pack_args *args,
*refs = newlist;
 }
 
-static void mark_alternate_complete(const char *refname,
-   const struct object_id *oid,
-  

[PATCH v2 07/11] add oidset API

[Jeff King]

This is similar to many of our uses of sha1-array, but it
overcomes one limitation of a sha1-array: when you are
de-duplicating a large input with relatively few unique
entries, sha1-array uses 20 bytes per non-unique entry.
Whereas this set will use memory linear in the number of
unique entries (albeit a few more than 20 bytes due to
hashmap overhead).

Signed-off-by: Jeff King 
---
 Makefile |  1 +
 oidset.c | 49 +
 oidset.h | 45 +
 3 files changed, 95 insertions(+)
 create mode 100644 oidset.c
 create mode 100644 oidset.h

diff --git a/Makefile b/Makefile
index 8e4081e06..a5433978e 100644
--- a/Makefile
+++ b/Makefile
@@ -781,6 +781,7 @@ LIB_OBJS += notes-cache.o
 LIB_OBJS += notes-merge.o
 LIB_OBJS += notes-utils.o
 LIB_OBJS += object.o
+LIB_OBJS += oidset.o
 LIB_OBJS += pack-bitmap.o
 LIB_OBJS += pack-bitmap-write.o
 LIB_OBJS += pack-check.o
diff --git a/oidset.c b/oidset.c
new file mode 100644
index 0..ac169f05d
--- /dev/null
+++ b/oidset.c
@@ -0,0 +1,49 @@
+#include "cache.h"
+#include "oidset.h"
+
+struct oidset_entry {
+   struct hashmap_entry hash;
+   struct object_id oid;
+};
+
+static int oidset_hashcmp(const void *va, const void *vb,
+ const void *vkey)
+{
+   const struct oidset_entry *a = va, *b = vb;
+   const struct object_id *key = vkey;
+   return oidcmp(>oid, key ? key : >oid);
+}
+
+int oidset_contains(const struct oidset *set, const struct object_id *oid)
+{
+   struct hashmap_entry key;
+
+   if (!set->map.cmpfn)
+   return 0;
+
+   hashmap_entry_init(, sha1hash(oid->hash));
+   return !!hashmap_get(>map, , oid);
+}
+
+int oidset_insert(struct oidset *set, const struct object_id *oid)
+{
+   struct oidset_entry *entry;
+
+   if (!set->map.cmpfn)
+   hashmap_init(>map, oidset_hashcmp, 0);
+
+   if (oidset_contains(set, oid))
+   return 1;
+
+   entry = xmalloc(sizeof(*entry));
+   hashmap_entry_init(>hash, sha1hash(oid->hash));
+   oidcpy(>oid, oid);
+
+   hashmap_add(>map, entry);
+   return 0;
+}
+
+void oidset_clear(struct oidset *set)
+{
+   hashmap_free(>map, 1);
+}
diff --git a/oidset.h b/oidset.h
new file mode 100644
index 0..b7eaab5b8
--- /dev/null
+++ b/oidset.h
@@ -0,0 +1,45 @@
+#ifndef OIDSET_H
+#define OIDSET_H
+
+/**
+ * This API is similar to sha1-array, in that it maintains a set of object ids
+ * in a memory-efficient way. The major differences are:
+ *
+ *   1. It uses a hash, so we can do online duplicate removal, rather than
+ *  sort-and-uniq at the end. This can reduce memory footprint if you have
+ *  a large list of oids with many duplicates.
+ *
+ *   2. The per-unique-oid memory footprint is slightly higher due to hash
+ *  table overhead.
+ */
+
+/**
+ * A single oidset; should be zero-initialized (or use OIDSET_INIT).
+ */
+struct oidset {
+   struct hashmap map;
+};
+
+#define OIDSET_INIT { { NULL } }
+
+/**
+ * Returns true iff `set` contains `oid`.
+ */
+int oidset_contains(const struct oidset *set, const struct object_id *oid);
+
+/**
+ * Insert the oid into the set; a copy is made, so "oid" does not need
+ * to persist after this function is called.
+ *
+ * Returns 1 if the oid was already in the set, 0 otherwise. This can be used
+ * to perform an efficient check-and-add.
+ */
+int oidset_insert(struct oidset *set, const struct object_id *oid);
+
+/**
+ * Remove all entries from the oidset, freeing any resources associated with
+ * it.
+ */
+void oidset_clear(struct oidset *set);
+
+#endif /* OIDSET_H */
-- 
2.12.0.rc0.371.ga6cf8653b

[PATCH v2 08/11] receive-pack: use oidset to de-duplicate .have lines

[Jeff King]

If you have an alternate object store with a very large
number of refs, the peak memory usage of the sha1_array can
grow high, even if most of them are duplicates that end up
not being printed at all.

The similar for_each_alternate_ref() code-paths in
fetch-pack solve this by using flags in "struct object" to
de-duplicate (and so are relying on obj_hash at the core).

But we don't have a "struct object" at all in this case. We
could call lookup_unknown_object() to get one, but if our
goal is reducing memory footprint, it's not great:

 - an unknown object is as large as the largest object type
   (a commit), which is bigger than an oidset entry

 - we can free the memory after our ref advertisement, but
   "struct object" entries persist forever (and the
   receive-pack may hang around for a long time, as the
   bottleneck is often client upload bandwidth).

So let's use an oidset. Note that unlike a sha1-array it
doesn't sort the output as a side effect. However, our
output is at least stable, because for_each_alternate_ref()
will give us the sha1s in ref-sorted order.

In one particularly pathological case with an alternate that
has 60,000 unique refs out of 80 million total, this reduced
the peak heap usage of "git receive-pack . 
---
 builtin/receive-pack.c | 26 --
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index d21332d9e..a4926fcfb 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -21,6 +21,7 @@
 #include "sigchain.h"
 #include "fsck.h"
 #include "tmp-objdir.h"
+#include "oidset.h"
 
 static const char * const receive_pack_usage[] = {
N_("git receive-pack "),
@@ -271,27 +272,24 @@ static int show_ref_cb(const char *path_full, const 
struct object_id *oid,
return 0;
 }
 
-static int show_one_alternate_sha1(const unsigned char sha1[20], void *unused)
+static void show_one_alternate_ref(const char *refname,
+  const struct object_id *oid,
+  void *data)
 {
-   show_ref(".have", sha1);
-   return 0;
-}
+   struct oidset *seen = data;
 
-static void collect_one_alternate_ref(const char *refname,
- const struct object_id *oid,
- void *data)
-{
-   struct sha1_array *sa = data;
-   sha1_array_append(sa, oid->hash);
+   if (oidset_insert(seen, oid))
+   return;
+
+   show_ref(".have", oid->hash);
 }
 
 static void write_head_info(void)
 {
-   struct sha1_array sa = SHA1_ARRAY_INIT;
+   static struct oidset seen = OIDSET_INIT;
 
-   for_each_alternate_ref(collect_one_alternate_ref, );
-   sha1_array_for_each_unique(, show_one_alternate_sha1, NULL);
-   sha1_array_clear();
+   for_each_alternate_ref(show_one_alternate_ref, );
+   oidset_clear();
for_each_ref(show_ref_cb, NULL);
if (!sent_capabilities)
show_ref("capabilities^{}", null_sha1);
-- 
2.12.0.rc0.371.ga6cf8653b

[PATCH v2 10/11] receive-pack: treat namespace .have lines like alternates

[Jeff King]

Namely, de-duplicate them. We use the same set as the
alternates, since we call them both ".have" (i.e., there is
no value in showing one versus the other).

Signed-off-by: Jeff King 
---
 builtin/receive-pack.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 1821ad5fa..c23b0cce8 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -251,8 +251,9 @@ static void show_ref(const char *path, const unsigned char 
*sha1)
 }
 
 static int show_ref_cb(const char *path_full, const struct object_id *oid,
-  int flag, void *unused)
+  int flag, void *data)
 {
+   struct oidset *seen = data;
const char *path = strip_namespace(path_full);
 
if (ref_is_hidden(path, path_full))
@@ -263,8 +264,11 @@ static int show_ref_cb(const char *path_full, const struct 
object_id *oid,
 * refs, so that the client can use them to minimize data
 * transfer but will otherwise ignore them.
 */
-   if (!path)
+   if (!path) {
+   if (oidset_insert(seen, oid))
+   return 0;
path = ".have";
+   }
show_ref(path, oid->hash);
return 0;
 }
@@ -287,7 +291,7 @@ static void write_head_info(void)
 
for_each_alternate_ref(show_one_alternate_ref, );
oidset_clear();
-   for_each_ref(show_ref_cb, NULL);
+   for_each_ref(show_ref_cb, );
if (!sent_capabilities)
show_ref("capabilities^{}", null_sha1);
 
-- 
2.12.0.rc0.371.ga6cf8653b

[PATCH v2 11/11] receive-pack: avoid duplicates between our refs and alternates

[Jeff King]

We de-duplicate ".have" refs among themselves, but never
check if they are duplicates of our local refs. It's not
unreasonable that they would be if we are a "--shared" or
"--reference" clone of a similar repository; we'd have all
the same tags.

We can handle this by inserting our local refs into the
oidset, but obviously not suppressing duplicates (since the
refnames are important).

Note that this also switches the order in which we advertise
refs, processing ours first and then any alternates. The
order shouldn't matter (and arguably showing our refs first
makes more sense).

Signed-off-by: Jeff King 
---
 builtin/receive-pack.c |  4 +++-
 t/t5400-send-pack.sh   | 38 ++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c23b0cce8..9ed8fbbfa 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -268,6 +268,8 @@ static int show_ref_cb(const char *path_full, const struct 
object_id *oid,
if (oidset_insert(seen, oid))
return 0;
path = ".have";
+   } else {
+   oidset_insert(seen, oid);
}
show_ref(path, oid->hash);
return 0;
@@ -289,9 +291,9 @@ static void write_head_info(void)
 {
static struct oidset seen = OIDSET_INIT;
 
+   for_each_ref(show_ref_cb, );
for_each_alternate_ref(show_one_alternate_ref, );
oidset_clear();
-   for_each_ref(show_ref_cb, );
if (!sent_capabilities)
show_ref("capabilities^{}", null_sha1);
 
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 305ca7a93..3331e0f53 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -255,4 +255,42 @@ test_expect_success 'deny pushing to delete current 
branch' '
)
 '
 
+extract_ref_advertisement () {
+   perl -lne '
+   # \\ is there to skip capabilities after \0
+   /push< ([^\\]+)/ or next;
+   exit 0 if $1 eq "";
+   print $1;
+   '
+}
+
+test_expect_success 'receive-pack de-dupes .have lines' '
+   git init shared &&
+   git -C shared commit --allow-empty -m both &&
+   git clone -s shared fork &&
+   (
+   cd shared &&
+   git checkout -b only-shared &&
+   git commit --allow-empty -m only-shared &&
+   git update-ref refs/heads/foo HEAD
+   ) &&
+
+   # Notable things in this expectation:
+   #  - local refs are not de-duped
+   #  - .have does not duplicate locals
+   #  - .have does not duplicate itself
+   local=$(git -C fork rev-parse HEAD) &&
+   shared=$(git -C shared rev-parse only-shared) &&
+   cat >expect <<-EOF &&
+   $local refs/heads/master
+   $local refs/remotes/origin/HEAD
+   $local refs/remotes/origin/master
+   $shared .have
+   EOF
+
+   GIT_TRACE_PACKET=$(pwd)/trace git push fork HEAD:foo &&
+   extract_ref_advertisement refs &&
+   test_cmp expect refs
+'
+
 test_done
-- 
2.12.0.rc0.371.ga6cf8653b

Re: [PATCH v2] Document the --no-gui option in difftool

[Junio C Hamano]

Denton Liu  writes:

> Prior to this, the `--no-gui` option was not documented in the manpage.
> This commit introduces this into the manpage
>
> Signed-off-by: Denton Liu 
> ---

Looks good, thanks.

>  Documentation/git-difftool.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
> index 224fb3090..96c26e6aa 100644
> --- a/Documentation/git-difftool.txt
> +++ b/Documentation/git-difftool.txt
> @@ -86,10 +86,11 @@ instead.  `--no-symlinks` is the default on Windows.
>   Additionally, `$BASE` is set in the environment.
>  
>  -g::
> ---gui::
> +--[no-]gui::
>   When 'git-difftool' is invoked with the `-g` or `--gui` option
>   the default diff tool will be read from the configured
> - `diff.guitool` variable instead of `diff.tool`.
> + `diff.guitool` variable instead of `diff.tool`. The `--no-gui`
> + option can be used to override this setting.
>  
>  --[no-]trust-exit-code::
>   'git-difftool' invokes a diff tool individually on each file.

[PATCHv2] push options: pass push options to the transport helper

[Stefan Beller]

When using non-builtin protocols relying on a transport helper
(such as http), push options are not propagated to the helper.

The user could ask for push options and a push would seemingly succeed,
but the push options would never be transported to the server,
misleading the users expectation.

Fix this by propagating the push options to the transport helper.

This is only addressing the first issue of
   (1) the helper protocol does not propagate push-option
   (2) the http helper is not prepared to handle push-option

Once we fix (2), the http transport helper can make use of push options
as well, but that happens as a follow up. (1) is a bug fix, whereas (2)
is a feature, which is why we only do (1) here.

Signed-off-by: Stefan Beller 
---
 Documentation/gitremote-helpers.txt |  4 
 t/t5545-push-options.sh | 15 +++
 transport-helper.c  |  7 +++
 3 files changed, 26 insertions(+)

diff --git a/Documentation/gitremote-helpers.txt 
b/Documentation/gitremote-helpers.txt
index 9e8681f9e1..23474b1eab 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -462,6 +462,10 @@ set by Git if the remote helper has the 'option' 
capability.
 'option pushcert {'true'|'false'}::
GPG sign pushes.
 
+'option push-option ::
+   Transmit  as a push option. As the a push option
+   must not contain LF or NUL characters, the string is not encoded.
+
 SEE ALSO
 
 linkgit:git-remote[1]
diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh
index ea813b9383..9a57a7d8f2 100755
--- a/t/t5545-push-options.sh
+++ b/t/t5545-push-options.sh
@@ -3,6 +3,8 @@
 test_description='pushing to a repository using push options'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
 
 mk_repo_pair () {
rm -rf workbench upstream &&
@@ -100,4 +102,17 @@ test_expect_success 'two push options work' '
test_cmp expect upstream/.git/hooks/post-receive.push_options
 '
 
+test_expect_success 'push option denied properly by http remote helper' '\
+   mk_repo_pair &&
+   git -C upstream config receive.advertisePushOptions false &&
+   git -C upstream config http.receivepack true &&
+   cp -R upstream/.git "$HTTPD_DOCUMENT_ROOT_PATH"/upstream.git &&
+   git clone "$HTTPD_URL"/smart/upstream test_http_clone &&
+   test_commit -C test_http_clone one &&
+   test_must_fail git -C test_http_clone push --push-option=asdf origin 
master &&
+   git -C test_http_clone push origin master
+'
+
+stop_httpd
+
 test_done
diff --git a/transport-helper.c b/transport-helper.c
index 91aed35ebb..1258d6aedd 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -826,6 +826,13 @@ static void set_common_push_options(struct transport 
*transport,
if (set_helper_option(transport, TRANS_OPT_PUSH_CERT, 
"if-asked") != 0)
die("helper %s does not support --signed=if-asked", 
name);
}
+
+   if (flags & TRANSPORT_PUSH_OPTIONS) {
+   struct string_list_item *item;
+   for_each_string_list_item(item, transport->push_options)
+   if (set_helper_option(transport, "push-option", 
item->string) != 0)
+   die("helper %s does not support 'push-option'", 
name);
+   }
 }
 
 static int push_refs_with_push(struct transport *transport,
-- 
2.12.0.rc0.1.g018cb5e6f4

Re: [PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec patterns

[Junio C Hamano]

Linus Torvalds  writes:

> @@ -546,10 +546,16 @@ void parse_pathspec(struct pathspec *pathspec,
>   pathspec->magic |= item[i].magic;
>   }
>  
> - if (nr_exclude == n)
> - die(_("There is nothing to exclude from by :(exclude) 
> patterns.\n"
> -   "Perhaps you forgot to add either ':/' or '.' ?"));
> -
> + /*
> +  * If everything is an exclude pattern, add one positive pattern
> +  * that matches everyting. We allocated an extra one for this.
> +  */
> + if (nr_exclude == n) {
> + if (!(flags & PATHSPEC_PREFER_CWD))
> + prefixlen = 0;
> + init_pathspec_item(item + n, 0, prefix, prefixlen, "");
> + pathspec->nr++;
> + }
>  
>   if (pathspec->magic & PATHSPEC_MAXDEPTH) {
>   if (flags & PATHSPEC_KEEP_ORDER)

Thanks.  Even though the current code does not refer to the original
prefixlen after the added hunk, I'd prefer not to destroy it to
avoid future troubles, so I'll queue with a bit of tweak there,
perhaps like the attached.

Also this has an obvious fallout to the tests, whose (minimum) fix
is rather trivial.

Thanks.

 pathspec.c  | 7 +++
 t/t6132-pathspec-exclude.sh | 6 --
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/pathspec.c b/pathspec.c
index d8f78088c8..b961f00c8c 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -522,7 +522,7 @@ void parse_pathspec(struct pathspec *pathspec,
}
 
pathspec->nr = n;
-   ALLOC_ARRAY(pathspec->items, n+1);
+   ALLOC_ARRAY(pathspec->items, n + 1);
item = pathspec->items;
prefixlen = prefix ? strlen(prefix) : 0;
 
@@ -551,9 +551,8 @@ void parse_pathspec(struct pathspec *pathspec,
 * that matches everyting. We allocated an extra one for this.
 */
if (nr_exclude == n) {
-   if (!(flags & PATHSPEC_PREFER_CWD))
-   prefixlen = 0;
-   init_pathspec_item(item + n, 0, prefix, prefixlen, "");
+   int plen = (!(flags & PATHSPEC_PREFER_CWD)) ? 0 : prefixlen;
+   init_pathspec_item(item + n, 0, prefix, plen, "");
pathspec->nr++;
}
 
diff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh
index d51595cf6b..9dd5cde5fc 100755
--- a/t/t6132-pathspec-exclude.sh
+++ b/t/t6132-pathspec-exclude.sh
@@ -25,8 +25,10 @@ EOF
test_cmp expect actual
 '
 
-test_expect_success 'exclude only should error out' '
-   test_must_fail git log --oneline --format=%s -- ":(exclude)sub"
+test_expect_success 'exclude only no longer errors out' '
+   git log --oneline --format=%s -- . ":(exclude)sub" >expect &&
+   git log --oneline --format=%s -- ":(exclude)sub" >actual &&
+   test_cmp expect actual
 '
 
 test_expect_success 't_e_i() exclude sub' '

Re: [PATCH] refs-internal.c: make files_log_ref_write() static

[Junio C Hamano]

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

> Created in 5f3c3a4e6f (files_log_ref_write: new function - 2015-11-10)
> but probably never used outside refs-internal.c
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---

It's been more than a year, so let's declare that when somebody
needs it it can easily be turned into extern again and take this
patch.

Thanks.

>  refs/files-backend.c | 3 +++
>  refs/refs-internal.h | 4 
>  2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index c041d4ba21..6a0bf94bf1 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -165,6 +165,9 @@ static struct ref_entry *create_dir_entry(struct 
> files_ref_store *ref_store,
> const char *dirname, size_t len,
> int incomplete);
>  static void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry);
> +static int files_log_ref_write(const char *refname, const unsigned char 
> *old_sha1,
> +const unsigned char *new_sha1, const char *msg,
> +int flags, struct strbuf *err);
>  
>  static struct ref_dir *get_ref_dir(struct ref_entry *entry)
>  {
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index 25444cf5b0..4c9215d33f 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -220,10 +220,6 @@ struct ref_transaction {
>   enum ref_transaction_state state;
>  };
>  
> -int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
> - const unsigned char *new_sha1, const char *msg,
> - int flags, struct strbuf *err);
> -
>  /*
>   * Check for entries in extras that are within the specified
>   * directory, where dirname is a reference directory name including

Automatically Add .gitignore Files

[Thangalin]

I frequently forget to add .gitignore files when creating new .gitignore files.

I'd like to request a command-line option to always add .gitignore
files (or, more generally, always add files that match a given file
specification).

Replicate

0. git init ...
1. echo "*.bak" >> .gitignore
2. touch file.txt
3. git add file.txt
4. git commit -a -m "..."
5. git push origin master

Expected Results

The .gitignore file is also added to the repository. (This is probably
the 80% use case.)

Actual Results

The .gitignore file is not added to the repository.

Additional Details

At step 4, there should be a prompt, warning, or (preferably) either a
command-line or configuration option to add the .gitignore file prior
to commit, automatically. Such as:

git commit --include-all-gitignore-files -a -m "..."

Or:

echo "**/.gitignore" >> .git/config/add-before-commit

Re: "disabling bitmap writing, as some objects are not being packed"?

[David Turner]

On Wed, 2017-02-08 at 14:08 -0500, Jeff King wrote:
> On Wed, Feb 08, 2017 at 02:05:42PM -0500, David Turner wrote:
> 
> > On Wed, 2017-02-08 at 09:44 -0800, Junio C Hamano wrote:
> > > Duy Nguyen  writes:
> > > 
> > > > On second thought, perhaps gc.autoDetach should default to false if
> > > > there's no tty, since its main point it to stop breaking interactive
> > > > usage. That would make the server side happy (no tty there).
> > > 
> > > Sounds like an idea, but wouldn't that keep the end-user coming over
> > > the network waiting after accepting a push until the GC completes, I
> > > wonder.  If an impatient user disconnects, would that end up killing
> > > an ongoing GC?  etc.
> > 
> > Regardless, it's impolite to keep the user waiting. So, I think we
> > should just not write the "too many unreachable loose objects" message
> > if auto-gc is on.  Does that sound OK?
> 
> I thought the point of that message was to prevent auto-gc from kicking
> in over and over again due to objects that won't actually get pruned.
> 
> I wonder if you'd want to either bump the auto-gc object limit, or
> possibly reduce the gc.pruneExpire limit to keep this situation from
> coming up in the first place (or at least mitigating the amount of time
> it's the case).

Auto-gc might not succeed in pruning objects, but it will at least
reduce the number of packs, which is super-important for performance.

I think the intent of automatic gc is to have a git repository be
relatively low-maintenance from a server-operator perspective.  (Side
note: it's fairly trivial for a user with push access to mess with the
check simply by pushing a bunch of objects whose shas start with 17).
It seems odd that git gets itself into a state where it refuses to do
any maintenance just because at some point some piece of the maintenance
didn't make progress.

Sure, I could change my configuration, but that doesn't help the other
folks (e.g. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=813084 )
who run into this.

I have three thoughts on this:

Idea 1: when gc --auto would issue this message, instead it could create
a file named gc.too-much-garbage (instead of gc.log), with this message.
If that file exists, and it is less than one day (?) old, then we don't
attempt to do a full gc; instead we just run git repack -A -d.  (If it's
more than one day old, we just delete it and continue anyway).

Idea 2 : Like idea 1, but instead of repacking, just smash the existing
packs together into one big pack.  In other words, don't consider
dangling objects, or recompute deltas.  Twitter has a tool called "git
combine-pack" that does this:
https://github.com/dturner-tw/git/blob/dturner/journal/builtin/combine-pack.c

That's less space-efficient than a true repack, but it's no worse than
having the packs separate, and it's a win for read performance because
there's no need to do a linear search over N packs to find an object.

Idea 3: As I suggested last time, separate fatal and non-fatal errors.
If gc fails because of EIO or something, we probably don't want to touch
the disk anymore. But here, the worst consequence is that we waste some
processing power. And it's better to occasionally waste processing power
in a non-interactive setting than it is to do so when a user will be
blocked on it.  So non-fatal warnings should go to gc.log, and fatal
errors should go to gc.fatal.  gc.log won't block gc from running. I
think this is my preferred option.

Re: [PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec patterns

[Linus Torvalds]

On Wed, Feb 8, 2017 at 1:59 PM, Junio C Hamano  wrote:
>
> Thanks.  Even though the current code does not refer to the original
> prefixlen after the added hunk, I'd prefer not to destroy it to
> avoid future troubles, so I'll queue with a bit of tweak there,
> perhaps like the attached.

Yeah, I considered that. Along with just passing in a NULL prefix
string too for that case. Not that it matters.

So ack on that change,

 Linus

Re: [PATCH v4] tag: generate useful reflog message

[Cornelius Weig]

On 02/08/2017 10:28 PM, Junio C Hamano wrote:
> cornelius.w...@tngtech.com writes:
> 
>> From: Cornelius Weig 
>>
>> When tags are created with `--create-reflog` or with the option
>> `core.logAllRefUpdates` set to 'always', a reflog is created for them.
>> So far, the description of reflog entries for tags was empty, making the
>> reflog hard to understand. For example:
>> 6e3a7b3 refs/tags/test@{0}:
>>
>> Now, a reflog message is generated when creating a tag, following the
>> pattern "tag: tagging  ()". If
>> GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
>> ()" instead. If the tag references a commit object, the
>> description is set to the subject line of the commit, followed by its
>> commit date. For example:
>> 6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 
>> 2017-02-03)
>>
>> If the tag points to a tree/blob/tag objects, the following static
>> strings are taken as description:
>>
>>  - "tree object"
>>  - "blob object"
>>  - "other tag object"
>>
>> Signed-off-by: Cornelius Weig 
>> Reviewed-by: Junio C Hamano 
> 
> This last line is inappropriate, as I didn't review _THIS_ version,
> which is different from the previous one, and I haven't checked if
> the way the comments on the previous review were addressed in this
> version is agreeable.

Sorry for that confusion. I'm still not used to when adding what
sign-off is appropriate. I thought that adding you as reviewer is also a
question of courtesy.

A version with revised tests will follow.

Re: [PATCH 1/2] pathspec magic: add '^' as alias for '!'

[Brandon Williams]

On 02/07, Linus Torvalds wrote:
> 
> From: Linus Torvalds 
> Date: Tue, 7 Feb 2017 21:05:28 -0800
> Subject: [PATCH 1/2] pathspec magic: add '^' as alias for '!'
> 
> The choice of '!' for a negative pathspec ends up not only not matching
> what we do for revisions, it's also a horrible character for shell
> expansion since it needs quoting.
> 
> So add '^' as an alternative alias for an excluding pathspec entry.
> 
> Signed-off-by: Linus Torvalds 
> ---
>  pathspec.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/pathspec.c b/pathspec.c
> index 7ababb315..ecad03406 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -224,6 +224,12 @@ static const char *parse_short_magic(unsigned *magic, 
> const char *elem)
>   char ch = *pos;
>   int i;
>  
> + /* Special case alias for '!' */
> + if (ch == '^') {
> + *magic |= PATHSPEC_EXCLUDE;
> + continue;
> + }
> +
>   if (!is_pathspec_magic(ch))
>   break;

I like adding '^' to be an alias for excluding patterns.  There have
been numerous times where I have wanted to use exclude patterns and
forgotten that I've needed to do some escape magic to get my shell to
leave '!' alone.

The only issue I see with doing this is that if a user supplies an
exclude pattern for a command which doesn't support exclude pathspec
magic the unsupported_magic() function will have slightly cryptic
output.

git cmd -- :^dir

would produce some output which says:
':^dir': pathspec magic not supported by this command: 'exclude' (mnemonic: '!')

And the user may scratch their head for a second since they didn't
supply the '!' character, but rather '^'.  That being said I think it
should be fine since the long name of the magic is also printed so the
user should be able to figure out what's wrong.  I also don't think
there are any users of pathspecs which disallow exclude magic so this
may not even be an issue.

-- 
Brandon Williams

[PATCH v2 05/11] for_each_alternate_ref: replace transport code with for-each-ref

[Jeff King]

The current method for getting the refs from an alternate is
to run upload-pack in the alternate and parse its output
using the normal transport code.  This works and is
reasonably short, but it has a very bad memory footprint
when there are a lot of refs in the alternate. There are two
problems:

  1. It reads in all of the refs before passing any back to
 us. Which means that our peak memory usage has to store
 every ref (including duplicates for peeled variants),
 even if our callback could determine that some are not
 interesting (e.g., because they point to the same sha1
 as another ref).

  2. It allocates a "struct ref" for each one. Among other
 things, this contains 3 separate 20-byte oids, along
 with the name and various pointers.  That can add up,
 especially if the callback is only interested in the
 sha1 (which it can store in a sha1_array as just 20
 bytes).

On a particularly pathological case, where the alternate had
over 80 million refs pointing to only around 60,000 unique
objects, the peak heap usage of "git clone --reference" grew
to over 25GB.

This patch instead calls git-for-each-ref in the alternate
repository, and passes each line to the callback as we read
it. That drops the peak heap of the same command to 50MB.

I considered and rejected a few alternatives.

We could read all of the refs in the alternate using our own
ref code, just as we do with submodules.  However, as memory
footprint is one of the concerns here, we want to avoid
loading those refs into our own memory as a whole.

It's possible that this will be a better technique in the
future when the ref code can more easily iterate without
loading all of packed-refs into memory.

Another option is to keep calling upload-pack, and just
parse its output ourselves in a streaming fashion. Besides
for-each-ref being simpler (we get to define the format
ourselves, and don't have to deal with speaking the git
protocol), it's more flexible for possible future changes.

For instance, it might be useful for the caller to be able
to limit the set of "interesting" alternate refs.  The
motivating example is one where many "forks" of a particular
repository share object storage, and the shared storage has
refs for each fork (which is why so many of the refs are
duplicates; each fork has the same tags).  A plausible
future optimization would be to ask for the alternate refs
for just _one_ fork (if you had some out-of-band way of
knowing which was the most interesting or important for the
current operation).

Similarly, no callbacks actually care about the symref value
of alternate refs, and as before, this patch ignores them
entirely.  However, if we wanted to add them, for-each-ref's
"%(symref)" is going to be more flexible than upload-pack,
because the latter only handles the HEAD symref due to
historical constraints.

There is one potential downside, though: unlike upload-pack,
our for-each-ref command doesn't report the peeled value of
refs. The existing code calls the alternate_ref_fn callback
twice for tags: once for the tag, and once for the peeled
value with the refname set to "ref^{}".

For the callers in fetch-pack, this doesn't matter at all.
We immediately peel each tag down to a commit either way (so
there's a slight improvement, as do not bother passing the
redundant data over the pipe). For the caller in
receive-pack, it means we will not advertise the peeled
values of tags in our alternate. However, we also don't
advertise peeled values for our _own_ tags, so this is
actually making things more consistent.

It's unclear whether receive-pack advertising peeled values
is a win or not. On one hand, giving more information to the
other side may let it omit some objects from the push. On
the other hand, for tags which both sides have, they simply
bloat the advertisement. The upload-pack advertisement of
git.git is about 30% larger than the receive-pack
advertisement due to its peeled information.

This patch omits the peeled information from
for_each_alternate_ref entirely, and leaves it up to the
caller whether they want to dig up the information.

Signed-off-by: Jeff King 
---
 transport.c | 48 ++--
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/transport.c b/transport.c
index c87147046..48864b3a9 100644
--- a/transport.c
+++ b/transport.c
@@ -1206,6 +1206,42 @@ char *transport_anonymize_url(const char *url)
return xstrdup(url);
 }
 
+static void read_alternate_refs(const char *path,
+   alternate_ref_fn *cb,
+   void *data)
+{
+   struct child_process cmd = CHILD_PROCESS_INIT;
+   struct strbuf line = STRBUF_INIT;
+   FILE *fh;
+
+   cmd.git_cmd = 1;
+   argv_array_pushf(, "--git-dir=%s", path);
+   argv_array_push(, "for-each-ref");
+   argv_array_push(, "--format=%(objectname) %(refname)");
+   cmd.env = 

[PATCH v2 09/11] receive-pack: fix misleading namespace/.have comment

[Jeff King]

The comment claims that we handle alternate ".have" lines
through this function, but that hasn't been the case since
85f251045 (write_head_info(): handle "extra refs" locally,
2012-01-06).

Signed-off-by: Jeff King 
---
 builtin/receive-pack.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index a4926fcfb..1821ad5fa 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -261,10 +261,7 @@ static int show_ref_cb(const char *path_full, const struct 
object_id *oid,
/*
 * Advertise refs outside our current namespace as ".have"
 * refs, so that the client can use them to minimize data
-* transfer but will otherwise ignore them. This happens to
-* cover ".have" that are thrown in by add_one_alternate_ref()
-* to mark histories that are complete in our alternates as
-* well.
+* transfer but will otherwise ignore them.
 */
if (!path)
path = ".have";
-- 
2.12.0.rc0.371.ga6cf8653b

Re: Trying to use xfuncname without success.

[Jack Adrian Zappa]

That was it.  I have a .gitattributes file in my home directory.
Ahhh, but it's not in my %userprofile% directory, but in my ~
directory.

A bit confusing having 2 home directories.  I made a link to my
.gitconfig, but forgot to make a link to my .gitattributes.

Thanks.


A

On Wed, Feb 8, 2017 at 4:05 PM, Samuel Lijin  wrote:
> Double check .gitattributes?
>
> On Feb 8, 2017 2:58 PM, "Jack Adrian Zappa"  wrote:
>>
>> Thanks Samuel,
>>
>> That example showed that there must be something wrong in my .git
>> directory, because with it, I'm getting the correct output.  Moving
>> the same lines to my .git/config didn't work.
>>
>> On Wed, Feb 8, 2017 at 3:46 PM, Samuel Lijin  wrote:
>> > I just put this togther: https://github.com/sxlijin/xfuncname-test
>> >
>> > Try cloning and then for any of config1 thru 3,
>> >
>> > $ cp configX .git/config
>> > $ git diff HEAD^ -- test.natvis
>> >
>> > On Wed, Feb 8, 2017 at 2:42 PM, Jack Adrian Zappa
>> >  wrote:
>> >> Thanks Samuel,
>> >>
>> >> So, the question is, what is causing this problem on my system?
>> >>
>> >> Anyone have an idea to help diagnose this problem?
>> >>
>> >> On Wed, Feb 8, 2017 at 3:24 PM, Samuel Lijin  wrote:
>> >>> On Windows 7, it works for me in both CMD and Git Bash:
>> >>>
>> >>> $ git --version
>> >>> git version 2.11.0.windows.3
>> >>>
>> >>> $ git diff HEAD^ --word-diff
>> >>> diff --git a/test.natvis b/test.natvis
>> >>> index 93396ad..1233b8c 100644
>> >>> --- a/test.natvis
>> >>> +++ b/test.natvis
>> >>> @@ -18,6 +18,7 @@ test
>> >>>
>> >>>
>> >>>   
>> >>>   {+added_var+}
>> >>>
>> >>>
>> >>>   var2
>> >>>
>> >>> On Wed, Feb 8, 2017 at 12:37 PM, René Scharfe  wrote:
>>  Am 08.02.2017 um 18:11 schrieb Jack Adrian Zappa:
>> > Thanks Rene, but you seem to have missed the point.  NOTHING is
>> > working.  No matter what I put there, it doesn't seem to get
>> > matched.
>> 
>>  I'm not so sure about that.  With your example I get this diff
>>  without
>>  setting diff.natvis.xfuncname:
>> 
>>  diff --git a/a.natvis b/a.natvis
>>  index 7f9bdf5..bc3c090 100644
>>  --- a/a.natvis
>>  +++ b/a.natvis
>>  @@ -19,7 +19,7 @@
>>  xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010;>
>> 
>> 
>> 
>>  -  added_var
>>  +  added_vars
>> 
>> 
>> var2
>> 
>>  Note the XML namespace in the hunk header.  It's put there by the
>>  default rule because "xmlns" starts at the beginning of the line.
>>  Your
>>  diff has nothing there, which means the default rule is not used,
>>  i.e.
>>  your user-defined rule is in effect.
>> 
>>  Come to think of it, this line break in the middle of the
>>  AutoVisualizer
>>  tab might have been added by your email client unintentionally, so
>>  that
>>  we use different test files, which then of course results in
>>  different
>>  diffs.  Is that the case?
>> 
>>  Anyway, if I run the following two commands:
>> 
>>  $ git config diff.natvis.xfuncname "^[\t ]*>  ]+Name=\"([^\"]+)\".*$"
>>  $ echo '*.natvis diff=natvis' >.gitattributes
>> 
>>  ... then I get this, both on Linux (git version 2.11.1) and on
>>  Windows
>>  (git version 2.11.1.windows.1):
>> 
>>  diff --git a/a.natvis b/a.natvis
>>  index 7f9bdf5..bc3c090 100644
>>  --- a/a.natvis
>>  +++ b/a.natvis
>>  @@ -19,7 +19,7 @@ test
>> 
>> 
>> 
>>  -  added_var
>>  +  added_vars
>> 
>> 
>> var2
>> 
>> > Just to be sure, I tested your regex and again it didn't work.
>> 
>>  At this point I'm out of ideas, sorry. :(  The only way I was able to
>>  break it was due to mistyping the extension as "netvis" several times
>>  for some reason.
>> 
>>  René

Re: Automatically Add .gitignore Files

[Junio C Hamano]

Thangalin  writes:

> I frequently forget to add .gitignore files when creating new .gitignore 
> files.
>
> I'd like to request a command-line option to always add .gitignore
> files (or, more generally, always add files that match a given file
> specification).

That is essentially what "Untracked files" listing in the editor
buffer given to you when you type "git commit" is about.  By not
saying you ignore .gitattributes, .gitignore, .gitmodules, etc., you
are reminded if you forgot to add them.  "git status" does the same,
and perhaps you want to make it a habit to run it before committing.

I am tempted to say that there should be a way to somehow forbid use
of the "-m" option to "git commit" by default, until the user gains
more familiarity with use of Git.  That way, they will learn to pay
more attention to the "Untracked" and "Changes not staged" sections
;-)

Re: Trying to use xfuncname without success.

[Jack Adrian Zappa]

>  where it has grabbed a line at 126 and is using that for the hunk header.

When I say that, I mean that it is using that line for *every* hunk
header, for every change, regardless if it has passed a hunk head that
it should have matched.





On Wed, Feb 8, 2017 at 7:01 PM, Jack Adrian Zappa  wrote:
> Tried to copy the .git/config file over to the non-working repository
> and it didn't seem to do anything.  Could the git database be
> partially corrupted?
>
> On Wed, Feb 8, 2017 at 7:00 PM, Jack Adrian Zappa  
> wrote:
>> Well, it mostly works, but I'm getting some weirdness where it has
>> grabbed a line at 126 and is using that for the hunk header.  Strange
>> thing is, I move the file to another repository, commit it, make a
>> change to the fileand do a diff, and it gets the correct hunk header.
>>
>> I'm at a loss. :(
>>
>> On Wed, Feb 8, 2017 at 4:12 PM, Jack Adrian Zappa  
>> wrote:
>>> That was it.  I have a .gitattributes file in my home directory.
>>> Ahhh, but it's not in my %userprofile% directory, but in my ~
>>> directory.
>>>
>>> A bit confusing having 2 home directories.  I made a link to my
>>> .gitconfig, but forgot to make a link to my .gitattributes.
>>>
>>> Thanks.
>>>
>>>
>>> A
>>>
>>> On Wed, Feb 8, 2017 at 4:05 PM, Samuel Lijin  wrote:
 Double check .gitattributes?

 On Feb 8, 2017 2:58 PM, "Jack Adrian Zappa"  wrote:
>
> Thanks Samuel,
>
> That example showed that there must be something wrong in my .git
> directory, because with it, I'm getting the correct output.  Moving
> the same lines to my .git/config didn't work.
>
> On Wed, Feb 8, 2017 at 3:46 PM, Samuel Lijin  wrote:
> > I just put this togther: https://github.com/sxlijin/xfuncname-test
> >
> > Try cloning and then for any of config1 thru 3,
> >
> > $ cp configX .git/config
> > $ git diff HEAD^ -- test.natvis
> >
> > On Wed, Feb 8, 2017 at 2:42 PM, Jack Adrian Zappa
> >  wrote:
> >> Thanks Samuel,
> >>
> >> So, the question is, what is causing this problem on my system?
> >>
> >> Anyone have an idea to help diagnose this problem?
> >>
> >> On Wed, Feb 8, 2017 at 3:24 PM, Samuel Lijin  wrote:
> >>> On Windows 7, it works for me in both CMD and Git Bash:
> >>>
> >>> $ git --version
> >>> git version 2.11.0.windows.3
> >>>
> >>> $ git diff HEAD^ --word-diff
> >>> diff --git a/test.natvis b/test.natvis
> >>> index 93396ad..1233b8c 100644
> >>> --- a/test.natvis
> >>> +++ b/test.natvis
> >>> @@ -18,6 +18,7 @@ test
> >>>
> >>>
> >>>   
> >>>   {+added_var+}
> >>>
> >>>
> >>>   var2
> >>>
> >>> On Wed, Feb 8, 2017 at 12:37 PM, René Scharfe  wrote:
>  Am 08.02.2017 um 18:11 schrieb Jack Adrian Zappa:
> > Thanks Rene, but you seem to have missed the point.  NOTHING is
> > working.  No matter what I put there, it doesn't seem to get
> > matched.
> 
>  I'm not so sure about that.  With your example I get this diff
>  without
>  setting diff.natvis.xfuncname:
> 
>  diff --git a/a.natvis b/a.natvis
>  index 7f9bdf5..bc3c090 100644
>  --- a/a.natvis
>  +++ b/a.natvis
>  @@ -19,7 +19,7 @@
>  xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010;>
> 
> 
> 
>  -  added_var
>  +  added_vars
> 
> 
> var2
> 
>  Note the XML namespace in the hunk header.  It's put there by the
>  default rule because "xmlns" starts at the beginning of the line.
>  Your
>  diff has nothing there, which means the default rule is not used,
>  i.e.
>  your user-defined rule is in effect.
> 
>  Come to think of it, this line break in the middle of the
>  AutoVisualizer
>  tab might have been added by your email client unintentionally, so
>  that
>  we use different test files, which then of course results in
>  different
>  diffs.  Is that the case?
> 
>  Anyway, if I run the following two commands:
> 
>  $ git config diff.natvis.xfuncname "^[\t ]*  ]+Name=\"([^\"]+)\".*$"
>  $ echo '*.natvis diff=natvis' >.gitattributes
> 
>  ... then I get this, both on Linux (git version 2.11.1) and on
>  Windows
>  (git version 2.11.1.windows.1):
> 
>  diff --git a/a.natvis b/a.natvis
>  index 7f9bdf5..bc3c090 100644
>  --- a/a.natvis
>  +++ b/a.natvis
>  @@ -19,7 +19,7 @@ test
> 
> 
> 
> 

Re: "disabling bitmap writing, as some objects are not being packed"?

[Jeff King]

On Wed, Feb 08, 2017 at 04:18:25PM -0800, Junio C Hamano wrote:

> > We wrote something similar at GitHub, too, but we never ended up using
> > it in production. We found that with a sane scheduler, it's not too big
> > a deal to just do maintenance once in a while.
> 
> Thanks again for this.  I've also been wondering about how effective
> a "concatenate packs without paying reachability penalty" would be.

For the sake of posterity, I'll include our patch at the end (sorry, not
chunked into nice readable commits; that never existed in the first
place).

> > I'm still not sure if it's worth making the fatal/non-fatal distinction.
> > Doing so is perhaps safer, but it does mean that somebody has to decide
> > which errors are important enough to block a retry totally, and which
> > are not. In theory, it would be safe to always _try_ and then the gc
> > process can decide when something is broken and abort. And all you've
> > wasted is some processing power each day.
> 
> Yup, and somebody or something need to monitor so that repeated
> failures can be dealt with.

Yes. I think that part is probably outside the scope of Git. But if
auto-gc leaves gc.log lying around, it would be easy to visit each repo
and collect the various failures.

-- >8 --
This is the "pack-fast" patch, for reference. It applies on v2.6.5,
though I had to do some wiggling due to a few of our other custom
patches, so it's possible I introduced new bugs. It compiles, but I
didn't actually re-test the result.  I _think_ the original at least
generated valid packs in all cases.

So I would certainly not recommend anybody run this. It's just a
possible base to work off of if anybody's interested in the topic. I
haven't looked at David's combine-packs at all to see if it is any less
gross. :)

---
 Makefile|   1 +
 builtin.h   |   1 +
 builtin/pack-fast.c | 618 +++
 cache.h |   5 +
 git.c   |   1 +
 pack-bitmap-write.c | 167 +-
 pack-bitmap.c   |   2 +-
 pack-bitmap.h   |   8 +
 sha1_file.c |   4 +-
 9 files changed, 792 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index 37e2d9e18..524b185ec 100644
--- a/Makefile
+++ b/Makefile
@@ -887,6 +887,7 @@ BUILTIN_OBJS += builtin/mv.o
 BUILTIN_OBJS += builtin/name-rev.o
 BUILTIN_OBJS += builtin/notes.o
 BUILTIN_OBJS += builtin/pack-objects.o
+BUILTIN_OBJS += builtin/pack-fast.o
 BUILTIN_OBJS += builtin/pack-redundant.o
 BUILTIN_OBJS += builtin/pack-refs.o
 BUILTIN_OBJS += builtin/patch-id.o
diff --git a/builtin.h b/builtin.h
index 79aaf0afe..df4e4d668 100644
--- a/builtin.h
+++ b/builtin.h
@@ -95,6 +95,7 @@ extern int cmd_mv(int argc, const char **argv, const char 
*prefix);
 extern int cmd_name_rev(int argc, const char **argv, const char *prefix);
 extern int cmd_notes(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_objects(int argc, const char **argv, const char *prefix);
+extern int cmd_pack_fast(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_redundant(int argc, const char **argv, const char *prefix);
 extern int cmd_patch_id(int argc, const char **argv, const char *prefix);
 extern int cmd_prune(int argc, const char **argv, const char *prefix);
diff --git a/builtin/pack-fast.c b/builtin/pack-fast.c
new file mode 100644
index 0..ad9f5e5f1
--- /dev/null
+++ b/builtin/pack-fast.c
@@ -0,0 +1,618 @@
+#include "builtin.h"
+#include "cache.h"
+#include "pack.h"
+#include "progress.h"
+#include "csum-file.h"
+#include "sha1-lookup.h"
+#include "parse-options.h"
+#include "tempfile.h"
+#include "pack-bitmap.h"
+#include "pack-revindex.h"
+
+static const char *pack_usage[] = {
+   N_("git pack-fast --quiet [options...] [base-name]"),
+   NULL
+};
+
+struct packwriter {
+   struct tempfile *tmp;
+   off_t total;
+   int fd;
+   uint32_t crc32;
+   unsigned do_crc;
+};
+
+static void packwriter_crc32_start(struct packwriter *w)
+{
+   w->crc32 = crc32(0, NULL, 0);
+   w->do_crc = 1;
+}
+
+static uint32_t packwriter_crc32_end(struct packwriter *w)
+{
+   w->do_crc = 0;
+   return w->crc32;
+}
+
+static void packwriter_write(struct packwriter *w, const void *buf, unsigned 
int count)
+{
+   if (w->do_crc)
+   w->crc32 = crc32(w->crc32, buf, count);
+   write_or_die(w->fd, buf, count);
+   w->total += count;
+}
+
+static off_t packwriter_total(struct packwriter *w)
+{
+   return w->total;
+}
+
+static void packwriter_init(struct packwriter *w)
+{
+   char tmpname[PATH_MAX];
+
+   w->fd = odb_mkstemp(tmpname, sizeof(tmpname), "pack/tmp_pack_XX");
+   w->total = 0;
+   w->do_crc = 0;
+   w->tmp = xcalloc(1, sizeof(*w->tmp));
+
+   register_tempfile(w->tmp, tmpname);
+}
+
+
+static int progress = 1;
+static struct progress *progress_state;
+static struct pack_idx_option pack_idx_opts;
+static const char *base_name = "pack-fast";
+static int 

Re: [PATCH 1/2] pathspec magic: add '^' as alias for '!'

[Junio C Hamano]

Brandon Williams  writes:

> git cmd -- :^dir
>
> would produce some output which says:
> ':^dir': pathspec magic not supported by this command: 'exclude' (mnemonic: 
> '!')
>
> And the user may scratch their head for a second since they didn't
> supply the '!' character, but rather '^'.

Yup, I am tempted to tweak Cornelius's glossary fixup and squash
this into the series, for two purposes.

 - it makes it clear that '^' and '!' mean the same thing (and
   clearer than Cornelius's original, "! or ^", which could leave
   the reader wondering "ok there are two ways to say negative; do
   they subtly mean different things?").

 - it hints that '!' is the more official spelling, making the
   output you showed above acceptable.

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index 8ad29e61a9..822ca83264 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -386,8 +386,8 @@ Glob magic is incompatible with literal magic.
 
 exclude;;
After a path matches any non-exclude pathspec, it will be run
-   through all exclude pathspec (magic signature: `!`). If it
-   matches, the path is ignored.
+   through all exclude pathspec (magic signature: `!` or its
+   synonym `^`). If it matches, the path is ignored.
 --
 
 [[def_parent]]parent::

Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree

[Junio C Hamano]

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

> As such, we can just linked worktree's path as a submodule. We just need

Lack of verb made me read this three times.  "We can just treat
linked worktree's path as if it were a submodule"?

I agree with you that the "submodule" in the name is misleading.  We
would want to be able to walk refs from other local repositories
(and repository-like entities, like the .git/ thing in a liked
worktree), like our own submodule, or the repository we borrow
objects from via the alternate mechanism.

[PATCH v5] tag: generate useful reflog message

[cornelius . weig]

From: Cornelius Weig 

When tags are created with `--create-reflog` or with the option
`core.logAllRefUpdates` set to 'always', a reflog is created for them.
So far, the description of reflog entries for tags was empty, making the
reflog hard to understand. For example:
6e3a7b3 refs/tags/test@{0}:

Now, a reflog message is generated when creating a tag, following the
pattern "tag: tagging  ()". If
GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
()" instead. If the tag references a commit object, the
description is set to the subject line of the commit, followed by its
commit date. For example:
6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)

If the tag points to a tree/blob/tag objects, the following static
strings are taken as description:

 - "tree object"
 - "blob object"
 - "other tag object"

Signed-off-by: Cornelius Weig 
---

Notes:
Interdiff v4..v5
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 894959f..1a3230f 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -80,9 +80,10 @@ test_expect_success 'creating a tag using default HEAD 
should succeed' '
test_must_fail git reflog exists refs/tags/mytag
 '

-git log -1 > expected \
-   --format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
 test_expect_success 'creating a tag with --create-reflog should create 
reflog' '
+   git log -1 \
+   --format="format:tag: tagging %h (%s, %cd)%n" \
+   --date=format:%Y-%m-%d >expected &&
test_when_finished "git tag -d tag_with_reflog" &&
git tag --create-reflog tag_with_reflog &&
git reflog exists refs/tags/tag_with_reflog &&
@@ -90,9 +91,10 @@ test_expect_success 'creating a tag with --create-reflog 
should create reflog' '
test_cmp expected actual
 '

-git log -1 > expected \
-   --format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
 test_expect_success 'annotated tag with --create-reflog has correct 
message' '
+   git log -1 \
+   --format="format:tag: tagging %h (%s, %cd)%n" \
+   --date=format:%Y-%m-%d >expected &&
test_when_finished "git tag -d tag_with_reflog" &&
git tag -m "annotated tag" --create-reflog tag_with_reflog &&
git reflog exists refs/tags/tag_with_reflog &&

 builtin/tag.c  | 54 +-
 t/t7004-tag.sh | 18 +-
 2 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index e40c4a9..bca890f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -302,6 +302,54 @@ static void create_tag(const unsigned char *object, const 
char *tag,
}
 }
 
+static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
+{
+   enum object_type type;
+   struct commit *c;
+   char *buf;
+   unsigned long size;
+   int subject_len = 0;
+   const char *subject_start;
+
+   char *rla = getenv("GIT_REFLOG_ACTION");
+   if (rla) {
+   strbuf_addstr(sb, rla);
+   } else {
+   strbuf_addstr(sb, _("tag: tagging "));
+   strbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);
+   }
+
+   strbuf_addstr(sb, " (");
+   type = sha1_object_info(sha1, NULL);
+   switch (type) {
+   default:
+   strbuf_addstr(sb, _("object of unknown type"));
+   break;
+   case OBJ_COMMIT:
+   if ((buf = read_sha1_file(sha1, , )) != NULL) {
+   subject_len = find_commit_subject(buf, _start);
+   strbuf_insert(sb, sb->len, subject_start, subject_len);
+   } else {
+   strbuf_addstr(sb, _("commit object"));
+   }
+   free(buf);
+
+   if ((c = lookup_commit_reference(sha1)) != NULL)
+   strbuf_addf(sb, ", %s", show_date(c->date, 0, 
DATE_MODE(SHORT)));
+   break;
+   case OBJ_TREE:
+   strbuf_addstr(sb, _("tree object"));
+   break;
+   case OBJ_BLOB:
+   strbuf_addstr(sb, _("blob object"));
+   break;
+   case OBJ_TAG:
+   strbuf_addstr(sb, _("other tag object"));
+   break;
+   }
+   strbuf_addch(sb, ')');
+}
+
 struct msg_arg {
int given;
struct strbuf buf;
@@ -335,6 +383,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 {
struct strbuf buf = STRBUF_INIT;
struct strbuf ref = STRBUF_INIT;
+   struct strbuf reflog_msg = STRBUF_INIT;
unsigned char object[20], prev[20];
const char *object_ref, *tag;
struct create_tag_options opt;
@@ -494,6 +543,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
else
die(_("Invalid cleanup mode %s"), cleanup_arg);
 
+   

Re: Trying to use xfuncname without success.

[Jack Adrian Zappa]

Tried to copy the .git/config file over to the non-working repository
and it didn't seem to do anything.  Could the git database be
partially corrupted?

On Wed, Feb 8, 2017 at 7:00 PM, Jack Adrian Zappa  wrote:
> Well, it mostly works, but I'm getting some weirdness where it has
> grabbed a line at 126 and is using that for the hunk header.  Strange
> thing is, I move the file to another repository, commit it, make a
> change to the fileand do a diff, and it gets the correct hunk header.
>
> I'm at a loss. :(
>
> On Wed, Feb 8, 2017 at 4:12 PM, Jack Adrian Zappa  
> wrote:
>> That was it.  I have a .gitattributes file in my home directory.
>> Ahhh, but it's not in my %userprofile% directory, but in my ~
>> directory.
>>
>> A bit confusing having 2 home directories.  I made a link to my
>> .gitconfig, but forgot to make a link to my .gitattributes.
>>
>> Thanks.
>>
>>
>> A
>>
>> On Wed, Feb 8, 2017 at 4:05 PM, Samuel Lijin  wrote:
>>> Double check .gitattributes?
>>>
>>> On Feb 8, 2017 2:58 PM, "Jack Adrian Zappa"  wrote:

 Thanks Samuel,

 That example showed that there must be something wrong in my .git
 directory, because with it, I'm getting the correct output.  Moving
 the same lines to my .git/config didn't work.

 On Wed, Feb 8, 2017 at 3:46 PM, Samuel Lijin  wrote:
 > I just put this togther: https://github.com/sxlijin/xfuncname-test
 >
 > Try cloning and then for any of config1 thru 3,
 >
 > $ cp configX .git/config
 > $ git diff HEAD^ -- test.natvis
 >
 > On Wed, Feb 8, 2017 at 2:42 PM, Jack Adrian Zappa
 >  wrote:
 >> Thanks Samuel,
 >>
 >> So, the question is, what is causing this problem on my system?
 >>
 >> Anyone have an idea to help diagnose this problem?
 >>
 >> On Wed, Feb 8, 2017 at 3:24 PM, Samuel Lijin  wrote:
 >>> On Windows 7, it works for me in both CMD and Git Bash:
 >>>
 >>> $ git --version
 >>> git version 2.11.0.windows.3
 >>>
 >>> $ git diff HEAD^ --word-diff
 >>> diff --git a/test.natvis b/test.natvis
 >>> index 93396ad..1233b8c 100644
 >>> --- a/test.natvis
 >>> +++ b/test.natvis
 >>> @@ -18,6 +18,7 @@ test
 >>>
 >>>
 >>>   
 >>>   {+added_var+}
 >>>
 >>>
 >>>   var2
 >>>
 >>> On Wed, Feb 8, 2017 at 12:37 PM, René Scharfe  wrote:
  Am 08.02.2017 um 18:11 schrieb Jack Adrian Zappa:
 > Thanks Rene, but you seem to have missed the point.  NOTHING is
 > working.  No matter what I put there, it doesn't seem to get
 > matched.
 
  I'm not so sure about that.  With your example I get this diff
  without
  setting diff.natvis.xfuncname:
 
  diff --git a/a.natvis b/a.natvis
  index 7f9bdf5..bc3c090 100644
  --- a/a.natvis
  +++ b/a.natvis
  @@ -19,7 +19,7 @@
  xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010;>
 
 
 
  -  added_var
  +  added_vars
 
 
 var2
 
  Note the XML namespace in the hunk header.  It's put there by the
  default rule because "xmlns" starts at the beginning of the line.
  Your
  diff has nothing there, which means the default rule is not used,
  i.e.
  your user-defined rule is in effect.
 
  Come to think of it, this line break in the middle of the
  AutoVisualizer
  tab might have been added by your email client unintentionally, so
  that
  we use different test files, which then of course results in
  different
  diffs.  Is that the case?
 
  Anyway, if I run the following two commands:
 
  $ git config diff.natvis.xfuncname "^[\t ]*.gitattributes
 
  ... then I get this, both on Linux (git version 2.11.1) and on
  Windows
  (git version 2.11.1.windows.1):
 
  diff --git a/a.natvis b/a.natvis
  index 7f9bdf5..bc3c090 100644
  --- a/a.natvis
  +++ b/a.natvis
  @@ -19,7 +19,7 @@ test
 
 
 
  -  added_var
  +  added_vars
 
 
 var2
 
 > Just to be sure, I tested your regex and again it didn't work.
 
  At this point I'm out of ideas, sorry. :(  The only way I was able to
  break it was due to mistyping the extension as "netvis" several times
  for some reason.
 
  René

Re: Automatically Add .gitignore Files

[Stephan Beyer]

> I am tempted to say that there should be a way to somehow forbid use
> of the "-m" option to "git commit" by default, until the user gains
> more familiarity with use of Git.

Since I am using git, I am tempted to say that "git commit -m" should be
abolished. If I tell somebody how to use git, I never mention "git
commit -m". Unluckily they find it out themselves... ;D

The typical observations I have when people use "git commit -m":

 * The commit messages are either too long (in one line!) or
   too meaningless.

 * People don't check what they added and commit wrong stuff.

 * People make fast temporary commits with "asdafsd" commit messages.
   Then they get distracted for some time and work on another branch.
   When they turn back to the old branch they don't know what "asdafsd"
   was...

Thanks to git rebase -i and/or git commit --amend, it is all not too bad
after all ;D

Best
  Stephan

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Stefan Beller]

> * sb/submodule-doc (2017-01-12) 3 commits
>  - submodules: add a background story
>  - submodule update documentation: don't repeat ourselves
>  - submodule documentation: add options to the subcommand
>
>  Needs review.
>

The first two commits are good to go IIRC as you seemed to be
positive about them at the time. Though I have a hard time finding
evidence of such.

I am currently reworking the 3/3 "add a background story" patch as it is
RFC-ish, so no need to review that any more.

Maybe we can get 1&2 merged and then 3 comes on its own?

Thanks,
Stefan

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Junio C Hamano]

Stefan Beller  writes:

>> * sb/submodule-doc (2017-01-12) 3 commits
>>  - submodules: add a background story
>>  - submodule update documentation: don't repeat ourselves
>>  - submodule documentation: add options to the subcommand
>
> The first two commits are good to go IIRC as you seemed to be
> positive about them at the time. Though I have a hard time finding
> evidence of such.
>
> I am currently reworking the 3/3 "add a background story" patch as it is
> RFC-ish, so no need to review that any more.
>
> Maybe we can get 1&2 merged and then 3 comes on its own?

Yeah, sorry, I keep forgetting this topic, it seems.  Let's advance
the earlier two.  Tentatively I'd drop the third one as you plan to
redo it separately.

Thanks for prodding.

Re: Trying to use xfuncname without success.

[Jack Adrian Zappa]

Well, it mostly works, but I'm getting some weirdness where it has
grabbed a line at 126 and is using that for the hunk header.  Strange
thing is, I move the file to another repository, commit it, make a
change to the fileand do a diff, and it gets the correct hunk header.

I'm at a loss. :(

On Wed, Feb 8, 2017 at 4:12 PM, Jack Adrian Zappa  wrote:
> That was it.  I have a .gitattributes file in my home directory.
> Ahhh, but it's not in my %userprofile% directory, but in my ~
> directory.
>
> A bit confusing having 2 home directories.  I made a link to my
> .gitconfig, but forgot to make a link to my .gitattributes.
>
> Thanks.
>
>
> A
>
> On Wed, Feb 8, 2017 at 4:05 PM, Samuel Lijin  wrote:
>> Double check .gitattributes?
>>
>> On Feb 8, 2017 2:58 PM, "Jack Adrian Zappa"  wrote:
>>>
>>> Thanks Samuel,
>>>
>>> That example showed that there must be something wrong in my .git
>>> directory, because with it, I'm getting the correct output.  Moving
>>> the same lines to my .git/config didn't work.
>>>
>>> On Wed, Feb 8, 2017 at 3:46 PM, Samuel Lijin  wrote:
>>> > I just put this togther: https://github.com/sxlijin/xfuncname-test
>>> >
>>> > Try cloning and then for any of config1 thru 3,
>>> >
>>> > $ cp configX .git/config
>>> > $ git diff HEAD^ -- test.natvis
>>> >
>>> > On Wed, Feb 8, 2017 at 2:42 PM, Jack Adrian Zappa
>>> >  wrote:
>>> >> Thanks Samuel,
>>> >>
>>> >> So, the question is, what is causing this problem on my system?
>>> >>
>>> >> Anyone have an idea to help diagnose this problem?
>>> >>
>>> >> On Wed, Feb 8, 2017 at 3:24 PM, Samuel Lijin  wrote:
>>> >>> On Windows 7, it works for me in both CMD and Git Bash:
>>> >>>
>>> >>> $ git --version
>>> >>> git version 2.11.0.windows.3
>>> >>>
>>> >>> $ git diff HEAD^ --word-diff
>>> >>> diff --git a/test.natvis b/test.natvis
>>> >>> index 93396ad..1233b8c 100644
>>> >>> --- a/test.natvis
>>> >>> +++ b/test.natvis
>>> >>> @@ -18,6 +18,7 @@ test
>>> >>>
>>> >>>
>>> >>>   
>>> >>>   {+added_var+}
>>> >>>
>>> >>>
>>> >>>   var2
>>> >>>
>>> >>> On Wed, Feb 8, 2017 at 12:37 PM, René Scharfe  wrote:
>>>  Am 08.02.2017 um 18:11 schrieb Jack Adrian Zappa:
>>> > Thanks Rene, but you seem to have missed the point.  NOTHING is
>>> > working.  No matter what I put there, it doesn't seem to get
>>> > matched.
>>> 
>>>  I'm not so sure about that.  With your example I get this diff
>>>  without
>>>  setting diff.natvis.xfuncname:
>>> 
>>>  diff --git a/a.natvis b/a.natvis
>>>  index 7f9bdf5..bc3c090 100644
>>>  --- a/a.natvis
>>>  +++ b/a.natvis
>>>  @@ -19,7 +19,7 @@
>>>  xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010;>
>>> 
>>> 
>>> 
>>>  -  added_var
>>>  +  added_vars
>>> 
>>> 
>>> var2
>>> 
>>>  Note the XML namespace in the hunk header.  It's put there by the
>>>  default rule because "xmlns" starts at the beginning of the line.
>>>  Your
>>>  diff has nothing there, which means the default rule is not used,
>>>  i.e.
>>>  your user-defined rule is in effect.
>>> 
>>>  Come to think of it, this line break in the middle of the
>>>  AutoVisualizer
>>>  tab might have been added by your email client unintentionally, so
>>>  that
>>>  we use different test files, which then of course results in
>>>  different
>>>  diffs.  Is that the case?
>>> 
>>>  Anyway, if I run the following two commands:
>>> 
>>>  $ git config diff.natvis.xfuncname "^[\t ]*>>  ]+Name=\"([^\"]+)\".*$"
>>>  $ echo '*.natvis diff=natvis' >.gitattributes
>>> 
>>>  ... then I get this, both on Linux (git version 2.11.1) and on
>>>  Windows
>>>  (git version 2.11.1.windows.1):
>>> 
>>>  diff --git a/a.natvis b/a.natvis
>>>  index 7f9bdf5..bc3c090 100644
>>>  --- a/a.natvis
>>>  +++ b/a.natvis
>>>  @@ -19,7 +19,7 @@ test
>>> 
>>> 
>>> 
>>>  -  added_var
>>>  +  added_vars
>>> 
>>> 
>>> var2
>>> 
>>> > Just to be sure, I tested your regex and again it didn't work.
>>> 
>>>  At this point I'm out of ideas, sorry. :(  The only way I was able to
>>>  break it was due to mistyping the extension as "netvis" several times
>>>  for some reason.
>>> 
>>>  René

Re: [PATCH v4] tag: generate useful reflog message

[Junio C Hamano]

Cornelius Weig  writes:

> A version with revised tests will follow.

Thanks; I think this is clean enough.  Let's queue this one and
advance it to 'next' soonish.

Re: [PATCH/RFC] WIP: log: allow "-" as a short-hand for "previous branch"

[Siddharth Kannan]

Hello Matthieu,

On 8 February 2017 at 20:10, Matthieu Moy  wrote:
> In a previous discussion, I made an analogy with "cd -" (which is the
> source of inspiration of this shorthand AFAIK): "-" did not magically
> become "the last visited directory" for all Unix commands, just for
> "cd". And in this case, I'm happy with it. For example, I never need
> "mkdir -", and I'm happy I can't "rm -fr -" by mistake.
>
> So, it's debatable whether it's a good thing to have all commands
> support "-". For example, forcing users to explicitly type "git branch
> -d @{1}" and not providing them with a shortcut might be a good thing.

builtin/branch.c does not call setup_revisions and remains unaffected
by this patch :)

In my original patch post, I was not explicit about what files call
setup_revisions.
I would like to rectify that with this (grep-ed) list:

* builtin/blame.c
* builtin/diff.c
* builtin/diff-files.c
* builtin/diff-index.c
* builtin/diff-tree.c
* builtin/log.c
* builtin/rev-list.c
* builtin/shortlog.c
* builtin/fast-export.c
* builtin/fmt-merge-msg.c
builtin/add.c
builtin/checkout.c
builtin/commit.c
builtin/merge.c
builtin/pack-objects.c
builtin/revert.c

* marked commands only show information, and don't change anything.

As you might notice, in this list, most commands are not of the `rm` variety,
i.e. something that would delete stuff.

In the next version of this patch, I will definitely include the list
of commands
which are "rm-ish" and affected by this patch.

>
> I don't have strong opinion on this: I tend to favor consistency and
> supporting "-" everywhere goes in this direction, but I think the
> downsides should be considered too. A large part of the exercice here is
> to write a good commit message!

Yes, I prefer consistency very much as well! Having "-" mean the same thing
across a lot of commands is better than having that shorthand only in a few
commands, as it is now. Unless there is a specific confusion that might arise
because of this shorthand inclusion, I think that this shorthand
should be supported
across the board.
(I especially like typing `git checkout - ` which is very handy!)

>
> Another issue with this is: - is also a common way to say "use stdin
> instead of a file", so before enabling - for "previous branch", we need
> to make sure it does not introduce any ambiguity. Git does not seem to
> use "- for stdin" much (most commands able to read from stdin have an
> explicit --stdin option for that), a quick grep in the docs shows only
> "git blame --contents -" which is OK because a revision wouldn't make
> sense here anyway.

Yes, just to jog your memory, this was discussed here [1]

Junio said:

As long as the addition is carefully prepared so that we know it
will not conflict (or be confused by users) with possible other uses
of "-", I do not think we would mind "git branch -D -" and other
commands to learn "-" as a synonym for @{-1}.

>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/

Thanks a lot for the review on this patch, Matthieu!

-- 

Best Regards,

- Siddharth.

[1]: https://public-inbox.org/git/7vmwpitb6k@alter.siamese.dyndns.org/

Re: "disabling bitmap writing, as some objects are not being packed"?

[Jeff King]

On Wed, Feb 08, 2017 at 05:14:03PM -0500, David Turner wrote:

> > I wonder if you'd want to either bump the auto-gc object limit, or
> > possibly reduce the gc.pruneExpire limit to keep this situation from
> > coming up in the first place (or at least mitigating the amount of time
> > it's the case).
> 
> Auto-gc might not succeed in pruning objects, but it will at least
> reduce the number of packs, which is super-important for performance.

Right, I mean to bump the loose-object limit but keep the
gc.autoPackLimit at 50. If you couple that with setting
transfer.unpackLimit, then each push creates a single pack, and you
repack after 50 pushes.

You don't have to care about loose objects, because you know you only
get them when a "gc" ejects loose objects (so they're not as efficient,
but nothing actually accesses them; they just hang around until their
mtime grace period is up).

> I think the intent of automatic gc is to have a git repository be
> relatively low-maintenance from a server-operator perspective.  (Side
> note: it's fairly trivial for a user with push access to mess with the
> check simply by pushing a bunch of objects whose shas start with 17).
> It seems odd that git gets itself into a state where it refuses to do
> any maintenance just because at some point some piece of the maintenance
> didn't make progress.

In my experience, auto-gc has never been a low-maintenance operation on
the server side (and I do think it was primarily designed with clients
in mind).

At GitHub we disable it entirely, and do our own gc based on a throttled
job queue (one reason to throttle is that repacking is memory and I/O
intensive, so you really don't want to a bunch of repacks kicking off
all at once). So basically any repo that gets pushed to goes on the
queue, and then we pick the worst cases from the queue based on how
badly they need packing[1].

I wish regular Git were more turn-key in that respect. Maybe it is for
smaller sites, but we certainly didn't find it so. And I don't know that
it's feasible to really share the solution. It's entangled with our
database (to store last-pushed and last-maintenance values for repos)
and our job scheduler.

[1] The "how bad" thing is a heuristic, and we found it's generally
proportional to the number of bytes stored in objects _outside_ of
the big "main" pack. So 2 big pushes may need maintenance more
than 10 tiny pushes, because they have more objects (and our goal
with maintenance isn't just saving disk space or avoiding the linear
pack search, but having up-to-date bitmaps and good on-disk deltas
to make serving fetches as cheap as possible).

> Sure, I could change my configuration, but that doesn't help the other
> folks (e.g. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=813084 )
> who run into this.

Yeah, I'm certainly open to improving Git's defaults. If it's not clear
from the above, I mostly just gave up for a site the size of GitHub. :)

> Idea 1: when gc --auto would issue this message, instead it could create
> a file named gc.too-much-garbage (instead of gc.log), with this message.
> If that file exists, and it is less than one day (?) old, then we don't
> attempt to do a full gc; instead we just run git repack -A -d.  (If it's
> more than one day old, we just delete it and continue anyway).

I kind of wonder if this should apply to _any_ error. I.e., just check
the mtime of gc.log and forcibly remove it when it's older than a day.
You never want to get into a state that will fail to resolve itself
eventually. That might still happen (e.g., corrupt repo), but at the
very least it won't be because Git is too dumb to try again.

> Idea 2 : Like idea 1, but instead of repacking, just smash the existing
> packs together into one big pack.  In other words, don't consider
> dangling objects, or recompute deltas.  Twitter has a tool called "git
> combine-pack" that does this:
> https://github.com/dturner-tw/git/blob/dturner/journal/builtin/combine-pack.c

We wrote something similar at GitHub, too, but we never ended up using
it in production. We found that with a sane scheduler, it's not too big
a deal to just do maintenance once in a while.

  Also, our original problem was that repos which have gotten out of
  hand (say, 5000 packs) repacked _very_ slowly with a normal repack. So
  a "fast pack" followed by a real pack was a viable way out of that. In
  the end, I just made pack-objects handle this case better, and we
  don't need the fast-pack.

> That's less space-efficient than a true repack, but it's no worse than
> having the packs separate, and it's a win for read performance because
> there's no need to do a linear search over N packs to find an object.

Over the long term you may end up with worse packs, because the true
repack will drop some delta opportunities between objects in the same
pack (reasoning that they weren't made into deltas last time, so it's
not worth trying again). You'd probably need to use "-f" 

Re: [PATCH v2] rev-list-options.txt: update --all about HEAD

[Jeff King]

On Wed, Feb 08, 2017 at 01:06:41PM +0700, Nguyễn Thái Ngọc Duy wrote:

> This is the document patch for f0298cf1c6 (revision walker: include a
> detached HEAD in --all - 2009-01-16).
> 
> Even though that commit is about detached HEAD, as Jeff pointed out,
> always adding HEAD in that case may have subtle differences with
> --source or --exclude. So the document mentions nothing about the
> detached-ness.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  v2 drops "detached".

Thanks. Seems like an obvious improvement to me.

-Peff

Re: [PATCH] push options: fail properly in the stateless case

[Junio C Hamano]

Stefan Beller  writes:

>>> +'option push-option ::
>>> + Transmit this push option.
>>> +
>>
>> There is no "c-string" in the current documentation used or
>> defined.  The closest thing I found is
>>
>> ... that field will be quoted in the manner of a C string ...
>>
>> in git-status page, but I do not think you send the value for an
>> push-option after running quote_c_style(), so I am puzzled.
>
> When implementing push options, we discussed that and according to
> Documentation/git-push:
>
> The given string must not contain a NUL or LF character.

OK, so "Transmit  as a push option" is sufficient, as the
string is sent as-is.  OK.

Thanks.

Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree

[Junio C Hamano]

Stefan Beller  writes:

> On Wed, Feb 8, 2017 at 3:31 AM, Nguyễn Thái Ngọc Duy  
> wrote:
>> (**) At this point, we may want to rename refs *_submodule API to
>> something more neutral, maybe s/_submodule/_remote/
>
> I agree on (**), except that I am not sure if /_remote/ is a better name,
> because there is already a concept of a "remote" as well as
> "remote-tracking" in Git. (Usually it is not reachable on the same
> FS, but resides on another machine).

I agree with you that the concept of remote is quite detached from
the concept of wt and submodule whose refs need to be peeked at from
the local repository.  After all, "remote" tracking branches are
part of local repository's refs.

> My gut reaction would be to s/submodule/alternative/ here,
> but we also have a thing called alternates already.

... and I tend to think that is far closer a concept.  You borrow
objects from your alternate object store, and that alternate object
store may have its own set of refs you would need to peek when you
are computing reachability from refs.

Also don't we already enumerate such refs that pin objects in the
alternate object store when doing object transfer negotiation in
order to send ".have" entries for their tips?  What API do we use to
do that, I wonder.

Re: "disabling bitmap writing, as some objects are not being packed"?

[Junio C Hamano]

Jeff King  writes:

> In my experience, auto-gc has never been a low-maintenance operation on
> the server side (and I do think it was primarily designed with clients
> in mind).

I do not think auto-gc was ever tweaked to help server usage, in its
history since it was invented strictly to help end-users (mostly new
ones).

> At GitHub we disable it entirely, and do our own gc based on a throttled
> job queue ...
> I wish regular Git were more turn-key in that respect. Maybe it is for
> smaller sites, but we certainly didn't find it so. And I don't know that
> it's feasible to really share the solution. It's entangled with our
> database (to store last-pushed and last-maintenance values for repos)
> and our job scheduler.

Thanks for sharing the insights from the trenches ;-)

> Yeah, I'm certainly open to improving Git's defaults. If it's not clear
> from the above, I mostly just gave up for a site the size of GitHub. :)
>
>> Idea 1: when gc --auto would issue this message, instead it could create
>> a file named gc.too-much-garbage (instead of gc.log), with this message.
>> If that file exists, and it is less than one day (?) old, then we don't
>> attempt to do a full gc; instead we just run git repack -A -d.  (If it's
>> more than one day old, we just delete it and continue anyway).
>
> I kind of wonder if this should apply to _any_ error. I.e., just check
> the mtime of gc.log and forcibly remove it when it's older than a day.
> You never want to get into a state that will fail to resolve itself
> eventually. That might still happen (e.g., corrupt repo), but at the
> very least it won't be because Git is too dumb to try again.

;-)

>> Idea 2 : Like idea 1, but instead of repacking, just smash the existing
>> packs together into one big pack.  In other words, don't consider
>> dangling objects, or recompute deltas.  Twitter has a tool called "git
>> combine-pack" that does this:
>> https://github.com/dturner-tw/git/blob/dturner/journal/builtin/combine-pack.c
>
> We wrote something similar at GitHub, too, but we never ended up using
> it in production. We found that with a sane scheduler, it's not too big
> a deal to just do maintenance once in a while.

Thanks again for this.  I've also been wondering about how effective
a "concatenate packs without paying reachability penalty" would be.

> I'm still not sure if it's worth making the fatal/non-fatal distinction.
> Doing so is perhaps safer, but it does mean that somebody has to decide
> which errors are important enough to block a retry totally, and which
> are not. In theory, it would be safe to always _try_ and then the gc
> process can decide when something is broken and abort. And all you've
> wasted is some processing power each day.

Yup, and somebody or something need to monitor so that repeated
failures can be dealt with.

Re: [PATCH 1/2] pathspec magic: add '^' as alias for '!'

[Stefan Beller]

On Wed, Feb 8, 2017 at 3:05 PM, Junio C Hamano  wrote:
>  - it hints that '!' is the more official spelling, making the
>output you showed above acceptable.

Long term , I'd rather have ^ as the "official" spelling as that is easier
to teach to people. ! being a historic mistake as it is hard to type?

Re: [RFC 00/14] Allow fetch-pack to send ref names (globs allowed)

[Junio C Hamano]

Jonathan Tan  writes:

> Usages of the list of remote refs or the remote-local ref
> map are updated as follows:
>  - check_not_current_branch (which checks that the current branch is not
>affected by the fetch) is performed both on the original ref map (to
>die before the fetch if we can, as an optimization) and on the new
>ref map (since the new ref map is the one actually applied).

OK.

>  - Pruning is done based on the new ref map.

OK.  As that is what eventually gets "installed" on the local side,
it makes sense to become consistent with that set, not the set the
original server gave you.

>  - backfill_tags (for tag following) is performed using the original
>list of remote refs because the new list of fetched refs is not
>guaranteed to contain tag information. (Since backfill_tags performs
>another fetch, it does not need to be fully consistent with the
>just-returned packfile.)

This smells correct but I need to think about this one a bit more.

Overall I think the strategy is agreeable.

[PATCH] gc: ignore old gc.log files

[David Turner]

The intent of automatic gc is to have a git repository be relatively
low-maintenance from a server-operator perspective.  Of course, large
operators like GitHub will need a more complicated management strategy,
but for ordinary usage, git should just work.

In this commit, git learns to ignore gc.log files which are older than
(by default) one day old.  It also learns about a config, gc.maxLogAge
to manage this.

So git should never get itself into a state where it refuses to do any
maintenance, just because at some point some piece of the maintenance
didn't make progress.  That might still happen (e.g. because the repo
is corrupt), but at the very least it won't be because Git is too dumb
to try again.

Signed-off-by: David Turner 
Helped-by: Jeff King 
---
 Documentation/config.txt |  5 +
 builtin/gc.c | 15 ++-
 t/t6500-gc.sh| 13 +
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fc5a28a32..6751371cf 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1402,6 +1402,11 @@ gc.autoDetach::
Make `git gc --auto` return immediately and run in background
if the system supports it. Default is true.
 
+gc.maxLogAge::
+   If the file gc.log exists, then `git gc --auto` won't run
+   unless that file is more than maxLogAge seconds old.  Default
+   is 86400, one day.
+
 gc.packRefs::
Running `git pack-refs` in a repository renders it
unclonable by Git versions prior to 1.5.1.2 over dumb
diff --git a/builtin/gc.c b/builtin/gc.c
index 331f21926..62fc84815 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -33,6 +33,7 @@ static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 50;
 static int detach_auto = 1;
+static int gc_max_log_age_seconds = 86400;
 static const char *prune_expire = "2.weeks.ago";
 static const char *prune_worktrees_expire = "3.months.ago";
 
@@ -111,6 +112,7 @@ static void gc_config(void)
git_config_get_int("gc.auto", _auto_threshold);
git_config_get_int("gc.autopacklimit", _auto_pack_limit);
git_config_get_bool("gc.autodetach", _auto);
+   git_config_get_int("gc.maxlogage", _max_log_age_seconds);
git_config_date_string("gc.pruneexpire", _expire);
git_config_date_string("gc.worktreepruneexpire", 
_worktrees_expire);
git_config(git_default_config, NULL);
@@ -291,8 +293,19 @@ static int report_last_gc_error(void)
 {
struct strbuf sb = STRBUF_INIT;
int ret;
+   struct stat st;
+   const char *gc_log_path = git_path("gc.log");
+
+   if (stat(gc_log_path, )) {
+   if (errno == ENOENT)
+   return 0;
+   return error(_("Can't read %s"), gc_log_path);
+   }
+
+   if (time(NULL) - st.st_mtime > gc_max_log_age_seconds)
+   return 0;
 
-   ret = strbuf_read_file(, git_path("gc.log"), 0);
+   ret = strbuf_read_file(, gc_log_path, 0);
if (ret > 0)
return error(_("The last gc run reported the following. "
   "Please correct the root cause\n"
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 1762dfa6a..b69c2c190 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -67,5 +67,18 @@ test_expect_success 'auto gc with too many loose objects 
does not attempt to cre
test_line_count = 2 new # There is one new pack and its .idx
 '
 
+test_expect_success 'background auto gc does not run if gc.log is present and 
recent but does if it is old' '
+   keep=$(ls .git/objects/pack/*.pack|head -1|sed -e "s/pack$/keep/") &&
+   test_commit foo &&
+   test_commit bar &&
+   git repack &&
+   test_config gc.autopacklimit 1 &&
+   test_config gc.autodetach true &&
+   echo fleem> .git/gc.log &&
+   test_must_fail git gc --auto 2>err &&
+   test_i18ngrep "^error:" err &&
+   test-chmtime =-86401 .git/gc.log &&
+   git gc --auto
+'
 
 test_done
-- 
2.11.GIT

Re: git-scm.com status report

[brian m. carlson]

On Mon, Feb 06, 2017 at 07:27:54PM +0100, Jeff King wrote:
>   - It's mostly silly for this to be a Rails app at all. It's a static
> site which occasionally sucks in and formats new content (like the
> latest git version, new manpages, etc). The intent here was to make
> something that would "just run" forever and pick up new versions
> without human intervention. And that _does_ work, but it also makes
> things more expensive and complicated than they need to be.
> 
> So a viable alternative is to use some kind of static site
> generator and have someone (or something) responsible for pulling in
> the new git versions occasionally.
> 
> A few people have expressed interesting this. There's some
> preliminary work here:
> 
>   https://github.com/git/git-scm.com/pull/941
> 
> and at least GitLab has expressed some interest. So I'll let people
> coordinate in that PR or a new one what the result should look like.
> Working patches trump discussion. :)
> 
> I have also talked with the GitHub Pages people, and they think
> hosting it as a Jekyll page wouldn't be a big deal performance-wise
> (with the caveat that we'd need to pre-render the asciidoctor bits
> ourselves, as Jekyll doesn't do asciidoc). So that's a viable option
> for hosting it for effectively free (though I think we _would_ still
> want to put a CDN in front of it). But if somebody has an
> alternative option, that's fine, too.

My only concern with using GitHub Pages is that I don't believe it
currently supports TLS on custom domains.  Since we currently have TLS
enabled, along with HTTP Strict Transport Security (as we should), such
a configuration literally wouldn't work[0].  I think it's important that
we continue to serve HTTPS only, anyway.

I agree that a static site is the way to go from a maintenance
perspective, though.  Jekyll does support Asciidoctor with a plugin,
just not on GitHub Pages, so it would theoretically be possible to build
the site as one big unit if we did it that way.  I've played around with
that plugin, so I'm happy to provide guidance if we want to do that.

[0] HSTS would prevent anyone who had visited the page from downgrading
to an insecure connection for the next year.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

[PATCH] Documentation: unify bottom "part of git suite" lines

[Stefan Beller]

We currently have 168 man pages that mention they are part of Git, you
can check yourself easily via:
  $ git grep "Part of the linkgit:git\[1\] suite" |wc -l
  168
However some have a trailing period, i.e.
  $ git grep "Part of the linkgit:git\[1\] suite." |wc -l
  8

Unify the bottom line in all man pages to not end with a period.

Signed-off-by: Stefan Beller 
---
 Documentation/gitcore-tutorial.txt | 2 +-
 Documentation/gitcvs-migration.txt | 2 +-
 Documentation/gitdiffcore.txt  | 2 +-
 Documentation/gitglossary.txt  | 2 +-
 Documentation/gitrepository-layout.txt | 2 +-
 Documentation/gittutorial-2.txt| 2 +-
 Documentation/gittutorial.txt  | 2 +-
 Documentation/gitworkflows.txt | 2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/gitcore-tutorial.txt 
b/Documentation/gitcore-tutorial.txt
index 22309cfb48..3a0ec8c53a 100644
--- a/Documentation/gitcore-tutorial.txt
+++ b/Documentation/gitcore-tutorial.txt
@@ -1658,4 +1658,4 @@ link:user-manual.html[The Git User's Manual]
 
 GIT
 ---
-Part of the linkgit:git[1] suite.
+Part of the linkgit:git[1] suite
diff --git a/Documentation/gitcvs-migration.txt 
b/Documentation/gitcvs-migration.txt
index 4c6143c511..1cd1283d0f 100644
--- a/Documentation/gitcvs-migration.txt
+++ b/Documentation/gitcvs-migration.txt
@@ -203,4 +203,4 @@ link:user-manual.html[The Git User's Manual]
 
 GIT
 ---
-Part of the linkgit:git[1] suite.
+Part of the linkgit:git[1] suite
diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt
index 08cf62278e..46bc6d077c 100644
--- a/Documentation/gitdiffcore.txt
+++ b/Documentation/gitdiffcore.txt
@@ -288,4 +288,4 @@ link:user-manual.html[The Git User's Manual]
 
 GIT
 ---
-Part of the linkgit:git[1] suite.
+Part of the linkgit:git[1] suite
diff --git a/Documentation/gitglossary.txt b/Documentation/gitglossary.txt
index 212e254adc..571f640f5c 100644
--- a/Documentation/gitglossary.txt
+++ b/Documentation/gitglossary.txt
@@ -24,4 +24,4 @@ link:user-manual.html[The Git User's Manual]
 
 GIT
 ---
-Part of the linkgit:git[1] suite.
+Part of the linkgit:git[1] suite
diff --git a/Documentation/gitrepository-layout.txt 
b/Documentation/gitrepository-layout.txt
index a5f99cbb11..f51ed4e37c 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -289,4 +289,4 @@ link:user-manual.html[The Git User's Manual]
 
 GIT
 ---
-Part of the linkgit:git[1] suite.
+Part of the linkgit:git[1] suite
diff --git a/Documentation/gittutorial-2.txt b/Documentation/gittutorial-2.txt
index 30d2119565..e0976f6017 100644
--- a/Documentation/gittutorial-2.txt
+++ b/Documentation/gittutorial-2.txt
@@ -433,4 +433,4 @@ link:user-manual.html[The Git User's Manual]
 
 GIT
 ---
-Part of the linkgit:git[1] suite.
+Part of the linkgit:git[1] suite
diff --git a/Documentation/gittutorial.txt b/Documentation/gittutorial.txt
index b3b58d324e..794b83393e 100644
--- a/Documentation/gittutorial.txt
+++ b/Documentation/gittutorial.txt
@@ -674,4 +674,4 @@ link:user-manual.html[The Git User's Manual]
 
 GIT
 ---
-Part of the linkgit:git[1] suite.
+Part of the linkgit:git[1] suite
diff --git a/Documentation/gitworkflows.txt b/Documentation/gitworkflows.txt
index f16c414ea7..177610e44e 100644
--- a/Documentation/gitworkflows.txt
+++ b/Documentation/gitworkflows.txt
@@ -477,4 +477,4 @@ linkgit:git-am[1]
 
 GIT
 ---
-Part of the linkgit:git[1] suite.
+Part of the linkgit:git[1] suite
-- 
2.12.0.rc0.1.g018cb5e6f4

Re: git-scm.com status report

[Jeff King]

On Thu, Feb 09, 2017 at 02:12:09AM +, brian m. carlson wrote:

> My only concern with using GitHub Pages is that I don't believe it
> currently supports TLS on custom domains.  Since we currently have TLS
> enabled, along with HTTP Strict Transport Security (as we should), such
> a configuration literally wouldn't work[0].  I think it's important that
> we continue to serve HTTPS only, anyway.

I agree we should continue to serve HTTPS. The usual solution for our
use case is to stick a CDN like Cloudflare in front of GitHub Pages (and
I think we'd want to do that anyway for performance).

I haven't done it, but there are various guides. Here's the one from
Cloudflare:

  https://blog.cloudflare.com/secure-and-fast-github-pages-with-cloudflare/

> I agree that a static site is the way to go from a maintenance
> perspective, though.  Jekyll does support Asciidoctor with a plugin,
> just not on GitHub Pages, so it would theoretically be possible to build
> the site as one big unit if we did it that way.  I've played around with
> that plugin, so I'm happy to provide guidance if we want to do that.

We already massage the data coming from Git (and from the Pro Git books)
a bit before and after feeding it to asciidoctor. So I always assumed
that any static site would involve some import steps for those things,
and we'd commit the intermediate product into the repository.

-Peff

Re: [PATCH] gc: ignore old gc.log files

[Jeff King]

On Wed, Feb 08, 2017 at 09:02:22PM -0500, David Turner wrote:

> The intent of automatic gc is to have a git repository be relatively
> low-maintenance from a server-operator perspective.  Of course, large
> operators like GitHub will need a more complicated management strategy,
> but for ordinary usage, git should just work.
> 
> In this commit, git learns to ignore gc.log files which are older than
> (by default) one day old.  It also learns about a config, gc.maxLogAge
> to manage this.
> 
> So git should never get itself into a state where it refuses to do any
> maintenance, just because at some point some piece of the maintenance
> didn't make progress.  That might still happen (e.g. because the repo
> is corrupt), but at the very least it won't be because Git is too dumb
> to try again.

Sounds like a good goal and approach.

> +gc.maxLogAge::
> + If the file gc.log exists, then `git gc --auto` won't run
> + unless that file is more than maxLogAge seconds old.  Default
> + is 86400, one day.

For other time-based config, we use approxidate with a relative time,
like "1 day ago". I think it would make sense for this to match, as it
makes the config a little more readable.

You can follow the prune_expire example which is right below your new
config variable in all of the hunks of your patch. Though I think
ultimately that isn't parsed inside gc, so you'd eventually look at how
"prune --expire" is handled (which I think is via parse_expiry_date()).

> @@ -291,8 +293,19 @@ static int report_last_gc_error(void)
>  {
>   struct strbuf sb = STRBUF_INIT;
>   int ret;
> + struct stat st;
> + const char *gc_log_path = git_path("gc.log");

I usually try to avoid assigning the result of git_path(), because it's
a static buffer. In this case it's fine, because you don't call any
complex calls during the lifetime of the variable. But I consider
assigning to be a bad practice.

Using git_pathdup() or git_path_buf() is safer (but you do need to
remember to free() as appropriate).

Speaking of leaks, I think report_last_gc_error() will always leak the
strbuf contents when it is non-empty.

> + if (stat(gc_log_path, )) {
> + if (errno == ENOENT)
> + return 0;
> + return error(_("Can't read %s"), gc_log_path);
> + }

I'm not quite clear on the life-cycle of this log file.

I think the current code works like this:

  - if a non-empty gc.log exists, we bail

  - when we daemonize, we take a lock via gc.log.lock

  - if we wrote anything to the lockfile log, then we move it into place
(essentially blocking further auto-gc)

  - otherwise, we rollback the lockfile and leave gc.log untouched

That leaves a few quirks with your new strategy:

  - if our new run was unsuccessful (as judged by having non-empty log
output), we'd probably want to overwrite the old logfile with our
new log. Following the current-code logic we do, which is good.

  - if our new run is successful (empty log), we'll leave the old,
crufty log in place. Probably process_log_file() should unlink() the
original gc.log while holding the lock.

And here are a few things I noticed that I think aren't caused by your
patch, but are in the same area and might be worth examining:

  - we're not very atomic. After a day, two simultaneous auto-gc's might
both ignore the gc.log file and continue to run. I don't think it
matters, though. One of them will win the race to pick up
gc.log.lock, and the other will immediately fail.

  - It looks like we clear the gc.log file only under another detached
auto-gc. It seems like manually running a successful "git gc" should
clear it, too.

  - We block further gc only based on the presence of data on stderr
from the sub-programs. But _not_ if the sub-programs fail. So a
program silently exiting with code 128 will stop further gc
processing, but not prevent another auto-gc from running. Which
is...weird. Maybe this can't happen, though, because I think we
write our own error() in such cases, which makes such a failure
inherently non-silent.

> + if (time(NULL) - st.st_mtime > gc_max_log_age_seconds)
> + return 0;

Hmm. What happens if the file has a timestamp in the future due to clock
skew? As long as time_t is signed, I think it's OK, but if it wraps, it
does the wrong thing here. You could rearrange the subtraction to handle
this. But I think if you start using approxidate, it will give you an
a cutoff time, and you can just do:

  if (st.st_mtime < gc_log_expiration)
return 0; /* too old to care about */

> - ret = strbuf_read_file(, git_path("gc.log"), 0);
> + ret = strbuf_read_file(, gc_log_path, 0);

I would have written this as an open(), followed by an fstat() on the
file we opened, and then finally reading the contents if it's fresh
enough. But I'm not sure if that level of atomicity matters. We're not
doing any of this under lock.

-Peff

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Jeff King]

On Mon, Feb 06, 2017 at 02:34:08PM -0800, Junio C Hamano wrote:

> * sk/parse-remote-cleanup (2017-02-06) 1 commit
>   (merged to 'next' on 2017-02-06 at 6ec89f72d5)
>  + parse-remote: remove reference to unused op_prep
> 
>  Code clean-up.
> 
>  Will merge to 'master'.

Hrm. Are the functions in git-parse-remote.sh part of the public API?
That is, do we expect third-party scripts to do:

  . "$(git rev-parse --exec)/git-parse-remote.sh
  error_on_missing_default_upstream "$a" "$b" "$c" "$d"

? If so, then they may be surprised by the change in function signature.

I generally think of git-sh-setup as the one that external scripts would
use. There _is_ a manpage for git-parse-remote, but it doesn't list any
functions. So maybe they're all fair game for changing?

I just didn't see any discussion of this in the original patch thread,
so I wanted to make sure we were making that decision consciously, and
not accidentally. :)

-Peff

Re: git-scm.com status report

[Eric Wong]

Jeff King  wrote:
> I agree we should continue to serve HTTPS. The usual solution for our
> use case is to stick a CDN like Cloudflare in front of GitHub Pages (and
> I think we'd want to do that anyway for performance).
> 
> I haven't done it, but there are various guides. Here's the one from
> Cloudflare:
> 
>   https://blog.cloudflare.com/secure-and-fast-github-pages-with-cloudflare/

AFAIK, there's a way to keep CloudFlare stuff accessible to Tor
users.  If there is, please do so.  As a Tor user, it's been
disappointing to see so much of the web walled off by CAPTCHAs.

Thank you.

Heck, maybe a .onion mirror would be nice :)
I wouldn't mind hosting one myself if it's static.

Re: Bug with fixup and autosquash

[Ashutosh Bapat]

On Thu, Feb 9, 2017 at 4:25 AM, Junio C Hamano  wrote:
> Ashutosh Bapat  writes:
>
>> I have been using git rebase heavily these days and seem to have found a bug.
>>
>> If there are two commit messages which have same prefix e.g.
>> yy This is prefix
>> xx This is prefix and message
>>
>> xx comitted before yy
>>
>> Now I commit a fixup to yy using git commit --fixup yy
>> zz fixup! This is prefix
>>
>> When I run git rebase -i --autosquash, the script it shows me looks like
>> pick xx This is prefix and message
>> fixup zz fixup! This is prefix
>> pick yy This is prefix
>>
>> I think the correct order is
>> pick xx This is prefix and message
>> pick yy This is prefix
>> fixup zz fixup! This is prefix
>>
>> Is that right?
>
> Because "commit" pretends as if it took the exact commit object name
> to be fixed up (after all, it accepts "yy" that is a name of the
> commit object), it would be nice if the fixup is applied to that
> exact commit, even if you had many commits that share exactly the
> same title (i.e. not just shared prefix).
>
> Unfortunately, "rebase -i --autosquash" reorders the entries by
> identifying the commit by its title, and it goes with prefix match
> so that fix-up commits created without using --fixup option but
> manually records the title's prefix substring can also work.
>
> We could argue that the logic should notice that there is one exact
> match and another non-exact prefix match and favor the former, and
> certainly such a change would make your made-up example (you didn't
> actually have a commit whose title is literally "This is prefix")
> above work better.

This is a "real life situation" I ended up with yesterday. I can't
share exact commit message here so redacted it, keeping its essence. I
executed git rebase -i --autosquash and got into a conflict merge
since the fixup was applied after wrong commit. I had to execute git
rebase --abort and git rebase -i --autosquash. That's when I noticed
what's gone wrong.

>
> But I am not sure if adding such heuristics would really help in
> general.  It would not help those whose commits are mostly titled
> ultra-vaguely, like "fix", "bugfix", "docfix", etc.
>
> Another possibility is to teach "commit --fixup" to cast exact
> commit object name in stone.  That certainly would solve your
> immediate problem, but it has a grave negative impact when the user
> rebases the same topic many times (which happens often).
>
> For example, I may have a series of commits A and B, notice that A
> could be done a bit better and have "fixup A" on top, build a new
> commit C on it, and then realize that the next step (i.e. D) would
> need support from a newer codebase than where I started (i.e. A^).
>
> At that point, I would have a 4-commit series (A, B, "fixup A", and
> C), and I would rebase them on top of something newer.  I am
> undecided if that "fixup A" is really an improvement or unnecessary,
> when I do this, but I do know that I want to build the series on top
> of a different commit.  So I do rebase without --autosquash (I would
> probably rebase without --interactive for this one).
>
> Then I keep working and add a new commit D on top.  After all that,
> I would have a more-or-less completed series and would be ready to
> re-assess the whole series.  I perhaps decide that "fixup A" is
> really an improvement.  And then I would "rebase -i" to squash the
> fix-up into A.
>
> But notice that at this point, what we are calling A has different
> object name than the original A the fixup was written for, because
> we rebased once on top of a newer codebase.  That commit can still
> be identified by its title, but not with its original commit object
> name.
>
> I think that is why "commit --fixup " turns the commit
> object name (your "yy") into a string (your "This is prefix")
> and that is a reasonable design decision [*1*].
>
> So from that point of view, if we were to address your issue, it
> should happen in "rebase -i --autosquash" side, not "commit --fixup"
> side.

I agree with this analysis.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Re: [PATCH] gc: ignore old gc.log files

[Eric Wong]

David Turner  wrote:
> + echo fleem> .git/gc.log &&

A minor nit:

echo fleem >.git/gc.log &&

Otherwise, it's good to know there's attention paid to this
issue.  I've been ignoring cron mails from my mirrors for too
long :x  Thanks.

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Junio C Hamano]

Jeff King  writes:

> On Mon, Feb 06, 2017 at 02:34:08PM -0800, Junio C Hamano wrote:
>
>> * sk/parse-remote-cleanup (2017-02-06) 1 commit
>>   (merged to 'next' on 2017-02-06 at 6ec89f72d5)
>>  + parse-remote: remove reference to unused op_prep
>> 
>>  Code clean-up.
>> 
>>  Will merge to 'master'.
>
> Hrm. Are the functions in git-parse-remote.sh part of the public API?
> That is, do we expect third-party scripts to do:
>
>   . "$(git rev-parse --exec)/git-parse-remote.sh
>   error_on_missing_default_upstream "$a" "$b" "$c" "$d"
>
> ? If so, then they may be surprised by the change in function signature.
>
> I generally think of git-sh-setup as the one that external scripts would
> use. There _is_ a manpage for git-parse-remote, but it doesn't list any
> functions. So maybe they're all fair game for changing?
>
> I just didn't see any discussion of this in the original patch thread,
> so I wanted to make sure we were making that decision consciously, and
> not accidentally. :)

Ummm, yes, I admit that this was accidental.  I didn't really think
of parse-remote as an externally visible and supported interface,
but users have tendency to break our expectations, so, I dunno.

Re: [PATCH v2 2/2] grep: use '/' delimiter for paths

[Junio C Hamano]

Jeff King  writes:

>   master:a:a:a:a:a:a:a:a:a:a:a
>
> I think there are 2^(n-1) possible paths (each colon can be a real colon
> or a slash). Though I guess if you walk the trees as you go, you only
> have to examine at most "n" paths to find the first-level tree, and then
> at most "n-1" paths at the second level, and so on.
>
> Unless you really do have ambiguous trees, in which case you have to
> walk down multiple paths.
>
> It certainly would not be the first combinatoric explosion you can
> convince Git to perform. But it does seem like a lot of complication for
> something as simple as path lookups.

That is true, and we may want to avoid the implementation complexity
of the backtracking name resolution.  If you are on the other hand
worried about the runtime cost, it will be an issue to begin with
only for those who do "git grep -e pattern HEAD:t/perf", which is an
unnatural way to do "git grep -e pattern HEAD -- t/perf", and the
output from the latter won't have such an issue, so...

Re: [PATCH v2 2/2] grep: use '/' delimiter for paths

[Jeff King]

On Tue, Feb 07, 2017 at 12:24:30PM -0800, Junio C Hamano wrote:

> Having said that, I actually think "make it more convenient" without
> making anything incorrect would be to teach the revision parser to
> understand
> 
>  
> as an extended SHA-1 expression to name the blob or the tree at that
> path in the tree-ish, e.g. if we can make the revision parser to
> take this
> 
> master:Documentation:git.txt

So here I was wondering what happens when you have a filename with a
colon in it, but then...

> to be able to show the same thing as well.  You'd need to backtrack
> the parsing (e.g. attempt to find "Documentation:git.txt" in
> "master", fail to find any, then fall back to find "git.txt" in
> "master:Documentation", find one, and be happy, or something like
> that), and define how to resolve potential ambiguity (e.g. there may
> indeed be "Documentation:git.txt" and "Documentation/git.txt" in the
> tree-ish "master"), though.

...you obviously did think of that. Backtracking sounds pretty nasty,
though. What's the time complexity of parsing:

  master:a:a:a:a:a:a:a:a:a:a:a

I think there are 2^(n-1) possible paths (each colon can be a real colon
or a slash). Though I guess if you walk the trees as you go, you only
have to examine at most "n" paths to find the first-level tree, and then
at most "n-1" paths at the second level, and so on.

Unless you really do have ambiguous trees, in which case you have to
walk down multiple paths.

It certainly would not be the first combinatoric explosion you can
convince Git to perform. But it does seem like a lot of complication for
something as simple as path lookups.

-Peff

[RFC-PATCHv2] submodules: add a background story

[Stefan Beller]

Just like gitmodules(5), gitattributes(5), gitcredentials(7),
gitnamespaces(7), gittutorial(7), we'd like to provide some background
on submodules, which is not specific to the `submodule` command, but
elaborates on the background and its intended usage.

Add gitsubmodules(7), that explains the states, structure and usage of
submodules.

Signed-off-by: Stefan Beller 
---

This would replace the last patch of  sb/submodule-doc, though it's still
RFC. In this revision I took care of the technical details (i.e. proper
formatting, spelling), and only slight rewording of the text.

The main issue persists; see bottom of the patch:

  SAMPLE WORKFLOWS (RFC/TODO)
  ---
  
  Do we need
  
  * an opinionated way to check for a specific state of a submodule
  * (submodule helper to be plumbing?)
  * expose the design mistake of having the (name->path) mapping inside the
working tree, i.e. never remove a name from the submodule config even when
the submodule doesn't exist any more.

Any opinion on these would be welcome!
Thanks,
Stefan

 Documentation/Makefile  |   1 +
 Documentation/git-submodule.txt |  36 ++--
 Documentation/gitsubmodules.txt | 194 
 3 files changed, 200 insertions(+), 31 deletions(-)
 create mode 100644 Documentation/gitsubmodules.txt

diff --git a/Documentation/Makefile b/Documentation/Makefile
index b43d66eae6..325c4735a7 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -31,6 +31,7 @@ MAN7_TXT += giteveryday.txt
 MAN7_TXT += gitglossary.txt
 MAN7_TXT += gitnamespaces.txt
 MAN7_TXT += gitrevisions.txt
+MAN7_TXT += gitsubmodules.txt
 MAN7_TXT += gittutorial-2.txt
 MAN7_TXT += gittutorial.txt
 MAN7_TXT += gitworkflows.txt
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 4a4cede144..d38aa2d53a 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -24,37 +24,7 @@ DESCRIPTION
 ---
 Inspects, updates and manages submodules.
 
-A submodule allows you to keep another Git repository in a subdirectory
-of your repository. The other repository has its own history, which does not
-interfere with the history of the current repository. This can be used to
-have external dependencies such as third party libraries for example.
-
-When cloning or pulling a repository containing submodules however,
-these will not be checked out by default; the 'init' and 'update'
-subcommands will maintain submodules checked out and at
-appropriate revision in your working tree.
-
-Submodules are composed from a so-called `gitlink` tree entry
-in the main repository that refers to a particular commit object
-within the inner repository that is completely separate.
-A record in the `.gitmodules` (see linkgit:gitmodules[5]) file at the
-root of the source tree assigns a logical name to the submodule and
-describes the default URL the submodule shall be cloned from.
-The logical name can be used for overriding this URL within your
-local repository configuration (see 'submodule init').
-
-Submodules are not to be confused with remotes, which are other
-repositories of the same project; submodules are meant for
-different projects you would like to make part of your source tree,
-while the history of the two projects still stays completely
-independent and you cannot modify the contents of the submodule
-from within the main project.
-If you want to merge the project histories and want to treat the
-aggregated whole as a single project from then on, you may want to
-add a remote for the other project and use the 'subtree' merge strategy,
-instead of treating the other project as a submodule. Directories
-that come from both projects can be cloned and checked out as a whole
-if you choose to go that route.
+For more information about submodules, see linkgit:gitsubmodules[5]
 
 COMMANDS
 
@@ -420,6 +390,10 @@ This file should be formatted in the same way as 
`$GIT_DIR/config`. The key
 to each submodule url is "submodule.$name.url".  See linkgit:gitmodules[5]
 for details.
 
+SEE ALSO
+
+linkgit:gitsubmodules[1], linkgit:gitmodules[1].
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt
new file mode 100644
index 00..3369d55ae9
--- /dev/null
+++ b/Documentation/gitsubmodules.txt
@@ -0,0 +1,194 @@
+gitsubmodules(7)
+
+
+NAME
+
+gitsubmodules - information about submodules
+
+SYNOPSIS
+
+$GIT_DIR/config, .gitmodules
+
+--
+git submodule
+--
+
+DESCRIPTION
+---
+
+A submodule allows you to keep another Git repository in a subdirectory
+of your repository. The other repository has its own history, which does not
+interfere with the history of the current repository. This can be used to
+have external dependencies such as third party libraries for example.
+
+Submodules are 

Re: [PATCH v2 2/2] grep: use '/' delimiter for paths

[Jeff King]

On Wed, Feb 08, 2017 at 09:14:17PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> >   master:a:a:a:a:a:a:a:a:a:a:a
> >
> > I think there are 2^(n-1) possible paths (each colon can be a real colon
> > or a slash). Though I guess if you walk the trees as you go, you only
> > have to examine at most "n" paths to find the first-level tree, and then
> > at most "n-1" paths at the second level, and so on.
> >
> > Unless you really do have ambiguous trees, in which case you have to
> > walk down multiple paths.
> >
> > It certainly would not be the first combinatoric explosion you can
> > convince Git to perform. But it does seem like a lot of complication for
> > something as simple as path lookups.
> 
> That is true, and we may want to avoid the implementation complexity
> of the backtracking name resolution.  If you are on the other hand
> worried about the runtime cost, it will be an issue to begin with
> only for those who do "git grep -e pattern HEAD:t/perf", which is an
> unnatural way to do "git grep -e pattern HEAD -- t/perf", and the
> output from the latter won't have such an issue, so...

I thought your point was to move it into the get_sha1() parser (so that
while the form is only generated by "git grep", it can be accepted by
any git command). That exposes it in a lot of places, including ones
which are network accessible to things like gitweb (or GitHub, of
course, which is my concern).

Even without the runtime cost, though, I think the general complexity
makes it an ugly path to go down (e.g., handling ambiguous cases). I
wouldn't want to have to write the documentation for it. :)

(I _do_ think Stefan's proposed direction is worth it simply because the
result is easier to read, but I agree the whole thing can be avoided by
using pathspecs, as you've noted).

-Peff

Re: [PATCH 1/2] refs.c: add resolve_ref_submodule()

[Michael Haggerty]

On 02/08/2017 12:31 PM, Nguyễn Thái Ngọc Duy wrote:
> This is basically the extended version of resolve_gitlink_ref() where we
> have access to more info from the underlying resolve_ref_recursively() call.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  refs.c | 20 ++--
>  refs.h |  3 +++
>  2 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/refs.c b/refs.c
> index cd36b64ed9..02e35d83f3 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1325,18 +1325,18 @@ const char *resolve_ref_unsafe(const char *refname, 
> int resolve_flags,
>  resolve_flags, sha1, flags);
>  }
>  
> -int resolve_gitlink_ref(const char *submodule, const char *refname,
> - unsigned char *sha1)
> +const char *resolve_ref_submodule(const char *submodule, const char *refname,
> +   int resolve_flags, unsigned char *sha1,
> +   int *flags)
>  {
>   size_t len = strlen(submodule);
>   struct ref_store *refs;
> - int flags;
>  
>   while (len && submodule[len - 1] == '/')
>   len--;
>  
>   if (!len)
> - return -1;
> + return NULL;
>  
>   if (submodule[len]) {
>   /* We need to strip off one or more trailing slashes */
> @@ -1349,9 +1349,17 @@ int resolve_gitlink_ref(const char *submodule, const 
> char *refname,
>   }
>  
>   if (!refs)
> - return -1;
> + return NULL;
> +
> + return resolve_ref_recursively(refs, refname, resolve_flags, sha1, 
> flags);
> +}
> +
> +int resolve_gitlink_ref(const char *submodule, const char *refname,
> + unsigned char *sha1)
> +{
> + int flags;
>  
> - if (!resolve_ref_recursively(refs, refname, 0, sha1, ) ||
> + if (!resolve_ref_submodule(submodule, refname, 0, sha1, ) ||
>   is_null_sha1(sha1))
>   return -1;
>   return 0;
> diff --git a/refs.h b/refs.h
> index 9fbff90e79..74542468d8 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -88,6 +88,9 @@ int peel_ref(const char *refname, unsigned char *sha1);
>   */
>  int resolve_gitlink_ref(const char *submodule, const char *refname,
>   unsigned char *sha1);
> +const char *resolve_ref_submodule(const char *submodule, const char *refname,
> +   int resolve_flags, unsigned char *sha1,
> +   int *flags);

This function is the analog of resolve_ref_unsafe(); i.e., it returns a
pointer to either a static buffer or a pointer into the refname
argument. Therefore, I think it should have "unsafe" in its name. And/or
maybe there should be a safe version of the function analogous to
resolve_refdup().

Moreover, this function has inherited the code for stripping trailing
slashes from the submodule name. I have the feeling that this is a wart,
not a feature, and that it would be sad to see it spread. How about
moving the slash-stripping code to resolve_gitlink_ref() and making
resolve_ref_submodule() assume that its submodule name is already clean?

It would be nice to have a docstring here.

I also have some higher-level concerns about the approach of this patch
series, which I'll write about in a comment to patch 2/2.

Michael

Re: [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree

[Michael Haggerty]

On 02/08/2017 12:31 PM, Nguyễn Thái Ngọc Duy wrote:
> The patch itself is relatively simple: manual parsing code is replaced
> with a call to resolve_ref_submodule(). The manual parsing code must die
> because only refs/files-backend.c should do that. Why submodule here is
> a more interesting question.
> 
> From an outside look, any .git/worktrees/foo is seen as a "normal"
> repository. You can set GIT_DIR to it and have access to everything,
> even shared things that are not literally inside that directory, like
> object db or shared refs.
> 
> On top of that, linked worktrees point to those directories with ".git"
> files. These two make a linked worktree's path "X" a "submodule" (*) (**)
> because X/.git is a file that points to a repository somewhere.
> 
> As such, we can just linked worktree's path as a submodule. We just need
> to make sure they are unique because they are used to lookup submodule
> refs store.
> 
> Main worktree is a a bit trickier. If we stand at a linked worktree, we
> may still need to peek into main worktree's HEAD, for example. We can
> treat main worktree's path as submodule as well since git_path_submodule()
> can tolerate ".git" dirs, in addition to ".git" files.
> 
> The constraint is, if main worktree is X, then the git repo must be at
> X/.git. If the user separates .git repo far away and tell git to point
> to it via GIT_DIR or something else, then the "main worktree as submodule"
> trick fails. Within multiple worktree context, I think we can limit
> support to "standard" layout, at least for now.
> 
> (*) The differences in sharing object database and refs between
> submodules and linked worktrees don't really matter in this context.
> 
> (**) At this point, we may want to rename refs *_submodule API to
> something more neutral, maybe s/_submodule/_remote/

It is unquestionably a good goal to avoid parsing references outside of
`refs/files-backend.c`. But I'm not a fan of this approach.

There are two meanings of the concept of a "ref store", and I think this
change muddles them:

1. The references that happen to be *physically* stored in a particular
   location, for example the `refs/bisect/*` references in a worktree.

2. The references that *logically* should be considered part of a
   particular repository. This might require stitching together
   references from multiple sources, for example `HEAD` and
   `refs/bisect` from a worktree's own directory with other
   references from the main repository.

Either of these concepts can be implemented via the `ref_store` abstraction.

The `ref_store` for a submodule should represent the references
logically visible from the submodule. The main program shouldn't care
whether the references are stored in a single physical location or
spread across multiple locations (for example, if the submodule were
itself a linked worktree).

The `ref_store` that you want here for a worktree is not the worktree's
*logical* `ref_store`. You want the worktree's *physical* `ref_store`.
Mixing logical and physical reference stores together is a bad idea
(even if we were willing to ignore the fact that worktrees are not
submodules in the accepted sense of the word).

The point of my `submodule-hash` branch [1] was to separate these
concepts better by breaking the current 1:1 connection between
`ref_store`s and submodules. This would allow `ref_store`s to be created
for other purposes, such as to represent worktree refs. If you want the
*logical* `ref_store` for a submodule, you access it through the
`submodule_ref_stores` table. If you want the *physical* `ref_store` for
a worktree, you should access it through a different table.

I think the best solution would be to expose the concept of `ref_store`
in the public refs API. Then users of submodules would essentially do

struct ref_store *refs = get_submodule_refs(submodule_path);
... resolve_ref_recursively(refs, refname, 0, sha1, ) ...
... for_each_ref(refs, fn, cb_data) ...

whereas for a worktree you'd have to look up the `ref_store` instance
somewhere else (or maybe keep it as part of some worktree structure, if
there is one) but you would use it via the same API.

Michael

[1] https://github.com/mhagger/git

> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  branch.c   |  3 +-
>  worktree.c | 99 
> +++---
>  worktree.h |  2 +-
>  3 files changed, 27 insertions(+), 77 deletions(-)
> 
> diff --git a/branch.c b/branch.c
> index b955d4f316..db5843718f 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -354,7 +354,8 @@ int replace_each_worktree_head_symref(const char *oldref, 
> const char *newref)
>   for (i = 0; worktrees[i]; i++) {
>   if (worktrees[i]->is_detached)
>   continue;
> - if (strcmp(oldref, worktrees[i]->head_ref))
> + if (worktrees[i]->head_ref &&
> + strcmp(oldref, worktrees[i]->head_ref))
>   

Re: "disabling bitmap writing, as some objects are not being packed"?

[David Turner]

On Wed, 2017-02-08 at 13:45 +0700, Duy Nguyen wrote:
> On Wed, Feb 8, 2017 at 8:03 AM, David Turner  wrote:
> > On Sat, 2016-12-17 at 14:50 +0700, Duy Nguyen wrote:
> >> And we can't grep for fatal errors anyway. The problem that led to
> >> 329e6e8794 was this line
> >>
> >> warning: There are too many unreachable loose objects; run 'git
> >> prune' to remove them.
> >>
> >> which is not fatal.
> >
> > So, speaking of that message, I noticed that our git servers were
> > getting slow again and found that message in gc.log.
> >
> > I propose to make auto gc not write that message either. Any objections?
> 
> Does that really help? auto gc would run more often, but unreachable
> loose objects are still present and potentially make your servers
> slow? Should these servers run periodic and explicit gc/prune?

At least pack files wouldn't accumulate.  This is the major cause of
slowdown, since each pack file must be checked for each object.

(And, also, maybe those unreachable loose objects are too new to get
gc'd, but if we retry next week, we'll gc them).

Re: "disabling bitmap writing, as some objects are not being packed"?

[Duy Nguyen]

On Wed, Feb 8, 2017 at 3:24 PM, David Turner  wrote:
> On Wed, 2017-02-08 at 13:45 +0700, Duy Nguyen wrote:
>> On Wed, Feb 8, 2017 at 8:03 AM, David Turner  wrote:
>> > On Sat, 2016-12-17 at 14:50 +0700, Duy Nguyen wrote:
>> >> And we can't grep for fatal errors anyway. The problem that led to
>> >> 329e6e8794 was this line
>> >>
>> >> warning: There are too many unreachable loose objects; run 'git
>> >> prune' to remove them.
>> >>
>> >> which is not fatal.
>> >
>> > So, speaking of that message, I noticed that our git servers were
>> > getting slow again and found that message in gc.log.
>> >
>> > I propose to make auto gc not write that message either. Any objections?
>>
>> Does that really help? auto gc would run more often, but unreachable
>> loose objects are still present and potentially make your servers
>> slow? Should these servers run periodic and explicit gc/prune?
>
> At least pack files wouldn't accumulate.  This is the major cause of
> slowdown, since each pack file must be checked for each object.
>
> (And, also, maybe those unreachable loose objects are too new to get
> gc'd, but if we retry next week, we'll gc them).

I was about to suggest a config option that lets you run auto gc
unconditionally, which, I think, is better than suppressing the
message. Then I found gc.autoDetach. If you set it to false globally,
I think you'll get the behavior you want.

On second thought, perhaps gc.autoDetach should default to false if
there's no tty, since its main point it to stop breaking interactive
usage. That would make the server side happy (no tty there).
-- 
Duy

[PATCH] refs-internal.c: make files_log_ref_write() static

[Nguyễn Thái Ngọc Duy]

Created in 5f3c3a4e6f (files_log_ref_write: new function - 2015-11-10)
but probably never used outside refs-internal.c

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 refs/files-backend.c | 3 +++
 refs/refs-internal.h | 4 
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index c041d4ba21..6a0bf94bf1 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -165,6 +165,9 @@ static struct ref_entry *create_dir_entry(struct 
files_ref_store *ref_store,
  const char *dirname, size_t len,
  int incomplete);
 static void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry);
+static int files_log_ref_write(const char *refname, const unsigned char 
*old_sha1,
+  const unsigned char *new_sha1, const char *msg,
+  int flags, struct strbuf *err);
 
 static struct ref_dir *get_ref_dir(struct ref_entry *entry)
 {
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 25444cf5b0..4c9215d33f 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -220,10 +220,6 @@ struct ref_transaction {
enum ref_transaction_state state;
 };
 
-int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
-   const unsigned char *new_sha1, const char *msg,
-   int flags, struct strbuf *err);
-
 /*
  * Check for entries in extras that are within the specified
  * directory, where dirname is a reference directory name including
-- 
2.11.0.157.gd943d85

Bug with fixup and autosquash

[Ashutosh Bapat]

Hi Git-developers,
First of all thanks for developing this wonderful scm tool. It's sooo versatile.

I have been using git rebase heavily these days and seem to have found a bug.

If there are two commit messages which have same prefix e.g.
yy This is prefix
xx This is prefix and message

xx comitted before yy

Now I commit a fixup to yy using git commit --fixup yy
zz fixup! This is prefix

When I run git rebase -i --autosquash, the script it shows me looks like
pick xx This is prefix and message
fixup zz fixup! This is prefix
pick yy This is prefix

I think the correct order is
pick xx This is prefix and message
pick yy This is prefix
fixup zz fixup! This is prefix

Is that right?

I am using
[ubuntu]git --version
git version 1.7.9.5

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Re: Non-zero exit code without error

[Christian Couder]

Hi,

On Tue, Feb 7, 2017 at 12:27 PM, Serdar Sahin  wrote:
> Hi,
>
> When we execute the following lines, the exit code is 1, but it is
> unclear what is the reason of this exit code. Do you have any idea?
>
> git clone --mirror --depth 50 --no-single-branch
> g...@github.hede.com:Casual/hodo-server.git

First, could you tell us the git version you are using on the client
and on the server, and if this a new problem with newer versions?
Also is the repos accessible publicly or is it possible to reproduce
on another repo?
And what happens using other protocols like HTTP/S?

> Cloning into bare repository 'hodo-server.git'...
> remote: Counting objects: 3371, done.
> remote: Compressing objects: 100% (1219/1219), done.
> remote: Total 3371 (delta 2344), reused 2971 (delta 2098), pack-reused 0
> Receiving objects: 100% (3371/3371), 56.77 MiB | 2.18 MiB/s, done.
> Resolving deltas: 100% (2344/2344), done.
>
> echo $?
> 0
>
> cd hodo-server.git/
>
> GIT_CURL_VERBOSE=1 GIT_TRACE=1  git fetch --depth 50 origin
> cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee
> 14:12:35.215889 git.c:350   trace: built-in: git 'fetch'
> '--depth' '50' 'origin' 'cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee'
> 14:12:35.217273 run-command.c:336   trace: run_command: 'ssh'
> 'g...@github.hede.com' 'git-upload-pack '\''Casual/hodo-server.git'\'''
> 14:12:37.301122 run-command.c:336   trace: run_command: 'gc' '--auto'
> 14:12:37.301866 exec_cmd.c:189  trace: exec: 'git' 'gc' '--auto'
> 14:12:37.304473 git.c:350   trace: built-in: git 'gc' '--auto'
>
> echo $?
> 1

What happens if you just run 'git gc --auto' after that?

Re: subtree merging fails

[Stavros Liaskos]

@Samuel Lijin
I tried now and I get:
"merge: branch_name
- not something we can merge".
Maybe that is something easy to fix but currently I am using a
workaround script so I am not putting any more effort at this at the
moment.

@David Aguilar
That's true but the trailing slash is already there. This commands
looks promising. An update would be GREAT!

FYI that's the script I am using to address this problem:

#!/bin/bash

function initial {
  if git remote | grep -q lisa_remote
  then
echo "Subtree delete & update"
git checkout lisa_branch
git pull
git checkout master
git merge --squash -s subtree --no-commit lisa_branch
git merge --squash --allow-unrelated-histories -s subtree
--no-commit lisa_branch
  else
echo "Add subtree"
git remote add lisa_remote git@:lisa/lisa.git
git fetch lisa_remote
git checkout -b lisa_branch lisa_remote/master
git checkout master
git read-tree --prefix=lisaSubTree/ -u lisa_branch
gitrm
git rm --cached -r lisaSubTree/.gitignore
git rm --cached -r lisaSubTree/*
  fi
}
initial



On Tue, Feb 7, 2017 at 7:44 PM, David Aguilar  wrote:
> On Tue, Feb 07, 2017 at 08:59:06AM -0600, Samuel Lijin wrote:
>> Have you tried using (without -s subtree) -X subtree=path/to/add/subtree/at?
>>
>> From the man page:
>>
>>   subtree[=]
>>This option is a more advanced form of subtree
>> strategy, where the strategy
>>makes a guess on how two trees must be shifted to match
>> with each other when
>>merging. Instead, the specified path is prefixed (or
>> stripped from the
>>beginning) to make the shape of two trees to match.
>
> I'm not 100% certain, but it's highly likely that the subtree=
> argument needs to include a trailing slash "/" in the prefix,
> otherwise files will be named e.g. "fooREADME" instead of
> "foo/README" when prefix=foo.
>
> These days I would steer users towards the "git-subtree" command in
> contrib/ so that users don't need to deal with these details.  It
> handles all of this stuff for you.
>
> https://github.com/git/git/blob/master/contrib/subtree/git-subtree.txt
>
> https://github.com/git/git/tree/master/contrib/subtree
>
> Updating the progit book to also mention git-subtree, in addition to the
> low-level methods, would probably be a good user-centric change.
> --
> David

Re: Non-zero exit code without error

[Serdar Sahin]

Hi Christian,


We are using a private repo (Github Enterprise). Let me give you the
details you requested.


On Git Server: git version 2.6.5.1574.g5e6a493

On my client: git version 2.10.1 (Apple Git-78)


I’ve tried to reproduce it with public repos, but couldn’t do so. If I
could get an error/log output, that would be sufficient.


I am also including the full output below. (also git gc)


MacOSX:test serdar$ git clone --mirror --depth 50 --no-single-branch
g...@git.privateserver.com:Casual/code_repository.git

Cloning into bare repository 'code_repository.git'...

remote: Counting objects: 3362, done.

remote: Compressing objects: 100% (1214/1214), done.

remote: Total 3362 (delta 2335), reused 2968 (delta 2094), pack-reused 0

Receiving objects: 100% (3362/3362), 56.77 MiB | 1.83 MiB/s, done.

Resolving deltas: 100% (2335/2335), done.

MacOSX:test serdar$ cd code_repository.git/

MacOSX:code_repository.git serdar$ GIT_CURL_VERBOSE=1 GIT_TRACE=1  git
fetch --depth 50 origin cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee

13:23:15.648337 git.c:350   trace: built-in: git 'fetch'
'--depth' '50' 'origin' 'cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee'

13:23:15.651127 run-command.c:336   trace: run_command: 'ssh'
'g...@git.privateserver.com' 'git-upload-pack
'\''Casual/code_repository.git'\'''

13:23:17.750015 run-command.c:336   trace: run_command: 'gc' '--auto'

13:23:17.750829 exec_cmd.c:189  trace: exec: 'git' 'gc' '--auto'

13:23:17.753983 git.c:350   trace: built-in: git 'gc' '--auto'

MacOSX:code_repository.git serdar$ echo $?

1

MacOSX:code_repository.git serdar$ GIT_CURL_VERBOSE=1 GIT_TRACE=1 git gc --auto

13:23:45.899038 git.c:350   trace: built-in: git 'gc' '--auto'

MacOSX:code_repository.git serdar$ echo $?

0

MacOSX:code_repository.git serdar$

On Wed, Feb 8, 2017 at 1:07 PM, Christian Couder
 wrote:
> Hi,
>
> On Tue, Feb 7, 2017 at 12:27 PM, Serdar Sahin  wrote:
>> Hi,
>>
>> When we execute the following lines, the exit code is 1, but it is
>> unclear what is the reason of this exit code. Do you have any idea?
>>
>> git clone --mirror --depth 50 --no-single-branch
>> g...@github.hede.com:Casual/hodo-server.git
>
> First, could you tell us the git version you are using on the client
> and on the server, and if this a new problem with newer versions?
> Also is the repos accessible publicly or is it possible to reproduce
> on another repo?
> And what happens using other protocols like HTTP/S?
>
>> Cloning into bare repository 'hodo-server.git'...
>> remote: Counting objects: 3371, done.
>> remote: Compressing objects: 100% (1219/1219), done.
>> remote: Total 3371 (delta 2344), reused 2971 (delta 2098), pack-reused 0
>> Receiving objects: 100% (3371/3371), 56.77 MiB | 2.18 MiB/s, done.
>> Resolving deltas: 100% (2344/2344), done.
>>
>> echo $?
>> 0
>>
>> cd hodo-server.git/
>>
>> GIT_CURL_VERBOSE=1 GIT_TRACE=1  git fetch --depth 50 origin
>> cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee
>> 14:12:35.215889 git.c:350   trace: built-in: git 'fetch'
>> '--depth' '50' 'origin' 'cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee'
>> 14:12:35.217273 run-command.c:336   trace: run_command: 'ssh'
>> 'g...@github.hede.com' 'git-upload-pack '\''Casual/hodo-server.git'\'''
>> 14:12:37.301122 run-command.c:336   trace: run_command: 'gc' '--auto'
>> 14:12:37.301866 exec_cmd.c:189  trace: exec: 'git' 'gc' '--auto'
>> 14:12:37.304473 git.c:350   trace: built-in: git 'gc' '--auto'
>>
>> echo $?
>> 1
>
> What happens if you just run 'git gc --auto' after that?

[PATCH 1/2] refs.c: add resolve_ref_submodule()

[Nguyễn Thái Ngọc Duy]

This is basically the extended version of resolve_gitlink_ref() where we
have access to more info from the underlying resolve_ref_recursively() call.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 refs.c | 20 ++--
 refs.h |  3 +++
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/refs.c b/refs.c
index cd36b64ed9..02e35d83f3 100644
--- a/refs.c
+++ b/refs.c
@@ -1325,18 +1325,18 @@ const char *resolve_ref_unsafe(const char *refname, int 
resolve_flags,
   resolve_flags, sha1, flags);
 }
 
-int resolve_gitlink_ref(const char *submodule, const char *refname,
-   unsigned char *sha1)
+const char *resolve_ref_submodule(const char *submodule, const char *refname,
+ int resolve_flags, unsigned char *sha1,
+ int *flags)
 {
size_t len = strlen(submodule);
struct ref_store *refs;
-   int flags;
 
while (len && submodule[len - 1] == '/')
len--;
 
if (!len)
-   return -1;
+   return NULL;
 
if (submodule[len]) {
/* We need to strip off one or more trailing slashes */
@@ -1349,9 +1349,17 @@ int resolve_gitlink_ref(const char *submodule, const 
char *refname,
}
 
if (!refs)
-   return -1;
+   return NULL;
+
+   return resolve_ref_recursively(refs, refname, resolve_flags, sha1, 
flags);
+}
+
+int resolve_gitlink_ref(const char *submodule, const char *refname,
+   unsigned char *sha1)
+{
+   int flags;
 
-   if (!resolve_ref_recursively(refs, refname, 0, sha1, ) ||
+   if (!resolve_ref_submodule(submodule, refname, 0, sha1, ) ||
is_null_sha1(sha1))
return -1;
return 0;
diff --git a/refs.h b/refs.h
index 9fbff90e79..74542468d8 100644
--- a/refs.h
+++ b/refs.h
@@ -88,6 +88,9 @@ int peel_ref(const char *refname, unsigned char *sha1);
  */
 int resolve_gitlink_ref(const char *submodule, const char *refname,
unsigned char *sha1);
+const char *resolve_ref_submodule(const char *submodule, const char *refname,
+ int resolve_flags, unsigned char *sha1,
+ int *flags);
 
 /*
  * Return true iff abbrev_name is a possible abbreviation for
-- 
2.11.0.157.gd943d85

[PATCH 2/2] worktree: demonstrate a half-working workaround for auto-gc

[Johannes Schindelin]

While worktrees are marked "experimental", there is really no alternative
to using them when trying to work on multiple patch series in parallel.

Sadly, there is still a bug that causes irretrievable data loss caused by
worktree's incomplete handling of reflogs (and of the multiple indices of
the various worktrees), as this developer can testify a dozen times over.

In the --auto case, we can install a hook that runs before-hand,
accumulates all the worktree-specific refs and reflogs and installs them
into a very special reflog in the common refs namespace. The only
purpose of this stunt is to let gc pick up on those refs and reflogs, of
course, and *not* ignore them.

Unfortunately, this still does not address the "git gc" case, but
hopefully a real fix will be here some time soon.

Also, if there are simply too many loose objects for Git's liking, it will
kick off auto-gc (including the hook, which takes a couple of seconds to
run) that subsequently only says that it won't do anything, so the next
Git operation will kick off another auto-gc (including the multi-second
hook run).

Needless to say: this patch is not actually intended for inclusion, but
instead tries to demonstrate the pain and the dear need for a real bug fix
(and not another 8 months of buggy Git).

So here is hoping to a timely fix. Cheers.

Signed-off-by: Johannes Schindelin 
---
 t/t6500-gc.sh | 26 +-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index e24a4fb611..2f1e52825d 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -67,6 +67,30 @@ test_expect_success 'auto gc with too many loose objects 
does not attempt to cre
test_line_count = 2 new # There is one new pack and its .idx
 '
 
+test_expect_success 'install pre-auto-gc hook for worktrees' '
+   mkdir -p .git/hooks &&
+   write_script .git/hooks/pre-auto-gc <<-\EOF
+   echo "Preserving refs/reflogs of worktrees" >&2 &&
+   dir="$(git rev-parse --git-common-dir)" &&
+   refsdir="$dir/logs/refs" &&
+   rm -f "$refsdir"/preserve &&
+   ident="$(GIT_COMMITTER_DATE= git var GIT_COMMITTER_IDENT)" &&
+   (
+   find "$dir"/logs "$dir"/worktrees/*/logs \
+   -type f -exec cat {} \; |
+   cut -d" " -f1
+   find "$dir"/HEAD "$dir"/worktrees/*/HEAD "$dir"/refs \
+   "$dir"/worktrees/*/refs -type f -exec cat {} \; |
+   grep -v "^ref:"
+   ) 2>/dev/null |
+   sort |
+   uniq |
+   sed "s/.*/& & $identdummy/" >"$dir"/preserve &&
+   mkdir -p "$refsdir" &&
+   mv "$dir"/preserve "$refsdir"/
+   EOF
+'
+
 trigger_auto_gc () {
# This is unfortunately very, very ugly
gdir="$(git rev-parse --git-common-dir)" &&
@@ -76,7 +100,7 @@ trigger_auto_gc () {
git -c gc.auto=1 -c gc.pruneexpire=now -c gc.autodetach=0 gc --auto
 }
 
-test_expect_failure 'gc respects refs/reflogs in all worktrees' '
+test_expect_success 'gc respects refs/reflogs in all worktrees' '
test_commit something &&
git worktree add worktree &&
(
-- 
2.11.1.windows.1

[PATCH 1/2] worktree: demonstrate data lost to auto-gc

[Johannes Schindelin]

When gc --auto is called in the presence of worktrees, it fails to take
*all* reflogs into account when trying to retain reachable objects, and as
a consequence data integrity goes pretty much to hell.

This patch provides a test case in the hopes that this bug gets fixed,
after an initial attempt has stalled for eight months already.

Signed-off-by: Johannes Schindelin 
---
 t/t6500-gc.sh | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 1762dfa6a3..e24a4fb611 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -67,5 +67,44 @@ test_expect_success 'auto gc with too many loose objects 
does not attempt to cre
test_line_count = 2 new # There is one new pack and its .idx
 '
 
+trigger_auto_gc () {
+   # This is unfortunately very, very ugly
+   gdir="$(git rev-parse --git-common-dir)" &&
+   mkdir -p "$gdir"/objects/17 &&
+   touch "$gdir"/objects/17/17171717171717171717171717171717171717 &&
+   touch "$gdir"/objects/17/17171717171717171717171717171717171718 &&
+   git -c gc.auto=1 -c gc.pruneexpire=now -c gc.autodetach=0 gc --auto
+}
+
+test_expect_failure 'gc respects refs/reflogs in all worktrees' '
+   test_commit something &&
+   git worktree add worktree &&
+   (
+   cd worktree &&
+   git checkout --detach &&
+   # avoid implicit tagging of test_commit
+   echo 1 >something.t &&
+   test_tick &&
+   git commit -m worktree-reflog something.t &&
+   git rev-parse --verify HEAD >../commit-reflog &&
+   echo 2 >something.t &&
+   test_tick &&
+   git commit -m worktree-ref something.t &&
+   git rev-parse --verify HEAD >../commit-ref
+   ) &&
+   trigger_auto_gc &&
+   git rev-parse --verify $(cat commit-ref)^{commit} &&
+   git rev-parse --verify $(cat commit-reflog)^{commit} &&
+
+   # Now, add a reflog in the top-level dir and verify that `git gc` in
+   # the worktree does not purge that
+   git checkout --detach &&
+   echo 3 >something.t &&
+   test_tick &&
+   git commit -m commondir-reflog something.t &&
+   git rev-parse --verify HEAD >commondir-reflog &&
+   (cd worktree && trigger_auto_gc) &&
+   git rev-parse --verify $(cat commondir-reflog)^{commit}
+'
 
 test_done
-- 
2.11.1.windows.1