Re: inotify support, nearly there

[Duy Nguyen]

On Wed, Jan 29, 2014 at 2:44 PM, Mike Hommey m...@glandium.org wrote:
 Haven't looked at the code, so I don't know if you've done that, but in
 case you haven't, it would be nice to have an environment variable or a
 config option to make git use the file-watcher *and* normal lstat
 operations, to check consistency.

No I haven't. So far I depend on my manual tests and the test suite to
check for consistency. Will do.
-- 
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

commit-msg hook and merges

[Søren Holm]

Hi. 

I'm running a speciallized commit-msg hook to help me fill out commit 
messages. This all works nicely for alle commits except for merges.

What I normally do to circumvent this is this :

$ git merge somebranch

here I append to autogenerated message with my own text

$ git commit --ammend

I just save the message without changes and the commit-message is run 
throught my filter


Is that intentional or an error ?

-- 
Søren Holm
--
To unsubscribe from this list: send the line 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] Makefile: add cppcheck target

[Elia Pinto]

Add cppcheck target to Makefile. Cppcheck is a static
analysis tool for C/C++ code. Cppcheck primarily detects
the types of bugs that the compilers normally do not detect.
It is an useful target for doing QA analysis.

Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
 Makefile  |6 ++
 config.mak.in |1 +
 2 files changed, 7 insertions(+)

diff --git a/Makefile b/Makefile
index dddaf4f..1d25a70 100644
--- a/Makefile
+++ b/Makefile
@@ -2602,3 +2602,9 @@ cover_db: coverage-report
 cover_db_html: cover_db
cover -report html -outputdir cover_db_html cover_db
 
+### cppcheck static coverage analysis
+#
+.PHONY: cppcheck
+
+cppcheck:
+   cppcheck --enable=all -q $(top_srcdir)
diff --git a/config.mak.in b/config.mak.in
index e6a6d0f..86b95fb 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -22,3 +22,4 @@ docdir = @docdir@
 
 mandir = @mandir@
 htmldir = @htmldir@
+top_srcdir = @top_srcdir@
-- 
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 3/4] combine-diff: Optimize combine_diff_path sets intersection

[Kirill Smelkov]

On Tue, Jan 28, 2014 at 01:55:09PM -0800, Junio C Hamano wrote:
 Kirill Smelkov k...@mns.spb.ru writes:
 
  diff --git a/combine-diff.c b/combine-diff.c
  index 3b92c448..98c2562 100644
  --- a/combine-diff.c
  +++ b/combine-diff.c
  @@ -15,8 +15,8 @@
  ...
  +   while (1) {
  ...
  +   if (cmp  0) {
  +   if (pprev)
  +   pprev-next = p-next;
  +   ptmp = p;
  +   p = p-next;
  +   free(ptmp);
  +   if (curr == ptmp)
  +   curr = p;
  continue;
  ...
  +   if (cmp  0) {
  +   i++;
  +   continue;
  }
  ...
  +
  +   pprev = p;
  +   p = p-next;
  +   i++;
  }
  return curr;
   }
 
 Thanks. I very much like the approach.
 
 I was staring at the above part of the code, but couldn't help
 recalling this gem (look for understand pointers in the article):
 
   
 http://meta.slashdot.org/story/12/10/11/0030249/linus-torvalds-answers-your-questions
 
 How about doing it this way (on top of your patch)?  It reduces 7
 lines even though it adds two comment lines ;-)
 
  combine-diff.c | 37 +++--
  1 file changed, 15 insertions(+), 22 deletions(-)

Thanks, this is sound approach and adding guiding comments is good, and
also now some of us with self-taught heritage understand (or at least
they think so) pointers a bit better :)

