Re: [PATCH v5] blame: add support for --[no-]progress option

2015-12-08 Thread Eric Sunshine
On Tue, Nov 24, 2015 at 11:36 PM, Edmundo Carmona Antoranz
 wrote:
> * created struct progress_info in builtin/blame.c
>   this struct holds the information used to display progress so
>   that only one additional parameter is passed to
>   found_guilty_entry().

Commit messages typically are composed of proper sentences and
paragraphs rather than bullets points. Also, write in imperative mood:
"Create progress_info..."

In this case, though, the information in this bullet point isn't all
that interesting for the commit message since it's a detail of
implementation easily gleaned by reading the patch itself. There's
nothing particularly tricky about it, thus it doesn't really deserve
to be called out as special in the commit message.

What might be more interesting and deserve mention in the commit
message is how this new option interacts with porcelain and
incremental modes and why the choice was made.

More below...

> * --[no-]progress option is also inherited by git-annotate.
>
> Signed-off-by: Edmundo Carmona Antoranz 
> ---
> diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
> @@ -69,6 +69,13 @@ include::line-range-format.txt[]
> iso format is used. For supported values, see the discussion
> of the --date option at linkgit:git-log[1].
>
> +--[no-]progress::
> +   Progress status is reported on the standard error stream
> +   by default when it is attached to a terminal. This flag
> +   enables progress reporting even if not attached to a
> +   terminal. Progress information won't be displayed if using
> +   `--porcelain` or `--incremental`.

Is silently ignoring --progress a good idea when combined with
--porcelain or --incremental, or would it be better to error out with
a "mutually exclusive options" diagnostic? (Genuine question.)

> diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
> @@ -10,7 +10,8 @@ SYNOPSIS
>  [verse]
>  'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] 
> [--incremental]
> [-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
> -   [--abbrev=] [ | --contents  | --reverse ] [--] 
> 
> +   [--[no-]progress] [--abbrev=] [ | --contents  | 
> --reverse ]

Hmm, is [--[no-]progress] common in Git documentation? (Genuine
question.) For the synopsis, I'd have expected to see only
[--progress] and leave it to the more detailed description of the
option to mention the possibility of negation (but I haven't
double-checked other documentation, so my expectation may be skewed).

>  DESCRIPTION
>  ---
> diff --git a/builtin/blame.c b/builtin/blame.c
> @@ -127,6 +129,11 @@ struct origin {
> +struct progress_info {
> +   struct progress *progress;
> +   int blamed_lines;
> +};
> @@ -1746,7 +1753,8 @@ static int emit_one_suspect_detail(struct origin 
> *suspect, int repeat)
> -static void found_guilty_entry(struct blame_entry *ent)
> +static void found_guilty_entry(struct blame_entry *ent,
> +  struct progress_info *pi)
> @@ -1758,6 +1766,8 @@ static void found_guilty_entry(struct blame_entry *ent)
> +   if (pi)
> +   display_progress(pi->progress, pi->blamed_lines += 
> ent->num_lines);

This is updating of 'blamed_lines' is rather subtle and easily
overlooked. Splitting it out as a separate statement could improve
readability:

pi->blamed_lines += ent->num_lines;
display_progress(pi->progress, pi->blamed_lines);

>  }
> @@ -1768,6 +1778,16 @@ static void assign_blame(struct scoreboard *sb, int 
> opt)
>  {
> struct rev_info *revs = sb->revs;
> struct commit *commit = prio_queue_get(>commits);
> +   struct progress_info *pi = NULL;
> +
> +   if (show_progress) {
> +   pi = malloc(sizeof(*pi));
> +   if (pi) {

xmalloc(), unlike malloc(), will die() upon allocation failure which
would allow you to avoid the "if (pi)" conditional.

> +   pi->progress = start_progress_delay(_("Blaming 
> lines"),
> +   sb->num_lines, 
> 50, 1);
> +   pi->blamed_lines = 0;
> +   }
> +   }
>
> while (commit) {
> struct blame_entry *ent;
> @@ -1809,7 +1829,7 @@ static void assign_blame(struct scoreboard *sb, int opt)
> suspect->guilty = 1;
> for (;;) {
> struct blame_entry *next = ent->next;
> -   found_guilty_entry(ent);
> +   found_guilty_entry(ent, pi);
> if (next) {
> ent = next;
> continue;
> @@ -1825,6 +1845,11 @@ static void assign_blame(struct scoreboard *sb, int 
> opt)
> if (DEBUG) /* sanity */
> 

[PATCH v2] git-p4: Add option to ignore empty commits

2015-12-08 Thread larsxschneider
From: Lars Schneider 

diff to v1:
* change the default behavior and replace "ignore empty commits" option
  with "keep empty commits" (thanks Junio/Luke)
  --> I kept the original topic name in the subject letter to ease finding
  v1, ok?
* add 'an' to fix grammar in commit message (thanks Luke)
* remove quotes around 'true' in docs to avoid confusion (thanks Luke)
* add deletion test (thanks Junio/Luke)
* use print statement to avoid ugly \n (thanks Luke)
* use positional argument specifiers in Python format string syntax to
  support Python 2.6 (thanks Pete Harlan)

Thanks,
Lars

Lars Schneider (1):
  git-p4: Add option to keep empty commits

 Documentation/git-p4.txt |   4 ++
 git-p4.py|  44 +++-
 t/t9826-git-p4-keep-empty-commits.sh | 134 +++
 3 files changed, 165 insertions(+), 17 deletions(-)
 create mode 100755 t/t9826-git-p4-keep-empty-commits.sh

--
2.5.1

--
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/2] git-p4: support multiple depot paths in p4 submit

2015-12-08 Thread Lars Schneider

On 07 Dec 2015, at 19:51, Sam Hocevar  wrote:

> On Sun, Dec 06, 2015, Lars Schneider wrote:
>> Thanks for the patch! Do you see a way to demonstrate the bug in a test case 
>> similar to t9821 [1]?
> 
>   Not yet, I'm afraid. It's proving trickier than expected because for
> now I can only reproduce the bug when the view uses multiples depots
> (solution #2 on http://answers.perforce.com/articles/KB/2437), and
> unfortunately the test case system in Git was designed for a single
> depot.
> 
>   Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to
> support multiple depots be acceptable and/or welcome? I prefer to ask
> before I dig into the task.

Can you outline your idea a bit? Are you aware of the following way to define 
client specs: [1] ? Would that help?
I haven't used multiple depots, yet. Therefore please bare with me :-)

Thanks,
Lars

[1] 
https://github.com/git/git/blob/362d2fc2f8ab9ee22072f76fb36ec16918511944/t/t9821-git-p4-path-variations.sh#L109-L111

--
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] git-p4: Add option to keep empty commits

2015-12-08 Thread larsxschneider
From: Lars Schneider 

A changelist that contains only excluded files due to a client spec was
imported as an empty commit. Fix that issue by ignoring these commits.
Add option "git-p4.keepEmptyCommits" to make the previous behavior
available.

Signed-off-by: Lars Schneider 
Helped-by: Pete Harlan
---
 Documentation/git-p4.txt |   4 ++
 git-p4.py|  44 +++-
 t/t9826-git-p4-keep-empty-commits.sh | 134 +++
 3 files changed, 165 insertions(+), 17 deletions(-)
 create mode 100755 t/t9826-git-p4-keep-empty-commits.sh

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 82aa5d6..b3e768e 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -510,6 +510,10 @@ git-p4.useClientSpec::
option '--use-client-spec'.  See the "CLIENT SPEC" section above.
This variable is a boolean, not the name of a p4 client.
 
+git-p4.keepEmptyCommits::
+   A changelist that contains only excluded files will be imported
+   as an empty commit if this boolean option is set to true.
+
 Submit variables
 
 git-p4.detectRenames::
diff --git a/git-p4.py b/git-p4.py
index 0093fa3..62c26bc 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2288,12 +2288,6 @@ class P4Sync(Command, P4UserMap):
 filesToDelete = []
 
 for f in files:
-# if using a client spec, only add the files that have
-# a path in the client
-if self.clientSpecDirs:
-if self.clientSpecDirs.map_in_client(f['path']) == "":
-continue
-
 filesForCommit.append(f)
 if f['action'] in self.delete_actions:
 filesToDelete.append(f)
@@ -2361,25 +2355,41 @@ class P4Sync(Command, P4UserMap):
 gitStream.write(description)
 gitStream.write("\n")
 
+def inClientSpec(self, path):
+if not self.clientSpecDirs:
+return True
+inClientSpec = self.clientSpecDirs.map_in_client(path)
+if not inClientSpec and self.verbose:
+print('Ignoring file outside of client spec: {0}'.format(path))
+return inClientSpec
+
+def hasBranchPrefix(self, path):
+if not self.branchPrefixes:
+return True
+hasPrefix = [p for p in self.branchPrefixes
+if p4PathStartsWith(path, p)]
+if hasPrefix and self.verbose:
+print('Ignoring file outside of prefix: {0}'.format(path))
+return hasPrefix
+
 def commit(self, details, files, branch, parent = ""):
 epoch = details["time"]
 author = details["user"]
 
 if self.verbose:
-print "commit into %s" % branch
-
-# start with reading files; if that fails, we should not
-# create a commit.
-new_files = []
-for f in files:
-if [p for p in self.branchPrefixes if p4PathStartsWith(f['path'], 
p)]:
-new_files.append (f)
-else:
-sys.stderr.write("Ignoring file outside of prefix: %s\n" % 
f['path'])
+print('commit into {0}'.format(branch))
 
 if self.clientSpecDirs:
 self.clientSpecDirs.update_client_spec_path_cache(files)
 
+files = [f for f in files
+if self.inClientSpec(f['path']) and 
self.hasBranchPrefix(f['path'])]
+
+if not files and not gitConfigBool('git-p4.keepEmptyCommits'):
+print('Ignoring revision {0} as it would produce an empty commit.'
+.format(details['change']))
+return
+
 self.gitStream.write("commit %s\n" % branch)
 #gitStream.write("mark :%s\n" % details["change"])
 self.committedChanges.add(int(details["change"]))
@@ -2403,7 +2413,7 @@ class P4Sync(Command, P4UserMap):
 print "parent %s" % parent
 self.gitStream.write("from %s\n" % parent)
 
-self.streamP4Files(new_files)
+self.streamP4Files(files)
 self.gitStream.write("\n")
 
 change = int(details["change"])
diff --git a/t/t9826-git-p4-keep-empty-commits.sh 
b/t/t9826-git-p4-keep-empty-commits.sh
new file mode 100755
index 000..be12960
--- /dev/null
+++ b/t/t9826-git-p4-keep-empty-commits.sh
@@ -0,0 +1,134 @@
+#!/bin/sh
+
+test_description='Clone repositories and keep empty commits'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+   start_p4d
+'
+
+test_expect_success 'Create a repo' '
+   client_view "//depot/... //client/..." &&
+   (
+   cd "$cli" &&
+
+   mkdir -p subdir &&
+
+   >subdir/file1.txt &&
+   p4 add subdir/file1.txt &&
+   p4 submit -d "Add file 1" &&
+
+   >file2.txt &&
+   p4 add file2.txt &&
+   p4 submit -d "Add file 2" &&
+
+   >subdir/file3.txt &&
+   p4 add 

Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit

2015-12-08 Thread Luke Diamand
On 8 December 2015 at 11:41, Sam Hocevar  wrote:
> On Tue, Dec 08, 2015, Lars Schneider wrote:
>
>> >   Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to
>> > support multiple depots be acceptable and/or welcome? I prefer to ask
>> > before I dig into the task.
>>
>> Can you outline your idea a bit? Are you aware of the following way to 
>> define client specs: [1] ? Would that help?
>
>That's the idea, but the bug occurs when the client view looks like this:
>
>  //depot/... //client/dir1/...
>  //depot2/... //client/dir2/...
>
>And is then cloned with (it is not legal in Perforce to specify //...
> directly to grab both depots at once):
>
>  git p4 clone --use-client-spec //depot/... //depot2/...
>
>Then when a file is modified in dir2/, git p4 submit does not elect it
> for the changelist. A file in dir1/ will work fine.
>
>Unfortunately the current test suite assumes everything is under
> //depot/ so in order to write a test for this situation there are a few
> things to change in lib-git-p4.sh.

I think the existing structure ought to mostly work, but it might need
a bit of tweaking.

You would need to create a new depot, but you can do that in your test script.

And you would need a client spec that pointed at this depot, but again
you can do that in your script with the client_view shell function.

I've not tried it myself though, so maybe it's harder than that.

Luke


>
> Regards,
> --
> Sam.
--
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 (Dec 2015, #01; Tue, 1)

2015-12-08 Thread Patrick Steinhardt
On Mon, Dec 07, 2015 at 11:24:52AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt  writes:
> 
> > On Wed, Dec 02, 2015 at 05:31:14PM -0500, Jeff King wrote:
> >> On Wed, Dec 02, 2015 at 02:11:32PM -0800, Junio C Hamano wrote:
> > [snip]
> >> > "--keep-empty" has always been about keeping an originally empty
> >> > commit, not a commit that becomes empty because of rebasing
> >> > (i.e. what has already been applied to the updated base).  The
> >> > documentation, if it leads to any other interpretation, needs to be
> >> > fixed.
> >> > 
> >> > Besides, if "--keep-empty" were to mean "keep redundant ones that
> >> > are already in the updated base", the patch must do a lot more,
> >> > e.g. stop filtering with git-cherry patch equivalence.
> >> > 
> >> > I'm inclined to eject this topic.
> >> 
> >> That was my thinking too (and I notice it didn't get any review from
> >> anybody else).
> > [snip]
> >
> > Well, I kind of agree. The cherry-pick command has both
> > --allow-empty and --keep-redundant flags, where the second one is
> > the kind of behavior I want to achieve in my case. As an
> > alternative to the proposed change to `--keep-empty` I could
> > instead introduce a new flag `--keep-redundant-commits` to `git
> > rebase` which would then pass the flag through to the
> > cherry-pick.
> >
> > Any opinions on this?
> 
> Honestly, I am not interested if that is only about passing that
> option and doing nothing else.
> 
> Imagine that you have two changes from the branch being rebased
> already in the updated base, one was accepted verbatim, while the
> other one was accepted with a slight tweak.  Perhaps there were some
> conflicts in the context, or something.
> 
> When you run your rebase, the former will not even be considered
> because command will notice, via patch equivalence, that you do not
> need it, even before it attempts to replay it.  But not the latter.
> The command will attempt to replay it, and only in this step it may
> notice, by seeing that the result becomes a no-op change, that the
> change has already been included in the updated base.
> 
> I cannot quite see how adding "--keep-redundant-commits" to the
> command would help anybody by allowing the latter in the history but
> not the former.  That is primarily because you can imagine a future
> in which the patch equivalence determination becomes smarter.  With
> or without "--keep-redundant-commits", both of these two changes
> would not appear in the resulting history when that happens.
> 
> The "--keep-redundant" option to "cherry-pick" makes sense, as the
> user will be the one who is deciding which commit to replay on top
> of a new base.  If the user fed a commit that is already in the new
> base, and the command is run with the option, that is a deliberate
> request to leave a no-op cruft.
> 
> We cannot say the same thing for "rebase", as it tries to avoid
> redundant cruft, and it cannot do as good a job as humans in doing
> so.  The "--keep-redundant-commits" option will become a way to make
> a distinction between what got dropped by the command automatically
> and what didn't get dropped because the command did not look deeply
> enough.  That distinction is not at all useful, as that can change
> as the tool improves.
> 
> A "--keep-redundant-commits" to "rebase" that also disables the
> patch equivalence filtering (not just keeping empty crufts in the
> resulting history) might make sense, but I do not see myself (or
> anybody sane) using it.  "In what situation would such a feature be
> useful?" is a question I cannot answer offhand.

The scenario I require this feature for is somewhat more exotic,
yes. We've got a CI where published branches can be rebased but
should not be modified in their commit history. That is, the CI
determines in a hook wether the branch that is being pushed drops
or re-orders commits and if so, rejects the branch.

So when a commit that is already included in origin/master gets
rebased `git-rebase` currently simply drops the commit, which
causes the CI to refuse the branch on a push.

So obviously you are right in your conclusion that
`--keep-redundant-commits` has to skip the patch equivalence
determination in order to not drop any commits. Otherwise my
change would only work for certain scenarios.

That said, I do not care too deeply about this patch, especially
as my scenario is rather uncommon. If you deem this to not have
any greater benefit you may simply drop it.

Patrick


signature.asc
Description: Digital signature


Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit

2015-12-08 Thread Sam Hocevar
On Tue, Dec 08, 2015, Lars Schneider wrote:

> >   Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to
> > support multiple depots be acceptable and/or welcome? I prefer to ask
> > before I dig into the task.
> 
> Can you outline your idea a bit? Are you aware of the following way to define 
> client specs: [1] ? Would that help?

   That's the idea, but the bug occurs when the client view looks like this:

 //depot/... //client/dir1/...
 //depot2/... //client/dir2/...

   And is then cloned with (it is not legal in Perforce to specify //...
directly to grab both depots at once):

 git p4 clone --use-client-spec //depot/... //depot2/...

   Then when a file is modified in dir2/, git p4 submit does not elect it
for the changelist. A file in dir1/ will work fine.

   Unfortunately the current test suite assumes everything is under
//depot/ so in order to write a test for this situation there are a few
things to change in lib-git-p4.sh.

Regards,
-- 
Sam.
--
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 3/2] git.c: make sure we do not leak GIT_* to alias scripts

2015-12-08 Thread Duy Nguyen
On Mon, Dec 7, 2015 at 7:54 PM, Junio C Hamano  wrote:
> Nguyễn Thái Ngọc Duy   writes:
>
>>  Let's hope there will be no third report about this commit..
>
> Hmm, why does this additional test fail only under prove but pass
> without it?

It passes with prove for me. Some mysterious variable leaks through somehow?

>> + env | grep GIT_ | sed "s/=.*//" | sort >actual
>
> This is more about coding discipline than style, but piping grep
> output to sed is wasteful.  "sed -ne '/^GIT_/s/=.*//p'" or something
> like that, perhaps?

OK will fix.

> I wondered what happens if the user has an unrelated stray variable
> whose name happens to begin with GIT_ in her environment, but it
> turns out that we cleanse them in test-lib.sh fairly early, so that
> would be fine.  You need to tighten your "grep" pattern, though.

OK
-- 
Duy
--
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 3/2] git.c: make sure we do not leak GIT_* to alias scripts

2015-12-08 Thread Jeff King
On Tue, Dec 08, 2015 at 05:55:20PM +0100, Duy Nguyen wrote:

> On Mon, Dec 7, 2015 at 7:54 PM, Junio C Hamano  wrote:
> > Nguyễn Thái Ngọc Duy   writes:
> >
> >>  Let's hope there will be no third report about this commit..
> >
> > Hmm, why does this additional test fail only under prove but pass
> > without it?
> 
> It passes with prove for me. Some mysterious variable leaks through somehow?

It fails for me when run via "make" (with prove or without) but not as
"./t0001-init.sh". Looks like extra variables from my config.mak leak
through:

  $ make t0001-init.sh GIT_TEST_OPTS="-v -i"
  [...]
  --- expected2015-12-08 17:18:06.304699181 +
  +++ actual  2015-12-08 17:18:06.312699180 +
  @@ -9,5 +9,9 @@
   GIT_MERGE_VERBOSITY
   GIT_PREFIX
   GIT_TEMPLATE_DIR
  +GIT_TEST_GIT_DAEMON
  +GIT_TEST_HTTPD
  +GIT_TEST_OPTS
   GIT_TEXTDOMAINDIR
   GIT_TRACE_BARE
  +MAKEFLAGS
  not ok 6 - No extra GIT_* on alias scripts

Any GIT_TEST_* is allowed through by test-lib.sh.

For the MAKEFLAGS one, I suspect it's hitting

  MAKEFLAGS=GIT_TEST_OPTS=...

You should probably anchor your regex. :)

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

2015-12-08 Thread 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


Re: [PATCH 2/2] format-patch: add an option to suppress commit hash

2015-12-08 Thread Junio C Hamano
"brian m. carlson"  writes:

>> Don't test "any number of '0'"; test 40 '0's.  This is because the
>> line format was designed to be usable by things like /etc/magic to
>> detect format-patch output, and we want to notice if/when we break
>> that aspect of our output format.
>
> My idea was to future-proof it against changes in the hash function,
> although that's in the distant future.

Making sure that the test fails when somebody accidentally lengthens
(or fails to truncate to 40-hex, when you start using a longer hash)
the displayed name there is a good future-proofing.

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


[PATCH 0/8] Untracked cache improvements

2015-12-08 Thread 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()

2015-12-08 Thread Christian Couder
This new function will be used in a later patch.

Signed-off-by: Christian Couder 
---
 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

2015-12-08 Thread 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 
---
 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

2015-12-08 Thread 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 
---
 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 options.
@@ -406,6 +412,8 @@ It 

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

2015-12-08 Thread Christian Couder
Signed-off-by: Christian Couder 
---
 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

2015-12-08 Thread Christian Couder
Signed-off-by: Christian Couder 
---
 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

2015-12-08 Thread Christian Couder
Signed-off-by: Christian Couder 
---
 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()

2015-12-08 Thread Christian Couder
This new function will be used in a later patch.

Signed-off-by: Christian Couder 
---
 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


RE: Why does send-pack call pack-objects for all remote refs?

2015-12-08 Thread Daniel Koverman
Your interpretation of my email was correct. As you picked up on, I
had a fundamental misunderstanding of what pack-objects was doing.
Thanks for the explanation, I have a much better idea of what is
going on now.

Given my use pattern, it may be reasonable for me to patch in an
option to compute

git rev-list --objects $my_topic --not $subset_of_remote_refs

capitalizing on my knowledge of this particular repository to come
up with heuristics for picking a reasonable subset. This will
come with the risk of sometimes producing an unnecessarily large
(potentially an obscenely large) packfile. You have thoroughly
convinced me that an option like that will not generalize and would
be unsuitable for main line git.

It is also good to know that 2000 remote refs is insane. The lower
hanging fruit here sounds like trimming that to a reasonable
number, so I'll try that approach first.

Thanks again, Junio and Peff.

Daniel

-Original Message-
From: Jeff King [mailto:p...@peff.net] 
Sent: Monday, December 07, 2015 5:57 PM
To: Daniel Koverman
Cc: Junio C Hamano; git@vger.kernel.org
Subject: Re: Why does send-pack call pack-objects for all remote refs?

On Mon, Dec 07, 2015 at 02:41:00PM -0800, Junio C Hamano wrote:

> Also it was unclear if you are working with a shallow repository.
> The performance trade-off made between the packsize and the cycles
> is somewhat different between a normal and a shallow repository,
> e.g. 2dacf26d (pack-objects: use --objects-edge-aggressive for
> shallow repos, 2014-12-24) might be a good starting point to think
> about this issue.

Also note that for a while the "aggressive" form was used everywhere. I
think that started in fbd4a70 (list-objects: mark more commits as edges
in mark_edges_uninteresting - 2013-08-16), and was fixed in 1684c1b
(rev-list: add an option to mark fewer edges as uninteresting,
2014-12-24).

So it was present from v1.8.4.2 up to v2.3.0.

-Peff
N�r��yb�X��ǧv�^�)޺{.n�+ا���ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

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

2015-12-08 Thread Christian Couder
It is nice to just be able to test if untracked cache is
supported without enabling it.

Signed-off-by: Christian Couder 
---
 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


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

2015-12-08 Thread Junio C Hamano
Christian Couder  writes:

> Signed-off-by: Christian Couder 
> ---
>  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?

>  __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"));
--
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

2015-12-08 Thread Junio C Hamano
Christian Couder  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.

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.

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.

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.

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

2015-12-08 Thread Junio C Hamano
Christian Couder  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 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.

The logic in this paragraph is fuzzy to me.  Shouldn't the config
give a sensible default, that is overriden by command line options?
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)?  Why is it a good idea to allow an explicit operation from the
command line to muck with the configuration?  If the user wants to
change the configuration permanently, "git config" has been there
for the purpose for all the configuration variables to do so for
almost forever.  Why is it a good idea to make this variable so
special that the user does not have to use "git config" but disable
or enable it in some other way?

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

Good change.

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

A tip to write the explanation.  Every time you say "This means
that...", you are (perhaps unconsciously) admitting that what you
wrote immedidately before that may not be understandable and what
comes after it may be a better explanation.  Discard the sentence
before "This means that", and try to formulate your explanation
around what you wrote after it, adding information in the discarded
earlier sentence back to the later one.  Doing this exercise often
helps the result read much better.

>
> Signed-off-by: Christian Couder 
> ---
>  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 

GPG public keys

2015-12-08 Thread Jamie Evans
Hello,

Can you please point me to the public GPG keys used for source code signing?

Thanks,
Jamie
--
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 6/8] dir: add remove_untracked_cache()

2015-12-08 Thread Junio C Hamano
Christian Couder  writes:

> This new function will be used in a later patch.
>
> Signed-off-by: Christian Couder 
> ---

Up to this step I looked at and they made sense (I am not saying the
remainder of the series do not make sense).

I however wonder where the memory used for untracked cache goes when
this is called?

>  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
--
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] contrib/subtree: fix "subtree split" skipped-merge bug

2015-12-08 Thread Dave Ware
A bug occurs in 'git-subtree split' where a merge is skipped even when
both parents act on the subtree, provided the merge results in a tree
identical to one of the parents. Fix by copying the merge if at least
one parent is non-identical, and the non-identical parent is not an
ancestor of the identical parent.

Also, add a test case which checks that a descendant can be pushed to
its ancestor in this case.

Signed-off-by: Dave Ware 
---

Notes:
Many thanks to Eric Sunshine for his adivce on this patch
Changes since v2:
- Minor improvements to commit message
- Changed space indentation to tab indentation in test case
- Changed use of rev-list for obtaining commit id to use rev-parse instead
Changes since v1:
- Minor improvements to commit message
- Added sign off
- Moved test case from own file into t7900-subtree.sh
- Added subshell to test around 'cd'
- Moved record of commit for cherry-pick to variable instead of dumping 
into file

[v2]: 
http://thread.gmane.org/gmane.comp.version-control.git/282065/focus=282121
[v1]: http://thread.gmane.org/gmane.comp.version-control.git/282065

 contrib/subtree/git-subtree.sh | 12 +++--
 contrib/subtree/t/t7900-subtree.sh | 52 ++
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 9f06571..b837531 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -479,8 +479,16 @@ copy_or_skip()
p="$p -p $parent"
fi
done
-   
-   if [ -n "$identical" ]; then
+
+   copycommit=
+   if [ -n "$identical" ] && [ -n "$nonidentical" ]; then
+   extras=$(git rev-list --boundary $identical..$nonidentical)
+   if [ -n "$extras" ]; then
+   # we need to preserve history along the other branch
+   copycommit=1
+   fi
+   fi
+   if [ -n "$identical" ] && [ -z "$copycommit" ]; then
echo $identical
else
copy_commit $rev $tree "$p" || exit $?
diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index 9051982..710278c 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -468,4 +468,56 @@ test_expect_success 'verify one file change per commit' '
))
 '
 
