Re: [PATCH 0/5] Use watchman to reduce index refresh time

[Christian Couder]

On Tue, Nov 3, 2015 at 10:21 AM, Duy Nguyen  wrote:
> On Mon, Nov 2, 2015 at 8:23 PM, Duy Nguyen  wrote:
>> On Mon, Nov 2, 2015 at 3:54 PM, Paolo Ciarrocchi
>>  wrote:
>>> On Sun, Nov 1, 2015 at 2:55 PM, Nguyễn Thái Ngọc Duy  
>>> wrote:
>>>
>>> Hi Duy,
>>>
 This series builds on top of the index-helper series I just sent and
 uses watchman to keep track of file changes in order to avoid lstat()
 at refresh time. The series can also be found at [1]

 When I started this work, watchman did not support Windows yet. It
 does now, even if still experimental [2]. So Windows people, please
 try it out if you have time.

 To put all pieces so far together, we have split-index to reduce index
 write time, untracked cache to reduce I/O as well as computation for
 .gitignore, index-helper for index read time and this series for
 lstat() at refresh time. The remaining piece is killing lstat() from
 untracked cache, but right now it's just some idea and incomplete
 code.
>>>
>>> Did you manage to measure the speedup introduced by this series?
>>
>> It was from last year. I may have measured it but because I didn't
>> save it in the commit message, it was lost anyway. Installing watchman
>> and measuring with webkit.git soon..
>
> Test repo: webkit.git with 104665 tracked files, 5615 untracked,
> 3517 dirs. Best numbers out of a few tries. This is best case
> scenario. Normal usage could have worse numbers.

Thank you for the tests!

I tried to replicate your results on webkit.git with 184672 tracked
files, 5631 untracked, 9330 dirs. Also best numbers out of a few
tries.

> There is something strange about the "-uno" measurements. I don't
> think watchman+untracked cache can beat -uno..  Maybe I did something
> wrong.
>
> 0m0.383s   index v2
> 0m0.351s   index v4
> 0m0.352s   v2 split-index
> 0m0.309s   v2 split index-helper
> 0m0.159s   v2 split helper untracked-cache
> 0m0.123s   v2 split helper "status -uno"
> 0m0.098s   v2 split helper untracked watchman
> 0m0.071s   v2 split helper watchman "status -uno"

I got the following results from "time git status ...":

0m0.774s
0m0.799s split
0m0.766s split helper
0m0.335s split helper untracked
0m0.232s split helper untracked -uno
0m0.284s split helper untracked watchman
0m0.188s split helper untracked watchman -uno

Using David's series I get worse results than all of the above but I
guess it's because his series is based on an ancient git version
(v2.0.0-rc0).

> Note, the watchman series needs
> s/free_watchman_shm/release_watchman_shm/ (I didn't do a good job
> of testing after rebase). And there's a small bug in index-helper
> --detach code writing incorrect PID..

I did the s/free_watchman_shm/release_watchman_shm/ change, but I
didn't notice the index-helper bug.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[ANNOUNCE] Git Rev News edition 9

[Christian Couder]

Hi everyone,

I'm happy announce that the 9th edition of Git Rev News is now published:

http://git.github.io/rev_news/2015/11/11/edition-9/

Thanks a lot to all the contributors, especially Matthieu!

Enjoy,
Christian, Thomas and Nicola.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Watchman/inotify support and other ways to speed up git status

[Christian Couder]

On Tue, Nov 3, 2015 at 6:45 AM, Duy Nguyen <pclo...@gmail.com> wrote:
> On Mon, Nov 2, 2015 at 9:56 PM, David Turner <dtur...@twopensource.com> wrote:
>> On Thu, 2015-10-29 at 09:10 +0100, Christian Couder wrote:
>>> > We're using Watchman at Twitter.  A week or two ago posted a dump of our
>>> > code to github, but I would advise waiting a day or two to use it, as
>>> > I'm about to pull a large number of bugfixes into it (I'll update this
>>> > thread and provide a link once I do so).
>>>
>>> Great, I will have a look at it then!
>>
>> Here's the most recent version:
>>
>> https://github.com/dturner-tw/git/tree/dturner/watchman
>
> Christian, the index-helper/watchman series are posted because you
> showed interest in this area. I'm not rerolling to address David's
> comments on the series for now.

Ok no problem. Thanks a lot to you and David for posting your rebased series!

> Take your time evaluate the two
> approaches, then you can pick one (and let me know if you want me to
> hand my series over, very glad to do so).

Yeah, I will do that, thanks again!
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Draft of Git Rev News edition 9

[Christian Couder]

Hi,

A draft of Git Rev News edition 9 is available here:

https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-9.md

Everyone is welcome to contribute in any section either by editing the
above page on GitHub and sending a pull request, or by commenting on
this GitHub issue:

https://github.com/git/git.github.io/issues/108

You can also reply to this email.

I tried to cc everyone who appears in this edition but maybe I missed
some people, sorry about that.

Thomas, Nicola and myself plan to publish this edition on Wednesday
the 11th of November.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[ANNOUNCE] Git Rev News edition 8

[Christian Couder]

Hi everyone,

I'm happy announce that the 8th edition of Git Rev News is now published:

https://git.github.io/rev_news/2015/10/14/edition-8/

Thanks a lot to all the contributors!

Enjoy,
Christian, Thomas and Nicola.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] quote: move comment before sq_quote_buf()

[Christian Couder]

A big comment at the beginning of quote.c is really
related to sq_quote_buf(), so let's move it in front
of this function.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 quote.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/quote.c b/quote.c
index 890885a..fe884d2 100644
--- a/quote.c
+++ b/quote.c
@@ -4,6 +4,11 @@
 
 int quote_path_fully = 1;
 
+static inline int need_bs_quote(char c)
+{
+   return (c == '\'' || c == '!');
+}
+
 /* Help to copy the thing properly quoted for the shell safety.
  * any single quote is replaced with '\'', any exclamation point
  * is replaced with '\!', and the whole thing is enclosed in a
@@ -16,11 +21,6 @@ int quote_path_fully = 1;
  *  a'b  ==> a'\''b==> 'a'\''b'
  *  a!b  ==> a'\!'b==> 'a'\!'b'
  */
-static inline int need_bs_quote(char c)
-{
-   return (c == '\'' || c == '!');
-}
-
 void sq_quote_buf(struct strbuf *dst, const char *src)
 {
char *to_free = NULL;
-- 
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] quote: fix broken sq_quote_buf() related comment

[Christian Couder]

Since 77d604c (Enhanced sq_quote(), 10 Oct 2005), the
comment at the beginning of quote.c is broken.
Let's fix it.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
The only change in this v2 is the added Signed-off-by ;-)
Sorry for the spam.

 quote.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/quote.c b/quote.c
index 7920e18..890885a 100644
--- a/quote.c
+++ b/quote.c
@@ -7,6 +7,7 @@ int quote_path_fully = 1;
 /* Help to copy the thing properly quoted for the shell safety.
  * any single quote is replaced with '\'', any exclamation point
  * is replaced with '\!', and the whole thing is enclosed in a
+ * single quote pair.
  *
  * E.g.
  *  original sq_quote result
-- 
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/4] stripspace: Implement --count-lines option

[Christian Couder]

On Mon, Oct 19, 2015 at 3:46 PM, Tobias Klauser  wrote:
> On 2015-10-18 at 19:18:53 +0200, Junio C Hamano  wrote:
>> Eric Sunshine  writes:
>>
>> > Is there any application beyond git-rebase--interactive where a
>> > --count-lines options is expected to be useful? It's not obvious from
>> > the commit message that this change is necessarily a win for later
>> > porting of git-rebase--interactive to C since the amount of extra code
>> > and support material added by this patch probably outweighs the amount
>> > of code a C version of git-rebase--interactive would need to count the
>> > lines itself.
>> >
>> > Stated differently, are the two or three instances of piping through
>> > 'wc' in git-rebase--interactive sufficient justification for
>> > introducing extra complexity into git-stripspace and its documentation
>> > and tests?
>>
>> Interesting thought.  When somebody rewrites "rebase -i" in C,
>> nobody needs to count lines in "stripspace" output.  The rewritten
>> "rebase -i" would internally run strbuf_stripspace() and the question
>> becomes what is the best way to let that code find out how many lines
>> the result contains.
>>
>> When viewed from that angle, I agree that "stripspace --count" does
>> not add anything to further the goal of helping "rebase -i" to move
>> to C.  Adding strbuf_count_lines() that counts the number of lines
>> in the given strbuf (if there is no such helper yet; I didn't check),
>> though.
>
> I check before implementing this series and didn't find any helper. I
> also didn't find any other uses of line counting in the code.

This shows that implementing "git stripspace --count-lines" could
indirectly help porting "git rebase -i" to C as you could implement
strbuf_count_lines() for the former and it could then be reused in the
latter.

> After considering your and Eric's reply, I'll drop these patches for
> now and only resubmit patches 1/4 and 2/4 for v3 (also see my reply to
> Eric).

It would be sad in my opinion.

>> >> +test_expect_success '--count-lines with newline only' '
>> >> +   printf "0\n" >expect &&
>> >> +   printf "\n" | git stripspace --count-lines >actual &&
>> >> +   test_cmp expect actual
>> >> +'
>> >
>> > What is the expected behavior when the input is an empty file, a file
>> > with content but no newline, a file with one or more lines but lacking
>> > a newline on the final line? Should these cases be tested, as well?
>>
>> Good point here, too.  If we were to add strbuf_count_lines()
>> helper, whoever adds that function needs to take a possible
>> incomplete line at the end into account.
>
> Yes, makes more sense like this (even though it doesn't correspond to
> what 'wc -l' does).

Tests for "git stripspace --count-lines" would test
strbuf_count_lines() which would also help when porting git rebase -i
to C.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Draft of Git Rev News edition 8

[Christian Couder]

On Sun, Oct 11, 2015 at 1:45 AM, Eric Sunshine <sunsh...@sunshineco.com> wrote:
> On Sat, Oct 10, 2015 at 7:36 PM, Christian Couder
> <christian.cou...@gmail.com> wrote:
>> A draft of Git Rev News edition 8 is available here:
>> https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-8.md
>
> Does Karsten's comprehensive post[1] about nanosecond-related racy
> detection problems merit mention?

Yeah, probably, would you like to write something which can be very
short, about it.

We would also gladly accept something about Git version 2.6 (hint, hint :-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Draft of Git Rev News edition 8

[Christian Couder]

Hi,

A draft of Git Rev News edition 8 is available here:

https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-8.md

Everyone is welcome to contribute in any section either by editing the
above page on GitHub and sending a pull request, or by commenting on
this GitHub issue:

https://github.com/git/git.github.io/issues/100

You can also reply to this email.

I tried to cc everyone who appears in this edition but maybe I missed
some people, sorry about that.

Thomas, Nicola and myself plan to publish this edition on Wednesday
the 14th of October.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[ANNOUNCE] Git Rev News edition 5

[Christian Couder]

Hi,

Git Rev News edition 5 is now available:

https://git.github.io/rev_news/2015/07/08/edition-5/

Thanks a lot to all the helpers!

Enjoy,
Christian, Thomas and Nicola.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Draft of Git Rev News edition 5

[Christian Couder]

On Wed, Jul 8, 2015 at 9:43 AM, Junio C Hamano gits...@pobox.com wrote:
 Michael J Gruber g...@drmicha.warpmail.net writes:

 Maybe a matter of taste, but I think in general we could do with a bit
 less of narrating and more of summarizing.

 True.

I think sometimes the details might be interesting for different reasons.

 Just as an example, in the section on visualizing merge diffs after the
 fact, few people will be interested in the detail that I pointed out
 the --merges option of rev-list to Dscho. While that recollection is
 true and everything on the git-ml is public, I consider Git Rev News
 to be more public, targetted to a wider audience than the regulars.
 They don't all know how much Git owes to Dscho. If things like this end
 up in the news it makes me ponder for each on-list reply whether I'd
 rather reply in private. Maybe I'm being overly sensitive (though not
 affected in this case), but I just feel there are different degrees of
 public.

 I do not see Michael pointed out that there was a slightly better
 way to do that as saying anything bad about his contribution.

On the contrary I think that the way Dscho used sed shows some cli
proficiency and might be interesting to some people.

 I however do agree with you that we want to see the newsletter aim
 to summarize things better.  Instead of saying Dscho suggested X,
 Michael then refined it to Y, with full details of what X and Y
 looked like, it would be more appropriate for the target audience to
 say Dscho and Michael worked together to come up with a solution
 Y.

With the details, I think readers are more likely to remember the
--merges option.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 06/10] Documentation/tag: remove double occurance of pattern

[Christian Couder]

On Thu, Jul 9, 2015 at 12:27 PM, Karthik Nayak karthik@gmail.com wrote:
 Mentored-by: Christian Couder christian.cou...@gmail.com
 Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
 Signed-off-by: Karthik Nayak karthik@gmail.com
 ---
  Documentation/git-tag.txt | 1 -
  1 file changed, 1 deletion(-)

 diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
 index 034d10d..4b04c2b 100644
 --- a/Documentation/git-tag.txt
 +++ b/Documentation/git-tag.txt
 @@ -14,7 +14,6 @@ SYNOPSIS
  'git tag' -d tagname...
  'git tag' [-n[num]] -l [--contains commit] [--points-at object]
 [--column[=options] | --no-column] [pattern...]
 -   [pattern...]
  'git tag' -v tagname...

As this patch could be applied directly to master and to maint maybe
you could send it at the top of this patch series or alone outside of
this patch series.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] trailer: ignore first line of message

[Christian Couder]

When looking for the start of the trailers in the message
we are passed, we should ignore the first line of the message.

The reason is that if we are passed a patch or commit message
then the first line should be the patch title.
If we are passed only trailers we can expect that they start
with an empty line that can be ignored too.

This way we can properly process commit messages that have
only one line with something that looks like a trailer, for
example like area of code: change we made.
---
 t/t7513-interpret-trailers.sh | 15 ++-
 trailer.c |  4 +++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index bd0ab46..9577b4e 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -93,12 +93,25 @@ test_expect_success 'with config option on the command 
line' '
Acked-by: Johan
Reviewed-by: Peff
EOF
-   echo Acked-by: Johan |
+   { echo; echo Acked-by: Johan; } |
git -c trailer.Acked-by.ifexists=addifdifferent interpret-trailers \
--trailer Reviewed-by: Peff --trailer Acked-by: Johan 
actual 
test_cmp expected actual
 '
 
+test_expect_success 'with only a title in the message' '
+   cat expected -\EOF 
+   area: change
+
+   Reviewed-by: Peff
+   Acked-by: Johan
+   EOF
+   echo area: change |
+   git interpret-trailers --trailer Reviewed-by: Peff \
+   --trailer Acked-by: Johan actual 
+   test_cmp expected actual
+'
+
 test_expect_success 'with config setup' '
git config trailer.ack.key Acked-by:  
cat expected -\EOF 
diff --git a/trailer.c b/trailer.c
index 4b14a56..b808868 100644
--- a/trailer.c
+++ b/trailer.c
@@ -740,8 +740,10 @@ static int find_trailer_start(struct strbuf **lines, int 
count)
/*
 * Get the start of the trailers by looking starting from the end
 * for a line with only spaces before lines with one separator.
+* The first line must not be analyzed as the others as it
+* should be either the message title or a blank line.
 */
-   for (start = count - 1; start = 0; start--) {
+   for (start = count - 1; start = 1; start--) {
if (lines[start]-buf[0] == comment_line_char)
continue;
if (contains_only_spaces(lines[start]-buf)) {
-- 
2.5.0.401.g009ef9b.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] trailer: support multiline title

[Christian Couder]

We currently ignore the first line passed to `git interpret-trailers`,
when looking for the beginning of the trailers.

Unfortunately this does not work well when a commit is created with a
line break in the title, using for example the following command:

git commit -m 'place of
code: change we made'

In this special case, it is best to look at the first line and if it
does not contain only spaces, consider that the second line is not a
trailer.
---
 t/t7513-interpret-trailers.sh | 14 ++
 trailer.c |  8 +++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 9577b4e..56efe88 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -112,6 +112,20 @@ test_expect_success 'with only a title in the message' '
test_cmp expected actual
 '
 
+test_expect_success 'with multiline title in the message' '
+   cat expected -\EOF 
+   place of
+   code: change
+
+   Reviewed-by: Peff
+   Acked-by: Johan
+   EOF
+   printf %s\n%s\n place of code: change |
+   git interpret-trailers --trailer Reviewed-by: Peff \
+   --trailer Acked-by: Johan actual 
+   test_cmp expected actual
+'
+
 test_expect_success 'with config setup' '
git config trailer.ack.key Acked-by:  
cat expected -\EOF 
diff --git a/trailer.c b/trailer.c
index b808868..9a54449 100644
--- a/trailer.c
+++ b/trailer.c
@@ -759,7 +759,13 @@ static int find_trailer_start(struct strbuf **lines, int 
count)
return count;
}
 
-   return only_spaces ? count : 0;
+   if (only_spaces)
+   return count;
+
+   if (contains_only_spaces(lines[0]-buf))
+   return 1;
+
+   return count;
 }
 
 /* Get the index of the end of the trailers */
-- 
2.5.0.401.g009ef9b.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Aug 2015, #05; Fri, 28)

