Re: msysgit works on wine

2014-10-13 Thread Michael Stefaniuc
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

2014-10-13 Thread Dun Peal
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

2014-10-13 Thread Felipe Franciosi
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

2014-10-13 Thread Etienne Buira
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

2014-10-13 Thread Etienne Buira
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

2014-10-13 Thread Jonathan Nieder
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

2014-10-13 Thread Andreas Schwab
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread John Norris

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

2014-10-13 Thread Crabtree, Andrew
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

2014-10-13 Thread Christian Couder
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

2014-10-13 Thread Christian Couder
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

2014-10-13 Thread Christian Couder
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

2014-10-13 Thread Christian Couder
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

2014-10-13 Thread Christian Couder
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

2014-10-13 Thread Christian Couder
[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

2014-10-13 Thread Christian Couder
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

2014-10-13 Thread Christian Couder
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

2014-10-13 Thread Christian Couder
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'

2014-10-13 Thread Christian Couder
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'

2014-10-13 Thread Christian Couder
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

2014-10-13 Thread Christian Couder
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Etienne Buira
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Fabian Ruch
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

2014-10-13 Thread Konstantin Khomoutov
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()

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Jeff King
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

2014-10-13 Thread Jeff King
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

2014-10-13 Thread Johannes Schindelin
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

2014-10-13 Thread Jonathan Nieder
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Jeff King
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

2014-10-13 Thread Jonathan Nieder
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

2014-10-13 Thread Felipe Franciosi
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Jeff King
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Jeff King
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread brian m. carlson
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

2014-10-13 Thread Jeff King
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

2014-10-13 Thread Mark Levedahl

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

2014-10-13 Thread Junio C Hamano
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

2014-10-13 Thread Jess Austin
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

2014-10-13 Thread postmaster
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

2014-10-13 Thread postmaster
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

2014-10-13 Thread Christian Couder
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