+test_expect_success 'subtree descendent check' '
+   mkdir git_subtree_split_check &&
+   (
+   cd git_subtree_split_check &&
+   git init &&
+
+   mkdir folder &&
+
+   echo a >folder/a &&
+   git add . &&
+   git commit -m "first commit" &&
+
+   git branch branch &&
+
+   echo 0 >folder/0 &&
+   git add . &&
+   git commit -m "adding 0 to folder" &&
+
+   echo b >folder/b &&
+   git add . &&
+   git commit -m "adding b to folder" &&
+   cherry=$(git rev-parse HEAD) &&
+
+   git checkout branch &&
+   echo text >textBranch.txt &&
+   git add . &&
+   git commit -m "commit to fiddle with branch: branch" &&
+
+   git cherry-pick $cherry &&
+   git checkout master &&
+   git merge -m "merge" branch &&
+
+   git branch noop_branch &&
+
+   echo d >folder/d &&
+   git add . &&
+   git commit -m "adding d to folder" &&
+
+   git checkout noop_branch &&
+   echo moreText >anotherText.txt &&
+   git add . &&
+   git commit -m "irrelevant" &&
+
+   git checkout master &&
+   git merge -m "second merge" noop_branch &&
+
+   git subtree split --prefix folder/ --branch subtree_tip master 
&&
+   git subtree split --prefix folder/ --branch subtree_branch 
branch &&
+   git push . subtree_tip:subtree_branch
+   )
+   '
+
 test_done
