Re: [PATCH v4] Add git-grep threads param

2015-11-03 Thread Jeff King
On Tue, Nov 03, 2015 at 09:22:09AM -0800, Junio C Hamano wrote:

> > +grep.threads::
> > +   Number of grep worker threads, use it to tune up performance on
> > +   multicore machines. Default value is 8. Set to 0 to disable threading.
> > +
> 
> I am not enthused by this "Set to 0 to disable".  As Zero is
> magical, it would be more useful if 1 meant that threading is not
> used (i.e. there is only 1 worker), and 0 meant that we would
> automatically pick some reasonable parallelism for you (and we
> promise that the our choice would not be outrageously wrong), or
> something like that.

Not just useful, but consistent with other parts of git, like
pack.threads, where "0" already means "autodetect".

-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: Strange diff-index output

2015-11-03 Thread Jeff King
On Tue, Nov 03, 2015 at 02:00:33PM +1300, Ch'Gans wrote:

> I first tried "git update-index" but it didn't work. However "git
> update-index --refresh" seems to fix our problem.
> I didn't get why "--refresh" is needed thought, I'm really not
> familiar with the caching aspect of git.

It is because update-index is a general command for manipulating the
index. For example, you can add, delete, or change entries without
regard to what is in the working tree.

One of the manipulations is "refresh the index based on what is in the
working tree", and that is spelled "--refresh". Most porcelain-level git
commands (like "git diff") will do this for you automatically and
transparently. But when using the scriptable plumbing (like diff-index),
git gives you more control. This lets you do things more efficiently
(e.g,. you might refresh once and then issue several low-level
commands), at the cost of convenience.

You could also have used "git diff-index --cached HEAD", which instructs
diff-index not to look at the working tree at all (so you would compare
whatever is in the index, whether it is up to date with what is in the
working tree or not). Depending on what you are trying to achieve, that
might be fine (it's also more efficient in general, as it does not
require an lstat() of every file in the working tree).

-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: Bug: stash save -u removes (some) ignored files

2015-11-03 Thread Atousa Duprat
> felipe@felipe:testgit% git stash save -u

This does the following:
$ GIT_TRACE=1 git stash save -u
[...]
21:59:10.606094 git.c:348   trace: built-in: git 'clean'
'--force' '--quiet' '-d'

git-clean -d removes untracked directories in addition to untracked files.
Should 'git stash save -u' issue a 'git clean -d' or simply a 'git clean'?

Atousa

On Tue, Nov 3, 2015 at 2:37 PM, Felipe Sateler  wrote:
> I have seen the following problem:
>
> felipe@felipe:testgit% cat .gitignore
> **/notrack/*
> !**/notrack/track/
> notrackfile**
>
> felipe@felipe:testgit% find *
> newfile
> notrack
> notrack/1
> notrackfile1
>
> felipe@felipe:testgit% git status --porcelain
> ?? newfile
>
> felipe@felipe:testgit% git stash save -u
> Saved working directory and index state WIP on master: 523cb39 Initial commit
> HEAD is now at 523cb39 Initial commit
>
> felipe@felipe:testgit% find *
> notrackfile1
>
> So, after a stash save, git decided to keep the untracked file in the
> current directory, but not the ones inside the untracked directory.
> However, the files were "correctly" ignored and did not appear on the stash:
>
> felipe@felipe:testgit% git stash pop
> Already up-to-date!
> On branch master
> Untracked files:
>   (use "git add ..." to include in what will be committed)
>
> newfile
>
> nothing added to commit but untracked files present (use "git add" to track)
> Dropped refs/stash@{0} (e6508f345af1dd207557ad0291e7e3bce8a89b1e)
>
> felipe@felipe:testgit% find *
> newfile
> notrackfile1
>
> I think the correct behaviour should be to left the ignored files
> untouched in both scenarios.
>
> This is with git 2.6.1 (from debian).
>
> I note that if I add a file inside the notrack/track directory, it is
> correctly kept, but the files in notrack/ are still deleted.
>
> --
>
> Saludos,
> Felipe Sateler
> --
> 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



-- 
Atousa Pahlevan, PhD
M.Math. University of Waterloo, Canada
Ph.D. Department of Computer Science, University of Victoria, Canada
Voice: 415-341-6206
Email: apahle...@ieee.org
Website: www.apahlevan.org
--
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: Feature request: commit count in git-describe should use a different method

2015-11-03 Thread Mike Hommey
On Wed, Nov 04, 2015 at 12:11:27PM +0700, Robin Munn wrote:
> Several people (including me) seem to expect git-describe's commit
> count to be calculated differently than how it's actually calculated.
> For example, see the following three Stack Overflow questions:
> 
> http://stackoverflow.com/questions/31852885/git-describe-inexplicable-commit-count
> http://stackoverflow.com/questions/33116182/can-i-change-how-git-describe-counts-commits
> http://stackoverflow.com/questions/13568372/commit-count-calculation-in-git-describe
> 
> The scenario that all three questions is asking about is the following:
> 
> 1) I'm working along on a branch whose most recent tag is v1.1,
> created 96 commits ago.
> 2) Someone else merges some work into master, and tags with v1.2. I
> want to incorporate their work into my own, so I merge master into my
> branch.
> 3) I now have a branch that is one commit "forward" from tag v1.2. I
> therefore expect git-describe to say "v1.2-1-g1234567". Instead, I get
> "v1.2-97-g1234567".
> 
> Now, git-describe is working precisely as documented here. The
> documentation describes the commit count as being "the number of
> commits which would be displayed by 'git log (tag commit)..(described
> commit)' " and that is indeed what I'm getting. If I do "git log
> v1.2..HEAD", there will be 97 log entries, because the latest commit
> that is an ancestor of both v1.2 and HEAD is where my branch was
> created from master 97 commits ago.
> 
> However, this is unexpected behavior for me. I was expecting to get a
> commit count of 1, not a commit count of 97. Instead of a count of all
> the commits since I forked from master 97 commits ago, I was expecting
> a count of all the commits since the tag that git-describe has picked
> as the latest tag. In other words, instead of the count to match "git
> log v1.2..HEAD", I was expecting the count to match "git log
> --ancestry-path v1.2..HEAD".

If your branch had been merged into v1.2, and you merged v1.2 back, then
you would have a lower count. One way to look at it is that the count
tells you how much your branch differs from the tag, and 97 is a more
realistic indicator of the amount of difference between the tag and your
branch head than 1 would be.

I, for one, would be confused if the count was 1.

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


Feature request: commit count in git-describe should use a different method

2015-11-03 Thread Robin Munn
Several people (including me) seem to expect git-describe's commit
count to be calculated differently than how it's actually calculated.
For example, see the following three Stack Overflow questions:

http://stackoverflow.com/questions/31852885/git-describe-inexplicable-commit-count
http://stackoverflow.com/questions/33116182/can-i-change-how-git-describe-counts-commits
http://stackoverflow.com/questions/13568372/commit-count-calculation-in-git-describe

The scenario that all three questions is asking about is the following:

1) I'm working along on a branch whose most recent tag is v1.1,
created 96 commits ago.
2) Someone else merges some work into master, and tags with v1.2. I
want to incorporate their work into my own, so I merge master into my
branch.
3) I now have a branch that is one commit "forward" from tag v1.2. I
therefore expect git-describe to say "v1.2-1-g1234567". Instead, I get
"v1.2-97-g1234567".

Now, git-describe is working precisely as documented here. The
documentation describes the commit count as being "the number of
commits which would be displayed by 'git log (tag commit)..(described
commit)' " and that is indeed what I'm getting. If I do "git log
v1.2..HEAD", there will be 97 log entries, because the latest commit
that is an ancestor of both v1.2 and HEAD is where my branch was
created from master 97 commits ago.

However, this is unexpected behavior for me. I was expecting to get a
commit count of 1, not a commit count of 97. Instead of a count of all
the commits since I forked from master 97 commits ago, I was expecting
a count of all the commits since the tag that git-describe has picked
as the latest tag. In other words, instead of the count to match "git
log v1.2..HEAD", I was expecting the count to match "git log
--ancestry-path v1.2..HEAD".

As shown by the Stack Overflow questions above (one of which is mine),
I am not alone in finding this behavior to be surprising. I would like
to request that git-describe acquire an additional option,
"--ancestry-path", to use the same method as "git log --ancestry-path"
to count commits. I would prefer that this become the default, but I
realize that that might be a breaking change (some people might have
build scripts that relied on git-describe's current behavior).

This is either a bug report or a feature request, depending on how
intended the current commit-count behavior is.

I've reproduced this behavior on both older versions of git (1.9.1)
and recent versions (2.6.2).

-- 
Robin Munn
robin.m...@gmail.com
GPG key 0x4543D577
--
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] Limit the size of the data block passed to SHA1_Update()

2015-11-03 Thread Atousa Duprat
Thank you for the feedback. The patch is updated as suggested.


On Tue, Nov 3, 2015 at 3:51 AM, Torsten Bögershausen  wrote:
> On 11/03/2015 07:58 AM, atous...@gmail.com wrote:
>>
>> From: Atousa Pahlevan Duprat 
>
> Minor comments inline
>>
>> diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
>> index b864df6..d085412 100644
>> --- a/block-sha1/sha1.h
>> +++ b/block-sha1/sha1.h
>> @@ -18,5 +18,5 @@ void blk_SHA1_Final(unsigned char hashout[20],
>> blk_SHA_CTX *ctx);
>> #define git_SHA_CTX blk_SHA_CTX
>>   #define git_SHA1_Init blk_SHA1_Init
>> -#define git_SHA1_Updateblk_SHA1_Update
>> +#define platform_SHA1_Update   blk_SHA1_Update
>>   #define git_SHA1_Finalblk_SHA1_Final
>> diff --git a/cache.h b/cache.h
>> index 79066e5..a501652 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -10,12 +10,21 @@
>>   #include "trace.h"
>>   #include "string-list.h"
>>   +// platform's underlying implementation of SHA1
>
> Please use /* */ for comments
>
>>   #include SHA1_HEADER
>>   #ifndef git_SHA_CTX
>> -#define git_SHA_CTXSHA_CTX
>> -#define git_SHA1_Init  SHA1_Init
>> -#define git_SHA1_UpdateSHA1_Update
>> -#define git_SHA1_Final SHA1_Final
>> +#define git_SHA_CTXSHA_CTX
>> +#define git_SHA1_Init  SHA1_Init
>> +#define platform_SHA1_Update   SHA1_Update
>> +#define git_SHA1_Final SHA1_Final
>> +#endif
>> +
>> +// choose whether chunked implementation or not
>> +#ifdef SHA1_MAX_BLOCK_SIZE
>> +int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len);
>> +#define git_SHA1_Update   git_SHA1_Update_Chunked
>> +#else
>> +#define git_SHA1_Update   platform_SHA1_Update
>>   #endif
>> #include 
>> diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
>> index c8b9b0e..d3fb264 100644
>> --- a/compat/apple-common-crypto.h
>> +++ b/compat/apple-common-crypto.h
>> @@ -16,6 +16,10 @@
>>   #undef TYPE_BOOL
>>   #endif
>>   +#ifndef SHA1_MAX_BLOCK_SIZE
>> +#error Using Apple Common Crypto library requires setting
>> SHA1_MAX_BLOCK_SIZE
>> +#endif
>> +
>>   #ifdef APPLE_LION_OR_NEWER
>>   #define git_CC_error_check(pattern, err) \
>> do { \
>> diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c
>> new file mode 100644
>> index 000..61f67de
>> --- /dev/null
>> +++ b/compat/sha1_chunked.c
>> @@ -0,0 +1,19 @@
>> +#include "cache.h"
>> +
>> +int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len)
>> +{
>> +   size_t nr;
>> +   size_t total = 0;
>> +   const char *cdata = (const char*)data;
>> +
>> +   while (len > 0) {
>
> size_t is unsigned, isn't it ?
> Better to use  "while (len) {"
>
>> +   nr = len;
>> +   if (nr > SHA1_MAX_BLOCK_SIZE)
>> +   nr = SHA1_MAX_BLOCK_SIZE;
>> +   platform_SHA1_Update(c, cdata, nr);
>> +   total += nr;
>> +   cdata += nr;
>> +   len -= nr;
>> +   }
>> +   return total;
>> +}
>
>



-- 
Atousa Pahlevan, PhD
M.Math. University of Waterloo, Canada
Ph.D. Department of Computer Science, University of Victoria, Canada
Voice: 415-341-6206
Email: apahle...@ieee.org
Website: www.apahlevan.org
--
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] Limit the size of the data block passed to SHA1_Update()

2015-11-03 Thread atousa . p
From: Atousa Pahlevan Duprat 

Some implementations of SHA_Updates have inherent limits
on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined
to set the max chunk size supported, if required.  This is
enabled for OSX CommonCrypto library and set to 1GiB.

Signed-off-by: Atousa Pahlevan Duprat 
---
 Makefile | 16 +++-
 block-sha1/sha1.h|  2 +-
 cache.h  | 17 +
 compat/apple-common-crypto.h |  4 
 compat/sha1_chunked.c| 19 +++
 5 files changed, 52 insertions(+), 6 deletions(-)
 create mode 100644 compat/sha1_chunked.c

diff --git a/Makefile b/Makefile
index 04c2231..1b098cc 100644
--- a/Makefile
+++ b/Makefile
@@ -136,11 +136,15 @@ all::
 # to provide your own OpenSSL library, for example from MacPorts.
 #
 # Define BLK_SHA1 environment variable to make use of the bundled
-# optimized C SHA1 routine.
+# optimized C SHA1 routine.  This implies NO_APPLE_COMMON_CRYPTO.
 #
 # Define PPC_SHA1 environment variable when running make to make use of
 # a bundled SHA1 routine optimized for PowerPC.
 #
+# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can
+# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO
+# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).
+#
 # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
 #
 # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
@@ -986,6 +990,10 @@ ifeq (no,$(USE_PARENS_AROUND_GETTEXT_N))
 endif
 endif
 
+ifdef BLK_SHA1
+   NO_APPLE_COMMON_CRYPTO=1
+endif
+
 ifeq ($(uname_S),Darwin)
ifndef NO_FINK
ifeq ($(shell test -d /sw/lib && echo y),y)
@@ -1346,6 +1354,8 @@ else
 ifdef APPLE_COMMON_CRYPTO
COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
SHA1_HEADER = 
+   # Apple CommonCrypto requires chunking
+   SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
 else
SHA1_HEADER = 
EXTLIBS += $(LIB_4_CRYPTO)
@@ -1353,6 +1363,10 @@ endif
 endif
 endif
 
+ifdef SHA1_MAX_BLOCK_SIZE
+   LIB_OBJS += compat/sha1_chunked.o
+   BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
+endif
 ifdef NO_PERL_MAKEMAKER
export NO_PERL_MAKEMAKER
 endif
diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
index b864df6..d085412 100644
--- a/block-sha1/sha1.h
+++ b/block-sha1/sha1.h
@@ -18,5 +18,5 @@ void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX 
*ctx);
 
 #define git_SHA_CTXblk_SHA_CTX
 #define git_SHA1_Init  blk_SHA1_Init
-#define git_SHA1_Updateblk_SHA1_Update
+#define platform_SHA1_Update   blk_SHA1_Update
 #define git_SHA1_Final blk_SHA1_Final
diff --git a/cache.h b/cache.h
index 79066e5..e345e38 100644
--- a/cache.h
+++ b/cache.h
@@ -10,12 +10,21 @@
 #include "trace.h"
 #include "string-list.h"
 
+/* platform's underlying implementation of SHA1 */
 #include SHA1_HEADER
 #ifndef git_SHA_CTX
-#define git_SHA_CTXSHA_CTX
-#define git_SHA1_Init  SHA1_Init
-#define git_SHA1_UpdateSHA1_Update
-#define git_SHA1_Final SHA1_Final
+#define git_SHA_CTXSHA_CTX
+#define git_SHA1_Init  SHA1_Init
+#define platform_SHA1_Update   SHA1_Update
+#define git_SHA1_Final SHA1_Final
+#endif
+
+/* choose chunked implementation or not */
+#ifdef SHA1_MAX_BLOCK_SIZE
+int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len);
+#define git_SHA1_Update   git_SHA1_Update_Chunked
+#else
+#define git_SHA1_Update   platform_SHA1_Update
 #endif
 
 #include 
diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
index c8b9b0e..d3fb264 100644
--- a/compat/apple-common-crypto.h
+++ b/compat/apple-common-crypto.h
@@ -16,6 +16,10 @@
 #undef TYPE_BOOL
 #endif
 
+#ifndef SHA1_MAX_BLOCK_SIZE
+#error Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE
+#endif
+
 #ifdef APPLE_LION_OR_NEWER
 #define git_CC_error_check(pattern, err) \