[Christian Couder]

 * dt/refs-bisection (2015-08-28) 5 commits
  - bisect: make bisection refs per-worktree
  - refs: make refs/worktree/* per-worktree
  - SQUASH???
  - path: optimize common dir checking
  - refs: clean up common_list

  Move the refs used during a git bisect session to per-worktree
  hierarchy refs/worktree/* so that independent bisect sessions can
  be done in different worktrees.

  Will merge to 'next' after squashing the update in.

Sorry if I am missing something or repeating what myself or someone
else like Michael already said, but in the current doc there is:

   Eventually there will be no more revisions left to bisect, and
you will have been left with the first bad kernel revision in
   refs/bisect/bad.

If we now just use refs/worktree/bisect/bad instead of
refs/bisect/bad, it might break scripts that rely using
refs/bisect/bad.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] trailer: support multiline title

[Christian Couder]

On Wed, Aug 26, 2015 at 9:48 PM, Junio C Hamano gits...@pobox.com wrote:
 Christian Couder christian.cou...@gmail.com writes:

 We currently ignore the first line passed to `git interpret-trailers`,
 when looking for the beginning of the trailers.

 Unfortunately this does not work well when a commit is created with a
 line break in the title, using for example the following command:

 git commit -m 'place of
 code: change we made'

 In this special case, it is best to look at the first line and if it
 does not contain only spaces, consider that the second line is not a
 trailer.
 ---

 Missing sign-off,

Ok, will add it.

[...]

 I think the analysis behind the first patch is correct.  It stops
 the backward scan of the main loop to reach there by realizing that
 the first line, which must be the first line of the patch title
 paragraph, can never be a trailer.

 To extend that correct realization to cover the case where the title
 paragraph has more than one line, the right thing to do is to scan
 forward from the beginning to find the first paragraph break, which
 must be the end of the title paragraph, and exclude the whole thing,
 wouldn't it?

 That is, I am wondering why the patch is not more like this (there
 may be off-by-one, but just to illustrate the approach; I didn't
 even compile test this one so...)?

 Puzzled...

  static int find_trailer_start(struct strbuf **lines, int count)
  {
 -   int start, only_spaces = 1;
 +   int start, end_of_title, only_spaces = 1;
 +
 +   /* The first paragraph is the title and cannot be trailer */
 +   for (start = 0; start  count; start++)
 +   if (!lines[start]-len)
 +   break; /* paragraph break */
 +   end_of_title = start;

 /*
  * Get the start of the trailers by looking starting from the end
  * for a line with only spaces before lines with one separator.
 -* The first line must not be analyzed as the others as it
 -* should be either the message title or a blank line.
  */
 -   for (start = count - 1; start = 1; start--) {
 +   for (start = count - 1; start = end_of_title; start--) {
 if (lines[start]-buf[0] == comment_line_char)
 continue;
 if (contains_only_spaces(lines[start]-buf)) {

Yeah, we can do that. It will be clearer.

Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] trailer: support multiline title

[Christian Couder]

We currently ignore the first line passed to `git interpret-trailers`,
when looking for the beginning of the trailers.

Unfortunately this does not work well when a commit is created with a
line break in the title, using for example the following command:

git commit -m 'place of
code: change we made'

That's why instead of ignoring only the first line, it is better to
ignore the first paragraph.
---
 t/t7513-interpret-trailers.sh | 14 ++
 trailer.c | 15 +++
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 9577b4e..56efe88 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -112,6 +112,20 @@ test_expect_success 'with only a title in the message' '
test_cmp expected actual
 '
 
+test_expect_success 'with multiline title in the message' '
+   cat expected -\EOF 
+   place of
+   code: change
+
+   Reviewed-by: Peff
+   Acked-by: Johan
+   EOF
+   printf %s\n%s\n place of code: change |
+   git interpret-trailers --trailer Reviewed-by: Peff \
+   --trailer Acked-by: Johan actual 
+   test_cmp expected actual
+'
+
 test_expect_success 'with config setup' '
git config trailer.ack.key Acked-by:  
cat expected -\EOF 
diff --git a/trailer.c b/trailer.c
index b808868..6f3416f 100644
--- a/trailer.c
+++ b/trailer.c
@@ -735,15 +735,22 @@ static int find_patch_start(struct strbuf **lines, int 
count)
  */
 static int find_trailer_start(struct strbuf **lines, int count)
 {
-   int start, only_spaces = 1;
+   int start, end_of_title, only_spaces = 1;
+
+   /* The first paragraph is the title and cannot be trailers */
+   for (start = 0; start  count; start++) {
+   if (lines[start]-buf[0] == comment_line_char)
+   continue;
+   if (contains_only_spaces(lines[start]-buf))
+   break;
+   }
+   end_of_title = start;
 
/*
 * Get the start of the trailers by looking starting from the end
 * for a line with only spaces before lines with one separator.
-* The first line must not be analyzed as the others as it
-* should be either the message title or a blank line.
 */
-   for (start = count - 1; start = 1; start--) {
+   for (start = count - 1; start = end_of_title; start--) {
if (lines[start]-buf[0] == comment_line_char)
continue;
if (contains_only_spaces(lines[start]-buf)) {
-- 
2.5.0.402.gcabd5e3.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] trailer: support multiline title

[Christian Couder]

Sorry I sent the part below privately by mistake:

On Tue, Aug 25, 2015 at 11:07 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:

 Now, I found another issue: I still have this interpret-trailers in my
 hooks/commit-msg, and it behaves badly when I use git commit -v. With
 -v, I get a diff in COMMIT_EDITMSG, and interpret-trailers tries to
 insert my Sign-off within the diff, like this:

   # Do not touch the line above.
   # Everything below will be removed.
   diff --git a/git-multimail/README b/git-multimail/README
   index f41906b..93d4751 100644

   Signed-off-by: Matthieu Moy matthieu@imag.fr
   --- a/git-multimail/README
   +++ b/git-multimail/README

This might be a bug. I will have a look.

 Either commit-msg should be called after stripping the diff from
 COMMIT_MSG, or interpret-trailers should learn to stop reading when the
 patch starts.

There is already code to detect a patch in interpret-trailers, but it
relies on the patch starting with a line with only three dashes.

So another option would be to make commit -v emit a line with three
dashes just under the # Everything below will be removed. line.

 I think the first option is better, since it means that
 any commit-msg hook does not have to deal with the patch stuff (my guess
 is that there are many broken commit-msg hooks out there, but people
 didn't notice because they don't use commit -v).

Maybe. I don't know if there is a reason why the commit-msg is called
before removing the patch.

On Wed, Aug 26, 2015 at 8:28 AM, Jacob Keller jacob.kel...@gmail.com wrote:

 It's always confused me why commit -v doesn't prepend every inserted
 line with # to mark it as a comment.

I think that would make interpret-trailers work properly too.

Thanks both,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] trailer: support multiline title

[Christian Couder]

On Mon, Aug 31, 2015 at 8:13 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Christian Couder <christian.cou...@gmail.com> writes:
>
>> On Mon, Aug 31, 2015 at 10:38 AM, Matthieu Moy
>> <matthieu@grenoble-inp.fr> wrote:
>>> Christian Couder <christian.cou...@gmail.com> writes:
>>>
>>>> That's why instead of ignoring only the first line, it is better to
>>>> ignore the first paragraph.
>>>> ---
>>>
>>> Lacks sign-off again.
>>
>> Ooops, sorry.
>
> Let me forge one just for this time.

Great, thanks!
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Aug 2015, #05; Fri, 28)

[Christian Couder]

On Mon, Aug 31, 2015 at 10:06 PM, Junio C Hamano  wrote:
> David Turner  writes:
>
>>> Christian, thanks for raising this one.
>>>
>>> I do recall the thread and I might be the somebody like Michael you
>>> remember, e.g. $gmane/275105---which did mention that "git bisect"
>>> would not need changing if we kept refs/bisect/.
>>>
>>> What was the reason why we chose to move to refs/worktree/ again?  I
>>> do not think there was an issue that we cannot keep refs/* in
>>> general shared while having one (or more) subhierarchy of it per
>>> worktree (otherwise we would not be using refs/worktree/*, but using
>>> something outside refs/, like $GIT_DIR/worktree-refs/).  Was there an
>>> objection to refs/bisect being private from aesthetics point of view
>>> (i.e. forcing everything per-worktree in refs/worktree/ would prevent
>>> proliferation of refs/this and refs/that that need to be private
>>> case by case), ignoring the practical issue of compatibility issues
>>> around existing tools?
>>
>> That is correct.  IIRC, on one of these patch sets, I proposed accepting
>> both new and old refs, but you said that would be unnecessary (it might
>> have been the notes/merge one instead of this one).
>
> I suspect it was notes-merge thing, but anyway, if you asked me
> right now, I certainly would say it is not OK to drop the old
> location but I may still say it is not worth having old and new with
> funny directory symlink like thing, because refs backend thing
> cannot say "I'll follow the symbolic link refs/bisect that points
> at refs/worktrees/bisect".
>
> But the reason why I say it is not worth is not because I do not
> think we need refs/bisect, but because I do not think we need
> refs/worktree/ at this point.  In other words, throwing new
> hierarchies that are private to worktree into refs/worktree/ is fine
> if we discover the need for some new hierarchies in the future, but
> being able to access the bisection points as refs/worktree/bisect is
> not necessary.  If people and tools are familiar with it being in
> refs/bisect, that location is fine.
>
>>> I think one example of script, "gitk --bisect", does want to show
>>> the DAG limited by bisect refs, but it does so using plumbing
>>> without having to say refs/bisect/bad itself.  Perhaps the thinking
>>> (or lack of enough of it) went that no other uses by scripts need to
>>> peek directly into refs/bisect/ hierarchy?
>>
>> I did a quick search on github, and did not see any scripts that said
>> "refs/bisect".
>
> That's one data point, but not a very confidence-building one.
>
> Christian, did you see your private script break with this change,
> or as one of the larger stakeholder of "bisect" subsystem you wanted
> to proceed with caution (the latter I myself would share, actually)?

Yeah, it's the latter.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PUB]What's cooking in git.git (Aug 2015, #05; Fri, 28)

[Christian Couder]

On Mon, Aug 31, 2015 at 9:36 AM, Matthieu Moy
 wrote:
> Junio C Hamano  writes:
>
>> * ad/bisect-terms (2015-08-03) 4 commits
>>  - bisect: allow setting any user-specified in 'git bisect start'
>>  - bisect: add 'git bisect terms' to view the current terms
>>  - bisect: add the terms old/new
>>  - bisect: sanity check on terms
>>
>>  The use of 'good/bad' in "git bisect" made it confusing to use when
>>  hunting for a state change that is not a regression (e.g. bugfix).
>>  The command learned 'old/new' and then allows the end user to
>>  say e.g. "bisect start --term-old=fast --term=new=slow" to find a
>>  performance regression.
>>
>>  Michael's idea to make 'good/bad' more intelligent does have
>>  certain attractiveness ($gname/272867), and makes some of the work
>>  on this topic a moot point.
>>
>>  Will hold.
>
> This topic has been there for a while and unless I missed a discussion,
> nothing happened. While I agree that Michael's idea is good and makes
> this series less useful, I think this topic also makes sense.
>
> I'd be in favor of merging it.

My opinion on this is that Michael's idea, even if it is good, is not
compatible with the current way bisect works, so, if it is ever
implemented, it will probably need a special flag like "git start
--possibly-find-fix".

That's why I'd also be in favor of merging this series.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] trailer: support multiline title

[Christian Couder]

On Mon, Aug 31, 2015 at 10:38 AM, Matthieu Moy
<matthieu@grenoble-inp.fr> wrote:
> Christian Couder <christian.cou...@gmail.com> writes:
>
>> That's why instead of ignoring only the first line, it is better to
>> ignore the first paragraph.
>> ---
>
> Lacks sign-off again.

Ooops, sorry.

> This replaces PATCH 2/2 in your previous series, but requires PATCH 1/2,
> right? If so that would be simpler to resend both patches IMHO.

This first patch is already in master. That's why I only sent the
second one. But yeah I could have explained that after the three
dashes.

> The patch works for me, thanks.

Thanks for reviewing and testing,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git bisect replay produces wrong result

[Christian Couder]

Hi,

On Sun, Aug 30, 2015 at 6:38 AM, Neil Brown  wrote:
>
> Hi,
>  the following git-bisect log - applied to a recent linux-kernel tree
>  produced different end results when I use "git bisect replay"
>  and when I just run it as a shell script.
>
> $ git bisect replay /tmp/log
> We are not bisecting.
> Bisecting: a merge base must be tested
> [2decb2682f80759f631c8332f9a2a34a02150a03] Merge 
> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
>
>
> $ bash /tmp/log
> 
> Bisecting: 2 revisions left to test after this (roughly 1 step)
> [57127645d79d2e83e801f141f7d03f64accf28aa] s390/zcrypt: Introduce new SHA-512 
> based Pseudo Random Generator.
>
> Is "git bisect replay" doing the wrong thing, or am I confused?

I think you are right and "git bisect replay" is buggy.
"git bisect replay" should display the same thing.
I will take a look at fixing this.

Thanks for the report,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Draft of Git Rev News edition 7

[Christian Couder]

Hi,

A draft of Git Rev News edition 7 is available here:

https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-7.md

Everyone is welcome to contribute in any section, like Matthieu
already did, either by editing the above page on GitHub and sending a
pull request, or by commenting on this GitHub issue:

https://github.com/git/git.github.io/issues/94

You can also reply to this email.

I tried to cc everyone who appears in this edition but maybe I missed
some people, sorry about that.

Thomas, Nicola and myself plan to publish this edition on Wednesday
the 9th of September.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: GSoC 2015 is over

[Christian Couder]

Hi,

On Thu, Sep 3, 2015 at 12:46 AM, Johannes Schindelin
 wrote:
> Hi,
>
> On Wed, 2 Sep 2015, Paul Tan wrote:
>
>> On Wed, Sep 2, 2015 at 12:55 AM, Matthieu Moy
>>  wrote:
>> > I consider this GSoC as a great success and a pleasant experience.
>> > Congratulation to Paul and Karthik, and a warm "thank you" to everybody
>> > who contributed: administrators, mentors, reviewers, and obviously
>> > Junio! (not to mention Google, who made all this possible)
>> >
>> > Thanks all!
>>
>> Thanks!
>
> Well, with so many thankyous going on, who am I to hide behind a rock?

Yeah, not to hide behind a rock myself either, I will say that I agree
Karthik's GSoC has been a great success, thanks to everyone involved
especially Karthik who has been very responsive and very persistant,
Matthieu who has been a great co-mentor, a great reviewer and a great
admin, and Eric, Michael and Junio who have been great reviewers of
Karthik's work.

I also feel very lucky to be the Git project mentor who will go to the
GSoC mentor summit on November 6 & 7. Thanks!

By the way while in the Bay Area from October 31 to November 14 it
would be nice for me to meet some Git developers living there. As Peff
suggested we could even have a mini GitTogether if enough developers
are interested. It doesn't need to be big. It could be just a lunch or
diner. Tell me if you are interested. We will see depending on the
number of people interested what we can do.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] interpret-trailers: allow running outside a repository

[Christian Couder]

On Sat, Sep 5, 2015 at 3:39 PM, John Keeping <j...@keeping.me.uk> wrote:
> It may be useful to run git-interpret-trailers without needing to be in
> a repository.

Yeah, it looks like it works fine outside a repo.

Tested-by: Christian Couder <chrisc...@tuxfamily.org>

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: More builtin git-am issues..

[Christian Couder]

On Sat, Sep 5, 2015 at 9:39 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> To salvage "interpret-trailers" needs a lot more, as we are
>> realizing that the definition that led to its external design does
>> not match the way users use footers in the real world.  This affects
>> the internal data representation and the whole thing needs to be
>> rethought.
>
> Note that I am not saying that you personally did any bad job while
> working on the interpret-trailers topic.  We collectively designed
> its feature based on a much narrower definition of what the trailer
> block is than what is used in the real world in practice, so we do
> not have a good way to locate an existing entry that is not a (key,
> value), no matter what the syntax to denote which is key and which
> is value is, insert a new entry that is not a (key, value), or
> remove an existing entry that is not a (key, value), all of which
> will be necessary to mutate trailer blocks people use in the real
> life.
>
> I think a good way forward would be to go like this:
>
>  * a helper function that starts from a flat  (or a
>strbuf) and identifies the end of the body of the message,
>i.e. find "^---$", "^Conflicts:$", etc. and skip blank lines
>backwards.  That is what ignore_non_trailer() in commit.c does,
>and that can be shared across everybody that mutates a log
>message.
>
>  * a helper function that takes the result of the above as a flat
> (or a strbuf) and identifies the beginning of a
>trailer block.  That may be just the matter of scanning backwards
>from the end of the trailer block ignore_non_trailer() identified
>for the first blank line, as I agree with Linus's "So quite
>frankly, at that point if git doesn't recognize it as a sign-off
>block, I don't think it's a big deal" in the thread.
>
>Not having that and not calling that function can reintroduce the
>recent "interpret-trailers corner case" bug Matthieu brought up.
>
> With these, everybody except interpret-trailers that mutates a log
> message can add a new signoff consistently.  And then, building on
> these, "interpret-trailers" can be written like this:
>
>  (1) starting from a flat  (or a strbuf), using the above
>  helpers, identify the parts of the log message that is the
>  trailer block (and you will know what is before and what is
>  after the trailer block).
>
>  (2) keep the part before the trailer block and the part after the
>  trailer block (this could be empty) in one strbuf each; we do
>  not want to mutate these parts, and it is pointless to split
>  them further into individual lines.
>
>  (3) express the lines in the trailer block in a richer data
>  structure that is easier to manipulate (i.e. reorder the lines,
>  insert, delete, etc.) and work on it.
>
>  (4) when manipulation of the trailer block is finished, reconstruct
>  the resulting message by concatenating the "before trailer"
>  part, "trailer" part, and "after trailer" part.
>
> As to the exact design of "a richer data structure" and the
> manipulation we may want on the trailer, I currently do not have a
> strong "it should be this way" opinion yet, but after looking at
> various examples Linus gave us in the discussion, my gut feelig is
> that it would be best to keep the operation simple and generic,
> e.g. "find a line that matches this regexp and replace it with this
> line", "insert this line at the end", "delete all lines that match
> this regexp", etc.

I will see what I can do about this.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[ANNOUNCE] Git Rev News edition 7

[Christian Couder]

Hi everyone,

I'm happy announce that the 7th edition of Git Rev News is now published:

https://git.github.io/rev_news/2015/09/09/edition-7/

Thanks a lot to all the contributors!

Enjoy,
Christian, Thomas and Nicola.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Developing- Where to Start

[Christian Couder]

Hi,

On Mon, Sep 14, 2015 at 10:42 PM, Breanna Devore-McDonald
 wrote:
> Hello all,
>
> I'm a third year Computer Science student at the University of Notre
> Dame, and for the final project of my Data Structures class, my group
> and I have to find a way to contribute our (hopefully) newly-found
> data structures and optimization knowledge to a well-known open source
> project. We thought Git would be a great project to work on, but we
> have no idea where to start. Can anyone offer ideas or parts of the
> code that we could possibly help with? (a problem that is data
> structures-related would be extremely helpful!)

The Git project often participate in the Google Summer of Code.

This year we used the following resources to help potential GSoC
students get involved in developing Git and find a project to work on:

http://git.github.io/SoC-2015-Microprojects.html
http://git.github.io/SoC-2015-Ideas.html

A GSoC wrap up has been written recently in Git Rev News edition 7:

http://git.github.io/rev_news/2015/09/09/edition-7/

Earlier this year students from Ensimag (Grenoble, France) also
contributed to Git and a small wrap up is available in Git Rev News
edition 5:

http://git.github.io/rev_news/2015/07/08/edition-5/

I hope this will help you get started in your Git development journey.

Best,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] quote: fix broken sq_quote_buf() related comment

[Christian Couder]

Since 77d604c (Enhanced sq_quote(), 10 Oct 2005), the
comment at the beginning of quote.c is broken.
Let's fix it.
---
 quote.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/quote.c b/quote.c
index 7920e18..890885a 100644
--- a/quote.c
+++ b/quote.c
@@ -7,6 +7,7 @@ int quote_path_fully = 1;
 /* Help to copy the thing properly quoted for the shell safety.
  * any single quote is replaced with '\'', any exclamation point
  * is replaced with '\!', and the whole thing is enclosed in a
+ * single quote pair.
  *
  * E.g.
  *  original sq_quote result
-- 
2.6.0.rc2.23.g0e57679

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] quote: move comment before sq_quote_buf()

[Christian Couder]

A big comment at the beginning of quote.c is really
related to sq_quote_buf(), so let's move it in front
of this function.
---
 quote.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/quote.c b/quote.c
index 890885a..fe884d2 100644
--- a/quote.c
+++ b/quote.c
@@ -4,6 +4,11 @@
 
 int quote_path_fully = 1;
 
+static inline int need_bs_quote(char c)
+{
+   return (c == '\'' || c == '!');
+}
+
 /* Help to copy the thing properly quoted for the shell safety.
  * any single quote is replaced with '\'', any exclamation point
  * is replaced with '\!', and the whole thing is enclosed in a
@@ -16,11 +21,6 @@ int quote_path_fully = 1;
  *  a'b  ==> a'\''b==> 'a'\''b'
  *  a!b  ==> a'\!'b==> 'a'\!'b'
  */
-static inline int need_bs_quote(char c)
-{
-   return (c == '\'' || c == '!');
-}
-
 void sq_quote_buf(struct strbuf *dst, const char *src)
 {
char *to_free = NULL;
-- 
2.6.0.rc2.23.g0e57679

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: best practices against long git rebase times?

[Christian Couder]

On Mon, Dec 7, 2015 at 11:59 PM, Jeff King  wrote:
> On Mon, Dec 07, 2015 at 02:56:33PM -0800, Junio C Hamano wrote:
>
>> Jeff King  writes:
>>
>> > You're computing the patch against the parent for each of those 3000
>> > commits (to get a hash of it to compare against the single hash on the
>> > other side). Twelve minutes sounds long, but if you have a really
>> > gigantic tree, it might not be unreasonable.
>> >
>> > You can also try compiling with "make XDL_FAST_HASH=" (i.e., setting
>> > that option to the empty string). Last year I found there were some
>> > pretty suboptimal corner cases, and you may be hitting one (we should
>> > probably turn that option off by default; I got stuck on trying to find
>> > a hash that would perform faster and never followed up[1].
>> >
>> > I doubt that is your problem, but it's possible).
>> >
>> > -Peff
>> >
>> > [1] http://thread.gmane.org/gmane.comp.version-control.git/261638
>>
>> I vaguely recall having discussed caching the patch-ids somewhere so
>> that this does not have to be done every time.  Would such an
>> extension help here, I wonder?
>
> I think you missed John's earlier response which gave several pointers
> to such caching schemes. :)

Yeah, he also gave very interesting performance numbers. Thanks John!

> I used to run with patch-id-caching in my personal fork (I frequently
> use "git log --cherry-mark" to see what has made it upstream), but I
> haven't for a while. It did make a big difference in speed, but I never
> resolved the corner cases around cache invalidation.

I will see if I can work on that after I am done with untracked cache...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/8] Untracked cache improvements

[Christian Couder]

Following the previous RFC version of this patch series and the
related discussions, here is a new version that tries to improve the
untracked cache feature.

This patch series implements core.untrackedCache as a bool config
variable. When it's true git should always try to use the untracked
cache, and when false git should never use it.

Patchs 1/8 and 3/8 add some features that are missing.

Patch 2/8, which is new, adds an enum as suggested by Duy.

Patchs 4/8, 5/8 and 6/8 are some refactoring to prepare for patch 7/8
which implements core.untrackedCache.

Patch 7/8 is the result of squashing the last 3 patches of the RFC
series. As discussed this sacrifies backward compatibility for a
simpler interface.

Patch 8/8, which is new, add some tests.

So the changes compared to the RFC version are just small bug fixes
and patchs 2/8 and 8/8.

The patch series is also available there:

https://github.com/chriscool/git/tree/uc-notifs14

Christian Couder (8):
  update-index: add untracked cache notifications
  update-index: use enum for untracked cache options
  update-index: add --test-untracked-cache
  update-index: move 'uc' var declaration
  dir: add add_untracked_cache()
  dir: add remove_untracked_cache()
  config: add core.untrackedCache
  t7063: add tests for core.untrackedCache

 Documentation/config.txt   |  7 +
 Documentation/git-update-index.txt | 31 ++--
 builtin/update-index.c | 52 +++---
 cache.h|  1 +
 config.c   |  4 +++
 contrib/completion/git-completion.bash |  1 +
 dir.c  | 22 +-
 dir.h  |  2 ++
 environment.c  |  1 +
 t/t7063-status-untracked-cache.sh  | 52 ++
 wt-status.c|  9 ++
 11 files changed, 145 insertions(+), 37 deletions(-)

-- 
2.6.3.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/8] dir: add add_untracked_cache()

[Christian Couder]

This new function will be used in a later patch.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 11 +--
 dir.c  | 14 ++
 dir.h  |  1 +
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 21f74b2..40530b0 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1123,16 +1123,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
if (untracked_cache == TEST_UC)
return 0;
}
-   if (!the_index.untracked) {
-   struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
-   strbuf_init(>ident, 100);
-   uc->exclude_per_dir = ".gitignore";
-   /* should be the same flags used by git-status */
-   uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES | 
DIR_HIDE_EMPTY_DIRECTORIES;
-   the_index.untracked = uc;
-   }
-   add_untracked_ident(the_index.untracked);
-   the_index.cache_changed |= UNTRACKED_CHANGED;
+   add_untracked_cache();
fprintf(stderr, _("Untracked cache enabled for '%s'\n"), 
get_git_work_tree());
} else if (untracked_cache == NO_UC && the_index.untracked) {
the_index.untracked = NULL;
diff --git a/dir.c b/dir.c
index d2a8f06..0f7e293 100644
--- a/dir.c
+++ b/dir.c
@@ -1938,6 +1938,20 @@ void add_untracked_ident(struct untracked_cache *uc)
strbuf_addch(>ident, 0);
 }
 
+void add_untracked_cache(void)
+{
+   if (!the_index.untracked) {
+   struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
+   strbuf_init(>ident, 100);
+   uc->exclude_per_dir = ".gitignore";
+   /* should be the same flags used by git-status */
+   uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES | 
DIR_HIDE_EMPTY_DIRECTORIES;
+   the_index.untracked = uc;
+   }
+   add_untracked_ident(the_index.untracked);
+   the_index.cache_changed |= UNTRACKED_CHANGED;
+}
+
 static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct 
*dir,
  int base_len,
  const struct pathspec 
*pathspec)
diff --git a/dir.h b/dir.h
index 7b5855d..ee94c76 100644
--- a/dir.h
+++ b/dir.h
@@ -308,4 +308,5 @@ void free_untracked_cache(struct untracked_cache *);
 struct untracked_cache *read_untracked_extension(const void *data, unsigned 
long sz);
 void write_untracked_extension(struct strbuf *out, struct untracked_cache 
*untracked);
 void add_untracked_ident(struct untracked_cache *);