-- 
1.9.1

--
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] contrib/subtree: fix "subtree split" skipped-merge bug

2015-12-08 Thread Eric Sunshine
On Tue, Dec 8, 2015 at 7:19 PM, Dave Ware  wrote:
> 'git subtree split' can incorrectly skip a merge even when both parents
> act on the subtree, provided the merge results in a tree identical to
> one of the parents. Fix by copying the merge if at least one parent is
> non-identical, and the non-identical parent is not an ancestor of the
> identical parent.
>
> Also, add a test case which checks that a descendant can be pushed to
> its ancestor in this case.
>
> Signed-off-by: Dave Ware 
> ---
> diff --git a/contrib/subtree/t/t7900-subtree.sh 
> b/contrib/subtree/t/t7900-subtree.sh
> @@ -468,4 +468,56 @@ test_expect_success 'verify one file change per commit' '
> ))
>  '
>
> +test_expect_success 'subtree descendent check' '

s/descendent/descendant/

> +   mkdir git_subtree_split_check &&
> +   (
> +   cd git_subtree_split_check &&
> +[...]
> +   git push . subtree_tip:subtree_branch
> +   )
> +   '

Style nit: don't indent closing quotation mark

>  test_done
> --
> 1.9.1
--
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 6/8] dir: add remove_untracked_cache()