do { \
diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c
new file mode 100644
index 000..6d0062b
--- /dev/null
+++ b/compat/sha1_chunked.c
@@ -0,0 +1,19 @@
+#include "cache.h"
+
+int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len)
+{
+   size_t nr;
+   size_t total = 0;
+   const char *cdata = (const char*)data;
+
+   while (len) {
+   nr = len;
+   if (nr > SHA1_MAX_BLOCK_SIZE)
+   nr = SHA1_MAX_BLOCK_SIZE;
+   platform_SHA1_Update(c, cdata, nr);
+   total += nr;
+   cdata += nr;
+   len -= nr;
+   }
+   return total;
+}
-- 
2.4.9 (Apple Git-60)

--
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] prepare_packed_git(): refactor garbage reporting in pack directory

2015-11-03 Thread Doug Kelly
On Wed, Oct 28, 2015 at 5:43 PM, Doug Kelly  wrote:
> On Wed, Oct 28, 2015 at 12:48 PM, Junio C Hamano  wrote:
>> Junio C Hamano  writes:
>>
>>> Eric Sunshine  writes:
>>>
> -static void real_report_garbage(const char *desc, const char *path)
> +const char *bits_to_msg(unsigned seen_bits)

 If you don't expect other callers outside this file, then this should
 be declared 'static'. If you do expect future external callers, then
 this should be declared in a public header file (but renamed to be
 more meaningful).
>>>
>>> I think this can be private to this file.  The sole point of moving
>>> this logic to this file is to make it private, after all ;-)  Thanks
>>> for sharp eyes.
>>>
>>> Together with the need for a description on "why", this probably
>>> deserves a test or two, probably at the end of t5304.
>>>
>>> Thanks.
>>
>> Does somebody want to help tying the final loose ends on this topic?
>> It has been listed in the [Stalled] section for too long, I _think_
>> what it attempts to do is a worthy thing, and it is shame to see the
>> initial implementation and review cycles we have spent so far go to
>> waste.
>>
>> If I find nothing else to do before any taker appears, I could
>> volunteer myself, but thought I should ask first.
>>
>> Thanks.
>
> I agree; I've been wanting to get back to it, but had some
> higher-priority things at work for a while, so I've not had time.  I'd
> be happy to get back into it, but if you get to it first, believe me,
> I'm not going to be offended. :)
>
> I'll see if I can't devote a little extra time to it this upcoming
> week, though.  Hopefully it doesn't need too much additional polishing
> to be ready.
>
> P.S. Does a Googler want to tell the Inbox team that the inability to
> send plain-text email is really annoying? :P

I think the patches I sent (a bit prematurely) address the remaining
comments... I did find there was a relevant test in t5304 already, so
I added a new test in the same section (and cleaned up some of the
garbage it wasn't removing before).  I'm not sure if it's poor form to
move tests around like this, but I figured it might be best to keep
them logically grouped.

Let me know if there's anything I can do, and once again, sorry for the delay!
--
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/3] t5304: Add test for cleaning pack garbage

2015-11-03 Thread Doug Kelly
Pack garbage, noticeably stale .idx files, can be cleaned up during
a garbage collection.  This tests to ensure such garbage is properly
cleaned up.

Note that the prior test for checking pack garbage with count-objects
left some stale garbage after the test exited.  This has also been
corrected.

Signed-off-by: Doug Kelly 
---
 t/t5304-prune.sh | 21 +
 1 file changed, 21 insertions(+)

diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 023d7c6..0297515 100755
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -219,6 +219,7 @@ test_expect_success 'gc: prune old objects after local 
clone' '
 
 test_expect_success 'garbage report in count-objects -v' '
test_when_finished "rm -f .git/objects/pack/fake*" &&
+   test_when_finished "rm -f .git/objects/pack/foo*" &&
: >.git/objects/pack/foo &&
: >.git/objects/pack/foo.bar &&
: >.git/objects/pack/foo.keep &&
@@ -244,6 +245,26 @@ EOF
test_cmp expected actual
 '
 
+test_expect_failure 'clean pack garbage with gc' '
+   test_when_finished "rm -f .git/objects/pack/fake*" &&
+   test_when_finished "rm -f .git/objects/pack/foo*" &&
+   : >.git/objects/pack/foo.keep &&
+   : >.git/objects/pack/foo.pack &&
+   : >.git/objects/pack/fake.idx &&
+   : >.git/objects/pack/fake2.keep &&
+   : >.git/objects/pack/fake2.idx &&
+   : >.git/objects/pack/fake3.keep &&
+   git gc &&
+   git count-objects -v 2>stderr &&
+   grep "^warning:" stderr | sort >actual &&
+   cat >expected <<\EOF &&
+warning: no corresponding .idx or .pack: .git/objects/pack/fake3.keep
+warning: no corresponding .idx: .git/objects/pack/foo.keep
+warning: no corresponding .idx: .git/objects/pack/foo.pack
+EOF
+   test_cmp expected actual
+'
+
 test_expect_success 'prune .git/shallow' '
SHA1=`echo hi|git commit-tree HEAD^{tree}` &&
echo $SHA1 >.git/shallow &&
-- 
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


[PATCH 3/3] gc: Remove garbage .idx files from pack dir

2015-11-03 Thread Doug Kelly
Add a custom report_garbage handler to collect and remove garbage
.idx files from the pack directory.

Signed-off-by: Doug Kelly 
---
 builtin/gc.c | 20 
 t/t5304-prune.sh |  2 +-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index df3e454..668f975 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -45,6 +45,21 @@ static struct argv_array rerere = ARGV_ARRAY_INIT;
 
 static struct tempfile pidfile;
 static struct lock_file log_lock;
+static struct string_list pack_garbage = STRING_LIST_INIT_DUP;
+
+static void clean_pack_garbage(void)
+{
+   int i;
+   for (i = 0; i < pack_garbage.nr; i++)
+   unlink_or_warn(pack_garbage.items[i].string);
+   string_list_clear(&pack_garbage, 0);
+}
+
+static void report_pack_garbage(unsigned seen_bits, const char *path)
+{
+   if (seen_bits == PACKDIR_FILE_IDX)
+   string_list_append(&pack_garbage, path);
+}
 
 static void git_config_date_string(const char *key, const char **output)
 {
@@ -416,6 +431,11 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
if (run_command_v_opt(rerere.argv, RUN_GIT_CMD))
return error(FAILED_RUN, rerere.argv[0]);
 
+   report_garbage = report_pack_garbage;
+   reprepare_packed_git();
+   if (pack_garbage.nr > 0)
+   clean_pack_garbage();
+
if (auto_gc && too_many_loose_objects())
warning(_("There are too many unreachable loose objects; "
"run 'git prune' to remove them."));
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 0297515..def203c 100755
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -245,7 +245,7 @@ EOF
test_cmp expected actual
 '
 
-test_expect_failure 'clean pack garbage with gc' '
+test_expect_success 'clean pack garbage with gc' '
test_when_finished "rm -f .git/objects/pack/fake*" &&
test_when_finished "rm -f .git/objects/pack/foo*" &&
: >.git/objects/pack/foo.keep &&
-- 
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


[PATCH 1/3] prepare_packed_git(): refactor garbage reporting in pack directory

2015-11-03 Thread Doug Kelly
From: Junio C Hamano 

The hook to report "garbage" files in $GIT_OBJECT_DIRECTORY/pack/
could be generic but is too specific to count-object's needs.

Move the part to produce human-readable messages to count-objects,
and refine the interface to callback with the "bits" with values
defined in the cache.h header file, so that other callers (e.g.
prune) can later use the same mechanism to enumerate different
kinds of garbage files and do something intelligent about them,
other than reporting in textual messages.

Signed-off-by: Junio C Hamano 
Signed-off-by: Doug Kelly 
---
 builtin/count-objects.c | 26 --
 cache.h |  7 +--
 path.c  |  2 +-
 sha1_file.c | 23 ++-
 4 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index ad0c799..ba92919 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -15,9 +15,31 @@ static int verbose;
 static unsigned long loose, packed, packed_loose;
 static off_t loose_size;
 
-static void real_report_garbage(const char *desc, const char *path)
+static const char *bits_to_msg(unsigned seen_bits)
+{
+   switch (seen_bits) {
+   case 0:
+   return "no corresponding .idx or .pack";
+   case PACKDIR_FILE_GARBAGE:
+   return "garbage found";
+   case PACKDIR_FILE_PACK:
+   return "no corresponding .idx";
+   case PACKDIR_FILE_IDX:
+   return "no corresponding .pack";
+   case PACKDIR_FILE_PACK|PACKDIR_FILE_IDX:
+   default:
+   return NULL;
+   }
+}
+
+static void real_report_garbage(unsigned seen_bits, const char *path)
 {
struct stat st;
+   const char *desc = bits_to_msg(seen_bits);
+
+   if (!desc)
+   return;
+
if (!stat(path, &st))
size_garbage += st.st_size;
warning("%s: %s", desc, path);
@@ -27,7 +49,7 @@ static void real_report_garbage(const char *desc, const char 
*path)
 static void loose_garbage(const char *path)
 {
if (verbose)
-   report_garbage("garbage found", path);
+   report_garbage(PACKDIR_FILE_GARBAGE, path);
 }
 
 static int count_loose(const unsigned char *sha1, const char *path, void *data)
diff --git a/cache.h b/cache.h
index 3ba0b8f..736abc0 100644
--- a/cache.h
+++ b/cache.h
@@ -1289,8 +1289,11 @@ struct pack_entry {
 
 extern struct packed_git *parse_pack_index(unsigned char *sha1, const char 
*idx_path);
 
-/* A hook for count-objects to report invalid files in pack directory */
-extern void (*report_garbage)(const char *desc, const char *path);
+/* A hook to report invalid files in pack directory */
+#define PACKDIR_FILE_PACK 1
+#define PACKDIR_FILE_IDX 2
+#define PACKDIR_FILE_GARBAGE 4
+extern void (*report_garbage)(unsigned seen_bits, const char *path);
 
 extern void prepare_packed_git(void);
 extern void reprepare_packed_git(void);
diff --git a/path.c b/path.c
index c740c4f..f28ace2 100644
--- a/path.c
+++ b/path.c
@@ -363,7 +363,7 @@ void report_linked_checkout_garbage(void)
strbuf_setlen(&sb, len);
strbuf_addstr(&sb, path);
if (file_exists(sb.buf))
-   report_garbage("unused in linked checkout", sb.buf);
+   report_garbage(PACKDIR_FILE_GARBAGE, sb.buf);
}
strbuf_release(&sb);
 }
diff --git a/sha1_file.c b/sha1_file.c
index c5b31de..3d56746 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1217,27 +1217,16 @@ void install_packed_git(struct packed_git *pack)
packed_git = pack;
 }
 
-void (*report_garbage)(const char *desc, const char *path);
+void (*report_garbage)(unsigned seen_bits, const char *path);
 
 static void report_helper(const struct string_list *list,
  int seen_bits, int first, int last)
 {
-   const char *msg;
-   switch (seen_bits) {
-   case 0:
-   msg = "no corresponding .idx or .pack";
-   break;
-   case 1:
-   msg = "no corresponding .idx";
-   break;
-   case 2:
-   msg = "no corresponding .pack";
-   break;
-   default:
+   if (seen_bits == (PACKDIR_FILE_PACK|PACKDIR_FILE_IDX))
return;
-   }
+
for (; first < last; first++)
-   report_garbage(msg, list->items[first].string);
+   report_garbage(seen_bits, list->items[first].string);
 }
 
 static void report_pack_garbage(struct string_list *list)