+void add_untracked_cache(void);
 #endif
-- 
2.6.3.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/8] update-index: add untracked cache notifications

[Christian Couder]

Doing:

  cd /tmp
  git --git-dir=/git/somewhere/else/.git update-index --untracked-cache

doesn't work how one would expect. It hardcodes "/tmp" as the directory
that "works" into the index, so if you use the working tree, you'll
never use the untracked cache.

With this patch "git update-index --untracked-cache" tells the user in
which directory tests are performed and in which working directory the
untracked cache is allowed.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 7431938..6f6b289 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void)
if (!mkdtemp(mtime_dir.buf))
die_errno("Could not make temporary directory");
 
-   fprintf(stderr, _("Testing "));
+   fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd());
atexit(remove_test_directory);
xstat_mtime_dir();
fill_stat_data(, );
@@ -1122,9 +1122,11 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
}
add_untracked_ident(the_index.untracked);
the_index.cache_changed |= UNTRACKED_CHANGED;
+   fprintf(stderr, _("Untracked cache enabled for '%s'\n"), 
get_git_work_tree());
} else if (!untracked_cache && the_index.untracked) {
the_index.untracked = NULL;
the_index.cache_changed |= UNTRACKED_CHANGED;
+   fprintf(stderr, _("Untracked cache disabled\n"));
}
 
if (active_cache_changed) {
-- 
2.6.3.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/8] config: add core.untrackedCache

[Christian Couder]

When we know that mtime is fully supported by the environment, we
might want the untracked cache to be always used by default without
any mtime test or kernel version check being performed.

Also when we know that mtime is not supported by the environment,
for example because the repo is shared over a network file system,
then we might want 'git update-index --untracked-cache' to fail
immediately instead of preforming tests (because it might work on
some systems using the repo over the network file system but not
others).

The normal way to achieve the above in Git is to use a config
variable. That's why this patch introduces "core.untrackedCache".

To keep things simple, this variable is a bool which default to
false.

When "git status" is run, it now adds or removes the untracked
cache in the index to respect the value of this variable.

This means that `git update-index --[no-|force-]untracked-cache`,
to be compatible with the new config variable, have to set or
unset it. This new behavior is backward incompatible change, but
that is deliberate.

Also `--untracked-cache` used to check that the underlying
operating system and file system change `st_mtime` field of a
directory if files are added or deleted in that directory. But
those tests take a long time and there is now
`--test-untracked-cache` to perform them.

So to be more consistent with other git commands, this patch
prevents `--untracked-cache` to perform tests. This means that
after this patch there is no difference any more between
`--untracked-cache` and `--force-untracked-cache`.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 Documentation/config.txt   |  7 +++
 Documentation/git-update-index.txt | 28 ++--
 builtin/update-index.c | 23 +--
 cache.h|  1 +
 config.c   |  4 
 contrib/completion/git-completion.bash |  1 +
 dir.c  |  2 +-
 environment.c  |  1 +
 t/t7063-status-untracked-cache.sh  |  4 +---
 wt-status.c|  9 +
 10 files changed, 56 insertions(+), 24 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2d06b11..94820eb 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -308,6 +308,13 @@ core.trustctime::
crawlers and some backup systems).
See linkgit:git-update-index[1]. True by default.
 
+core.untrackedCache::
+   Determines if untracked cache will be enabled. Using
+   'git update-index --[no-|force-]untracked-cache' will set
+   this variable. Before setting it to true, you should check
+   that mtime is working properly on your system.
+   See linkgit:git-update-index[1]. False by default.
+
 core.checkStat::
Determines which stat fields to match between the index
and work tree. The user can set this to 'default' or
diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index 0ff7396..0fb39db 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -175,22 +175,28 @@ may not support it yet.
 --no-untracked-cache::
Enable or disable untracked cache extension. This could speed
up for commands that involve determining untracked files such
-   as `git status`. The underlying operating system and file
-   system must change `st_mtime` field of a directory if files
-   are added or deleted in that directory.
+   as `git status`.
++
+The underlying operating system and file system must change `st_mtime`
+field of a directory if files are added or deleted in that
+directory. You can test that using
+`--test-untracked-cache`. `--untracked-cache` used to test that too
+but it doesn't anymore.
++
+This sets the `core.untrackedCache` configuration variable to 'true'
+or 'false' in the repo config file, (see linkgit:git-config[1]), so
+that the untracked cache stays enabled or disabled.
 
 --test-untracked-cache::
Only perform tests on the working directory to make sure
untracked cache can be used. You have to manually enable
-   untracked cache using `--force-untracked-cache` (or
-   `--untracked-cache` but this will run the tests again)
-   afterwards if you really want to use it.
+   untracked cache using `--untracked-cache` or
+   `--force-untracked-cache` or the `core.untrackedCache`
+   configuration variable afterwards if you really want to use
+   it.
 
 --force-untracked-cache::
-   For safety, `--untracked-cache` performs tests on the working
-   directory to make sure untracked cache can be used. These
-   tests can take a few seconds. `--force-untracked-cache` can be
-   used to skip the tests.
+   Same as `--untracked-cache`.
 
 \--::
Do not interpret any more arguments as

[PATCH 2/8] update-index: use enum for untracked cache options

[Christian Couder]

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 6f6b289..246b3d3 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -35,6 +35,14 @@ static int mark_skip_worktree_only;
 #define UNMARK_FLAG 2
 static struct strbuf mtime_dir = STRBUF_INIT;
 
+/* Untracked cache mode */
+enum uc_mode {
+   UNDEF_UC = -1,
+   NO_UC = 0,
+   UC,
+   FORCE_UC
+};
+
 __attribute__((format (printf, 1, 2)))
 static void report(const char *fmt, ...)
 {
@@ -902,7 +910,7 @@ static int reupdate_callback(struct parse_opt_ctx_t *ctx,
 int cmd_update_index(int argc, const char **argv, const char *prefix)
 {
int newfd, entries, has_errors = 0, line_termination = '\n';
-   int untracked_cache = -1;
+   enum uc_mode untracked_cache = UNDEF_UC;
int read_from_stdin = 0;
int prefix_length = prefix ? strlen(prefix) : 0;
int preferred_index_format = 0;
@@ -997,7 +1005,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "untracked-cache", _cache,
N_("enable/disable untracked cache")),
OPT_SET_INT(0, "force-untracked-cache", _cache,
-   N_("enable untracked cache without testing the 
filesystem"), 2),
+   N_("enable untracked cache without testing the 
filesystem"), FORCE_UC),
OPT_END()
};
 
@@ -1104,10 +1112,10 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
the_index.split_index = NULL;
the_index.cache_changed |= SOMETHING_CHANGED;
}
-   if (untracked_cache > 0) {
+   if (untracked_cache > NO_UC) {
struct untracked_cache *uc;
 
-   if (untracked_cache < 2) {
+   if (untracked_cache < FORCE_UC) {
setup_work_tree();
if (!test_if_untracked_cache_is_supported())
return 1;
@@ -1123,7 +1131,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
add_untracked_ident(the_index.untracked);
the_index.cache_changed |= UNTRACKED_CHANGED;
fprintf(stderr, _("Untracked cache enabled for '%s'\n"), 
get_git_work_tree());
-   } else if (!untracked_cache && the_index.untracked) {
+   } else if (untracked_cache == NO_UC && the_index.untracked) {
the_index.untracked = NULL;
the_index.cache_changed |= UNTRACKED_CHANGED;
fprintf(stderr, _("Untracked cache disabled\n"));
-- 
2.6.3.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/8] update-index: move 'uc' var declaration

[Christian Couder]

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index ecb685d..21f74b2 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1116,8 +1116,6 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
the_index.cache_changed |= SOMETHING_CHANGED;
}
if (untracked_cache > NO_UC) {
-   struct untracked_cache *uc;
-
if (untracked_cache < FORCE_UC) {
setup_work_tree();
if (!test_if_untracked_cache_is_supported())
@@ -1126,7 +1124,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
return 0;
}
if (!the_index.untracked) {
-   uc = xcalloc(1, sizeof(*uc));
+   struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
strbuf_init(>ident, 100);
uc->exclude_per_dir = ".gitignore";
/* should be the same flags used by git-status */
-- 
2.6.3.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 8/8] t7063: add tests for core.untrackedCache

[Christian Couder]

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 t/t7063-status-untracked-cache.sh | 48 +--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/t/t7063-status-untracked-cache.sh 
b/t/t7063-status-untracked-cache.sh
index 253160a..f0af41c 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -18,6 +18,10 @@ if ! test_have_prereq UNTRACKED_CACHE; then
test_done
 fi
 
+test_expect_success 'core.untrackedCache is unset' '
+   test_must_fail git config --get core.untrackedCache
+'
+
 test_expect_success 'setup' '
git init worktree &&
cd worktree &&
@@ -28,6 +32,11 @@ test_expect_success 'setup' '
git update-index --untracked-cache
 '
 
+test_expect_success 'core.untrackedCache is true' '
+   UC=$(git config core.untrackedCache) &&
+   test "$UC" = "true"
+'
+
 test_expect_success 'untracked cache is empty' '
test-dump-untracked-cache >../actual &&
cat >../expect <../actual &&
-   cat >../expect <../expect-from-test-dump <../actual &&
+   echo "no untracked cache" >../expect &&
+   test_cmp ../expect ../actual
+'
+
+test_expect_success 'git status does not change anything' '
+   git status &&
+   test-dump-untracked-cache >../actual &&
+   test_cmp ../expect ../actual &&
+   UC=$(git config core.untrackedCache) &&
+   test "$UC" = "false"
+'
+
+test_expect_success 'setting core.untrackedCache and using git status creates 
the cache' '
+   git config core.untrackedCache true &&
+   test-dump-untracked-cache >../actual &&
+   test_cmp ../expect ../actual &&
+   git status &&
+   test-dump-untracked-cache >../actual &&
+   test_cmp ../expect-from-test-dump ../actual
+'
+
+test_expect_success 'unsetting core.untrackedCache and using git status 
removes the cache' '
+   git config --unset core.untrackedCache &&
+   test-dump-untracked-cache >../actual &&
+   test_cmp ../expect-from-test-dump ../actual &&
+   git status &&
+   test-dump-untracked-cache >../actual &&
+   test_cmp ../expect ../actual
+'
+
 test_done
-- 
2.6.3.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/8] dir: add remove_untracked_cache()

[Christian Couder]

This new function will be used in a later patch.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 3 +--
 dir.c  | 6 ++
 dir.h  | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 40530b0..e427657 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1126,8 +1126,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
add_untracked_cache();
fprintf(stderr, _("Untracked cache enabled for '%s'\n"), 
get_git_work_tree());
} else if (untracked_cache == NO_UC && the_index.untracked) {
-   the_index.untracked = NULL;
-   the_index.cache_changed |= UNTRACKED_CHANGED;
+   remove_untracked_cache();
fprintf(stderr, _("Untracked cache disabled\n"));
}
 
diff --git a/dir.c b/dir.c
index 0f7e293..ffc0286 100644
--- a/dir.c
+++ b/dir.c
@@ -1952,6 +1952,12 @@ void add_untracked_cache(void)
the_index.cache_changed |= UNTRACKED_CHANGED;
 }
 
+void remove_untracked_cache(void)
+{
+   the_index.untracked = NULL;
+   the_index.cache_changed |= UNTRACKED_CHANGED;
+}
+
 static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct 
*dir,
  int base_len,
  const struct pathspec 
*pathspec)
diff --git a/dir.h b/dir.h
index ee94c76..3e5114d 100644
--- a/dir.h
+++ b/dir.h
@@ -309,4 +309,5 @@ struct untracked_cache *read_untracked_extension(const void 
*data, unsigned long
 void write_untracked_extension(struct strbuf *out, struct untracked_cache 
*untracked);
 void add_untracked_ident(struct untracked_cache *);
 void add_untracked_cache(void);
+void remove_untracked_cache(void);
 #endif
-- 
2.6.3.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/8] update-index: add --test-untracked-cache

[Christian Couder]

It is nice to just be able to test if untracked cache is
supported without enabling it.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 Documentation/git-update-index.txt | 9 -
 builtin/update-index.c | 5 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index 3df9c26..0ff7396 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -17,7 +17,7 @@ SYNOPSIS
 [--[no-]assume-unchanged]
 [--[no-]skip-worktree]
 [--ignore-submodules]
-[--[no-|force-]untracked-cache]
+[--[no-|test-|force-]untracked-cache]
 [--really-refresh] [--unresolve] [--again | -g]
 [--info-only] [--index-info]
 [-z] [--stdin] [--index-version ]
@@ -179,6 +179,13 @@ may not support it yet.
system must change `st_mtime` field of a directory if files
are added or deleted in that directory.
 
+--test-untracked-cache::
+   Only perform tests on the working directory to make sure
+   untracked cache can be used. You have to manually enable
+   untracked cache using `--force-untracked-cache` (or
+   `--untracked-cache` but this will run the tests again)
+   afterwards if you really want to use it.
+
 --force-untracked-cache::
For safety, `--untracked-cache` performs tests on the working
directory to make sure untracked cache can be used. These
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 246b3d3..ecb685d 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -40,6 +40,7 @@ enum uc_mode {
UNDEF_UC = -1,
NO_UC = 0,
UC,
+   TEST_UC,
FORCE_UC
 };
 
@@ -1004,6 +1005,8 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
N_("enable or disable split index")),
OPT_BOOL(0, "untracked-cache", _cache,
N_("enable/disable untracked cache")),
+   OPT_SET_INT(0, "test-untracked-cache", _cache,
+   N_("test if the filesystem supports untracked 
cache"), TEST_UC),
OPT_SET_INT(0, "force-untracked-cache", _cache,
N_("enable untracked cache without testing the 
filesystem"), FORCE_UC),
OPT_END()
@@ -1119,6 +1122,8 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
setup_work_tree();
if (!test_if_untracked_cache_is_supported())
return 1;
+   if (untracked_cache == TEST_UC)
+   return 0;
}
if (!the_index.untracked) {
uc = xcalloc(1, sizeof(*uc));
-- 
2.6.3.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC/PATCH 3/8] update-index: move 'uc' var declaration

[Christian Couder]

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index b7b5108..b54ddc3 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1107,8 +1107,6 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
the_index.cache_changed |= SOMETHING_CHANGED;
}
if (untracked_cache > 0) {
-   struct untracked_cache *uc;
-
if (untracked_cache < 3) {
setup_work_tree();
if (!test_if_untracked_cache_is_supported())
@@ -1117,7 +1115,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
return 0;
}
if (!the_index.untracked) {
-   uc = xcalloc(1, sizeof(*uc));
+   struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
strbuf_init(>ident, 100);
uc->exclude_per_dir = ".gitignore";
/* should be the same flags used by git-status */
-- 
2.6.3.391.g95a3a5c

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC/PATCH 8/8] update-index: make core.untrackedCache a bool

[Christian Couder]

Most features in Git can be enabled or disabled using a simple
bool config variable and it would be nice if untracked cache
behaved the same way.

This makes --[no-|force-]untracked-cache change the value of
core.untrackedCache in the repo config file, to avoid making
those options useless and because this avoids the untracked
cache being disabled by a kernel change or a directory
change. Of course this breaks some backward compatibility,
but the simplification and increased useability is worth it.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 Documentation/git-update-index.txt | 13 ++---
 builtin/update-index.c | 10 --
 config.c   |  8 +---
 3 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index 5f74cc7..256b9c5 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -181,10 +181,11 @@ The underlying operating system and file system must 
change `st_mtime`
 field of a directory if files are added or deleted in that
 directory. You can test that using
 `--test-untracked-cache`. `--untracked-cache` used to test that too
-but it doesn't anymore. If you want to always enable, or always
-disable, untracked cache, you can set the `core.untrackedCache`
-configuration variable to 'true' or 'false' respectively, (see
-linkgit:git-config[1]).
+but it doesn't anymore.
++
+This sets the `core.untrackedCache` configuration variable to 'true'
+or 'false' in the repo config file, (see linkgit:git-config[1]), so
+that the untracked cache stays enabled or disabled.
 
 --test-untracked-cache::
Only perform tests on the working directory to make sure
@@ -194,9 +195,7 @@ linkgit:git-config[1]).
it.
 
 --force-untracked-cache::
-   Same as `--untracked-cache`. Note that this option cannot
-   enable untracked cache if `core.untrackedCache` configuration
-   variable is set to false (see linkgit:git-config[1]).
+   Same as `--untracked-cache`.
 
 \--::
Do not interpret any more arguments as options.
diff --git a/builtin/update-index.c b/builtin/update-index.c
index fb0ea3d..048c293 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -,15 +,13 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
return !test_if_untracked_cache_is_supported();
}
if (untracked_cache > 0) {
-   if (!use_untracked_cache)
-   die("core.untrackedCache is set to false; "
-   "the untracked cache will not be enabled");
+   if (!use_untracked_cache && 
git_config_set("core.untrackedCache", "true"))
+   die("could not set core.untrackedCache to true");
add_untracked_cache();
fprintf(stderr, _("Untracked cache enabled for '%s'\n"), 
get_git_work_tree());
} else if (!untracked_cache) {
-   if (use_untracked_cache > 0)
-   die("core.untrackedCache is set to true; "
-   "the untracked cache will not be disabled");
+   if (use_untracked_cache > 0 && 
git_config_set("core.untrackedCache", "false"))
+   die("could not set core.untrackedCache to false");
if (the_index.untracked) {
remove_untracked_cache();
fprintf(stderr, _("Untracked disabled\n"));
diff --git a/config.c b/config.c
index 7d50f43..f023ee7 100644
--- a/config.c
+++ b/config.c
@@ -692,13 +692,7 @@ static int git_default_core_config(const char *var, const 
char *value)
return 0;
}
if (!strcmp(var, "core.untrackedcache")) {
-   if (!strcasecmp(value, "default") || !strcasecmp(value, 
"check"))
-   use_untracked_cache = -1;
-   else {
-   use_untracked_cache = git_config_maybe_bool(var, value);
-   if (use_untracked_cache == -1)
-   error("unknown core.untrackedCache value '%s'; 
using default", value);
-   }
+   use_untracked_cache = git_config_bool(var, value);
return 0;
}
if (!strcmp(var, "core.checkstat")) {
-- 
2.6.3.391.g95a3a5c

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC/PATCH 4/8] dir: add add_untracked_cache()

[Christian Couder]

This new function will be used in a later patch.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 11 +--
 dir.c  | 14 ++
 dir.h  |  1 +
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index b54ddc3..ec67d14 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1114,16 +1114,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
if (untracked_cache == 2)
return 0;
}
-   if (!the_index.untracked) {
-   struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
-   strbuf_init(>ident, 100);
-   uc->exclude_per_dir = ".gitignore";
-   /* should be the same flags used by git-status */
-   uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES | 
DIR_HIDE_EMPTY_DIRECTORIES;
-   the_index.untracked = uc;
-   }
-   add_untracked_ident(the_index.untracked);
-   the_index.cache_changed |= UNTRACKED_CHANGED;
+   add_untracked_cache();
fprintf(stderr, _("Untracked cache enabled for '%s'\n"), 
get_git_work_tree());
} else if (!untracked_cache && the_index.untracked) {
the_index.untracked = NULL;
diff --git a/dir.c b/dir.c
index d2a8f06..0f7e293 100644
--- a/dir.c
+++ b/dir.c
@@ -1938,6 +1938,20 @@ void add_untracked_ident(struct untracked_cache *uc)
strbuf_addch(>ident, 0);
 }
 
+void add_untracked_cache(void)
+{
+   if (!the_index.untracked) {
+   struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
+   strbuf_init(>ident, 100);
+   uc->exclude_per_dir = ".gitignore";
+   /* should be the same flags used by git-status */
+   uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES | 
DIR_HIDE_EMPTY_DIRECTORIES;
+   the_index.untracked = uc;
+   }
+   add_untracked_ident(the_index.untracked);
+   the_index.cache_changed |= UNTRACKED_CHANGED;
+}
+
 static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct 
*dir,
  int base_len,
  const struct pathspec 
*pathspec)
diff --git a/dir.h b/dir.h
index 7b5855d..ee94c76 100644
--- a/dir.h
+++ b/dir.h
@@ -308,4 +308,5 @@ void free_untracked_cache(struct untracked_cache *);
 struct untracked_cache *read_untracked_extension(const void *data, unsigned 
long sz);
 void write_untracked_extension(struct strbuf *out, struct untracked_cache 
*untracked);
 void add_untracked_ident(struct untracked_cache *);
+void add_untracked_cache(void);
 #endif
-- 
2.6.3.391.g95a3a5c

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC/PATCH 1/8] update-index: add untracked cache notifications

[Christian Couder]

Doing:

  cd /tmp
  git --git-dir=/git/somewhere/else/.git update-index --untracked-cache

doesn't work how one would expect. It hardcodes "/tmp" as the directory
that "works" into the index, so if you use the working tree, you'll
never use the untracked cache.