2015-12-08 Thread Torsten Bögershausen
On 08.12.15 18:15, Christian Couder wrote:
> This new function will be used in a later patch.
Why not combine 5/8 with 6/8 into a single patch ?

(And please consider to mark the series with v2)


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

2015-12-08 Thread Torsten Bögershausen
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.

--
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 3/2] git.c: make sure we do not leak GIT_* to alias scripts

2015-12-08 Thread Junio C Hamano
Jeff King  writes:

> It fails for me when run via "make" (with prove or without) but not as
> "./t0001-init.sh". Looks like extra variables from my config.mak leak
> through:
>
>   $ make t0001-init.sh GIT_TEST_OPTS="-v -i"
>   [...]
>   --- expected2015-12-08 17:18:06.304699181 +
>   +++ actual  2015-12-08 17:18:06.312699180 +
>   @@ -9,5 +9,9 @@
>GIT_MERGE_VERBOSITY
>GIT_PREFIX
>GIT_TEMPLATE_DIR
>   +GIT_TEST_GIT_DAEMON
>   +GIT_TEST_HTTPD
>   +GIT_TEST_OPTS
>GIT_TEXTDOMAINDIR
>GIT_TRACE_BARE
>   +MAKEFLAGS
>   not ok 6 - No extra GIT_* on alias scripts
>
> Any GIT_TEST_* is allowed through by test-lib.sh.

