Re: [PATCH 3/2] t5004: resurrect original empty tar archive test
Hi, René Scharfe wrote: [Subject: t5004: resurrect original empty tar archive test] [...] The different approaches test different things: The existing one is for empty trees, for which we know the exact expected output and thus we can simply check it without extracting; the new one is for commits with empty trees, whose archives include stamps and so the more natural check by extraction is a better fit because it focuses on the interesting aspect, namely the absence of any archive entries. Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx When first reading I was a little confused: does this patch resurrect the original, existing test for empty tree handling in the form it had before patch 2/3, or is it adding a new, distinct test that complements the existing one that patch 2/3 modified? A quick glance back at v1.8.2.2~7^2 (t5004: fix issue with empty archive test and bsdtar, 2013-04-10) cleared matters up. The original test that is being resurrected is the one from before that commit. Maybe a reminder in the commit message would help. E.g., The earlier version of the same check (before 24676f02, t5004: fix issue with empty archive test and bsdtar) revived by this patch tests a different thing: The modified check is for empty trees, for which we know the exact expected output and thus we can simply check it without extracting; the original one is for commits with empty trees, whose archives include stamps and so the more natural check by extraction is a better fit because it focuses on the interesting aspect, namely the absence of any archive entries. With or without such a change, Reviewed-by: Jonathan Nieder jrnie...@gmail.com Would it make sense to define HEADER_ONLY_TAR_OK as a lazy prereq in the same file (even though it is only used once), so the code that checks tar is not run if this test is being skipped (e.g., using GIT_TEST_SKIP) for some other reason? Thanks, Jonathan -- To unsubscribe from this list: send the line 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] gitk: add support for -G'regex' pickaxe variant
Paul Mackerras wrote: I thought I had replied to this patch; maybe I only thought about it. Given that we already have a selector to choose between exact and regexp matching, it seems more natural to use that rather than add a new selector entry. Arguably the IgnCase option should be disabled when adding/removing string is selected. Thanks. I think I disagree: log -G and log -S are different operations, not variations on the same one. The description Find next commit adding/removing string: very clearly conveys what -S means. Maybe -G would be more clearly described as Find next commit changing line that matches regex: or Find next commit changing line containing:? Jonathan -- To unsubscribe from this list: send the line 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] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8
Hi, David Aguilar wrote: Mac OS X Mountain Lion prints warnings when building git: warning: 'SHA1_Init' is deprecated (declared at /usr/include/openssl/sha.h:121) Silence the warnings by using the Common Digest SHA-1 functions for SHA1_Init(), SHA1_Update(), and SHA1_Final(). Thanks. Does this perform better or worse than just setting BLK_SHA1=YesPlease? I'd naively think it could go either way: on one hand adding another library dependency can slow down startup, and on the other hand the implementation may or may not be optimized better than the generic block-sha1/ implementation. Curious, Jonathan -- To unsubscribe from this list: send the line 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] imap-send: eliminate HMAC warnings on OS X 10.8
David Aguilar wrote: Mac OS X Mountain Lion warns that HMAC_Init() and friends are deprecated. Use CommonCrypto's HMAC to eliminate the warnings. Makes sense, and if the #define trick stops working some day due to some conflicting macro in an openssl header some day, it would just break the build, so this should be safe. For what it's worth, Reviewed-by: Jonathan Nieder jrnie...@gmail.com [...] This builds upon the patch I sent earlier, so technically it's 2/2 I'm less sure about whether patch 1/2 is a good idea. Luckily the conflicts when trying to untangle the two are only textual. Thanks, Jonathan -- To unsubscribe from this list: send the line 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] Makefile: fix default regex settings on Darwin
David Aguilar wrote: t0070-fundamental.sh fails on Mac OS X 10.8 by default. Fix it by using Git's regex library. Can you say more about the failure? What does ./t0070-fundamental.sh -v say? Curious, Jonathan -- To unsubscribe from this list: send the line 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] Makefile: fix default regex settings on Darwin
On Fri, May 10, 2013 at 11:31 PM, Jonathan Nieder jrnie...@gmail.com wrote: David Aguilar wrote: t0070-fundamental.sh fails on Mac OS X 10.8 by default. Fix it by using Git's regex library. Can you say more about the failure? What does ./t0070-fundamental.sh -v say? [..snip...] expecting success: # if this test fails, re-build git with NO_REGEX=1 test-regex fatal: regex bug confirmed: re-build git with NO_REGEX=1 not ok 4 - check for a bug in the regex routines # # # if this test fails, re-build git with NO_REGEX=1 # test-regex # # failed 1 among 4 test(s) 1..4 -- David -- To unsubscribe from this list: send the line 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] Makefile: fix default regex settings on Darwin
David Aguilar wrote: expecting success: # if this test fails, re-build git with NO_REGEX=1 test-regex fatal: regex bug confirmed: re-build git with NO_REGEX=1 Thanks. Gah. That means that regcomp() with REG_NEWLINE is letting [^={} \t]+ match the newline in ={}\nfred despite the POSIX requirement A newline in string shall not be matched by a period outside a bracket expression or by any form of a non-matching list where A non-matching list expression begins with a circumflex ('^') and specifies a list that shall match any single-character collating element except for the expressions represented in the list after the leading circumflex. and if I understand you correctly, this is a regression in Apple libc. :( With the commit message modified to mention the fatal: regex bug confirmed: re-build git with NO_REGEX=1 and uname -a output, Reviewed-by: Jonathan Nieder jrnie...@gmail.com Do you know if this has been reported to Apple and openradar.appspot.com? -- To unsubscribe from this list: send the line 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] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8
On Fri, May 10, 2013 at 11:23 PM, Jonathan Nieder jrnie...@gmail.com wrote: Hi, David Aguilar wrote: Mac OS X Mountain Lion prints warnings when building git: warning: 'SHA1_Init' is deprecated (declared at /usr/include/openssl/sha.h:121) Silence the warnings by using the Common Digest SHA-1 functions for SHA1_Init(), SHA1_Update(), and SHA1_Final(). Thanks. Does this perform better or worse than just setting BLK_SHA1=YesPlease? I'd naively think it could go either way: on one hand adding another library dependency can slow down startup, and on the other hand the implementation may or may not be optimized better than the generic block-sha1/ implementation. Pretty much identical. Here are the timings (I should probably read t/perf/README and get better numbers): Best of ten $ time git rev-list --all --objects /dev/null # master git rev-list --all --objects /dev/null 5.16s user 0.11s system 99% cpu 5.277 total # BLK_SHA1 git rev-list --all --objects /dev/null 5.17s user 0.11s system 99% cpu 5.299 total # CommonDigest git rev-list --all --objects /dev/null 5.18s user 0.11s system 99% cpu 5.301 total The library startup cost is identical to master since at the end of the day it's still libcrypto. FWIW the way it's done in this patch still allows defining BLK_SHA1, which is parity with how the Makefile behaves on Linux. Re-roll? -- David -- To unsubscribe from this list: send the line 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: Outdated and broken online versions of user-manual.html
W. Trevor King wking at tremily.us writes: I'm also surprised that I couldn't find a more obvious link to the manual from git-scm.com (I ended up taking a “See Also” link from gittutorial(7) [3]). I'm not sure if this is intentional or not, since git-scm.com does prominently link Pro Git, and that overlaps fairly significantly with the manual. Folks with Git installed will generally have man pages, so it's not a big deal, but having current docs somewhere online to link against would be nice. I'm also curious if I should be linking against a particular location. IMHO user-manual is a natural step for a Git beginner after reading one of the books like Pro Git and before he is ready to digest the man pages. But up to now there are several problems with user-manual besides the problems described by Trevor: (1) Very poor html formatting (document type book causes ugly TOCs per section and there's a Part I without a Part II) (2) Partly outdated content (3) Sub-optimal structuring (to-do list as part of the document, glossary not at the end of the document) (4) User-manual.PDF uses an independent tool chain which makes it harder to do improvements for user-manual.html and also is the only pdf doc we are creating. IMHO we should remove this altogether. (5) Large overlapping with the tutorials. IMHO all of the tutorials should be blended into user-manual I am currently working on (1)-(4) and then aiming for (5). Comments are welcome ... --- Thomas -- To unsubscribe from this list: send the line 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] Makefile: fix default regex settings on Darwin
On Sat, May 11, 2013 at 12:04 AM, Jonathan Nieder jrnie...@gmail.com wrote: David Aguilar wrote: expecting success: # if this test fails, re-build git with NO_REGEX=1 test-regex fatal: regex bug confirmed: re-build git with NO_REGEX=1 Thanks. Gah. That means that regcomp() with REG_NEWLINE is letting [^={} \t]+ match the newline in ={}\nfred despite the POSIX requirement A newline in string shall not be matched by a period outside a bracket expression or by any form of a non-matching list where A non-matching list expression begins with a circumflex ('^') and specifies a list that shall match any single-character collating element except for the expressions represented in the list after the leading circumflex. and if I understand you correctly, this is a regression in Apple libc. :( With the commit message modified to mention the fatal: regex bug confirmed: re-build git with NO_REGEX=1 and uname -a output, Reviewed-by: Jonathan Nieder jrnie...@gmail.com Thanks. I can adjust the commit message and re-roll to make this patch 1/3. Do you know if this has been reported to Apple and openradar.appspot.com? I checked and nope, there are no bug reports. I tried to submit one by creating a new radar but that page said to go to bugreport.apple.com. I tried to login there but after entering my credentials I got, An error has occurred. Please report the error to Apple Inc. by emailing the error detail to devb...@apple.com. Oh bother ;-) I refreshed the page and finally got in. I submitted a bug.. Problem # 13868200 I hop on this laptop to test for cross-platform issues (I'm a Linux user myself ;-) and it's been pretty good at finding 'em. -- David -- To unsubscribe from this list: send the line 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] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8
On Sat, May 11, 2013 at 12:11:05AM -0700, David Aguilar wrote: Does this perform better or worse than just setting BLK_SHA1=YesPlease? I'd naively think it could go either way: on one hand adding another library dependency can slow down startup, and on the other hand the implementation may or may not be optimized better than the generic block-sha1/ implementation. Pretty much identical. Here are the timings (I should probably read t/perf/README and get better numbers): Best of ten $ time git rev-list --all --objects /dev/null [...] I'm not sure that's a great test of sha1 performance. It will hash the commit and tree objects it loads during the traversal, but that time is almost certainly dwarfed by zlib inflation and by lookup_object. Adding --verify-objects would sha1 the blobs, too, which might be more reasonable (or running git fsck). Something like git add on a large blob would also be a good test. -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
[PATCH v2 1/3] Makefile: fix default regex settings on Darwin
t0070-fundamental.sh fails on Mac OS X 10.8: $ uname -a Darwin lustrous 12.2.0 Darwin Kernel Version 12.2.0: Sat Aug 25 00:48:52 PDT 2012; root:xnu-2050.18.24~1/RELEASE_X86_64 x86_64 $ ./t0070-fundamental.sh -v fatal: regex bug confirmed: re-build git with NO_REGEX=1 Fix it by using Git's regex library. Reviewed-by: Jonathan Nieder jrnie...@gmail.com Signed-off-by: David Aguilar dav...@gmail.com --- Added uname output and Jonathan's review tag. Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 0f931a2..f698c1a 100644 --- a/Makefile +++ b/Makefile @@ -1054,6 +1054,7 @@ ifeq ($(uname_S),Darwin) BASIC_LDFLAGS += -L/opt/local/lib endif endif + NO_REGEX = YesPlease PTHREAD_LIBS = endif -- 1.8.3.rc1.47.g41936fa -- To unsubscribe from this list: send the line 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/3] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8
Mac OS X Mountain Lion prints warnings when building git: warning: 'SHA1_Init' is deprecated (declared at /usr/include/openssl/sha.h:121) Silence the warnings by using the Common Digest SHA-1 functions for SHA1_Init(), SHA1_Update(), and SHA1_Final(). Add a COMMON_DIGEST_SHA1 knob to the Makefile to allow choosing this implementation and define it by default on Darwin. Signed-off-by: David Aguilar dav...@gmail.com --- Unchanged since last time; rebased to 3/3. Makefile | 7 +++ cache.h | 7 +++ 2 files changed, 14 insertions(+) diff --git a/Makefile b/Makefile index 25282b4..8b2f9cc 100644 --- a/Makefile +++ b/Makefile @@ -1055,6 +1055,7 @@ ifeq ($(uname_S),Darwin) endif endif COMMON_DIGEST_HMAC = YesPlease + COMMON_DIGEST_SHA1 = YesPlease NO_REGEX = YesPlease PTHREAD_LIBS = endif @@ -1390,10 +1391,16 @@ ifdef PPC_SHA1 LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o LIB_H += ppc/sha1.h else +ifdef COMMON_DIGEST_SHA1 + BASIC_CFLAGS += -DCOMMON_DIGEST_FOR_SHA1=1 + SHA1_HEADER = CommonCrypto/CommonDigest.h + EXTLIBS += $(LIB_4_CRYPTO) +else SHA1_HEADER = openssl/sha.h EXTLIBS += $(LIB_4_CRYPTO) endif endif +endif ifdef COMMON_DIGEST_HMAC BASIC_CFLAGS += -DCOMMON_DIGEST_FOR_HMAC=1 diff --git a/cache.h b/cache.h index 94ca1ac..e2b24c6 100644 --- a/cache.h +++ b/cache.h @@ -10,11 +10,18 @@ #include SHA1_HEADER #ifndef git_SHA_CTX +#ifdef COMMON_DIGEST_FOR_SHA1 +#define git_SHA_CTXCC_SHA1_CTX +#define git_SHA1_Init CC_SHA1_Init +#define git_SHA1_UpdateCC_SHA1_Update +#define git_SHA1_Final CC_SHA1_Final +#else #define git_SHA_CTXSHA_CTX #define git_SHA1_Init SHA1_Init #define git_SHA1_UpdateSHA1_Update #define git_SHA1_Final SHA1_Final #endif +#endif #include zlib.h typedef struct git_zstream { -- 1.8.3.rc1.47.g41936fa -- To unsubscribe from this list: send the line 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] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8
On Sat, May 11, 2013 at 1:22 AM, Jeff King p...@peff.net wrote: On Sat, May 11, 2013 at 12:11:05AM -0700, David Aguilar wrote: Does this perform better or worse than just setting BLK_SHA1=YesPlease? I'd naively think it could go either way: on one hand adding another library dependency can slow down startup, and on the other hand the implementation may or may not be optimized better than the generic block-sha1/ implementation. Pretty much identical. Here are the timings (I should probably read t/perf/README and get better numbers): Best of ten $ time git rev-list --all --objects /dev/null [...] I'm not sure that's a great test of sha1 performance. It will hash the commit and tree objects it loads during the traversal, but that time is almost certainly dwarfed by zlib inflation and by lookup_object. Adding --verify-objects would sha1 the blobs, too, which might be more reasonable (or running git fsck). Something like git add on a large blob would also be a good test. Thanks. Here are the numbers with --verify-objects: $ time git rev-list --all --objects --verify-objects /dev/null # CommonCrypto 32.24s user 4.65s system 99% cpu 37.098 total # master 33.00s user 4.68s system 99% cpu 37.852 total # BLK_SHA1 54.17s user 4.67s system 99% cpu 58.928 total Doing BLK_SHA1 seems like less of a good idea now, so I think my latest re-roll might be the way to go... -- David -- To unsubscribe from this list: send the line 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] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8
On Sat, May 11, 2013 at 01:38:32AM -0700, David Aguilar wrote: Adding --verify-objects would sha1 the blobs, too, which might be more reasonable (or running git fsck). Something like git add on a large blob would also be a good test. Thanks. Here are the numbers with --verify-objects: $ time git rev-list --all --objects --verify-objects /dev/null # CommonCrypto 32.24s user 4.65s system 99% cpu 37.098 total # master 33.00s user 4.68s system 99% cpu 37.852 total # BLK_SHA1 54.17s user 4.67s system 99% cpu 58.928 total Doing BLK_SHA1 seems like less of a good idea now, so I think my latest re-roll might be the way to go... Wow, that's really terrible. What level of optimization do you compile with? With the other implementations, you are calling into (presumably) optimized library code, but with BLK_SHA1 you are getting whatever you just compiled. Here are three timings that show how big a difference that can make: openssl, -O00m21.152s BLK_SHA1, -O00m31.920s BLK_SHA1, -O30m19.652s So it is a win over openssl with optimizations on, but a pretty big loss otherwise. -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: Re: [PATCH v3 5/5] do not die when error in config parsing of buf occurs
On Fri, May 10, 2013 at 12:39:37AM +0200, Jeff King wrote: On Thu, May 09, 2013 at 06:21:02PM +0200, Heiko Voigt wrote: diff --git a/builtin/config.c b/builtin/config.c index 8d01b7a..de32977 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -219,9 +219,11 @@ static int get_value(const char *key_, const char *regex_) } } - git_config_with_options(collect_config, values, + ret = git_config_with_options(collect_config, values, given_config_file, given_config_blob, respect_includes); + if (ret 0) + goto free_values; ret = !values.nr; @@ -231,6 +233,7 @@ static int get_value(const char *key_, const char *regex_) fwrite(buf-buf, 1, buf-len, stdout); strbuf_release(buf); } +free_values: free(values.items); free_strings: Hmm. values is a strbuf_list. Don't we need to strbuf_release() its individual members before freeing it? The writing loop directly above your label frees each one after writing. It would probably make sense to just split it into two loops, one for writing, and then one for free-ing. This is not a critical performance path that we can't iterate over the (probably handful of) entries twice. Thanks for catching that. I missed the strbuf_release in the loop somehow. Will fix in the next iteration. diff --git a/t/t1307-config-blob.sh b/t/t1307-config-blob.sh index fdc257e..a4929eb 100755 --- a/t/t1307-config-blob.sh +++ b/t/t1307-config-blob.sh @@ -55,12 +55,17 @@ test_expect_success 'editing a blob is an error' ' test_expect_success 'parse errors in blobs are properly attributed' ' cat config -\EOF [some] - value = + value = 1 + error = EOF git add config git commit -m broken - test_must_fail git config --blob=HEAD:config some.value 2err + test_must_fail git config --blob=HEAD:config some.value 1out 2err + + # Make sure there is no output of values on stdout + touch out.expect + test_cmp out.expect out I'm not clear on what is being tested here. Were we outputting to stdout before this patch (I would have thought our die() meant we did not. We where not outputting to stdout before so the test is correct there as well. I extended the test when implementing the non-dying version of git_config_with_options() so I could see the test fail. If just returning the error it would still output the values read until then. So if you think that it belongs into the initial version of this test (maybe including some comment why we need an extra parseable value) I am happy to move it there. Cheers Heiko -- To unsubscribe from this list: send the line 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] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8
On Sat, May 11, 2013 at 1:45 AM, Jeff King p...@peff.net wrote: On Sat, May 11, 2013 at 01:38:32AM -0700, David Aguilar wrote: Adding --verify-objects would sha1 the blobs, too, which might be more reasonable (or running git fsck). Something like git add on a large blob would also be a good test. Thanks. Here are the numbers with --verify-objects: $ time git rev-list --all --objects --verify-objects /dev/null # CommonCrypto 32.24s user 4.65s system 99% cpu 37.098 total # master 33.00s user 4.68s system 99% cpu 37.852 total # BLK_SHA1 54.17s user 4.67s system 99% cpu 58.928 total Doing BLK_SHA1 seems like less of a good idea now, so I think my latest re-roll might be the way to go... Wow, that's really terrible. What level of optimization do you compile with? With the other implementations, you are calling into (presumably) optimized library code, but with BLK_SHA1 you are getting whatever you just compiled. Here are three timings that show how big a difference that can make: openssl, -O00m21.152s BLK_SHA1, -O00m31.920s BLK_SHA1, -O30m19.652s So it is a win over openssl with optimizations on, but a pretty big loss otherwise. Good catch. I had a config.mak without any -O flags in CFLAGS. Here are the timings with -O3. We're back to parity. $ time git rev-list --all --objects --verify-objects /dev/null # CommonCrypto 28.95s user 4.62s system 99% cpu 33.630 total # master 29.81s user 4.70s system 99% cpu 34.760 total # BLK_SHA1 29.80s user 4.62s system 99% cpu 34.505 total If BLK_SHA1 were the default on all platforms then I wouldn't have bothered with the SHA-1 patch. With this patch it makes it like Linux in that Git can choose between the built-in functions and the external library. That's why I moved this patch to 3/3.. it could go either way. -- David -- To unsubscribe from this list: send the line 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: [PATCHv3 3/7] show: honor --textconv for blobs
Junio C Hamano venit, vidit, dixit 10.05.2013 19:02: Michael J Gruber g...@drmicha.warpmail.net writes: Currently, diff and cat-file for blobs honor --textconv options (with the former defaulting to --textconv and the latter to --no-textconv) whereas show does not honor this option, even though it takes diff options. Make show on blobs behave like diff, i.e. honor --textconv by default and --no-textconv when given. Hmm... Sorry, I overlooked that ;) +static int show_blob_object(const unsigned char *sha1, struct rev_info *rev, const char *obj_name) { +unsigned char sha1c[20]; +struct object_context obj_context; +char *buf; +unsigned long size; + fflush(stdout); -return stream_blob_to_fd(1, sha1, NULL, 0); +if (!DIFF_OPT_TOUCHED(rev-diffopt, ALLOW_TEXTCONV) || +!DIFF_OPT_TST(rev-diffopt, ALLOW_TEXTCONV)) +return stream_blob_to_fd(1, sha1, NULL, 0); It is surprising that the necessary change is only this, but I think it is correct ;-). We ignore textconv when the command line did not mention --[no-]textconv, or the command line said --no-textconv explicitly. This (especially the first condition) may deserve an in-code comment for anybody who wonders where this default behaviour is implemented. It's not as if we would document behavior by in-code comments in general, do we? The usual answer is git log -S or git blame. So show on blobs does show the raw contents by default, but the user can explicitly ask to enable textconv with --[no-]textconv. Is the second paragraph in the log message still valid? +if (get_sha1_with_context(obj_name, 0, sha1c, obj_context)) +die(Not a valid object name %s, obj_name); This looks somewhat unfortunate. We already have sha1[]; actually we not just know sha1[] but have the struct object for it. How did we obtain it before we got here? Will we always have a valid name in rev.pending.objects-name? Will that name convert back to the same sha1 we got in sha1[]? I think the answers are Yes (it is a command line argument), Yes (that is what setup_revisions() got by feeding the name to give us sha1[]). I wonder if enriching rev_info-pending with the context information might be a clean solution to avoid this redundant but unavoidable conversion, but that is a separate and future topic, I think. Yes, I think both Jeff and I have thought it and came to the same conclusion - later ;) +if (!obj_context.path[0] || +!textconv_object(obj_context.path, obj_context.mode, sha1c, 1, buf, size)) +return stream_blob_to_fd(1, sha1, NULL, 0); + +if (!buf) +die(git show %s: bad file, obj_name); + +write_or_die(1, buf, size); +return 0; } static int show_tag_object(const unsigned char *sha1, struct rev_info *rev) @@ -526,7 +545,7 @@ int cmd_show(int argc, const char **argv, const char *prefix) const char *name = objects[i].name; switch (o-type) { case OBJ_BLOB: -ret = show_blob_object(o-sha1, NULL); +ret = show_blob_object(o-sha1, rev, name); break; case OBJ_TAG: { struct tag *t = (struct tag *)o; diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh index 3950fc9..0ebb028 100755 --- a/t/t4030-diff-textconv.sh +++ b/t/t4030-diff-textconv.sh @@ -96,14 +96,14 @@ test_expect_success 'show blob produces binary' ' test_cmp expect actual ' -test_expect_failure 'show --textconv blob produces text' ' +test_expect_success 'show --textconv blob produces text' ' git show --textconv HEAD:file actual printf 0\\n1\\n expect test_cmp expect actual ' -test_success 'show --no-textconv blob produces binary' ' -git show --textconv HEAD:file actual +test_expect_success 'show --no-textconv blob produces binary' ' +git show --no-textconv HEAD:file actual printf \\0\\n\\01\\n expect test_cmp expect actual ' -- To unsubscribe from this list: send the line 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] gitk: add support for -G'regex' pickaxe variant
On Fri, May 10, 2013 at 11:13:22PM -0700, Jonathan Nieder wrote: Paul Mackerras wrote: I thought I had replied to this patch; maybe I only thought about it. Given that we already have a selector to choose between exact and regexp matching, it seems more natural to use that rather than add a new selector entry. Arguably the IgnCase option should be disabled when adding/removing string is selected. Thanks. I think I disagree: log -G and log -S are different operations, not variations on the same one. OK, fair enough, and I see there is in fact a --pickaxe-regex we could use. The description Find next commit adding/removing string: very clearly conveys what -S means. Maybe -G would be more clearly described as Find next commit changing line that matches regex: or Find next commit changing line containing:? How about changing lines matching:? If it gets too long it will take up too much horizontal room. Paul. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thoughts about eliminating reachability races
How to get rid of reachability races? This email is meant more as a basis for discussion (including at the GitMerge conference that is currently running) than as a fully-baked proposal. A lot of reachability-related races have been discussed recently: * If a reference is renamed while a prune is reading the loose references, it could be that prune sees neither the old nor the new version of the reference and could prune the objects referred to by the reference. * If a process is building a tree, it could find that some objects are already in the DB, in which case it will refer to the existing objects. But these objects might be unreachable, and could be pruned before the process gets around to creating a root reference that makes them reachable. * Other races involving packed refs vs. loose refs. These races are most critical for busy hosting sites like GitHub but could theoretically impact anybody. It has been discussed that we should offer, as an alternative to packed-refs/loose refs in the filesystem, a way to store refs in a single database store of some kind that offers ACID guarantees. This would solve the first and last kind of race in one step. Other mechanisms are being discussed to address the second race, maybe involving moving pruned objects into a kind of purgatory for a period of time before they are finally deleted. But I have the feeling that these races could also be solved via the use of the same database. I am thinking of a scheme in which the database stores two more things during prunes: 1. The names of objects that are being pruned. Such objects are considered deleted (even though they might not yet have been deleted from the object database) and no new references may be made to them. 2. The names of objects to which new links have been added. Prune is not allowed to delete this objects or objects to which they refer. like those used for incremental garbage collection of memory, in which *during* a garbage collection the VM keeps tracks of new references that are made to objects that are in the generation that is currently being garbage collected. The garbage collector has to consider these links in its reachability analysis. Our situation is a little bit more complicated, because it is not easy to inform a git process that a prune has been started. At best, the process can check occasionally whether a prune is running. On the other hand, it *is* permissible for Git processes to fail in exceptional circumstances, as long as it happens rarely and leaves the object store in a valid state. So I'm thinking of a scheme in which 1. prune has to list doomed objects to the database before deleting them. Such objects are considered deleted by other processes (*if* they know that a prune is running). 2. If a process knows that a prune is running, then it records to the database any new links that it makes to existing objects. Prune is not allowed to prune such objects. 3. To catch the situation that a process didn't know that a prune is running, we keep a prune generation number. If that number changes between the beginning/end of a process, then the process fails without recording any new references. Design goals: * Correctness * Normal processes don't block on each other or on prune. * Normal processes don't get significantly slower in the usual case (but are allowed to get slower during pruning). Assumptions: * There is a store with atomic transactions. * Most normal processes run more quickly than a prune. It will be rare for an entire prune to run while a normal process is running (though even if it does we must behave correctly). * Normal processes are allowed to fail occasionally as long as they leave the DB in a valid state. In the store, record the following values: * Git references -- (refname, object_name) pairs * prune_in_progress (boolean) -- a destructive operation is running * prune_generation (integer) -- incremented each time objects are invalidated * doomed_objects -- a list of the names of objects that a prune is in the process of deleting. These objects *are considered deleted* and may not be referred to. Only used when prune_in_progress. * new_links -- a list of the names of objects newly created or newly referenced during a prune. These objects are considered referenced and *may not be pruned*. Only used when prune_in_progress. A nondestructive operation will usually have two interactions with the store. At start:: def record_new_link(sha1): Record that there is a new link to object sha1. if prune_in_progress: if sha1 in doomed_objects: die() append sha1 to new_links (RAM copy) with transaction: read prune_generation read prune_in_progress if prune_in_progress: read doomed_objects read any needed Git refs for each new object or new link to
Re: [PATCH v2] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8
On Sat, May 11, 2013 at 02:17:10AM -0700, David Aguilar wrote: Good catch. I had a config.mak without any -O flags in CFLAGS. Here are the timings with -O3. We're back to parity. $ time git rev-list --all --objects --verify-objects /dev/null # CommonCrypto 28.95s user 4.62s system 99% cpu 33.630 total # master 29.81s user 4.70s system 99% cpu 34.760 total # BLK_SHA1 29.80s user 4.62s system 99% cpu 34.505 total If BLK_SHA1 were the default on all platforms then I wouldn't have bothered with the SHA-1 patch. With this patch it makes it like Linux in that Git can choose between the built-in functions and the external library. With those numbers above, it seems like CommonCrypto is still a reasonable choice. I don't know anything about availability or other OS X specific issues, but assuming there are no issues there, your patch makes sense to me. Thanks for double-checking the timings. -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: Re: [PATCH v3 5/5] do not die when error in config parsing of buf occurs
On Sat, May 11, 2013 at 10:55:31AM +0200, Heiko Voigt wrote: diff --git a/t/t1307-config-blob.sh b/t/t1307-config-blob.sh index fdc257e..a4929eb 100755 --- a/t/t1307-config-blob.sh +++ b/t/t1307-config-blob.sh @@ -55,12 +55,17 @@ test_expect_success 'editing a blob is an error' ' test_expect_success 'parse errors in blobs are properly attributed' ' cat config -\EOF [some] - value = + value = 1 + error = EOF git add config git commit -m broken - test_must_fail git config --blob=HEAD:config some.value 2err + test_must_fail git config --blob=HEAD:config some.value 1out 2err + + # Make sure there is no output of values on stdout + touch out.expect + test_cmp out.expect out I'm not clear on what is being tested here. Were we outputting to stdout before this patch (I would have thought our die() meant we did not. We where not outputting to stdout before so the test is correct there as well. I extended the test when implementing the non-dying version of git_config_with_options() so I could see the test fail. If just returning the error it would still output the values read until then. So if you think that it belongs into the initial version of this test (maybe including some comment why we need an extra parseable value) I am happy to move it there. From what you wrote above, it sounds like we would expect git-config to write out some.value before failing on some.error. But that isn't what the test is checking, is it? It is checking that nothing is output. I do not think the output matters much either way when git-config has failed; if it returns a non-zero exit code, then the results of stdout cannot be trusted (after all, it may have been killed by signal after it had written out half of the output). Still slightly puzzled... -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] gitk: Add menu item for reverting commits
On Sat, Apr 27, 2013 at 04:36:13PM +0200, Knut Franke wrote: Sometimes it's helpful (at least psychologically) to have this feature easily accessible. Code borrows heavily from cherrypick. Signed-off-by: Knut Franke knut.fra...@gmx.de Thanks, applied, after undoing the linewrapping (done by your mailer?). Paul. -- To unsubscribe from this list: send the line 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] gitk: simplify file filtering
On Sat, Apr 27, 2013 at 05:01:39PM -0500, Felipe Contreras wrote: git diff is perfectly able to do this with '-- files', no need for manual filtering. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com Thanks, applied, with the commit message expanded to say that this makes gettreediffs do the same as getblobdiffs. Paul. -- To unsubscribe from this list: send the line 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: Re: [PATCH v3 5/5] do not die when error in config parsing of buf occurs
On Sat, May 11, 2013 at 11:59:36AM +0200, Jeff King wrote: On Sat, May 11, 2013 at 10:55:31AM +0200, Heiko Voigt wrote: We where not outputting to stdout before so the test is correct there as well. I extended the test when implementing the non-dying version of git_config_with_options() so I could see the test fail. If just returning the error it would still output the values read until then. So if you think that it belongs into the initial version of this test (maybe including some comment why we need an extra parseable value) I am happy to move it there. From what you wrote above, it sounds like we would expect git-config to write out some.value before failing on some.error. But that isn't what the test is checking, is it? It is checking that nothing is output. I do not think the output matters much either way when git-config has failed; if it returns a non-zero exit code, then the results of stdout cannot be trusted (after all, it may have been killed by signal after it had written out half of the output). Still slightly puzzled... Well before my do not die patch there was no output to stdout since git already died during reading. Afterwards there would be output of values to stdout until the error occurred and then the error code would be returned. So my intend was to keep the output the same. Since the blob reading is new I thinks its fine either way. So currently I am thinking about just removing the whole stop to write anything on error part of this patch and just return the error. What do you think? Cheers Heiko -- To unsubscribe from this list: send the line 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 log --cherry with disjoint roots
I use my own integration branch manager[1] to manage my WIP changes to various projects, including git.git and one of the features of this is a --status option that shows whether anything that I'm tracking has been merged to the base branch I'm building on top of. Since the commit IDs will be different after being sent to the list, this uses the --cherry option to git-log. Today I realised that I wasn't tracking a git-gui change I sent to the list a couple of weeks ago [2] and so I pulled that in and added it. Getting the status for this is significantly slower than anything else because it does this: git log --cherry --oneline origin/master...git-gui-fix-subdir and although there are only 5 commits in git-gui-fix-subdir not in master there are 31964 commits in master not in git-gui-fix-subdir and --cherry seems to inspect all of those. So I get: $ time git log --oneline --cherry master...git-gui-fix-subdir \ /dev/null real0m41.378s user0m40.248s sys 0m0.986s Now I know that I don't need to check anything older than commit 8ead1bf (Merge tag 'gitgui-0.17.0' of git://repo.or.cz/git-gui, 2012-10-17) and if I tell Git that it gets significantly better: $ time git log --oneline --cherry master...git-gui-fix-subdir \ --not 8ead1bfe111085ef1ad7759e67340f074996b244 /dev/null real0m2.163s user0m2.070s sys 0m0.084s I'd like a way to automatically find the last merge that pulled in a subtree so that my script can construct the above command line without me needing to tell it the correct SHA-1 every time I have a branch that is affected by this. I guess this boils down to having a way to ask Git to list merges that have a parent in a specified range, but perhaps I'm missing an easier solution to this. I also wonder if it would be interesting to cache patch IDs rather than generating them every time... [1] http://johnkeeping.github.io/git-integration/ [2] http://article.gmane.org/gmane.comp.version-control.git/222646 -- To unsubscribe from this list: send the line 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: Outdated and broken online versions of user-manual.html
From: Thomas Ackermann th.ac...@arcor.de Sent: Saturday, May 11, 2013 8:48 AM W. Trevor King wking at tremily.us writes: I'm also surprised that I couldn't find a more obvious link to the manual from git-scm.com (I ended up taking a “See Also” link from gittutorial(7) [3]). I'm not sure if this is intentional or not, since git-scm.com does prominently link Pro Git, and that overlaps fairly significantly with the manual. Folks with Git installed will generally have man pages, so it's not a big deal, but having current docs somewhere online to link against would be nice. I'm also curious if I should be linking against a particular location. IMHO user-manual is a natural step for a Git beginner after reading one of the books like Pro Git and before he is ready to digest the man pages. But up to now there are several problems with user-manual besides the problems described by Trevor: (1) Very poor html formatting (document type book causes ugly TOCs per section and there's a Part I without a Part II) (2) Partly outdated content (3) Sub-optimal structuring (to-do list as part of the document, glossary not at the end of the document) (4) User-manual.PDF uses an independent tool chain which makes it harder to do improvements for user-manual.html and also is the only pdf doc we are creating. IMHO we should remove this altogether. (5) Large overlapping with the tutorials. IMHO all of the tutorials should be blended into user-manual I am currently working on (1)-(4) and then aiming for (5). Comments are welcome ... --- Thomas I too had noticed that both the User-Manual and git Everyday are not formatted in a manner that allows them to be accessed via 'git help'. I recently added a '--guides' option to 'git help' (v1.8.2-377-g73903d0) which will show the common guides but was frustrated that I couldn't include either the user-manual or everday in the list. I'd welcome the provision of a 'man'/'html' formatted version of the two guides. Are you expecting to make then compatible with the man format? I would be a little cautious of your point 5 if it squoze everything into one overlong document at the expense of losing the shorter documents - one can't eat a whole melon in one bite ;-) That said there is a significant opportunity for rationalisation and clarity improvement, even if it is only a proper realisation of where we deliberately duplicate information for ease of user learning. I also liked the idea of all the documenation being collated into one Humongous pdf as a reference (there was a patch a while back - don't have a reference right now). Philip -- To unsubscribe from this list: send the line 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] t4300 (rebase): don't unnecessarily set GIT_TRACE
Junio C Hamano wrote: But the output from passing -v before the test that breaks is not very useful for two reasons. I sometimes checkout the Good branch in a different worktree, compare the output/ state of the passing test with the failing one. I've never really found the outputs from earlier tests enlightening. From my experience, the failure is often due to an earlier test not imposing tighter passing conditions: but because it's shell, the debugging time is very small. I always just patch-locally and run. I'm not sure how to make the testing framework more useful. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 0/2] merge-base: add --merge-child option
This is helpful when examining branches with disjoint roots, for example because one is periodically merged into a subtree of the other. With the --merge-child option, git merge-base will print a first-parent ancestor of the first revision given, where the commit printed is either a merge-base of the supplied revisions or a merge for which one of its parents (not the first) is a merge-base. For example, given the history: A---C---G \ B-D---F \ E we have: $ git merge-base F E B $ git merge-base --merge-child F E D $ git merge-base F G C $ git merge-base --merge-child F G C $ git log --left-right F...E F D C A E $ git log --left-right F...E --not $(git merge-base --merge-child F E) F E The git-log case is useful because it allows us to limit the range of commits that we are examining for patch-identical changes when using --cherry. For example with git-gui in git.git I know that anything before the last merge of git-gui is not interesting: $ time git log --cherry master...git-gui/master /dev/null real0m32.731s user0m31.956s sys 0m0.664s $ time git log --cherry master...git-gui/master --not \ $(git merge-base --merge-child master git-gui/master) \ /dev/null real0m2.296s user0m2.193s sys 0m0.092s The first commit is a small prerequisite to extract a useful function from builtin/tag.c to commit.c. The second is the main change (the commit message is identical to the text before this paragraph). I'm not convinced that '--merge-child' is the right name for this but I think the functionality itself is useful. John Keeping (2): commit: add commit_list_contains function merge-base: add --merge-child option Documentation/git-merge-base.txt | 6 builtin/merge-base.c | 61 ++-- builtin/tag.c| 10 +-- commit.c | 8 ++ commit.h | 1 + t/t6010-merge-base.sh| 25 ++-- 6 files changed, 98 insertions(+), 13 deletions(-) -- 1.8.3.rc1.289.gcb3647f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 1/2] commit: add commit_list_contains function
This is the same as the in_commit_list function already in builtin/tag.c so we also replace that by the new commit_list_contains function. Signed-off-by: John Keeping j...@keeping.me.uk --- builtin/tag.c | 10 +- commit.c | 8 commit.h | 1 + 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index af3af3f..1c24772 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -65,14 +65,6 @@ static const unsigned char *match_points_at(const char *refname, return NULL; } -static int in_commit_list(const struct commit_list *want, struct commit *c) -{ - for (; want; want = want-next) - if (!hashcmp(want-item-object.sha1, c-object.sha1)) - return 1; - return 0; -} - static int contains_recurse(struct commit *candidate, const struct commit_list *want) { @@ -85,7 +77,7 @@ static int contains_recurse(struct commit *candidate, if (candidate-object.flags UNINTERESTING) return 0; /* or are we it? */ - if (in_commit_list(want, candidate)) + if (commit_list_contains(want, candidate)) return 1; if (parse_commit(candidate) 0) diff --git a/commit.c b/commit.c index 888e02a..a8263c3 100644 --- a/commit.c +++ b/commit.c @@ -420,6 +420,14 @@ void commit_list_sort_by_date(struct commit_list **list) commit_list_compare_by_date); } +int commit_list_contains(const struct commit_list *list, const struct commit *item) +{ + for (; list; list = list-next) + if (!hashcmp(list-item-object.sha1, item-object.sha1)) + return 1; + return 0; +} + struct commit *pop_most_recent_commit(struct commit_list **list, unsigned int mark) { diff --git a/commit.h b/commit.h index 67bd509..d686ea8 100644 --- a/commit.h +++ b/commit.h @@ -60,6 +60,7 @@ unsigned commit_list_count(const struct commit_list *l); struct commit_list *commit_list_insert_by_date(struct commit *item, struct commit_list **list); void commit_list_sort_by_date(struct commit_list **list); +int commit_list_contains(const struct commit_list *list, const struct commit *item); void free_commit_list(struct commit_list *list); -- 1.8.3.rc1.289.gcb3647f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 2/2] merge-base: add --merge-child option
This is helpful when examining branches with disjoint roots, for example because one is periodically merged into a subtree of the other. With the --merge-child option, git merge-base will print a first-parent ancestor of the first revision given, where the commit printed is either a merge-base of the supplied revisions or a merge for which one of its parents (not the first) is a merge-base. For example, given the history: A---C---G \ B-D---F \ E we have: $ git merge-base F E B $ git merge-base --merge-child F E D $ git merge-base F G C $ git merge-base --merge-child F G C $ git log --left-right F...E F D C A E $ git log --left-right F...E --not $(git merge-base --merge-child F E) F E The git-log case is useful because it allows us to limit the range of commits that we are examining for patch-identical changes when using --cherry. For example with git-gui in git.git I know that anything before the last merge of git-gui is not interesting: $ time git log --cherry master...git-gui/master /dev/null real0m32.731s user0m31.956s sys 0m0.664s $ time git log --cherry master...git-gui/master --not \ $(git merge-base --merge-child master git-gui/master) \ /dev/null real0m2.296s user0m2.193s sys 0m0.092s Signed-off-by: John Keeping j...@keeping.me.uk --- Documentation/git-merge-base.txt | 6 builtin/merge-base.c | 61 ++-- t/t6010-merge-base.sh| 25 ++-- 3 files changed, 88 insertions(+), 4 deletions(-) diff --git a/Documentation/git-merge-base.txt b/Documentation/git-merge-base.txt index 87842e3..a1404e1 100644 --- a/Documentation/git-merge-base.txt +++ b/Documentation/git-merge-base.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git merge-base' [-a|--all] commit commit... +'git merge-base' [-a|--all] --merge-child commit... 'git merge-base' [-a|--all] --octopus commit... 'git merge-base' --is-ancestor commit commit 'git merge-base' --independent commit... @@ -39,6 +40,11 @@ As a consequence, the 'merge base' is not necessarily contained in each of the commit arguments if more than two commits are specified. This is different from linkgit:git-show-branch[1] when used with the `--merge-base` option. +--merge-child:: + Find the first-parent ancestor of the first commit that is either + reachable from all of the supplied commits or which has a parent that + is. + --octopus:: Compute the best common ancestors of all supplied commits, in preparation for an n-way merge. This mimics the behavior diff --git a/builtin/merge-base.c b/builtin/merge-base.c index 1bc7991..0bf9f6d 100644 --- a/builtin/merge-base.c +++ b/builtin/merge-base.c @@ -1,7 +1,9 @@ #include builtin.h #include cache.h #include commit.h +#include diff.h #include parse-options.h +#include revision.h static int show_merge_base(struct commit **rev, int rev_nr, int show_all) { @@ -24,12 +26,61 @@ static int show_merge_base(struct commit **rev, int rev_nr, int show_all) static const char * const merge_base_usage[] = { N_(git merge-base [-a|--all] commit commit...), + N_(git merge-base [-a|--all] --merge-child commit...), N_(git merge-base [-a|--all] --octopus commit...), N_(git merge-base --independent commit...), N_(git merge-base --is-ancestor commit commit), NULL }; +static int handle_merge_child(struct commit **rev, int rev_nr, const char *prefix, int show_all) +{ + struct commit_list *merge_bases; + struct rev_info revs; + struct commit *commit; + + merge_bases = get_merge_bases_many(rev[0], rev_nr - 1, rev + 1, 0); + + if (!merge_bases) + return 1; + + init_revisions(revs, prefix); + revs.first_parent_only = 1; + + clear_commit_marks(rev[0], SEEN | UNINTERESTING | SHOWN | ADDED); + add_pending_object(revs, rev[0]-object, rev0); + if (prepare_revision_walk(revs)) + die(_(revision walk setup failed)); + + while ((commit = get_revision(revs)) != NULL) { + /* +* If a merge base is a first-parent ancestor of rev[0] then +* we print the commit itself instead of a merge which is its +* child. +*/ + if (commit_list_contains(merge_bases, commit)) { + printf(%s\n, sha1_to_hex(commit-object.sha1)); + if (!show_all) + return 0; + } + + struct commit_list *parent; + for (parent = commit-parents; parent; parent = parent-next) { + /* Skip the
Cannot push anything via export transport helper after push fails.
I noticed that using git-remote-bzr, but as far as I can tell this is generic for all transport helpers using fast-export. What happened was git push failed due to merge conflict. So far so good - but from now on git assumes everything is up to date. bor@opensuse:/tmp/test/git git push origin master To bzr::bzr+ssh://bor@localhost/tmp/test/bzr ! [rejected]master - master (non-fast-forward) error: failed to push some refs to 'bzr::bzr+ssh://bor@localhost/tmp/test/bzr' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Merge the remote changes (e.g. 'git pull') hint: before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details. bor@opensuse:/tmp/test/git git push origin master Everything up-to-date bor@opensuse:/tmp/test/git The problem seems to be that git fast-export updates marks unconditionally, whether export actually applied or not. So next time it assumes everything is already exported and does nothing. Is it expected behavior? TIA -andrey -- To unsubscribe from this list: send the line 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: Cannot push anything via export transport helper after push fails.
On Sat, May 11, 2013 at 04:29:36PM +0400, Andrey Borzenkov wrote: I noticed that using git-remote-bzr, but as far as I can tell this is generic for all transport helpers using fast-export. What happened was git push failed due to merge conflict. So far so good - but from now on git assumes everything is up to date. bor@opensuse:/tmp/test/git git push origin master To bzr::bzr+ssh://bor@localhost/tmp/test/bzr ! [rejected]master - master (non-fast-forward) error: failed to push some refs to 'bzr::bzr+ssh://bor@localhost/tmp/test/bzr' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Merge the remote changes (e.g. 'git pull') hint: before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details. bor@opensuse:/tmp/test/git git push origin master Everything up-to-date bor@opensuse:/tmp/test/git The problem seems to be that git fast-export updates marks unconditionally, whether export actually applied or not. So next time it assumes everything is already exported and does nothing. Is it expected behavior? What version of Git are you using? This sounds similar to the regression fixed by commit 126aac5 (transport-helper: fix remote helper namespace regression, 2013-05-10) but that was only introduced in commit 664059f (transport-helper: update remote helper namespace, 2013-04-17) which isn't in any released versions of Git. -- To unsubscribe from this list: send the line 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 v4 0/5] allow more sources for config values
Hi, all the comments to the last iteration[1] incorporated. You can also find this on my github[2]. This is only for review. I will resubmit this once 1.8.3 is out. Cheers Heiko [1] http://article.gmane.org/gmane.comp.version-control.git/223743 [2] https://github.com/hvoigt/git/commits/hv/strbuf_config_parsing-series4 Heiko Voigt (5): config: factor out config file stack management config: drop cf validity check in get_next_char() config: make parsing stack struct independent from actual data source teach config --blob option to parse config from database do not die when error in config parsing of buf occurs builtin/config.c | 31 ++- cache.h| 6 +- config.c | 217 ++--- t/t1307-config-blob.sh | 70 4 files changed, 271 insertions(+), 53 deletions(-) create mode 100755 t/t1307-config-blob.sh -- 1.8.3.rc1.53.g0126843 -- To unsubscribe from this list: send the line 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 v4 1/5] config: factor out config file stack management
Because a config callback may start parsing a new file, the global context regarding the current config file is stored as a stack. Currently we only need to manage that stack from git_config_from_file. Let's factor it out to allow new sources of config data. Signed-off-by: Heiko Voigt hvo...@hvoigt.net --- config.c | 42 -- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/config.c b/config.c index aefd80b..f0494f3 100644 --- a/config.c +++ b/config.c @@ -896,6 +896,32 @@ int git_default_config(const char *var, const char *value, void *dummy) return 0; } +/* + * The fields f and name of top need to be initialized before calling + * this function. + */ +static int do_config_from(struct config_file *top, config_fn_t fn, void *data) +{ + int ret; + + /* push config-file parsing state stack */ + top-prev = cf; + top-linenr = 1; + top-eof = 0; + strbuf_init(top-value, 1024); + strbuf_init(top-var, 1024); + cf = top; + + ret = git_parse_file(fn, data); + + /* pop config-file parsing state stack */ + strbuf_release(top-value); + strbuf_release(top-var); + cf = top-prev; + + return ret; +} + int git_config_from_file(config_fn_t fn, const char *filename, void *data) { int ret; @@ -905,22 +931,10 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data) if (f) { config_file top; - /* push config-file parsing state stack */ - top.prev = cf; top.f = f; top.name = filename; - top.linenr = 1; - top.eof = 0; - strbuf_init(top.value, 1024); - strbuf_init(top.var, 1024); - cf = top; - - ret = git_parse_file(fn, data); - - /* pop config-file parsing state stack */ - strbuf_release(top.value); - strbuf_release(top.var); - cf = top.prev; + + ret = do_config_from(top, fn, data); fclose(f); } -- 1.8.3.rc1.53.g0126843 -- To unsubscribe from this list: send the line 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 v4 2/5] config: drop cf validity check in get_next_char()
The global variable cf is set with an initialized value in all codepaths before calling this function. The complete call graph looks like this: git_config_from_file - do_config_from - git_parse_file - get_next_char - get_value - get_next_char - parse_value - get_next_char - get_base_var - get_next_char - get_extended_base_var - get_next_char The variable is initialized in do_config_from. Signed-off-by: Heiko Voigt hvo...@hvoigt.net --- config.c | 29 + 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/config.c b/config.c index f0494f3..046642b 100644 --- a/config.c +++ b/config.c @@ -169,26 +169,23 @@ int git_config_from_parameters(config_fn_t fn, void *data) static int get_next_char(void) { int c; - FILE *f; + FILE *f = cf-f; - c = '\n'; - if (cf ((f = cf-f) != NULL)) { + c = fgetc(f); + if (c == '\r') { + /* DOS like systems */ c = fgetc(f); - if (c == '\r') { - /* DOS like systems */ - c = fgetc(f); - if (c != '\n') { - ungetc(c, f); - c = '\r'; - } - } - if (c == '\n') - cf-linenr++; - if (c == EOF) { - cf-eof = 1; - c = '\n'; + if (c != '\n') { + ungetc(c, f); + c = '\r'; } } + if (c == '\n') + cf-linenr++; + if (c == EOF) { + cf-eof = 1; + c = '\n'; + } return c; } -- 1.8.3.rc1.53.g0126843 -- To unsubscribe from this list: send the line 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 v4 3/5] config: make parsing stack struct independent from actual data source
To simplify adding other sources we extract all functions needed for parsing into a list of callbacks. We implement those callbacks for the current file parsing. A new source can implement its own set of callbacks. Instead of storing the concrete FILE pointer for parsing we store a void pointer. A new source can use this to store its custom data. Signed-off-by: Heiko Voigt hvo...@hvoigt.net --- config.c | 66 ++-- 1 file changed, 44 insertions(+), 22 deletions(-) diff --git a/config.c b/config.c index 046642b..e21c24c 100644 --- a/config.c +++ b/config.c @@ -10,20 +10,41 @@ #include strbuf.h #include quote.h -typedef struct config_file { - struct config_file *prev; - FILE *f; +struct config_source { + struct config_source *prev; + union { + FILE *file; + } u; const char *name; int linenr; int eof; struct strbuf value; struct strbuf var; -} config_file; -static config_file *cf; + int (*fgetc)(struct config_source *c); + int (*ungetc)(int c, struct config_source *conf); + long (*ftell)(struct config_source *c); +}; + +static struct config_source *cf; static int zlib_compression_seen; +static int config_file_fgetc(struct config_source *conf) +{ + return fgetc(conf-u.file); +} + +static int config_file_ungetc(int c, struct config_source *conf) +{ + return ungetc(c, conf-u.file); +} + +static long config_file_ftell(struct config_source *conf) +{ + return ftell(conf-u.file); +} + #define MAX_INCLUDE_DEPTH 10 static const char include_depth_advice[] = exceeded maximum include depth (%d) while including\n @@ -168,15 +189,13 @@ int git_config_from_parameters(config_fn_t fn, void *data) static int get_next_char(void) { - int c; - FILE *f = cf-f; + int c = cf-fgetc(cf); - c = fgetc(f); if (c == '\r') { /* DOS like systems */ - c = fgetc(f); + c = cf-fgetc(cf); if (c != '\n') { - ungetc(c, f); + cf-ungetc(c, cf); c = '\r'; } } @@ -336,7 +355,7 @@ static int get_base_var(struct strbuf *name) } } -static int git_parse_file(config_fn_t fn, void *data) +static int git_parse_source(config_fn_t fn, void *data) { int comment = 0; int baselen = 0; @@ -894,10 +913,11 @@ int git_default_config(const char *var, const char *value, void *dummy) } /* - * The fields f and name of top need to be initialized before calling + * All source specific fields in the union, name and the callbacks + * fgetc, ungetc, ftell of top need to be initialized before calling * this function. */ -static int do_config_from(struct config_file *top, config_fn_t fn, void *data) +static int do_config_from_source(struct config_source *top, config_fn_t fn, void *data) { int ret; @@ -909,7 +929,7 @@ static int do_config_from(struct config_file *top, config_fn_t fn, void *data) strbuf_init(top-var, 1024); cf = top; - ret = git_parse_file(fn, data); + ret = git_parse_source(fn, data); /* pop config-file parsing state stack */ strbuf_release(top-value); @@ -926,12 +946,15 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data) ret = -1; if (f) { - config_file top; + struct config_source top; - top.f = f; + top.u.file = f; top.name = filename; + top.fgetc = config_file_fgetc; + top.ungetc = config_file_ungetc; + top.ftell = config_file_ftell; - ret = do_config_from(top, fn, data); + ret = do_config_from_source(top, fn, data); fclose(f); } @@ -1064,7 +1087,6 @@ static int store_aux(const char *key, const char *value, void *cb) { const char *ep; size_t section_len; - FILE *f = cf-f; switch (store.state) { case KEY_SEEN: @@ -1076,7 +1098,7 @@ static int store_aux(const char *key, const char *value, void *cb) return 1; } - store.offset[store.seen] = ftell(f); + store.offset[store.seen] = cf-ftell(cf); store.seen++; } break; @@ -1103,19 +1125,19 @@ static int store_aux(const char *key, const char *value, void *cb) * Do not increment matches: this is no match, but we * just made sure we are in the desired section. */ - store.offset[store.seen] = ftell(f); + store.offset[store.seen] = cf-ftell(cf); /* fallthru */ case SECTION_END_SEEN: case START: if
[PATCH v4 4/5] teach config --blob option to parse config from database
This can be used to read configuration values directly from git's database. For example it is useful for reading to be checked out .gitmodules files directly from the database. Signed-off-by: Heiko Voigt hvo...@hvoigt.net --- builtin/config.c | 31 +++--- cache.h| 6 +++- config.c | 86 -- t/t1307-config-blob.sh | 70 4 files changed, 186 insertions(+), 7 deletions(-) create mode 100755 t/t1307-config-blob.sh diff --git a/builtin/config.c b/builtin/config.c index 33c9bf9..8d01b7a 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -21,6 +21,7 @@ static char term = '\n'; static int use_global_config, use_system_config, use_local_config; static const char *given_config_file; +static const char *given_config_blob; static int actions, types; static const char *get_color_slot, *get_colorbool_slot; static int end_null; @@ -53,6 +54,7 @@ static struct option builtin_config_options[] = { OPT_BOOLEAN(0, system, use_system_config, N_(use system config file)), OPT_BOOLEAN(0, local, use_local_config, N_(use repository config file)), OPT_STRING('f', file, given_config_file, N_(file), N_(use given config file)), + OPT_STRING(0, blob, given_config_blob, N_(blob-id), N_(read config from given blob object)), OPT_GROUP(N_(Action)), OPT_BIT(0, get, actions, N_(get value: name [value-regex]), ACTION_GET), OPT_BIT(0, get-all, actions, N_(get all values: key [value-regex]), ACTION_GET_ALL), @@ -218,7 +220,8 @@ static int get_value(const char *key_, const char *regex_) } git_config_with_options(collect_config, values, - given_config_file, respect_includes); + given_config_file, given_config_blob, + respect_includes); ret = !values.nr; @@ -302,7 +305,8 @@ static void get_color(const char *def_color) get_color_found = 0; parsed_color[0] = '\0'; git_config_with_options(git_get_color_config, NULL, - given_config_file, respect_includes); + given_config_file, given_config_blob, + respect_includes); if (!get_color_found def_color) color_parse(def_color, command line, parsed_color); @@ -330,7 +334,8 @@ static int get_colorbool(int print) get_colorbool_found = -1; get_diff_color_found = -1; git_config_with_options(git_get_colorbool_config, NULL, - given_config_file, respect_includes); + given_config_file, given_config_blob, + respect_includes); if (get_colorbool_found 0) { if (!strcmp(get_colorbool_slot, color.diff)) @@ -348,6 +353,12 @@ static int get_colorbool(int print) return get_colorbool_found ? 0 : 1; } +static void check_blob_write(void) +{ + if (given_config_blob) + die(writing config blobs is not supported); +} + int cmd_config(int argc, const char **argv, const char *prefix) { int nongit = !startup_info-have_repository; @@ -359,7 +370,8 @@ int cmd_config(int argc, const char **argv, const char *prefix) builtin_config_usage, PARSE_OPT_STOP_AT_NON_OPTION); - if (use_global_config + use_system_config + use_local_config + !!given_config_file 1) { + if (use_global_config + use_system_config + use_local_config + + !!given_config_file + !!given_config_blob 1) { error(only one config file at a time.); usage_with_options(builtin_config_usage, builtin_config_options); } @@ -438,6 +450,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) check_argc(argc, 0, 0); if (git_config_with_options(show_all_config, NULL, given_config_file, + given_config_blob, respect_includes) 0) { if (given_config_file) die_errno(unable to read config file '%s', @@ -450,6 +463,8 @@ int cmd_config(int argc, const char **argv, const char *prefix) check_argc(argc, 0, 0); if (!given_config_file nongit) die(not in a git directory); + if (given_config_blob) + die(editing blobs is not supported); git_config(git_default_config, NULL); launch_editor(given_config_file ? given_config_file : git_path(config), @@ -457,6 +472,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) }
[PATCH v4 5/5] do not die when error in config parsing of buf occurs
If a config parsing error in a file occurs we can die and let the user fix the issue. This is different for the buf parsing function since it can be used to parse blobs of .gitmodules files. If a parsing error occurs here we should proceed since otherwise a database containing such an error in a single revision could be rendered unusable. Signed-off-by: Heiko Voigt hvo...@hvoigt.net --- config.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/config.c b/config.c index 23b14f4..a8d3dcf 100644 --- a/config.c +++ b/config.c @@ -21,6 +21,7 @@ struct config_source { } buf; } u; const char *name; + int die_on_error; int linenr; int eof; struct strbuf value; @@ -442,7 +443,10 @@ static int git_parse_source(config_fn_t fn, void *data) if (get_value(fn, data, var) 0) break; } - die(bad config file line %d in %s, cf-linenr, cf-name); + if (cf-die_on_error) + die(bad config file line %d in %s, cf-linenr, cf-name); + else + return error(bad config file line %d in %s, cf-linenr, cf-name); } static int parse_unit_factor(const char *end, uintmax_t *val) @@ -940,7 +944,7 @@ int git_default_config(const char *var, const char *value, void *dummy) } /* - * All source specific fields in the union, name and the callbacks + * All source specific fields in the union, die_on_error, name and the callbacks * fgetc, ungetc, ftell of top need to be initialized before calling * this function. */ @@ -977,6 +981,7 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data) top.u.file = f; top.name = filename; + top.die_on_error = 1; top.fgetc = config_file_fgetc; top.ungetc = config_file_ungetc; top.ftell = config_file_ftell; @@ -997,6 +1002,7 @@ int git_config_from_buf(config_fn_t fn, const char *name, const char *buf, top.u.buf.len = len; top.u.buf.pos = 0; top.name = name; + top.die_on_error = 0; top.fgetc = config_buf_fgetc; top.ungetc = config_buf_ungetc; top.ftell = config_buf_ftell; -- 1.8.3.rc1.53.g0126843 -- To unsubscribe from this list: send the line 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] test-bzr: Do not use unportable sed \+
Using sed -e '/[0-9]\+//' to find one ore more digits is not portable. Use the Basic Regular Expression '/[0-9][0-9]*//' instead Signed-off-by: Torsten Bögershausen tbo...@web.de --- contrib/remote-helpers/test-bzr.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh index d9c32f4..5dfa070 100755 --- a/contrib/remote-helpers/test-bzr.sh +++ b/contrib/remote-helpers/test-bzr.sh @@ -328,7 +328,7 @@ test_expect_success 'strip' ' echo four content bzr commit -m four - bzr log --line | sed -e s/^[0-9]\+: // ../expected + bzr log --line | sed -e s/^[0-9][0-9]*: // ../expected ) (cd gitrepo -- 1.8.2.1.614.g66d7af5 -- To unsubscribe from this list: send the line 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: Cannot push anything via export transport helper after push fails.
On Sat, May 11, 2013 at 7:29 AM, Andrey Borzenkov arvidj...@gmail.com wrote: I noticed that using git-remote-bzr, but as far as I can tell this is generic for all transport helpers using fast-export. What happened was git push failed due to merge conflict. So far so good - but from now on git assumes everything is up to date. bor@opensuse:/tmp/test/git git push origin master To bzr::bzr+ssh://bor@localhost/tmp/test/bzr ! [rejected]master - master (non-fast-forward) error: failed to push some refs to 'bzr::bzr+ssh://bor@localhost/tmp/test/bzr' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Merge the remote changes (e.g. 'git pull') hint: before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details. bor@opensuse:/tmp/test/git git push origin master Everything up-to-date bor@opensuse:/tmp/test/git The problem seems to be that git fast-export updates marks unconditionally, whether export actually applied or not. So next time it assumes everything is already exported and does nothing. Is it expected behavior? Indeed, this is the way it currently works, and it's not easy to fix. We would need some way to make fast-export wait until we know the exit status of the remote helper, and then tell it when it failed, so the marks are not updated. However, the way remote-bzr/hg work is that the commits are still there anyway. So if you merge the next time you push those commits are already converted, so it's not a problem if fast-export is not exporting them again. So even though it's not ideal, it should work. The problem is when the remote-helper crashes and the marks of fast-export and the remote-helper are out of sync, and then the user is really screwed. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Aw: Re: Outdated and broken online versions of user-manual.html
(1) Very poor html formatting (document type book causes ugly TOCs per section and there's a Part I without a Part II) (2) Partly outdated content (3) Sub-optimal structuring (to-do list as part of the document, glossary not at the end of the document) (4) User-manual.PDF uses an independent tool chain which makes it harder to do improvements for user-manual.html and also is the only pdf doc we are creating. IMHO we should remove this altogether. (5) Large overlapping with the tutorials. IMHO all of the tutorials should be blended into user-manual I'd welcome the provision of a 'man'/'html' formatted version of the two guides. Are you expecting to make then compatible with the man format? This is another topic on my to-do list ... I would be a little cautious of your point 5 if it squoze everything into one overlong document at the expense of losing the shorter documents - one can't eat a whole melon in one bite ;-) That said there is a significant opportunity for rationalisation and clarity improvement, even if it is only a proper realisation of where we deliberately duplicate information for ease of user learning. I did not have a closer look on this but just from reading the documents I got the impression that the tutorials are already more or less contained in the user-manual and by a little restructuring the melon will fall into handy pieces ;-) I also liked the idea of all the documenation being collated into one Humongous pdf as a reference (there was a patch a while back - don't have a reference right now). Thanks :-) My patches were rejected but (4) shows that now I know why ;-) Nevertheless I maintain this in https://github.com/tacker66/git.git and provide the documents for every major version of Git in https://github.com/tacker66/git-docpdf.git. (4) is also the reason why I stopped using the pdf target and instead create user-manual.pdf with wkhtmltopdf from user-manual.html. Additionally I use Amazons kindlegen to create mobi version from the html documents which look quite good on a Kindle. --- Thomas -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANN] git-tbdiff: topic branch diff
Hi, Spawned by discussion here at git-merge, I created a script that diffs the state of branches. You can grab it from https://github.com/trast/tbdiff.git The usage is git tbdiff A..B C..D Make sure to pass two ranges; it doesn't check that at all, and just hands it off to format-patch, so that a lone commit 'foo' means 'foo..HEAD' in the usual format-patch semantics. It does not use a longest-common subsequence approach like all other diffs, since the commits on the branch could have been reordered. Unlike in code, where only special cases of reorderings do not change the meaning[1], these reorderings frequently do not change the semantics of the series. I tried an approach that goes looking for a minimum cost bipartite matching, where the cost is given by the interdiff between the commits. This seems to be working nicely; for example, between v2 and v3 of the fc/remote-hg topic[2] it shows (in glorious color that cannot be represented here): $ git tbdiff origin/master~2..fc/remote-hg-2 origin/master..fc/remote-hg-3 1: 95bb1c2 = 1: 832f6a6 remote-hg: trivial test cleanups 2: ebff16c = 2: 1512332 remote-hg: make sure fake bookmarks are updated --: 3: 1584d8b remote-hg: fix bad state issue 3: 8db4546 = 4: 0d6054b remote-hg: redirect buggy mercurial output --: 5: ff9def9 remote-hg: fix bad file paths 5: 2406703 = 6: f467a2d remote-hg: refactor export 6: e3018d1 = 7: 70e4855 remote-hg: fix for files with spaces 7: a35adfb = 8: 7af557e remote-hg: update remote bookmarks --: 9: 99fe20c remote-hg: document location of stored hg repository 8: 4ec2382 = 10: d360d7c remote-hg: add missing config variable in doc 9: 1c94a96 = 11: 0c51226 remote-hg: trivial cleanups 4: fd646af = 12: e9f3e36 remote-hg: update tags globally 10: dc097ea = 13: 1ed339d remote-hg: properly report errors on bookmark pushes 12: d8a548c = 14: f887082 remote-hg: split bookmark handling --: 15: a9ac462 remote-hg: add basic author tests --: 16: c68bed9 remote-hg: add simple mail test --: 17: a350abe remote-hg: add 'insecure' option 11: 0870eb6 ! 18: 66e283d remote-hg: push to the appropriate branch @@ -11,7 +11,7 @@ +# Check if the ref is supposed to be a named branch +if ref.startswith('refs/heads/branches/'): -+extra['branch'] = ref.rpartition('/')[2] ++extra['branch'] = ref.replace('refs/heads/branches/', '') + if mode == 'hg': i = data.find('\n--HG--\n') 13: a583fa6 --: remote-hg: force remote push --: 19: ff4571d remote-hg: show more proper errors --: 20: 34b0b77 remote-hg: force remote push So you can clearly see that commit old-4 moved to position 12, commit 3 is new, etc. The failure to match up old-13 with new-20 is not a bug; the two are so different that the heuristics split it into a creation/deletion. Another use-case is when Junio picked up a series that you submitted. Comparing Peff's jk/packed-refs-race on github with what was actually merged into git.git shows $ git tbdiff origin/next..peff/jk/packed-refs-race origin/next..7c95033 1: eb4f175 ! 1: ed485e2 resolve_ref: close race condition for packed refs @@ -25,6 +25,9 @@ resolve_ref_unsafe. In the situation described above, we may still be depending on a cached view of the packed-refs file; that race will be dealt with in a future patch. + +Signed-off-by: Jeff King p...@peff.net +Signed-off-by: Junio C Hamano gits...@pobox.com diff --git a/refs.c b/refs.c --- a/refs.c +++ b/refs.c 2: d9d9e0f ! 2: 40aba32 add a stat_validity struct @@ -13,6 +13,9 @@ reuse the logic about which stat entries to trust for a particular platform, but hides the complexity behind two simple functions: check and update. + +Signed-off-by: Jeff King p...@peff.net +Signed-off-by: Junio C Hamano gits...@pobox.com diff --git a/cache.h b/cache.h --- a/cache.h +++ b/cache.h 3: 4e566d4 ! 3: 7c95033 for_each_ref: load all loose refs before packed refs @@ -40,6 +40,17 @@ from that tip (the same thing can happen if repack -ad is used, as it simply drops unreachable objects that are packed). + +This patch solves it by loading all of the loose refs for +our traversal into our in-memory cache, and then refreshing +the packed-refs cache. Because a pack-refs writer will +always put the new packed-refs file into place before +starting the prune, we know that any loose refs we fail to +see will either truly be missing, or will have already been +put in the packed-refs file by the time we refresh. + +Signed-off-by: Jeff King p...@peff.net +Signed-off-by: Junio C Hamano gits...@pobox.com
Re: [ANN] git-tbdiff: topic branch diff
Thomas Rast tr...@inf.ethz.ch writes: Hi, Spawned by discussion here at git-merge, I created a script that diffs the state of branches. You can grab it from https://github.com/trast/tbdiff.git The usage is git tbdiff A..B C..D BTW, I should've mentioned this here too (it's in the README shown on github): it currently depends on the 'hungarian' module for the matching algorithm, found here: https://pypi.python.org/pypi/hungarian It's easy to install, but in the interests of wider usability I'll probably write the same in pure python eventually. -- Thomas Rast trast@{inf,student}.ethz.ch -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 0/4] update tracking refs on explicit fetch
This is a cleaned-up version of the oft-discussed[1] concept that git pull origin master should update refs/remotes/origin/master. It is a little bit of a risky change, in that anybody who deeply cares about when their tracking ref branches are updated would be impacted. But I think the general consensus is that while you can come up with a hypothetical use case, the logic is quite tortured, and you could accomplish the same thing by keeping a local branch. Most people seem to have the mental model that remote tracking branches are basically a cache for what's on the remote, updated opportunistically as appropriate. [1/4]: t5510: start tracking-ref tests from a known state [2/4]: fetch/pull doc: untangle meaning of bare ref [3/4]: refactor ref-merge flag [4/4]: fetch: opportunistically update tracking refs The final patch is the moral equivalent of the patch I've posted before when this topic comes up (and referenced in the threads below). But that patch did not take care not to write duplicate (mergable!) entries into FETCH_HEAD, leading to great confusion from git-pull. This series deals with that, and takes care to write the exact same FETCH_HEAD we would have written before the series. However, Jonathan raised an interesting question: what is the point of not-for-merge entries at all? We are not going to merge them, so none of the pull/merge machinery cares about them. We do not consider extra entries in FETCH_HEAD for reachability or anything like that. Is there anybody who reads them? If we do not care about them, we can simplify this much further, and either write the duplicates as not-for-merge, or even omit not-for-merge entries entirely. However, it is possible that some third-party scripts process FETCH_HEAD, so I took the conservative route. Note that this patch is as greedy as possible[2]; whenever we see the LHS is fetched, we update the RHS side, even if the user did: git fetch origin master:foobar Some of the discussions below argue that the behavior should not kick in for such a case. I am not sure I agree (and argue against it in those discussions). I think at this point that is the only potentially contentious item. -Peff [1] Past discussions: http://thread.gmane.org/gmane.comp.version-control.git/127163/focus=127215 http://thread.gmane.org/gmane.comp.version-control.git/192252 http://thread.gmane.org/gmane.comp.version-control.git/203357/focus=203442 http://article.gmane.org/gmane.comp.version-control.git/221234 -- To unsubscribe from this list: send the line 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/4] t5510: start tracking-ref tests from a known state
We have three sequential tests for for whether tracking refs are updated by various fetches and pulls; the first two should not update the ref, and the third should. Each test depends on the state left by the test before. This is fragile (a failing early test will confuse later tests), and means we cannot add more should update tests after the third one. Let's instead save the initial state before these tests, and then reset to a known state before running each test. Signed-off-by: Jeff King p...@peff.net --- t/t5510-fetch.sh | 10 ++ 1 file changed, 10 insertions(+) diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index d7a19a1..789c228 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -370,12 +370,20 @@ test_expect_success 'explicit fetch should not update tracking' ' ' +test_expect_success 'mark initial state of origin/master' ' + ( + cd three + git tag base-origin-master refs/remotes/origin/master + ) +' + test_expect_success 'explicit fetch should not update tracking' ' cd $D git branch -f side ( cd three + git update-ref refs/remotes/origin/master base-origin-master o=$(git rev-parse --verify refs/remotes/origin/master) git fetch origin master n=$(git rev-parse --verify refs/remotes/origin/master) @@ -390,6 +398,7 @@ test_expect_success 'explicit pull should not update tracking' ' git branch -f side ( cd three + git update-ref refs/remotes/origin/master base-origin-master o=$(git rev-parse --verify refs/remotes/origin/master) git pull origin master n=$(git rev-parse --verify refs/remotes/origin/master) @@ -404,6 +413,7 @@ test_expect_success 'configured fetch updates tracking' ' git branch -f side ( cd three + git update-ref refs/remotes/origin/master base-origin-master o=$(git rev-parse --verify refs/remotes/origin/master) git fetch origin n=$(git rev-parse --verify refs/remotes/origin/master) -- 1.8.3.rc1.2.g12db477 -- To unsubscribe from this list: send the line 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/4] fetch/pull doc: untangle meaning of bare ref
From: Thomas Rast tr...@inf.ethz.ch The documentation erroneously used the same wording for both fetch and pull, stating that something will be merged even in git-fetch(1). In addition, saying that ref is equivalent to ref: doesn't really help anyone who still needs to read manpages. Clarify what is actually going on. Signed-off-by: Thomas Rast tr...@inf.ethz.ch Signed-off-by: Jeff King p...@peff.net --- Documentation/pull-fetch-param.txt | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Documentation/pull-fetch-param.txt b/Documentation/pull-fetch-param.txt index 94a9d32..6f5ca21 100644 --- a/Documentation/pull-fetch-param.txt +++ b/Documentation/pull-fetch-param.txt @@ -68,6 +68,11 @@ Some short-cut notations are also supported. + * `tag tag` means the same as `refs/tags/tag:refs/tags/tag`; it requests fetching everything up to the given tag. -* A parameter ref without a colon is equivalent to - ref: when pulling/fetching, so it merges ref into the current - branch without storing the remote branch anywhere locally +ifndef::git-pull[] +* A parameter ref without a colon fetches that ref into FETCH_HEAD, +endif::git-pull[] +ifdef::git-pull[] +* A parameter ref without a colon merges ref into the current + branch, +endif::git-pull[] + while not storing the branch anywhere locally. -- 1.8.3.rc1.2.g12db477 -- To unsubscribe from this list: send the line 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/4] refactor ref-merge flag
Each struct ref has a boolean flag that is set by the fetch code to determine whether the ref should be marked as not-for-merge or not when we write it out to FETCH_HEAD. It would be useful to turn this boolean into a tri-state, with the third state meaning do not bother writing it out to FETCH_HEAD at all. That would let us add extra refs to the set of refs to be stored (e.g., to store copies of things we fetched) without impacting FETCH_HEAD. This patch turns it into an enum that covers the tri-state case, and hopefully makes the code more explicit and easier to read. Signed-off-by: Jeff King p...@peff.net --- builtin/fetch.c | 57 +++-- cache.h | 14 +- 2 files changed, 48 insertions(+), 23 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 4b6b1df..287cf4c 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -119,7 +119,7 @@ static void add_merge_config(struct ref **head, for (rm = *head; rm; rm = rm-next) { if (branch_merge_matches(branch, i, rm-name)) { - rm-merge = 1; + rm-fetch_head_status = FETCH_HEAD_MERGE; break; } } @@ -140,7 +140,7 @@ static void add_merge_config(struct ref **head, refspec.src = branch-merge[i]-src; get_fetch_map(remote_refs, refspec, tail, 1); for (rm = *old_tail; rm; rm = rm-next) - rm-merge = 1; + rm-fetch_head_status = FETCH_HEAD_MERGE; } } @@ -167,7 +167,7 @@ static struct ref *get_ref_map(struct transport *transport, } /* Merge everything on the command line, but not --tags */ for (rm = ref_map; rm; rm = rm-next) - rm-merge = 1; + rm-fetch_head_status = FETCH_HEAD_MERGE; if (tags == TAGS_SET) get_fetch_map(remote_refs, tag_refspec, tail, 0); } else { @@ -186,7 +186,7 @@ static struct ref *get_ref_map(struct transport *transport, *autotags = 1; if (!i !has_merge ref_map !remote-fetch[0].pattern) - ref_map-merge = 1; + ref_map-fetch_head_status = FETCH_HEAD_MERGE; } /* * if the remote we're fetching from is the same @@ -202,7 +202,7 @@ static struct ref *get_ref_map(struct transport *transport, ref_map = get_remote_ref(remote_refs, HEAD); if (!ref_map) die(_(Couldn't find remote ref HEAD)); - ref_map-merge = 1; + ref_map-fetch_head_status = FETCH_HEAD_MERGE; tail = ref_map-next; } } @@ -389,7 +389,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, const char *what, *kind; struct ref *rm; char *url, *filename = dry_run ? /dev/null : git_path(FETCH_HEAD); - int want_merge; + int want_status; fp = fopen(filename, a); if (!fp) @@ -407,19 +407,22 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, } /* -* The first pass writes objects to be merged and then the -* second pass writes the rest, in order to allow using -* FETCH_HEAD as a refname to refer to the ref to be merged. +* We do a pass for each fetch_head_status type in their enum order, so +* merged entries are written before not-for-merge. That lets readers +* use FETCH_HEAD as a refname to refer to the ref to be merged. */ - for (want_merge = 1; 0 = want_merge; want_merge--) { + for (want_status = FETCH_HEAD_MERGE; +want_status = FETCH_HEAD_IGNORE; +want_status++) { for (rm = ref_map; rm; rm = rm-next) { struct ref *ref = NULL; + const char *merge_status_marker = ; commit = lookup_commit_reference_gently(rm-old_sha1, 1); if (!commit) - rm-merge = 0; + rm-fetch_head_status = FETCH_HEAD_NOT_FOR_MERGE; - if (rm-merge != want_merge) + if (rm-fetch_head_status != want_status) continue; if (rm-peer_ref) { @@ -465,16 +468,26 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, strbuf_addf(note, %s , kind);
[PATCH 4/4] fetch: opportunistically update tracking refs
When we run a regular git fetch without arguments, we update the tracking refs according to the configured refspec. However, when we run git fetch origin master (or git pull origin master), we do not look at the configured refspecs at all, and just update FETCH_HEAD. We miss an opportunity to update refs/remotes/origin/master (or whatever the user has configured). Some users find this confusing, because they would want to do further comparisons against the old state of the remote master, like: $ git pull origin master $ git log HEAD...origin/master In the currnet code, they are comparing against whatever commit happened to be in origin/master from the last time they did a complete git fetch. This patch will update a ref from the RHS of a configured refspec whenever we happen to be fetching its LHS. That makes the case above work. The downside is that any users who really care about whether and when their tracking branches are updated may be surprised. Signed-off-by: Jeff King p...@peff.net --- Documentation/pull-fetch-param.txt | 2 +- builtin/fetch.c| 16 t/t5510-fetch.sh | 8 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/Documentation/pull-fetch-param.txt b/Documentation/pull-fetch-param.txt index 6f5ca21..18cffc2 100644 --- a/Documentation/pull-fetch-param.txt +++ b/Documentation/pull-fetch-param.txt @@ -75,4 +75,4 @@ endif::git-pull[] * A parameter ref without a colon merges ref into the current branch, endif::git-pull[] - while not storing the branch anywhere locally. + and updates the remote-tracking branches (if any). diff --git a/builtin/fetch.c b/builtin/fetch.c index 287cf4c..e41cc0d 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -160,6 +160,8 @@ static struct ref *get_ref_map(struct transport *transport, const struct ref *remote_refs = transport_get_remote_refs(transport); if (ref_count || tags == TAGS_SET) { + struct ref **old_tail; + for (i = 0; i ref_count; i++) { get_fetch_map(remote_refs, refs[i], tail, 0); if (refs[i].dst refs[i].dst[0]) @@ -170,6 +172,20 @@ static struct ref *get_ref_map(struct transport *transport, rm-fetch_head_status = FETCH_HEAD_MERGE; if (tags == TAGS_SET) get_fetch_map(remote_refs, tag_refspec, tail, 0); + + /* +* For any refs that we happen to be fetching via command-line +* arguments, take the opportunity to update their configured +* counterparts. However, we do not want to mention these +* entries in FETCH_HEAD at all, as they would simply be +* duplicates of existing entries. +*/ + old_tail = tail; + for (i = 0; i transport-remote-fetch_refspec_nr; i++) + get_fetch_map(ref_map, transport-remote-fetch[i], + tail, 0); + for (rm = *old_tail; rm; rm = rm-next) + rm-fetch_head_status = FETCH_HEAD_IGNORE; } else { /* Use the defaults */ struct remote *remote = transport-remote; diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 789c228..ff43e08 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -377,7 +377,7 @@ test_expect_success 'mark initial state of origin/master' ' ) ' -test_expect_success 'explicit fetch should not update tracking' ' +test_expect_success 'explicit fetch should update tracking' ' cd $D git branch -f side @@ -387,12 +387,12 @@ test_expect_success 'explicit fetch should not update tracking' ' o=$(git rev-parse --verify refs/remotes/origin/master) git fetch origin master n=$(git rev-parse --verify refs/remotes/origin/master) - test $o = $n + test $o != $n test_must_fail git rev-parse --verify refs/remotes/origin/side ) ' -test_expect_success 'explicit pull should not update tracking' ' +test_expect_success 'explicit pull should update tracking' ' cd $D git branch -f side @@ -402,7 +402,7 @@ test_expect_success 'explicit pull should not update tracking' ' o=$(git rev-parse --verify refs/remotes/origin/master) git pull origin master n=$(git rev-parse --verify refs/remotes/origin/master) - test $o = $n + test $o != $n test_must_fail git rev-parse --verify refs/remotes/origin/side ) ' -- 1.8.3.rc1.2.g12db477 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 00/10] Prepare for alternative remote-tracking branch location
Hi, Here is the second iteration of the current series for teaching git to work with remote ref namespaces. This iteration is pretty much a full rework of the first iteration, based on Junio's comments to the first iteration, and discussion with other Git developers at the Git Merge conference in Berlin, particularily Jonathan Nieder has helped to work out some of these issues. Patches #1 - #2 are pretty much unchanged from v1, except that we now put the remote refs in refs/peers/* instead of /refs/remotes/*. Patches #3 - #5 gets us to the point where git rev-parse origin/master will expand to refs/peers/origin/heads/master, and vice versa for shortening Patches #6-#10 (except #7 which is a small unrelated test addition) is about making git branch -a and git branch -r output remote-tracking branches from the refs/peers/* hierarchy. Note that this patch series is based on top of the jh/shorten-refname series in 'pu'. Have fun! ...Johan Johan Herland (10): t7900: Start testing usability of namespaced remote refs t7900: Demonstrate failure to expand $peer/$branch according to refspecs refs.c: Refactor code for mapping between shorthand names and full refnames remote: Reject remote names containing '/' refs.c: Add support for expanding/shortening refs in refs/peers/* t7900: Test git branch -r/-a output w/remote-tracking branches in refs/peers/* t3203: Add testcase for fix in 1603ade81352a526ccb206f41ff81ecbc855df2d builtin/branch.c: Refactor ref_item.name and .dest into strbufs builtin/branch.c: Refactor remotes/ prepending to remote-tracking branches branch: Fix display of remote branches in refs/peers/* builtin/branch.c | 114 +-- builtin/remote.c | 4 +- cache.h| 4 - refs.c | 256 ++--- refs.h | 6 + remote.c | 6 +- t/t3203-branch-output.sh | 15 ++ t/t5505-remote.sh | 12 ++ t/t7900-working-with-namespaced-remote-refs.sh | 131 + t/t7901-multi-level-remote-name-failure.sh | 20 ++ 10 files changed, 430 insertions(+), 138 deletions(-) create mode 100755 t/t7900-working-with-namespaced-remote-refs.sh create mode 100755 t/t7901-multi-level-remote-name-failure.sh -- 1.8.1.3.704.g33f7d4f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 01/10] t7900: Start testing usability of namespaced remote refs
Some users are interested in fetching remote refs into a separate namespace in the local repo. E.g. instead of the usual remote config: [remote origin] fetch = +refs/heads/*:refs/remotes/origin/* url = ... they want to keep remote tags separate from local tags, and they may also want to fetch other ref types: [remote origin] fetch = +refs/heads/*:refs/peers/origin/heads/* fetch = +refs/tags/*:refs/peers/origin/tags/* fetch = +refs/notes/*:refs/peers/origin/notes/* fetch = +refs/replace/*:refs/peers/origin/replace/* tagopt = --no-tags url = ... This configuration creates a separate namespace under refs/peers/origin/* mirroring the structure of local refs (under refs/*) where all the relevant refs from the 'origin' remote can be found. The reason for renaming the refs/remotes/ prefix to refs/peers/ is to allow Git to treat the two hierarchies somewhat differently with respect to how shorthand names are expanded by the ref machinery. This patch introduces a test whose main purpose is to verify that git will work comfortably with this kind of setup. For now, we only verify that it is possible (though not exactly easy) to establish a clone with the above configuration, and that fetching into it yields the expected result. Signed-off-by: Johan Herland jo...@herland.net --- t/t7900-working-with-namespaced-remote-refs.sh | 82 ++ 1 file changed, 82 insertions(+) create mode 100755 t/t7900-working-with-namespaced-remote-refs.sh diff --git a/t/t7900-working-with-namespaced-remote-refs.sh b/t/t7900-working-with-namespaced-remote-refs.sh new file mode 100755 index 000..dfd916b --- /dev/null +++ b/t/t7900-working-with-namespaced-remote-refs.sh @@ -0,0 +1,82 @@ +#!/bin/sh + +test_description='testing end-user usability of namespaced remote refs + +Set up a local repo with namespaced remote refs, like this: + +[remote origin] + fetch = +refs/heads/*:refs/peers/origin/heads/* + fetch = +refs/tags/*:refs/peers/origin/tags/* + fetch = +refs/notes/*:refs/peers/origin/notes/* + fetch = +refs/replace/*:refs/peers/origin/replace/* + tagopt = --no-tags + url = ... + +Test that the usual end-user operations work as expected with this setup. +' + +. ./test-lib.sh + +test_expect_success 'setup server repo' ' + git init server + ( + cd server + test_commit server_master_a + git checkout -b other + test_commit server_other_b + git checkout master + test_commit server_master_b + ) +' + +server_master_a=$(git --git-dir=server/.git rev-parse --verify server_master_a) +server_master_b=$(git --git-dir=server/.git rev-parse --verify server_master_b) +server_other_b=$(git --git-dir=server/.git rev-parse --verify server_other_b) + +cat expect.refspecs EOF ++refs/heads/*:refs/peers/origin/heads/* ++refs/tags/*:refs/peers/origin/tags/* ++refs/notes/*:refs/peers/origin/notes/* ++refs/replace/*:refs/peers/origin/replace/* +EOF + +cat expect.show-ref EOF +$server_master_b refs/heads/master +$server_master_b refs/peers/origin/heads/master +$server_other_b refs/peers/origin/heads/other +$server_master_a refs/peers/origin/tags/server_master_a +$server_master_b refs/peers/origin/tags/server_master_b +$server_other_b refs/peers/origin/tags/server_other_b +EOF + +test_clone() { + ( cd $1 git config --get-all remote.origin.fetch ) actual.refspecs + test_cmp expect.refspecs actual.refspecs + ( cd $1 git show-ref ) actual.show-ref + test_cmp expect.show-ref actual.show-ref +} + +test_expect_failure 'clone with namespaced remote refs' ' + git clone --layout=peers server client + test_clone client +' + +# Work-around for the not-yet-existing clone option used above +test_expect_success 'work-around clone with namespaced remote refs' ' + rm -rf client + git init client + ( + cd client + git config remote.origin.url ../server + git config --add remote.origin.fetch +refs/heads/*:refs/peers/origin/heads/* + git config --add remote.origin.fetch +refs/tags/*:refs/peers/origin/tags/* + git config --add remote.origin.fetch +refs/notes/*:refs/peers/origin/notes/* + git config --add remote.origin.fetch +refs/replace/*:refs/peers/origin/replace/* + git config remote.origin.tagopt --no-tags + git fetch + git checkout master + ) + test_clone client +' + +test_done -- 1.8.1.3.704.g33f7d4f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 02/10] t7900: Demonstrate failure to expand $peer/$branch according to refspecs
This test verifies that the following expressions all evaluate to the full refname refs/peers/origin/heads/master: - refs/peers/origin/heads/master - peers/origin/heads/master - origin/heads/master - origin/master (We assume that there are no other conflicting refs for which the above expressions would cause ambiguity.) Currently none of these work, because the refs machinery don't know how to expand shorthand names within the refs/peers/* hierarchy. Mirroring the expansion of the above 4 expressions into the full refname, the same 4 expressions should also be shortened into origin/master when abbreviating them into their shortest unambiguous representation, e.g. when running git rev-parse --abbrev-ref on them. A (currently failing) test verifying this behavior is also added by this patch. Signed-off-by: Johan Herland jo...@herland.net --- t/t7900-working-with-namespaced-remote-refs.sh | 28 ++ 1 file changed, 28 insertions(+) diff --git a/t/t7900-working-with-namespaced-remote-refs.sh b/t/t7900-working-with-namespaced-remote-refs.sh index dfd916b..109e9b8 100755 --- a/t/t7900-working-with-namespaced-remote-refs.sh +++ b/t/t7900-working-with-namespaced-remote-refs.sh @@ -79,4 +79,32 @@ test_expect_success 'work-around clone with namespaced remote refs' ' test_clone client ' +test_expect_success 'enter client repo' ' + cd client +' + +test_expect_failure 'short-hand notation expands correctly for remote-tracking branches' ' + echo refs/peers/origin/heads/master expect + git rev-parse --symbolic-full-name refs/peers/origin/heads/master actual + test_cmp expect actual + git rev-parse --symbolic-full-name peers/origin/heads/master actual + test_cmp expect actual + git rev-parse --symbolic-full-name origin/heads/master actual + test_cmp expect actual + git rev-parse --symbolic-full-name origin/master actual + test_cmp expect actual +' + +test_expect_failure 'remote-tracking branches are shortened correctly' ' + echo origin/master expect + git rev-parse --abbrev-ref refs/peers/origin/heads/master actual + test_cmp expect actual + git rev-parse --abbrev-ref peers/origin/heads/master actual + test_cmp expect actual + git rev-parse --abbrev-ref origin/heads/master actual + test_cmp expect actual + git rev-parse --abbrev-ref origin/master actual + test_cmp expect actual +' + test_done -- 1.8.1.3.704.g33f7d4f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 03/10] refs.c: Refactor code for mapping between shorthand names and full refnames
This patch is in preparation for extending the ways in which we expand shorthand names into full refnames, and shorten full refnames into unambiguous shorthand names. We collect the logic for performing the expansion and shortening into two functions: refname_expand() and refname_shorten(). refname_expand() takes a shorthand name, and a pattern containing a wildcard (more on those below), and substitutes the shorthand name for the wildcard. The result is written into a strbuf provided by the caller. Correspondingly, refname_shorten() takes a full refname and matches it against the given pattern, extracting the portion of the full refname that matches the pattern wildcard. The resulting shorthand name is written into a strbuf provided by the caller. The refname_expand() function no longer uses mkpath()/mksnpath() to perform the pattern expansion. Instead, it uses strbuf_expand(), which removes the need for using fixed-length buffers from the code. This also means we no longer call cleanup_path() on the resulting refs, but AFAICS, this is not really needed anyway (also it breaks no existing tests). Since the pattern expansion is now done by strbuf_expand(), the syntax of the pattern wildcard must change as well. Whereas a pattern used to look like this: refs/remotes/%.*s/HEAD it now looks like this: refs/remotes/%*/HEAD where the %* wildcard is meant to be read as the full shorthand name. More wildcards will be added by future patches that require more complex wildcard matching. The list of patterns has been renamed from ref_rev_parse_rules to refname_patterns (which should be more descriptive), and it has lost the NULL terminator, which is no longer needed, since all iterations of the list now uses ARRAY_SIZE(refname_patterns) to end iteration, instead of testing each member against NULL. Furthermore, refname_match() used to take the list of patterns as an argument, but since all callers pass the one and only pattern list, we can drop the argument altogether, and use refname_patterns directly in refname_match(). This also enables us to make the pattern list static. The code for shortening full refnames into shorthands, has been adjusted to deal with the new %* patterns. It now uses strbuf_split_str() with the '%' as terminator, to split the prefix part from the suffix part. Then - as before - it matches the prefix from the start of the given refname and the suffix from the end of the refname. If both matches are successful, the middle portion is extracted as the resulting shorthand name. The end result of this refactoring is equivalent in behavior to the existing code, but makes it easier for future patches to adjust how expansion and shortening happens. All of the existing users of ref_rev_parse_rules are converted to refname_patterns by this patch, so ref_rev_parse_rules is removed as well. Signed-off-by: Johan Herland jo...@herland.net --- cache.h | 4 -- refs.c | 187 ++- refs.h | 3 + remote.c | 6 +- 4 files changed, 119 insertions(+), 81 deletions(-) diff --git a/cache.h b/cache.h index ac620b2..0cc43b2 100644 --- a/cache.h +++ b/cache.h @@ -874,10 +874,6 @@ extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref); extern int interpret_branch_name(const char *str, struct strbuf *); extern int get_sha1_mb(const char *str, unsigned char *sha1); -extern int refname_match(const char *abbrev_name, const char *full_name, const char **rules); -extern const char *ref_rev_parse_rules[]; -#define ref_fetch_rules ref_rev_parse_rules - extern int create_symref(const char *ref, const char *refs_heads_master, const char *logmsg); extern int validate_headref(const char *ref); diff --git a/refs.c b/refs.c index 006f369..ab5e120 100644 --- a/refs.c +++ b/refs.c @@ -1783,27 +1783,91 @@ const char *prettify_refname(const char *name) 0); } -const char *ref_rev_parse_rules[] = { - %.*s, - refs/%.*s, - refs/tags/%.*s, - refs/heads/%.*s, - refs/remotes/%.*s, - refs/remotes/%.*s/HEAD, - NULL +static const char *refname_patterns[] = { + %*, + refs/%*, + refs/tags/%*, + refs/heads/%*, + refs/remotes/%*, + refs/remotes/%*/HEAD }; -int refname_match(const char *abbrev_name, const char *full_name, const char **rules) +struct wildcard_data { + const char *s; + size_t len; + int done; +}; + +static size_t refname_expand_helper(struct strbuf *sb, const char *placeholder, + void *context) { - const char **p; - const int abbrev_name_len = strlen(abbrev_name); + struct wildcard_data *cb = context; + if (*placeholder == '*' !cb-done) { + strbuf_add(sb, cb-s, cb-len); + cb-done = 1; + return 1; + } + return 0; +} - for (p = rules; *p; p++) { - if
[PATCHv2 04/10] remote: Reject remote names containing '/'
Although we definitely support and encourage use of multi-level branch names, we have never conciously tried to give support for multi-level remote names. Currently, they are allowed, but there is no evidence that they are commonly used. Now, they do provide a source of problems when trying to expand the $nick/$name shorthand notation (where $nick matches a remote name) into a full refname. Consider the shorthand foo/bar/baz: Does this parse as $nick = foo, $name = bar/baz, or $nick = foo/bar, $name = baz? Since we need to be unambiguous about these things, we hereby declare that a remote name shall never contain a '/' character, and that the only correct way to parse foo/bar/baz is $nick = foo, $name = bar/baz. This patch teaches 'git remote' to reject remote names with slashes, and adds tests verifying this. Signed-off-by: Johan Herland jo...@herland.net --- builtin/remote.c | 4 ++-- t/t5505-remote.sh | 12 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/builtin/remote.c b/builtin/remote.c index 5e54d36..6e7369d 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -195,7 +195,7 @@ static int add(int argc, const char **argv) die(_(remote %s already exists.), name); strbuf_addf(buf2, refs/heads/test:refs/remotes/%s/test, name); - if (!valid_fetch_refspec(buf2.buf)) + if (!valid_fetch_refspec(buf2.buf) || strchr(name, '/')) die(_('%s' is not a valid remote name), name); strbuf_addf(buf, remote.%s.url, name); @@ -646,7 +646,7 @@ static int mv(int argc, const char **argv) die(_(remote %s already exists.), rename.new); strbuf_addf(buf, refs/heads/test:refs/remotes/%s/test, rename.new); - if (!valid_fetch_refspec(buf.buf)) + if (!valid_fetch_refspec(buf.buf) || strchr(rename.new, '/')) die(_('%s' is not a valid remote name), rename.new); strbuf_reset(buf); diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index dd10ff0..f7098fe 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -621,6 +621,12 @@ test_expect_success 'reject adding remote with an invalid name' ' ' +test_expect_success 'reject adding remote with slash in name' ' + + test_must_fail git remote add multi/level . + +' + # The first three test if the tracking branches are properly renamed, # the last two ones check if the config is updated. @@ -668,6 +674,12 @@ test_expect_success 'rename a remote with name prefix of other remote' ' ' +test_expect_success 'rename a remote with slash in name' ' + + test_must_fail git remote rename upstream up/stream + +' + cat remotes_origin EOF URL: $(pwd)/one Push: refs/heads/master:refs/heads/upstream -- 1.8.1.3.704.g33f7d4f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 06/10] t7900: Test git branch -r/-a output w/remote-tracking branches in refs/peers/*
These tests will be made to pass by subsequent patches. Signed-off-by: Johan Herland jo...@herland.net --- t/t7900-working-with-namespaced-remote-refs.sh | 21 + 1 file changed, 21 insertions(+) diff --git a/t/t7900-working-with-namespaced-remote-refs.sh b/t/t7900-working-with-namespaced-remote-refs.sh index 33266e0..279664c 100755 --- a/t/t7900-working-with-namespaced-remote-refs.sh +++ b/t/t7900-working-with-namespaced-remote-refs.sh @@ -107,4 +107,25 @@ test_expect_success 'remote-tracking branches are shortened correctly' ' test_cmp expect actual ' +cat expect.branch-r EOF + origin/master + origin/other +EOF + +test_expect_failure 'git branch -r should show remote-tracking branches' ' + git branch -r actual.branch-r + test_cmp expect.branch-r actual.branch-r +' + +cat expect.branch-a EOF +* master + peers/origin/heads/master + peers/origin/heads/other +EOF + +test_expect_failure 'git branch -a should also show remote-tracking branches' ' + git branch -a actual.branch-a + test_cmp expect.branch-a actual.branch-a +' + test_done -- 1.8.1.3.704.g33f7d4f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 07/10] t3203: Add testcase for fix in 1603ade81352a526ccb206f41ff81ecbc855df2d
This adds a testcase for some behavior that I very nearly broke while refactoring some stuff in builtin/branch.c. Signed-off-by: Johan Herland jo...@herland.net --- t/t3203-branch-output.sh | 15 +++ 1 file changed, 15 insertions(+) diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh index ba4f98e..c312d3a 100755 --- a/t/t3203-branch-output.sh +++ b/t/t3203-branch-output.sh @@ -106,4 +106,19 @@ EOF test_i18ncmp expect actual ' +test_expect_success 'git branch -a --merged handles broken refs gracefully' ' + cat expect EOF + branch-one + branch-two +* master + remotes/origin/HEAD - origin/branch-one + remotes/origin/branch-one + remotes/origin/branch-two +EOF + git checkout master + echo broken ref .git/refs/heads/broken + test_must_fail git branch -a --merged actual + test_cmp expect actual +' + test_done -- 1.8.1.3.704.g33f7d4f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 08/10] builtin/branch.c: Refactor ref_item.name and .dest into strbufs
In preparation of a future patch which reorganizes how the display of the ref_list is done. Signed-off-by: Johan Herland jo...@herland.net --- builtin/branch.c | 37 + 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 0836890..c8b49e3 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -276,8 +276,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, } struct ref_item { - char *name; - char *dest; + struct strbuf name; + struct strbuf dest; unsigned int kind, width; struct commit *commit; }; @@ -380,11 +380,15 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags, /* Record the new item */ newitem = (ref_list-list[ref_list-index++]); - newitem-name = xstrdup(refname); + strbuf_init(newitem-name, 0); + strbuf_addstr(newitem-name, refname); newitem-kind = kind; newitem-commit = commit; newitem-width = utf8_strwidth(refname); - newitem-dest = resolve_symref(orig_refname, prefix); + strbuf_init(newitem-dest, 0); + orig_refname = resolve_symref(orig_refname, prefix); + if (orig_refname) + strbuf_addstr(newitem-dest, orig_refname); /* adjust for remotes/ */ if (newitem-kind == REF_REMOTE_BRANCH ref_list-kinds != REF_REMOTE_BRANCH) @@ -400,8 +404,8 @@ static void free_ref_list(struct ref_list *ref_list) int i; for (i = 0; i ref_list-index; i++) { - free(ref_list-list[i].name); - free(ref_list-list[i].dest); + strbuf_release(ref_list-list[i].name); + strbuf_release(ref_list-list[i].dest); } free(ref_list-list); } @@ -413,7 +417,7 @@ static int ref_cmp(const void *r1, const void *r2) if (c1-kind != c2-kind) return c1-kind - c2-kind; - return strcmp(c1-name, c2-name); + return strbuf_cmp(c1-name, c2-name); } static void fill_tracking_info(struct strbuf *stat, const char *branch_name, @@ -496,7 +500,7 @@ static void add_verbose_info(struct strbuf *out, struct ref_item *item, } if (item-kind == REF_LOCAL_BRANCH) - fill_tracking_info(stat, item-name, verbose 1); + fill_tracking_info(stat, item-name.buf, verbose 1); strbuf_addf(out, %s %s%s, find_unique_abbrev(item-commit-object.sha1, abbrev), @@ -534,7 +538,7 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, color = BRANCH_COLOR_CURRENT; } - strbuf_addf(name, %s%s, prefix, item-name); + strbuf_addf(name, %s%s, prefix, item-name.buf); if (verbose) { int utf8_compensation = strlen(name.buf) - utf8_strwidth(name.buf); strbuf_addf(out, %c %s%-*s%s, c, branch_get_color(color), @@ -544,8 +548,8 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, strbuf_addf(out, %c %s%s%s, c, branch_get_color(color), name.buf, branch_get_color(BRANCH_COLOR_RESET)); - if (item-dest) - strbuf_addf(out, - %s, item-dest); + if (item-dest.len) + strbuf_addf(out, - %s, item-dest.buf); else if (verbose) /* f7c0c00 [ahead 58, behind 197] vcs-svn: drop obj_pool.h */ add_verbose_info(out, item, verbose, abbrev); @@ -601,15 +605,16 @@ static void show_detached(struct ref_list *ref_list) if (head_commit is_descendant_of(head_commit, ref_list-with_commit)) { struct ref_item item; - item.name = get_head_description(); - item.width = utf8_strwidth(item.name); + strbuf_init(item.name, 0); + strbuf_addstr(item.name, get_head_description()); + item.width = utf8_strwidth(item.name.buf); item.kind = REF_LOCAL_BRANCH; - item.dest = NULL; + strbuf_init(item.dest, 0); item.commit = head_commit; if (item.width ref_list-maxwidth) ref_list-maxwidth = item.width; print_ref_item(item, ref_list-maxwidth, ref_list-verbose, ref_list-abbrev, 1, ); - free(item.name); + strbuf_release(item.name); } } @@ -655,7 +660,7 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru for (i = 0; i ref_list.index; i++) { int current = !detached (ref_list.list[i].kind == REF_LOCAL_BRANCH) - !strcmp(ref_list.list[i].name, head); + !strcmp(ref_list.list[i].name.buf, head); char *prefix = (kinds != REF_REMOTE_BRANCH
[PATCHv2 09/10] builtin/branch.c: Refactor remotes/ prepending to remote-tracking branches
When running git branch -a (instead of git branch -r), we prefix the remote-tracking branches with remotes/ to allow the user to more easily tell them apart from the local branches. The code that prepended remotes/ to remote-tracking branches was located in print_ref_item(), while the code that adjusted the ref_item.width (to account for the prepended remotes/) was located in append_ref(). This code moves the prepending of remotes/ up into append_ref(), which is a nice cleanup of the code, as well as a preparation for changing the logic when remote-tracking branches start showing up in refs/peers/*. Signed-off-by: Johan Herland jo...@herland.net --- builtin/branch.c | 23 +-- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index c8b49e3..4480be2 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -384,7 +384,6 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags, strbuf_addstr(newitem-name, refname); newitem-kind = kind; newitem-commit = commit; - newitem-width = utf8_strwidth(refname); strbuf_init(newitem-dest, 0); orig_refname = resolve_symref(orig_refname, prefix); if (orig_refname) @@ -392,7 +391,8 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags, /* adjust for remotes/ */ if (newitem-kind == REF_REMOTE_BRANCH ref_list-kinds != REF_REMOTE_BRANCH) - newitem-width += 8; + strbuf_insert(newitem-name, 0, remotes/, 8); + newitem-width = utf8_strwidth(newitem-name.buf); if (newitem-width ref_list-maxwidth) ref_list-maxwidth = newitem-width; @@ -510,12 +510,12 @@ static void add_verbose_info(struct strbuf *out, struct ref_item *item, } static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, - int abbrev, int current, char *prefix) + int abbrev, int current) { char c; int color; struct commit *commit = item-commit; - struct strbuf out = STRBUF_INIT, name = STRBUF_INIT; + struct strbuf out = STRBUF_INIT; if (!matches_merge_filter(commit)) return; @@ -538,15 +538,14 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, color = BRANCH_COLOR_CURRENT; } - strbuf_addf(name, %s%s, prefix, item-name.buf); if (verbose) { - int utf8_compensation = strlen(name.buf) - utf8_strwidth(name.buf); + int utf8_compensation = strlen(item-name.buf) - utf8_strwidth(item-name.buf); strbuf_addf(out, %c %s%-*s%s, c, branch_get_color(color), - maxwidth + utf8_compensation, name.buf, + maxwidth + utf8_compensation, item-name.buf, branch_get_color(BRANCH_COLOR_RESET)); } else strbuf_addf(out, %c %s%s%s, c, branch_get_color(color), - name.buf, branch_get_color(BRANCH_COLOR_RESET)); + item-name.buf, branch_get_color(BRANCH_COLOR_RESET)); if (item-dest.len) strbuf_addf(out, - %s, item-dest.buf); @@ -559,7 +558,6 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, } else { printf(%s\n, out.buf); } - strbuf_release(name); strbuf_release(out); } @@ -613,7 +611,7 @@ static void show_detached(struct ref_list *ref_list) item.commit = head_commit; if (item.width ref_list-maxwidth) ref_list-maxwidth = item.width; - print_ref_item(item, ref_list-maxwidth, ref_list-verbose, ref_list-abbrev, 1, ); + print_ref_item(item, ref_list-maxwidth, ref_list-verbose, ref_list-abbrev, 1); strbuf_release(item.name); } } @@ -661,11 +659,8 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru int current = !detached (ref_list.list[i].kind == REF_LOCAL_BRANCH) !strcmp(ref_list.list[i].name.buf, head); - char *prefix = (kinds != REF_REMOTE_BRANCH - ref_list.list[i].kind == REF_REMOTE_BRANCH) - ? remotes/ : ; print_ref_item(ref_list.list[i], ref_list.maxwidth, verbose, - abbrev, current, prefix); + abbrev, current); } free_ref_list(ref_list); -- 1.8.1.3.704.g33f7d4f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 10/10] branch: Fix display of remote branches in refs/peers/*
The current branch display code assumes all remote-tracking branches live inside refs/remotes/*, and that their appropriate shorthand names can be retrieved by simply stripping off the refs/remotes/ prefix. When we add remote-tracking branches in refs/peers/$remote/heads/*, the code must not only be taught how to list refs from the new location, but also how to appropriately derive shorthand names for the remote-tracking branches there (instead of stripping off a hardcoded prefix, we must use the refname_shorten() function introduced in an earlier patch.) There is already a good set of tests for the expected output from git branch, and this patch does not break any of them. However, we do fix the two tests with remote-tracking branches in refs/peers/* introduced in a previous patch. Signed-off-by: Johan Herland jo...@herland.net --- builtin/branch.c | 66 ++ refs.c | 4 +- refs.h | 3 ++ t/t7900-working-with-namespaced-remote-refs.sh | 4 +- 4 files changed, 42 insertions(+), 35 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 4480be2..9a6bce8 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -290,18 +290,16 @@ struct ref_list { int kinds; }; -static char *resolve_symref(const char *src, const char *prefix) +static void resolve_symref(struct strbuf *shortname, const char *pattern, const char *src) { unsigned char sha1[20]; int flag; - const char *dst, *cp; + const char *dst; dst = resolve_ref_unsafe(src, sha1, 0, flag); - if (!(dst (flag REF_ISSYMREF))) - return NULL; - if (prefix (cp = skip_prefix(dst, prefix))) - dst = cp; - return xstrdup(dst); + if (dst (flag REF_ISSYMREF) + refname_shorten(shortname, pattern, dst, strlen(dst))) + strbuf_addstr(shortname, dst); } struct append_ref_cb { @@ -328,74 +326,80 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags, struct ref_list *ref_list = cb-ref_list; struct ref_item *newitem; struct commit *commit; + struct strbuf ref = STRBUF_INIT; int kind, i; - const char *prefix, *orig_refname = refname; + const char *pattern; + size_t refname_len = strlen(refname); static struct { int kind; - const char *prefix; - int pfxlen; + const char *pattern; } ref_kind[] = { - { REF_LOCAL_BRANCH, refs/heads/, 11 }, - { REF_REMOTE_BRANCH, refs/remotes/, 13 }, + { REF_LOCAL_BRANCH, refs/heads/%* }, + { REF_REMOTE_BRANCH, refs/remotes/%* }, + { REF_REMOTE_BRANCH, refs/peers/%1/heads/%* }, }; /* Detect kind */ for (i = 0; i ARRAY_SIZE(ref_kind); i++) { - prefix = ref_kind[i].prefix; - if (strncmp(refname, prefix, ref_kind[i].pfxlen)) + if (refname_shorten(ref, ref_kind[i].pattern, refname, refname_len)) continue; kind = ref_kind[i].kind; - refname += ref_kind[i].pfxlen; + pattern = ref_kind[i].pattern; break; } if (ARRAY_SIZE(ref_kind) = i) - return 0; + goto out; /* Don't add types the caller doesn't want */ if ((kind ref_list-kinds) == 0) - return 0; + goto out; - if (!match_patterns(cb-pattern, refname)) - return 0; + if (!match_patterns(cb-pattern, ref.buf)) + goto out; commit = NULL; if (ref_list-verbose || ref_list-with_commit || merge_filter != NO_FILTER) { commit = lookup_commit_reference_gently(sha1, 1); if (!commit) { - cb-ret = error(_(branch '%s' does not point at a commit), refname); - return 0; + cb-ret = error(_(branch '%s' does not point at a commit), ref.buf); + goto out; } /* Filter with with_commit if specified */ if (!is_descendant_of(commit, ref_list-with_commit)) - return 0; + goto out; if (merge_filter != NO_FILTER) add_pending_object(ref_list-revs, - (struct object *)commit, refname); + (struct object *)commit, ref.buf); } + /* +* When displaying more then just remote-tracking branches, make the +* remote-tracking branches more explicit, e.g. instead of printing +* origin/master, we should print remote/origin/master (or +*
[PATCHv2 05/10] refs.c: Add support for expanding/shortening refs in refs/peers/*
This patch adds the following patterns for expanding/shortening refs: refs/peers/%* refs/peers/%1/tags/%* refs/peers/%1/heads/%* These allow shorthand names like origin/master to expand to refs in the refs/peers/* hierarchy (in this case, the likely expansion would be by the middle rule above, resulting in refs/peers/origin/heads/master). To accomplish this, we have added the new %1 wildcard which shall expand into the first component (i.e. up to the first '/') of the given shorthand. The other wildcard (%*) shall then expand into the remainder of the shorthand (i.e. following the first '/'). Correspondingly, when shortening according to a pattern with %1, a single component (not including any '/' character) shall be extracted from that point in the given refname, and shall be added to the resulting shorthand, with a trailing '/'. Then, when hitting the %*, the remainder of the given refname (modulo a trailing match in the pattern) shall be extracted and appended to the portion previously extracted by the %1 wildcard. The need to split the $remote/$ref into its $remote and $ref parts is the reason why multi-level remote names will no longer work (and hence were disallowed in the previous patch). A testcase demonstrating how multi-level remote names fail is therefore included in this patch. Signed-off-by: Johan Herland jo...@herland.net --- refs.c | 101 + t/t7900-working-with-namespaced-remote-refs.sh | 4 +- t/t7901-multi-level-remote-name-failure.sh | 20 + 3 files changed, 107 insertions(+), 18 deletions(-) create mode 100755 t/t7901-multi-level-remote-name-failure.sh diff --git a/refs.c b/refs.c index ab5e120..188a9eb 100644 --- a/refs.c +++ b/refs.c @@ -1789,7 +1789,10 @@ static const char *refname_patterns[] = { refs/tags/%*, refs/heads/%*, refs/remotes/%*, - refs/remotes/%*/HEAD + refs/remotes/%*/HEAD, + refs/peers/%*, + refs/peers/%1/tags/%*, + refs/peers/%1/heads/%* }; struct wildcard_data { @@ -1806,6 +1809,15 @@ static size_t refname_expand_helper(struct strbuf *sb, const char *placeholder, strbuf_add(sb, cb-s, cb-len); cb-done = 1; return 1; + } else if (*placeholder == '1' !cb-done) { + const char *p = memchr(cb-s, '/', cb-len); + size_t copy_len = p ? p - cb-s : cb-len; + strbuf_add(sb, cb-s, copy_len); + if (copy_len cb-len cb-s[copy_len] == '/') + copy_len++; + cb-s += copy_len; + cb-len -= copy_len; + return 1; } return 0; } @@ -1821,6 +1833,46 @@ static int refname_expand(struct strbuf *dst, const char *pattern, return !cbdata.done; } +static int handle_fragment(struct strbuf *dst, struct strbuf *fragment, + int first, int last, + const char *refname, size_t refname_len) +{ + const char *ref = refname, *p; + size_t ref_len = refname_len, trail_len; + if (!first) { + /* extract wildcard according to wildcard char */ + switch (fragment-buf[0]) { + case '1': + /* extract up to next '/' from refname */ + p = memchr(ref, '/', ref_len); + if (!p) + return -1; + strbuf_add(dst, ref, p - ref); + strbuf_addch(dst, '/'); + ref_len -= p - ref; + ref += p - ref; + break; + case '*': + /* extract all up to matching trailer */ + assert(last); + trail_len = fragment-len - 1; + if (trail_len ref_len) + return -1; + strbuf_add(dst, ref, ref_len - trail_len); + ref += ref_len - trail_len; + ref_len -= ref_len - trail_len; + break; + } + } + + /* match rest of fragment verbatim */ + p = fragment-buf + (first ? 0 : 1); /* skip wildcard character */ + trail_len = fragment-len - ((first ? 0 : 1) + (last ? 0 : 1)); + if (trail_len ref_len || memcmp(ref, p, trail_len)) + return -1; + return (ref - refname) + trail_len; +} + static int refname_shorten(struct strbuf *dst, const char *pattern, const char *refname, size_t refname_len) { @@ -1830,26 +1882,43 @@ static int refname_shorten(struct strbuf *dst, const char *pattern, * Return 0 on success (positive match, wildcard-matching portion * copied into dst), and non-zero on failure (no match, dst empty). */ - const char *match_end; - struct strbuf
[PATCH 0/2] Colored prompt for zsh
Hi, I was using ZSH's vcs_info until now, and just discovered contrib/completion/git-prompt.sh. Other differences aside, it's simple enough that I can hack on it. The first patch strips a small whitespace (we can't waste valuable real estate on unnecessary whitespace), and the second patch should get you a beautiful colored prompt in ZSH. Enjoy! Ramkumar Ramachandra (2): git-prompt.sh: strip unnecessary space in prompt string git-prompt.zsh: introduce thin ZSH wrapper contrib/completion/git-prompt.sh | 80 --- contrib/completion/git-prompt.zsh | 59 + 2 files changed, 101 insertions(+), 38 deletions(-) create mode 100644 contrib/completion/git-prompt.zsh -- 1.8.3.rc1.52.g4537cf1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] git-prompt.sh: strip unnecessary space in prompt string
Nobody has branch names that end with + or *. Then why put a space after the branch name and before [*|+][=||] in the prompt string? Before this, your prompt might have looked like: artagnon|master *=:~/src/git$ Now, it will look like: artagnon|master*=:~/src/git$ Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- contrib/completion/git-prompt.sh | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index eaf5c36..08c9b22 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -383,9 +383,6 @@ __git_ps1 () # is necessary to prevent wrapping issues! gitstring=\[$branch_color\]$branchstring\[$c_clear\] - if [ -n $w$i$s$u$r$p ]; then - gitstring=$gitstring - fi if [ $w = * ]; then gitstring=$gitstring\[$bad_color\]$w fi @@ -400,13 +397,13 @@ __git_ps1 () fi gitstring=$gitstring\[$c_clear\]$r$p else - gitstring=$c${b##refs/heads/}${f:+ $f}$r$p + gitstring=$c${b##refs/heads/}${f:+$f}$r$p fi gitstring=$(printf -- $printf_format $gitstring) PS1=$ps1pc_start$gitstring$ps1pc_end else # NO color option unless in PROMPT_COMMAND mode - printf -- $printf_format $c${b##refs/heads/}${f:+ $f}$r$p + printf -- $printf_format $c${b##refs/heads/}${f:+$f}$r$p fi fi } -- 1.8.3.rc1.52.g4537cf1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] git-prompt.zsh: introduce thin ZSH wrapper
To facilitate a colored prompt in ZSH, write a thin wrapper around git-prompt.sh, factoring out and overriding the coloring logic. Since ZSH lacks a PROMPT_COMMAND, instruct the user to execute __git_ps1 inside precmd(). Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- contrib/completion/git-prompt.sh | 73 +-- contrib/completion/git-prompt.zsh | 59 +++ 2 files changed, 99 insertions(+), 33 deletions(-) create mode 100644 contrib/completion/git-prompt.zsh diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 08c9b22..0bc51ad 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -222,6 +222,45 @@ __git_ps1_show_upstream () } +# Helper function that is meant to be called from __git_ps1. It +# builds up a gitstring injecting color codes into the appropriate +# places. +__git_ps1_colorize_gitstring () +{ + local c_red='\e[31m' + local c_green='\e[32m' + local c_lblue='\e[1;34m' + local c_clear='\e[0m' + local bad_color=$c_red + local ok_color=$c_green + local branch_color=$c_clear + local flags_color=$c_lblue + local branchstring=$c${b##refs/heads/} + + if [ $detached = no ]; then + branch_color=$ok_color + else + branch_color=$bad_color + fi + + # Setting gitstring directly with \[ and \] around colors + # is necessary to prevent wrapping issues! + gitstring=\[$branch_color\]$branchstring\[$c_clear\] + + if [ $w = * ]; then + gitstring=$gitstring\[$bad_color\]$w + fi + if [ -n $i ]; then + gitstring=$gitstring\[$ok_color\]$i + fi + if [ -n $s ]; then + gitstring=$gitstring\[$flags_color\]$s + fi + if [ -n $u ]; then + gitstring=$gitstring\[$bad_color\]$u + fi + gitstring=$gitstring\[$c_clear\]$r$p +} # __git_ps1 accepts 0 or 1 arguments (i.e., format string) # when called from PS1 using command substitution @@ -363,39 +402,7 @@ __git_ps1 () if [ $pcmode = yes ]; then local gitstring= if [ -n ${GIT_PS1_SHOWCOLORHINTS-} ]; then - local c_red='\e[31m' - local c_green='\e[32m' - local c_lblue='\e[1;34m' - local c_clear='\e[0m' - local bad_color=$c_red - local ok_color=$c_green - local branch_color=$c_clear - local flags_color=$c_lblue - local branchstring=$c${b##refs/heads/} - - if [ $detached = no ]; then - branch_color=$ok_color - else - branch_color=$bad_color - fi - - # Setting gitstring directly with \[ and \] around colors - # is necessary to prevent wrapping issues! - gitstring=\[$branch_color\]$branchstring\[$c_clear\] - - if [ $w = * ]; then - gitstring=$gitstring\[$bad_color\]$w - fi - if [ -n $i ]; then - gitstring=$gitstring\[$ok_color\]$i - fi - if [ -n $s ]; then - gitstring=$gitstring\[$flags_color\]$s - fi - if [ -n $u ]; then - gitstring=$gitstring\[$bad_color\]$u - fi - gitstring=$gitstring\[$c_clear\]$r$p + __git_ps1_colorize_gitstring else gitstring=$c${b##refs/heads/}${f:+$f}$r$p fi diff --git a/contrib/completion/git-prompt.zsh b/contrib/completion/git-prompt.zsh new file mode 100644 index 000..dc164dd --- /dev/null +++ b/contrib/completion/git-prompt.zsh @@ -0,0 +1,59 @@ +# git prompt support for zsh: wrapper around git-prompt.sh +# +# To enable: +# +#1) Copy this file and git-prompt.sh to ~/.zsh/prompt +#2) Add the following lines to your .zshrc: +# +# source ~/.zsh/prompt/git-prompt.zsh +# GIT_PS1_SHOWCOLORHINTS=true +# precmd () { __git_ps1 %n| :%~$ %s } +# +#3) You can now add the following to ~/.zshrc and expect the +# characters to be displayed in color: +# +# GIT_PS1_DESCRIBE_STYLE=branch +# GIT_PS1_SHOWUPSTREAM=auto +#
Re: Misusing git: trimming old commits
On Thu, May 9, 2013 at 11:40 AM, Martin Langhoff martin.langh...@gmail.com wrote: With the exaction of the final destination, I want to expire reports that are old and successfully transferred. OK, that took some effort to make work. Make sure you are not using reflogs (or that reflogs are promptly expired). # right after a successful push of all heads to the receiving server... for head in $(git branch|sed 's/^..//'); do # FIXME period graft_sha1=$(git log --until=one.month.ago -n1 --pretty=format:%H ${head}) if [[ -z $graft_sha1 ]]; then # nothing to prune continue fi # is it already grafted? if grep -q ^${graft_sha1} ${GIT_DIR}/info/grafts /dev/null ; then # don't duplicate the graft continue fi some_grafted='true' # prep empty commit # skip git read-tree --empty, we get the same with export GIT_INDEX_FILE=/tmp/ppg-emptytree.$$ empty_tree=$(git write-tree) rm ${GIT_INDEX_FILE} unset GIT_INDEX_FILE empty_commit=$(git commit-tree -m empty ${empty_tree}) echo ${graft_sha1} ${empty_commit} ${GIT_DIR}/info/grafts done if [[ -z $some_grafted ]]; then # nothing to do exit 0 fi # ppg-repack makes the unreachable objects loose # (it is git-repack hacked to remove --keep-true-parents), # git prune --expire actually deletes them $PPG_EXEC_PATH/ppg-repack -AFfd git prune --expire=now ### Cleanup stale grafts # current grafts points are reachable, # pruned graft points (made obsolete by a newer graft) # cannot be retrieved and git cat-file exit code is 128 touch ${GIT_DIR}/info/grafts.tmp.$$ while read line; do graftpoint=$(echo ${line} | cut -d' ' -f1) if git cat-file commit ${graftpoint} /dev/null ; then echo ${line} ${GIT_DIR}/info/grafts.tmp.$$ fi done ${GIT_DIR}/info/grafts if [ -s ${GIT_DIR}/info/grafts.tmp.$$ ]; then mv ${GIT_DIR}/info/grafts.tmp.$$ ${GIT_DIR}/info/grafts fi Perhaps it helps someone else trying to run git as a spooler :-) cheers, m -- martin.langh...@gmail.com - ask interesting questions - don't get distracted with shiny stuff - working code first ~ http://docs.moodle.org/en/User:Martin_Langhoff -- To unsubscribe from this list: send the line 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: [PATCHv3 3/7] show: honor --textconv for blobs
Michael J Gruber g...@drmicha.warpmail.net writes: + if (!DIFF_OPT_TOUCHED(rev-diffopt, ALLOW_TEXTCONV) || + !DIFF_OPT_TST(rev-diffopt, ALLOW_TEXTCONV)) + return stream_blob_to_fd(1, sha1, NULL, 0); It is surprising that the necessary change is only this, but I think it is correct ;-). We ignore textconv when the command line did not mention --[no-]textconv, or the command line said --no-textconv explicitly. This (especially the first condition) may deserve an in-code comment for anybody who wonders where this default behaviour is implemented. It's not as if we would document behavior by in-code comments in general, do we? The usual answer is git log -S or git blame. The comment and the future reader I had in mind was more like Default to --no-textconv, even though cmd_log_init_defaults() sets the bit, when the user did not explicitly ask for it. sought by somebody who wonders _where_ in the code we ignore ALLOW_TEXTCONV that is set in cmd_log_init_defaults(). That is not something you can find with log -S or blame, is it? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/2] merge-base: add --merge-child option
John Keeping j...@keeping.me.uk writes: This is helpful when examining branches with disjoint roots, for example because one is periodically merged into a subtree of the other. With the --merge-child option, git merge-base will print a first-parent ancestor of the first revision given, where the commit printed is either a merge-base of the supplied revisions or a merge for which one of its parents (not the first) is a merge-base. The above two doe snot connect at least to me. The second paragraph seems to describe how this mysterious mode decides its output to a sufficient detail, but what the output _means_, and it is unclear how it relates to gitk/git-gui style merges. For example, given the history: A---C---G \ B-D---F \ E we have: ... $ git log --left-right F...E --not $(git merge-base --merge-child F E) F E The git-log case is useful because it allows us to limit the range of commits that we are examining for patch-identical changes when using --cherry. Hmph, is this reinventing ancestry-path in a different way? At the low level machinery, you are finding D to show only F and E, and your goal seems to be to ignore the side ancestry A--C--G, but it is not clear if you prefer E D F(which would be what F...E would give in a history limited to ancestry-path, ignoring C) over E F. For example with git-gui in git.git I know that anything before the last merge of git-gui is not interesting: Can this be extended to find the second last such merge? Or is the last one always special? Still skeptical, but I'll let people discuss it during the feature freeze ;-). -- To unsubscribe from this list: send the line 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: [PULL] git-svn updates for 1.8.3-rc2
Thanks, pulled. -- To unsubscribe from this list: send the line 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: Cannot push anything via export transport helper after push fails.
В Sat, 11 May 2013 08:57:14 -0500 Felipe Contreras felipe.contre...@gmail.com пишет: The problem seems to be that git fast-export updates marks unconditionally, whether export actually applied or not. So next time it assumes everything is already exported and does nothing. Is it expected behavior? Indeed, this is the way it currently works, and it's not easy to fix. We would need some way to make fast-export wait until we know the exit status of the remote helper, and then tell it when it failed, so the marks are not updated. One possibility would be to omit *export-marks and manage GIT marks in remote helper as well. Helper would then update synchronously both GIT and BZR marks if no errors were detected. Or even better, it could update just those commits that had been successful. However, the way remote-bzr/hg work is that the commits are still there anyway. So if you merge the next time you push those commits are already converted, so it's not a problem if fast-export is not exporting them again. As I understand bzr commit ID is stable. What happens if we try to commit the same ID second time? So even though it's not ideal, it should work. I'm more concerned about transport errors. Any network glitch during push renders you repository unusable (at least, without much efforts). The problem is when the remote-helper crashes and the marks of fast-export and the remote-helper are out of sync, and then the user is really screwed. This case would benefit from moving processing of GIT marks into remote helper as well. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/2] merge-base: add --merge-child option
On Sat, May 11, 2013 at 10:54:12AM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: This is helpful when examining branches with disjoint roots, for example because one is periodically merged into a subtree of the other. With the --merge-child option, git merge-base will print a first-parent ancestor of the first revision given, where the commit printed is either a merge-base of the supplied revisions or a merge for which one of its parents (not the first) is a merge-base. The above two doe snot connect at least to me. The second paragraph seems to describe how this mysterious mode decides its output to a sufficient detail, but what the output _means_, and it is unclear how it relates to gitk/git-gui style merges. For example, given the history: A---C---G \ B-D---F \ E we have: ... $ git log --left-right F...E --not $(git merge-base --merge-child F E) F E The git-log case is useful because it allows us to limit the range of commits that we are examining for patch-identical changes when using --cherry. Hmph, is this reinventing ancestry-path in a different way? At the low level machinery, you are finding D to show only F and E, and your goal seems to be to ignore the side ancestry A--C--G, but it is not clear if you prefer E D F(which would be what F...E would give in a history limited to ancestry-path, ignoring C) over E F. I hadn't considered ancestry-path, but I don't think it does what I want. What I want if for LEFT to be B--D--F and RIGHT to be B--E, ignoring A--C--G because I know that none of those are patch identical to anything in B--E. So what I want is more descendant-path than ancestry path in that I don't want anything that isn't a descendant of the merge base of the supplied arguments. For example with git-gui in git.git I know that anything before the last merge of git-gui is not interesting: Can this be extended to find the second last such merge? Or is the last one always special? In this implementation it only finds the last one because that's where the merge base is. Still skeptical, but I'll let people discuss it during the feature freeze ;-). I'm not convinced this is easy to explain myself, which may make it a bad idea. Perhaps a --descendant-path argument to git-log is a better way to help with this case. -- To unsubscribe from this list: send the line 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] test-bzr: Do not use unportable sed \+
Torsten Bögershausen tbo...@web.de writes: Using sed -e '/[0-9]\+//' to find one ore more digits is not portable. Use the Basic Regular Expression '/[0-9][0-9]*//' instead Signed-off-by: Torsten Bögershausen tbo...@web.de Thanks. Is there another one in t/t5551-http-fetch.sh that checks the tags? --- contrib/remote-helpers/test-bzr.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh index d9c32f4..5dfa070 100755 --- a/contrib/remote-helpers/test-bzr.sh +++ b/contrib/remote-helpers/test-bzr.sh @@ -328,7 +328,7 @@ test_expect_success 'strip' ' echo four content bzr commit -m four - bzr log --line | sed -e s/^[0-9]\+: // ../expected + bzr log --line | sed -e s/^[0-9][0-9]*: // ../expected ) (cd gitrepo -- To unsubscribe from this list: send the line 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: [ANN] git-tbdiff: topic branch diff
Thomas Rast tr...@inf.ethz.ch writes: In this case the matching up is trivial, but you can see that it clearly shows the added Signoffs and edited parts of message and patch. Have fun, and let me know if it breaks! Nice. I need to do this kind of comparison quite often, when a rerolled series is received. Since there is no reason to re_base_ the series with with an updated one on a different fork-point, I tend to do: $ git checkout master...tr/topic $ git am -s3c ./+tr-topic-2.mbox ... inspect the differences between tr/topic and HEAD ... $ git branch -f tr/topic where the first step finds the bottom boundary of the previous iteration and detaches the HEAD at the commit, the second step applies the rerolled series and the topic branch is updated in the final step. The inspect the differences step can be helped if I can use this tool to say $ git tbdiff master..tr/topic master..HEAD It would probably be trivial to extend the command line parser to also accept $ git tbdiff tr/topic... but that is an icing on the cake. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANN] Mestral – Hook management for Git
Hi, today I'd like to introduce a new tool to manage Git hooks. It's called Mestral and is available at GitHub: https://github.com/mestral/mestral It is written in Ruby and inspired by Homebrew (http://brew.sh). It uses Git itself to load and update hooks and aims to be easy to use for both users and hook authors. Currently, it's an early proof of concept and I'm looking for some early adopters who want to try it, give valuable feedback and maybe contribute to the development. That means any feedback is appreciated, may it be a short tweet to @k0raktor, a reply to this mail, an issue on GitHub or a pull request. Best regards, Sebastian -- To unsubscribe from this list: send the line 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] test-bzr: Do not use unportable sed \+
Junio C Hamano gits...@pobox.com writes: Thanks. Is there another one in t/t5551-http-fetch.sh that checks the tags? I think your sed will see the same breakage for the one in 5551 (my sed is unfortunately GNU and .\+ does not break it). Could you test this patch with: GIT_TEST_LONG=YesPlease GIT_TEST_HTTPD=YesPlease \ ./t5551-http-fetch.sh Thanks. t/t5551-http-fetch.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh index b23efbb..4a3184e 100755 --- a/t/t5551-http-fetch.sh +++ b/t/t5551-http-fetch.sh @@ -209,7 +209,7 @@ test_expect_success EXPENSIVE 'create 50,000 tags in the repo' ' # now assign tags to all the dangling commits we created above tag=$($PERL_PATH -e print \bla\ x 30) - sed -e s/^:\(.\+\) \(.\+\)$/\2 refs\/tags\/$tag-\1/ marks packed-refs + sed -e s|^:\([^ ]*\) \(.*\)$|\2 refs/tags/$tag-\1| marks packed-refs ) ' -- To unsubscribe from this list: send the line 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] patch-ids.c: cache patch IDs in a notes tree
This adds a new configuration variable patchid.cacheRef which controls whether (and where) patch IDs will be cached in a notes tree. Caching patch IDs in this way results in a performance improvement in every case I have tried, for example when comparing the git-gui tree to git.git (where git-gui has been merged into git.git but most git.git commits do not appear in git-gui): Without patch ID caching: $ time git log --cherry master...git-gui/master /dev/null real0m32.981s user0m32.364s sys 0m0.621s Prime the cache: $ time git -c patchid.cacheref=patchids log --cherry \ master...git-gui/master /dev/null real0m33.860s user0m32.832s sys 0m0.986s With all patch IDs cached: $ time git -c patchid.cacheref=patchids log --cherry \ master...git-gui/master /dev/null real0m1.041s user0m0.679s sys 0m0.363s Signed-off-by: John Keeping j...@keeping.me.uk --- This is another approach to fixing the log --cherry takes a long time issue I encountered comparing commits built on git-gui to those in git.git [1][2]. I think this is a useful feature even outside that use case. I measured a small improvement (when the cache is primed) even comparing two branches with 1 and 2 different commits respectively. [1] http://article.gmane.org/gmane.comp.version-control.git/223959 [2] http://article.gmane.org/gmane.comp.version-control.git/223956 Documentation/config.txt | 7 +++ patch-ids.c | 91 +++- patch-ids.h | 1 + t/t6007-rev-list-cherry-pick-file.sh | 16 +++ 4 files changed, 114 insertions(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 6e53fc5..e36585c 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1798,6 +1798,13 @@ pager.cmd:: precedence over this option. To disable pagination for all commands, set `core.pager` or `GIT_PAGER` to `cat`. +patchid.cacheRef:: + The name of a notes ref in which to store patch IDs for commits. + The ref is taken to be in `refs/notes/` if it is not qualified. ++ +Note that the patch ID notes are never pruned automatically, so you may +wish to periodically run `git notes --ref ref prune` against this ref. + pretty.name:: Alias for a --pretty= format string, as specified in linkgit:git-log[1]. Any aliases defined here can be used just diff --git a/patch-ids.c b/patch-ids.c index bc8a28f..cb05eec 100644 --- a/patch-ids.c +++ b/patch-ids.c @@ -1,6 +1,9 @@ #include cache.h +#include blob.h #include diff.h #include commit.h +#include notes.h +#include refs.h #include sha1-lookup.h #include patch-ids.h @@ -34,12 +37,38 @@ struct patch_id_bucket { struct patch_id bucket[BUCKET_SIZE]; }; +static int patch_id_config(const char *var, const char *value, void *cb) +{ + const char **cacheref = cb; + + if (!strcmp(var, patchid.cacheref)) + return git_config_string(cacheref, var, value); + + return 0; +} + int init_patch_ids(struct patch_ids *ids) { + char *cacheref = NULL; + memset(ids, 0, sizeof(*ids)); diff_setup(ids-diffopts); DIFF_OPT_SET(ids-diffopts, RECURSIVE); diff_setup_done(ids-diffopts); + + git_config(patch_id_config, cacheref); + if (cacheref) { + struct strbuf sb = STRBUF_INIT; + strbuf_addstr(sb, cacheref); + expand_notes_ref(sb); + + ids-cache = xcalloc(1, sizeof(*ids-cache)); + init_notes(ids-cache, sb.buf, combine_notes_overwrite, 0); + + strbuf_release(sb); + free(cacheref); + } + return 0; } @@ -52,9 +81,67 @@ int free_patch_ids(struct patch_ids *ids) next = patches-next; free(patches); } + + if (ids-cache) { + unsigned char notes_sha1[20]; + if (write_notes_tree(ids-cache, notes_sha1) || + update_ref(patch-id: update cache, ids-cache-ref, + notes_sha1, NULL, 0, QUIET_ON_ERR)) + error(_(failed to write patch ID cache)); + + free_notes(ids-cache); + ids-cache = NULL; + } + return 0; } +static int load_cached_patch_id(struct commit *commit, + struct notes_tree *cache, unsigned char *sha1) +{ + const unsigned char *note_sha1; + char *note; + enum object_type type; + unsigned long notelen; + int result = 1; + + if (!cache) + return 1; + + note_sha1 = get_note(cache, commit-object.sha1); + if (!note_sha1) + return 1; + + if (!(note = read_sha1_file(note_sha1, type, notelen)) || !notelen || + type != OBJ_BLOB) + goto out; + + if (get_sha1_hex(note, sha1)) + goto out; + +
Re: [PATCH] test-bzr: Do not use unportable sed \+
On 11.05.13 21:45, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Thanks. Is there another one in t/t5551-http-fetch.sh that checks the tags? I think your sed will see the same breakage for the one in 5551 (my sed is unfortunately GNU and .\+ does not break it). Could you test this patch with: GIT_TEST_LONG=YesPlease GIT_TEST_HTTPD=YesPlease \ ./t5551-http-fetch.sh Thanks. t/t5551-http-fetch.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh index b23efbb..4a3184e 100755 --- a/t/t5551-http-fetch.sh +++ b/t/t5551-http-fetch.sh @@ -209,7 +209,7 @@ test_expect_success EXPENSIVE 'create 50,000 tags in the repo' ' # now assign tags to all the dangling commits we created above tag=$($PERL_PATH -e print \bla\ x 30) - sed -e s/^:\(.\+\) \(.\+\)$/\2 refs\/tags\/$tag-\1/ marks packed-refs + sed -e s|^:\([^ ]*\) \(.*\)$|\2 refs/tags/$tag-\1| marks packed-refs ) I did, the interesting thing is that the test passes with and without your patch. (After enabling GIT_TEST_LONG and GIT_TEST_HTTPD in both cases) Side note: I added this line in in t/check-non-portable-shell.pl /sed[^]+[^]+\\([+])/ and err sed \\$1 is not portable); -- To unsubscribe from this list: send the line 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] test-bzr: Do not use unportable sed \+
Torsten Bögershausen tbo...@web.de writes: I did, the interesting thing is that the test passes with and without your patch. (After enabling GIT_TEST_LONG and GIT_TEST_HTTPD in both cases) Strange. Do you see differences between the produced packed-refs file? -- To unsubscribe from this list: send the line 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] test-bzr: Do not use unportable sed \+
On 05/11/2013 03:25 PM, Torsten Bögershausen wrote: Sorry, I forgot to mention that there is another test case that fails in test-bzr.sh (Both Linux and MacOS): Cloning into 'gitrepo'... --- ../expected 2013-05-11 20:07:17.678360248 + +++ actual 2013-05-11 20:07:21.510312073 + @@ -1,3 +1,3 @@ -origin/HEAD +origin origin/branch origin/trunk not ok 11 - proper bzr repo -- To unsubscribe from this list: send the line 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] test-bzr: Do not use unportable sed \+
On 11.05.13 22:09, Junio C Hamano wrote: Torsten Bögershausen tbo...@web.de writes: I did, the interesting thing is that the test passes with and without your patch. (After enabling GIT_TEST_LONG and GIT_TEST_HTTPD in both cases) Strange. Do you see differences between the produced packed-refs file? The original version seems to look like this: :1 666527db455708922859283c673094002092910b :2 1e2acf73c6db881cfb1d56d67662e3d9260be2cf [snip] The fixed POSIX version follows that style: 666527db455708922859283c673094002092910b refs/tags/blablablablablablablablablablablablablablablablablablablablablablablablablablablablablabla-1 1e2acf73c6db881cfb1d56d67662e3d9260be2cf refs/tags/blablablablablablablablablablablablablablablablablablablablablablablablablablablablablabla-2 [snip] (Just to verify the changes:) diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh index b23efbb..68965f0 100755 --- a/t/t5551-http-fetch.sh +++ b/t/t5551-http-fetch.sh @@ -210,8 +210,11 @@ test_expect_success EXPENSIVE 'create 50,000 tags in the repo' ' # now assign tags to all the dangling commits we created above tag=$($PERL_PATH -e print \bla\ x 30) sed -e s/^:\(.\+\) \(.\+\)$/\2 refs\/tags\/$tag-\1/ marks packed-refs + sed -e s|^:\([^ ]*\) \(.*\)$|\2 refs/tags/$tag-\1| marks packed-refs.junio + sed -e s/^:\(.\+\) \(.\+\)$/\2 refs\/tags\/$tag-\1/ marks packed-refs.orig ) ' + exit 0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] patch-ids.c: cache patch IDs in a notes tree
On Sat, May 11, 2013 at 12:54 PM, John Keeping j...@keeping.me.uk wrote: This adds a new configuration variable patchid.cacheRef which controls whether (and where) patch IDs will be cached in a notes tree. Patch ID's aren't stable wrt different diff options, so I think this is potentially a very bad idea. Linus -- To unsubscribe from this list: send the line 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: Cannot push anything via export transport helper after push fails.
On Sat, May 11, 2013 at 1:48 PM, Andrey Borzenkov arvidj...@gmail.com wrote: В Sat, 11 May 2013 08:57:14 -0500 Felipe Contreras felipe.contre...@gmail.com пишет: The problem seems to be that git fast-export updates marks unconditionally, whether export actually applied or not. So next time it assumes everything is already exported and does nothing. Is it expected behavior? Indeed, this is the way it currently works, and it's not easy to fix. We would need some way to make fast-export wait until we know the exit status of the remote helper, and then tell it when it failed, so the marks are not updated. One possibility would be to omit *export-marks and manage GIT marks in remote helper as well. Helper would then update synchronously both GIT and BZR marks if no errors were detected. Or even better, it could update just those commits that had been successful. That would need to change the whole architecture, because right now the remote helpers are agnostic of Git SHA-1s. However, the way remote-bzr/hg work is that the commits are still there anyway. So if you merge the next time you push those commits are already converted, so it's not a problem if fast-export is not exporting them again. As I understand bzr commit ID is stable. What happens if we try to commit the same ID second time? It's skipped, because it's already converted. So even though it's not ideal, it should work. I'm more concerned about transport errors. Any network glitch during push renders you repository unusable (at least, without much efforts). No, it doesn't. If the remote-helper fails gracefully, the bzr revisions are converted and stored in the bzr repo, even if they were not pushed to the remote. So it's OK if fast-export never exports them again; we already have them. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line 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] git-prompt.zsh: introduce thin ZSH wrapper
On Sat, May 11, 2013 at 11:25 AM, Ramkumar Ramachandra artag...@gmail.com wrote: To facilitate a colored prompt in ZSH, write a thin wrapper around git-prompt.sh, factoring out and overriding the coloring logic. Since ZSH lacks a PROMPT_COMMAND, instruct the user to execute __git_ps1 inside precmd(). I think this is two logical changes into one. --- /dev/null +++ b/contrib/completion/git-prompt.zsh @@ -0,0 +1,59 @@ +# git prompt support for zsh: wrapper around git-prompt.sh +# +# To enable: +# +#1) Copy this file and git-prompt.sh to ~/.zsh/prompt +#2) Add the following lines to your .zshrc: +# +# source ~/.zsh/prompt/git-prompt.zsh +# GIT_PS1_SHOWCOLORHINTS=true +# precmd () { __git_ps1 %n| :%~$ %s } +# +#3) You can now add the following to ~/.zshrc and expect the +# characters to be displayed in color: +# +# GIT_PS1_DESCRIBE_STYLE=branch +# GIT_PS1_SHOWUPSTREAM=auto +# GIT_PS1_SHOWDIRTYSTATE=true +# GIT_PS1_SHOWUNTRACKEDFILES=true + +test -z $script script=$(dirname ${funcsourcetrace[1]%:*})/git-prompt.sh +ZSH_VERSION='' . $script I've been thinking that this method of loading the script is not the best; we should probably have a list of locations where distributions usually put this file. So if we could avoid the creation of of a new file, that would be great. +autoload colors +colors + +__git_ps1_colorize_gitstring () +{ + local c_red='%F{red}' + local c_green='%F{green}' + local c_lblue='%F{blue}' + local c_clear='%f' + local bad_color=$c_red + local ok_color=$c_green + local branch_color=$c_clear + local flags_color=$c_lblue + local branchstring=$c${b##refs/heads/} This is the only real difference with bash colors, no? I think it's overkill to create a new file, and load the old one, only to override part of a function. We should probably start with everything in the same file, and only later if we find it's necessary, split. + if [ $detached = no ]; then + branch_color=$ok_color + else + branch_color=$bad_color + fi + + gitstring=$branch_color$branchstring$c_clear + + if [ $w = * ]; then + gitstring=$gitstring$bad_color$w + fi + if [ -n $i ]; then + gitstring=$gitstring$ok_color$i + fi + if [ -n $s ]; then + gitstring=$gitstring$flags_color$s + fi + if [ -n $u ]; then + gitstring=$gitstring$bad_color$u + fi + gitstring=$gitstring$c_clear$r$p +} -- -- Felipe Contreras -- To unsubscribe from this list: send the line 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] patch-ids.c: cache patch IDs in a notes tree
On Sat, May 11, 2013 at 02:10:01PM -0700, Linus Torvalds wrote: On Sat, May 11, 2013 at 12:54 PM, John Keeping j...@keeping.me.uk wrote: This adds a new configuration variable patchid.cacheRef which controls whether (and where) patch IDs will be cached in a notes tree. Patch ID's aren't stable wrt different diff options, so I think this is potentially a very bad idea. Hmm... I hadn't realised that. Looking a bit closer, it looks like init_patch_ids sets up its own diffopts so its not affected by the command line (except for pathspecs which would be easy to check for). Of course that still means it can be affected by settings in the user's configuration. It's a pity that this can't be done since it gives a significant performance improvement for some tasks that I perform relatively frequently. Is there a reason patch IDs couldn't be generated using fixed diff options? Since there's no way to control it from the command line it seems surprising that the results of log --cherry might be different based on what's in your config. That could go either way I suppose - is it useful to be able to change patch IDs based on command line arguments or is it wrong that as we add persistent diff settings to the configuration we've been silently changing the behaviour of git patch-id and git cherry. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] git-prompt.sh: colorize ZSH prompt
Add colors suitable for use in the ZSH prompt. Having learnt that the ZSH equivalent of PROMPT_COMMAND is precmd (), you can now use GIT_PS1_SHOWCOLORHINTS with ZSH. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- You like this more? I don't mind going either way. contrib/completion/git-prompt.sh | 67 1 file changed, 47 insertions(+), 20 deletions(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 08c9b22..26e5bc2 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -20,7 +20,8 @@ #post, which are strings you would put in $PS1 before #and after the status string generated by the git-prompt #machinery. e.g. -# PROMPT_COMMAND='__git_ps1 \u@\h:\w \\\$ ' +#Bash: PROMPT_COMMAND='__git_ps1 \u@\h:\w \\\$ ' +#ZSH: precmd () { __git_ps1 %n :%~$ |%s } #will show username, at-sign, host, colon, cwd, then #various status string, followed by dollar and SP, as #your prompt. @@ -363,10 +364,18 @@ __git_ps1 () if [ $pcmode = yes ]; then local gitstring= if [ -n ${GIT_PS1_SHOWCOLORHINTS-} ]; then - local c_red='\e[31m' - local c_green='\e[32m' - local c_lblue='\e[1;34m' - local c_clear='\e[0m' + if [[ -n ${ZSH_VERSION-} ]]; then + local c_red='%F{red}' + local c_green='%F{green}' + local c_lblue='%F{blue}' + local c_clear='%f' + else + local c_red='\e[31m' + local c_green='\e[32m' + local c_lblue='\e[1;34m' + local c_clear='\e[0m' + fi + local bad_color=$c_red local ok_color=$c_green local branch_color=$c_clear @@ -379,23 +388,41 @@ __git_ps1 () branch_color=$bad_color fi - # Setting gitstring directly with \[ and \] around colors - # is necessary to prevent wrapping issues! - gitstring=\[$branch_color\]$branchstring\[$c_clear\] + if [[ -n ${ZSH_VERSION-} ]]; then + gitstring=$branch_color$branchstring$c_clear - if [ $w = * ]; then - gitstring=$gitstring\[$bad_color\]$w - fi - if [ -n $i ]; then - gitstring=$gitstring\[$ok_color\]$i - fi - if [ -n $s ]; then - gitstring=$gitstring\[$flags_color\]$s - fi - if [ -n $u ]; then - gitstring=$gitstring\[$bad_color\]$u + if [ $w = * ]; then + gitstring=$gitstring$bad_color$w + fi + if [ -n $i ]; then + gitstring=$gitstring$ok_color$i + fi + if [ -n $s ]; then + gitstring=$gitstring$flags_color$s + fi + if [ -n $u ]; then + gitstring=$gitstring$bad_color$u + fi + gitstring=$gitstring$c_clear$r$p + else + # Setting gitstring directly with \[ and \] around colors + # is necessary to prevent wrapping issues! + gitstring=\[$branch_color\]$branchstring\[$c_clear\] + + if [ $w = * ]; then + gitstring=$gitstring\[$bad_color\]$w + fi + if [ -n $i ]; then + gitstring=$gitstring\[$ok_color\]$i + fi +
Re: [PATCH v2] git-prompt.sh: colorize ZSH prompt
On Sat, May 11, 2013 at 5:18 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Add colors suitable for use in the ZSH prompt. Having learnt that the ZSH equivalent of PROMPT_COMMAND is precmd (), you can now use GIT_PS1_SHOWCOLORHINTS with ZSH. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- You like this more? I don't mind going either way. Not really. If we need to avoid the \[\], it makes sense to have a separate function, but what I meant is that this function should be initially on the same file, and created in a separate patch. -- Felipe Contreras -- To unsubscribe from this list: send the line 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] git-prompt.sh: colorize ZSH prompt
Felipe Contreras wrote: Not really. If we need to avoid the \[\], it makes sense to have a separate function, but what I meant is that this function should be initially on the same file, and created in a separate patch. What are you saying? 1. It makes sense to have a separate function specifically for ZSH, but it should be located in the same physical file (why?) 2. You like the original patch, but would prefer two different parts (a nit) -- To unsubscribe from this list: send the line 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] patch-ids.c: cache patch IDs in a notes tree
On Sat, May 11, 2013 at 2:49 PM, John Keeping j...@keeping.me.uk wrote: Hmm... I hadn't realised that. Looking a bit closer, it looks like init_patch_ids sets up its own diffopts so its not affected by the command line (except for pathspecs which would be easy to check for). Of course that still means it can be affected by settings in the user's configuration. .. and in the actual diff algorithm. The thing is - patches ARE NOT STABLE. There are many valid ways to get a patch from one version to another, and even without command line changes, we've had different versions of git generating different patches. There are heuristics in xdiff to avoid some nasty use up tons of CPU-time things that have been tweaked over time. And even for simple cases there are ambiguous ways to describe the patch. Now, maybe we don't care, because in practice, most of the time this just doesn't much matter. And anybody who uses patch-ID's had better not rely on them being guaranteed to be unique anyway, so it's more of a if the patch ID is the same, it's almost guaranteed to be the same patch, but that's a big almost. And no, it's not some theoretical SHA1 collisions are very unlikely kind of thing, it's a the patch ID actually ignores a lot of data in order to give the same ID even if lins have been added above it, and the patch is at different line numbers etc. So maybe it doesn't matter. But at the same time, I really think caching patch ID's should be something people should be aware of is fundamentally wrong, even if it might work. And quite frankly, if you do rebases etc so much that you think patch ID's are so important that they need to be cached, you may be doing odd/wrong things. Linus -- To unsubscribe from this list: send the line 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] git-prompt.sh: colorize ZSH prompt
On Sat, May 11, 2013 at 5:40 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Felipe Contreras wrote: Not really. If we need to avoid the \[\], it makes sense to have a separate function, but what I meant is that this function should be initially on the same file, and created in a separate patch. What are you saying? 1. It makes sense to have a separate function specifically for ZSH, but it should be located in the same physical file (why?) This is what I mean: commit 4726a5f76ec7d96c34863d6640488e2183cc8f00 Author: Ramkumar Ramachandra artag...@gmail.com Date: Sat May 11 21:55:13 2013 +0530 prompt: split code into __git_ps1_colorize_gitstring() Will be useful for reorganizations. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index eaf5c36..1187292 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -222,6 +222,45 @@ __git_ps1_show_upstream () } +__git_ps1_colorize_gitstring () +{ + local c_red='\e[31m' + local c_green='\e[32m' + local c_lblue='\e[1;34m' + local c_clear='\e[0m' + local bad_color=$c_red + local ok_color=$c_green + local branch_color=$c_clear + local flags_color=$c_lblue + local branchstring=$c${b##refs/heads/} + + if [ $detached = no ]; then + branch_color=$ok_color + else + branch_color=$bad_color + fi + + # Setting gitstring directly with \[ and \] around colors + # is necessary to prevent wrapping issues! + gitstring=\[$branch_color\]$branchstring\[$c_clear\] + + if [ -n $w$i$s$u$r$p ]; then + gitstring=$gitstring + fi + if [ $w = * ]; then + gitstring=$gitstring\[$bad_color\]$w + fi + if [ -n $i ]; then + gitstring=$gitstring\[$ok_color\]$i + fi + if [ -n $s ]; then + gitstring=$gitstring\[$flags_color\]$s + fi + if [ -n $u ]; then + gitstring=$gitstring\[$bad_color\]$u + fi + gitstring=$gitstring\[$c_clear\]$r$p +} # __git_ps1 accepts 0 or 1 arguments (i.e., format string) # when called from PS1 using command substitution @@ -363,42 +402,7 @@ __git_ps1 () if [ $pcmode = yes ]; then local gitstring= if [ -n ${GIT_PS1_SHOWCOLORHINTS-} ]; then - local c_red='\e[31m' - local c_green='\e[32m' - local c_lblue='\e[1;34m' - local c_clear='\e[0m' - local bad_color=$c_red - local ok_color=$c_green - local branch_color=$c_clear - local flags_color=$c_lblue - local branchstring=$c${b##refs/heads/} - - if [ $detached = no ]; then - branch_color=$ok_color - else - branch_color=$bad_color - fi - - # Setting gitstring directly with \[ and \] around colors - # is necessary to prevent wrapping issues! - gitstring=\[$branch_color\]$branchstring\[$c_clear\] - - if [ -n $w$i$s$u$r$p ]; then - gitstring=$gitstring - fi - if [ $w = * ]; then - gitstring=$gitstring\[$bad_color\]$w - fi - if [ -n $i ]; then - gitstring=$gitstring\[$ok_color\]$i - fi - if [ -n $s ]; then - gitstring=$gitstring\[$flags_color\]$s - fi - if [ -n $u ]; then - gitstring=$gitstring\[$bad_color\]$u - fi - gitstring=$gitstring\[$c_clear\]$r$p + __git_ps1_colorize_gitstring else gitstring=$c${b##refs/heads/}${f:+ $f}$r$p fi commit 23d89b370ac36401da1d9a98754e222a7a4c93e5 Author: Felipe Contreras felipe.contre...@gmail.com Date: Sat May 11 18:03:39 2013 -0500 prompt: add colorization for zsh Signed-off-by: Felipe Contreras felipe.contre...@gmail.com diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 1187292..8946a29 100644 --- a/contrib/completion/git-prompt.sh +++
Re: [PATCH 1/8] am: suppress error output from a conditional
Junio C Hamano wrote: If you are introducing dotest exists but next/last may not be present as a valid new state, it probably should check the presence and/or absence of them explicitly, Um, a test -f preceding the cat? Why, when cat implicitly checks existence anyway? but more importantly, it is a good idea to move test $# != 0 higher. This? diff --git a/git-am.sh b/git-am.sh index 47c1021..1e10e31 100755 --- a/git-am.sh +++ b/git-am.sh @@ -446,9 +446,9 @@ done # If the dotest directory exists, but we have finished applying all the # patches in them, clear it out. if test -d $dotest + test $# != 0 last=$(cat $dotest/last 2/dev/null) next=$(cat $dotest/next 2/dev/null) - test $# != 0 test $next -gt $last then rm -fr $dotest Earlier it did not matter because when $dotest existed, next/last were supposed to be there and absence of them was an error codepath. Now, missing these files is not an error but is a perfectly normal state, It's not a normal state for the purposes of git-am.sh. There is no codepath in the program that depends only on $dotest, but not next/last. I would frame this as: checking for $dotest is not a sufficient prerequisite anymore; we have to additionally look for next/last. To git-am.sh, an empty $dotest or just a $dotest/autostash is equivalent to no $dotest at all. so checking what can be checked more cheaply makes sense as a general code hygiene. Yeah, I agree. See am: tighten a conditional that checks for $dotest, where I get away with just checking $dotest/last (and assume that $dotest/next is present by extension). How do I apply your suggestion to this particular patch? -- To unsubscribe from this list: send the line 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] patch-ids.c: cache patch IDs in a notes tree
Hi John Linus, On Sat, 11 May 2013, Linus Torvalds wrote: [...] I really think caching patch ID's should be something people should be aware of is fundamentally wrong, even if it might work. Given the incredible performance win, I would say it is still worth looking into. If you store also a hash of Git version and diff options (may even be the hash of the raw bytes of the diff options if you do not plan to share the ref between machines) with the patch ID, you can make it safe. That hash would be generated at patch_id init time and load_cached_patch_id() would check this hash in addition to the return value of get_sha1() (and ignore the note if the version/diff options differ). If you are following git.git slavishly, maybe hashing just the major/minor Git version would be in order to avoid frequent regeneration of identical patch IDs. And quite frankly, if you do rebases etc so much that you think patch ID's are so important that they need to be cached, you may be doing odd/wrong things. AFAICT John actually gave a very valid scenario that validates his use case: git-gui patches are best tested in the git.git scenario but have to be contributed via git-gui.git. It's not John's fault that this typically requires a lot of rebasing between vastly divergent histories. John, do you think you can incorporate that finger-print of the patch ID generation and store it in the same note? Ciao, Johannes -- To unsubscribe from this list: send the line 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] git-gui: fix file name handling with non-empty prefix
Sorry for the late reply. I was able to reproduce the problem that you were describing a while ago. And your patch indeed fixes it. It's a much more elegant way of dealing with the absolute vs relative path problem that I was trying to fix. Thanks! As for Pat, I'm not sure wha'ts going on with his email address. It was working back in October, and his username still seems to be active over at SourceForge... let's see if this email reaches him. Here's a link for his reference just in case he missed your original email: http://thread.gmane.org/gmane.comp.version-control.git/222646 On 04/27/13 10:18, John Keeping wrote: I got a bounce with 550 no such user for Pat's email address when sending this. Does anyone have more up-to-date contact details? Or is it just SourceForge being broken? On Sat, Apr 27, 2013 at 02:24:16PM +0100, John Keeping wrote: Commit e3d06ca (git-gui: Detect full path when parsing arguments - 2012-10-02) fixed the handling of absolute paths passed to the browser and blame subcommands by checking whether the file exists without the prefix before prepending the prefix and checking again. Since we have chdir'd to the top level of the working tree before doing this, this does not work if a file with the same name exists in a subdirectory and at the top level (for example Makefile in git.git's t/ directory). Instead of doing this, revert that patch and fix absolute path issue by using file join to prepend the prefix to the supplied path. This will correctly handle absolute paths by skipping the prefix in that case. Signed-off-by: John Keeping j...@keeping.me.uk --- git-gui.sh | 14 +++--- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index e11..a94ad7f 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -3003,19 +3003,11 @@ blame { set jump_spec {} set is_path 0 foreach a $argv { -if {[file exists $a]} { -if {$path ne {}} usage -set path [normalize_relpath $a] -break -} elseif {[file exists $_prefix$a]} { -if {$path ne {}} usage -set path [normalize_relpath $_prefix$a] -break -} +set p [file join $_prefix $a] -if {$is_path} { +if {$is_path || [file exists $p]} { if {$path ne {}} usage -set path [normalize_relpath $_prefix$a] +set path [normalize_relpath $p] break } elseif {$a eq {--}} { if {$path ne {}} { -- 1.8.3.rc0.149.g98a72f2.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] patch-ids.c: cache patch IDs in a notes tree
Linus Torvalds torva...@linux-foundation.org writes: On Sat, May 11, 2013 at 2:49 PM, John Keeping j...@keeping.me.uk wrote: Hmm... I hadn't realised that. Looking a bit closer, it looks like init_patch_ids sets up its own diffopts so its not affected by the command line (except for pathspecs which would be easy to check for). Of course that still means it can be affected by settings in the user's configuration. .. and in the actual diff algorithm. As to the objection side of the argument, I already said essentially the same thing several months ago: http://thread.gmane.org/gmane.comp.version-control.git/202654/focus=202898 and do not have much to add [*1*]. However. The use of patch-id in cherry and rebase is to facilitate avoiding to replay commits that are obviously identical to the ones you have in your history. The cached patch id for an existing old commit may differ from a patch id you freshly compute for a new commit you are trying to see if it truly new, even though they may represent the same change. So we may incorrectly think such a new commit is not yet in your history and attempt to replay it. But it is not a big problem. Either 3-way merge notices that there is nothing new, or you get a conflict and have chance to inspect what is going on. A conceptually much larger and more problematic issue is that we may discard a truly new change that you still need as an old one you already have due to a hash collision and discard it. Because the hash space of SHA-1 is so large, however, it is not a problem in practice, and more importantly, that hash space is just as large as the hash space used by Git to reduce a patch to a patch id, the filtering done with patch-id in cherry and rebase _already_ have that exact problem with or without this additional cache layer. A stale cache may make the possibility of lost change due to such a hash collision merely twice as likely. ... it's a the patch ID actually ignores a lot of data in order to give the same ID even if lins have been added above it, and the patch is at different line numbers etc. Yes. So maybe it doesn't matter. But at the same time, I really think caching patch ID's should be something people should be aware of is fundamentally wrong, even if it might work. I do not think it is caching patch ID that people should be aware of is fundamentally wrong. What is fundamentally wrong, even if it might work, is using patch ID itself. And quite frankly, if you do rebases etc so much that you think patch ID's are so important that they need to be cached, you may be doing odd/wrong things. And that, too ;-) [Footnote] *1* For people listening from the sidelines, the fact that Git algorithm can improve over time is a real issue, and and has caused one issue that still hasn't been solved in the k.org upload process. Somebody who has a repository there could *theoretically*: - push her v1.1 release via Git (git push origin v1.1); - create a tarball (git archive -o v1.1.tar v1.1) and diff since the last release (git diff v1.0 v1.1 v1.0-v1.1.diff) locally; - GPG sign them (gpg -b v1.1.tar, gpg -b v1.0-v1.1.diff); and - upload only the signature files and have k.org create the tarballs and diff to save bandwidth of uploading logically derivable stuff over and over again. But that can be done only when output from git archive and git diff are stable, which is not the case. We changed how extended header fields are used in the tar archive for a long pathname recently, and also we changed use of XDF_NEED_MINIMAL a couple of years ago in git diff; both of these affect the 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 1/8] am: suppress error output from a conditional
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: If you are introducing dotest exists but next/last may not be present as a valid new state, it probably should check the presence and/or absence of them explicitly, Um, a test -f preceding the cat? Why, when cat implicitly checks existence anyway? With that logic, 'test -d $dotest' on the first line would also be redundant, as we read from $dotest/last and that will catch an error. It is more about what is conceptually right. You need to think what the condition _means_. We protect the remove $dotest directory with the precondition you are changing, but what is that precondition really trying to say? If $dotest does not exist, obviously we do not need to remove it, but what is the essence of that sereis of tests? It is what the comment says: We have finished applying all the patches in them Earlier, presense of $dotest _must_ have meant that last and next should exist (otherwise you have a corrupt state and we did want to see error messages), and the check original did was perfectly fine. Now, a mere presense of $dotest does _not_ mean we have finished applying all the patches, because sometimes you create it without having anything to put in last or next yet. That is why it starts to make sense to do if test -d $dotest test -f $dotest/last test -f $dotest/next last=$(cat $dotest/last) ... -- To unsubscribe from this list: send the line 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] test-bzr: Do not use unportable sed \+
Torsten Bögershausen tbo...@web.de writes: On 11.05.13 22:09, Junio C Hamano wrote: Torsten Bögershausen tbo...@web.de writes: I did, the interesting thing is that the test passes with and without your patch. (After enabling GIT_TEST_LONG and GIT_TEST_HTTPD in both cases) Strange. Do you see differences between the produced packed-refs file? The original version seems to look like this: :1 666527db455708922859283c673094002092910b :2 1e2acf73c6db881cfb1d56d67662e3d9260be2cf [snip] The fixed POSIX version follows that style: 666527db455708922859283c673094002092910b refs/tags/blablablablablablablablablablablablablablablablablablablablablablablablablablablablablabla-1 1e2acf73c6db881cfb1d56d67662e3d9260be2cf refs/tags/blablablablablablablablablablablablablablablablablablablablablablablablablablablablablabla-2 [snip] That means whatever the test that comes after that set-up step to populate the packed-refs file does not need the packed-refs file. In fact, the test only sees if clone succeeds, without checking what refs are present in the resulting repository. The original repository lacks these 5 tags due to the non-portable sed construct (it has a corrupt packed-refs file) and it may or may not have other refs, but because cloning an empty repository does not error out these days, the test just passes. Sloppy. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html