With this patch "git update-index --untracked-cache" tells the user in
which directory tests are performed and in which working directory the
untracked cache is allowed.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 7431938..e568acc 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void)
if (!mkdtemp(mtime_dir.buf))
die_errno("Could not make temporary directory");
 
-   fprintf(stderr, _("Testing "));
+   fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd());
atexit(remove_test_directory);
xstat_mtime_dir();
fill_stat_data(, );
@@ -1122,9 +1122,11 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
}
add_untracked_ident(the_index.untracked);
the_index.cache_changed |= UNTRACKED_CHANGED;
+   fprintf(stderr, _("Untracked cache enabled for '%s'\n"), 
get_git_work_tree());
} else if (!untracked_cache && the_index.untracked) {
the_index.untracked = NULL;
the_index.cache_changed |= UNTRACKED_CHANGED;
+   fprintf(stderr, _("Untracked disabled\n"));
}
 
if (active_cache_changed) {
-- 
2.6.3.391.g95a3a5c

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC/PATCH 2/8] update-index: add --test-untracked-cache

[Christian Couder]

It is nice to just be able to test if untracked cache is
supported without enabling it.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 Documentation/git-update-index.txt | 9 -
 builtin/update-index.c | 8 ++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index 3df9c26..0ff7396 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -17,7 +17,7 @@ SYNOPSIS
 [--[no-]assume-unchanged]
 [--[no-]skip-worktree]
 [--ignore-submodules]
-[--[no-|force-]untracked-cache]
+[--[no-|test-|force-]untracked-cache]
 [--really-refresh] [--unresolve] [--again | -g]
 [--info-only] [--index-info]
 [-z] [--stdin] [--index-version ]
@@ -179,6 +179,13 @@ may not support it yet.
system must change `st_mtime` field of a directory if files
are added or deleted in that directory.
 
+--test-untracked-cache::
+   Only perform tests on the working directory to make sure
+   untracked cache can be used. You have to manually enable
+   untracked cache using `--force-untracked-cache` (or
+   `--untracked-cache` but this will run the tests again)
+   afterwards if you really want to use it.
+
 --force-untracked-cache::
For safety, `--untracked-cache` performs tests on the working
directory to make sure untracked cache can be used. These
diff --git a/builtin/update-index.c b/builtin/update-index.c
index e568acc..b7b5108 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -996,8 +996,10 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
N_("enable or disable split index")),
OPT_BOOL(0, "untracked-cache", _cache,
N_("enable/disable untracked cache")),
+   OPT_SET_INT(0, "test-untracked-cache", _cache,
+   N_("test if the filesystem supports untracked 
cache"), 2),
OPT_SET_INT(0, "force-untracked-cache", _cache,
-   N_("enable untracked cache without testing the 
filesystem"), 2),
+   N_("enable untracked cache without testing the 
filesystem"), 3),
OPT_END()
};
 
@@ -1107,10 +1109,12 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
if (untracked_cache > 0) {
struct untracked_cache *uc;
 
-   if (untracked_cache < 2) {
+   if (untracked_cache < 3) {
setup_work_tree();
if (!test_if_untracked_cache_is_supported())
return 1;
+   if (untracked_cache == 2)
+   return 0;
}
if (!the_index.untracked) {
uc = xcalloc(1, sizeof(*uc));
-- 
2.6.3.391.g95a3a5c

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC/PATCH 0/8] Untracked cache improvements

[Christian Couder]

Following the discussions on the "config: add core.trustmtime"
patch I previously sent, here is a patch series that tries to
improve the untracked cache feature.

This patch series implements core.untrackedCache instead of
core.trustmtime. core.untrackedCache is more complex because
basically when it's set to true git should always try to use
the untracked cache, and when set to false git should never
use it.

Patchs 1/8 and 2/8 add some features that are missing.
Patchs 3/8, 4/8 and 5/8 are some refactoring to prepare for
patch 6/8 which implements core.untrackedCache.

Up to patch 6/8 backward compatibility is preserved.
Patchs 7/8 and 8/8 are trying to improve usability by making
the untracked cache cli and config options more in line with
other git cli and config options, but this sacrifies some
backward compatibility.

Christian Couder (8):
  update-index: add untracked cache notifications
  update-index: add --test-untracked-cache
  update-index: move 'uc' var declaration
  dir: add add_untracked_cache()
  dir: add remove_untracked_cache()
  config: add core.untrackedCache
  update-index: prevent --untracked-cache from performing tests
  update-index: make core.untrackedCache a bool

 Documentation/config.txt   | 10 +
 Documentation/git-update-index.txt | 30 +++---
 builtin/update-index.c | 39 --
 cache.h|  1 +
 config.c   |  4 
 contrib/completion/git-completion.bash |  1 +
 dir.c  | 22 ++-
 dir.h  |  2 ++
 environment.c  |  1 +
 t/t7063-status-untracked-cache.sh  |  2 +-
 wt-status.c|  9 
 11 files changed, 90 insertions(+), 31 deletions(-)

-- 
2.6.3.391.g95a3a5c

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC/PATCH 5/8] dir: add remove_untracked_cache()

[Christian Couder]

This new function will be used in a later patch.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 3 +--
 dir.c  | 6 ++
 dir.h  | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index ec67d14..2cbaee0 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1117,8 +1117,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
add_untracked_cache();
fprintf(stderr, _("Untracked cache enabled for '%s'\n"), 
get_git_work_tree());
} else if (!untracked_cache && the_index.untracked) {
-   the_index.untracked = NULL;
-   the_index.cache_changed |= UNTRACKED_CHANGED;
+   remove_untracked_cache();
fprintf(stderr, _("Untracked disabled\n"));
}
 
diff --git a/dir.c b/dir.c
index 0f7e293..ffc0286 100644
--- a/dir.c
+++ b/dir.c
@@ -1952,6 +1952,12 @@ void add_untracked_cache(void)
the_index.cache_changed |= UNTRACKED_CHANGED;
 }
 
+void remove_untracked_cache(void)
+{
+   the_index.untracked = NULL;
+   the_index.cache_changed |= UNTRACKED_CHANGED;
+}
+
 static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct 
*dir,
  int base_len,
  const struct pathspec 
*pathspec)
diff --git a/dir.h b/dir.h
index ee94c76..3e5114d 100644
--- a/dir.h
+++ b/dir.h
@@ -309,4 +309,5 @@ struct untracked_cache *read_untracked_extension(const void 
*data, unsigned long
 void write_untracked_extension(struct strbuf *out, struct untracked_cache 
*untracked);
 void add_untracked_ident(struct untracked_cache *);
 void add_untracked_cache(void);
+void remove_untracked_cache(void);
 #endif
-- 
2.6.3.391.g95a3a5c

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC/PATCH 7/8] update-index: prevent --untracked-cache from performing tests

[Christian Couder]

`git update-index --untracked-cache` used to perform tests to
check that the underlying operating system and file system
change `st_mtime` field of a directory if files are added or
deleted in that directory. But those tests take a long time
and there is now `--test-untracked-cache` to perform them.

So to be more consistent with other git commands it's better
to make `--untracked-cache` not perform them, which is the
purpose of this patch.

Note that after this patch there is no difference any more
between `--untracked-cache` and `--force-untracked-cache`.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 Documentation/git-update-index.txt | 31 ---
 builtin/update-index.c |  7 ++-
 t/t7063-status-untracked-cache.sh  |  2 +-
 3 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index 4e6078b..5f74cc7 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -175,27 +175,28 @@ may not support it yet.
 --no-untracked-cache::
Enable or disable untracked cache extension. This could speed
up for commands that involve determining untracked files such
-   as `git status`. The underlying operating system and file
-   system must change `st_mtime` field of a directory if files
-   are added or deleted in that directory. If you want to always
-   enable, or always disable, untracked cache, you can set the
-   `core.untrackedCache` configuration variable to 'true' or
-   'false' respectively, (see linkgit:git-config[1]).
+   as `git status`.
++
+The underlying operating system and file system must change `st_mtime`
+field of a directory if files are added or deleted in that
+directory. You can test that using
+`--test-untracked-cache`. `--untracked-cache` used to test that too
+but it doesn't anymore. If you want to always enable, or always
+disable, untracked cache, you can set the `core.untrackedCache`
+configuration variable to 'true' or 'false' respectively, (see
+linkgit:git-config[1]).
 
 --test-untracked-cache::
Only perform tests on the working directory to make sure
untracked cache can be used. You have to manually enable
-   untracked cache using `--force-untracked-cache` (or
-   `--untracked-cache` but this will run the tests again)
-   afterwards if you really want to use it.
+   untracked cache using `--untracked-cache` or
+   `--force-untracked-cache` afterwards if you really want to use
+   it.
 
 --force-untracked-cache::
-   For safety, `--untracked-cache` performs tests on the working
-   directory to make sure untracked cache can be used. These
-   tests can take a few seconds. `--force-untracked-cache` can be
-   used to skip the tests. It cannot enable untracked cache if
-   `core.untrackedCache` configuration variable is set to false
-   (see linkgit:git-config[1]).
+   Same as `--untracked-cache`. Note that this option cannot
+   enable untracked cache if `core.untrackedCache` configuration
+   variable is set to false (see linkgit:git-config[1]).
 
 \--::
Do not interpret any more arguments as options.
diff --git a/builtin/update-index.c b/builtin/update-index.c
index bf697a5..fb0ea3d 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1106,12 +1106,9 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
the_index.split_index = NULL;
the_index.cache_changed |= SOMETHING_CHANGED;
}
-   if (untracked_cache == 2 || (untracked_cache == 1 && 
use_untracked_cache == -1)) {
+   if (untracked_cache == 2) {
setup_work_tree();
-   if (!test_if_untracked_cache_is_supported())
-   return 1;
-   if (untracked_cache == 2)
-   return 0;
+   return !test_if_untracked_cache_is_supported();
}
if (untracked_cache > 0) {
if (!use_untracked_cache)
diff --git a/t/t7063-status-untracked-cache.sh 
b/t/t7063-status-untracked-cache.sh
index 0e8d0d4..8c3e703 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -11,7 +11,7 @@ avoid_racy() {
 # It's fine if git update-index returns an error code other than one,
 # it'll be caught in the first test.
 test_lazy_prereq UNTRACKED_CACHE '
-   { git update-index --untracked-cache; ret=$?; } &&
+   { git update-index --test-untracked-cache; ret=$?; } &&
test $ret -ne 1
 '
 
-- 
2.6.3.391.g95a3a5c

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC/PATCH 6/8] config: add core.untrackedCache

[Christian Couder]

When we know that mtime is fully supported by the environment, we
might want the untracked cache to be always used by default without
any mtime test or kernel version check being performed.

Also when we know that mtime is not supported by the environment,
for example because the repo is shared over a network file system,
then we might want 'git update-index --untracked-cache' to fail
immediately instead of it testing if it works (because it might
work on some systems using the repo over the network file system
but not others).

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 Documentation/config.txt   | 10 ++
 Documentation/git-update-index.txt | 11 +--
 builtin/update-index.c | 28 ++--
 cache.h|  1 +
 config.c   | 10 ++
 contrib/completion/git-completion.bash |  1 +
 dir.c  |  2 +-
 environment.c  |  1 +
 wt-status.c|  9 +
 9 files changed, 60 insertions(+), 13 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index b4b0194..bf176ff 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -308,6 +308,16 @@ core.trustctime::
crawlers and some backup systems).
See linkgit:git-update-index[1]. True by default.
 
+core.untrackedCache::
+   If unset or set to 'default' or 'check', untracked cache will
+   not be enabled by default and when
+   'update-index --untracked-cache' is called, Git will test if
+   mtime is working properly before enabling it. If set to false,
+   Git will refuse to enable untracked cache even if
+   '--force-untracked-cache' is used. If set to true, Git will
+   blindly enabled untracked cache by default without testing if
+   it works. See linkgit:git-update-index[1].
+
 core.checkStat::
Determines which stat fields to match between the index
and work tree. The user can set this to 'default' or
diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index 0ff7396..4e6078b 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -177,7 +177,10 @@ may not support it yet.
up for commands that involve determining untracked files such
as `git status`. The underlying operating system and file
system must change `st_mtime` field of a directory if files
-   are added or deleted in that directory.
+   are added or deleted in that directory. If you want to always
+   enable, or always disable, untracked cache, you can set the
+   `core.untrackedCache` configuration variable to 'true' or
+   'false' respectively, (see linkgit:git-config[1]).
 
 --test-untracked-cache::
Only perform tests on the working directory to make sure
@@ -190,7 +193,9 @@ may not support it yet.
For safety, `--untracked-cache` performs tests on the working
directory to make sure untracked cache can be used. These
tests can take a few seconds. `--force-untracked-cache` can be
-   used to skip the tests.
+   used to skip the tests. It cannot enable untracked cache if
+   `core.untrackedCache` configuration variable is set to false
+   (see linkgit:git-config[1]).
 
 \--::
Do not interpret any more arguments as options.
@@ -406,6 +411,8 @@ It can be useful when the inode change time is regularly 
modified by
 something outside Git (file system crawlers and backup systems use
 ctime for marking files processed) (see linkgit:git-config[1]).
 
+Untracked cache look at `core.untrackedCache` configuration variable
+(see linkgit:git-config[1]).
 
 SEE ALSO
 
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 2cbaee0..bf697a5 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1106,19 +1106,27 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
the_index.split_index = NULL;
the_index.cache_changed |= SOMETHING_CHANGED;
}
+   if (untracked_cache == 2 || (untracked_cache == 1 && 
use_untracked_cache == -1)) {
+   setup_work_tree();
+   if (!test_if_untracked_cache_is_supported())
+   return 1;
+   if (untracked_cache == 2)
+   return 0;
+   }
if (untracked_cache > 0) {
-   if (untracked_cache < 3) {
-   setup_work_tree();
-   if (!test_if_untracked_cache_is_supported())
-   return 1;
-   if (untracked_cache == 2)
-   return 0;
-   }
+   if (!use_untracked_cache)
+   die("core.untrackedCache is set to false; "
+ 

Re: [RFC/PATCH 6/8] config: add core.untrackedCache

[Christian Couder]

On Fri, Dec 4, 2015 at 6:54 PM, Torsten Bögershausen  wrote:
>> Current state of affairs:
>>
>>  * Enable on a per-repo basis: git update-index --untracked-cache
>>  * Disable on a per-repo basis: git update-index --no-cache
>>  * Enable system-wide: N/A
>>  * Disable system-wide: N/A
>>
>> With this patch:
>>
>>  * Enable on a per-repo basis: git update-index --untracked-cache OR
>> "git config core.untrackedCache true"
>>  * Disable on a per-repo basis: git update-index --no-cache OR "git
>> config core.untrackedCache false"
>>  * Enable system-wide: git config --global core.untrackedCache true
>>  * Disable system-wide: git config --global core.untrackedCache false
>>  * Caveat: The core.untrackedCache config has precidence over "git 
>> update-index"
>>
>> With the rest of the patches in this series:
>>
>>  * Enable system-wide & per-repo the same, just tweak
>> core.untrackedCache either for the local .git or --globally
>>  * If you want to test things either per-repo or globally just use
>> "git update-index --test-untracked-cache"
>>  * If you want something exactly like the old --untracked-cache do:
>> "git update-index --test-untracked-cache && git config
>> core.untrackedCache true"
>>
>> I think applying this whole series makes sense. Enabling this feature
>> doesn't work like anything else in Git, usually you just tweak a
>> config option and thus can easily enable things either system-wide or
>> per-repo (or any combination of the two), which makes both system
>> administration and local configuration easy.
>>
>> A much saner UI for the CLI tools if we're going to ship git with
>> tests for filesystem features is to separate the testing from the
>> enabling of those features.
>
> My spontanous feeling: squash 6-8 together and add this nice explanation
> to the commit message.

Ok, I will do that then.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 2/8] update-index: add --test-untracked-cache

[Christian Couder]

On Wed, Dec 2, 2015 at 8:17 PM, Duy Nguyen <pclo...@gmail.com> wrote:
> On Tue, Dec 1, 2015 at 9:31 PM, Christian Couder
> <christian.cou...@gmail.com> wrote:
>> diff --git a/builtin/update-index.c b/builtin/update-index.c
>> index e568acc..b7b5108 100644
>> --- a/builtin/update-index.c
>> +++ b/builtin/update-index.c
>> @@ -996,8 +996,10 @@ int cmd_update_index(int argc, const char **argv, const 
>> char *prefix)
>> N_("enable or disable split index")),
>> OPT_BOOL(0, "untracked-cache", _cache,
>> N_("enable/disable untracked cache")),
>> +   OPT_SET_INT(0, "test-untracked-cache", _cache,
>> +   N_("test if the filesystem supports untracked 
>> cache"), 2),
>> OPT_SET_INT(0, "force-untracked-cache", _cache,
>> -   N_("enable untracked cache without testing the 
>> filesystem"), 2),
>> +   N_("enable untracked cache without testing the 
>> filesystem"), 3),
>> OPT_END()
>> };
>
> I think we got enough numbers to start using enum instead.

Ok, I will use an enum.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] config: add core.trustmtime

[Christian Couder]

On Wed, Dec 2, 2015 at 8:28 PM, Duy Nguyen  wrote:
> On Wed, Nov 25, 2015 at 10:00 AM, Ævar Arnfjörð Bjarmason
>  wrote:
>> Aside from the slight hassle of enabling this and keeping it enabled
>> this feature is great. It's sped up "git status" across the board by
>> about 40%. Slightly less than that on faster spinning disks, slightly
>> more than that on slower ones.
>
> Before I forget again, you should also enable split-index. That
> feature was added because index update time cut so much into the
> saving from untracked cache (unless you have small indexes, unlikely).
> And untracked cache can update the index often. Then maybe you can
> also think about improving the usability for it to ;-)

Yeah, we will probably have a look at split-index next.

Thanks for developing it too,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 7/8] update-index: prevent --untracked-cache from performing tests

[Christian Couder]

On Wed, Dec 2, 2015 at 8:18 PM, Duy Nguyen <pclo...@gmail.com> wrote:
> On Tue, Dec 1, 2015 at 9:31 PM, Christian Couder
> <christian.cou...@gmail.com> wrote:
>  diff --git a/t/t7063-status-untracked-cache.sh
> b/t/t7063-status-untracked-cache.sh
>> index 0e8d0d4..8c3e703 100755
>> --- a/t/t7063-status-untracked-cache.sh
>> +++ b/t/t7063-status-untracked-cache.sh
>> @@ -11,7 +11,7 @@ avoid_racy() {
>>  # It's fine if git update-index returns an error code other than one,
>>  # it'll be caught in the first test.
>
> Notice this comment. You probably have to chance --test-untr.. for the
> first test too if it's stilll true, or delete the comment.

Ok, I think I will remove the comment and still use "git update-index
--untracked-cache" in the first test.

>>  test_lazy_prereq UNTRACKED_CACHE '
>> -   { git update-index --untracked-cache; ret=$?; } &&
>> +   { git update-index --test-untracked-cache; ret=$?; } &&
>> test $ret -ne 1
>>  '

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 1/8] update-index: add untracked cache notifications

[Christian Couder]

On Wed, Dec 2, 2015 at 8:16 PM, Duy Nguyen <pclo...@gmail.com> wrote:
> On Tue, Dec 1, 2015 at 9:31 PM, Christian Couder
> <christian.cou...@gmail.com> wrote:
>> Doing:
>>
>>   cd /tmp
>>   git --git-dir=/git/somewhere/else/.git update-index --untracked-cache
>>
>> doesn't work how one would expect. It hardcodes "/tmp" as the directory
>> that "works" into the index, so if you use the working tree, you'll
>> never use the untracked cache.
>>
>> With this patch "git update-index --untracked-cache" tells the user in
>> which directory tests are performed and in which working directory the
>> untracked cache is allowed.
>>
>> Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
>> ---
>>  builtin/update-index.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/builtin/update-index.c b/builtin/update-index.c
>> index 7431938..e568acc 100644
>> --- a/builtin/update-index.c
>> +++ b/builtin/update-index.c
>> @@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void)
>> if (!mkdtemp(mtime_dir.buf))
>> die_errno("Could not make temporary directory");
>>
>> -   fprintf(stderr, _("Testing "));
>> +   fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd());
>
> We probably should respect --verbose. I know I violated it in the first place.

The verbose help is:

--verbose report actions to standard output

so yeah, it is not respected first because the output is on by
default, and second because the output is on stderr instead of stdout.
Anyway it can be a separate patch or patch series to make it respect
one or both of these points.

I am not very much interested in doing it myself as I think it's
interesting to have the output by default especially if the above
patch is applied. But if people agree that it would be a good thing, I
will do it.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 8/8] update-index: make core.untrackedCache a bool

[Christian Couder]

(Sorry I already sent a private reply to Tosten by mistake.)

On Sat, Dec 5, 2015 at 1:44 PM, Torsten Bögershausen <tbo...@web.de> wrote:
> On 01.12.15 21:31, Christian Couder wrote:
>> Most features in Git can be enabled or disabled using a simple
>> bool config variable and it would be nice if untracked cache
>> behaved the same way.
>>
>> This makes --[no-|force-]untracked-cache change the value of
>> core.untrackedCache in the repo config file, to avoid making
>> those options useless and because this avoids the untracked
>> cache being disabled by a kernel change or a directory
>> change. Of course this breaks some backward compatibility,
>> but the simplification and increased useability is worth it.
>
> Some loose thinking, how the core.untrackedcache and the
> command line can work together:
>
> core.untrackedcache = unset
>   everything is as today
>   if --test-untracked-cache is used on command line,
>   the test is run, additionally the result is stored
>   in the config variable core.untrackedcache

I don't like storing the result in core.untrackedcache as it means
that --test-untracked-cache would not just test.
And I agree with Aevar that it's a good thing to be able to completely
separate testing and configuring.