Also GIT_PROVE_OPTS (which caused false positive for me).

Perhaps grab "env" output outside the alias to create the expected
(instead of a random handcrafted list that you have to maintain as
the test suite evolves), and compare it from within the alias, or
something?
--
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

2015-12-08 Thread Junio C Hamano
Junio C Hamano  writes:

> Christian Couder  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?
> 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.

 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.

It is OK to then add an "auto-detect" on top of the above, that
would only affect the second bullet point, like so:

 2a When creating an index file in an empty repository, if the
configuration is set to 'auto', do the lengthy runtime check and
enable the untracked cache in the index (even without the user
runninng "update-index --untracked-cache").

without changing any other in the first 4-bullet list.

Am I missing some other requirements?
--
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: GPG public keys

2015-12-08 Thread Junio C Hamano
Jamie Evans  writes:

> Can you please point me to the public GPG keys used for source code signing?

I suspect that you are asking about our project, but instead of
throwing you a fish, I'll show you how to catch one yourself.

In a copy of linux kernel repository I have lying around from a
random past, I did this:

$ git log --show-signature

and saw something like this:

commit c6fa8e6de3dc420cba092bf155b2ed25bcd537f7
merged tag 'arm64-fixes'
gpg: Signature made Wed 07 Oct 2015 03:10:34 AM PDT using RSA key ID 
84C16334
gpg: Can't check signature: public key not found
Merge: e82fa92 62c6c61
Author: Linus Torvalds 
Date:   Wed Oct 7 18:17:46 2015 +0100

Merge tag 'arm64-fixes' of git://git.kernel.org/pub/scm/li...

I do not have the public key with key ID 84C16334, but I can ask
public keyservers.  Put 0x84C16334 in "Search String" in pgp.mit.edu
and click "Do the search!"--it would result in the key that was used
to sign the merge request that resulted in this merge.

I also can do this:

$ git tag -v v3.0

and I would see something like:

object 02f8c6aee8df3cdc935e9bdd4f2d020306035dbe
type commit
tag v3.0
tagger Linus Torvalds  1311301049 -0700

Linux 3.0

w00t!
gpg: Signature made Thu 21 Jul 2011 07:17:44 PM PDT using DSA key ID 
76E21CBB
gpg: Good signature from "Linus Torvalds (tag signing key) 
"
...

to find that Linus's tag signing key has ID 0x76E21CBB (I do have
his key in my keyring, so this does not say "Can't check").

Perhaps you can do the same to whatever project you are interested
in.  For example, here is a starting point to do the same for our
recent v2.6.4 tag:

$ git tag -v v2.6.4
gpg: Signature made Tue 08 Dec 2015 02:12:50 PM PST using RSA key ID 
96AFE6CB
gpg: Can't check signature: public key not found
error: could not verify the tag 'v2.6.4'

--
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 v2.6.4

2015-12-08 Thread Junio C Hamano
The latest maintenance release Git v2.6.4 is now available at
the usual places, with a handful of fixes that are already in
the 'master' front.

The tarballs are found at:

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

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

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git



Git v2.6.4 Release Notes


Fixes since v2.6.3
--

 * The "configure" script did not test for -lpthread correctly, which
   upset some linkers.

 * Add support for talking http/https over socks proxy.

 * Portability fix for Windows, which may rewrite $SHELL variable using
   non-POSIX paths.

 * We now consistently allow all hooks to ignore their standard input,
   rather than having git complain of SIGPIPE.

 * Fix shell quoting in contrib script.

 * Test portability fix for a topic in v2.6.1.

 * Allow tilde-expansion in some http config variables.

 * Give a useful special case "diff/show --word-diff-regex=." as an
   example in the documentation.

 * Fix for a corner case in filter-branch.

 * Make git-p4 work on a detached head.

 * Documentation clarification for "check-ignore" without "--verbose".

 * Just like the working tree is cleaned up when the user cancelled
   submission in P4Submit.applyCommit(), clean up the mess if "p4
   submit" fails.

 * Having a leftover .idx file without corresponding .pack file in
   the repository hurts performance; "git gc" learned to prune them.

 * The code to prepare the working tree side of temporary directory
   for the "dir-diff" feature forgot that symbolic links need not be
   copied (or symlinked) to the temporary area, as the code already
   special cases and overwrites them.  Besides, it was wrong to try
   computing the object name of the target of symbolic link, which may
   not even exist or may be a directory.

 * There was no way to defeat a configured rebase.autostash variable
   from the command line, as "git rebase --no-autostash" was missing.

 * Allow "git interpret-trailers" to run outside of a Git repository.

 * Produce correct "dirty" marker for shell prompts, even when we
   are on an orphan or an unborn branch.

 * Some corner cases have been fixed in string-matching done in "git
   status".

 * Apple's common crypto implementation of SHA1_Update() does not take
   more than 4GB at a time, and we now have a compile-time workaround
   for it.

Also contains typofixes, documentation updates and trivial code
clean-ups.



Changes since v2.6.3 are as follows:

Atousa Pahlevan Duprat (2):
  sha1: provide another level of indirection for the SHA-1 functions
  sha1: allow limiting the size of the data passed to SHA1_Update()

