[PATCH v2 0/2] fix broken range_set tests and coalescing
This is a re-roll of a patch [1] which fixes the line-log.c:sort_and_merge_range_set() coalescing bug. This re-roll inserts a new patch before the lone patch from v1. patch 1/2: Fix broken tests in t4211 which should have detected the sort_and_merge_range_set() bug but didn't due to incorrect expected state. Mark the tests as expect-failure. patch 2/2: Fix the sort_and_merge_range_set() coalesce bug. Same as v1 but also flips the tests to expect-success. [1]: http://article.gmane.org/gmane.comp.version-control.git/229774 Eric Sunshine (2): t4211: fix broken test when one -L range is subset of another range_set: fix coalescing bug when range is a subset of another line-log.c | 3 +- t/t4211/expect.multiple-superset | 134 ++- 2 files changed, 133 insertions(+), 4 deletions(-) -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-http: use argv-array
On Tue, Jul 9, 2013 at 7:18 AM, Junio C Hamano gits...@pobox.com wrote: Instead of using a hand-managed argument array, use argv-array API to manage dynamically formulated command line. Signed-off-by: Junio C Hamano gits...@pobox.com --- remote-curl.c | 31 +++ 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/remote-curl.c b/remote-curl.c index 60eda63..884b3a3 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -7,6 +7,7 @@ #include run-command.h #include pkt-line.h #include sideband.h +#include argv-array.h static struct remote *remote; static const char *url; /* always ends with a trailing slash */ @@ -787,36 +788,34 @@ static int push_dav(int nr_spec, char **specs) static int push_git(struct discovery *heads, int nr_spec, char **specs) { struct rpc_state rpc; - const char **argv; - int argc = 0, i, err; + int i, err; + struct argv_array args; + + argv_array_init(args); + argv_array_pushl(args, send-pack, --stateless-rpc, --helper-status); missing NULL sentinel. GCC has the 'sentinel' [1] attribute to catch such errors. Or use macro magic: void argv_array_pushl_(struct argv_array *array, ...); #define argv_array_pushl(array, ...) argv_array_pushl_(array, __VA_ARGS__, NULL) Bert [1] http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html#index-g_t_0040code_007bsentinel_007d-function-attribute-2708 -- To unsubscribe from this list: send the line 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] remote-http: use argv-array
On Tue, Jul 09, 2013 at 08:05:19AM +0200, Bert Wesarg wrote: + argv_array_pushl(args, send-pack, --stateless-rpc, --helper-status); missing NULL sentinel. GCC has the 'sentinel' [1] attribute to catch such errors. Or use macro magic: void argv_array_pushl_(struct argv_array *array, ...); #define argv_array_pushl(array, ...) argv_array_pushl_(array, __VA_ARGS__, NULL) Nice catch. We cannot use variadic macros, because we support pre-C99 compilers that do not have them. But the sentinel attribute is a good idea. Here's a patch. -- 8 -- Subject: [PATCH] argv-array: add sentinel attribute to argv_array_pushl This attribute can help gcc notice when callers forget to add a NULL sentinel to the end of the function. We shouldn't need to #ifdef for other compilers, as __attribute__ is already a no-op on non-gcc-compatible compilers. Suggested-by: Bert Wesarg bert.wes...@googlemail.com Signed-off-by: Jeff King p...@peff.net --- This is our first use of an __attribute__ that is not noreturn or format. I assume this one should be supported on other gcc-compatible compilers like clang. argv-array.h | 1 + 1 file changed, 1 insertion(+) diff --git a/argv-array.h b/argv-array.h index 40248d4..e805748 100644 --- a/argv-array.h +++ b/argv-array.h @@ -15,6 +15,7 @@ void argv_array_pushf(struct argv_array *, const char *fmt, ...); void argv_array_push(struct argv_array *, const char *); __attribute__((format (printf,2,3))) void argv_array_pushf(struct argv_array *, const char *fmt, ...); +__attribute__((sentinel)) void argv_array_pushl(struct argv_array *, ...); void argv_array_pop(struct argv_array *); void argv_array_clear(struct argv_array *); -- 1.8.3.rc3.24.gec82cb9 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] git svn geotrust certificate problem
Thanks for trying to help me. I'v found an issue - that happend because i had system username written in russian language. When i'v changed to user with english characres username everything started to work ok _ C уважением Холинов Илья Алексеевич, Ведущий инженер-программист Группа компаний «Нетвокс» Моб: +7 (964) 7075157 Рабочий: +7 (499) 502 2020 доб 547 Email: holi...@netvoxab.ru, - Группа компаний «Нетвокс» объединяет компании в области разработки программного обеспечения, интеграции информационных систем и осуществления бизнес-консалтинга: ООО «Netvox Lab» ___ 2013/7/8 Fredrik Gustafsson iv...@iveqy.com: On Fri, Jul 05, 2013 at 07:16:01PM +0400, Ilya Holinov wrote: I have svn repository on https singed with GeoTrust issued certificate. Every time i try to access this repository i have message : $ git svn rebase Error validating server certificate for 'https://svn.egspace.ru:443': - The certificate is not issued by a trusted authority. Use the fingerprint to validate the certificate manually! Certificate information: - Hostname: *.egspace.ru - Valid: from Apr 28 01:38:17 2013 GMT until Apr 30 12:00:40 2014 GMT - Issuer: GeoTrust, Inc., US - Fingerprint: b2:8d:f8:3b:7c:d2:a2:36:e2:1d:c3:5c:56:ec:87:6f:22:3e:4b:a8 Certificate problem. (R)eject, accept (t)emporarily or accept (p)ermanently? p Authentication realm: https://svn.egspace.ru:443 VisualSVN Server Username: holinov Password for 'holinov': Even if i choose permanently every next attempt to access in i have same issue. And this happens on svn rebase on every commit. I mean if i have 10 commits in local repository i will be asked about cert and user login\passwor for every one of them (and that's is verry annoying). But if i use TortoiseSVN i have no problem with checking that cert. P.S.: I'm using Windows 8 x64. P.P.S: I like git very much but in this case it makes me impossible to work in this way. This isn't really my thing to answer, I don't know windows well enough. However since you still haven't got an answer I'll give it a try. Please see the following link: https://confluence.atlassian.com/display/SOURCETREEKB/Resolving+SSL+Self-Signed+Certificate+Errors -- Med vänliga hälsningar Fredrik Gustafsson tel: 0733-608274 e-post: iv...@iveqy.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 1/2] t9902: fix 'test A == B' to use = operator
Junio C Hamano gits...@pobox.com writes: Thomas Rast tr...@inf.ethz.ch writes: The == operator as an alias to = is not POSIX. This doesn't actually matter for the execution of the script, because it only runs when the shell is bash. However, it trips up test-lint, so it's nicer to use the standard form. OK, my knee-jerk reaction was this is only for bash as you said, but the test-lint part I agree with. But then test-lint _ought_ to also catch the use of local in the ideal world, so perhaps in the longer term we would need to treat this bash-only script differently from others anyway??? True. I didn't really think about wider implications; I just noticed that there was an easy-to-fix complaint from test-lint :-) -- 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
Re: [PATCH 2/2] test-lint: detect 'export FOO=bar'
Torsten Bögershausen tbo...@web.de writes: +/^\s*export\s+[^=]*=/ and err 'export FOO=bar is not portable (please use FOO=bar export FOO)'; I have a slightly tighter reg exp in my tree, but credits should go to Thomas: /^\s*export\s+\S+=\S+/ Hmm, is that correct? I would expect shells that have problems with 'export FOO=bar' to also fail on 'export FOO=' (i.e. set to empty string and export). -- 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
Re: git subtree push-all and pull-all
On 08-07-13 17:20, Gareth Collins wrote: Hello Fredrik, Thanks for the suggestion! Adding in Paul Campbell and Herman Van Rink who worked on this before. Paul Campbell has been working hard on multiple branches in https://github.com/kemitix/git But work appears to have stopped. :( There is also a recent blogpost: http://ruleant.blogspot.nl/2013/06/git-subtree-module-with-gittrees-config.html Thanks for your interest, I'd love to see this get integrated thanks again, Gareth On Sun, Jul 7, 2013 at 8:54 AM, Fredrik Gustafsson iv...@iveqy.com wrote: On Wed, Jul 03, 2013 at 03:56:36PM -0400, Gareth Collins wrote: Hello, I see over the last year (on the web and in this mailing list) there was some activity to extend subtree with a .gittrees file and push-all/pull-all commands. Perhaps I missed it, but looking through the latest git code on the github mirror I can't find any reference to the .gittrees file or these commands. Does anyone know the status of this feature? Was it decided that this was a bad idea and the feature has been rejected? Or is this a feature still cooking...which will likely make it into git mainline at some point? I ask because I would like to use something like this to be able to keep a combined repository and separate project repositories in sync. Of course, if it was decided that this feature is fundamentally a bad idea then I will do something different. Any pointers would be a big help. thanks in advance, Gareth Collins Still no answer to this? I suggest that you CC the persons discussing this the last time. -- Med vänliga hälsningar Fredrik Gustafsson tel: 0733-608274 e-post: iv...@iveqy.com -- Met vriendelijke groet / Regards, Herman van Rink Initfour websolutions -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] range_set: fix coalescing bug when range is a subset of another
Eric Sunshine sunsh...@sunshineco.com writes: When coalescing ranges, sort_and_merge_range_set() unconditionally assumes that the end of a range being folded into a preceding range should become the end of the coalesced range. This assumption, however, is invalid when one range is a subset of another. For example, given ranges 1-5 and 2-3 added via range_set_append_unsafe(), sort_and_merge_range_set() incorrectly coalesces them to range 1-3 rather than the correct union range 1-5. Fix this bug. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- line-log.c | 3 ++- t/t4211-line-log.sh | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/line-log.c b/line-log.c index 4bbb09b..8cc29a0 100644 --- a/line-log.c +++ b/line-log.c @@ -116,7 +116,8 @@ static void sort_and_merge_range_set(struct range_set *rs) for (i = 1; i rs-nr; i++) { if (rs-ranges[i].start = rs-ranges[o-1].end) { - rs-ranges[o-1].end = rs-ranges[i].end; + if (rs-ranges[o-1].end rs-ranges[i].end) + rs-ranges[o-1].end = rs-ranges[i].end; Ouch. Thanks for finding and fixing this. Acked-by: Thomas Rast tr...@inf.ethz.ch -- 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
Re: [PATCH 0/3] merge -Xindex-only
On 07/08/2013 05:44 PM, Thomas Rast wrote: Michael Haggerty mhag...@alum.mit.edu writes: [Resend because of address confusion in replied-to email.] On 07/07/2013 08:00 PM, Thomas Rast wrote: I recently looked into making merge-recursive more useful as a modular piece in various tasks, e.g. Michael's git-imerge and the experiments I made in showing evil merges. This miniseries is the extremely low-hanging fruit. If it makes a good first step for git-imerge, perhaps it can go in like this. It's not a big speedup (about 2.2s vs 2.4s in a sample conflicting merge in git.git), but it does feel much cleaner to avoid touching the worktree unless actually necessary. Otherwise it's probably not worth it just yet; for what I want to do with it, we need some more reshuffling of things. Interesting. For git-imerge, it would be nice to speed up merges by skipping the working tree updates. 10% might not be so noticeable, but every little bit helps :-) But the killer benefit would be if git-imerge could do some of its automatic merge-testing and autofilling in the background while the user is resolving conflicts in the main index and working tree. Is it possible to use this option with an alternate index file (e.g., via the GIT_INDEX_FILE environment variable)? Can it be made to leave other shared state (e.g., MERGE_HEAD) alone? If so, maybe it's already there. GIT_INDEX_FILE yes, that one works out of the box. I think for the shared state, the following is a (probably) ridiculously unsupported yet magic way of achieving this: mkdir -p unshared/.git cd unshared/.git for f in ../../.git/*; do case $f in *HEAD | index) cp $f . ;; *) ln -s $f . ;; esac done That gives you a repository that propagates ref changes and object writing, but does not propagate changes to index, HEAD, FETCH_HEAD or MERGE_HEAD. Which might just be what you need? Note that as far as I'm concerned, this is a live handgrenade. It could blow up in your face at any time, but it probably has its applications... I might consider such a thing for my own use, but I don't think I'll lob live hand grenades at innocent git-imerge users :-) Since you've already implemented a way to merge into the index (even an alternative index) without touching the working copy, I'll just cross my fingers and hope for the appearance of an option that makes merge leave HEAD, MERGE_HEAD, etc. untouched. It's not like I have any free time to work on git-imerge anyway :-( Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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 2/2] test-lint: detect 'export FOO=bar'
On 09.07.13 11:28, Thomas Rast wrote: Torsten Bögershausen tbo...@web.de writes: + /^\s*export\s+[^=]*=/ and err 'export FOO=bar is not portable (please use FOO=bar export FOO)'; I have a slightly tighter reg exp in my tree, but credits should go to Thomas: /^\s*export\s+\S+=\S+/ Hmm, is that correct? I would expect shells that have problems with 'export FOO=bar' to also fail on 'export FOO=' (i.e. set to empty string and export). Good point, thanks -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] cat-file: add --batch-disk-sizes option
Duy Nguyen wrote: - We might overlook something. The best way to avoid missing is finish and verify it. - A promise to do things later could happen really late, or never happens. As you are sastisfied with the functionality you have less motivation to clean the code. Meanwhile the maintainer takes extra maintenance cost. I know. You know what my counter-argument looks like already: A promise to deliver a perfect series sometime in the future risks never reaching that perfection, and stalling everyone else's work. Even if we do manage to complete that perfect series, there is no guarantee that we'll get sufficient reviewer-interest or traction for merge. You think people are more likely to look at a 50-part series than a 15-part series? Either way, I'm not interested in arguing: for now, I'll repost the old 15-part series and try to get some reviews. Start writing code, and let's finish this thing. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/15] pretty: allow passing NULL commit to format_commit_message()
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com The new formatter, for-each-ref, may use non-commit placeholders only. While it could audit the format line and warn/exclude commit placeholders, that's a lot more work than simply ignore them. Unrecognized placeholders are displayed as-is, pretty obvious that they are not handled. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- pretty.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/pretty.c b/pretty.c index 0063f2d..816aa32 100644 --- a/pretty.c +++ b/pretty.c @@ -1156,6 +1156,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ } /* these depend on the commit */ + if (!commit) + return 0; + if (!commit-object.parsed) parse_object(commit-object.sha1); @@ -1276,6 +1279,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ } + if (!c-message) + return 0; + /* For the rest we have to parse the commit header. */ if (!c-commit_header_parsed) parse_commit_header(c); @@ -1510,9 +1516,10 @@ void format_commit_message(const struct commit *commit, context.commit = commit; context.pretty_ctx = pretty_ctx; context.wrap_start = sb-len; - context.message = logmsg_reencode(commit, - context.commit_encoding, - output_enc); + if (commit) + context.message = logmsg_reencode(commit, + context.commit_encoding, + output_enc); strbuf_expand(sb, format, format_commit_item, context); rewrap_message_tail(sb, context, 0, 0, 0); @@ -1535,7 +1542,8 @@ void format_commit_message(const struct commit *commit, } free(context.commit_encoding); - logmsg_free(context.message, commit); + if (commit) + logmsg_free(context.message, commit); free(context.signature_check.gpg_output); free(context.signature_check.signer); } -- 1.8.3.2.736.g869de25 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND][PATCH 00/15] Towards a more awesome git branch
Hi, I'm sending this out in the hope of attracting some reviews. It's an unedited resend, and there were zero conflicts from the rebase. Thanks. Nguyễn Thái Ngọc Duy (8): for-each-ref, quote: convert *_quote_print - *_quote_buf for-each-ref: don't print out elements directly pretty: extend pretty_print_context with callback pretty: allow passing NULL commit to format_commit_message() for-each-ref: get --pretty using format_commit_message() for-each-ref: teach verify_format() about pretty's syntax for-each-ref: introduce format specifier %(*) and %(*) for-each-ref: improve responsiveness of %(upstream:track) Ramkumar Ramachandra (7): tar-tree: remove dependency on sq_quote_print() quote: remove sq_quote_print() pretty: limit recursion in format_commit_one() for-each-ref: introduce %(HEAD) marker for-each-ref: introduce %(upstream:track[short]) pretty: introduce get_pretty_userformat for-each-ref: use get_pretty_userformat in --pretty Documentation/git-for-each-ref.txt | 43 +- builtin/for-each-ref.c | 279 ++--- builtin/tar-tree.c | 11 +- commit.h | 9 ++ pretty.c | 77 +- quote.c| 61 +++- quote.h| 8 +- t/t6300-for-each-ref.sh| 143 +++ 8 files changed, 521 insertions(+), 110 deletions(-) -- 1.8.3.2.736.g869de25 -- To unsubscribe from this list: send the line 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 02/15] for-each-ref: don't print out elements directly
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com Currently, the entire callchain starting from show_ref() parses and prints immediately. This inflexibility limits our ability to extend the parser. So, convert the entire callchain to accept a strbuf argument to write to. Also introduce a show_refs() helper that calls show_ref() in a loop to avoid cluttering up cmd_for_each_ref() with the task of initializing/freeing the strbuf. [rr: commit message] Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- builtin/for-each-ref.c | 55 -- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 1d4083c..e2d6c5a 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -864,31 +864,31 @@ static void sort_refs(struct ref_sort *sort, struct refinfo **refs, int num_refs qsort(refs, num_refs, sizeof(struct refinfo *), compare_refs); } -static void print_value(struct refinfo *ref, int atom, int quote_style) +static void print_value(struct strbuf *sb, struct refinfo *ref, + int atom, int quote_style) { struct atom_value *v; - struct strbuf sb = STRBUF_INIT; get_value(ref, atom, v); switch (quote_style) { case QUOTE_NONE: - fputs(v-s, stdout); + strbuf_addstr(sb, v-s); break; case QUOTE_SHELL: - sq_quote_buf(sb, v-s); + sq_quote_buf(sb, v-s); break; case QUOTE_PERL: - perl_quote_buf(sb, v-s); + perl_quote_buf(sb, v-s); break; case QUOTE_PYTHON: - python_quote_buf(sb, v-s); + python_quote_buf(sb, v-s); break; case QUOTE_TCL: - tcl_quote_buf(sb, v-s); + tcl_quote_buf(sb, v-s); break; } if (quote_style != QUOTE_NONE) { - fputs(sb.buf, stdout); - strbuf_release(sb); + fputs(sb-buf, stdout); + strbuf_release(sb); } } @@ -910,7 +910,7 @@ static int hex2(const char *cp) return -1; } -static void emit(const char *cp, const char *ep) +static void emit(struct strbuf *sb, const char *cp, const char *ep) { while (*cp (!ep || cp ep)) { if (*cp == '%') { @@ -919,32 +919,47 @@ static void emit(const char *cp, const char *ep) else { int ch = hex2(cp + 1); if (0 = ch) { - putchar(ch); + strbuf_addch(sb, ch); cp += 3; continue; } } } - putchar(*cp); + strbuf_addch(sb, *cp); cp++; } } -static void show_ref(struct refinfo *info, const char *format, int quote_style) +static void show_ref(struct strbuf *sb, struct refinfo *info, +const char *format, int quote_style) { const char *cp, *sp, *ep; for (cp = format; *cp (sp = find_next(cp)); cp = ep + 1) { ep = strchr(sp, ')'); if (cp sp) - emit(cp, sp); - print_value(info, parse_atom(sp + 2, ep), quote_style); + emit(sb, cp, sp); + print_value(sb, info, parse_atom(sp + 2, ep), quote_style); } if (*cp) { sp = cp + strlen(cp); - emit(cp, sp); + emit(sb, cp, sp); } - putchar('\n'); + strbuf_addch(sb, '\n'); +} + +static void show_refs(struct refinfo **refs, int maxcount, + const char *format, int quote_style) +{ + struct strbuf sb = STRBUF_INIT; + int i; + + for (i = 0; i maxcount; i++) { + strbuf_reset(sb); + show_ref(sb, refs[i], format, quote_style); + fputs(sb.buf, stdout); + } + strbuf_release(sb); } static struct ref_sort *default_sort(void) @@ -987,7 +1002,7 @@ static char const * const for_each_ref_usage[] = { int cmd_for_each_ref(int argc, const char **argv, const char *prefix) { - int i, num_refs; + int num_refs; const char *format = %(objectname) %(objecttype)\t%(refname); struct ref_sort *sort = NULL, **sort_tail = sort; int maxcount = 0, quote_style = 0; @@ -1041,7 +1056,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) if (!maxcount || num_refs maxcount) maxcount = num_refs; - for (i = 0; i maxcount; i++) - show_ref(refs[i], format, quote_style); + +
[PATCH 04/15] quote: remove sq_quote_print()
Remove sq_quote_print() since it has no callers. A nicer alternative sq_quote_buf() exists: its callers aren't forced to print immediately. For historical context, sq_quote_print() was first introduced in 575ba9d6 (GIT_TRACE: show which built-in/external commands are executed, 2006-06-25) for the purpose of printing argv for $GIT_TRACE. Today, we achieve this using trace_argv_printf() - sq_quote_argv() - sq_quote_buf(), which ultimately fills in a strbuf. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- quote.c | 17 - quote.h | 2 -- 2 files changed, 19 deletions(-) diff --git a/quote.c b/quote.c index 8c294df..778b39a 100644 --- a/quote.c +++ b/quote.c @@ -42,23 +42,6 @@ void sq_quote_buf(struct strbuf *dst, const char *src) free(to_free); } -void sq_quote_print(FILE *stream, const char *src) -{ - char c; - - fputc('\'', stream); - while ((c = *src++)) { - if (need_bs_quote(c)) { - fputs('\\, stream); - fputc(c, stream); - fputc('\'', stream); - } else { - fputc(c, stream); - } - } - fputc('\'', stream); -} - void sq_quote_argv(struct strbuf *dst, const char** argv, size_t maxlen) { int i; diff --git a/quote.h b/quote.h index ed06df5..251f3cc 100644 --- a/quote.h +++ b/quote.h @@ -27,8 +27,6 @@ struct strbuf; * excluding the final null regardless of the buffer size. */ -extern void sq_quote_print(FILE *stream, const char *src); - extern void sq_quote_buf(struct strbuf *, const char *src); extern void sq_quote_argv(struct strbuf *, const char **argv, size_t maxlen); -- 1.8.3.2.736.g869de25 -- To unsubscribe from this list: send the line 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 08/15] for-each-ref: get --pretty using format_commit_message()
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com --format is very limited in its capabilities. Introduce --pretty, which extends the existing --format with pretty-formats. In --pretty: - Existing --format %(atom) is available. They also accept some pretty magic. For example, you can use % (atom) to only display a leading space if the atom produces something. - %ab to display a hex character 0xab is not available as it may conflict with other pretty's placeholders. Use %xab instead. - Many pretty placeholders are designed to work on commits. While some of them should work on tags too, they don't (yet). - Unsupported atoms cause for-each-ref to exit early and report. Unsupported pretty placeholders are displayed as-is. - Pretty placeholders can not be used as a sorting criteria. --format is considered deprecated. If the user hits a bug specific in --format code, they are advised to migrate to --pretty. [rr: documentation] Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- Documentation/git-for-each-ref.txt | 23 ++- builtin/for-each-ref.c | 72 +- t/t6300-for-each-ref.sh| 123 + 3 files changed, 214 insertions(+), 4 deletions(-) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index f2e08d1..d8ad758 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -9,7 +9,8 @@ SYNOPSIS [verse] 'git for-each-ref' [--count=count] [--shell|--perl|--python|--tcl] - [(--sort=key)...] [--format=format] [pattern...] + [(--sort=key)...] [--format=format|--pretty=pretty] + [pattern...] DESCRIPTION --- @@ -47,6 +48,26 @@ OPTIONS `xx`; for example `%00` interpolates to `\0` (NUL), `%09` to `\t` (TAB) and `%0a` to `\n` (LF). +pretty:: + A format string with supporting placeholders described in the + PRETTY FORMATS section in linkgit:git-log[1]. Additionally + supports placeholders from `format` + (i.e. `%[*](fieldname)`). ++ +Caveats: + +1. Many of the placeholders in PRETTY FORMATS are designed to work + specifically on commit objects: when non-commit objects are + supplied, those placeholders won't work (i.e. they will be emitted + literally). + +2. Does not interpolate `%ab` (where `ab` are hex digits) with the + corresponding hex code. To print a byte from a hex code, use + `%xab` (from pretty-formats) instead. + +3. Only the placeholders inherited from `format` will respect + quoting settings. + pattern...:: If one or more patterns are given, only refs are shown that match against at least one pattern, either using fnmatch(3) or diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index e2d6c5a..8611777 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -962,6 +962,63 @@ static void show_refs(struct refinfo **refs, int maxcount, strbuf_release(sb); } +struct format_one_atom_context { + struct refinfo *info; + int quote_style; +}; + +static size_t format_one_atom(struct strbuf *sb, const char *placeholder, + void *format_context, void *user_data, + struct strbuf *subst) +{ + struct format_one_atom_context *ctx = user_data; + const char *ep; + + if (*placeholder == '%') { + strbuf_addch(sb, '%'); + return 1; + } + + if (*placeholder != '(') + return 0; + + ep = strchr(placeholder + 1, ')'); + if (!ep) + return 0; + print_value(sb, ctx-info, parse_atom(placeholder + 1, ep), + ctx-quote_style); + return ep + 1 - placeholder; +} + +static void show_pretty_refs(struct refinfo **refs, int maxcount, +const char *format, int quote_style) +{ + struct pretty_print_context ctx = {0}; + struct format_one_atom_context fctx; + struct strbuf sb = STRBUF_INIT; + int i; + + /* +* FIXME: add --date= for %ad, --decorate for %d and --color +* for %C +*/ + ctx.abbrev = DEFAULT_ABBREV; + ctx.format = format_one_atom; + ctx.user_data = fctx; + fctx.quote_style = quote_style; + for (i = 0; i maxcount; i++) { + struct commit *commit = NULL; + fctx.info = refs[i]; + if (sha1_object_info(refs[i]-objectname, NULL) == OBJ_COMMIT) + commit = lookup_commit(refs[i]-objectname); + strbuf_reset(sb); + format_commit_message(commit, format, sb, ctx); + strbuf_addch(sb, '\n'); + fputs(sb.buf, stdout); + } + strbuf_release(sb); +} + static struct ref_sort *default_sort(void) { static const
[PATCH 01/15] for-each-ref, quote: convert *_quote_print - *_quote_buf
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com for-each-ref.c:print_value() currently prints values to stdout immediately using {sq|perl|python|tcl}_quote_print, giving us no opportunity to do any further processing. In preparation for getting print_value() to accept an additional strbuf argument to write to, convert the *_quote_print functions and callers to *_quote_buf. [rr: commit message, minor modifications] Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- builtin/for-each-ref.c | 13 + quote.c| 44 ++-- quote.h| 6 +++--- 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 7f059c3..1d4083c 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -867,24 +867,29 @@ static void sort_refs(struct ref_sort *sort, struct refinfo **refs, int num_refs static void print_value(struct refinfo *ref, int atom, int quote_style) { struct atom_value *v; + struct strbuf sb = STRBUF_INIT; get_value(ref, atom, v); switch (quote_style) { case QUOTE_NONE: fputs(v-s, stdout); break; case QUOTE_SHELL: - sq_quote_print(stdout, v-s); + sq_quote_buf(sb, v-s); break; case QUOTE_PERL: - perl_quote_print(stdout, v-s); + perl_quote_buf(sb, v-s); break; case QUOTE_PYTHON: - python_quote_print(stdout, v-s); + python_quote_buf(sb, v-s); break; case QUOTE_TCL: - tcl_quote_print(stdout, v-s); + tcl_quote_buf(sb, v-s); break; } + if (quote_style != QUOTE_NONE) { + fputs(sb.buf, stdout); + strbuf_release(sb); + } } static int hex1(char ch) diff --git a/quote.c b/quote.c index 911229f..8c294df 100644 --- a/quote.c +++ b/quote.c @@ -463,72 +463,72 @@ int unquote_c_style(struct strbuf *sb, const char *quoted, const char **endp) /* quoting as a string literal for other languages */ -void perl_quote_print(FILE *stream, const char *src) +void perl_quote_buf(struct strbuf *sb, const char *src) { const char sq = '\''; const char bq = '\\'; char c; - fputc(sq, stream); + strbuf_addch(sb, sq); while ((c = *src++)) { if (c == sq || c == bq) - fputc(bq, stream); - fputc(c, stream); + strbuf_addch(sb, bq); + strbuf_addch(sb, c); } - fputc(sq, stream); + strbuf_addch(sb, sq); } -void python_quote_print(FILE *stream, const char *src) +void python_quote_buf(struct strbuf *sb, const char *src) { const char sq = '\''; const char bq = '\\'; const char nl = '\n'; char c; - fputc(sq, stream); + strbuf_addch(sb, sq); while ((c = *src++)) { if (c == nl) { - fputc(bq, stream); - fputc('n', stream); + strbuf_addch(sb, bq); + strbuf_addch(sb, 'n'); continue; } if (c == sq || c == bq) - fputc(bq, stream); - fputc(c, stream); + strbuf_addch(sb, bq); + strbuf_addch(sb, c); } - fputc(sq, stream); + strbuf_addch(sb, sq); } -void tcl_quote_print(FILE *stream, const char *src) +void tcl_quote_buf(struct strbuf *sb, const char *src) { char c; - fputc('', stream); + strbuf_addch(sb, ''); while ((c = *src++)) { switch (c) { case '[': case ']': case '{': case '}': case '$': case '\\': case '': - fputc('\\', stream); + strbuf_addch(sb, '\\'); default: - fputc(c, stream); + strbuf_addch(sb, c); break; case '\f': - fputs(\\f, stream); + strbuf_addstr(sb, \\f); break; case '\r': - fputs(\\r, stream); + strbuf_addstr(sb, \\r); break; case '\n': - fputs(\\n, stream); + strbuf_addstr(sb, \\n); break; case '\t': - fputs(\\t, stream); + strbuf_addstr(sb, \\t); break; case '\v': - fputs(\\v, stream); + strbuf_addstr(sb, \\v); break;
[PATCH 05/15] pretty: extend pretty_print_context with callback
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com The struct pretty_print_context contains the context in which the placeholders in format_commit_one() should be parsed. Although format_commit_one() primarily acts as a parser, there is no way for a caller to plug in custom callbacks. Now, callers can: 1. Parse a custom placeholder that is not supported by format_commit_one(), and act on it independently of the pretty machinery. 2. Parse a custom placeholder to substitute the custom placeholder with a placeholder that format_commit_one() understands. This is especially useful for supporting %(*), where * is substituted with a length computed by the caller. To support these two usecases, the interface for the function looks like: typedef size_t (*format_message_fn)(struct strbuf *sb, const char *placeholder, void *format_context, void *user_data, struct strbuf *placeholder_subst) It is exactly like format_commit_one(), except that there are two additional fields: user_data (to pass custom data to the callback), and placeholder_subst (to set the substitution). The callback should return the length of the original string parsed, and optionally set placeholder_subst. [rr: commit message, minor modifications] Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- commit.h | 8 pretty.c | 25 + 2 files changed, 33 insertions(+) diff --git a/commit.h b/commit.h index 4d452dc..ced7100 100644 --- a/commit.h +++ b/commit.h @@ -78,6 +78,12 @@ enum cmit_fmt { CMIT_FMT_UNSPECIFIED }; +typedef size_t (*format_message_fn)(struct strbuf *sb, + const char *placeholder, + void *format_context, + void *user_data, + struct strbuf *placeholder_subst); + struct pretty_print_context { enum cmit_fmt fmt; int abbrev; @@ -92,6 +98,8 @@ struct pretty_print_context { const char *output_encoding; struct string_list *mailmap; int color; + format_message_fn format; + void *user_data; }; struct userformat_want { diff --git a/pretty.c b/pretty.c index 9e43154..095e5ba 100644 --- a/pretty.c +++ b/pretty.c @@ -1069,6 +1069,31 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ struct commit_list *p; int h1, h2; + if (c-pretty_ctx-format) { + struct strbuf subst = STRBUF_INIT; + int ret = c-pretty_ctx-format(sb, placeholder, context, + c-pretty_ctx-user_data, + subst); + if (ret subst.len) { + /* +* Something was parsed by format(), and a +* placeholder-substitution was set. +* Recursion is required to override the +* return value of format_commit_one() with +* ret: the length of the original string +* before substitution. +*/ + ret = format_commit_one(sb, subst.buf, context) ? ret : 0; + strbuf_release(subst); + return ret; + } else if (ret) + /* +* Something was parsed by format(), but +* no placeholder-substitution was set. +*/ + return ret; + } + /* these are independent of the commit */ switch (placeholder[0]) { case 'C': -- 1.8.3.2.736.g869de25 -- To unsubscribe from this list: send the line 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 06/15] pretty: limit recursion in format_commit_one()
To make sure that a pretty_ctx-format substitution doesn't result in an infinite recursion, change the prototype of format_commit_one() to accept one last argument: no_recurse. So, a single substitution by format() must yield a result that can be parsed by format_commit_one() without the help of format(). Signed-off-by: Ramkumar Ramachandra artag...@gmail.com Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- pretty.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pretty.c b/pretty.c index 095e5ba..0063f2d 100644 --- a/pretty.c +++ b/pretty.c @@ -1061,7 +1061,8 @@ static size_t parse_padding_placeholder(struct strbuf *sb, static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ const char *placeholder, - void *context) + void *context, + int no_recurse) { struct format_commit_context *c = context; const struct commit *commit = c-commit; @@ -1069,7 +1070,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ struct commit_list *p; int h1, h2; - if (c-pretty_ctx-format) { + if (!no_recurse c-pretty_ctx-format) { struct strbuf subst = STRBUF_INIT; int ret = c-pretty_ctx-format(sb, placeholder, context, c-pretty_ctx-user_data, @@ -1083,7 +1084,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ * ret: the length of the original string * before substitution. */ - ret = format_commit_one(sb, subst.buf, context) ? ret : 0; + ret = format_commit_one(sb, subst.buf, context, 1) ? ret : 0; strbuf_release(subst); return ret; } else if (ret) @@ -1332,7 +1333,7 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */ } while (1) { int modifier = *placeholder == 'C'; - int consumed = format_commit_one(local_sb, placeholder, c); + int consumed = format_commit_one(local_sb, placeholder, c, 0); total_consumed += consumed; if (!modifier) @@ -1452,7 +1453,7 @@ static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */ if (((struct format_commit_context *)context)-flush_type != no_flush) consumed = format_and_pad_commit(sb, placeholder, context); else - consumed = format_commit_one(sb, placeholder, context); + consumed = format_commit_one(sb, placeholder, context, 0); if (magic == NO_MAGIC) return consumed; -- 1.8.3.2.736.g869de25 -- To unsubscribe from this list: send the line 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 03/15] tar-tree: remove dependency on sq_quote_print()
Currently, there is exactly one caller of sq_quote_print(), namely cmd_tar_tree(). In the interest of removing sq_quote_print() and simplification, replace it with an equivalent call to sq_quote_argv(). No functional changes intended. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/tar-tree.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/builtin/tar-tree.c b/builtin/tar-tree.c index 3f1e701..ba3ffe6 100644 --- a/builtin/tar-tree.c +++ b/builtin/tar-tree.c @@ -26,8 +26,8 @@ int cmd_tar_tree(int argc, const char **argv, const char *prefix) * $0 tree-ish basedir == * git archive --format-tar --prefix=basedir tree-ish */ - int i; const char **nargv = xcalloc(sizeof(*nargv), argc + 3); + struct strbuf sb = STRBUF_INIT; char *basedir_arg; int nargc = 0; @@ -65,11 +65,10 @@ int cmd_tar_tree(int argc, const char **argv, const char *prefix) fprintf(stderr, *** \git tar-tree\ is now deprecated.\n *** Running \git archive\ instead.\n***); - for (i = 0; i nargc; i++) { - fputc(' ', stderr); - sq_quote_print(stderr, nargv[i]); - } - fputc('\n', stderr); + sq_quote_argv(sb, nargv, 0); + strbuf_addch(sb, '\n'); + fputs(sb.buf, stderr); + strbuf_release(sb); return cmd_archive(nargc, nargv, prefix); } -- 1.8.3.2.736.g869de25 -- To unsubscribe from this list: send the line 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 11/15] for-each-ref: introduce %(HEAD) marker
'git branch' shows which branch you are currently on with an '*', but 'git for-each-ref' misses this feature. So, extend the format with %(HEAD) to do exactly the same thing. Now you can use the following format in for-each-ref: %C(red)%(HEAD)%C(reset) %C(green)%(refname:short)%C(reset) to display a red asterisk next to the current ref. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/git-for-each-ref.txt | 4 builtin/for-each-ref.c | 13 +++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 8cbc08c..8d982e3 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -121,6 +121,10 @@ upstream:: from the displayed ref. Respects `:short` in the same way as `refname` above. +HEAD:: + Useful to indicate the currently checked out branch. Is '*' + if HEAD points to the current ref, and ' ' otherwise. + In addition to the above, for commit and tag objects, the header field names (`tree`, `parent`, `object`, `type`, and `tag`) can be used to specify the value in the header field. diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index da479d1..3d357a9 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -76,6 +76,7 @@ static struct { { upstream }, { symref }, { flag }, + { HEAD }, }; /* @@ -679,8 +680,16 @@ static void populate_value(struct refinfo *ref) v-s = xstrdup(buf + 1); } continue; - } - else + } else if (!strcmp(name, HEAD)) { + const char *head; + unsigned char sha1[20]; + head = resolve_ref_unsafe(HEAD, sha1, 1, NULL); + if (!strcmp(ref-refname, head)) + v-s = *; + else + v-s = ; + continue; + } else continue; formatp = strchr(name, ':'); -- 1.8.3.2.736.g869de25 -- To unsubscribe from this list: send the line 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 13/15] for-each-ref: improve responsiveness of %(upstream:track)
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com Before anything is printed, for-each-ref sorts all refs first. As part of the sorting, populate_value() is called to fill the values in for all atoms/placeholders per entry. By the time sort_refs() is done, pretty much all data is already retrieved. This works fine when data can be cheaply retrieved before %(upstream:track) comes into the picture. It may take a noticeable amount of time to process %(upstream:track) for each entry. All entries add up and make --format='%(refname)%(upstream:track)' seem hung for a few seconds, then display everything at once. Improve the responsiveness by only processing the one atom (*) at a time so that processing one atom for all entries (e.g. sorting) won't cause much delay (unless you choose a heavy atom to process). (*) This is not entirely correct. If you sort by an atom that needs object database access, then it will fill all atoms that need odb. Which is not a bad thing. We don't want to access odb once at sorting phase and again at display phase. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- builtin/for-each-ref.c | 49 + 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 72b33ee..25764aa 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -565,20 +565,6 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj } /* - * We want to have empty print-string for field requests - * that do not apply (e.g. authordate for a tag object) - */ -static void fill_missing_values(struct atom_value *val) -{ - int i; - for (i = 0; i used_atom_cnt; i++) { - struct atom_value *v = val[i]; - if (v-s == NULL) - v-s = ; - } -} - -/* * val is a list of atom_value to hold returned values. Extract * the values for atoms in used_atom array out of (obj, buf, sz). * when deref is false, (obj, buf, sz) is the object that is @@ -621,7 +607,7 @@ static inline char *copy_advance(char *dst, const char *src) /* * Parse the object referred by ref, and grab needed value. */ -static void populate_value(struct refinfo *ref) +static void populate_value(struct refinfo *ref, int only_this_atom) { void *buf; struct object *obj; @@ -630,13 +616,15 @@ static void populate_value(struct refinfo *ref) const unsigned char *tagged; int upstream_present = 0; - ref-value = xcalloc(sizeof(struct atom_value), used_atom_cnt); + if (!ref-value) { + ref-value = xcalloc(sizeof(struct atom_value), used_atom_cnt); - if (need_symref (ref-flag REF_ISSYMREF) !ref-symref) { - unsigned char unused1[20]; - ref-symref = resolve_refdup(ref-refname, unused1, 1, NULL); - if (!ref-symref) - ref-symref = ; + if (need_symref (ref-flag REF_ISSYMREF) !ref-symref) { + unsigned char unused1[20]; + ref-symref = resolve_refdup(ref-refname, unused1, 1, NULL); + if (!ref-symref) + ref-symref = ; + } } /* Fill in specials first */ @@ -648,6 +636,9 @@ static void populate_value(struct refinfo *ref) const char *formatp; struct branch *branch; + if (only_this_atom != -1 only_this_atom != i) + continue; + if (*name == '*') { deref = 1; name++; @@ -754,6 +745,10 @@ static void populate_value(struct refinfo *ref) for (i = 0; i used_atom_cnt; i++) { struct atom_value *v = ref-value[i]; + + if (only_this_atom != -1 only_this_atom != i) + continue; + if (v-s == NULL) goto need_obj; } @@ -809,9 +804,15 @@ static void populate_value(struct refinfo *ref) */ static void get_value(struct refinfo *ref, int atom, struct atom_value **v) { - if (!ref-value) { - populate_value(ref); - fill_missing_values(ref-value); + if (!ref-value || !ref-value[atom].s) { + populate_value(ref, atom); + /* +* We want to have empty print-string for field +* requests that do not apply (e.g. authordate for a +* tag object) +*/ + if (!ref-value[atom].s) + ref-value[atom].s = ; } *v = ref-value[atom]; } -- 1.8.3.2.736.g869de25 -- To unsubscribe from this list: send the line 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 09/15] for-each-ref: teach verify_format() about pretty's syntax
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com Pretty format accepts either ' ', '+' or '-' after '%' and before the placeholder name to modify certain behaviors. Teach verify_format() about this so that it finds atom upstream in, for example, '% (upstream)'. This is important because verify_format populates used_atom, which get_value() and populate_value() later rely on. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- builtin/for-each-ref.c | 15 +-- pretty.c | 4 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 8611777..39454fb 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -150,7 +150,7 @@ static int parse_atom(const char *atom, const char *ep) /* * In a format string, find the next occurrence of %(atom). */ -static const char *find_next(const char *cp) +static const char *find_next(const char *cp, int pretty) { while (*cp) { if (*cp == '%') { @@ -160,6 +160,9 @@ static const char *find_next(const char *cp) */ if (cp[1] == '(') return cp; + else if (pretty cp[1] cp[2] == '(' +strchr( +-, cp[1])) /* see format_commit_item() */ + return cp + 1; else if (cp[1] == '%') cp++; /* skip over two % */ /* otherwise this is a singleton, literal % */ @@ -173,10 +176,10 @@ static const char *find_next(const char *cp) * Make sure the format string is well formed, and parse out * the used atoms. */ -static int verify_format(const char *format) +static int verify_format(const char *format, int pretty) { const char *cp, *sp; - for (cp = format; *cp (sp = find_next(cp)); ) { + for (cp = format; *cp (sp = find_next(cp, pretty)); ) { const char *ep = strchr(sp, ')'); if (!ep) return error(malformed format string %s, sp); @@ -935,7 +938,7 @@ static void show_ref(struct strbuf *sb, struct refinfo *info, { const char *cp, *sp, *ep; - for (cp = format; *cp (sp = find_next(cp)); cp = ep + 1) { + for (cp = format; *cp (sp = find_next(cp, 0)); cp = ep + 1) { ep = strchr(sp, ')'); if (cp sp) emit(sb, cp, sp); @@ -1098,8 +1101,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) } if (format != default_format pretty) die(--format and --pretty cannot be used together); - if ((pretty verify_format(pretty)) || - (!pretty verify_format(format))) + if ((pretty verify_format(pretty, 1)) || + (!pretty verify_format(format, 0))) usage_with_options(for_each_ref_usage, opts); if (!sort) diff --git a/pretty.c b/pretty.c index 816aa32..28c0a72 100644 --- a/pretty.c +++ b/pretty.c @@ -1439,6 +1439,10 @@ static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */ ADD_SP_BEFORE_NON_EMPTY } magic = NO_MAGIC; + /* +* Note: any modification in what placeholder[0] contains +* should be redone in builtin/for-each-ref.c:find_next(). +*/ switch (placeholder[0]) { case '-': magic = DEL_LF_BEFORE_EMPTY; -- 1.8.3.2.736.g869de25 -- To unsubscribe from this list: send the line 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 14/15] pretty: introduce get_pretty_userformat
This helper function is intended to be used by callers implementing --pretty themselves; it parses pretty.* configuration variables recursively and hands the user-defined format back to the caller. No builtins are supported, as CMT_FMT_* are really only useful when displaying commits. Callers might like to define their own builtins in the future. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- commit.h | 1 + pretty.c | 25 + 2 files changed, 26 insertions(+) diff --git a/commit.h b/commit.h index ced7100..331a2db 100644 --- a/commit.h +++ b/commit.h @@ -113,6 +113,7 @@ extern char *logmsg_reencode(const struct commit *commit, const char *output_encoding); extern void logmsg_free(char *msg, const struct commit *commit); extern void get_commit_format(const char *arg, struct rev_info *); +extern const char *get_pretty_userformat(const char *arg); extern const char *format_subject(struct strbuf *sb, const char *msg, const char *line_separator); extern void userformat_find_requirements(const char *fmt, struct userformat_want *w); diff --git a/pretty.c b/pretty.c index 28c0a72..70e4e44 100644 --- a/pretty.c +++ b/pretty.c @@ -174,6 +174,31 @@ void get_commit_format(const char *arg, struct rev_info *rev) } /* + * Function to parse --pretty string, lookup pretty.* configuration + * variables and return the format string, assuming no builtin + * formats. Not limited to commits, unlike get_commit_format(). + */ +const char *get_pretty_userformat(const char *arg) +{ + struct cmt_fmt_map *commit_format; + + if (!arg || !*arg) + return NULL; + + if (!prefixcmp(arg, format:) || !prefixcmp(arg, tformat:)) + return xstrdup(strchr(arg, ':' + 1)); + + if (strchr(arg, '%')) + return xstrdup(arg); + + commit_format = find_commit_format(arg); + if (!commit_format || commit_format-format != CMIT_FMT_USERFORMAT) + die(invalid --pretty format: %s, arg); + + return xstrdup(commit_format-user_format); +} + +/* * Generic support for pretty-printing the header */ static int get_one_line(const char *msg) -- 1.8.3.2.736.g869de25 -- To unsubscribe from this list: send the line 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 15/15] for-each-ref: use get_pretty_userformat in --pretty
Use get_pretty_userformat() to interpret the --pretty string. This means that you can now reference a format specified in a pretty.* configuration variable as an argument to 'git for-each-ref --pretty='. There are two caveats: 1. A leading format: or tformat: is automatically stripped and ignored. Separator semantics are not configurable (yet). 2. No built-in formats are available. The ones specified in pretty-formats (oneline, short etc) don't make sense when displaying refs anyway. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/git-for-each-ref.txt | 3 +++ builtin/for-each-ref.c | 16 +--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index d666ebd..ef39f2a 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -60,6 +60,9 @@ calculated. + Caveats: +0. No built-in formats from PRETTY FORMATS (like oneline, short) are + available. + 1. Many of the placeholders in PRETTY FORMATS are designed to work specifically on commit objects: when non-commit objects are supplied, those placeholders won't work (i.e. they will be emitted diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 25764aa..ed7bd7d 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -1151,7 +1151,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) int num_refs; const char *default_format = %(objectname) %(objecttype)\t%(refname); const char *format = default_format; - const char *pretty = NULL; + const char *pretty_raw = NULL, *pretty_userformat = NULL; struct ref_sort *sort = NULL, **sort_tail = sort; int maxcount = 0, quote_style = 0; struct refinfo **refs; @@ -1170,13 +1170,15 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) OPT_GROUP(), OPT_INTEGER( 0 , count, maxcount, N_(show only n matched refs)), OPT_STRING( 0 , format, format, N_(format), N_(format to use for the output)), - OPT_STRING( 0 , pretty, pretty, N_(format), N_(alternative format to use for the output)), + OPT_STRING( 0 , pretty, pretty_raw, N_(format), N_(alternative format to use for the output)), OPT_CALLBACK(0 , sort, sort_tail, N_(key), N_(field name to sort on), opt_parse_sort), OPT_END(), }; parse_options(argc, argv, prefix, opts, for_each_ref_usage, 0); + if (pretty_raw) + pretty_userformat = get_pretty_userformat(pretty_raw); if (maxcount 0) { error(invalid --count argument: `%d', maxcount); usage_with_options(for_each_ref_usage, opts); @@ -1185,10 +1187,10 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) error(more than one quoting style?); usage_with_options(for_each_ref_usage, opts); } - if (format != default_format pretty) + if (format != default_format pretty_userformat) die(--format and --pretty cannot be used together); - if ((pretty verify_format(pretty, 1)) || - (!pretty verify_format(format, 0))) + if ((pretty_userformat verify_format(pretty_userformat, 1)) || + (!pretty_userformat verify_format(format, 0))) usage_with_options(for_each_ref_usage, opts); if (!sort) @@ -1209,8 +1211,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) if (!maxcount || num_refs maxcount) maxcount = num_refs; - if (pretty) - show_pretty_refs(refs, maxcount, pretty, quote_style); + if (pretty_userformat) + show_pretty_refs(refs, maxcount, pretty_userformat, quote_style); else show_refs(refs, maxcount, format, quote_style); return 0; -- 1.8.3.2.736.g869de25 -- To unsubscribe from this list: send the line 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 10/15] for-each-ref: introduce format specifier %(*) and %(*)
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com Pretty placeholders %(N) and %(N) require a user provided width N, which makes sense because the commit chain could be really long and the user only needs to look at a few at the top, going to the end just to calculate the best width wastes CPU cycles. for-each-ref is different; the display set is small, and we display them all at once. We even support sorting, which goes through all display items anyway. This patch introduces new %(*) and %(*), which are supposed to be followed immediately by %(fieldname) (i.e. original for-each-ref specifiers, not ones coming from pretty.c). They calculate the best width for the %(fieldname), ignoring ansi escape sequences if any. [rr: documentation] Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- Documentation/git-for-each-ref.txt | 7 +++ builtin/for-each-ref.c | 38 ++ t/t6300-for-each-ref.sh| 20 3 files changed, 65 insertions(+) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index d8ad758..8cbc08c 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -47,6 +47,10 @@ OPTIONS are hex digits interpolates to character with hex code `xx`; for example `%00` interpolates to `\0` (NUL), `%09` to `\t` (TAB) and `%0a` to `\n` (LF). ++ +Placeholders `%(*)` and `%(*)` work like `%(N)` and `%(N)` +respectively, except that the width of the next placeholder is +calculated. pretty:: A format string with supporting placeholders described in the @@ -68,6 +72,9 @@ Caveats: 3. Only the placeholders inherited from `format` will respect quoting settings. +3. Only the placeholders inherited from `format` will work with the + alignment placeholders `%(*)` and '%(*)`. + pattern...:: If one or more patterns are given, only refs are shown that match against at least one pattern, either using fnmatch(3) or diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 39454fb..da479d1 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -9,6 +9,7 @@ #include quote.h #include parse-options.h #include remote.h +#include utf8.h /* Quoting styles */ #define QUOTE_NONE 0 @@ -966,10 +967,30 @@ static void show_refs(struct refinfo **refs, int maxcount, } struct format_one_atom_context { + struct refinfo **refs; + int maxcount; + struct refinfo *info; int quote_style; }; +static unsigned int get_atom_width(struct format_one_atom_context *ctx, + const char *start, const char *end) +{ + struct strbuf sb = STRBUF_INIT; + int i, atom = parse_atom(start, end); + unsigned int len = 0, sb_len; + for (i = 0; i ctx-maxcount; i++) { + print_value(sb, ctx-refs[i], atom, ctx-quote_style); + sb_len = utf8_strnwidth(sb.buf, sb.len, 1); + if (sb_len len) + len = sb_len; + strbuf_reset(sb); + } + strbuf_release(sb); + return len; +} + static size_t format_one_atom(struct strbuf *sb, const char *placeholder, void *format_context, void *user_data, struct strbuf *subst) @@ -982,6 +1003,21 @@ static size_t format_one_atom(struct strbuf *sb, const char *placeholder, return 1; } + /* +* Substitute %(*)%(atom) and friends with real width. +*/ + if (*placeholder == '' || *placeholder == '') { + const char *star = placeholder + 1; + if (!prefixcmp(star, (*)%() + ((ep = strchr(star + strlen((*)%(), ')')) != NULL)) { + star++; + strbuf_addf(subst, %c(%u), + *placeholder, + get_atom_width(ctx, star + strlen(*)%(), ep)); + return 1 + strlen((*)); + } + } + if (*placeholder != '(') return 0; @@ -1008,6 +1044,8 @@ static void show_pretty_refs(struct refinfo **refs, int maxcount, ctx.abbrev = DEFAULT_ABBREV; ctx.format = format_one_atom; ctx.user_data = fctx; + fctx.refs = refs; + fctx.maxcount = maxcount; fctx.quote_style = quote_style; for (i = 0; i maxcount; i++) { struct commit *commit = NULL; diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index d39e0b4..160018c 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -196,6 +196,26 @@ test_pretty head '%(20)%(committername) end' 'C O Mitter end' test_pretty head '%(20)%(committername) end' ' C O Mitter end' test_pretty head '%(20)%(committername) end' ' C O Mitter
Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
On 30.06.13 19:28, Ramsay Jones wrote: Ramsay Jones wrote: Michael Haggerty wrote: On 06/27/2013 12:35 AM, Jeff King wrote: [ ... ] I think Michael's assessment above is missing one thing. Peff is absolutely right; for some unknown reason I was thinking of the consistency check as having been already fixed. Well, the cygwin: Remove the Win32 l/stat() functions patch *does* fix the problem. :-D It's just a pity we can't use it on performance grounds. :( [...#ifdef out consistency check on cygwin when lock is held...] Yes, this would work. But, taking a step back, I think it is a bad idea to have an unreliable stat() masquerading as a real stat(). If we want to allow the use of an unreliable stat for certain purposes, let's have two stat() interfaces: * the true stat() (in this case I guess cygwin's slow-but-correct implementation) * some fast_but_maybe_unreliable_stat(), which would map to stat() on most platforms but might map to the Windows stat() on cygwin when so configured. By default the true stat() would always be used. It should have to be a conscious decision, taken only in specific, vetted scenarios, to use the unreliable stat. You have just described my second patch! :D Unfortunately, I have not had any time to work on the patch this weekend. However, despite the patch being a bit rough around the edges, I decided to send it out (see below) to get some early feedback. Note that it passes the t3210, t3211, t5500, t3200, t3301, t7606 and t1301 tests, but I have not run the full test suite. Comments welcome. ATB, Ramsay Jones -- 8 -- Subject: [PATCH] cygwin: Add fast_[l]stat() functions Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- builtin/apply.c| 6 +++--- builtin/commit.c | 2 +- builtin/ls-files.c | 2 +- builtin/rm.c | 2 +- builtin/update-index.c | 2 +- check-racy.c | 2 +- compat/cygwin.c| 12 compat/cygwin.h| 10 +++--- diff-lib.c | 2 +- diff.c | 2 +- entry.c| 4 ++-- git-compat-util.h | 13 +++-- help.c | 5 + path.c | 9 + preload-index.c| 2 +- read-cache.c | 6 +++--- unpack-trees.c | 8 17 files changed, 36 insertions(+), 53 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index 0e9b631..ca26caa 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -3253,7 +3253,7 @@ static int load_current(struct image *image, struct patch *patch) if (pos 0) return error(_(%s: does not exist in index), name); ce = active_cache[pos]; - if (lstat(name, st)) { + if (fast_lstat(name, st)) { if (errno != ENOENT) return error(_(%s: %s), name, strerror(errno)); if (checkout_target(ce, st)) @@ -3396,7 +3396,7 @@ static int check_preimage(struct patch *patch, struct cache_entry **ce, struct s if (previous) { st_mode = previous-new_mode; } else if (!cached) { - stat_ret = lstat(old_name, st); + stat_ret = fast_lstat(old_name, st); if (stat_ret errno != ENOENT) return error(_(%s: %s), old_name, strerror(errno)); } @@ -3850,7 +3850,7 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned die(_(corrupt patch for subproject %s), path); } else { if (!cached) { - if (lstat(path, st) 0) + if (fast_lstat(path, st) 0) die_errno(_(unable to stat newly created file '%s'), path); fill_stat_cache_info(ce, st); diff --git a/builtin/commit.c b/builtin/commit.c index 6b693c1..1d208c6 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -231,7 +231,7 @@ static void add_remove_files(struct string_list *list) if (p-util) continue; - if (!lstat(p-string, st)) { + if (!fast_lstat(p-string, st)) { if (add_to_cache(p-string, st, 0)) die(_(updating files failed)); } else diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 08d9786..db66a0e 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -251,7 +251,7 @@ static void show_files(struct dir_struct *dir) continue; if (ce_skip_worktree(ce)) continue; - err = lstat(ce-name, st); + err = fast_lstat(ce-name, st); if (show_deleted err) show_ce_entry(tag_removed, ce); if (show_modified ce_modified(ce, st,
Git --file doesn't override $HOME in version 1.8.1.2
Using Ubuntu 13.04 with Git 1.8.1.2 I stumbled upon a problem using Puppet. In Puppet we launch Git with the command: '/usr/bin/git config --file /home/qt/.gitconfig --get user.name Qt Continuous Integration System' However, puppet logs: fatal: unable to access '/root/.config/git/config': Permission denied. Puppet is run as root, so HOME points to /root, but still -file should override the environment variable. If the same command is run directly from terminal as root, it works as well. With 1.8.3.2 the problem didn't reoccur. -Tony --- Digia, Qt -- To unsubscribe from this list: send the line 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] commit: reject non-characters
Unicode clause D14 defines all characters U+nFFFE and U+n (where 0 = n = 10h) as well as the range U+FDD0..U+FDEF as non-characters, reserved for internal use only. Disallow these characters in commit messages as they are normally not recommended for interchange. Signed-off-by: Peter Krefting pe...@softwolves.pp.se --- Junio C Hamano: Yeah, while we are at it, doing this may not hurt. I think Brian's two patches are in fairly good shape otherwise, so perhaps you can do this as a follow-up patch on top of the tip of the topic, e82bd6cc (commit: reject overlong UTF-8 sequences, 2013-07-04)? OK, here you are. Enjoy :) commit.c | 7 +-- t/t3900-i18n-commit.sh | 18 ++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/commit.c b/commit.c index 5097dba..0587732 100644 --- a/commit.c +++ b/commit.c @@ -1305,8 +1305,11 @@ static int find_invalid_utf8(const char *buf, int len) /* Surrogates are only for UTF-16 and cannot be encoded in UTF-8. */ if ((codepoint 0x1ff800) == 0xd800) return bad_offset; - /* U+FFFE and U+ are guaranteed non-characters. */ - if ((codepoint 0x1e) == 0xfffe) + /* U+xxFFFE and U+xx are guaranteed non-characters. */ + if ((codepoint 0xe) == 0xfffe) + return bad_offset; + /* So are anything in the range U+FDD0..U+FDEF. */ + if (codepoint = 0xfdd0 codepoint = 0xfdef) return bad_offset; } return -1; diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh index 051ea9d..38b00c3 100755 --- a/t/t3900-i18n-commit.sh +++ b/t/t3900-i18n-commit.sh @@ -58,6 +58,24 @@ test_expect_success 'UTF-8 overlong sequences rejected' ' grep did not conform $HOME/stderr ' +test_expect_success 'UTF-8 non-characters refused' ' + test_when_finished rm -f $HOME/stderr $HOME/invalid + echo UTF-8 non-character 1 F + printf Commit message\n\nNon-character:\364\217\277\276\n \ + $HOME/invalid + git commit -a -F $HOME/invalid 2$HOME/stderr + grep did not conform $HOME/stderr +' + +test_expect_success 'UTF-8 non-characters refused' ' + test_when_finished rm -f $HOME/stderr $HOME/invalid + echo UTF-8 non-character 2. F + printf Commit message\n\nNon-character:\357\267\220\n \ + $HOME/invalid + git commit -a -F $HOME/invalid 2$HOME/stderr + grep did not conform $HOME/stderr +' + for H in ISO8859-1 eucJP ISO-2022-JP do test_expect_success $H setup ' -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] describe/name-rev: tell name-rev to peel the incoming object to commit first
Jeff King p...@peff.net writes: Although I am still not clear on why it would not be up to the caller of git-describe in the first place to decide which they wanted. Thanks for a dose of sanity. Even though the part of the miniseries that makes sure that X (Y) output from name-rev always satisfies that rev-parse on X and Y give the same thing is an improvement, the whole thing about describe is misguided and wrong, I think. It started from the observation that these do not match: $ git describe $(git rev-parse v1.8.3) v1.8.3 $ git describe --contains $(git rev-parse v1.8.3) v1.8.3^0 and the miniseries veered in a wrong direction of fixing the latter to match the former. But the thing is, what is incosistent from the rest of the world is the describe output without --contains for a commit that is exactly at a tag (i.e. the former), and there is no need to fix this inconsistency, as we see below. The form without --contains in general reads like this: $ git describe --long $(git rev-parse v1.8.3) a717d9e v1.8.3-0-gedca415 v1.8.3-2-ga717d9e They both name a commit object, but that is sort of an afterthought; the support for describe name came late at 7dd45e15 (sha1_name.c: understand describe output as a valid object name, 2006-09-20). The primary purpose of git describe without --contains is to give a string that is suitable for a version number to be embedded in an executable. For that purpose, v1.8.3 is more convenient than v1.8.3-0-gedca415. But this convenient format breaks the consistency. While any other describe name for a commmit names a commit, the output for a commit that is exactly at a tag does not (in ancient times, describe output were not even extended SHA-1 expressions, so this inconsistency did not matter, but the afterthought brought the consistency to the foreground). The user chooses the convenience over the consistency by not using --long. And the short form cannot be v1.8.3^0 or v1.8.3~0 for the sake of consistency, as these are no more suitable as a version number than a short and sweet v1.8.3. The --contains form does not even aim to come up with a pleasant looking version string without using funny line noise characters, so it is perfectly fine for it to say: $ git describe --contains $(git rev-parse v1.8.3) a717d9e v1.8.3^0 v1.8.3.1~9 and these are internally consistent (they both roundtrip via rev-parse). Stripping ^0 from the former will break the consistency, even though it may make the output look prettier, but the --contains output is not even meant to be pretty in the first place. So let's drop 4/4; it is breaking the system by trying to solve a problem that does not exist. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git --file doesn't override $HOME in version 1.8.1.2
Sarajärvi Tony tony.saraja...@digia.com writes: Using Ubuntu 13.04 with Git 1.8.1.2 I stumbled upon a problem using Puppet. In Puppet we launch Git with the command: '/usr/bin/git config --file /home/qt/.gitconfig --get user.name Qt Continuous Integration System' However, puppet logs: fatal: unable to access '/root/.config/git/config': Permission denied. Puppet is run as root, so HOME points to /root, but still -file should override the environment variable. If the same command is run directly from terminal as root, it works as well. To elaborate (I briefly talked to Sarajärvi on IRC): this isn't about the fatal error; we downgraded this to a nonfatal error in 4698c8f (config: allow inaccessible configuration under $HOME, 2013-04-12). Rather, it's very strange that 'git config --file foo' tries to look at any config file other than 'foo'. In a git repo: $ strace git config --file fooconfig test.var 21 | grep 'open.*config' open(/home/thomas/.gitconfig, O_RDONLY) = 3 open(.git/config, O_RDONLY) = 3 open(/home/thomas/.gitconfig, O_RDONLY) = 3 open(.git/config, O_RDONLY) = 3 open(fooconfig, O_RDONLY) = 3 I haven't looked into the code yet. Probably it's simply following the usual code paths to discover a repo and read its config. However, with the --file option, it shouldn't. -- 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
Re: Git --file doesn't override $HOME in version 1.8.1.2
Sarajärvi Tony tony.saraja...@digia.com writes: Using Ubuntu 13.04 with Git 1.8.1.2 I stumbled upon a problem using Puppet. In Puppet we launch Git with the command: '/usr/bin/git config --file /home/qt/.gitconfig --get user.name Qt Continuous Integration System' Hmph. What does this even mean? git config --get user.name Qt CIS I have a feeling that the command will exit with an error status 1. However, puppet logs: fatal: unable to access '/root/.config/git/config': Permission denied. Puppet is run as root, so HOME points to /root, but still -file should override the environment variable. Probably. If the same command is run directly from terminal as root, it works as well. I am not sure what you mean by works as well. It behaves differently and does not fail the same way? With 1.8.3.2 the problem didn't reoccur. That is probably due to b1c418e1 (Merge branch 'jn/config-ignore-inaccessible' into maint, 2013-06-09) But it is puzzling. The error out upon an inaccessible configuration file in usual places check we had since v1.8.1.1 was meant to make sure that you will not be missing a basic configuration before running any command (including git config itself). As root, you shouldn't even have triggered the unable to access: Permission denined in the first place. There is something else going on. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] merge -Xindex-only
Michael Haggerty mhag...@alum.mit.edu writes: Since you've already implemented a way to merge into the index (even an alternative index) without touching the working copy, I'll just cross my fingers and hope for the appearance of an option that makes merge leave HEAD, MERGE_HEAD, etc. untouched. The most annoying part is probably where to put the output, since merging is more or less defined to do one of: - update HEAD and return 0 - update MERGE_HEAD and return 1 I'm not sure how much flexibility is worth having. Would it be sufficient if you had an option, e.g. -Xresult-ref=refs/heads/foo, that changes it to: - update refs/heads/foo and return 0 - return 1, not updating any refs That would mean that it would only work for noninteractive use. In the conflicting case, the driving script would need to remember what it wanted to merge so as have the information when finally committing. -- 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
RE: Git --file doesn't override $HOME in version 1.8.1.2
-Original Message- From: Junio C Hamano [mailto:gits...@pobox.com] Sent: 9. heinäkuuta 2013 15:01 To: Sarajärvi Tony Cc: git@vger.kernel.org Subject: Re: Git --file doesn't override $HOME in version 1.8.1.2 Sarajärvi Tony tony.saraja...@digia.com writes: Using Ubuntu 13.04 with Git 1.8.1.2 I stumbled upon a problem using Puppet. In Puppet we launch Git with the command: '/usr/bin/git config --file /home/qt/.gitconfig --get user.name Qt Continuous Integration System' Hmph. What does this even mean? git config --get user.name Qt CIS Our puppet configuration is available for anyone here: https://qt.gitorious.org/qtqa/sysadmin/blobs/master/puppet/modules/git/manifests/config.pp We check with 'unless' if the user.name has been set to what we expect it to be. The return value is different from Git if I typo the name. root@dev-ubuntu1304-x64-01:~# git config --file /home/qt/.gitconfig --get user.name Qt Continuous Integration Systed root@dev-ubuntu1304-x64-01:~# echo $? 1 root@dev-ubuntu1304-x64-01:~# git config --file /home/qt/.gitconfig --get user.name Qt Continuous Integration System Qt Continuous Integration System root@dev-ubuntu1304-x64-01:~# echo $? 0 I have a feeling that the command will exit with an error status 1. However, puppet logs: fatal: unable to access '/root/.config/git/config': Permission denied. Puppet is run as root, so HOME points to /root, but still -file should override the environment variable. Probably. If the same command is run directly from terminal as root, it works as well. I am not sure what you mean by works as well. It behaves differently and does not fail the same way? Must have been writing something else and changed my mind :) I was trying to say that everything works when run from the command line. The permission denied appears when running with Puppet. And Puppet is run as root. And even if Puppet was run as some other user, I still don't think that Git should be trying to use anything from /root, as we're giving it the --file parameter. As if Puppet removed the whole --file parameter altogether. But as it works with a newer or older version of Git, it can't be dropping the parameter out. Root user is the only one with HOME set to /root, so it must be getting that value from root's HOME variable. With 1.8.3.2 the problem didn't reoccur. That is probably due to b1c418e1 (Merge branch 'jn/config-ignore-inaccessible' into maint, 2013-06-09) But it is puzzling. The error out upon an inaccessible configuration file in usual places check we had since v1.8.1.1 was meant to make sure that you will not be missing a basic configuration before running any command (including git config itself). As root, you shouldn't even have triggered the unable to access: Permission denined in the first place. There is something else going on. That is also weird, agreed.
Re: [PATCH 4/4] describe/name-rev: tell name-rev to peel the incoming object to commit first
Junio C Hamano wrote: $ git describe $(git rev-parse v1.8.3) v1.8.3 $ git describe --contains $(git rev-parse v1.8.3) v1.8.3^0 This is a correct observation, and I've already submitted the correct fix: name-rev: strip trailing ^0 in when --name-only. $ git describe --contains $(git rev-parse v1.8.3) a717d9e v1.8.3^0 v1.8.3.1~9 and these are internally consistent (they both roundtrip via rev-parse). Stripping ^0 from the former will break the consistency, even though it may make the output look prettier, but the --contains output is not even meant to be pretty in the first place. Incorrect. The --contains output _is_ meant to be pretty. That's the whole reason name-rev --name-only was invented. [2/4] is correct in that it fixes --stdin for annotated tags (although the implementation could be simpler, and the commit message is completely misleading). -- To unsubscribe from this list: send the line 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 clone depth of 0 not possible.
Hi Junio, Doing it correctly (in the shorter term) would involve: - adding a capability on the sending side fixed-off-by-one-depth to the protocol, and teaching the sending side to advertise the capability; - teaching the requestor that got --depth=N from the end user to pay attention to the new capability in such a way that: - when talking to an old sender (i.e. without the off-by-one fix), send N-1 for N greater than 1. Punt on N==1; - when talking to a fixed sender, ask to enable the capability, and send N as is (including N==1). - teaching the sending side to see if the new behaviour to fix off-by-one is asked by the requestor, and stop at the correct number of commits, not oversending one more. Otherwise retain the old behaviour. While implementing the above, I noticed my fix now introduced an off-by-one error the other way. When investigating, I found this commit: commit 682c7d2f1a2d1a5443777237450505738af2ff1a Author: Nguyễn Thái Ngọc Duy pclo...@gmail.com Date: Fri Jan 11 16:05:47 2013 +0700 upload-pack: fix off-by-one depth calculation in shallow clone get_shallow_commits() is used to determine the cut points at a given depth (i.e. the number of commits in a chain that the user likes to get). However we count current depth up to the commit commit but we do the cutting at its parents (i.e. current depth + 1). This makes upload-pack always return one commit more than requested. This patch fixes it. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com Which actually seems to fix the off-by-one bug that is described in this thread, but without going through the hoops of preserving current behaviour for older git versions (that is, it makes behaviour dependent on server version instead of client version). Does this mean the discussion in this thread is meaningless, or is that commit not intended to be the final fix? In any case, IIUC that particular patch makes a piece of the existing code dead, which needs to be removed. Gr. Matthijs -- To unsubscribe from this list: send the line 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 in nutshell Inbox
Howdy? I'm bzr user and I want to migrate to git. Generally I use bzr through Bazaar Explorer which is very easy neat GUI utility for managing bzr repositories. May you please guide me to most easy way to migrate to Git? P.S. * I'm very comfortable with bzr (it has awesome GUI utility) but I decided to leave it because of its technical weakness by comparing to git. * I hate to use terminal in everything so please don't give me links with terminal commands I look for rapid and user friendly migration. * I don't care about commercial solutions I'm open source fan :) * Most git hosting I'm interesting in is gitorious.org because it's from open source family (that's what I read about) may you correct me if I'm wrong? -- Best Regards Muhammad Bashir Al-Noimi -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git in nutshell Inbox
Hi, If you are on Windows or OS X, you can try SourceTree from Atlassian (free). It is really nice to work with. Cheers, Matthieu 2013/7/9 Muhammad Bashir Al-Noimi mbno...@gmail.com: Howdy? I'm bzr user and I want to migrate to git. Generally I use bzr through Bazaar Explorer which is very easy neat GUI utility for managing bzr repositories. May you please guide me to most easy way to migrate to Git? P.S. * I'm very comfortable with bzr (it has awesome GUI utility) but I decided to leave it because of its technical weakness by comparing to git. * I hate to use terminal in everything so please don't give me links with terminal commands I look for rapid and user friendly migration. * I don't care about commercial solutions I'm open source fan :) * Most git hosting I'm interesting in is gitorious.org because it's from open source family (that's what I read about) may you correct me if I'm wrong? -- Best Regards Muhammad Bashir Al-Noimi -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Information System Engineer, Ph.D. Blog: http://matt.eifelle.com LinkedIn: http://www.linkedin.com/in/matthieubrucher Music band: http://liliejay.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: Git in nutshell Inbox
Muhammad Bashir Al-Noimi mbno...@gmail.com writes: I'm bzr user and I want to migrate to git. Generally I use bzr through Bazaar Explorer which is very easy neat GUI utility for managing bzr repositories. May you please guide me to most easy way to migrate to Git? (I think it's a mistake to stick to GUI: the concepts are easier to understand with Git commands IMHO) But there are nice GUIs for Git too. The official one is git gui, distributed with Git. I do not like the visual aspect a lot, but it has a very good coverage of Git's functionalities. Otherwise, have a look at https://git.wiki.kernel.org/index.php/InterfacesFrontendsAndTools#Graphical_Interfaces git-cola is a good one. * I'm very comfortable with bzr (it has awesome GUI utility) but I decided to leave it because of its technical weakness by comparing to git. Another reason to migrate is that bzr's developpement has stopped :-(. * Most git hosting I'm interesting in is gitorious.org because it's from open source family (that's what I read about) may you correct me if I'm wrong? gitorious.org runs with free software (you can host a gitorious at home if you want). github.com is probably the most popular hosting site, it uses a lot of free software (and GitHub contributes a lot to free software), but the complete solution is closed-source (you'll have to pay if you want to run it at home, and won't be able to modify it). There are tons of alternatives. See e.g. https://git.wiki.kernel.org/index.php/GitHosting -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git in nutshell Inbox
On Tue, Jul 9, 2013 at 4:39 PM, Matthieu Moy matthieu@imag.fr wrote: But there are nice GUIs for Git too. The official one is git gui, distributed with Git. I do not like the visual aspect a lot, but it has a very good coverage of Git's functionalities. Otherwise, have a look at https://git.wiki.kernel.org/index.php/InterfacesFrontendsAndTools#Graphical_Interfaces git-cola is a good one. I tried to use git-cola and found it complicated and not user friendly (maybe because I came from Bazaar Explorer) I noticed that it needs terminal to be able to work (ex. it can't create new repository). I can't even ask for merge modifications (I'm still know nothing in git)! so do you know better GUI utility? -- Best Regards Muhammad Bashir Al-Noimi -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] describe: use argv-array
Jeff King p...@peff.net writes: On Sun, Jul 07, 2013 at 03:33:43PM -0700, Junio C Hamano wrote: +argv_array_init(args); +argv_array_push(args, name-rev); +argv_array_push(args, --name-only); +argv_array_push(args, --no-undefined); [...] -memcpy(args + i, argv, argc * sizeof(char *)); -args[i + argc] = NULL; -return cmd_name_rev(i + argc, args, prefix); +return cmd_name_rev(args.argc, args.argv, prefix); This leaks the memory allocated by args. The original did, too, and it is probably not that big a deal (we exit right after anyway). The fix would be something like: rc = cmd_name_rev(args.argc, args.argv, prefix); argv_array_clear(args); return rc; Yes; this was meant as a straight rewrite and I did not bother, but I should have cleaned it up as I meant to build on top. Will amend, even though I do not think we need to build anything on top. -- To unsubscribe from this list: send the line 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/RFC] blame: accept multiple -L ranges
[Added Cc:Thomas Rast] On Sun, Jul 7, 2013 at 5:58 AM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: git-blame accepts only zero or one -L option. Clients requiring blame information for multiple disjoint ranges are therefore forced either to invoke git-blame multiple times, once for each range, or only once with no -L option to cover the entire file, which can be costly. Teach git-blame to accept multiple -L ranges. Overlapping and out-of-order ranges are accepted and handled gracefully. For example: git blame -L 3,+4 -L 91,+7 -L 2,3 -L 89,100 source.c emits blame information for lines 2-6 and 89-100. --- This is RFC because it lacks documentation and test updates, and because I want to make sure the approach is sound and not abusive of the blame machinery. A few commments (without reading too deep in the patch, so do not take any of these as complaint---if you did it the way I said I'd prefer, take it as a praise ;-). - I'd prefer to see the command parser for multiple -L options to ensure that they are in strictly increasing order without overlap. Error out with a message if the input ranges are out of order or with overlap. Doing it that way, it would be easier to explain to the users how blame -L /A/,/B/ -L /C/,/D/ should work. It would find the first line that matches C _after_ the end of the first range. This is in line with the way we find the end of the range (e.g. the line that matches B) starting from the last line previously specified (e.g. the line that matches A). As implemented by this patch, the behavior of git-blame with multiple -L's is consistent with that of git-log with multiple -L's. The implemented behavior feels intuitive to me, but I can see how the behavior you suggest could feel intuitive to others. If I re-do the patch to work the way you describe above, how should we deal with the inconsistent behaviors between the two commands? - I'd be somewhat unhappy to see coalesce() butchered to blindly accept overlapping ranges (if anything, I'd rather see it tightened to detect such input as a programming error), but this is a minor point. Loosening the behavior bothered enough that I mentioned it centrally in the patch commentary. I can re-implement without (ab)using coalesce(). (In fact, I already have an implementation which re-uses the machinery employed by git-log -L.) -- ES -- To unsubscribe from this list: send the line 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] Convert struct cache_entry * to const ... wherever possible
I attempted to make index_state-cache[] a const struct cache_entry ** to find out how existing entries in index are modified and where. The question I have is what do we do if we really need to keep track of on-disk changes in the index. The result is - diff-lib.c: setting CE_UPTODATE - name-hash.c: setting CE_HASHED - preload-index.c, read-cache.c, unpack-trees.c and builtin/update-index: obvious - entry.c: write_entry() may refresh the checked out entry via fill_stat_cache_info(). This causes non-const struct cache_entry * in builtin/apply.c, builtin/checkout-index.c and builtin/checkout.c - builtin/ls-files.c: --with-tree changes stagemask and may set CE_UPDATE Of these, write_entry() and its call sites are probably most interesting because it modifies on-disk info. But this is stat info and can be retrieved via refresh, at least for porcelain commands. Other just uses ce_flags for local purposes. So, keeping track of dirty entries is just a matter of setting a flag in index modification functions exposed by read-cache.c. Except unpack-trees, the rest of the code base does not do anything funny behind read-cache's back. The actual patch is less valueable than the summary above. But if anyone wants to re-identify the above sites. Applying this patch, then this: diff --git a/cache.h b/cache.h index 430d021..1692891 100644 --- a/cache.h +++ b/cache.h @@ -267,7 +267,7 @@ static inline unsigned int canon_mode(unsigned int mode) #define cache_entry_size(len) (offsetof(struct cache_entry,name) + (len) + 1) struct index_state { - struct cache_entry **cache; + const struct cache_entry **cache; unsigned int version; unsigned int cache_nr, cache_alloc, cache_changed; struct string_list *resolve_undo; will help quickly identify them without bogus warnings. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/apply.c| 13 +++-- builtin/checkout.c | 8 builtin/clean.c| 2 +- builtin/commit.c | 2 +- builtin/grep.c | 2 +- builtin/ls-files.c | 12 ++-- builtin/merge-index.c | 4 ++-- builtin/merge.c| 2 +- builtin/rm.c | 6 +++--- builtin/update-index.c | 14 +++--- cache-tree.c | 19 ++- cache-tree.h | 2 +- cache.h| 2 +- diff.c | 2 +- dir.c | 6 +++--- entry.c| 12 +++- merge-recursive.c | 7 --- pathspec.c | 4 ++-- read-cache.c | 2 +- rerere.c | 12 ++-- resolve-undo.c | 6 +++--- revision.c | 2 +- sequencer.c| 7 --- sha1_name.c| 4 ++-- submodule.c| 2 +- test-dump-cache-tree.c | 4 +++- tree.c | 2 +- unpack-trees.c | 14 +++--- wt-status.c| 4 ++-- 29 files changed, 93 insertions(+), 85 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index 0e9b631..023bb3a 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -2999,7 +2999,7 @@ static int read_blob_object(struct strbuf *buf, const unsigned char *sha1, unsig return 0; } -static int read_file_or_gitlink(struct cache_entry *ce, struct strbuf *buf) +static int read_file_or_gitlink(const struct cache_entry *ce, struct strbuf *buf) { if (!ce) return 0; @@ -3117,7 +3117,7 @@ static struct patch *previous_patch(struct patch *patch, int *gone) return previous; } -static int verify_index_match(struct cache_entry *ce, struct stat *st) +static int verify_index_match(const struct cache_entry *ce, struct stat *st) { if (S_ISGITLINK(ce-ce_mode)) { if (!S_ISDIR(st-st_mode)) @@ -3130,7 +3130,7 @@ static int verify_index_match(struct cache_entry *ce, struct stat *st) #define SUBMODULE_PATCH_WITHOUT_INDEX 1 static int load_patch_target(struct strbuf *buf, -struct cache_entry *ce, +const struct cache_entry *ce, struct stat *st, const char *name, unsigned expected_mode) @@ -3160,7 +3160,8 @@ static int load_patch_target(struct strbuf *buf, * we read from the result of a previous diff. */ static int load_preimage(struct image *image, -struct patch *patch, struct stat *st, struct cache_entry *ce) +struct patch *patch, struct stat *st, +const struct cache_entry *ce) { struct strbuf buf = STRBUF_INIT; size_t len; @@ -3273,7 +3274,7 @@ static int load_current(struct image *image, struct patch *patch) } static int try_threeway(struct image *image, struct patch *patch, - struct stat *st, struct cache_entry *ce) + struct stat
[PATCH] TIG: Implement mkstemps() work-around for platforms lacking it
The function mkstemps() isn't available in all libc implementations. In glibc it first became available in 2.11, so platforms such as RHEL 5 Slackware 13 lack it. This is likely true of many non-LINUX platforms as well. This fixes breakage that was introduced with a0fdac29 Create temporary file with name as suffix. Signed-off-by: Drew Northup n1xim.em...@gmail.com --- This work-around is taken from Git and was inspired by code in libiberty. It is presumed that this isn't a problem due to compatible license terms. A (virtually identical) version of this available in https://github.com/n1xim/tig/tree/mkstemps_wkarnd (differences only in the commit message). configure.ac | 4 io.c | 77 io.h | 14 +++ 3 files changed, 95 insertions(+) diff --git a/configure.ac b/configure.ac index 8dd2508..40e1f85 100644 --- a/configure.ac +++ b/configure.ac @@ -21,6 +21,10 @@ AC_SUBST(CURSES_LIB) AM_ICONV +dnl Not all platforms have mkstemps +AC_CHECK_FUNC([mkstemps], [AC_DEFINE([HAVE_MKSTEMPS], [1], + [Define if mkstemps is available.])]) + AC_PROG_CC AC_CHECK_PROGS(ASCIIDOC, [asciidoc], [false]) diff --git a/io.c b/io.c index 3ff1d1c..f1b6fbc 100644 --- a/io.c +++ b/io.c @@ -237,6 +237,83 @@ encoding_convert(struct encoding *encoding, char *line) } /* + * Compatibility: no mkstemps() + */ + +/* Adapted from libiberty's mkstemp.c via Git's wrapper.c. */ + +#undef TMP_MAX +#define TMP_MAX 16384 + +int tig_mkstemps_mode(char *pattern, int suffix_len, int mode) +{ + static const char letters[] = + abcdefghijklmnopqrstuvwxyz + ABCDEFGHIJKLMNOPQRSTUVWXYZ + 0123456789; + static const int num_letters = 62; + uint64_t value; + struct timeval tv; + char *template; + size_t len; + int fd, count; + + len = strlen(pattern); + + if (len 6 + suffix_len) { + errno = EINVAL; + return -1; + } + + if (strncmp(pattern[len - 6 - suffix_len], XX, 6)) { + errno = EINVAL; + return -1; + } + + /* +* Replace pattern's XX characters with randomness. +* Try TMP_MAX different filenames. +*/ + gettimeofday(tv, NULL); + value = ((size_t)(tv.tv_usec 16)) ^ tv.tv_sec ^ getpid(); + template = pattern[len - 6 - suffix_len]; + for (count = 0; count TMP_MAX; ++count) { + uint64_t v = value; + /* Fill in the random bits. */ + template[0] = letters[v % num_letters]; v /= num_letters; + template[1] = letters[v % num_letters]; v /= num_letters; + template[2] = letters[v % num_letters]; v /= num_letters; + template[3] = letters[v % num_letters]; v /= num_letters; + template[4] = letters[v % num_letters]; v /= num_letters; + template[5] = letters[v % num_letters]; v /= num_letters; + + fd = open(pattern, O_CREAT | O_EXCL | O_RDWR, mode); + if (fd 0) + return fd; + /* +* Fatal error (EPERM, ENOSPC etc). +* It doesn't make sense to loop. +*/ + if (errno != EEXIST) + break; + /* +* This is a random value. It is only necessary that +* the next TMP_MAX values generated by adding to +* VALUE are different with (module 2^32). +*/ + value += ; + } + /* We return the null string if we can't find a unique file name. */ + pattern[0] = '\0'; + return -1; +} + +int tigmkstemps(char *pattern, int suffix_len) +{ + return tig_mkstemps_mode(pattern, suffix_len, 0600); +} + +/* * Executing external commands. */ diff --git a/io.h b/io.h index 646989d..8f43216 100644 --- a/io.h +++ b/io.h @@ -16,6 +16,9 @@ #include tig.h +/* Needed for mkstemps workaround */ +#include stdint.h + /* * Argument array helpers. */ @@ -41,6 +44,17 @@ struct encoding *encoding_open(const char *fromcode); char *encoding_convert(struct encoding *encoding, char *line); /* + * Compatibility: no mkstemps() + */ + +#ifndef HAVE_MKSTEMPS +#define mkstemps tigmkstemps +#endif + +int tigmkstemps(char *, int); +int tig_mkstemps_mode(char *pattern, int suffix_len, int mode); + +/* * Executing external commands. */ -- 1.8.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 5.5/22] Add documentation for the index api
On Tue, Jul 9, 2013 at 3:54 AM, Thomas Gummerer t.gumme...@gmail.com wrote: As promised, a draft for a documentation for the index api as it is in this series. First of all, it may be a good idea to acknowledge index_state-cache[] as part of the API for now. Not hiding it simplifies a few things (no need for new next_ce field, no worries about rewinding in unpack-trees..). Supporting partial loading (and maybe partial update in some cases) with this API and index_state-cache[] part of the API are good enough for me. We can do another tree-based API or something update later when it's formed (I looked at your index-v5api branch but I don't think a tree-based api was there, my concern is how much extra head pre-v5 has to pay to use tree-based api). +`read_index_filtered(opts)`:: + This method behaves differently for index-v2 and index-v5. + + For index-v2 it simply reads the whole index as read_index() + does, so we are sure we don't have to reload anything if the + user wants a different filter. It also sets the filter_opts + in the index_state, which is used to limit the results when + iterating over the index with for_each_index_entry(). + + The whole index is read to avoid the need to eventually + re-read the index later, because the performance is no + different when reading it partially. + + For index-v5 it creates an adjusted_pathspec to filter the + reading. First all the directory entries are read and then + the cache_entries in the directories that match the adjusted + pathspec are read. The filter_opts in the index_state are set + to filter out the rest of the cache_entries that are matched + by the adjusted pathspec but not by the pathspec given. The + rest of the index entries are filtered out when iterating over + the cache with for_each_index_entries. You can state in the API that the input pathspec is used as a hint to load only a portion of the index. read_index_filtered may load _more_ than necessary. It's the caller's responsibility to verify again which is matched and which is not. That's how read_directory is done. I think it gives you more liberty in loading strategy. It's already true for v2 because full index is loaded regardless of the given pathspec. In the end, we have a linear list (from public view) of cache entries, accessible via index_state-cache[]. If you happen to know that certain entries match the given pathspec, you could help the caller avoid match_pathspec'ing again by set a bit in ce_flags. To know which entry exists in the index and which is new, use another flag. Most reader code won't change if we do it this way, all match_pathspec() remain where they are. +`for_each_index_entry(fn, cb_data)`:: + Iterates over all cache_entries in the index filtered by + filter_opts in the index_stat. For each cache entry fn is + executed with cb_data as callback data. From within the loop + do `return 0` to continue, or `return 1` to break the loop. Because we don't attempt to hide index_state-cache[], this one may be for convenience, the user is not required to convert to it. Actually I think this may be slower because of the cost of calling function pointer. +`next_index_entry(ce)`:: + Returns the cache_entry that follows after ce next_ce field and this method may be gone too, just access index_state-cache[] +`index_change_filter_opts(opts)`:: + This function again has a slightly different functionality for + index-v2 and index-v5. + + For index-v2 it simply changes the filter_opts, so + for_each_index_entry uses the changed index_opts, to iterate + over a different set of cache entries. + + For index-v5 it refreshes the index if the filter_opts have + changed and sets the new filter_opts in the index state, again + to iterate over a different set of cache entries as with + index-v2. + + This has some optimization potential, in the case that the + opts get stricter (less of the index should be read) it + doesn't have to reload anything, but currently does. The only use case I see so far is converting a partial index_state back to a full one. Apart from doing so in order to write the new index, I think some operation (like rename tracking in diff or unpack-trees) may expect full index. I think we should support that. I doubt we need to change pathspec to something different than the one we used to load the index. When a user passes a pathspec to a command, the user expects the command to operate on that set only, not outside. If you take the input pathspec at loading just as a hint, you could load all related directory blocks and all files in those blocks, so that expanding to full index is simply adding more files from missing directory blocks (and their files). An advantage of not strictly follow the input
RE: standarize mtime when git checkout
Thanks René~ I'll use git archive command to create the tarball. Rick -Original Message- From: René Scharfe [mailto:rene.scha...@lsrfire.ath.cx] Sent: Monday, July 08, 2013 8:54 PM To: Rick Liu Cc: git@vger.kernel.org Subject: Re: standarize mtime when git checkout Am 08.07.2013 23:39, schrieb Rick Liu: Hi, Currently when doing git checkout (either for a branch or a tag), if the file doesn't exist before, the file will be created using current datetime. This causes problem while trying to tar the git repository source files (excluding .git folder). The tar binary can be different even all of file contents are the same (eg. from the same GIT commit) because the mtime for the files might be different due to different git checkout time. eg: User A checkout the commit at time A and then tarball the folder. User B checkout the same commit as time B and then tarball the folder. The result tarball are binary different even though all of tarball contents are the same except the mtime for each file. Can we use GIT's commit time as the mtime for all of files/folders when we do git checkout? That would break tools like make which rely on a files mtime to build them. They wouldn't be able to detect switching between source file versions that are older than the latest build. You can use git archive to create tar files in which all entries have their mtime set to the commit time. Such archives only contain tracked (committed) files, though. And different versions of git can create slightly different archives, but such changes have been rare. René -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] describe: use argv-array
Junio C Hamano gits...@pobox.com writes: Jeff King p...@peff.net writes: On Sun, Jul 07, 2013 at 03:33:43PM -0700, Junio C Hamano wrote: + argv_array_init(args); + argv_array_push(args, name-rev); + argv_array_push(args, --name-only); + argv_array_push(args, --no-undefined); [...] - memcpy(args + i, argv, argc * sizeof(char *)); - args[i + argc] = NULL; - return cmd_name_rev(i + argc, args, prefix); + return cmd_name_rev(args.argc, args.argv, prefix); This leaks the memory allocated by args. The original did, too, and it is probably not that big a deal (we exit right after anyway). The fix would be something like: rc = cmd_name_rev(args.argc, args.argv, prefix); argv_array_clear(args); return rc; Yes; this was meant as a straight rewrite and I did not bother, but I should have cleaned it up as I meant to build on top. Will amend, even though I do not think we need to build anything on top. Heh, you fooled me. cmd_name_rev() uses the usual parse-options machinery that updates args.argv[]. Dashed options that were consumed will not remain in args.argv[] and argv_array_clear() will not have a chance to free them, and besides, args.argc and args.argv will be out of sync and wreaks havoc in argv_array_clear(). We could expose argv_array_push_nodup() and use it in this caller and then free the args.argv[] but not its contents, but I do not think it is worth 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: [PATCH] Convert struct cache_entry * to const ... wherever possible
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: I attempted to make index_state-cache[] a const struct cache_entry ** to find out how existing entries in index are modified and where. The question I have is what do we do if we really need to keep track of on-disk changes in the index. The result is - diff-lib.c: setting CE_UPTODATE - name-hash.c: setting CE_HASHED - preload-index.c, read-cache.c, unpack-trees.c and builtin/update-index: obvious - entry.c: write_entry() may refresh the checked out entry via fill_stat_cache_info(). This causes non-const struct cache_entry * in builtin/apply.c, builtin/checkout-index.c and builtin/checkout.c - builtin/ls-files.c: --with-tree changes stagemask and may set CE_UPDATE Of these, write_entry() and its call sites are probably most interesting because it modifies on-disk info. But this is stat info and can be retrieved via refresh, at least for porcelain commands. Other just uses ce_flags for local purposes. So, keeping track of dirty entries is just a matter of setting a flag in index modification functions exposed by read-cache.c. Except unpack-trees, the rest of the code base does not do anything funny behind read-cache's back. The actual patch is less valueable than the summary above. But if anyone wants to re-identify the above sites. Applying this patch, then this: diff --git a/cache.h b/cache.h index 430d021..1692891 100644 --- a/cache.h +++ b/cache.h @@ -267,7 +267,7 @@ static inline unsigned int canon_mode(unsigned int mode) #define cache_entry_size(len) (offsetof(struct cache_entry,name) + (len) + 1) struct index_state { - struct cache_entry **cache; + const struct cache_entry **cache; unsigned int version; unsigned int cache_nr, cache_alloc, cache_changed; struct string_list *resolve_undo; will help quickly identify them without bogus warnings. Nicely done and a very interesting result. I quickly eyeballed the output of $ git grep -e 'struct cache_entry' --and --not -e 'const struct cache_entry' and the result matches what I would expect to see. -- To unsubscribe from this list: send the line 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] gitweb: Ensure OPML text fits inside its box.
The rss_logo CSS style has a fixed width which is too narrow for the string OPML. Replace the fixed width with horizontal padding so the text fits with nice margins. --- For before/after examples, see http://dotat.at/cgi/git (overflow) and https://git.csx.cam.ac.uk/x/ucs/ (padded). gitweb/static/gitweb.css | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css index cb86d2d..a869be1 100644 --- a/gitweb/static/gitweb.css +++ b/gitweb/static/gitweb.css @@ -548,8 +548,7 @@ a.linenr { a.rss_logo { float: right; - padding: 3px 0px; - width: 35px; + padding: 3px 5px; line-height: 10px; border: 1px solid; border-color: #fcc7a5 #7d3302 #3e1a01 #ff954e; -- 1.8.3.1.605.g85318f5 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git in nutshell Inbox
On Tue, Jul 09, 2013 at 04:50:20PM +0200, Muhammad Bashir Al-Noimi wrote: I tried to use git-cola and found it complicated and not user friendly (maybe because I came from Bazaar Explorer) I noticed that it needs terminal to be able to work (ex. it can't create new repository). I can't even ask for merge modifications (I'm still know nothing in git)! so do you know better GUI utility? You don't need the terminal to create a new repository. When starting git-cola from a non-git directory you get a menu where you can open an existing git-repository, clone an existing git-repository or create a new one. However I agree with Matthieu that it's easier to understand git from the command line. Git is developed and mostly used from the command line. It's the commandline that will give you most controll and most functionality. -- Med vänliga hälsningar Fredrik Gustafsson tel: 0733-608274 e-post: iv...@iveqy.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/RFC] blame: accept multiple -L ranges
Eric Sunshine sunsh...@sunshineco.com writes: As implemented by this patch, the behavior of git-blame with multiple -L's is consistent with that of git-log with multiple -L's. The implemented behavior feels intuitive to me, but I can see how the behavior you suggest could feel intuitive to others. If I re-do the patch to work the way you describe above, how should we deal with the inconsistent behaviors between the two commands? To be extremely honest, I do not care too deeply about what log -L does today, because it is still in may have rough edges but is an interesting toy to play with state in my mind ;-) The suggestion to error out was more about start simple, strict and obvious to make it easy to explain and nothing else. If we start with a simple and strict version, we can later loosen it without making an input that was valid earlier invalid. If we start with too loose, on the other hand, it would be hard to tighten it later. But the only two things I care deeply about are, in a file whose contents is: C B A B C D (1) The range -L /A/,/B/ finds the first A from the beginning, and then chooses B that comes _after_ it, making it equivalent to -L3,4 (not -L3,2 or -L2,3). (2) In the ranges -L anything,/B/ -L /C/,anything, the beginning of the second range is found by choosing C that comes _after_ the end of the previous range (/B/ may choose either the second or the 4th line, and the only C that comes after either of them is the 5th line and that is where the second range should begin, not at the beginning of the file). The same for -L 1,3 -L /C/ (only C that comes after 3 is eligible to be the beginning of the second range). I view it as a nice addition to coalesce two overlapping ranges given exactly by numbers, e.g. -L 100,200 -L 50,102. I do not have a strong objection to it, as long as it does not interfere negatively with ranges specified by patterns. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gitweb: vertically centre contents of page footer
--- gitweb/static/gitweb.css | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css index a869be1..3b4d833 100644 --- a/gitweb/static/gitweb.css +++ b/gitweb/static/gitweb.css @@ -68,12 +68,13 @@ div.page_path { } div.page_footer { - height: 17px; + height: 22px; padding: 4px 8px; background-color: #d9d8d1; } div.page_footer_text { + line-height: 22px; float: left; color: #55; font-style: italic; -- 1.8.3.1.605.g85318f5 -- To unsubscribe from this list: send the line 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/RFC] blame: accept multiple -L ranges
On Tue, Jul 9, 2013 at 12:39 PM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: As implemented by this patch, the behavior of git-blame with multiple -L's is consistent with that of git-log with multiple -L's. The implemented behavior feels intuitive to me, but I can see how the behavior you suggest could feel intuitive to others. If I re-do the patch to work the way you describe above, how should we deal with the inconsistent behaviors between the two commands? The suggestion to error out was more about start simple, strict and obvious to make it easy to explain and nothing else. If we start with a simple and strict version, we can later loosen it without making an input that was valid earlier invalid. If we start with too loose, on the other hand, it would be hard to tighten it later. Makes sense. But the only two things I care deeply about are, in a file whose contents is: C B A B C D (1) The range -L /A/,/B/ finds the first A from the beginning, and then chooses B that comes _after_ it, making it equivalent to -L3,4 (not -L3,2 or -L2,3). (2) In the ranges -L anything,/B/ -L /C/,anything, the beginning of the second range is found by choosing C that comes _after_ the end of the previous range (/B/ may choose either the second or the 4th line, and the only C that comes after either of them is the 5th line and that is where the second range should begin, not at the beginning of the file). The same for -L 1,3 -L /C/ (only C that comes after 3 is eligible to be the beginning of the second range). Thinking aloud: Thus, for -L 1,5 -L /C/, no C would be found. I view it as a nice addition to coalesce two overlapping ranges given exactly by numbers, e.g. -L 100,200 -L 50,102. I do not have a strong objection to it, as long as it does not interfere negatively with ranges specified by patterns. Okay, I will base my rewrite upon the above constraints, along with other observations from your initial response. -- ES -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git in nutshell Inbox
On Tue, Jul 9, 2013 at 7:02 PM, Fredrik Gustafsson iv...@iveqy.com wrote: You don't need the terminal to create a new repository. When starting git-cola from a non-git directory you get a menu where you can open an existing git-repository, clone an existing git-repository or create a new one I'm using git-cola 1.4.3.5-1 it doesn't create a new repository it just open or clone. Its interface doesn't appear unless the user launched git repository folder -- Best Regards Muhammad Bashir Al-Noimi -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git in nutshell Inbox
On Tue, Jul 09, 2013 at 07:26:57PM +0200, Muhammad Bashir Al-Noimi wrote: On Tue, Jul 9, 2013 at 7:02 PM, Fredrik Gustafsson iv...@iveqy.com wrote: You don't need the terminal to create a new repository. When starting git-cola from a non-git directory you get a menu where you can open an existing git-repository, clone an existing git-repository or create a new one I'm using git-cola 1.4.3.5-1 it doesn't create a new repository it just open or clone. Its interface doesn't appear unless the user launched git repository folder Well, I can't speak for such old version. git-cola is about six years old and you're using a two year old release. You miss 1/3 of all development that has been done on git-cola. I suggest you update to a newer version. Too bad that ubuntu shipped such old version. -- Med vänliga hälsningar Fredrik Gustafsson tel: 0733-608274 e-post: iv...@iveqy.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/RFC] blame: accept multiple -L ranges
Junio C Hamano gits...@pobox.com writes: (2) In the ranges -L anything,/B/ -L /C/,anything, the beginning of the second range is found by choosing C that comes _after_ the end of the previous range (/B/ may choose either the second or the 4th line, and the only C that comes after either of them is the 5th line and that is where the second range should begin, not at the beginning of the file). The same for -L 1,3 -L /C/ (only C that comes after 3 is eligible to be the beginning of the second range). So passing several -L arguments does not blame the union of what each argument would blame individually? Doesn't that make it rather harder to explain? In any case, if you define it like that for blame, log -L should be changed to match. -- 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
Re: [PATCH/RFC] blame: accept multiple -L ranges
On Tue, Jul 9, 2013 at 1:42 PM, Thomas Rast tr...@inf.ethz.ch wrote: Junio C Hamano gits...@pobox.com writes: (2) In the ranges -L anything,/B/ -L /C/,anything, the beginning of the second range is found by choosing C that comes _after_ the end of the previous range (/B/ may choose either the second or the 4th line, and the only C that comes after either of them is the 5th line and that is where the second range should begin, not at the beginning of the file). The same for -L 1,3 -L /C/ (only C that comes after 3 is eligible to be the beginning of the second range). So passing several -L arguments does not blame the union of what each argument would blame individually? Doesn't that make it rather harder to explain? I don't think Junio meant to imply that. Collecting the blame ranges can/should be a distinct step from coalescing them. Junio is saying that an -L /re/ range search should start after the maximum line number already specified by any preceding range. Once all input ranges are collected, they can be coalesced. (If a -L /re/ range happens to be coalesced with or into some other range, that's fine: you're still seeing blame output for the requested lines.) -- ES -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Diff colorizer confused by dos newlines
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 When I try to look at a color diff of a file using dos newlines, the output gets an odd sequence of ansi escapes and a stray carriage return showing up only on the + lines, but not the -. The normal looking - lines look like this: \r\n ( from previous line ), ansi color escape, '-', whitespace, text, terminating ansi escpae ( [m ), \r\n. The broken + lines look like this: \r\n ( from previous line ), ansi color sequence, '+', terminating ansi escape ( [m ), whitespace, ansi color sequence, text, terminating ansi escape, ansi color sequence, stray \r, terminating ansi escape, \n. Any suggestions on how to resolve this? -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.17 (MingW32) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJR3FZQAAoJEJrBOlT6nu75mvIIAKVKcK2GeVNHKRBkkljqE1U5 2piftz+CO6sVZGba8DUxMdbA5tCDQrz11yzowuKXDtyr1hxhgjBoXcsN36RZhYdu gijE6qF5w/na6MdPgJ7LMizHo8xOeVGhrDr+qhM/5nD77rVumtEnGAdoEqdY+uY3 mYfHaz2dHAG3W7mOlfvycb4HhRBao64pGh5JnuyvvnZKSXkOyJozjzTEzC7tuNU8 b9qofVKnTMse7Ek6jGp64GNaxxtcQCt1J8cd2uOJtROUK2g9KgVhy2QSRFqoZ1yO zEtTD28bj7nWJubsgVyOdtx0ClxiO1RHQRqQH2/zQM6NlfntAljD15bHcPNJaKo= =g3R+ -END PGP SIGNATURE- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git in nutshell Inbox
On Tue, Jul 9, 2013 at 10:39 AM, Matthieu Moy matthieu@imag.fr wrote: Muhammad Bashir Al-Noimi mbno...@gmail.com writes: I'm bzr user and I want to migrate to git. Generally I use bzr through Bazaar Explorer which is very easy neat GUI utility for managing bzr repositories. May you please guide me to most easy way to migrate to Git? (I think it's a mistake to stick to GUI: the concepts are easier to understand with Git commands IMHO) Ditto. I think learning the command line makes the process much easier in the end. But there are nice GUIs for Git too. The official one is git gui, distributed with Git. I do not like the visual aspect a lot, but it has a very good coverage of Git's functionalities. Otherwise, have a look at https://git.wiki.kernel.org/index.php/InterfacesFrontendsAndTools#Graphical_Interfaces git-cola is a good one. You might want to consider giggle. I've not used it, but it's more BzrExplorer-like, IIRC. -John -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] describe: use argv-array
On Tue, Jul 09, 2013 at 09:00:20AM -0700, Junio C Hamano wrote: + return cmd_name_rev(args.argc, args.argv, prefix); This leaks the memory allocated by args. The original did, too, and it is probably not that big a deal (we exit right after anyway). The fix would be something like: rc = cmd_name_rev(args.argc, args.argv, prefix); argv_array_clear(args); return rc; Yes; this was meant as a straight rewrite and I did not bother, but I should have cleaned it up as I meant to build on top. Will amend, even though I do not think we need to build anything on top. Heh, you fooled me. cmd_name_rev() uses the usual parse-options machinery that updates args.argv[]. Dashed options that were consumed will not remain in args.argv[] and argv_array_clear() will not have a chance to free them, and besides, args.argc and args.argv will be out of sync and wreaks havoc in argv_array_clear(). Ick, yeah, I forgot about that. Let's just leave it as a leak, then. We are exiting immediately afterwards, anyway. -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/RFC] blame: accept multiple -L ranges
Eric Sunshine sunsh...@sunshineco.com writes: On Tue, Jul 9, 2013 at 1:42 PM, Thomas Rast tr...@inf.ethz.ch wrote: Junio C Hamano gits...@pobox.com writes: (2) In the ranges -L anything,/B/ -L /C/,anything, the beginning of the second range is found by choosing C that comes _after_ the end of the previous range (/B/ may choose either the second or the 4th line, and the only C that comes after either of them is the 5th line and that is where the second range should begin, not at the beginning of the file). The same for -L 1,3 -L /C/ (only C that comes after 3 is eligible to be the beginning of the second range). So passing several -L arguments does not blame the union of what each argument would blame individually? Doesn't that make it rather harder to explain? I don't think Junio meant to imply that. Collecting the blame ranges can/should be a distinct step from coalescing them. Junio is saying that an -L /re/ range search should start after the maximum line number already specified by any preceding range. I am not sure if I want maximum specified so far. I meant start searching at the last location, e.g. -L 100,200 -L 4,6 -L /A/,+20 would want to find the first A after line 6, not after line 200. Once all input ranges are collected, they can be coalesced. (If a -L /re/ range happens to be coalesced with or into some other range, that's fine: you're still seeing blame output for the requested lines.) Yes. -- To unsubscribe from this list: send the line 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/RFC] blame: accept multiple -L ranges
Thomas Rast tr...@inf.ethz.ch writes: Junio C Hamano gits...@pobox.com writes: (2) In the ranges -L anything,/B/ -L /C/,anything, the beginning of the second range is found by choosing C that comes _after_ the end of the previous range (/B/ may choose either the second or the 4th line, and the only C that comes after either of them is the 5th line and that is where the second range should begin, not at the beginning of the file). The same for -L 1,3 -L /C/ (only C that comes after 3 is eligible to be the beginning of the second range). So passing several -L arguments does not blame the union of what each argument would blame individually? Doesn't that make it rather harder to explain? In any case, if you define it like that for blame, log -L should be changed to match. I thought log -L was fundamnetally different as it is per path. For example, -L1,100:hello.c and -L5,105:goodbye.c would not intersect/overlap. If you mean to coalesce -L1,100:hello.c and -L5,105:hello.c into a single -L1,105:hello.c, then yes I think it makes sense. -- To unsubscribe from this list: send the line 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/RFC] blame: accept multiple -L ranges
On Tue, Jul 9, 2013 at 2:55 PM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: On Tue, Jul 9, 2013 at 1:42 PM, Thomas Rast tr...@inf.ethz.ch wrote: Junio C Hamano gits...@pobox.com writes: (2) In the ranges -L anything,/B/ -L /C/,anything, the beginning of the second range is found by choosing C that comes _after_ the end of the previous range (/B/ may choose either the second or the 4th line, and the only C that comes after either of them is the 5th line and that is where the second range should begin, not at the beginning of the file). The same for -L 1,3 -L /C/ (only C that comes after 3 is eligible to be the beginning of the second range). So passing several -L arguments does not blame the union of what each argument would blame individually? Doesn't that make it rather harder to explain? I don't think Junio meant to imply that. Collecting the blame ranges can/should be a distinct step from coalescing them. Junio is saying that an -L /re/ range search should start after the maximum line number already specified by any preceding range. I am not sure if I want maximum specified so far. I meant start searching at the last location, e.g. -L 100,200 -L 4,6 -L /A/,+20 would want to find the first A after line 6, not after line 200. Okay. Once all input ranges are collected, they can be coalesced. (If a -L /re/ range happens to be coalesced with or into some other range, that's fine: you're still seeing blame output for the requested lines.) Yes. -- To unsubscribe from this list: send the line 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/RFC] blame: accept multiple -L ranges
Junio C Hamano gits...@pobox.com writes: Eric Sunshine sunsh...@sunshineco.com writes: On Tue, Jul 9, 2013 at 1:42 PM, Thomas Rast tr...@inf.ethz.ch wrote: Junio C Hamano gits...@pobox.com writes: (2) In the ranges -L anything,/B/ -L /C/,anything, the beginning of the second range is found by choosing C that comes _after_ the end of the previous range (/B/ may choose either the second or the 4th line, and the only C that comes after either of them is the 5th line and that is where the second range should begin, not at the beginning of the file). The same for -L 1,3 -L /C/ (only C that comes after 3 is eligible to be the beginning of the second range). So passing several -L arguments does not blame the union of what each argument would blame individually? Doesn't that make it rather harder to explain? I don't think Junio meant to imply that. Collecting the blame ranges can/should be a distinct step from coalescing them. Junio is saying that an -L /re/ range search should start after the maximum line number already specified by any preceding range. I am not sure if I want maximum specified so far. I meant start searching at the last location, e.g. -L 100,200 -L 4,6 -L /A/,+20 would want to find the first A after line 6, not after line 200. Ok, so my point (in new words, since the old one was apparently too terse) is: If you define it that way, the output of git blame -L 4,6; git blame -L /A/,+20 is significantly different from git blame -L 4,6 -L /A/,+20 Not just in the presentation or any possible coalescing, but in the meaning of the ranges. Do you really want to make it that way? -- 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
Re: Git --file doesn't override $HOME in version 1.8.1.2
On Tue, Jul 09, 2013 at 01:49:21PM +0200, Thomas Rast wrote: Rather, it's very strange that 'git config --file foo' tries to look at any config file other than 'foo'. In a git repo: $ strace git config --file fooconfig test.var 21 | grep 'open.*config' open(/home/thomas/.gitconfig, O_RDONLY) = 3 open(.git/config, O_RDONLY) = 3 open(/home/thomas/.gitconfig, O_RDONLY) = 3 open(.git/config, O_RDONLY) = 3 open(fooconfig, O_RDONLY) = 3 I haven't looked into the code yet. Probably it's simply following the usual code paths to discover a repo and read its config. However, with the --file option, it shouldn't. I'm not so sure. It is (in theory) OK to read the usual config files to find out _how_ git-config should behave, but then return results from a specific file. The former should read the normal files, and the latter should read whatever is specified by the options (--file, a specific level like --global, or the usual set of files). There are probably not many config options that can affect git-config's behavior. The few I can think of are: 1. core.editor should affect git config --edit 2. pager.config would auto-start a pager. I am not sure if that is a sane thing to do or not. 3. In theory you could have advice.* affect git-config, but I do not think any currently do. 4. Currently git-config does not read objects, but there are patches proposed to do so. In that case, things like core.packedGitWindowSize might be important. So I think you could probably drop the config parsing, special-case (1), and ignore (2) as silly. But I think (3) and (4) show that it isn't the right thing to do; you will never know which config options affect git-config's behavior in the future. The real issue here is not the extra normal config parsing; it is that the normal parsing does not work in some cases. And that has already been fixed by Jonathan's 4698c8f (config: allow inaccessible configuration under $HOME, 2013-04-12). -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/RFC] blame: accept multiple -L ranges
Thomas Rast tr...@inf.ethz.ch writes: If you define it that way, the output of git blame -L 4,6; git blame -L /A/,+20 is significantly different from git blame -L 4,6 -L /A/,+20 Not just in the presentation or any possible coalescing, but in the meaning of the ranges. Do you really want to make it that way? Absolutely. The primary reason I want to be able to specify two ranges at the same time is to follow two functions in a file that appear in separate places, and /A/ might not be unique. When I want to say I want to see from here to there, and then from here to there, and then from here to there, it would be very frustrating if and then resets what I mean by here every time and make these three evaluated independently. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] merge -Xindex-only
On 07/09/2013 02:08 PM, Thomas Rast wrote: Michael Haggerty mhag...@alum.mit.edu writes: Since you've already implemented a way to merge into the index (even an alternative index) without touching the working copy, I'll just cross my fingers and hope for the appearance of an option that makes merge leave HEAD, MERGE_HEAD, etc. untouched. The most annoying part is probably where to put the output, since merging is more or less defined to do one of: - update HEAD and return 0 - update MERGE_HEAD and return 1 I don't understand what you mean here. Why does *any* reference need to be updated? Why not * load arbitrary commit-ish A into index * merge arbitrary commit-ish B into index * return error/OK depending on whether there was a conflict ? The script that started the whole process would know what A and B are and could create the commit itself using git write-tree and git commit-tree -p A -p B. And if the index were an alternative index chosen via GIT_INDEX_FILE then the rest of the git repo would be none the wiser. I'm not sure how much flexibility is worth having. Would it be sufficient if you had an option, e.g. -Xresult-ref=refs/heads/foo, that changes it to: - update refs/heads/foo and return 0 - return 1, not updating any refs That would mean that it would only work for noninteractive use. In the conflicting case, the driving script would need to remember what it wanted to merge so as have the information when finally committing. That would be fine with me. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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: Diff colorizer confused by dos newlines
On Tue, Jul 09, 2013 at 02:28:32PM -0400, Phillip Susi wrote: When I try to look at a color diff of a file using dos newlines, the output gets an odd sequence of ansi escapes and a stray carriage return showing up only on the + lines, but not the -. The normal looking - lines look like this: \r\n ( from previous line ), ansi color escape, '-', whitespace, text, terminating ansi escpae ( [m ), \r\n. The broken + lines look like this: \r\n ( from previous line ), ansi color sequence, '+', terminating ansi escape ( [m ), whitespace, ansi color sequence, text, terminating ansi escape, ansi color sequence, stray \r, terminating ansi escape, \n. That's intentional; the added lines go through the whitespace checker to help you identify potential whitespace problems (there is not much point showing them on lines going away, since you are getting rid of them). Any suggestions on how to resolve this? Try: git config core.whitespace cr-at-eol See the description of core.whitespace in git help config for more details. -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 0/7] safer push --force with compare-and-swap
When you have to replace an already published branch with its rebased version, you would have to --force, but then you risk losing work, if any, that was pushed by somebody else while you are working on rebasing, as your earlier decision that replacing the old one with its rebase is OK was based on the assumption that there is nothing else going on. Unfortunately, --force is blind, and did not offer this ... but fail if the tip has moved from what I expect. And here is a series to remedy it. It lets you specify what the expected current values of refs you are attempting to update, and if you are lazy, the value is taken from your remote tracking branch for the ref you are attempting to update. I am not married to the lockref name, but I think the semantics implemented here is what we discussed to do in the earlier discussion. cf. http://thread.gmane.org/gmane.comp.version-control.git/229430 This may still be rough at edges, but a basic testset seems to pass. I haven't bothered to check the smart-http transport; help is greatly appreciated. Junio C Hamano (7): cache.h: move remote/connect API out of it builtin/push.c: use OPT_BOOL, not OPT_BOOLEAN push: beginning of compare-and-swap force/delete safety remote.c: add command line option parser for --lockref push --lockref: implement logic to populate old_sha1_expect[] t5533: test push --lockref push: document --lockref Documentation/git-push.txt | 26 +++ builtin/fetch-pack.c | 2 + builtin/push.c | 18 - builtin/receive-pack.c | 1 + builtin/send-pack.c| 26 +++ cache.h| 62 connect.c | 1 + connect.h | 13 fetch-pack.c | 1 + fetch-pack.h | 1 + refs.c | 8 --- remote.c | 149 +- remote.h | 83 + send-pack.c| 2 + t/t5533-push-cas.sh| 176 + transport-helper.c | 6 ++ transport.c| 13 transport.h| 5 ++ upload-pack.c | 1 + 19 files changed, 519 insertions(+), 75 deletions(-) create mode 100644 connect.h create mode 100755 t/t5533-push-cas.sh -- 1.8.3.2-875-g76c723c -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/7] t5533: test push --lockref
Prepare two repositories, src and dst, the latter of which is a clone of the former (with tracking branches), and push from the latter into the former, using --lockref=name (using tracking ref for name when updating name), --lockref=name:value, --lockref=name: (i.e. check creation), and --lockref (using tracking ref for anything that we update). Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t5533-push-cas.sh | 176 1 file changed, 176 insertions(+) create mode 100755 t/t5533-push-cas.sh diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh new file mode 100755 index 000..c080467 --- /dev/null +++ b/t/t5533-push-cas.sh @@ -0,0 +1,176 @@ +#!/bin/sh + +test_description='compare swap push force/delete safety' + +. ./test-lib.sh + +setup_srcdst_basic () { + rm -fr src dst + git clone --no-local . src + git clone --no-local src dst + ( + cd src git checkout HEAD^0 + ) +} + +test_expect_success setup ' + : create template repository + test_commit A + test_commit B + test_commit C +' + +test_expect_success 'push to create (protected)' ' + setup_srcdst_basic + ( + cd dst + test_commit D + test_must_fail git push --lockref=master: origin master + test_must_fail git push --force --lockref=master: origin master + ) + expect + git ls-remote src refs/heads/naster actual + test_cmp expect actual +' + +test_expect_success 'push to create (allowed)' ' + setup_srcdst_basic + ( + cd dst + test_commit D + git push --lockref=naster: origin HEAD:naster + ) + git ls-remote dst refs/heads/master | + sed -e s/master/naster/ expect + git ls-remote src refs/heads/naster actual + test_cmp expect actual +' + +test_expect_success 'push to update (protected)' ' + setup_srcdst_basic + ( + cd dst + test_commit D + test_must_fail git push --lockref=master:master origin master + test_must_fail git push --force --lockref=master:master origin master + ) + git ls-remote . refs/heads/master expect + git ls-remote src refs/heads/master actual + test_cmp expect actual +' + +test_expect_success 'push to update (protected, tracking)' ' + setup_srcdst_basic + ( + cd src + git checkout master + test_commit D + git checkout HEAD^0 + ) + git ls-remote src refs/heads/master expect + ( + cd dst + test_commit E + git ls-remote . refs/remotes/origin/master expect + test_must_fail git push --lockref=master origin master + test_must_fail git push --force --lockref=master origin master + git ls-remote . refs/remotes/origin/master actual + test_cmp expect actual + ) + git ls-remote src refs/heads/master actual + test_cmp expect actual +' + +test_expect_success 'push to update (allowed)' ' + setup_srcdst_basic + ( + cd dst + test_commit D + git push --lockref=master:master^ origin master + ) + git ls-remote dst refs/heads/master expect + git ls-remote src refs/heads/master actual + test_cmp expect actual +' + +test_expect_success 'push to update (allowed, tracking)' ' + setup_srcdst_basic + ( + cd dst + test_commit D + git push --lockref=master origin master + ) + git ls-remote dst refs/heads/master expect + git ls-remote src refs/heads/master actual + test_cmp expect actual +' + +test_expect_success 'push to update (still rejected with non-ff check)' ' + setup_srcdst_basic + git ls-remote src refs/heads/master expect + ( + cd dst + git reset --hard HEAD^ + test_commit D + test_must_fail git push --lockref=master origin master + ) + git ls-remote src refs/heads/master actual + test_cmp expect actual +' + +test_expect_success 'push to delete (protected)' ' + setup_srcdst_basic + git ls-remote src refs/heads/master expect + ( + cd dst + test_must_fail git push --lockref=master:master^ origin :master + test_must_fail git push --force --lockref=master:master^ origin :master + ) + git ls-remote src refs/heads/master actual + test_cmp expect actual +' + +test_expect_success 'push to delete (allowed)' ' + setup_srcdst_basic + ( + cd dst + git push --lockref=master origin :master + ) + expect + git ls-remote src
[PATCH 1/7] cache.h: move remote/connect API out of it
The definition of struct ref in cache.h, a header file so central to the system, always confused me. This structure is not about the local ref used by sha1-name API to name local objects. It is what refspecs are expanded into, after finding out what refs the other side has, to define what refs are updated after object transfer succeeds to what values. It belongs to remote.h together with struct refspec. While we are at it, also move the types and functions related to the Git transport connection to a new header file connect.h Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/fetch-pack.c | 2 ++ builtin/receive-pack.c | 1 + builtin/send-pack.c| 1 + cache.h| 62 -- connect.c | 1 + connect.h | 13 +++ fetch-pack.c | 1 + fetch-pack.h | 1 + refs.c | 8 --- remote.c | 8 +++ remote.h | 54 +++ send-pack.c| 1 + transport.c| 2 ++ transport.h| 1 + upload-pack.c | 1 + 15 files changed, 87 insertions(+), 70 deletions(-) create mode 100644 connect.h diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index aba4465..c6888c6 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -1,6 +1,8 @@ #include builtin.h #include pkt-line.h #include fetch-pack.h +#include remote.h +#include connect.h static const char fetch_pack_usage[] = git fetch-pack [--all] [--stdin] [--quiet|-q] [--keep|-k] [--thin] diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index e3eb5fc..7434d9b 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -8,6 +8,7 @@ #include commit.h #include object.h #include remote.h +#include connect.h #include transport.h #include string-list.h #include sha1-array.h diff --git a/builtin/send-pack.c b/builtin/send-pack.c index 152c4ea..e86d3b5 100644 --- a/builtin/send-pack.c +++ b/builtin/send-pack.c @@ -5,6 +5,7 @@ #include sideband.h #include run-command.h #include remote.h +#include connect.h #include send-pack.h #include quote.h #include transport.h diff --git a/cache.h b/cache.h index dd0fb33..cb2891d 100644 --- a/cache.h +++ b/cache.h @@ -1035,68 +1035,6 @@ struct pack_entry { struct packed_git *p; }; -struct ref { - struct ref *next; - unsigned char old_sha1[20]; - unsigned char new_sha1[20]; - char *symref; - unsigned int - force:1, - forced_update:1, - deletion:1, - matched:1; - - /* -* Order is important here, as we write to FETCH_HEAD -* in numeric order. And the default NOT_FOR_MERGE -* should be 0, so that xcalloc'd structures get it -* by default. -*/ - enum { - FETCH_HEAD_MERGE = -1, - FETCH_HEAD_NOT_FOR_MERGE = 0, - FETCH_HEAD_IGNORE = 1 - } fetch_head_status; - - enum { - REF_STATUS_NONE = 0, - REF_STATUS_OK, - REF_STATUS_REJECT_NONFASTFORWARD, - REF_STATUS_REJECT_ALREADY_EXISTS, - REF_STATUS_REJECT_NODELETE, - REF_STATUS_REJECT_FETCH_FIRST, - REF_STATUS_REJECT_NEEDS_FORCE, - REF_STATUS_UPTODATE, - REF_STATUS_REMOTE_REJECT, - REF_STATUS_EXPECTING_REPORT - } status; - char *remote_status; - struct ref *peer_ref; /* when renaming */ - char name[FLEX_ARRAY]; /* more */ -}; - -#define REF_NORMAL (1u 0) -#define REF_HEADS (1u 1) -#define REF_TAGS (1u 2) - -extern struct ref *find_ref_by_name(const struct ref *list, const char *name); - -#define CONNECT_VERBOSE (1u 0) -extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags); -extern int finish_connect(struct child_process *conn); -extern int git_connection_is_socket(struct child_process *conn); -struct extra_have_objects { - int nr, alloc; - unsigned char (*array)[20]; -}; -extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len, -struct ref **list, unsigned int flags, -struct extra_have_objects *); -extern int server_supports(const char *feature); -extern int parse_feature_request(const char *features, const char *feature); -extern const char *server_feature_value(const char *feature, int *len_ret); -extern const char *parse_feature_value(const char *feature_list, const char *feature, int *len_ret); - extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path); /* A hook for count-objects to report invalid files in pack directory */ diff --git a/connect.c b/connect.c index a0783d4..a80ebd3 100644 --- a/connect.c +++ b/connect.c @@ -5,6 +5,7 @@
[PATCH 3/7] push: beginning of compare-and-swap force/delete safety
This teaches the deepest part of the callchain for git push (and git send-pack) to optionally allow the old value of the ref must be this, otherwise fail this push we discussed earlier. Nobody sets the new expect_old_sha1 and expect_old_no_trackback bitfields yet, so this is still a no-op. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/send-pack.c | 5 + remote.c| 21 +++-- remote.h| 4 send-pack.c | 1 + transport-helper.c | 6 ++ transport.c | 5 + 6 files changed, 40 insertions(+), 2 deletions(-) diff --git a/builtin/send-pack.c b/builtin/send-pack.c index e86d3b5..c86c556 100644 --- a/builtin/send-pack.c +++ b/builtin/send-pack.c @@ -55,6 +55,11 @@ static void print_helper_status(struct ref *ref) msg = needs force; break; + case REF_STATUS_REJECT_STALE: + res = error; + msg = stale info; + break; + case REF_STATUS_REJECT_ALREADY_EXISTS: res = error; msg = already exists; diff --git a/remote.c b/remote.c index b1ff7a2..81bc876 100644 --- a/remote.c +++ b/remote.c @@ -1416,13 +1416,30 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, } /* +* If we know what the old value of the remote ref +* should be, reject any push, even forced ones, +* if they do not match. +* +* It also is an error if the user told us to check +* with the remote-tracking branch to find the value +* to expect, but we did not have such a tracking +* branch. +*/ + if (ref-expect_old_sha1 + (ref-expect_old_no_trackback || +hashcmp(ref-old_sha1, ref-old_sha1_expect))) { + ref-status = REF_STATUS_REJECT_STALE; + continue; + } + + /* * Decide whether an individual refspec A:B can be * pushed. The push will succeed if any of the * following are true: * -* (1) the remote reference B does not exist +* (1) the remote reference B does not exist (i.e. create) * -* (2) the remote reference B is being removed (i.e., +* (2) the remote reference B is being removed (i.e. delete; * pushing :B where no source is specified) * * (3) the destination is not under refs/tags/, and diff --git a/remote.h b/remote.h index a850059..7ad37e6 100644 --- a/remote.h +++ b/remote.h @@ -75,10 +75,13 @@ struct ref { struct ref *next; unsigned char old_sha1[20]; unsigned char new_sha1[20]; + unsigned char old_sha1_expect[20]; /* used by expect-old */ char *symref; unsigned int force:1, forced_update:1, + expect_old_sha1:1, + expect_old_no_trackback:1, deletion:1, matched:1; @@ -102,6 +105,7 @@ struct ref { REF_STATUS_REJECT_NODELETE, REF_STATUS_REJECT_FETCH_FIRST, REF_STATUS_REJECT_NEEDS_FORCE, + REF_STATUS_REJECT_STALE, REF_STATUS_UPTODATE, REF_STATUS_REMOTE_REJECT, REF_STATUS_EXPECTING_REPORT diff --git a/send-pack.c b/send-pack.c index 9a9908c..b228d65 100644 --- a/send-pack.c +++ b/send-pack.c @@ -227,6 +227,7 @@ int send_pack(struct send_pack_args *args, case REF_STATUS_REJECT_ALREADY_EXISTS: case REF_STATUS_REJECT_FETCH_FIRST: case REF_STATUS_REJECT_NEEDS_FORCE: + case REF_STATUS_REJECT_STALE: case REF_STATUS_UPTODATE: continue; default: diff --git a/transport-helper.c b/transport-helper.c index db9bd18..95d22f8 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -683,6 +683,11 @@ static int push_update_ref_status(struct strbuf *buf, free(msg); msg = NULL; } + else if (!strcmp(msg, stale info)) { + status = REF_STATUS_REJECT_STALE; + free(msg); + msg = NULL; + } } if (*ref) @@ -756,6 +761,7 @@ static int push_refs_with_push(struct transport *transport, /* Check for statuses set by set_ref_status_for_push() */ switch (ref-status) { case REF_STATUS_REJECT_NONFASTFORWARD: + case REF_STATUS_REJECT_STALE: case REF_STATUS_REJECT_ALREADY_EXISTS:
[PATCH 4/7] remote.c: add command line option parser for --lockref
Update git push and git send-pack to parse this commnd line option. The intended sematics is: * --lockref alone, without specifying the details, will protect _all_ remote refs that are going to be updated by requiring their current value to be the same as the remote-tracking branch we have for them, unless otherwise specified; * --lockref=refname, without specifying the expected value, will protect that refname, if it is going to be updated, by requiring its current value to be the same as the remote-tracking branch we have for it; * --lockref=refname:value will protect that refname, if it is going to be updated, by requiring its current value to be the same as the specified value (which is allowed to be different from the remote-tracking branch we have for the refname, or we do not even have to have such a remote-tracking branch when this form is used); * --lockref=refname: (empty value) is to expect that refname does not exist (yet); and * --no-lockref will cancel all the previous --lockref on the command line. In any of the forms, when we try to use a remote-tracking branch for the remote ref being updated, it is an error if there is no such remote-tracking branch on our end. Because the command line options are parsed _before_ we know which remote we are pushing to, there needs further processing to the parsed data after we instantiate the transport object to: * expand refname given by the user to a full refname to be matched with the list of struct ref used in match_push_refs() and set_ref_status_for_push(); and * learning the actual local ref that is the remote-tracking branch for the specified remote ref. Further, some processing need to be deferred until we find the set of remote refs and match_push_refs() returns in order to find the ones that need to be checked after explicit ones have been processed for --lockref (no specific details). These post-processing will be the topic of the next patch. Oh, of course, the option name is still not cast in stone. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/push.c | 6 ++ builtin/send-pack.c | 17 +++ remote.c| 59 + remote.h| 22 4 files changed, 104 insertions(+) diff --git a/builtin/push.c b/builtin/push.c index 342d792..31a5ba0 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -21,6 +21,8 @@ static const char *receivepack; static int verbosity; static int progress = -1; +static struct push_cas_option cas; + static const char **refspec; static int refspec_nr; static int refspec_alloc; @@ -432,6 +434,10 @@ int cmd_push(int argc, const char **argv, const char *prefix) OPT_BIT('n' , dry-run, flags, N_(dry run), TRANSPORT_PUSH_DRY_RUN), OPT_BIT( 0, porcelain, flags, N_(machine-readable output), TRANSPORT_PUSH_PORCELAIN), OPT_BIT('f', force, flags, N_(force updates), TRANSPORT_PUSH_FORCE), + { OPTION_CALLBACK, + 0, CAS_OPT_NAME, cas, N_(refname:expect), + N_(require old value of ref to be at this value), + PARSE_OPT_OPTARG, parseopt_push_cas_option }, { OPTION_CALLBACK, 0, recurse-submodules, flags, N_(check), N_(control recursive pushing of submodules), PARSE_OPT_OPTARG, option_parse_recurse_submodules }, diff --git a/builtin/send-pack.c b/builtin/send-pack.c index c86c556..061e2b2 100644 --- a/builtin/send-pack.c +++ b/builtin/send-pack.c @@ -108,6 +108,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix) int flags; unsigned int reject_reasons; int progress = -1; + struct push_cas_option cas = {0}; argv++; for (i = 1; i argc; i++, argv++) { @@ -170,6 +171,22 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix) helper_status = 1; continue; } + if (!strcmp(arg, -- CAS_OPT_NAME)) { + if (parse_push_cas_option(cas, NULL, 0) 0) + exit(1); + continue; + } + if (!strcmp(arg, --no- CAS_OPT_NAME)) { + if (parse_push_cas_option(cas, NULL, 1) 0) + exit(1); + continue; + } + if (!prefixcmp(arg, -- CAS_OPT_NAME =)) { + if (parse_push_cas_option(cas, + strchr(arg, '=') + 1, 1) 0) + exit(1); + continue; + }
[PATCH 2/7] builtin/push.c: use OPT_BOOL, not OPT_BOOLEAN
The command line parser of git push for --tags, --delete, and --thin options still used outdated OPT_BOOLEAN. Because these options do not give escalating levels when given multiple times, they should use OPT_BOOL. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/push.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/push.c b/builtin/push.c index 2d84d10..342d792 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -427,15 +427,15 @@ int cmd_push(int argc, const char **argv, const char *prefix) OPT_BIT( 0 , all, flags, N_(push all refs), TRANSPORT_PUSH_ALL), OPT_BIT( 0 , mirror, flags, N_(mirror all refs), (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE)), - OPT_BOOLEAN( 0, delete, deleterefs, N_(delete refs)), - OPT_BOOLEAN( 0 , tags, tags, N_(push tags (can't be used with --all or --mirror))), + OPT_BOOL( 0, delete, deleterefs, N_(delete refs)), + OPT_BOOL( 0 , tags, tags, N_(push tags (can't be used with --all or --mirror))), OPT_BIT('n' , dry-run, flags, N_(dry run), TRANSPORT_PUSH_DRY_RUN), OPT_BIT( 0, porcelain, flags, N_(machine-readable output), TRANSPORT_PUSH_PORCELAIN), OPT_BIT('f', force, flags, N_(force updates), TRANSPORT_PUSH_FORCE), { OPTION_CALLBACK, 0, recurse-submodules, flags, N_(check), N_(control recursive pushing of submodules), PARSE_OPT_OPTARG, option_parse_recurse_submodules }, - OPT_BOOLEAN( 0 , thin, thin, N_(use thin pack)), + OPT_BOOL( 0 , thin, thin, N_(use thin pack)), OPT_STRING( 0 , receive-pack, receivepack, receive-pack, N_(receive pack program)), OPT_STRING( 0 , exec, receivepack, receive-pack, N_(receive pack program)), OPT_BIT('u', set-upstream, flags, N_(set upstream for git pull/status), -- 1.8.3.2-875-g76c723c -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/7] push --lockref: implement logic to populate old_sha1_expect[]
This plugs the push_cas_option data collected by the command line option parser to the transport system with a new function apply_push_cas(), which is called after match_push_refs() has already been called. At this point, we know which remote we are talking to, and what remote refs we are going to update, so we can fill in the details that may have been missing from the command line, such as (1) what abbreviated refname the user gave us matches the actual refname at the remote; and (2) which remote tracking branch in our local repository to read the value of the object to expect at the remote. to populate the old_sha1_expect[] field of each of the remote ref. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/push.c | 6 ++ builtin/send-pack.c | 3 +++ remote.c| 61 + remote.h| 3 +++ transport.c | 6 ++ transport.h | 4 6 files changed, 83 insertions(+) diff --git a/builtin/push.c b/builtin/push.c index 31a5ba0..b0e3691 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -299,6 +299,12 @@ static int push_with_options(struct transport *transport, int flags) if (thin) transport_set_option(transport, TRANS_OPT_THIN, yes); + if (!is_empty_cas(cas)) { + if (!transport-smart_options) + die(underlying transport does not support --lockref option); + transport-smart_options-cas = cas; + } + if (verbosity 0) fprintf(stderr, _(Pushing to %s\n), transport-url); err = transport_push(transport, refspec_nr, refspec, flags, diff --git a/builtin/send-pack.c b/builtin/send-pack.c index 061e2b2..41dc512 100644 --- a/builtin/send-pack.c +++ b/builtin/send-pack.c @@ -247,6 +247,9 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix) if (match_push_refs(local_refs, remote_refs, nr_refspecs, refspecs, flags)) return -1; + if (!is_empty_cas(cas)) + apply_push_cas(cas, remote, remote_refs); + set_ref_status_for_push(remote_refs, args.send_mirror, args.force_update); diff --git a/remote.c b/remote.c index e9b423a..db418ff 100644 --- a/remote.c +++ b/remote.c @@ -1997,3 +1997,64 @@ int parse_push_cas_option(struct push_cas_option *cas, const char *arg, int unse return error(cannot parse expected object name '%s', colon + 1); return 0; } + +int is_empty_cas(const struct push_cas_option *cas) +{ + return !cas-use_tracking_for_rest !cas-nr; +} + +/* + * Look at remote.fetch refspec and see if we have a remote + * tracking branch for the refname there. Fill its current + * value in sha1[]. + * If we cannot do so, return negative to signal an error. + */ +static int remote_tracking(struct remote *remote, const char *refname, + unsigned char sha1[20]) +{ + char *dst; + + dst = apply_refspecs(remote-fetch, remote-fetch_refspec_nr, refname); + if (!dst) + return -1; /* no tracking ref for refname at remote */ + if (read_ref(dst, sha1)) + return -1; /* we know what the tracking ref is but we cannot read it */ + return 0; +} + +static void apply_cas(struct push_cas_option *cas, + struct remote *remote, + struct ref *ref) +{ + int i; + + /* Find an explicit --lockref=name[:value] entry */ + for (i = 0; i cas-nr; i++) { + struct push_cas *entry = cas-entry[i]; + if (!refname_match(entry-refname, ref-name, ref_rev_parse_rules)) + continue; + ref-expect_old_sha1 = 1; + if (!entry-use_tracking) + hashcpy(ref-old_sha1_expect, cas-entry[i].expect); + else if (remote_tracking(remote, ref-name, ref-old_sha1_expect)) + ref-expect_old_no_trackback = 1; + return; + } + + /* Are we using --lockref to cover all? */ + if (!cas-use_tracking_for_rest) + return; + + ref-expect_old_sha1 = 1; + if (remote_tracking(remote, ref-name, ref-old_sha1_expect)) + ref-expect_old_no_trackback = 1; +} + +void apply_push_cas(struct push_cas_option *cas, + struct remote *remote, + struct ref *remote_refs) +{ + struct ref *ref; + for (ref = remote_refs; ref; ref = ref-next) + apply_cas(cas, remote, ref); +} diff --git a/remote.h b/remote.h index 28eb6a3..baa1c68 100644 --- a/remote.h +++ b/remote.h @@ -252,4 +252,7 @@ extern int parseopt_push_cas_option(const struct option *, const char *arg, int extern int parse_push_cas_option(struct push_cas_option *, const char *arg, int unset); extern void clear_cas_option(struct push_cas_option *); +extern int is_empty_cas(const struct
[PATCH 7/7] push: document --lockref
Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-push.txt | 26 ++ 1 file changed, 26 insertions(+) diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index f7dfe48..e7c8bd6 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -11,6 +11,7 @@ SYNOPSIS [verse] 'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] [--receive-pack=git-receive-pack] [--repo=repository] [-f | --force] [--prune] [-v | --verbose] [-u | --set-upstream] + [--lockref[=refname[:[expect [--no-verify] [repository [refspec...]] DESCRIPTION @@ -146,6 +147,31 @@ already exists on the remote side. to the `master` branch). See the `refspec...` section above for details. +--lockref:: +--lockref=refname:: +--lockref=refname:expect:: + When updating refname at the remote, make sure that the + ref currently points at expect (an object name), and else + fail the push, even if `--force` is specified. If only + refname is given, the expected value is taken from the + remote-tracking branch that holds the last-observed value of + the refname. expect given as an empty string means the + refname should not exist and this push must be creating + it. If `--lockref` (without any value) is given, make sure + each ref this push is going to update points at the object + our remote-tracking branch for it points at. ++ +This is meant to make `--force` safer to use. Imagine that you have +to rebase what you have already published. You will have to +`--force` the push to replace the history you originally published +with the rebased history. If somebody else built on top of your +original history while you are rebasing, the tip of the branch at +the remote may advance with her commit, and blindly pushing with +`--force` will lose her work. By using this option to specify that +you expect the history you are updating is what you rebased and want +to replace, you can make sure other people's work will not be losed +by a forced push. in such a case. + --repo=repository:: This option is only relevant if no repository argument is passed in the invocation. In this case, 'git push' derives the -- 1.8.3.2-875-g76c723c -- To unsubscribe from this list: send the line 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/RFC] blame: accept multiple -L ranges
On 07/09/2013 09:31 PM, Junio C Hamano wrote: Thomas Rast tr...@inf.ethz.ch writes: If you define it that way, the output of git blame -L 4,6; git blame -L /A/,+20 is significantly different from git blame -L 4,6 -L /A/,+20 Not just in the presentation or any possible coalescing, but in the meaning of the ranges. Do you really want to make it that way? Absolutely. The primary reason I want to be able to specify two ranges at the same time is to follow two functions in a file that appear in separate places, and /A/ might not be unique. When I want to say I want to see from here to there, and then from here to there, and then from here to there, it would be very frustrating if and then resets what I mean by here every time and make these three evaluated independently. It would be more general to support follow the second match to /A/ *independent* of whether the first match is also followed. I think your proposal only allows the second to be followed if the first is also followed. Therefore it seems to me that your wish is to add a side-effect to one feature so that you can use it to obtain a simulacrum of a second feature, instead of building the second feature directly. Perhaps allow start and end to be a sequence of forms like /A//A/,+20 Start at the second occurrence of /A/ an continue for 20 lines /A/+20,/B/ Start 20 lines after the first match of /A/ until the subsequent match of /B/ E.g., the body of function foo would be '/^int foo//^{/+1,/^}/-1'. That should provide hours of amusement to baffled users ;-) Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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 0/3] merge -Xindex-only
Michael Haggerty mhag...@alum.mit.edu writes: On 07/09/2013 02:08 PM, Thomas Rast wrote: Michael Haggerty mhag...@alum.mit.edu writes: Since you've already implemented a way to merge into the index (even an alternative index) without touching the working copy, I'll just cross my fingers and hope for the appearance of an option that makes merge leave HEAD, MERGE_HEAD, etc. untouched. The most annoying part is probably where to put the output, since merging is more or less defined to do one of: - update HEAD and return 0 - update MERGE_HEAD and return 1 I don't understand what you mean here. [...] I was simply trying to describe what the status quo is, as a basis for the next paragraph. Does that clarify it? I'm not sure how much flexibility is worth having. Would it be sufficient if you had an option, e.g. -Xresult-ref=refs/heads/foo, that changes it to: - update refs/heads/foo and return 0 - return 1, not updating any refs That would mean that it would only work for noninteractive use. In the conflicting case, the driving script would need to remember what it wanted to merge so as have the information when finally committing. That would be fine with me. On IRC you said you would like a version that always acts as --no-commit, and simply returns the conflict/no conflict bit as usual. The caller would then proceed using commit-tree itself. I think that is probably a saner solution than this output ref idea. -- 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
Re: [PATCH 5.5/22] Add documentation for the index api
Duy Nguyen pclo...@gmail.com writes: On Tue, Jul 9, 2013 at 3:54 AM, Thomas Gummerer t.gumme...@gmail.com wrote: As promised, a draft for a documentation for the index api as it is in this series. First of all, it may be a good idea to acknowledge index_state-cache[] as part of the API for now. Not hiding it simplifies a few things (no need for new next_ce field, no worries about rewinding in unpack-trees..). Supporting partial loading (and maybe partial update in some cases) with this API and index_state-cache[] part of the API are good enough for me. We can do another tree-based API or something update later when it's formed (I looked at your index-v5api branch but I don't think a tree-based api was there, my concern is how much extra head pre-v5 has to pay to use tree-based api). Yes, I think you're right, that simplifies everything a lot, while we still can have partial loading. Hiding index_state-cache[] was mainly thought for future changes for the in-memory format, but I think that will not be happening for a while. +`read_index_filtered(opts)`:: + This method behaves differently for index-v2 and index-v5. + + For index-v2 it simply reads the whole index as read_index() + does, so we are sure we don't have to reload anything if the + user wants a different filter. It also sets the filter_opts + in the index_state, which is used to limit the results when + iterating over the index with for_each_index_entry(). + + The whole index is read to avoid the need to eventually + re-read the index later, because the performance is no + different when reading it partially. + + For index-v5 it creates an adjusted_pathspec to filter the + reading. First all the directory entries are read and then + the cache_entries in the directories that match the adjusted + pathspec are read. The filter_opts in the index_state are set + to filter out the rest of the cache_entries that are matched + by the adjusted pathspec but not by the pathspec given. The + rest of the index entries are filtered out when iterating over + the cache with for_each_index_entries. You can state in the API that the input pathspec is used as a hint to load only a portion of the index. read_index_filtered may load _more_ than necessary. It's the caller's responsibility to verify again which is matched and which is not. That's how read_directory is done. I think it gives you more liberty in loading strategy. It's already true for v2 because full index is loaded regardless of the given pathspec. In the end, we have a linear list (from public view) of cache entries, accessible via index_state-cache[]. Yes, and it's also partly true for index-v5, as the full content of a directory is loaded even if only some files it it match the pathspec that's given. If you happen to know that certain entries match the given pathspec, you could help the caller avoid match_pathspec'ing again by set a bit in ce_flags. I currently don't know which entries do match the pathspec from just reading the index file, additional calls would be needed. I don't think that would be worth the overhead. To know which entry exists in the index and which is new, use another flag. Most reader code won't change if we do it this way, all match_pathspec() remain where they are. Hrm you mean to know which cache entries are added (or changed) in the in-memory index and will have to be written later? I'm not sure I understand correctly what you mean here. +`for_each_index_entry(fn, cb_data)`:: + Iterates over all cache_entries in the index filtered by + filter_opts in the index_stat. For each cache entry fn is + executed with cb_data as callback data. From within the loop + do `return 0` to continue, or `return 1` to break the loop. Because we don't attempt to hide index_state-cache[], this one may be for convenience, the user is not required to convert to it. Actually I think this may be slower because of the cost of calling function pointer. Yes right, I think you're right. In fact I just tested it, and it's slightly slower. I still think it would make sense to keep it around, for the callers that want the cache filtered exactly by the filter_opts, for convenience as you said. +`next_index_entry(ce)`:: + Returns the cache_entry that follows after ce next_ce field and this method may be gone too, just access index_state-cache[] Yes, this makes no sense when we're not hiding index_state-cache[]. The same goes for the get_index_entry_by_name function, which is essentially the same as using index_name_pos and then getting the cache entry from index_state-cache[]. +`index_change_filter_opts(opts)`:: + This function again has a slightly different functionality for + index-v2 and index-v5. + + For index-v2 it simply changes the filter_opts, so +
Re: [PATCH 05/22] read-cache: add index reading api
Junio C Hamano gits...@pobox.com writes: Thomas Gummerer t.gumme...@gmail.com writes: The reader often needs to rewind the read-pointer partially while walking the index (e.g. next_cache_entry() in unpack-trees.c and how the o-cache_bottom position is used throughout the subsystem). I am not sure if this singly-linked list is a good way to go. I'm not very familiar with the unpack-trees code, but from a quick look the pointer (or position in the cache) is always only moved forward. I am more worried about o-cache_bottom processing, where it currently is an index into an array. With your ce-next_in_list_of_read_entries change, a natural rewrite would be to point at the ce with o-cache_bottom, but then that would mean you cannot in-place replace the entries like we used to be able to in an array based implementation. But your series does not seem to touch unpack-trees yet, so I may be worried too much before it becomes necessary. Yes, you're right, as Duy mentioned in the other email I just responded to it makes sense to keep the index around for now. I looked at the unpack-trees code a bit, and adding a new api and hiding index_state-cache[] will probably be a bit harder to do than I originally thought, so it's best to keep that around for now, as we're still able to get the benefits from partial loading even if it's not hidden. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] push: document --lockref
Am 09.07.2013 21:53, schrieb Junio C Hamano: +--lockref:: +--lockref=refname:: +--lockref=refname:expect:: ... +This is meant to make `--force` safer to use. This is a contradiction. --force means I mean it, dude, and not I mean it sometimes. It would make sense if this sentence were This is meant to make `+refspec` safer to use. Do you intend to require users to opt in to safety by saying --lockref until the end of time? Which makes it actually usable only for scripts and aliases. How do you override when the safety triggers, e.g., in an alias that uses --force --lockref? Add --i-really-mean-it? Or do we want to make --lockref the default at least for cases where necessary ingredients can be derived automatically, perhaps in Git 3.0? Then, how do you override when the safety triggers? Add --i-really-mean-it? IMO, the way forward is: 1. Teach users to use +refspec to force-push. Do not encourage 'push --force'. 2. Add --lockref as an opt-in for +refspec. Do not apply the safety to 'push --force'. (Current users and scripts do not see a behavior change because they do not use --lockref, either.) 3. Make --lockref behavior the default at least for +refspec. Then 'push --force' is still able to override the safety. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] blame: accept multiple -L ranges
Michael Haggerty mhag...@alum.mit.edu writes: It would be more general to support follow the second match to /A/ *independent* of whether the first match is also followed. I think your proposal only allows the second to be followed if the first is also followed. Therefore it seems to me that your wish is to add a side-effect to one feature so that you can use it to obtain a simulacrum of a second feature, instead of building the second feature directly. Perhaps allow start and end to be a sequence of forms like /A//A/,+20 Remember A is just a placeholder and in real life it would be more than one character. It is just as annoying as hell you have to type it again. I am not saying that a mode that resets the start searching from here pointer to the beginning of the file is useless. For example, I would not mind typing a special character, e.g. -L begin1,end1 -L !begin2,end2 that resets the search pointer to the beginning, for a rare case where I want the search for begin2 to restart at the top. But the thing is, the default matters. And it is far more common, at least to me, when I want to say from here to there, and then from here to there, to expect the second from here would be below the first one I already specified, while I am looking at the current state of a single file from top to bottom and notice two places I am interested in. /A/+20,/B/ Start 20 lines after the first match of /A/ until the subsequent match of /B/ Yes, I think -L /^{/-4,/^}/ would be a nice thing to have, but I think it is orthogonal to the issue of where the search for /^{/ begins. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] push: document --lockref
At 12:53 -0700 09 Jul 2013, Junio C Hamano gits...@pobox.com wrote: +This is meant to make `--force` safer to use. Imagine that you have +to rebase what you have already published. You will have to +`--force` the push to replace the history you originally published +with the rebased history. If somebody else built on top of your +original history while you are rebasing, the tip of the branch at +the remote may advance with her commit, and blindly pushing with +`--force` will lose her work. By using this option to specify that +you expect the history you are updating is what you rebased and want +to replace, you can make sure other people's work will not be losed +by a forced push. in such a case. s/losed/lost/ How does this behave if --force is not used? I think it would be best if it was a no-op in that case to make it easy to add a config option to turn this on by default. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] push: document --lockref
On 07/09/2013 09:53 PM, Junio C Hamano wrote: Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-push.txt | 26 ++ 1 file changed, 26 insertions(+) diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index f7dfe48..e7c8bd6 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -11,6 +11,7 @@ SYNOPSIS [verse] 'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] [--receive-pack=git-receive-pack] [--repo=repository] [-f | --force] [--prune] [-v | --verbose] [-u | --set-upstream] +[--lockref[=refname[:[expect [--no-verify] [repository [refspec...]] DESCRIPTION @@ -146,6 +147,31 @@ already exists on the remote side. to the `master` branch). See the `refspec...` section above for details. +--lockref:: +--lockref=refname:: +--lockref=refname:expect:: + When updating refname at the remote, make sure that the + ref currently points at expect (an object name), and else + fail the push, even if `--force` is specified. If only + refname is given, the expected value is taken from the + remote-tracking branch that holds the last-observed value of + the refname. expect given as an empty string means the + refname should not exist and this push must be creating + it. If `--lockref` (without any value) is given, make sure + each ref this push is going to update points at the object + our remote-tracking branch for it points at. I thought that the explanation in your patch 4/7 log message was clearer. In particular, I think that documenting the forms separately, as you did in the log message, makes it unambiguous, whereas for example the distinction in prose between If only refname is given and expect given as an empty string is easy to miss. Does --lockref only apply to references that need non-ff updates, or to all references that are being pushed? This is mostly interesting for the zero-argument form (especially if a config option is invented to make this the default), but the question should also be answered for the other forms. +This is meant to make `--force` safer to use. Imagine that you have +to rebase what you have already published. You will have to +`--force` the push to replace the history you originally published +with the rebased history. If somebody else built on top of your +original history while you are rebasing, the tip of the branch at s/are/were/ +the remote may advance with her commit, and blindly pushing with s/advance/have advanced/ +`--force` will lose her work. By using this option to specify that +you expect the history you are updating is what you rebased and want +to replace, you can make sure other people's work will not be losed s/losed/lost/ +by a forced push. in such a case. s/push./push/ or s/in such a case.// + --repo=repository:: This option is only relevant if no repository argument is passed in the invocation. In this case, 'git push' derives the Another minor point: git update-ref allows either 40 0 or the empty string to check that the ref doesn't already exist. For consistency it might be nice to accept 40 0 here as well. I still really like the idea of the feature. bikeshed The name --lockref is OK, but for me it's less a question of locking, because as far as the user is concerned the push is an atomic operation so there is no sense of a lock that is being held for a finite period of time. For me it is more a question of checking or verifying. I see that the word verify already has a meaning for this command, so maybe --checkref or --checkold or --checkoldref? /bikeshed Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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 7/7] push: document --lockref
Johannes Sixt j...@kdbg.org writes: Am 09.07.2013 21:53, schrieb Junio C Hamano: +--lockref:: +--lockref=refname:: +--lockref=refname:expect:: ... +This is meant to make `--force` safer to use. This is a contradiction. --force means I mean it, dude, and not I mean it sometimes. It would make sense if this sentence were This is meant to make `+refspec` safer to use. No, this *IS* making --force safer by letting you to say in addition to --force alone which is blind, add --lockref to defeat it. I do not see any good reason to change the samentics of +refspec for something like this. +refspec and --force refspec have meant the same thing forever. If --lockref adds safety to +refspec, the same safety should apply to --force refspec. Do you intend to require users to opt in to safety by saying --lockref until the end of time? For normal users this is *NOT* necessary. I do not know where people are getting the idea of making it default. Rewinding a branch, needing to --force, is an exceptional case. Which makes it actually usable only for scripts and aliases. How do you override when the safety triggers, e.g., in an alias that uses --force --lockref? The original request for this feature did come from script writers, who want to spin until git fetch ... magic integrate of the ongoing work ... git push --lockref do : spin done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] push: document --lockref
Aaron Schrab aa...@schrab.com writes: How does this behave if --force is not used? Both the usual must fast-forward safety and the ref should not have moved safety apply. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] push: document --lockref
Michael Haggerty mhag...@alum.mit.edu writes: bikeshed The name --lockref is OK, but for me it's less a question of locking, because as far as the user is concerned the push is an atomic operation so there is no sense of a lock that is being held for a finite period of time. Yeah, I think this is more like taking a lease. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git in nutshell Inbox
On 07/09/2013 07:38 PM, Fredrik Gustafsson wrote: git-cola is about six years old and you're using a two year old release. You miss 1/3 of all development that has been done on git-cola. I suggest you update to a newer version. Too bad that ubuntu shipped such old version. Thanks for mentioning that. I get recent version from this PPS https://launchpad.net/~winski/+archive/git-cola There is a big difference between old and new version... voila. Now I've to make some tests because the new version is completely different than the old one so I'll send a feedback soon. Thanks again :) -- Best Regards, Muhammad Bashir Al-Noimi -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git in nutshell Inbox
On 07/09/2013 08:37 PM, John Szakmeister wrote: You might want to consider giggle. I've not used it, but it's more BzrExplorer-like, IIRC. No no no, git-cola much similar to Bazaar Explorer. Giggle is so far way from Bazaar Explorer like -- Best Regards, Muhammad Bashir Al-Noimi -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] push: document --lockref
Am 09.07.2013 22:37, schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: Am 09.07.2013 21:53, schrieb Junio C Hamano: +--lockref:: +--lockref=refname:: +--lockref=refname:expect:: ... +This is meant to make `--force` safer to use. This is a contradiction. --force means I mean it, dude, and not I mean it sometimes. It would make sense if this sentence were This is meant to make `+refspec` safer to use. No, this *IS* making --force safer by letting you to say in addition to --force alone which is blind, add --lockref to defeat it. I do not see any good reason to change the samentics of +refspec for something like this. +refspec and --force refspec have meant the same thing forever. So what? They still mean the same thing as long as --lockref is not used. If --lockref adds safety to +refspec, the same safety should apply to --force refspec. No. --force means I know what I am doing, no safety needed, thank you. By applying the safety to --force as well, you lose it as the obvious tool that overrides the safety. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Feature request: author branch in commit object
From: Junio C Hamano gits...@pobox.com Sent: Wednesday, July 03, 2013 7:08 PM [catching up on old emails] Ed Hutchins e...@demeterr.com writes: I'm not trying to change the way git does things (which works perfectly well), I'm asking for some extra information to be added to the commit so that analysis of the ancestry graph can be tied to the branch topics that the original author was working from. Currently if you have a rebase-branch/ff-merge-to-master workflow, the graph of commits looks like a single user produced all of the code. It would be very useful for both forensic and display purposes to categorize those commits by their original topics, but that history is lost in such a workflow. I am not following that a single user part. As long as these topics are done by different people, the authorship remains separate, no matter what the shape of the graph is. It all depends on what you show on the graph other than a circle and connecting lines, but I presume at least you would show the subject line. The graph would clearly show which groups of commits tackle what problems in your history, even if you excessively linearlized it by rebasing. You need subjects / commit log messages that are better than bugfix, of course, for it to work, though. Arguing that branch names are local and thus meaningless misses the point: branches are *names* which were meaningful to the author at the time the branch was being worked on. That is not necessarily true. Most of my commits start their life on a single branch that is named after a very broad theme (or even on a detached HEAD) that ends up touching different parts of the system and then later split into separate topic branches that are named after more detailed single issues. The name of the branch that happened to have been used to create them have almost no meaning after I am done with multiple and independent (but related in the larger scheme of things) topics. It is not just misleading but is actively wrong to recording the name of the original branch in commits and carrying them forward via rebase. If you want a record of what a group of commits were about, the right time to do so is when you merge. While the general arguments are true that in the main one shouldn't embed whatever random branch name was used into the commit messages, there are some workflows and some production (corporate) environments where adding a relevant branch name is suitable for that environment. If the existing branch name is poor then the user should do a rebase to transfer it to a better branch name, and then the 'git filter-branch' command would be the obvious method to add a Developed-on: branch final 'signoff line'. The 'git filter-branch' man page already includes an example for adding an acked by, which can easily be modified. -- If you need to add Acked-by lines to, say, the last 10 commits (none of which is a merge), use this command: git filter-branch --msg-filter ' cat echo Acked-by: Bugs Bunny bu...@bugzilla.org ' HEAD~10..HEAD -- I'll leave it to Ed to automate it as a script (and possibly making sure it's idempotent so only the final branch name is retained, etc.) Projects that care about the shape of the ancestry graph have an obvious option of not excessively/unnecessarily linearlizing their history. We even have the --no-ff mode of merge to create an otherwise unnecessary merge to mark the point where a topic is merged to the mainline, so that merge log messages can say what topic was merged (and also you can even have merge.log). Cleaning up a messy history created on a topic branch before presenting to others by lineralizing is one thing. It is a good practice. Requiring any update to fast-forward on top of the tip of the project is quite different. It does not make your history any easier to read. A topic that has been working fine on top of last week's trunk can have a subtle interaction with the work done by others on the trunk since it forked, and rebasing it on top of today's trunk, just before pushing it out on the trunk, risks breaking the topic in a subtle way without the person who does such a rebase without noticing, making later bisection harder. Any option to encourage such an artificially linear history _is_ actively detrimental. -- Philip The boss may not always be right, but she is the boss. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] push: document --lockref
On 13-07-09 04:37 PM, Junio C Hamano wrote: Johannes Sixt j...@kdbg.org writes: Am 09.07.2013 21:53, schrieb Junio C Hamano: +--lockref:: +--lockref=refname:: +--lockref=refname:expect:: ... +This is meant to make `--force` safer to use. This is a contradiction. --force means I mean it, dude, and not I mean it sometimes. It would make sense if this sentence were This is meant to make `+refspec` safer to use. No, this *IS* making --force safer by letting you to say in addition to --force alone which is blind, add --lockref to defeat it. I do not see any good reason to change the samentics of +refspec for something like this. +refspec and --force refspec have meant the same thing forever. If --lockref adds safety to +refspec, the same safety should apply to --force refspec. Do you intend to require users to opt in to safety by saying --lockref until the end of time? For normal users this is *NOT* necessary. I do not know where people are getting the idea of making it default. Rewinding a branch, needing to --force, is an exceptional case. Yes, rewinding is exceptional. However, when a rewind has to happen, I think most users would want to have this feature most of the time. I think anyone who rewinds a shared branch would hate to inadvertently throw away someone else's work. Rare is the person who really won't care about that. So I agree with those who say that this would be nice default behaviour. I also don't think we need to make --force different from +refspec, mainly because if the rewound ref turns out to have moved a simple git fetch will update it and likely allow the next rewind attempt to succeed. A helpful error message would make this plain. I also appreciate the desire to let this stew a while before making it the default. However, I don't think that leaving it as an option of push will give it enough exposure. I myself want this feature, and I do rewind or delete a branch every few months or so, but I'm almost certainly going to forget to use this option the next time the need arises. But if it was instead/also a configurable option I could just turn on, that would be awesome. bikeshed For the option name, how about --match-baseref ? /bikeshed M. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] push: document --lockref
Johannes Sixt j...@kdbg.org writes: No. --force means I know what I am doing, no safety needed, thank you. I sympathize the desire to keep a big red button to override everything, but it is still not clear how these two independent safety should work together and should possibly seletively be overriden. A proposed ref update can be in one of the four: 1. The update fast-forwards, and the ref to be updated is at the expected place (or you simply do not care what the current value is); 2. The update does not fast-forward, and the ref to be updated is at the expected place (or you simply do not care what the current value is); 3. The update fast-forwards, but the ref to be updated is not at the expected place; or 4. The update does not fast-forward, and the ref to be updated is not at the expected place. So far we had only 1. and 2. because we did not have this old value has to be at X. And --force has been the way to allow 2. to go through. Now we are adding 3. and 4. to the mix. If --force were the big red button that allows all four, is that sufficient to cover the necessary cases, especially given that some people seem to want to make the --lockref on by default (implying that 3. and 4. will both fail by default unless forced in some way)? For example, would there be a case where we want to allow 3. but not 4. (or vice versa)? You _could_ structure the safety into hierarchies: * safest: no-ff will be rejected, and current value at an unexpected place is also rejected. That would be: $ git push --lockref * --lockref only: no-ff is not even checked, but current value must be at an expected place. How would that be spelled??? $ git push --lockref ??? * --force: anything goes. $ git push --force --no-lockref Where does ff-check only fit in the hierarchy? This is one of the reasons why the original design of --lockref was to even countermand allow non-fast-forward (which is the original meaning of --force). I _think_ I am OK if we introduced --allow-no-ff that means the current --force (i.e. rewinding is OK), that does not defeat the --lockref safety. That is the intended application (you know that push does not fast-forward because you rebased, but you also want to make sure there is nothing you are losing by enforcing --lockref safety). If that is what happens, then I think --force that means anything goes makes sense. With the posted series, adding --force --no-lockref to the command line is how to spell that big red button. -- To unsubscribe from this list: send the line 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] remote-http: use argv-array
Jeff King p...@peff.net writes: On Tue, Jul 09, 2013 at 08:05:19AM +0200, Bert Wesarg wrote: + argv_array_pushl(args, send-pack, --stateless-rpc, --helper-status); missing NULL sentinel. GCC has the 'sentinel' [1] attribute to catch such errors. Or use macro magic: void argv_array_pushl_(struct argv_array *array, ...); #define argv_array_pushl(array, ...) argv_array_pushl_(array, __VA_ARGS__, NULL) Nice catch. We cannot use variadic macros, because we support pre-C99 compilers that do not have them. But the sentinel attribute is a good idea. Here's a patch. This attribute could also be used for builtin/revert.c:verify_opt_compatible, builtin/revert.c:verify_opt_mutually_compatible, exec_cmd.h:execl_git_cmd, and run-command.h:run_hook. -- Matt -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] push: document --lockref
Junio C Hamano gits...@pobox.com writes: I _think_ I am OK if we introduced --allow-no-ff that means the current --force (i.e. rewinding is OK), that does not defeat the --lockref safety. That is the intended application (you know that push does not fast-forward because you rebased, but you also want to make sure there is nothing you are losing by enforcing --lockref safety). If that is what happens, then I think --force that means anything goes makes sense. Or perhaps you were implicitly assuming that --lockref would automatically mean I know I am rewinding, so as soon as I say --lockref, I mean --allow-no-ff, and I did not realize that. If that is the semantics you are proposing, then I think it makes sense to make --force the big red button that lets anything go. I was considering --lockref to be orthogonal to the traditional ff only check, and rejecting a push when the updated ref's current value is expected (i.e. --lockref satisfied) but the update does not fast-forward, and that was where my resistance to allow --force to override --lockref comes from (because otherwise there is no way to say I know I want to bypass 'ff-only' check, but instead make sure the current value is this). -- To unsubscribe from this list: send the line 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: Diff colorizer confused by dos newlines
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Of course! Thanks, now it completely makes sense. On 07/09/2013 03:41 PM, Jeff King wrote: On Tue, Jul 09, 2013 at 02:28:32PM -0400, Phillip Susi wrote: When I try to look at a color diff of a file using dos newlines, the output gets an odd sequence of ansi escapes and a stray carriage return showing up only on the + lines, but not the -. The normal looking - lines look like this: \r\n ( from previous line ), ansi color escape, '-', whitespace, text, terminating ansi escpae ( [m ), \r\n. The broken + lines look like this: \r\n ( from previous line ), ansi color sequence, '+', terminating ansi escape ( [m ), whitespace, ansi color sequence, text, terminating ansi escape, ansi color sequence, stray \r, terminating ansi escape, \n. That's intentional; the added lines go through the whitespace checker to help you identify potential whitespace problems (there is not much point showing them on lines going away, since you are getting rid of them). Any suggestions on how to resolve this? Try: git config core.whitespace cr-at-eol See the description of core.whitespace in git help config for more details. -Peff -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJR3J5IAAoJEJrBOlT6nu757gIIAMx+w7oCXZt66z57VhZi2yl0 PmL1HKGQ9+aqerwBtcOFvQL+b8s8o0nLUg3jkHWUFLdEtTIsLSlEwyJBhCzbWQQh NzzuwtzGdoXWxj8lFf1PakLRpWg0IW0QJ6WuMltEZMvPvF4jyt3yCArS0StpZEG/ PhQoEsgg8XjgvP6KIpZDc/CoElKn0fyWf0HlLFwV+k+cCgQM+y7POnGdzGwKIM+O U5QJxWZmNFuznJHxkd+YWIsAZuHS/eH4Tz97/Abl9z30k+/4eHvD/Hd47RCTLrC1 90IiBKp9vvPWJHA+6c1WTwiTnn54CpudKstB499aaMRsvXBzXHcdD+Tkb38imwI= =WMkZ -END PGP SIGNATURE- -- To unsubscribe from this list: send the line 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] remote-http: use argv-array
On Tue, Jul 09, 2013 at 10:27:29PM +, Matt Kraai wrote: Nice catch. We cannot use variadic macros, because we support pre-C99 compilers that do not have them. But the sentinel attribute is a good idea. Here's a patch. This attribute could also be used for builtin/revert.c:verify_opt_compatible, builtin/revert.c:verify_opt_mutually_compatible, exec_cmd.h:execl_git_cmd, and run-command.h:run_hook. Thanks. I did a full grep of '\.\.\.' on the source, and found that we have missed some cases for the format attribute, too. This series fixes all of them in the main code base (not compat/ or contrib/). But see the comments in patch 3, as I'm not sure that case is worth doing. [1/3]: add missing format function attributes [2/3]: use sentinel function attribute for variadic lists [3/3]: wt-status: use format function attribute for status_printf -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 2/3] use sentinel function attribute for variadic lists
This attribute can help gcc notice when callers forget to add a NULL sentinel to the end of the function. This is our first use of the sentinel attribute, but we shouldn't need to #ifdef for other compilers, as __attribute__ is already a no-op on non-gcc-compatible compilers. Suggested-by: Bert Wesarg bert.wes...@googlemail.com More-Spots-Found-By: Matt Kraai kr...@ftbfs.org Signed-off-by: Jeff King p...@peff.net --- argv-array.h | 1 + builtin/revert.c | 2 ++ exec_cmd.h | 1 + run-command.h| 1 + 4 files changed, 5 insertions(+) diff --git a/argv-array.h b/argv-array.h index 40248d4..e805748 100644 --- a/argv-array.h +++ b/argv-array.h @@ -15,6 +15,7 @@ void argv_array_pushf(struct argv_array *, const char *fmt, ...); void argv_array_push(struct argv_array *, const char *); __attribute__((format (printf,2,3))) void argv_array_pushf(struct argv_array *, const char *fmt, ...); +__attribute__((sentinel)) void argv_array_pushl(struct argv_array *, ...); void argv_array_pop(struct argv_array *); void argv_array_clear(struct argv_array *); diff --git a/builtin/revert.c b/builtin/revert.c index 0401fdb..b8b5174 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -54,6 +54,7 @@ static int option_parse_x(const struct option *opt, return 0; } +__attribute__((sentinel)) static void verify_opt_compatible(const char *me, const char *base_opt, ...) { const char *this_opt; @@ -70,6 +71,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...) die(_(%s: %s cannot be used with %s), me, this_opt, base_opt); } +__attribute__((sentinel)) static void verify_opt_mutually_compatible(const char *me, ...) { const char *opt1, *opt2 = NULL; diff --git a/exec_cmd.h b/exec_cmd.h index e2b546b..307b55c 100644 --- a/exec_cmd.h +++ b/exec_cmd.h @@ -7,6 +7,7 @@ extern int execv_git_cmd(const char **argv); /* NULL terminated */ extern void setup_path(void); extern const char **prepare_git_cmd(const char **argv); extern int execv_git_cmd(const char **argv); /* NULL terminated */ +__attribute__((sentinel)) extern int execl_git_cmd(const char *cmd, ...); extern const char *system_path(const char *path); diff --git a/run-command.h b/run-command.h index 221ce33..0a47679 100644 --- a/run-command.h +++ b/run-command.h @@ -46,6 +46,7 @@ extern char *find_hook(const char *name); int run_command(struct child_process *); extern char *find_hook(const char *name); +__attribute__((sentinel)) extern int run_hook(const char *index_file, const char *name, ...); #define RUN_COMMAND_NO_STDIN 1 -- 1.8.3.rc3.24.gec82cb9 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2013, #03; Tue, 9)
On Tue, Jul 09, 2013 at 04:09:35PM -0700, Junio C Hamano wrote: * jk/argv-pushf-sentinel (2013-07-09) 1 commit - argv-array: add sentinel attribute to argv_array_pushl Will merge to 'next'. If you have not pushed it out already, I just posted a re-roll which covers many other places, too. -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
Error
I get this error message when trying to create a shared ssh key in Mac osx Lion scp .ssh/id_rsa.pub cvatt...@cook.cs.uno.edu:~/ ssh_exchange_identification: Connection closed by remote host lost connection -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html