Now some nitpicks:

 diff --git a/combine-diff.c b/combine-diff.c
 index 2d79312..0809e79 100644
 --- a/combine-diff.c
 +++ b/combine-diff.c
 @@ -15,11 +15,10 @@
  static struct combine_diff_path *intersect_paths(struct combine_diff_path 
 *curr, int n, int num_parent)
  {
   struct diff_queue_struct *q = diff_queued_diff;
 - struct combine_diff_path *p, *pprev, *ptmp;
 + struct combine_diff_path *p, **tail = curr;
   int i, cmp;
  
   if (!n) {
 - struct combine_diff_path *list = NULL, **tail = list;
   for (i = 0; i  q-nr; i++) {
   int len;
   const char *path;
 @@ -43,35 +42,30 @@ static struct combine_diff_path *intersect_paths(struct 
 combine_diff_path *curr,
   *tail = p;
   tail = p-next;
   }
 - return list;
 + return curr;
   }
  
   /*
 -  * NOTE paths are coming sorted here (= in tree order)
 +  * paths in curr (linked list) and q-queue[] (array) are
 +  * both sorted in the tree order.
*/
 -
 - pprev = NULL;
 - p = curr;
   i = 0;
 + while ((p = *tail) != NULL) {
 + cmp = ((i = q-nr)
 +? -1 : strcmp(p-path, q-queue[i]-two-path));

I liked cmp assignment being the original way - when -1 is on one line
and strcmp is on another - to me it reads better. I'm not insisting on
it though.


 - while (1) {
 - if (!p)
 - break;
 -
 - cmp = (i = q-nr) ? -1
 -: strcmp(p-path, q-queue[i]-two-path);
   if (cmp  0) {
 - if (pprev)
 - pprev-next = p-next;
 - ptmp = p;
 - p = p-next;
 - free(ptmp);
 - if (curr == ptmp)
 - curr = p;
 + /* p-path not in q-queue[]; drop it */
 + struct combine_diff_path *next = p-next;
 +
 + if ((*tail = next) != NULL)
 + tail = next-next;
 + free(p);
   continue;
   }

A bug crept in here - if we are removing the first element, i.e. when
p=curr, we have to advance curr as well - as we are returning curr back
as new intersected paths set list start. That's why there was curr
change.

Now curr stays the same, and if we'll remove the first element, curr
will be pointing to freed memory - oops. A possible fixup could be:

 8 
diff --git a/combine-diff.c b/combine-diff.c
index 0809e79..6a61a25 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -60,6 +60,8 @@ static struct combine_diff_path *intersect_paths(struct 
combine_diff_path *curr,
 
if ((*tail = next) != NULL)
tail = next-next;
+   if (curr == p)
+   curr = next;
free(p);
continue;
}
 8 

but this is blind code, as I had not tested it.


  
   if (cmp  0) {
 + /* q-queue[i] not in p-path; skip it */
   i++;
   continue;
   }
 @@ -80,8 +74,7 @@ static struct combine_diff_path *intersect_paths(struct 
 combine_diff_path *curr,
   p-parent[n].mode = 

git clone on out-of-space device causes incorrect errors

[Armin Ronacher]

Hi,

When cloning onto a device that is out of space to fulfill the whole clone 
operation, git will report that the remove repository does not exist:


$ git clone https://github.com/mozilla/flask
Cloning into 'flask'...
remote: Repository not found.
fatal: repository 'https://github.com/mozilla/flask/' not found

This error is misleading.  As an example here is what hg does for the same 
situation:


$ hg clone https://bitbucket.org/dholth/wheel
destination directory: wheel
requesting all changes
adding changesets
adding manifests
adding file changes
transaction abort!
failed to truncate 00changelog.i
rollback failed - please run hg recover
abort: No space left on device: /Volumes/Disk 
Image/wheel/.hg/store/.fncache-bt6dzr


Regards,
Armin
--
To unsubscribe from this list: send the line 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] archive.c: reduce scope of variables

[Elia Pinto]

Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
 archive.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/archive.c b/archive.c
index 346f3b2..49b79f8 100644
--- a/archive.c
+++ b/archive.c
@@ -112,7 +112,6 @@ static int write_archive_entry(const unsigned char *sha1, 
const char *base,
write_archive_entry_fn_t write_entry = c-write_entry;
struct git_attr_check check[2];
const char *path_without_prefix;
-   int err;
 
args-convert = 0;
strbuf_reset(path);
@@ -132,6 +131,7 @@ static int write_archive_entry(const unsigned char *sha1, 
const char *base,
}
 
if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
+int err;
if (args-verbose)
fprintf(stderr, %.*s\n, (int)path.len, path.buf);
err = write_entry(args, sha1, path.buf, path.len, mode);
@@ -319,7 +319,6 @@ static int parse_archive_args(int argc, const char **argv,
const char *output = NULL;
int compression_level = -1;
int verbose = 0;
-   int i;
int list = 0;
int worktree_attributes = 0;
struct option opts[] = {
@@ -366,6 +365,7 @@ static int parse_archive_args(int argc, const char **argv,
base = ;
 
if (list) {
+int i;
for (i = 0; i  nr_archivers; i++)
if (!is_remote || archivers[i]-flags  ARCHIVER_REMOTE)
printf(%s\n, archivers[i]-name);
-- 
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

[PATCH] bisect.c: reduce scope of variable

[Elia Pinto]

Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
 bisect.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bisect.c b/bisect.c
index 37200b4..8448d27 100644
--- a/bisect.c
+++ b/bisect.c
@@ -685,7 +685,6 @@ static void mark_expected_rev(char *bisect_rev_hex)
 
 static int bisect_checkout(char *bisect_rev_hex, int no_checkout)
 {
-   int res;
 
mark_expected_rev(bisect_rev_hex);
 
@@ -696,6 +695,7 @@ static int bisect_checkout(char *bisect_rev_hex, int 
no_checkout)
die(update-ref --no-deref HEAD failed on %s,
bisect_rev_hex);
} else {
+   int res;
res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
if (res)
exit(res);
-- 
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

[PATCH 2/2] builtin/apply.c: reduce scope of variables

[Elia Pinto]

Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
 builtin/apply.c |   12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index b0d0986..a7e72d5 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1943,13 +1943,7 @@ static int parse_chunk(char *buffer, unsigned long size, 
struct patch *patch)
   size - offset - hdrsize, patch);
 
if (!patchsize) {
-   static const char *binhdr[] = {
-   Binary files ,
-   Files ,
-   NULL,
-   };
static const char git_binary[] = GIT binary patch\n;
-   int i;
int hd = hdrsize + offset;
unsigned long llen = linelen(buffer + hd, size - hd);
 
@@ -1965,6 +1959,12 @@ static int parse_chunk(char *buffer, unsigned long size, 
struct patch *patch)
patchsize = 0;
}
else if (!memcmp( differ\n, buffer + hd + llen - 8, 8)) {
+   static const char *binhdr[] = {
+   Binary files ,
+   Files ,
+   NULL,
+   };
+   int i;
for (i = 0; binhdr[i]; i++) {
int len = strlen(binhdr[i]);
if (len  size - hd 
-- 
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

[PATCH 3/3] builtin/blame.c: reduce scope of variables

[Elia Pinto]

Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
 builtin/blame.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index e44a6bb..967a7c6 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1580,14 +1580,14 @@ static const char *format_time(unsigned long time, 
const char *tz_str,
   int show_raw_time)
 {
static char time_buf[128];
-   const char *time_str;
-   int time_len;
-   int tz;
 
if (show_raw_time) {
snprintf(time_buf, sizeof(time_buf), %lu %s, time, tz_str);
}
else {
+   const char *time_str;
+   int time_len;
+   int tz;
tz = atoi(tz_str);
time_str = show_date(time, tz, blame_date_mode);
time_len = strlen(time_str);
-- 
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

[PATCH 4/4] builtin/clean.c: reduce scope of variable

[Elia Pinto]

Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
 builtin/clean.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 2f26297..a1f8969 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -154,7 +154,7 @@ static int remove_dirs(struct strbuf *path, const char 
*prefix, int force_flag,
DIR *dir;
struct strbuf quoted = STRBUF_INIT;
struct dirent *e;
-   int res = 0, ret = 0, gone = 1, original_len = path-len, len, i;
+   int res = 0, ret = 0, gone = 1, original_len = path-len, len;
unsigned char submodule_head[20];
struct string_list dels = STRING_LIST_INIT_DUP;
 
@@ -242,6 +242,7 @@ static int remove_dirs(struct strbuf *path, const char 
*prefix, int force_flag,
}
 
if (!*dir_gone  !quiet) {
+   int i;
for (i = 0; i  dels.nr; i++)
printf(dry_run ?  _(msg_would_remove) : _(msg_remove), 
dels.items[i].string);
}
-- 
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

[PATCH 5/5] builtin/commit.c: reduce scope of variables

[Elia Pinto]

Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
 builtin/commit.c |   11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 3767478..eea4421 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -307,7 +307,6 @@ static char *prepare_index(int argc, const char **argv, 
const char *prefix,
int fd;
struct string_list partial;
struct pathspec pathspec;
-   char *old_index_env = NULL;
int refresh_flags = REFRESH_QUIET;
 
if (is_status)
@@ -320,6 +319,7 @@ static char *prepare_index(int argc, const char **argv, 
const char *prefix,
die(_(index file corrupt));
 
if (interactive) {
+   char *old_index_env = NULL;
fd = hold_locked_index(index_lock, 1);
 
refresh_cache_or_die(refresh_flags);
@@ -600,12 +600,10 @@ static int prepare_to_commit(const char *index_file, 
const char *prefix,
 {
struct stat statbuf;
struct strbuf committer_ident = STRBUF_INIT;
-   int commitable, saved_color_setting;
+   int commitable;
struct strbuf sb = STRBUF_INIT;
-   char *buffer;
const char *hook_arg1 = NULL;
const char *hook_arg2 = NULL;
-   int ident_shown = 0;
int clean_message_contents = (cleanup_mode != CLEANUP_NONE);
int old_display_comment_prefix;
 
@@ -649,6 +647,7 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
  logfile);
hook_arg1 = message;
} else if (use_message) {
+   char *buffer;
buffer = strstr(use_message_buffer, \n\n);
if (!use_editor  (!buffer || buffer[2] == '\0'))
die(_(commit has empty message));
@@ -753,6 +752,8 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
/* This checks if committer ident is explicitly given */
strbuf_addstr(committer_ident, git_committer_info(IDENT_STRICT));
if (use_editor  include_status) {
+   int ident_shown = 0;
+   int saved_color_setting;
char *ai_tmp, *ci_tmp;
if (whence != FROM_COMMIT)
status_printf_ln(s, GIT_COLOR_NORMAL,
@@ -1510,7 +1511,6 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
struct ref_lock *ref_lock;
struct commit_list *parents = NULL, **pptr = parents;
struct stat statbuf;
-   int allow_fast_forward = 1;
struct commit *current_head = NULL;
struct commit_extra_header *extra = NULL;
 
@@ -1576,6 +1576,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
}
fclose(fp);
strbuf_release(m);
+   int allow_fast_forward = 1;
if (!stat(git_path(MERGE_MODE), statbuf)) {
if (strbuf_read_file(sb, git_path(MERGE_MODE), 0)  
0)
die_errno(_(could not read MERGE_MODE));
-- 
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

[PATCH 6/6] builtin/fetch.c: reduce scope of variable

[Elia Pinto]

Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
 builtin/fetch.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 025bc3e..55f457c 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1026,7 +1026,6 @@ static int fetch_multiple(struct string_list *list)
 
 static int fetch_one(struct remote *remote, int argc, const char **argv)
 {
-   int i;
static const char **refs = NULL;
struct refspec *refspec;
int ref_nr = 0;
@@ -1050,6 +1049,7 @@ static int fetch_one(struct remote *remote, int argc, 
const char **argv)
 
if (argc  0) {
int j = 0;
+   int i;
refs = xcalloc(argc + 1, sizeof(const char *));
for (i = 0; i  argc; i++) {
if (!strcmp(argv[i], tag)) {
-- 
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

[PATCH 7/7] builtin/gc.c: reduce scope of variables

[Elia Pinto]

Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
 builtin/gc.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index c19545d..5bbb5e3 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -188,13 +188,12 @@ static int need_to_gc(void)
 static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
 {
static struct lock_file lock;
-   static char locking_host[128];
char my_host[128];
struct strbuf sb = STRBUF_INIT;
struct stat st;
uintmax_t pid;
FILE *fp;
-   int fd, should_exit;
+   int fd;
 
if (pidfile)
/* already locked */
@@ -206,6 +205,8 @@ static const char *lock_repo_for_gc(int force, pid_t* 
ret_pid)
fd = hold_lock_file_for_update(lock, git_path(gc.pid),
   LOCK_DIE_ON_ERROR);
if (!force) {
+   static char locking_host[128];
+   int should_exit;
fp = fopen(git_path(gc.pid), r);
memset(locking_host, 0, sizeof(locking_host));
should_exit =
-- 
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] repack.c: Use move_temp_to_file() instead of rename()

[Junio C Hamano]

Torsten Bögershausen tbo...@web.de writes:

 In a1bbc6c0 a shell command mv -f was replaced with the rename() function.

 Use move_temp_to_file() from sha1_file.c instead of rename().
 This is in line with the handling of other Git internal tmp files,
 and calls adjust_shared_perm()


 Signed-off-by: Torsten Bögershausen tbo...@web.de
 ---
 Thanks for all comments.
 I haven't been able to reproduce the problem here.
 Tips and information how to reproduce it are wellcome.

 I think this patch makes sense to support core.sharedRepository(),
 but I haven't made a test case for the pack/idx files.

 Jochen, doess this patch fix your problem, or do we need
 another patch on top of this?

Turning the call to rename(2) in the last hunk in your patch to
move_temp_to_file() may be an improvement, but the other changes are
wrong, I think.

There are two big differences between the rename(2) used here and
move_temp_to_file().  You need to understand what move_temp_to_file()
is from its name.

 - It is meant to move a temporary file we create ourselves, and the
   most important characteristic of such a file is that we do not
   create it read-only.  The function uses platform's rename(2) and
   returns a failure if the platform's rename(2) fails, but that is
   not a problem even on Windows because of this.

 - When returning a failure, it unlinks the temporary file.  This
   unlinking is necessary to clean up after our half-done mess.

The other two calls to rename(2) you changed in the patch are *not*
about turning a newly created temporary file into the final one.
They are about moving existing packfiles away just in case we fail
in the later phases of the command so that they can be moved back to
their original name to restore the state before we started, and
actually moving them back to their original name.  We do not want to
remove these files.  The platform's rename(2) must do the right
thing in these two calls and there is no guarantee these existing
packfiles are read-write.

Earlier, I said that turning the call to rename(2) in the last hunk
in your patch to move_temp_to_file() may be an improvement, because
I thought that it is a good thing that the latter makes a call to
adjust_shared_perm().

But after a closer inspection, I no longer think that hunk is an
improvement.  These new packfiles were created by pack-objects,
which finishes each packfile it produces by calling
finish_tmp_packfile(), and that is where adjust_shared_perm()
happens already.  As far as pack-objects that was called from
repack is concerned, these new packfiles are not temporary; they
are finished product.  It may be OK to remove them as part of
rewind back to the original state, as a later phase of repack
failed if we saw a failure (but note that the original
git-repack.sh didn't), but a plain vanilla rename(2) without any
frills is what we want to happen to them.

  builtin/repack.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/builtin/repack.c b/builtin/repack.c
 index ba66c6e..4b6d663 100644
 --- a/builtin/repack.c
 +++ b/builtin/repack.c
 @@ -271,7 +271,7 @@ int cmd_repack(int argc, const char **argv, const char 
 *prefix)
   if (unlink(fname_old))
   failed = 1;
  
 - if (!failed  rename(fname, fname_old)) {
 + if (!failed  move_temp_to_file(fname, fname_old)) {
   free(fname);
   failed = 1;
   break;
 @@ -288,7 +288,7 @@ int cmd_repack(int argc, const char **argv, const char 
 *prefix)
   char *fname, *fname_old;
   fname = mkpathdup(%s/%s, packdir, item-string);
   fname_old = mkpath(%s/old-%s, packdir, item-string);
 - if (rename(fname_old, fname))
 + if (move_temp_to_file(fname_old, fname))
   string_list_append(rollback_failure, fname);
   free(fname);
   }
 @@ -324,7 +324,7 @@ int cmd_repack(int argc, const char **argv, const char 
 *prefix)
   statbuffer.st_mode = ~(S_IWUSR | S_IWGRP | 
 S_IWOTH);
   chmod(fname_old, statbuffer.st_mode);
   }
 - if (rename(fname_old, fname))
 + if (move_temp_to_file(fname_old, fname))
   die_errno(_(renaming '%s' failed), fname_old);
   free(fname);
   free(fname_old);
 -- 
 1.8.5.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] Makefile: add cppcheck target

[Junio C Hamano]

Elia Pinto gitter.spi...@gmail.com writes:

 Add cppcheck target to Makefile. Cppcheck is a static
 analysis tool for C/C++ code. Cppcheck primarily detects
 the types of bugs that the compilers normally do not detect.
 It is an useful target for doing QA analysis.

 Signed-off-by: Elia Pinto gitter.spi...@gmail.com
 ---
  Makefile  |6 ++
  config.mak.in |1 +
  2 files changed, 7 insertions(+)

 diff --git a/Makefile b/Makefile
 index dddaf4f..1d25a70 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -2602,3 +2602,9 @@ cover_db: coverage-report
  cover_db_html: cover_db
   cover -report html -outputdir cover_db_html cover_db
  
 +### cppcheck static coverage analysis
 +#
 +.PHONY: cppcheck
 +
 +cppcheck:
 + cppcheck --enable=all -q $(top_srcdir)

Why isn't this .?

In other words, why is the change to config.mak.in even necessary?

 diff --git a/config.mak.in b/config.mak.in
 index e6a6d0f..86b95fb 100644
 --- a/config.mak.in
 +++ b/config.mak.in
 @@ -22,3 +22,4 @@ docdir = @docdir@
  
  mandir = @mandir@
  htmldir = @htmldir@
 +top_srcdir = @top_srcdir@

--
To unsubscribe from this list: send the line 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] archive.c: reduce scope of variables

[Junio C Hamano]

Elia Pinto gitter.spi...@gmail.com writes:

 Signed-off-by: Elia Pinto gitter.spi...@gmail.com
 ---

Either the patch is whitespace damaged during the mail transport, or
you are incorrectly indenting the lines with all spaces.

  archive.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/archive.c b/archive.c
 index 346f3b2..49b79f8 100644
 --- a/archive.c
 +++ b/archive.c
 @@ -112,7 +112,6 @@ static int write_archive_entry(const unsigned char *sha1, 
 const char *base,
   write_archive_entry_fn_t write_entry = c-write_entry;
   struct git_attr_check check[2];
   const char *path_without_prefix;
 - int err;
  
   args-convert = 0;
   strbuf_reset(path);
 @@ -132,6 +131,7 @@ static int write_archive_entry(const unsigned char *sha1, 
 const char *base,
   }
  
   if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
 +int err;
   if (args-verbose)
   fprintf(stderr, %.*s\n, (int)path.len, path.buf);
   err = write_entry(args, sha1, path.buf, path.len, mode);
 @@ -319,7 +319,6 @@ static int parse_archive_args(int argc, const char **argv,
   const char *output = NULL;
   int compression_level = -1;
   int verbose = 0;
 - int i;
   int list = 0;
   int worktree_attributes = 0;
   struct option opts[] = {
 @@ -366,6 +365,7 @@ static int parse_archive_args(int argc, const char **argv,
   base = ;
  
   if (list) {
 +int i;
   for (i = 0; i  nr_archivers; i++)
   if (!is_remote || archivers[i]-flags  ARCHIVER_REMOTE)
   printf(%s\n, archivers[i]-name);
--
To unsubscribe from this list: send the line 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] diff: turn skip_stat_unmatch on selectively

[Junio C Hamano]

Duy Nguyen pclo...@gmail.com writes:

 On Tue, Jan 28, 2014 at 02:51:45PM -0800, Junio C Hamano wrote:
   This replaces 'diff: turn off skip_stat_unmatch on diff --cached'
   The previous patch obviously leaves skip_stat_unmatch on in diff
   rev rev and maybe other cases.
 
  Oops, I lost track.  Sorry.
 
 Together with {1,2}/3 applied on maint-1.8.4, this sems to break
 t3417 (there may be others, but I didn't have time to check).

 My bad. I thought I covered all cases in my last patch (and didn't
 retest it!). It turns out I should have set skip_stat_unmatch in
 builtin_diff_b_f() too. This on top of 3/3 passes the tests

Thanks, will squash it in.

This however shows that the existing test *KNEW* that it was enough
to check just a few cases (especially, there is no reason to make
sure that blob vs file-in-working-tree case behaves sanely), because
the auto-refresh would kick in for all codepaths.  Now you are
making that assumption invalid, shouldn't the patch also split the
tests to cover individual cases?

 -- 8 --
 diff --git a/builtin/diff.c b/builtin/diff.c
 index 88542d9..8ab5e3d 100644
 --- a/builtin/diff.c
 +++ b/builtin/diff.c
 @@ -89,6 +89,7 @@ static int builtin_diff_b_f(struct rev_info *revs,
   if (blob[0].mode == S_IFINVALID)
   blob[0].mode = canon_mode(st.st_mode);
  
 + revs-diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
   stuff_change(revs-diffopt,
blob[0].mode, canon_mode(st.st_mode),
blob[0].sha1, null_sha1,
 -- 8 --
--
To unsubscribe from this list: send the line 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 00/17] Add interpret-trailers builtin

[Christian Couder]

From: Junio C Hamano gits...@pobox.com

 Is this from the same Christian?

Yes, ...

 The series seems to have unusually high rate of style violations
 compared to the usual submission, like these:
 
 ERROR: open brace '{' following function declarations go on the next line
 #78: FILE: trailer.c:44:
 +static size_t alnum_len(const char *buf, size_t len) {
 
 ERROR: trailing statements should be on next line
 #79: FILE: trailer.c:45:
 + while (--len = 0  !isalnum(buf[len]));
 
 ERROR: space required before the open parenthesis '('
 #70: FILE: trailer.c:90:
 + switch(arg_tok-conf-if_exist) {
 
 WARNING: braces {} are not necessary for any arm of this statement
 #66: FILE: trailer.c:270:
 + if (!strcasecmp(doNothing, value)) {
 [...]
 + } else if (!strcasecmp(add, value)) {
 [...]
 + } else
 [...]
 
 ERROR: that open brace { should be on the previous line
 #96: FILE: trailer.c:300:
 + for (previous = NULL, item = first_conf_item;
 +  item;
 +  previous = item, item = item-next)
 + {
 
 Just trying to see if there is an impersonation ;-)

... I guess the artistic part of my mind is trying to come out somehow.

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

Re: [PATCH v3 17/17] Documentation: add documentation for 'git interpret-trailers'

[Christian Couder]

From: Junio C Hamano gits...@pobox.com
Subject: Re: [PATCH v3 17/17] Documentation: add documentation for 'git 
interpret-trailers'
Date: Mon, 27 Jan 2014 13:20:18 -0800

 Christian Couder chrisc...@tuxfamily.org writes:
 
 +'git interpret-trailers' [--trim-empty] [--infile=file] 
 [token[=value]...]
 
 Would it be more consistent with existing documentation to format this as 
 so?
 
   [--infile=file] [token[=value]]...

 No, it would be very inconsistent:

 $ grep '\.\.\.\]' *.txt | wc -l
 103
 $ grep '\]\.\.\.' *.txt | wc -l
 0
 
 I have a feeling that you are missing the point Eric is making.

Yeah I realize I missed some of his points. Sorry about that.

 The
 value given to the --infile option can be anything, i.e. 'file'
 there is a placeholder, hence --infile=file not --infile=file
 as you wrote.

Ok, I agree with that.

 Also I think [token[=value]]... is the correct way to spell
 that there can be 0 or more token[=value].

Perhaps but it is not very consistent with what there is in other
synopsis strings. For example the commands that accept a path have a
synopsis that ends with [--] [path...] and not [--] [path]

Perhaps [(token[=value])...] is more correct, but not worth the
added complexity.

 token[=value]
 in the original does not make any sense, as  is meant to say this
 thing is a placeholder, and we do not try to say, with the string
 inside , what shape that placeholder takes.  In fact '=' part is
 _not_ a placeholder but is required syntactically when you want to
 supply a value to the token, so the original doubly is incorrect.

Yeah perhaps, except that in the code ':' is accepted instead of '=',
and the documention says that.

So perhaps [(token[(=|:)value])...] is even more correct.

 I find it a bad taste to allow unbound set of token on the LHS of
 '=' on the command line, but that is a separate issue in the design,
 not in the documentation of the design.

I don't understand this sentence, sorry.

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

Re: [PATCH 5/5] builtin/commit.c: reduce scope of variables

[Eric Sunshine]

On Wed, Jan 29, 2014 at 8:48 AM, Elia Pinto gitter.spi...@gmail.com wrote:
 diff --git a/builtin/commit.c b/builtin/commit.c
 index 3767478..eea4421 100644
 --- a/builtin/commit.c
 +++ b/builtin/commit.c
 @@ -1510,7 +1511,6 @@ int cmd_commit(int argc, const char **argv, const char 
 *prefix)
 struct ref_lock *ref_lock;
 struct commit_list *parents = NULL, **pptr = parents;
 struct stat statbuf;
 -   int allow_fast_forward = 1;
 struct commit *current_head = NULL;
 struct commit_extra_header *extra = NULL;

 @@ -1576,6 +1576,7 @@ int cmd_commit(int argc, const char **argv, const char 
 *prefix)
 }
 fclose(fp);
 strbuf_release(m);
 +   int allow_fast_forward = 1;

This introduces a declaration-after-statement, which is frowned upon
in this project.

 if (!stat(git_path(MERGE_MODE), statbuf)) {
 if (strbuf_read_file(sb, git_path(MERGE_MODE), 0) 
  0)
 die_errno(_(could not read MERGE_MODE));
 --
 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 v3 17/17] Documentation: add documentation for 'git interpret-trailers'

[Junio C Hamano]

Christian Couder chrisc...@tuxfamily.org writes:

 I find it a bad taste to allow unbound set of token on the LHS of
 '=' on the command line, but that is a separate issue in the design,
 not in the documentation of the design.

 I don't understand this sentence, sorry.

It is a bad design taste to structure the command line argument in
such a way that it takes

git cmd xyzzy=frotz nitfol=rezrov some other args

where these 'xyzzy', 'nitfol', etc. form an unbound set.  It
prevents future enhancements to the command from allowing anything
that contain '=' in some other args part.

Allowing not just '=' but also ':' makes it even worse.

But these are issues in the design itself.  Not the issue in the
patch 17/17 that is trying to document the design.  This patch under
discussion documents the bad design correctly ;-)


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

C standard compliance?

[David Kastrup]

Hi, I am wondering if I may compare pointers with  that have been
created using different calls of malloc.

The C standard does not allow this (inequalities are only allowed for
pointers into the same structure) to allow for some cheapskate sort of
comparison in segmented architectures.

Now of course being able to _sort_ pointers also allows to _collate_
them.  It totally does not matter just _what_ their ordering relation is
as long as it yields to a sorting function (namely obeys some basic
relations).

The question is whether this kind of undefined behavior (which almost
never is implemented in unexpected ways) is frowned upon in the Git
codebase or not.

-- 
David Kastrup

--
To unsubscribe from this list: send the line 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: C standard compliance?

[Junio C Hamano]

David Kastrup d...@gnu.org writes:

 Hi, I am wondering if I may compare pointers with  that have been
 created using different calls of malloc.

 The C standard does not allow this (inequalities are only allowed for
 pointers into the same structure) to allow for some cheapskate sort of
 comparison in segmented architectures.

Hmm... if you were to implement a set of pointers in such a way that
you can cheaply tell if an unknown pointer belongs to that set, you
would use a hashtable, keyed with something that is derived from the
value of the pointer casted to uintptr_t, I would think.  Is such a
use of ((uintptr_t)ptr) unallowed?  If it is allowed, comparing two
unrelated pointers after casting them to uintptr_t would equally be
valid, I would have to 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: C standard compliance?

[David Kastrup]

Junio C Hamano gits...@pobox.com writes:

 David Kastrup d...@gnu.org writes:

 Hi, I am wondering if I may compare pointers with  that have been
 created using different calls of malloc.

 The C standard does not allow this (inequalities are only allowed for
 pointers into the same structure) to allow for some cheapskate sort of
 comparison in segmented architectures.

 Hmm... if you were to implement a set of pointers in such a way that
 you can cheaply tell if an unknown pointer belongs to that set, you
 would use a hashtable, keyed with something that is derived from the
 value of the pointer casted to uintptr_t, I would think.

The types intptr_t and uintptr_t are optional in ISO/IEC 9899:1999
(C99).  So it would seem that I'd be covering fewer cases rather than
more in that manner.

I should think that architectures providing uintptr_t/intptr_t would
have very little incentive _not_ to offer pointer inequalities
equivalent to either the uintptr_t or intptr_t type conversion.

-- 
David Kastrup
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

git-draw - draws nearly the full content of a tiny git repository as a graph

[Flo]

I just want to present a small tool I wrote. I use it at work to have
a tool visualizing the Git basic concepts and data structures which
are really really really simple (Linus' words). That helps me
teaching my colleagues about Git and answering their questions when
Git did not behave as they expected.

https://github.com/sensorflo/git-draw/wiki
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Bug: Branch rename breaks local downstream branches

[Alex Vallée]

When tracking a local branch, renaming the tracked branch will not
update the downstream branch.

See transcript:

avallee@gust:/tmp/repo (master)$ git co -b foo
Switched to branch 'foo'
avallee@gust:/tmp/repo (foo)$ git co -b bar --track
Branch bar set up to track local branch foo.
Switched to a new branch 'bar'
avallee@gust:/tmp/repo (bar)$ git branch foo -m baz
avallee@gust:/tmp/repo (bar)$ git co bar
Already on 'bar'
Your branch is based on 'foo', but the upstream is gone.
  (use git branch --unset-upstream to fixup)
avallee@gust:/tmp/repo (bar)$
--
To unsubscribe from this list: send the line 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 in git.git (Jan 2014, #06; Wed, 29)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

The second release candidate is expected to happen this weekend.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[New Topics]

* bc/gpg-sign-everywhere (2014-01-27) 9 commits
 - pull: add the --gpg-sign option.
 - rebase: add the --gpg-sign option
 - rebase: parse options in stuck-long mode
 - rebase: don't try to match -M option
 - rebase: remove useless arguments check
 - am: add the --gpg-sign option
 - am: parse options in stuck-long mode
 - git-sh-setup.sh: add variable to use the stuck-long mode
 - cherry-pick, revert: add the --gpg-sign option

 Teach --gpg-sign option to many commands that create commits.

 Changes to some scripted Porcelains use unsafe variable
 substitutions and need to be tightened.

 Waiting for a reroll.


* ds/rev-parse-required-args (2014-01-28) 1 commit
 - rev-parse: check i before using argv[i] against argc

 git rev-parse --default without the required option argument did
 not diagnose it as an error.

 Will merge to 'next'.


* jk/config-path-include-fix (2014-01-28) 2 commits
 - handle_path_include: don't look at NULL value
 - expand_user_path: do not look at NULL path

 include.path variable (or any variable that expects a path that can
 use ~username expansion) in the configuration file is not a
 boolean, but the code failed to check it.

 Will merge to 'next'.


* jk/repack-honor-pack-keep (2014-01-28) 1 commit
 - repack: add `repack.honorpackkeep` config var
 (this branch uses jk/pack-bitmap.)

 Optionally allow git repack to include objects that exist in kept
 packs in newly created packfiles.

 Waiting for response to review comments.


* nd/submodule-pathspec-ending-with-slash (2014-01-27) 8 commits
 - clean: use cache_name_is_other()
 - clean: replace match_pathspec() with dir_path_match()
 - Pass directory indicator to match_pathspec_item()
 - match_pathspec: match pathspec foo/ against directory foo
 - dir.c: prepare match_pathspec_item for taking more flags
 - Rename match_pathspec_depth() to match_pathspec()
 - Convert some match_pathspec_depth() to dir_path_match()
 - Convert some match_pathspec_depth() to ce_path_match()

 Allow git cmd path/, when the 'path' is where a submodule is
 bound to the top-level working tree, to match 'path', despite the
 extra and unnecessary trailing slash.

 Will merge to 'next'.

--
[Stalled]

* jk/color-for-more-pagers (2014-01-17) 4 commits
 - pager: disable colors for some known-bad configurations
 - DONOTMERGE: needs matching change to git-sh-setup
 - setup_pager: set MORE=R
 - setup_pager: refactor LESS/LV environment setting

 'more' implementation of BSD wants to be told with MORE=R
 environment before it shows colored output, while 'more' on some
 other platforms will die when seeing MORE=R environment.

 It appears that we are coming to the consensus that trying to be
 too intimately knowledgeable about quirks of various pager
 implementations on different platforms is a losing proposition.

 Waiting for a reroll.


* po/everyday-doc (2014-01-27) 1 commit
 - Make 'git help everyday' work

 This may make the said command to emit something, but the source is
 not meant to be formatted into a manual pages to begin with, and
 also its contents are a bit stale.  It may be a good first step in
 the right direction, but needs more work to at least get the
 mark-up right before public consumption.

 Will hold.


* jk/branch-at-publish-rebased (2014-01-17) 5 commits
 - t1507 (rev-parse-upstream): fix typo in test title
 - implement @{publish} shorthand
 - branch_get: provide per-branch pushremote pointers
 - branch_get: return early on error
 - sha1_name: refactor upstream_mark

 Give an easier access to the tracking branches from other side in
 a triangular workflow by introducing B@{publish} that works in a
 similar way to how B@{upstream} does.

 Will hold.


* rb/merge-prepare-commit-msg-hook (2014-01-10) 4 commits
 - merge: drop unused arg from abort_commit method signature
 - merge: make prepare_to_commit responsible for write_merge_state
 - t7505: ensure cleanup after hook blocks merge
 - t7505: add missing 

 Expose more merge states (e.g. $GIT_DIR/MERGE_MODE) to hooks that
 run during git merge.  The log message stresses too much on one
 hook, prepare-commit-msg, but it would equally apply to other hooks
 like post-merge, I think.

 Waiting for a reroll.


* jl/submodule-recursive-checkout (2013-12-26) 5 commits
 - Teach checkout to recursively checkout submodules
 - submodule: teach unpack_trees() to update submodules
 - submodule: teach unpack_trees() to repopulate submodules
 - submodule: teach unpack_trees() to remove submodule contents
 - 

Re: C standard compliance?

[Philip Oakley]

From: David Kastrup d...@gnu.org

Junio C Hamano gits...@pobox.com writes:


David Kastrup d...@gnu.org writes:


Hi, I am wondering if I may compare pointers with  that have been
created using different calls of malloc.

The C standard does not allow this (inequalities are only allowed
for
pointers into the same structure) to allow for some cheapskate sort
of
comparison in segmented architectures.


Hmm... if you were to implement a set of pointers in such a way that
you can cheaply tell if an unknown pointer belongs to that set, you
would use a hashtable, keyed with something that is derived from the
value of the pointer casted to uintptr_t, I would think.


The types intptr_t and uintptr_t are optional in ISO/IEC 9899:1999
(C99).  So it would seem that I'd be covering fewer cases rather than
more in that manner.

I should think that architectures providing uintptr_t/intptr_t would
have very little incentive _not_ to offer pointer inequalities
equivalent to either the uintptr_t or intptr_t type conversion.


Undefined behaviours become hidden bugs of the future...
http://article.gmane.org/gmane.comp.version-control.git/230583

blog on the problems of unexpected optimization bugs,
such as dereferencing a null pointer. Finding Undefined Behavior Bugs
by Finding Dead Code http://blog.regehr.org/archives/970 which links to
the draft of an interesting paper
http://pdos.csail.mit.edu/~xi/papers/stack-sosp13.pdf;

The code has now been released http://css.csail.mit.edu/stack/
https://github.com/xiw/stack/, and a few potential errors in Git were 
caught by that tool by Stefan Beller.


The key point of the paper was never to try to use an 'obvious', but
undefined, behaviour.

--
Philip

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: C standard compliance?

[brian m. carlson]

On Wed, Jan 29, 2014 at 09:52:45PM +0100, David Kastrup wrote:
 Junio C Hamano gits...@pobox.com writes:
  Hmm... if you were to implement a set of pointers in such a way that
  you can cheaply tell if an unknown pointer belongs to that set, you
  would use a hashtable, keyed with something that is derived from the
  value of the pointer casted to uintptr_t, I would think.
 
 The types intptr_t and uintptr_t are optional in ISO/IEC 9899:1999
 (C99).  So it would seem that I'd be covering fewer cases rather than
 more in that manner.

I think we already use uintptr_t in the codebase, and if it's not
present, we typedef it to unsigned long.  So I think it should be fine
(and well-defined) if instead of doing

  void *p, *q;
  ...
  if (p  q)
...

you do:

  void *p, *q;
  ...
  if ((uintptr_t)p  (uintptr_t)q)
...

Then on those systems where the compiler has some bizarre undefined
behavior checking, the code will work.  On systems that don't have
uintptr_t, the compiler is probably not smart enough to perform such a
check anyway.

-- 
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* [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

[Junio C Hamano]

Junio C Hamano gits...@pobox.com writes:

 So there are two remaining items, I think.

  - After creating a tags/for-linus signed tag and pushing it to
tags/for-linus, asking request-pull to request that tag to be
pulled seems to lose the tag message from the output.

  - Docs.

 [Footnote]

 *1* Not that it is always acceptable to break the existing users as
 long as they are clueful ones and they are given an escape hatch.
 But this time I know I won't be in the middle of firestorm like
 the one we had immediately after 1.6.0, as long as I keep the
 URL of the message I am responding to in the list archive ;-)

I am not yet doing the docs, but here is a minimal (and I think is
the most sensible) fix to the If I asked a tag to be pulled, I used
to get the message from the tag in the output---the updated code no
longer does so problem.

With this fix, the updates to t5150 I queued on top of the two
patches can lose a test_expect_failure.

I would not be surprised if there are other regressions, though
[*1*].  I am worried about regressions when the user explicitly asks
a ref to be pulled---e.g the command refuses to produce output and
instead fails (perhaps because the ambiguity check is overly
stricter than it should be), or the command produces output that is
different from what we used to produce (this patch is a fix to the
problem in that latter category, but there may be other differences
the existing tests are not covering).

[Footnote]

*1* No, I do not count I used to be able to ask 'master' (or
implicitly 'HEAD' that I happen to be sitting on) to be pulled and
rely on that the command figures out that I have that commit on
'for-linus' in my publish repository, but that feature was removed
as a regression.  Removing that cleverness is the point of this
series.

-- 8 --
Subject: [PATCH] request-pull: pick up tag message as before

The previous two steps were meant to stop promoting the explicit
refname the user gave to the command to a different ref that points
at it.  Most notably, we no longer substitute a branch name the user
used with a name of the tqag that points at the commit at the tip of
the branch.

However, they also lost the code that included the message in a
tag when the user _did_ ask the tag to be pulled.  Resurrect it.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 git-request-pull.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index c8ab0e9..93b4135 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -132,6 +132,14 @@ for you to fetch changes up to %H:
 
 ' $headrev 
 
+if test $(git cat-file -t $head) = tag
+then
+   git cat-file tag $head |
+   sed -n -e '1,/^$/d' -e '/^-BEGIN PGP /q' -e p
+   echo
+   echo 
+fi 
+
 if test -n $branch_name
 then
echo (from the branch description for $branch_name local branch)
-- 
1.9-rc1-183-g614c158

--
To unsubscribe from this list: send the line 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: C standard compliance?

[David Kastrup]

brian m. carlson sand...@crustytoothpaste.net writes:

 On Wed, Jan 29, 2014 at 09:52:45PM +0100, David Kastrup wrote:
 Junio C Hamano gits...@pobox.com writes:
  Hmm... if you were to implement a set of pointers in such a way that
  you can cheaply tell if an unknown pointer belongs to that set, you
  would use a hashtable, keyed with something that is derived from the
  value of the pointer casted to uintptr_t, I would think.
 
 The types intptr_t and uintptr_t are optional in ISO/IEC 9899:1999
 (C99).  So it would seem that I'd be covering fewer cases rather than
 more in that manner.

 I think we already use uintptr_t in the codebase, and if it's not
 present, we typedef it to unsigned long.  So I think it should be fine
 (and well-defined) if instead of doing

   void *p, *q;
   ...
   if (p  q)
 ...

 you do:

   void *p, *q;
   ...
   if ((uintptr_t)p  (uintptr_t)q)
 ...

 Then on those systems where the compiler has some bizarre undefined
 behavior checking, the code will work.  On systems that don't have
 uintptr_t, the compiler is probably not smart enough to perform such a
 check anyway.

The use case is actually sorting a list such that entries pointing to
the same malloced origin data structure are in adjacent list
positions.  At list intptr_t seems used plentifully in Git.

-- 
David Kastrup

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Re* [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

[brian m. carlson]

On Wed, Jan 29, 2014 at 03:34:32PM -0800, Junio C Hamano wrote:
 The previous two steps were meant to stop promoting the explicit
 refname the user gave to the command to a different ref that points
 at it.  Most notably, we no longer substitute a branch name the user
 used with a name of the tqag that points at the commit at the tip of

s/tqag/tag/


-- 
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: Re* [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

[Linus Torvalds]

On Wed, Jan 29, 2014 at 3:34 PM, Junio C Hamano gits...@pobox.com wrote:

 I am not yet doing the docs, but here is a minimal (and I think is
 the most sensible) fix to the If I asked a tag to be pulled, I used
 to get the message from the tag in the output---the updated code no
 longer does so problem.

That was a complete oversight/bug on my part, due to just removing the
tag_name special cases, not thinking about the tag message.

Thinking some more about the tag_name issue, I realize that the other
patch (Make request-pull able to take a refspec of form
local:remote) broke another thing.

The first patch pretty-printed the local branch-name, removing refs/
and possibly heads/ from the local refname. So for a branch, it
would ask people to just pull from the branch-name, and for a tag it
would ask people to pull from tags/name, which is good policy. So if
you had a tag called for-linus, it would say so (using
tags/for-linus).

But the local:remote syntax thing ends up breaking that nice feature.
The old find_matching_refs would actually cause us to show the tags
part if it existed on the remote, but that had become pointless and
counter-productive with the first patch. But with the second patch,
maybe we should reinstate that logic..

Linus
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Feature Request Google Authenticator Support

[Max Rahm]

Github supports google authenticator 2-step authentication. I enabled it
and how can't figure out how to connect to my github account through git.
I've looked pretty hard in the man pages and on google and can't seem to
find anything on how to set up git to work with a repository with 2-step
verification. Here's a link to my stackoverflow question with my exact
problem if there's something I'm missing.

http://stackoverflow.com/questions/21447137/git-github-not-working-with-google-authenticator-osx

As far as I can tell the feature is not supported. I'd like to be able to
use the 2-step authentication but obviously I'd like to be able to push my
code :D

Thanks in advance,
Max Rahm

--
:wq
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Feature Request Google Authenticator Support

[Andrew Ardill]

On 30 January 2014 15:07, Max Rahm ac90b...@gmail.com wrote:
 Github supports google authenticator 2-step authentication. I enabled it
 and how can't figure out how to connect to my github account through git.
 I've looked pretty hard in the man pages and on google and can't seem to
 find anything on how to set up git to work with a repository with 2-step
 verification. Here's a link to my stackoverflow question with my exact
 problem if there's something I'm missing.

 http://stackoverflow.com/questions/21447137/git-github-not-working-with-google-authenticator-osx

 As far as I can tell the feature is not supported. I'd like to be able to
 use the 2-step authentication but obviously I'd like to be able to push my
 code :D

I was under the impression that private key authentication worked
regardless of two-factor authentication. Is using git over ssh an
option for you?

Regards,

Andrew Ardill
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Questions on local clone and push back

[Arshavir Grigorian]

Hi, I have the following use case - I have a set of files that are used for
various adhoc projects, each project is in its own directory, and the files
sometime need to be customized/enhanced for a specific project. Ideally
some of these enhancements should be merged to a central place. I realize
that this is not the most optimal setup for software development, however
these aren't software projects per se and this setup works very well.
To this end, I am thinking of creating a git repository in one directory
(central location), then cloning it (-l -n -s) into another directory in
hopes of making enhancements there and eventually pushing / merging some of
those enhancements back to the central location.

Questions: 1) is this a good approach to achieving what I need 2) I was
getting an error when I tied to run git push about the branch being
checked out and 3) how do I selectively push / merge only certain commits
back to the source repository / branch?

I am using pushing / merging interchangeably because I don't know which
action is more appropriate in this situation.

This is my first time using git so any insights would be much appreciated.
TIA
--
To unsubscribe from this list: send the line 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] pager: set LV=-c alongside LESS=FRSX

[Anders Kaseorg]

On 01/06/2014 09:14 PM, Jonathan Nieder wrote:

+test_expect_success TTY 'LESS and LV envvars are set for pagination' '
+   (
+   sane_unset LESS LV 
+   PAGER=env pager-env.out 
+   export PAGER 
+
+   test_terminal git log
+   ) 
+   grep ^LESS= pager-env.out 
+   grep ^LV= pager-env.out
+'
+


On the Ubuntu PPA builders, I’m seeing this new test fail with SIGPIPE 
about half the time:


died of signal 13 at /build/buildd/git-1.9~rc1/t/test-terminal.perl line 33.
not ok 6 - LESS and LV envvars are set for pagination

Although the test seems to work locally for me, I can reproduce a 
similar failure just by running


$ GIT_PAGER='env pager-env.out' ./test-terminal.perl git log
died of signal 13 at ./test-terminal.perl line 33.

Anders

--
To unsubscribe from this list: send the line 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 00/17] Add interpret-trailers builtin

[Christian Couder]

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 implement features for these trailers first in a
new command rather than in builtin/commit.c, because this way the
prepare-commit-msg and commit-msg hooks can reuse this command.

2) Current state:

Currently the usage string of this command is:

git interpret-trailers [--trim-empty] [--infile=file] 
[(token[(=|:)value])...]

The following features are implemented:

- the result is printed on stdout
- the [token[=value]] arguments are interpreted
- a commit message passed using the --infile=file option is 
interpreted
- if --infile is not used, a commit message is read from stdin
- the trailer.token.key options in the config 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
- ditto for GIT_{AUTHOR,COMMITTER}_{NAME,EMAIL} env variables
- there are some tests
- there is some documentation

The following features are planned but not yet implemented:
- add more tests related to commands
- add examples in documentation
- integration with git commit

Possible improvements:
- support GIT_COMMIT_PROTO env variable in commands

3) Changes since version 3, thanks to Eric and Junio:

* the usage string/synopsis of the command was improved
* some spelling/wording mistakes in the doc were fixed
* some style issues were fixed

Christian Couder (17):
  Add data structures and basic functions for commit trailers
  trailer: process trailers from file and arguments
  trailer: read and process config information
  trailer: process command line trailer arguments
  strbuf: add strbuf_isspace()
  trailer: parse trailers from input file
  trailer: put all the processing together and print
  trailer: add interpret-trailers command
  trailer: add tests for git interpret-trailers
  trailer: if no input file is passed, read from stdin
  trailer: add new_trailer_item() function
  strbuf: add strbuf_replace()
  trailer: execute command from 'trailer.name.command'
  trailer: add tests for trailer command
  trailer: set author and committer env variables
  trailer: add tests for commands using env variables
  Documentation: add documentation for 'git interpret-trailers'

 .gitignore   |   1 +
 Documentation/git-interpret-trailers.txt | 132 +++
 Makefile |   2 +
 builtin.h|   1 +
 builtin/interpret-trailers.c |  36 ++
 git.c|   1 +
 strbuf.c |  14 +
 strbuf.h |   4 +
 t/t7513-interpret-trailers.sh| 262 +
 trailer.c| 637 +++
 trailer.h|   6 +
 11 files changed, 1096 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

-- 
1.8.5.2.201.gacc5987

--
To unsubscribe from this list: send the line 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 01/17] Add data structures and basic functions for commit trailers

[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
---
 Makefile  |  1 +
 trailer.c | 48 
 2 files changed, 49 insertions(+)
 create mode 100644 trailer.c

diff --git a/Makefile b/Makefile
index b4af1e2..ec90feb 100644
--- a/Makefile
+++ b/Makefile
@@ -871,6 +871,7 @@ LIB_OBJS += submodule.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..aed25e1
--- /dev/null
+++ b/trailer.c
@@ -0,0 +1,48 @@
+#include cache.h
+/*
+ * Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
+ */
+
+enum action_where { WHERE_AFTER, WHERE_BEFORE };
+enum action_if_exist { EXIST_ADD_IF_DIFFERENT, EXIST_ADD_IF_DIFFERENT_NEIGHBOR,
+  EXIST_ADD, EXIST_OVERWRITE, EXIST_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_exist if_exist;
+   enum action_if_missing if_missing;
+};
+
+struct trailer_item {
+   struct trailer_item *previous;
+   struct trailer_item *next;
+   const char *token;
+   const char *value;
+   struct conf_info *conf;
+};
+
+static int same_token(struct trailer_item *a, struct trailer_item *b, int 
alnum_len)
+{
+   return !strncasecmp(a-token, b-token, alnum_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, int 
alnum_len)
+{
+   return same_token(a, b, alnum_len)  same_value(a, b);
+}
+
+/* Get the length of buf from its beginning until its last alphanumeric 
character */
+static size_t alnum_len(const char *buf, size_t len)
+{
+   while (--len = 0  !isalnum(buf[len]));
+   return len + 1;
+}
-- 
1.8.5.2.201.gacc5987


--
To unsubscribe from this list: send the line 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 03/17] trailer: read and process config information

[Christian Couder]

This patch implements reading the configuration
to get trailer information, and then processing
it and storing 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
---
 trailer.c | 127 ++
 1 file changed, 127 insertions(+)

diff --git a/trailer.c b/trailer.c
index e9ccfa5..d979a0f 100644
--- a/trailer.c
+++ b/trailer.c
@@ -25,6 +25,8 @@ struct trailer_item {
struct conf_info *conf;
 };
 
+static struct trailer_item *first_conf_item;
+
 static int same_token(struct trailer_item *a, struct trailer_item *b, int 
alnum_len)
 {
return !strncasecmp(a-token, b-token, alnum_len);
@@ -235,3 +237,128 @@ static void process_trailers_lists(struct trailer_item 
**infile_tok_first,
apply_arg_if_missing(infile_tok_first, infile_tok_last, 
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
+   return 1;
+   return 0;
+}
+
+static int set_if_exist(struct conf_info *item, const char *value)
+{
+   if (!strcasecmp(addIfDifferent, value))
+   item-if_exist = EXIST_ADD_IF_DIFFERENT;
+   else if (!strcasecmp(addIfDifferentNeighbor, value))
+   item-if_exist = EXIST_ADD_IF_DIFFERENT_NEIGHBOR;
+   else if (!strcasecmp(add, value))
+   item-if_exist = EXIST_ADD;
+   else if (!strcasecmp(overwrite, value))
+   item-if_exist = EXIST_OVERWRITE;
+   else if (!strcasecmp(doNothing, value))
+   item-if_exist = EXIST_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;
+}
+
+enum trailer_info_type { TRAILER_VALUE, TRAILER_COMMAND, TRAILER_WHERE,
+TRAILER_IF_EXIST, TRAILER_IF_MISSING };
+
+static int set_name_and_type(const char *conf_key, const char *suffix,
+enum trailer_info_type type,
+char **pname, enum trailer_info_type *ptype)
+{
+   int ret = ends_with(conf_key, suffix);
+   if (ret) {
+   *pname = xstrndup(conf_key, strlen(conf_key) - strlen(suffix));
+   *ptype = type;
+   }
+   return ret;
+}
+
+static struct trailer_item *get_conf_item(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);
+   item-conf = xcalloc(sizeof(struct conf_info), 1);
+   item-conf-name = xstrdup(name);
+
+   if (!previous)
+   first_conf_item = item;
+   else {
+   previous-next = item;
+   item-previous = previous;
+   }
+
+   return item;
+}
+
+static int git_trailer_config(const char *conf_key, const char *value, void 
*cb)
+{
+   if (starts_with(conf_key, trailer.)) {
+   const char *orig_conf_key = conf_key;
+   struct trailer_item *item;
+   struct conf_info *conf;
+   char *name;
+   enum trailer_info_type type;
+
+   conf_key += 8;
+   if (!set_name_and_type(conf_key, .key, TRAILER_VALUE, name, 
type) 
+   !set_name_and_type(conf_key, .command, TRAILER_COMMAND, 
name, type) 
+   !set_name_and_type(conf_key, .where, TRAILER_WHERE, 
name, type) 
+   !set_name_and_type(conf_key, .ifexist, TRAILER_IF_EXIST, 
name, type) 
+   !set_name_and_type(conf_key, .ifmissing, 
TRAILER_IF_MISSING, name, type))
+   return 0;
+
+   item = get_conf_item(name);
+   conf = item-conf;
+
+   if (type == TRAILER_VALUE) {
+   if (conf-key)
+   warning(_(more than one %s), orig_conf_key);
+   conf-key = xstrdup(value);
+   } else if (type == TRAILER_COMMAND) {
+   if (conf-command)
+   warning(_(more than one %s), orig_conf_key);
+   

[PATCH v4 06/17] trailer: parse trailers from input file

[Christian Couder]

This patch reads trailers from an input file, parses
them and puts the result into a doubly linked list.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 trailer.c | 62 ++
 1 file changed, 62 insertions(+)

diff --git a/trailer.c b/trailer.c
index f48fd94..084b3e1 100644
--- a/trailer.c
+++ b/trailer.c
@@ -439,3 +439,65 @@ static struct trailer_item *process_command_line_args(int 
argc, const char **arg
 
return arg_tok_first;
 }
+
+static struct strbuf **read_input_file(const char *infile)
+{
+   struct strbuf sb = STRBUF_INIT;
+
+   if (strbuf_read_file(sb, infile, 0)  0)
+   die_errno(_(could not read input file '%s'), infile);
+
+   return strbuf_split(sb, '\n');
+}
+
+/*
+ * Return the the (0 based) index of the first trailer line
+ * or the line count if there are no trailers.
+ */
+static int find_trailer_start(struct strbuf **lines)
+{
+   int count, start, empty = 1;
+
+   /* Get the line count */
+   for (count = 0; lines[count]; count++);
+
+   /*
+* Get the start of the trailers by looking starting from the end
+* for a line with only spaces before lines with one ':'.
+*/
+   for (start = count - 1; start = 0; start--) {
+   if (strbuf_isspace(lines[start])) {
+   if (empty)
+   continue;
+   return start + 1;
+   }
+   if (strchr(lines[start]-buf, ':')) {
+   if (empty)
+   empty = 0;
+   continue;
+   }
+   return count;
+   }
+
+   return empty ? count : start + 1;
+}
+
+static void process_input_file(const char *infile,
+  struct trailer_item **infile_tok_first,
+  struct trailer_item **infile_tok_last)
+{
+   struct strbuf **lines = read_input_file(infile);
+   int start = find_trailer_start(lines);
+   int i;
+
+   /* Print non trailer lines as is */
+   for (i = 0; lines[i]  i  start; i++) {
+   printf(%s, lines[i]-buf);
+   }
+
+   /* Parse trailer lines */
+   for (i = start; lines[i]; i++) {
+   struct trailer_item *new = create_trailer_item(lines[i]-buf);
+   add_trailer_item(infile_tok_first, infile_tok_last, new);
+   }
+}
-- 
1.8.5.2.201.gacc5987


--
To unsubscribe from this list: send the line 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 13/17] trailer: execute command from 'trailer.name.command'

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 trailer.c | 56 
 1 file changed, 56 insertions(+)

diff --git a/trailer.c b/trailer.c
index 430ff39..dc8908a 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,4 +1,5 @@
 #include cache.h
+#include run-command.h
 /*
  * Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
  */
@@ -12,11 +13,14 @@ struct conf_info {
char *name;
char *key;
char *command;
+   unsigned command_uses_arg : 1;
enum action_where where;
enum action_if_exist if_exist;
enum action_if_missing if_missing;
 };
 
+#define TRAILER_ARG_STRING $ARG
+
 struct trailer_item {
struct trailer_item *previous;
struct trailer_item *next;
@@ -368,6 +372,7 @@ static int git_trailer_config(const char *conf_key, const 
char *value, void *cb)
if (conf-command)
warning(_(more than one %s), orig_conf_key);
conf-command = xstrdup(value);
+   conf-command_uses_arg = !!strstr(conf-command, 
TRAILER_ARG_STRING);
} else if (type == TRAILER_WHERE) {
if (set_where(conf, value))
warning(_(unknown value '%s' for key '%s'), 
value, orig_conf_key);
@@ -399,6 +404,45 @@ static void parse_trailer(struct strbuf *tok, struct 
strbuf *val, const char *tr
}
 }
 
+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);
+   else
+   result = strbuf_detach(buf, NULL);
+
+   strbuf_release(cmd);
+   return result;
+}
+
 static struct trailer_item *new_trailer_item(struct trailer_item *conf_item,
 const char* tok, const char* val)
 {
@@ -408,6 +452,8 @@ static struct trailer_item *new_trailer_item(struct 
trailer_item *conf_item,
if (conf_item) {
new-conf = conf_item-conf;
new-token = xstrdup(conf_item-conf-key);
+   if (conf_item-conf-command_uses_arg || !val)
+   new-value = apply_command(conf_item-conf-command, 
val);
} else {
new-conf = xcalloc(sizeof(struct conf_info), 1);
new-token = tok;
@@ -458,12 +504,22 @@ static struct trailer_item *process_command_line_args(int 
argc, const char **arg
int i;
struct trailer_item *arg_tok_first = NULL;
struct trailer_item *arg_tok_last = NULL;
+   struct trailer_item *item;
 
for (i = 0; i  argc; i++) {
struct trailer_item *new = create_trailer_item(argv[i]);
add_trailer_item(arg_tok_first, arg_tok_last, new);
}
 
+   /* Add conf commands that don't use $ARG */
+   for (item = first_conf_item; item; item = item-next) {
+   if (item-conf-command  !item-conf-command_uses_arg)
+   {
+   struct trailer_item *new = new_trailer_item(item, NULL, 
NULL);
+   add_trailer_item(arg_tok_first, arg_tok_last, new);
+   }
+   }
+
return arg_tok_first;
 }
 
-- 
1.8.5.2.201.gacc5987


--
To unsubscribe from this list: send the line 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 08/17] trailer: add interpret-trailers command

[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
---
 .gitignore   |  1 +
 Makefile |  1 +
 builtin.h|  1 +
 builtin/interpret-trailers.c | 36 
 git.c|  1 +
 trailer.h|  6 ++
 6 files changed, 46 insertions(+)
 create mode 100644 builtin/interpret-trailers.c
 create mode 100644 trailer.h

diff --git a/.gitignore b/.gitignore
index b5f9def..c870ada 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 ec90feb..a91465e 100644
--- a/Makefile
+++ b/Makefile
@@ -935,6 +935,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 d4afbfe..30f4c30 100644
--- a/builtin.h
+++ b/builtin.h
@@ -71,6 +71,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..04b0ae2
--- /dev/null
+++ b/builtin/interpret-trailers.c
@@ -0,0 +1,36 @@
+/*
+ * Builtin git interpret-trailers
+ *
+ * Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
+ *
+ */
+
+#include cache.h
+#include builtin.h
+#include parse-options.h
+#include strbuf.h
+#include trailer.h
+
+static const char * const git_interpret_trailers_usage[] = {
+   N_(git interpret-trailers [--trim-empty] [--infile=file] 
[(token[(=|:)value])...]),
+   NULL
+};
+
+int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
+{
+   const char *infile = NULL;
+   int trim_empty = 0;
+
+   struct option options[] = {
+   OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty 
trailers)),
+   OPT_FILENAME(0, infile, infile, N_(use message from file)),
+   OPT_END()
+   };
+
+   argc = parse_options(argc, argv, prefix, options,
+git_interpret_trailers_usage, 0);
+
+   process_trailers(infile, trim_empty, argc, argv);
+
+   return 0;
+}
diff --git a/git.c b/git.c
index 3799514..1420b58 100644
--- a/git.c
+++ b/git.c
@@ -383,6 +383,7 @@ static void handle_internal_command(int argc, const char 
**argv)
{ index-pack, cmd_index_pack, RUN_SETUP_GENTLY },
{ init, cmd_init_db },
{ init-db, cmd_init_db },
+   { 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 },
diff --git a/trailer.h b/trailer.h
new file mode 100644
index 000..9db4459
--- /dev/null
+++ b/trailer.h
@@ -0,0 +1,6 @@
+#ifndef TRAILER_H
+#define TRAILER_H
+
+void process_trailers(const char *infile, int trim_empty, int argc, const char 
**argv);
+
+#endif /* TRAILER_H */
-- 
1.8.5.2.201.gacc5987


--
To unsubscribe from this list: send the line 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 05/17] strbuf: add strbuf_isspace()

[Christian Couder]

This helper function checks if a strbuf
contains only space chars or not.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 strbuf.c | 7 +++
 strbuf.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/strbuf.c b/strbuf.c
index 83caf4a..2124bb8 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -124,6 +124,13 @@ void strbuf_ltrim(struct strbuf *sb)
sb-buf[sb-len] = '\0';
 }
 
+int strbuf_isspace(struct strbuf *sb)
+{
+   char *b;
+   for (b = sb-buf; *b  isspace(*b); b++);
+   return !*b;
+}
+
 struct strbuf **strbuf_split_buf(const char *str, size_t slen,
 int terminator, int max)
 {
diff --git a/strbuf.h b/strbuf.h
index 73e80ce..02bff3a 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -42,6 +42,7 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t 
len) {
 extern void strbuf_trim(struct strbuf *);
 extern void strbuf_rtrim(struct strbuf *);
 extern void strbuf_ltrim(struct strbuf *);
+extern int strbuf_isspace(struct strbuf *);
 extern int strbuf_cmp(const struct strbuf *, const struct strbuf *);
 
 /*
-- 
1.8.5.2.201.gacc5987


--
To unsubscribe from this list: send the line 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 17/17] Documentation: add documentation for 'git interpret-trailers'

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-interpret-trailers.txt | 132 +++
 1 file changed, 132 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..7683889
--- /dev/null
+++ b/Documentation/git-interpret-trailers.txt
@@ -0,0 +1,132 @@
+git-interpret-trailers(1)
+=
+
+NAME
+
+git-interpret-trailers - help add stuctured information into commit messages
+
+SYNOPSIS
+
+[verse]
+'git interpret-trailers' [--trim-empty] [--infile=file] 
[(token[(=|:)value])...]
+
+DESCRIPTION
+---
+Help add RFC 822-like headers, called 'trailers', at the end of the
+otherwise free-form part of a commit message.
+
+Unless `--infile=file` is used, this command is a filter. It reads
+the standard input for a commit message and applies the `token`
+arguments, if any, to this message. The resulting message is emited on
+the standard output.
+
+Some configuration variables control the way the `token` arguments are
+applied to the message and the way any existing trailer in the message
+is changed. They also make it possible to automatically add some
+trailers.
+
+By default, a 'token=value' or 'token:value' argument will be added
+only if no trailer with the same (token, value) pair is already in the
+message. The 'token' and 'value' parts will be trimmed to remove
+starting and trailing whitespaces, and the resulting trimmed 'token'
+and 'value' will appear in the message like this:
+
+
+token: value
+
+
+By default, if there are already trailers with the same 'token' the
+new trailer will appear just after the last trailer with the same
+'token'. Otherwise it will appear at the end of the message.
+
+Note that 'trailers' 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.
+
+OPTIONS
+---
+--trim-empty::
+   If the 'value' part of any trailer contains only whitespaces,
+   the whole trailer will be removed from the resulting message.
+
+infile=file::
+   Read the commit message from `file` instead of the standard
+   input.
+
+CONFIGURATION VARIABLES
+---
+
+trailer.token.key::
+   This 'key' will be used instead of 'token' in the
+   trailer. After some alphanumeric characters, it can contain
+   some non alphanumeric characters like ':', '=' or '#' that will
+   be used instead of ':' to separate the token from the value in
+   the trailer, though the default ':' is more standard.
+
+trailer.token.where::
+   This can be either `after`, which is the default, or
+   `before`. If it is `before`, then a trailer with the specified
+   token, will appear before, instead of after, other trailers
+   with the same token, or otherwise at the beginning, instead of
+   at the end, of all the trailers.
+
+trailer.token.ifexist::
+   This option makes it possible to choose what action will be
+   performed when there is already at least one trailer with the
+   same token in the message.
++
+The valid values for this option are: `addIfDifferent` (this is the
+default), `addIfDifferentNeighbor`, `add`, `overwrite` or `doNothing`.
++
+With `addIfDifferent`, a new trailer will be added only if no trailer
+with the same (token, value) pair is already in the message.
++
+With `addIfDifferentNeighbor`, a new trailer will be added only if no
+trailer with the same (token, value) pair is above or below the line
+where the new trailer will be added.
++
+With `add`, a new trailer will be added, even if some trailers with
+the same (token, value) pair are already in the message.
++
+With `overwrite`, the new trailer will overwrite an existing trailer
+with the same token.
++
+With `doNothing`, nothing will be done, that is no new trailer will be
+added if there is already one with the same token in the message.
+
+trailer.token.ifmissing::
+   This option makes it possible to choose what action will be
+   performed when there is not yet any trailer with the same
+   token in the message.
++
+The valid values for this option are: `add` (this is the default) and
+`doNothing`.
++
+With `add`, a new trailer will be added.
++
+With `doNothing`, nothing will be done.
+
+trailer.token.command::
+   This option can be used to specify a shell command that will
+   be used to automatically add or modify a trailer with the
+   specified 'token'.
++
+When this option is specified, it is like if a special 'token=value'
+argument is added at the end of the command line, where 'value' will
+be given by the standard output of the specified command.
++
+If the 

[PATCH v4 10/17] trailer: if no input file is passed, read from stdin

[Christian Couder]

It is simpler and more natural if the git interpret-trailers
is made a filter as its output already goes to sdtout.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/interpret-trailers.c  |  2 +-
 t/t7513-interpret-trailers.sh |  7 +++
 trailer.c | 15 +--
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 04b0ae2..ae8e561 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -23,7 +23,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const 
char *prefix)
 
struct option options[] = {
OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty 
trailers)),
-   OPT_FILENAME(0, infile, infile, N_(use message from file)),
+   OPT_FILENAME(0, infile, infile, N_(use message from file, 
instead of stdin)),
OPT_END()
};
 
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 8be333c..f5ef81f 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -205,4 +205,11 @@ test_expect_success 'using ifMissing = doNothing' '
test_cmp expected actual
 '
 
+test_expect_success 'with input from stdin' '
+   cat complex_message_body expected 
+   printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Junio\nAcked-by= 
Peff\nReviewed-by: \nSigned-off-by: \n expected 
+   git interpret-trailers review: fix=53 cc=Linus ack: Junio 
fix=22 bug: 42 ack: Peff  complex_message actual 
+   test_cmp expected actual
+'
+
 test_done
diff --git a/trailer.c b/trailer.c
index 8681aed..73a65e0 100644
--- a/trailer.c
+++ b/trailer.c
@@ -464,8 +464,13 @@ static struct strbuf **read_input_file(const char *infile)
 {
struct strbuf sb = STRBUF_INIT;
 
-   if (strbuf_read_file(sb, infile, 0)  0)
-   die_errno(_(could not read input file '%s'), infile);
+   if (infile) {
+   if (strbuf_read_file(sb, infile, 0)  0)
+   die_errno(_(could not read input file '%s'), infile);
+   } else {
+   if (strbuf_read(sb, fileno(stdin), 0)  0)
+   die_errno(_(could not read from stdin));
+   }
 
return strbuf_split(sb, '\n');
 }
@@ -530,10 +535,8 @@ void process_trailers(const char *infile, int trim_empty, 
int argc, const char *
 
git_config(git_trailer_config, NULL);
 
-   /* Print the non trailer part of infile */
-   if (infile) {
-   process_input_file(infile, infile_tok_first, infile_tok_last);
-   }
+   /* Print the non trailer part of infile (or stdin if infile is NULL) */
+   process_input_file(infile, infile_tok_first, infile_tok_last);
 
arg_tok_first = process_command_line_args(argc, argv);
 
-- 
1.8.5.2.201.gacc5987


--
To unsubscribe from this list: send the line 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 11/17] trailer: add new_trailer_item() function

[Christian Couder]

This is a small refactoring to prepare for the next steps.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 trailer.c | 31 +++
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/trailer.c b/trailer.c
index 73a65e0..430ff39 100644
--- a/trailer.c
+++ b/trailer.c
@@ -399,11 +399,27 @@ static void parse_trailer(struct strbuf *tok, struct 
strbuf *val, const char *tr
}
 }
 
+static struct trailer_item *new_trailer_item(struct trailer_item *conf_item,
+const char* tok, const char* val)
+{
+   struct trailer_item *new = xcalloc(sizeof(struct trailer_item), 1);
+   new-value = val;
+
+   if (conf_item) {
+   new-conf = conf_item-conf;
+   new-token = xstrdup(conf_item-conf-key);
+   } else {
+   new-conf = xcalloc(sizeof(struct conf_info), 1);
+   new-token = tok;
+   }
+
+   return new;
+}
+
 static struct trailer_item *create_trailer_item(const char *string)
 {
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
-   struct trailer_item *new;
struct trailer_item *item;
int tok_alnum_len;
 
@@ -415,21 +431,12 @@ static struct trailer_item *create_trailer_item(const 
char *string)
for (item = first_conf_item; item; item = item-next) {
if (!strncasecmp(tok.buf, item-conf-key, tok_alnum_len) ||
!strncasecmp(tok.buf, item-conf-name, tok_alnum_len)) {
-   new = xcalloc(sizeof(struct trailer_item), 1);
-   new-conf = item-conf;
-   new-token = xstrdup(item-conf-key);
-   new-value = strbuf_detach(val, NULL);
strbuf_release(tok);
-   return new;
+   return new_trailer_item(item, NULL, strbuf_detach(val, 
NULL));
}
}
 
-   new = xcalloc(sizeof(struct trailer_item), 1);
-   new-conf = xcalloc(sizeof(struct conf_info), 1);
-   new-token = strbuf_detach(tok, NULL);
-   new-value = strbuf_detach(val, NULL);
-
-   return new;
+   return new_trailer_item(NULL, strbuf_detach(tok, NULL), 
strbuf_detach(val, NULL));;
 }
 
 static void add_trailer_item(struct trailer_item **first,
-- 
1.8.5.2.201.gacc5987


--
To unsubscribe from this list: send the line 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 16/17] trailer: add tests for commands using env variables

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t7513-interpret-trailers.sh | 20 
 1 file changed, 20 insertions(+)

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 2d50b7a..00894a8 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -223,6 +223,26 @@ test_expect_success 'with simple command' '
test_cmp expected actual
 '
 
+test_expect_success 'with command using commiter information' '
+   git config trailer.sign.ifExist addIfDifferent 
+   git config trailer.sign.command echo \\$GIT_COMMITTER_NAME 
\$GIT_COMMITTER_EMAIL\ 
+   cat complex_message_body expected 
+   printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: 
\nSigned-off-by: C O Mitter commit...@example.com\n expected 
+   git interpret-trailers review: 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.ifExist addIfDifferentNeighbor 
+   git config trailer.sign.command echo \\$GIT_AUTHOR_NAME 
\$GIT_AUTHOR_EMAIL\ 
+   cat complex_message_body expected 
+   printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: 
\nSigned-off-by: A U Thor aut...@example.com\n expected 
+   git interpret-trailers review: 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 
-- 
1.8.5.2.201.gacc5987


--
To unsubscribe from this list: send the line 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 09/17] trailer: add tests for git interpret-trailers

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t7513-interpret-trailers.sh | 208 ++
 1 file changed, 208 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..8be333c
--- /dev/null
+++ b/t/t7513-interpret-trailers.sh
@@ -0,0 +1,208 @@
+#!/bin/sh
+#
+# Copyright (c) 2013 Christian Couder
+#
+
+test_description='git interpret-trailers'
+
+. ./test-lib.sh
+
+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
+
+# 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.
+sed -e 's/ Z$/ /' complex_message_trailers -\EOF
+Fixes: Z
+Acked-by: Z
+Reviewed-by: Z
+Signed-off-by: Z
+EOF
+
+test_expect_success 'without config' '
+   printf ack: Peff\nReviewed-by: \nAcked-by: Johan\n expected 
+   git interpret-trailers ack = Peff Reviewed-by Acked-by: Johan 
actual 
+   test_cmp expected actual
+'
+
+test_expect_success '--trim-empty without config' '
+   printf ack: Peff\nAcked-by: Johan\n expected 
+   git interpret-trailers --trim-empty ack = Peff Reviewed-by 
Acked-by: Johan sob: actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'with config setup' '
+   git config trailer.ack.key Acked-by:  
+   printf Acked-by: Peff\n expected 
+   git interpret-trailers --trim-empty ack = Peff actual 
+   test_cmp expected actual 
+   git interpret-trailers --trim-empty Acked-by = Peff actual 
+   test_cmp expected actual 
+   git interpret-trailers --trim-empty Acked-by :Peff actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'with config setup and = sign' '
+   git config trailer.ack.key Acked-by=  
+   printf Acked-by= Peff\n expected 
+   git interpret-trailers --trim-empty ack = Peff actual 
+   test_cmp expected actual 
+   git interpret-trailers --trim-empty Acked-by= Peff actual 
+   test_cmp expected actual 
+   git interpret-trailers --trim-empty Acked-by : Peff actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'with config setup and # sign' '
+   git config trailer.bug.key Bug # 
+   printf Bug #42\n expected 
+   git interpret-trailers --trim-empty bug = 42 actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'with commit basic message' '
+   git interpret-trailers --infile basic_message actual 
+   test_cmp basic_message actual
+'
+
+test_expect_success 'with commit complex message' '
+   cat complex_message_body complex_message_trailers complex_message 
+   cat complex_message_body expected 
+   printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \n 
expected 
+   git interpret-trailers --infile complex_message actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'with commit complex message and args' '
+   cat complex_message_body expected 
+   printf Fixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: 
\nSigned-off-by: \nBug #42\n expected 
+   git interpret-trailers --infile complex_message ack: Peff bug: 42 
actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'with commit complex message, args and --trim-empty' '
+   cat complex_message_body expected 
+   printf Acked-by= Peff\nBug #42\n expected 
+   git interpret-trailers --trim-empty --infile complex_message ack: 
Peff bug: 42 actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'using where = before' '
+   git config trailer.bug.where before 
+   cat complex_message_body expected 
+   printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: 
\nSigned-off-by: \n expected 
+   git interpret-trailers --infile complex_message ack: Peff bug: 42 
actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'using where = before for a token in the middle of 
infile' '
+   git config trailer.review.key Reviewed-by: 
+   git config trailer.review.where before 
+   cat complex_message_body expected 
+   printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: 
Johan\nReviewed-by: \nSigned-off-by: \n expected 
+   git interpret-trailers --infile complex_message ack: Peff bug: 42 
review: Johan actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'using where = before and --trim-empty' '
+   cat complex_message_body expected 
+   printf Bug #46\nBug #42\nAcked-by= Peff\nReviewed-by: Johan\n 
expected 
+   git interpret-trailers --infile complex_message --trim-empty ack: 
Peff bug: 42 review: Johan Bug: 46  actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'the default is ifExist = addIfDifferent' '
+   cat complex_message_body expected 
+   

[PATCH v4 12/17] strbuf: add strbuf_replace()

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 strbuf.c | 7 +++
 strbuf.h | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/strbuf.c b/strbuf.c
index 2124bb8..e45e513 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -197,6 +197,13 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t 
len,
strbuf_setlen(sb, sb-len + dlen - len);
 }
 
+void strbuf_replace(struct strbuf *sb, const char *a, const char *b)
+{
+   char *ptr = strstr(sb-buf, a);
+   if (ptr)
+   strbuf_splice(sb, ptr - sb-buf, strlen(a), b, strlen(b));
+}
+
 void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len)
 {
strbuf_splice(sb, pos, 0, data, len);
diff --git a/strbuf.h b/strbuf.h
index 02bff3a..38faf70 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -111,6 +111,9 @@ extern void strbuf_remove(struct strbuf *, size_t pos, 
size_t len);
 extern void strbuf_splice(struct strbuf *, size_t pos, size_t len,
   const void *, size_t);
 
+/* first occurence of a replaced with b */
+extern void strbuf_replace(struct strbuf *, const char *a, const char *b);
+
 extern void strbuf_add_commented_lines(struct strbuf *out, const char *buf, 
size_t size);
 
 extern void strbuf_add(struct strbuf *, const void *, size_t);
-- 
1.8.5.2.201.gacc5987


--
To unsubscribe from this list: send the line 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 07/17] trailer: put all the processing together and print

[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
---
 trailer.c | 40 
 1 file changed, 40 insertions(+)

diff --git a/trailer.c b/trailer.c
index 084b3e1..8681aed 100644
--- a/trailer.c
+++ b/trailer.c
@@ -49,6 +49,26 @@ static size_t alnum_len(const char *buf, size_t len)
return len + 1;
 }
 
+static void print_tok_val(const char *tok, const char *val)
+{
+   char c = tok[strlen(tok) - 1];
+   if (isalnum(c))
+   printf(%s: %s\n, tok, val);
+   else if (isspace(c) || c == '#')
+   printf(%s%s\n, tok, val);
+   else
+   printf(%s %s\n, tok, 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 add_arg_to_infile(struct trailer_item *infile_tok,
  struct trailer_item *arg_tok)
 {
@@ -501,3 +521,23 @@ static void process_input_file(const char *infile,
add_trailer_item(infile_tok_first, infile_tok_last, new);
}
 }
+
+void process_trailers(const char *infile, int trim_empty, int argc, const char 
**argv)
+{
+   struct trailer_item *infile_tok_first = NULL;
+   struct trailer_item *infile_tok_last = NULL;
+   struct trailer_item *arg_tok_first;
+
+   git_config(git_trailer_config, NULL);
+
+   /* Print the non trailer part of infile */
+   if (infile) {
+   process_input_file(infile, infile_tok_first, infile_tok_last);
+   }
+
+   arg_tok_first = process_command_line_args(argc, argv);
+
+   process_trailers_lists(infile_tok_first, infile_tok_last, 
arg_tok_first);
+
+   print_all(infile_tok_first, trim_empty);
+}
-- 
1.8.5.2.201.gacc5987


--
To unsubscribe from this list: send the line 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 04/17] trailer: process command line trailer arguments

[Christian Couder]

This patch parses the trailer command line arguments
and put the result into an arg_tok doubly linked
list.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 trailer.c | 77 +++
 1 file changed, 77 insertions(+)

diff --git a/trailer.c b/trailer.c
index d979a0f..f48fd94 100644
--- a/trailer.c
+++ b/trailer.c
@@ -362,3 +362,80 @@ static int git_trailer_config(const char *conf_key, const 
char *value, void *cb)
}
return 0;
 }
+
+static void parse_trailer(struct strbuf *tok, struct strbuf *val, const char 
*trailer)
+{
+   char *end = strchr(trailer, '=');
+   if (!end)
+   end = strchr(trailer, ':');
+   if (end) {
+   strbuf_add(tok, trailer, end - trailer);
+   strbuf_trim(tok);
+   strbuf_addstr(val, end + 1);
+   strbuf_trim(val);
+   } else {
+   strbuf_addstr(tok, trailer);
+   strbuf_trim(tok);
+   }
+}
+
+static struct trailer_item *create_trailer_item(const char *string)
+{
+   struct strbuf tok = STRBUF_INIT;
+   struct strbuf val = STRBUF_INIT;
+   struct trailer_item *new;
+   struct trailer_item *item;
+   int tok_alnum_len;
+
+   parse_trailer(tok, val, string);
+
+   tok_alnum_len = alnum_len(tok.buf, tok.len);
+
+   /* Lookup if the token matches something in the config */
+   for (item = first_conf_item; item; item = item-next) {
+   if (!strncasecmp(tok.buf, item-conf-key, tok_alnum_len) ||
+   !strncasecmp(tok.buf, item-conf-name, tok_alnum_len)) {
+   new = xcalloc(sizeof(struct trailer_item), 1);
+   new-conf = item-conf;
+   new-token = xstrdup(item-conf-key);
+   new-value = strbuf_detach(val, NULL);
+   strbuf_release(tok);
+   return new;
+   }
+   }
+
+   new = xcalloc(sizeof(struct trailer_item), 1);
+   new-conf = xcalloc(sizeof(struct conf_info), 1);
+   new-token = strbuf_detach(tok, NULL);
+   new-value = strbuf_detach(val, NULL);
+
+   return new;
+}
+
+static void add_trailer_item(struct trailer_item **first,
+struct trailer_item **last,
+struct trailer_item *new)
+{
+   if (!*last) {
+   *first = new;
+   *last = new;
+   } else {
+   (*last)-next = new;
+   new-previous = *last;
+   *last = new;
+   }
+}
+
+static struct trailer_item *process_command_line_args(int argc, const char 
**argv)
+{
+   int i;
+   struct trailer_item *arg_tok_first = NULL;
+   struct trailer_item *arg_tok_last = NULL;
+
+   for (i = 0; i  argc; i++) {
+   struct trailer_item *new = create_trailer_item(argv[i]);
+   add_trailer_item(arg_tok_first, arg_tok_last, new);
+   }
+
+   return arg_tok_first;
+}
-- 
1.8.5.2.201.gacc5987


--
To unsubscribe from this list: send the line 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 02/17] trailer: process trailers from file and arguments

[Christian Couder]

This patch implements the logic that process trailers
from file and arguments.

At the beginning trailers from file are in their own
infile_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 infile_tok list.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 trailer.c | 189 ++
 1 file changed, 189 insertions(+)

diff --git a/trailer.c b/trailer.c
index aed25e1..e9ccfa5 100644
--- a/trailer.c
+++ b/trailer.c
@@ -46,3 +46,192 @@ static size_t alnum_len(const char *buf, size_t len)
while (--len = 0  !isalnum(buf[len]));
return len + 1;
 }
+
+static void add_arg_to_infile(struct trailer_item *infile_tok,
+ struct trailer_item *arg_tok)
+{
+   if (arg_tok-conf-where == WHERE_AFTER) {
+   arg_tok-next = infile_tok-next;
+   infile_tok-next = arg_tok;
+   arg_tok-previous = infile_tok;
+   if (arg_tok-next)
+   arg_tok-next-previous = arg_tok;
+   } else {
+   arg_tok-previous = infile_tok-previous;
+   infile_tok-previous = arg_tok;
+   arg_tok-next = infile_tok;
+   if (arg_tok-previous)
+   arg_tok-previous-next = arg_tok;
+   }
+}
+
+static int check_if_different(struct trailer_item *infile_tok,
+ struct trailer_item *arg_tok,
+ int alnum_len, int check_all)
+{
+   enum action_where where = arg_tok-conf-where;
+   do {
+   if (!infile_tok)
+   return 1;
+   if (same_trailer(infile_tok, arg_tok, alnum_len))
+   return 0;
+   /*
+* if we want to add a trailer after another one,
+* we have to check those before this one
+*/
+   infile_tok = (where == WHERE_AFTER) ? infile_tok-previous : 
infile_tok-next;
+   } while (check_all);
+   return 1;
+}
+
+static void apply_arg_if_exist(struct trailer_item *infile_tok,
+  struct trailer_item *arg_tok,
+  int alnum_len)
+{
+   switch (arg_tok-conf-if_exist) {
+   case EXIST_DO_NOTHING:
+   free(arg_tok);
+   break;
+   case EXIST_OVERWRITE:
+   free((char *)infile_tok-value);
+   infile_tok-value = xstrdup(arg_tok-value);
+   free(arg_tok);
+   break;
+   case EXIST_ADD:
+   add_arg_to_infile(infile_tok, arg_tok);
+   break;
+   case EXIST_ADD_IF_DIFFERENT:
+   if (check_if_different(infile_tok, arg_tok, alnum_len, 1))
+   add_arg_to_infile(infile_tok, arg_tok);
+   else
+   free(arg_tok);
+   break;
+   case EXIST_ADD_IF_DIFFERENT_NEIGHBOR:
+   if (check_if_different(infile_tok, arg_tok, alnum_len, 0))
+   add_arg_to_infile(infile_tok, arg_tok);
+   else
+   free(arg_tok);
+   break;
+   }
+}
+
+static void remove_from_list(struct trailer_item *item,
+struct trailer_item **first)
+{
+   if (item-next)
+   item-next-previous = item-previous;
+   if (item-previous)
+   item-previous-next = item-next;
+   else
+   *first = item-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 process_infile_tok(struct trailer_item *infile_tok,
+  struct trailer_item **arg_tok_first,
+  enum action_where where)
+{
+   struct trailer_item *arg_tok;
+   struct trailer_item *next_arg;
+
+   int tok_alnum_len = alnum_len(infile_tok-token, 
strlen(infile_tok-token));
+   for (arg_tok = *arg_tok_first; arg_tok; arg_tok = next_arg) {
+   next_arg = arg_tok-next;
+   if (same_token(infile_tok, arg_tok, tok_alnum_len) 
+   arg_tok-conf-where == where) {
+   /* Remove arg_tok from list */
+   remove_from_list(arg_tok, arg_tok_first);
+   /* Apply arg */
+   apply_arg_if_exist(infile_tok, arg_tok, tok_alnum_len);
+   /*
+* If arg has been added to infile,
+* then we need to process it too now.
+*/
+   if ((where == WHERE_AFTER ? 

[PATCH v4 14/17] trailer: add tests for trailer command

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t7513-interpret-trailers.sh | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index f5ef81f..2d50b7a 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -212,4 +212,31 @@ test_expect_success 'with input from stdin' '
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.ifExist addIfDifferentNeighbor 
+   git config trailer.sign.command echo \A U Thor 
aut...@example.com\ 
+   cat complex_message_body expected 
+   printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: 
\nSigned-off-by: A U Thor aut...@example.com\n expected 
+   git interpret-trailers review: 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.ifExist overwrite 
+   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 
+   printf Fixes: $FIXED\nAcked-by= \nReviewed-by: \nSigned-off-by: 
\nSigned-off-by: A U Thor aut...@example.com\n expected 
+   git interpret-trailers review: fix=HEAD  complex_message actual 
+   test_cmp expected actual
+'
+
 test_done
-- 
1.8.5.2.201.gacc5987


--
To unsubscribe from this list: send the line 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 15/17] trailer: set author and committer env variables

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 trailer.c | 30 +-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/trailer.c b/trailer.c
index dc8908a..e29b7f2 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,5 +1,6 @@
 #include cache.h
 #include run-command.h
+#include argv-array.h
 /*
  * Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
  */
@@ -414,14 +415,40 @@ static int read_from_command(struct child_process *cp, 
struct strbuf *buf)
return 0;
 }
 
+static void setup_ac_env(struct argv_array *env, const char *ac_name, const 
char *ac_mail, const char *(*read)(int))
+{
+   if (!getenv(ac_name) || !getenv(ac_mail)) {
+   struct ident_split ident;
+   const char *namebuf, *mailbuf;
+   int namelen, maillen;
+   const char *ac_info = read(IDENT_NO_DATE);
+
+   if (split_ident_line(ident, ac_info, strlen(ac_info)))
+   return;
+
+   namelen = ident.name_end - ident.name_begin;
+   namebuf = ident.name_begin;
+
+   maillen = ident.mail_end - ident.mail_begin;
+   mailbuf = ident.mail_begin;
+
+   argv_array_pushf(env, %s=%.*s, ac_name, namelen, namebuf);
+   argv_array_pushf(env, %s=%.*s, ac_mail, maillen, mailbuf);
+   }
+}
+
 static const char *apply_command(const char *command, const char *arg)
 {
+   struct argv_array env = ARGV_ARRAY_INIT;
struct strbuf cmd = STRBUF_INIT;
struct strbuf buf = STRBUF_INIT;
struct child_process cp;
const char *argv[] = {NULL, NULL};
const char *result = ;
 
+   setup_ac_env(env, GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, 
git_author_info);
+   setup_ac_env(env, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL, 
git_committer_info);
+
strbuf_addstr(cmd, command);
if (arg)
strbuf_replace(cmd, TRAILER_ARG_STRING, arg);
@@ -429,7 +456,7 @@ static const char *apply_command(const char *command, const 
char *arg)
argv[0] = cmd.buf;
memset(cp, 0, sizeof(cp));
cp.argv = argv;
-   cp.env = local_repo_env;
+   cp.env = env.argv;
cp.no_stdin = 1;
cp.out = -1;
cp.use_shell = 1;
@@ -440,6 +467,7 @@ static const char *apply_command(const char *command, const 
char *arg)
result = strbuf_detach(buf, NULL);
 
strbuf_release(cmd);
+   argv_array_clear(env);
return result;
 }
 
-- 
1.8.5.2.201.gacc5987


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html