Charles Bailey (1):
  http: treat config options sslCAPath and sslCAInfo as paths

Christian Couder (1):
  Documentation/git-update-index: add missing opts to synopsys

Clemens Buchacher (1):
  allow hooks to ignore their standard input stream

Daniel Knittl-Frank (1):
  Escape Git's exec path in contrib/rerere-train.sh script

David Aguilar (1):
  difftool: ignore symbolic links in use_wt_file

Dennis Kaarsemaker (2):
  t5813: avoid creating urls that break on cygwin
  check-ignore: correct documentation about output

Doug Kelly (2):
  t5304: test cleaning pack garbage
  gc: remove garbage .idx files from pack dir

Fredrik Medley (1):
  rebase-i-exec: Allow space in SHELL_PATH

GIRARD Etienne (1):
  git-p4: clean up after p4 submit failure

John Keeping (3):
  interpret-trailers: allow running outside a repository
  rebase: support --no-autostash
  Documentation/git-rebase: fix --no-autostash formatting

Junio C Hamano (3):
  prepare_packed_git(): refactor garbage reporting in pack directory
  Prepare for 2.6.4
  Git 2.6.4

Luke Diamand (3):
  git-p4: add failing test for submit from detached head
  git-p4: add option to system() to return subshell status
  git-p4: work with a detached head

Michael J Gruber (1):
  Documentation/diff: give --word-diff-regex=. example

Pat Thoyts (1):
  remote-http(s): support SOCKS proxies

Rainer M. Canavan (1):
  configure.ac: use $LIBS not $CFLAGS when testing -lpthread

René Scharfe (1):
  wt-status: correct and simplify check for detached HEAD

SZEDER Gábor (4):
  bash prompt: test dirty index and worktree while on an orphan branch
  bash prompt: remove a redundant 'git diff' option
  bash prompt: indicate dirty index even on orphan branches
  filter-branch: deal with object name vs. pathname ambiguity in 

[PATCH v4] contrib/subtree: fix "subtree split" skipped-merge bug

2015-12-08 Thread Dave Ware
'git subtree split' can incorrectly skip a merge even when both parents
act on the subtree, provided the merge results in a tree identical to
one of the parents. Fix by copying the merge if at least one parent is
non-identical, and the non-identical parent is not an ancestor of the
identical parent.

Also, add a test case which checks that a descendant can be pushed to
its ancestor in this case.

Signed-off-by: Dave Ware 
---

Notes:
Many thanks to Eric Sunshine and Junio Hamano for adivce on this patch

Changes since v3:
- Improvements to commit message
- Removed incorrect use of --boundary on rev-list
- Changed use of rev-list to use --count
Changes since v2:
- Minor improvements to commit message
- Changed space indentation to tab indentation in test case
- Changed use of rev-list for obtaining commit id to use rev-parse instead
Changes since v1:
- Minor improvements to commit message
- Added sign off
- Moved test case from own file into t7900-subtree.sh
- Added subshell to test around 'cd'
- Moved record of commit for cherry-pick to variable instead of dumping 
into file

[v3]: 
http://thread.gmane.org/gmane.comp.version-control.git/282065/focus=282176
[v2]: 
http://thread.gmane.org/gmane.comp.version-control.git/282065/focus=282121
[v1]: http://thread.gmane.org/gmane.comp.version-control.git/282065

 contrib/subtree/git-subtree.sh | 12 +++--
 contrib/subtree/t/t7900-subtree.sh | 52 ++
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 9f06571..ebf99d9 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -479,8 +479,16 @@ copy_or_skip()
p="$p -p $parent"
fi
done
-   
-   if [ -n "$identical" ]; then
+
+   copycommit=
+   if [ -n "$identical" ] && [ -n "$nonidentical" ]; then
+   extras=$(git rev-list --count $identical..$nonidentical)
+   if [ "$extras" -ne 0 ]; then
+   # we need to preserve history along the other branch
+   copycommit=1
+   fi
+   fi
+   if [ -n "$identical" ] && [ -z "$copycommit" ]; then
echo $identical
else
copy_commit $rev $tree "$p" || exit $?
diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index 9051982..710278c 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -468,4 +468,56 @@ test_expect_success 'verify one file change per commit' '
))
 '
 
+test_expect_success 'subtree descendent check' '
+   mkdir git_subtree_split_check &&
+   (
+   cd git_subtree_split_check &&
+   git init &&
+
+   mkdir folder &&
+
+   echo a >folder/a &&
+   git add . &&
+   git commit -m "first commit" &&
+
+   git branch branch &&
+
+   echo 0 >folder/0 &&
+   git add . &&
+   git commit -m "adding 0 to folder" &&
+
+   echo b >folder/b &&
+   git add . &&
+   git commit -m "adding b to folder" &&
+   cherry=$(git rev-parse HEAD) &&
+
+   git checkout branch &&
+   echo text >textBranch.txt &&
+   git add . &&
+   git commit -m "commit to fiddle with branch: branch" &&
+
+   git cherry-pick $cherry &&
+   git checkout master &&
+   git merge -m "merge" branch &&
+
+   git branch noop_branch &&
+
+   echo d >folder/d &&
+   git add . &&
+   git commit -m "adding d to folder" &&
+
+   git checkout noop_branch &&
+   echo moreText >anotherText.txt &&
+   git add . &&
+   git commit -m "irrelevant" &&
+
+   git checkout master &&
+   git merge -m "second merge" noop_branch &&
+
+   git subtree split --prefix folder/ --branch subtree_tip master 
&&
+   git subtree split --prefix folder/ --branch subtree_branch 
branch &&
+   git push . subtree_tip:subtree_branch
+   )
+   '
+
 test_done
-- 
1.9.1

--
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] contrib/subtree: fix "subtree split" skipped-merge bug

2015-12-08 Thread David Ware
On Wed, Dec 9, 2015 at 10:23 AM, Junio C Hamano  wrote:
> Dave Ware  writes:
>
>> A bug occurs in 'git-subtree split' where a merge is skipped even when
>> both parents act on the subtree, provided the merge results in a tree
>> identical to one of the parents. Fix by copying the merge if at least
>> one parent is non-identical, and the non-identical parent is not an
>> ancestor of the identical parent.
>>
>> Also, add a test case which checks that a descendant can be pushed to
>> its ancestor in this case.
>>
>> Signed-off-by: Dave Ware 
>> ---
>
> The first sentence may be made clearer if you rephrased the early
> part of the sentence this way:
>
> 'git subtree split' can incorrectly skip a merge even when
> both parents ...
>