@@ -1260,7 +1249,7 @@ static void report_pack_garbage(struct string_list *list)
if (baselen == -1) {
const char *dot = strrchr(path, '.');
if (!dot) {
-   report_garbage("garbage found", path);
+   report_garbage(PACKDIR_FILE_GARBAGE, path);
continue;
  

Re: [BUG] Wrong worktree path when using multiple worktree

2015-11-03 Thread Mike Rappazzo
On Tue, Nov 3, 2015 at 5:27 PM, Mike Rappazzo  wrote:
> On Tue, Nov 3, 2015 at 11:58 AM, Nicolas Morey-Chaisemartin
>  wrote:
>> Hi,
>>
>> There seem to be an issue with the path computed for a worktree when 
>> multiple worktree were created (using git worktree)
>> In my Setup, I have 3 repos:
>> A/repo (main One)
>> A/repo-patches (worktree, using branch dev)
>> B/repo (worktree, using branch next)
>>
>> I'm working in A/repo-patches an run:
>> $ git checkout next
>> fatal: 'next' is already checked out at 'A/repo-patches'
>>
>> Which is partially true but not completely.
>> I looked a bit in the code and found that the issue comes from here 
>> (get_linked_worktree):
>> if (!strbuf_strip_suffix(&worktree_path, "/.git")) {
>> strbuf_reset(&worktree_path);
>> strbuf_addstr(&worktree_path, absolute_path("."));
>> strbuf_strip_suffix(&worktree_path, "/.");
>> }
>> Because it wrongfully assumes that I am in the linked worktree.
>> I checked in the .git/worktree files and couldn't see anything that actually 
>> points to where the repo are setup.
>>
>> Nicolas
>
> This is code that I worked on, but I am unable to reproduce it locally
> just yet.  Before I dig too deep, could you report the contents of
> "A/repo/.git/worktrees/repo-patches/gitdir" (or similar)?  There is
> another issue reported[1] where the contents of the gitdir for a
> linked worktree are overwritten in some cases.  A fix for this is
> being worked on (see discussion).
>
> In the mean time, I will continue to try and reproduce.
>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/280307

Followup:  I was able to reproduce the error when I tried to simulate
the aforementioned bug.  Here is a test which I added to t2027 (not
intended to publish):

+test_expect_success '"checkout" branch already checked out' '
+ git worktree add -b linked_1_br linked_1 master &&
+ git worktree add -b linked_2_br linked_2 master &&
+ echo ".git" > .git/worktrees/linked_2/gitdir &&
+ test_must_fail git -C linked_1 checkout linked_2_br
+'
+

Test run result:

Preparing linked_1 (identifier linked_1)
HEAD is now at 2519212 init
Preparing linked_2 (identifier linked_2)
HEAD is now at 2519212 init
fatal: 'linked_2_br' is already checked out at
'/Users/mike/code/git-source/t/trash
directory.t2027-worktree-list/linked_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


[PATCHv3 04/11] submodule-config: keep update strategy around

2015-11-03 Thread Stefan Beller
We need the submodule update strategies in a later patch.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 submodule-config.c | 11 +++
 submodule-config.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/submodule-config.c b/submodule-config.c
index afe0ea8..4239b0e 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -194,6 +194,7 @@ static struct submodule *lookup_or_create_by_name(struct 
submodule_cache *cache,
 
submodule->path = NULL;
submodule->url = NULL;
+   submodule->update = NULL;
submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
submodule->ignore = NULL;
 
@@ -311,6 +312,16 @@ static int parse_config(const char *var, const char 
*value, void *data)
free((void *) submodule->url);
submodule->url = xstrdup(value);
}
+   } else if (!strcmp(item.buf, "update")) {
+   if (!value)
+   ret = config_error_nonbool(var);
+   else if (!me->overwrite && submodule->update != NULL)
+   warn_multiple_config(me->commit_sha1, submodule->name,
+"update");
+   else {
+   free((void *) submodule->update);
+   submodule->update = xstrdup(value);
+   }
}
 
strbuf_release(&name);
diff --git a/submodule-config.h b/submodule-config.h
index 9061e4e..f9e2a29 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -14,6 +14,7 @@ struct submodule {
const char *url;
int fetch_recurse;
const char *ignore;
+   const char *update;
/* the sha1 blob id of the responsible .gitmodules file */
unsigned char gitmodules_sha1[20];
 };
-- 
2.6.1.247.ge8f2a41.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


[PATCHv3 07/11] submodule-config: introduce parse_generic_submodule_config

2015-11-03 Thread Stefan Beller
This rewrites parse_config to distinguish between configs specific to
one submodule and configs which apply generically to all submodules.
We do not have generic submodule configs yet, but the next patch will
introduce "submodule.jobs".

Signed-off-by: Stefan Beller 
---
 submodule-config.c | 41 -
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/submodule-config.c b/submodule-config.c
index b826841..29e21b2 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -234,17 +234,22 @@ struct parse_config_parameter {
int overwrite;
 };
 
-static int parse_config(const char *var, const char *value, void *data)
+static int parse_generic_submodule_config(const char *key,
+ const char *var,
+ const char *value,
+ struct parse_config_parameter *me)
 {
-   struct parse_config_parameter *me = data;
-   struct submodule *submodule;
-   int subsection_len, ret = 0;
-   const char *subsection, *key;
-
-   if (parse_config_key(var, "submodule", &subsection,
-&subsection_len, &key) < 0 || !subsection_len)
-   return 0;
+   return 0;
+}
 
+static int parse_specific_submodule_config(const char *subsection, int 
subsection_len,
+  const char *key,
+  const char *var,
+  const char *value,
+  struct parse_config_parameter *me)
+{
+   int ret = 0;
+   struct submodule *submodule;
submodule = lookup_or_create_by_name(me->cache,
 me->gitmodules_sha1,
 subsection, subsection_len);
@@ -314,6 +319,24 @@ static int parse_config(const char *var, const char 
*value, void *data)
return ret;
 }
 
+static int parse_config(const char *var, const char *value, void *data)
+{
+   struct parse_config_parameter *me = data;
+   int subsection_len;
+   const char *subsection, *key;
+
+   if (parse_config_key(var, "submodule", &subsection,
+&subsection_len, &key) < 0)
+   return 0;
+
+   if (!subsection_len)
+   return parse_generic_submodule_config(key, var, value, me);
+   else
+   return parse_specific_submodule_config(subsection,
+  subsection_len, key,
+  var, value, me);
+}
+
 static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
  unsigned char *gitmodules_sha1)
 {
-- 
2.6.1.247.ge8f2a41.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


[PATCHv3 06/11] submodule-config: remove name_and_item_from_var

2015-11-03 Thread Stefan Beller
`name_and_item_from_var` does not provide the proper abstraction
we need here in a later patch.

Signed-off-by: Stefan Beller 
---
 submodule-config.c | 48 
 1 file changed, 16 insertions(+), 32 deletions(-)

diff --git a/submodule-config.c b/submodule-config.c
index 6d01941..b826841 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -161,31 +161,17 @@ static struct submodule *cache_lookup_name(struct 
submodule_cache *cache,
return NULL;
 }
 
-static int name_and_item_from_var(const char *var, struct strbuf *name,
- struct strbuf *item)
-{
-   const char *subsection, *key;
-   int subsection_len, parse;
-   parse = parse_config_key(var, "submodule", &subsection,
-   &subsection_len, &key);
-   if (parse < 0 || !subsection)
-   return 0;
-
-   strbuf_add(name, subsection, subsection_len);
-   strbuf_addstr(item, key);
-
-   return 1;
-}
-
 static struct submodule *lookup_or_create_by_name(struct submodule_cache 
*cache,
-   const unsigned char *gitmodules_sha1, const char *name)
+ const unsigned char 
*gitmodules_sha1,
+ const char *name_ptr, int 
name_len)
 {
struct submodule *submodule;
struct strbuf name_buf = STRBUF_INIT;
+   char *name = xmemdupz(name_ptr, name_len);
 
submodule = cache_lookup_name(cache, gitmodules_sha1, name);
if (submodule)
-   return submodule;
+   goto out;
 
submodule = xmalloc(sizeof(*submodule));
 
@@ -201,7 +187,8 @@ static struct submodule *lookup_or_create_by_name(struct 
submodule_cache *cache,
hashcpy(submodule->gitmodules_sha1, gitmodules_sha1);
 
cache_add(cache, submodule);
-
+out:
+   free(name);
return submodule;
 }
 
@@ -251,18 +238,18 @@ static int parse_config(const char *var, const char 
*value, void *data)
 {
struct parse_config_parameter *me = data;
struct submodule *submodule;
-   struct strbuf name = STRBUF_INIT, item = STRBUF_INIT;
-   int ret = 0;
+   int subsection_len, ret = 0;
+   const char *subsection, *key;
 
-   /* this also ensures that we only parse submodule entries */
-   if (!name_and_item_from_var(var, &name, &item))
+   if (parse_config_key(var, "submodule", &subsection,
+&subsection_len, &key) < 0 || !subsection_len)
return 0;
 
submodule = lookup_or_create_by_name(me->cache,
 me->gitmodules_sha1,
-name.buf);
+subsection, subsection_len);
 
-   if (!strcmp(item.buf, "path")) {
+   if (!strcmp(key, "path")) {
if (!value)
ret = config_error_nonbool(var);
else if (!me->overwrite && submodule->path)
@@ -275,7 +262,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
submodule->path = xstrdup(value);
cache_put_path(me->cache, submodule);
}
-   } else if (!strcmp(item.buf, "fetchrecursesubmodules")) {
+   } else if (!strcmp(key, "fetchrecursesubmodules")) {
/* when parsing worktree configurations we can die early */
int die_on_error = is_null_sha1(me->gitmodules_sha1);
if (!me->overwrite &&
@@ -286,7 +273,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
submodule->fetch_recurse = parse_fetch_recurse(
var, value,
die_on_error);
-   } else if (!strcmp(item.buf, "ignore")) {
+   } else if (!strcmp(key, "ignore")) {
if (!value)
ret = config_error_nonbool(var);
else if (!me->overwrite && submodule->ignore)
@@ -302,7 +289,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
free((void *) submodule->ignore);
submodule->ignore = xstrdup(value);
}
-   } else if (!strcmp(item.buf, "url")) {
+   } else if (!strcmp(key, "url")) {
if (!value) {
ret = config_error_nonbool(var);
} else if (!me->overwrite && submodule->url) {
@@ -312,7 +299,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
free((void *) submodule->url);
submodule->url = xstrdup(value);
}
-   } else if (!strcmp(item.buf, "update")) {
+   } else if (!strcmp(key, "update")) {
if (!value)
re

[PATCHv3 02/11] run-command: report failure for degraded output just once

2015-11-03 Thread Stefan Beller
The warning message is cluttering the output itself,
so just report it once.

Signed-off-by: Stefan Beller 
---
 run-command.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/run-command.c b/run-command.c
index 7c00c21..3ae563f 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1012,13 +1012,21 @@ static void pp_cleanup(struct parallel_processes *pp)
 
 static void set_nonblocking(int fd)
 {
+   static int reported_degrade = 0;
int flags = fcntl(fd, F_GETFL);
-   if (flags < 0)
-   warning("Could not get file status flags, "
-   "output will be degraded");
-   else if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))
-   warning("Could not set file status flags, "
-   "output will be degraded");
+   if (flags < 0) {
+   if (!reported_degrade) {
+   warning("Could not get file status flags, "
+   "output will be degraded");
+   reported_degrade = 1;
+   }
+   } else if (fcntl(fd, F_SETFL, flags | O_NONBLOCK)) {
+   if (!reported_degrade) {
+   warning("Could not set file status flags, "
+   "output will be degraded");
+   reported_degrade = 1;
+   }
+   }
 }
 
 /* returns
-- 
2.6.1.247.ge8f2a41.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


[PATCHv3 11/11] clone: allow an explicit argument for parallel submodule clones

2015-11-03 Thread Stefan Beller
Just pass it along to "git submodule update", which may pick reasonable
defaults if you don't specify an explicit number.

Signed-off-by: Stefan Beller 
---
 Documentation/git-clone.txt |  6 +-
 builtin/clone.c | 19 +--
 t/t7406-submodule-update.sh | 15 +++
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index f1f2a3f..01bd6b7 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -14,7 +14,7 @@ SYNOPSIS
  [-o ] [-b ] [-u ] [--reference ]
  [--dissociate] [--separate-git-dir ]
  [--depth ] [--[no-]single-branch]
- [--recursive | --recurse-submodules] [--] 
+ [--recursive | --recurse-submodules] [--jobs ] [--] 
  []
 
 DESCRIPTION
@@ -216,6 +216,10 @@ objects from the source repository into a pack in the 
cloned repository.
The result is Git repository can be separated from working
tree.
 
+-j ::
+--jobs ::
+   The number of submodules fetched at the same time.
+   Defaults to the `submodule.jobs` option.
 
 ::
The (possibly remote) repository to clone from.  See the
diff --git a/builtin/clone.c b/builtin/clone.c
index 9eaecd9..ce578d2 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -50,6 +50,7 @@ static int option_progress = -1;
 static struct string_list option_config;
 static struct string_list option_reference;
 static int option_dissociate;
+static int max_jobs = -1;
 
 static struct option builtin_clone_options[] = {
OPT__VERBOSITY(&option_verbosity),
@@ -72,6 +73,8 @@ static struct option builtin_clone_options[] = {
N_("initialize submodules in the clone")),
OPT_BOOL(0, "recurse-submodules", &option_recursive,
N_("initialize submodules in the clone")),
+   OPT_INTEGER('j', "jobs", &max_jobs,
+   N_("number of submodules cloned in parallel")),
OPT_STRING(0, "template", &option_template, N_("template-directory"),
   N_("directory from which templates will be used")),
OPT_STRING_LIST(0, "reference", &option_reference, N_("repo"),
@@ -95,10 +98,6 @@ static struct option builtin_clone_options[] = {
OPT_END()
 };
 
-static const char *argv_submodule[] = {
-   "submodule", "update", "--init", "--recursive", NULL
-};
-
 static const char *get_repo_path_1(struct strbuf *path, int *is_bundle)
 {
static char *suffix[] = { "/.git", "", ".git/.git", ".git" };
@@ -724,8 +723,16 @@ static int checkout(void)
err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
   sha1_to_hex(sha1), "1", NULL);
 
-   if (!err && option_recursive)
-   err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
+   if (!err && option_recursive) {
+   struct argv_array args = ARGV_ARRAY_INIT;
+   argv_array_pushl(&args, "submodule", "update", "--init", 
"--recursive", NULL);
+
+   if (max_jobs != -1)
+   argv_array_pushf(&args, "--jobs=%d", max_jobs);
+
+   err = run_command_v_opt(args.argv, RUN_GIT_CMD);
+   argv_array_clear(&args);
+   }
 
return err;
 }
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 05ea66f..ade0524 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -786,4 +786,19 @@ test_expect_success 'submodule update can be run in 
parallel' '
 grep "9 children" trace.out
)
 '
+
+test_expect_success 'git clone passes the parallel jobs config on to 
submodules' '
+   test_when_finished "rm -rf super4" &&
+   GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 7 . 
super4 &&
+   grep "7 children" trace.out &&
+   rm -rf super4 &&
+   git config --global submodule.jobs 8 &&
+   GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules . super4 &&
+   grep "8 children" trace.out &&
+   rm -rf super4 &&
+   GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 9 . 
super4 &&
+   grep "9 children" trace.out &&
+   rm -rf super4
+'
+
 test_done
-- 
2.6.1.247.ge8f2a41.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


[PATCHv3 08/11] fetching submodules: respect `submodule.jobs` config option

2015-11-03 Thread Stefan Beller
This allows to configure fetching and updating in parallel
without having the command line option.

This moved the responsibility to determine how many parallel processes
to start from builtin/fetch to submodule.c as we need a way to communicate
"The user did not specify the number of parallel processes in the command
line options" in the builtin fetch. The submodule code takes care of
the precedence (CLI > config > default)

Signed-off-by: Stefan Beller 
---
 Documentation/config.txt|  7 +++
 builtin/fetch.c |  2 +-
 submodule-config.c  | 15 +++
 submodule-config.h  |  2 ++
 submodule.c |  5 +
 t/t5526-fetch-submodules.sh | 14 ++
 6 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 391a0c3..70e1b88 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2643,6 +2643,13 @@ submodule..ignore::
"--ignore-submodules" option. The 'git submodule' commands are not
affected by this setting.
 
+submodule.jobs::
+   This is used to determine how many submodules can be operated on in
+   parallel. Specifying a positive integer allows up to that number
+   of submodules being fetched in parallel. This is used in fetch
+   and clone operations only. A value of 0 will give some reasonable
+   configuration. It defaults to 1.
+
 tag.sort::
This variable controls the sort ordering of tags when displayed by
linkgit:git-tag[1]. Without the "--sort=" option provided, the
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 9cc1c9d..60e6797 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -37,7 +37,7 @@ static int prune = -1; /* unspecified */
 static int all, append, dry_run, force, keep, multiple, update_head_ok, 
verbosity;
 static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static int tags = TAGS_DEFAULT, unshallow, update_shallow;
-static int max_children = 1;
+static int max_children = -1;
 static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
