Re: [PATCH v4] Add git-grep threads param
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
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
> 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
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
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()
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()
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
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
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
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
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
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
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
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
`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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
> 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)
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
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
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()
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
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
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
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
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
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
>> 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
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
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
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()
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
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 (