> core.untrackedcache = true
>   same as --force-untracked-cache
>   if --no-untracked-cache is given on command line,
>   this has higher priority

I guess you mean that --no-untracked-cache has priority over
"core.untrackedcache = true" without removing this config variable.
This means that --no-untracked-cache would only remove the untracked
cache (UC) from the index.

But what git should then do the next time "git status" is run?
Git sees that the index does not contain any UC, but it doesn't know
that "git update-index --no-untracked-cache" has just been run. It
might be "git config core.untrackedcache true" that has been run last.

If "git config core.untrackedcache true" has been run last, it would
be logical to add the UC to the index and use it.
If "git update-index --no-untracked-cache" has been run last, it would
be logical to not add the UC.

> core.untrackedcache = false
>   same as --no-untracked-cache
>   if --force-untracked-cache is given on command line,
>   this has higher priority

Then the same kind of problem happens as above. There is no clear
solution about what "git status" should do when the state of the index
conflicts with the value of core.untrackedcache.

> Does this support the workflow described by Ævar ?

I don't think so. I think that when we set "core.untrackedcache =
true" we want the UC to be added to the index and used as much as
possible, because we know that our OS/filesystem supports it.

This means that using "git update-index --no-untracked-cache" when
"core.untrackedcache = true" is set can only have two possible
outcomes. It could either just die saying that "core.untrackedcache"
is true, or it could just unset "core.untrackedcache" or set it to
false (which might mean the same thing).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] Documentation/git-update-index: add missing opts to synopsis

[Christian Couder]

On Wed, Nov 25, 2015 at 10:30 AM, Christian Couder
<christian.cou...@gmail.com> wrote:
> Split index related options should appear in the 'SYNOPSIS'
> section.
>
> These options are already documented in the 'OPTIONS' section.
>
> Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
> ---
> This v4 contains only the split-index options and applies on top
> of the new master that already contains the untracked-cache options.

It looks like this patch has not been applied.
Maybe I should have given it a different title to avoid confusion with
a previous patch that added [--[no-|force-]untracked-cache] in the
SYNOPSIS.

>  Documentation/git-update-index.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/git-update-index.txt 
> b/Documentation/git-update-index.txt
> index 3df9c26..f4e5a85 100644
> --- a/Documentation/git-update-index.txt
> +++ b/Documentation/git-update-index.txt
> @@ -17,6 +17,7 @@ SYNOPSIS
>  [--[no-]assume-unchanged]
>  [--[no-]skip-worktree]
>  [--ignore-submodules]
> +[--[no-]split-index]
>  [--[no-|force-]untracked-cache]
>  [--really-refresh] [--unresolve] [--again | -g]
>  [--info-only] [--index-info]
> --
> 2.6.3.380.g494b52d
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Draft of Git Rev News edition 10

[Christian Couder]

Hi,

A draft of Git Rev News edition 10 is available here:

  https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-10.md

Everyone is welcome to contribute in any section either by editing the
above page on GitHub and sending a pull request, or by commenting on
this GitHub issue:

  https://github.com/git/git.github.io/issues/112

You can also reply to this email.

I tried to cc everyone who appears in this edition but maybe I missed
some people, sorry about that.

Thomas, Nicola and myself plan to publish this edition on Wednesday
the 9th of December.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 6/8] config: add core.untrackedCache

[Christian Couder]

On Thu, Dec 3, 2015 at 5:10 PM, Torsten Bögershausen  wrote:
> [snip all good stuff]
>
> First of all:
> Thanks for explaining it so well
>
> I now can see the point in having this patch.
> (Do the commit messages reflect all this ? I need to re-read)

Maybe not. I will have a look at improving them, but your re-reading is welcome.

> The other question is: Do you have patch on a public repo ?

Yes, here: https://github.com/chriscool/git/commits/uc-notifs8

> And, of course, can we add 1 or 2 test cases ?

Yes I had planned to add tests for this.
But it would be nice if I could know first if the last two patches are
considered ok even though they are breaking compatibility a bit.
In this case I could squash them with previous patches and only write
tests for the resulting patches.

> Thanks for pushing this forward.

Thanks for your reviews,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[ANNOUNCE] Git Rev News edition 10

[Christian Couder]

Hi everyone,

I'm happy announce that the 10th edition of Git Rev News is now published:

https://git.github.io/rev_news/2015/12/09/edition-10/

It was supposed to be published yesterday but I got busy with other
things. Sorry about that.

Thanks a lot to all the contributors, especially Stefan!

Enjoy,
Christian, Thomas and Nicola.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 01/10] update-index: use enum for untracked cache options

[Christian Couder]

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 7431938..2430a68 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -35,6 +35,14 @@ static int mark_skip_worktree_only;
 #define UNMARK_FLAG 2
 static struct strbuf mtime_dir = STRBUF_INIT;
 
+/* Untracked cache mode */
+enum uc_mode {
+   UC_UNSPECIFIED = -1,
+   UC_DISABLE = 0,
+   UC_ENABLE,
+   UC_FORCE
+};
+
 __attribute__((format (printf, 1, 2)))
 static void report(const char *fmt, ...)
 {
@@ -902,7 +910,7 @@ static int reupdate_callback(struct parse_opt_ctx_t *ctx,
 int cmd_update_index(int argc, const char **argv, const char *prefix)
 {
int newfd, entries, has_errors = 0, line_termination = '\n';
-   int untracked_cache = -1;
+   enum uc_mode untracked_cache = UC_UNSPECIFIED;
int read_from_stdin = 0;
int prefix_length = prefix ? strlen(prefix) : 0;
int preferred_index_format = 0;
@@ -997,7 +1005,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "untracked-cache", _cache,
N_("enable/disable untracked cache")),
OPT_SET_INT(0, "force-untracked-cache", _cache,
-   N_("enable untracked cache without testing the 
filesystem"), 2),
+   N_("enable untracked cache without testing the 
filesystem"), UC_FORCE),
OPT_END()
};
 
@@ -1104,10 +1112,10 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
the_index.split_index = NULL;
the_index.cache_changed |= SOMETHING_CHANGED;
}
-   if (untracked_cache > 0) {
+   if (untracked_cache > UC_DISABLE) {
struct untracked_cache *uc;
 
-   if (untracked_cache < 2) {
+   if (untracked_cache < UC_FORCE) {
setup_work_tree();
if (!test_if_untracked_cache_is_supported())
return 1;
@@ -1122,7 +1130,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
}
add_untracked_ident(the_index.untracked);
the_index.cache_changed |= UNTRACKED_CHANGED;
-   } else if (!untracked_cache && the_index.untracked) {
+   } else if (untracked_cache == UC_DISABLE && the_index.untracked) {
the_index.untracked = NULL;
the_index.cache_changed |= UNTRACKED_CHANGED;
}
-- 
2.6.3.479.g8eb29d4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 00/10] Untracked cache improvements

[Christian Couder]

Here is a new version of a patch series to improve the untracked cache
feature.

This v2 still implements core.untrackedCache as a simple bool config
variable. When it's true, Git should always try to use the untracked
cache, and when false, Git should never use it.

Patchs 1/10 to 3/10 add some features that are missing. Patch 3/10 has
been moved after the two other patches and has been changed a bit
according to Duy's and Junio's suggestions. In patch 2/10 the enum
names have been changed as discussed with Junio.

Patchs 4/10, 5/10 and 6/10, which have not been changed, are some
refactoring to prepare for patch 8/10 which implements
core.untrackedCache.

Patch 7/10 is a small bug fix suggested by Junio.

Patch 8/10, which adds core.untrackedCache, contains many
documentation and commit message improvements, some by AEvar.

Patch 9/10 has not been changed.

Patch 10/10 is new and removes code that is now useless.

So the changes compared to v1 are mostly small updates, and patchs
7/10 and 10/10.

The patch series is also available there:

https://github.com/chriscool/git/tree/uc-notifs25

Thanks to the reviewers and helpers.

Christian Couder (10):
  update-index: use enum for untracked cache options
  update-index: add --test-untracked-cache
  update-index: add untracked cache notifications
  update-index: move 'uc' var declaration
  dir: add add_untracked_cache()
  dir: add remove_untracked_cache()
  dir: free untracked cache when removing it
  config: add core.untrackedCache
  t7063: add tests for core.untrackedCache
  dir: do not use untracked cache ident anymore

 Documentation/config.txt   |  7 
 Documentation/git-update-index.txt | 61 --
 builtin/update-index.c | 54 +-
 cache.h|  1 +
 config.c   |  4 +++
 contrib/completion/git-completion.bash |  1 +
 dir.c  | 53 +
 dir.h  |  4 ++-
 environment.c  |  1 +
 t/t7063-status-untracked-cache.sh  | 52 ++---
 wt-status.c|  9 +
 11 files changed, 178 insertions(+), 69 deletions(-)

-- 
2.6.3.479.g8eb29d4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 05/10] dir: add add_untracked_cache()

[Christian Couder]

Factor out code into add_untracked_cache(), which will be used
in a later commit.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 11 +--
 dir.c  | 14 ++
 dir.h  |  1 +
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index fffad79..5f009cf 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1123,16 +1123,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
if (untracked_cache == UC_TEST)
return 0;
}
-   if (!the_index.untracked) {
-   struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
-   strbuf_init(>ident, 100);
-   uc->exclude_per_dir = ".gitignore";
-   /* should be the same flags used by git-status */
-   uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES | 
DIR_HIDE_EMPTY_DIRECTORIES;
-   the_index.untracked = uc;
-   }
-   add_untracked_ident(the_index.untracked);
-   the_index.cache_changed |= UNTRACKED_CHANGED;
+   add_untracked_cache();
if (verbose)
printf(_("Untracked cache enabled\n"));
} else if (untracked_cache == UC_DISABLE && the_index.untracked) {
diff --git a/dir.c b/dir.c
index d2a8f06..0f7e293 100644
--- a/dir.c
+++ b/dir.c
@@ -1938,6 +1938,20 @@ void add_untracked_ident(struct untracked_cache *uc)
strbuf_addch(>ident, 0);
 }
 
+void add_untracked_cache(void)
+{
+   if (!the_index.untracked) {
+   struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
+   strbuf_init(>ident, 100);
+   uc->exclude_per_dir = ".gitignore";
+   /* should be the same flags used by git-status */
+   uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES | 
DIR_HIDE_EMPTY_DIRECTORIES;
+   the_index.untracked = uc;
+   }
+   add_untracked_ident(the_index.untracked);
+   the_index.cache_changed |= UNTRACKED_CHANGED;
+}
+
 static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct 
*dir,
  int base_len,
  const struct pathspec 
*pathspec)
diff --git a/dir.h b/dir.h
index 7b5855d..ee94c76 100644
--- a/dir.h
+++ b/dir.h
@@ -308,4 +308,5 @@ void free_untracked_cache(struct untracked_cache *);
 struct untracked_cache *read_untracked_extension(const void *data, unsigned 
long sz);
 void write_untracked_extension(struct strbuf *out, struct untracked_cache 
*untracked);
 void add_untracked_ident(struct untracked_cache *);
+void add_untracked_cache(void);
 #endif
-- 
2.6.3.479.g8eb29d4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 03/10] update-index: add untracked cache notifications

[Christian Couder]

Attempting to flip the untracked-cache feature on for a random index
file with

cd /random/unrelated/place
git --git-dir=/somewhere/else/.git update-index --untracked-cache

would not work as you might expect. Because flipping the feature on
in the index also records the location of the corresponding working
tree (/random/unrelated/place in the above example), when the index
is subsequently used to keep track of files in the working tree in
/somewhere/else, the feature is disabled.

With this patch "git update-index --[test-]untracked-cache" tells the
user in which directory tests are performed. This makes it easy to
spot any problem.

Also in verbose mode, let's tell the user when the cache is enabled
or disabled.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index e747a6c..e84674f 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -130,7 +130,7 @@ static int test_if_untracked_cache_is_supported(void)
if (!mkdtemp(mtime_dir.buf))
die_errno("Could not make temporary directory");
 
-   fprintf(stderr, _("Testing "));
+   fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd());
atexit(remove_test_directory);
xstat_mtime_dir();
fill_stat_data(, );
@@ -1135,9 +1135,13 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
}
add_untracked_ident(the_index.untracked);
the_index.cache_changed |= UNTRACKED_CHANGED;
+   if (verbose)
+   printf(_("Untracked cache enabled\n"));
} else if (untracked_cache == UC_DISABLE && the_index.untracked) {
the_index.untracked = NULL;
the_index.cache_changed |= UNTRACKED_CHANGED;
+   if (verbose)
+   printf(_("Untracked cache disabled\n"));
}
 
if (active_cache_changed) {
-- 
2.6.3.479.g8eb29d4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 10/10] dir: do not use untracked cache ident anymore

[Christian Couder]

A previous commit disabled the check to see if the untracked cache
ident field was matching the current environment. So this field is
now useless and we can remove some related code.

We don't remove the ident field from "struct untracked_cache" as
it would break compatibility with old indexes that already have an
untracked cache with this field.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 dir.c | 38 +-
 dir.h |  2 +-
 2 files changed, 6 insertions(+), 34 deletions(-)

diff --git a/dir.c b/dir.c
index 0b07ba7..94fba2a 100644
--- a/dir.c
+++ b/dir.c
@@ -1904,36 +1904,13 @@ static int treat_leading_path(struct dir_struct *dir,
return rc;
 }
 
-static const char *get_ident_string(void)
-{
-   static struct strbuf sb = STRBUF_INIT;
-   struct utsname uts;
-
-   if (sb.len)
-   return sb.buf;
-   if (uname() < 0)
-   die_errno(_("failed to get kernel name and information"));
-   strbuf_addf(, "Location %s, system %s %s %s", get_git_work_tree(),
-   uts.sysname, uts.release, uts.version);
-   return sb.buf;
-}
-
-static int ident_in_untracked(const struct untracked_cache *uc)
-{
-   const char *end = uc->ident.buf + uc->ident.len;
-   const char *p   = uc->ident.buf;
-
-   for (p = uc->ident.buf; p < end; p += strlen(p) + 1)
-   if (!strcmp(p, get_ident_string()))
-   return 1;
-   return 0;
-}
-
+/*
+ * We used to save the location of the work tree and the kernel version,
+ * but it was not a good idea, so we now just save an empty string.
+ */
 void add_untracked_ident(struct untracked_cache *uc)
 {
-   if (ident_in_untracked(uc))
-   return;
-   strbuf_addstr(>ident, get_ident_string());
+   strbuf_addstr(>ident, "");
/* this strbuf contains a list of strings, save NUL too */
strbuf_addch(>ident, 0);
 }
@@ -2015,11 +1992,6 @@ static struct untracked_cache_dir 
*validate_untracked_cache(struct dir_struct *d
if (dir->exclude_list_group[EXC_CMDL].nr)
return NULL;
 
-   if (use_untracked_cache != 1 && !ident_in_untracked(dir->untracked)) {
-   warning(_("Untracked cache is disabled on this system."));
-   return NULL;
-   }
-
if (!dir->untracked->root) {
const int len = sizeof(*dir->untracked->root);
dir->untracked->root = xmalloc(len);
diff --git a/dir.h b/dir.h
index 3e5114d..1935b76 100644
--- a/dir.h
+++ b/dir.h
@@ -127,7 +127,7 @@ struct untracked_cache {
struct sha1_stat ss_info_exclude;
struct sha1_stat ss_excludes_file;
const char *exclude_per_dir;
-   struct strbuf ident;
+   struct strbuf ident; /* unused now */
/*
 * dir_struct#flags must match dir_flags or the untracked
 * cache is ignored.
-- 
2.6.3.479.g8eb29d4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 04/10] update-index: move 'uc' var declaration

[Christian Couder]

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index e84674f..fffad79 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1116,8 +1116,6 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
the_index.cache_changed |= SOMETHING_CHANGED;
}
if (untracked_cache > UC_DISABLE) {
-   struct untracked_cache *uc;
-
if (untracked_cache < UC_FORCE) {
setup_work_tree();
if (!test_if_untracked_cache_is_supported())
@@ -1126,7 +1124,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
return 0;
}
if (!the_index.untracked) {
-   uc = xcalloc(1, sizeof(*uc));
+   struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
strbuf_init(>ident, 100);
uc->exclude_per_dir = ".gitignore";
/* should be the same flags used by git-status */
-- 
2.6.3.479.g8eb29d4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 08/10] config: add core.untrackedCache

[Christian Couder]

When we know that mtime is fully supported by the environment, we
might want the untracked cache to be always used by default without
any mtime test or kernel version check being performed.

Also when we know that mtime is not supported by the environment,
for example because the repo is shared over a network file system,
then we might want 'git update-index --untracked-cache' to fail
immediately instead of performing tests (because it might work on
some systems using the repo over the network file system but not
others).

The normal way to achieve the above in Git is to use a config
variable. That's why this patch introduces "core.untrackedCache".

To keep things simple, this variable is a bool which default to
false.

When "git status" is run, it now adds or removes the untracked
cache in the index to respect the value of this variable.

The job of `git update-index --[no-|force-]untracked-cache` was
to add or remove the untracked cache from the index. This was a
kind of configuration because this was persistant across git
commands. To make this kind of configuration compatible with the
new config variable, the simple thing to do, and what this patch
does, is to make `git update-index --[no-|force-]untracked-cache`
set or unset this config option.

This new behavior is a backward incompatible change, but that is
deliberate. The untracked cache feature has been experimental
and is very unlikely used by beginners.

When people will upgrade, this will remove any untracked cache
they used unless they set "core.untrackedCache" before upgrading.
This should be stated in the release notes.

Also `--untracked-cache` used to check that the underlying
operating system and file system change `st_mtime` field of a
directory if files are added or deleted in that directory. But
those tests take a long time and there is now
`--test-untracked-cache` to perform them.

That's why, to be more consistent with other git commands, this
patch prevents `--untracked-cache` to perform tests, so that
after this patch there is no difference any more between
`--untracked-cache` and `--force-untracked-cache`.

All the changes to `--[no-|force-]untracked-cache` make it
possible to deprecate those options in the future.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <ava...@gmail.com>
---
 Documentation/config.txt   |  7 
 Documentation/git-update-index.txt | 58 +++---
 builtin/update-index.c | 25 ---
 cache.h|  1 +
 config.c   |  4 +++
 contrib/completion/git-completion.bash |  1 +
 dir.c  |  2 +-
 environment.c  |  1 +
 t/t7063-status-untracked-cache.sh  |  4 +--
 wt-status.c|  9 ++
 10 files changed, 85 insertions(+), 27 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2d06b11..94820eb 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -308,6 +308,13 @@ core.trustctime::
crawlers and some backup systems).
See linkgit:git-update-index[1]. True by default.
 
+core.untrackedCache::
+   Determines if untracked cache will be enabled. Using
+   'git update-index --[no-|force-]untracked-cache' will set
+   this variable. Before setting it to true, you should check
+   that mtime is working properly on your system.
+   See linkgit:git-update-index[1]. False by default.
+
 core.checkStat::
Determines which stat fields to match between the index
and work tree. The user can set this to 'default' or
diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index 0ff7396..cd02de4 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -173,24 +173,29 @@ may not support it yet.
 
 --untracked-cache::
 --no-untracked-cache::
-   Enable or disable untracked cache extension. This could speed
-   up for commands that involve determining untracked files such
-   as `git status`. The underlying operating system and file
-   system must change `st_mtime` field of a directory if files
-   are added or deleted in that directory.
+   Enable or disable untracked cache extension. Please use
+   `--test-untracked-cache` before enabling it.
++
+These options are mostly aliases for setting the `core.untrackedCache`
+configuration variable to 'true' or 'false' in the local config file
+(see linkgit:git-config[1]). You can equivalently just set those
+configuration values directly. These options are just provided for
+backwards compatibility with the older versions of Git where this was
+the only way to enable or disable the untracked cache extension.
 
 --test-untracked-cache::
Only perform tests on the working directory to make sure
   

[PATCH v2 06/10] dir: add remove_untracked_cache()

[Christian Couder]

Factor out code into remove_untracked_cache(), which will be used
in a later commit.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 3 +--
 dir.c  | 6 ++
 dir.h  | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 5f009cf..4ca6d94 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1127,8 +1127,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
if (verbose)
printf(_("Untracked cache enabled\n"));
} else if (untracked_cache == UC_DISABLE && the_index.untracked) {
-   the_index.untracked = NULL;
-   the_index.cache_changed |= UNTRACKED_CHANGED;
+   remove_untracked_cache();
if (verbose)
printf(_("Untracked cache disabled\n"));
}
diff --git a/dir.c b/dir.c
index 0f7e293..ffc0286 100644
--- a/dir.c
+++ b/dir.c
@@ -1952,6 +1952,12 @@ void add_untracked_cache(void)
the_index.cache_changed |= UNTRACKED_CHANGED;
 }
 
+void remove_untracked_cache(void)
+{
+   the_index.untracked = NULL;
+   the_index.cache_changed |= UNTRACKED_CHANGED;
+}
+
 static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct 
*dir,
  int base_len,
  const struct pathspec 
*pathspec)
diff --git a/dir.h b/dir.h
index ee94c76..3e5114d 100644
--- a/dir.h
+++ b/dir.h
@@ -309,4 +309,5 @@ struct untracked_cache *read_untracked_extension(const void 
*data, unsigned long
 void write_untracked_extension(struct strbuf *out, struct untracked_cache 
*untracked);
 void add_untracked_ident(struct untracked_cache *);
 void add_untracked_cache(void);
+void remove_untracked_cache(void);
 #endif
-- 
2.6.3.479.g8eb29d4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/8] config: add core.untrackedCache

[Christian Couder]