diff --git a/submodule-config.c b/submodule-config.c
index 29e21b2..475551a 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -32,6 +32,7 @@ enum lookup_type {
 
 static struct submodule_cache cache;
 static int is_cache_init;
+static int parallel_jobs = -1;
 
 static int config_path_cmp(const struct submodule_entry *a,
   const struct submodule_entry *b,
@@ -239,6 +240,15 @@ static int parse_generic_submodule_config(const char *key,
  const char *value,
  struct parse_config_parameter *me)
 {
+   if (!strcmp(key, "jobs")) {
+   parallel_jobs = strtol(value, NULL, 10);
+   if (parallel_jobs < 0) {
+   warning("submodule.jobs not allowed to be negative.");
+   parallel_jobs = 1;
+   return 1;
+   }
+   }
+
return 0;
 }
 
@@ -482,3 +492,8 @@ void submodule_free(void)
cache_free(&cache);
is_cache_init = 0;
 }
+
+int config_parallel_submodules(void)
+{
+   return parallel_jobs;
+}
diff --git a/submodule-config.h b/submodule-config.h
index f9e2a29..d9bbf9a 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -27,4 +27,6 @@ const struct submodule *submodule_from_path(const unsigned 
char *commit_sha1,
const char *path);
 void submodule_free(void);
 
+int config_parallel_submodules(void);
+
 #endif /* SUBMODULE_CONFIG_H */
diff --git a/submodule.c b/submodule.c
index 0257ea3..188ba02 100644
--- a/submodule.c
+++ b/submodule.c
@@ -752,6 +752,11 @@ int fetch_populated_submodules(const struct argv_array 
*options,
argv_array_push(&spf.args, "--recurse-submodules-default");
/* default value, "--submodule-prefix" and its value are added later */
 
+   if (max_parallel_jobs < 0)
+   max_parallel_jobs = config_parallel_submodules();
+   if (max_parallel_jobs < 0)
+   max_parallel_jobs = 1;
+
calculate_changed_submodule_paths();
run_processes_parallel(max_parallel_jobs,
   get_next_submodule,
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 1b4ce69..5c3579c 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -470,4 +470,18 @@ test_expect_success "don't fetch submodule when newly 
recorded commits are alrea
test_i18ncmp expect.err actual.err
 '
 
+test_expect_success 'fetching submodules respects parallel settings' '
+   git config fetch.recurseSubmodules true &&
+   (
+   cd downstream &&
+   GIT_TRACE=$(pwd)/trace.out git fetch --jobs 7 &&
+   grep "7 children" trace.out &&
+   git config submodule.jobs 8 &&
+   GIT

[PATCHv3 05/11] submodule-config: drop check against NULL

2015-11-03 Thread Stefan Beller
Adhere to the common coding style of Git and not check explicitly
for NULL throughout the file. There are still other occurrences in the
code base but that is usually inside of conditions with side effects.

Signed-off-by: Stefan Beller 
---
 submodule-config.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/submodule-config.c b/submodule-config.c
index 4239b0e..6d01941 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -265,7 +265,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
if (!strcmp(item.buf, "path")) {
if (!value)
ret = config_error_nonbool(var);
-   else if (!me->overwrite && submodule->path != NULL)
+   else if (!me->overwrite && submodule->path)
warn_multiple_config(me->commit_sha1, submodule->name,
"path");
else {
@@ -289,7 +289,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
} else if (!strcmp(item.buf, "ignore")) {
if (!value)
ret = config_error_nonbool(var);
-   else if (!me->overwrite && submodule->ignore != NULL)
+   else if (!me->overwrite && submodule->ignore)
warn_multiple_config(me->commit_sha1, submodule->name,
"ignore");
else if (strcmp(value, "untracked") &&
@@ -305,7 +305,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
} else if (!strcmp(item.buf, "url")) {
if (!value) {
ret = config_error_nonbool(var);
-   } else if (!me->overwrite && submodule->url != NULL) {
+   } else if (!me->overwrite && submodule->url) {
warn_multiple_config(me->commit_sha1, submodule->name,
"url");
} else {
@@ -315,7 +315,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
} else if (!strcmp(item.buf, "update")) {
if (!value)
ret = config_error_nonbool(var);
-   else if (!me->overwrite && submodule->update != NULL)
+   else if (!me->overwrite && submodule->update)
warn_multiple_config(me->commit_sha1, submodule->name,
 "update");
else {
-- 
2.6.1.247.ge8f2a41.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


[PATCHv3 09/11] git submodule update: have a dedicated helper for cloning

2015-11-03 Thread Stefan Beller
This introduces a new helper function in git submodule--helper
which takes care of cloning all submodules, which we want to
parallelize eventually.

Some tests (such as empty URL, update_mode=none) are required in the
helper to make the decision for cloning. These checks have been
moved into the C function as well (no need to repeat them in the
shell script).

As we can only access the stderr channel from within the parallel
processing engine, we need to reroute the error message for
specified but initialized submodules to stderr. As it is an error
message, this should have gone to stderr in the first place, so it
is a bug fix along the way.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 builtin/submodule--helper.c | 229 
 git-submodule.sh|  45 +++--
 t/t7400-submodule-basic.sh  |   4 +-
 3 files changed, 242 insertions(+), 36 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f4c3eff..95b45a2 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -255,6 +255,234 @@ static int module_clone(int argc, const char **argv, 
const char *prefix)
return 0;
 }
 
+static int git_submodule_config(const char *var, const char *value, void *cb)
+{
+   return parse_submodule_config_option(var, value);
+}
+
+struct submodule_update_clone {
+   /* states */
+   int count;
+   int print_unmatched;
+   /* configuration */
+   int quiet;
+   const char *reference;
+   const char *depth;
+   const char *update;
+   const char *recursive_prefix;
+   const char *prefix;
+   struct module_list list;
+   struct string_list projectlines;
+   struct pathspec pathspec;
+};
+#define SUBMODULE_UPDATE_CLONE_INIT {0, 0, 0, NULL, NULL, NULL, NULL, NULL, 
MODULE_LIST_INIT, STRING_LIST_INIT_DUP}
+
+static void fill_clone_command(struct child_process *cp, int quiet,
+  const char *prefix, const char *path,
+  const char *name, const char *url,
+  const char *reference, const char *depth)
+{
+   cp->git_cmd = 1;
+   cp->no_stdin = 1;
+   cp->stdout_to_stderr = 1;
+   cp->err = -1;
+   argv_array_push(&cp->args, "submodule--helper");
+   argv_array_push(&cp->args, "clone");
+   if (quiet)
+   argv_array_push(&cp->args, "--quiet");
+
+   if (prefix)
+   argv_array_pushl(&cp->args, "--prefix", prefix, NULL);
+
+   argv_array_pushl(&cp->args, "--path", path, NULL);
+   argv_array_pushl(&cp->args, "--name", name, NULL);
+   argv_array_pushl(&cp->args, "--url", url, NULL);
+   if (reference)
+   argv_array_push(&cp->args, reference);
+   if (depth)
+   argv_array_push(&cp->args, depth);
+}
+
+static int update_clone_get_next_task(void **pp_task_cb,
+ struct child_process *cp,
+ struct strbuf *err,
+ void *pp_cb)
+{
+   struct submodule_update_clone *pp = pp_cb;
+
+   for (; pp->count < pp->list.nr; pp->count++) {
+   const struct submodule *sub = NULL;
+   const char *displaypath = NULL;
+   const struct cache_entry *ce = pp->list.entries[pp->count];
+   struct strbuf sb = STRBUF_INIT;
+   const char *update_module = NULL;
+   char *url = NULL;
+   int needs_cloning = 0;
+
+   if (ce_stage(ce)) {
+   if (pp->recursive_prefix)
+   strbuf_addf(err, "Skipping unmerged submodule 
%s/%s\n",
+   pp->recursive_prefix, ce->name);
+   else
+   strbuf_addf(err, "Skipping unmerged submodule 
%s\n",
+   ce->name);
+   continue;
+   }
+
+   sub = submodule_from_path(null_sha1, ce->name);
+   if (!sub) {
+   strbuf_addf(err, "BUG: internal error managing 
submodules. "
+   "The cache could not locate '%s'", 
ce->name);
+   pp->print_unmatched = 1;
+   continue;
+   }
+
+   if (pp->recursive_prefix)
+   displaypath = relative_path(pp->recursive_prefix, 
ce->name, &sb);
+   else
+   displaypath = ce->name;
+
+   if (pp->update)
+   update_module = pp->update;
+   if (!update_module)
+   update_module = sub->update;
+   if (!update_module)
+   update_module = "checkout";
+   if (!strcmp(update_module, "none")) {
+   strbuf_addf(err, "Skipping submodule '%s'\n", 
d

[PATCHv3 03/11] run-command: omit setting file descriptors to non blocking in Windows

2015-11-03 Thread Stefan Beller
In Windows there is no fcntl apparently and as it only affects output
slightly we can just run with degraded output in Windows.

Signed-off-by: Stefan Beller 
---
 run-command.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/run-command.c b/run-command.c
index 3ae563f..8db7df8 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1012,6 +1012,7 @@ static void pp_cleanup(struct parallel_processes *pp)
 
 static void set_nonblocking(int fd)
 {
+#ifndef GIT_WINDOWS_NATIVE
static int reported_degrade = 0;
int flags = fcntl(fd, F_GETFL);
if (flags < 0) {
@@ -1027,6 +1028,7 @@ static void set_nonblocking(int fd)
reported_degrade = 1;
}
}
+#endif
 }
 
 /* returns
-- 
2.6.1.247.ge8f2a41.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


[PATCHv3 01/11] run_processes_parallel: delimit intermixed task output

2015-11-03 Thread Stefan Beller
This commit serves 2 purposes. First this may help the user who
tries to diagnose intermixed process calls. Second this may be used
in a later patch for testing. As the output of a command should not
change visibly except for going faster, grepping for the trace output
seems like a viable testing strategy.

Signed-off-by: Stefan Beller 
---
 run-command.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/run-command.c b/run-command.c
index 0a3c24e..7c00c21 100644
--- a/run-command.c
+++ b/run-command.c
@@ -959,6 +959,9 @@ static struct parallel_processes *pp_init(int n,
n = online_cpus();
 
pp->max_processes = n;
+
+   trace_printf("run_processes_parallel: preparing to run up to %d tasks", 
n);
+
pp->data = data;
if (!get_next_task)
die("BUG: you need to specify a get_next_task function");
@@ -988,6 +991,7 @@ static void pp_cleanup(struct parallel_processes *pp)
 {
int i;
 
+   trace_printf("run_processes_parallel: done");
for (i = 0; i < pp->max_processes; i++) {
strbuf_release(&pp->children[i].err);
child_process_clear(&pp->children[i].process);
-- 
2.6.1.247.ge8f2a41.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


[PATCHv3 10/11] submodule update: expose parallelism to the user

2015-11-03 Thread Stefan Beller
Expose possible parallelism either via the "--jobs" CLI parameter or
the "submodule.jobs" setting.

By having the variable initialized to -1, we make sure 0 can be passed
into the parallel processing machine, which will then pick as many parallel
workers as there are CPUs.

Signed-off-by: Stefan Beller 
---
 Documentation/git-submodule.txt |  7 ++-
 builtin/submodule--helper.c | 18 ++
 git-submodule.sh|  9 +
 t/t7406-submodule-update.sh | 12 
 4 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index f17687e..c70fafd 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -16,7 +16,7 @@ SYNOPSIS
 'git submodule' [--quiet] deinit [-f|--force] [--] ...
 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
  [-f|--force] [--rebase|--merge] [--reference ]
- [--depth ] [--recursive] [--] [...]
+ [--depth ] [--recursive] [--jobs ] [--] [...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) ]
  [commit] [--] [...]
 'git submodule' [--quiet] foreach [--recursive] 
@@ -374,6 +374,11 @@ for linkgit:git-clone[1]'s `--reference` and `--shared` 
options carefully.
clone with a history truncated to the specified number of revisions.
See linkgit:git-clone[1]
 
+-j ::
+--jobs ::
+   This option is only valid for the update command.
+   Clone new submodules in parallel with as many jobs.
+   Defaults to the `submodule.jobs` option.
 
 ...::
Paths to submodule(s). When specified this will restrict the command
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 95b45a2..662d329 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -426,6 +426,7 @@ static int update_clone_task_finished(int result,
 
 static int update_clone(int argc, const char **argv, const char *prefix)
 {
+   int max_jobs = -1;
struct string_list_item *item;
struct submodule_update_clone pp = SUBMODULE_UPDATE_CLONE_INIT;
 
@@ -446,6 +447,8 @@ static int update_clone(int argc, const char **argv, const 
char *prefix)
OPT_STRING(0, "depth", &pp.depth, "",
   N_("Create a shallow clone truncated to the "
  "specified number of revisions")),
+   OPT_INTEGER('j', "jobs", &max_jobs,
+   N_("parallel jobs")),
OPT__QUIET(&pp.quiet, N_("do't print cloning progress")),
OPT_END()
};
@@ -467,10 +470,17 @@ static int update_clone(int argc, const char **argv, 
const char *prefix)
gitmodules_config();
/* Overlay the parsed .gitmodules file with .git/config */
git_config(git_submodule_config, NULL);
-   run_processes_parallel(1, update_clone_get_next_task,
- update_clone_start_failure,
- update_clone_task_finished,
- &pp);
+
+   if (max_jobs < 0)
+   max_jobs = config_parallel_submodules();
+   if (max_jobs < 0)
+   max_jobs = 1;
+
+   run_processes_parallel(max_jobs,
+  update_clone_get_next_task,
+  update_clone_start_failure,
+  update_clone_task_finished,
+  &pp);
 
if (pp.print_unmatched) {
printf("#unmatched\n");
diff --git a/git-submodule.sh b/git-submodule.sh
index 9f554fb..10c5af9 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -645,6 +645,14 @@ cmd_update()
--depth=*)
depth=$1
;;
+   -j|--jobs)
+   case "$2" in '') usage ;; esac
+   jobs="--jobs=$2"
+   shift
+   ;;
+   --jobs=*)
+   jobs=$1
+   ;;
--)
shift
break
@@ -670,6 +678,7 @@ cmd_update()
${update:+--update "$update"} \
${reference:+--reference "$reference"} \
${depth:+--depth "$depth"} \
+   ${jobs:+$jobs} \
"$@" | {
err=
while read mode sha1 stage just_cloned sm_path
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index dda3929..05ea66f 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -774,4 +774,16 @@ test_expect_success 'submodule update --recursive drops 
module name before recur
 test_i18ngrep "Submodule path .deeper/submodule/subsubmodule.: checked 
out" actual
)
 '
+
+test_expect_success 'submodule update can be run in parallel' '
+   (cd super2 &&
+GIT_TRACE=$(

[PATCHv3 00/11] Expose the submodule parallelism to the user

2015-11-03 Thread Stefan Beller
Where does it apply?
---
This series applies on top of d075d2604c0f92045caa8d5bb6ab86cf4921a4ae (Merge
branch 'rs/daemon-plug-child-leak' into sb/submodule-parallel-update) and 
replaces
the previous patches in sb/submodule-parallel-update

What does it do?
---
This series should finish the on going efforts of parallelizing
submodule network traffic. The patches contain tests for clone,
fetch and submodule update to use the actual parallelism both via
command line as well as a configured option. I decided to go with
"submodule.jobs" for all three for now.

What's new in v3?
---

 * 3 new patches (make it compile in Windows, better warnings in posix 
environment
   for setting fds to non blocking, drop check against NULL)
 * adressed reviews by Eric for readability. :) 
 * addressed Junios comments for the new clone helper function

Stefan Beller (11):
  run_processes_parallel: delimit intermixed task output
  run-command: report failure for degraded output just once
  run-command: omit setting file descriptors to non blocking in Windows
  submodule-config: keep update strategy around
  submodule-config: drop check against NULL
  submodule-config: remove name_and_item_from_var
  submodule-config: introduce parse_generic_submodule_config
  fetching submodules: respect `submodule.jobs` config option
  git submodule update: have a dedicated helper for cloning
  submodule update: expose parallelism to the user
  clone: allow an explicit argument for parallel submodule clones

 Documentation/config.txt|   7 ++
 Documentation/git-clone.txt |   6 +-
 Documentation/git-submodule.txt |   7 +-
 builtin/clone.c |  19 +++-
 builtin/fetch.c |   2 +-
 builtin/submodule--helper.c | 239 
 git-submodule.sh|  54 -
 run-command.c   |  26 -
 submodule-config.c  | 109 +++---
 submodule-config.h  |   3 +
 submodule.c |   5 +
 t/t5526-fetch-submodules.sh |  14 +++
 t/t7400-submodule-basic.sh  |   4 +-
 t/t7406-submodule-update.sh |  27 +
 14 files changed, 433 insertions(+), 89 deletions(-)

-- 
2.6.1.247.ge8f2a41.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: git.git as of tonight

2015-11-03 Thread Stefan Beller
On Tue, Nov 3, 2015 at 1:03 PM, Johannes Sixt  wrote:
> Am 03.11.2015 um 19:18 schrieb Stefan Beller:
>>
>> ... ReadFileEx ... "overlapped" operation.
>
>
> Let's not go there just yet.
>
>>>   1. Make this an optional feature so that platforms can compile it
>>>  out, if it is not already done.  My preference, even if we go
>>>  that route, would be to see if we can find a way to preserve the
>>>  overall code structure (e.g. instead of spawning multiple
>>>  workers, which is why the code needs NONBLOCK to avoid getting
>>>  stuck on reading from one while others are working, perhaps we
>>>  can spawn only one and not do a nonblock read?).
>>
>>
>> Yeah that would be my understanding as well. If we don't come up with
>> a good solution for parallelism in Windows now, we'd need to make it at
>> least working in the jobs=1 case as well as it worked before.
>
>
> That should be possible. I discovered today that we have this function:
>
> static void set_nonblocking(int fd)
> {
> int flags = fcntl(fd, F_GETFL);
> if (flags < 0)
> warning("Could not get file status flags, "
> "output will be degraded");
> else if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))
> warning("Could not set file status flags, "
> "output will be degraded");
> }
>
> Notice that it is not a fatal condition if O_NONBLOCK cannot be established.
> (BTW, did you ever test this condition?)

No, as I viewed it more like a severe problem, not to be happen in the
near future.
But if it were to happen we would still need to finish the command
instead of giving
up because of degraded output. (I would not know how to test this
system call to fail,
so maybe I am just making up excuses)

I added an #ifdef just as you proposed below and the output itself
doesn't look too bad
except for the warning message themselves. If we'd just remove them it
would look
better to me.

So maybe we could just go with

static void set_nonblocking(int fd)
{
#ifndef GIT_WINDOWS_NATIVE
int flags = fcntl(fd, F_GETFL);
if (!(flags < 0))
fcntl(fd, F_SETFL, flags | O_NONBLOCK)
#endif
}

and see how people react to the output then?


> If we add two lines (which remove
> the stuff that does not work on Windows) like this:
>
> static void set_nonblocking(int fd)
> {
> #ifndef GIT_WINDOWS_NATIVE
> int flags = fcntl(fd, F_GETFL);
> if (flags < 0)
> warning("Could not get file status flags, "
> "output will be degraded");
> else if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))
> #endif
> warning("Could not set file status flags, "
> "output will be degraded");
> }
>
> we should get something that works, theoretically. We still need a more
> complete waitpid emulation, but that does not look like rocket science. I'll
> investigate further in this direction.
>
> -- Hannes
>
--
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


Bug: stash save -u removes (some) ignored files

2015-11-03 Thread Felipe Sateler
I have seen the following problem:

felipe@felipe:testgit% cat .gitignore
**/notrack/*
!**/notrack/track/
notrackfile**

felipe@felipe:testgit% find *
newfile
notrack
notrack/1
notrackfile1

felipe@felipe:testgit% git status --porcelain
?? newfile

felipe@felipe:testgit% git stash save -u
Saved working directory and index state WIP on master: 523cb39 Initial commit
HEAD is now at 523cb39 Initial commit

felipe@felipe:testgit% find *
notrackfile1

So, after a stash save, git decided to keep the untracked file in the
current directory, but not the ones inside the untracked directory.
However, the files were "correctly" ignored and did not appear on the stash:

felipe@felipe:testgit% git stash pop
Already up-to-date!
On branch master
Untracked files:
  (use "git add ..." to include in what will be committed)

newfile

nothing added to commit but untracked files present (use "git add" to track)
Dropped refs/stash@{0} (e6508f345af1dd207557ad0291e7e3bce8a89b1e)

felipe@felipe:testgit% find *
newfile
notrackfile1

I think the correct behaviour should be to left the ignored files
untouched in both scenarios.

This is with git 2.6.1 (from debian).

I note that if I add a file inside the notrack/track directory, it is
correctly kept, but the files in notrack/ are still deleted.

-- 

Saludos,
Felipe Sateler
--
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: [BUG] Wrong worktree path when using multiple worktree

2015-11-03 Thread Mike Rappazzo
On Tue, Nov 3, 2015 at 11:58 AM, Nicolas Morey-Chaisemartin
 wrote:
> Hi,
>
> There seem to be an issue with the path computed for a worktree when multiple 
> worktree were created (using git worktree)
> In my Setup, I have 3 repos:
> A/repo (main One)
> A/repo-patches (worktree, using branch dev)
> B/repo (worktree, using branch next)
>
> I'm working in A/repo-patches an run:
> $ git checkout next
> fatal: 'next' is already checked out at 'A/repo-patches'
>
> Which is partially true but not completely.
> I looked a bit in the code and found that the issue comes from here 
> (get_linked_worktree):
> if (!strbuf_strip_suffix(&worktree_path, "/.git")) {
> strbuf_reset(&worktree_path);
> strbuf_addstr(&worktree_path, absolute_path("."));
> strbuf_strip_suffix(&worktree_path, "/.");
> }
> Because it wrongfully assumes that I am in the linked worktree.
> I checked in the .git/worktree files and couldn't see anything that actually 
> points to where the repo are setup.
>
> Nicolas

This is code that I worked on, but I am unable to reproduce it locally
just yet.  Before I dig too deep, could you report the contents of
"A/repo/.git/worktrees/repo-patches/gitdir" (or similar)?  There is
another issue reported[1] where the contents of the gitdir for a
linked worktree are overwritten in some cases.  A fix for this is
being worked on (see discussion).

In the mean time, I will continue to try and reproduce.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/280307
--
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 0/4] diff-highlight: make a few improvements

2015-11-03 Thread Jeff King
On Mon, Nov 02, 2015 at 09:05:30PM -0500, Jonathan Lebon wrote:

> These patches bring a few improvements to the contrib/diff-highlight
> Perl script. The major improvement is done in patch 3/4, which improves
> diff-highlighting accuracy by implementing a recursive line matching
> algorithm.
> 
> Please note that I have limited experience with Perl, so there may be
> better ways to do things. (Let me know if that is the case!)

I forgot to mention in the rest of my review: thank you for working on
this, and for making a nice clean series. It was very pleasant to read.

I think the output overall is nicer. I am concerned with the performance
implications. I see this as a stopgap until we have a true diff inside
the hunks, but if we can overcome the performance implications, I think
it's worth applying in the meantime.

-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: [PATCH 3/4] diff-highlight: match up lines before highlighting

2015-11-03 Thread Jeff King
On Tue, Nov 03, 2015 at 04:44:16PM -0500, Jeff King wrote:

> Have you looked at a diff of the old/new output on something like
> git.git?

This should be pretty easy to do (and time). I tried:

  git log --oneline --color -p >base
  time perl highlight.old old
  time perl highlight.new new
  diff -c old new | less

where the "highlight.*" scripts are the versions at master, and master
with your series applied.

Your is _much_ slower. I get:

  real0m25.538s
  user0m25.420s
  sys 0m0.120s

for the old versus:

  real2m3.580s
  user2m3.548s
  sys 0m0.156s

for your series. In an interactive setting, the latency may not be that
noticeable, but if you are digging far into history (e.g., "git log -p",
then using "/" in less to search for a commit or some test), I suspect
it would be very noticeable.

I was thinking there was some low-hanging fruit in memoizing the
calculations, but even the prefix/suffix computation is pairwise. I'm
not really sure how to make this much faster.

As for the output itself, the diff between the two looks promising. The
first several cases I looked at ar strict improvements. Some of them are
kind of weird, especially in English text. For example, see the RelNotes
update in 2635c2b. The base diff is:

* "git rebase -i" had a minor regression recently, which stopped
considering a line that begins with an indented '#' in its insn
-   sheet not a comment, which is now fixed.
-   (merge 1db168e gr/rebase-i-drop-warn later to maint).
+   sheet not a comment. Further, the code was still too picky on
+   Windows where CRLF left by the editor is turned into a trailing CR
+   on the line read via the "read" built-in command of bash.  Both of
+   these issues are now fixed.
+   (merge 39743cf gr/rebase-i-drop-warn later to maint).

Before we highlighted nothing, and now we hone in on "now fixed". Which
is _sort of_ a match, but really the whole sentence structure has
changed. If this is the worst regression, I can certainly live with it.
And even a proper LCS diff would probably end up making spaghetti of a
text change like this.

In the other thread I mentioned earlier, the solution I cooked up was
dropping highlighting entirely for hunks over a certain percentage of
highlighting. I wonder if we could do something similar here (e.g.,
don't match lines where more than 50% of the line would be highlighted).

-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: [PATCH 1/4] diff-highlight: add `less -r` to cmd in README

2015-11-03 Thread Jeff King
On Tue, Nov 03, 2015 at 01:51:54PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > I agree with Junio that "-R" is a more sensible default, but I don't
> > think that should be necessary. We already set LESS when running the
> > pager (and if you are overriding it, you are already in trouble, because
> > git itself will generate ANSI codes by default).
> 
> I do not quite understand this part.  Inside git itself when we
> spawn the pager we export LESS with a sensible default, exactly to
> help users who do not have LESS set and exported.  This one however
> is not spawned by git but the end-user piping output of diff-hilite.

Oh, you're right. I mistook it as instructions for what to put in
pager.log, but that is clearly not what this is. I agree that telling
the user to use "-R" here is a good idea.

And indeed, the instructions just below the hunk in question describe
setting it properly in your config (with less, and no "-R"). So I was
simply confused and not reading carefully enough.

-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: [PATCH 1/4] diff-highlight: add `less -r` to cmd in README

2015-11-03 Thread Junio C Hamano
Jeff King  writes:

> I agree with Junio that "-R" is a more sensible default, but I don't
> think that should be necessary. We already set LESS when running the
> pager (and if you are overriding it, you are already in trouble, because
> git itself will generate ANSI codes by default).

I do not quite understand this part.  Inside git itself when we
spawn the pager we export LESS with a sensible default, exactly to
help users who do not have LESS set and exported.  This one however
is not spawned by git but the end-user piping output of diff-hilite.

I agree that if the user has LESS exported without -R that would not
be pretty while viewing coloured output.

--
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 4/4] diff-highlight: add maxhunksize config option

2015-11-03 Thread Jeff King
On Mon, Nov 02, 2015 at 09:05:34PM -0500, Jonathan Lebon wrote:

> As the size of the hunk gets bigger, it becomes harder to jump back and
> forth between the removed and added lines, and highlighting becomes less
> beneficial. We add a new config option called
> 
>   diff-highlight.maxhunksize
> 
> which controls the maximum size of the hunk allowed for which
> highlighting is still performed. The default value is set to 20.

I think this makes sense. I'm pleased it is here to limit the problem
space for patch 3, but I think it has value on its own as a heuristic.

-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: [PATCH 3/4] diff-highlight: match up lines before highlighting

2015-11-03 Thread Jeff King
On Mon, Nov 02, 2015 at 09:05:33PM -0500, Jonathan Lebon wrote:

> As mentioned in the README, one of the current limitations of
> diff-highlight is that it only calculates highlights when the hunk
> contains the same number of removed lines as added lines.
> 
> A further limitation upon this is that diff-highlight assumes that the
> first line removed matches the first line added, similarly with the
> second, the third, etc... As was demonstrated in the "Bugs" section of
> the README, this poses limitations since that assumption does not always
> give the best result.
> 
> With this patch, we eliminate those limitations by trying to match up
> the removed and added lines before highlighting them. This is done using
> a recursive algorithm.

Hmm. So this seems like a reasonable incremental feature. I do think it
is a hack (piled on the hack that is the existing script :) ), and the
right solution is to actually do an LCS diff for each hunk that crosses
line boundaries.

I made some headway on that this summer as part of this discussion:

  http://thread.gmane.org/gmane.comp.version-control.git/271692

It's long, but there's some good stuff in there.  But I think I came to
the conclusion that this really needs to go inside of diff.c itself
(which will also require some heavy refactoring of the whitespace code;
see the referenced thread for details).

But I'm not opposed to an incremental feature like this in the meantime.
The real test for me is: how does it look in practice? These are all
heuristics, so I don't think we have anything better than eyeballing the
output.

Have you looked at a diff of the old/new output on something like
git.git?

> Note that I did not bother with some common optimizations such as
> memoization since the usual number of removed/added lines in a single
> hunk are small. In practice, I have not felt any lag at all during
> paging.

I'd worry less about normal use, and more about hitting some extreme
corner case. Your algorithm looks roughly quadratic in the size of the
hunk. I guess that is canceled out by the max-hunk-size option in the
next patch, though.

I don't think it's easy to make your algorithm non-quadratic, either, as
it inherently relies on pairwise comparisons (and not, say, generating a
fingerprint of each line and sorting them or something like that).

It might be worth memo-izing find_common_* calls, though, as that is
just repeated work (quadratic or not). It should be easy to time.

> + # prime the loop
> + my ($besti, $bestj) = ($a_first, $b_first);
> + my $bestn = calculate_match($a->[$a_first], $b->[$b_first]) + 1;
> +
> + for (my $i = $a_first; $i < $a_last; $i++) {
> + for (my $j = $b_first; $j < $b_last; $j++) {
> + my $n = calculate_match($a->[$i], $b->[$j]);
> + if ($n < $bestn) {
> + ($besti, $bestj, $bestn) = ($i, $j, $n);
> + }
> + }
> + }
> +
> + # find the best matches in the lower pairs
> + match_and_highlight_pairs($a, $a_first, $besti, $b, $b_first, $bestj, 
> $queue);

Hmm, is this actually O(n^3)? We have a quadratic loop, and then we
recurse for the remainder.

If we have two candidates for which calculate_match returns the same
value, how do we break the tie? It looks like we'll just pick the
lowest-numbered match. I'd think we would want to prefer the one with
the closest line number.  But not having thought too hard about it, I
wonder:

  1. Does it actually make a difference which line we pick? The
 interesting bit is how much we highlight, so in that sense do we
 care only about the prefix and suffix sizes?

  2. Do you end up picking the closest line with your algorithm anyway,
 as will tend to match as we go, skipping over only lines that will
 likely have no match?

-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: git log --author=me

2015-11-03 Thread Junio C Hamano
Michael J Gruber  writes:

> Alternatively: How about teaching git-completion to complete the
> argument to --author? Expensive, I know, but faster than typing it out
> or realising "Michael J" is not as unique as you think ;)

