A good Git technique for referring back to original files

2013-02-12 Thread MikeW
Hi,

I have a client with an SDK product. Normally the SDK is used in its unpackaged
form by the end-user, and that is the directory structure and set of files in
which development work on the SDK functionality is performed.

However the SDK directory and content is generated from a packager which first
runs on numerous other version controlled projects (currently CVS projects -
this is unlikely to change).

This means that once changes to the unpackaged SDK have been tested, they have
to be cross-referred back to the original projects and the changes ported back.

I have found it most convenient to control my in-SDK changes with git.

However it's still a royal pain to cross-refer and diff the changes back to the
originals, especially since duplicate file names exist across the original
projects which get filtered down to one relevant instance by the packager.

Since git is so good at tracking file content, I wondered whether there was any
technique using git that would simplify the back-referencing task.

Failing a method using git 'normally', perhaps building a script on top of the
git file system might be a possibility - if that is feasible ...

Thanks,
MikeW

--
To unsubscribe from this list: send the line 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: A good Git technique for referring back to original files

2013-02-12 Thread Matthieu Moy
MikeW mw_p...@yahoo.co.uk writes:

 Since git is so good at tracking file content, I wondered whether there was 
 any
 technique using git that would simplify the back-referencing task.

I'm not sure I understand the question, but if you want to add meta-data
to Git commits (e.g. this Git commit is revision 42 in CVS repository
foo), then have a look at git-notes. It won't give you directly
reference to other VCS, but at least can be used as a storage
mechanism to store these references.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC/PATCH] Replace filepattern with pathspec for consistency

2013-02-12 Thread Matthieu Moy
pathspec is the most widely used term, and is the one defined in
gitglossary.txt. filepattern was used only in the synopsys for git-add
and git-commit, and in git-add.txt. Get rid of it.

This patch is obtained with by running:

  perl -pi -e 's/filepattern/pathspec/' `git grep -l filepattern`

Signed-off-by: Matthieu Moy matthieu@imag.fr
---
I'm a bit unsure about the changes to the .po files, but I guess doing
the substitution there too does the right thing.

 Documentation/git-add.txt | 12 ++--
 builtin/add.c |  2 +-
 builtin/commit.c  |  4 ++--
 po/de.po  |  6 +++---
 po/git.pot|  6 +++---
 po/sv.po  |  6 +++---
 po/vi.po  |  6 +++---
 po/zh_CN.po   |  6 +++---
 8 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 5333559..388a225 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
  [--edit | -e] [--all | [--update | -u]] [--intent-to-add | -N]
  [--refresh] [--ignore-errors] [--ignore-missing] [--]
- [filepattern...]
+ [pathspec...]
 
 DESCRIPTION
 ---
@@ -49,7 +49,7 @@ commit.
 
 OPTIONS
 ---
-filepattern...::
+pathspec...::
Files to add content from.  Fileglobs (e.g. `*.c`) can
be given to add all matching files.  Also a
leading directory name (e.g. `dir` to add `dir/file1`
@@ -100,21 +100,21 @@ apply to the index. See EDITING PATCHES below.
 
 -u::
 --update::
-   Only match filepattern against already tracked files in
+   Only match pathspec against already tracked files in
the index rather than the working tree. That means that it
will never stage new files, but that it will stage modified
new contents of tracked files and that it will remove files
from the index if the corresponding files in the working tree
have been removed.
 +
-If no filepattern is given, the current version of Git defaults to
+If no pathspec is given, the current version of Git defaults to
 .; in other words, update all tracked files in the current directory
 and its subdirectories. This default will change in a future version
-of Git, hence the form without filepattern should not be used.
+of Git, hence the form without pathspec should not be used.
 
 -A::
 --all::
-   Like `-u`, but match filepattern against files in the
+   Like `-u`, but match pathspec against files in the
working tree in addition to the index. That means that it
will find new files as well as staging modified content and
removing files that are no longer in the working tree.
diff --git a/builtin/add.c b/builtin/add.c
index 7738025..0dd014e 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -17,7 +17,7 @@
 #include bulk-checkin.h
 
 static const char * const builtin_add_usage[] = {
-   N_(git add [options] [--] filepattern...),
+   N_(git add [options] [--] pathspec...),
NULL
 };
 static int patch_interactive, add_interactive, edit_interactive;
diff --git a/builtin/commit.c b/builtin/commit.c
index 1a0e5f1..3348aa1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -31,12 +31,12 @@
 #include sequencer.h
 
 static const char * const builtin_commit_usage[] = {
-   N_(git commit [options] [--] filepattern...),
+   N_(git commit [options] [--] pathspec...),
NULL
 };
 
 static const char * const builtin_status_usage[] = {
-   N_(git status [options] [--] filepattern...),
+   N_(git status [options] [--] pathspec...),
NULL
 };
 
diff --git a/po/de.po b/po/de.po
index c8ad2f0..0183c28 100644
--- a/po/de.po
+++ b/po/de.po
@@ -1410,7 +1410,7 @@ msgid failed to unlink '%s'
 msgstr Konnte '%s' nicht entfernen
 
 #: builtin/add.c:19
-msgid git add [options] [--] filepattern...
+msgid git add [options] [--] pathspec...
 msgstr git add [Optionen] [--] [Dateimuster...]
 
 #: builtin/add.c:62
@@ -3296,11 +3296,11 @@ msgid --command must be the first argument
 msgstr Option --command muss zuerst angegeben werden
 
 #: builtin/commit.c:34
-msgid git commit [options] [--] filepattern...
+msgid git commit [options] [--] pathspec...
 msgstr git commit [Optionen] [--] Dateimuster...
 
 #: builtin/commit.c:39
-msgid git status [options] [--] filepattern...
+msgid git status [options] [--] pathspec...
 msgstr git status [Optionen] [--] Dateimuster...
 
 #: builtin/commit.c:44
diff --git a/po/git.pot b/po/git.pot
index 430d033..4941fd7 100644
--- a/po/git.pot
+++ b/po/git.pot
@@ -1328,7 +1328,7 @@ msgid failed to unlink '%s'
 msgstr 
 
 #: builtin/add.c:19
-msgid git add [options] [--] filepattern...
+msgid git add [options] [--] pathspec...
 msgstr 
 
 #: builtin/add.c:62
@@ -3128,11 +3128,11 @@ msgid --command must be the first argument
 msgstr 
 
 #: builtin/commit.c:34
-msgid git 

[PATCH v3 1/4] git-count-objects.txt: describe each line in -v output

2013-02-12 Thread Nguyễn Thái Ngọc Duy
The current description requires a bit of guessing (what clause
corresponds to what printed line?) and lacks information, such as
the unit of size and size-pack.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git-count-objects.txt | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-count-objects.txt 
b/Documentation/git-count-objects.txt
index 23c80ce..e816823 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -20,11 +20,21 @@ OPTIONS
 ---
 -v::
 --verbose::
-   In addition to the number of loose objects and disk
-   space consumed, it reports the number of in-pack
-   objects, number of packs, disk space consumed by those packs,
-   and number of objects that can be removed by running
-   `git prune-packed`.
+   Report in more detail:
++
+count: the number of loose objects
++
+size: disk space consumed by loose objects, in KiB
++
+in-pack: the number of in-pack objects
++
+size-pack: disk space consumed by the packs, in KiB
++
+prune-packable: the number of loose objects that are also present in
+the packs. These objects could be pruned using `git prune-packed`.
++
+garbage: the number of files in loose object database that are not
+valid loose objects
 
 GIT
 ---
-- 
1.8.1.2.536.gf441e6d

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


[PATCH v3 2/4] sha1_file: reorder code in prepare_packed_git_one()

2013-02-12 Thread Nguyễn Thái Ngọc Duy
The current loop does

while (...) {
if (!not .idx file)
continue;
process .idx file;
}

and is reordered to

while (...) {
if (!.idx file) {
process .idx file;
}
}

This makes it easier to add new extension file processing.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 sha1_file.c | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 40b2329..8d7da1d 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1024,27 +1024,28 @@ static void prepare_packed_git_one(char *objdir, int 
local)
int namelen = strlen(de-d_name);
struct packed_git *p;
 
-   if (!has_extension(de-d_name, .idx))
-   continue;
-
if (len + namelen + 1  sizeof(path))
continue;
 
-   /* Don't reopen a pack we already have. */
strcpy(path + len, de-d_name);
-   for (p = packed_git; p; p = p-next) {
-   if (!memcmp(path, p-pack_name, len + namelen - 4))
-   break;
+
+   if (has_extension(de-d_name, .idx)) {
+   /* Don't reopen a pack we already have. */
+   for (p = packed_git; p; p = p-next) {
+   if (!memcmp(path, p-pack_name, len + namelen - 
4))
+   break;
+   }
+   if (p)
+   continue;
+   /*
+* See if it really is a valid .idx file with
+* corresponding .pack file that we can map.
+*/
+   p = add_packed_git(path, len + namelen, local);
+   if (!p)
+   continue;
+   install_packed_git(p);
}
-   if (p)
-   continue;
-   /* See if it really is a valid .idx file with corresponding
-* .pack file that we can map.
-*/
-   p = add_packed_git(path, len + namelen, local);
-   if (!p)
-   continue;
-   install_packed_git(p);
}
closedir(dir);
 }
-- 
1.8.1.2.536.gf441e6d

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


[PATCH v3 3/4] count-objects: report garbage files in pack directory too

2013-02-12 Thread Nguyễn Thái Ngọc Duy
prepare_packed_git_one() is modified to allow count-objects to hook a
report function to so we don't need to duplicate the pack searching
logic in count-objects.c. When report_pack_garbage is NULL, the
overhead is insignificant.

The garbage is reported with warning() instead of error() in packed
garbage case because it's not an error to have garbage. Loose garbage
is still reported as errors and will be converted to warnings later.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git-count-objects.txt |  4 +-
 builtin/count-objects.c | 22 +-
 cache.h |  3 ++
 sha1_file.c | 83 -
 4 files changed, 107 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-count-objects.txt 
b/Documentation/git-count-objects.txt
index e816823..1611d7c 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -33,8 +33,8 @@ size-pack: disk space consumed by the packs, in KiB
 prune-packable: the number of loose objects that are also present in
 the packs. These objects could be pruned using `git prune-packed`.
 +
-garbage: the number of files in loose object database that are not
-valid loose objects
+garbage: the number of files in object database that are not valid
+loose objects nor valid packs
 
 GIT
 ---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 9afaa88..639c9a5 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -9,6 +9,24 @@
 #include builtin.h
 #include parse-options.h
 