On Tue, Dec 15, 2015 at 10:49 AM, Torsten Bögershausen <tbo...@web.de> wrote:
> On 15.12.15 10:34, Christian Couder wrote:
>> On Mon, Dec 14, 2015 at 10:30 PM, Junio C Hamano <gits...@pobox.com> wrote:
>>> Junio C Hamano <gits...@pobox.com> writes:
>>>
>>> The primary reason why I do not like your "configuration decides" is
>>> it will be a huge source of confusions and bugs.  Imagine what
>>> should happen in this sequence, and when should a stale cached
>>> information be discarded?
>>>
>>>  - the configuration is set to 'yes'.
>>>  - the index is updated and written by various commands.
>>>  - more work is done in the working tree without updating the index.
>>>  - the configuration is set to 'no'.
>>>  - more work is done in the working tree without updating the index.
>>>  - the configuration is set to 'yes'.
>>>  - more work is done in the working tree without updating the index.
>>>  - somebody asks "what untracked paths are there?"
>>
>
>> As far as I understand the UC just stores the mtime of the directories
>> in the working tree to avoid the need of lstat'ing all the files in
>> the directories.
>
> This is what I understand:
> UC stores the mtime of the directories in the working tree to avoid the need
> opendir() readdir() closedir() to find new, yet untracked, files.
> (including sub-directories)

I think you are probably right too.

In the v2 patch series I just sent, there is:

+This feature works by recording the mtime of the working tree
+directories and then omitting reading directories and stat calls
+against files in those directories whose mtime hasn't changed.

I hope it is better.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 09/10] t7063: add tests for core.untrackedCache

[Christian Couder]

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 t/t7063-status-untracked-cache.sh | 48 +--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/t/t7063-status-untracked-cache.sh 
b/t/t7063-status-untracked-cache.sh
index 253160a..f0af41c 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -18,6 +18,10 @@ if ! test_have_prereq UNTRACKED_CACHE; then
test_done
 fi
 
+test_expect_success 'core.untrackedCache is unset' '
+   test_must_fail git config --get core.untrackedCache
+'
+
 test_expect_success 'setup' '
git init worktree &&
cd worktree &&
@@ -28,6 +32,11 @@ test_expect_success 'setup' '
git update-index --untracked-cache
 '
 
+test_expect_success 'core.untrackedCache is true' '
+   UC=$(git config core.untrackedCache) &&
+   test "$UC" = "true"
+'
+
 test_expect_success 'untracked cache is empty' '
test-dump-untracked-cache >../actual &&
cat >../expect <../actual &&
-   cat >../expect <../expect-from-test-dump <../actual &&
+   echo "no untracked cache" >../expect &&
+   test_cmp ../expect ../actual
+'
+
+test_expect_success 'git status does not change anything' '
+   git status &&
+   test-dump-untracked-cache >../actual &&
+   test_cmp ../expect ../actual &&
+   UC=$(git config core.untrackedCache) &&
+   test "$UC" = "false"
+'
+
+test_expect_success 'setting core.untrackedCache and using git status creates 
the cache' '
+   git config core.untrackedCache true &&
+   test-dump-untracked-cache >../actual &&
+   test_cmp ../expect ../actual &&
+   git status &&
+   test-dump-untracked-cache >../actual &&
+   test_cmp ../expect-from-test-dump ../actual
+'
+
+test_expect_success 'unsetting core.untrackedCache and using git status 
removes the cache' '
+   git config --unset core.untrackedCache &&
+   test-dump-untracked-cache >../actual &&
+   test_cmp ../expect-from-test-dump ../actual &&
+   git status &&
+   test-dump-untracked-cache >../actual &&
+   test_cmp ../expect ../actual
+'
+
 test_done
-- 
2.6.3.479.g8eb29d4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/8] config: add core.untrackedCache

[Christian Couder]

On Tue, Dec 15, 2015 at 11:02 AM, Duy Nguyen <pclo...@gmail.com> wrote:
> On Tue, Dec 15, 2015 at 4:34 PM, Christian Couder
> <christian.cou...@gmail.com> wrote:
>> On Mon, Dec 14, 2015 at 10:30 PM, Junio C Hamano <gits...@pobox.com> wrote:
>>> Junio C Hamano <gits...@pobox.com> writes:
>>>
>>> The primary reason why I do not like your "configuration decides" is
>>> it will be a huge source of confusions and bugs.  Imagine what
>>> should happen in this sequence, and when should a stale cached
>>> information be discarded?
>>>
>>>  - the configuration is set to 'yes'.
>>>  - the index is updated and written by various commands.
>>>  - more work is done in the working tree without updating the index.
>>>  - the configuration is set to 'no'.
>>>  - more work is done in the working tree without updating the index.
>>>  - the configuration is set to 'yes'.
>>>  - more work is done in the working tree without updating the index.
>>>  - somebody asks "what untracked paths are there?"
>>
>> As far as I understand the UC just stores the mtime of the directories
>> in the working tree to avoid the need of lstat'ing all the files in
>> the directories.
>>
>> When somebody asks "what untracked paths are there", if the UC has not
>> been discarded when the configuration was set to no, then git will
>> just ask for the mtimes of the directories in the working tree and
>> compare them with what is in the UC.
>>
>> I don't see how it can create confusion and bugs, as the work done in
>> the working tree should anyway have changed the mtime of the
>> directories where work has been done.
>
> Any operation that can add or remove an entry from the index may lead
> to UC update. For example, if file "foo" is tracked, then the user
> does "git rm --cached foo", "foo" may become either untracked or
> ignored. So if you disable UC while doing this removal, then re-enable
> UC again, "git-status" may show incorrect output. So, as long as UC
> extension exists in the index, it must be updated, or it may become
> outdated and useless.

When UC is disabled, it is removed from the index, and when it is
(re-)enabled, it is recreated, so I don't think that your example
applies to the code in this patch.

Thanks anyway for this insight,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 07/10] dir: free untracked cache when removing it

[Christian Couder]

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 dir.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dir.c b/dir.c
index ffc0286..3b83cc0 100644
--- a/dir.c
+++ b/dir.c
@@ -1954,6 +1954,7 @@ void add_untracked_cache(void)
 
 void remove_untracked_cache(void)
 {
+   free_untracked_cache(the_index.untracked);
the_index.untracked = NULL;
the_index.cache_changed |= UNTRACKED_CHANGED;
 }
-- 
2.6.3.479.g8eb29d4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 02/10] update-index: add --test-untracked-cache

[Christian Couder]

It is nice to just be able to test if untracked cache is
supported without enabling it.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 Documentation/git-update-index.txt | 9 -
 builtin/update-index.c | 5 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index 3df9c26..0ff7396 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -17,7 +17,7 @@ SYNOPSIS
 [--[no-]assume-unchanged]
 [--[no-]skip-worktree]
 [--ignore-submodules]
-[--[no-|force-]untracked-cache]
+[--[no-|test-|force-]untracked-cache]
 [--really-refresh] [--unresolve] [--again | -g]
 [--info-only] [--index-info]
 [-z] [--stdin] [--index-version ]
@@ -179,6 +179,13 @@ may not support it yet.
system must change `st_mtime` field of a directory if files
are added or deleted in that directory.
 
+--test-untracked-cache::
+   Only perform tests on the working directory to make sure
+   untracked cache can be used. You have to manually enable
+   untracked cache using `--force-untracked-cache` (or
+   `--untracked-cache` but this will run the tests again)
+   afterwards if you really want to use it.
+
 --force-untracked-cache::
For safety, `--untracked-cache` performs tests on the working
directory to make sure untracked cache can be used. These
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 2430a68..e747a6c 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -40,6 +40,7 @@ enum uc_mode {
UC_UNSPECIFIED = -1,
UC_DISABLE = 0,
UC_ENABLE,
+   UC_TEST,
UC_FORCE
 };
 
@@ -1004,6 +1005,8 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
N_("enable or disable split index")),
OPT_BOOL(0, "untracked-cache", _cache,
N_("enable/disable untracked cache")),
+   OPT_SET_INT(0, "test-untracked-cache", _cache,
+   N_("test if the filesystem supports untracked 
cache"), UC_TEST),
OPT_SET_INT(0, "force-untracked-cache", _cache,
N_("enable untracked cache without testing the 
filesystem"), UC_FORCE),
OPT_END()
@@ -1119,6 +1122,8 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
setup_work_tree();
if (!test_if_untracked_cache_is_supported())
return 1;
+   if (untracked_cache == UC_TEST)
+   return 0;
}
if (!the_index.untracked) {
uc = xcalloc(1, sizeof(*uc));
-- 
2.6.3.479.g8eb29d4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/8] config: add core.untrackedCache

[Christian Couder]

On Tue, Dec 8, 2015 at 11:43 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Junio C Hamano <gits...@pobox.com> writes:
>
>> Christian Couder <christian.cou...@gmail.com> writes:
>>
>>> When we know that mtime is fully supported by the environment, we
>>> might want the untracked cache to be always used by default without
>>> any mtime test or kernel version check being performed.
>>>
>>> Also when we know that mtime is not supported by the environment,
>>> for example because the repo is shared over a network file system,
>>> then we might want 'git update-index --untracked-cache' to fail
>>> immediately instead of preforming tests (because it might work on
>>> some systems using the repo over the network file system but not
>>> others).
>>> ...
>> The logic in this paragraph is fuzzy to me.  Shouldn't the config
>> give a sensible default, that is overriden by command line options?

The problem is that "git update-index --[no-|force-]untracked-cache"
is not just changing things for the duration of the current command.
It is in fact changing the configuration of the repository, because it
is adding the UC (untracked cache) in the index and saving the index,
which will persist the use of the UC for the following commands.

So it is very different from something like "git status
--no-untracked-cache" that would perform a "git status" without using
the UC.

In fact "git update-index --[no-|force-]untracked-cache" is very bad
because it means that two repositories can be configured differently
even if they have the same config files.

>> I agree that it is insane to do a runtime check when the user says
>> "update-index --untracked-cache" to enable it, as the user _knows_
>> that enabling it would help (or the user _knows_ that she wants to
>> play with it).  Similarly, shouldn't the config be ignored when the
>> user says "update-index --no-untracked-cache" (hence removing the
>> untracked cache from the resulting index no matter the config is set
>> to)?  ...
>
> As I think about this more, it really seems to me that we shouldn't
> need to make this configuration variable that special.  Because I
> think it is a *BUG* in the current implementation to do the runtime
> check even when the user explicitly tells us to use untracked-cache,
> I'd imagine something along the lines of the following would be a
> lot simpler, easier to understand and a generally more useful
> bugfix:
>
>  1 Add one boolean configuration variable, core.untrackedCache, that
>defaults to 'false'.
>
>  2 When creating an index file in an empty repository, enable the
>untracked cache in the index (even without the user runninng
>"update-index --untracked-cache") iff the configuration is set to
>'true'.  No runtime check necessary.

I guess this means that when cloning a repo, it would not use the UC
unless either "git -c core.untrackedCache=true clone ..." is used, or
core.untrackedCache is set in the global config.

>  3 When working on an existing index file, unless the operation is
>"update-index --[no-]untracked-cache", keep the current setting,
>regardless of this configuration variable.  No runtime check
>necessary.
>
>  4 "update-index --[no-]untracked-cache" should enable or disable
>the untracked cache as the user tells us, regardless of the
>configuration variable.  No runtime check necessary.

If you want only some repos to use the UC, you will set
core.untrackedCache in the repo config. Then after cloning such a
repo, you will copy the config file, and this will not be enough to
enable the UC. The original repo will use the UC but the cloned one
will not, and you might wonder why is "git status" slower in the
cloned repo despite the machines and the config being the same for
both repos?

And if you have set core.untrackedCache in the global config when you
clone, UC is enabled, but if you have just set it in the repo config
after the clone, it is not enabled.

This is quite bad in my opinion.

Also think about system administrators who have a lot of machines with
lots of repos everywhere on these machines and just upgrade Git from
let's say v2.4.0 to v2.8.0. They find out that using UC git status
will be faster and want to provide that to the machine's users knowing
that they only use recent Linux machines where mtime works well.
Shouldn't it be nice if they could just enable core.untrackedCache in
the global config files without having to also cd into every repo and
use "git update-index --untracked-cache" there?

If we consider that "git update-index --[no-|force-]untracked-cache"
should not have bee

Re: [PATCH 7/8] config: add core.untrackedCache

[Christian Couder]

On Mon, Dec 14, 2015 at 10:30 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
> The primary reason why I do not like your "configuration decides" is
> it will be a huge source of confusions and bugs.  Imagine what
> should happen in this sequence, and when should a stale cached
> information be discarded?
>
>  - the configuration is set to 'yes'.
>  - the index is updated and written by various commands.
>  - more work is done in the working tree without updating the index.
>  - the configuration is set to 'no'.
>  - more work is done in the working tree without updating the index.
>  - the configuration is set to 'yes'.
>  - more work is done in the working tree without updating the index.
>  - somebody asks "what untracked paths are there?"

As far as I understand the UC just stores the mtime of the directories
in the working tree to avoid the need of lstat'ing all the files in
the directories.

When somebody asks "what untracked paths are there", if the UC has not
been discarded when the configuration was set to no, then git will
just ask for the mtimes of the directories in the working tree and
compare them with what is in the UC.

I don't see how it can create confusion and bugs, as the work done in
the working tree should anyway have changed the mtime of the
directories where work has been done.

Maybe as the UC has not been updated for a long time, there will be a
lot of mtimes that are different, so there will not be a big speed up
or it could be even slower than if the UC was not used, but that's
all.

> In the "configuration decides" world, I am not sure how a sane
> implementation efficiently invalidates the cache as needed, without
> the config subsystem having intimate knowledge with the untracked
> cache (so far, the config subsystem is merely a key-value store and
> does not care _what_ it stores; you would want to invalidate the
> cache in the index when somebody sets the variable to 'no', which
> means the config subsystem needs to know that this variable is
> special).

In the current patch series and in the one I am preparing and will
probably send soon, this hunk takes care of removing or addind the UC
if needed:

diff --git a/wt-status.c b/wt-status.c
index 435fc28..3e0fe02 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -586,6 +586,15 @@ static void wt_status_collect_untracked(struct
wt_status *s)
dir.flags |= DIR_SHOW_IGNORED_TOO;
else
dir.untracked = the_index.untracked;
+
+   if (!dir.untracked && use_untracked_cache == 1) {
+   add_untracked_cache();
+   dir.untracked = the_index.untracked;
+   } else if (dir.untracked && use_untracked_cache == 0) {
+   remove_untracked_cache();
+   dir.untracked = NULL;
+   }
+
setup_standard_excludes();

fill_directory(, >pathspec);

So when the config option is changed, the UC is removed or recreated
only the next time "git status" (and maybe also "git commit" and a few
other commands) is called.

And anyway I don't think people will change the UC config very often.
Maybe they will play with it a bit when they discover it, but
afterwards they will just set it or not and be done with it. So I
think it is not worth trying to optimize what happens when the config
is set or unset. We should just make sure that it works and it is well
documented.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] update-index: use enum for untracked cache options

[Christian Couder]

On Fri, Dec 11, 2015 at 6:44 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Christian Couder <christian.cou...@gmail.com> writes:
>
>>> As you know I am bad at bikeshedding; the only suggestion in the
>>> above is to have common UC_ prefix ;-)  Don't take what follow UC_
>>> as my suggestion.
>>
>> I am bad at finding names too, so I think what you wrote is pretty good.
>>
>> I thought about the following:
>>
>>  UC_UNDEF
>>  UC_DISABLE
>>  UC_ENABLE
>>  UC_TEST
>>  UC_FORCE
>>
>> but I am not sure it is much better.
>
> I hated the DONTUSE/USE I listed, and I think DISABLE/ENABLE is much
> much better.  I do prefer unspecified over undef, though.

Ok, then it will be:

UC_UNSPECIFIED
UC_DISABLE
UC_ENABLE
UC_TEST
UC_FORCE

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] update-index: use enum for untracked cache options

[Christian Couder]

On Thu, Dec 10, 2015 at 7:46 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Christian Couder <christian.cou...@gmail.com> writes:
>
>>>> +/* Untracked cache mode */
>>>> +enum uc_mode {
>>>> + UNDEF_UC = -1,
>>>> + NO_UC = 0,
>>>> + UC,
>>>> + FORCE_UC
>>>> +};
>>>> +
>>>
>>> With these, the code is much easier to read than with the mystery
>>> constants, but did you consider making UC_ a common prefix for
>>> further ease-of-reading?  E.g.
>>>
>>> UC_UNSPECIFIED
>>> UC_DONTUSE
>>> UC_USE
>>> UC_FORCE
>>>
>>> or something?
>>
>> Ok, I will use what you suggest.
>
> As you know I am bad at bikeshedding; the only suggestion in the
> above is to have common UC_ prefix ;-)  Don't take what follow UC_
> as my suggestion.

I am bad at finding names too, so I think what you wrote is pretty good.

I thought about the following:

 UC_UNDEF
 UC_DISABLE
 UC_ENABLE
 UC_TEST
 UC_FORCE

but I am not sure it is much better.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 10/10] dir: do not use untracked cache ident anymore

[Christian Couder]

On Tue, Dec 15, 2015 at 8:49 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Christian Couder <christian.cou...@gmail.com> writes:
>
>> +/*
>> + * We used to save the location of the work tree and the kernel version,
>> + * but it was not a good idea, so we now just save an empty string.
>> + */
>
> I do agree that storing the kernel version (or hostname or whatever
> specific to the machine) was not a good idea.  I however suspect
> that you must save and check the location of the working tree,
> though, for correctness.  If you use one GIT_DIR and GIT_WORK_TREE
> to do "git add" or whatever, and then use the same GIT_DIR but a
> different GIT_WORK_TREE, you should be able to notice that a
> directory D in the old GIT_WORK_TREE whose modification time you
> recorded is different from the directory D with the same name but in
> the new GIT_WORK_TREE, no?

Yeah, if people use many worktrees made using "git worktree", the code
above should be fine because there is one index per worktree, but if
they just use one GIT_DIR with many GIT_WORK_TREE then there will be
only one index used.

I am wondering about the case when the worktree is moved and then
GIT_WORK_TREE is changed to reflect the new path were the worktree has
been moved.

In the "git worktree" documentation there is:

"If you move a linked working tree to another file system, or within a
file system that does not support hard links, you need to run at least
one git command inside the linked working tree (e.g. git status) in
order to update its administrative files in the repository so that
they do not get automatically pruned."

It looks like git can detect when a worktree created with "git
worktree" has been moved and I wonder if it would be possible to
detect if the main worktree pointed to by GIT_WORK_TREE as moved.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 10/10] dir: do not use untracked cache ident anymore

[Christian Couder]

On Thu, Dec 17, 2015 at 7:33 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Christian Couder <christian.cou...@gmail.com> writes:
>
>> In the "git worktree" documentation there is:
>>
>> "If you move a linked working tree to another file system, or within a
>> file system that does not support hard links, you need to run at least
>> one git command inside the linked working tree (e.g. git status) in
>> order to update its administrative files in the repository so that
>> they do not get automatically pruned."
>>
>> It looks like git can detect when a worktree created with "git
>> worktree" has been moved and I wonder if it would be possible to
>> detect if the main worktree pointed to by GIT_WORK_TREE as moved.
>
> As I personally do not find "moving a working tree" a very
> compelling use case, I'd be fine if cached information is not used
> when the actual worktree and the root of the cached untracked paths
> are different.

Yeah, I could just discard and recreate the UC from scratch if the
actual worktree and the root of the UC paths are different.

> If you are going to change the in-index data of untracked cache
> anyway (like you attempted with 10/10 patch), I think a lot more
> sensible simplification may be to make the mechanism _always_ keep
> track of the worktree that is rooted one level above the index, and
> not use the cache in all other cases.  That way, if you move the
> working tree in its entirety (i.e. $foo/{Makefile,.git/,untracked}
> all move to $bar/. at the same time), the untracked cache data that
> was in $foo/.git/index, which knew about $foo/untracked, will now
> know about $bar/untracked when the index is moved to $bar/.git/index
> automatically.

I am ok with that, though I worry a bit about some people having a
setup where they always use a worktree that is not one level above the
.git directory.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/8] config: add core.untrackedCache

[Christian Couder]

