Re: [PATCH] packfile: Correct zlib buffer handling

[Duy Nguyen]

On Sun, May 27, 2018 at 1:57 AM, Junio C Hamano  wrote:
> Duy Nguyen  writes:
>
>> On Sat, May 26, 2018 at 12:56 AM, Jeremy Linton  
>> wrote:
>>> @@ -1416,7 +1416,7 @@ static void *unpack_compressed_entry(struct 
>>> packed_git *p,
>>> return NULL;
>>> memset(, 0, sizeof(stream));
>>> stream.next_out = buffer;
>>> -   stream.avail_out = size + 1;
>>> +   stream.avail_out = size;
>>
>> You may want to include in your commit message a reference to
>> 39eea7bdd9 (Fix incorrect error check while reading deflated pack data
>> - 2009-10-21) which adds this plus one with a fascinating story
>> behind.
>
> A bit puzzled---are you saying that this recent patch breaks the old
> fix and must be done in some other way?

No. I actually wanted to answer that question when I tried to track
down the commit that adds " + 1" but I did not spend enough time to
understand the old problem. I guess your puzzle means you didn't think
it would break anything, which is good.
-- 
Duy

Re: [PATCH] t990X: use '.git/objects' as 'deep inside .git' path

[Michael Haggerty]

On Sat, May 26, 2018 at 8:47 AM, Christian Couder
 wrote:
> Tests t9902-completion.sh and t9903-bash-prompt.sh each have tests
> that check what happens when we are "in the '.git' directory" and
> when we are "deep inside the '.git' directory".
>
> To test the case when we are "deep inside the '.git' directory" the
> test scripts used to perform a `cd .git/refs/heads`.
>
> As there are plans to implement other ref storage systems, let's
> use '.git/objects' instead of '.git/refs/heads' as the "deep inside
> the '.git' directory" path.

Seems reasonable to me. +1.

Michael

Re: 2.17.0 Regression When Adding Patches Without Whitespace In Initial Column

[Jeff Felchner]

Todd, it looks like that may very well be the same issue.  And it looks like 
it's planning on being fixed in the next release?  Would that be 2.17.1 since 
it's a regression?

> On 2018 May 26, at 16:07, Todd Zullinger  wrote:
> 
> Hi Jeff,
> 
> Jeff Felchner wrote:
>> Ever since 2.17.0, when saving a patch (using add --patch
>> but probably other ways as well), if the whitespace is
>> removed from the initial column, the patch doesn't apply.
> 
> This sounds a bit like the issue discussed in this thread a
> few weeks ago:
> 
> https://public-inbox.org/git/e8aedc6b-5b3e-cfb2-be9d-971bfd9ad...@talktalk.net/
> 
> But I didn't download or watch the video, so I'm not positive.
> 
>> Full walkthrough (including comparison with 2.16.3) in the
>> video attached to this link:
>> 
>> https://www.dropbox.com/s/s1ophi4mwmf9ogv/git-add-patch-whitespace-bug.mp4?dl=1
> 
> -- 
> Todd
> ~~
> Everybody knows how to raise children, except the people who have
> them.
>-- P.J. O'Rourke
> 

Re: [RFC] git gc "--prune=now" semantics considered harmful

[Linus Torvalds]

On Sat, May 26, 2018 at 4:31 PM Junio C Hamano  wrote:

> *That* is something I don't do.  After all, I am fully aware that I
> have started end-of-day ritual by that time, so I won't even look at
> a new patch (or a pull request for that matter).

Sounds like you're more organized about the end-of-day ritual than I am.
For me the gc is not quite so structured.

> I however have to wonder if there are opposite "oops" end-user
> operation we also need to worry about, i.e. we are doing a large-ish
> fetch, and get bored and run a gc fron another terminal.  Perhaps
> *that* is a bit too stupid to worry about?  Auto-gc deliberately
> does not use 'now' because it wants to leave a grace period to avoid
> exactly that kind of race.

For me, a "pull" never takes that long.  Sure, any manual merging and the
writing of the commit message might take a while, but it's "foreground"
activity for me, I'd not start a gc in the middle of it.

So at least to me, doing "git fsck --full" and "git gc --prune=now" are
somewhat special because they take a while and tend to be background things
that I "start and forget" about (the same way I sometimes start and forget
a kernel build).

Which is why that current "git gc --prune=now" behavior seems a bit
dangerous.

Linus

Re: [PATCH 2/5] t6036, t6042: use test_line_count instead of wc -l

[Junio C Hamano]

Elijah Newren  writes:

>> I'd expect that a reader of the commit who cares enough to bother to
>> wonder by looking at the patch and seeing that 2 became 3 would know
>> why already.  And a reader of the resulting file would not know that
>> the 3 used to be 2, and won't be helped by "we used to count to 2,
>> now we have 'out' also counted" that much, especially in the commit
>> log message.  What would help the latter would be to name which
>> three paths we expect to see in the comment (or test against the
>> exact list of paths, instead of using test_line_count).
>>
>>> An alternative to consider would be to add a .gitignore file in the
>>> initial commit to ignore 'out', then the number of untracked files
>>> don't have to be adjusted.
>>
>> I think that is a preferred solution that we've used in ls-files and
>> status tests successfully.
>
> ...except that if we add a .gitignore to each initial commit (we use
> test_create_repo for nearly every test to keep them separable meaning
> we'd have to do this many times), then four lines above we have to
> adjust the number of expected tracked files.  And, for it to work,
> we'd have to add an --exclude-standard flag to ls-files -o.

Yeah, unless the original planned to use the .gitignore mechanism,
converting it to use it now will become noisy.

Re: [PATCH] packfile: Correct zlib buffer handling

[Junio C Hamano]

Duy Nguyen  writes:

> On Sat, May 26, 2018 at 12:56 AM, Jeremy Linton  
> wrote:
>> @@ -1416,7 +1416,7 @@ static void *unpack_compressed_entry(struct packed_git 
>> *p,
>> return NULL;
>> memset(, 0, sizeof(stream));
>> stream.next_out = buffer;
>> -   stream.avail_out = size + 1;
>> +   stream.avail_out = size;
>
> You may want to include in your commit message a reference to
> 39eea7bdd9 (Fix incorrect error check while reading deflated pack data
> - 2009-10-21) which adds this plus one with a fascinating story
> behind.

A bit puzzled---are you saying that this recent patch breaks the old
fix and must be done in some other way?

Re: [RFC] git gc "--prune=now" semantics considered harmful

[Junio C Hamano]

Linus Torvalds  writes:

> Soes my use pattern of "git gc --prune=now" make sense? Maybe not. But 
> it's what I've gotten used to, and it's at least not entirely insane.

FWIW, my end-of-day ritual is to do repack -a -d -f with a large
window and a small depth, followed by prune, which boils down to
about the same.  So I'd hope that is not entirely insane.  I however
do not think I bother with an explicit --expire=now when running the
prune, though.

In any case, that makes two of us, and the suggested patch protects
only one of the two ;-)

> But at least once now, I've done that "git gc" at the end of the day, and 
> a new pull request comes in, so I do the "git pull" without even thinking 
> about the fact that "git gc" is still running.

*That* is something I don't do.  After all, I am fully aware that I
have started end-of-day ritual by that time, so I won't even look at
a new patch (or a pull request for that matter).

> So I actually would much prefer that foir git gc, "--prune=now" means
>
>  (a) "now"
>
>  (b) now at the _start_ of the "git gc" operation, not the time at
>  the _end_ of the operation when we've already spent a minute or
>  two doing repacking and are now doing the final pruning.
>
> anyway, with that explanation in mind, I'm appending a patch that is 
> pretty small and does that. It's a bit hacky, but I think it still makes 
> sense.
>
> Comments?

Closing the possiblity of racing a running "gc" and new object
creation like the above generally makes sense, I would think,
whether the creation is due to 'pull/fetch', 'add', or even 'push'.

I however have to wonder if there are opposite "oops" end-user
operation we also need to worry about, i.e. we are doing a large-ish
fetch, and get bored and run a gc fron another terminal.  Perhaps
*that* is a bit too stupid to worry about?  Auto-gc deliberately
does not use 'now' because it wants to leave a grace period to avoid
exactly that kind of race.

> diff --git a/builtin/gc.c b/builtin/gc.c
> index c4777b244..98368c8b5 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -535,8 +535,12 @@ int cmd_gc(int argc, const char **argv, const char 
> *prefix)
>   if (argc > 0)
>   usage_with_options(builtin_gc_usage, builtin_gc_options);
>  
> - if (prune_expire && parse_expiry_date(prune_expire, ))
> - die(_("failed to parse prune expiry value %s"), prune_expire);
> + if (prune_expire) {
> + if (!strcmp(prune_expire, "now"))
> + prune_expire = show_date(time(NULL), 0, 
> DATE_MODE(ISO8601));
> + if (parse_expiry_date(prune_expire, ))
> + die(_("failed to parse prune expiry value %s"), 
> prune_expire);
> + }
>  
>   if (aggressive) {
>   argv_array_push(, "-f");

[RFC] git gc "--prune=now" semantics considered harmful

[Linus Torvalds]

So this is a RFC patch, I'm not sure how much people really care, but I 
find the current behavior of "git gc --prune=now" to be unnecessarily 
dangerous.

There's two issues with it:

 (a) parse_expiry_date() considers "now" to be special, and it actually
 doesn't mean "now" at all, it means "everything".

 (b) the date parsing isn't actually done "now", it's done *after* gc has 
 already run, and we run "git prune --expire". So even if (a) wasn't 
 true, "--prune=now" wouldn't actually mean "now" when the user 
expects it to happen, but "after doing repacking".

I actually think that the "parse_expiry_date()" behavior makes sense 
within the context of "git prune --expire", so I'm not really complaining 
about (a) per se. I just think that what makes sense within the context of 
"git prune" does *not* necessarily make sense within the context of "git 
gc".

Why do I care? I end up doing lots of random things in my local 
repository, and I also end up wanting to keep my repository fairly clean, 
so I tend to do "git gc --prune=now" to just make sure everything is 
packed and I've gotten rid of all the temporary stuff that so often 
happens when doing lots of git merges (which is what I do). 

You won't see those temporary objects for the usual trivial merges, but 
whenever you have a real recursive merge with automated conflict 
resolution, there will be things like those temporary merge-only objects 
for the 3-way base merge state. 

Soes my use pattern of "git gc --prune=now" make sense? Maybe not. But 
it's what I've gotten used to, and it's at least not entirely insane.

But at least once now, I've done that "git gc" at the end of the day, and 
a new pull request comes in, so I do the "git pull" without even thinking 
about the fact that "git gc" is still running.

And then the "--prune=now" behavior is actually really pretty dangerous. 
Because it will prune *all* unreachable objects, even if they are only 
*currently* unreachable because they are in the process of being unpacked 
by the concurrent "git fetch" (and I didn't check - I might just have been 
unlocky, bit I think "git prune" ignores FETCH_HEAD).

So I actually would much prefer that foir git gc, "--prune=now" means

 (a) "now"

 (b) now at the _start_ of the "git gc" operation, not the time at
 the _end_ of the operation when we've already spent a minute or
 two doing repacking and are now doing the final pruning.

anyway, with that explanation in mind, I'm appending a patch that is 
pretty small and does that. It's a bit hacky, but I think it still makes 
sense.

Comments?

Note that this really isn't likely very noticeable on most projects. When 
I do "git gc" on a fairly well-packed repo of git itself, it takes under 
4s for me. So the window for that whole "do git pull at the same time" is 
simply not much of an issue.

For the kernel, "git gc" takes a minute and a half on the same machine 
(again, this is already a packed repo, it can be worse). So there's a much 
bigger window there to do something stupid,

 Linus

---
 builtin/gc.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index c4777b244..98368c8b5 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -535,8 +535,12 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
if (argc > 0)
usage_with_options(builtin_gc_usage, builtin_gc_options);
 
-   if (prune_expire && parse_expiry_date(prune_expire, ))
-   die(_("failed to parse prune expiry value %s"), prune_expire);
+   if (prune_expire) {
+   if (!strcmp(prune_expire, "now"))
+   prune_expire = show_date(time(NULL), 0, 
DATE_MODE(ISO8601));
+   if (parse_expiry_date(prune_expire, ))
+   die(_("failed to parse prune expiry value %s"), 
prune_expire);
+   }
 
if (aggressive) {
argv_array_push(, "-f");

From: Mr James Tan (Urgent & Confidential)

[James Tan]

-- 
-- 
(From: Mr.James Tan  (Urgent & Confidential)

Good Day, Please Read.

My name is Mr.James Tan , I am the Bill and Exchange manager here in Bank
of Africa (BOA) Lome-Togo.West-Africa. I have a business proposal in the
tune of $9.7m, (Nine Million Seven Hundred Thousand Dollars Only) after the
successful transfer, we shall share in ratio of 40% for you and 60% for me.

Should you be interested, please contact me through my private email (
jamestanta...@gmail.com) so we can commence all arrangements and i
will give you
more information on how we would handle this project.


Please treat this business with utmost confidentiality and send me the
Following:

1. Your Full Name:
2. Your Phone Number:..
3. Your Age:
4. Your Sex:...
5. Your Occupations:
6. Your Country and City:

Kind Regards,

Mr.James Tan .
Bill & Exchange manager
(BOA) Bank of Africa.
Lome-Togo.
Email: jamestanta...@gmail.com

Re: 2.17.0 Regression When Adding Patches Without Whitespace In Initial Column

[Todd Zullinger]

Hi Jeff,

Jeff Felchner wrote:
> Ever since 2.17.0, when saving a patch (using add --patch
> but probably other ways as well), if the whitespace is
> removed from the initial column, the patch doesn't apply.

This sounds a bit like the issue discussed in this thread a
few weeks ago:

https://public-inbox.org/git/e8aedc6b-5b3e-cfb2-be9d-971bfd9ad...@talktalk.net/

But I didn't download or watch the video, so I'm not positive.

> Full walkthrough (including comparison with 2.16.3) in the
> video attached to this link:
> 
> https://www.dropbox.com/s/s1ophi4mwmf9ogv/git-add-patch-whitespace-bug.mp4?dl=1

-- 
Todd
~~
Everybody knows how to raise children, except the people who have
them.
-- P.J. O'Rourke

2.17.0 Regression When Adding Patches Without Whitespace In Initial Column

[Jeff Felchner]

Ever since 2.17.0, when saving a patch (using add --patch but probably other 
ways as well), if the whitespace is removed from the initial column, the patch 
doesn't apply.

Full walkthrough (including comparison with 2.16.3) in the video attached to 
this link:

https://www.dropbox.com/s/s1ophi4mwmf9ogv/git-add-patch-whitespace-bug.mp4?dl=1

Re: [PATCH v3 02/20] commit-graph: fix GRAPH_MIN_SIZE

[brian m. carlson]

On Sat, May 26, 2018 at 08:46:09PM +0200, Jakub Narebski wrote:
> One issue: in the future when Git moves to NewHash, it could encounter
> then both commit-graph files using SHA-1 and using NewHash.  What about
> GRPH_OID_LEN then: for one of those it would be incorrect.  Unless it is
> about minimal length of checksum, that is we assume that NewHash would
> be longer than SHA-1, but ten why name it GRAPH_OID_LEN?

My proposal is that whatever we're using in the .git directory is
consistent.  If we're using SHA-1 for objects, then everything is SHA-1.
If we're using NewHash for objects, then all data is stored in NewHash
(except translation tables and such).  Any conversions between SHA-1 and
NewHash require a lookup through the standard techniques.

I agree that here it would be more helpful if it were a reference to
the_hash_algo, and I've applied a patch to my object-id-part14 series to
make that conversion.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH] branch: issue "-l" deprecation warning after pager starts

[Kaartic Sivaraam]

On Friday 25 May 2018 08:10 AM, Jeff King wrote:
> Subject: [PATCH] branch: customize "-l" warning in list mode
> 
> People mistakenly use "git branch -l", thinking that it
> triggers list mode. It doesn't, but the lack of non-option
> arguments in that command does (and the "-l" becomes a
> silent noop).
> > Since afc968e579 (branch: deprecate "-l" option, 2018-03-26)
> 
> we've warned that "-l" is going away. But the warning text
> is primarily aimed at people who _meant_ to use "-l", as in
> "git branch -l foo". People who mistakenly said "git branch
> -l" may be left puzzled.
> 

So, this patch is to improve the user experience of people who use "git
branch -l" for listing and not for the people who forget to give a new
branch name argument for "-l". In that case, this makes sense.

BTW, I hope people don't start wondering why "git branch -d" doesn't
trigger list mode ;-)


> + warning("the '-l' option is an alias for 
> '--create-reflog' and");
> + warning("has no effect in list mode. This option will 
> soon be");
> + warning("removed and you should omit it (or use 
> '--list' instead).");

I suppose s/alias/deprecated alias/ makes the point that '-l' will be
removed more stronger.


-- 
Sivaraam

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v3 02/20] commit-graph: fix GRAPH_MIN_SIZE

[Jakub Narebski]

Derrick Stolee  writes:

> The GRAPH_MIN_SIZE macro should be the smallest size of a parsable
> commit-graph file. However, the minimum number of chunks was wrong.
> It is possible to write a commit-graph file with zero commits, and
> that violates this macro's value.
>
> Rewrite the macro, and use extra macros to better explain the magic
> constants.
>
> Signed-off-by: Derrick Stolee 
> ---
>  commit-graph.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/commit-graph.c b/commit-graph.c
> index a8c337dd77..82295f0975 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -33,10 +33,11 @@
>  
>  #define GRAPH_LAST_EDGE 0x8000
>  
> +#define GRAPH_HEADER_SIZE 8

Nice.

>  #define GRAPH_FANOUT_SIZE (4 * 256)
>  #define GRAPH_CHUNKLOOKUP_WIDTH 12
> -#define GRAPH_MIN_SIZE (5 * GRAPH_CHUNKLOOKUP_WIDTH + GRAPH_FANOUT_SIZE + \
> - GRAPH_OID_LEN + 8)
> +#define GRAPH_MIN_SIZE (GRAPH_HEADER_SIZE + 4 * GRAPH_CHUNKLOOKUP_WIDTH \
> + + GRAPH_FANOUT_SIZE + GRAPH_OID_LEN)

So in this case we have file header (4-byte signature, 1-byte version
number, 1-byte oid/hash version, 1-byte number of chunks, 1-byte
reserved: 4+1+1+1+1 = 8 bytes), chunk lookup: 3 required chunks plus
terminating label = 4 entries, constant-size fanout chunks, and
checksum.  Two remaining required chunks (OID Lookup and Commit Data)
can have length of 0.


One issue: in the future when Git moves to NewHash, it could encounter
then both commit-graph files using SHA-1 and using NewHash.  What about
GRPH_OID_LEN then: for one of those it would be incorrect.  Unless it is
about minimal length of checksum, that is we assume that NewHash would
be longer than SHA-1, but ten why name it GRAPH_OID_LEN?

This may be going too much in the future; there is no need to borrow
trouble now, where we have only SHA-1 supported as OID.  Still...

>  
>  char *get_commit_graph_filename(const char *obj_dir)
>  {

Best,
-- 
Jakub Narębski

Re: [PATCH] branch: issue "-l" deprecation warning after pager starts

[Kaartic Sivaraam]

On Friday 25 May 2018 01:01 AM, Jeff King wrote:
> On Thu, May 24, 2018 at 03:22:14PM -0400, Jeff King wrote:
> 
> Hmm, actually, I suppose the true value of the warning is to help people
> doing "git branch -l foo", and it would still work there. The "more
> extreme" from your suggested patch would only affect "branch -l".
> 
> Still, I think I prefer the gentler version that we get by keeping it as
> a warning even in the latter case.
>

I never wanted to suppress the warning message in the latter case. I
just wanted to avoid listing the branches. Actually the patch I sent in
one the previous threads[1] that avoids listing the branches has the
following behaviour,

$ git branch -l
warning: the '-l' alias for '--create-reflog' is deprecated;
warning: it will be removed in a future version of Git
usage: git branch [] [-r | -a] [--merged | --no-merged]
   or: git branch [] [-l] [-f]  []
   or: git branch [] [-r] (-d | -D) ...
   or: git branch [] (-m | -M) [] 
   or: git branch [] (-c | -C) [] 
   or: git branch [] [-r | -a] [--points-at]
   or: git branch [] [-r | -a] [--format]


So, the warning message isn't lost. It just prevents the listing of
branches.

Wait, maybe I'm misunderstanding what you mean by "warning". You're
probably meaning something related to the way Git exits in both cases.
With your patch "git branch -l" prints a warning, lists the branches and
has an exit status of 0. With my patch it prints the warning, the usage
specifications with exit status 128. In that case, I still don't think
it's bad to turn "git branch -l" into an error now as it's been
incorrect for a long time now and it's not wrong if we correct it now.

Anyways, if you think it mustn't turn into an error now and only in the
next stage, a suggestion follows in another thread.


[1]: https://public-inbox.org/git/1527174618.10589.4.ca...@gmail.com/


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - John Sonmez


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: RFC: New reference iteration paradigm

[Jakub Narebski]

Jeff King  writes:
> On Thu, Mar 31, 2016 at 11:01:44AM -0700, Junio C Hamano wrote:
>> Michael Haggerty  writes:
>> 
>>> the backend now has to implement
>>>
 struct ref_iterator *ref_iterator_begin_fn(const char *submodule,
const char *prefix,
unsigned int flags);

The problem with callbacks, including their lack of composability, is
often solved in high-level languages by using Promises / Futures (or
Channels / Supplies).

But I think in this case implementing a straightforward Iterator pattern
would be a better and simpler solution, especially in C.

>>> The ref_iterator itself has to implement two main methods:
>>>
 int iterator_advance_fn(struct ref_iterator *ref_iterator);
 void iterator_free_fn(struct ref_iterator *ref_iterator);
>>>
>>> A loop over references now looks something like
>>>
 struct ref_iterator *iter = each_ref_in_iterator("refs/tags/");
 while (ref_iterator_advance(iter)) {
 /* code using iter->refname, iter->oid, iter->flags */
 }
>> 
>> We'd want to take advantage of the tree-like organization of the
>> refs (i.e. refs/tags/a and refs/tags/b sit next to each other and
>> they are closer to each other than they are to refs/heads/a) so that
>> a request "I want to iterate only over tags, even though I may have
>> millions of other kinds of refs" can be done with cost that is
>> proportional to how many tags you have.
>> 
>> The current implementation of for_each_tag_ref() that goes down to
>> do_for_each_entry() in files-backend.c has that propertly, and the
>> new iteration mechanism with the above design seems to keep it,
>> which is very nice.
>
> Actually, that is a slight fiction. :)
>
> We traverse only the loose ref directories we need, but we populate the
> entire packed-refs tree in one go. That's fine if you have a reasonable
> number of refs and care about the cold-cache case (where you care a lot
> about hitting directories you don't need to, but reading the entire
> packed-refs file isn't a big deal). But it's really bad if you have an
> 800MB packed-refs file, as looking up one tiny subset of the entries
> wastes a lot of RAM and CPU pulling that into our internal
> representation[1].
>
> At one point I wrote a patch to binary search the packed-refs file, find
> the first "refs/tags/" entry, and then walk linearly through there. What
> stopped me is that the current refs.c code (I guess file-backend.c these
> days) was not happy with me partially filling in the ref_dir structs in
> this "inside out" way.
[...]

Isn't this what reftable - an alternative way of storing refs in Git,
tested by being used by JGit - would solve?  See Christian Couder post
"Implementing reftable in Git"

  
https://public-inbox.org/git/cap8ufd0ppzsjbnxca7ez91vbuatchkq+juwvtd1ihcxzpbj...@mail.gmail.com/t/#u

'Efficient lookup of an entire namespace, such as refs/tags/' is
allegedly one of the objectives of this format.

Regards,
-- 
Jakub Narębski

[GSoC] GSoC with git, week 4

[Alban Gruin]

Hi,

I published my blog post about this week. You can read it here:

https://blog.pa1ch.fr/posts/2018/05/26/en/gsoc2018-week-4.html

All comments are welcome!

Cheers,
Alban

Proposal

[Miss Zeliha Omer Faruk]

Hello

Greetings to you please i have a business proposal for you contact me
for more detailes asap thanks.

Best Regards,
Miss.Zeliha ömer faruk
Esentepe Mahallesi Büyükdere
Caddesi Kristal Kule Binasi
No:215
Sisli - Istanbul, Turkey

[PATCH v2 05/11] fsck: produce camelCase config key names

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

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 fsck.c | 22 ++
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/fsck.c b/fsck.c
index b65ff2d856..ff1547d3c7 100644
--- a/fsck.c
+++ b/fsck.c
@@ -74,14 +74,15 @@ enum fsck_msg_id {
 #undef MSG_ID
 
 #define STR(x) #x
-#define MSG_ID(id, msg_type) { STR(id), NULL, FSCK_##msg_type },
+#define MSG_ID(id, msg_type) { STR(id), NULL, NULL, FSCK_##msg_type },
 static struct {
const char *id_string;
const char *downcased;
+   const char *camelcased;
int msg_type;
 } msg_id_info[FSCK_MSG_MAX + 1] = {
FOREACH_MSG_ID(MSG_ID)
-   { NULL, NULL, -1 }
+   { NULL, NULL, NULL, -1 }
 };
 #undef MSG_ID
 
@@ -105,6 +106,20 @@ static void prepare_msg_ids(void)
else
*(q)++ = tolower(*(p)++);
*q = '\0';
+
+   p = msg_id_info[i].id_string;
+   q = xmalloc(len);
+   msg_id_info[i].camelcased = q;
+   while (*p) {
+   if (*p == '_') {
+   p++;
+   if (*p)
+   *q++ = *p++;
+   } else {
+   *q++ = tolower(*p++);
+   }
+   }
+   *q = '\0';
}
 }
 
@@ -127,9 +142,8 @@ void list_config_fsck_msg_ids(struct string_list *list, 
const char *prefix)
 
prepare_msg_ids();
 
-   /* TODO: we can do better by producing camelCase names */
for (i = 0; i < FSCK_MSG_MAX; i++)
-   list_config_item(list, prefix, msg_id_info[i].downcased);
+   list_config_item(list, prefix, msg_id_info[i].camelcased);
 }
 
 static int fsck_msg_type(enum fsck_msg_id msg_id,
-- 
2.17.0.705.g3525833791

[PATCH v2 11/11] log-tree: allow to customize 'grafted' color

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

Commit 76f5df305b (log: decorate grafted commits with "grafted" -
2011-08-18) lets us decorate grafted commits but I forgot about the
color.decorate.* config.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/config.txt | 3 ++-
 log-tree.c   | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ab641bf5a9..1cc18a828c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1162,7 +1162,8 @@ color.diff.::
 color.decorate.::
Use customized color for 'git log --decorate' output.  `` is one
of `branch`, `remoteBranch`, `tag`, `stash` or `HEAD` for local
-   branches, remote-tracking branches, tags, stash and HEAD, respectively.
+   branches, remote-tracking branches, tags, stash and HEAD, respectively
+   and `grafted` for grafted commits.
 
 color.grep::
When set to `always`, always highlight matches.  When `false` (or
diff --git a/log-tree.c b/log-tree.c
index 284ce0e3c5..d3a43e29cd 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -34,6 +34,7 @@ static const char *color_decorate_slots[] = {
[DECORATION_REF_TAG]= "tag",
[DECORATION_REF_STASH]  = "stash",
[DECORATION_REF_HEAD]   = "HEAD",
+   [DECORATION_GRAFTED]= "grafted",
 };
 
 static const char *decorate_get_color(int decorate_use_color, enum 
decoration_type ix)
-- 
2.17.0.705.g3525833791

[PATCH v2 10/11] completion: support case-insensitive config vars

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

Config variables are case-insensitive but this case/esac construct is
case-sensitive by default. For bash v4, it'll be easy. For platforms
that are stuck with older versions, we need an external command, but
that is not that critical. And where this additional overhead matters
the most is Windows, but luckily Git for Windows ships with Bash v4.

Helped-by: SZEDER Gábor 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 contrib/completion/git-completion.bash | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index efc930c9d1..2cbd14b346 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2151,7 +2151,15 @@ __git_compute_config_vars ()
 
 _git_config ()
 {
-   case "$prev" in
+   local varname
+
+   if [ "${BASH_VERSINFO[0]:-0}" -ge 4 ]; then
+   varname="${prev,,}"
+   else
+   varname="$(echo "$prev" |tr A-Z a-z)"
+   fi
+
+   case "$varname" in
branch.*.remote|branch.*.pushremote)
__gitcomp_nl "$(__git_remotes)"
return
-- 
2.17.0.705.g3525833791

[PATCH v2 03/11] fsck: factor out msg_id_info[] lazy initialization code

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

This array will be used by some other function than parse_msg_id() in
the following commit. Factor out this prep code so it could be called
from that one.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 fsck.c | 40 
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/fsck.c b/fsck.c
index 1722f6cab0..e2a6f33891 100644
--- a/fsck.c
+++ b/fsck.c
@@ -84,26 +84,34 @@ static struct {
 };
 #undef MSG_ID
 
-static int parse_msg_id(const char *text)
+static void prepare_msg_ids(void)
 {
int i;
 
-   if (!msg_id_info[0].downcased) {
-   /* convert id_string to lower case, without underscores. */
-   for (i = 0; i < FSCK_MSG_MAX; i++) {
-   const char *p = msg_id_info[i].id_string;
-   int len = strlen(p);
-   char *q = xmalloc(len);
-
-   msg_id_info[i].downcased = q;
-   while (*p)
-   if (*p == '_')
-   p++;
-   else
-   *(q)++ = tolower(*(p)++);
-   *q = '\0';
-   }
+   if (msg_id_info[0].downcased)
+   return;
+
+   /* convert id_string to lower case, without underscores. */
+   for (i = 0; i < FSCK_MSG_MAX; i++) {
+   const char *p = msg_id_info[i].id_string;
+   int len = strlen(p);
+   char *q = xmalloc(len);
+
+   msg_id_info[i].downcased = q;
+   while (*p)
+   if (*p == '_')
+   p++;
+   else
+   *(q)++ = tolower(*(p)++);
+   *q = '\0';
}
+}
+
+static int parse_msg_id(const char *text)
+{
+   int i;
+
+   prepare_msg_ids();
 
for (i = 0; i < FSCK_MSG_MAX; i++)
if (!strcmp(text, msg_id_info[i].downcased))
-- 
2.17.0.705.g3525833791

[PATCH v2 08/11] completion: drop the hard coded list of config vars

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

The new help option --config-for-completion is a machine friendlier
version of --config where all the placeholders and wildcards are
dropped, leaving only the good, completable prefixes for
git-completion.bash to consume.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/help.c |   9 +-
 contrib/completion/git-completion.bash | 335 +
 help.c |  34 ++-
 help.h |   2 +-
 4 files changed, 49 insertions(+), 331 deletions(-)

diff --git a/builtin/help.c b/builtin/help.c
index ccb206e1d4..8d4f6dd301 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -47,6 +47,7 @@ static struct option builtin_help_options[] = {
OPT_HIDDEN_BOOL(0, "exclude-guides", _guides, N_("exclude 
guides")),
OPT_BOOL('g', "guides", _guides, N_("print list of useful 
guides")),
OPT_BOOL('c', "config", _config, N_("print all configuration 
variable names")),
+   OPT_SET_INT_F(0, "config-for-completion", _config, "", 2, 
PARSE_OPT_HIDDEN),
OPT_SET_INT('m', "man", _format, N_("show man page"), 
HELP_FORMAT_MAN),
OPT_SET_INT('w', "web", _format, N_("show manual in web browser"),
HELP_FORMAT_WEB),
@@ -447,8 +448,14 @@ int cmd_help(int argc, const char **argv, const char 
*prefix)
}
 
if (show_config) {
+   int for_human = show_config == 1;
+
+   if (!for_human) {
+   list_config_help(for_human);
+   return 0;
+   }
setup_pager();
-   list_config_help();
+   list_config_help(for_human);
printf("\n%s\n", _("'git help config' for more information"));
return 0;
}
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 12814e9bbf..1b0c30ed9a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2142,6 +2142,13 @@ __git_config_get_set_variables ()
__git config $config_file --name-only --list
 }
 
+__git_config_vars=
+__git_compute_config_vars ()
+{
+   test -n "$__git_config_vars" ||
+   __git_config_vars="$(git help --config-for-completion | sort | uniq)"
+}
+
 _git_config ()
 {
case "$prev" in
@@ -2300,332 +2307,8 @@ _git_config ()
return
;;
esac
-   __gitcomp "
-   add.ignoreErrors
-   advice.amWorkDir
-   advice.commitBeforeMerge
-   advice.detachedHead
-   advice.implicitIdentity
-   advice.pushAlreadyExists
-   advice.pushFetchFirst
-   advice.pushNeedsForce
-   advice.pushNonFFCurrent
-   advice.pushNonFFMatching
-   advice.pushUpdateRejected
-   advice.resolveConflict
-   advice.rmHints
-   advice.statusHints
-   advice.statusUoption
-   advice.ignoredHook
-   alias.
-   am.keepcr
-   am.threeWay
-   apply.ignorewhitespace
-   apply.whitespace
-   branch.autosetupmerge
-   branch.autosetuprebase
-   browser.
-   clean.requireForce
-   color.branch
-   color.branch.current
-   color.branch.local
-   color.branch.plain
-   color.branch.remote
-   color.decorate.HEAD
-   color.decorate.branch
-   color.decorate.remoteBranch
-   color.decorate.stash
-   color.decorate.tag
-   color.diff
-   color.diff.commit
-   color.diff.frag
-   color.diff.func
-   color.diff.meta
-   color.diff.new
-   color.diff.old
-   color.diff.plain
-   color.diff.whitespace
-   color.grep
-   color.grep.context
-   color.grep.filename
-   color.grep.function
-   color.grep.linenumber
-   color.grep.match
-   color.grep.selected
-   color.grep.separator
-   color.interactive
-   color.interactive.error
-   color.interactive.header
-   color.interactive.help
-   color.interactive.prompt
-   color.pager
-   color.showbranch
-   color.status
-   color.status.added
-   color.status.changed
-   color.status.header
-   color.status.localBranch
-   color.status.nobranch
-   color.status.remoteBranch
-   color.status.unmerged
-   color.status.untracked
-   color.status.updated
-   color.ui
-   commit.cleanup
-  

[PATCH v2 02/11] grep: keep all colors in an array

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

This is more inline with how we handle color slots in other code. It
also allows us to get the list of configurable color slots later.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 grep.c | 106 ++---
 grep.h |  21 +++-
 2 files changed, 62 insertions(+), 65 deletions(-)

diff --git a/grep.c b/grep.c
index 45ec7e636c..d94e2c4aeb 100644
--- a/grep.c
+++ b/grep.c
@@ -13,6 +13,17 @@ static int grep_source_is_binary(struct grep_source *gs);
 
 static struct grep_opt grep_defaults;
 
+static const char *color_grep_slots[] = {
+   [GREP_COLOR_CONTEXT]= "context",
+   [GREP_COLOR_FILENAME]   = "filename",
+   [GREP_COLOR_FUNCTION]   = "function",
+   [GREP_COLOR_LINENO] = "lineNumber",
+   [GREP_COLOR_MATCH_CONTEXT]  = "matchContext",
+   [GREP_COLOR_MATCH_SELECTED] = "matchSelected",
+   [GREP_COLOR_SELECTED]   = "selected",
+   [GREP_COLOR_SEP]= "separator",
+};
+
 static void std_output(struct grep_opt *opt, const void *buf, size_t size)
 {
fwrite(buf, size, 1, stdout);
@@ -42,14 +53,14 @@ void init_grep_defaults(void)
opt->pathname = 1;
opt->max_depth = -1;
opt->pattern_type_option = GREP_PATTERN_TYPE_UNSPECIFIED;
-   color_set(opt->color_context, "");
-   color_set(opt->color_filename, "");
-   color_set(opt->color_function, "");
-   color_set(opt->color_lineno, "");
-   color_set(opt->color_match_context, GIT_COLOR_BOLD_RED);
-   color_set(opt->color_match_selected, GIT_COLOR_BOLD_RED);
-   color_set(opt->color_selected, "");
-   color_set(opt->color_sep, GIT_COLOR_CYAN);
+   color_set(opt->colors[GREP_COLOR_CONTEXT], "");
+   color_set(opt->colors[GREP_COLOR_FILENAME], "");
+   color_set(opt->colors[GREP_COLOR_FUNCTION], "");
+   color_set(opt->colors[GREP_COLOR_LINENO], "");
+   color_set(opt->colors[GREP_COLOR_MATCH_CONTEXT], GIT_COLOR_BOLD_RED);
+   color_set(opt->colors[GREP_COLOR_MATCH_SELECTED], GIT_COLOR_BOLD_RED);
+   color_set(opt->colors[GREP_COLOR_SELECTED], "");
+   color_set(opt->colors[GREP_COLOR_SEP], GIT_COLOR_CYAN);
opt->color = -1;
opt->output = std_output;
 }
@@ -76,7 +87,7 @@ static int parse_pattern_type_arg(const char *opt, const char 
*arg)
 int grep_config(const char *var, const char *value, void *cb)
 {
struct grep_opt *opt = _defaults;
-   char *color = NULL;
+   const char *slot;
 
if (userdiff_config(var, value) < 0)
return -1;
@@ -103,32 +114,18 @@ int grep_config(const char *var, const char *value, void 
*cb)
 
if (!strcmp(var, "color.grep"))
opt->color = git_config_colorbool(var, value);
-   else if (!strcmp(var, "color.grep.context"))
-   color = opt->color_context;
-   else if (!strcmp(var, "color.grep.filename"))
-   color = opt->color_filename;
-   else if (!strcmp(var, "color.grep.function"))
-   color = opt->color_function;
-   else if (!strcmp(var, "color.grep.linenumber"))
-   color = opt->color_lineno;
-   else if (!strcmp(var, "color.grep.matchcontext"))
-   color = opt->color_match_context;
-   else if (!strcmp(var, "color.grep.matchselected"))
-   color = opt->color_match_selected;
-   else if (!strcmp(var, "color.grep.selected"))
-   color = opt->color_selected;
-   else if (!strcmp(var, "color.grep.separator"))
-   color = opt->color_sep;
-   else if (!strcmp(var, "color.grep.match")) {
-   int rc = 0;
-   if (!value)
-   return config_error_nonbool(var);
-   rc |= color_parse(value, opt->color_match_context);
-   rc |= color_parse(value, opt->color_match_selected);
-   return rc;
-   }
-
-   if (color) {
+   if (!strcmp(var, "color.grep.match")) {
+   if (grep_config("color.grep.matchcontext", value, cb) < 0)
+   return -1;
+   if (grep_config("color.grep.matchselected", value, cb) < 0)
+   return -1;
+   } else if (skip_prefix(var, "color.grep.", )) {
+   int i = LOOKUP_CONFIG(color_grep_slots, slot);
+   char *color;
+
+   if (i < 0)
+   return -1;
+   color = opt->colors[i];
if (!value)
return config_error_nonbool(var);
return color_parse(value, color);
@@ -144,6 +141,7 @@ int grep_config(const char *var, const char *value, void 
*cb)
 void grep_init(struct grep_opt *opt, const char *prefix)
 {
struct grep_opt *def = _defaults;
+   int i;
 
memset(opt, 0, sizeof(*opt));
opt->prefix = prefix;
@@ -160,14 +158,8 @@ void grep_init(struct grep_opt *opt, const char *prefix)
opt->relative = 

[PATCH v2 06/11] advice: keep config name in camelCase in advice_config[]

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

For parsing, we don't really need this because the main config parser
will lowercase everything so we can do exact matching. But this array
now is also used for printing in `git help --config`. Keep camelCase
so we have a nice printout.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 advice.c | 42 +-
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/advice.c b/advice.c
index cf6c673ed7..2aca11f45e 100644
--- a/advice.c
+++ b/advice.c
@@ -54,28 +54,28 @@ static struct {
const char *name;
int *preference;
 } advice_config[] = {
-   { "pushupdaterejected", _push_update_rejected },
-   { "pushnonffcurrent", _push_non_ff_current },
-   { "pushnonffmatching", _push_non_ff_matching },
-   { "pushalreadyexists", _push_already_exists },
-   { "pushfetchfirst", _push_fetch_first },
-   { "pushneedsforce", _push_needs_force },
-   { "statushints", _status_hints },
-   { "statusuoption", _status_u_option },
-   { "commitbeforemerge", _commit_before_merge },
-   { "resolveconflict", _resolve_conflict },
-   { "implicitidentity", _implicit_identity },
-   { "detachedhead", _detached_head },
-   { "setupstreamfailure", _set_upstream_failure },
-   { "objectnamewarning", _object_name_warning },
-   { "rmhints", _rm_hints },
-   { "addembeddedrepo", _add_embedded_repo },
-   { "ignoredhook", _ignored_hook },
-   { "waitingforeditor", _waiting_for_editor },
-   { "graftfiledeprecated", _graft_file_deprecated },
+   { "pushUpdateRejected", _push_update_rejected },
+   { "pushNonFFCurrent", _push_non_ff_current },
+   { "pushNonFFMatching", _push_non_ff_matching },
+   { "pushAlreadyExists", _push_already_exists },
+   { "pushFetchFirst", _push_fetch_first },
+   { "pushNeedsForce", _push_needs_force },
+   { "statusHints", _status_hints },
+   { "statusUoption", _status_u_option },
+   { "commitBeforeMerge", _commit_before_merge },
+   { "resolveConflict", _resolve_conflict },
+   { "implicitIdentity", _implicit_identity },
+   { "detachedHead", _detached_head },
+   { "setupStreamFailure", _set_upstream_failure },
+   { "objectNameWarning", _object_name_warning },
+   { "rmHints", _rm_hints },
+   { "addEmbeddedRepo", _add_embedded_repo },
+   { "ignoredHook", _ignored_hook },
+   { "waitingForEditor", _waiting_for_editor },
+   { "graftFileDeprecated", _graft_file_deprecated },
 
/* make this an alias for backward compatibility */
-   { "pushnonfastforward", _push_update_rejected }
+   { "pushNonFastForward", _push_update_rejected }
 };
 
 void advise(const char *advice, ...)
@@ -123,7 +123,7 @@ int git_default_advice_config(const char *var, const char 
*value)
return 0;
 
for (i = 0; i < ARRAY_SIZE(advice_config); i++) {
-   if (strcmp(k, advice_config[i].name))
+   if (strcasecmp(k, advice_config[i].name))
continue;
*advice_config[i].preference = git_config_bool(var, value);
return 0;
-- 
2.17.0.705.g3525833791

[PATCH v2 04/11] help: add --config to list all available config

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

Sometimes it helps to list all available config vars so the user can
search for something they want. The config man page can also be used
but it's harder to search if you want to focus on the variable name,
for example.

This is not the best way to collect the available config since it's
not precise. Ideally we should have a centralized list of config in C
code (pretty much like 'struct option'), but that's a lot more work.
This will do for now.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-help.txt |  5 
 advice.c   |  9 ++
 builtin/branch.c   |  3 ++
 builtin/clean.c|  3 ++
 builtin/commit.c   |  3 ++
 builtin/help.c |  9 ++
 diff.c |  3 ++
 fsck.c | 12 
 generate-cmdlist.sh| 19 +
 grep.c |  3 ++
 help.c | 56 ++
 help.h | 45 +-
 log-tree.c |  3 ++
 13 files changed, 172 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index a40fc38d8b..83d25d825a 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -45,6 +45,11 @@ OPTIONS
When used with `--verbose` print description for all recognized
commands.
 
+-c::
+--config::
+   List all available configuration variables. This is a short
+   summary of the list in linkgit:git-config[1].
+
 -g::
 --guides::
Prints a list of useful guides on the standard output. This
diff --git a/advice.c b/advice.c
index 370a56d054..cf6c673ed7 100644
--- a/advice.c
+++ b/advice.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "config.h"
 #include "color.h"
+#include "help.h"
 
 int advice_push_update_rejected = 1;
 int advice_push_non_ff_current = 1;
@@ -131,6 +132,14 @@ int git_default_advice_config(const char *var, const char 
*value)
return 0;
 }
 
+void list_config_advices(struct string_list *list, const char *prefix)
+{
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(advice_config); i++)
+   list_config_item(list, prefix, advice_config[i].name);
+}
+
 int error_resolve_conflict(const char *me)
 {
if (!strcmp(me, "cherry-pick"))
diff --git a/builtin/branch.c b/builtin/branch.c
index 09232576b6..ddc48ca931 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -22,6 +22,7 @@
 #include "wt-status.h"
 #include "ref-filter.h"
 #include "worktree.h"
+#include "help.h"
 
 static const char * const builtin_branch_usage[] = {
N_("git branch [] [-r | -a] [--merged | --no-merged]"),
@@ -67,6 +68,8 @@ static const char *color_branch_slots[] = {
 static struct string_list output = STRING_LIST_INIT_DUP;
 static unsigned int colopts;
 
+define_list_config_array(color_branch_slots);
+
 static int git_branch_config(const char *var, const char *value, void *cb)
 {
const char *slot_name;
diff --git a/builtin/clean.c b/builtin/clean.c
index 0ccd95e693..ab402c204c 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -16,6 +16,7 @@
 #include "column.h"
 #include "color.h"
 #include "pathspec.h"
+#include "help.h"
 
 static int force = -1; /* unset */
 static int interactive;
@@ -91,6 +92,8 @@ struct menu_stuff {
void *stuff;
 };
 
+define_list_config_array(color_interactive_slots);
+
 static int git_clean_config(const char *var, const char *value, void *cb)
 {
const char *slot_name;
diff --git a/builtin/commit.c b/builtin/commit.c
index 2b43a6c48a..9bcbb0c25c 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -32,6 +32,7 @@
 #include "column.h"
 #include "sequencer.h"
 #include "mailmap.h"
+#include "help.h"
 
 static const char * const builtin_commit_usage[] = {
N_("git commit [] [--] ..."),
@@ -1195,6 +1196,8 @@ static int dry_run_commit(int argc, const char **argv, 
const char *prefix,
return commitable ? 0 : 1;
 }
 
+define_list_config_array_extra(color_status_slots, {"added"});
+
 static int parse_status_slot(const char *slot)
 {
if (!strcasecmp(slot, "added"))
diff --git a/builtin/help.c b/builtin/help.c
index 58e0a5507f..ccb206e1d4 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -37,6 +37,7 @@ static const char *html_path;
 
 static int show_all = 0;
 static int show_guides = 0;
+static int show_config;
 static int verbose;
 static unsigned int colopts;
 static enum help_format help_format = HELP_FORMAT_NONE;
@@ -45,6 +46,7 @@ static struct option builtin_help_options[] = {
OPT_BOOL('a', "all", _all, N_("print all available commands")),
OPT_HIDDEN_BOOL(0, "exclude-guides", _guides, N_("exclude 
guides")),
OPT_BOOL('g', "guides", _guides, N_("print list of useful 
guides")),
+   OPT_BOOL('c', "config", _config, N_("print all configuration 
variable names")),
OPT_SET_INT('m', "man", _format, N_("show man page"), 
HELP_FORMAT_MAN),
OPT_SET_INT('w', 

[PATCH v2 07/11] am: move advice.amWorkDir parsing back to advice.c

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

The only benefit from this move (apart from cleaner code) is that
advice.amWorkDir should now show up in `git help --config`. There
should be no regression since advice config is always read by the
git_default_config().

While at there, use advise() like other code. We now get "hint: "
prefix and the output is stderr instead of stdout (which is also the
reason for the test update because stderr is checked in a following
test and the extra advice can fail it).

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 advice.c  | 2 ++
 advice.h  | 1 +
 builtin/am.c  | 6 +-
 t/t4254-am-corrupt.sh | 2 +-
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/advice.c b/advice.c
index 2aca11f45e..52aa85bdfd 100644
--- a/advice.c
+++ b/advice.c
@@ -17,6 +17,7 @@ int advice_implicit_identity = 1;
 int advice_detached_head = 1;
 int advice_set_upstream_failure = 1;
 int advice_object_name_warning = 1;
+int advice_amworkdir = 1;
 int advice_rm_hints = 1;
 int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
@@ -68,6 +69,7 @@ static struct {
{ "detachedHead", _detached_head },
{ "setupStreamFailure", _set_upstream_failure },
{ "objectNameWarning", _object_name_warning },
+   { "amWorkDir", _amworkdir },
{ "rmHints", _rm_hints },
{ "addEmbeddedRepo", _add_embedded_repo },
{ "ignoredHook", _ignored_hook },
diff --git a/advice.h b/advice.h
index 9f5064e82a..7e9377864f 100644
--- a/advice.h
+++ b/advice.h
@@ -17,6 +17,7 @@ extern int advice_implicit_identity;
 extern int advice_detached_head;
 extern int advice_set_upstream_failure;
 extern int advice_object_name_warning;
+extern int advice_amworkdir;
 extern int advice_rm_hints;
 extern int advice_add_embedded_repo;
 extern int advice_ignored_hook;
diff --git a/builtin/am.c b/builtin/am.c
index aa989e7390..735d016525 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1827,15 +1827,11 @@ static void am_run(struct am_state *state, int resume)
}
 
if (apply_status) {
-   int advice_amworkdir = 1;
-
printf_ln(_("Patch failed at %s %.*s"), msgnum(state),
linelen(state->msg), state->msg);
 
-   git_config_get_bool("advice.amworkdir", 
_amworkdir);
-
if (advice_amworkdir)
-   printf_ln(_("Use 'git am --show-current-patch' 
to see the failed patch"));
+   advise(_("Use 'git am --show-current-patch' to 
see the failed patch"));
 
die_user_resolve(state);
}
diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh
index 168739c721..fd3bdbfe2c 100755
--- a/t/t4254-am-corrupt.sh
+++ b/t/t4254-am-corrupt.sh
@@ -25,7 +25,7 @@ test_expect_success setup '
 #   fatal: unable to write file '(null)' mode 100644: Bad address
 # Also, it had the unwanted side-effect of deleting f.
 test_expect_success 'try to apply corrupted patch' '
-   test_must_fail git am bad-patch.diff 2>actual
+   test_must_fail git -c advice.amWorkDir=false am bad-patch.diff 2>actual
 '
 
 test_expect_success 'compare diagnostic; ensure file is still here' '
-- 
2.17.0.705.g3525833791

[PATCH v2 09/11] completion: keep other config var completion in camelCase

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

The last patch makes "git config " shows camelCase names because
that's what's in the source: config.txt. There are still a couple
manual var completion in this code. Let's make them follow the naming
convention as well.

In theory we could automate this part too because we have the
information. But let's stick to one step at a time and leave this for
later.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 contrib/completion/git-completion.bash | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 1b0c30ed9a..efc930c9d1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2249,20 +2249,20 @@ _git_config ()
;;
branch.*.*)
local pfx="${cur%.*}." cur_="${cur##*.}"
-   __gitcomp "remote pushremote merge mergeoptions rebase" "$pfx" 
"$cur_"
+   __gitcomp "remote pushRemote merge mergeOptions rebase" "$pfx" 
"$cur_"
return
;;
branch.*)
local pfx="${cur%.*}." cur_="${cur#*.}"
__gitcomp_direct "$(__git_heads "$pfx" "$cur_" ".")"
-   __gitcomp_nl_append $'autosetupmerge\nautosetuprebase\n' "$pfx" 
"$cur_"
+   __gitcomp_nl_append $'autoSetupMerge\nautoSetupRebase\n' "$pfx" 
"$cur_"
return
;;
guitool.*.*)
local pfx="${cur%.*}." cur_="${cur##*.}"
__gitcomp "
-   argprompt cmd confirm needsfile noconsole norescan
-   prompt revprompt revunmerged title
+   argPrompt cmd confirm needsFile noConsole noRescan
+   prompt revPrompt revUnmerged title
" "$pfx" "$cur_"
return
;;
@@ -2291,14 +2291,14 @@ _git_config ()
local pfx="${cur%.*}." cur_="${cur##*.}"
__gitcomp "
url proxy fetch push mirror skipDefaultUpdate
-   receivepack uploadpack tagopt pushurl
+   receivepack uploadpack tagOpt pushurl
" "$pfx" "$cur_"
return
;;
remote.*)
local pfx="${cur%.*}." cur_="${cur#*.}"
__gitcomp_nl "$(__git_remotes)" "$pfx" "$cur_" "."
-   __gitcomp_nl_append "pushdefault" "$pfx" "$cur_"
+   __gitcomp_nl_append "pushDefault" "$pfx" "$cur_"
return
;;
url.*.*)
-- 
2.17.0.705.g3525833791

[PATCH v2 00/11] completion: avoid hard coding config var list

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

v2 changes:

- based on 'next'
- C99 is now mentioned in the commit message
- fixed Eric's comments
- the old 8/9 partial case-insensitive support patch is replaced
  with Szeder's version.
  
  Szeder I claimed authorship because I wrote the commit message which
  may not be what you like. If you want to claim it instead, I'll be
  glad to resend.

- There is no levenshtein support yet. But the code is updated to keep
  this config list in memory instead of printf'ing directly to make it
  easier in the future to do that.

Nguyễn Thái Ngọc Duy (11):
  Add and use generic name->id mapping code for color slot parsing
  grep: keep all colors in an array
  fsck: factor out msg_id_info[] lazy initialization code
  help: add --config to list all available config
  fsck: produce camelCase config key names
  advice: keep config name in camelCase in advice_config[]
  am: move advice.amWorkDir parsing back to advice.c
  completion: drop the hard coded list of config vars
  completion: keep other config var completion in camelCase
  completion: support case-insensitive config vars
  log-tree: allow to customize 'grafted' color

 Documentation/config.txt   |   3 +-
 Documentation/git-help.txt |   5 +
 advice.c   |  53 ++--
 advice.h   |   1 +
 builtin/am.c   |   6 +-
 builtin/branch.c   |  29 +-
 builtin/clean.c|  29 +-
 builtin/commit.c   |  36 +--
 builtin/help.c |  16 ++
 config.c   |  13 +
 config.h   |   4 +
 contrib/completion/git-completion.bash | 357 ++---
 diff.c |  56 ++--
 fsck.c |  68 +++--
 generate-cmdlist.sh|  19 ++
 grep.c | 109 
 grep.h |  21 +-
 help.c |  84 ++
 help.h |  45 +++-
 log-tree.c |  37 +--
 t/t4254-am-corrupt.sh  |   2 +-
 21 files changed, 440 insertions(+), 553 deletions(-)

-- 
2.17.0.705.g3525833791

[PATCH v2 01/11] Add and use generic name->id mapping code for color slot parsing

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

Instead of hard coding the name-to-id mapping in C code, keep it in an
array and use a common function to do the parsing. This reduces code
and also allows us to list all possible color slots later.

This starts using C99 designated initializers more for convenience
(the first designated initializers have been introduced in builtin/clean.c
for some time without complaints)

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/branch.c | 28 +
 builtin/clean.c  | 28 +
 builtin/commit.c | 33 ++
 config.c | 13 
 config.h |  4 
 diff.c   | 53 +++-
 log-tree.c   | 35 
 7 files changed, 82 insertions(+), 112 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index f85d3de531..09232576b6 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -55,26 +55,18 @@ enum color_branch {
BRANCH_COLOR_UPSTREAM = 5
 };
 
+static const char *color_branch_slots[] = {
+   [BRANCH_COLOR_RESET]= "reset",
+   [BRANCH_COLOR_PLAIN]= "plain",
+   [BRANCH_COLOR_REMOTE]   = "remote",
+   [BRANCH_COLOR_LOCAL]= "local",
+   [BRANCH_COLOR_CURRENT]  = "current",
+   [BRANCH_COLOR_UPSTREAM] = "upstream",
+};
+
 static struct string_list output = STRING_LIST_INIT_DUP;
 static unsigned int colopts;
 
-static int parse_branch_color_slot(const char *slot)
-{
-   if (!strcasecmp(slot, "plain"))
-   return BRANCH_COLOR_PLAIN;
-   if (!strcasecmp(slot, "reset"))
-   return BRANCH_COLOR_RESET;
-   if (!strcasecmp(slot, "remote"))
-   return BRANCH_COLOR_REMOTE;
-   if (!strcasecmp(slot, "local"))
-   return BRANCH_COLOR_LOCAL;
-   if (!strcasecmp(slot, "current"))
-   return BRANCH_COLOR_CURRENT;
-   if (!strcasecmp(slot, "upstream"))
-   return BRANCH_COLOR_UPSTREAM;
-   return -1;
-}
-
 static int git_branch_config(const char *var, const char *value, void *cb)
 {
const char *slot_name;
@@ -86,7 +78,7 @@ static int git_branch_config(const char *var, const char 
*value, void *cb)
return 0;
}
if (skip_prefix(var, "color.branch.", _name)) {
-   int slot = parse_branch_color_slot(slot_name);
+   int slot = LOOKUP_CONFIG(color_branch_slots, slot_name);
if (slot < 0)
return 0;
if (!value)
diff --git a/builtin/clean.c b/builtin/clean.c
index fad533a0a7..0ccd95e693 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -42,6 +42,15 @@ enum color_clean {
CLEAN_COLOR_ERROR = 5
 };
 
+static const char *color_interactive_slots[] = {
+   [CLEAN_COLOR_ERROR]  = "error",
+   [CLEAN_COLOR_HEADER] = "header",
+   [CLEAN_COLOR_HELP]   = "help",
+   [CLEAN_COLOR_PLAIN]  = "plain",
+   [CLEAN_COLOR_PROMPT] = "prompt",
+   [CLEAN_COLOR_RESET]  = "reset",
+};
+
 static int clean_use_color = -1;
 static char clean_colors[][COLOR_MAXLEN] = {
[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
@@ -82,23 +91,6 @@ struct menu_stuff {
void *stuff;
 };
 
-static int parse_clean_color_slot(const char *var)
-{
-   if (!strcasecmp(var, "reset"))
-   return CLEAN_COLOR_RESET;
-   if (!strcasecmp(var, "plain"))
-   return CLEAN_COLOR_PLAIN;
-   if (!strcasecmp(var, "prompt"))
-   return CLEAN_COLOR_PROMPT;
-   if (!strcasecmp(var, "header"))
-   return CLEAN_COLOR_HEADER;
-   if (!strcasecmp(var, "help"))
-   return CLEAN_COLOR_HELP;
-   if (!strcasecmp(var, "error"))
-   return CLEAN_COLOR_ERROR;
-   return -1;
-}
-
 static int git_clean_config(const char *var, const char *value, void *cb)
 {
const char *slot_name;
@@ -113,7 +105,7 @@ static int git_clean_config(const char *var, const char 
*value, void *cb)
return 0;
}
if (skip_prefix(var, "color.interactive.", _name)) {
-   int slot = parse_clean_color_slot(slot_name);
+   int slot = LOOKUP_CONFIG(color_interactive_slots, slot_name);
if (slot < 0)
return 0;
if (!value)
diff --git a/builtin/commit.c b/builtin/commit.c
index a842fea666..2b43a6c48a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -66,6 +66,18 @@ N_("If you wish to skip this commit, use:\n"
 "Then \"git cherry-pick --continue\" will resume cherry-picking\n"
 "the remaining commits.\n");
 
+static const char *color_status_slots[] = {
+   [WT_STATUS_HEADER]= "header",
+   [WT_STATUS_UPDATED]   = "updated",
+   [WT_STATUS_CHANGED]   = "changed",
+   [WT_STATUS_UNTRACKED] = "untracked",
+   [WT_STATUS_NOBRANCH]  = "noBranch",
+   [WT_STATUS_UNMERGED]  = "unmerged",
+   

Re: [PATCH v3] checkout & worktree: introduce checkout.implicitRemote

[Duy Nguyen]

On Fri, May 25, 2018 at 8:38 PM, Ævar Arnfjörð Bjarmason
 wrote:
>
> On Fri, May 25 2018, Duy Nguyen wrote:
>
>> On Fri, May 25, 2018 at 10:12 AM, Junio C Hamano  wrote:
 +Currently this is used by linkgit:git-checkout[1] when 'git checkout
 +' will checkout the '' branch on another remote,
 +and by linkgit:git-worktree[1] when 'git worktree add' refers to a
 +remote branch. This setting might be used for other checkout-like
 +commands or functionality in the future.
>>>
>>> Hmph, that is an interesting direction.  You foresee that you'll
>>> have a single repository with multiple remotes to grab and share
>>> objects from different people working on the same project, and use
>>> multiple worktrees to work on different branches, yet you are happy
>>> to declare that each worktree is to work with one particular remote?
>>>
>>> We'd need a per-worktree config file to make it work, I guess, or
>>> a three-level checkout.$worktree_id.defaultRemote configuration
>>> variable, perhaps?
>>
>> I still plan to revisit per-worktree config support [1] at some point
>> and finish it off. Or is it decided that we don't need a generic
>> mechanism and will add a new level like this for each config var that
>> is per-worktree? If defaultRemote sets a precedence, then it'll be the
>> way to go from now on or we'll have another mess when some config does
>> "foo.$worktree.bar" while others use per-worktree config files.
>
> What do you think about including worktree in this at this time? I'm not
> very familiar with it and just included it because I worked my way
> backwards from the DWIM function, but I could exclude it if you think
> it's too much trouble, i.e. if you'd like to hold out for some facility
> like the per-worktree config Junio mentioned.

I think Junio was talking about the future. What is currently in the
patch, both code and document, is fine. But if you're going to
implement core.$worktree.defaultRemote at this time, maybe wait a bit.
I could try to resurrect the per-worktree config topic in a month or
so and if it does not work out, then everybody will add new config
vars if they want per-worktree support.
-- 
Duy

[PATCH v2 3/4] t2203: add a test about "diff HEAD" case

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

Previous attempts to fix ita-related diffs breaks this case. To make
sure that does not happen again, add a test to verify the behavior
wrt. ita entries when we diff a worktree and a tree.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 t/t2203-add-intent.sh | 17 +
 1 file changed, 17 insertions(+)

diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 1115347712..546fead6ad 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -245,4 +245,21 @@ test_expect_success 'diff-files/diff-cached shows ita as 
new/not-new files' '
test_cmp expected2 actual2
 '
 
+test_expect_success '"diff HEAD" includes ita as new files' '
+   git reset --hard &&
+   echo new >new-ita &&
+   git add -N new-ita &&
+   git diff HEAD >actual &&
+   cat >expected <<-\EOF &&
+   diff --git a/new-ita b/new-ita
+   new file mode 100644
+   index 000..3e75765
+   --- /dev/null
+   +++ b/new-ita
+   @@ -0,0 +1 @@
+   +new
+   EOF
+   test_cmp expected actual
+'
+
 test_done
-- 
2.17.0.705.g3525833791

[PATCH v2 4/4] apply: add --intent-to-add

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

Similar to 'git reset -N', this option makes 'git apply' automatically
mark new files as intent-to-add so they are visible in the following
'git diff' command and could also be committed with 'git commit -a'.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-apply.txt | 10 +-
 apply.c | 19 ---
 apply.h |  1 +
 t/t2203-add-intent.sh   | 13 +
 4 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index 4ebc3d3271..e3b966c656 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -9,7 +9,7 @@ git-apply - Apply a patch to files and/or to the index
 SYNOPSIS
 
 [verse]
-'git apply' [--stat] [--numstat] [--summary] [--check] [--index] [--3way]
+'git apply' [--stat] [--numstat] [--summary] [--check] [--index | 
--intent-to-add] [--3way]
  [--apply] [--no-add] [--build-fake-ancestor=] [-R | --reverse]
  [--allow-binary-replacement | --binary] [--reject] [-z]
  [-p] [-C] [--inaccurate-eof] [--recount] [--cached]
@@ -74,6 +74,14 @@ OPTIONS
cached data, apply the patch, and store the result in the index
without using the working tree. This implies `--index`.
 
+--intent-to-add::
+   When applying the patch only to the working tree, mark new
+   files to be added to the index later (see `--intent-to-add`
+   option in linkgit:git-add[1]). This option is ignored unless
+   running in a Git repository and `--index` is not specified.
+   Note that `--index` could be implied by other options such
+   as `--cached` or `--3way`.
+
 -3::
 --3way::
When the patch does not apply cleanly, fall back on 3-way merge if
diff --git a/apply.c b/apply.c
index 7e5792c996..899c5cc0ac 100644
--- a/apply.c
+++ b/apply.c
@@ -141,6 +141,8 @@ int check_apply_state(struct apply_state *state, int 
force_apply)
return error(_("--cached outside a repository"));
state->check_index = 1;
}
+   if (state->ita_only && (state->check_index || is_not_gitdir))
+   state->ita_only = 0;
if (state->check_index)
state->unsafe_paths = 0;
 
@@ -4242,7 +4244,7 @@ static void patch_stats(struct apply_state *state, struct 
patch *patch)
 
 static int remove_file(struct apply_state *state, struct patch *patch, int 
rmdir_empty)
 {
-   if (state->update_index) {
+   if (state->update_index && !state->ita_only) {
if (remove_file_from_cache(patch->old_name) < 0)
return error(_("unable to remove %s from index"), 
patch->old_name);
}
@@ -4265,15 +4267,15 @@ static int add_index_file(struct apply_state *state,
int namelen = strlen(path);
unsigned ce_size = cache_entry_size(namelen);
 
-   if (!state->update_index)
-   return 0;
-
ce = xcalloc(1, ce_size);
memcpy(ce->name, path, namelen);
ce->ce_mode = create_ce_mode(mode);
ce->ce_flags = create_ce_flags(0);
ce->ce_namelen = namelen;
-   if (S_ISGITLINK(mode)) {
+   if (state->ita_only) {
+   ce->ce_flags |= CE_INTENT_TO_ADD;
+   set_object_name_for_intent_to_add_entry(ce);
+   } else if (S_ISGITLINK(mode)) {
const char *s;
 
if (!skip_prefix(buf, "Subproject commit ", ) ||
@@ -4465,8 +4467,9 @@ static int create_file(struct apply_state *state, struct 
patch *patch)
 
if (patch->conflicted_threeway)
return add_conflicted_stages_file(state, patch);
-   else
+   else if (state->update_index)
return add_index_file(state, path, mode, buf, size);
+   return 0;
 }
 
 /* phase zero is to remove, phase one is to create */
@@ -4686,7 +4689,7 @@ static int apply_patch(struct apply_state *state,
if (state->whitespace_error && (state->ws_error_action == 
die_on_ws_error))
state->apply = 0;
 
-   state->update_index = state->check_index && state->apply;
+   state->update_index = (state->check_index || state->ita_only) && 
state->apply;
if (state->update_index && !is_lock_file_locked(>lock_file)) {
if (state->index_file)
hold_lock_file_for_update(>lock_file,
@@ -4941,6 +4944,8 @@ int apply_parse_options(int argc, const char **argv,
N_("instead of applying the patch, see if the patch is 
applicable")),
OPT_BOOL(0, "index", >check_index,
N_("make sure the patch is applicable to the current 
index")),
+   OPT_BOOL('N', "intent-to-add", >ita_only,
+   N_("mark new files with `git add --intent-to-add`")),
OPT_BOOL(0, "cached", >cached,
N_("apply a patch without touching the working tree")),
OPT_BOOL_F(0, 

[PATCH v2 1/4] diff: ignore --ita-[in]visible-in-index when diffing worktree-to-tree

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

This option is supposed to fix the diff of "diff-files" (not reporting
ita entries as new files) and "diff-index --cached " ( showing
ita entries as present in the index with empty content) but not
"diff-index ".

When --ita-invisible-in-index is set on "git diff-index ",
unpack_trees() will eventually call oneway_diff() on the ita entry
with the same code flow as "diff-index --cached ". We want to
ignore the ita entry for "diff-index --cached " but not
"diff-index " since the latter will examine and produce a diff
based on worktree entry's (real) content, not ita index entry's
(empty) content.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 diff-lib.c| 8 ++--
 t/t2203-add-intent.sh | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 104f954a25..a9f38eb5a3 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -389,8 +389,12 @@ static void do_oneway_diff(struct unpack_trees_options *o,
struct rev_info *revs = o->unpack_data;
int match_missing, cached;
 
-   /* i-t-a entries do not actually exist in the index */
-   if (revs->diffopt.ita_invisible_in_index &&
+   /*
+* i-t-a entries do not actually exist in the index (if we're
+* looking at its content)
+*/
+   if (o->index_only &&
+   revs->diffopt.ita_invisible_in_index &&
idx && ce_intent_to_add(idx)) {
idx = NULL;
if (!tree)
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 78236dc7d8..3ab07cb404 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -70,7 +70,7 @@ test_expect_success 'i-t-a entry is simply ignored' '
git commit -m second &&
test $(git ls-tree HEAD -- nitfol | wc -l) = 0 &&
test $(git diff --name-only HEAD -- nitfol | wc -l) = 1 &&
-   test $(git diff --name-only --ita-invisible-in-index HEAD -- nitfol | 
wc -l) = 0 &&
+   test $(git diff --name-only --ita-invisible-in-index HEAD -- nitfol | 
wc -l) = 1 &&
test $(git diff --name-only --ita-invisible-in-index -- nitfol | wc -l) 
= 1
 '
 
-- 
2.17.0.705.g3525833791

[PATCH v2 2/4] diff: turn --ita-invisible-in-index on by default

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

Due to the implementation detail of intent-to-add entries, the current
"git diff" (i.e. no treeish or --cached argument) would show the
changes in the i-t-a file, but it does not mark the file as new, while
"diff --cached" would mark the file as new while showing its content
as empty.

 $ git diff | $ diff --cached
|---
 diff --git a/new b/new | diff --git a/new b/new
 index e69de29..5ad28e2 100644  | new file mode 100644
 --- a/new  | index 000..e69de29
 +++ b/new  |
 @@ -0,0 +1 @@  |
 +haha  |

One evidence of the current output being wrong is that, the output
from "git diff" (with ita entries) cannot be applied because it
assumes empty files exist before applying.

Turning on --ita-invisible-in-index [1] [2] would fix this. The result
is "new file" line moving from "git diff --cached" to "git diff".

 $ git diff | $ diff --cached
|---
 diff --git a/new b/new |
 new file mode 100644   |
 index 000..5ad28e2 |
 --- /dev/null  |
 +++ b/new  |
 @@ -0,0 +1 @@  |
 +haha  |

This option is on by default in git-status [1] but we need more fixup
in rename detection code [3]. Luckily we don't need to do anything
else for the rename detection code in diff.c (wt-status.c uses a
customized one).

[1] 425a28e0a4 (diff-lib: allow ita entries treated as "not yet exist
in index" - 2016-10-24)
[2] b42b451919 (diff: add --ita-[in]visible-in-index - 2016-10-24)
[3] bc3dca07f4 (Merge branch 'nd/ita-wt-renames-in-status' - 2018-01-23)

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/diff.c  |  7 +++
 t/t2203-add-intent.sh   | 34 --
 t/t4011-diff-symlink.sh | 10 ++
 3 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index 16bfb22f73..00424c296d 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -352,6 +352,13 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
rev.diffopt.flags.allow_external = 1;
rev.diffopt.flags.allow_textconv = 1;
 
+   /*
+* Default to intent-to-add entries invisible in the
+* index. This makes them show up as new files in diff-files
+* and not at all in diff-cached.
+*/
+   rev.diffopt.ita_invisible_in_index = 1;
+
if (nongit)
die(_("Not a git repository"));
argc = setup_revisions(argc, argv, , NULL);
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 3ab07cb404..1115347712 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -70,8 +70,7 @@ test_expect_success 'i-t-a entry is simply ignored' '
git commit -m second &&
test $(git ls-tree HEAD -- nitfol | wc -l) = 0 &&
test $(git diff --name-only HEAD -- nitfol | wc -l) = 1 &&
-   test $(git diff --name-only --ita-invisible-in-index HEAD -- nitfol | 
wc -l) = 1 &&
-   test $(git diff --name-only --ita-invisible-in-index -- nitfol | wc -l) 
= 1
+   test $(git diff --name-only -- nitfol | wc -l) = 1
 '
 
 test_expect_success 'can commit with an unrelated i-t-a entry in index' '
@@ -99,13 +98,13 @@ test_expect_success 'cache-tree invalidates i-t-a paths' '
 
: >dir/bar &&
git add -N dir/bar &&
-   git diff --cached --name-only >actual &&
+   git diff --name-only >actual &&
echo dir/bar >expect &&
test_cmp expect actual &&
 
git write-tree >/dev/null &&
 
-   git diff --cached --name-only >actual &&
+   git diff --name-only >actual &&
echo dir/bar >expect &&
test_cmp expect actual
 '
@@ -186,7 +185,19 @@ test_expect_success 'rename detection finds the right 
names' '
cat >expected.3 <<-EOF &&
2 .R N... 100644 100644 100644 $hash $hash R100 third   first
EOF
-   test_cmp expected.3 actual.3
+   test_cmp expected.3 actual.3 &&
+
+   git diff --stat >actual.4 &&
+   cat >expected.4 <<-EOF &&
+first => third | 0
+1 file changed, 0 insertions(+), 0 deletions(-)
+   EOF
+   test_cmp expected.4 actual.4 &&
+
+   git diff --cached --stat >actual.5 &&
+   : >expected.5 &&
+   test_cmp expected.5 actual.5
+
)
 '
 
@@ -222,5 +233,16 @@ test_expect_success 'double rename detection in status' '
)
 '
 
-test_done
+test_expect_success 'diff-files/diff-cached shows ita as new/not-new files' '
+   git reset --hard &&
+   echo new >new-ita &&
+   git add -N new-ita &&
+   git diff --summary 

[PATCH v2 0/4] Fix i-t-a entries in git-diff and git-apply

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

v2 first fixes a bug in --ita-invisible-in-index that accidentally
affects diffing a worktree and a tree. This caused a regression when
v1's 1/2 turned this option on by default.

Other than that, 4/4 should address Junio's comments on v1's 2/2.

Nguyễn Thái Ngọc Duy (4):
  diff: ignore --ita-[in]visible-in-index when diffing worktree-to-tree
  diff: turn --ita-invisible-in-index on by default
  t2203: add a test about "diff HEAD" case
  apply: add --intent-to-add

 Documentation/git-apply.txt | 10 +-
 apply.c | 19 +++
 apply.h |  1 +
 builtin/diff.c  |  7 
 diff-lib.c  |  8 +++--
 t/t2203-add-intent.sh   | 64 +
 t/t4011-diff-symlink.sh | 10 +++---
 7 files changed, 99 insertions(+), 20 deletions(-)

-- 
2.17.0.705.g3525833791

[PATCH] upload-pack: reject shallow requests that would return nothing

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

Shallow clones with --shallow-since or --shalow-exclude work by
running rev-list to get all reachable commits, then draw a boundary
between reachable and unreachable and send "shallow" requests based on
that.

The code does miss one corner case: if rev-list returns nothing, we'll
have no border and we'll send no shallow requests back to the client
(i.e. no history cuts). This essentially means a full clone (or a full
branch if the client requests just one branch). One example is the
oldest commit is older than what is specified by --shallow-since.

To avoid this, if rev-list returns nothing, we abort the clone/fetch.
The user could adjust their request (e.g. --shallow-since further back
in the past) and retry.

Another possible option for this case is to fall back to a default
depth (like depth 1). But I don't like too much magic that way because
we may return something unexpected to the user. If they request
"history since 2008" and we return a single depth at 2000, that might
break stuff for them. It is better to tell them that something is
wrong and let them take the best course of action.

Note that we need to die() in get_shallow_commits_by_rev_list()
instead of just checking for empty result from its caller
deepen_by_rev_list() and handling the error there. The reason is,
empty result could be a valid case: if you have commits in year 2013
and you request --shallow-since=year.2000 then you should get a full
clone (i.e. empty result).

Reported-by: Andreas Krey 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 shallow.c |  3 +++
 t/t5500-fetch-pack.sh | 11 +++
 2 files changed, 14 insertions(+)

diff --git a/shallow.c b/shallow.c
index df4d44ea7a..44fdca1ace 100644
--- a/shallow.c
+++ b/shallow.c
@@ -175,6 +175,9 @@ struct commit_list *get_shallow_commits_by_rev_list(int ac, 
const char **av,
die("revision walk setup failed");
traverse_commit_list(, show_commit, NULL, _shallow_list);
 
+   if (!not_shallow_list)
+   die("no commits selected for shallow requests");
+
/* Mark all reachable commits as NOT_SHALLOW */
for (p = not_shallow_list; p; p = p->next)
p->item->object.flags |= not_shallow_flag;
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 0680dec808..c8f2d38a58 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -711,6 +711,17 @@ test_expect_success 'fetch shallow since ...' '
test_cmp expected actual
 '
 
+test_expect_success 'clone shallow since selects no commits' '
+   test_create_repo shallow-since-the-future &&
+   (
+   cd shallow-since-the-future &&
+   GIT_COMMITTER_DATE="1 +0700" git commit --allow-empty -m one &&
+   GIT_COMMITTER_DATE="2 +0700" git commit --allow-empty -m two &&
+   GIT_COMMITTER_DATE="3 +0700" git commit --allow-empty -m three 
&&
+   test_must_fail git clone --shallow-since "9 +0700" 
"file://$(pwd)/." ../shallow111
+   )
+'
+
 test_expect_success 'shallow clone exclude tag two' '
test_create_repo shallow-exclude &&
(
-- 
2.17.0.705.g3525833791

RE: [PATCH 1/2] remote-curl: accept all encoding supported by curl

[anton.golubev]

Hi Jonathan,

I'd like to confirm, that your patch fixes my problem: I can sync with
google repository now using git and curl version without gzip support. 
Do you know how this patch is going to be released? Just HEAD now and GA in
the next planned release?

Communication looks like follow now:

root@bsb:~# GIT_TRACE_PACKET=1 GIT_CURL_VERBOSE=1 git ls-remote
https://source.developers.google.co m/p/wired-balm-187912/r/dotfiles 2>&1 |
sed -e 's/\(git-[^=]*\)=.*/\1=REDACTED/' -e 's/Authorizatio n:
.*/Authorization: REDACTED/'
> GET /p/wired-balm-187912/r/dotfiles/info/refs?service=git-upload-pack
HTTP/1.1
Host: source.developers.google.com
User-Agent: git/2.17.0
Accept: */*
Accept-Encoding: identity
Cookie: o=git-anton.golubev.gmail.com=REDACTED
Pragma: no-cache

< HTTP/1.1 200 OK
< Cache-Control: no-cache, max-age=0, must-revalidate
< Content-Length: 374
< Content-Type: application/x-git-upload-pack-advertisement
< Expires: Fri, 01 Jan 1980 00:00:00 GMT
< Pragma: no-cache
< X-Content-Type-Options: nosniff
< X-Frame-Options: SAMEORIGIN
< X-Xss-Protection: 1; mode=block
< Date: Sat, 26 May 2018 11:04:41 GMT
< Alt-Svc: hq=":443"; ma=2592000; quic=51303433; quic=51303432;
quic=51303431; quic=51303339; quic=51303335,quic=":443"; ma=2592000;
v="43,42,41,39,35"
<
13:04:41.274561 pkt-line.c:80   packet:  git< #
service=git-upload-pack
13:04:41.274634 pkt-line.c:80   packet:  git< 
13:04:41.274693 pkt-line.c:80   packet:  git<
45e2c99dd1790529cc4b7e029b1e9dfcc817d18e HEAD\0 include-tag
multi_ack_detailed multi_ack ofs-delta side-band side-band-64k thin-pack
no-progress shallow no-done allow-tip-sha1-in-want
allow-reachable-sha1-in-want agent=JGit/4-google filter
symref=HEAD:refs/heads/master
13:04:41.274739 pkt-line.c:80   packet:  git<
45e2c99dd1790529cc4b7e029b1e9dfcc817d18e refs/heads/master
13:04:41.274777 pkt-line.c:80   packet:  git< 
45e2c99dd1790529cc4b7e029b1e9dfcc817d18eHEAD
45e2c99dd1790529cc4b7e029b1e9dfcc817d18erefs/heads/master

Kind regards,
Anton Golubev



-Original Message-
From: Jonathan Nieder [mailto:jrnie...@gmail.com] 
Sent: Dienstag, 22. Mai 2018 02:01
To: Brandon Williams 
Cc: git@vger.kernel.org; Anton Golubev 
Subject: Re: [PATCH 1/2] remote-curl: accept all encoding supported by curl

Hi,

Brandon Williams wrote:

> Subject: remote-curl: accept all encoding supported by curl

nit: s/encoding/encodings

> Configure curl to accept all encoding which curl supports instead of 
> only accepting gzip responses.

Likewise.

> This is necessary to fix a bug when using an installation of curl 
> which doesn't support gzip.  Since curl doesn't do any checking to 
> verify that it supports the encoding set when calling 
> 'curl_easy_setopt()', curl can end up sending an "Accept-Encoding" 
> header indicating that it supports a particular encoding when in fact 
> it doesn't.  Instead when the empty string "" is used when setting 
> `CURLOPT_ENCODING`, curl will send an "Accept-Encoding" header 
> containing only the encoding methods curl supports.
>
> Signed-off-by: Brandon Williams 

Thanks for the analysis and fix.

Reported-by: Anton Golubev 

Also ccing the reporter so we can hopefully get a tested-by.  Anton, can you
test this patch and let us know how it goes?  You can apply it as follows:

  curl \
 
https://public-inbox.org/git/20180521234004.142548-1-bmw...@google.com/raw \
>patch.txt
  git am -3 patch.txt

Brandon, can the commit message also say a little more about the motivating
context and symptoms?

  $ curl --version
  curl 7.52.1 (arm-openwrt-linux-gnu) libcurl/7.52.1 mbedTLS/2.6.0
  Protocols: file ftp ftps http https
  Features: IPv6 Largefile SSL

The issue is that when curl is built without the "zlib" feature, since
v1.8.0-rc0~14^2 (Enable info/refs gzip decompression in HTTP client,
2012-09-19) we end up requesting "gzip" encoding anyway despite libcurl not
being able to decode it.  Worse, instead of getting a clear error message
indicating so, we end up falling back to "dumb"
http, producing a confusing and difficult to debug result.

> ---
>  http.c  | 2 +-
>  remote-curl.c   | 2 +-
>  t/t5551-http-fetch-smart.sh | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/http.c b/http.c
> index fed13b216..709150fc7 100644
> --- a/http.c
> +++ b/http.c
> @@ -1788,7 +1788,7 @@ static int http_request(const char *url,
>  
>   curl_easy_setopt(slot->curl, CURLOPT_URL, url);
>   curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
> - curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
> + curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");
>  
>   ret = run_one_slot(slot, );
>  
> diff --git a/remote-curl.c b/remote-curl.c index ceb05347b..565bba104 
> 100644
> --- a/remote-curl.c
> +++ 

[PATCH] t990X: use '.git/objects' as 'deep inside .git' path

[Christian Couder]

Tests t9902-completion.sh and t9903-bash-prompt.sh each have tests
that check what happens when we are "in the '.git' directory" and
when we are "deep inside the '.git' directory".

To test the case when we are "deep inside the '.git' directory" the
test scripts used to perform a `cd .git/refs/heads`.

As there are plans to implement other ref storage systems, let's
use '.git/objects' instead of '.git/refs/heads' as the "deep inside
the '.git' directory" path.

This makes it clear to readers that these tests do not depend on
which ref backend is used.

The internals of the loose refs backend are still tested in
t1400-update-ref.sh.

Helped-by: SZEDER Gábor 
Signed-off-by: David Turner 
Signed-off-by: Christian Couder 
---
 t/t9902-completion.sh  | 2 +-
 t/t9903-bash-prompt.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 36deb0b123..a28640ce1a 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -181,7 +181,7 @@ test_expect_success '__git_find_repo_path - cwd is a .git 
directory' '
 test_expect_success '__git_find_repo_path - parent is a .git directory' '
echo "$ROOT/.git" >expected &&
(
-   cd .git/refs/heads &&
+   cd .git/objects &&
__git_find_repo_path &&
echo "$__git_repo_path" >"$actual"
) &&
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 8f5c811dd7..c3b89ae783 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -148,7 +148,7 @@ test_expect_success 'prompt - inside .git directory' '
 test_expect_success 'prompt - deep inside .git directory' '
printf " (GIT_DIR!)" >expected &&
(
-   cd .git/refs/heads &&
+   cd .git/objects &&
__git_ps1 >"$actual"
) &&
test_cmp expected "$actual"
-- 
2.17.0.1181.g093e983b05.dirty

Fwd: [PATCH v2 02/18] Add a new builtin: branch-diff

[Øyvind Rønningstad]

Just want to throw my support in for range-diff since ranges is what
you pass to the command.

Alternatively, diff-diff since that's how I've crudely tried to
accomplish this before.

git diff A..B > diff1
git diff C..D > diff2
winmerge diff1 diff2