+static unsigned long garbage;
+
+static void real_report_garbage(const char *desc,
+   const char *path, int len,
+   const char *name)
+{
+   struct strbuf sb = STRBUF_INIT;
+   if (len  name)
+   strbuf_addf(sb, %.*s/%s, len, path, name);
+   else if (!len  name)
+   strbuf_addf(sb, %s%s, path, name);
+   else
+   strbuf_addf(sb, %s, path);
+   warning(%s: %s, desc, sb.buf);
+   garbage++;
+   strbuf_release(sb);
+}
+
 static void count_objects(DIR *d, char *path, int len, int verbose,
  unsigned long *loose,
  off_t *loose_size,
@@ -76,7 +94,7 @@ int cmd_count_objects(int argc, const char **argv, const char 
*prefix)
const char *objdir = get_object_directory();
int len = strlen(objdir);
char *path = xmalloc(len + 50);
-   unsigned long loose = 0, packed = 0, packed_loose = 0, garbage = 0;
+   unsigned long loose = 0, packed = 0, packed_loose = 0;
off_t loose_size = 0;
struct option opts[] = {
OPT__VERBOSE(verbose, N_(be verbose)),
@@ -87,6 +105,8 @@ int cmd_count_objects(int argc, const char **argv, const 
char *prefix)
/* we do not take arguments other than flags for now */
if (argc)
usage_with_options(count_objects_usage, opts);
+   if (verbose)
+   report_garbage = real_report_garbage;
memcpy(path, objdir, len);
if (len  objdir[len-1] != '/')
path[len++] = '/';
diff --git a/cache.h b/cache.h
index 7339f21..e486499 100644
--- a/cache.h
+++ b/cache.h
@@ -1051,6 +1051,9 @@ extern const char *parse_feature_value(const char 
*feature_list, const char *fea
 
 extern struct packed_git *parse_pack_index(unsigned char *sha1, const char 
*idx_path);
 
+/* A hook for count-objects to report invalid files in pack directory */
+extern void (*report_garbage)(const char *desc, const char *path, int len, 
const char *name);
+
 extern void prepare_packed_git(void);
 extern void reprepare_packed_git(void);
 extern void install_packed_git(struct packed_git *pack);
diff --git a/sha1_file.c b/sha1_file.c
index 8d7da1d..290e348 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -21,6 +21,7 @@
 #include sha1-lookup.h
 #include bulk-checkin.h
 #include streaming.h
+#include dir.h
 
 #ifndef O_NOATIME
 #if defined(__linux__)  (defined(__i386__) || defined(__PPC__))
@@ -1000,6 +1001,52 @@ void install_packed_git(struct packed_git *pack)
packed_git = pack;
 }
 
+void (*report_garbage)(const char *desc, const char *path,
+  int len, const char *name);
+
+static void report_pack_garbage(struct string_list *list)
+{
+   struct strbuf sb = STRBUF_INIT;
+   struct packed_git *p;
+   int i;
+
+   if (!report_garbage)
+   return;
+
+   sort_string_list(list);
+
+   for (p = packed_git; p; p = p-next) {
+   struct string_list_item *item;
+   if (!p-pack_local)
+   continue;
+   strbuf_reset(sb);
+   strbuf_add(sb, p-pack_name,
+  strlen(p-pack_name) - 5); /* .pack */
+   item = string_list_lookup(list, sb.buf);
+   if (!item)
+   continue;
+   /*
+ 

[PATCH v3 4/4] count-objects: report how much disk space taken by garbage files

2013-02-12 Thread Nguyễn Thái Ngọc Duy
Also issue warnings on loose garbages instead of errors as a result of
using report_garbage() function in count_objects()

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git-count-objects.txt |  2 ++
 builtin/count-objects.c | 21 +
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-count-objects.txt 
b/Documentation/git-count-objects.txt
index 1611d7c..da6e72e 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -35,6 +35,8 @@ the packs. These objects could be pruned using `git 
prune-packed`.
 +
 garbage: the number of files in object database that are not valid
 loose objects nor valid packs
++
+size-garbage: disk space consumed by garbage files, in KiB
 
 GIT
 ---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 639c9a5..75feee5 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -10,12 +10,15 @@
 #include parse-options.h
 
 static unsigned long garbage;
+static off_t size_garbage;
 
 static void real_report_garbage(const char *desc,
const char *path, int len,
const char *name)
 {
struct strbuf sb = STRBUF_INIT;
+   struct stat st;
+
if (len  name)
strbuf_addf(sb, %.*s/%s, len, path, name);
else if (!len  name)
@@ -23,6 +26,10 @@ static void real_report_garbage(const char *desc,
else
strbuf_addf(sb, %s, path);
warning(%s: %s, desc, sb.buf);
+
+   if (!stat(sb.buf, st))
+   size_garbage += st.st_size;
+
garbage++;
strbuf_release(sb);
 }
@@ -30,8 +37,7 @@ static void real_report_garbage(const char *desc,
 static void count_objects(DIR *d, char *path, int len, int verbose,
  unsigned long *loose,
  off_t *loose_size,
- unsigned long *packed_loose,
- unsigned long *garbage)
+ unsigned long *packed_loose)
 {
struct dirent *ent;
while ((ent = readdir(d)) != NULL) {
@@ -63,11 +69,9 @@ static void count_objects(DIR *d, char *path, int len, int 
verbose,
(*loose_size) += xsize_t(on_disk_bytes(st));
}
if (bad) {
-   if (verbose) {
-   error(garbage found: %.*s/%s,
- len + 2, path, ent-d_name);
-   (*garbage)++;
-   }
+   if (verbose)
+   report_garbage(garbage found,
+  path, len + 2, ent-d_name);
continue;
}
(*loose)++;
@@ -117,7 +121,7 @@ int cmd_count_objects(int argc, const char **argv, const 
char *prefix)
if (!d)
continue;
count_objects(d, path, len, verbose,
- loose, loose_size, packed_loose, garbage);
+ loose, loose_size, packed_loose);
closedir(d);
}
if (verbose) {
@@ -142,6 +146,7 @@ int cmd_count_objects(int argc, const char **argv, const 
char *prefix)
printf(size-pack: %lu\n, (unsigned long) (size_pack / 1024));
printf(prune-packable: %lu\n, packed_loose);
printf(garbage: %lu\n, garbage);
+   printf(size-garbage: %lu\n, (unsigned long) (size_garbage / 
1024));
}
else
printf(%lu objects, %lu kilobytes\n,
-- 
1.8.1.2.536.gf441e6d

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


[PATCH v3 0/4] count-objects improvements

2013-02-12 Thread Nguyễn Thái Ngọc Duy
Compared to v2 [1], this version
 - fixes sparse warning
 - restructures 2/3 (now 3/4) to make it easier to read
 - report path too long instead of garbage found in
   .git/path/too/long/pack-xxx.pack case
 - changes output prefix error: to warning:

[1] http://thread.gmane.org/gmane.comp.version-control.git/215378/focus=215744

Nguyễn Thái Ngọc Duy (4):
  git-count-objects.txt: describe each line in -v output
  sha1_file: reorder code in prepare_packed_git_one()
  count-objects: report garbage files in pack directory too
  count-objects: report how much disk space taken by garbage files

 Documentation/git-count-objects.txt |  22 ++--
 builtin/count-objects.c |  43 +++---
 cache.h |   3 +
 sha1_file.c | 108 +++-
 4 files changed, 148 insertions(+), 28 deletions(-)

-- 
1.8.1.2.536.gf441e6d

--
To unsubscribe from this list: send the line 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] rebase -i: respect core.commentchar

2013-02-12 Thread John Keeping
On Mon, Feb 11, 2013 at 04:13:31PM -0800, Junio C Hamano wrote:
 John Keeping j...@keeping.me.uk writes:
 
  @@ -179,7 +182,9 @@ die_abort () {
   }
   
   has_action () {
  -   sane_grep '^[^#]' $1 /dev/null
  +   echo space stripped actions: 2
  +   git stripspace --strip-comments $1 2
  +   test -n $(git stripspace --strip-comments $1)
   }
 
 I'll remove the debugging remnants while queuing.

Thanks.  I don't think I was fully awake when I finished this last
night - the following fixup is also needed to avoid relying on the shell
emitting a literal backslash when a backslash isn't followed by a known
escape character.

-- 8 --

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index cbe36bf..84bd525 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -947,7 +947,7 @@ test_expect_success 'rebase -i respects core.commentchar' '
test_when_finished git config --unset core.commentchar 
cat comment-lines.sh EOF 
 #!$SHELL_PATH
-sed -e 2,\$ s/^/\\/ \$1 \$1.tmp
+sed -e 2,\$ s/^// \$1 \$1.tmp
 mv \$1.tmp \$1
 EOF
chmod a+x comment-lines.sh 

-- 8 --

  @@ -942,20 +948,18 @@ test -s $todo || echo noop  $todo
   test -n $autosquash  rearrange_squash $todo
   test -n $cmd  add_exec_commands $todo
   
  -cat  $todo  EOF
  -
  -# Rebase $shortrevisions onto $shortonto
  -EOF
  +echo $todo
  +printf '%s\n' $comment_char Rebase $shortrevisions onto $shortonto 
  $todo
 
 I think you can still do
 
   cat $todo EOF
 
   $comment_char Rebase $shortrevisions onto...
   EOF
 
 here with any funny comment character.  Doing this with two separate
 I/O does not hurt very much, but the resulting code may be easier to
 scan if left as here-text with a single cat.
 
 Please eyeball what is in 'pu' (I have a separate squashable fixup
 on top of your patch) and let me know if I made mistakes.

The fixup commit looks good to me.


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


[PATCH v4 00/12] unify appending of sob

2013-02-12 Thread Brandon Casey
Round 4.

Interdiff against round 3 follows the diff stat.

-Brandon

Brandon Casey (9):
  commit, cherry-pick -s: remove broken support for multiline rfc2822
fields
  t/test-lib-functions.sh: allow to specify the tag name to test_commit
  t/t3511: add some tests of 'cherry-pick -s' functionality
  sequencer.c: recognize (cherry picked from ... as part of s-o-b
footer
  sequencer.c: require a conforming footer to be preceded by a blank
line
  sequencer.c: always separate (cherry picked from from commit body
  sequencer.c: teach append_signoff how to detect duplicate s-o-b
  sequencer.c: teach append_signoff to avoid adding a duplicate newline
  Unify appending signoff in format-patch, commit and sequencer

Jonathan Nieder (1):
  sequencer.c: rework search for start of footer to improve clarity

Nguyễn Thái Ngọc Duy (2):
  t4014: more tests about appending s-o-b lines
  format-patch: update append_signoff prototype

 builtin/commit.c |   2 +-
 builtin/log.c|  13 +--
 log-tree.c   |  92 ++---
 revision.h   |   2 +-
 sequencer.c  | 168 +-
 sequencer.h  |   4 +-
 t/t3511-cherry-pick-x.sh | 219 +++
 t/t4014-format-patch.sh  | 262 +++
 t/test-lib-functions.sh  |   8 +-
 9 files changed, 614 insertions(+), 156 deletions(-)
 create mode 100755 t/t3511-cherry-pick-x.sh

-- 
1.8.1.3.579.gd9af3b6

diff --git a/sequencer.c b/sequencer.c
index 404b786..3c63e3a 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -27,13 +27,12 @@ static int is_rfc2822_line(const char *buf, int len)
for (i = 0; i  len; i++) {
int ch = buf[i];
if (ch == ':')
+   return 1;
+   if (!isalnum(ch)  ch != '-')
break;
-   if (isalnum(ch) || (ch == '-'))
-   continue;
-   return 0;
}
 
-   return 1;
+   return 0;
 }
 
 static int is_cherry_picked_from_line(const char *buf, int len)
@@ -41,9 +40,8 @@ static int is_cherry_picked_from_line(const char *buf, int 
len)
/*
 * We only care that it looks roughly like (cherry picked from ...)
 */
-   return !prefixcmp(buf, cherry_picked_prefix) 
-   (buf[len - 1] == ')' ||
-(buf[len - 1] == '\n'  buf[len - 2] == ')'));
+   return len  strlen(cherry_picked_prefix) + 1 
+   !prefixcmp(buf, cherry_picked_prefix)  buf[len - 1] == ')';
 }
 
 /*
@@ -55,25 +53,29 @@ static int is_cherry_picked_from_line(const char *buf, int 
len)
 static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
int ignore_footer)
 {
-   int last_char_was_nl, this_char_is_nl;
+   char prev;
int i, k;
int len = sb-len - ignore_footer;
const char *buf = sb-buf;
int found_sob = 0;
 
-   /* find start of last paragraph */
-   last_char_was_nl = 0;
+   /* footer must end with newline */
+   if (!len || buf[len - 1] != '\n')
+   return 0;
+
+   prev = '\0';
for (i = len - 1; i  0; i--) {
-   this_char_is_nl = (buf[i] == '\n');
-   if (last_char_was_nl  this_char_is_nl)
+   char ch = buf[i];
+   if (prev == '\n'  ch == '\n') /* paragraph break */
break;
-   last_char_was_nl = this_char_is_nl;
+   prev = ch;
}
 
/* require at least one blank line */
-   if (!last_char_was_nl || buf[i] != '\n')
+   if (prev != '\n' || buf[i] != '\n')
return 0;
 
+   /* advance to start of last paragraph */
while (i  len - 1  buf[i] == '\n')
i++;
 
@@ -84,13 +86,13 @@ static int has_conforming_footer(struct strbuf *sb, struct 
strbuf *sob,
; /* do nothing */
k++;
 
-   found_rfc2822 = is_rfc2822_line(buf + i, k - i);
+   found_rfc2822 = is_rfc2822_line(buf + i, k - i - 1);
if (found_rfc2822  sob 
-   !strncmp(buf + i, sob-buf, sob-len))
+   !strncmp(buf + i, sob-buf, sob-len))
found_sob = k;
 
if (!(found_rfc2822 ||
-   is_cherry_picked_from_line(buf + i, k - i)))
+ is_cherry_picked_from_line(buf + i, k - i - 1)))
return 0;
}
if (found_sob == i)
@@ -1108,26 +1110,36 @@ void append_signoff(struct strbuf *msgbuf, int 
ignore_footer, unsigned flag)
 {
unsigned no_dup_sob = flag  APPEND_SIGNOFF_DEDUP;
struct strbuf sob = STRBUF_INIT;
-   int has_footer = 0;
-   int i;
+   const char *append_newlines = NULL;
+   int has_footer;
 
strbuf_addstr(sob, sign_off_header);
strbuf_addstr(sob, 

[PATCH v4 01/12] sequencer.c: rework search for start of footer to improve clarity

2013-02-12 Thread Brandon Casey
From: Jonathan Nieder jrnie...@gmail.com

This code sequence is somewhat difficult to read.  Let's rewrite it and add
some comments to improve clarity.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 sequencer.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index aef5e8a..dbeff01 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1023,19 +1023,21 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 
 static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
 {
-   int ch;
-   int hit = 0;
+   char ch, prev;
int i, j, k;
int len = sb-len - ignore_footer;
int first = 1;
const char *buf = sb-buf;
 
+   prev = '\0';
for (i = len - 1; i  0; i--) {
-   if (hit  buf[i] == '\n')
+   ch = buf[i];
+   if (prev == '\n'  ch == '\n') /* paragraph break */
break;
-   hit = (buf[i] == '\n');
+   prev = ch;
}
 
+   /* advance to start of last paragraph */
while (i  len - 1  buf[i] == '\n')
i++;
 
-- 
1.8.1.3.579.gd9af3b6

--
To unsubscribe from this list: send the line 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/12] commit, cherry-pick -s: remove broken support for multiline rfc2822 fields

2013-02-12 Thread Brandon Casey
Starting with c1e01b0c (commit: More generous accepting of RFC-2822 footer
lines, 2009-10-28), git commit -s carefully parses the last paragraph of
each commit message to check if it consists only of RFC2822-style headers,
in which case the signoff will be added as a new line in the same list:

   Reported-by: Reporter repor...@example.com
   Signed-off-by: Author aut...@example.com
   Acked-by: Lieutenant l...@example.com

It even included support for accepting indented continuation lines for
multiline fields.  Unfortunately the multiline field support is broken
because it checks whether buf[k] (the first character of the *next* line)
instead of buf[i] is a whitespace character.  The result is that any footer
with a continuation line is not accepted, since the last continuation line
neither starts with an RFC2822 field name nor is followed by a continuation
line.

That this has remained broken for so long is good evidence that nobody
actually needed multiline fields.  Rip out the broken continuation support.

There should be no functional change.

[Thanks to Jonathan Nieder for the excellent commit message]

Signed-off-by: Brandon Casey bca...@nvidia.com
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
---
 sequencer.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index dbeff01..aa2cb8e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1026,7 +1026,6 @@ static int ends_rfc2822_footer(struct strbuf *sb, int 
ignore_footer)
char ch, prev;
int i, j, k;
int len = sb-len - ignore_footer;
-   int first = 1;
const char *buf = sb-buf;
 
prev = '\0';
@@ -1046,11 +1045,6 @@ static int ends_rfc2822_footer(struct strbuf *sb, int 
ignore_footer)
; /* do nothing */
k++;
 
-   if ((buf[k] == ' ' || buf[k] == '\t')  !first)
-   continue;
-
-   first = 0;
-
for (j = 0; i + j  len; j++) {
ch = buf[i + j];
if (ch == ':')
-- 
1.8.1.3.579.gd9af3b6

--
To unsubscribe from this list: send the line 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/12] t/test-lib-functions.sh: allow to specify the tag name to test_commit

2013-02-12 Thread Brandon Casey
The message part of test_commit() may not be appropriate for a tag name.
So let's allow test_commit to accept a fourth argument to specify the tag
name.

Signed-off-by: Brandon Casey bca...@nvidia.com
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
---
 t/test-lib-functions.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index fa62d01..61d0804 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -135,12 +135,12 @@ test_pause () {
fi
 }
 
-# Call test_commit with the arguments message [file [contents]]
+# Call test_commit with the arguments message [file [contents [tag]]]
 #
 # This will commit a file with the given contents and the given commit
-# message.  It will also add a tag with message as name.
+# message, and tag the resulting commit with the given tag name.
 #
-# Both file and contents default to message.
+# file, contents, and tag all default to message.
 
 test_commit () {
notick= 
@@ -168,7 +168,7 @@ test_commit () {
test_tick
fi 
git commit $signoff -m $1 
-   git tag $1
+   git tag ${4:-$1}
 }
 
 # Call test_merge with the arguments message commit, where commit
-- 
1.8.1.3.579.gd9af3b6

--
To unsubscribe from this list: send the line 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/12] t/t3511: add some tests of 'cherry-pick -s' functionality

2013-02-12 Thread Brandon Casey
Add some tests to ensure that 'cherry-pick -s' operates in the following
manner:

   * Inserts a blank line before appending a s-o-b to a commit message that
 does not contain a s-o-b footer

   * Does not mistake first line subject: description as a s-o-b footer

   * Does not mistake single word message body as conforming to rfc2822

   * Appends a s-o-b when last s-o-b in footer does not match committer
 s-o-b, even when committer's s-o-b exists elsewhere in footer.

   * Does not append a s-o-b when last s-o-b matches committer s-o-b

   * Correctly detects a non-conforming footer containing a mix of s-o-b
 like elements and s-o-b elements. (marked expect failure)

Signed-off-by: Brandon Casey bca...@nvidia.com
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
---
 t/t3511-cherry-pick-x.sh | 111 +++
 1 file changed, 111 insertions(+)
 create mode 100755 t/t3511-cherry-pick-x.sh

diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
new file mode 100755
index 000..2a040b7
--- /dev/null
+++ b/t/t3511-cherry-pick-x.sh
@@ -0,0 +1,111 @@
+#!/bin/sh
+
+test_description='Test cherry-pick -x and -s'
+
+. ./test-lib.sh
+
+pristine_detach () {
+   git cherry-pick --quit 
+   git checkout -f $1^0 
+   git read-tree -u --reset HEAD 
+   git clean -d -f -f -q -x
+}
+
+mesg_one_line='base: commit message'
+
+mesg_no_footer=$mesg_one_line
+
+OneWordBodyThatsNotA-S-o-B
+
+mesg_with_footer=$mesg_no_footer
+
+Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL
+Signed-off-by: A.U. Thor aut...@example.com
+Signed-off-by: B.U. Thor but...@example.com
+
+mesg_broken_footer=$mesg_no_footer
+
+The signed-off-by string should begin with the words Signed-off-by followed
+by a colon and space, and then the signers name and email address. e.g.
+Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL
+
+mesg_with_footer_sob=$mesg_with_footer
+Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL
+
+
+test_expect_success setup '
+   git config advice.detachedhead false 
+   echo unrelated unrelated 
+   git add unrelated 
+   test_commit initial foo a 
+   test_commit $mesg_one_line foo b mesg-one-line 
+   git reset --hard initial 
+   test_commit $mesg_no_footer foo b mesg-no-footer 
+   git reset --hard initial 
+   test_commit $mesg_broken_footer foo b mesg-broken-footer 
+   git reset --hard initial 
+   test_commit $mesg_with_footer foo b mesg-with-footer 
+   git reset --hard initial 
+   test_commit $mesg_with_footer_sob foo b mesg-with-footer-sob 
+   pristine_detach initial 
+   test_commit conflicting unrelated
+'
+
+test_expect_success 'cherry-pick -s inserts blank line after one line subject' 
'
+   pristine_detach initial 
+   git cherry-pick -s mesg-one-line 
+   cat -EOF expect 
+   $mesg_one_line
+
+   Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL
+   EOF
+   git log -1 --pretty=format:%B actual 
+   test_cmp expect actual
+'
+
+test_expect_failure 'cherry-pick -s inserts blank line after non-conforming 
footer' '
+   pristine_detach initial 
+   git cherry-pick -s mesg-broken-footer 
+   cat -EOF expect 
+   $mesg_broken_footer
+
+   Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL
+   EOF
+   git log -1 --pretty=format:%B actual 
+   test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -s inserts blank line when conforming footer 
not found' '
+   pristine_detach initial 
+   git cherry-pick -s mesg-no-footer 
+   cat -EOF expect 
+   $mesg_no_footer
+
+   Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL
+   EOF
+   git log -1 --pretty=format:%B actual 
+   test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -s adds sob when last sob doesnt match 
committer' '
+   pristine_detach initial 
+   git cherry-pick -s mesg-with-footer 
+   cat -EOF expect 
+   $mesg_with_footer
+   Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL
+   EOF
+   git log -1 --pretty=format:%B actual 
+   test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -s refrains from adding duplicate trailing 
sob' '
+   pristine_detach initial 
+   git cherry-pick -s mesg-with-footer-sob 
+   cat -EOF expect 
+   $mesg_with_footer_sob
+   EOF
+   git log -1 --pretty=format:%B actual 
+   test_cmp expect actual
+'
+
+test_done
-- 
1.8.1.3.579.gd9af3b6

--
To unsubscribe from this list: send the line 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/12] sequencer.c: recognize (cherry picked from ... as part of s-o-b footer

2013-02-12 Thread Brandon Casey
When 'cherry-pick -s' is used to append a signed-off-by line to a cherry
picked commit, it does not currently detect the (cherry picked from...
that may have been appended by a previous 'cherry-pick -x' as part of the
s-o-b footer and it will insert a blank line before appending a new s-o-b.

Let's detect (cherry picked from...) as part of the footer so that we
will produce this:

   Signed-off-by: A U Thor aut...@example.com
   (cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)
   Signed-off-by: C O Mmitter commit...@example.com

instead of this:

   Signed-off-by: A U Thor aut...@example.com
   (cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)

   Signed-off-by: C O Mmitter commit...@example.com

[with improvements from Jonathan Nieder]

Signed-off-by: Brandon Casey bca...@nvidia.com
Acked-by: Jonathan Nieder jrnie...@gmail.com
---
 sequencer.c  | 51 
 t/t3511-cherry-pick-x.sh | 55 
 2 files changed, 92 insertions(+), 14 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index aa2cb8e..93495b0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -18,6 +18,7 @@
 #define GIT_REFLOG_ACTION GIT_REFLOG_ACTION
 
 const char sign_off_header[] = Signed-off-by: ;
+static const char cherry_picked_prefix[] = (cherry picked from commit ;
 
 static void remove_sequencer_state(void)
 {
@@ -496,7 +497,7 @@ static int do_pick_commit(struct commit *commit, struct 
replay_opts *opts)
}
 
if (opts-record_origin) {
-   strbuf_addstr(msgbuf, (cherry picked from commit );
+   strbuf_addstr(msgbuf, cherry_picked_prefix);
strbuf_addstr(msgbuf, 
sha1_to_hex(commit-object.sha1));
strbuf_addstr(msgbuf, )\n);
}
@@ -1021,16 +1022,44 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return pick_commits(todo_list, opts);
 }
 
-static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
+static int is_rfc2822_line(const char *buf, int len)
 {
-   char ch, prev;
-   int i, j, k;
+   int i;
+
+   for (i = 0; i  len; i++) {
+   int ch = buf[i];
+   if (ch == ':')
+   return 1;
+   if (!isalnum(ch)  ch != '-')
+   break;
+   }
+
+   return 0;
+}
+
+static int is_cherry_picked_from_line(const char *buf, int len)
+{
+   /*
+* We only care that it looks roughly like (cherry picked from ...)
+*/
+   return len  strlen(cherry_picked_prefix) + 1 
+   !prefixcmp(buf, cherry_picked_prefix)  buf[len - 1] == ')';
+}
+
+static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
+{
+   char prev;
+   int i, k;
int len = sb-len - ignore_footer;
const char *buf = sb-buf;
 
+   /* footer must end with newline */
+   if (!len || buf[len - 1] != '\n')
+   return 0;
+
prev = '\0';
for (i = len - 1; i  0; i--) {
-   ch = buf[i];
+   char ch = buf[i];
if (prev == '\n'  ch == '\n') /* paragraph break */
break;
prev = ch;
@@ -1045,15 +1074,9 @@ static int ends_rfc2822_footer(struct strbuf *sb, int 
ignore_footer)
; /* do nothing */
k++;
 
-   for (j = 0; i + j  len; j++) {
-   ch = buf[i + j];
-   if (ch == ':')
-   break;
-   if (isalnum(ch) ||
-   (ch == '-'))
-   continue;
+   if (!is_rfc2822_line(buf + i, k - i - 1) 
+   !is_cherry_picked_from_line(buf + i, k - i - 1))
return 0;
-   }
}
return 1;
 }
@@ -1070,7 +1093,7 @@ void append_signoff(struct strbuf *msgbuf, int 
ignore_footer)
for (i = msgbuf-len - 1 - ignore_footer; i  0  msgbuf-buf[i - 1] 
!= '\n'; i--)
; /* do nothing */
if (prefixcmp(msgbuf-buf + i, sob.buf)) {
-   if (!i || !ends_rfc2822_footer(msgbuf, ignore_footer))
+   if (!i || !has_conforming_footer(msgbuf, ignore_footer))
strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0, 
\n, 1);
strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0, sob.buf, 
sob.len);
}
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index 2a040b7..73da182 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -32,6 +32,10 @@ Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL
 mesg_with_footer_sob=$mesg_with_footer
 Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL
 
+mesg_with_cherry_footer=$mesg_with_footer_sob
+(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)
+Tested-by: 

[PATCH v4 06/12] sequencer.c: require a conforming footer to be preceded by a blank line

2013-02-12 Thread Brandon Casey
Currently, append_signoff() performs a search for the last line of the
commit buffer by searching back from the end until it hits a newline.  If
it reaches the beginning of the buffer without finding a newline, that
means either the commit message was empty, or there was only one line in it.
In this case, append_signoff will skip the call to has_conforming_footer
since it already knows that it is necessary to append a newline before
appending the sob.

Let's perform this function inside of has_conforming_footer where it
appropriately belongs and generalize it so that we require that the
footer paragraph be an actual distinct paragraph separated by a blank
line.

Signed-off-by: Brandon Casey draf...@gmail.com
---
 sequencer.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index 93495b0..178e84b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1065,6 +1065,10 @@ static int has_conforming_footer(struct strbuf *sb, int 
ignore_footer)
prev = ch;
}
 
+   /* require at least one blank line */
+   if (prev != '\n' || buf[i] != '\n')
+   return 0;
+
/* advance to start of last paragraph */
while (i  len - 1  buf[i] == '\n')
i++;
@@ -1093,7 +1097,7 @@ void append_signoff(struct strbuf *msgbuf, int 
ignore_footer)
for (i = msgbuf-len - 1 - ignore_footer; i  0  msgbuf-buf[i - 1] 
!= '\n'; i--)
; /* do nothing */
if (prefixcmp(msgbuf-buf + i, sob.buf)) {
-   if (!i || !has_conforming_footer(msgbuf, ignore_footer))
+   if (!has_conforming_footer(msgbuf, ignore_footer))
strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0, 
\n, 1);
strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0, sob.buf, 
sob.len);
}
-- 
1.8.1.3.579.gd9af3b6

--
To unsubscribe from this list: send the line 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/12] sequencer.c: always separate (cherry picked from from commit body

2013-02-12 Thread Brandon Casey
Start treating the (cherry picked from line added by cherry-pick -x
the same way that the s-o-b lines are treated.  Namely, separate them
from the main commit message body with an empty line.

Introduce tests to test this functionality.

Signed-off-by: Brandon Casey bca...@nvidia.com
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
---
 sequencer.c  | 128 ---
 t/t3511-cherry-pick-x.sh |  53 
 2 files changed, 118 insertions(+), 63 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 178e84b..249c4a0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -20,6 +20,69 @@
 const char sign_off_header[] = Signed-off-by: ;
 static const char cherry_picked_prefix[] = (cherry picked from commit ;
 
+static int is_rfc2822_line(const char *buf, int len)
+{
+   int i;
+
+   for (i = 0; i  len; i++) {
+   int ch = buf[i];
+   if (ch == ':')
+   return 1;
+   if (!isalnum(ch)  ch != '-')
+   break;
+   }
+
+   return 0;
+}
+
+static int is_cherry_picked_from_line(const char *buf, int len)
+{
+   /*
+* We only care that it looks roughly like (cherry picked from ...)
+*/
+   return len  strlen(cherry_picked_prefix) + 1 
+   !prefixcmp(buf, cherry_picked_prefix)  buf[len - 1] == ')';
+}
+
+static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
+{
+   char prev;
+   int i, k;
+   int len = sb-len - ignore_footer;
+   const char *buf = sb-buf;
+
+   /* footer must end with newline */
+   if (!len || buf[len - 1] != '\n')
+   return 0;
+
+   prev = '\0';
+   for (i = len - 1; i  0; i--) {
+   char ch = buf[i];
+   if (prev == '\n'  ch == '\n') /* paragraph break */
+   break;
+   prev = ch;
+   }
+
+   /* require at least one blank line */
+   if (prev != '\n' || buf[i] != '\n')
+   return 0;
+
+   /* advance to start of last paragraph */
+   while (i  len - 1  buf[i] == '\n')
+   i++;
+
+   for (; i  len; i = k) {
+   for (k = i; k  len  buf[k] != '\n'; k++)
+   ; /* do nothing */
+   k++;
+
+   if (!is_rfc2822_line(buf + i, k - i - 1) 
+   !is_cherry_picked_from_line(buf + i, k - i - 1))
+   return 0;
+   }
+   return 1;
+}
+
 static void remove_sequencer_state(void)
 {
struct strbuf seq_dir = STRBUF_INIT;
@@ -497,6 +560,8 @@ static int do_pick_commit(struct commit *commit, struct 
replay_opts *opts)
}
 
if (opts-record_origin) {
+   if (!has_conforming_footer(msgbuf, 0))
+   strbuf_addch(msgbuf, '\n');
strbuf_addstr(msgbuf, cherry_picked_prefix);
strbuf_addstr(msgbuf, 
sha1_to_hex(commit-object.sha1));
strbuf_addstr(msgbuf, )\n);
@@ -1022,69 +1087,6 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return pick_commits(todo_list, opts);
 }
 
-static int is_rfc2822_line(const char *buf, int len)
-{
-   int i;
-
-   for (i = 0; i  len; i++) {
-   int ch = buf[i];
-   if (ch == ':')
-   return 1;
-   if (!isalnum(ch)  ch != '-')
-   break;
-   }
-
-   return 0;
-}
-
-static int is_cherry_picked_from_line(const char *buf, int len)
-{
-   /*
-* We only care that it looks roughly like (cherry picked from ...)
-*/
-   return len  strlen(cherry_picked_prefix) + 1 
-   !prefixcmp(buf, cherry_picked_prefix)  buf[len - 1] == ')';
-}
-
-static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
-{
-   char prev;
-   int i, k;
-   int len = sb-len - ignore_footer;
-   const char *buf = sb-buf;
-
-   /* footer must end with newline */
-   if (!len || buf[len - 1] != '\n')
-   return 0;
-
-   prev = '\0';
-   for (i = len - 1; i  0; i--) {
-   char ch = buf[i];
-   if (prev == '\n'  ch == '\n') /* paragraph break */
-   break;
-   prev = ch;
-   }
-
-   /* require at least one blank line */
-   if (prev != '\n' || buf[i] != '\n')
-   return 0;
-
-   /* advance to start of last paragraph */
-   while (i  len - 1  buf[i] == '\n')
-   i++;
-
-   for (; i  len; i = k) {
-   for (k = i; k  len  buf[k] != '\n'; k++)
-   ; /* do nothing */
-   k++;
-
-   if (!is_rfc2822_line(buf + i, k - i - 1) 
-   !is_cherry_picked_from_line(buf + i, k - i - 1))
-   return 0;
-   }
-   return 1;
-}
-
 void append_signoff(struct strbuf *msgbuf, int 

[PATCH v4 08/12] sequencer.c: teach append_signoff how to detect duplicate s-o-b

2013-02-12 Thread Brandon Casey
Teach append_signoff how to detect a duplicate s-o-b in the commit footer.
This is in preparation to unify the append_signoff implementations in
log-tree.c and sequencer.c.

Fixes test in t3511.

Signed-off-by: Brandon Casey bca...@nvidia.com
---
 builtin/commit.c |  2 +-
 sequencer.c  | 59 
 sequencer.h  |  4 +++-
 t/t3511-cherry-pick-x.sh |  2 +-
 4 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 1a0e5f1..94c96b7 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -700,7 +700,7 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
previous = eol;
}
 
-   append_signoff(sb, ignore_footer);
+   append_signoff(sb, ignore_footer, 0);
}
 
if (fwrite(sb.buf, 1, sb.len, s-fp)  sb.len)
diff --git a/sequencer.c b/sequencer.c
index 249c4a0..3364faa 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -44,12 +44,20 @@ static int is_cherry_picked_from_line(const char *buf, int 
len)
!prefixcmp(buf, cherry_picked_prefix)  buf[len - 1] == ')';
 }
 
-static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
+/*
+ * Returns 0 for non-conforming footer
+ * Returns 1 for conforming footer
+ * Returns 2 when sob exists within conforming footer
+ * Returns 3 when sob exists within conforming footer as last entry
+ */
+static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
+   int ignore_footer)
 {
char prev;
int i, k;
int len = sb-len - ignore_footer;
const char *buf = sb-buf;
+   int found_sob = 0;
 
/* footer must end with newline */
if (!len || buf[len - 1] != '\n')
@@ -72,14 +80,25 @@ static int has_conforming_footer(struct strbuf *sb, int 
ignore_footer)
i++;
 
for (; i  len; i = k) {
+   int found_rfc2822;
+
for (k = i; k  len  buf[k] != '\n'; k++)
; /* do nothing */
k++;
 
-   if (!is_rfc2822_line(buf + i, k - i - 1) 
-   !is_cherry_picked_from_line(buf + i, k - i - 1))
+   found_rfc2822 = is_rfc2822_line(buf + i, k - i - 1);
+   if (found_rfc2822  sob 
+   !strncmp(buf + i, sob-buf, sob-len))
+   found_sob = k;
+
+   if (!(found_rfc2822 ||
+ is_cherry_picked_from_line(buf + i, k - i - 1)))
return 0;
}
+   if (found_sob == i)
+   return 3;
+   if (found_sob)
+   return 2;
return 1;
 }
 