On Tue, Dec 15, 2015 at 2:04 PM, Ævar Arnfjörð Bjarmason
<ava...@gmail.com> wrote:
> On Mon, Dec 14, 2015 at 8:44 PM, Junio C Hamano <gits...@pobox.com> wrote:
>
> I'm replying to & quoting from two E-Mails of yours at once here for
> clarity & less noise. I'm working wich Christian on getting this
> integrated, and we both thought it would be good to have some fresh
> input on the matter from me.
>
>> Christian Couder <christian.cou...@gmail.com> writes:
>
>>> If you want only some repos to use the UC, you will set
>>> core.untrackedCache in the repo config. Then after cloning such a
>>> repo, you will copy the config file, and this will not be enough to
>>> enable the UC.
>>
>> Surely.  "Does this index file keeps track of the untracked files'
>> states?" is a property of the index.  Cloning does not propagate the
>> configuration and copying or not copying is irrelevant.  If you want
>> to enable, running "update-index --untracked-cache" is a way to do
>> so.  I cannot see what's so hard about it.
>>
>>> And if you have set core.untrackedCache in the global config when you
>>> clone, UC is enabled, but if you have just set it in the repo config
>>> after the clone, it is not enabled.
>>
>> That's fine.  In your patch series, if you set it in the global, you
>> will get the cache in the new one.  With the cleaned-up semantics I
>> suggested, the same thing will happen.
>>
>> And with the cleaned-up semantics, the configuration is *ONLY* used
>> to give the *DEFAULT* before other things happen, i.e. creation of
>> the index file for the first time.  Because the configuration is
>> only the default, an explicit "update-index --[no-]untracked-cache"
>> will defeat it, just like any other config/option interaction.
>
> As you know Christian is working on this for Booking.com to integrate
> features we find useful into git.git in such a way that we don't have
> to maintain some internal fork of Git.
>
> What we're trying to do, and what a lot of other big deployments of
> Git elsewhere would also find useful, is to ship a default sensible
> configuration for all users on the system in /etc/gitconfig.
>
> I'd like to be able to easily enable some feature that aids Git
> performance globally on our thousands of machines and for our hundreds
> of users by just tweaking something in puppet to change
> /etc/gitconfig, and more importantly if that change ends up being bad
> reverting that config in /etc/gitconfig should undo the change.
>
> It's an unacceptable level of complexity for system-level automation
> to have to scour the filesystem for existing Git repositories and run
> "git update-index" on each of them, that's why we're submitting
> patches to make this a config option, so we can simply flip a flag in
> /etc/gitconfig.
>
> It's also unacceptable to have the config simply provide the default
> which'll be frozen either at clone time or after an initial "git
> status".
>
> Let's say I ship a /etc/gitconfig that says "new clones should use the
> untracked cache". Now I roll that out across our fleet of machines and
> it turns out the morning after that the feature doesn't work properly
> for whatever reason. If it's just a "default until clone or status"
> type of thing even if I revert the configuration a lot of users &
> their repositories in the wild will still be broken, and will have to
> be manually fixed. Which again leads to the scouring the filesystem
> problem.
>
> So that gives some more context for why we're pushing for this change.
> I believe this feature breaks no existing use-case and just supports
> new ones, and I think that your objections to it are based on a simple
> misunderstanding as will become apparent if you read on below.
>
>> The biggest issue I had with your patch series, IIRC, is that
>> configuration will defeat the command line option.
>
> I think it's a moot point to focus on configuration v.s. command-line
> option. The important question is whether or not this feature can
> still be configured on a repo-local basis with this series as before.
> That's still the case since --local git configuration overrides
> --global and --system, so users who want to enable/disable this
> per-repo still can.
>
>>> Shouldn't it be nice if they could just enable core.untrackedCache in
>>> the global config files without having to also cd into every repo and
>>> use "git update-index --untracked-cache" there?
>>
>> NO.  It is bad to change the behaviour behind users' back.
>
> I'm not qui

Re: [PATCH 2/8] update-index: use enum for untracked cache options

[Christian Couder]

On Tue, Dec 8, 2015 at 8:11 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Christian Couder <christian.cou...@gmail.com> writes:
>
>> Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
>> ---
>>  builtin/update-index.c | 18 +-
>>  1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/builtin/update-index.c b/builtin/update-index.c
>> index 6f6b289..246b3d3 100644
>> --- a/builtin/update-index.c
>> +++ b/builtin/update-index.c
>> @@ -35,6 +35,14 @@ static int mark_skip_worktree_only;
>>  #define UNMARK_FLAG 2
>>  static struct strbuf mtime_dir = STRBUF_INIT;
>>
>> +/* Untracked cache mode */
>> +enum uc_mode {
>> + UNDEF_UC = -1,
>> + NO_UC = 0,
>> + UC,
>> + FORCE_UC
>> +};
>> +
>
> With these, the code is much easier to read than with the mystery
> constants, but did you consider making UC_ a common prefix for
> further ease-of-reading?  E.g.
>
> UC_UNSPECIFIED
> UC_DONTUSE
> UC_USE
> UC_FORCE
>
> or something?

Ok, I will use what you suggest.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] update-index: add untracked cache notifications

[Christian Couder]

On Tue, Dec 8, 2015 at 8:03 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Christian Couder <christian.cou...@gmail.com> writes:
>
>> Doing:
>>
>>   cd /tmp
>>   git --git-dir=/git/somewhere/else/.git update-index --untracked-cache
>>
>> doesn't work how one would expect. It hardcodes "/tmp" as the directory
>> that "works" into the index, so if you use the working tree, you'll
>> never use the untracked cache.
>
> I think your "expectation" needs to be more explicitly spelled out.
>
> "git -C /tmp --git-dir=/git/somewhere/else/.git" is a valid way to
> use that repository you have in somewhere else to track things under
> /tmp/ (as you are only passing GIT_DIR but not GIT_WORK_TREE, the
> cwd, i.e. /tmp, is the root level of the working tree), and for such
> a usage, the above command works as expected.  Perhaps
>
> Attempting to flip the untracked-cache feature on for a random index
> file with
>
>cd /random/unrelated/place
>git --git-dir=/somewhere/else/.git update-index --untracked-cache
>
> would not work as you might expect.  Because flipping the
> feature on in the index also records the location of the
> corresponding working tree (/random/unrelated/place in the above
> example), when the index is subsequently used to keep track of
> files in the working tree in /somewhere/else, the feature is
> disabled.
>
> may be an improvement.

Yeah, I agree that it is better. I have included your explanations in
the next version I will send. Thanks.

> The index already implicitly records where the working tree was and
> that is not limited to untracked-cache option.  For example, if you
> have your repository and its working tree in /git/somewhere/else,
> which does not have a path X, then doing:
>
> cd /tmp && >tmp/X
> git --git-dir=/git/somewhere/else/.git update-index --add X
>
> would store X taken from /tmp in the index, so subsequent use of the
> index "knows" about X that was taken outside /git/somewhere/else/
> after the above command finishes and the subsequent use is made
> without the --git-dir parameter, e.g.
>
> cd /git/somewhere/else/ && git diff-index --cached HEAD'
>
> would say that you added X, even though /git/somewhere/else/ may not
> have that X at all.  And this is not limited to update-index,
> either.  You can temporarily use --git-dir with "git add X" and the
> result would persist the same way in the index.
>
> I think the moral of the story is that you are not expected to
> randomly use git-dir and git-work-tree to point at different places
> without knowing what you are doing, and we may need a way to help
> people understand what is going on when it is done by a mistake.

Yeah, I agree, and I think displaying more information might be a good way.

> The patch to show result from xgetcwd() and get_git_work_tree() may
> be a step in the right direction, but if the user is not doing
> anything fancy, "Testing mtime in /long/path/to/the/directory" would
> be irritatingly verbose.

Yeah, but after this series only "--test-untracked-cache" does any
testing, and the 10 seconds time it takes are probably more irritating
than its output.

> I wonder if it is easy to tell when the user is doing such an
> unnatural thing.  Off the top of my head, when the working tree is
> anywhere other than one level above $GIT_DIR, the user is doing
> something unusual; I do not know if there are cases where the user
> is doing something unnatural if $GIT_WORK_TREE is one level above
> $GIT_DIR, though.

Yeah, it could only print a warning in case something unusual is done.
I am not sure it is worth it though.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/8] dir: add add_untracked_cache()

[Christian Couder]

On Wed, Dec 9, 2015 at 8:37 AM, Torsten Bögershausen <tbo...@web.de> wrote:
> On 08.12.15 18:15, Christian Couder wrote:
>> This new function will be used in a later patch.
> May be
> Factor out code into add_untracked_cache(), which will be used in the next 
> commit.

Thanks for the suggestion. I think I will put something like this:

Factor out code into add_untracked_cache(), which will be used
in a later commit.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 09/10] config: add core.untrackedCache

[Christian Couder]

On Thu, Dec 31, 2015 at 12:23 AM, Junio C Hamano <gits...@pobox.com> wrote:
> Christian Couder <christian.cou...@gmail.com> writes:
>
>> On Tue, Dec 29, 2015 at 11:35 PM, Junio C Hamano <gits...@pobox.com> wrote:
>> ...
>>> While the above is not wrong per-se, from the point of those who
>>> looked for these options (that is, those who wanted to do a one-shot
>>> enabling or disabling of the feature, perhaps to try it out to see
>>> how well it helps on their system), I think the explanation of the
>>> interaction between the option and the config is backwards.  For
>>> their purpose, setting it to `true` or `false` will be hinderance,
>>> because the options are made no-op by such a setting.  IOW, the
>>> advertisement "once you decided that you want to use the feature,
>>> configuration is a better place to set it" does not belong here.
>>>
>>> If I were writing this documentation, I'd probably rephrase the
>>> second paragraph like so:
>>>
>>> These options take effect only when the
>>> `core.untrackedCache` configuration variable (see
>>> linkgit:git-config[1]) is set to `keep` (or it is left
>>> unset).  When the configuration variable is set to `true`,
>>> the untracked cache feature is always enabled (and when it
>>> is set to `false`, it is always disabled), making these
>>> options no-op.
>>>
>>> perhaps.
>>
>> Yeah, but "no-op" is not technically true, as if you just set the
>> config variable to true for example and then use "--untracked-cache"
>> then the cache will be added immediately.
>
> The "update-index --[no-]untracked-cache" command is made to no
> longer follow the user's immediate desire (i.e. enable or disable)
> expressed by the invocation of the command.  That is what I meant by
> 'no-op'.
>
>> ... for example "git update-index --untracked-cache" will die() if
>> the config variable is set to false.
>
> As I think it is a BUG to make these die(), it is an irrelevant
> objection ;-).

Ok I think I will just use what you suggest except the "no-op" part:

  These options take effect only when the
  `core.untrackedCache` configuration variable (see
  linkgit:git-config[1]) is set to `keep` (or it is left
  unset).  When the configuration variable is set to `true`,
  the untracked cache feature is always enabled (and when it
  is set to `false`, it is always disabled).


>>> I somehow thought, per the previous discussion with Duy, that the
>>> check and addition of an empty one (if missing in the index and
>>> configuration is set to `true`) or removal of an existing one (if
>>> configuration is set to `false`) were to be done when the index is
>>> read by read_index_from(), though.  If you have to say "After
>>> setting the configuration, you must run this command", that does not
>>> sound like a huge improvement over "If you want to enable this
>>> feature, you must run this command".
>>
>> The untracked-cache feature, as I tried to explain in an email in the
>> previous discussion, has an effect only on git status when it has to
>> show untracked files. Otherwise the untracked-cache is simply not
>> used. It might be a goal to use it more often, but it's not my patch
>> series' goal.
>
> In the future we may extend the system to allow "git add somedir/"
> to take advantage of the untracked cache (i.e. we may already know
> what untracked paths there are without letting readdir(3) to scan it
> in somedir/), for example.  Is it reasonable to force users to say
> "git status", in such a future?

If a new feature takes advantage of the untracked cache, it is
reasonable that this feature enables or disables it if should be
enabled or disabled. Maybe we can just say in the doc something like:

When the `core.untrackedCache` configuration variable is changed, the
untracked cache is added to or removed from the index the next time
a command that can use the untracked cache is run; while when
`--[no-|force-]untracked-cache` are used, the untracked cache is
immediately added to or removed from the index. Currently only
"git status" can use the untracked cache.

> And more importantly, when that
> future comes, is it reasonable to force users to re-learn Git to do
> something else to do things differently at that point?

I don't think people will have to relearn anything or do things
differently especially if things are explained like above.

If they want to

Re: Feature request: git bisect merge to usable base

[Christian Couder]

Hi,

On Wed, Dec 30, 2015 at 11:40 AM, Andy Lutomirski  wrote:
> Hi-
>
> I'm currently bisecting a Linux bug on my laptop.  The starting good
> commit is v4.4-rc3 and the starting bad commit is v4.4-rc7.
> Unfortunately, anything much older than v4.4-rc3 doesn't boot at all.
>
> I'd like to say:
>
> $ git bisect merge-to v4.4-rc3
>
> or similar.  The effect would be that, rather than testing commits in
> between the good and bad commits, it would test the result of merging
> those commits with v4.4-rc3.
>
> Obviously the syntax could be tweaked a lot, but I think the concept
> could be quite handy.

There have been talks about bisecting on the first parents and I think
it would be useful in your case.
After bisecting on the first parents you would know which merged
branch is responsible for the bug and you could for example rebase it
on top of v4.4-rc3 and then bisect on it.

You may find some scripts that might help you by searching for "bisect
first parent", like maybe there:

http://stackoverflow.com/questions/20240526/how-to-git-bisect-only-on-one-branchs-commits

Or maybe some old patchs on this list.

Best,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dir.h: remove orphaned declaration

[Christian Couder]

Hi Ramsay,

On Wed, Dec 30, 2015 at 6:11 PM, Ramsay Jones
 wrote:
>
> Signed-off-by: Ramsay Jones 
> ---
>
> Hi Christian,
>
> If you need to re-roll your 'cc/untracked' branch, could you
> please squash this into your patches.
>
> You seem to have only half applied my last fixup patch! ;-)
> (I'm guessing that you had already renamed the function
> locally before I sent the fixup).

Sorry I completely overlooked your previous patch.

This patch is now squashed into my current series:

https://github.com/chriscool/git/commits/uc-notifs42

(See 
https://github.com/chriscool/git/commit/699cd541a863ce027e20ded1c3d7300e146ccfd5.)

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 09/10] config: add core.untrackedCache

[Christian Couder]

On Tue, Dec 29, 2015 at 11:35 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Christian Couder <christian.cou...@gmail.com> writes:
>
>> +core.untrackedCache::
>> + Determines if untracked cache will be automatically enabled or
>> + disabled. It can be `keep`, `true` or `false`. Before setting
>> + it to `true`, you should check that mtime is working properly
>> + on your system.
>> + See linkgit:git-update-index[1]. `keep` by default.
>> +
>
> Before "Before setting it to `true`", the reader needs to be told
> what would happen when this is set to each of these three values (as
> well as what happens when this is not set at all).

Ok, then I think I will write something like:

core.untrackedCache::
 Determines what to do about the untracked cache feature of the
index. It will
 be kept, if this variable is unset or set to `keep`. It will
automatically be added
 if set to `true`. And it will automatically be removed, if set to
`false`. Before
 setting it to `true`, you should check that mtime is working
properly on your
 system.
 See linkgit:git-update-index[1]. `keep` by default.

>> diff --git a/Documentation/git-update-index.txt 
>> b/Documentation/git-update-index.txt
>> index a0afe17..813f6cc 100644
>> --- a/Documentation/git-update-index.txt
>> +++ b/Documentation/git-update-index.txt
>> @@ -174,27 +174,31 @@ may not support it yet.
>>
>>  --untracked-cache::
>>  --no-untracked-cache::
>> - Enable or disable untracked cache extension. This could speed
>> - up for commands that involve determining untracked files such
>> - as `git status`. The underlying operating system and file
>> - system must change `st_mtime` field of a directory if files
>> - are added or deleted in that directory.
>> + Enable or disable untracked cache extension. Please use
>> + `--test-untracked-cache` before enabling it.
>
> "extension" is a term that is fairly close to the machinery.  In
> other parts of the documentation (like the added text below in this
> patch), it is called "untracked cache FEATURE", which probably is a
> better word to use here as well.

Ok, I will use "feature".

>> +These options cannot override the `core.untrackedCache` configuration
>> +variable when it is set to `true` or `false` (see
>> +linkgit:git-config[1]). They can override it only when it is unset or
>> +set to `keep`. If you want untracked cache to persist, it is better
>> +anyway to just set this configuration variable to `true` or `false`
>> +directly.
>
> While the above is not wrong per-se, from the point of those who
> looked for these options (that is, those who wanted to do a one-shot
> enabling or disabling of the feature, perhaps to try it out to see
> how well it helps on their system), I think the explanation of the
> interaction between the option and the config is backwards.  For
> their purpose, setting it to `true` or `false` will be hinderance,
> because the options are made no-op by such a setting.  IOW, the
> advertisement "once you decided that you want to use the feature,
> configuration is a better place to set it" does not belong here.
>
> If I were writing this documentation, I'd probably rephrase the
> second paragraph like so:
>
> These options take effect only when the
> `core.untrackedCache` configuration variable (see
> linkgit:git-config[1]) is set to `keep` (or it is left
> unset).  When the configuration variable is set to `true`,
> the untracked cache feature is always enabled (and when it
> is set to `false`, it is always disabled), making these
> options no-op.
>
> perhaps.

Yeah, but "no-op" is not technically true, as if you just set the
config variable to true for example and then use "--untracked-cache"
then the cache will be added immediately. Also it does not explain
that for example "git update-index --untracked-cache" will die() if
the config variable is set to false.

>> @@ -385,6 +389,34 @@ Although this bit looks similar to assume-unchanged 
>> bit, its goal is
>>  different from assume-unchanged bit's. Skip-worktree also takes
>>  precedence over assume-unchanged bit when both are set.
>>
>> +Untracked cache
>> +---
>> +
>> +This cache could speed up commands that involve determining untracked
>> +...
>> +It is recommended to use the `core.untrackedCache` configuration
>> +variable (see linkgit:git-config[1]) to enable or disable this feature
>> +instead of using the `--[no-|force-]untracked-cache`, as it is mor

Re: [PATCH v4 08/10] dir: simplify untracked cache "ident" field

[Christian Couder]

On Wed, Dec 30, 2015 at 12:47 PM, Torsten Bögershausen <tbo...@web.de> wrote:
> On 2015-12-29 08.09, Christian Couder wrote:
>> It is not a good idea to compare kernel versions and disable
>> the untracked cache if it changes as people may upgrade and
>> still want the untracked cache to work. So let's just
>> compare work tree locations to decide if we should disable
>> it.
> OK with that.
>>
>> Also it's not useful to store many locations in the ident
>> field and compare to any of them. It can even be dangerous
>> if GIT_WORK_TREE is used with different values. So let's
>> just store one location, the location of the current work
>> tree.
> I don't think I fully agree in all details.

Suppose I use "git update-index --untracked-cache" first with
GIT_WORK_TREE=/foo and then with GIT_WORK_TREE=/bar. It will store
both /foo and /bar as valid locations for the worktree in the "ident"
field.

Then I work a bit with GIT_WORK_TREE=/foo and then a bit with
GIT_WORK_TREE=/bar. Now does the untracked cache info in the index
reflect the state of /foo or the state of /bar?

With a config variable, the problem is worse, because, as we
automatically add the untracked cache when git status is used, we are
even less likely to pay attention to GIT_WORK_TREE when the cache is
added.

> The list is there to distinguish between different clients when sharing
> a Git workspace over a network to different clients:
>
> A file system is exported from Linux to Linux via NFS,
> including a Git workspace)
> The same Workspace is exported via SAMBA to Windows and, in an extreme case,
> AFS to Mac OS.
>
> Other combinations could be
> Linux - SAMBA - Linux
> Linux - AFP - Linux
>
> Mac OS - NFS - Linux
> Mac OS - AFP - Mac OS
> Mac OS - SMB - Linux, Mac OS, Windows
> The list is incomplete (BSD and other Unix is missing),
> there are other combinations of network protocols,
> there are NAS boxes which mostly Linux under the hood, but
> may have vendor specific tweaks.
>
> Now we want to know, which of the combinations work.

In my opinion at this point it is a bit insane to want untracked cache
to work when it is accessed by different clients over the network. It
is much safer to just disable it (with a config option) in this case.
But I am ok to try to improve things for your use case to work if it
doesn't complicate things too much.

> The different clients need to test separately.
> E.g. for a given server:
>
> NFS - Linux
> SMB - Linux
> SMB Windows.
>
> At this point I would agree that the old code from dir.c:
>
> static const char *get_ident_string(void)
> {
> static struct strbuf sb = STRBUF_INIT;
> struct utsname uts;
>
> if (sb.len)
> return sb.buf;
> if (uname() < 0)
> die_errno(_("failed to get kernel name and information"));
> strbuf_addf(, "Location %s, system %s %s %s", get_git_work_tree(),
> uts.sysname, uts.release, uts.version);
> return sb.buf;
> }
> --
> is unneccessary picky, and the sysname should be enough:
>
> strbuf_addf(, "Location %s, system %s", get_git_work_tree(),
> uts.sysname);
>
> The old code did not find out, which network protocol that we used,
> (you need to call "mount", and grep for the directory, and try to get
> the FS information, which may be ext4, btrfs, smbfs, nfs)
> This is unnecessary complicated, so we endet up in using the path.

I am ok with the change to compare only the location and the system
name. I wonder a bit though if this change is completely backward
compatible.

> If I was a system administrator, (I sometimes am), I would set up a
> bunch of Linux boxes in a similar way, so that the repo is under
> /nfs/server1/projects/myproject
> (and the same path is used for all Linux boxes)
>
> The Windows machines may use something like
> N:/projects/myproject
> or
> //server1/projects/myproject
>
> and Mac OS may use
> /Volumes/projects/myproject
> (If mounted from the finder)
> or
> /nfs/projects/myproject
> (When autofs is used)
>
> I may have missed the discussion somewhat, could you explain why having
> different uname/location combinations are not usable at your site ?

I tried to explain above why using a config option to set the
untracked cache implies that you have to be careful at what happens
when people use GIT_WORK_TREE.

But it should be usable if indeed we put only the system name as you
suggest. So I think I will do that.

> How much burden is actually put on your system, and is there a way to keep a
> list of different clients ?

About a list of different client, instead of just one, I do

Re: [PATCH v4 08/10] dir: simplify untracked cache "ident" field

[Christian Couder]

On Tue, Dec 29, 2015 at 11:32 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Christian Couder <christian.cou...@gmail.com> writes:
>
>> -static int ident_in_untracked(const struct untracked_cache *uc)
>> +static int ident_current_location_in_untracked(const struct untracked_cache 
>> *uc)
>>  {
>> - const char *end = uc->ident.buf + uc->ident.len;
>> - const char *p   = uc->ident.buf;
>> + struct strbuf cur_loc = STRBUF_INIT;
>> + int res = 0;
>>
>> - for (p = uc->ident.buf; p < end; p += strlen(p) + 1)
>> - if (!strcmp(p, get_ident_string()))
>> - return 1;
>> - return 0;
>> + /*
>> +  * Previous git versions may have saved many strings in the
>> +  * "ident" field, but it is insane to manage many locations,
>> +  * so just take care of the first one.
>> +  */
>> +
>> + strbuf_addf(_loc, "Location %s, system ", get_git_work_tree());
>> +
>> + if (starts_with(uc->ident.buf, cur_loc.buf))
>> + res = 1;
>> +
>> + strbuf_release(_loc);
>> +
>> + return res;
>>  }
>
> The ", system " part looks funny.  I think I know what you are
> trying to do, though.
>
> If the path to the working tree has ", system " as a substring,
> I wonder if funny things may happen with this starts_with()?

