Re: msysgit works on wine
On 10/10/2014 02:04 PM, Duy Nguyen wrote: On Fri, Oct 10, 2014 at 7:02 PM, Thomas Braun thomas.br...@virtuell-zuhause.de wrote: Are you compiling git.git or msysgit.git? git.git And how about the test suite? running right now, fingers crossed.. kinda slow, not sure if it's wine or it's the msys thing. We (Wine) are interested in bug reports for git tests failing on Wine that succeed on Windows/Linux. Performance issues compared to Windows are highly desired too. thanks bye michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: $GIT_DIR/info/exclude fails to negate a pattern defined by core.excludesfile
Hey Duy, I'm not sure why the pattern would have to be as you describe - I'm just looking to ignore `*.out` as a general configuration, and disable it for one specific project, so it would seem a plain `!*.out` should work. In any case, I added a `.gitignore` file with the single pattern `!*.out` at the root of the project, and now .out files are no longer ignored for the project. It's not an ideal solution because now all the other developers of the project who do not have `*.out` in their `core.excludesfile` will have an unnecessary pattern in their exclusion logic, but it does work as expected. Thanks, D. On Sun, Oct 12, 2014 at 7:53 PM, Duy Nguyen pclo...@gmail.com wrote: On Sun, Oct 12, 2014 at 9:58 AM, Dun Peal dunpea...@gmail.com wrote: I have the pattern `*.out` defined in my `core.excludesfile`. According to the documentation[1], patterns defined in `$GIT_DIR/info/exclude` take precedence over `core.excludesfile`, so for one particular project that needs to track some `.out` files, I created `$GIT_DIR/info/exclude` with just one pattern: `!*.out`. Yet for some reason, `git status` still fails to report newly created `.out` files for that project. Am I misunderstanding the documentation? We process in groups, so rules in core.excludesfile are in one group, those in $GIT_DIR/info/exclude in another group. Negative patterns only has effects within their group, so !*out in .../exclude can't revert *.out in core.excludesfile. Probably implementation limitation, not by design.. But even if we flatten them into one group, i'm not sure you can achieve that. The patterns would be !*.out *.out !*.out has nothing to revert because it's before *.out. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Initialise hash variable to prevent compiler warnings
The 'hash' variable in test-hashmap.c is not initialised properly which causes some 'gcc' versions to complain during compilation. Signed-off-by: Felipe Franciosi fel...@paradoxo.org --- test-hashmap.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-hashmap.c b/test-hashmap.c index 07aa7ec..cc2891d 100644 --- a/test-hashmap.c +++ b/test-hashmap.c @@ -47,7 +47,7 @@ static struct test_entry *alloc_test_entry(int hash, char *key, int klen, static unsigned int hash(unsigned int method, unsigned int i, const char *key) { - unsigned int hash; + unsigned int hash = 0; switch (method 3) { case HASH_METHOD_FNV: -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Handle atexit list internaly for unthreaded builds
On Mon, Oct 13, 2014 at 2:56 AM, Duy Nguyen pclo...@gmail.com wrote: On Sun, Oct 12, 2014 at 4:09 PM, Etienne Buira etienne.bu...@gmail.com wrote: Replace atexit()s calls with cmd_atexit that is atexit() on threaded builds, but handles the callbacks list internally for unthreaded builds. Maybe hide this in git-compat-util.h and #define atexit(x) cmd_atexit(x)? Updated. cmd_ is usually for commands' main functions. Maybe rename it to git_atexit(). Indeed, renamed. Thank you. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] Handle atexit list internaly for unthreaded builds
Wrap atexit()s calls on unthreaded builds to handle callback list internally. This is needed because on unthreaded builds, asyncs inherits parent's atexit() list, that gets run as soon as the async exit()s (and again at the end of the parent process). That led to remove temporary and lock files too early. Fixes test 5537 (temporary shallow file vanished before unpack-objects could open it) V2: remove an obvious mistake V3: wrap, rename and add define in git-compat-util.h Thanks-to: Duy Nguyen pclo...@gmail.com Signed-off-by: Etienne Buira etienne.bu...@gmail.com --- builtin/clone.c | 5 - git-compat-util.h | 5 + run-command.c | 42 ++ shallow.c | 7 ++- 4 files changed, 49 insertions(+), 10 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index bbd169c..e122f33 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo) static const char *junk_work_tree; static const char *junk_git_dir; -static pid_t junk_pid; static enum { JUNK_LEAVE_NONE, JUNK_LEAVE_REPO, @@ -417,8 +416,6 @@ static void remove_junk(void) break; } - if (getpid() != junk_pid) - return; if (junk_git_dir) { strbuf_addstr(sb, junk_git_dir); remove_dir_recursively(sb, 0); @@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix) struct refspec *refspec; const char *fetch_pattern; - junk_pid = getpid(); - packet_trace_identity(clone); argc = parse_options(argc, argv, prefix, builtin_clone_options, builtin_clone_usage, 0); diff --git a/git-compat-util.h b/git-compat-util.h index f587749..6dd63dd 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -577,6 +577,11 @@ int inet_pton(int af, const char *src, void *dst); const char *inet_ntop(int af, const void *src, char *dst, size_t size); #endif +#ifdef NO_PTHREADS +#define atexit git_atexit +extern int git_atexit(void (*handler)(void)); +#endif + extern void release_pack_memory(size_t); typedef void (*try_to_free_t)(size_t); diff --git a/run-command.c b/run-command.c index 35a3ebf..f8dc969 100644 --- a/run-command.c +++ b/run-command.c @@ -624,6 +624,47 @@ static int async_die_is_recursing(void) return ret != NULL; } +#else + +static struct { + void (**handlers)(void); + size_t nr; + size_t alloc; +} git_atexit_hdlrs; + +static int git_atexit_installed; + +static void git_atexit_dispatch() +{ + size_t i; + + for (i=git_atexit_hdlrs.nr ; i ; i--) + git_atexit_hdlrs.handlers[i-1](); +} + +static void git_atexit_clear() +{ + free(git_atexit_hdlrs.handlers); + memset(git_atexit_hdlrs, 0, sizeof(git_atexit_hdlrs)); + git_atexit_installed = 0; +} + +#define tmp_atexit atexit +#undef atexit +int git_atexit(void (*handler)(void)) +{ + ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc); + git_atexit_hdlrs.handlers[git_atexit_hdlrs.nr++] = handler; + if (!git_atexit_installed) { + if (atexit(git_atexit_dispatch)) + return -1; + git_atexit_installed = 1; + } + return 0; +} +#define atexit tmp_atexit +#undef tmp_atexit + #endif int start_async(struct async *async) @@ -682,6 +723,7 @@ int start_async(struct async *async) close(fdin[1]); if (need_out) close(fdout[0]); + git_atexit_clear(); exit(!!async-proc(proc_in, proc_out, async-data)); } diff --git a/shallow.c b/shallow.c index de07709..f067811 100644 --- a/shallow.c +++ b/shallow.c @@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo) const char *setup_temporary_shallow(const struct sha1_array *extra) { - static int installed_handler; struct strbuf sb = STRBUF_INIT; int fd; @@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra) strbuf_addstr(temporary_shallow, git_path(shallow_XX)); fd = xmkstemp(temporary_shallow.buf); - if (!installed_handler) { - atexit(remove_temporary_shallow); - sigchain_push_common(remove_temporary_shallow_on_signal); - } + atexit(remove_temporary_shallow); + sigchain_push_common(remove_temporary_shallow_on_signal); if (write_in_full(fd, sb.buf, sb.len) != sb.len) die_errno(failed to write to %s, -- 2.0.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] t5304: use helper to report failure of test foo = bar
Jeff King wrote: For small outputs, we sometimes use: test $(some_cmd) = something we expect instead of a full test_cmp. The downside of this is that when it fails, there is no output at all from the script. There's another downside to that construct: it loses the exit status from some_cmd. [...] --- a/t/t5304-prune.sh +++ b/t/t5304-prune.sh @@ -13,7 +13,7 @@ add_blob() { before=$(git count-objects | sed s/ .*//) BLOB=$(echo aleph_0 | git hash-object -w --stdin) BLOB_FILE=.git/objects/$(echo $BLOB | sed s/^../\//) - test $((1 + $before)) = $(git count-objects | sed s/ .*//) + verbose test $((1 + $before)) = $(git count-objects | sed s/ .*//) So ideally this would be something like: git count-objects output verbose test $((1 + $before)) = $(sed s/ .*// output) [...] @@ -45,11 +45,11 @@ test_expect_success 'prune --expire' ' add_blob git prune --expire=1.hour.ago - test $((1 + $before)) = $(git count-objects | sed s/ .*//) + verbose test $((1 + $before)) = $(git count-objects | sed s/ .*//) and likewise elsewhere in the file. Alternatively, maybe there could be a helper in the same spirit as test_cmp_rev? test_object_count () { git count-objects output sed s/ .*// output count printf %s\n $1 expect test_cmp expect count } My two cents, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] Handle atexit list internaly for unthreaded builds
Etienne Buira etienne.bu...@gmail.com writes: +#define tmp_atexit atexit +#define atexit tmp_atexit +#undef tmp_atexit What is this supposed to do? Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- To unsubscribe from this list: send the line 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.c - Make remote definition require a url
Mark Levedahl mleved...@gmail.com writes: Some options may be configured globally for a remote (e.g, tagopt). Or some remotes may have only pushurl and not url. git remote output for me has a few such remotes but wouldn't this patch break it? If a caller that walks the list of remotes misbehaves only because it assumes that r-url always is always valid, isn't that assumption what needs to be fixed? for_each_remote() should be kept as a way to enumerate all the [remote foo], I would think. -- To unsubscribe from this list: send the line 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: Performance Issues with Git Rebase
Crabtree, Andrew andrew.crabt...@hp.com writes: I'm getting the same output with both the triple and double dot for my specific case, but I have no idea if that change makes sense for all cases or not. Any guidance? The difference only matters if any of your 4 patches have been sent to your upstream and accepted and appear among the 4665 changes they have. The --cherry-pick option is about cross checking the combinations of 4 x 4665 and filter out matching ones (if any). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Smart HTTP
Hi, I guess this comment is aimed at Scott Chacon. I have read your blog post on Smart HTTP (http://git-scm.com/blog/2010/03/04/smart-http.html) and wondered if there is any documentation that compares in terms of thoroughness with your sections in the book on using SSH, which does explain the basics so that anyone can get it working. I have tried setting up authenticated pull and push with HTTP (not HTTPS) and Apache never asks for authentication during a pull and refuses a push with a 403 error. Regards, John Norris -- To unsubscribe from this list: send the line 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: Performance Issues with Git Rebase
Ah gotcha. That makes sense. Default behavior is to do a patch-id check on all of them which is exactly what you would normally want to happen, and suppressing that speeds things up considerably at the risk of attempting to re-apply an already existing patch. Thanks much for the explanation. Perhaps I'll add a progress indicator since my organization will be doing a significant number of these types of rebases in the near future. Regards, -Andrew -Original Message- From: Junio C Hamano [mailto:gits...@pobox.com] Sent: Monday, October 13, 2014 10:26 AM To: Crabtree, Andrew Cc: git@vger.kernel.org Subject: Re: Performance Issues with Git Rebase Crabtree, Andrew andrew.crabt...@hp.com writes: I'm getting the same output with both the triple and double dot for my specific case, but I have no idea if that change makes sense for all cases or not. Any guidance? The difference only matters if any of your 4 patches have been sent to your upstream and accepted and appear among the 4665 changes they have. The --cherry-pick option is about cross checking the combinations of 4 x 4665 and filter out matching ones (if any). -- To unsubscribe from this list: send the line 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 v16 03/11] trailer: read and process config information
Read the configuration to get trailer information, and then process it and store it in a doubly linked list. The config information is stored in the list whose first item is pointed to by: static struct trailer_item *first_conf_item; Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 185 ++ 1 file changed, 185 insertions(+) diff --git a/trailer.c b/trailer.c index be0ad65..668dc33 100644 --- a/trailer.c +++ b/trailer.c @@ -277,3 +277,188 @@ static void process_trailers_lists(struct trailer_item **in_tok_first, arg_tok); } } + +static int set_where(struct conf_info *item, const char *value) +{ + if (!strcasecmp(after, value)) + item-where = WHERE_AFTER; + else if (!strcasecmp(before, value)) + item-where = WHERE_BEFORE; + else if (!strcasecmp(end, value)) + item-where = WHERE_END; + else if (!strcasecmp(start, value)) + item-where = WHERE_START; + else + return -1; + return 0; +} + +static int set_if_exists(struct conf_info *item, const char *value) +{ + if (!strcasecmp(addIfDifferent, value)) + item-if_exists = EXISTS_ADD_IF_DIFFERENT; + else if (!strcasecmp(addIfDifferentNeighbor, value)) + item-if_exists = EXISTS_ADD_IF_DIFFERENT_NEIGHBOR; + else if (!strcasecmp(add, value)) + item-if_exists = EXISTS_ADD; + else if (!strcasecmp(replace, value)) + item-if_exists = EXISTS_REPLACE; + else if (!strcasecmp(doNothing, value)) + item-if_exists = EXISTS_DO_NOTHING; + else + return -1; + return 0; +} + +static int set_if_missing(struct conf_info *item, const char *value) +{ + if (!strcasecmp(doNothing, value)) + item-if_missing = MISSING_DO_NOTHING; + else if (!strcasecmp(add, value)) + item-if_missing = MISSING_ADD; + else + return -1; + return 0; +} + +static void duplicate_conf(struct conf_info *dst, struct conf_info *src) +{ + *dst = *src; + if (src-name) + dst-name = xstrdup(src-name); + if (src-key) + dst-key = xstrdup(src-key); + if (src-command) + dst-command = xstrdup(src-command); +} + +static struct trailer_item *get_conf_item(const char *name) +{ + struct trailer_item *item; + struct trailer_item *previous; + + /* Look up item with same name */ + for (previous = NULL, item = first_conf_item; +item; +previous = item, item = item-next) { + if (!strcasecmp(item-conf.name, name)) + return item; + } + + /* Item does not already exists, create it */ + item = xcalloc(sizeof(struct trailer_item), 1); + duplicate_conf(item-conf, default_conf_info); + item-conf.name = xstrdup(name); + + if (!previous) + first_conf_item = item; + else { + previous-next = item; + item-previous = previous; + } + + return item; +} + +enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_WHERE, +TRAILER_IF_EXISTS, TRAILER_IF_MISSING }; + +static struct { + const char *name; + enum trailer_info_type type; +} trailer_config_items[] = { + { key, TRAILER_KEY }, + { command, TRAILER_COMMAND }, + { where, TRAILER_WHERE }, + { ifexists, TRAILER_IF_EXISTS }, + { ifmissing, TRAILER_IF_MISSING } +}; + +static int git_trailer_default_config(const char *conf_key, const char *value, void *cb) +{ + const char *trailer_item, *variable_name; + + if (!skip_prefix(conf_key, trailer., trailer_item)) + return 0; + + variable_name = strrchr(trailer_item, '.'); + if (!variable_name) { + if (!strcmp(trailer_item, where)) { + if (set_where(default_conf_info, value) 0) + warning(_(unknown value '%s' for key '%s'), + value, conf_key); + } else if (!strcmp(trailer_item, ifexists)) { + if (set_if_exists(default_conf_info, value) 0) + warning(_(unknown value '%s' for key '%s'), + value, conf_key); + } else if (!strcmp(trailer_item, ifmissing)) { + if (set_if_missing(default_conf_info, value) 0) + warning(_(unknown value '%s' for key '%s'), + value, conf_key); + } else if (!strcmp(trailer_item, separators)) { + separators = xstrdup(value); + } + } + return 0;
[PATCH v16 02/11] trailer: process trailers from input message and arguments
Implement the logic to process trailers from the input message and from arguments. At the beginning trailers from the input message are in their own in_tok doubly linked list, and trailers from arguments are in their own arg_tok doubly linked list. The lists are traversed and when an arg_tok should be applied, it is removed from its list and inserted into the in_tok list. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 210 ++ 1 file changed, 210 insertions(+) diff --git a/trailer.c b/trailer.c index ac323b1..be0ad65 100644 --- a/trailer.c +++ b/trailer.c @@ -67,3 +67,213 @@ static int same_trailer(struct trailer_item *a, struct trailer_item *b) { return same_token(a, b) same_value(a, b); } + +static void free_trailer_item(struct trailer_item *item) +{ + free(item-conf.name); + free(item-conf.key); + free(item-conf.command); + free((char *)item-token); + free((char *)item-value); + free(item); +} + +static void update_last(struct trailer_item **last) +{ + if (*last) + while ((*last)-next != NULL) + *last = (*last)-next; +} + +static void update_first(struct trailer_item **first) +{ + if (*first) + while ((*first)-previous != NULL) + *first = (*first)-previous; +} + +static void add_arg_to_input_list(struct trailer_item *on_tok, + struct trailer_item *arg_tok, + struct trailer_item **first, + struct trailer_item **last) +{ + if (after_or_end(arg_tok-conf.where)) { + arg_tok-next = on_tok-next; + on_tok-next = arg_tok; + arg_tok-previous = on_tok; + if (arg_tok-next) + arg_tok-next-previous = arg_tok; + update_last(last); + } else { + arg_tok-previous = on_tok-previous; + on_tok-previous = arg_tok; + arg_tok-next = on_tok; + if (arg_tok-previous) + arg_tok-previous-next = arg_tok; + update_first(first); + } +} + +static int check_if_different(struct trailer_item *in_tok, + struct trailer_item *arg_tok, + int check_all) +{ + enum action_where where = arg_tok-conf.where; + do { + if (!in_tok) + return 1; + if (same_trailer(in_tok, arg_tok)) + return 0; + /* +* if we want to add a trailer after another one, +* we have to check those before this one +*/ + in_tok = after_or_end(where) ? in_tok-previous : in_tok-next; + } while (check_all); + return 1; +} + +static void remove_from_list(struct trailer_item *item, +struct trailer_item **first, +struct trailer_item **last) +{ + struct trailer_item *next = item-next; + struct trailer_item *previous = item-previous; + + if (next) { + item-next-previous = previous; + item-next = NULL; + } else if (last) + *last = previous; + + if (previous) { + item-previous-next = next; + item-previous = NULL; + } else if (first) + *first = next; +} + +static struct trailer_item *remove_first(struct trailer_item **first) +{ + struct trailer_item *item = *first; + *first = item-next; + if (item-next) { + item-next-previous = NULL; + item-next = NULL; + } + return item; +} + +static void apply_arg_if_exists(struct trailer_item *in_tok, + struct trailer_item *arg_tok, + struct trailer_item *on_tok, + struct trailer_item **in_tok_first, + struct trailer_item **in_tok_last) +{ + switch (arg_tok-conf.if_exists) { + case EXISTS_DO_NOTHING: + free_trailer_item(arg_tok); + break; + case EXISTS_REPLACE: + add_arg_to_input_list(on_tok, arg_tok, + in_tok_first, in_tok_last); + remove_from_list(in_tok, in_tok_first, in_tok_last); + free_trailer_item(in_tok); + break; + case EXISTS_ADD: + add_arg_to_input_list(on_tok, arg_tok, + in_tok_first, in_tok_last); + break; + case EXISTS_ADD_IF_DIFFERENT: + if (check_if_different(in_tok, arg_tok, 1)) + add_arg_to_input_list(on_tok, arg_tok, +
[PATCH v16 04/11] trailer: process command line trailer arguments
Parse the trailer command line arguments and put the result into an arg_tok doubly linked list. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 112 ++ 1 file changed, 112 insertions(+) diff --git a/trailer.c b/trailer.c index 668dc33..b5666b3 100644 --- a/trailer.c +++ b/trailer.c @@ -1,4 +1,5 @@ #include cache.h +#include string-list.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org */ @@ -462,3 +463,114 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb) } return 0; } + +static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *trailer) +{ + size_t len; + struct strbuf seps = STRBUF_INIT; + strbuf_addstr(seps, separators); + strbuf_addch(seps, '='); + len = strcspn(trailer, seps.buf); + strbuf_release(seps); + if (len == 0) + return error(_(empty trailer token in trailer '%s'), trailer); + if (len strlen(trailer)) { + strbuf_add(tok, trailer, len); + strbuf_trim(tok); + strbuf_addstr(val, trailer + len + 1); + strbuf_trim(val); + } else { + strbuf_addstr(tok, trailer); + strbuf_trim(tok); + } + return 0; +} + +static const char *token_from_item(struct trailer_item *item, char *tok) +{ + if (item-conf.key) + return item-conf.key; + if (tok) + return tok; + return item-conf.name; +} + +static struct trailer_item *new_trailer_item(struct trailer_item *conf_item, +char *tok, char *val) +{ + struct trailer_item *new = xcalloc(sizeof(*new), 1); + new-value = val; + + if (conf_item) { + duplicate_conf(new-conf, conf_item-conf); + new-token = xstrdup(token_from_item(conf_item, tok)); + free(tok); + } else { + duplicate_conf(new-conf, default_conf_info); + new-token = tok; + } + + return new; +} + +static int token_matches_item(const char *tok, struct trailer_item *item, int tok_len) +{ + if (!strncasecmp(tok, item-conf.name, tok_len)) + return 1; + return item-conf.key ? !strncasecmp(tok, item-conf.key, tok_len) : 0; +} + +static struct trailer_item *create_trailer_item(const char *string) +{ + struct strbuf tok = STRBUF_INIT; + struct strbuf val = STRBUF_INIT; + struct trailer_item *item; + int tok_len; + + if (parse_trailer(tok, val, string)) + return NULL; + + tok_len = token_len_without_separator(tok.buf, tok.len); + + /* Lookup if the token matches something in the config */ + for (item = first_conf_item; item; item = item-next) { + if (token_matches_item(tok.buf, item, tok_len)) + return new_trailer_item(item, + strbuf_detach(tok, NULL), + strbuf_detach(val, NULL)); + } + + return new_trailer_item(NULL, + strbuf_detach(tok, NULL), + strbuf_detach(val, NULL)); +} + +static void add_trailer_item(struct trailer_item **first, +struct trailer_item **last, +struct trailer_item *new) +{ + if (!new) + return; + if (!*last) { + *first = new; + *last = new; + } else { + (*last)-next = new; + new-previous = *last; + *last = new; + } +} + +static struct trailer_item *process_command_line_args(struct string_list *trailers) +{ + struct trailer_item *arg_tok_first = NULL; + struct trailer_item *arg_tok_last = NULL; + struct string_list_item *tr; + + for_each_string_list_item(tr, trailers) { + struct trailer_item *new = create_trailer_item(tr-string); + add_trailer_item(arg_tok_first, arg_tok_last, new); + } + + return arg_tok_first; +} -- 2.1.0.rc0.248.gb91fdbc -- To unsubscribe from this list: send the line 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 v16 01/11] trailer: add data structures and basic functions
We will use a doubly linked list to store all information about trailers and their configuration. This way we can easily remove or add trailers to or from trailer lists while traversing the lists in either direction. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- Makefile | 1 + trailer.c | 69 +++ 2 files changed, 70 insertions(+) create mode 100644 trailer.c diff --git a/Makefile b/Makefile index 63a210d..ef82972 100644 --- a/Makefile +++ b/Makefile @@ -888,6 +888,7 @@ LIB_OBJS += submodule-config.o LIB_OBJS += symlinks.o LIB_OBJS += tag.o LIB_OBJS += trace.o +LIB_OBJS += trailer.o LIB_OBJS += transport.o LIB_OBJS += transport-helper.o LIB_OBJS += tree-diff.o diff --git a/trailer.c b/trailer.c new file mode 100644 index 000..ac323b1 --- /dev/null +++ b/trailer.c @@ -0,0 +1,69 @@ +#include cache.h +/* + * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org + */ + +enum action_where { WHERE_END, WHERE_AFTER, WHERE_BEFORE, WHERE_START }; +enum action_if_exists { EXISTS_ADD_IF_DIFFERENT_NEIGHBOR, EXISTS_ADD_IF_DIFFERENT, + EXISTS_ADD, EXISTS_REPLACE, EXISTS_DO_NOTHING }; +enum action_if_missing { MISSING_ADD, MISSING_DO_NOTHING }; + +struct conf_info { + char *name; + char *key; + char *command; + enum action_where where; + enum action_if_exists if_exists; + enum action_if_missing if_missing; +}; + +static struct conf_info default_conf_info; + +struct trailer_item { + struct trailer_item *previous; + struct trailer_item *next; + const char *token; + const char *value; + struct conf_info conf; +}; + +static struct trailer_item *first_conf_item; + +static char *separators = :; + +static int after_or_end(enum action_where where) +{ + return (where == WHERE_AFTER) || (where == WHERE_END); +} + +/* + * Return the length of the string not including any final + * punctuation. E.g., the input Signed-off-by: would return + * 13, stripping the trailing punctuation but retaining + * internal punctuation. + */ +static size_t token_len_without_separator(const char *token, size_t len) +{ + while (len 0 !isalnum(token[len - 1])) + len--; + return len; +} + +static int same_token(struct trailer_item *a, struct trailer_item *b) +{ + size_t a_len = token_len_without_separator(a-token, strlen(a-token)); + size_t b_len = token_len_without_separator(b-token, strlen(b-token)); + size_t min_len = (a_len b_len) ? b_len : a_len; + + return !strncasecmp(a-token, b-token, min_len); +} + +static int same_value(struct trailer_item *a, struct trailer_item *b) +{ + return !strcasecmp(a-value, b-value); +} + +static int same_trailer(struct trailer_item *a, struct trailer_item *b) +{ + return same_token(a, b) same_value(a, b); +} -- 2.1.0.rc0.248.gb91fdbc -- To unsubscribe from this list: send the line 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 v16 10/11] trailer: add tests for commands in config file
And add a few other tests for some special cases. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t7513-interpret-trailers.sh | 125 ++ 1 file changed, 125 insertions(+) diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index ad36cf8..fee41e8 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -735,4 +735,129 @@ test_expect_success 'default where is now after' ' test_cmp expected actual ' +test_expect_success 'with simple command' ' + git config trailer.sign.key Signed-off-by: + git config trailer.sign.where after + git config trailer.sign.ifExists addIfDifferentNeighbor + git config trailer.sign.command echo \A U Thor aut...@example.com\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=22 \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with command using commiter information' ' + git config trailer.sign.ifExists addIfDifferent + git config trailer.sign.command echo \\$GIT_COMMITTER_NAME \$GIT_COMMITTER_EMAIL\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: C O Mitter commit...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=22 \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with command using author information' ' + git config trailer.sign.key Signed-off-by: + git config trailer.sign.where after + git config trailer.sign.ifExists addIfDifferentNeighbor + git config trailer.sign.command echo \\$GIT_AUTHOR_NAME \$GIT_AUTHOR_EMAIL\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=22 \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'setup a commit' ' + echo Content of the first commit. a.txt + git add a.txt + git commit -m Add file a.txt +' + +test_expect_success 'with command using $ARG' ' + git config trailer.fix.ifExists replace + git config trailer.fix.command git log -1 --oneline --format=\%h (%s)\ --abbrev-commit --abbrev=14 \$ARG + FIXED=$(git log -1 --oneline --format=%h (%s) --abbrev-commit --abbrev=14 HEAD) + cat complex_message_body expected + sed -e s/ Z\$/ / expected -EOF + Fixes: $FIXED + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=HEAD \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with failing command using $ARG' ' + git config trailer.fix.ifExists replace + git config trailer.fix.command false \$ARG + cat complex_message_body expected + sed -e s/ Z\$/ / expected -EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=HEAD \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with empty tokens' ' + git config --unset trailer.fix.command + cat expected -EOF + + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer : --trailer :test actual -EOF + EOF + test_cmp expected actual +' + +test_expect_success 'with command but no key' ' + git config --unset trailer.sign.key + cat expected -EOF + + sign: A U Thor aut...@example.com + EOF + git interpret-trailers actual -EOF + EOF + test_cmp expected actual +' + +test_expect_success 'with no command and no key' ' + git config --unset trailer.review.key + cat expected -EOF + + review: Junio + sign: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review:Junio actual -EOF + EOF + test_cmp expected
[PATCH v16 00/11] Add interpret-trailers builtin
[Sorry to resend this v16, but the series didn't make it to list the first time...] This patch series implements a new command: git interpret-trailers and an infrastructure to process trailers that can be reused, for example in commit.c. 1) Rationale This command should help with RFC 822 style headers, called trailers, that are found at the end of commit messages. (Note that these headers do not follow and are not intended to follow many rules that are in RFC 822. For example they do not follow the line breaking rules, the encoding rules and probably many other rules.) For a long time, these trailers have become a de facto standard way to add helpful information into commit messages. Until now git commit has only supported the well known Signed-off-by: trailer, that is used by many projects like the Linux kernel and Git. It is better to keep builtin/commit.c uncontaminated by any more hard-wired logic, like what we have for the signed-off-by line. Any new things can and should be doable in hooks, and this filter would help writing these hooks. And that is why the design goal of the filter is to make it at least as powerful as the built-in logic we have for signed-off-by lines; that would allow us to later eject the hard-wired logic for signed-off-by line from the main codepath, if/when we wanted to. Alternatively, we could build a library-ish API around this filter code and replace the hard-wired logic for signed-off-by line with a call into that API, if/when we wanted to, but that requires (in addition to the at least as powerful as the built-in logic) that the implementation of this stand-alone filter can be cleanly made into a reusable library, so that is a bit higher bar to cross than everything can be doable with hooks alternative. 2) Current state Currently the usage string of this command is: git interpret-trailers [--trim-empty] [(--trailer token[(=|:)value])...] [file...] The following features are implemented: - the result is printed on stdout - the --trailer arguments are interpreted - messages read from file... or stdin are interpreted - the trailer.separators option in the config is interpreted - the trailer.where option is interpreted - the trailer.ifexists option is interpreted - the trailer.ifmissing option is interpreted - the trailer.token.key options are interpreted - the trailer.token.where options are interpreted - the trailer.token.ifexist options are interpreted - the trailer.token.ifmissing options are interpreted - the trailer.token.command config works - $ARG can be used in commands - messages can contain a patch - lines in messages starting with a comment char are ignored - there are 50 tests - there is some documentation - there are examples in the documentation 3) Changes since version 15, thanks to Michael S. T. and Junio * avoid trailing whitespaces in the documentation by using sed (patch 11/11) * fix a bug when a config option is passed on the command line (patch 4/11 and 8/11) Christian Couder (11): trailer: add data structures and basic functions trailer: process trailers from input message and arguments trailer: read and process config information trailer: process command line trailer arguments trailer: parse trailers from file or stdin trailer: put all the processing together and print trailer: add interpret-trailers command trailer: add tests for git interpret-trailers trailer: execute command from 'trailer.name.command' trailer: add tests for commands in config file Documentation: add documentation for 'git interpret-trailers' .gitignore | 1 + Documentation/git-interpret-trailers.txt | 314 +++ Makefile | 2 + builtin.h| 1 + builtin/interpret-trailers.c | 44 ++ command-list.txt | 1 + git.c| 1 + t/t7513-interpret-trailers.sh| 863 +++ trailer.c| 851 ++ trailer.h| 6 + 10 files changed, 2084 insertions(+) create mode 100644 Documentation/git-interpret-trailers.txt create mode 100644 builtin/interpret-trailers.c create mode 100755 t/t7513-interpret-trailers.sh create mode 100644 trailer.c create mode 100644 trailer.h -- 2.1.0.rc0.248.gb91fdbc -- To unsubscribe from this list: send the line 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 v16 06/11] trailer: put all the processing together and print
This patch adds the process_trailers() function that calls all the previously added processing functions and then prints the results on the standard output. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 69 +++ trailer.h | 6 ++ 2 files changed, 75 insertions(+) create mode 100644 trailer.h diff --git a/trailer.c b/trailer.c index 4f0de3b..b0be0d7 100644 --- a/trailer.c +++ b/trailer.c @@ -1,5 +1,6 @@ #include cache.h #include string-list.h +#include trailer.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org */ @@ -87,6 +88,35 @@ static void free_trailer_item(struct trailer_item *item) free(item); } +static char last_non_space_char(const char *s) +{ + int i; + for (i = strlen(s) - 1; i = 0; i--) + if (!isspace(s[i])) + return s[i]; + return '\0'; +} + +static void print_tok_val(const char *tok, const char *val) +{ + char c = last_non_space_char(tok); + if (!c) + return; + if (strchr(separators, c)) + printf(%s%s\n, tok, val); + else + printf(%s%c %s\n, tok, separators[0], val); +} + +static void print_all(struct trailer_item *first, int trim_empty) +{ + struct trailer_item *item; + for (item = first; item; item = item-next) { + if (!trim_empty || strlen(item-value) 0) + print_tok_val(item-token, item-value); + } +} + static void update_last(struct trailer_item **last) { if (*last) @@ -697,3 +727,42 @@ static int process_input_file(struct strbuf **lines, return patch_start; } + +static void free_all(struct trailer_item **first) +{ + while (*first) { + struct trailer_item *item = remove_first(first); + free_trailer_item(item); + } +} + +void process_trailers(const char *file, int trim_empty, struct string_list *trailers) +{ + struct trailer_item *in_tok_first = NULL; + struct trailer_item *in_tok_last = NULL; + struct trailer_item *arg_tok_first; + struct strbuf **lines; + int patch_start; + + /* Default config must be setup first */ + git_config(git_trailer_default_config, NULL); + git_config(git_trailer_config, NULL); + + lines = read_input_file(file); + + /* Print the lines before the trailers */ + patch_start = process_input_file(lines, in_tok_first, in_tok_last); + + arg_tok_first = process_command_line_args(trailers); + + process_trailers_lists(in_tok_first, in_tok_last, arg_tok_first); + + print_all(in_tok_first, trim_empty); + + free_all(in_tok_first); + + /* Print the lines after the trailers as is */ + print_lines(lines, patch_start, INT_MAX); + + strbuf_list_free(lines); +} diff --git a/trailer.h b/trailer.h new file mode 100644 index 000..8eb25d5 --- /dev/null +++ b/trailer.h @@ -0,0 +1,6 @@ +#ifndef TRAILER_H +#define TRAILER_H + +void process_trailers(const char *file, int trim_empty, struct string_list *trailers); + +#endif /* TRAILER_H */ -- 2.1.0.rc0.248.gb91fdbc -- To unsubscribe from this list: send the line 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 v16 05/11] trailer: parse trailers from file or stdin
Read trailers from a file or from stdin, parse the trailers and then put the result into a doubly linked list. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 123 ++ 1 file changed, 123 insertions(+) diff --git a/trailer.c b/trailer.c index b5666b3..4f0de3b 100644 --- a/trailer.c +++ b/trailer.c @@ -69,6 +69,14 @@ static int same_trailer(struct trailer_item *a, struct trailer_item *b) return same_token(a, b) same_value(a, b); } +static inline int contains_only_spaces(const char *str) +{ + const char *s = str; + while (*s isspace(*s)) + s++; + return !*s; +} + static void free_trailer_item(struct trailer_item *item) { free(item-conf.name); @@ -574,3 +582,118 @@ static struct trailer_item *process_command_line_args(struct string_list *traile return arg_tok_first; } + +static struct strbuf **read_input_file(const char *file) +{ + struct strbuf **lines; + struct strbuf sb = STRBUF_INIT; + + if (file) { + if (strbuf_read_file(sb, file, 0) 0) + die_errno(_(could not read input file '%s'), file); + } else { + if (strbuf_read(sb, fileno(stdin), 0) 0) + die_errno(_(could not read from stdin)); + } + + lines = strbuf_split(sb, '\n'); + + strbuf_release(sb); + + return lines; +} + +/* + * Return the (0 based) index of the start of the patch or the line + * count if there is no patch in the message. + */ +static int find_patch_start(struct strbuf **lines, int count) +{ + int i; + + /* Get the start of the patch part if any */ + for (i = 0; i count; i++) { + if (starts_with(lines[i]-buf, ---)) + return i; + } + + return count; +} + +/* + * Return the (0 based) index of the first trailer line or count if + * there are no trailers. Trailers are searched only in the lines from + * index (count - 1) down to index 0. + */ +static int find_trailer_start(struct strbuf **lines, int count) +{ + int start, only_spaces = 1; + + /* +* Get the start of the trailers by looking starting from the end +* for a line with only spaces before lines with one separator. +*/ + for (start = count - 1; start = 0; start--) { + if (lines[start]-buf[0] == comment_line_char) + continue; + if (contains_only_spaces(lines[start]-buf)) { + if (only_spaces) + continue; + return start + 1; + } + if (strcspn(lines[start]-buf, separators) lines[start]-len) { + if (only_spaces) + only_spaces = 0; + continue; + } + return count; + } + + return only_spaces ? count : 0; +} + +static int has_blank_line_before(struct strbuf **lines, int start) +{ + for (;start = 0; start--) { + if (lines[start]-buf[0] == comment_line_char) + continue; + return contains_only_spaces(lines[start]-buf); + } + return 0; +} + +static void print_lines(struct strbuf **lines, int start, int end) +{ + int i; + for (i = start; lines[i] i end; i++) + printf(%s, lines[i]-buf); +} + +static int process_input_file(struct strbuf **lines, + struct trailer_item **in_tok_first, + struct trailer_item **in_tok_last) +{ + int count = 0; + int patch_start, trailer_start, i; + + /* Get the line count */ + while (lines[count]) + count++; + + patch_start = find_patch_start(lines, count); + trailer_start = find_trailer_start(lines, patch_start); + + /* Print lines before the trailers as is */ + print_lines(lines, 0, trailer_start); + + if (!has_blank_line_before(lines, trailer_start - 1)) + printf(\n); + + /* Parse trailer lines */ + for (i = trailer_start; i patch_start; i++) { + struct trailer_item *new = create_trailer_item(lines[i]-buf); + add_trailer_item(in_tok_first, in_tok_last, new); + } + + return patch_start; +} -- 2.1.0.rc0.248.gb91fdbc -- To unsubscribe from this list: send the line 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 v16 08/11] trailer: add tests for git interpret-trailers
Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t7513-interpret-trailers.sh | 738 ++ 1 file changed, 738 insertions(+) create mode 100755 t/t7513-interpret-trailers.sh diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh new file mode 100755 index 000..ad36cf8 --- /dev/null +++ b/t/t7513-interpret-trailers.sh @@ -0,0 +1,738 @@ +#!/bin/sh +# +# Copyright (c) 2013, 2014 Christian Couder +# + +test_description='git interpret-trailers' + +. ./test-lib.sh + +# When we want one trailing space at the end of each line, let's use sed +# to make sure that these spaces are not removed by any automatic tool. + +test_expect_success 'setup' ' + : empty + cat basic_message -\EOF + subject + + body + EOF + cat complex_message_body -\EOF + my subject + + my body which is long + and contains some special + chars like : = ? ! + + EOF + sed -e s/ Z\$/ / complex_message_trailers -\EOF + Fixes: Z + Acked-by: Z + Reviewed-by: Z + Signed-off-by: Z + EOF + cat basic_patch -\EOF + --- +foo.txt | 2 +- +1 file changed, 1 insertion(+), 1 deletion(-) + + diff --git a/foo.txt b/foo.txt + index 0353767..1d91aa1 100644 + --- a/foo.txt + +++ b/foo.txt + @@ -1,3 +1,3 @@ + + -bar + +baz + + -- + 1.9.rc0.11.ga562ddc + + EOF +' + +test_expect_success 'without config' ' + sed -e s/ Z\$/ / expected -\EOF + + ack: Peff + Reviewed-by: Z + Acked-by: Johan + EOF + git interpret-trailers --trailer ack = Peff --trailer Reviewed-by \ + --trailer Acked-by: Johan empty actual + test_cmp expected actual +' + +test_expect_success 'without config in another order' ' + sed -e s/ Z\$/ / expected -\EOF + + Acked-by: Johan + Reviewed-by: Z + ack: Peff + EOF + git interpret-trailers --trailer Acked-by: Johan --trailer Reviewed-by \ + --trailer ack = Peff empty actual + test_cmp expected actual +' + +test_expect_success '--trim-empty without config' ' + cat expected -\EOF + + ack: Peff + Acked-by: Johan + EOF + git interpret-trailers --trim-empty --trailer ack=Peff \ + --trailer Reviewed-by --trailer Acked-by: Johan \ + --trailer sob: empty actual + test_cmp expected actual +' + +test_expect_success 'with config option on the command line' ' + cat expected -\EOF + + Acked-by: Johan + Reviewed-by: Peff + EOF + echo Acked-by: Johan | \ + git -c trailer.Acked-by.ifexists=addifdifferent interpret-trailers \ + --trailer Reviewed-by: Peff --trailer Acked-by: Johan actual + test_cmp expected actual +' + +test_expect_success 'with config setup' ' + git config trailer.ack.key Acked-by: + cat expected -\EOF + + Acked-by: Peff + EOF + git interpret-trailers --trim-empty --trailer ack = Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by = Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by :Peff empty actual + test_cmp expected actual +' + +test_expect_success 'with config setup and := as separators' ' + git config trailer.separators := + git config trailer.ack.key Acked-by= + cat expected -\EOF + + Acked-by= Peff + EOF + git interpret-trailers --trim-empty --trailer ack = Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by= Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by : Peff empty actual + test_cmp expected actual +' + +test_expect_success 'with config setup and % as separators' ' + git config trailer.separators % + cat expected -\EOF + + bug% 42 + count% 10 + bug% 422 + EOF + git interpret-trailers --trim-empty --trailer bug = 42 \ + --trailer count%10 --trailer test: stuff \ + --trailer bug % 422 empty actual + test_cmp expected actual +' + +test_expect_success 'with % as separators and a message with trailers' ' + cat special_message -\EOF + Special Message + + bug% 42 + count% 10 + bug% 422 + EOF + cat expected
[PATCH v16 09/11] trailer: execute command from 'trailer.name.command'
Let the user specify a command that will give on its standard output the value to use for the specified trailer. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 85 ++- 1 file changed, 84 insertions(+), 1 deletion(-) diff --git a/trailer.c b/trailer.c index b0be0d7..8514566 100644 --- a/trailer.c +++ b/trailer.c @@ -1,5 +1,7 @@ #include cache.h #include string-list.h +#include run-command.h +#include string-list.h #include trailer.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org @@ -33,6 +35,8 @@ static struct trailer_item *first_conf_item; static char *separators = :; +#define TRAILER_ARG_STRING $ARG + static int after_or_end(enum action_where where) { return (where == WHERE_AFTER) || (where == WHERE_END); @@ -78,6 +82,13 @@ static inline int contains_only_spaces(const char *str) return !*s; } +static inline void strbuf_replace(struct strbuf *sb, const char *a, const char *b) +{ + const char *ptr = strstr(sb-buf, a); + if (ptr) + strbuf_splice(sb, ptr - sb-buf, strlen(a), b, strlen(b)); +} + static void free_trailer_item(struct trailer_item *item) { free(item-conf.name); @@ -203,6 +214,63 @@ static struct trailer_item *remove_first(struct trailer_item **first) return item; } +static int read_from_command(struct child_process *cp, struct strbuf *buf) +{ + if (run_command(cp)) + return error(running trailer command '%s' failed, cp-argv[0]); + if (strbuf_read(buf, cp-out, 1024) 1) + return error(reading from trailer command '%s' failed, cp-argv[0]); + strbuf_trim(buf); + return 0; +} + +static const char *apply_command(const char *command, const char *arg) +{ + struct strbuf cmd = STRBUF_INIT; + struct strbuf buf = STRBUF_INIT; + struct child_process cp; + const char *argv[] = {NULL, NULL}; + const char *result; + + strbuf_addstr(cmd, command); + if (arg) + strbuf_replace(cmd, TRAILER_ARG_STRING, arg); + + argv[0] = cmd.buf; + memset(cp, 0, sizeof(cp)); + cp.argv = argv; + cp.env = local_repo_env; + cp.no_stdin = 1; + cp.out = -1; + cp.use_shell = 1; + + if (read_from_command(cp, buf)) { + strbuf_release(buf); + result = xstrdup(); + } else + result = strbuf_detach(buf, NULL); + + strbuf_release(cmd); + return result; +} + +static void apply_item_command(struct trailer_item *in_tok, struct trailer_item *arg_tok) +{ + if (arg_tok-conf.command) { + const char *arg; + if (arg_tok-value arg_tok-value[0]) { + arg = arg_tok-value; + } else { + if (in_tok in_tok-value) + arg = xstrdup(in_tok-value); + else + arg = xstrdup(); + } + arg_tok-value = apply_command(arg_tok-conf.command, arg); + free((char *)arg); + } +} + static void apply_arg_if_exists(struct trailer_item *in_tok, struct trailer_item *arg_tok, struct trailer_item *on_tok, @@ -214,16 +282,19 @@ static void apply_arg_if_exists(struct trailer_item *in_tok, free_trailer_item(arg_tok); break; case EXISTS_REPLACE: + apply_item_command(in_tok, arg_tok); add_arg_to_input_list(on_tok, arg_tok, in_tok_first, in_tok_last); remove_from_list(in_tok, in_tok_first, in_tok_last); free_trailer_item(in_tok); break; case EXISTS_ADD: + apply_item_command(in_tok, arg_tok); add_arg_to_input_list(on_tok, arg_tok, in_tok_first, in_tok_last); break; case EXISTS_ADD_IF_DIFFERENT: + apply_item_command(in_tok, arg_tok); if (check_if_different(in_tok, arg_tok, 1)) add_arg_to_input_list(on_tok, arg_tok, in_tok_first, in_tok_last); @@ -231,6 +302,7 @@ static void apply_arg_if_exists(struct trailer_item *in_tok, free_trailer_item(arg_tok); break; case EXISTS_ADD_IF_DIFFERENT_NEIGHBOR: + apply_item_command(in_tok, arg_tok); if (check_if_different(on_tok, arg_tok, 0)) add_arg_to_input_list(on_tok, arg_tok, in_tok_first, in_tok_last); @@ -254,6 +326,7 @@ static void apply_arg_if_missing(struct trailer_item **in_tok_first, case MISSING_ADD:
[PATCH v16 11/11] Documentation: add documentation for 'git interpret-trailers'
While at it add git-interpret-trailers to command-list.txt. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-interpret-trailers.txt | 314 +++ command-list.txt | 1 + 2 files changed, 315 insertions(+) create mode 100644 Documentation/git-interpret-trailers.txt diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt new file mode 100644 index 000..81fac3d --- /dev/null +++ b/Documentation/git-interpret-trailers.txt @@ -0,0 +1,314 @@ +git-interpret-trailers(1) += + +NAME + +git-interpret-trailers - help add stuctured information into commit messages + +SYNOPSIS + +[verse] +'git interpret-trailers' [--trim-empty] [(--trailer token[(=|:)value])...] [file...] + +DESCRIPTION +--- +Help adding 'trailers' lines, that look similar to RFC 822 e-mail +headers, at the end of the otherwise free-form part of a commit +message. + +This command reads some patches or commit messages from either the +file arguments or the standard input if no file is specified. Then +this command applies the arguments passed using the `--trailer` +option, if any, to the commit message part of each input file. The +result is emitted on the standard output. + +Some configuration variables control the way the `--trailer` arguments +are applied to each commit message and the way any existing trailer in +the commit message is changed. They also make it possible to +automatically add some trailers. + +By default, a 'token=value' or 'token:value' argument given +using `--trailer` will be appended after the existing trailers only if +the last trailer has a different (token, value) pair (or if there +is no existing trailer). The token and value parts will be trimmed +to remove starting and trailing whitespace, and the resulting trimmed +token and value will appear in the message like this: + + +token: value + + +This means that the trimmed token and value will be separated by +`': '` (one colon followed by one space). + +By default the new trailer will appear at the end of all the existing +trailers. If there is no existing trailer, the new trailer will appear +after the commit message part of the ouput, and, if there is no line +with only spaces at the end of the commit message part, one blank line +will be added before the new trailer. + +Existing trailers are extracted from the input message by looking for +a group of one or more lines that contain a colon (by default), where +the group is preceded by one or more empty (or whitespace-only) lines. +The group must either be at the end of the message or be the last +non-whitespace lines before a line that starts with '---'. Such three +minus signs start the patch part of the message. + +When reading trailers, there can be whitespaces before and after the +token, the separator and the value. There can also be whitespaces +indide the token and the value. + +Note that 'trailers' do not follow and are not intended to follow many +rules for RFC 822 headers. For example they do not follow the line +folding rules, the encoding rules and probably many other rules. + +OPTIONS +--- +--trim-empty:: + If the value part of any trailer contains only whitespace, + the whole trailer will be removed from the resulting message. + This apply to existing trailers as well as new trailers. + +--trailer token[(=|:)value]:: + Specify a (token, value) pair that should be applied as a + trailer to the input messages. See the description of this + command. + +CONFIGURATION VARIABLES +--- + +trailer.separators:: + This option tells which characters are recognized as trailer + separators. By default only ':' is recognized as a trailer + separator, except that '=' is always accepted on the command + line for compatibility with other git commands. ++ +The first character given by this option will be the default character +used when another separator is not specified in the config for this +trailer. ++ +For example, if the value for this option is %=$, then only lines +using the format 'tokensepvalue' with sep containing '%', '=' +or '$' and then spaces will be considered trailers. And '%' will be +the default separator used, so by default trailers will appear like: +'token% value' (one percent sign and one space will appear between +the token and the value). + +trailer.where:: + This option tells where a new trailer will be added. ++ +This can be `end`, which is the default, `start`, `after` or `before`. ++ +If it is `end`, then each new trailer will appear at the end of the +existing trailers. ++ +If it is `start`, then each new trailer will appear at the start, +instead of the end, of the existing trailers. ++ +If it is
[PATCH v16 07/11] trailer: add interpret-trailers command
This patch adds the git interpret-trailers command. This command uses the previously added process_trailers() function in trailer.c. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- .gitignore | 1 + Makefile | 1 + builtin.h| 1 + builtin/interpret-trailers.c | 44 git.c| 1 + 5 files changed, 48 insertions(+) create mode 100644 builtin/interpret-trailers.c diff --git a/.gitignore b/.gitignore index cf0d191..85593de 100644 --- a/.gitignore +++ b/.gitignore @@ -74,6 +74,7 @@ /git-index-pack /git-init /git-init-db +/git-interpret-trailers /git-instaweb /git-log /git-ls-files diff --git a/Makefile b/Makefile index ef82972..0b06ae0 100644 --- a/Makefile +++ b/Makefile @@ -953,6 +953,7 @@ BUILTIN_OBJS += builtin/hash-object.o BUILTIN_OBJS += builtin/help.o BUILTIN_OBJS += builtin/index-pack.o BUILTIN_OBJS += builtin/init-db.o +BUILTIN_OBJS += builtin/interpret-trailers.o BUILTIN_OBJS += builtin/log.o BUILTIN_OBJS += builtin/ls-files.o BUILTIN_OBJS += builtin/ls-remote.o diff --git a/builtin.h b/builtin.h index 5d91f31..b87df70 100644 --- a/builtin.h +++ b/builtin.h @@ -73,6 +73,7 @@ extern int cmd_hash_object(int argc, const char **argv, const char *prefix); extern int cmd_help(int argc, const char **argv, const char *prefix); extern int cmd_index_pack(int argc, const char **argv, const char *prefix); extern int cmd_init_db(int argc, const char **argv, const char *prefix); +extern int cmd_interpret_trailers(int argc, const char **argv, const char *prefix); extern int cmd_log(int argc, const char **argv, const char *prefix); extern int cmd_log_reflog(int argc, const char **argv, const char *prefix); extern int cmd_ls_files(int argc, const char **argv, const char *prefix); diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c new file mode 100644 index 000..46838d2 --- /dev/null +++ b/builtin/interpret-trailers.c @@ -0,0 +1,44 @@ +/* + * Builtin git interpret-trailers + * + * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org + * + */ + +#include cache.h +#include builtin.h +#include parse-options.h +#include string-list.h +#include trailer.h + +static const char * const git_interpret_trailers_usage[] = { + N_(git interpret-trailers [--trim-empty] [(--trailer token[(=|:)value])...] [file...]), + NULL +}; + +int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) +{ + int trim_empty = 0; + struct string_list trailers = STRING_LIST_INIT_DUP; + + struct option options[] = { + OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty trailers)), + OPT_STRING_LIST(0, trailer, trailers, N_(trailer), + N_(trailer(s) to add)), + OPT_END() + }; + + argc = parse_options(argc, argv, prefix, options, +git_interpret_trailers_usage, 0); + + if (argc) { + int i; + for (i = 0; i argc; i++) + process_trailers(argv[i], trim_empty, trailers); + } else + process_trailers(NULL, trim_empty, trailers); + + string_list_clear(trailers, 0); + + return 0; +} diff --git a/git.c b/git.c index 5ebb32f..e327a90 100644 --- a/git.c +++ b/git.c @@ -417,6 +417,7 @@ static struct cmd_struct commands[] = { { index-pack, cmd_index_pack, RUN_SETUP_GENTLY }, { init, cmd_init_db, NO_SETUP }, { init-db, cmd_init_db, NO_SETUP }, + { interpret-trailers, cmd_interpret_trailers, RUN_SETUP }, { log, cmd_log, RUN_SETUP }, { ls-files, cmd_ls_files, RUN_SETUP }, { ls-remote, cmd_ls_remote, RUN_SETUP_GENTLY }, -- 2.1.0.rc0.248.gb91fdbc -- To unsubscribe from this list: send the line 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 0/5] add unset.variable for unsetting previously set variables
Jeff King p...@peff.net writes: ... Which makes me wonder if safe-include is really helping that much versus a project shipping a shell script that munges the repository config. The latter is less safe (you are, after all, running code, but you would at least have the chance to examine it), but is way more flexible. And the safety is comparable to running make on a cloned project. I dunno. I do not have anything against the safe-include idea, but each time it comes up, I think we are often left guessing about exactly which config options projects would want to set, and to what values. I tend to agree. Every time somebody says a project wants to give its participants suggested settings, we seem to tell them to ship an instruction to their participants, either in BUILDING or setup.sh or whatever. It certainly is simpler and more flexible. The only real difference it might make is an attempt to push to an unattended place and automatically making the changes to take effect, aka push to deploy, which is not what we encourage anyway, so... -- To unsubscribe from this list: send the line 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] -x tracing option for tests
Jeff King p...@peff.net writes: These patches are pulled out of the prune-mtime series I posted earlier[1]. The discussion veered off and there's no reason that the two topics need to be part of the same series. These look all sensible. Is your plan to reroll the prune-mtime stuff on top of these? The additional safety those patches would give us is valuable and they are pretty straight-forward---I was hoping to have them in the 2.2 release. 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 v4] Handle atexit list internaly for unthreaded builds
Wrap atexit()s calls on unthreaded builds to handle callback list internally. This is needed because on unthreaded builds, asyncs inherits parent's atexit() list, that gets run as soon as the async exit()s (and again at the end of the parent process). That led to remove temporary and lock files too early. Fixes test 5537 (temporary shallow file vanished before unpack-objects could open it) V4: fix bogus preprocessor directives Thanks-to: Duy Nguyen pclo...@gmail.com Thanks-to: Andreas Schwab sch...@linux-m68k.org Signed-off-by: Etienne Buira etienne.bu...@gmail.com --- builtin/clone.c | 5 - git-compat-util.h | 5 + run-command.c | 40 shallow.c | 7 ++- 4 files changed, 47 insertions(+), 10 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index bbd169c..e122f33 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo) static const char *junk_work_tree; static const char *junk_git_dir; -static pid_t junk_pid; static enum { JUNK_LEAVE_NONE, JUNK_LEAVE_REPO, @@ -417,8 +416,6 @@ static void remove_junk(void) break; } - if (getpid() != junk_pid) - return; if (junk_git_dir) { strbuf_addstr(sb, junk_git_dir); remove_dir_recursively(sb, 0); @@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix) struct refspec *refspec; const char *fetch_pattern; - junk_pid = getpid(); - packet_trace_identity(clone); argc = parse_options(argc, argv, prefix, builtin_clone_options, builtin_clone_usage, 0); diff --git a/git-compat-util.h b/git-compat-util.h index f587749..6dd63dd 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -577,6 +577,11 @@ int inet_pton(int af, const char *src, void *dst); const char *inet_ntop(int af, const void *src, char *dst, size_t size); #endif +#ifdef NO_PTHREADS +#define atexit git_atexit +extern int git_atexit(void (*handler)(void)); +#endif + extern void release_pack_memory(size_t); typedef void (*try_to_free_t)(size_t); diff --git a/run-command.c b/run-command.c index 35a3ebf..0f9a9b0 100644 --- a/run-command.c +++ b/run-command.c @@ -624,6 +624,45 @@ static int async_die_is_recursing(void) return ret != NULL; } +#else + +static struct { + void (**handlers)(void); + size_t nr; + size_t alloc; +} git_atexit_hdlrs; + +static int git_atexit_installed; + +static void git_atexit_dispatch() +{ + size_t i; + + for (i=git_atexit_hdlrs.nr ; i ; i--) + git_atexit_hdlrs.handlers[i-1](); +} + +static void git_atexit_clear() +{ + free(git_atexit_hdlrs.handlers); + memset(git_atexit_hdlrs, 0, sizeof(git_atexit_hdlrs)); + git_atexit_installed = 0; +} + +#undef atexit +int git_atexit(void (*handler)(void)) +{ + ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc); + git_atexit_hdlrs.handlers[git_atexit_hdlrs.nr++] = handler; + if (!git_atexit_installed) { + if (atexit(git_atexit_dispatch)) + return -1; + git_atexit_installed = 1; + } + return 0; +} +#define atexit git_atexit + #endif int start_async(struct async *async) @@ -682,6 +721,7 @@ int start_async(struct async *async) close(fdin[1]); if (need_out) close(fdout[0]); + git_atexit_clear(); exit(!!async-proc(proc_in, proc_out, async-data)); } diff --git a/shallow.c b/shallow.c index de07709..f067811 100644 --- a/shallow.c +++ b/shallow.c @@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo) const char *setup_temporary_shallow(const struct sha1_array *extra) { - static int installed_handler; struct strbuf sb = STRBUF_INIT; int fd; @@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra) strbuf_addstr(temporary_shallow, git_path(shallow_XX)); fd = xmkstemp(temporary_shallow.buf); - if (!installed_handler) { - atexit(remove_temporary_shallow); - sigchain_push_common(remove_temporary_shallow_on_signal); - } + atexit(remove_temporary_shallow); + sigchain_push_common(remove_temporary_shallow_on_signal); if (write_in_full(fd, sb.buf, sb.len) != sb.len) die_errno(failed to write to %s, -- 2.0.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] test-lib.sh: support -x option for shell-tracing
Jeff King p...@peff.net writes: Hmph. I had originally intended to make this set -x; with a semicolon, to keep it split from $*. But I forgot to, and much to my surprise, all of the tests still passed. Yup, I was wondering why you posted a version without the semicolon, which looked obviously bogus, as I've never seen you post an untested thing without marking as such. + # The test itself is run with stderr put back to 4 (so either to + # /dev/null, or to the original stderr if --verbose was used). + { + test_eval_inner_ $@ /dev/null 3 24 + test_eval_ret_=$? + if test $trace = t + then + set +x + if test $test_eval_ret_ != 0 + then + say_color error 4 error: last command exited with \$?=$test_eval_ret_ + fi + fi + } 2/dev/null Hmph, that is a clever way to squelch output from set+x (and everything that runs after the eval returns) I never thought of. Nice. -- To unsubscribe from this list: send the line 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 v1] rebase -m: Use empty tree base for parentless commits
Hi, Junio C Hamano writes: Fabian Ruch baf...@gmail.com writes: diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh index d3fb67d..3f754ae 100644 --- a/git-rebase--merge.sh +++ b/git-rebase--merge.sh @@ -67,7 +67,13 @@ call_merge () { GIT_MERGE_VERBOSITY=1 export GIT_MERGE_VERBOSITY fi test -z $strategy strategy=recursive -eval 'git-merge-$strategy' $strategy_opts '$cmt^ -- $hd $cmt' +base=$(git rev-list --parents -1 $cmt | cut -d ' ' -s -f 2 -) +if test -z $base +then +# the empty tree sha1 +base=4b825dc642cb6eb9a060e54bf8d69288fbee4904 +fi +eval 'git-merge-$strategy' $strategy_opts '$base -- $hd $cmt' This looks wrong. The interface to git-merge-$strategy is designed in such a way that each strategy should be capable of taking _no_ base at all. Ok, but doesn't this use of the git-merge-$strategy interface (as shown in the example below) apply only to the case where one wants to merge two histories by creating a merge commit? When a merge commit is being created, the documentation states that git-merge abstracts from the commit history considering the _total change_ since a merge base on each branch. In contrast, here (i.e., in the case of git-rebase--merge) we care about how the changes introduced by the _individual commits_ are applied. Therefore, don't we want to be explicit about the base and tell git-merge-$strategy exactly which changes it should merge into the current head? The codebase has always been doing this both for git-rebase--merge and git-cherry-pick. What leads to the reported bug is that the latter covers the case where the commit object has no parents but the former doesn't. Root commits are handled by git-cherry-pick (and should be by git-rebase--merge) using an explicit base for the same reason why $cmt^ is given. See how unquoted $common is given to git-merge-$strategy in contrib/examples/git-merge.sh, i.e. eval 'git-merge-$strategy '$xopt' $common -- $head_arg $@' where common comes from common=$(git merge-base ...) which would be empty when you are looking at disjoint histories. If there are still objections to the patch because of the magic number and the cut, it might be worth considering an implementation of git-rebase--merge using git-cherry-pick's merge strategy option. Fabian -- To unsubscribe from this list: send the line 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: Smart HTTP
On Mon, 13 Oct 2014 17:29:05 + John Norris j...@norricorp.f9.co.uk wrote: I guess this comment is aimed at Scott Chacon. I have read your blog post on Smart HTTP (http://git-scm.com/blog/2010/03/04/smart-http.html) and wondered if there is any documentation that compares in terms of thoroughness with your sections in the book on using SSH, which does explain the basics so that anyone can get it working. I have tried setting up authenticated pull and push with HTTP (not HTTPS) and Apache never asks for authentication during a pull and refuses a push with a 403 error. Looks like a sort-of followup to this discussion [1]. (John, being a good netizen, you should have included the link to that discussion yourself, to put your uh comment in context and may be actually get some useful responses.) 1. https://groups.google.com/d/topic/git-users/zcXYY1Le_F4/discussion -- To unsubscribe from this list: send the line 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] receive-pack: plug minor memory leak in unpack()
Jeff King p...@peff.net writes: On Sat, Oct 11, 2014 at 01:00:16PM +0200, René Scharfe wrote: The argv_array used in unpack() is never freed. Instead of adding explicit calls to argv_array_clear() use the args member of struct child_process and let run_command() and friends clean up for us. Looks good. I notice that the recently added prepare_push_cert_sha1 uses an argv_array to create the child_process.env, and we leak the result. You're right. finish_command() runs argv_array_clear() on cmd-args, but does not care about cmd-env so anybody that use the (optional) use these environment variable settings while running the command would leak unless the caller takes care of it. I wonder if run-command should provide a managed env array similar to the args array. I took a look at a few of them: - setup_pager() uses a static set of environment variables that are not built with argv_array(); should be easy to convert, though. - wt_status_print_submodule_summary() does use two argv_arrays, for env and argv. It can use the managed args today, and with such a change it can also use the managed envs. - receive-pack.c::prepare_push_cert_sha1() would benefit from managed envs. - http-backend.c::run_service() would benefit from managed envs. - daemon.c::handle() uses a static set of environment variables that are not built with argv_array(). Same for argv. It shouldn't be too hard to catch and convert all of them. -- To unsubscribe from this list: send the line 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] fix compilation with --disable-pthreads
Etienne Buira etienne.bu...@gmail.com writes: Subject: Re: [PATCH 1/2] fix compilation with --disable-pthreads That probably is a typo of NO_PTHREADS=NoThanks or something. Thanks. Just out of curiosity, are you porting to some exotic platforms? Signed-off-by: Etienne Buira etienne.bu...@gmail.com --- builtin/index-pack.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builtin/index-pack.c b/builtin/index-pack.c index eebf1a8..0f88f4b 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -185,6 +185,9 @@ static void cleanup_thread(void) #define deepest_delta_lock() #define deepest_delta_unlock() +#define type_cas_lock() +#define type_cas_unlock() + #endif -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/3] mergetool: don't require a work tree for --tool-help
David Aguilar dav...@gmail.com writes: From: Charles Bailey cbaile...@bloomberg.net Signed-off-by: Charles Bailey cbaile...@bloomberg.net Signed-off-by: David Aguilar dav...@gmail.com --- Changes since v2: This now uses the new git_dir_init function. git-mergetool.sh | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/git-mergetool.sh b/git-mergetool.sh index 96a61ba..cddb533 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -10,11 +10,11 @@ USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] ...' SUBDIRECTORY_OK=Yes +NONGIT_OK=Yes OPTIONS_SPEC= TOOL_MODE=merge . git-sh-setup . git-mergetool--lib -require_work_tree # Returns true if the mode reflects a symlink is_symlink () { @@ -378,6 +378,9 @@ prompt_after_failed_merge () { done } +require_work_tree +git_dir_init This is somewhat curious. Shouldn't the order of these swapped? + if test -z $merge_tool then # Check if a merge tool has been configured -- To unsubscribe from this list: send the line 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] mergetool: add an option for writing to a temporary directory
David Aguilar dav...@gmail.com writes: Teach mergetool to write files in a temporary directory when 'mergetool.writeToTemp' is true. This is helpful for tools such as Eclipse which cannot cope with multiple copies of the same file in the worktree. Suggested-by: Charles Bailey char...@hashpling.org Signed-off-by: David Aguilar dav...@gmail.com --- This patch is dependent on my previous mergetool patches: use more conservative temporary... and the subsequent --tool-help series. I can understand why it depends on the foo_BACKUP_1234.c change, but why does it need to depend on the other one? Documentation/config.txt | 6 ++ git-mergetool.sh | 35 +++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 04a1e2f..be6cf35 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1768,6 +1768,12 @@ mergetool.keepTemporaries:: preserved, otherwise they will be removed after the tool has exited. Defaults to `false`. +mergetool.writeToTemp:: + Git writes temporary 'BASE', 'LOCAL', and 'REMOTE' versions of + conflicting files in the worktree by default. Git will attempt + to use a temporary directory for these files when set `true`. + Defaults to `false`. + mergetool.prompt:: Prompt before each invocation of the merge resolution program. diff --git a/git-mergetool.sh b/git-mergetool.sh index 10782b8..2b788c5 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -37,6 +37,19 @@ base_present () { test -n $base_mode } +mergetool_tmpdir_init () { + if test $(git config --bool mergetool.writeToTemp) != true + then + MERGETOOL_TMPDIR=. + return 0 + fi + if MERGETOOL_TMPDIR=$(mktemp -d -t git-mergetool-XX 2/dev/null) + then + return 0 + fi + die error: mktemp is needed when 'mergetool.writeToTemp' is true +} + cleanup_temp_files () { if test $1 = --save-backup then @@ -46,6 +59,10 @@ cleanup_temp_files () { else rm -f -- $LOCAL $REMOTE $BASE $BACKUP fi + if test $MERGETOOL_TMPDIR != . + then + rmdir $MERGETOOL_TMPDIR + fi } describe_file () { @@ -235,10 +252,20 @@ merge_file () { BASE=$MERGED ext= fi - BACKUP=./${BASE}_BACKUP_$$$ext - LOCAL=./${BASE}_LOCAL_$$$ext - REMOTE=./${BASE}_REMOTE_$$$ext - BASE=./${BASE}_BASE_$$$ext + + mergetool_tmpdir_init + + if test $MERGETOOL_TMPDIR != . + then + # If we're using a temporary directory then write to the + # top-level of that directory. + BASE=${BASE##*/} + fi + + BACKUP=$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext + LOCAL=$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext + REMOTE=$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext + BASE=$MERGETOOL_TMPDIR/${BASE}_BASE_$$$ext base_mode=$(git ls-files -u -- $MERGED | awk '{if ($3==1) print $1;}') local_mode=$(git ls-files -u -- $MERGED | awk '{if ($3==2) print $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] mergetool: add an option for writing to a temporary directory
David Aguilar dav...@gmail.com writes: Teach mergetool to write files in a temporary directory when 'mergetool.writeToTemp' is true. This is helpful for tools such as Eclipse which cannot cope with multiple copies of the same file in the worktree. With this can we drop the change the temporary file name patch by Robin Rosenberg? http://thread.gmane.org/gmane.comp.version-control.git/255457/focus=255599 Message-Id: 1408607240-11369-1-git-send-email-robin.rosenb...@dewire.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mergetool: use more conservative temporary filenames
David Aguilar dav...@gmail.com writes: Avoid filenames with multiple dots so that overly-picky tools do not misinterpret their extension. Previously, foo/bar.ext in the worktree would result in e.g. ./foo/bar.ext.BASE.1234.ext This can be improved by having only a single .ext and using underscore instead of dot so that the extension cannot be misinterpreted. The resulting path becomes: ./foo/bar_BASE_1234.ext Suggested-by: Sergio Ferrero sferr...@ensoftcorp.com Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: David Aguilar dav...@gmail.com --- Changes since v1 The commit message changed to say ./foo instead of foo. The patch now uses Junio's suggestion to minimize variables, and preserves the original leading ./ just in case there are tools that rely on having ./ in front of relative paths. ;-) Perhaps together with the allow temporary directory patch, we would want to have a few tests for these changes? git-mergetool.sh | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/git-mergetool.sh b/git-mergetool.sh index 9a046b7..96a61ba 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -228,11 +228,17 @@ merge_file () { return 1 fi - ext=$$$(expr $MERGED : '.*\(\.[^/]*\)$') - BACKUP=./$MERGED.BACKUP.$ext - LOCAL=./$MERGED.LOCAL.$ext - REMOTE=./$MERGED.REMOTE.$ext - BASE=./$MERGED.BASE.$ext + if BASE=$(expr $MERGED : '\(.*\)\.[^/]*$') + then + ext=$(expr $MERGED : '.*\(\.[^/]*\)$') + else + BASE=$MERGED + ext= + fi + BACKUP=./${BASE}_BACKUP_$$$ext + LOCAL=./${BASE}_LOCAL_$$$ext + REMOTE=./${BASE}_REMOTE_$$$ext + BASE=./${BASE}_BASE_$$$ext base_mode=$(git ls-files -u -- $MERGED | awk '{if ($3==1) print $1;}') local_mode=$(git ls-files -u -- $MERGED | awk '{if ($3==2) print $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 2/2] Remove spurious 'no threads support' warnings
Etienne Buira etienne.bu...@gmail.com writes: Threads count being defaulted to 0 (autodetect), and --disable-pthreads build checking that thread count==1, there were spurious warnings about threads being ignored, despite not specified on command line/conf. Fixes tests 5521 and 5526 that were broken in --disable-pthreads builds because of those warnings. Signed-off-by: Etienne Buira etienne.bu...@gmail.com --- I am not sure if this is the right fix. Shouldn't a --threads=0 from the command line (when there is a pack.threads configuration hardcoding some number to override it) give a chance to the auto detection codepath to ask online_cpus() and receive 1 on NO_PTHREADS build to avoid triggering the same warning you are squelching with this patch? That is, something like this instead, perhaps? -- 8 -- Subject: [PATCH] pack-objects: set number of threads before checking and warning Under NO_PTHREADS build, we warn when delta_search_threads is not set to 1, because that is the only sensible value on a single threaded build. However, the auto detection that kicks in when that variable is set to 0 (e.g. there is no configuration variable or command line option, or an explicit --threads=0 is given from the command line to override the pack.threads configuration to force auto-detection) was not done before the condition to issue this warning was tested. Move the auto-detection code and place it at an appropriate spot. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/pack-objects.c | 6 -- thread-utils.h | 4 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index d391934..a715237 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1972,8 +1972,6 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size, init_threaded_search(); - if (!delta_search_threads) /* --threads=0 means autodetect */ - delta_search_threads = online_cpus(); if (delta_search_threads = 1) { find_deltas(list, list_size, window, depth, processed); cleanup_threaded_search(); @@ -2685,6 +2683,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) pack_compression_level = Z_DEFAULT_COMPRESSION; else if (pack_compression_level 0 || pack_compression_level Z_BEST_COMPRESSION) die(bad pack compression level %d, pack_compression_level); + + if (!delta_search_threads) /* --threads=0 means autodetect */ + delta_search_threads = online_cpus(); + #ifdef NO_PTHREADS if (delta_search_threads != 1) warning(no threads support, ignoring --threads); diff --git a/thread-utils.h b/thread-utils.h index 6fb98c3..d9a769d 100644 --- a/thread-utils.h +++ b/thread-utils.h @@ -7,5 +7,9 @@ extern int online_cpus(void); extern int init_recursive_mutex(pthread_mutex_t*); +#else + +#define online_cpus() 1 + #endif #endif /* THREAD_COMPAT_H */ -- 2.1.2-468-g1a77c5b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] Handle atexit list internaly for unthreaded builds
Etienne Buira etienne.bu...@gmail.com writes: Wrap atexit()s calls on unthreaded builds to handle callback list internally. This is needed because on unthreaded builds, asyncs inherits parent's atexit() list, that gets run as soon as the async exit()s (and again at the end of the parent process). That led to remove temporary and lock files too early. ... that does not explain what you did to builtin/clone.c at all. Care to enlighten us, please? Fixes test 5537 (temporary shallow file vanished before unpack-objects could open it) V4: fix bogus preprocessor directives Please do not write this V4: line in the log message. People who read git log output and find this description would not know anything about the previous faulty ones. Putting it _after_ the three-dash line below will help the reviewers a lot. Thanks-to: Duy Nguyen pclo...@gmail.com Thanks-to: Andreas Schwab sch...@linux-m68k.org Usually we spell these Helped-by: instead. Signed-off-by: Etienne Buira etienne.bu...@gmail.com --- Thanks. builtin/clone.c | 5 - git-compat-util.h | 5 + run-command.c | 40 shallow.c | 7 ++- 4 files changed, 47 insertions(+), 10 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index bbd169c..e122f33 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo) static const char *junk_work_tree; static const char *junk_git_dir; -static pid_t junk_pid; static enum { JUNK_LEAVE_NONE, JUNK_LEAVE_REPO, @@ -417,8 +416,6 @@ static void remove_junk(void) break; } - if (getpid() != junk_pid) - return; if (junk_git_dir) { strbuf_addstr(sb, junk_git_dir); remove_dir_recursively(sb, 0); @@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix) struct refspec *refspec; const char *fetch_pattern; - junk_pid = getpid(); - packet_trace_identity(clone); argc = parse_options(argc, argv, prefix, builtin_clone_options, builtin_clone_usage, 0); diff --git a/git-compat-util.h b/git-compat-util.h index f587749..6dd63dd 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -577,6 +577,11 @@ int inet_pton(int af, const char *src, void *dst); const char *inet_ntop(int af, const void *src, char *dst, size_t size); #endif +#ifdef NO_PTHREADS +#define atexit git_atexit +extern int git_atexit(void (*handler)(void)); +#endif + extern void release_pack_memory(size_t); typedef void (*try_to_free_t)(size_t); diff --git a/run-command.c b/run-command.c index 35a3ebf..0f9a9b0 100644 --- a/run-command.c +++ b/run-command.c @@ -624,6 +624,45 @@ static int async_die_is_recursing(void) return ret != NULL; } +#else + +static struct { + void (**handlers)(void); + size_t nr; + size_t alloc; +} git_atexit_hdlrs; + +static int git_atexit_installed; + +static void git_atexit_dispatch() +{ + size_t i; + + for (i=git_atexit_hdlrs.nr ; i ; i--) + git_atexit_hdlrs.handlers[i-1](); +} + +static void git_atexit_clear() +{ + free(git_atexit_hdlrs.handlers); + memset(git_atexit_hdlrs, 0, sizeof(git_atexit_hdlrs)); + git_atexit_installed = 0; +} + +#undef atexit +int git_atexit(void (*handler)(void)) +{ + ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc); + git_atexit_hdlrs.handlers[git_atexit_hdlrs.nr++] = handler; + if (!git_atexit_installed) { + if (atexit(git_atexit_dispatch)) + return -1; + git_atexit_installed = 1; + } + return 0; +} +#define atexit git_atexit + #endif int start_async(struct async *async) @@ -682,6 +721,7 @@ int start_async(struct async *async) close(fdin[1]); if (need_out) close(fdout[0]); + git_atexit_clear(); exit(!!async-proc(proc_in, proc_out, async-data)); } diff --git a/shallow.c b/shallow.c index de07709..f067811 100644 --- a/shallow.c +++ b/shallow.c @@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo) const char *setup_temporary_shallow(const struct sha1_array *extra) { - static int installed_handler; struct strbuf sb = STRBUF_INIT; int fd; @@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra) strbuf_addstr(temporary_shallow, git_path(shallow_XX)); fd = xmkstemp(temporary_shallow.buf); - if (!installed_handler) { - atexit(remove_temporary_shallow); - sigchain_push_common(remove_temporary_shallow_on_signal); - } +
Re: [PATCH] Initialise hash variable to prevent compiler warnings
Felipe Franciosi fel...@paradoxo.org writes: The 'hash' variable in test-hashmap.c is not initialised properly which causes some 'gcc' versions to complain during compilation. FNV/I/IDIV10/0 covers all the possibilities of (method 3), I would have to say that the compiler needs to be fixed. Or insert default: just before case HASH_METHOD_0: line? I dunno. Signed-off-by: Felipe Franciosi fel...@paradoxo.org --- test-hashmap.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-hashmap.c b/test-hashmap.c index 07aa7ec..cc2891d 100644 --- a/test-hashmap.c +++ b/test-hashmap.c @@ -47,7 +47,7 @@ static struct test_entry *alloc_test_entry(int hash, char *key, int klen, static unsigned int hash(unsigned int method, unsigned int i, const char *key) { - unsigned int hash; + unsigned int hash = 0; switch (method 3) { case HASH_METHOD_FNV: -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] Documentation: adjust document title underlining
Thanks, obviously correct. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Allow building Git with Asciidoctor
brian m. carlson sand...@crustytoothpaste.net writes: This series is designed to implement the changes necessary to build Git using Asciidoctor instead of AsciiDoc. The first two patches are bug fixes. Asciidoctor is stricter about title underline lengths (± 1 character instead of 2) and requires matching delimiter lengths[0]. They're needed regardless of whether the other two patches are accepted because git-scm.com uses Asciidoctor to render the documentation, so we might as well render it correctly. Even with these patches, Asciidoctor warns about everyday.txt and user-manual.txt. I'm not sending patches for these right now because I've seen recent series including those and don't want to cause a merge conflict. Sounds good. The second two patches implement some basic support for building with Asciidoctor. The first of these moves some items into variables due to some differences between the AsciiDoc and Asciidoctor command lines. The user can then override these values when invoking make. The final patch adds support for the linkgit macro. Asciidoctor uses Ruby extensions to implement macro support, unlike AsciiDoc, which uses a configuration file. What I do not understand is that 3/4 lets you drop inclusion of asciidoc.conf which contains a lot more than just linkgit: definition. For now I'll queue only the first two, which unquestionably take us in the right direction. 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 0/3] -x tracing option for tests
On Mon, Oct 13, 2014 at 11:24:42AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: These patches are pulled out of the prune-mtime series I posted earlier[1]. The discussion veered off and there's no reason that the two topics need to be part of the same series. These look all sensible. Is your plan to reroll the prune-mtime stuff on top of these? The additional safety those patches would give us is valuable and they are pretty straight-forward---I was hoping to have them in the 2.2 release. Yes, I've delayed while thinking about the issues that Michael raised. There are basically two paths I see: 1. These do not solve all problems/races, but are a solid base and sensible path forward for further changes which we can worry about later. 2. There is a better way to provide prune safety, and these patches will get in the way of doing that. I wanted to make sure we are on path (1) and not path (2). :) I'll try to send out a re-roll tonight. -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 2/3] t5304: use helper to report failure of test foo = bar
On Mon, Oct 13, 2014 at 09:10:22AM -0700, Jonathan Nieder wrote: Jeff King wrote: For small outputs, we sometimes use: test $(some_cmd) = something we expect instead of a full test_cmp. The downside of this is that when it fails, there is no output at all from the script. There's another downside to that construct: it loses the exit status from some_cmd. Yes, although I think in many cases it's not a big deal. For example, here we lose the exit code of count-objects, but it also is very unlikely to fail _and_ produce our expected output. Alternatively, maybe there could be a helper in the same spirit as test_cmp_rev? test_object_count () { git count-objects output sed s/ .*// output count printf %s\n $1 expect test_cmp expect count } One of my goals was to provide a more generic helper so that we don't have to make little helpers like this for every command. So I'd much rather something like: test_output () { printf %s\n $1 expect shift $@ output test_cmp expect output } The \n handling there feels a little hacky, but is probably OK in practice (the few commands that really do generate an output without a newline can use test_cmp manually). It should probably also rm the files on success to avoid polluting the working tree. It also doesn't help cases that want to do test $foo -lt 3 or other non-equality checks. But those are probably the minority. I dunno. I am OK with what I posted, but if we feel strongly about testing the exit code, I can re-roll as test_output as above. -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: [msysGit] Re: msysgit works on wine
Hi Michael, On Mon, 13 Oct 2014, Michael Stefaniuc wrote: On 10/10/2014 02:04 PM, Duy Nguyen wrote: On Fri, Oct 10, 2014 at 7:02 PM, Thomas Braun Are you compiling git.git or msysgit.git? git.git And how about the test suite? running right now, fingers crossed.. kinda slow, not sure if it's wine or it's the msys thing. We (Wine) are interested in bug reports for git tests failing on Wine that succeed on Windows/Linux. Performance issues compared to Windows are highly desired too. Awesome, thank you for the offer! I haven't tried this in a long time, mainly because at some stage Wine required a separate console from my terminal to run command-line programs. And it seemed to me as if in particular process-spawning and heavy-duty file system operations were the bottleneck. Hopefully I will find some time soon to perform those tests again. Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] t5304: use helper to report failure of test foo = bar
Jeff King wrote: On Mon, Oct 13, 2014 at 09:10:22AM -0700, Jonathan Nieder wrote: There's another downside to that construct: it loses the exit status from some_cmd. Yes, although I think in many cases it's not a big deal. For example, here we lose the exit code of count-objects, but it also is very unlikely to fail _and_ produce our expected output. It could segfault after producing the good output, but sure, count-objects code doesn't change very often. [...] One of my goals was to provide a more generic helper so that we don't have to make little helpers like this for every command. So I'd much rather something like: test_output () { printf %s\n $1 expect shift $@ output test_cmp expect output } I agree with the principle in general. Unfortunately that wouldn't help here --- the $@ is a command with a pipe to sed in it and we still lose the exit status from count-objects. Hoping that clarifies, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] t5304: use helper to report failure of test foo = bar
On Mon, Oct 13, 2014 at 2:31 PM, Jonathan Nieder jrnie...@gmail.com wrote: Jeff King wrote: On Mon, Oct 13, 2014 at 09:10:22AM -0700, Jonathan Nieder wrote: There's another downside to that construct: it loses the exit status from some_cmd. Yes, although I think in many cases it's not a big deal. For example, here we lose the exit code of count-objects, but it also is very unlikely to fail _and_ produce our expected output. It could segfault after producing the good output, but sure, count-objects code doesn't change very often. Doesn't change very often is not the issue. Here we are not testing if it can count correctly without crashing, which *is* the real reason why it is perfectly fine to use $(git count-objects | sed ...) pattern here. There of course should be a test for count-objects to make sure it counts correctly without crashing. -- To unsubscribe from this list: send the line 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/3] t5304: use helper to report failure of test foo = bar
On Mon, Oct 13, 2014 at 02:31:32PM -0700, Jonathan Nieder wrote: One of my goals was to provide a more generic helper so that we don't have to make little helpers like this for every command. So I'd much rather something like: test_output () { printf %s\n $1 expect shift $@ output test_cmp expect output } I agree with the principle in general. Unfortunately that wouldn't help here --- the $@ is a command with a pipe to sed in it and we still lose the exit status from count-objects. Thanks, I missed that subtlety from what you posted earlier. That's another good reason that something like test_output is not really sufficient (you could eval() a snippet of shell, but then we have not really improved on the verbose test $a = $b version, since as you note we are still missing the exit code). -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 2/3] t5304: use helper to report failure of test foo = bar
Junio C Hamano wrote: On Mon, Oct 13, 2014 at 2:31 PM, Jonathan Nieder jrnie...@gmail.com wrote: It could segfault after producing the good output, but sure, count-objects code doesn't change very often. Doesn't change very often is not the issue. Here we are not testing if it can count correctly without crashing, which *is* the real reason why it is perfectly fine to use $(git count-objects | sed ...) pattern here. There of course should be a test for count-objects to make sure it counts correctly without crashing. That also makes sense, but it is a pretty big change from the general strategy used in git tests today. Currently they use -chaining to check the status from git commands used along the way. That way, unrelated bugs in git commands that are only used incidentally get caught, as long as they cause the command to crash. This has helped me in the past to find weird bugs early when they cause some particular command to crash on some platforms. Sometimes they are races that show up on more popular platforms later. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Initialise hash variable to prevent compiler warnings
On Mon, Oct 13, 2014 at 10:53 PM, Felipe Franciosi fel...@paradoxo.org wrote: On Mon, Oct 13, 2014 at 9:12 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Franciosi fel...@paradoxo.org writes: The 'hash' variable in test-hashmap.c is not initialised properly which causes some 'gcc' versions to complain during compilation. FNV/I/IDIV10/0 covers all the possibilities of (method 3), I would have to say that the compiler needs to be fixed. Or insert default: just before case HASH_METHOD_0: line? I dunno. Hmm... The default: would work, but is it really that bad to initialise a local variable in this case? In any case, the compilation warning is annoying. Do you prefer the default or the initialisation? Cheers, F. Hmm... The default: would work, but is it really that bad to initialise a local variable in this case? In any case, the compilation warning is annoying. Do you prefer the default or the initialisation? Cheers, F. Signed-off-by: Felipe Franciosi fel...@paradoxo.org --- test-hashmap.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-hashmap.c b/test-hashmap.c index 07aa7ec..cc2891d 100644 --- a/test-hashmap.c +++ b/test-hashmap.c @@ -47,7 +47,7 @@ static struct test_entry *alloc_test_entry(int hash, char *key, int klen, static unsigned int hash(unsigned int method, unsigned int i, const char *key) { - unsigned int hash; + unsigned int hash = 0; switch (method 3) { case HASH_METHOD_FNV: -- To unsubscribe from this list: send the line 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/3] t5304: use helper to report failure of test foo = bar
Jonathan Nieder jrnie...@gmail.com writes: Junio C Hamano wrote: On Mon, Oct 13, 2014 at 2:31 PM, Jonathan Nieder jrnie...@gmail.com wrote: It could segfault after producing the good output, but sure, count-objects code doesn't change very often. Doesn't change very often is not the issue. Here we are not testing if it can count correctly without crashing, which *is* the real reason why it is perfectly fine to use $(git count-objects | sed ...) pattern here. There of course should be a test for count-objects to make sure it counts correctly without crashing. That also makes sense, but it is a pretty big change from the general strategy used in git tests today. I would have to say that you are mistaken in reading what the strategy used today is. There is a subtle trade-off involved. When we test, say, git add a b, we may want to test these things: - git add when given addable paths a and b finishes without crashing; - git add will leave these paths in the index as expected. And we write git add a b test_write_lines a b expect git ls-files a b actual test_cmp expect actual NOT because we expect printf (which underlies test_write_lines) or git ls-files could somehow misbehave and dump core, but primarily because compared to an alternative, e.g. git add a b || return 1 test_write_lines a b expect git ls-files a b actual test_cmp expect actual it is far cleaner and easier to read with a rhythm. It is just an added bonus that we may catch errors due to filesystem quota when writing to expect or ls-files crashing. If the alternative had enough advantage over the established pattern (and here is where the trade off comes in---you need a certain taste to judge the advantage), it is perfectly fine to trade the exit value off and favor the advantage the alternative offers (e.g. a test that is easier to read). Between these two, it is very sensible to take A. over B. A. git create-garbage test $(git count-objects | sed ... | wc -l) = 0 B. git create-garbage test_when_finished rm -f tmp git count-objects tmp test $(sed ... tmp | wc -l) = 0 It will shift the trade-off if the more verbose alternative gets wrapped into a helper that is well constructed, though, because readability advantage of A over B melts away when we do so. -- To unsubscribe from this list: send the line 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] git-am: support any number of signatures
Christian Couder christian.cou...@gmail.com writes: On Sun, Oct 12, 2014 at 11:36 AM, Christian Couder christian.cou...@gmail.com wrote: With v16 you can easily choose if you want to have the S-o-b in the output or not, when there is already one, ... By the way, I sent v16 just before the above email, but the series still hasn't hit the mailing list. Did some of you guys in cc receive something? I see them and picked them up to replace. Are these now ready for 'next'? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] test-lib.sh: support -x option for shell-tracing
Jeff King p...@peff.net writes: On Fri, Oct 10, 2014 at 02:21:56AM -0400, Jeff King wrote: diff --git a/t/test-lib.sh b/t/test-lib.sh index a60ec75..81ceb23 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -237,7 +237,11 @@ do shift ;; -x) test_eval_start_='set -x' -test_eval_end_='set +x' +test_eval_end_=' +set +x +test $test_eval_ret_ = 0 || + say_color error 4 last command exited with \$?=$? That should be \$?=$test_eval_ret_, of course. The patch below fixes it. Rerolled patch is below. Sorry for all the emails. I'll stop looking at it now to give you guys a chance to find any remaining mistakes. ;) Does 1308 pass with this patch for you (running it without -x)? The original that expects a hardcoded line number (not relative to the original or something) is a bad taste, and also the test setup procedure is broken (see below for a fix of that breakage, which does not fix the breakage this patch seems to bring in anyway). But still it is disturbing to see that there is a blank line difference with and without this change in the file created by the test (i.e. the client of the code this patch touches). t/t1308-config-set.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index ea0bce2..462bb64 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -23,7 +23,7 @@ check_config () { } test_expect_success 'setup default config' ' - cat .git/config \EOF + cat .git/config -\EOF [case] penguin = very blue Movie = BadPhysics -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] test-lib.sh: support -x option for shell-tracing
Junio C Hamano gits...@pobox.com writes: Does 1308 pass with this patch for you (running it without -x)? The original that expects a hardcoded line number (not relative to the original or something) is a bad taste, and also the test setup procedure is broken (see below for a fix of that breakage, which does not fix the breakage this patch seems to bring in anyway). But still it is disturbing to see that there is a blank line difference with and without this change in the file created by the test (i.e. the client of the code this patch touches). This is even more disturbing. With this fix (which is correct as far as I can tell) queued on top of ta/config-set, the shell-tracing patch does not fail any more. I suspect that the broken test in the original ended the .git/config file with an incomplete line or something, and with the attached fix we no longer do so and that is why shell-tracing patch no longer breaks it, it seems. So if there is a test that does want to create a file that ends with an incomplete line, we may see the real breakage again with the shell-tracing patch in. -- 8 -- Subject: [PATCH] t1308: fix broken here document in test script Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t1308-config-set.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index 7fdf840..243d612 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -23,7 +23,7 @@ check_config () { } test_expect_success 'setup default config' ' - cat .git/config \EOF + cat .git/config -\EOF [case] penguin = very blue Movie = BadPhysics @@ -185,7 +185,7 @@ test_expect_success 'proper error on error in default config files' ' cp .git/config .git/config.old test_when_finished mv .git/config.old .git/config echo [ .git/config - echo fatal: bad config file line 35 in .git/config expect + echo fatal: bad config file line 34 in .git/config expect test_expect_code 128 test-config get_value foo.bar 2actual test_cmp expect actual ' -- 2.1.2-468-g1a77c5b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] test-lib.sh: support -x option for shell-tracing
On Mon, Oct 13, 2014 at 03:22:50PM -0700, Junio C Hamano wrote: Rerolled patch is below. Sorry for all the emails. I'll stop looking at it now to give you guys a chance to find any remaining mistakes. ;) Does 1308 pass with this patch for you (running it without -x)? Hmph. It does not. I know that make test passed with an earlier iteration, but I must have gotten so wrapped up in testing make GIT_TEST_OPTS=-x test that I never ran a vanilla make test on what I finally posted. Sorry. The original that expects a hardcoded line number (not relative to the original or something) is a bad taste, and also the test setup procedure is broken (see below for a fix of that breakage, which does not fix the breakage this patch seems to bring in anyway). Yeah, I agree, and your patch below looks reasonable. But still it is disturbing to see that there is a blank line difference with and without this change in the file created by the test (i.e. the client of the code this patch touches). This fixes it: diff --git a/t/test-lib.sh b/t/test-lib.sh index 4dab575..059bb25 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -528,8 +528,7 @@ maybe_setup_valgrind () { test_eval_inner_ () { eval test \$trace\ = t set -x - $* - + $* } test_eval_ () { My patch definitely expands the snippet with an extra trailing newline. But what I really don't understand is why that would impact the _contents_ of the config file. I'll dig further, but I'm about to leave the computer for dinner for a few hours, so please don't hold your breath. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] test-lib.sh: support -x option for shell-tracing
Jeff King p...@peff.net writes: This fixes it: diff --git a/t/test-lib.sh b/t/test-lib.sh index 4dab575..059bb25 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -528,8 +528,7 @@ maybe_setup_valgrind () { test_eval_inner_ () { eval test \$trace\ = t set -x - $* - + $* } test_eval_ () { My patch definitely expands the snippet with an extra trailing newline. But what I really don't understand is why that would impact the _contents_ of the config file. Ahh, OK. The inside of eval begins with cat .git/config \EOF ... and ends with HTEOF, which is _ignored_ because it is not EOF alone on a line, so the shell takes everything, i.e. including the additional LF your dq. No wonder we got one extra line. And with that additional LF fixed, even if we do not fix t1308, it should work (with some definition of work) as before. Thanks, that clears it up. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] test-lib.sh: support -x option for shell-tracing
On Mon, Oct 13, 2014 at 06:33:03PM -0400, Jeff King wrote: But still it is disturbing to see that there is a blank line difference with and without this change in the file created by the test (i.e. the client of the code this patch touches). This fixes it: diff --git a/t/test-lib.sh b/t/test-lib.sh index 4dab575..059bb25 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -528,8 +528,7 @@ maybe_setup_valgrind () { test_eval_inner_ () { eval test \$trace\ = t set -x - $* - + $* } test_eval_ () { My patch definitely expands the snippet with an extra trailing newline. But what I really don't understand is why that would impact the _contents_ of the config file. I'll dig further, but I'm about to leave the computer for dinner for a few hours, so please don't hold your breath. :) OK, I lied. I couldn't resist spending 5 more minutes on it. If you instrument t1308 on master to look at the contents of .git/config directly after the setup step, you'll see that the file ends with (tabs marked as ^I): [...] ^I^Ihorns ^IEOF Which makes sense. We forgot the tab-eating - in the here-doc, so the tab-indented EOF was not counted as the end of the input. So this test is bogus and broken, and the breakage introduced by my patch is only triggered because of that (which isn't to say we shouldn't necessarily adjust my patch, but we definitely should fix this test). What really surprises me is that the shell is fine with a here-doc ending inside an eval. Bash at least warns: $ bash -c eval 'cat EOF content' bash: line 2: warning: here-document at line 1 delimited by end-of-file (wanted `EOF') content but dash silently accepts it: $ dash -c eval 'cat EOF content' content Maybe this is something that every shell does, but it certainly seems like something we should not be relying on (and it was definitely not something the test meant to rely on, as evidenced by the bogus EOF marker it included). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] test-lib.sh: support -x option for shell-tracing
Jeff King p...@peff.net writes: OK, I lied. I couldn't resist spending 5 more minutes on it. If you instrument t1308 on master to look at the contents of .git/config directly after the setup step, you'll see that the file ends with (tabs marked as ^I): [...] ^I^Ihorns ^IEOF Which makes sense. We forgot the tab-eating - in the here-doc, so the tab-indented EOF was not counted as the end of the input. So this test is bogus and broken, and the breakage introduced by my patch is only triggered because of that (which isn't to say we shouldn't necessarily adjust my patch, but we definitely should fix this test). We came to more or less the same conclusion. With your $* fixed, the test works as before, with the same definition of works, because without your patch the file ends with HTEOFLF and with your original $*LFHT the file ends with HTEOFLF with these extra LFHT appended, which was what made me notice, and with $* the file ends with the same HTEOFLF as before. I've queued a fix for the original test on ta/config-set and also amended your $*. What really surprises me is that the shell is fine with a here-doc ending inside an eval. Yup, it smells somewhat mis-feature-ish, doesn't it? Anyway, you do not need to respond to this message in a hurry. Enjoy your dinner ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking summary snapshot
I do not want to do too many What's cooking report, so here is just a summary of the status of various topics. Hopefully I'll do the real one later this week after a few more integrations. Thanks. $ Meta/cook -w Expecting a reroll. - jk/makefile 02-05 #16 - bg/rebase-off-of-previous-branch 04-16 #1 - jk/tag-contains 06-30 #8 - jk/prune-mtime 10-04 #18 Expecting an Ack/Sign-off or update from Jonathan on the bottom one. - jn/parse-config-slot 10-07 #2 Expecting the final reroll. - rs/ref-transaction 09-10 #19 Stalled - jc/graph-post-root-gap 12-30 #3 - jn/gitweb-utf8-in-links 05-27 #1 - ss/userdiff-update-csharp-java 06-02 #2 - hv/submodule-config 06-30 #4 - jk/pack-bitmap 08-04 #1 - jt/timer-settime 08-29 #6 Undecided - nd/multiple-work-trees 09-27 #32 - da/mergetool-tool-help 10-13 #4 - eb/no-pthreads 10-13 #2 - cc/interpret-trailers10-13 #11 Waiting for a reroll ($gmane/256591). - tr/remerge-diff 09-08 #8 Waiting for a reroll. - rb/merge-prepare-commit-msg-hook 01-10 #4 - ab/add-interactive-show-diff-func-name 05-12 #2 Waiting for an Ack. - je/quiltimport-no-fuzz 09-26 #2 Waiting for the final step to lift the hard-limit before sending it out. - jc/show-branch 03-24 #5 Will hold. - tg/perf-lib-test-perf-cleanup09-19 #2 Will merge to 'master'. + da/include-compat-util-first-in-c09-15/10-07 #1 + so/rebase-doc-fork-point 09-29/10-07 #1 + dt/cache-tree-repair 09-30/10-07 #1 + rs/daemon-fixes 10-01/10-07 #3 + da/completion-show-signature 10-07/10-07 #1 + sk/tag-contains-wo-recursion 09-23/10-08 #1 + mh/lockfile-stdio10-01/10-08 #3 + rs/sha1-array-test 10-01/10-08 #2 + mh/lockfile 10-01/10-08 #38 + rs/mailsplit 10-07/10-08 #1 + rs/more-uses-of-skip-prefix 10-07/10-08 #1 + rs/plug-leak-in-bundle 10-07/10-08 #1 + bc/asciidoc-pretty-formats-fix 10-08/10-13 #1 + po/everyday-doc 10-10/10-13 #3 Will merge to 'next'. - bw/trace-no-inline-getnanotime 09-29 #1 - jc/completion-no-chdir 10-09 #1 - bc/asciidoc 10-13 #2 - jk/test-shell-trace 10-13 #3 - rs/receive-pack-argv-leak-fix10-13 #1 - ta/config-set10-13 #1 - da/mergetool-temporary-filename 10-13 #2 Will perhaps drop. - mt/patch-id-stable 06-10 #1 - jc/push-cert-hmac-optim 09-25 #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 0/4] Allow building Git with Asciidoctor
On Mon, Oct 13, 2014 at 01:41:31PM -0700, Junio C Hamano wrote: brian m. carlson sand...@crustytoothpaste.net writes: The second two patches implement some basic support for building with Asciidoctor. The first of these moves some items into variables due to some differences between the AsciiDoc and Asciidoctor command lines. The user can then override these values when invoking make. The final patch adds support for the linkgit macro. Asciidoctor uses Ruby extensions to implement macro support, unlike AsciiDoc, which uses a configuration file. What I do not understand is that 3/4 lets you drop inclusion of asciidoc.conf which contains a lot more than just linkgit: definition. Asciidoctor just doesn't understand the -f argument, so trying to pass it is going to fail. For Asciidoctor, you're going to want to do something like -I. -rasciidoctor/extensions -rextensions there instead. As for the rest of the asciidoc.conf file, the DocBook manpage header declarations are implemented automatically by Asciidoctor after my recent patches. The paragraph hacks do not appear to be necessary with Asciidoctor, so they've been omitted. That leaves the attributes. All but litdd are built-in to Asciidoctor, and I can reroll with a modification to extensions.rb that implements that one. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH v2 3/3] test-lib.sh: support -x option for shell-tracing
On Mon, Oct 13, 2014 at 04:14:59PM -0700, Junio C Hamano wrote: We came to more or less the same conclusion. With your $* fixed, the test works as before, with the same definition of works, because without your patch the file ends with HTEOFLF and with your original $*LFHT the file ends with HTEOFLF with these extra LFHT appended, which was what made me notice, and with $* the file ends with the same HTEOFLF as before. Yeah. I think the fact that fixing the test to properly respect EOF required you to later change the line number is a good indication that the test was broken in the first place. :) I've queued a fix for the original test on ta/config-set and also amended your $*. Thanks. Note that my patch still technically adds a tab in front of the $*. I can't imagine that would matter, but if we wanted to be extra conservative, we would make it: eval test ... set -x $* with no indentation at all. Enjoy your dinner ;-) Thanks, I did. :) -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] remote.c - Make remote definition require a url
On 10/13/2014 01:19 PM, Junio C Hamano wrote: Mark Levedahl mleved...@gmail.com writes: Some options may be configured globally for a remote (e.g, tagopt). Or some remotes may have only pushurl and not url. git remote output for me has a few such remotes but wouldn't this patch break it? If a caller that walks the list of remotes misbehaves only because it assumes that r-url always is always valid, isn't that assumption what needs to be fixed? for_each_remote() should be kept as a way to enumerate all the [remote foo], I would think. As long as the rule is that for_each_remote will enumerate every remote that has anything defined at all, even if only in the global config outside of a user's control, I'm not really sure how to tell whether the missing url / pushurl / whatever is intentional, or a misconfiguration, so having the code complain that it didn't find what it wanted (the current condition) is probably no worse than the alternatives. Patch withdrawn. Mark -- To unsubscribe from this list: send the line 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] Initialise hash variable to prevent compiler warnings
On Mon, Oct 13, 2014 at 2:53 PM, Felipe Franciosi fel...@paradoxo.org wrote: On Mon, Oct 13, 2014 at 9:12 PM, Junio C Hamano gits...@pobox.com wrote: FNV/I/IDIV10/0 covers all the possibilities of (method 3), I would have to say that the compiler needs to be fixed. Or insert default: just before case HASH_METHOD_0: line? I dunno. Hmm... The default: would work, but is it really that bad to initialise a local variable in this case? In any case, the compilation warning is annoying. Do you prefer the default or the initialisation? If I really had to choose between the two, adding a useless initialization would be the less harmful choice. Adding a meaningless default: robs another chance from the compilers to diagnose a future breakage we might add (namely, we may extend methods and forget to write a corresponding case arm for the new method value, which a smart compiler can and do diagnose as a switch that does not handle all the possible values. 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] git-prompt.sh: Hide prompt for ignored pwd
Set __git_ps1 to display nothing when present working directory is ignored, triggered by either the new environmental variable GIT_PS1_HIDE_ON_IGNORED_PWD or the new repository configuration variable bash.hideOnIgnoredPwd (or both). In the absence of these settings this change has no effect. Many people manage e.g. dotfiles in their home directory with git. This causes the prompt generated by __git_ps1 to refer to that top level repo while working in any descendant directory. That can be distracting, so this patch helps one shut off that noise. Signed-off-by: Jess Austin jess.aus...@gmail.com --- On Thu, Oct 9, 2014 at 5:09 PM, Richard Hansen rhan...@bbn.com wrote: On 2014-10-09 06:27, Jess Austin wrote: Would you want this configured in each repo (i.e. via a line in .git/config), or would you prefer something global so that it only need be set in one place? I'm not sure how the latter technique would work, so if that seems better please advise on how to go about that. A 'git config' variable is fine. The bash.showDirtyState, bash.showUntrackedFiles, and bash.showUpstream config variables seem like good examples to follow. I think this is what you meant. I changed the name of the envvar. Now the variables are GIT_PS1_HIDE_ON_IGNORED_PWD and bash.hideOnIgnoredPwd. I admit these are still kind of unwieldy, but maybe now they're more descriptive? Please advise! cheers, Jess contrib/completion/git-prompt.sh | 12 t/t9903-bash-prompt.sh | 42 2 files changed, 54 insertions(+) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index c5473dc..d7559ff 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -84,6 +84,11 @@ # GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on # the colored output of git status -sb and are available only when # using __git_ps1 for PROMPT_COMMAND or precmd. +# +# If you would like __git_ps1 to do nothing in the case when the current +# directory is set up to be ignored by git, then set +# GIT_PS1_HIDE_ON_IGNORED_PWD to a nonempty value, or set +# bash.hideOnIgnoredPwd to true in the repository configuration. # check whether printf supports -v __git_printf_supports_v= @@ -501,6 +506,13 @@ __git_ps1 () local f=$w$i$s$u local gitstring=$c$b${f:+$z$f}$r$p + if [ -n $(git check-ignore .) ] + ( [ -n ${GIT_PS1_HIDE_ON_IGNORED_PWD} ] || +[ $(git config --bool bash.hideOnIgnoredPwd) = true ] ) + then + printf_format= + fi + if [ $pcmode = yes ]; then if [ ${__git_printf_supports_v-} != yes ]; then gitstring=$(printf -- $printf_format $gitstring) diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index 9150984..a8ef8a3 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -35,6 +35,8 @@ test_expect_success 'setup for prompt tests' ' git commit -m another b2 file echo 000 file git commit -m yet another b2 file + mkdir ignored_dir + echo ignored_dir/ .gitignore git checkout master ' @@ -588,4 +590,44 @@ test_expect_success 'prompt - zsh color pc mode' ' test_cmp expected $actual ' +test_expect_success 'prompt - hide on ignored pwd - shell variable unset with config disabled' ' + printf (master) expected + ( + cd ignored_dir + __git_ps1 $actual + ) + test_cmp expected $actual +' + +test_expect_success 'prompt - hide on ignored pwd - shell variable unset with config enabled' ' + printf expected + test_config bash.hideOnIgnoredPwd true + ( + cd ignored_dir + __git_ps1 $actual + ) + test_cmp expected $actual +' + +test_expect_success 'prompt - hide on ignored pwd - shell variable set with config disabled' ' + printf expected + ( + cd ignored_dir + GIT_PS1_HIDE_ON_IGNORED_PWD=y + __git_ps1 $actual + ) + test_cmp expected $actual +' + +test_expect_success 'prompt - hide on ignored pwd - shell variable set with config enabled' ' + printf expected + test_config bash.hideOnIgnoredPwd true + ( + cd ignored_dir + GIT_PS1_HIDE_ON_IGNORED_PWD=y + __git_ps1 $actual + ) + test_cmp expected $actual +' + test_done -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
You are sending a virus mail
You sent a virus mail, please check you computer for virus. Mail header info: From: git@vger.kernel.org To: yanhua.d...@wolong.com Subject: Returned mail: see transcript for details Date: Tue, 14 Oct 2014 11:32:19 +0700 N�r��yb�X��ǧv�^�){.n�+ا���ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf
You are sending a virus mail
You sent a virus mail, please check you computer for virus. Mail header info: From: git@vger.kernel.org To: yanhua.d...@wolong.com Subject: Returned mail: see transcript for details Date: Tue, 14 Oct 2014 11:32:19 +0700 N�r��yb�X��ǧv�^�){.n�+ا���ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf
Re: [PATCH RFC] git-am: support any number of signatures
From: Junio C Hamano gits...@pobox.com Christian Couder christian.cou...@gmail.com writes: On Sun, Oct 12, 2014 at 11:36 AM, Christian Couder christian.cou...@gmail.com wrote: With v16 you can easily choose if you want to have the S-o-b in the output or not, when there is already one, ... By the way, I sent v16 just before the above email, but the series still hasn't hit the mailing list. Did some of you guys in cc receive something? I see them and picked them up to replace. Thanks! Are these now ready for 'next'? Yeah, I think so. Thanks, Christian. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html