@@ -301,7 +320,7 @@ static int do_recursive_merge(struct commit *base, struct 
commit *next,
rollback_lock_file(index_lock);
 
if (opts-signoff)
-   append_signoff(msgbuf, 0);
+   append_signoff(msgbuf, 0, 0);
 
if (!clean) {
int i;
@@ -560,7 +579,7 @@ static int do_pick_commit(struct commit *commit, struct 
replay_opts *opts)
}
 
if (opts-record_origin) {
-   if (!has_conforming_footer(msgbuf, 0))
+   if (!has_conforming_footer(msgbuf, NULL, 0))
strbuf_addch(msgbuf, '\n');
strbuf_addstr(msgbuf, cherry_picked_prefix);
strbuf_addstr(msgbuf, 
sha1_to_hex(commit-object.sha1));
@@ -1087,21 +1106,33 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return pick_commits(todo_list, opts);
 }
 
-void append_signoff(struct strbuf *msgbuf, int ignore_footer)
+void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
 {
+   unsigned no_dup_sob = flag  APPEND_SIGNOFF_DEDUP;
struct strbuf sob = STRBUF_INIT;
-   int i;
+   int has_footer;
 
strbuf_addstr(sob, sign_off_header);
strbuf_addstr(sob, fmt_name(getenv(GIT_COMMITTER_NAME),
getenv(GIT_COMMITTER_EMAIL)));
strbuf_addch(sob, '\n');
-   for (i = msgbuf-len - 1 - ignore_footer; i  0  msgbuf-buf[i - 1] 
!= '\n'; i--)
-   ; /* do nothing */
-   if (prefixcmp(msgbuf-buf + i, sob.buf)) {
-   if (!has_conforming_footer(msgbuf, ignore_footer))
-   strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0, 
\n, 1);
-   strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0, sob.buf, 
sob.len);
-   }
+
+   /*
+* If the whole message buffer is equal to the sob, pretend that we
+* found a conforming footer with a matching sob
+*/
+   if (msgbuf-len - ignore_footer == sob.len 
+   !strncmp(msgbuf-buf, sob.buf, sob.len))
+   has_footer = 3;
+   else
+   has_footer = has_conforming_footer(msgbuf, sob, ignore_footer);
+
+   if (!has_footer)
+

[PATCH v4 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline

2013-02-12 Thread Brandon Casey
Teach append_signoff to detect whether a blank line exists at the position
that the signed-off-by line will be added, and refrain from adding an
additional one if one already exists.  Or, add an additional line if one
is needed to make sure the new footer is separated from the message body
by a blank line.

Signed-off-by: Brandon Casey bca...@nvidia.com
---
 sequencer.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 3364faa..3c63e3a 100644


I dropped the mention of format-patch in the commit message.  This
implementation is less about making format-patch work and more about
cleaning up a strangely formatted commit message.

-Brandon

--- a/sequencer.c
+++ b/sequencer.c
@@ -1110,6 +1110,7 @@ void append_signoff(struct strbuf *msgbuf, int 
ignore_footer, unsigned flag)
 {
unsigned no_dup_sob = flag  APPEND_SIGNOFF_DEDUP;
struct strbuf sob = STRBUF_INIT;
+   const char *append_newlines = NULL;
int has_footer;
 
strbuf_addstr(sob, sign_off_header);
@@ -1127,8 +1128,17 @@ void append_signoff(struct strbuf *msgbuf, int 
ignore_footer, unsigned flag)
else
has_footer = has_conforming_footer(msgbuf, sob, ignore_footer);
 
-   if (!has_footer)
-   strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0, \n, 1);
+   if (!has_footer) {
+   size_t len = msgbuf-len - ignore_footer;
+   if (len  msgbuf-buf[len - 1] != '\n')
+   append_newlines = \n\n;
+   else if (len  1  msgbuf-buf[len - 2] != '\n')
+   append_newlines = \n;
+   }
+
+   if (append_newlines)
+   strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0,
+   append_newlines, strlen(append_newlines));
 
if (has_footer != 3  (!no_dup_sob || has_footer != 2))
strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0,
-- 
1.8.1.3.579.gd9af3b6

--
To unsubscribe from this list: send the line 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 10/12] t4014: more tests about appending s-o-b lines

2013-02-12 Thread Brandon Casey
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com

[bc: Squash the tests from Duy's original unify-appending-sob series.

 Fix test 90 signoff: some random signoff-alike and mark as failing.
 Correct behavior should insert a blank line after message body and
 signed-off-by.

 Add two additional tests:

   1. failure to detect non-conforming elements in the footer when last
  line matches committer's s-o-b.
   2. ensure various s-o-b -like elements in the footer are handled as
  conforming. e.g. Change-id: I or Bug: 1234
]

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Signed-off-by: Brandon Casey bca...@nvidia.com
---
 t/t4014-format-patch.sh | 241 
 1 file changed, 241 insertions(+)

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 7fa3647..a415b89 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1021,4 +1021,245 @@ test_expect_success 'cover letter using branch 
description (6)' '
grep hello actual /dev/null
 '
 
+append_signoff()
+{
+   C=$(git commit-tree HEAD^^{tree} -p HEAD) 
+   git format-patch --stdout --signoff $C^..$C append_signoff.patch 
+   sed -n -e 1,/^---$/p append_signoff.patch |
+   egrep -n ^Subject|Sign|^$
+}
+
+test_expect_success 'signoff: commit with no body' '
+   append_signoff /dev/null actual 
+   cat \EOF | sed s/EOL$// expected 
+4:Subject: [PATCH] EOL
+8:
+9:Signed-off-by: C O Mitter commit...@example.com
+EOF
+   test_cmp expected actual
+'
+
+test_expect_success 'signoff: commit with only subject' '
+   echo subject | append_signoff actual 
+   cat expected \EOF 
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter commit...@example.com
+EOF
+   test_cmp expected actual
+'
+
+test_expect_success 'signoff: commit with only subject that does not end with 
NL' '
+   printf subject | append_signoff actual 
+   cat expected \EOF 
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter commit...@example.com
+EOF
+   test_cmp expected actual
+'
+
+test_expect_success 'signoff: no existing signoffs' '
+   append_signoff \EOF actual 
+subject
+
+body
+EOF
+   cat expected \EOF 
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter commit...@example.com
+EOF
+   test_cmp expected actual
+'
+
+test_expect_success 'signoff: no existing signoffs and no trailing NL' '
+   printf subject\n\nbody | append_signoff actual 
+   cat expected \EOF 
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter commit...@example.com
+EOF
+   test_cmp expected actual
+'
+
+test_expect_success 'signoff: some random signoff' '
+   append_signoff \EOF actual 
+subject
+
+body
+
+Signed-off-by: my@house
+EOF
+   cat expected \EOF 
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: my@house
+12:Signed-off-by: C O Mitter commit...@example.com
+EOF
+   test_cmp expected actual
+'
+
+test_expect_failure 'signoff: some random signoff-alike' '
+   append_signoff \EOF actual 
+subject
+
+body
+Fooled-by-me: my@house
+EOF
+   cat expected \EOF 
+4:Subject: [PATCH] subject
+8:
+11:
+12:Signed-off-by: C O Mitter commit...@example.com
+EOF
+   test_cmp expected actual
+'
+
+test_expect_failure 'signoff: not really a signoff' '
+   append_signoff \EOF actual 
+subject
+
+I want to mention about Signed-off-by: here.
+EOF
+   cat expected \EOF 
+4:Subject: [PATCH] subject
+8:
+9:I want to mention about Signed-off-by: here.
+10:
+11:Signed-off-by: C O Mitter commit...@example.com
+EOF
+   test_cmp expected actual
+'
+
+test_expect_failure 'signoff: not really a signoff (2)' '
+   append_signoff \EOF actual 
+subject
+
+My unfortunate
+Signed-off-by: example happens to be wrapped here.
+EOF
+   cat expected \EOF 
+4:Subject: [PATCH] subject
+8:
+10:Signed-off-by: example happens to be wrapped here.
+11:
+12:Signed-off-by: C O Mitter commit...@example.com
+EOF
+   test_cmp expected actual
+'
+
+test_expect_failure 'signoff: valid S-o-b paragraph in the middle' '
+   append_signoff \EOF actual 
+subject
+
+Signed-off-by: my@house
+Signed-off-by: your@house
+
+A lot of houses.
+EOF
+   cat expected \EOF 
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: my@house
+10:Signed-off-by: your@house
+11:
+13:
+14:Signed-off-by: C O Mitter commit...@example.com
+EOF
+   test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff at the end' '
+   append_signoff \EOF actual 
+subject
+
+body
+
+Signed-off-by: C O Mitter commit...@example.com
+EOF
+   cat expected \EOF 
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter commit...@example.com
+EOF
+   test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff at the end, no trailing NL' '
+   printf subject\n\nSigned-off-by: C O Mitter commit...@example.com |
+   append_signoff 

[PATCH v4 11/12] format-patch: update append_signoff prototype

2013-02-12 Thread Brandon Casey
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com

This is a preparation step for merging with append_signoff from
sequencer.c

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Signed-off-by: Brandon Casey bca...@nvidia.com
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
---
 builtin/log.c | 13 +
 log-tree.c| 17 +
 revision.h|  2 +-
 3 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 8f0b2e8..59de484 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1086,7 +1086,6 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
struct commit *origin = NULL, *head = NULL;
const char *in_reply_to = NULL;
struct patch_ids ids;
-   char *add_signoff = NULL;
struct strbuf buf = STRBUF_INIT;
int use_patch_format = 0;
int quiet = 0;
@@ -1193,16 +1192,6 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
rev.subject_prefix = strbuf_detach(sprefix, NULL);
}
 
-   if (do_signoff) {
-   const char *committer;
-   const char *endpos;
-   committer = git_committer_info(IDENT_STRICT);
-   endpos = strchr(committer, '');
-   if (!endpos)
-   die(_(bogus committer info %s), committer);
-   add_signoff = xmemdupz(committer, endpos - committer + 1);
-   }
-
for (i = 0; i  extra_hdr.nr; i++) {
strbuf_addstr(buf, extra_hdr.items[i].string);
strbuf_addch(buf, '\n');
@@ -1393,7 +1382,7 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
total++;
start_number--;
}
-   rev.add_signoff = add_signoff;
+   rev.add_signoff = do_signoff;
while (0 = --nr) {
int shown;
commit = list[nr];
diff --git a/log-tree.c b/log-tree.c
index 5dc45c4..ac1cd68 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -10,6 +10,8 @@
 #include color.h
 #include gpg-interface.h
 
+#define APPEND_SIGNOFF_DEDUP (1u 0)
+
 struct decoration name_decoration = { object names };
 
 enum decoration_type {
@@ -253,9 +255,12 @@ static int detect_any_signoff(char *letter, int size)
return seen_head  seen_name;
 }
 
-static void append_signoff(struct strbuf *sb, const char *signoff)
+static void append_signoff(struct strbuf *sb, int ignore_footer, unsigned flag)
 {
+   unsigned no_dup_sob = flag  APPEND_SIGNOFF_DEDUP;
static const char signed_off_by[] = Signed-off-by: ;
+   char *signoff = xstrdup(fmt_name(getenv(GIT_COMMITTER_NAME),
+  getenv(GIT_COMMITTER_EMAIL)));
size_t signoff_len = strlen(signoff);
int has_signoff = 0;
char *cp;
@@ -275,6 +280,7 @@ static void append_signoff(struct strbuf *sb, const char 
*signoff)
if (!isspace(cp[signoff_len]))
continue;
/* we already have him */
+   free(signoff);
return;
}
 
@@ -287,6 +293,7 @@ static void append_signoff(struct strbuf *sb, const char 
*signoff)
strbuf_addstr(sb, signed_off_by);
strbuf_add(sb, signoff, signoff_len);
strbuf_addch(sb, '\n');
+   free(signoff);
 }
 
 static unsigned int digits_in_number(unsigned int number)
@@ -672,8 +679,10 @@ void show_log(struct rev_info *opt)
/*
 * And then the pretty-printed message itself
 */
-   if (ctx.need_8bit_cte = 0)
-   ctx.need_8bit_cte = has_non_ascii(opt-add_signoff);
+   if (ctx.need_8bit_cte = 0  opt-add_signoff)
+   ctx.need_8bit_cte =
+   has_non_ascii(fmt_name(getenv(GIT_COMMITTER_NAME),
+  getenv(GIT_COMMITTER_EMAIL)));
ctx.date_mode = opt-date_mode;
ctx.date_mode_explicit = opt-date_mode_explicit;
ctx.abbrev = opt-diffopt.abbrev;
@@ -686,7 +695,7 @@ void show_log(struct rev_info *opt)
pretty_print_commit(ctx, commit, msgbuf);
 
if (opt-add_signoff)
-   append_signoff(msgbuf, opt-add_signoff);
+   append_signoff(msgbuf, 0, APPEND_SIGNOFF_DEDUP);
 
if ((ctx.fmt != CMIT_FMT_USERFORMAT) 
ctx.notes_message  *ctx.notes_message) {
diff --git a/revision.h b/revision.h
index 5da09ee..01bd2b7 100644
--- a/revision.h
+++ b/revision.h
@@ -138,7 +138,7 @@ struct rev_info {
int reroll_count;
char*message_id;
struct string_list *ref_message_ids;
-   const char  *add_signoff;
+   int add_signoff;
const char  *extra_headers;
const char  *log_reencode;
const char  *subject_prefix;
-- 
1.8.1.3.579.gd9af3b6

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

[PATCH v4 12/12] Unify appending signoff in format-patch, commit and sequencer

2013-02-12 Thread Brandon Casey
There are two implementations of append_signoff in log-tree.c and
sequencer.c, which do more or less the same thing.  Unify on top of the
sequencer.c implementation.

Add a test in t4014 to demonstrate support for non-s-o-b elements in the
commit footer provided by sequence.c:append_sob.  Mark tests fixed as
appropriate.

[Commit message mostly stolen from Nguyễn Thái Ngọc Duy's original
 unification patch]

Signed-off-by: Brandon Casey bca...@nvidia.com
---
 log-tree.c  | 91 +
 t/t4014-format-patch.sh | 31 ++---
 2 files changed, 27 insertions(+), 95 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index ac1cd68..c9d9a37 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -9,8 +9,7 @@
 #include string-list.h
 #include color.h
 #include gpg-interface.h
-
-#define APPEND_SIGNOFF_DEDUP (1u 0)
+#include sequencer.h
 
 struct decoration name_decoration = { object names };
 
@@ -208,94 +207,6 @@ void show_decorations(struct rev_info *opt, struct commit 
*commit)
putchar(')');
 }
 
-/*
- * Search for ^[-A-Za-z]+: [^@]+@ pattern. It usually matches
- * Signed-off-by: and Acked-by: lines.
- */
-static int detect_any_signoff(char *letter, int size)
-{
-   char *cp;
-   int seen_colon = 0;
-   int seen_at = 0;
-   int seen_name = 0;
-   int seen_head = 0;
-
-   cp = letter + size;
-   while (letter = --cp  *cp == '\n')
-   continue;
-
-   while (letter = cp) {
-   char ch = *cp--;
-   if (ch == '\n')
-   break;
-
-   if (!seen_at) {
-   if (ch == '@')
-   seen_at = 1;
-   continue;
-   }
-   if (!seen_colon) {
-   if (ch == '@')
-   return 0;
-   else if (ch == ':')
-   seen_colon = 1;
-   else
-   seen_name = 1;
-   continue;
-   }
-   if (('A' = ch  ch = 'Z') ||
-   ('a' = ch  ch = 'z') ||
-   ch == '-') {
-   seen_head = 1;
-   continue;
-   }
-   /* no empty last line doesn't match */
-   return 0;
-   }
-   return seen_head  seen_name;
-}
-
-static void append_signoff(struct strbuf *sb, int ignore_footer, unsigned flag)
-{
-   unsigned no_dup_sob = flag  APPEND_SIGNOFF_DEDUP;
-   static const char signed_off_by[] = Signed-off-by: ;
-   char *signoff = xstrdup(fmt_name(getenv(GIT_COMMITTER_NAME),
-  getenv(GIT_COMMITTER_EMAIL)));
-   size_t signoff_len = strlen(signoff);
-   int has_signoff = 0;
-   char *cp;
-
-   cp = sb-buf;
-
-   /* First see if we already have the sign-off by the signer */
-   while ((cp = strstr(cp, signed_off_by))) {
-
-   has_signoff = 1;
-
-   cp += strlen(signed_off_by);
-   if (cp + signoff_len = sb-buf + sb-len)
-   break;
-   if (strncmp(cp, signoff, signoff_len))
-   continue;
-   if (!isspace(cp[signoff_len]))
-   continue;
-   /* we already have him */
-   free(signoff);
-   return;
-   }
-
-   if (!has_signoff)
-   has_signoff = detect_any_signoff(sb-buf, sb-len);
-
-   if (!has_signoff)
-   strbuf_addch(sb, '\n');
-
-   strbuf_addstr(sb, signed_off_by);
-   strbuf_add(sb, signoff, signoff_len);
-   strbuf_addch(sb, '\n');
-   free(signoff);
-}
-
 static unsigned int digits_in_number(unsigned int number)
 {
unsigned int i = 10, result = 1;
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index a415b89..97fde9e 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1103,7 +1103,28 @@ EOF
test_cmp expected actual
 '
 
-test_expect_failure 'signoff: some random signoff-alike' '
+test_expect_success 'signoff: misc conforming footer elements' '
+   append_signoff \EOF actual 
+subject
+
+body
+
+Signed-off-by: my@house
+(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)
+Tested-by: Some One some...@example.com
+Bug: 1234
+EOF
+   cat expected \EOF 
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: my@house
+15:Signed-off-by: C O Mitter commit...@example.com
+EOF
+   test_cmp expected actual
+'
+
+test_expect_success 'signoff: some random signoff-alike' '
append_signoff \EOF actual 
 subject
 
@@ -1119,7 +1140,7 @@ EOF
test_cmp expected actual
 '
 
-test_expect_failure 'signoff: not really a signoff' '
+test_expect_success 'signoff: not really a signoff' '
append_signoff \EOF actual 
 subject
 
@@ -1135,7 +1156,7 @@ EOF
test_cmp 

[PATCH/FYI v4 13/12] fixup! t/t3511: add some tests of 'cherry-pick -s' functionality

2013-02-12 Thread Brandon Casey
---

This test tests the behavior of 'cherry-pick -s' of a commit with an empty
commit message.

I created the test when I noticed during my series that cherry-pick was
adding a sob twice when a commit with an empty commit message was
cherry-picked.

I'm not sure we should apply this though.  I'm leaning towards saying that
the 'cherry-pick -s' behavior with respect to a commit with an empty message
body should be undefined.  If we want it to be undefined then we probably
shouldn't introduce a test which would have the effect of defining it.

Junio, if you think we should apply it, it's prepared as a fixup commit and
should autosquash easily.

-Brandon

 t/t3505-cherry-pick-empty.sh | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/t/t3505-cherry-pick-empty.sh b/t/t3505-cherry-pick-empty.sh
index a0c6e30..da4c60e 100755
--- a/t/t3505-cherry-pick-empty.sh
+++ b/t/t3505-cherry-pick-empty.sh
@@ -58,6 +58,16 @@ test_expect_success 'cherry-pick a commit with an empty 
message with --allow-emp
git cherry-pick --allow-empty-message empty-branch
 '
 
+test_expect_success 'cherry-pick a commit with an empty message with 
--allow-empty-message and -s' '
+   git reset --hard HEAD^ 
+   git cherry-pick --allow-empty-message -s empty-branch 
+   { git show --pretty=format:%B -s empty-branch 
+ printf Signed-off-by: %s %s\n $GIT_COMMITTER_NAME 
$GIT_COMMITTER_EMAIL
+   } expect 
+   git show --pretty=format:%B -s HEAD actual 
+   test_cmp expect actual
+'
+
 test_expect_success 'cherry pick an empty non-ff commit without --allow-empty' 
'
git checkout master 
echo fourth file2 
-- 
1.8.1.1.252.gdb33759

--
To unsubscribe from this list: send the line 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: A good Git technique for referring back to original files

2013-02-12 Thread MikeW
Matthieu Moy Matthieu.Moy at grenoble-inp.fr writes:

 
 MikeW mw_phil at yahoo.co.uk writes:
 
  Since git is so good at tracking file content, I wondered whether
there was any
  technique using git that would simplify the back-referencing task.
 
 I'm not sure I understand the question, but if you want to add meta-data
 to Git commits (e.g. this Git commit is revision 42 in CVS repository
 foo), then have a look at git-notes. It won't give you directly
 reference to other VCS, but at least can be used as a storage
 mechanism to store these references.
 
Thanks for the reply.

In my work environment both the SDK and the original files are available
(in an enclosing directory).

--SDK_content
  |
  SDK_subproj1-- ...
  ||
  |content
  |
  SDK_subproj2- ...
  ||
  |content
  |
  SDK_subprojN- ...
  ||
  |content
  |
  Working_SDK ... (under git, baseline generated from subproj1..N)
   |
   content derived from subproj1..N


What I had in mind was something I could run over, say, SDK_content
(alternatively, from within Working_SDK, referring back to SDK_content)
which would note the changed files in Working_SDK and locate the
original files in SDK_subproj1..N letting me merge the changes back.


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


[PATCH v4.1 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline

2013-02-12 Thread Brandon Casey
Teach append_signoff to detect whether a blank line exists at the position
that the signed-off-by line will be added, and refrain from adding an
additional one if one already exists.  Or, add an additional line if one
is needed to make sure the new footer is separated from the message body
by a blank line.

Signed-off-by: Brandon Casey bca...@nvidia.com
---


A slight tweak.  And I promise, no more are coming.

-Brandon


 sequencer.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 3364faa..084573b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1127,8 +1127,19 @@ void append_signoff(struct strbuf *msgbuf, int 
ignore_footer, unsigned flag)
else
has_footer = has_conforming_footer(msgbuf, sob, ignore_footer);
 
-   if (!has_footer)
-   strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0, \n, 1);
+   if (!has_footer) {
+   const char *append_newlines = NULL;
+   size_t len = msgbuf-len - ignore_footer;
+
+   if (len  msgbuf-buf[len - 1] != '\n')
+   append_newlines = \n\n;
+   else if (len  1  msgbuf-buf[len - 2] != '\n')
+   append_newlines = \n;
+
+   if (append_newlines)
+   strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0,
+   append_newlines, strlen(append_newlines));
+   }
 
if (has_footer != 3  (!no_dup_sob || has_footer != 2))
strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0,
-- 
1.8.1.1.252.gdb33759

--
To unsubscribe from this list: send the line 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: Improve 'git help' with basic user guide linkss

2013-02-12 Thread Philip Oakley

From: Philip Oakley philipoak...@iee.org
Sent: Friday, February 08, 2013 11:16 PM

From: Junio C Hamano gits...@pobox.com
Sent: Friday, February 08, 2013 10:54 PM

Philip Oakley philipoak...@iee.org writes:


My initial https://github.com/PhilipOakley/git/commit/e6217d simply
updates
-  N_(See 'git help command' for more information on a specific
command.);
+  N_(See 'git help command' for more information on a specific
command.\n
+ Or 'git help guide', such as 'tutorial' for an introduction
to Git.);
as a starter for the new users.


Yeah, that would be a good change to make to git helpRETURN
output.


I'll sort some patches early next week (the weekend's committed 
elsewhere)