Noted.

>> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
>> index 9f06571..b837531 100755
>> --- a/contrib/subtree/git-subtree.sh
>> +++ b/contrib/subtree/git-subtree.sh
>> @@ -479,8 +479,16 @@ copy_or_skip()
>>   p="$p -p $parent"
>>   fi
>>   done
>> -
>> - if [ -n "$identical" ]; then
>> +
>> + copycommit=
>> + if [ -n "$identical" ] && [ -n "$nonidentical" ]; then
>> + extras=$(git rev-list --boundary $identical..$nonidentical)
>> + if [ -n "$extras" ]; then
>> + # we need to preserve history along the other branch
>> + copycommit=1
>> + fi
>
> What is the significance of "--boundary" here?  I think for the
> purpose of "is the identical one part of the nonidentical one?" you
> do not need it, but there may be something subtle I missed.  I am
> asking this because use of "rev-list --boundary" in scripts is
> almost always a bug.
>

The other way around actually I'm trying to determine if nonidentical
contains any commits
 not in identical.  I'll confess I don't actually know specifically
what the --boundary option
does, this probably came from a stack overflow example while we were
looking up how to
best do the check. Further experimentation with the option suggests
that it does not do what
I want, so I will remove it. Thank you.

> Also, depending on how huge the output from the rev-list could be,
> you might want to use "rev-list --count $i..$n" and compare it with
> 0 instead--that way, you would not have to be worried about having
> to carry around a huge string that you would otherwise not use, only
> to see if that string is empty.

Thanks, I didn't know about that 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


Update

2015-12-08 Thread robert
Good day, hoping you read this email and respond to me in good time.I do not 
intend to solicit for funds but your time and energy in using my own resources 
to assist the less privileged becauseI am medically ill and confined at the 
moment hence I request your indulgence.I will give you a comprehensive brief 
once I hear from you.  Thanks and reply. Robert Grondahl
--
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

2015-12-08 Thread Christian Couder
On Wed, Nov 25, 2015 at 10:30 AM, Christian Couder
 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 
> ---
> 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


Re: [PATCH v3] contrib/subtree: fix "subtree split" skipped-merge bug

2015-12-08 Thread Junio C Hamano
Dave Ware  writes:

> A bug occurs in 'git-subtree split' where a merge is skipped even when
> both parents act on the subtree, provided the merge results in a tree
> identical to one of the parents. Fix by copying the merge if at least
> one parent is non-identical, and the non-identical parent is not an
> ancestor of the identical parent.
>
> Also, add a test case which checks that a descendant can be pushed to
> its ancestor in this case.
>
> Signed-off-by: Dave Ware 
> ---

The first sentence may be made clearer if you rephrased the early
part of the sentence this way:

'git subtree split' can incorrectly skip a merge even when
both parents ...

> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 9f06571..b837531 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -479,8 +479,16 @@ copy_or_skip()
>   p="$p -p $parent"
>   fi
>   done
> - 
> - if [ -n "$identical" ]; then
> +
> + copycommit=
> + if [ -n "$identical" ] && [ -n "$nonidentical" ]; then
> + extras=$(git rev-list --boundary $identical..$nonidentical)
> + if [ -n "$extras" ]; then
> + # we need to preserve history along the other branch
> + copycommit=1
> + fi

What is the significance of "--boundary" here?  I think for the
purpose of "is the identical one part of the nonidentical one?" you
do not need it, but there may be something subtle I missed.  I am
asking this because use of "rev-list --boundary" in scripts is
almost always a bug.

Also, depending on how huge the output from the rev-list could be,
you might want to use "rev-list --count $i..$n" and compare it with
0 instead--that way, you would not have to be worried about having
to carry around a huge string that you would otherwise not use, only
to see if that string is empty.

Thanks.

> + fi
> + if [ -n "$identical" ] && [ -z "$copycommit" ]; then
>   echo $identical
>   else
>   copy_commit $rev $tree "$p" || exit $?
> diff --git a/contrib/subtree/t/t7900-subtree.sh 
> b/contrib/subtree/t/t7900-subtree.sh
> index 9051982..710278c 100755
> --- a/contrib/subtree/t/t7900-subtree.sh
> +++ b/contrib/subtree/t/t7900-subtree.sh
> @@ -468,4 +468,56 @@ test_expect_success 'verify one file change per commit' '
>   ))
>  '
>  
> +test_expect_success 'subtree descendent check' '
> + mkdir git_subtree_split_check &&
> + (
> + cd git_subtree_split_check &&
> + git init &&
> +
> + mkdir folder &&
> +
> + echo a >folder/a &&
> + git add . &&
> + git commit -m "first commit" &&
> +
> + git branch branch &&
> +
> + echo 0 >folder/0 &&
> + git add . &&
> + git commit -m "adding 0 to folder" &&
> +
> + echo b >folder/b &&
> + git add . &&
> + git commit -m "adding b to folder" &&
> + cherry=$(git rev-parse HEAD) &&
> +
> + git checkout branch &&
> + echo text >textBranch.txt &&
> + git add . &&
> + git commit -m "commit to fiddle with branch: branch" &&
> +
> + git cherry-pick $cherry &&
> + git checkout master &&
> + git merge -m "merge" branch &&
> +
> + git branch noop_branch &&
> +
> + echo d >folder/d &&
> + git add . &&
> + git commit -m "adding d to folder" &&
> +
> + git checkout noop_branch &&
> + echo moreText >anotherText.txt &&
> + git add . &&
> + git commit -m "irrelevant" &&
> +
> + git checkout master &&
> + git merge -m "second merge" noop_branch &&
> +
> + git subtree split --prefix folder/ --branch subtree_tip master 
> &&
> + git subtree split --prefix folder/ --branch subtree_branch 
> branch &&
> + git push . subtree_tip:subtree_branch
> + )
> + '
> +
>  test_done
--
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