Or

$ git log --author="$me"

with

me='Michael J Gruber '

in your $HOME/.profile or somewhere?

I personally would find "--author=me" not so bad (especially if
finding _my_ work were a very important workflow element), and I may
even tell people to do

$ git log --author='[m]e'

if they want to find anybody with substring 'me' in their ident.

But that is only if it weren't for existing users, and we live in an
imperfect world where we seem to have many of existing users
already.

--
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/4] diff-highlight: factor out prefix/suffix functions

2015-11-03 Thread Jeff King
On Mon, Nov 02, 2015 at 09:05:32PM -0500, Jonathan Lebon wrote:

> In preparation for the next patch, we factor out the functions for
> finding the common prefix and suffix between two lines.

Looks like a pretty straightforward movement. Your perl adjustments to
use array refs look good, and I think the multi-valued return is a good
interface.

-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: [PATCH 1/4] diff-highlight: add `less -r` to cmd in README

2015-11-03 Thread Jeff King
On Mon, Nov 02, 2015 at 09:05:31PM -0500, Jonathan Lebon wrote:

> As it is, the suggested command for trying out diff-highlight will just
> dump the whole git log output to the terminal. Let's pipe it through
> `less` so users aren't surprised on the first try.

That makes sense. I use "diff-highlight | less" myself, of course. I
think I excluded it from the README in the name of simplicity, but you
are right that it is probably better to give a complete working example.

I agree with Junio that "-R" is a more sensible default, but I don't
think that should be necessary. We already set LESS when running the
pager (and if you are overriding it, you are already in trouble, because
git itself will generate ANSI codes by default).

-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: [PATCH v2 1/4] Document the semantics of hideRefs with namespaces

2015-11-03 Thread Junio C Hamano
Lukas Fleischer  writes:

> ++
> +If a namespace is set, references are stripped before matching. For example, 
> if

The first sentence is probably not understood by many readers, as
you do not define what is "to strip references".  The namespace
prefix is stripped from the reference before a reference is matched
against the hideRefs setting.

> +the prefix `refs/heads/master` is specified in `transfer.hideRefs` and the
> +current namespace is `foo`, then `refs/namespaces/foo/refs/heads/master` is
> +omitted from the advertisements but `refs/heads/master` and
> +`refs/namespaces/bar/refs/heads/master` are still advertised as so-called
> +"have" lines.

--
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] Add support for matching full refs in hideRefs

2015-11-03 Thread Junio C Hamano
Lukas Fleischer  writes:

>  static void show_ref(const char *path, const unsigned char *sha1)
>  {
> - if (ref_is_hidden(path))
> - return;
> -
>   if (sent_capabilities) {
>   packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
>   } else {
> @@ -219,9 +216,14 @@ static void show_ref(const char *path, const unsigned 
> char *sha1)
>   }
>  }
>  
> -static int show_ref_cb(const char *path, const struct object_id *oid, int 
> flag, void *unused)
> +static int show_ref_cb(const char *path_full, const struct object_id *oid,
> +int flag, void *unused)
>  {
> - path = strip_namespace(path);
> + const char *path = strip_namespace(path_full);
> +
> + if (ref_is_hidden(path, path_full))
> + return 1;
> +

These two hunks are doing a bit more than the primary topic of this
step, aren't they?

The semantics of "hideRefs" used to be that "refs can be hidden from
ls-remote" (i.e. prevents fetching, avoids contaminating "--mirror")
and "refs can be hidden from 'push'", but the objects being known
tips of histories that are complete, they still participate in
common ancestor discovery.  

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 3da97a1..91ed6a5 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2690,7 +2690,8 @@ the prefix `refs/heads/master` is specified in 
> `transfer.hideRefs` and the
>  current namespace is `foo`, then `refs/namespaces/foo/refs/heads/master` is
>  omitted from the advertisements but `refs/heads/master` and
>  `refs/namespaces/bar/refs/heads/master` are still advertised as so-called
> -"have" lines.
> +"have" lines. In order to match refs before stripping, add a `^` in front of
> +the ref name. If you combine `!` and `^`, `!` must be specified first.

I think the changes in the above two hunks prevent the hidden refs
from participating in common ancestor discovery (which is probably a
good thing), making the above description stale.

--
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.git as of tonight

2015-11-03 Thread Johannes Sixt

Am 03.11.2015 um 19:18 schrieb Stefan Beller:

... ReadFileEx ... "overlapped" operation.


Let's not go there just yet.


  1. Make this an optional feature so that platforms can compile it
 out, if it is not already done.  My preference, even if we go
 that route, would be to see if we can find a way to preserve the
 overall code structure (e.g. instead of spawning multiple
 workers, which is why the code needs NONBLOCK to avoid getting
 stuck on reading from one while others are working, perhaps we
 can spawn only one and not do a nonblock read?).


Yeah that would be my understanding as well. If we don't come up with
a good solution for parallelism in Windows now, we'd need to make it at
least working in the jobs=1 case as well as it worked before.


That should be possible. I discovered today that we have this function:

static void set_nonblocking(int fd)
{
int flags = fcntl(fd, F_GETFL);
if (flags < 0)
warning("Could not get file status flags, "
"output will be degraded");
else if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))
warning("Could not set file status flags, "
"output will be degraded");
}

Notice that it is not a fatal condition if O_NONBLOCK cannot be 
established. (BTW, did you ever test this condition?) If we add two 
lines (which remove the stuff that does not work on Windows) like this:


static void set_nonblocking(int fd)
{
#ifndef GIT_WINDOWS_NATIVE
int flags = fcntl(fd, F_GETFL);
if (flags < 0)
warning("Could not get file status flags, "
"output will be degraded");
else if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))
#endif
warning("Could not set file status flags, "
"output will be degraded");
}

we should get something that works, theoretically. We still need a more 
complete waitpid emulation, but that does not look like rocket science. 
I'll investigate further in this direction.


-- Hannes

--
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] Update japanese translation

2015-11-03 Thread Junio C Hamano
miur...@linux.com writes:

> From: Hiroshi Miura 
>
> - Update untranslated terms
>
> Signed-off-by: Hiroshi Miura 
> ---
>  gitk-git/po/ja.po | 97 
> ---
>  1 file changed, 42 insertions(+), 55 deletions(-)

Sorry but I do not take patches directly to gitk or git-gui, both of
which are independent projects of their own.

Could you redo this change against Paul Mackerras's tree, which is
our upstream "gitk" project, that is found here:

git://git.kernel.org/pub/scm/gitk/gitk.git

and Cc Paul Mackerras   when you post your patch?

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: Watchman/inotify support and other ways to speed up git status

2015-11-03 Thread David Turner
On Tue, 2015-11-03 at 08:09 +0100, Christian Couder wrote:
> On Tue, Nov 3, 2015 at 6:45 AM, Duy Nguyen  wrote:
> > On Mon, Nov 2, 2015 at 9:56 PM, David Turner  
> > 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 be clear: I think Duy's approach is probably best in the long term.

--
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] setup: do not create $X/gitdir unnecessarily when accessing git file $X

2015-11-03 Thread Junio C Hamano
Duy Nguyen  writes:

> The whole prune strategy is a bit messy trying to cover all cases
> while still keeping out of the user's way. Perhaps if we implement
> "git worktree mv", or even "worktree fixup" so the user can do it
> manually (back when the prune strategy commit was implemented, there
> was no git-worktree), then we don't need this magic any more.
>
> So, which way to go?

I'd prefer to see "conservative and minimal first and carefully
build up" instead of "snapping something together quickly and having
to patch it up in rapid succession before people get hurt." and that
is not limited to the "worktree" topic.

I think "if you move, you are on your own, do not do it." would be a
good starting point.  The user education material would say: if you
create worktree B out of the primary A, and if you do not like the
location B, you "rm -fr B" and then create a new C out of the
primary A at a desired place, and do not reuse location B for any
other purpose.  The list of worktrees kept somewhere in A would then
name the old location B, and it is OK to notice the staleness and
remove it, but we do not have to.  At that point, the magic can and
should go.

After setting the user expectation that way, it is fine to design
how we would give "worktree rm" and "worktree mv".  As long as
users' initial expectation is set to "you do not mv, ever", these
can be introduced without hurting their established use pattern that
would involve no 'mv'.
--
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/5] read-cache: add watchman 'WAMA' extension