My view is that help --all (-a) is essentially incomplete as it
currently doesn't provide all the help.


It has always been about tell me all subcommands, not about give
me all the help you could give me.  You are not adding a help
subcommand to a system you wrote last week.  Changing the semantics
this late feels, eh, too late.


OK, I'll limit the the follow-ons to just an extra --guides option 
(probably just a list of the common guides initially), and 
leave --all(-a) for just the commands.


The Git man page includes the different command types listed by category 
(Main porcelain, Ancillary {manipulators, interrogators}, Interacting 
with others, etc.).


Obviously (?) this is generated from the command-list.txt file, though I 
don't see a shell script that would generate the 
'cmds-mainporcelain.txt' (etc.) files 
(//github.com/gitster/git-htmldocs). They are also part of the msysgit 
install.


Where should I be looking to see how they are generated?

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: Pushing a git repository to a new server

2013-02-12 Thread Michael J Gruber
Jeff King venit, vidit, dixit 11.02.2013 17:27:
 On Mon, Feb 11, 2013 at 02:57:51AM -0500, Ethan Reesor wrote:
 
 On Mon, Feb 11, 2013 at 2:50 AM, Konstantin Khomoutov
 kostix+...@007spb.ru wrote:
 What's wrong with
 $ ssh myuser@remotehost 'mkdir /path/to/MyRepo.git; cd $_; git init --bare'
 $ git push --all git@remotehost:MyOtherRepo.git
 ?

 Nothing, I just wanted to make myself a command to do that for me.
 
 We talked about this a long time ago. One problem is that it's
 inherently unportable, as the procedure to make a repo is potentially
 different on every server (and certainly that is the case between a
 regular user running stock git and something like GitHub or Google Code;
 I imagine even gitolite has some special procedures for creating repos,
 too).
 
 One proposal made in the previous discussion was to define a microformat
 for repository administration commands. So that you could connect and
 say git admin-create-repo /path/to/MyRepo.git, and the server-provided
 admin-create-repo command would take care of the details. Then stock git
 could forward it to git init --bare, GitHub could do the same and
 create the necessary database records, etc.

 And once that standardized method was in place, it would be easy to add
 a --create option to git push to request an admin-create-repo
 before pushing.
 
 I still think that's a reasonable way forward, but nobody was interested
 enough to start writing code for it.
 
 -Peff
 

I'm not sure providers like GitHub would fancy an interface which allows
the programmatic creation of repos (giving a new meaning to fork
bomb). But I bet you know better ;-)

An alternative would be to teach git (the client) about repo types and
how to create them. After all, a repo URL ssh://host/path gives a
clear indication that ssh host git init path will create a repo. I'm
wondering whether it's more likely to convince providers (the server
side) or more is gained by covering the simpler cases client-side (our
side).

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


Re: Improve 'git help' with basic user guide linkss

2013-02-12 Thread John Keeping
On Tue, Feb 12, 2013 at 11:11:17AM -, Philip Oakley wrote:
 Obviously (?) this is generated from the command-list.txt file, though I 
 don't see a shell script that would generate the 
 'cmds-mainporcelain.txt' (etc.) files 
 (//github.com/gitster/git-htmldocs). They are also part of the msysgit 
 install.
 
 Where should I be looking to see how they are generated?

Documentation/cmd-list.perl

If you look in Documentation/Makefile, cmds-mainporcelain.txt is a
member of cmds_txt, which depends on cmd-list.made, which is a stamp
file created after invoking cmd-list.perl.


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


Re: Improve 'git help' with basic user guide linkss

2013-02-12 Thread Philip Oakley

From: John Keeping j...@keeping.me.uk
Sent: Tuesday, February 12, 2013 11:37 AM

On Tue, Feb 12, 2013 at 11:11:17AM -, Philip Oakley wrote:
Obviously (?) this is generated from the command-list.txt file, 
though I

don't see a shell script that would generate the
'cmds-mainporcelain.txt' (etc.) files
(//github.com/gitster/git-htmldocs). They are also part of the 
msysgit

install.

Where should I be looking to see how they are generated?


Documentation/cmd-list.perl

If you look in Documentation/Makefile, cmds-mainporcelain.txt is a
member of cmds_txt, which depends on cmd-list.made, which is a stamp
file created after invoking cmd-list.perl.


John


Many thanks - some easy reading while unwell ;-) 


--
To unsubscribe from this list: send the line 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] completion: support 'git config --local'

2013-02-12 Thread Matthieu Moy
This needs to be done in two places: __git_config_get_set_variables to
allow clever completion of git config --local --get footab, and
_git_config to allow git config --loctab to complete to --local.

While we're there, change the order of options in the code to match
git-config.txt.

Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 I think this line should include --local:
 
 https://github.com/git/git/blob/next/contrib/completion/git-completion.bash#L1782
 --global|--system|--file=*)
 
 This would help for:
 git config -l --local

Yes, but not only ;-)

 contrib/completion/git-completion.bash | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 5770b6f..2e1ad67 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1570,7 +1570,7 @@ __git_config_get_set_variables ()
while [ $c -gt 1 ]; do
word=${words[c]}
case $word in
-   --global|--system|--file=*)
+   --system|--global|--local|--file=*)
config_file=$word
break
;;
@@ -1676,7 +1676,7 @@ _git_config ()
case $cur in
--*)
__gitcomp 
-   --global --system --file=
+   --system --global --local --file=
--list --replace-all
--get --get-all --get-regexp
--add --unset --unset-all
-- 
1.8.1.2.548.g956380a.dirty

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


[PATCHv4 4/6] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe

2013-02-12 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@mina86.com

The command_close_bidi_pipe() function will insist on closing both
input and output pipes returned by command_bidi_pipe().  With this
change it is possible to close one of the pipes in advance and
pass undef as an argument.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 6bc9a3c..d6e6c9e 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -426,12 +426,25 @@ Note that you should not rely on whatever actually is in 
CCTX;
 currently it is simply the command name but in future the context might
 have more complicated structure.
 