Yeah, I think in the next iteration I will do what Torsten suggests.
It looks like the only sane solution, other than just not using the
"ident" field anymore and relying only on config variables like the
example I give in my reply to Torsten.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git Rebase Issue

[Christian Couder]

Hi,

On Tue, Dec 22, 2015 at 6:53 PM, Pierre-Luc Loyer
 wrote:
> Hi,
>
> I've encountered a situation using rebase for which I don't understand the 
> results, even after reading the documentation.
> I'm currently working in my feature branch and then I want to squash commits, 
> thus I use interactive rebase. After successfully completing the rebase, I 
> end up in a detached HEAD state, rather than back on my branch, which is 
> confusing. The command that is causing me to be in detached HEAD mode is: git 
> rebase -i HEAD~2 HEAD
> From the documentation, I read that my second parameter (HEAD) is the 
>  parameter:
>
>git rebase [-i | --interactive] [options] [--exec ] [--onto ]
>
>[] []
>
>If  is specified, git rebase will perform an automatic git 
> checkout  before doing anything else. Otherwise it remains on the 
> current branch.
>  Working branch; defaults to HEAD.
>
>Upon completion,  will be the current branch.
>
> Here is a full example than can be used to easily repro the issue. Go to an 
> empty folder.
> git init
> git echo text > file.txt
> git add .
> git commit -m "Add file.txt"
> git echo text2 > file.txt
> git commit -am "Modify file.txt"
> git echo text3 > file.txt
> git commit -am "Remodify file.txt"
>
> Now the interesting part:
> $ git rebase -i HEAD~2 HEAD
> [detached HEAD 9178b93] Modify file
> 1 file changed, 1 insertion(+), 1 deletion(-)
> Successfully rebased and updated detached HEAD.
>
> From the documentation it says that  (which is HEAD) will be checked 
> out before doing anything and that upon completion,  will be the 
> current branch. However, this doesn't seem to happen. In fact, it seems more 
> like the following is happening during the rebase:
> 1) detach HEAD
> 2) rebase
> 3) reattach to 
>
> If  is HEAD, then is does nothing and remains detached.
> I find this behavior confusing since I would expect it to return to whatever 
> HEAD was pointing to at the start of the command, such as my branch. Also, 
> the documentation says that the  parameter defaults to HEAD, so 
> passing 'HEAD' explicitly should result in the same behavior as not passing 
> it:
>  Working branch; defaults to HEAD.

You are right, it is probably a bug.
I guess usually people just use "git rebase -i HEAD~2" or "git rebase
-i master" and don't give the [] argument when using -i.

If you are interested in helping us debug this you might first want to
check if older git versions behaved like this.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 10/11] config: add core.untrackedCache

[Christian Couder]

On Thu, Dec 24, 2015 at 11:13 AM, Duy Nguyen <pclo...@gmail.com> wrote:
> On Thu, Dec 24, 2015 at 4:03 AM, Christian Couder
> <christian.cou...@gmail.com> wrote:
>>  --force-untracked-cache::
>> -   For safety, `--untracked-cache` performs tests on the working
>> -   directory to make sure untracked cache can be used. These
>> -   tests can take a few seconds. `--force-untracked-cache` can be
>> -   used to skip the tests.
>> +   Same as `--untracked-cache`. Provided for backwards
>> +   compatibility with older versions of Git where
>> +   `--untracked-cache` used to imply `--test-untracked-cache` but
>> +   this option would enable the extension unconditionally.
>
> Nit. The reason --force-untracked-cache remains can probably stay in
> the commit message. Here we can simply say "synonym of
> --untracked-cache, deprecated" or something like that (or even ".. to
> be deleted in version N.M").

Yeah, I think "deprecated" should be enough and clearer, but I will
see with AEvar who helped me on this.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/8] config: add core.untrackedCache

[Christian Couder]

On Thu, Dec 24, 2015 at 2:56 AM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> Duy Nguyen  writes:
>>
>>> In that case we can just check config once in read_index_from and
>>> destroy UNTR extension. Or the middle ground, we check config once in
>>> that place, make a note in struct index_state, and make invalidate_*
>>> check that note instead of config file. The middle ground has an
>>> advantage over destroying UNTR: (probably) many operations will touch
>>> index but do not add or remove entries.
>>
>> Or we can even teach read_index_from() to skip reading the extension
>> without even parsing when the configuration tells it that the
>> feature is force-disabled.  It can also add an empty one when the
>> configuration tells it that the feature is force-enabled and there
>> is no UNTR extension yet.
>
> Thinking about this a bit more, I am starting to feel that the
> config that can be used to optionally override the presence of
> in-index UNTR extension does not have to be too bad a thing,
> provided if it is done with a few tweaks to the design I read in
> Christian & Ævar's messages.

Great!

> One tweak is to address the following from Ævar's message:
>
>>> Once this series is applied and git is shipped with it existing
>>> users that have set "git update-index --untracked-cache" will have
>>> their UC feature disabled. This is because we fall back to "oh no
>>> config here? Must have been disabled, rm it from the index" clause
>>> which keeps our UC from going stale in the face of config
>>> flip-flopping.
>
> The above would happen only if the configuration is a boolean that
> defaults to false.  I'd think we can have this a tristate instead.
> That is, config.untrackedCache can be explicitly set to 'true',
> 'false', or 'keep'.  And make a missing config.untrackedCache
> default to 'keep'.

Ok. The first RFC patch series about this had a tristate and I
switched to a boolean as it seemed that people prefered a boolean, but
you are right that a tristate is more backward compatible.

> When read_index_from() reads an existing index:
>
> When the configuration is set to 'true':
> an existing UNTR is kept, otherwise a new UNTR gets added.
> When the configuration is set to 'false:
> an existing UNTR is dropped.
> When the configuration is set to 'keep':
> an existing UNTR is kept, otherwise nothing happens.
>
> When write_index() writes out an index that wasn't initialized from
> the filesystem, a new UNTR gets added only when the configuration is
> set to 'true' and there is no in-core UNTR already.

My current patch series does these changes in
wt_status_collect_untracked() because currently the UNTR is mostly
used only in git status, so it feels safer to me to not affect other
code paths.

> That way, those who use /etc/gitconfig to force the feature over
> their users would be able to set it to 'true', those who have been
> using the feature in some but not all of their repositories won't
> see any different behaviour until they muck with the configuration
> (and if they are paranoid and want to opt out of their administrator's
> setting, they can set it to 'keep' in their $HOME/.gitconfig to make
> sure their repositories are not affected).
>
> Orthogonal to the "config overrides the existing users' practice"
> issue, I still think that [PATCH v2 10/10] goes too far to remove
> the safety based on the working tree location.  Checking kernel
> version and other thing may be too much, but the check based on the
> working tree location is a cheap way to make sure that those who do
> unusual things (namely, use $GIT_DIR/$GIT_WORK_TREE or their
> equivalents to tell Git that the working tree for this invocation is
> at a place different from what UNTR data was prepared for) are not
> harmed by incorrectly reusing the cached data for an unrelated
> location.  So another tweak I'd feel better to see is to resurrect
> that safety.

This has been changed in v3, see patch 09/11, and I think it should
now work as you suggest.

> I wouldn't have as much issue with such a scheme as I had with the
> earlier description of the design in the Christian's series.

Great! I am preparing a v4 that I hope to send in a few days.
By the way the v3 does not pass its own tests due to a bug introduced
in last minute changes.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 04/11] update-index: add untracked cache notifications

[Christian Couder]

On Thu, Dec 24, 2015 at 11:01 AM, Duy Nguyen <pclo...@gmail.com> wrote:
> On Thu, Dec 24, 2015 at 4:03 AM, Christian Couder
> <christian.cou...@gmail.com> wrote:
>> @@ -1135,10 +1135,16 @@ int cmd_update_index(int argc, const char **argv, 
>> const char *prefix)
>> }
>> add_untracked_ident(the_index.untracked);
>> the_index.cache_changed |= UNTRACKED_CHANGED;
>> -   } else if (untracked_cache == UC_DISABLE && the_index.untracked) {
>> -   free_untracked_cache(the_index.untracked);
>> -   the_index.untracked = NULL;
>> -   the_index.cache_changed |= UNTRACKED_CHANGED;
>> +   if (verbose)
>> +   printf(_("Untracked cache enabled for '%s'\n"), 
>> get_git_work_tree());
>
> Nit. If you use report(), then you can skip "if (verbose)" because
> it's done inside report().

Ok, will do.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/8] config: add core.untrackedCache

[Christian Couder]

On Thu, Dec 17, 2015 at 1:36 PM, Duy Nguyen  wrote:
> On Wed, Dec 16, 2015 at 4:53 AM, Ævar Arnfjörð Bjarmason
>  wrote:
>> On Tue, Dec 15, 2015 at 8:40 PM, Junio C Hamano  wrote:
>>> Ævar Arnfjörð Bjarmason  writes:
>>> I still have a problem with the approach from "design cleanliness"
>>> point of view[...]
>>>
>>> In any case I think we already have agreed to disagree on this
>>> point, so there is no use discussing it any longer from my side.  I
>>> am not closing the door to this series, but I am not convinced,
>>> either.  At least not yet.
>>
>> In general the fantastic thing about the git configuration facility is
>> that it provides both systems administrators and normal users with
>> what they want. It's possible to configure things system-wide and
>> override those on a user or repository basis.
>>
>> Of course hindsight is 20/20, but I think that given what's been
>> covered in this thread it's been established that it's categorically
>> better that if we introduce features like these that they be
>> configured through the normal configuration facility rather than the
>> configuration being sticky to the index.
>
> A minor note for implementers. We need to check that config is loaded
> first. read-cache.c, being part of the core, does not bother itself
> with config loading. And I think so far it has not used any config
> vars. If a command forgets (*) to load the config, the cache may be
> deleted (if we do it the safe way).
>
> (*) is there any command deliberately avoid loading config? git-clone
> and git-init are special, but for this particular case it's probably
> fine.

Thanks for this note.

Looking at the current patch, the global variable in which the value
of the core.untrackedCache config var is stored is
"use_untracked_cache".
It is used in the following places:

- wt_status_collect_untracked() in wt-status.c which is called only by
"git status" and "git commit" after the config has been loaded.

- cmd_update_index() in builtin/update-index.c which loads the config
before using it

- validate_untracked_cache() in dir.c where it is used in:

   if (use_untracked_cache != 1 && !ident_in_untracked(dir->untracked)) {
warning(_("Untracked cache is disabled on this system."));
return NULL;
}

but this "if" and its contents are removed by patch 10/10 in v2.

So at the end of this patch series, there is no risk of
use_untracked_cache not being properly setup.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Fwd: [PATCH 7/8] config: add core.untrackedCache

[Christian Couder]

(Sorry I sent this one privately to Duy by mistake too.)


-- Forwarded message --
From: Christian Couder <christian.cou...@gmail.com>
Date: Fri, Dec 18, 2015 at 11:35 PM
Subject: Re: [PATCH 7/8] config: add core.untrackedCache
To: Duy Nguyen <pclo...@gmail.com>


On Thu, Dec 17, 2015 at 1:26 PM, Duy Nguyen <pclo...@gmail.com> wrote:
> On Thu, Dec 17, 2015 at 2:44 PM, Jeff King <p...@peff.net> wrote:
>> I think we may actually be thinking of the same thing. Naively, I would
>> expect:
>>
>> ..
>>   - if there is cache data in the index but that config flag is not set,
>> presumably we would not update it (we could even explicitly drop it,
>> but my understanding is that is not necessary for correctness, but
>> only as a possible optimization).
>
> No, if somebody adds or removes something from the index, we either
> update or drop it, or it's stale. There's the invalidate_untracked()
> or something in dir.c that we can hook in, check config var and do
> that. And because config is cached recently, it should be a cheap
> operation.

I think I understand what you are saying but it took me a long time,
and I am not sure it is very clear to others.

What I understand is that you are talking about
validate_untracked_cache() in dir.c.
And you suggest to check there if the core.untrackedCache config var
is set, and, if it is not set, then to drop the UC there.
And the reason for that is that git operations on other parts of the
index should update the UC if it exists otherwise the UC content could
be wrong as you explained previously in your "git rm --cached foo"
example.

In the current patch, the code to create or remove the UC to reflect
the state of the core.untrackedCache config var is in
wt_status_collect_untracked().
I think it works well there, but if you are saying that it's better if
it is in validate_untracked_cache(), I am willing to consider moving
it to validate_untracked_cache().
Could you tell a bit though about why you think it's better in
validate_untracked_cache()?

> Apart from that the idea is fine.

Ok so, except maybe the part about wt_status_collect_untracked() vs
validate_untracked_cache(), what the patch does is ok for you?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Fwd: [PATCH 7/8] config: add core.untrackedCache

[Christian Couder]

Sorry I sent this privately to Peff by mistake (once again).

-- Forwarded message --
From: Christian Couder <christian.cou...@gmail.com>
Date: Fri, Dec 18, 2015 at 11:09 PM
Subject: Re: [PATCH 7/8] config: add core.untrackedCache
To: Jeff King <p...@peff.net>


On Thu, Dec 17, 2015 at 8:44 AM, Jeff King <p...@peff.net> wrote:
> On Tue, Dec 15, 2015 at 10:05:57PM -0800, Junio C Hamano wrote:
>>
>> To put it another way, the "bit" in the index (i.e. the presence of
>> the cached data) is "Am I using the feature now?".  The effect of
>> the feature has to (and is designed to) persist, as it is a cache
>> and you do not want a stale cache to give you wrong answers.

Sorry if I repeat myself or misunderstood something, but in what I
implemented we either use the UC fully or we remove it, so it cannot
be stale vs git operations (like other changes in the index Duy talks
about).

And as we are caching directory mtimes and as we are comparing each
cached mtime with the current mtime of the original directory before
trusting the cached content related to the directory, a UC that is
stale vs working tree operations could result in time lost but not in
bad cached content used.

Or maybe if we are very unlucky and have two directories with exactly
the same mtime and the same name but not the same content, and if we
move away the directory the UC was made from, and then put the other
one at the path where the first one was. Yeah in this case you may get
bad results because bad UC content is used. But note that this case
can already happen now. It is a case that is inherent in using the UC.

>> There
>> is no "Do I want to use the feature?" preference, in other words.
>>
>> And I do not mind creating such a preference bit as a configuration.
>>
>> That is why I suggested such a configuration to cause the equivalent
>> of "update-index --untracked-cache" when a new index is created from
>> scratch (as opposed to the case where the previously created cache
>> data is carried forward across read_cache() -> do things to the
>> index -> write_cache() flow).  Doing it that way will not have to
>> involve additional complexity that comes from the desire that
>> setting a single configuration on (or off) has to suddenly change
>> the behaviour of an index file that is already using (or not using)
>> the feature.
>
> I think we may actually be thinking of the same thing. Naively, I would
> expect:
>
>   - if there is untracked cache data in the index, we will make use of
> it when enumerating untracked files (and my understanding is that if
> it is stale, we can detect that)

I agree for "git status", but I am not sure that the UC is used in all
the code paths that enumerate untracked files.
I recall Duy mentioning that it was not used by "git add" and in some
other cases, but I might be wrong.

I also agree that we can detect when the UC content should not be used
because it is stale vs working tree operations (see above).

Now as Duy says the UC should never be stale vs git operations, but
that is easy to do if we just remove the UC when we stop using it.

>   - if core.untrackedCache is set, we will update and write out an
> untracked cache when we are enumerating the untracked files

In the current patch this happens when "git status" is called,
actually in wt_status_collect_untracked(), again I am not sure about
all the other code paths.

>   - if there is cache data in the index but that config flag is not set,
> presumably we would not update it (we could even explicitly drop it,
> but my understanding is that is not necessary for correctness, but
> only as a possible optimization).

In the current patch we drop the UC, also in
wt_status_collect_untracked(), if the config flag is not set (or set
to false).

It is necessary to drop it for correctness for the following reasons:

- git operations on (other parts of) the index must update the UC if
it exists according to Duy who gave the following example in a
previous email:

"... if file "foo" is tracked, then the user does "git rm --cached
foo", "foo" may become either untracked or ignored. So if you disable
UC while doing this removal, then re-enable UC again, "git-status" may
show incorrect output."

- the user may have decided to unset the config flag because the mtime
features we rely on are in fact not well supported by the system.
(Though it's true that the user should have used "git update-index
--test-untracked-cache" before setting the config flag in the first
place, but maybe it was a mistake, or maybe the file system will be
accessed for some time by another system that will mount it or
something.)

> You could

[PATCH v3 01/11] dir: free untracked cache when removing it

[Christian Couder]

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 7431938..a6fff87 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1123,6 +1123,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
add_untracked_ident(the_index.untracked);
the_index.cache_changed |= UNTRACKED_CHANGED;
} else if (!untracked_cache && the_index.untracked) {
+   free_untracked_cache(the_index.untracked);
the_index.untracked = NULL;
the_index.cache_changed |= UNTRACKED_CHANGED;
}
-- 
2.7.0.rc2.11.g68ccdd4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 02/11] update-index: use enum for untracked cache options

[Christian Couder]

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/update-index.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index a6fff87..1e546a3 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -35,6 +35,14 @@ static int mark_skip_worktree_only;
 #define UNMARK_FLAG 2
 static struct strbuf mtime_dir = STRBUF_INIT;
 
+/* Untracked cache mode */
+enum uc_mode {
+   UC_UNSPECIFIED = -1,
+   UC_DISABLE = 0,
+   UC_ENABLE,
+   UC_FORCE
+};
+
 __attribute__((format (printf, 1, 2)))
 static void report(const char *fmt, ...)
 {
@@ -902,7 +910,7 @@ static int reupdate_callback(struct parse_opt_ctx_t *ctx,
 int cmd_update_index(int argc, const char **argv, const char *prefix)
 {
int newfd, entries, has_errors = 0, line_termination = '\n';
-   int untracked_cache = -1;
+   enum uc_mode untracked_cache = UC_UNSPECIFIED;
int read_from_stdin = 0;
int prefix_length = prefix ? strlen(prefix) : 0;
int preferred_index_format = 0;
@@ -997,7 +1005,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "untracked-cache", _cache,
N_("enable/disable untracked cache")),
OPT_SET_INT(0, "force-untracked-cache", _cache,
-   N_("enable untracked cache without testing the 
filesystem"), 2),
+   N_("enable untracked cache without testing the 
filesystem"), UC_FORCE),
OPT_END()
};
 
@@ -1104,10 +1112,10 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
the_index.split_index = NULL;
the_index.cache_changed |= SOMETHING_CHANGED;
}
-   if (untracked_cache > 0) {
+   if (untracked_cache > UC_DISABLE) {
struct untracked_cache *uc;
 
-   if (untracked_cache < 2) {
+   if (untracked_cache < UC_FORCE) {
setup_work_tree();
if (!test_if_untracked_cache_is_supported())
return 1;
@@ -1122,7 +1130,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
}
add_untracked_ident(the_index.untracked);
the_index.cache_changed |= UNTRACKED_CHANGED;
-   } else if (!untracked_cache && the_index.untracked) {
+   } else if (untracked_cache == UC_DISABLE && the_index.untracked) {
free_untracked_cache(the_index.untracked);
the_index.untracked = NULL;
the_index.cache_changed |= UNTRACKED_CHANGED;
-- 
2.7.0.rc2.11.g68ccdd4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 03/11] update-index: add --test-untracked-cache

[Christian Couder]

It is nice to just be able to test if untracked cache is
supported without enabling it.

Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 Documentation/git-update-index.txt | 9 -
 builtin/update-index.c | 5 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index f4e5a85..a0ee0c9 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -18,7 +18,7 @@ SYNOPSIS
 [--[no-]skip-worktree]
 [--ignore-submodules]
 [--[no-]split-index]
-[--[no-|force-]untracked-cache]
+[--[no-|test-|force-]untracked-cache]
 [--really-refresh] [--unresolve] [--again | -g]
 [--info-only] [--index-info]
 [-z] [--stdin] [--index-version ]
@@ -180,6 +180,13 @@ may not support it yet.
system must change `st_mtime` field of a directory if files
are added or deleted in that directory.
 
+--test-untracked-cache::
+   Only perform tests on the working directory to make sure
+   untracked cache can be used. You have to manually enable
+   untracked cache using `--force-untracked-cache` (or
+   `--untracked-cache` but this will run the tests again)
+   afterwards if you really want to use it.
+
 --force-untracked-cache::
For safety, `--untracked-cache` performs tests on the working
directory to make sure untracked cache can be used. These
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 1e546a3..6dd 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -40,6 +40,7 @@ enum uc_mode {
UC_UNSPECIFIED = -1,
UC_DISABLE = 0,
UC_ENABLE,
+   UC_TEST,
UC_FORCE
 };
 
@@ -1004,6 +1005,8 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
N_("enable or disable split index")),
OPT_BOOL(0, "untracked-cache", _cache,
N_("enable/disable untracked cache")),
+   OPT_SET_INT(0, "test-untracked-cache", _cache,
+   N_("test if the filesystem supports untracked 
cache"), UC_TEST),
OPT_SET_INT(0, "force-untracked-cache", _cache,
N_("enable untracked cache without testing the 
filesystem"), UC_FORCE),
OPT_END()
@@ -1119,6 +1122,8 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
setup_work_tree();
if (!test_if_untracked_cache_is_supported())
return 1;
+   if (untracked_cache == UC_TEST)
+   return 0;
}
if (!the_index.untracked) {
uc = xcalloc(1, sizeof(*uc));
-- 
2.7.0.rc2.11.g68ccdd4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html