2015-11-03 Thread David Turner
On Tue, 2015-11-03 at 20:17 +0100, Duy Nguyen wrote:
> On Mon, Nov 2, 2015 at 11:03 PM, David Turner  
> wrote:
> > On Sun, 2015-11-01 at 14:55 +0100, Nguyễn Thái Ngọc Duy wrote:
> >>
> >>+#define CE_NO_WATCH  (0x0001)
> >
> > This name seems very confusing to me.  CE_NO_WATCHMAN_STAT?
> > CE_UNKNOWN_TO_WATCHMAN?
> 
> Files that are known updated. Maybe CE_WATCHMAN_DIRTY?

+1

> > (one reason it may seem more confusing to me than to others is that
> > Twitter's code has a concept of files that we don't watch at all e.g.
> > Intellij's .idea dir).
> >
> >> @@ -322,6 +325,7 @@ struct index_state {
> >>   struct untracked_cache *untracked;
> >>   void *mmap;
> >>   size_t mmap_size;
> >> + char *last_update;
> >
> > Might be worth a comment explaining what this is.
> 
> It's the clock value from watchman when we query file status. Will
> make a note. Or maybe I should just rename it to watchman_clock.

Either way would work for me.

--
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] http: allow selection of proxy authentication method

2015-11-03 Thread Junio C Hamano
Knut Franke  writes:

> On 2015-11-02 14:46, Junio C Hamano wrote:
>> > Reviewed-by: Junio C Hamano 
>> > Reviewed-by: Eric Sunshine 
>> 
>> Please add these only when you are doing the final submission,
>> sending the same version reviewed by these people after they said
>> the patch(es) look good.  To credit others for helping you to polish
>> your patch, Helped-by: would be more appropriate.
>
> Sorry about that.
>
> However, may I suggest that Documentation/SubmittingPatches could do with a
> little rewording in this respect?
>
>> Do not forget to add trailers such as "Acked-by:", "Reviewed-by:" and
>> "Tested-by:" lines as necessary to credit people who helped your
>> patch.
>
> "Helped-by:" isn't even mentioned.

That is because it is not actively encouraged.

The only thing I care about is that people do not incorrectly use
Reviewed/Acked-by when reviewers did not say "this version looks
good"; that would mislead the maintainer to think "ah, if that
reviewer said this is good, whose judgment I can trust, then I do
not have to read it carefully myself."
--
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: [PATCHv2 0/8] Expose the submodule parallelism to the user

2015-11-03 Thread Stefan Beller
On Thu, Oct 29, 2015 at 4:50 PM, Ramsay Jones
 wrote:
>
>
> On 29/10/15 15:51, Stefan Beller wrote:
>> On Thu, Oct 29, 2015 at 6:19 AM, Ramsay Jones
>>  wrote:
>>
>>> Hmm, is there a way to _not_ fetch in parallel (override the
>>> config) from the command line for a given command?
>>>
>>> ATB,
>>> Ramsay Jones
>>
>> git config submodule.jobs 42
>> git  --jobs 1 # should run just one task, despite having 42 configured
>
> Heh, yes ... I didn't pose the question quite right ...
>>
>> It does use the parallel processing machinery though, but with a maximum of
>> one subcommand being spawned. Is that what you're asking?
>
> ... but, despite that, you correctly inferred what I was really
> asking about! :)
>
> I was just wondering what overhead the parallel processing machinery
> adds to the original 'non-parallel' code path (for the j=1 case).
> I suspect the answer is 'not much', but that's just a guess.
> Have you measured it?

Totally unscientific:
 * Make a copy of my current gerrit repository and time the fetch.
 * That repo contains 5 submodules, one needs fetching

time git fetch --recurse-submodules=yes --jobs=1 # this series
real 0m7.150s
user 0m3.459s
sys 0m1.126s

time git fetch --recurse-submodules=yes # origin/master
real 0m7.667s
user 0m3.439s
sys 0m1.190s

Now let's test a few more times repeatedly to avoid cold caches or
network hiccups, (also there is nothing to fetch, so it's more like doing
6 ls-remotes in a row, one for gerrit and 5 submodules)

this series, best out of 5:
real 0m3.971s
user 0m2.447s
sys 0m0.452s

this series, worst out of 5:
real 0m4.229s
user 0m2.506s
sys 0m0.413s

origin/master, best out of 5:
real 0m3.968s
user 0m2.516s
sys 0m0.380s

origin/master, worst out of 5:
real 0m4.217s
user 0m2.472s
sys 0m0.408s

The ratio of real time taken longer is < 1 % in
both the best and worst case.

If you really care about 1 % of performance, you'd want to fetch in
parallel anyway?


> What happens if there is only a single
> submodule to fetch?

Ok let's see. I created https://github.com/stefanbeller/test-sub-1
to play around with it. However
time git fetch --recurse-submodules=yes
or
time git fetch --recurse-submodules=yes --jobs 100
seems to be lost in the noise.

So I am not sure what the question is w.r.t. having just one
submodule.


>
> ATB,
> Ramsay Jones
>
>
--
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/5] read-cache: add watchman 'WAMA' extension

2015-11-03 Thread Duy Nguyen
On Mon, Nov 2, 2015 at 11:03 PM, David Turner  wrote:
> On Sun, 2015-11-01 at 14:55 +0100, Nguyễn Thái Ngọc Duy wrote:
>>
>>+#define CE_NO_WATCH  (0x0001)
>
> This name seems very confusing to me.  CE_NO_WATCHMAN_STAT?
> CE_UNKNOWN_TO_WATCHMAN?

Files that are known updated. Maybe CE_WATCHMAN_DIRTY?

> (one reason it may seem more confusing to me than to others is that
> Twitter's code has a concept of files that we don't watch at all e.g.
> Intellij's .idea dir).
>
>> @@ -322,6 +325,7 @@ struct index_state {
>>   struct untracked_cache *untracked;
>>   void *mmap;
>>   size_t mmap_size;
>> + char *last_update;
>
> Might be worth a comment explaining what this is.

It's the clock value from watchman when we query file status. Will
make a note. Or maybe I should just rename it to watchman_clock.
-- 
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 1/1] gitk: add --cwd=path commandline parameter to change path

2015-11-03 Thread Eric Sunshine
On Tue, Nov 3, 2015 at 10:00 AM, Juha-Pekka Heikkila
 wrote:
> This patch adds --cwd (change working directory) parameter to
> gitk. With this parameter, instead of need to cd to directory
> with .git folder, one can point the correct folder from
> commandline.

git itself supports this sort of functionality via -C, as does GNU
tar, which suggests such an option in gitk should be named -C, as
well.

> Signed-off-by: Juha-Pekka Heikkila 
> ---
> diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
> @@ -146,6 +146,11 @@ gitk-specific options
> Select the specified commit after loading the graph.
> Default behavior is equivalent to specifying '--select-commit=HEAD'.
>
> +--cwd=::
> +
> +   Change working direcoty to . If the git tree exist elsewhere

s/direcoty/directory/

> +   gitk first cd to given path before start to operate.

Taking git's -C documentation as a template, perhaps this could be
written instead as:

Run as if gitk was started in '' instead of the current
working directory. When multiple `-C` options are given, each
subsequent non-absolute `-C ` is interpreted relative to
the preceding `-C `.

This description correctly reflects your implementation which allows
--cwd to be specified multiple times.

>  Examples
>  
>  gitk v2.6.12.. include/scsi drivers/scsi::
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index fcc606e..5fdf459 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -12279,12 +12279,6 @@ setui $uicolor
>
>  setoptions
>
> -# check that we can find a .git directory somewhere...
> -if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
> -show_error {} . [mc "Cannot find a git repository here."]
> -exit 1
> -}
> -
>  set selecthead {}
>  set selectheadid {}
>
> @@ -12305,6 +12299,9 @@ foreach arg $argv {
> "--argscmd=*" {
> set revtreeargscmd [string range $arg 10 end]
> }
> +   "--cwd=*" {
> +   cd [string range $arg 6 end]
> +   }
> default {
> lappend revtreeargs $arg
> }
> @@ -12312,6 +12309,12 @@ foreach arg $argv {
>  incr i
>  }
>
> +# check that we can find a .git directory somewhere...
> +if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
> +show_error {} . [mc "Cannot find a git repository here."]
> +exit 1
> +}
> +
>  if {$selecthead eq "HEAD"} {
>  set selecthead {}
>  }
> --
> 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


You

2015-11-03 Thread Barbara Wolf


Sent from my iPhone
--
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.git as of tonight

2015-11-03 Thread Stefan Beller
On Tue, Nov 3, 2015 at 9:05 AM, Junio C Hamano  wrote:
> Johannes Sixt  writes:
>
>> My findings so far are negative. The only short-term and mid-term
>> solution I see so far is to opt-out from the framework during
>> build-time.

So I started reading up on that[1].
As far as I understand, we don't need to mark a file descriptor
to be non blocking, but rather we could use ReadFileEx[2] with
a flag set for "overlapped" operation.

So that said, we can make set_nonblocking a noop and
provide another implementation for strbuf_read_once
depending on NO_PTHREADS being set.
Maybe not even strbuf_read_once, but rather the underlying
xread_nonblock ?



[1] http://tinyclouds.org/iocp-links.html
[2] https://msdn.microsoft.com/en-us/library/aa365468(v=VS.85).aspx

>
> Now, from where I sit, it seems that the way forward would be
>
>  1. Make this an optional feature so that platforms can compile it
> out, if it is not already done.  My preference, even if we go
> that route, would be to see if we can find a way to preserve the
> overall code structure (e.g. instead of spawning multiple
> workers, which is why the code needs NONBLOCK to avoid getting
> stuck on reading from one while others are working, perhaps we
> can spawn only one and not do a nonblock read?).

Yeah that would be my understanding as well. If we don't come up with
a good solution for parallelism in Windows now, we'd need to make it at
least working in the jobs=1 case as well as it worked before.

>
>  2. After that is done, the feature could graduate to 'master'.  As
> this is a bigger framework change than others, however, we do
> not necessarily want to rush it.  On the other hand, because
> this only affects submodules, which means it has fewer users and
> testers that would give us feedback while it is on 'next', we
> may want to push it to 'master' sooner to give it a larger
> exposure.  I dunno, and I do not want to decide this myself the
> week before I'll go offline for a few weeks (i.e. today).

Yeah I guess cooking this well done has its benefits.

>
>  3. Then we would enlist help from folks who are more familiar with
> Windows platform (like you) to see how the "run parallel workers
> and collect from them" can be (re)done with a nice level of
> abstraction.  I am hoping that we can continue the tradition of
> the evolution of run-command.c API (I am specifically impressed
> by what you did for "async" that allows the callers not to worry
> about threads and processes) aroundt this area.  That is
> obviously a mid- to longer term goal.

I just wonder if we can skip step 1) and 2) by having the discussion
now how to change the framework to work well without posix file
descriptors here.

>
> Thanks for working together well, you two.
--
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] http: use credential API to handle proxy authentication

2015-11-03 Thread Eric Sunshine
On Tue, Nov 3, 2015 at 4:31 AM, Knut Franke
 wrote:
> On 2015-11-02 14:54, Junio C Hamano wrote:
>> >  static void init_curl_proxy_auth(CURL *result)
>> >  {
>> > +   if (proxy_auth.username) {
>> > +   if (!proxy_auth.password)
>> > +   credential_fill(&proxy_auth);
>> > +#if LIBCURL_VERSION_NUM >= 0x071301
>> > +   curl_easy_setopt(result, CURLOPT_PROXYUSERNAME,
>> > +   proxy_auth.username);
>> > +   curl_easy_setopt(result, CURLOPT_PROXYPASSWORD,
>> > +   proxy_auth.password);
>> > +#else
>> > +   struct strbuf s = STRBUF_INIT;
>> > +   strbuf_addstr_urlencode(&s, proxy_auth.username, 1);
>> > +   strbuf_addch(&s, ':');
>> > +   strbuf_addstr_urlencode(&s, proxy_auth.password, 1);
>> > +   curl_proxyuserpwd = strbuf_detach(&s, NULL);
>> > +   curl_easy_setopt(result, CURLOPT_PROXYUSERPWD, 
>> > curl_proxyuserpwd);
>> > +#endif
>>
>> I think #else clause of this thing would introduce decl-after-stmt
>> compilation error.
>
> I've actually tested this with CURL 7.15.5 (0x070f05), and didn't get any
> compilation error.

Junio meant that, on this project, code still follows the rule that
all declarations in a block must come before statements in order to
appease older compilers. That is (good):

{
int foo; /* declaration */

if (doodad) /* statement */
snorg();
glop(&foo);
}

rather than (bad):

{
if (doodad) /* statement */
snorg();

int foo; /* declaration after statement */
glop(&foo);
}

The gcc option -Wdeclaration-after-statement can help.
--
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


[BUG] Wrong worktree path when using multiple worktree

2015-11-03 Thread Nicolas Morey-Chaisemartin
Hi,

There seem to be an issue with the path computed for a worktree when multiple 
worktree were created (using git worktree)
In my Setup, I have 3 repos:
A/repo (main One)
A/repo-patches (worktree, using branch dev)
B/repo (worktree, using branch next)

I'm working in A/repo-patches an run:
$ git checkout next
fatal: 'next' is already checked out at 'A/repo-patches'

Which is partially true but not completely.
I looked a bit in the code and found that the issue comes from here 
(get_linked_worktree):
if (!strbuf_strip_suffix(&worktree_path, "/.git")) {
strbuf_reset(&worktree_path);
strbuf_addstr(&worktree_path, absolute_path("."));
strbuf_strip_suffix(&worktree_path, "/.");
}
Because it wrongfully assumes that I am in the linked worktree.
I checked in the .git/worktree files and couldn't see anything that actually 
points to where the repo are setup.

Nicolas
--
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] Add git-grep threads param

2015-11-03 Thread Junio C Hamano
Victor Leschuk  writes:

> Make number of git-grep worker threads a configuration parameter.
> According to several tests on systems with different number of CPU cores
> the hard-coded number of 8 threads is not optimal for all systems:
> tuning this parameter can significantly speed up grep performance.
>
> Signed-off-by: Victor Leschuk 
> ---
>  Documentation/config.txt   |  4 +++
>  Documentation/git-grep.txt |  9 ++
>  builtin/grep.c | 56 
> --
>  contrib/completion/git-completion.bash |  1 +
>  4 files changed, 54 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 391a0c3..1dd2a61 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1447,6 +1447,10 @@ grep.extendedRegexp::
>   option is ignored when the 'grep.patternType' option is set to a value
>   other than 'default'.
>  
> +grep.threads::
> + Number of grep worker threads, use it to tune up performance on
> + multicore machines. Default value is 8. Set to 0 to disable threading.
> +

I am not enthused by this "Set to 0 to disable".  As Zero is
magical, it would be more useful if 1 meant that threading is not
used (i.e. there is only 1 worker), and 0 meant that we would
automatically pick some reasonable parallelism for you (and we
promise that the our choice would not be outrageously wrong), or
something like that.

Of course, we can do grep.threads=auto to make it even more
explicit, but I'd imagine that the in-core code to parse the option
and config to the num_threads variable would need one "int" value to
represent that "auto", and 0 would be a natural choice for that.

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: git.git as of tonight

2015-11-03 Thread Junio C Hamano
Johannes Sixt  writes:

> My findings so far are negative. The only short-term and mid-term
> solution I see so far is to opt-out from the framework during
> build-time.

Now, from where I sit, it seems that the way forward would be

 1. Make this an optional feature so that platforms can compile it
out, if it is not already done.  My preference, even if we go
that route, would be to see if we can find a way to preserve the
overall code structure (e.g. instead of spawning multiple
workers, which is why the code needs NONBLOCK to avoid getting
stuck on reading from one while others are working, perhaps we
can spawn only one and not do a nonblock read?).

 2. After that is done, the feature could graduate to 'master'.  As
this is a bigger framework change than others, however, we do
not necessarily want to rush it.  On the other hand, because
this only affects submodules, which means it has fewer users and
testers that would give us feedback while it is on 'next', we
may want to push it to 'master' sooner to give it a larger
exposure.  I dunno, and I do not want to decide this myself the
week before I'll go offline for a few weeks (i.e. today).

 3. Then we would enlist help from folks who are more familiar with
Windows platform (like you) to see how the "run parallel workers
and collect from them" can be (re)done with a nice level of
abstraction.  I am hoping that we can continue the tradition of
the evolution of run-command.c API (I am specifically impressed
by what you did for "async" that allows the callers not to worry
about threads and processes) aroundt this area.  That is
obviously a mid- to longer term goal.

Thanks for working together well, you two.
--
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 gc/prune runs again and again

2015-11-03 Thread Andreas Krey
On Tue, 03 Nov 2015 16:01:24 +, Duy Nguyen wrote:

> > I have a bit of an annoying behaviour in git gc;
> > there is a repo I regularly do a fetch in, and
> > this kicks off a gc/prune every time. I remember
> > there being a heuristic with being that many files
> > in .git/objects/17 as the gc trigger.

(It might be a good idea to use a random slot here.)

...
> > What can I do there? (This git is a bit old, 2.2.2)
> 
> Run "git prune". 2.2.2 hides this suggestion to run git-prune in this
> case. The next git version will show it back.

Actually, there is a 'git prune --expire 2.weeks.ago --no-progress'
running under the 'git gc --auto --quiet', and I don't want to drop
the expiry time lightly.

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds 
Date: Fri, 22 Jan 2010 07:29:21 -0800
--
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/1] gitk: add --cwd=path commandline parameter to change path