+CPIPE_IN and CPIPE_OUT may be Cundef if they have been closed prior to
+calling this function.  This may be useful in a query-response type of
+commands where caller first writes a query and later reads response, eg:
+
+   my ($pid, $in, $out, $ctx) = $r-command_bidi_pipe('cat-file 
--batch-check');
+   print $out 0\n;
+   close $out;
+   while ($in) { ... }
+   $r-command_close_bidi_pipe($pid, $in, undef, $ctx);
+
+This idiom may prevent potential dead locks caused by data sent to the output
+pipe not being flushed and thus not reaching the executed command.
+
 =cut
 
 sub command_close_bidi_pipe {
local $?;
my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
-   _cmd_close($ctx, $in, $out);
+   _cmd_close($ctx, grep defined, $in, $out);
waitpid $pid, 0;
if ($?  8) {
throw Git::Error::Command($ctx, $? 8);
-- 
1.8.1.3.572.g32bae1f

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


[PATCHv4 6/6] git-send-email: use git credential to obtain password

2013-02-12 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@mina86.com

If smtp_user is provided but smtp_pass is not, instead of
prompting for password, make git-send-email use git
credential command instead.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 Documentation/git-send-email.txt |  4 +--
 git-send-email.perl  | 59 +++-
 2 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 44a1f7c..0cffef8 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -164,8 +164,8 @@ Sending
 Furthermore, passwords need not be specified in configuration files
 or on the command line. If a username has been specified (with
 '--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass' or 'sendemail.smtppass'), then
+a password is obtained using 'git-credential'.
 
 --smtp-server=host::
If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..76bbfc3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,39 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
 }
 
+# Returns 1 if authentication succeeded or was not necessary
+# (smtp_user was not specified), and 0 otherwise.
+
+sub smtp_auth_maybe {
+   if (!defined $smtp_authuser || $auth) {
+   return 1;
+   }
+
+   # Workaround AUTH PLAIN/LOGIN interaction defect
+   # with Authen::SASL::Cyrus
+   eval {
+   require Authen::SASL;
+   Authen::SASL-import(qw(Perl));
+   };
+
+   # TODO: Authentication may fail not because credentials were
+   # invalid but due to other reasons, in which we should not
+   # reject credentials.
+   $auth = Git::credential({
+   'protocol' = 'smtp',
+   'host' = join(':', $smtp_server, $smtp_server_port),
+   'username' = $smtp_authuser,
+   # if there's no password, git credential fill will
+   # give us one, otherwise it'll just pass this one.
+   'password' = $smtp_authpass
+   }, sub {
+   my $cred = shift;
+   return !!$smtp-auth($cred-{'username'}, $cred-{'password'});
+   });
+
+   return $auth;
+}
+
 # Returns 1 if the message was sent, and 0 otherwise.
 # In actuality, the whole program dies when there
 # is an error sending a message.
@@ -1185,31 +1218,7 @@ X-Mailer: git-send-email $gitversion
defined $smtp_server_port ?  
port=$smtp_server_port : ;
}
 
-   if (defined $smtp_authuser) {
-   # Workaround AUTH PLAIN/LOGIN interaction defect
-   # with Authen::SASL::Cyrus
-   eval {
-   require Authen::SASL;
-   Authen::SASL-import(qw(Perl));
-   };
-
-   if (!defined $smtp_authpass) {
-
-   system stty -echo;
-
-   do {
-   print Password: ;
-   $_ = STDIN;
-   print \n;
-   } while (!defined $_);
-
-   chomp($smtp_authpass = $_);
-
-   system stty echo;
-   }
-
-   $auth ||= $smtp-auth( $smtp_authuser, $smtp_authpass ) 
or die $smtp-message;
-   }
+   smtp_auth_maybe or die $smtp-message;
 
$smtp-mail( $raw_from ) or die $smtp-message;
$smtp-to( @recipients ) or die $smtp-message;
-- 
1.8.1.3.572.g32bae1f

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


[PATCHv4 0/6] git-credential support in git-send-email

2013-02-12 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@mina86.com

Besids git-credential support in git-send-email, there are some other
minor improvements to Git.pm in this patchset.  Patch 3/6 is new
compared to the previous patchset.

Michal Nazarewicz (6):
  Git.pm: allow command_close_bidi_pipe to be called as method
  Git.pm: fix example in command_close_bidi_pipe documentation
  Git.pm: refactor command_close_bidi_pipe to use _cmd_close
  Git.pm: allow pipes to be closed prior to calling
command_close_bidi_pipe
  Git.pm: add interface for git credential command
  git-send-email: use git credential to obtain password

 Documentation/git-send-email.txt |   4 +-
 git-send-email.perl  |  59 +++-
 perl/Git.pm  | 198 ++-
 3 files changed, 213 insertions(+), 48 deletions(-)

-- 
1.8.1.3.572.g32bae1f

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


[PATCHv4 1/6] Git.pm: allow command_close_bidi_pipe to be called as method

2013-02-12 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@mina86.com

The documentation of command_close_bidi_pipe() claims that it can
be called as a method, but it does not check whether the first
argument is $self or not assuming the latter.  Using _maybe_self()
fixes this.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..bbb753a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -430,7 +430,7 @@ have more complicated structure.
 
 sub command_close_bidi_pipe {
local $?;
-   my ($pid, $in, $out, $ctx) = @_;
+   my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
unless (close $fh) {
if ($!) {
-- 
1.8.1.3.572.g32bae1f

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


[PATCHv4 5/6] Git.pm: add interface for git credential command

2013-02-12 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@mina86.com

Add a credential() function which is an interface to the git
credential command.  The code is heavily based on credential_*
functions in contrib/mw-to-git/git-remote-mediawiki.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 151 
 1 file changed, 151 insertions(+)

diff --git a/perl/Git.pm b/perl/Git.pm
index d6e6c9e..a24458c 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -59,6 +59,7 @@ require Exporter;
 command_bidi_pipe command_close_bidi_pipe
 version exec_path html_path hash_object git_cmd_try
 remote_refs prompt
+credential credential_read credential_write
 temp_acquire temp_release temp_reset temp_path);
 
 
@@ -1003,6 +1004,156 @@ sub _close_cat_blob {
 }
 
 
+=item credential_read( FILEHANDLE )
+
+Reads credential key-value pairs from CFILEHANDLE.  Reading stops at EOF or
+when an empty line is encountered.  Each line must be of the form Ckey=value
+with a non-empty key.  Function returns hash with all read values.  Any white
+space (other then new-line character) is preserved.
+
+=cut
+
+sub credential_read {
+   my ($self, $reader) = _maybe_self(@_);
+   my %credential;
+   while ($reader) {
+   chomp;
+   if ($_ eq '') {
+   last;
+   } elsif (!/^([^=]+)=(.*)$/) {
+   throw Error::Simple(unable to parse git credential 
data:\n$_);
+   }
+   $credential{$1} = $2;
+   }
+   return %credential;
+}
+
+=item credential_write( FILEHANDLE, CREDENTIAL_HASHREF )
+
+Writes credential key-value pairs from hash referenced by
+CCREDENTIAL_HASHREF to CFILEHANDLE.  Keys and values cannot contain
+new-lines or NUL bytes characters, and key cannot contain equal signs nor be
+empty (if they do Error::Simple is thrown).  Any white space is preserved.  If
+value for a key is Cundef, it will be skipped.
+
+If C'url' key exists it will be written first.  (All the other key-value
+pairs are written in sorted order but you should not depend on that).  Once
+all lines are written, an empty line is printed.
+
+=cut
+
+sub credential_write {
+   my ($self, $writer, $credential) = _maybe_self(@_);
+   my ($key, $value);
+
+   # Check if $credential is valid prior to writing anything
+   while (($key, $value) = each %$credential) {
+   if (!defined $key || !length $key) {
+   throw Error::Simple(credential key empty or 
undefined);
+   } elsif ($key =~ /[=\n\0]/) {
+   throw Error::Simple(credential key contains invalid 
characters: $key);
+   } elsif (defined $value  $value =~ /[\n\0]/) {
+   throw Error::Simple(credential value for key=$key 
contains invalid characters: $value);
+   }
+   }
+
+   for $key (sort {
+   # url overwrites other fields, so it must come first
+   return -1 if $a eq 'url';
+   return  1 if $b eq 'url';
+   return $a cmp $b;
+   } keys %$credential) {
+   if (defined $credential-{$key}) {
+   print $writer $key, '=', $credential-{$key}, \n;
+   }
+   }
+   print $writer \n;
+}
+
+sub _credential_run {
+   my ($self, $credential, $op) = _maybe_self(@_);
+   my ($pid, $reader, $writer, $ctx) = command_bidi_pipe('credential', 
$op);
+
+   credential_write $writer, $credential;
+   close $writer;
+
+   if ($op eq fill) {
+   %$credential = credential_read $reader;
+   }
+   if ($reader) {
+   throw Error::Simple(unexpected output from git credential $op 
response:\n$_\n);
+   }
+
+   command_close_bidi_pipe($pid, $reader, undef, $ctx);
+}
+
+=item credential( CREDENTIAL_HASHREF [, OPERATION ] )
+
+=item credential( CREDENTIAL_HASHREF, CODE )
+
+Executes Cgit credential for a given set of credentials and specified
+operation.  In both forms CCREDENTIAL_HASHREF needs to be a reference to
+a hash which stores credentials.  Under certain conditions the hash can
+change.
+
+In the first form, COPERATION can be C'fill', C'approve' or C'reject',
+and function will execute corresponding Cgit credential sub-command.  If
+it's omitted C'fill' is assumed.  In case of C'fill' the values stored in
+CCREDENTIAL_HASHREF will be changed to the ones returned by the Cgit
+credential fill command.  The usual usage would look something like:
+
+   my %cred = (
+   'protocol' = 'https',
+   'host' = 'example.com',
+   'username' = 'bob'
+   );
+   Git::credential \%cred;
+   if (try_to_authenticate($cred{'username'}, $cred{'password'})) {
+   Git::credential \%cred, 'approve';
+   ... do more stuff ...
+   } else {
+   

[PATCHv4 3/6] Git.pm: refactor command_close_bidi_pipe to use _cmd_close

2013-02-12 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@mina86.com

The body of the loop in command_close_bidi_pipe function is identical to
what _cmd_close function does so instead of duplicating, refactor change
_cmd_close so that it accepts list of file handlers to be closed, which
makes it usable with command_close_bidi_pipe.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 30 +++---
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 11f310a..6bc9a3c 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -267,13 +267,13 @@ sub command {
 
if (not defined wantarray) {
# Nothing to pepper the possible exception with.
-   _cmd_close($fh, $ctx);
+   _cmd_close($ctx, $fh);
 
} elsif (not wantarray) {
local $/;
my $text = $fh;
try {
-   _cmd_close($fh, $ctx);
+   _cmd_close($ctx, $fh);
} catch Git::Error::Command with {
# Pepper with the output:
my $E = shift;
@@ -286,7 +286,7 @@ sub command {
my @lines = $fh;
defined and chomp for @lines;
try {
-   _cmd_close($fh, $ctx);
+   _cmd_close($ctx, $fh);
} catch Git::Error::Command with {
my $E = shift;
$E-{'-outputref'} = \@lines;
@@ -313,7 +313,7 @@ sub command_oneline {
my $line = $fh;
defined $line and chomp $line;
try {
-   _cmd_close($fh, $ctx);
+   _cmd_close($ctx, $fh);
} catch Git::Error::Command with {
# Pepper with the output:
my $E = shift;
@@ -381,7 +381,7 @@ have more complicated structure.
 sub command_close_pipe {
my ($self, $fh, $ctx) = _maybe_self(@_);
$ctx ||= 'unknown';
-   _cmd_close($fh, $ctx);
+   _cmd_close($ctx, $fh);
 }
 
 =item command_bidi_pipe ( COMMAND [, ARGUMENTS... ] )
@@ -431,18 +431,8 @@ have more complicated structure.
 sub command_close_bidi_pipe {
local $?;
my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
-   foreach my $fh ($in, $out) {
-   unless (close $fh) {
-   if ($!) {
-   carp error closing pipe: $!;
-   } elsif ($?  8) {
-   throw Git::Error::Command($ctx, $? 8);
-   }
-   }
-   }
-
+   _cmd_close($ctx, $in, $out);
waitpid $pid, 0;
-
if ($?  8) {
throw Git::Error::Command($ctx, $? 8);
}
@@ -1355,9 +1345,11 @@ sub _execv_git_cmd { exec('git', @_); }
 
 # Close pipe to a subprocess.
 sub _cmd_close {
-   my ($fh, $ctx) = @_;
-   if (not close $fh) {
-   if ($!) {
+   my $ctx = shift @_;
+   foreach my $fh (@_) {
+   if (close $fh) {
+   # nop;
+   } elsif ($!) {
# It's just close, no point in fatalities
carp error closing pipe: $!;
} elsif ($?  8) {
-- 
1.8.1.3.572.g32bae1f

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


[PATCHv4 2/6] Git.pm: fix example in command_close_bidi_pipe documentation

2013-02-12 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@mina86.com


Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index bbb753a..11f310a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -418,7 +418,7 @@ and it is the fourth value returned by 
Ccommand_bidi_pipe().  The call idiom
 is:
 
my ($pid, $in, $out, $ctx) = $r-command_bidi_pipe('cat-file 
--batch-check');
-   print 0\n $out;
+   print $out 0\n;
while ($in) { ... }
$r-command_close_bidi_pipe($pid, $in, $out, $ctx);
 
-- 
1.8.1.3.572.g32bae1f

--
To unsubscribe from this list: send the line 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: Fwd: Re: [git-multimail] License unknown (#1)

2013-02-12 Thread Andy Parkins
On Tuesday 12 February 2013 09:01:14 Michael Haggerty wrote:

 I assume you are the Andy Parkins who originally submitted
 post-commit-email to the Git project...

I am indeed.  Hello.

 I have derived another script from yours:
 
 https://github.com/mhagger/git-multimail
 
 I affixed the GPLv2 boilerplate to the code under the assumption that it
 inherited this license from the Git project.  But afterwards I realized
 that I am not entirely confident that the GPLv2 applies to
 post-commit-email.

I'm sorry, that's entirely my fault.  I had read somewhere in the git 
submission policy that everything submitted was naturally under the same 
license as git itself.
 
 Obviously the situation is even more complicated because other people
 have contributed patches to the script since your original submission.
 But it would nevertheless be very helpful if you would clarify your
 original intention, *especially* if your intention was to put the script
 under a license different than GPLv2.

My intention was to put my code under exactly the license the git itself is 
under.  I am delighted for you or anyone else to use or derive from it.

I'm also sorry I didn't respond to your first email, it accidentally got 
sorted into spam.

If anyone wants some sort of legal declaration, I'm happy to give it.

  Original Message 
 Subject: Re: [git-multimail] License unknown (#1)
 Date: Sun, 27 Jan 2013 19:52:58 +0100
 From: Michael Haggerty mhag...@alum.mit.edu
 To: git discussion list git@vger.kernel.org,  Andy Parkins
 andypark...@gmail.com
 CC: mhagger/git-multimail
 reply+i-10159725-60cb2c338c594bd09d77fe2f8d628aa55114a3f6-119...@reply.git
 hub.com, Michiel Holtkamp notificati...@github.com
 
 I have a question about the license of contrib/hooks/post-commit-email.
  I had assumed that since it is in the git project, which is GPLv2, and
 since it contains no contrary information, it would by implication also
 fall under GPLv2.  But the file itself contains no explicit license
 information, and it is not clear to me that the signed-off-by line
 implies a particular license, either.  (The signed-off-by *does* seem to
 imply that the source code is under some kind of open source license,
 but not which one.)
 
 If somebody can explain what license the code is under and how they come
 to that conclusion, I would be very grateful.
 
 And if Andy Parkins (the original author) is listening, please indicate
 whether you had any intent *other* than GPLv2.
 
 For anybody who is interested, the file was first committed in
 4557e0de5b and has been modified by several authors since then.
 
 Given the pretty clear open-sourciness of the script, I don't think this
 has to be made into a big issue.  But it would be nice to state the
 license explicitly for future users' information.
 
 Thanks,
 Michael
 
 On 01/27/2013 02:38 PM, Michiel Holtkamp wrote:
  Actually, I'm not sure that it is GPLv2 for the original script. The
  COPYING file in the main project declares the project as GPLv2, but it
  also says that people contributing should make their preferences (for
  licensing) known. Maybe we can assume it's GPLv2, (as the original
  writer might have assumed it was GPLv2), but it's not explicitly stated
  so I'm not sure (IANAL).

-- 
Dr Andy Parkins
Director
FussyLogic Ltd

tel:  0845 557 7645

web:  www.fussylogic.co.uk

Company Registration No. 8198285  Registered in England and Wales

  This email and any attachments to it may be confidential and are
  intended solely for the use of the individual to whom it is addressed.
  Any views or opinions expressed are solely those of the author and do
  not necessarily represent those of FussyLogic Ltd.

  If you are not the intended recipient of this email, you must neither
  take any action based upon its contents, nor copy or show it to
  anyone.
  
  Please contact the sender if you believe you have received this email
  in error.

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


Re: [git-multimail] License unknown (#1)

2013-02-12 Thread Andy Parkins
On Sunday 27 January 2013 18:52:58 Michael Haggerty wrote:
 I have a question about the license of contrib/hooks/post-commit-email.
  I had assumed that since it is in the git project, which is GPLv2, and
 since it contains no contrary information, it would by implication also
 fall under GPLv2.  But the file itself contains no explicit license
 information, and it is not clear to me that the signed-off-by line
 implies a particular license, either.  (The signed-off-by *does* seem to
 imply that the source code is under some kind of open source license,
 but not which one.)

Keeping up with the git mailing list got a bit much, but my original filters 
to sort messages from it into a (now hidden) folder where still in place.  
Michael contacted me independently, which was enough of a prod to see this.

My apologies to everyone; I've been lax supporting the script -- I never 
really expected it to be as widely used as it has been, I was always expecting 
someone would come along and replace it so I'm pleased that Michael has done 
so (although it's a little disheartening to read of it being called hacky, 
when I tried very hard to make it as clear and modular as I could).

 If somebody can explain what license the code is under and how they come
 to that conclusion, I would be very grateful.
 
 And if Andy Parkins (the original author) is listening, please indicate
 whether you had any intent *other* than GPLv2.

I intended it to be under the same license as Git.  I had read in one of the 
patch submission files (which I can't seem to find now) that all submissions 
were considered part of git.

 For anybody who is interested, the file was first committed in
 4557e0de5b and has been modified by several authors since then.

I haven't looked at it for a long time.  I can't speak for the other authors, 
but for any part of it that is still mine -- anyone is free to do anything 
they wish with it.

 Given the pretty clear open-sourciness of the script, I don't think this
 has to be made into a big issue.  But it would be nice to state the
 license explicitly for future users' information.

If an explicit declaration is needed, I am happy to give it.  Let me know what 
form this should take and I'll supply it.



Andy

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


Re: [RFC/PATCH] Replace filepattern with pathspec for consistency

2013-02-12 Thread Junio C Hamano
Matthieu Moy matthieu@imag.fr writes:

 pathspec is the most widely used term, and is the one defined in
 gitglossary.txt. filepattern was used only in the synopsys for git-add
 and git-commit, and in git-add.txt. Get rid of it.

 This patch is obtained with by running:

   perl -pi -e 's/filepattern/pathspec/' `git grep -l filepattern`

 Signed-off-by: Matthieu Moy matthieu@imag.fr
 ---
 I'm a bit unsure about the changes to the .po files, but I guess doing
 the substitution there too does the right thing.

I am not sure if that is the right thing from the workflow point of
view, though.

The strings that are fed to _() would be updated with your patch,
but the replacement will stay to be filepattern translated to the
target language.  Translators have to actively hunt for the messages
to update them.  If you left the .po files untouched, they would
notice when git.pot is updated the next time and that will ensure
that the affected messages get translated, no?

  Documentation/git-add.txt | 12 ++--
  builtin/add.c |  2 +-
  builtin/commit.c  |  4 ++--
  po/de.po  |  6 +++---
  po/git.pot|  6 +++---
  po/sv.po  |  6 +++---
  po/vi.po  |  6 +++---
  po/zh_CN.po   |  6 +++---
  8 files changed, 24 insertions(+), 24 deletions(-)

For the above reason, I am inclined to take the first three and drop
the rest.

Thanks.

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


Re: [RFC/PATCH] Replace filepattern with pathspec for consistency

2013-02-12 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 The strings that are fed to _() would be updated with your patch,
 but the replacement will stay to be filepattern translated to the
 target language.  Translators have to actively hunt for the messages
 to update them.  If you left the .po files untouched, they would
 notice when git.pot is updated the next time and that will ensure
 that the affected messages get translated, no?

Right, and I'm not competent to say how filepattern was translated in
any of the languages. Let me know if you want a resend without the po/
part.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4] submodule: add 'deinit' command

2013-02-12 Thread Phil Hord
I haven't tried it yet, but I have some comments.

On Wed, Feb 6, 2013 at 4:11 PM, Jens Lehmann jens.lehm...@web.de wrote:
 With git submodule init the user is able to tell git he cares about one
 or more submodules and wants to have it populated on the next call to git
 submodule update. But currently there is no easy way he could tell git he
 does not care about a submodule anymore and wants to get rid of his local
 work tree (except he knows a lot about submodule internals and removes the
 submodule.$name.url setting from .git/config together with the work tree
 himself).

 Help those users by providing a 'deinit' command. This removes the whole
 submodule.name section from .git/config either for the given
 submodule(s) or for all those which have been initialized if '.' is
 given. Fail if the current work tree contains modifications unless
 forced. Complain when for a submodule given on the command line the url
 setting can't be found in .git/config, but nonetheless don't fail.

 Add tests and link the man pages of git submodule deinit and git rm
 to assist the user in deciding whether removing or unregistering the
 submodule is the right thing to do for him.

 Signed-off-by: Jens Lehmann jens.lehm...@web.de
 ---

 Changes since v3:
 - Add deinit to the --force documentation of git submodule
 - Never remove submodules containing a .git dir, even when forced
 - Diagnostic output when rm -rf or mkdir fails
 - More test cases


  Documentation/git-rm.txt|   4 ++
  Documentation/git-submodule.txt |  18 +++-
  git-submodule.sh|  78 ++-
  t/t7400-submodule-basic.sh  | 100 
 
  4 files changed, 198 insertions(+), 2 deletions(-)

 diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
 index 92bac27..1d876c2 100644
 --- a/Documentation/git-rm.txt
 +++ b/Documentation/git-rm.txt
 @@ -149,6 +149,10 @@ files that aren't ignored are present in the submodules 
 work tree.
  Ignored files are deemed expendable and won't stop a submodule's work
  tree from being removed.

 +If you only want to remove the local checkout of a submodule from your
 +work tree without committing the removal,
 +use linkgit:git-submodule[1] `deinit` instead.
 +
  EXAMPLES
  
  `git rm Documentation/\*.txt`::
 diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
 index a0c9df8..bc06159 100644
 --- a/Documentation/git-submodule.txt
 +++ b/Documentation/git-submodule.txt
 @@ -13,6 +13,7 @@ SYNOPSIS
   [--reference repository] [--] repository [path]
  'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...]
  'git submodule' [--quiet] init [--] [path...]
 +'git submodule' [--quiet] deinit [-f|--force] [--] path...
  'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] 
 [--rebase]
   [--reference repository] [--merge] [--recursive] [--] 
 [path...]
  'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) 
 n]
 @@ -134,6 +135,19 @@ init::
 the explicit 'init' step if you do not intend to customize
 any submodule locations.

 +deinit::
 +   Unregister the given submodules, i.e. remove the whole
 +   `submodule.$name` section from .git/config together with their work
 +   tree. Further calls to `git submodule update`, `git submodule foreach`
 +   and `git submodule sync` will skip any unregistered submodules until
 +   they are initialized again, so use this command if you don't want to
 +   have a local checkout of the submodule in your work tree anymore. If
 +   you really want to remove a submodule from the repository and commit
 +   that use linkgit:git-rm[1] instead.
 ++
 +If `--force` is specified, the submodule's work tree will be removed even if
 +it contains local modifications.
 +
  update::
 Update the registered submodules, i.e. clone missing submodules and
 checkout the commit specified in the index of the containing 
 repository.
 @@ -213,8 +227,10 @@ OPTIONS

  -f::
  --force::
 -   This option is only valid for add and update commands.
 +   This option is only valid for add, deinit and update commands.
 When running add, allow adding an otherwise ignored submodule path.
 +   When running deinit the submodule work trees will be removed even if
 +   they contain local changes.
 When running update, throw away local changes in submodules when
 switching to a different commit; and always run a checkout operation
 in the submodule, even if the commit listed in the index of the
 diff --git a/git-submodule.sh b/git-submodule.sh
 index 004c034..f1b552f 100755
 --- a/git-submodule.sh
 +++ b/git-submodule.sh
 @@ -8,6 +8,7 @@ dashless=$(basename $0 | sed -e 's/-/ /')
  USAGE=[--quiet] add [-b branch] [-f|--force] [--name name] [--reference 
 repository] [--] repository [path]
 or: $dashless [--quiet] status 

Re: [PATCH v3 3/4] count-objects: report garbage files in pack directory too

2013-02-12 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:

 +/* A hook for count-objects to report invalid files in pack directory */
 +extern void (*report_garbage)(const char *desc, const char *path, int len, 
 const char *name);

We may want to document the strange way the last three parameters
are used somewhere.  e.g.

shows path (if name is NULL), or prepends path in
front of name (otherwise); only for the latter, path can
be a string that is not NUL-terminated but its length
specified with len and in that case a slash is inserted
between the path and the name.

When described clearly, it sounds somewhat ugly and incoherent API,
even though it covers the immediate need X-.

 + sort_string_list(list);
 +
 + for (p = packed_git; p; p = p-next) {
 + struct string_list_item *item;
 + if (!p-pack_local)
 + continue;
 + strbuf_reset(sb);
 + strbuf_add(sb, p-pack_name,
 +strlen(p-pack_name) - 5); /* .pack */
 + item = string_list_lookup(list, sb.buf);
 + if (!item)
 + continue;
 + /*
 +  * string_list_lookup does not guarantee to return the
 +  * first matched string if it's duplicated.
 +  */

Do we need to even allow duplication?  Why does prepare_packed_git_one()
below have to split the pathname into the base and extension in the first
place?

If you collect all pathnames that end with known extensions (.idx,
.pack and .keep) in the sorted garbage list as separate entries in
prepare_packed_git_one(), report_pack_garbage() can do the right thing
without trusting or iterating over packed_git list at all, I think.  As
the entries are sorted, .pack, .idx and all other valid .ext are grouped
together for the same basename.  If you see a group that have both .pack
and .idx, the group is good.  Otherwise, everybody in the group is bad
(e.g. a lonely .pack file without .idx is an unusable garbage).

How about doing it something along this line, perhaps?

int i;
int beginning_of_this_name = -1;
int seen_bits = 0; /* 01 for .idx, 02 for .pack */
for (i = 0; i  list-nr; i++) {
if (beginning_of_this_name  0)
beginning_of_this_name = i;
else if (list-items[i] and list-items[beginning_of_this_name]
 share the same basename)
; /* keep scanning */
else {
/* one name ended at (i-1) */
if (seen_bits == 3)
; /* both .idx and .pack exist; good */
else
report_garbage_for_one_name(list, 
beginning_of_this_name, i,
seen_bits);
seen_bits = 0;
beginning_of_this_name = i;
}
if (list-items[i] is .idx)
seen_bits |= 1;
if (list-items[i] is .pack)
seen_bits |= 2;

}
if (0 = beginning_of_this_name  seen_bits != 3)
report_garbages_for_one_name(list, beginning_of_this_name, 
list-nr, seen_bits);

with a helper function report_garbage_for_one_name() that would look like this:

report_garbage_for_one_name(...) {
int j;
const char *msg;
switch (seen_bits) {
case 0: msg = no corresponding .idx nor .pack; break;
case 1: msg = no corresponding .pack; break;
case 2: msg = no corresponding .idx; break;
}
for (j = beginning_of_this_name; j  i; j++)
report_garbage(msg, list-items[j]);
}

For the above to work, prepare_packed_git_one() needs to retain only the
paths with known extensions in garbage list. pack-deadbeef.unk can and
should be reported as a garbage immediately when it is seen without being
placed in the list.

 @@ -1045,9 +1100,33 @@ static void prepare_packed_git_one(char *objdir, int 
 local)
   if (!p)
   continue;
   install_packed_git(p);
 - }
 + } else if (!report_garbage) {
 + /*
 +  * the rest of this if-chain requires
 +  * report_garbage != NULL. Stop the chain if
 +  * report_garbage is NULL.
 +  */
 + ;
 + } else if (has_extension(de-d_name, .pack)) {
 + struct string_list_item *item;
 + int n = strlen(path) - 5;
 + item = string_list_append_nodup(garbage,
 +  xstrndup(path, n));
 + 

Re: [PATCH v2] rebase -i: respect core.commentchar

2013-02-12 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes:

 ... the following fixup is also needed to avoid relying on the shell
 emitting a literal backslash when a backslash isn't followed by a known
 escape character.

 -- 8 --

 diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
 index cbe36bf..84bd525 100755
 --- a/t/t3404-rebase-interactive.sh
 +++ b/t/t3404-rebase-interactive.sh
 @@ -947,7 +947,7 @@ test_expect_success 'rebase -i respects core.commentchar' 
 '
   test_when_finished git config --unset core.commentchar 
   cat comment-lines.sh EOF 
  #!$SHELL_PATH
 -sed -e 2,\$ s/^/\\/ \$1 \$1.tmp
 +sed -e 2,\$ s/^// \$1 \$1.tmp
  mv \$1.tmp \$1
  EOF
   chmod a+x comment-lines.sh 

Yeek.  If you used write_script with here-text that does not
interpolate,

write_script remove-all-but-the-first.sh \EOF
sed -e '2,$s/^/\\/'  $1 $1.tmp 
mv $1.tmp $1
EOF

the above would be much more readable.

I am not sure if I understand what you meant by literal backslash
blah blah, though.
--
To unsubscribe from this list: send the line 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] completion: support 'git config --local'

2013-02-12 Thread Junio C Hamano
Matthieu Moy matthieu@imag.fr writes:

 This needs to be done in two places: __git_config_get_set_variables to
 allow clever completion of git config --local --get footab, and
 _git_config to allow git config --loctab to complete to --local.

 While we're there, change the order of options in the code to match
 git-config.txt.

 Signed-off-by: Matthieu Moy matthieu@imag.fr
 ---
 I think this line should include --local:
 
 https://github.com/git/git/blob/next/contrib/completion/git-completion.bash#L1782
 --global|--system|--file=*)
 
 This would help for:
 git config -l --local

 Yes, but not only ;-)

I see the second hunk is new.  Comments?

How would this interract with the writing side of git config?
git config --local foo.bar value and git config foo.bar value
are the same, no?

Is it yes they are the same but it does not hurt?

  contrib/completion/git-completion.bash | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/contrib/completion/git-completion.bash 
 b/contrib/completion/git-completion.bash
 index 5770b6f..2e1ad67 100644
 --- a/contrib/completion/git-completion.bash
 +++ b/contrib/completion/git-completion.bash
 @@ -1570,7 +1570,7 @@ __git_config_get_set_variables ()
   while [ $c -gt 1 ]; do
   word=${words[c]}
   case $word in
 - --global|--system|--file=*)
 + --system|--global|--local|--file=*)
   config_file=$word
   break
   ;;
 @@ -1676,7 +1676,7 @@ _git_config ()
   case $cur in
   --*)
   __gitcomp 
 - --global --system --file=
 + --system --global --local --file=
   --list --replace-all
   --get --get-all --get-regexp
   --add --unset --unset-all
--
To unsubscribe from this list: send the line 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] rebase -i: respect core.commentchar

2013-02-12 Thread John Keeping
On Tue, Feb 12, 2013 at 09:29:26AM -0800, Junio C Hamano wrote:
 John Keeping j...@keeping.me.uk writes:
 
  ... the following fixup is also needed to avoid relying on the shell
  emitting a literal backslash when a backslash isn't followed by a known
  escape character.
 
  -- 8 --
 
  diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
  index cbe36bf..84bd525 100755
  --- a/t/t3404-rebase-interactive.sh
  +++ b/t/t3404-rebase-interactive.sh
  @@ -947,7 +947,7 @@ test_expect_success 'rebase -i respects 
  core.commentchar' '
  test_when_finished git config --unset core.commentchar 
  cat comment-lines.sh EOF 
   #!$SHELL_PATH
  -sed -e 2,\$ s/^/\\/ \$1 \$1.tmp
  +sed -e 2,\$ s/^// \$1 \$1.tmp
   mv \$1.tmp \$1
   EOF
  chmod a+x comment-lines.sh 
 
 Yeek.  If you used write_script with here-text that does not
 interpolate,
 
   write_script remove-all-but-the-first.sh \EOF
   sed -e '2,$s/^/\\/'  $1 $1.tmp 
 mv $1.tmp $1
   EOF
 
 the above would be much more readable.

Yet another thing for me to learn about ;-)

Do you mean to use that outside the test case, so that the single quotes
work?  Or do I still need some level of escaping?

 I am not sure if I understand what you meant by literal backslash
 blah blah, though.

It turns out that having this in the script works (in bash and dash
although I haven't checked what Posix has to say about it):

sed -e 2,$ s/^/\\\/

and is equivalent to:

sed -e '2,$ s/^/\\/'

because backslashes that aren't recognised as part of an escape sequence
are not treated specially.


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


Re: [PATCH v2] rebase -i: respect core.commentchar

2013-02-12 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

  cat comment-lines.sh EOF 
  #!$SHELL_PATH
 -sed -e 2,\$ s/^/\\/ \$1 \$1.tmp
 +sed -e 2,\$ s/^// \$1 \$1.tmp
  mv \$1.tmp \$1
  EOF
  chmod a+x comment-lines.sh 

 Yeek.  If you used write_script with here-text that does not
 interpolate,

   write_script remove-all-but-the-first.sh \EOF
   sed -e '2,$s/^/\\/'  $1 $1.tmp 
 mv $1.tmp $1
   EOF

 the above would be much more readable.

As this is already inside a pair of '', we cannot use single quote
around sed expression without doing the ugly '\''.

So it needs to be more like this, and I think it still is more
readable.

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index cbe36bf..8b3e2cd 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -945,13 +945,11 @@ test_expect_success 'rebase -i respects core.commentchar' 
'
git checkout E^0 
git config core.commentchar \\ 
test_when_finished git config --unset core.commentchar 
-   cat comment-lines.sh EOF 
-#!$SHELL_PATH
-sed -e 2,\$ s/^/\\/ \$1 \$1.tmp
-mv \$1.tmp \$1
-EOF
-   chmod a+x comment-lines.sh 
-   test_set_editor $(pwd)/comment-lines.sh 
+   write_script remove-all-but-first.sh -\EOF 
+   sed -e 2,\$s/^// $1 $1.tmp 
+   mv $1.tmp $1
+   EOF
+   test_set_editor $(pwd)/remove-all-but-first.sh 
git rebase -i B 
test B = $(git cat-file commit HEAD^ | sed -ne \$p)
 '
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4] submodule: add 'deinit' command

2013-02-12 Thread Junio C Hamano
Phil Hord phil.h...@gmail.com writes:

 +   if test $# = 0
 +   then
 +   die $(eval_gettext Use '.' if you really want to 
 deinitialize all submodules)
 +   fi
 +
 +   module_list $@ |
 +   while read mode sha1 stage sm_path
 +   do
 +   die_if_unmatched $mode
 +   name=$(module_name $sm_path) || exit
 +   url=$(git config submodule.$name.url)
 +   if test -z $url
 +   then
 +   say $(eval_gettext No url found for submodule path 
 '\$sm_path' in .git/config)

 Is it safe to shelter the user a little bit more from the git
 internals here and say instead:

Submodule '\$sm_path' is not initialized.

Sounds like a sensible suggestion.

 Also, I think this code will show this message for each submodule on
 'git submodule deinit .'  But I think I would prefer to suppress it in
 that case.  If I have not explicitly stated which submodules to
 deinit,...

But isn't it the way to explicitly say everything under the sun?
After all, what does the message say to git submodule deinit?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH] Replace filepattern with pathspec for consistency

2013-02-12 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

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

 The strings that are fed to _() would be updated with your patch,
 but the replacement will stay to be filepattern translated to the
 target language.  Translators have to actively hunt for the messages
 to update them.  If you left the .po files untouched, they would
 notice when git.pot is updated the next time and that will ensure
 that the affected messages get translated, no?

 Right, and I'm not competent to say how filepattern was translated in
 any of the languages. Let me know if you want a resend without the po/
 part.

I'll remove the tail part of the patch myself.  Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] rebase -i: respect core.commentchar

2013-02-12 Thread John Keeping
On Tue, Feb 12, 2013 at 10:00:26AM -0800, Junio C Hamano wrote:

 So it needs to be more like this, and I think it still is more
 readable.

Agreed.  Will you squash this in or do you want a re-roll?

 diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
 index cbe36bf..8b3e2cd 100755
 --- a/t/t3404-rebase-interactive.sh
 +++ b/t/t3404-rebase-interactive.sh
 @@ -945,13 +945,11 @@ test_expect_success 'rebase -i respects 
 core.commentchar' '
   git checkout E^0 
   git config core.commentchar \\ 
   test_when_finished git config --unset core.commentchar 
 - cat comment-lines.sh EOF 
 -#!$SHELL_PATH
 -sed -e 2,\$ s/^/\\/ \$1 \$1.tmp
 -mv \$1.tmp \$1
 -EOF
 - chmod a+x comment-lines.sh 
 - test_set_editor $(pwd)/comment-lines.sh 
 + write_script remove-all-but-first.sh -\EOF 
 + sed -e 2,\$s/^// $1 $1.tmp 
 + mv $1.tmp $1
 + EOF
 + test_set_editor $(pwd)/remove-all-but-first.sh 
   git rebase -i B 
   test B = $(git cat-file commit HEAD^ | sed -ne \$p)
  '
--
To unsubscribe from this list: send the line 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] rebase -i: respect core.commentchar

2013-02-12 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes:

 I am not sure if I understand what you meant by literal backslash
 blah blah, though.

 It turns out that having this in the script works (in bash and dash
 although I haven't checked what Posix has to say about it):

 sed -e 2,$ s/^/\\\/

 and is equivalent to:

 sed -e '2,$ s/^/\\/'

 because backslashes that aren't recognised as part of an escape sequence
 are not treated specially.

That's POSIX.  Inside a dq pair:

\
The backslash shall retain its special meaning as an escape
character (see Escape Character (Backslash)) only when followed by
one of the following characters when considered special:
$   `  \   newline

So in your example \\\/, the first backslash escapes the second
backslash and together they produce a single backslash, the third
backslash is followed by a slash that is not special at all, so it
produces a second backslash, and the slash stands for itself,
resulting in \\/.
--
To unsubscribe from this list: send the line 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] rebase -i: respect core.commentchar

2013-02-12 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes:

 On Tue, Feb 12, 2013 at 10:00:26AM -0800, Junio C Hamano wrote:

 So it needs to be more like this, and I think it still is more
 readable.

 Agreed.  Will you squash this in or do you want a re-roll?

I can squash this and the previous one into your original to a
single commit.  Thanks.


 diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
 index cbe36bf..8b3e2cd 100755
 --- a/t/t3404-rebase-interactive.sh
 +++ b/t/t3404-rebase-interactive.sh
 @@ -945,13 +945,11 @@ test_expect_success 'rebase -i respects 
 core.commentchar' '
  git checkout E^0 
  git config core.commentchar \\ 
  test_when_finished git config --unset core.commentchar 
 -cat comment-lines.sh EOF 
 -#!$SHELL_PATH
 -sed -e 2,\$ s/^/\\/ \$1 \$1.tmp
 -mv \$1.tmp \$1
 -EOF
 -chmod a+x comment-lines.sh 
 -test_set_editor $(pwd)/comment-lines.sh 
 +write_script remove-all-but-first.sh -\EOF 
 +sed -e 2,\$s/^// $1 $1.tmp 
 +mv $1.tmp $1
 +EOF
 +test_set_editor $(pwd)/remove-all-but-first.sh 
  git rebase -i B 
  test B = $(git cat-file commit HEAD^ | sed -ne \$p)
  '
--
To unsubscribe from this list: send the line 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: Fetch and -t

2013-02-12 Thread Olsen, Alan R
[Sorry for the top-posting. I *hate* Outlook.]

I will need to check why my system is showing old man pages. I am running 
something compiled from the git tree on kernel.org.

Thanks!

-Original Message-
From: Junio C Hamano [mailto:gits...@pobox.com] 
Sent: Monday, February 11, 2013 6:25 PM
To: Olsen, Alan R
Cc: git@vger.kernel.org
Subject: Re: Fetch and -t

Olsen, Alan R alan.r.ol...@intel.com writes:

 I have found that if I add a remote and do a git fetch -t -f 
 remote_name that it *only* pulls tags.

 Reading the man page it seems like it should pull all the remotes and 
 all the tags and the commits only reachable by tags.

This is what appears in the documentation we ship these days.

-t::
--tags::
This is a short-hand for giving refs/tags/*:refs/tags/*
refspec from the command line, to ask all tags to be fetched
and stored locally.  Because this acts as an explicit
refspec, the default refspecs (configured with the
remote.$name.fetch variable) are overridden and not used.

http://git-htmldocs.googlecode.com/git/git-fetch.html

Previous discussion:

http://thread.gmane.org/gmane.comp.version-control.git/180636

A more recent one:

http://thread.gmane.org/gmane.comp.version-control.git/211439/focus=211464

--
To unsubscribe from this list: send the line 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: [PATCHv4 3/6] Git.pm: refactor command_close_bidi_pipe to use _cmd_close

2013-02-12 Thread Junio C Hamano
Michal Nazarewicz m...@google.com writes:

 From: Michal Nazarewicz min...@mina86.com

 The body of the loop in command_close_bidi_pipe function is identical to
 what _cmd_close function does so instead of duplicating, refactor change
 _cmd_close so that it accepts list of file handlers to be closed, which

s/file handlers/file handles/, I 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: [PATCH v4 05/12] sequencer.c: recognize (cherry picked from ... as part of s-o-b footer

2013-02-12 Thread Junio C Hamano
Brandon Casey draf...@gmail.com writes:

 When 'cherry-pick -s' is used to append a signed-off-by line to a cherry
 picked commit, it does not currently detect the (cherry picked from...
 that may have been appended by a previous 'cherry-pick -x' as part of the
 s-o-b footer and it will insert a blank line before appending a new s-o-b.

 Let's detect (cherry picked from...) as part of the footer so that we
 will produce this:
 ...
 +static int is_cherry_picked_from_line(const char *buf, int len)
 +{
 + /*
 +  * We only care that it looks roughly like (cherry picked from ...)
 +  */
 + return len  strlen(cherry_picked_prefix) + 1 
 + !prefixcmp(buf, cherry_picked_prefix)  buf[len - 1] == ')';
 +}

Does the first is it longer than the prefix? check matter?  If it
is not, prefixcmp() would not match anyway, no?

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


Re: [PATCH v4 11/12] format-patch: update append_signoff prototype

2013-02-12 Thread Junio C Hamano
Brandon Casey draf...@gmail.com writes:

 From: Nguyễn Thái Ngọc Duy pclo...@gmail.com

 This is a preparation step for merging with append_signoff from
 sequencer.c

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 Signed-off-by: Brandon Casey bca...@nvidia.com
 Reviewed-by: Jonathan Nieder jrnie...@gmail.com
 ---
  builtin/log.c | 13 +
  log-tree.c| 17 +
  revision.h|  2 +-
  3 files changed, 15 insertions(+), 17 deletions(-)

 diff --git a/builtin/log.c b/builtin/log.c
 index 8f0b2e8..59de484 100644
 --- a/builtin/log.c
 +++ b/builtin/log.c
 @@ -1086,7 +1086,6 @@ int cmd_format_patch(int argc, const char **argv, const 
 char *prefix)
   struct commit *origin = NULL, *head = NULL;
   const char *in_reply_to = NULL;
   struct patch_ids ids;
 - char *add_signoff = NULL;
   struct strbuf buf = STRBUF_INIT;
   int use_patch_format = 0;
   int quiet = 0;
 @@ -1193,16 +1192,6 @@ int cmd_format_patch(int argc, const char **argv, 
 const char *prefix)
   rev.subject_prefix = strbuf_detach(sprefix, NULL);
   }
  
 - if (do_signoff) {
 - const char *committer;
 - const char *endpos;
 - committer = git_committer_info(IDENT_STRICT);
 - endpos = strchr(committer, '');
 - if (!endpos)
 - die(_(bogus committer info %s), committer);
 - add_signoff = xmemdupz(committer, endpos - committer + 1);
 - }
 -
   for (i = 0; i  extra_hdr.nr; i++) {
   strbuf_addstr(buf, extra_hdr.items[i].string);
   strbuf_addch(buf, '\n');
 @@ -1393,7 +1382,7 @@ int cmd_format_patch(int argc, const char **argv, const 
 char *prefix)
   total++;
   start_number--;
   }
 - rev.add_signoff = add_signoff;
 + rev.add_signoff = do_signoff;
   while (0 = --nr) {
   int shown;
   commit = list[nr];
 diff --git a/log-tree.c b/log-tree.c
 index 5dc45c4..ac1cd68 100644
 --- a/log-tree.c
 +++ b/log-tree.c
 @@ -10,6 +10,8 @@
  #include color.h
  #include gpg-interface.h
  
 +#define APPEND_SIGNOFF_DEDUP (1u 0)
 +
  struct decoration name_decoration = { object names };
  
  enum decoration_type {
 @@ -253,9 +255,12 @@ static int detect_any_signoff(char *letter, int size)
   return seen_head  seen_name;
  }
  
 -static void append_signoff(struct strbuf *sb, const char *signoff)
 +static void append_signoff(struct strbuf *sb, int ignore_footer, unsigned 
 flag)
  {
 + unsigned no_dup_sob = flag  APPEND_SIGNOFF_DEDUP;

Unused variable at this step?

   static const char signed_off_by[] = Signed-off-by: ;
 + char *signoff = xstrdup(fmt_name(getenv(GIT_COMMITTER_NAME),
 +getenv(GIT_COMMITTER_EMAIL)));
   size_t signoff_len = strlen(signoff);
   int has_signoff = 0;
   char *cp;
 @@ -275,6 +280,7 @@ static void append_signoff(struct strbuf *sb, const char 
 *signoff)
   if (!isspace(cp[signoff_len]))
   continue;
   /* we already have him */
 + free(signoff);
   return;
   }
  
 @@ -287,6 +293,7 @@ static void append_signoff(struct strbuf *sb, const char 
 *signoff)
   strbuf_addstr(sb, signed_off_by);
   strbuf_add(sb, signoff, signoff_len);
   strbuf_addch(sb, '\n');
 + free(signoff);
  }
  
  static unsigned int digits_in_number(unsigned int number)
 @@ -672,8 +679,10 @@ void show_log(struct rev_info *opt)
   /*
* And then the pretty-printed message itself
*/
 - if (ctx.need_8bit_cte = 0)
 - ctx.need_8bit_cte = has_non_ascii(opt-add_signoff);
 + if (ctx.need_8bit_cte = 0  opt-add_signoff)
 + ctx.need_8bit_cte =
 + has_non_ascii(fmt_name(getenv(GIT_COMMITTER_NAME),
 +getenv(GIT_COMMITTER_EMAIL)));
   ctx.date_mode = opt-date_mode;
   ctx.date_mode_explicit = opt-date_mode_explicit;
   ctx.abbrev = opt-diffopt.abbrev;
 @@ -686,7 +695,7 @@ void show_log(struct rev_info *opt)
   pretty_print_commit(ctx, commit, msgbuf);
  
   if (opt-add_signoff)
 - append_signoff(msgbuf, opt-add_signoff);
 + append_signoff(msgbuf, 0, APPEND_SIGNOFF_DEDUP);
  
   if ((ctx.fmt != CMIT_FMT_USERFORMAT) 
   ctx.notes_message  *ctx.notes_message) {
 diff --git a/revision.h b/revision.h
 index 5da09ee..01bd2b7 100644
 --- a/revision.h
 +++ b/revision.h
 @@ -138,7 +138,7 @@ struct rev_info {
   int reroll_count;
   char*message_id;
   struct string_list *ref_message_ids;
 - const char  *add_signoff;
 + int add_signoff;
   const char  *extra_headers;
   const char  *log_reencode;
   const char  *subject_prefix;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to 

Re: [PATCH v4 05/12] sequencer.c: recognize (cherry picked from ... as part of s-o-b footer

2013-02-12 Thread Brandon Casey
On 2/12/2013 11:13 AM, Junio C Hamano wrote:
 Brandon Casey draf...@gmail.com writes:
 
 When 'cherry-pick -s' is used to append a signed-off-by line to a cherry
 picked commit, it does not currently detect the (cherry picked from...
 that may have been appended by a previous 'cherry-pick -x' as part of the
 s-o-b footer and it will insert a blank line before appending a new s-o-b.

 Let's detect (cherry picked from...) as part of the footer so that we
 will produce this:
 ...
 +static int is_cherry_picked_from_line(const char *buf, int len)
 +{
 +/*
 + * We only care that it looks roughly like (cherry picked from ...)
 + */
 +return len  strlen(cherry_picked_prefix) + 1 
 +!prefixcmp(buf, cherry_picked_prefix)  buf[len - 1] == ')';
 +}
 
 Does the first is it longer than the prefix? check matter?  If it
 is not, prefixcmp() would not match anyway, no?

Probably not in practice, but technically we should only be accessing
len characters in buf even though buf may be longer than len.  So the
check is just making sure the function doesn't access chars it's not
supposed to.

-Brandon


---
This email message is for the sole use of the intended recipient(s) and may 
contain
confidential information.  Any unauthorized review, use, disclosure or 
distribution
is prohibited.  If you are not the intended recipient, please contact the 
sender by
reply email and destroy all copies of the original message.
---
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 05/12] sequencer.c: recognize (cherry picked from ... as part of s-o-b footer

2013-02-12 Thread Junio C Hamano
Brandon Casey bca...@nvidia.com writes:

 +   return len  strlen(cherry_picked_prefix) + 1 
 +   !prefixcmp(buf, cherry_picked_prefix)  buf[len - 1] == ')';
 +}
 
 Does the first is it longer than the prefix? check matter?  If it
 is not, prefixcmp() would not match anyway, no?

 Probably not in practice, but technically we should only be accessing
 len characters in buf even though buf may be longer than len.  So the
 check is just making sure the function doesn't access chars it's not
 supposed to.

Sorry, I do not follow.  Isn't caller's buf terminated with LF at buf[len],
which would never match cherry_picked_prefix even if len is shorter
than the prefix?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 05/12] sequencer.c: recognize (cherry picked from ... as part of s-o-b footer

2013-02-12 Thread Jonathan Nieder
Brandon Casey wrote:
 On 2/12/2013 11:13 AM, Junio C Hamano wrote:
 Brandon Casey draf...@gmail.com writes:

 +static int is_cherry_picked_from_line(const char *buf, int len)
 +{
 +   /*
 +* We only care that it looks roughly like (cherry picked from ...)
 +*/
 +   return len  strlen(cherry_picked_prefix) + 1 
 +   !prefixcmp(buf, cherry_picked_prefix)  buf[len - 1] == ')';
 +}

 Does the first is it longer than the prefix? check matter?  If it
 is not, prefixcmp() would not match anyway, no?

 Probably not in practice, but technically we should only be accessing
 len characters in buf even though buf may be longer than len.

Yep.  Technically the buf[len - 1] == ')' check is enough to avoid
false positives, but if it and the 'len' check were dropped then this
would be checking that buf is a (cherry-picked from line instead of
checking that its first 'len' bytes are one.

So it's just paranoid futureproofing.  In the long term, it would be
nice to drop the number of bytes to ignore at the end argument to
append_signoff to avoid having to think about this kind of thing.

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


Re: [PATCH v4 05/12] sequencer.c: recognize (cherry picked from ... as part of s-o-b footer

2013-02-12 Thread Brandon Casey
On 2/12/2013 11:36 AM, Junio C Hamano wrote:
 Brandon Casey bca...@nvidia.com writes:
 
 +  return len  strlen(cherry_picked_prefix) + 1 
 +  !prefixcmp(buf, cherry_picked_prefix)  buf[len - 1] == ')';
 +}

 Does the first is it longer than the prefix? check matter?  If it
 is not, prefixcmp() would not match anyway, no?

 Probably not in practice, but technically we should only be accessing
 len characters in buf even though buf may be longer than len.  So the
 check is just making sure the function doesn't access chars it's not
 supposed to.
 
 Sorry, I do not follow.  Isn't caller's buf terminated with LF at buf[len],
 which would never match cherry_picked_prefix even if len is shorter
 than the prefix?

Heh, I almost pointed that out in my reply.  Yes, buf will be terminated
with LF at buf[len].  And yes, that means that we will never get a false
positive from prefixcmp even if the comparison overruns buf+len while
doing its comparison.  That's why the check doesn't matter in practice,
i.e. based on the way that is_cherry_picked_from_line is being called
right now and the content of cherry_picked_prefix.

But, hasn't is_cherry_picked_from_line entered into a contract with the
caller and said I will not access more than len characters?

It's ok with me if you think it reads better without the check.

-Brandon


---
This email message is for the sole use of the intended recipient(s) and may 
contain
confidential information.  Any unauthorized review, use, disclosure or 
distribution
is prohibited.  If you are not the intended recipient, please contact the 
sender by
reply email and destroy all copies of the original message.
---
--
To unsubscribe from this list: send the line 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/FYI v4 13/12] fixup! t/t3511: add some tests of 'cherry-pick -s' functionality

2013-02-12 Thread Jonathan Nieder
Brandon Casey wrote:

 I'm not sure we should apply this though.  I'm leaning towards saying that
 the 'cherry-pick -s' behavior with respect to a commit with an empty message
 body should be undefined.  If we want it to be undefined then we probably
 shouldn't introduce a test which would have the effect of defining it.

Maybe it would make sense to just check that cherry-pick doesn't
segfault in this case?

That is, compute the output but don't compare it to expected output, as
in:

test_expect_success 'adding signoff to empty message does something 
sane' '
git reset --hard HEAD^ 
git cherry-pick --allow-empty-message -s empty-branch 
git show --pretty=format:%B -s empty-branch actual 

# sign-off is included *somewhere*
grep ^Signed-off-by:.*\$ actual
'

Alternatively, if there are only a few sane behaviors, a test can check
for all of them and pass as long as git follows one.  I haven't thought
carefully enough about this example to suggest doing that.

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


Re: [PATCH v4 05/12] sequencer.c: recognize (cherry picked from ... as part of s-o-b footer

2013-02-12 Thread Junio C Hamano
Brandon Casey bca...@nvidia.com writes:

 On 2/12/2013 11:36 AM, Junio C Hamano wrote:
 Brandon Casey bca...@nvidia.com writes:
 
 + return len  strlen(cherry_picked_prefix) + 1 
 + !prefixcmp(buf, cherry_picked_prefix)  buf[len - 1] == ')';
 +}

 Does the first is it longer than the prefix? check matter?  If it
 is not, prefixcmp() would not match anyway, no?

 Probably not in practice, but technically we should only be accessing
 len characters in buf even though buf may be longer than len.  So the
 check is just making sure the function doesn't access chars it's not
 supposed to.
 
 Sorry, I do not follow.  Isn't caller's buf terminated with LF at buf[len],
 which would never match cherry_picked_prefix even if len is shorter
 than the prefix?

 Heh, I almost pointed that out in my reply.  Yes, buf will be terminated
 with LF at buf[len].  And yes, that means that we will never get a false
 positive from prefixcmp even if the comparison overruns buf+len while
 doing its comparison.  That's why the check doesn't matter in practice,
 i.e. based on the way that is_cherry_picked_from_line is being called
 right now and the content of cherry_picked_prefix.

 But, hasn't is_cherry_picked_from_line entered into a contract with the
 caller and said I will not access more than len characters?

 It's ok with me if you think it reads better without the check.

As Jonathan says, if you rewrite it to

return buf[len - 1] == ')'  !prefixcmp(buf, cherry_picked_prefix);

then the code can keep its promise without the length check, because
it knows there is no ')' in cherry-picked-prefix, and it also knows
prefixcmp() stops at the first difference.

It is not a huge deal; I was primarily reacting to the ugly multi-line
boolean expresion that is not inside a pair of parentheses (and because
this is a return statement, there is no good reason to have parentheses
except that this is a multi-line expression), which looked odd.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/2] Add bash.showUntrackedFiles config option

2013-02-12 Thread Martin Erik Werner
Hi,

Here is a patch adding a config option for showing untracked files in
the shell prompt, I've noticed having it enabled tends to make the
prompt act very sluggish in some cases (large repos / unfriendly
filesystems). So it would be nice to have a more fine-grained control
over it, similar to what exists for bash.showDirtyState.

Martin Erik Werner (2):
  bash completion: add bash.showUntrackedFiles option
  t9903: add test case for bash.showUntrackedFiles

 contrib/completion/git-prompt.sh |   11 ---
 t/t9903-bash-prompt.sh   |   11 +++
 2 files changed, 19 insertions(+), 3 deletions(-)

-- 
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] t9903: add test case for bash.showUntrackedFiles

2013-02-12 Thread Martin Erik Werner
Add a test case for the bash.showUntrackedFiles config option, which
checks that the config option can disable the global effect of the
GIT_PS1_SHOWUNTRACKEDFILES environmant variable.

Signed-off-by: Martin Erik Werner martinerikwer...@gmail.com
---
 t/t9903-bash-prompt.sh |   11 +++
 1 file changed, 11 insertions(+)

diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index f17c1f8..c9417b9 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -447,6 +447,17 @@ test_expect_success 'prompt - untracked files status 
indicator - not shown insid
test_cmp expected $actual
 '
 
+test_expect_success 'prompt - untracked files status indicator - disabled by 
config' '
+   printf  (master)  expected 
+   echo untracked  file_untracked 
+   test_config bash.showUntrackedFiles false 
+   (
+   GIT_PS1_SHOWUNTRACKEDFILES=y 
+   __git_ps1  $actual
+   ) 
+   test_cmp expected $actual
+'
+
 test_expect_success 'prompt - format string starting with dash' '
printf -- -master  expected 
__git_ps1 -%s  $actual 
-- 
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 1/2] bash completion: add bash.showUntrackedFiles option

2013-02-12 Thread Martin Erik Werner
Add a config option 'bash.showUntrackedFiles' which allows enabling
the prompt showing untracked files on a per-repository basis. This is
useful for some repositories where the 'git ls-files ...' command may
take a long time.

Signed-off-by: Martin Erik Werner martinerikwer...@gmail.com
---
 contrib/completion/git-prompt.sh |   11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 9bef053..9b2eec2 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -43,7 +43,10 @@
 #
 # If you would like to see if there're untracked files, then you can set
 # GIT_PS1_SHOWUNTRACKEDFILES to a nonempty value. If there're untracked
-# files, then a '%' will be shown next to the branch name.
+# files, then a '%' will be shown next to the branch name.  You can
+# configure this per-repository with the bash.showUntrackedFiles
+# variable, which defaults to true once GIT_PS1_SHOWUNTRACKEDFILES is
+# enabled.
 #
 # If you would like to see the difference between HEAD and its upstream,
 # set GIT_PS1_SHOWUPSTREAM=auto.  A  indicates you are behind, 
@@ -332,8 +335,10 @@ __git_ps1 ()
fi
 
if [ -n ${GIT_PS1_SHOWUNTRACKEDFILES-} ]; then
-   if [ -n $(git ls-files --others 
--exclude-standard) ]; then
-   u=%
+   if [ $(git config --bool 
bash.showUntrackedFiles) != false ]; then
+   if [ -n $(git ls-files --others 
--exclude-standard) ]; then
+   u=%
+   fi
fi
fi
 
-- 
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 v4 00/12] unify appending of sob

2013-02-12 Thread Jonathan Nieder
Brandon Casey wrote:

 Round 4.

Yay.  I think this is cooked now and a good foundation for later
changes on top.

For what it's worth, with or without the two tweaks Junio suggested
(simplifying (cherry picked from detection, deferring introduction
of no_dup_sob variable until it is used),
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/FYI v4 13/12] fixup! t/t3511: add some tests of 'cherry-pick -s' functionality

2013-02-12 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes:

 Brandon Casey wrote:

 I'm not sure we should apply this though.  I'm leaning towards saying that
 the 'cherry-pick -s' behavior with respect to a commit with an empty message
 body should be undefined.  If we want it to be undefined then we probably
 shouldn't introduce a test which would have the effect of defining it.

 Maybe it would make sense to just check that cherry-pick doesn't
 segfault in this case?

;-)


 That is, compute the output but don't compare it to expected output, as
 in:

   test_expect_success 'adding signoff to empty message does something 
 sane' '
   git reset --hard HEAD^ 
   git cherry-pick --allow-empty-message -s empty-branch 
   git show --pretty=format:%B -s empty-branch actual 

   # sign-off is included *somewhere*
   grep ^Signed-off-by:.*\$ actual
   '

Isn't what the current code happens to do is the best we could do?
We would end up showing one entry whose title appears to be
Signed-off-by: ... in the shortlog output if we did so.  If we
added an empty line, then the shortlog output will have a single
empty line that is equally unsightly.

We could force a message like this:

tree d7f87518a26e9f00714675706f165b94f3625177
parent f459a4b602c0f4d371e1717572de6d0c4d39c6b1
author Junio C Hamano gits...@pobox.com 1360699963 -0800
committer Junio C Hamano gits...@pobox.com 1360699980 -0800

!!cherry-picked from a commit without any message!!

Signed-off-by: Junio C Hamano gits...@pobox.com

but I do not think that buys us much; it only replaces a totally
uninformative empty line with another totally uninformative junk.

That ugliness is a price the insane person, who is cherry picking a
commit without any justification made by another insane person,
indicates that he is willing to pay by doing so.  At that point I do
not think we should care.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] Documentation/Makefile: fix spaces around assignments

2013-02-12 Thread John Keeping
A simple style fix; no functional change.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 Documentation/Makefile | 42 +-
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 62dbd9a..af3d8a4 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -31,11 +31,11 @@ MAN7_TXT += gittutorial.txt
 MAN7_TXT += gitworkflows.txt
 
 MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)
-MAN_XML=$(patsubst %.txt,%.xml,$(MAN_TXT))
-MAN_HTML=$(patsubst %.txt,%.html,$(MAN_TXT))
+MAN_XML = $(patsubst %.txt,%.xml,$(MAN_TXT))
+MAN_HTML = $(patsubst %.txt,%.html,$(MAN_TXT))
 
 OBSOLETE_HTML = git-remote-helpers.html
-DOC_HTML=$(MAN_HTML) $(OBSOLETE_HTML)
+DOC_HTML = $(MAN_HTML) $(OBSOLETE_HTML)
 
 ARTICLES = howto-index
 ARTICLES += everyday
@@ -74,35 +74,35 @@ SP_ARTICLES += technical/api-index
 
 DOC_HTML += $(patsubst %,%.html,$(ARTICLES) $(SP_ARTICLES))
 
-DOC_MAN1=$(patsubst %.txt,%.1,$(MAN1_TXT))
-DOC_MAN5=$(patsubst %.txt,%.5,$(MAN5_TXT))
-DOC_MAN7=$(patsubst %.txt,%.7,$(MAN7_TXT))
+DOC_MAN1 = $(patsubst %.txt,%.1,$(MAN1_TXT))
+DOC_MAN5 = $(patsubst %.txt,%.5,$(MAN5_TXT))
+DOC_MAN7 = $(patsubst %.txt,%.7,$(MAN7_TXT))
 
-prefix?=$(HOME)
-bindir?=$(prefix)/bin
-htmldir?=$(prefix)/share/doc/git-doc
-pdfdir?=$(prefix)/share/doc/git-doc
-mandir?=$(prefix)/share/man
-man1dir=$(mandir)/man1
-man5dir=$(mandir)/man5
-man7dir=$(mandir)/man7
-# DESTDIR=
+prefix ?= $(HOME)
+bindir ?= $(prefix)/bin
+htmldir ?= $(prefix)/share/doc/git-doc
+pdfdir ?= $(prefix)/share/doc/git-doc
+mandir ?= $(prefix)/share/man
+man1dir = $(mandir)/man1
+man5dir = $(mandir)/man5
+man7dir = $(mandir)/man7
+# DESTDIR =
 
 ASCIIDOC = asciidoc
 ASCIIDOC_EXTRA =
 MANPAGE_XSL = manpage-normal.xsl
 XMLTO = xmlto
 XMLTO_EXTRA =
-INSTALL?=install
+INSTALL ?= install
 RM ?= rm -f
 MAN_REPO = ../../git-manpages
 HTML_REPO = ../../git-htmldocs
 
-infodir?=$(prefix)/share/info
-MAKEINFO=makeinfo
-INSTALL_INFO=install-info
-DOCBOOK2X_TEXI=docbook2x-texi
-DBLATEX=dblatex
+infodir ?= $(prefix)/share/info
+MAKEINFO = makeinfo
+INSTALL_INFO = install-info
+DOCBOOK2X_TEXI = docbook2x-texi
+DBLATEX = dblatex
 ifndef PERL_PATH
PERL_PATH = /usr/bin/perl
 endif
-- 
1.8.1.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


[PATCH 0/3] Fix installation paths with make install-doc

2013-02-12 Thread John Keeping
When using the top-level install-doc target the html, info and man
target directories are inherited from the top-level Makefile by the
documentation Makefile as relative paths, which is not expected and
results in the files being installed in an unexpected location.

The first two patches are simple style fixes.  The third one fixes the
issue described above.

John Keeping (3):
  Documentation/Makefile: fix spaces around assignments
  Documentation/Makefile: move infodir to be with other '*dir's
  Documentation/Makefile: fix inherited {html,info,man}dir

 Documentation/Makefile | 88 --
 1 file changed, 56 insertions(+), 32 deletions(-)

-- 
1.8.1.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


[PATCH 3/3] Documentation/Makefile: fix inherited {html,info,man}dir

2013-02-12 Thread John Keeping
Commit e14421b (Allow INSTALL, bindir, mandir to be set in main Makefile
- 2006-06-29) changed Documentation/Makefile to inherit the value of
mandir from the top-level Makefile when invoked as make install-doc at
the top-level.  This was inherited by infodir and htmldir when they were
added.

This was broken by commit 026fa0d (Move computation of absolute paths
from Makefile to runtime (in preparation for RUNTIME_PREFIX) -
2009-01-18) which changed these variables to have relative paths in the
top-level Makefile, causing the documentation to be installed into the
path without $(prefix) prepended.

Fix this by changing the defaults to be paths relative to $(prefix) and
introducing new variables {html,info,man}_instdir which contain the full
installation paths.

Signed-off-by: John Keeping j...@keeping.me.uk
---
I'm not sure if this is the best approach - the alternative would be to
change the top-level Makefile to use {html,info,man}dir_relative and
derive the {html,info,man}dir variables from that.

The top-level Makefile is inconsistent in the approach it takes - bindir
is derived from bindir_relative but gitexecdir and template_dir have
gitexec_instdir and template_instdir derived from them.

 Documentation/Makefile | 56 +++---
 1 file changed, 40 insertions(+), 16 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 0cfdc36..34cd9f2 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -78,15 +78,21 @@ DOC_MAN1 = $(patsubst %.txt,%.1,$(MAN1_TXT))
 DOC_MAN5 = $(patsubst %.txt,%.5,$(MAN5_TXT))
 DOC_MAN7 = $(patsubst %.txt,%.7,$(MAN7_TXT))
 
+# The following variables can be relative paths due to the way they can be
+# inherited from the top-level Makefile:
+#   htmldir
+#   infodir
+#   mandir
+# Note that pdfdir is an exception to this since it is not used by git-help.
 prefix ?= $(HOME)
 bindir ?= $(prefix)/bin
-htmldir ?= $(prefix)/share/doc/git-doc
-infodir ?= $(prefix)/share/info
 pdfdir ?= $(prefix)/share/doc/git-doc
-mandir ?= $(prefix)/share/man
-man1dir = $(mandir)/man1
-man5dir = $(mandir)/man5
-man7dir = $(mandir)/man7
+htmldir ?= share/doc/git-doc
+infodir ?= share/info
+mandir ?= share/man
+man1dir = $(man_instdir)/man1
+man5dir = $(man_instdir)/man5
+man7dir = $(man_instdir)/man7
 # DESTDIR =
 
 ASCIIDOC = asciidoc
@@ -110,6 +116,24 @@ endif
 -include ../config.mak.autogen
 -include ../config.mak
 
+ifneq ($(filter /%,$(firstword $(htmldir))),)
+html_instdir = $(htmldir)
+else
+html_instdir = $(prefix)/$(htmldir)
+endif
+
+ifneq ($(filter /%,$(firstword $(infodir))),)
+info_instdir = $(infodir)
+else
+info_instdir = $(prefix)/$(infodir)
+endif
+
+ifneq ($(filter /%,$(firstword $(mandir))),)
+man_instdir = $(mandir)
+else
+man_instdir = $(prefix)/$(mandir)
+endif
+
 #
 # For docbook-xsl ...
 #  -1.68.1,no extra settings are needed?
@@ -144,7 +168,7 @@ endif
 # Distros may want to use MAN_BASE_URL=file:///path/to/git/docs/
 # or similar.
 ifndef MAN_BASE_URL
-MAN_BASE_URL = file://$(htmldir)/
+MAN_BASE_URL = file://$(html_instdir)/
 endif
 XMLTO_EXTRA += -m manpage-base-url.xsl
 
@@ -220,13 +244,13 @@ install-man: man
$(INSTALL) -m 644 $(DOC_MAN7) $(DESTDIR)$(man7dir)
 
 install-info: info
-   $(INSTALL) -d -m 755 $(DESTDIR)$(infodir)
-   $(INSTALL) -m 644 git.info gitman.info $(DESTDIR)$(infodir)
-   if test -r $(DESTDIR)$(infodir)/dir; then \
- $(INSTALL_INFO) --info-dir=$(DESTDIR)$(infodir) git.info ;\
- $(INSTALL_INFO) --info-dir=$(DESTDIR)$(infodir) gitman.info ;\
+   $(INSTALL) -d -m 755 $(DESTDIR)$(info_instdir)
+   $(INSTALL) -m 644 git.info gitman.info $(DESTDIR)$(info_instdir)
+   if test -r $(DESTDIR)$(info_instdir)/dir; then \
+ $(INSTALL_INFO) --info-dir=$(DESTDIR)$(info_instdir) git.info ;\
+ $(INSTALL_INFO) --info-dir=$(DESTDIR)$(info_instdir) gitman.info ;\
else \
- echo No directory found in $(DESTDIR)$(infodir) 2 ; \
+ echo No directory found in $(DESTDIR)$(info_instdir) 2 ; \
fi
 
 install-pdf: pdf
@@ -234,7 +258,7 @@ install-pdf: pdf
$(INSTALL) -m 644 user-manual.pdf $(DESTDIR)$(pdfdir)
 
 install-html: html
-   '$(SHELL_PATH_SQ)' ./install-webdoc.sh $(DESTDIR)$(htmldir)
+   '$(SHELL_PATH_SQ)' ./install-webdoc.sh $(DESTDIR)$(html_instdir)
 
 ../GIT-VERSION-FILE: FORCE
$(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE
@@ -402,14 +426,14 @@ require-manrepo::
then echo git-manpages repository must exist at $(MAN_REPO); exit 1; 
fi
 
 quick-install-man: require-manrepo
-   '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(MAN_REPO) 
$(DESTDIR)$(mandir)
+   '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(MAN_REPO) 
$(DESTDIR)$(man_instdir)
 
 require-htmlrepo::
@if test ! -d $(HTML_REPO); \
then echo git-htmldocs repository must exist at $(HTML_REPO); exit 1; 
fi
 
 quick-install-html: require-htmlrepo
-   '$(SHELL_PATH_SQ)' 

[PATCH 2/3] Documentation/Makefile: move infodir to be with other '*dir's

2013-02-12 Thread John Keeping
Signed-off-by: John Keeping j...@keeping.me.uk
---
 Documentation/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index af3d8a4..0cfdc36 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -81,6 +81,7 @@ DOC_MAN7 = $(patsubst %.txt,%.7,$(MAN7_TXT))
 prefix ?= $(HOME)
 bindir ?= $(prefix)/bin
 htmldir ?= $(prefix)/share/doc/git-doc
+infodir ?= $(prefix)/share/info
 pdfdir ?= $(prefix)/share/doc/git-doc
 mandir ?= $(prefix)/share/man
 man1dir = $(mandir)/man1
@@ -98,7 +99,6 @@ RM ?= rm -f
 MAN_REPO = ../../git-manpages
 HTML_REPO = ../../git-htmldocs
 
-infodir ?= $(prefix)/share/info
 MAKEINFO = makeinfo
 INSTALL_INFO = install-info
 DOCBOOK2X_TEXI = docbook2x-texi
-- 
1.8.1.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: Pushing a git repository to a new server

2013-02-12 Thread Jeff King
On Tue, Feb 12, 2013 at 12:28:53PM +0100, Michael J Gruber wrote:

 I'm not sure providers like GitHub would fancy an interface which allows
 the programmatic creation of repos (giving a new meaning to fork
 bomb). But I bet you know better ;-)

You can already do that:

  http://developer.github.com/v3/repos/#create

We rate-limit API requests, and I imagine we might do something similar
with create-over-git. But that is exactly the kind of implementation
detail that can go into a custom create-repo script.

 An alternative would be to teach git (the client) about repo types and
 how to create them. After all, a repo URL ssh://host/path gives a
 clear indication that ssh host git init path will create a repo.

But that's the point of a microformat. It _doesn't_ always work, because
the server may not allow arbitrary commands, or may have special
requirements on top of the init. You can make the microformat be git
init path, and servers can intercept calls to git init and translate
them into custom magic. But I think the world is a little simpler if we
define a new service type (alongside git-upload-pack, git-receive-pack,
etc), and let clients request it. Then it's clear what the client is
trying to do, it's easy for servers to hook into it, we can request it
over http, etc. And it can be extended over time to take more fields
(like repo description, etc).

I'm really not suggesting anything drastic. The wrapper case for ssh
would be as simple as a 3-line shell script which calls git init under
the hood, but it provides one level of indirection that makes
replacing/hooking it much simpler for servers. So the parts that are in
stock git would not be much work (most of the work would be on _calling_
it, but that is the same for adding a call to git init).

I think the main reason the idea hasn't gone anywhere is that nobody
really cares _that_ much. People just don't create repositories that
often. I feel like this is one of those topics that comes up once a
year, and then nothing happens on it, because people just make their
repo manually and then stop caring about it.

Just my two cents, of course. :)

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


Re: [PATCH v4 00/12] unify appending of sob

2013-02-12 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes:

 Brandon Casey wrote:

 Round 4.

 Yay.  I think this is cooked now and a good foundation for later
 changes on top.

 For what it's worth, with or without the two tweaks Junio suggested
 (simplifying (cherry picked from detection, deferring introduction
 of no_dup_sob variable until it is used),
 Reviewed-by: Jonathan Nieder jrnie...@gmail.com

Yeah, I am inclined to merge this to 'next' without any tweak, and
let it cook and get polished incrementally.  I am not sure if we
have enough time to graduate it to 'master' for the upcoming
release, though.

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


Re: [PATCHv4 3/6] Git.pm: refactor command_close_bidi_pipe to use _cmd_close

2013-02-12 Thread Jeff King
On Tue, Feb 12, 2013 at 10:55:05AM -0800, Junio C Hamano wrote:

 Michal Nazarewicz m...@google.com writes:
 
  From: Michal Nazarewicz min...@mina86.com
 
  The body of the loop in command_close_bidi_pipe function is identical to
  what _cmd_close function does so instead of duplicating, refactor change
  _cmd_close so that it accepts list of file handlers to be closed, which
 
 s/file handlers/file handles/, I think.

And s/refactor change/refactor/.

Other than that, I think the series looks OK. I have one style micro-nit
on patch 4 which I'll reply in-line. But it is either fix while
applying or ignore, I don't think it will be worth a re-roll.

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


Re: inotify to minimize stat() calls

2013-02-12 Thread Karsten Blees
Am 11.02.2013 04:53, schrieb Duy Nguyen:
 On Sun, Feb 10, 2013 at 11:58 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 Karsten Blees has done something similar-ish on Windows, and he posted
 the results here:

 https://groups.google.com/forum/#!topic/msysgit/fL_jykUmUNE/discussion


The new hashtable implementation in fscache [1] supports O(1) removal and has 
no mingw dependencies - might come in handy for anyone trying to implement an 
inotify daemon.

[1] https://github.com/kblees/git/commit/f7eb85c2

 I also seem to remember he doing a ReadDirectoryChangesW version, but
 I don't remember what happened with that.
 
 Thanks. I came across that but did not remember. For one thing, we
 know the inotify alternative for Windows: ReadDirectoryChangesW.
 

I dropped ReadDirectoryChangesW because maintaining a 'live' file system cache 
became more and more complicated. For example, according to MSDN docs, 
ReadDirectoryChangesW *may* report short DOS 8.3 names (i.e. PROGRA~1 instead 
of Program Files), so a correct and fast cache implementation would have to 
be indexed by long *and* short names...

Another problem was that the 'live' cache had quite negative performance impact 
on mutating git commands (checkout, reset...). An inotify daemon running as a 
background process (not in-process as fscache) will probably affect everyone 
that modifies the working copy, e.g. running 'make' or the test-suite. This 
should be considered in the design.

 I copy git status's (impressive) numbers from fscache-v0 for those
 who are interested in:
 
 preload | -u  | normal | cached | gain
 +-+++--
 false   | all | 25.144 | 3.055  |  8.2
 false   | no  | 22.822 | 1.748  | 12.8
 true| all |  9.234 | 2.179  |  4.2
 true| no  |  6.833 | 0.955  |  7.2
 

Note that I wasn't able to reproduce such bad 'normal' values in later tests, I 
guess disk fragmentation and/or virus scanner must have tricked me on that 
day...gain factors of 2.5 - 5 are more appropriate.


However, the difference between git status -uall and -uno was always about 1.3 
s in all fscache versions, even though opendir/readdir/closedir was served 
entirely from the cache. I added a bit of performance tracing to find the 
cause, and I think most of the time spent in wt_status_collect_untracked can be 
eliminated:

1.) 0.939 s is spent in dir.c/excluded (i.e. checking .gitignore). This check 
is done for *every* file in the working copy, including files in the index. 
Checking the index first could eliminate most of that, i.e.:

(Note: patches are for discussion only, I'm aware that they may have unintended 
side effects...)

@@ -1097,6 +1097,8 @@ static enum path_treatment treat_path(struct dir_struct 
*dir,
return path_ignored;
strbuf_setlen(path, baselen);
strbuf_addstr(path, de-d_name);
+   if (cache_name_exists(path-buf, path-len, ignore_case))
+   return path_ignored;
if (simplify_away(path-buf, path-len, simplify))
return path_ignored;
---


2.) 0.135 s is spent in name-hash.c/hash_index_entry_directories, reindexing 
the same directories over and over again. In the end, the hashtable contains 
939k directory entries, even though the WebKit test repo only has 7k 
directories. Checking if a directory entry already exists could reduce that, 
i.e.:

@@ -53,14 +55,23 @@ static void hash_index_entry_directories(struct index_state 
*istate, struct cach
unsigned int hash;
void **pos;
double t = ticks();
+   struct cache_entry *ce2;
+   int len = ce_namelen(ce);
 
-   const char *ptr = ce-name;
-   while (*ptr) {
-   while (*ptr  *ptr != '/')
-   ++ptr;
-   if (*ptr == '/') {
-   ++ptr;
-   hash = hash_name(ce-name, ptr - ce-name);
+   while (len  0) {
+   while (len  0  ce-name[len - 1] != '/')
+   len--;
+   if (len  0) {
+   hash = hash_name(ce-name, len);
+   ce2 = lookup_hash(hash, istate-name_hash);
+   while (ce2) {
+   if (same_name(ce2, ce-name, len, ignore_case)) 
{
+   add_since(t, hash_dirs);
+   return;
+   }
+   ce2 = ce2-dir_next;
+   }
+   len--;
pos = insert_hash(hash, ce, istate-name_hash);
if (pos) {
ce-dir_next = *pos;
---


Tests were done with the WebKit repo (~200k files, ~7k directories, 15 
.gitignore files, ~100 entries in root .gitignore). Instrumented code can be 
found here: https://github.com/kblees/git/tree/kb/git-status-performance-tracing

Here's the performance traces of 'git status -s -uall'

Before patches:

trace: at 

Re: [PATCHv4 4/6] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe

2013-02-12 Thread Jeff King
On Tue, Feb 12, 2013 at 03:02:31PM +0100, Michal Nazarewicz wrote:

  sub command_close_bidi_pipe {
   local $?;
   my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
 - _cmd_close($ctx, $in, $out);
 + _cmd_close($ctx, grep defined, $in, $out);

Maybe it is just me, but I find the grep EXPR form a little subtle
inside an argument list. Either:

  _cmd_close($ctx, grep { defined } $in, $out);

or

  _cmd_close($ctx, grep(defined, $in, $out));

is a little more obvious to me.

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


Re: [PATCH 1/3] Documentation/Makefile: fix spaces around assignments

2013-02-12 Thread Jonathan Nieder
Hi,

John Keeping wrote:

 [Subject: [PATCH 1/3] Documentation/Makefile: fix spaces around assignments]

It's not so much fix spaces as use consistent spacing, no?

Aside from that nit, looks like a sensible no-op to me, so
Reviewed-by: Jonathan Nieder jrnie...@gmail.com

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


Re: [PATCH 2/3] Documentation/Makefile: move infodir to be with other '*dir's

2013-02-12 Thread Jonathan Nieder
John Keeping wrote:

 Signed-off-by: John Keeping j...@keeping.me.uk
[...]
 --- a/Documentation/Makefile
 +++ b/Documentation/Makefile
 @@ -81,6 +81,7 @@ DOC_MAN7 = $(patsubst %.txt,%.7,$(MAN7_TXT))
  prefix ?= $(HOME)
  bindir ?= $(prefix)/bin
  htmldir ?= $(prefix)/share/doc/git-doc
 +infodir ?= $(prefix)/share/info
  pdfdir ?= $(prefix)/share/doc/git-doc
  mandir ?= $(prefix)/share/man
  man1dir = $(mandir)/man1
 @@ -98,7 +99,6 @@ RM ?= rm -f
  MAN_REPO = ../../git-manpages
  HTML_REPO = ../../git-htmldocs
  
 -infodir ?= $(prefix)/share/info
  MAKEINFO = makeinfo

Is this another stylefix or is there a functional reason for this
change?
--
To unsubscribe from this list: send the line 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] completion: support 'git config --local'

2013-02-12 Thread Jeff King
On Tue, Feb 12, 2013 at 09:34:39AM -0800, Junio C Hamano wrote:

 I see the second hunk is new.  Comments?
 [...]
  @@ -1676,7 +1676,7 @@ _git_config ()
  case $cur in
  --*)
  __gitcomp 
  -   --global --system --file=
  +   --system --global --local --file=
  --list --replace-all
  --get --get-all --get-regexp
  --add --unset --unset-all

It makes sense to me. It just means that --local itself gets completed
(while the other hunk is about using the presence of --local impacting
other completion). It's an orthogonal issue, but I don't mind them in
the same patch.

 How would this interract with the writing side of git config?
 git config --local foo.bar value and git config foo.bar value
 are the same, no?
 
 Is it yes they are the same but it does not hurt?

It doesn't affect writing at all. The change is in
__git_config_get_set_variables, which is used only here:

  --get|--get-all|--unset|--unset-all)
__gitcomp_nl $(__git_config_get_set_variables)

So it is purely about completing existing variables, and it's right to
limit itself to a particular file if we know that is what has been
given.

I'm not sure I understand the original poster's point about git config
-l --local. -l does not take a limiter, does it?

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


Re: [PATCHv4 3/6] Git.pm: refactor command_close_bidi_pipe to use _cmd_close

2013-02-12 Thread Michal Nazarewicz
 Michal Nazarewicz m...@google.com writes:
  The body of the loop in command_close_bidi_pipe function is identical to
  what _cmd_close function does so instead of duplicating, refactor change
  _cmd_close so that it accepts list of file handlers to be closed, which

 On Tue, Feb 12, 2013 at 10:55:05AM -0800, Junio C Hamano wrote:
 s/file handlers/file handles/, I think.

On Tue, Feb 12 2013, Jeff King wrote:
 And s/refactor change/refactor/.

 Other than that, I think the series looks OK. I have one style micro-nit
 on patch 4 which I'll reply in-line. But it is either fix while
 applying or ignore, I don't think it will be worth a re-roll.

All fixed.

Junio, do you want me to resend or would you be fine with just pulling:

git://github.com/mina86/git.git master

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

pgpLiQYC6X4hj.pgp
Description: PGP signature


Re: [PATCHv4 4/6] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe

2013-02-12 Thread Michal Nazarewicz
On Tue, Feb 12 2013, Jeff King wrote:
 On Tue, Feb 12, 2013 at 03:02:31PM +0100, Michal Nazarewicz wrote:

  sub command_close_bidi_pipe {
  local $?;
  my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
 -_cmd_close($ctx, $in, $out);
 +_cmd_close($ctx, grep defined, $in, $out);

 Maybe it is just me, but I find the grep EXPR form a little subtle
 inside an argument list. Either:

   _cmd_close($ctx, grep { defined } $in, $out);

 or

   _cmd_close($ctx, grep(defined, $in, $out));

 is a little more obvious to me.

I personally avoid parens whenever possible in Perl, but Git.pm seem to
favour them so I went with the second option.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

pgpkczeNsdtRQ.pgp
Description: PGP signature


Re: [PATCHv4 4/6] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe

2013-02-12 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Tue, Feb 12, 2013 at 03:02:31PM +0100, Michal Nazarewicz wrote:

  sub command_close_bidi_pipe {
  local $?;
  my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
 -_cmd_close($ctx, $in, $out);
 +_cmd_close($ctx, grep defined, $in, $out);

 Maybe it is just me, but I find the grep EXPR form a little subtle
 inside an argument list. Either:

   _cmd_close($ctx, grep { defined } $in, $out);

 or

   _cmd_close($ctx, grep(defined, $in, $out));

 is a little more obvious to me.

I would actually vote for the most explicit:

_cmd_close($ctx, (grep { defined } ($in, $out)));

--
To unsubscribe from this list: send the line 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: [PATCHv4 3/6] Git.pm: refactor command_close_bidi_pipe to use _cmd_close

2013-02-12 Thread Junio C Hamano
Michal Nazarewicz min...@mina86.com writes:

 Michal Nazarewicz m...@google.com writes:
  The body of the loop in command_close_bidi_pipe function is identical to
  what _cmd_close function does so instead of duplicating, refactor change
  _cmd_close so that it accepts list of file handlers to be closed, which

 On Tue, Feb 12, 2013 at 10:55:05AM -0800, Junio C Hamano wrote:
 s/file handlers/file handles/, I think.

 On Tue, Feb 12 2013, Jeff King wrote:
 And s/refactor change/refactor/.

 Other than that, I think the series looks OK. I have one style micro-nit
 on patch 4 which I'll reply in-line. But it is either fix while
 applying or ignore, I don't think it will be worth a re-roll.

 All fixed.

 Junio, do you want me to resend or would you be fine with just pulling:

   git://github.com/mina86/git.git master

Neither.  I agree with Peff that these micronits are not enough
reason for the trouble of rerolling the series, so I'll just amend
them at my end.  Please double-check what you see on the 'pu' branch
when I push today's integration result out later.

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


Re: [PATCHv4 4/6] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe

2013-02-12 Thread Jeff King
On Tue, Feb 12, 2013 at 01:14:57PM -0800, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  On Tue, Feb 12, 2013 at 03:02:31PM +0100, Michal Nazarewicz wrote:
 
   sub command_close_bidi_pipe {
 local $?;
 my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
  -  _cmd_close($ctx, $in, $out);
  +  _cmd_close($ctx, grep defined, $in, $out);
 
  Maybe it is just me, but I find the grep EXPR form a little subtle
  inside an argument list. Either:
 
_cmd_close($ctx, grep { defined } $in, $out);
 
  or
 
_cmd_close($ctx, grep(defined, $in, $out));
 
  is a little more obvious to me.
 
 I would actually vote for the most explicit:
 
   _cmd_close($ctx, (grep { defined } ($in, $out)));

Gross. My perl spider-sense tingles at seeing that many optional
punctuation characters, but it should at least be obvious to a casual or
new perl programmer what is going on. I'm fine with it.

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


Re: [PATCH 2/3] Documentation/Makefile: move infodir to be with other '*dir's

2013-02-12 Thread John Keeping
On Tue, Feb 12, 2013 at 01:01:38PM -0800, Jonathan Nieder wrote:
 John Keeping wrote:
 
  Signed-off-by: John Keeping j...@keeping.me.uk
 [...]
  --- a/Documentation/Makefile
  +++ b/Documentation/Makefile
  @@ -81,6 +81,7 @@ DOC_MAN7 = $(patsubst %.txt,%.7,$(MAN7_TXT))
   prefix ?= $(HOME)
   bindir ?= $(prefix)/bin
   htmldir ?= $(prefix)/share/doc/git-doc
  +infodir ?= $(prefix)/share/info
   pdfdir ?= $(prefix)/share/doc/git-doc
   mandir ?= $(prefix)/share/man
   man1dir = $(mandir)/man1
  @@ -98,7 +99,6 @@ RM ?= rm -f
   MAN_REPO = ../../git-manpages
   HTML_REPO = ../../git-htmldocs
   
  -infodir ?= $(prefix)/share/info
   MAKEINFO = makeinfo
 
 Is this another stylefix or is there a functional reason for this
 change?

Another stylefix - this arrangement seems more logical to me and makes
the comment in the next patch simpler.
--
To unsubscribe from this list: send the line 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] completion: support 'git config --local'

2013-02-12 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 I'm not sure I understand the original poster's point about git config
 -l --local. -l does not take a limiter, does it?

git config -l core.\* will just die without limiting the output to
everything under core. hierarchy, so you are right---the combination
does not make any sense.  You have to say

git config -l | grep ^core\\.

or something like that.

Completing git config -l --loTAB still may help, though.


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


Re: [PATCH 1/2] bash completion: add bash.showUntrackedFiles option

2013-02-12 Thread Junio C Hamano
Martin Erik Werner martinerikwer...@gmail.com writes:

 Add a config option 'bash.showUntrackedFiles' which allows enabling
 the prompt showing untracked files on a per-repository basis. This is
 useful for some repositories where the 'git ls-files ...' command may
 take a long time.

 Signed-off-by: Martin Erik Werner martinerikwer...@gmail.com
 ---

OK.

The subject needs to be corrected, though ;-) No need to resend.
I'll do s/completion/prompt/ while applying.

Thanks.

  contrib/completion/git-prompt.sh |   11 ---
  1 file changed, 8 insertions(+), 3 deletions(-)

 diff --git a/contrib/completion/git-prompt.sh 
 b/contrib/completion/git-prompt.sh
 index 9bef053..9b2eec2 100644
 --- a/contrib/completion/git-prompt.sh
 +++ b/contrib/completion/git-prompt.sh
 @@ -43,7 +43,10 @@
  #
  # If you would like to see if there're untracked files, then you can set
  # GIT_PS1_SHOWUNTRACKEDFILES to a nonempty value. If there're untracked
 -# files, then a '%' will be shown next to the branch name.
 +# files, then a '%' will be shown next to the branch name.  You can
 +# configure this per-repository with the bash.showUntrackedFiles
 +# variable, which defaults to true once GIT_PS1_SHOWUNTRACKEDFILES is
 +# enabled.
  #
  # If you would like to see the difference between HEAD and its upstream,
  # set GIT_PS1_SHOWUPSTREAM=auto.  A  indicates you are behind, 
 @@ -332,8 +335,10 @@ __git_ps1 ()
   fi
  
   if [ -n ${GIT_PS1_SHOWUNTRACKEDFILES-} ]; then
 - if [ -n $(git ls-files --others 
 --exclude-standard) ]; then
 - u=%
 + if [ $(git config --bool 
 bash.showUntrackedFiles) != false ]; then
 + if [ -n $(git ls-files --others 
 --exclude-standard) ]; then
 + u=%
 + fi
   fi
   fi
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] Fix installation paths with make install-doc

2013-02-12 Thread Jonathan Nieder
John Keeping wrote:

   Documentation/Makefile: fix inherited {html,info,man}dir

This doesn't seem to have hit the list.

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


Re: [PATCH 2/2] t9903: add test case for bash.showUntrackedFiles

2013-02-12 Thread Junio C Hamano
Martin Erik Werner martinerikwer...@gmail.com writes:

 Add a test case for the bash.showUntrackedFiles config option, which
 checks that the config option can disable the global effect of the
 GIT_PS1_SHOWUNTRACKEDFILES environmant variable.

 Signed-off-by: Martin Erik Werner martinerikwer...@gmail.com
 ---
  t/t9903-bash-prompt.sh |   11 +++
  1 file changed, 11 insertions(+)

 diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
 index f17c1f8..c9417b9 100755
 --- a/t/t9903-bash-prompt.sh
 +++ b/t/t9903-bash-prompt.sh
 @@ -447,6 +447,17 @@ test_expect_success 'prompt - untracked files status 
 indicator - not shown insid
   test_cmp expected $actual
  '
  
 +test_expect_success 'prompt - untracked files status indicator - disabled by 
 config' '
 + printf  (master)  expected 
 + echo untracked  file_untracked 
 + test_config bash.showUntrackedFiles false 
 + (
 + GIT_PS1_SHOWUNTRACKEDFILES=y 
 + __git_ps1  $actual
 + ) 
 + test_cmp expected $actual
 +'

All six combinations need checking:

 * not having the configuration at all and not having the shell
   variable should not show the untracked indicator (already tested).

 * not having the configuration at all and having the shell variable
   should show the untracked indicator (already tested).

 * setting configuration to true without having the shell variable
   should not show the untracked indicator.

 * setting configuration to true and having the shell variable
   should show the unttracked indicator.

 * setting configuration to false and having the shell variable
   should not show the untracked indicator (the above test checks
   this).

 * setting configuration to false without having the shell variable
   should not show the untracked indicator.

to prevent others from breaking the code you wrote for [PATCH 1/2],
so you need three more tests, I guess?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] Fix installation paths with make install-doc

2013-02-12 Thread John Keeping
On Tue, Feb 12, 2013 at 02:25:08PM -0800, Jonathan Nieder wrote:
 John Keeping wrote:
 
Documentation/Makefile: fix inherited {html,info,man}dir
 
 This doesn't seem to have hit the list.

Hmm... it made it to gmane:

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


Re: [PATCH 0/3] Fix installation paths with make install-doc

2013-02-12 Thread Jonathan Nieder
John Keeping wrote:
 On Tue, Feb 12, 2013 at 02:25:08PM -0800, Jonathan Nieder wrote:
 John Keeping wrote:

   Documentation/Makefile: fix inherited {html,info,man}dir

 This doesn't seem to have hit the list.

 Hmm... it made it to gmane:

 http://article.gmane.org/gmane.comp.version-control.git/216188

So it did.  Sorry for the noise --- I'll grab it from there.

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


Re: [PATCH 0/3] Fix installation paths with make install-doc

2013-02-12 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes:

 John Keeping wrote:

   Documentation/Makefile: fix inherited {html,info,man}dir

 This doesn't seem to have hit the list.

More importantly, 

 When using the top-level install-doc target the html, info and man
 target directories are inherited from the top-level Makefile by the
 documentation Makefile as relative paths, which is not expected and
 results in the files being installed in an unexpected location.

I am not sure what problem it is trying to address.  During every
cycle make doc  make install-man install-html is run for all
integration branches and it didn't cause any problems.

A wild guess.  John, are you using config.mak.autogen?

I _think_ exporting mandir/html/infodir from the top-level Makefile
is wrong to begin with.  We should drop the export mandir from
there.

Giving them unusual meaning (e.g. mandir = share/man) is already
bad and that needs to be fixed by limiting this oh, on some
platforms we compile-in GIT_MAN_PATH as a relative path to an
unspecified place insanity only to where -DGIT_MAN_PATH=path is
defined.  The path used there does not help the building and
installation of the documentation at all, so the variable used for
the purpose of giving that path should not be named the same way
as the variable used on Documentation/Makefile to name the real path
in the first place.

Perhaps rename these to runtime_{man,html,info}dir or something and
make sure {man,html,info}dir are defined as the real paths whose
default values begin with $(prefix)?
--
To unsubscribe from this list: send the line 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: [PATCHv4 4/6] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe

2013-02-12 Thread Michal Nazarewicz
On Tue, Feb 12 2013, Junio C Hamano wrote:
 I would actually vote for the most explicit:

   _cmd_close($ctx, (grep { defined } ($in, $out)));

To me that looks weird at best, but I don't have strong opinions on that
matter.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

pgp4dBTjMexcA.pgp
Description: PGP signature


Re: [PATCH v4 11/12] format-patch: update append_signoff prototype

2013-02-12 Thread Brandon Casey
On Tue, Feb 12, 2013 at 11:29 AM, Junio C Hamano gits...@pobox.com wrote:
 Brandon Casey draf...@gmail.com writes:

 diff --git a/builtin/log.c b/builtin/log.c
 index 8f0b2e8..59de484 100644
 --- a/builtin/log.c
 +++ b/builtin/log.c

 @@ -253,9 +255,12 @@ static int detect_any_signoff(char *letter, int size)
   return seen_head  seen_name;
  }

 -static void append_signoff(struct strbuf *sb, const char *signoff)
 +static void append_signoff(struct strbuf *sb, int ignore_footer, unsigned 
 flag)
  {
 + unsigned no_dup_sob = flag  APPEND_SIGNOFF_DEDUP;

 Unused variable at this step?

Yeah, looks like that line can be dropped.

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


Re: [PATCH 0/3] Fix installation paths with make install-doc

2013-02-12 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 I _think_ exporting mandir/html/infodir from the top-level Makefile
 is wrong to begin with.  We should drop the export mandir from
 there.

Ah, it is this thing, isn't it?

commit d8cf908cb6012cd4dc3d1089a849daf646150c2e
Author: Junio C Hamano gits...@pobox.com
Date:   Sat Feb 2 17:58:49 2013 -0800

config.mak.in: remove unused definitions

When 5566771 (autoconf: Use autoconf to write installation
directories to config.mak.autogen, 2006-07-03) introduced support
for autoconf generated config.mak file, it added an export for a
few common makefile variables, in addition to definitions of srcdir
and VPATH.

The export logically does not belong there.  The make variables
like mandir, prefix, etc, should be exported to submakes for people
who use config.mak and people who use config.mak.autogen the same
way; if we want to get these exported, that should be in the main
Makefile.

We do use mandir and htmldir in Documentation/Makefile, so let's
add export for them in the main Makefile instead.

We may eventually want to support VPATH, and srcdir may turn out to
be useful for that purpose, but right now nobody uses it, so it is
useless to define them in this file.

Signed-off-by: Junio C Hamano gits...@pobox.com

config.mak.in shouldn't have exported mandir in the first place, and
the commit made it worse by moving that broken export to the main
Makefile, and also added an export to htmldir as well, which was
totally wrong.

Let me revert that bit first.

I still think making mandir to have the real path in both the
top-level Makefile and Documentation/Makefile and renaming the
variable that is used to form the -DGIT_MAN_PATH=path to
optionally compile in a path relative to an unspecified location
that is discovered at runtime to something else is the sane thing to
do, but that is a separate issue, I 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: [PATCH 0/3] Fix installation paths with make install-doc

2013-02-12 Thread John Keeping
On Tue, Feb 12, 2013 at 02:45:34PM -0800, Junio C Hamano wrote:
 Jonathan Nieder jrnie...@gmail.com writes:
 
  John Keeping wrote:
 
Documentation/Makefile: fix inherited {html,info,man}dir
 
  This doesn't seem to have hit the list.
 
 More importantly, 
 
  When using the top-level install-doc target the html, info and man
  target directories are inherited from the top-level Makefile by the
  documentation Makefile as relative paths, which is not expected and
  results in the files being installed in an unexpected location.
 
 I am not sure what problem it is trying to address.  During every
 cycle make doc  make install-man install-html is run for all
 integration branches and it didn't cause any problems.
 
 A wild guess.  John, are you using config.mak.autogen?

Close - plain config.mak.

I set $prefix there and ran make install-doc.  That installed the man
pages in Documentation/share/man/ in my Git source directory.

 I _think_ exporting mandir/html/infodir from the top-level Makefile
 is wrong to begin with.  We should drop the export mandir from
 there.
 
 Giving them unusual meaning (e.g. mandir = share/man) is already
 bad and that needs to be fixed by limiting this oh, on some
 platforms we compile-in GIT_MAN_PATH as a relative path to an
 unspecified place insanity only to where -DGIT_MAN_PATH=path is
 defined.  The path used there does not help the building and
 installation of the documentation at all, so the variable used for
 the purpose of giving that path should not be named the same way
 as the variable used on Documentation/Makefile to name the real path
 in the first place.
 
 Perhaps rename these to runtime_{man,html,info}dir or something and
 make sure {man,html,info}dir are defined as the real paths whose
 default values begin with $(prefix)?

Would it be sensible to define the values for these variables (with
absolute paths) in a separate top-level file like config.mak.uname
(defaults.mak maybe?) and include that in both Documentation/Makefile
and Makefile, then calculate the relative path from $(prefix) to
$({man,html,info}dir) for the compiled-in values.


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


Re: [PATCH 0/3] Fix installation paths with make install-doc

2013-02-12 Thread John Keeping
On Tue, Feb 12, 2013 at 02:57:25PM -0800, Junio C Hamano wrote:
 Junio C Hamano gits...@pobox.com writes:
 
  I _think_ exporting mandir/html/infodir from the top-level Makefile
  is wrong to begin with.  We should drop the export mandir from
  there.
 
 Ah, it is this thing, isn't it?
 
 commit d8cf908cb6012cd4dc3d1089a849daf646150c2e
 Author: Junio C Hamano gits...@pobox.com
 Date:   Sat Feb 2 17:58:49 2013 -0800
 
 config.mak.in: remove unused definitions
 
 When 5566771 (autoconf: Use autoconf to write installation
 directories to config.mak.autogen, 2006-07-03) introduced support
 for autoconf generated config.mak file, it added an export for a
 few common makefile variables, in addition to definitions of srcdir
 and VPATH.
 
 The export logically does not belong there.  The make variables
 like mandir, prefix, etc, should be exported to submakes for people
 who use config.mak and people who use config.mak.autogen the same
 way; if we want to get these exported, that should be in the main
 Makefile.
 
 We do use mandir and htmldir in Documentation/Makefile, so let's
 add export for them in the main Makefile instead.
 
 We may eventually want to support VPATH, and srcdir may turn out to
 be useful for that purpose, but right now nobody uses it, so it is
 useless to define them in this file.
 
 Signed-off-by: Junio C Hamano gits...@pobox.com

Looks like it - I tried this for the first time today (with pu) so I
didn't realise it was a recent change, and I didn't think to blame the
export line.


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


[PATCH] Makefile: do not export mandir/htmldir/infodir

2013-02-12 Thread Junio C Hamano
These are defined in the main Makefile to be funny values that are
optionally relative to an unspecified location that is determined at
runtime.  They are only suitable for hardcoding in the binary via
the -DGIT_{MAN,HTML,INFO}_PATH=value C preprocessor options, and
are not real paths, contrary to what any sane person, and more
importantly, the Makefile in the documentation directory, would
expect.

A longer term fix is to introduce runtime_{man,html,info}dir variables
to hold these funny values, and make {man,html,info}dir variables
to have real paths whose default values begin with $(prefix), but
as a first step, stop exporting them from the top-level Makefile.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 1dae2c5..26b697d 100644
--- a/Makefile
+++ b/Makefile
@@ -359,7 +359,7 @@ lib = lib
 # DESTDIR=
 pathsep = :
 
-export prefix bindir sharedir mandir htmldir sysconfdir gitwebdir localedir
+export prefix bindir sharedir sysconfdir gitwebdir localedir
 
 CC = cc
 AR = ar
-- 
1.8.1.3.675.g524d3b9
--
To unsubscribe from this list: send the line 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: A good Git technique for referring back to original files

2013-02-12 Thread Paul Campbell
Hi Mike,

I think git-cvsimport and git-subtree could help you here.

Roughly:

# Create a git version of each SDK_subproj
git cvsimport -r upstream -d $CVSREPO1 $CVSMODULE1 -C SDK_subproj1
git cvsimport -r upstream -d $CVSREPO2 $CVSMODULE2 -C SDK_subproj2

# Create your Working_SDK
git init Working_SDK
cd Working_SDK

# Import the SDK_subprojN repos
git subtree add --prefix=subproj1 ../SDK_subproj1 upstream/master
git subtree add --prefix=subproj2 ../SDK_subproj2 upstream/master

# Edit and commit your files
# N.B. when committing don't commit to more than one subproj in a single commit

# Update from the upstream CVS as needed
git cvsimport -r upstream -d $CVSREPO1 $CVSMODULE1 -C ../SDK_subproj1
git subtree pull --prefix=subproj1 ../SDK_subproj1 upstream/master
git cvsimport -r upstream -d $CVSREPO2 $CVSMODULE2 -C ../SDK_subproj2
git subtree pull --prefix=subproj2 ../SDK_subproj2 upstream/master

# Push your changes back to SDK_subproj repos into a branch other than master
git subtree push --prefix=subproj1 ../SDK_subproj1 new-branch
git subtree push --prefix=subproj1 ../SDK_subproj2 new-branch

# Prepare patches to apply to a real CVS copy or submit upstream
(cd ../SDK_subproj1  git format-patch upstream/master..new-branch)
(cd ../SDK_subproj2  git format-patch upstream/master..new-branch)

Hope that helps.

--
Paul

On Tue, Feb 12, 2013 at 10:19 AM, MikeW mw_p...@yahoo.co.uk wrote:
 Matthieu Moy Matthieu.Moy at grenoble-inp.fr writes:


 MikeW mw_phil at yahoo.co.uk writes:

  Since git is so good at tracking file content, I wondered whether
 there was any
  technique using git that would simplify the back-referencing task.

 I'm not sure I understand the question, but if you want to add meta-data
 to Git commits (e.g. this Git commit is revision 42 in CVS repository
 foo), then have a look at git-notes. It won't give you directly
 reference to other VCS, but at least can be used as a storage
 mechanism to store these references.

 Thanks for the reply.

 In my work environment both the SDK and the original files are available
 (in an enclosing directory).

 --SDK_content
   |
   SDK_subproj1-- ...
   ||
   |content
   |
   SDK_subproj2- ...
   ||
   |content
   |
   SDK_subprojN- ...
   ||
   |content
   |
   Working_SDK ... (under git, baseline generated from subproj1..N)
|
content derived from subproj1..N


 What I had in mind was something I could run over, say, SDK_content
 (alternatively, from within Working_SDK, referring back to SDK_content)
 which would note the changed files in Working_SDK and locate the
 original files in SDK_subproj1..N letting me merge the changes back.


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



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


Re: [PATCH 0/3] Fix installation paths with make install-doc

2013-02-12 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes:

 Looks like it - I tried this for the first time today (with pu) so I
 didn't realise it was a recent change, and I didn't think to blame the
 export line.

Unfortunately that bogus change is already in 'next', but luckily we
caught it before it graduated to 'master' ;-)

I'll queue the regression fix patch (already sent separately); the
patch should bring everything in Makefile back to the same state as
in 'master', so in that sense it fixes the regression, but it does
not address the real cause of the confusion, solution to which is
hinted in the log message of the patch.

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


  1   2   >