2015-11-03 Thread Juha-Pekka Heikkila
This patch adds --cwd (change working directory) parameter to
gitk. With this parameter, instead of need to cd to directory
with .git folder, one can point the correct folder from
commandline.

Signed-off-by: Juha-Pekka Heikkila 
---
 Documentation/gitk.txt |  5 +
 gitk-git/gitk  | 15 +--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index 6ade002..1f42198 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -146,6 +146,11 @@ gitk-specific options
Select the specified commit after loading the graph.
Default behavior is equivalent to specifying '--select-commit=HEAD'.
 
+--cwd=::
+
+   Change working direcoty to . If the git tree exist elsewhere
+   gitk first cd to given path before start to operate.
+
 Examples
 
 gitk v2.6.12.. include/scsi drivers/scsi::
diff --git a/gitk-git/gitk b/gitk-git/gitk
index fcc606e..5fdf459 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -12279,12 +12279,6 @@ setui $uicolor
 
 setoptions
 
-# check that we can find a .git directory somewhere...
-if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
-show_error {} . [mc "Cannot find a git repository here."]
-exit 1
-}
-
 set selecthead {}
 set selectheadid {}
 
@@ -12305,6 +12299,9 @@ foreach arg $argv {
"--argscmd=*" {
set revtreeargscmd [string range $arg 10 end]
}
+   "--cwd=*" {
+   cd [string range $arg 6 end]
+   }
default {
lappend revtreeargs $arg
}
@@ -12312,6 +12309,12 @@ foreach arg $argv {
 incr i
 }
 
+# check that we can find a .git directory somewhere...
+if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
+show_error {} . [mc "Cannot find a git repository here."]
+exit 1
+}
+
 if {$selecthead eq "HEAD"} {
 set selecthead {}
 }
-- 
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


[PATCH 0/1] gitk: add --cwd=path commandline parameter to change path

2015-11-03 Thread Juha-Pekka Heikkila
I found I needed this option thus made it for myself, maybe others find it 
useful too. I'm not skilled with tcl so if there is something totally wrong 
with my change this is why.

/Juha-Pekka

Juha-Pekka Heikkila (1):
  gitk: add --cwd=path commandline parameter to change path

 Documentation/gitk.txt |  5 +
 gitk-git/gitk  | 15 +--
 2 files changed, 14 insertions(+), 6 deletions(-)

-- 
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: git gc/prune runs again and again

2015-11-03 Thread Duy Nguyen
On Tue, Nov 3, 2015 at 1:44 PM, Andreas Krey  wrote:
> Hi all,
>
> I have a bit of an annoying behaviour in git gc;
> there is a repo I regularly do a fetch in, and
> this kicks off a gc/prune every time. I remember
> there being a heuristic with being that many files
> in .git/objects/17 as the gc trigger.
>
> Which is unfortunate if the gc does not actually
> reduce the number of files there because they
> aren't old enough => gc comes right back.
>
> What can I do there? (This git is a bit old, 2.2.2)

Run "git prune". 2.2.2 hides this suggestion to run git-prune in this
case. The next git version will show it back.


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


git gc/prune runs again and again

2015-11-03 Thread Andreas Krey
Hi all,

I have a bit of an annoying behaviour in git gc;
there is a repo I regularly do a fetch in, and
this kicks off a gc/prune every time. I remember
there being a heuristic with being that many files
in .git/objects/17 as the gc trigger.

Which is unfortunate if the gc does not actually
reduce the number of files there because they
aren't old enough => gc comes right back.

What can I do there? (This git is a bit old, 2.2.2)

Andreas


-- 
"Totally trivial. Famous last words."
From: Linus Torvalds 
Date: Fri, 22 Jan 2010 07:29:21 -0800
--
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: Automatic download at git-scm.com does not work (Was: [git-users] Cant download Git)

2015-11-03 Thread MH

> On Nov 3, 2015, at 8:14 AM, Konstantin Khomoutov  wrote:
> 
> On Tue, 3 Nov 2015 04:55:22 -0800 (PST)
> M Hendrickson  wrote:
> 
>> I am trying to download Git. When I click download for Mac OS X it
>> says "download starting" but the download doesn't start.
>> 
>> There is a manual download option from sourceforge. But some have
>> warned me that downloading from sourceforge often contains unwanted
>> things included in the download.
> 
> That was a blunder of the (recent) past so I'd say it's OK to download
> directly from SF.  Note that the Mac OS X build accessible from
> git-scm.com, while being "officially blessed" is not prepared by the
> upstream Git developers (as they only distribute the source code) but
> rather this is a port coordinated there at that SF project you referred
> to.
> 
> Well, I've just verified that what would be automacitally downloaded
> has the same URL as that "download manually" link.
> So you should be fine with the manual download.
> 
>> Does anyone know why the download from git-scm.com is not working?

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


Automatic download at git-scm.com does not work (Was: [git-users] Cant download Git)

2015-11-03 Thread Konstantin Khomoutov
On Tue, 3 Nov 2015 04:55:22 -0800 (PST)
M Hendrickson  wrote:

> I am trying to download Git. When I click download for Mac OS X it
> says "download starting" but the download doesn't start.
> 
> There is a manual download option from sourceforge. But some have
> warned me that downloading from sourceforge often contains unwanted
> things included in the download.

That was a blunder of the (recent) past so I'd say it's OK to download
directly from SF.  Note that the Mac OS X build accessible from
git-scm.com, while being "officially blessed" is not prepared by the
upstream Git developers (as they only distribute the source code) but
rather this is a port coordinated there at that SF project you referred
to.

Well, I've just verified that what would be automacitally downloaded
has the same URL as that "download manually" link.
So you should be fine with the manual download.

> Does anyone know why the download from git-scm.com is not working?
--
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] Update japanese translation

2015-11-03 Thread miurahr
From: Hiroshi Miura 

- Update untranslated terms

Signed-off-by: Hiroshi Miura 
---
 gitk-git/po/ja.po | 97 ---
 1 file changed, 42 insertions(+), 55 deletions(-)

diff --git a/gitk-git/po/ja.po b/gitk-git/po/ja.po
index 59e42a8..cb55909 100644
--- a/gitk-git/po/ja.po
+++ b/gitk-git/po/ja.po
@@ -9,14 +9,15 @@ msgstr ""
 "Project-Id-Version: gitk\n"
 "Report-Msgid-Bugs-To: \n"
 "POT-Creation-Date: 2015-05-17 14:32+1000\n"
-"PO-Revision-Date: 2009-11-06 01:45+0900\n"
-"Last-Translator: Mizar \n"
+"PO-Revision-Date: 2015-11-03 16:38+0900\n"
+"Last-Translator: Hiroshi Miura \n"
 "Language-Team: Japanese\n"
-"Language: \n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
 "Plural-Forms: nplurals=1; plural=0;\n"
+"X-Generator: Poedit 1.5.4\n"
+"Language: ja\n"
 
 #: gitk:140
 msgid "Couldn't get list of unmerged files:"
@@ -24,11 +25,11 @@ msgstr "マージされていないファイルのリストを取得できませ
 
 #: gitk:212 gitk:2381
 msgid "Color words"
-msgstr ""
+msgstr "単語に色を付ける"
 
 #: gitk:217 gitk:2381 gitk:8220 gitk:8253
 msgid "Markup words"
-msgstr ""
+msgstr "単語をマークする"
 
 #: gitk:324
 msgid "Error parsing revisions:"
@@ -308,19 +309,16 @@ msgid "Compare with marked commit"
 msgstr "マークを付けたコミットと比較する"
 
 #: gitk:2629 gitk:2640
-#, fuzzy
 msgid "Diff this -> marked commit"
-msgstr "これと選択したコミットのdiffを見る"
+msgstr "これ->選択したコミットのdiff"
 
 #: gitk:2630 gitk:2641
-#, fuzzy
 msgid "Diff marked commit -> this"
-msgstr "選択したコミットとこれのdiffを見る"
+msgstr "選択したコミット->これのdiff"
 
 #: gitk:2631
-#, fuzzy
 msgid "Revert this commit"
-msgstr "このコミットにマークをつける"
+msgstr "このコミットをリバート"
 
 #: gitk:2647
 msgid "Check out this branch"
@@ -332,7 +330,7 @@ msgstr "このブランチを除去する"
 
 #: gitk:2649
 msgid "Copy branch name"
-msgstr ""
+msgstr "ブランチ名をコピー"
 
 #: gitk:2656
 msgid "Highlight this too"
@@ -352,7 +350,7 @@ msgstr "親コミットから blame をかける"
 
 #: gitk:2660
 msgid "Copy path"
-msgstr ""
+msgstr "パスをコピー"
 
 #: gitk:2667
 msgid "Show origin of this line"
@@ -363,7 +361,6 @@ msgid "Run git gui blame on this line"
 msgstr "この行に git gui で blame をかける"
 
 #: gitk:3014
-#, fuzzy
 msgid ""
 "\n"
 "Gitk - a commit viewer for git\n"
@@ -375,7 +372,7 @@ msgstr ""
 "\n"
 "Gitk - gitコミットビューア\n"
 "\n"
-"Copyright \\u00a9 2005-2010 Paul Mackerras\n"
+"Copyright © 2005-2014 Paul Mackerras\n"
 "\n"
 "使用および再配布は GNU General Public License に従ってください"
 
@@ -397,9 +394,9 @@ msgid "<%s-Q>\t\tQuit"
 msgstr "<%s-Q>\t\t終了"
 
 #: gitk:3049
-#, fuzzy, tcl-format
+#, tcl-format
 msgid "<%s-W>\t\tClose window"
-msgstr "<%s-F>\t\t検索"
+msgstr "<%s-W>\t\tウインドウを閉じる"
 
 #: gitk:3050
 msgid "\t\tMove to first commit"
@@ -410,19 +407,16 @@ msgid "\t\tMove to last commit"
 msgstr "\t\t最後のコミットに移動"
 
 #: gitk:3052
-#, fuzzy
 msgid ", p, k\tMove up one commit"
-msgstr ", p, i\t一つ上のコミットに移動"
+msgstr ", p, k\t一つ上のコミットに移動"
 
 #: gitk:3053
-#, fuzzy
 msgid ", n, j\tMove down one commit"
-msgstr ", n, k\t一つ下のコミットに移動"
+msgstr ", n, j\t一つ下のコミットに移動"
 
 #: gitk:3054
-#, fuzzy
 msgid ", z, h\tGo back in history list"
-msgstr ", z, j\t履歴の前に戻る"
+msgstr ", z, h\t履歴の前に戻る"
 
 #: gitk:3055
 msgid ", x, l\tGo forward in history list"
@@ -431,7 +425,7 @@ msgstr ", x, l\t履歴の次へ進む"
 #: gitk:3056
 #, tcl-format
 msgid "<%s-n>\tGo to n-th parent of current commit in history list"
-msgstr ""
+msgstr "<%s-n>\tヒストリの現在からn番目の親コミットへ移動する"
 
 #: gitk:3057
 msgid "\tMove up one page in commit list"
@@ -514,9 +508,8 @@ msgid "\tMove to next find hit"
 msgstr "\t次を検索して移動"
 
 #: gitk:3075
-#, fuzzy
 msgid "g\t\tGo to commit"
-msgstr "\t\t最後のコミットに移動"
+msgstr "g\t\tコミットに移動"
 
 #: gitk:3076
 msgid "/\t\tFocus the search box"
@@ -672,9 +665,8 @@ msgid "Matches all Commit Info criteria"
 msgstr "コミット情報の全ての条件に一致"
 
 #: gitk:4086
-#, fuzzy
 msgid "Matches no Commit Info criteria"
-msgstr "コミット情報の全ての条件に一致"
+msgstr "コミット情報に一致しません"
 
 #: gitk:4087
 msgid "Changes to Files:"
@@ -761,9 +753,8 @@ msgid "-- criteria for selecting revisions"
 msgstr "― リビジョンの選択条件"
 
 #: gitk:4241
-#, fuzzy
 msgid "View Name"
-msgstr "ビュー名:"
+msgstr "ビュー名"
 
 #: gitk:4316
 msgid "Apply (F5)"
@@ -984,12 +975,11 @@ msgstr "タグ名:"
 
 #: gitk:9268
 msgid "Tag message is optional"
-msgstr ""
+msgstr "タグメッセージは任意です"
 
 #: gitk:9270
-#, fuzzy
 msgid "Tag message:"
-msgstr "タグ名:"
+msgstr "タグメッセージ:"
 
 #: gitk:9274 gitk:9439
 msgid "Create"
@@ -1066,33 +1056,32 @@ msgid "No changes committed"
 msgstr "何の変更もコミットされていません"
 
 #: gitk:9593
-#, fuzzy, tcl-format
+#, tcl-format
 msgid "Commit %s is not included in branch %s -- really revert it?"
 msgstr ""
-"コミット %s は既にブランチ %s に含まれています ― 本当にこれを再適用しますか?"
+"コミット %s は既にブランチ %s に含まれていません ― 本当にこれをリバートします"
+"か?"
 
 #: gitk:9598
-#, fuzzy
 msgid "Reverting"
-msgstr "リセット中"
+msgstr "リバート中"
 
 #: gitk:9606
-#, fuzzy, tcl-format
+#, tcl-format
 msgid ""
 "Revert failed because of local changes to the following files:%s Please "
 "commit, reset or stash  your changes and try again."
 msgstr ""
-"ファイル '%s' のローカルな変更のためにチェリーピックは失敗しました。\n"
+"ファイル '%s' のローカルな変更のためにリ

Re: When a file was locked by some program, git will work stupidly

2015-11-03 Thread Mr Guillaume Seren
Quoting Mr Guillaume Seren (2015-11-02 15:41:22)
> Quoting dayong xie (2015-11-02 05:56:55)
> > To be specific
> > In my Unity project, there is a native plugin,  and plugin's extension
> > is .dll, and this plugin is
> > under git version control, when Unity is running, the plugin file will
> > be locked.
> > If i merge another branch, which contains modification of the plugin,
> > git will report error, looks
> > like:
> > error: unable to unlink old 'xxx/xxx.dll' (Permission denied)
> > This is not bad, however, the unfinished merge action will not revert
> > by git, a lot of changes
> > produced in repository.
> > usually it makes me crazy, even worse, some of my partners are not
> > good at using git.
> > Of course, this problem can be avoided by quit Unity, but not every
> > time we can remember. In
> > my opinion, git should revert the unfinished action when the error
> > occurred, not just stop.
> > 
> > -- 
> > --
> > Best Regards,
> > John Xie
> > --
> > --
> > 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
> 
> 
> 
> | Mr Guillaume Seren
> |
> | website: http://www.guillaumeseren.com/
> | github: https://github.com/GuillaumeSeren
> 



| Mr Guillaume Seren
|
| website: http://www.guillaumeseren.com/
| github: https://github.com/GuillaumeSeren

--
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] Limit the size of the data block passed to SHA1_Update()

2015-11-03 Thread Torsten Bögershausen

On 11/03/2015 07:58 AM, atous...@gmail.com wrote:

From: Atousa Pahlevan Duprat 

Minor comments inline

diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
index b864df6..d085412 100644
--- a/block-sha1/sha1.h
+++ b/block-sha1/sha1.h
@@ -18,5 +18,5 @@ void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX 
*ctx);
  
  #define git_SHA_CTX	blk_SHA_CTX

  #define git_SHA1_Init blk_SHA1_Init
-#define git_SHA1_Updateblk_SHA1_Update
+#define platform_SHA1_Update   blk_SHA1_Update
  #define git_SHA1_Finalblk_SHA1_Final
diff --git a/cache.h b/cache.h
index 79066e5..a501652 100644
--- a/cache.h
+++ b/cache.h
@@ -10,12 +10,21 @@
  #include "trace.h"
  #include "string-list.h"
  
+// platform's underlying implementation of SHA1

Please use /* */ for comments

  #include SHA1_HEADER
  #ifndef git_SHA_CTX
-#define git_SHA_CTXSHA_CTX
-#define git_SHA1_Init  SHA1_Init
-#define git_SHA1_UpdateSHA1_Update
-#define git_SHA1_Final SHA1_Final
+#define git_SHA_CTXSHA_CTX
+#define git_SHA1_Init  SHA1_Init
+#define platform_SHA1_Update   SHA1_Update
+#define git_SHA1_Final SHA1_Final
+#endif
+
+// choose whether chunked implementation or not
+#ifdef SHA1_MAX_BLOCK_SIZE
+int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len);
+#define git_SHA1_Update   git_SHA1_Update_Chunked
+#else
+#define git_SHA1_Update   platform_SHA1_Update
  #endif
  
  #include 

diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
index c8b9b0e..d3fb264 100644
--- a/compat/apple-common-crypto.h
+++ b/compat/apple-common-crypto.h
@@ -16,6 +16,10 @@
  #undef TYPE_BOOL
  #endif
  
+#ifndef SHA1_MAX_BLOCK_SIZE

+#error Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE
+#endif
+
  #ifdef APPLE_LION_OR_NEWER
  #define git_CC_error_check(pattern, err) \
do { \
diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c
new file mode 100644
index 000..61f67de
--- /dev/null
+++ b/compat/sha1_chunked.c
@@ -0,0 +1,19 @@
+#include "cache.h"
+
+int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len)
+{
+   size_t nr;
+   size_t total = 0;
+   const char *cdata = (const char*)data;
+
+   while (len > 0) {

size_t is unsigned, isn't it ?
Better to use  "while (len) {"

+   nr = len;
+   if (nr > SHA1_MAX_BLOCK_SIZE)
+   nr = SHA1_MAX_BLOCK_SIZE;
+   platform_SHA1_Update(c, cdata, nr);
+   total += nr;
+   cdata += nr;
+   len -= nr;
+   }
+   return total;
+}


--
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 log --author=me

2015-11-03 Thread Michael J Gruber
Duy Nguyen venit, vidit, dixit 02.11.2015 19:37:
> On Mon, Nov 2, 2015 at 2:27 PM, Harry Jeffery  wrote:
>> Hi,
>>
>> I've written a patch that allows `me` to be used as shorthand for
>> $(user.name) or $(user.email) in the `--author` and `--commiter` fields.
>>
>> The purpose being to make finding your own commits quicker and easier:
>> git log --author=me
>>
> 
> It would be even cooler if it accepts mail aliases, then you can
> define "me" to your address and also have shortcuts to a few of your
> best friends. Though as Andreas pointed out --author is not a good fit
> because it accepts regex, and you can't use --mine either, so more
> work..
> 

Alternatively: How about teaching git-completion to complete the
argument to --author? Expensive, I know, but faster than typing it out
or realising "Michael J" is not as unique as you think ;)

Michael
--
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 0/5] Use watchman to reduce index refresh time

2015-11-03 Thread Paolo Ciarrocchi
On Tue, Nov 3, 2015 at 10:21 AM, Duy Nguyen  wrote:
>> 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.
>
> 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"
>
> 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..


Impressive improvements!

Ciao,
Paolo

-- 
Paolo
--
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] http: use credential API to handle proxy authentication

2015-11-03 Thread Knut Franke
On 2015-11-02 14:54, Junio C Hamano wrote:
> >  static void init_curl_proxy_auth(CURL *result)
> >  {
> > +   if (proxy_auth.username) {
> > +   if (!proxy_auth.password)
> > +   credential_fill(&proxy_auth);
> > +#if LIBCURL_VERSION_NUM >= 0x071301
> > +   curl_easy_setopt(result, CURLOPT_PROXYUSERNAME,
> > +   proxy_auth.username);
> > +   curl_easy_setopt(result, CURLOPT_PROXYPASSWORD,
> > +   proxy_auth.password);
> > +#else
> > +   struct strbuf s = STRBUF_INIT;
> > +   strbuf_addstr_urlencode(&s, proxy_auth.username, 1);
> > +   strbuf_addch(&s, ':');
> > +   strbuf_addstr_urlencode(&s, proxy_auth.password, 1);
> > +   curl_proxyuserpwd = strbuf_detach(&s, NULL);
> > +   curl_easy_setopt(result, CURLOPT_PROXYUSERPWD, 
> > curl_proxyuserpwd);
> > +#endif
> 
> I think #else clause of this thing would introduce decl-after-stmt
> compilation error.

I've actually tested this with CURL 7.15.5 (0x070f05), and didn't get any
compilation error.


Cheers,
Knut
-- 
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Dr. Arno Steitz
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196

--
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 0/5] Use watchman to reduce index refresh time

2015-11-03 Thread Duy Nguyen
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.

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"

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..
-- 
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 1/2] http: allow selection of proxy authentication method

2015-11-03 Thread Knut Franke
On 2015-11-02 14:46, Junio C Hamano wrote:
> > Reviewed-by: Junio C Hamano 
> > Reviewed-by: Eric Sunshine 
> 
> Please add these only when you are doing the final submission,
> sending the same version reviewed by these people after they said
> the patch(es) look good.  To credit others for helping you to polish
> your patch, Helped-by: would be more appropriate.

Sorry about that.

However, may I suggest that Documentation/SubmittingPatches could do with a
little rewording in this respect?

> Do not forget to add trailers such as "Acked-by:", "Reviewed-by:" and
> "Tested-by:" lines as necessary to credit people who helped your
> patch.

"Helped-by:" isn't even mentioned.

> > +static void init_curl_proxy_auth(CURL *result)
> > +{
> > +   env_override(&http_proxy_authmethod, "GIT_HTTP_PROXY_AUTHMETHOD");
> 
> Shouldn't this also be part of the #if/#endif?

The idea here was to have as little code as possible within the #if/#endif, as a
matter of principle. It may be a little construed in this case, but supposing
there's some subtle bug with env_override, or a future change introduces one,
having it occur only for certain CURL versions would tend to make it harder to
track down.

> and this code would be:
> 
>   if (remote)
>   var_override(&http_proxy_authmethod, 
> remote->http_proxy_authmethod);

Good catch.


Cheers,
Knut
-- 
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Dr. Arno Steitz
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196

--
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] Add git-grep threads param

2015-11-03 Thread Victor Leschuk
>> do we have any objections on this patch?

> The question you should be asking is "do we have any support".

Hello all, CCing participated reviewers. As Junio  has correctly mentioned: "Do 
we have any support for including this functionality?"

I think this kind of customization can be useful in tuning up performance as 
confirmed by conducted tests. And in most cases having anything hard-coded is 
worse than giving users opportunity to change it, isn't it?
--
Best Regards,
Victor--
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 4/4] t5509: add basic tests for hideRefs

2015-11-03 Thread Lukas Fleischer
Test whether regular and full hideRefs patterns work as expected when
namespaces are used.

Helped-by: Eric Sunshine 
Signed-off-by: Lukas Fleischer 
---
 t/t5509-fetch-push-namespaces.sh | 41 
 1 file changed, 41 insertions(+)

diff --git a/t/t5509-fetch-push-namespaces.sh b/t/t5509-fetch-push-namespaces.sh
index cc0b31f..bc44ac3 100755
--- a/t/t5509-fetch-push-namespaces.sh
+++ b/t/t5509-fetch-push-namespaces.sh
@@ -82,4 +82,45 @@ test_expect_success 'mirroring a repository using a ref 
namespace' '
)
 '
 
+test_expect_success 'hide namespaced refs with transfer.hideRefs' '
+   GIT_NAMESPACE=namespace \
+   git -C pushee -c transfer.hideRefs=refs/tags \
+   ls-remote "ext::git %s ." >actual &&
+   printf "$commit1\trefs/heads/master\n" >expected &&
+   test_cmp expected actual
+'
+
+test_expect_success 'check that transfer.hideRefs does not match unstripped 
refs' '
+   GIT_NAMESPACE=namespace \
+   git -C pushee -c 
transfer.hideRefs=refs/namespaces/namespace/refs/tags \
+   ls-remote "ext::git %s ." >actual &&
+   printf "$commit1\trefs/heads/master\n" >expected &&
+   printf "$commit0\trefs/tags/0\n" >>expected &&
+   printf "$commit1\trefs/tags/1\n" >>expected &&
+   test_cmp expected actual
+'
+
+test_expect_success 'hide full refs with transfer.hideRefs' '
+   GIT_NAMESPACE=namespace \
+   git -C pushee -c 
transfer.hideRefs="^refs/namespaces/namespace/refs/tags" \
+   ls-remote "ext::git %s ." >actual &&
+   printf "$commit1\trefs/heads/master\n" >expected &&
+   test_cmp expected actual
+'
+
+test_expect_success 'try to update a hidden ref' '
+   test_config -C pushee transfer.hideRefs refs/heads/master &&
+   test_must_fail git -C original push pushee-namespaced master
+'
+
+test_expect_success 'try to update a ref that is not hidden' '
+   test_config -C pushee transfer.hideRefs 
refs/namespaces/namespace/refs/heads/master &&
+   git -C original push pushee-namespaced master
+'
+
+test_expect_success 'try to update a hidden full ref' '
+   test_config -C pushee transfer.hideRefs 
"^refs/namespaces/namespace/refs/heads/master" &&
+   test_must_fail git -C original push pushee-namespaced master
+'
+
 test_done
-- 
2.6.2

--
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 0/4] Improve hideRefs when used with namespaces

2015-11-03 Thread Lukas Fleischer
This is a first set of patches improving documentation and behavior of
the transfer.hideRefs feature as discussed in [1]. In particular,
hideRefs is changed to generally match stripped refs by default and
match full refs when prefixed with "^". The documentation is updated
accordingly. Basic tests are added.

Changes since v1:

* Improvements in the code for denying pushing hidden refs.
* Simplification of the tests as suggested by Eric.
* New tests for the code for denying pushing hidden refs.
* Comments.

[1] http://marc.info/?l=git&m=144604694223920

Lukas Fleischer (4):
  Document the semantics of hideRefs with namespaces
  upload-pack: strip refs before calling ref_is_hidden()
  Add support for matching full refs in hideRefs
  t5509: add basic tests for hideRefs

 Documentation/config.txt |  8 
 builtin/receive-pack.c   | 27 --
 refs.c   | 15 ---
 refs.h   | 10 +-
 t/t5509-fetch-push-namespaces.sh | 41 
 upload-pack.c| 13 -
 6 files changed, 99 insertions(+), 15 deletions(-)

-- 
2.6.2

--
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/4] Document the semantics of hideRefs with namespaces

2015-11-03 Thread Lukas Fleischer
Right now, there is no clear definition of how transfer.hideRefs should
behave when a namespace is set. Explain that hideRefs prefixes match
stripped names in that case. This is how hideRefs patterns are currently
handled in receive-pack.

Signed-off-by: Lukas Fleischer 
---
 Documentation/config.txt | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1204072..3da97a1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2684,6 +2684,13 @@ You may also include a `!` in front of the ref name to 
negate the entry,
 explicitly exposing it, even if an earlier entry marked it as hidden.
 If you have multiple hideRefs values, later entries override earlier ones
 (and entries in more-specific config files override less-specific ones).
++
+If a namespace is set, references are stripped before matching. For example, if
+the prefix `refs/heads/master` is specified in `transfer.hideRefs` and the
+current namespace is `foo`, then `refs/namespaces/foo/refs/heads/master` is
+omitted from the advertisements but `refs/heads/master` and
+`refs/namespaces/bar/refs/heads/master` are still advertised as so-called
+"have" lines.
 
 transfer.unpackLimit::
When `fetch.unpackLimit` or `receive.unpackLimit` are
-- 
2.6.2

--
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/4] upload-pack: strip refs before calling ref_is_hidden()

2015-11-03 Thread Lukas Fleischer
Make hideRefs handling in upload-pack consistent with the behavior
described in the documentation by stripping refs before comparing them
with prefixes in hideRefs.

Signed-off-by: Lukas Fleischer 
---
 upload-pack.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/upload-pack.c b/upload-pack.c
index d0bc3ca..4ca960e 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -692,7 +692,7 @@ static int mark_our_ref(const char *refname, const struct 
object_id *oid)
 {
struct object *o = lookup_unknown_object(oid->hash);
 
-   if (ref_is_hidden(refname)) {
+   if (refname && ref_is_hidden(refname)) {
o->flags |= HIDDEN_REF;
return 1;
}
@@ -703,7 +703,7 @@ static int mark_our_ref(const char *refname, const struct 
object_id *oid)
 static int check_ref(const char *refname, const struct object_id *oid,
 int flag, void *cb_data)
 {
-   mark_our_ref(refname, oid);
+   mark_our_ref(strip_namespace(refname), oid);
return 0;
 }
 
@@ -726,7 +726,7 @@ static int send_ref(const char *refname, const struct 
object_id *oid,
const char *refname_nons = strip_namespace(refname);
struct object_id peeled;
 
-   if (mark_our_ref(refname, oid))
+   if (mark_our_ref(refname_nons, oid))
return 0;
 
if (capabilities) {
-- 
2.6.2

--
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 3/4] Add support for matching full refs in hideRefs

2015-11-03 Thread Lukas Fleischer
In addition to matching stripped refs, one can now add hideRefs patterns
that the full (unstripped) ref is matched against. To distinguish
between stripped and full matches, those new patterns must be prefixed
with a circumflex (^).

This commit also removes support for the undocumented and unintended
hideRefs settings "have" (suppressing all "have" lines) and
"capabilities^{}" (suppressing the capabilities line).

Signed-off-by: Lukas Fleischer 
---
 Documentation/config.txt |  3 ++-
 builtin/receive-pack.c   | 27 +--
 refs.c   | 15 ---
 refs.h   | 10 +-
 upload-pack.c| 13 -
 5 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3da97a1..91ed6a5 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2690,7 +2690,8 @@ the prefix `refs/heads/master` is specified in 
`transfer.hideRefs` and the
 current namespace is `foo`, then `refs/namespaces/foo/refs/heads/master` is
 omitted from the advertisements but `refs/heads/master` and
 `refs/namespaces/bar/refs/heads/master` are still advertised as so-called
-"have" lines.
+"have" lines. In order to match refs before stripping, add a `^` in front of
+the ref name. If you combine `!` and `^`, `!` must be specified first.
 
 transfer.unpackLimit::
When `fetch.unpackLimit` or `receive.unpackLimit` are
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index bcb624b..9939de1 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -195,9 +195,6 @@ static int receive_pack_config(const char *var, const char 
*value, void *cb)
 
 static void show_ref(const char *path, const unsigned char *sha1)
 {
-   if (ref_is_hidden(path))
-   return;
-
if (sent_capabilities) {
packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
} else {
@@ -219,9 +216,14 @@ static void show_ref(const char *path, const unsigned char 
*sha1)
}
 }
 
-static int show_ref_cb(const char *path, const struct object_id *oid, int 
flag, void *unused)
+static int show_ref_cb(const char *path_full, const struct object_id *oid,
+  int flag, void *unused)
 {
-   path = strip_namespace(path);
+   const char *path = strip_namespace(path_full);
+
+   if (ref_is_hidden(path, path_full))
+   return 1;
+
/*
 * Advertise refs outside our current namespace as ".have"
 * refs, so that the client can use them to minimize data
@@ -1195,16 +1197,29 @@ static int iterate_receive_command_list(void *cb_data, 
unsigned char sha1[20])
 
 static void reject_updates_to_hidden(struct command *commands)
 {
+   struct strbuf refname_full = STRBUF_INIT;
+   size_t prefix_len;
struct command *cmd;
 
+   strbuf_addstr(&refname_full, get_git_namespace());
+   prefix_len = refname_full.len;
+
for (cmd = commands; cmd; cmd = cmd->next) {
-   if (cmd->error_string || !ref_is_hidden(cmd->ref_name))
+   if (cmd->error_string)
+   continue;
+
+   strbuf_setlen(&refname_full, prefix_len);
+   strbuf_addstr(&refname_full, cmd->ref_name);
+
+   if (!ref_is_hidden(cmd->ref_name, refname_full.buf))
continue;
if (is_null_sha1(cmd->new_sha1))
cmd->error_string = "deny deleting a hidden ref";
else
cmd->error_string = "deny updating a hidden ref";
}
+
+   strbuf_release(&refname_full);
 }
 
 static int should_process_cmd(struct command *cmd)
diff --git a/refs.c b/refs.c
index 72d96ed..892fffb 100644
--- a/refs.c
+++ b/refs.c
@@ -321,7 +321,7 @@ int parse_hide_refs_config(const char *var, const char 
*value, const char *secti
return 0;
 }
 
-int ref_is_hidden(const char *refname)
+int ref_is_hidden(const char *refname, const char *refname_full)
 {
int i;
 
@@ -329,6 +329,7 @@ int ref_is_hidden(const char *refname)
return 0;
for (i = hide_refs->nr - 1; i >= 0; i--) {
const char *match = hide_refs->items[i].string;
+   const char *subject;
int neg = 0;
int len;
 
@@ -337,10 +338,18 @@ int ref_is_hidden(const char *refname)
match++;
}
 
-   if (!starts_with(refname, match))
+   if (*match == '^') {
+   subject = refname_full;
+   match++;
+   } else {
+   subject = refname;
+   }
+
+   /* refname can be NULL when namespaces are used. */
+   if (!subject || !starts_with(subject, match))
continue;
len = strlen(match);
-   if (!refname[len] || refname[len] == '/')
+   if (