[RFC] i18n.pathencoding

2012-09-01 Thread Torsten Bögershausen
Allow path names to be encoded in UTF-8 in the repository
and checkout out as e.g. ISO-8859-1 in the working tree.

Introduce a config variable i18n.pathEncoding.
If empty, no re-encoding of path names is done.

Add t3911 to test encoding back and forth

The re-encoding is done in compat/reencode_pathname.c,
where all file system functions like open(), stat(),
readdir() are re-defined.

reencode_pathname.c includes all functionality from
precompose_utf8.c, which should be removed

Signed-off-by: Torsten Bögershausen tbo...@web.de
---
Please read this as an RFC, so there several limitations:

 compat/reencode_pathname.h defines struct dirent_psx with d_name[2].
   This is done to test renc_pn_readdir() in compat/reencode_pathname.c

 test case t1450 failes even on one of my linux machines. At first glance
  it looks as the same failure which has been sometimes observed on Mac OS X.

 compat/precompose_utf8.[ch] had been integrated into reencode_pathname.[ch],
  and should be removed.

 The patch should work on v7.1.12, it's not tested against latest master 

 Comments are welcome.


 Documentation/config.txt  |  10 +
 Makefile  |  11 +-
 builtin/init-db.c |   3 +
 cache.h   |   1 +
 compat/reencode_pathname.c| 441 ++
 compat/reencode_pathname.h|  72 +++
 config.c  |   3 +
 environment.c |   1 +
 git-compat-util.h |  20 +-
 parse-options.c   |   2 +-
 t/t3911-i18n-filename-8859.sh | 251 
 wt-status.c   |  21 +-
 12 files changed, 827 insertions(+), 9 deletions(-)
 create mode 100644 compat/reencode_pathname.c
 create mode 100644 compat/reencode_pathname.h
 create mode 100755 t/t3911-i18n-filename-8859.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a95e5a4..d633d54 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1442,6 +1442,16 @@ i18n.logOutputEncoding::
Character encoding the commit messages are converted to when
running 'git log' and friends.
 
+i18n.pathEncoding::
+   This option is only used by some implementations of git.
+   When git init sets core.supportspathencoding to true,
+   i18n.pathEncoding can be set to re-encode path names when
+   a working tree is checked out.
+   Path names may be e.g. encoded in ISO-8859-1 and are stored as
+   UTF-8 encoded in the repository.
+   When not set, the encoding of path names is the same in working tree
+   and the repository.
+
 imap::
The configuration variables in the 'imap' section are described
in linkgit:git-imap-send[1].
diff --git a/Makefile b/Makefile
index 6b0c961..141562e 100644
--- a/Makefile
+++ b/Makefile
@@ -143,6 +143,9 @@ all::
 #
 # Define NEEDS_LIBICONV if linking with libc is not enough (Darwin).
 #
+# Define PATH_ENCODING if the encoding of file names
+# differs from the encoding in the git repo
+#
 # Define NEEDS_SOCKET if linking with libc is not enough (SunOS,
 # Patrick Mauritz).
 #
@@ -595,6 +598,7 @@ LIB_H += compat/bswap.h
 LIB_H += compat/cygwin.h
 LIB_H += compat/mingw.h
 LIB_H += compat/obstack.h
+LIB_H += compat/reencode_pathname.h
 LIB_H += compat/precompose_utf8.h
 LIB_H += compat/terminal.h
 LIB_H += compat/win32/dirent.h
@@ -932,6 +936,7 @@ ifeq ($(uname_S),OSF1)
NO_NSEC = YesPlease
 endif
 ifeq ($(uname_S),Linux)
+   PATH_ENCODING = YesPlease
NO_STRLCPY = YesPlease
NO_MKSTEMPS = YesPlease
HAVE_PATHS_H = YesPlease
@@ -999,7 +1004,7 @@ ifeq ($(uname_S),Darwin)
NO_MEMMEM = YesPlease
USE_ST_TIMESPEC = YesPlease
HAVE_DEV_TTY = YesPlease
-   COMPAT_OBJS += compat/precompose_utf8.o
+   COMPAT_OBJS += compat/reencode_pathname.o
BASIC_CFLAGS += -DPRECOMPOSE_UNICODE
 endif
 ifeq ($(uname_S),SunOS)
@@ -1591,6 +1596,10 @@ ifdef FREAD_READS_DIRECTORIES
COMPAT_CFLAGS += -DFREAD_READS_DIRECTORIES
COMPAT_OBJS += compat/fopen.o
 endif
+ifdef PATH_ENCODING
+   COMPAT_CFLAGS += -DPATH_ENCODING
+   COMPAT_OBJS += compat/reencode_pathname.o
+endif
 ifdef NO_SYMLINK_HEAD
BASIC_CFLAGS += -DNO_SYMLINK_HEAD
 endif
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 244fb7f..f159d43 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -291,6 +291,9 @@ static int create_default_files(const char *template_path)
if (!access(path, F_OK))
git_config_set(core.ignorecase, true);
probe_utf8_pathname_composition(path, len);
+#ifdef PATH_ENCODING
+   git_config_set(core.supportspathencoding, true);
+#endif
}
 
return reinit;
diff --git a/cache.h b/cache.h
index 67f28b4..8023767 100644
--- a/cache.h
+++ b/cache.h
@@ -1160,6 +1160,7 @@ extern int user_ident_sufficiently_given(void);
 extern const char *git_commit_encoding;
 extern const char 

Re: Grafting Alternate History

2012-09-01 Thread Andreas Schwab
d...@cray.com writes:

 For example, let's say we take a local copy of the private repository.
 I would like to somehow create an empty branch in that local copy and
 then populate it with the history of the public repository.  I would
 like to be able to specify a git diff-like command on the public graft
 branch to get some changes over a range of commits and apply those
 changes to one of the private branches (say master).

You can set up (the git mirror of) the public repository as a remote in
the private repository and git cherry-pick the commits you want to copy
over.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Aug 2012, #10; Fri, 31)

2012-09-01 Thread Michael Haggerty
On 08/31/2012 11:32 PM, Junio C Hamano wrote:
 [...]
 * mh/fetch-filter-refs (2012-08-26) 17 commits
  - filter_refs(): simplify logic
  - fetch_pack(): free matching heads
  - cmd_fetch_pack(): simplify computation of return value
  - fetch-pack: report missing refs even if no existing refs were received
  - cmd_fetch_pack: return early if finish_connect() returns an error
  - filter_refs(): compress unmatched refs in heads array
  - filter_refs(): do not leave gaps in return_refs
  - filter_refs(): simplify by removing optimization
  - Pass nr_heads to filter_refs() by reference
  - Pass nr_heads to everything_local() by reference
  - Pass nr_heads to do_pack_ref() by reference
  - Let fetch_pack() inform caller about number of unique heads
  - filter_refs(): do not check the same head_pos twice
  - fetch-pack.c: name local variables more consistently
  - fetch_pack(): reindent function decl and defn
  - Rename static function fetch_pack() to http_fetch_pack()
  - t5500: add tests of error output for missing refs
 
 Code simplification and clarification.
 Waiting for a follow-up patch based on Peff's idea.

Please hold off on merging this anywhere for now.  I realized that
nr_heads,heads is basically a string_list, and I'm going to try
re-implementing on that basis.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4] Thunderbird: fix appp.sh format problems

2012-09-01 Thread Marco Stornelli

Il 31/08/2012 23:35, Johannes Sixt ha scritto:

Am 31.08.2012 16:09, schrieb Marco Stornelli:

+CCS=`perl -e 'local $/=undef; open FILE, $ENV{'PATCHTMP'}; $text=FILE;
+close FILE; $addr = $1 if $text =~ /Cc: (.*?(,\n .*?)*)\n/s; $addr =~ s/\n//g;
+print $addr;'`


The quoting is broken in this line (sq within sq does not work).


I don't understand what you mean, I'm using this script and it works 
perfectly.




Am I correct that you intend to treat continuation lines with this
non-trivial perl script? All this processing gets tedious and
unreadable. Let me suggest this pattern instead:


The perl script is used to have To: and Cc: email addresses in single 
line, without using \n. I tried with sed, but it's a little bit 
difficult (at least for me) because sed usually works on a single line, 
it's tricky with multilines.




# translate to Thunderbird language
LANG_TO=To:
LANG_SUBJ=Subject:
LANG_CC=Cc:

LF= # terminates the _previous_ line
while read -r line
do
case $line in
'To: '*)
printf ${LF}%s $LANG_TO ${line#To: }
;;
'Cc: '*) ...similar...
'Subject: '*) ...similar...
' '*)   # continuation line
printf %s $line
;;
'')
print ${LF}\n
cat
;;
esac
LF='\n'
done $PATCH $1

Instead of printing right away, you can also accumulate the data in
variables and print them right before the 'cat'. I would do that only if
a particular order of To:, Cc:, and Subject: is required in the output.

(I don't know how the do not delete this line line fits in the
picture, but I'm sure you can figure it out.)

-- Hannes



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


Re: [PATCH v4] Thunderbird: fix appp.sh format problems

2012-09-01 Thread Marco Stornelli

Il 31/08/2012 19:08, Junio C Hamano ha scritto:

Marco Stornelli marco.storne...@gmail.com writes:


The current script has got the following problems:

1) It doesn't work if the language used by Thunderbird is not english;
2) The field To: filled by format-patch is not evaluated;
3) The field Cc: is loaded from Cc used in the commit message
instead of using the Cc field filled by format-patch in the email
header.

Added comments for point 1), added parsing of To: for point 2) and
added parsing of Cc: in the email header for point 3), removing the
Cc: parsing from commit message.

Signed-off-by: Marco Stornelli marco.storne...@gmail.com
---


Thanks. [+cc Lukas].

A few new issues your patch introduced:

  - MAILHEADER is only set once to read from sed, and then used once
to be echoed to another file.  Just send sed output to the file.

  - The s/Subject/Oggetto/g bit in my previous review.

(find the fix-up at the end).



v4: create a tmp file to allow correct perl parsing
v3: parse only To: and Cc: in the email header, fix some comments
v2: changed the commit message to reflect better the script implementation
v1: first draft

  contrib/thunderbird-patch-inline/appp.sh |   26 ++
  1 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/contrib/thunderbird-patch-inline/appp.sh 
b/contrib/thunderbird-patch-inline/appp.sh
index 5eb4a51..0daeb29 100755
--- a/contrib/thunderbird-patch-inline/appp.sh
+++ b/contrib/thunderbird-patch-inline/appp.sh
@@ -6,6 +6,9 @@

  # ExternalEditor can be downloaded at 
http://globs.org/articles.php?lng=enpg=2

+# NOTE: You must change some words in this script according to the language
+# used by Mozilla Thunderbird, as Subject, To, Don't remove this line.
+
  CONFFILE=~/.appprc

  SEP=-=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-
@@ -26,17 +29,32 @@ fi
  cd -  /dev/null

  SUBJECT=`sed -n -e '/^Subject: /p' ${PATCH}`
-HEADERS=`sed -e '/^'${SEP}'$/,$d' $1`
  BODY=`sed -e 1,/${SEP}/d $1`
  CMT_MSG=`sed -e '1,/^$/d' -e '/^---$/,$d' ${PATCH}`
  DIFF=`sed -e '1,/^---$/d' ${PATCH}`
+MAILHEADER=`sed '/^$/q' ${PATCH}`
+PATCHTMP=${PATCH}.tmp
+
+echo $MAILHEADER  $PATCHTMP
+
+export PATCHTMP
+CCS=`perl -e 'local $/=undef; open FILE, $ENV{'PATCHTMP'}; $text=FILE;
+close FILE; $addr = $1 if $text =~ /Cc: (.*?(,\n .*?)*)\n/s; $addr =~ s/\n//g;
+print $addr;'`
+
+TO=`perl -e 'local $/=undef; open FILE, $ENV{'PATCHTMP'}; $text=FILE;
+close FILE; $addr = $1 if $text =~ /To: (.*?(,\n .*?)*)\n/s; $addr =~ s/\n//g;
+print $addr;'`

-CCS=`echo -e $CMT_MSG\n$HEADERS | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
-   -e 's/^Signed-off-by: \(.*\)/\1,/gp'`
+rm -rf $PATCHTMP

+# Change Subject: before next line according to Thunderbird language
+# for example:
+# SUBJECT=`echo $SUBJECT | sed -e 's/Subject/Oggetto/g'`
  echo $SUBJECT  $1
+# Change To: according to Thunderbird language
+echo To: $TO  $1
  echo Cc: $CCS  $1
-echo $HEADERS | sed -e '/^Subject: /d' -e '/^Cc: /d'  $1
  echo $SEP  $1

  echo $CMT_MSG  $1



I also wonder what would happen if To: and Cc: in the input were
split into continuation lines, but that was already present in the


Do you mean To: mail1,.mailN\nCc: mail1,.mailN?


version before your patch, so the attached fix-up won't touch that
part, but you may want to think about it.


Actually I'm trying the script in two ways: with --to and --cc of git 
format patch = multilines, and with a little script (see below) to have 
automatically Cc: from other script, in this case get_maintainer.pl from 
Linux kernel source tree, and it works perfectly. In the last case Cc: 
mail addresses are on the same line.

Maybe we can add even this script, but maybe it's too kernel-specific.


#!/bin/bash

if [[ ! $# -eq 1 ]]
then
echo Usage: command PATH, where PATH contains kernel and 
patches

exit
fi

PATCHPATH=${1%/}
for i in `ls $PATCHPATH/*.patch`
do

CCN=`grep Cc: $i | wc -l`
if [ $CCN -ge 1 ]
then
echo Cc: list already present, skip...
else

CCLIST=`$PATCHPATH/scripts/get_maintainer.pl 
--norolestats --no-git --separator , $i`


sed -n -e /^$/,999 ! p $i  $i.new
echo Cc: $CCLIST  $i.new
sed -n -e /^$/,999 p $i  $i.new
mv $i.new $i
fi
done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5] Thunderbird: fix appp.sh format problems

2012-09-01 Thread Marco Stornelli
The current script has got the following problems:

1) It doesn't work if the language used by Thunderbird is not English;
2) The field To: filled by format-patch is not evaluated;
3) The field Cc: is loaded from Cc used in the commit message
instead of using the Cc field filled by format-patch in the email
header.

Added comments for point 1), added parsing of To: for point 2) and
added parsing of Cc: in the email header for point 3), removing the
Cc: parsing from commit message.

Signed-off-by: Marco Stornelli marco.storne...@gmail.com
---

v5: fixed comments by Junio C Hamano
v4: create a tmp file to allow correct perl parsing
v3: parse only To: and Cc: in the email header, fix some comments
v2: changed the commit message to reflect better the script implementation
v1: first draft

 contrib/thunderbird-patch-inline/appp.sh |   25 +
 1 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/contrib/thunderbird-patch-inline/appp.sh 
b/contrib/thunderbird-patch-inline/appp.sh
index 5eb4a51..5e4e892 100755
--- a/contrib/thunderbird-patch-inline/appp.sh
+++ b/contrib/thunderbird-patch-inline/appp.sh
@@ -6,6 +6,9 @@
 
 # ExternalEditor can be downloaded at http://globs.org/articles.php?lng=enpg=2
 
+# NOTE: You must change some words in this script according to the language
+# used by Mozilla Thunderbird, as Subject, To, Don't remove this line.
+
 CONFFILE=~/.appprc
 
 SEP=-=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-
@@ -26,17 +29,31 @@ fi
 cd -  /dev/null
 
 SUBJECT=`sed -n -e '/^Subject: /p' ${PATCH}`
-HEADERS=`sed -e '/^'${SEP}'$/,$d' $1`
 BODY=`sed -e 1,/${SEP}/d $1`
 CMT_MSG=`sed -e '1,/^$/d' -e '/^---$/,$d' ${PATCH}`
 DIFF=`sed -e '1,/^---$/d' ${PATCH}`
+PATCHTMP=${PATCH}.tmp
+
+sed '/^$/q' ${PATCH}  ${PATCHTMP}
+
+export PATCHTMP
+CCS=`perl -e 'local $/=undef; open FILE, $ENV{'PATCHTMP'}; $text=FILE;
+close FILE; $addr = $1 if $text =~ /Cc: (.*?(,\n .*?)*)\n/s; $addr =~ s/\n//g;
+print $addr;'`
+
+TO=`perl -e 'local $/=undef; open FILE, $ENV{'PATCHTMP'}; $text=FILE;
+close FILE; $addr = $1 if $text =~ /To: (.*?(,\n .*?)*)\n/s; $addr =~ s/\n//g;
+print $addr;'`
 
-CCS=`echo -e $CMT_MSG\n$HEADERS | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
-   -e 's/^Signed-off-by: \(.*\)/\1,/gp'`
+rm -f ${PATCHTMP}
 
+# Change Subject: before next line according to Thunderbird language
+# for example to translate in Italian:
+# SUBJECT=`echo $SUBJECT | sed -e 's/^Subject:/Oggetto:/g'`
 echo $SUBJECT  $1
+# Change To: according to Thunderbird language
+echo To: $TO  $1
 echo Cc: $CCS  $1
-echo $HEADERS | sed -e '/^Subject: /d' -e '/^Cc: /d'  $1
 echo $SEP  $1
 
 echo $CMT_MSG  $1
-- 
1.7.3.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 1/2] Support for setitimer() on platforms lacking it

2012-09-01 Thread Joachim Schmitz
 From: Joachim Schmitz [mailto:j...@schmitz-digital.de]
 Sent: Thursday, August 30, 2012 7:23 PM
 To: 'Junio C Hamano'
 Cc: 'git@vger.kernel.org'
 Subject: RE: [PATCH 1/2] Support for setitimer() on platforms lacking it
 
  From: Junio C Hamano [mailto:gits...@pobox.com]
  Sent: Thursday, August 30, 2012 7:14 PM
  To: Joachim Schmitz
  Cc: git@vger.kernel.org
  Subject: Re: [PATCH 1/2] Support for setitimer() on platforms lacking it
 
  Joachim Schmitz j...@schmitz-digital.de writes:
 
   I see no existing code calls setitimer() with non-NULL ovalue, and I
   do not think we would add a new caller that would do so in any time
   soon, so it may not be a bad idea to drop support of returning the
   remaining timer altogether from this emulation layer (just like
   giving anything other than ITIMER_REAL gives us ENOTSUP).  That
   would sidestep the whole we cannot answer how many milliseconds are
   still remaining on the timer when using emulation based on alarm().
  
   Should we leave tv_usec untouched then? That was we round up on
   the next (and subsequent?) round(s). Or just set to ENOTSUP in
   setitimer if ovalue is !NULL?
 
  I was alluding to the latter.
 
 OK, will do that then.
 
+  switch (which) {
+  case ITIMER_REAL:
+  alarm(value-it_value.tv_sec +
+  (value-it_value.tv_usec  0) ? 1 : 0);
  
   Why is this capped to 1 second?  Is this because no existing code
   uses the timer for anything other than 1 second or shorter?  If that
   is the case, that needs at least some documenting (or a possibly
   support for longer expiration, if it is not too cumbersome to add).
  
   As you mention alarm() has only seconds resolution. It is tv_sec
   plus 1 if there are tv_usecs  0, it is rounding up, so we don't
   cancel the alarm() if tv_sec is 0 but tv_usec is not. Looks OK to
   me?
 
  Can a caller use setitimer to be notified in 5 seconds?
 
 Yes, by setting tv_sec to 5 and tv_usec to 0, or be setting tv_sec to 4 and 
 tv_usec to something  0.
 
 Unless I screwed up the operator precedence?
 To make it clearer (any possibly correct?):
 
   switch (which) {
   case ITIMER_REAL:
   alarm(value-it_value.tv_sec +
   ((value-it_value.tv_usec  0) ? 1 : 0));
 
 Or even just
   switch (which) {
   case ITIMER_REAL:
   alarm(value-it_value.tv_sec + (value-it_value.tv_usec 
  0));

OK, here it goes again, not yet as a patch, just plain code for comment:

$ cat itimer.c
/* 
 * Rely on system headers (sys/time.h) to contain struct itimerval
 * and git-compat-util.h to have the prototype for git_getitimer().
 * As soon as there's a platform where that is not the case, we'd need
 * an itimer .h.
 */
#include ../git-compat-util.h

#ifndef NO_GETITIMER /* not yet needed anywhere else in git */
static
#endif
int git_getitimer(int which, struct itimerval *value)
{
int ret = 0;

if (!value) {
errno = EFAULT;
return -1;
}

switch (which) {
case ITIMER_REAL:
#if 0
value-it_value.tv_usec = 0;
value-it_value.tv_sec = alarm(0);
ret = 0; /* if alarm() fails, we get a SIGLIMIT */
break;
#else
/*
 * As an emulation via alarm(0) won't tell us how many
 * usecs are left, we don't support it altogether.
 */
#endif
case ITIMER_VIRTUAL:
case ITIMER_PROF:
errno = ENOTSUP;
ret = -1;
break;
default:
errno = EINVAL;
ret = -1;
break;
}
return ret;
}

int git_setitimer(int which, const struct itimerval *value,
struct itimerval *ovalue)
{
int ret = 0;

if (!value ) {
errno = EFAULT;
return -1;
}

if ( value-it_value.tv_sec  0
|| value-it_value.tv_usec  100
|| value-it_value.tv_usec  0) {
errno = EINVAL;
return -1;
}

if ((ovalue)  (git_getitimer(which, ovalue) == -1))
return -1; /* errno set in git_getitimer() */

switch (which) {
case ITIMER_REAL:
 /* If tv_usec is  0, round up to next full sec */
alarm(value-it_value.tv_sec + (value-it_value.tv_usec  0));
ret = 0; /* if alarm() fails, we get a SIGLIMIT */
break;
case ITIMER_VIRTUAL:
case ITIMER_PROF:
errno = ENOTSUP;
ret = -1;
break;
default:
errno = EINVAL;
ret = -1;
break;
}

return ret;
}

Would this pass muster? The previous version had a bug too, of ovalue was !NULL 

Re: [PATCH] fetch --all: pass --tags/--no-tags through to each remote

2012-09-01 Thread Jeff King
On Sat, Sep 01, 2012 at 12:25:33AM -0400, Dan Johnson wrote:

 diff --git a/builtin/fetch.c b/builtin/fetch.c
 index bb9a074..c6bcbdc 100644
 --- a/builtin/fetch.c
 +++ b/builtin/fetch.c
 @@ -857,6 +857,10 @@ static void add_options_to_argv(int *argc, const char 
 **argv)
   argv[(*argc)++] = --recurse-submodules;
   else if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)
   argv[(*argc)++] = --recurse-submodules=on-demand;
 + if (tags == TAGS_SET)
 + argv[(*argc)++] = --tags;
 + else if (tags == TAGS_UNSET)
 + argv[(*argc)++] = --no-tags;
   if (verbosity = 2)
   argv[(*argc)++] = -v;
   if (verbosity = 1)

Hmm. We allocate argv in fetch_multiple like this:

  const char *argv[12] = { fetch, --append };

and then add a bunch of options to it, along with the name of the
remote. By my count, the current code can hit exactly 12 (including the
terminating NULL) if all options are set. Your patch would make it
possible to overflow. Of course, I may be miscounting since it is
extremely error-prone to figure out the right number by tracing each
possible conditional.

Maybe we should switch it to a dynamic argv_array? Like this:

  [1/2]: argv-array: add pop function
  [2/2]: fetch: use argv_array instead of hand-building arrays

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


[PATCH 1/2] argv-array: add pop function

2012-09-01 Thread Jeff King
Sometimes we build a set of similar command lines, differing
only in the final arguments (e.g., fetch --multiple). To
use argv_array for this, you have to either push the same
set of elements repeatedly, or break the abstraction by
manually manipulating the array's internal members.

Instead, let's provide a sanctioned pop function to remove
elements from the end.

Signed-off-by: Jeff King p...@peff.net
---
 Documentation/technical/api-argv-array.txt | 4 
 argv-array.c   | 9 +
 argv-array.h   | 1 +
 3 files changed, 14 insertions(+)

diff --git a/Documentation/technical/api-argv-array.txt 
b/Documentation/technical/api-argv-array.txt
index 1b7d8f1..1a79781 100644
--- a/Documentation/technical/api-argv-array.txt
+++ b/Documentation/technical/api-argv-array.txt
@@ -46,6 +46,10 @@ Functions
Format a string and push it onto the end of the array. This is a
convenience wrapper combining `strbuf_addf` and `argv_array_push`.
 
+`argv_array_pop`::
+   Remove the final element from the array. If there are no
+   elements in the array, do nothing.
+
 `argv_array_clear`::
Free all memory associated with the array and return it to the
initial, empty state.
diff --git a/argv-array.c b/argv-array.c
index 0b5f889..55e8443 100644
--- a/argv-array.c
+++ b/argv-array.c
@@ -49,6 +49,15 @@ void argv_array_pushl(struct argv_array *array, ...)
va_end(ap);
 }
 
+void argv_array_pop(struct argv_array *array)
+{
+   if (!array-argc)
+   return;
+   free((char *)array-argv[array-argc - 1]);
+   array-argv[array-argc - 1] = NULL;
+   array-argc--;
+}
+
 void argv_array_clear(struct argv_array *array)
 {
if (array-argv != empty_argv) {
diff --git a/argv-array.h b/argv-array.h
index b93a69c..f4b9866 100644
--- a/argv-array.h
+++ b/argv-array.h
@@ -16,6 +16,7 @@ void argv_array_pushl(struct argv_array *, ...);
 __attribute__((format (printf,2,3)))
 void argv_array_pushf(struct argv_array *, const char *fmt, ...);
 void argv_array_pushl(struct argv_array *, ...);
+void argv_array_pop(struct argv_array *);
 void argv_array_clear(struct argv_array *);
 
 #endif /* ARGV_ARRAY_H */
-- 
1.7.12.rc3.8.g89db099

--
To unsubscribe from this list: send the line 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] fetch: use argv_array instead of hand-building arrays

2012-09-01 Thread Jeff King
Fetch invokes itself recursively when recursing into
submodules or handling fetch --multiple. In both cases, it
builds the child's command line by pushing options onto a
statically-sized array. In both cases, the array is
currently just big enough to handle the largest possible
case. However, this technique is brittle and error-prone, so
let's replace it with a dynamic argv_array.

Signed-off-by: Jeff King p...@peff.net
---
Not very well tested by me, but hopefully it is simple enough that I
managed not to screw it up.

It may be that fetch_populated_submodules would also benefit from
conversion (here I just pass in the argc and argv separately), but I
didn't look.

 builtin/fetch.c | 47 +--
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index bb9a074..b6a8be0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -14,6 +14,7 @@
 #include transport.h
 #include submodule.h
 #include connected.h
+#include argv-array.h
 
 static const char * const builtin_fetch_usage[] = {
git fetch [options] [repository [refspec...]],
@@ -841,38 +842,35 @@ static int fetch_multiple(struct string_list *list)
return 1;
 }
 
-static void add_options_to_argv(int *argc, const char **argv)
+static void add_options_to_argv(struct argv_array *argv)
 {
if (dry_run)
-   argv[(*argc)++] = --dry-run;
+   argv_array_push(argv, --dry-run);
if (prune)
-   argv[(*argc)++] = --prune;
+   argv_array_push(argv, --prune);
if (update_head_ok)
-   argv[(*argc)++] = --update-head-ok;
+   argv_array_push(argv, --update-head-ok);
if (force)
-   argv[(*argc)++] = --force;
+   argv_array_push(argv, --force);
if (keep)
-   argv[(*argc)++] = --keep;
+   argv_array_push(argv, --keep);
if (recurse_submodules == RECURSE_SUBMODULES_ON)
-   argv[(*argc)++] = --recurse-submodules;
+   argv_array_push(argv, --recurse-submodules);
else if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)
-   argv[(*argc)++] = --recurse-submodules=on-demand;
+   argv_array_push(argv, --recurse-submodules=on-demand);
if (verbosity = 2)
-   argv[(*argc)++] = -v;
+   argv_array_push(argv, -v);
if (verbosity = 1)
-   argv[(*argc)++] = -v;
+   argv_array_push(argv, -v);
else if (verbosity  0)
-   argv[(*argc)++] = -q;
+   argv_array_push(argv, -q);
 
 }
 
 static int fetch_multiple(struct string_list *list)
 {
int i, result = 0;
-   const char *argv[12] = { fetch, --append };
-   int argc = 2;
-
-   add_options_to_argv(argc, argv);
+   struct argv_array argv = ARGV_ARRAY_INIT;
 
if (!append  !dry_run) {
int errcode = truncate_fetch_head();
@@ -880,18 +878,22 @@ static int fetch_multiple(struct string_list *list)
return errcode;
}
 
+   argv_array_pushl(argv, fetch, --append, NULL);
+   add_options_to_argv(argv);
+
for (i = 0; i  list-nr; i++) {
const char *name = list-items[i].string;
-   argv[argc] = name;
-   argv[argc + 1] = NULL;
+   argv_array_push(argv, name);
if (verbosity = 0)
printf(_(Fetching %s\n), name);
-   if (run_command_v_opt(argv, RUN_GIT_CMD)) {
+   if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
error(_(Could not fetch %s), name);
result = 1;
}
+   argv_array_pop(argv);
}
 
+   argv_array_clear(argv);
return result;
 }
 
@@ -1007,13 +1009,14 @@ int cmd_fetch(int argc, const char **argv, const char 
*prefix)
}
 
if (!result  (recurse_submodules != RECURSE_SUBMODULES_OFF)) {
-   const char *options[10];
-   int num_options = 0;
-   add_options_to_argv(num_options, options);
-   result = fetch_populated_submodules(num_options, options,
+   struct argv_array options = ARGV_ARRAY_INIT;
+
+   add_options_to_argv(options);
+   result = fetch_populated_submodules(options.argc, options.argv,
submodule_prefix,
recurse_submodules,
verbosity  0);
+   argv_array_clear(options);
}
 
/* All names were strdup()ed or strndup()ed */
-- 
1.7.12.rc3.8.g89db099
--
To unsubscribe from this list: send the line 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] fetch --all: pass --tags/--no-tags through to each remote

2012-09-01 Thread Jeff King
Since the array struct stores a const char ** argv member
(for compatibility with most of our argv-taking functions),
we have to cast away the const-ness when freeing its
elements.

However, we used the wrong type when doing so.  It doesn't
make a difference since free() take a void pointer anyway,
but it can be slightly confusing to a reader.

Signed-off-by: Jeff King p...@peff.net
---
Noticed this while I was adding the other free in argv_array_pop...

 argv-array.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/argv-array.c b/argv-array.c
index 55e8443..256741d 100644
--- a/argv-array.c
+++ b/argv-array.c
@@ -63,7 +63,7 @@ void argv_array_clear(struct argv_array *array)
if (array-argv != empty_argv) {
int i;
for (i = 0; i  array-argc; i++)
-   free((char **)array-argv[i]);
+   free((char *)array-argv[i]);
free(array-argv);
}
argv_array_init(array);
-- 
1.7.12.rc3.8.g89db099

--
To unsubscribe from this list: send the line 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/2] argv-array: fix bogus cast when freeing array

2012-09-01 Thread Jeff King
On Sat, Sep 01, 2012 at 07:32:07AM -0400, Jeff King wrote:

 Since the array struct stores a const char ** argv member
 (for compatibility with most of our argv-taking functions),
 we have to cast away the const-ness when freeing its
 elements.
 
 However, we used the wrong type when doing so.  It doesn't
 make a difference since free() take a void pointer anyway,
 but it can be slightly confusing to a reader.
 
 Signed-off-by: Jeff King p...@peff.net
 ---
 Noticed this while I was adding the other free in argv_array_pop...

Argh, managed to botch the subject line. Here it is for real.

-- 8 --
Since the array struct stores a const char ** argv member
(for compatibility with most of our argv-taking functions),
we have to cast away the const-ness when freeing its
elements.

However, we used the wrong type when doing so.  It doesn't
make a difference since free() take a void pointer anyway,
but it can be slightly confusing to a reader.

Signed-off-by: Jeff King p...@peff.net
---
 argv-array.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/argv-array.c b/argv-array.c
index 55e8443..256741d 100644
--- a/argv-array.c
+++ b/argv-array.c
@@ -63,7 +63,7 @@ void argv_array_clear(struct argv_array *array)
if (array-argv != empty_argv) {
int i;
for (i = 0; i  array-argc; i++)
-   free((char **)array-argv[i]);
+   free((char *)array-argv[i]);
free(array-argv);
}
argv_array_init(array);
-- 
1.7.12.rc3.8.g89db099

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


[PATCH 5/4 v3] wincred: port to generic credential helper

2012-09-01 Thread Philipp A. Hartmann
From: Philipp A. Hartmann p...@qo.cx

In addition to porting the helper to the generic API,
this patch clears up all passwords from memory, which
reduces the total amount to saved lines.

This version will now pass t0303 if you do

   GIT_TEST_CREDENTIAL_HELPER=wincred \
   ./t0303-credential-external.sh

Signed-off-by: Philipp A. Hartmann p...@qo.cx
---

Erik, thanks for testing!

I finally managed to test the wincred implementation against t0303
and found another stupid bug in the credential matching of the port:

  -  (c-port  match_attr(cred, Lgit_port, port_buf)) 
  +  (!c-port || match_attr(cred, Lgit_port, port_buf)) 

Together with the fixes from Erik, the tests (and additional manual tests
with explicit ports, etc) now pass.

If this makes the generic implementation useful enough, I'd re-roll at least
the patches #2-#5 (#1 is already merged to next)  to a new series by adding
the final generic helper in one step and adding the ports afterwards.

[Apologies for reposting, I somehow broke the From: header in my
 original mail.]

 contrib/credential/helper/credential_helper.c  |   41 ++-
 contrib/credential/helper/credential_helper.h  |   16 +-
 contrib/credential/wincred/.gitignore  |1 +
 contrib/credential/wincred/Makefile|   20 +-
 .../credential/wincred/git-credential-wincred.c|  312 +---
 5 files changed, 214 insertions(+), 176 deletions(-)
 create mode 100644 contrib/credential/wincred/.gitignore

diff --git a/contrib/credential/helper/credential_helper.c 
b/contrib/credential/helper/credential_helper.c
index e99c2ec..15536a6 100644
--- a/contrib/credential/helper/credential_helper.c
+++ b/contrib/credential/helper/credential_helper.c
@@ -11,6 +11,11 @@
 
 #include credential_helper.h
 
+#ifdef WIN32
+#include fcntl.h
+#include io.h
+#endif
+
 void credential_init(struct credential *c)
 {
memset(c, 0, sizeof(*c));
@@ -23,6 +28,7 @@ void credential_clear(struct credential *c)
free(c-path);
free(c-username);
free_password(c-password);
+   free(c-url);
 
credential_init(c);
 }
@@ -79,6 +85,29 @@ int credential_read(struct credential *c)
 * learn new lines, and the helpers are updated to match.
 */
}
+
+   /* Rebuild URI from parts */
+   *buf = '\0';
+   if (c-protocol) {
+   strncat(buf, c-protocol, sizeof(buf));
+   strncat(buf, ://, sizeof(buf));
+   }
+   if (c-username) {
+   strncat(buf, c-username, sizeof(buf));
+   strncat(buf, @, sizeof(buf));
+   }
+   if (c-host)
+   strncat(buf, c-host, sizeof(buf));
+   if (c-port) {
+   value = buf + strlen(buf);
+   snprintf(value, sizeof(buf)-(value-buf), :%hd, c-port);
+   }
+   if (c-path) {
+   strncat(buf, /, sizeof(buf));
+   strncat(buf, c-path, sizeof(buf));
+   }
+   c-url = xstrdup(buf);
+
return 0;
 }
 
@@ -126,6 +155,12 @@ int main(int argc, char *argv[])
goto out;
}
 
+#ifdef WIN32
+   /* git on Windows uses binary pipes to avoid CRLF-issues */
+   _setmode(_fileno(stdin), _O_BINARY);
+   _setmode(_fileno(stdout), _O_BINARY);
+#endif
+
/* lookup operation callback */
while(try_op-name  strcmp(argv[1], try_op-name))
try_op++;
@@ -138,11 +173,15 @@ int main(int argc, char *argv[])
if(ret)
goto out;
 
+   if (!cred.protocol || !(cred.host || cred.path)) {
+   ret = EXIT_FAILURE;
+   goto out;
+   }
+
/* perform credential operation */
ret = (*try_op-op)(cred);
 
credential_write(cred);
-
 out:
credential_clear(cred);
return ret;
diff --git a/contrib/credential/helper/credential_helper.h 
b/contrib/credential/helper/credential_helper.h
index 76b6e50..b5ffa5b 100644
--- a/contrib/credential/helper/credential_helper.h
+++ b/contrib/credential/helper/credential_helper.h
@@ -25,10 +25,11 @@ struct credential
char  *path;
char  *username;
char  *password;
+   char  *url; /* protocol://[username@][host[:port]][/path] 
*/
 };
 
 #define CREDENTIAL_INIT \
-  { NULL,NULL,0,NULL,NULL,NULL }
+  { NULL,NULL,0,NULL,NULL,NULL,NULL }
 
 void credential_init(struct credential *c);
 void credential_clear(struct credential *c);
@@ -104,6 +105,17 @@ static inline void die_errno(int err)
exit(EXIT_FAILURE);
 }
 
+static inline void *xmalloc(size_t size)
+{
+  void *ret = malloc(size);
+   if (!ret  !size)
+   ret = malloc(1);
+   if (!ret)
+die_errno(errno);
+
+   return ret;
+}
+
 static inline char *xstrdup(const char *str)
 {
char *ret = strdup(str);
@@ -113,6 +125,7 @@ static inline char *xstrdup(const char *str)
return ret;
 }
 
+#ifndef NO_STRNDUP
 static 

Re: [PATCH v4] Thunderbird: fix appp.sh format problems

2012-09-01 Thread Johannes Sixt
Am 01.09.2012 09:43, schrieb Marco Stornelli:
 Il 31/08/2012 23:35, Johannes Sixt ha scritto:
 Am 31.08.2012 16:09, schrieb Marco Stornelli:
 +CCS=`perl -e 'local $/=undef; open FILE, $ENV{'PATCHTMP'};
 $text=FILE;
 +close FILE; $addr = $1 if $text =~ /Cc: (.*?(,\n .*?)*)\n/s; $addr
 =~ s/\n//g;
 +print $addr;'`

 The quoting is broken in this line (sq within sq does not work).
 
 I don't understand what you mean, I'm using this script and it works
 perfectly.

Look how you write:

  perl -e '... $ENV{'PATCHTMP'} ...'

That is, perl actually sees this script:

  ... $ENV{PATCHTMP} ...

(no quotes around PATCHTMP). That my be perfectly valid perl, but is not
what you intended.

-- Hannes

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


Re: [PATCH 2/2] fetch: use argv_array instead of hand-building arrays

2012-09-01 Thread Jens Lehmann
Am 01.09.2012 13:27, schrieb Jeff King:
 Fetch invokes itself recursively when recursing into
 submodules or handling fetch --multiple. In both cases, it
 builds the child's command line by pushing options onto a
 statically-sized array. In both cases, the array is
 currently just big enough to handle the largest possible
 case. However, this technique is brittle and error-prone, so
 let's replace it with a dynamic argv_array.
 
 Signed-off-by: Jeff King p...@peff.net
 ---
 Not very well tested by me, but hopefully it is simple enough that I
 managed not to screw it up.

This is definitely an improvement, and I can't spot any problems
either.

 It may be that fetch_populated_submodules would also benefit from
 conversion (here I just pass in the argc and argv separately), but I
 didn't look.

Yes, it does some similar brittle stuff and should be changed to use
the argv-array too. I'll look into that.
--
To unsubscribe from this list: send the line 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] submodule: use argv_array instead of hand-building arrays

2012-09-01 Thread Jens Lehmann
fetch_populated_submodules() allocates the full argv array it uses to
recurse into the submodules from the number of given options plus the six
argv values it is going to add. It then initializes it with those values
which won't change during the iteration and copies the given options into
it. Inside the loop the two argv values different for each submodule get
replaced with those currently valid.

However, this technique is brittle and error-prone (as the comment to
explain the magic number 6 indicates), so let's replace it with an
argv_array. Instead of replacing the argv values, push them to the
argv_array just before the run_command() call (including the option
separating them) and pop them from the argv_array right after that.

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


Am 01.09.2012 16:34, schrieb Jens Lehmann:
 Am 01.09.2012 13:27, schrieb Jeff King:
 It may be that fetch_populated_submodules would also benefit from
 conversion (here I just pass in the argc and argv separately), but I
 didn't look.
 
 Yes, it does some similar brittle stuff and should be changed to use
 the argv-array too. I'll look into that.

Maybe something like this on top of your two patches?

I thought about adding an argv_array_cat() function to replace the
for() loop copying the option values into the argv-array built inside
fetch_populated_submodules(), but I suspect saving one line from the
code is not worth it. Yet I didn't check if others would benefit from
such a function too.


 builtin/fetch.c |  2 +-
 submodule.c | 31 ---
 submodule.h |  3 ++-
 3 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index b6a8be0..aaba61e 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1012,7 +1012,7 @@ int cmd_fetch(int argc, const char **argv, const char 
*prefix)
struct argv_array options = ARGV_ARRAY_INIT;

add_options_to_argv(options);
-   result = fetch_populated_submodules(options.argc, options.argv,
+   result = fetch_populated_submodules(options,
submodule_prefix,
recurse_submodules,
verbosity  0);
diff --git a/submodule.c b/submodule.c
index 19dc6a6..51d48c2 100644
--- a/submodule.c
+++ b/submodule.c
@@ -588,13 +588,13 @@ static void calculate_changed_submodule_paths(void)
initialized_fetch_ref_tips = 0;
 }

-int fetch_populated_submodules(int num_options, const char **options,
+int fetch_populated_submodules(const struct argv_array *options,
   const char *prefix, int command_line_option,
   int quiet)
 {
-   int i, result = 0, argc = 0, default_argc;
+   int i, result = 0;
struct child_process cp;
-   const char **argv;
+   struct argv_array argv = ARGV_ARRAY_INIT;
struct string_list_item *name_for_path;
const char *work_tree = get_git_work_tree();
if (!work_tree)
@@ -604,17 +604,13 @@ int fetch_populated_submodules(int num_options, const 
char **options,
if (read_cache()  0)
die(index file corrupt);

-   /* 6: fetch (options) --recurse-submodules-default default 
--submodule-prefix prefix NULL */
-   argv = xcalloc(num_options + 6, sizeof(const char *));
-   argv[argc++] = fetch;
-   for (i = 0; i  num_options; i++)
-   argv[argc++] = options[i];
-   argv[argc++] = --recurse-submodules-default;
-   default_argc = argc++;
-   argv[argc++] = --submodule-prefix;
+   argv_array_push(argv, fetch);
+   for (i = 0; i  options-argc; i++)
+   argv_array_push(argv, options-argv[i]);
+   argv_array_push(argv, --recurse-submodules-default);
+   /* default value, --submodule-prefix and its value are added later */

memset(cp, 0, sizeof(cp));
-   cp.argv = argv;
cp.env = local_repo_env;
cp.git_cmd = 1;
cp.no_stdin = 1;
@@ -674,16 +670,21 @@ int fetch_populated_submodules(int num_options, const 
char **options,
if (!quiet)
printf(Fetching submodule %s%s\n, prefix, 
ce-name);
cp.dir = submodule_path.buf;
-   argv[default_argc] = default_argv;
-   argv[argc] = submodule_prefix.buf;
+   argv_array_push(argv, default_argv);
+   argv_array_push(argv, --submodule-prefix);
+   argv_array_push(argv, submodule_prefix.buf);
+   cp.argv = argv.argv;
if (run_command(cp))
result = 1;
+   argv_array_pop(argv);
+   argv_array_pop(argv);
+   argv_array_pop(argv);
}
  

Should GIT_AUTHOR_{NAME,EMAIL} set the tagger name/email?

2012-09-01 Thread Ævar Arnfjörð Bjarmason
Maybe this is documented in some place I didn't spot, but I expected
that when I set GIT_AUTHOR_{NAME,EMAIL} it would affect the operation
of git-tag, but it doesn't seem to. When I create tags it seems to
completely ignore those variables.

Should it be doing that? Here's a test script demonstrating the issue:

#!/bin/sh -e
# Set defaults
git config --global user.name Ævar Arnfjörð Bjarmason
git config --global user.email ava...@gmail.com

rm -rf /tmp/test-git
git init /tmp/test-git
cd /tmp/test-git

make_commit() {
file=$1
content=$2
echo $content $file
git add $file
git commit -m$file: $content $file
git --no-pager log -1 HEAD | grep ^Author
}

make_commit README testing content
git config user.name Test User
git config user.email t...@example.com
make_commit README testing content again
git tag -a -mannotated tag tag-name-1
git --no-pager show tag-name-1 | grep ^Author

GIT_AUTHOR_NAME=Tag Test User
GIT_AUTHOR_EMAIL=tagt...@example.com git tag -a -manother annotated
tag tag-name-2
git --no-pager show tag-name-2 | grep ^Author

Which outputs:

$ sh /tmp/test-tag.sh
Initialized empty Git repository in /tmp/test-git/.git/
[master (root-commit) 9816756] README: testing content
 1 file changed, 1 insertion(+)
 create mode 100644 README
Author: Ævar Arnfjörð Bjarmason ava...@gmail.com
[master 304b71e] README: testing content again
 1 file changed, 1 insertion(+), 1 deletion(-)
Author: Test User t...@example.com
Author: Test User t...@example.com
Author: Test User t...@example.com

I'd expect references to Tag Test User tagt...@example.com for the
second tag I created.
--
To unsubscribe from this list: send the line 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: Should GIT_AUTHOR_{NAME,EMAIL} set the tagger name/email?

2012-09-01 Thread Andreas Schwab
Ævar Arnfjörð Bjarmason ava...@gmail.com writes:

 git --no-pager show tag-name-1 | grep ^Author

A tag doesn't have an author, it has a tagger.  This shows the author of
the *commit*.

 GIT_AUTHOR_NAME=Tag Test User
 GIT_AUTHOR_EMAIL=tagt...@example.com git tag -a -manother annotated
 tag tag-name-2

The tagger is controlled by the committer info.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Should GIT_AUTHOR_{NAME,EMAIL} set the tagger name/email?

2012-09-01 Thread Ævar Arnfjörð Bjarmason
On Sat, Sep 1, 2012 at 5:57 PM, Andreas Schwab sch...@linux-m68k.org wrote:
 Ævar Arnfjörð Bjarmason ava...@gmail.com writes:

 git --no-pager show tag-name-1 | grep ^Author

 A tag doesn't have an author, it has a tagger.  This shows the author of
 the *commit*.

I got the grep wrong, I meant that I expected the tagger to be set
according to GIT_AUTHOR_{NAME,EMAIL}, but it isn't either:

$ sh /tmp/test-tag.sh
Initialized empty Git repository in /tmp/test-git/.git/
[master (root-commit) f83fc11] README: testing content
 1 file changed, 1 insertion(+)
 create mode 100644 README
Author: Ævar Arnfjörð Bjarmason ava...@gmail.com
[master ef65731] README: testing content again
 1 file changed, 1 insertion(+), 1 deletion(-)
Author: Test User t...@example.com
Tagger: Test User t...@example.com
Author: Test User t...@example.com
Tagger: Test User t...@example.com
Author: Test User t...@example.com

 GIT_AUTHOR_NAME=Tag Test User
 GIT_AUTHOR_EMAIL=tagt...@example.com git tag -a -manother annotated
 tag tag-name-2

 The tagger is controlled by the committer info.

I don't get what you mean, what committer info?
--
To unsubscribe from this list: send the line 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: Should GIT_AUTHOR_{NAME,EMAIL} set the tagger name/email?

2012-09-01 Thread Andreas Schwab
Ævar Arnfjörð Bjarmason ava...@gmail.com writes:

 I don't get what you mean, what committer info?

GIT_COMMITTER_{NAME,EMAIL}.  A tagger isn't really an author.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Test failures in t4034

2012-09-01 Thread Ramsay Jones
Junio C Hamano wrote:
 Ramsay Jones ram...@ramsay1.demon.co.uk writes:
 

[snip]

 diff --git a/test-regex.c b/test-regex.c
 new file mode 100644
 index 000..9259985
 --- /dev/null
 +++ b/test-regex.c
 @@ -0,0 +1,35 @@
 +#include stdlib.h
 +#include stdio.h
 +#include stdarg.h
 +#include sys/types.h
 +#include regex.h
 +
 +static void die(const char *fmt, ...)
 +{
 +va_list p;
 +
 +va_start(p, fmt);
 +vfprintf(stderr, fmt, p);
 +va_end(p);
 +fputc('\n', stderr);
 +exit(128);
 +}
 
 Looks like a bit of overkill for only two call sites, whose output
 we would never see because it is behind the test, but OK.

Yes, there was a net increase in the line count when I introduced
die(), but the main program flow was less cluttered by error handling.
The net result looked much better, so I thought it was worth it.

What may not be too obvious, however, is that test-regex.c was written
to be independent of git. You should be able to compile the (single) file
on any POSIX system to determine if the system regex routines suffer this
problem. (It was also supposed to be quiet, unless it die()-ed, and
provide the result via the exit code).

Given that I'm now building it as part of git, I should have simply
#included git-compat-util.h and used the die() routine from libgit.a
(since I'm now *relying* on test-regex being linked with libgit.a).

 +int main(int argc, char **argv)
 +{
 +char *pat = [^={} \t]+;
 +char *str = ={}\nfred;
 +regex_t r;
 +regmatch_t m[1];
 +
 +if (regcomp(r, pat, REG_EXTENDED | REG_NEWLINE))
 +die(failed regcomp() for pattern '%s', pat);
 +if (regexec(r, str, 1, m, 0))
 +die(no match of pattern '%s' to string '%s', pat, str);
 +
 +/* http://sourceware.org/bugzilla/show_bug.cgi?id=3957  */
 +if (m[0].rm_so == 3) /* matches '\n' when it should not */
 +exit(1);
 
 This could be the third call site of die() that tells the user to
 build with NO_REGEX=1.  Then cd t  sh t0070-fundamental.sh -i -v would
 give that message directly to the user.

Hmm, even without -i -v, it's *very* clear what is going on, but sure
it wouldn't hurt either. (Also, I wanted to be able to distinguish an exit
via die() from a test failed error return).

So, new (tested) version of the patch comming.

ATB,
Ramsay Jones


--
To unsubscribe from this list: send the line 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 v2] test-regex: Add a test to check for a bug in the regex routines

2012-09-01 Thread Ramsay Jones

Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
 .gitignore |  1 +
 Makefile   |  1 +
 t/t0070-fundamental.sh |  5 +
 test-regex.c   | 20 
 4 files changed, 27 insertions(+)
 create mode 100644 test-regex.c

diff --git a/.gitignore b/.gitignore
index bb5c91e..68fe464 100644
--- a/.gitignore
+++ b/.gitignore
@@ -189,6 +189,7 @@
 /test-mktemp
 /test-parse-options
 /test-path-utils
+/test-regex
 /test-revision-walking
 /test-run-command
 /test-sha1
diff --git a/Makefile b/Makefile
index 6b0c961..3b760d3 100644
--- a/Makefile
+++ b/Makefile
@@ -496,6 +496,7 @@ TEST_PROGRAMS_NEED_X += test-mergesort
 TEST_PROGRAMS_NEED_X += test-mktemp
 TEST_PROGRAMS_NEED_X += test-parse-options
 TEST_PROGRAMS_NEED_X += test-path-utils
+TEST_PROGRAMS_NEED_X += test-regex
 TEST_PROGRAMS_NEED_X += test-revision-walking
 TEST_PROGRAMS_NEED_X += test-run-command
 TEST_PROGRAMS_NEED_X += test-scrap-cache-tree
diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 9bee8bf..da2c504 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -25,4 +25,9 @@ test_expect_success POSIXPERM 'mktemp to unwritable directory 
prints filename' '
grep cannotwrite/test err
 '
 
+test_expect_success 'check for a bug in the regex routines' '
+   # if this test fails, re-build git with NO_REGEX=1
+   test-regex
+'
+
 test_done
diff --git a/test-regex.c b/test-regex.c
new file mode 100644
index 000..b5bfd54
--- /dev/null
+++ b/test-regex.c
@@ -0,0 +1,20 @@
+#include git-compat-util.h
+
+int main(int argc, char **argv)
+{
+   char *pat = [^={} \t]+;
+   char *str = ={}\nfred;
+   regex_t r;
+   regmatch_t m[1];
+
+   if (regcomp(r, pat, REG_EXTENDED | REG_NEWLINE))
+   die(failed regcomp() for pattern '%s', pat);
+   if (regexec(r, str, 1, m, 0))
+   die(no match of pattern '%s' to string '%s', pat, str);
+
+   /* http://sourceware.org/bugzilla/show_bug.cgi?id=3957  */
+   if (m[0].rm_so == 3) /* matches '\n' when it should not */
+   die(regex bug confirmed: re-build git with NO_REGEX=1);
+
+   exit(0);
+}
-- 
1.7.12

--
To unsubscribe from this list: send the line 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/6] Fix Tap parse error etc.

2012-09-01 Thread Ramsay Jones
Hi Junio,

I have several branches that I've been meaning to finish up for some
time. Here, for example, I had intended to add some more patches to
merge the TABS_IN_FILENAMES and FUNNYNAMES test prerequisites and
then define a single lazy prerequisite in test-lib.sh. But I just
haven't found the time ...

[BTW, the pu branch is *very* broken on cygwin (it's the v5 index
branch). I have a patch to fix it up; I just need to write it up.]

Ramsay Jones (6):
  t3300-*.sh: Fix a TAP parse error
  t3902-*.sh: Skip all tests rather than each test
  t4016-*.sh: Skip all tests rather than each test
  test-lib.sh: Fix some shell coding style violations
  test-lib.sh: Add check for invalid use of 'skip_all' facility
  test-lib.sh: Suppress the passed all ... message if no tests run

 t/t3300-funny-names.sh | 24 --
 t/t3902-quoted.sh  | 31 +++
 t/t4016-diff-quote.sh  | 20 +++
 t/test-lib.sh  | 69 +-
 4 files changed, 81 insertions(+), 63 deletions(-)

-- 
1.7.12

--
To unsubscribe from this list: send the line 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/6] t3300-*.sh: Fix a TAP parse error

2012-09-01 Thread Ramsay Jones

At present, running the t3300-*.sh test on cygwin looks like:

$ cd t
$ ./t3300-funny-names.sh
ok 1 - setup
# passed all 1 test(s)
1..1 # SKIP Your filesystem does not allow tabs in filenames
$

Unfortunately, this is not valid TAP output, which prove notes
as follows:

$ prove --exec sh t3300-funny-names.sh
t3300-funny-names.sh .. All 1 subtests passed

Test Summary Report
---
t3300-funny-names.sh (Wstat: 0 Tests: 1 Failed: 0)
  Parse errors: No plan found in TAP output
Files=1, Tests=1,  2 wallclock secs ( 0.05 usr  0.00 sys +  \
0.90 cusr  0.49 csys =  1.43 CPU)
Result: FAIL
$

This is due to the 'trailing_plan' having a 'skip_directive'
attached to it. This is not allowed by the TAP grammar, which
only allows a 'leading_plan' to be followed by an optional
'skip_directive'. (see perldoc TAP::Parser::Grammar).

A trailing_plan is one that appears in the TAP output after one or
more test status lines (that start 'not '? 'ok ' ...), whereas a
leading_plan must appear before all test status lines (if any).

In practice, this means that the test script cannot contain a use
of the 'skip all' facility:

skip_all='Some reason to skip *all* tests in this file'
test_done

after having already executed one or more tests with (for example)
'test_expect_success'. Unfortunately, this is exactly what this
test script is doing. The first 'setup' test is actually used to
determine if the test prerequisite is satisfied by the filesystem
(ie does it allow tabs in filenames?).

In order to fix the parse errors, place the code to determine the
test prerequisite at the top level of the script, prior to the
first test, rather than as a parameter to test_expect_success.
This allows us to correctly use 'skip_all', thus:

$ ./t3300-funny-names.sh
# passed all 0 test(s)
1..0 # SKIP Your filesystem does not allow tabs in filenames
$

$ prove --exec sh t3300-funny-names.sh
t3300-funny-names.sh .. skipped: Your filesystem does not \
allow tabs in filenames
Files=1, Tests=0,  2 wallclock secs ( 0.02 usr  0.03 sys +  \
0.84 cusr  0.41 csys =  1.29 CPU)
Result: NOTESTS
$

Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
 t/t3300-funny-names.sh | 24 +++-
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/t/t3300-funny-names.sh b/t/t3300-funny-names.sh
index 1f35e55..7480d6e 100755
--- a/t/t3300-funny-names.sh
+++ b/t/t3300-funny-names.sh
@@ -11,6 +11,16 @@ tree, index, and tree objects.
 
 . ./test-lib.sh
 
+HT='   '
+
+echo 2/dev/null  Name with an${HT}HT
+if ! test -f Name with an${HT}HT
+then
+   # since FAT/NTFS does not allow tabs in filenames, skip this test
+   skip_all='Your filesystem does not allow tabs in filenames'
+   test_done
+fi
+
 p0='no-funny'
 p1='tabs   , (dq) and spaces'
 p2='just space'
@@ -23,21 +33,9 @@ test_expect_success 'setup' '
EOF
 
{ cat $p0 $p1 || :; } 
-   { echo Foo Bar Baz $p2 || :; } 
-
-   if test -f $p1  cmp $p0 $p1
-   then
-   test_set_prereq TABS_IN_FILENAMES
-   fi
+   { echo Foo Bar Baz $p2 || :; }
 '
 
-if ! test_have_prereq TABS_IN_FILENAMES
-then
-   # since FAT/NTFS does not allow tabs in filenames, skip this test
-   skip_all='Your filesystem does not allow tabs in filenames'
-   test_done
-fi
-
 test_expect_success 'setup: populate index and tree' '
git update-index --add $p0 $p2 
t0=$(git write-tree)
-- 
1.7.12

--
To unsubscribe from this list: send the line 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/6] t3902-*.sh: Skip all tests rather than each test

2012-09-01 Thread Ramsay Jones

Each test in this file is skipped if the TABS_IN_FILENAMES test
prerequisite is set. Use the 'skip_all' facility at the head of
the file to skip all of the tests instead.

Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
 t/t3902-quoted.sh | 31 +++
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/t/t3902-quoted.sh b/t/t3902-quoted.sh
index 534ee08..892f567 100755
--- a/t/t3902-quoted.sh
+++ b/t/t3902-quoted.sh
@@ -16,9 +16,8 @@ echo foo 2/dev/null  Name and an${HT}HT
 if ! test -f Name and an${HT}HT
 then
# FAT/NTFS does not allow tabs in filenames
-   say 'Your filesystem does not allow tabs in filenames'
-else
-   test_set_prereq TABS_IN_FILENAMES
+   skip_all='Your filesystem does not allow tabs in filenames'
+   test_done
 fi
 
 for_each_name () {
@@ -31,7 +30,7 @@ for_each_name () {
done
 }
 
-test_expect_success TABS_IN_FILENAMES 'setup' '
+test_expect_success 'setup' '
 
mkdir $FN 
for_each_name echo initial \\$name\ 
@@ -45,7 +44,7 @@ test_expect_success TABS_IN_FILENAMES 'setup' '
 
 '
 
-test_expect_success TABS_IN_FILENAMES 'setup expected files' '
+test_expect_success 'setup expected files' '
 cat expect.quoted \EOF 
 Name
 Name and a\nLF
@@ -75,74 +74,74 @@ With SP in it
 EOF
 '
 
-test_expect_success TABS_IN_FILENAMES 'check fully quoted output from 
ls-files' '
+test_expect_success 'check fully quoted output from ls-files' '
 
git ls-files current  test_cmp expect.quoted current
 
 '
 
-test_expect_success TABS_IN_FILENAMES 'check fully quoted output from 
diff-files' '
+test_expect_success 'check fully quoted output from diff-files' '
 
git diff --name-only current 
test_cmp expect.quoted current
 
 '
 
-test_expect_success TABS_IN_FILENAMES 'check fully quoted output from 
diff-index' '
+test_expect_success 'check fully quoted output from diff-index' '
 
git diff --name-only HEAD current 
test_cmp expect.quoted current
 
 '
 
-test_expect_success TABS_IN_FILENAMES 'check fully quoted output from 
diff-tree' '
+test_expect_success 'check fully quoted output from diff-tree' '
 
git diff --name-only HEAD^ HEAD current 
test_cmp expect.quoted current
 
 '
 
-test_expect_success TABS_IN_FILENAMES 'check fully quoted output from ls-tree' 
'
+test_expect_success 'check fully quoted output from ls-tree' '
 
git ls-tree --name-only -r HEAD current 
test_cmp expect.quoted current
 
 '
 
-test_expect_success TABS_IN_FILENAMES 'setting core.quotepath' '
+test_expect_success 'setting core.quotepath' '
 
git config --bool core.quotepath false
 
 '
 
-test_expect_success TABS_IN_FILENAMES 'check fully quoted output from 
ls-files' '
+test_expect_success 'check fully quoted output from ls-files' '
 
git ls-files current  test_cmp expect.raw current
 
 '
 
-test_expect_success TABS_IN_FILENAMES 'check fully quoted output from 
diff-files' '
+test_expect_success 'check fully quoted output from diff-files' '
 
git diff --name-only current 
test_cmp expect.raw current
 
 '
 
-test_expect_success TABS_IN_FILENAMES 'check fully quoted output from 
diff-index' '
+test_expect_success 'check fully quoted output from diff-index' '
 
git diff --name-only HEAD current 
test_cmp expect.raw current
 
 '
 
-test_expect_success TABS_IN_FILENAMES 'check fully quoted output from 
diff-tree' '
+test_expect_success 'check fully quoted output from diff-tree' '
 
git diff --name-only HEAD^ HEAD current 
test_cmp expect.raw current
 
 '
 
-test_expect_success TABS_IN_FILENAMES 'check fully quoted output from ls-tree' 
'
+test_expect_success 'check fully quoted output from ls-tree' '
 
git ls-tree --name-only -r HEAD current 
test_cmp expect.raw current
-- 
1.7.12

--
To unsubscribe from this list: send the line 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/6] t4016-*.sh: Skip all tests rather than each test

2012-09-01 Thread Ramsay Jones

Each test in this file is skipped if the TABS_IN_FILENAMES test
prerequisite is set. Use the 'skip_all' facility at the head of
the file to skip all of the tests instead.

Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
 t/t4016-diff-quote.sh | 20 +---
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/t/t4016-diff-quote.sh b/t/t4016-diff-quote.sh
index 97b8177..cd543ec 100755
--- a/t/t4016-diff-quote.sh
+++ b/t/t4016-diff-quote.sh
@@ -13,14 +13,12 @@ P1='pathnamewith HT'
 P2='pathname with SP'
 P3='pathname
 with LF'
-if : 2/dev/null $P1  test -f $P1  rm -f $P1
-then
-   test_set_prereq TABS_IN_FILENAMES
-else
-   say 'Your filesystem does not allow tabs in filenames'
-fi
+echo 2/dev/null $P1  test -f $P1  rm -f $P1 || {
+   skip_all='Your filesystem does not allow tabs in filenames'
+   test_done
+}
 
-test_expect_success TABS_IN_FILENAMES setup '
+test_expect_success setup '
echo P0.0 $P0.0 
echo P0.1 $P0.1 
echo P0.2 $P0.2 
@@ -40,7 +38,7 @@ test_expect_success TABS_IN_FILENAMES setup '
:
 '
 
-test_expect_success TABS_IN_FILENAMES 'setup expected files' '
+test_expect_success 'setup expected files' '
 cat expect \EOF
  rename pathname.1 = Rpathname\twith HT.0 (100%)
  rename pathname.3 = Rpathname\nwith LF.0 (100%)
@@ -52,12 +50,12 @@ cat expect \EOF
 EOF
 '
 
-test_expect_success TABS_IN_FILENAMES 'git diff --summary -M HEAD' '
+test_expect_success 'git diff --summary -M HEAD' '
git diff --summary -M HEAD actual 
test_cmp expect actual
 '
 
-test_expect_success TABS_IN_FILENAMES 'git diff --numstat -M HEAD' '
+test_expect_success 'git diff --numstat -M HEAD' '
cat expect -\EOF 
0   0   pathname.1 = Rpathname\twith HT.0
0   0   pathname.3 = Rpathname\nwith LF.0
@@ -71,7 +69,7 @@ test_expect_success TABS_IN_FILENAMES 'git diff --numstat -M 
HEAD' '
test_cmp expect actual
 '
 
-test_expect_success TABS_IN_FILENAMES 'git diff --stat -M HEAD' '
+test_expect_success 'git diff --stat -M HEAD' '
cat expect -\EOF 
 pathname.1 = Rpathname\twith HT.0| 0
 pathname.3 = Rpathname\nwith LF.0| 0
-- 
1.7.12

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


[PATCH 4/6] test-lib.sh: Fix some shell coding style violations

2012-09-01 Thread Ramsay Jones


Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
 t/test-lib.sh | 60 +--
 1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 78c4286..56b028c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -100,12 +100,12 @@ unset CDPATH
 unset GREP_OPTIONS
 
 case $(echo $GIT_TRACE |tr [A-Z] [a-z]) in
-   1|2|true)
-   echo * warning: Some tests will not work if GIT_TRACE \
-   is set as to trace on STDERR ! *
-   echo * warning: Please set GIT_TRACE to something \
-   other than 1, 2 or true ! *
-   ;;
+1|2|true)
+   echo * warning: Some tests will not work if GIT_TRACE \
+   is set as to trace on STDERR ! *
+   echo * warning: Please set GIT_TRACE to something \
+   other than 1, 2 or true ! *
+   ;;
 esac
 
 # Convenience
@@ -172,17 +172,23 @@ do
esac
 done
 
-if test -n $color; then
+if test -n $color
+then
say_color () {
(
TERM=$ORIGINAL_TERM
export TERM
case $1 in
-   error) tput bold; tput setaf 1;; # bold red
-   skip)  tput bold; tput setaf 2;; # bold green
-   pass)  tput setaf 2;;# green
-   info)  tput setaf 3;;# brown
-   *) test -n $quiet  return;;
+   error)
+   tput bold; tput setaf 1;; # bold red
+   skip)
+   tput bold; tput setaf 2;; # bold green
+   pass)
+   tput setaf 2;;# green
+   info)
+   tput setaf 3;;# brown
+   *)
+   test -n $quiet  return;;
esac
shift
printf %s $*
@@ -298,7 +304,8 @@ test_run_ () {
then
test_eval_ $test_cleanup
fi
-   if test $verbose = t  test -n $HARNESS_ACTIVE; then
+   if test $verbose = t  test -n $HARNESS_ACTIVE
+   then
echo 
fi
return $eval_ret
@@ -346,7 +353,8 @@ test_at_end_hook_ () {
 test_done () {
GIT_EXIT_OK=t
 
-   if test -z $HARNESS_ACTIVE; then
+   if test -z $HARNESS_ACTIVE
+   then
test_results_dir=$TEST_OUTPUT_DIRECTORY/test-results
mkdir -p $test_results_dir
test_results_path=$test_results_dir/${0%.sh}-$$.counts
@@ -377,7 +385,8 @@ test_done () {
# Maybe print SKIP message
[ -z $skip_all ] || skip_all= # SKIP $skip_all
 
-   if test $test_external_has_tap -eq 0; then
+   if test $test_external_has_tap -eq 0
+   then
say_color pass # passed all $msg
say 1..$test_count$skip_all
fi
@@ -391,7 +400,8 @@ test_done () {
exit 0 ;;
 
*)
-   if test $test_external_has_tap -eq 0; then
+   if test $test_external_has_tap -eq 0
+   then
say_color error # failed $test_failure among $msg
say 1..$test_count
fi
@@ -471,22 +481,26 @@ then
PATH=$GIT_VALGRIND/bin:$PATH
GIT_EXEC_PATH=$GIT_VALGRIND/bin
export GIT_VALGRIND
-elif test -n $GIT_TEST_INSTALLED ; then
+elif test -n $GIT_TEST_INSTALLED
+then
GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path)  ||
error Cannot run git from $GIT_TEST_INSTALLED.
PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR:$PATH
GIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}
 else # normal case, use ../bin-wrappers only unless $with_dashes:
git_bin_dir=$GIT_BUILD_DIR/bin-wrappers
-   if ! test -x $git_bin_dir/git ; then
-   if test -z $with_dashes ; then
+   if ! test -x $git_bin_dir/git
+   then
+   if test -z $with_dashes
+   then
say $git_bin_dir/git is not executable; using 
GIT_EXEC_PATH
fi
with_dashes=t
fi
PATH=$git_bin_dir:$PATH
GIT_EXEC_PATH=$GIT_BUILD_DIR
-   if test -n $with_dashes ; then
+   if test -n $with_dashes
+   then
PATH=$GIT_BUILD_DIR:$PATH
fi
 fi
@@ -521,7 +535,8 @@ then
}
 fi
 
-if ! test -x $GIT_BUILD_DIR/test-chmtime; then
+if ! test -x $GIT_BUILD_DIR/test-chmtime
+then
echo 2 'You need to build test-chmtime:'
echo 2 'Run make test-chmtime in the source (toplevel) directory'
exit 1
@@ -544,7 +559,8 @@ rm -fr $test || {
 HOME=$TRASH_DIRECTORY
 export HOME
 
-if test -z $TEST_NO_CREATE_REPO; then
+if test -z $TEST_NO_CREATE_REPO
+then
test_create_repo $test
 else
mkdir -p $test
-- 
1.7.12

--
To unsubscribe 

[PATCH 5/6] test-lib.sh: Add check for invalid use of 'skip_all' facility

2012-09-01 Thread Ramsay Jones

The 'skip_all' facility cannot be used after one or more tests
have been executed using (for example) 'test_expect_success'.
To do so results in invalid TAP output, which leads to 'prove'
complaining of Parse errors: No plan found in TAP output.

Add a check for such invalid usage and abort the test with an
error message to alert the test author.

Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
 t/test-lib.sh | 4 
 1 file changed, 4 insertions(+)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 56b028c..283d27a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -383,6 +383,10 @@ test_done () {
case $test_failure in
0)
# Maybe print SKIP message
+   if test -n $skip_all  test $test_count -gt 0
+   then
+   error Can't use skip_all after running some tests
+   fi
[ -z $skip_all ] || skip_all= # SKIP $skip_all
 
if test $test_external_has_tap -eq 0
-- 
1.7.12

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


[PATCH 6/6] test-lib.sh: Suppress the passed all ... message if no tests run

2012-09-01 Thread Ramsay Jones

If a test script issues a test_done without executing any tests, for
example when using the 'skip_all' facility, the output looks something
like this:

$ ./t9159-git-svn-no-parent-mergeinfo.sh
# passed all 0 test(s)
1..0 # SKIP skipping git svn tests, svn not found
$

The passed all 0 test(s) comment line, while correct, looks a little
strange. Add a check to suppress this message if no tests have actually
been run.

Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---

Hi Junio,

I suspect some people would disagree with this one. Indeed, this only
irritates me when I'm feeling grumpy (so most days then). :-D
It's clearly not important (I just happened to be making changes in
this area), so drop this one if you feel it's not justified.

ATB,
Ramsay Jones

 t/test-lib.sh | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 283d27a..f8e3733 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -391,7 +391,10 @@ test_done () {
 
if test $test_external_has_tap -eq 0
then
-   say_color pass # passed all $msg
+   if test $test_count -gt 0
+   then
+   say_color pass # passed all $msg
+   fi
say 1..$test_count$skip_all
fi
 
-- 
1.7.12

--
To unsubscribe from this list: send the line 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] Thunderbird: fix appp.sh format problems

2012-09-01 Thread Marco Stornelli

Il 01/09/2012 15:59, Johannes Sixt ha scritto:

Am 01.09.2012 09:43, schrieb Marco Stornelli:

Il 31/08/2012 23:35, Johannes Sixt ha scritto:

Am 31.08.2012 16:09, schrieb Marco Stornelli:

+CCS=`perl -e 'local $/=undef; open FILE, $ENV{'PATCHTMP'};
$text=FILE;
+close FILE; $addr = $1 if $text =~ /Cc: (.*?(,\n .*?)*)\n/s; $addr
=~ s/\n//g;
+print $addr;'`


The quoting is broken in this line (sq within sq does not work).


I don't understand what you mean, I'm using this script and it works
perfectly.


Look how you write:

   perl -e '... $ENV{'PATCHTMP'} ...'

That is, perl actually sees this script:

   ... $ENV{PATCHTMP} ...

(no quotes around PATCHTMP). That my be perfectly valid perl, but is not
what you intended.

-- Hannes




I don't understand what you mean when you say is not what you intended 
because perl and this script does *exactly* what I want and (with the 
test I've done) it works. Have you got a test case where it doesn't work 
at all? However, since I'm not a perl guru, if someone says that double 
quotes are needed, ok the result is however the same.


Git guys, let me know if a version 6 of the patch is needed, no problem 
to submit another version.


Regards,

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


Re: diff/merge tool that ignores whitespace changes

2012-09-01 Thread Enrico Weigelt

snip

Thanks folks, but that doesn't solve my problem. I'm looking for something
that's usable on command line or in scripts.

Usecase a)

* git-diff or git-format-patch or tig should not show differences
  that are only whitespace changes (eg. differing linefeeds or
  tabs vs. spaces, changed indentions, etc)

Usecase b)

* when doing merges or rebases, changes in whitespaces only should be
  either ignored or resolved fully automtically.
* For example:
  - A changes spaces into tabs or adds leading/trailing spaces
  - B changes some non-spaces 

cu
-- 
Mit freundlichen Grüßen / Kind regards 

Enrico Weigelt 
VNC - Virtual Network Consult GmbH 
Head Of Development 

Pariser Platz 4a, D-10117 Berlin
Tel.: +49 (30) 3464615-20
Fax: +49 (30) 3464615-59

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


Re: diff/merge tool that ignores whitespace changes

2012-09-01 Thread Torsten Bögershausen
On 01.09.12 22:11, Enrico Weigelt wrote:
 
 snip
 
 Thanks folks, but that doesn't solve my problem. I'm looking for something
 that's usable on command line or in scripts.
 
 Usecase a)
 
 * git-diff or git-format-patch or tig should not show differences
   that are only whitespace changes (eg. differing linefeeds or
   tabs vs. spaces, changed indentions, etc)

Would that help ?
git help diff
[snip]
 --ignore-space-at-eol
   Ignore changes in whitespace at EOL.

   -b, --ignore-space-change
   Ignore changes in amount of whitespace. This ignores whitespace at
   line end, and considers all other sequences of one or more
   whitespace characters to be equivalent.

   -w, --ignore-all-space
   Ignore whitespace when comparing lines. This ignores differences
   even if one line has whitespace where the other line has none.

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


Re: diff/merge tool that ignores whitespace changes

2012-09-01 Thread Andreas Schwab
Enrico Weigelt enrico.weig...@vnc.biz writes:

 * git-diff or git-format-patch or tig should not show differences
   that are only whitespace changes (eg. differing linefeeds or
   tabs vs. spaces, changed indentions, etc)

--ignore-all-space

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Suggested behavior when pulling .gitattributes

2012-09-01 Thread Aleksandr Dubinsky
Hi,

I ran into a problem with line endings that .gitattributes is supposed
to fix. However, I ran into a headache with this not giving the
desired result. This headache could have easily been avoided if:

When pulling .gitattributes, git should parse the file and anything
new in it should be acted upon. Specifically, if a file already exists
with modified endings and .gitattributes now says it should be treated
as binary, then please automatically re-checkout the file with the
correct, un-modified line endings.

Possibly this suggestion also applies to pushing/committing but that
wasn't the problem I ran into just now.
--
To unsubscribe from this list: send the line 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/9] Rename cryptic 'which' variable to more consistent name

2012-09-01 Thread Adam Spiers
'el' is only *slightly* less cryptic, but is already used as the
variable name for a struct exclude_list pointer in numerous other
places, so this reduces the number of cryptic variable names in use by
one :-)

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 dir.c | 10 +-
 dir.h |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/dir.c b/dir.c
index c9f341a..57a5d11 100644
--- a/dir.c
+++ b/dir.c
@@ -311,7 +311,7 @@ static int no_wildcard(const char *string)
 }
 
 void add_exclude(const char *string, const char *base,
-int baselen, struct exclude_list *which)
+int baselen, struct exclude_list *el)
 {
struct exclude *x;
size_t len;
@@ -346,8 +346,8 @@ void add_exclude(const char *string, const char *base,
x-nowildcardlen = simple_length(string);
if (*string == '*'  no_wildcard(string+1))
x-flags |= EXC_FLAG_ENDSWITH;
-   ALLOC_GROW(which-excludes, which-nr + 1, which-alloc);
-   which-excludes[which-nr++] = x;
+   ALLOC_GROW(el-excludes, el-nr + 1, el-alloc);
+   el-excludes[el-nr++] = x;
 }
 
 static void *read_skip_worktree_file_from_index(const char *path, size_t *size)
@@ -389,7 +389,7 @@ int add_excludes_from_file_to_list(const char *fname,
   const char *base,
   int baselen,
   char **buf_p,
-  struct exclude_list *which,
+  struct exclude_list *el,
   int check_index)
 {
struct stat st;
@@ -436,7 +436,7 @@ int add_excludes_from_file_to_list(const char *fname,
if (buf[i] == '\n') {
if (entry != buf + i  entry[0] != '#') {
buf[i - (i  buf[i-1] == '\r')] = 0;
-   add_exclude(entry, base, baselen, which);
+   add_exclude(entry, base, baselen, el);
}
entry = buf + i + 1;
}
diff --git a/dir.h b/dir.h
index a226fbc..549a187 100644
--- a/dir.h
+++ b/dir.h
@@ -117,10 +117,10 @@ extern int path_excluded(struct path_exclude_check *, 
const char *, int namelen,
 
 
 extern int add_excludes_from_file_to_list(const char *fname, const char *base, 
int baselen,
- char **buf_p, struct exclude_list 
*which, int check_index);
+ char **buf_p, struct exclude_list 
*el, int check_index);
 extern void add_excludes_from_file(struct dir_struct *, const char *fname);
 extern void add_exclude(const char *string, const char *base,
-   int baselen, struct exclude_list *which);
+   int baselen, struct exclude_list *el);
 extern void free_excludes(struct exclude_list *el);
 extern int file_exists(const char *);
 
-- 
1.7.12.155.ge5750d5.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


[PATCH 7/9] Extract some useful pathspec handling code from builtin/add.c into a library

2012-09-01 Thread Adam Spiers
This is in preparation for reuse by a new git check-ignore command.

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 Makefile  |  2 ++
 builtin/add.c | 82 +++
 pathspec.c| 87 +++
 pathspec.h|  6 +
 4 files changed, 98 insertions(+), 79 deletions(-)
 create mode 100644 pathspec.c
 create mode 100644 pathspec.h

diff --git a/Makefile b/Makefile
index 66e8216..272ab69 100644
--- a/Makefile
+++ b/Makefile
@@ -640,6 +640,7 @@ LIB_H += pack-revindex.h
 LIB_H += pack.h
 LIB_H += parse-options.h
 LIB_H += patch-ids.h
+LIB_H += pathspec.h
 LIB_H += pkt-line.h
 LIB_H += progress.h
 LIB_H += prompt.h
@@ -760,6 +761,7 @@ LIB_OBJS += parse-options-cb.o
 LIB_OBJS += patch-delta.o
 LIB_OBJS += patch-ids.o
 LIB_OBJS += path.o
+LIB_OBJS += pathspec.o
 LIB_OBJS += pkt-line.o
 LIB_OBJS += preload-index.o
 LIB_OBJS += pretty.o
diff --git a/builtin/add.c b/builtin/add.c
index 89dce56..a7ed2ad 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -6,6 +6,7 @@
 #include cache.h
 #include builtin.h
 #include dir.h
+#include pathspec.h
 #include exec_cmd.h
 #include cache-tree.h
 #include run-command.h
@@ -97,39 +98,6 @@ int add_files_to_cache(const char *prefix, const char 
**pathspec, int flags)
return !!data.add_errors;
 }
 
-static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
-{
-   int num_unmatched = 0, i;
-
-   /*
-* Since we are walking the index as if we were walking the directory,
-* we have to mark the matched pathspec as seen; otherwise we will
-* mistakenly think that the user gave a pathspec that did not match
-* anything.
-*/
-   for (i = 0; i  specs; i++)
-   if (!seen[i])
-   num_unmatched++;
-   if (!num_unmatched)
-   return;
-   for (i = 0; i  active_nr; i++) {
-   struct cache_entry *ce = active_cache[i];
-   match_pathspec(pathspec, ce-name, ce_namelen(ce), 0, seen);
-   }
-}
-
-static char *find_used_pathspec(const char **pathspec)
-{
-   char *seen;
-   int i;
-
-   for (i = 0; pathspec[i];  i++)
-   ; /* just counting */
-   seen = xcalloc(i, 1);
-   fill_pathspec_matches(pathspec, seen, i);
-   return seen;
-}
-
 static char *prune_directory(struct dir_struct *dir, const char **pathspec, 
int prefix)
 {
char *seen;
@@ -153,33 +121,6 @@ static char *prune_directory(struct dir_struct *dir, const 
char **pathspec, int
return seen;
 }
 
-static void treat_gitlinks(const char **pathspec)
-{
-   int i;
-
-   if (!pathspec || !*pathspec)
-   return;
-
-   for (i = 0; i  active_nr; i++) {
-   struct cache_entry *ce = active_cache[i];
-   if (S_ISGITLINK(ce-ce_mode)) {
-   int len = ce_namelen(ce), j;
-   for (j = 0; pathspec[j]; j++) {
-   int len2 = strlen(pathspec[j]);
-   if (len2 = len || pathspec[j][len] != '/' ||
-   memcmp(ce-name, pathspec[j], len))
-   continue;
-   if (len2 == len + 1)
-   /* strip trailing slash */
-   pathspec[j] = xstrndup(ce-name, len);
-   else
-   die (_(Path '%s' is in submodule 
'%.*s'),
-   pathspec[j], len, ce-name);
-   }
-   }
-   }
-}
-
 static void refresh(int verbose, const char **pathspec)
 {
char *seen;
@@ -197,23 +138,6 @@ static void refresh(int verbose, const char **pathspec)
 free(seen);
 }
 
-static const char **validate_pathspec(int argc, const char **argv, const char 
*prefix)
-{
-   const char **pathspec = get_pathspec(prefix, argv);
-
-   if (pathspec) {
-   const char **p;
-   for (p = pathspec; *p; p++) {
-   if (has_symlink_leading_path(*p, strlen(*p))) {
-   int len = prefix ? strlen(prefix) : 0;
-   die(_('%s' is beyond a symbolic link), *p + 
len);
-   }
-   }
-   }
-
-   return pathspec;
-}
-
 int run_add_interactive(const char *revision, const char *patch_mode,
const char **pathspec)
 {
@@ -248,7 +172,7 @@ int interactive_add(int argc, const char **argv, const char 
*prefix, int patch)
const char **pathspec = NULL;
 
if (argc) {
-   pathspec = validate_pathspec(argc, argv, prefix);
+   pathspec = validate_pathspec(prefix, argv);
if (!pathspec)
return -1;
}
@@ -414,7 +338,7 @@ int cmd_add(int argc, 

[PATCH 6/9] For each exclude pattern, store information about where it came from

2012-09-01 Thread Adam Spiers
For exclude patterns read in from files, the filename is stored together
with the corresponding line number (counting starting at 1).

For exclude patterns provided on the command line, the sequence number
is negative, with counting starting at -1, so for example the 2nd
pattern provided via --exclude would be numbered -2.  This allows any
future consumers of that data to easily distinguish between exclude
patterns from files vs. from the CLI.

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 builtin/clean.c|  2 +-
 builtin/ls-files.c |  3 ++-
 dir.c  | 25 +++--
 dir.h  |  5 -
 4 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 0c7b3d0..f618231 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -99,7 +99,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 
for (i = 0; i  exclude_list.nr; i++)
add_exclude(exclude_list.items[i].string, , 0,
-   dir.exclude_list[EXC_CMDL]);
+   dir.exclude_list[EXC_CMDL], --exclude option, 
-(i+1));
 
pathspec = get_pathspec(prefix, argv);
 
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 31b3f2d..420ff40 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -35,6 +35,7 @@ static int error_unmatch;
 static char *ps_matched;
 static const char *with_tree;
 static int exc_given;
+static int exclude_args = 0;
 
 static const char *tag_cached = ;
 static const char *tag_unmerged = ;
@@ -423,7 +424,7 @@ static int option_parse_exclude(const struct option *opt,
struct exclude_list *list = opt-value;
 
exc_given = 1;
-   add_exclude(arg, , 0, list);
+   add_exclude(arg, , 0, list, --exclude option, --exclude_args);
 
return 0;
 }
diff --git a/dir.c b/dir.c
index 3d438c3..ac8c838 100644
--- a/dir.c
+++ b/dir.c
@@ -311,7 +311,7 @@ static int no_wildcard(const char *string)
 }
 
 void add_exclude(const char *string, const char *base,
-int baselen, struct exclude_list *el)
+int baselen, struct exclude_list *el, const char *src, int 
srcpos)
 {
struct exclude *x;
size_t len;
@@ -341,6 +341,8 @@ void add_exclude(const char *string, const char *base,
x-base = base;
x-baselen = baselen;
x-flags = flags;
+   x-src = src;
+   x-srcpos = srcpos;
if (!strchr(string, '/'))
x-flags |= EXC_FLAG_NODIR;
x-nowildcardlen = simple_length(string);
@@ -393,7 +395,7 @@ int add_excludes_from_file_to_list(const char *fname,
   int check_index)
 {
struct stat st;
-   int fd, i;
+   int fd, i, lineno = 1;
size_t size = 0;
char *buf, *entry;
 
@@ -436,8 +438,9 @@ int add_excludes_from_file_to_list(const char *fname,
if (buf[i] == '\n') {
if (entry != buf + i  entry[0] != '#') {
buf[i - (i  buf[i-1] == '\r')] = 0;
-   add_exclude(entry, base, baselen, el);
+   add_exclude(entry, base, baselen, el, fname, 
lineno);
}
+   lineno++;
entry = buf + i + 1;
}
}
@@ -472,8 +475,10 @@ static void prep_exclude(struct dir_struct *dir, const 
char *base, int baselen)
!strncmp(dir-basebuf, base, stk-baselen))
break;
dir-exclude_stack = stk-prev;
-   while (stk-exclude_ix  el-nr)
-   free(el-excludes[--el-nr]);
+   while (stk-exclude_ix  el-nr) {
+   struct exclude *exclude = el-excludes[--el-nr];
+   free(exclude);
+   }
free(stk-filebuf);
free(stk);
}
@@ -500,7 +505,15 @@ static void prep_exclude(struct dir_struct *dir, const 
char *base, int baselen)
memcpy(dir-basebuf + current, base + current,
   stk-baselen - current);
strcpy(dir-basebuf + stk-baselen, dir-exclude_per_dir);
-   add_excludes_from_file_to_list(dir-basebuf,
+
+   /* dir-basebuf gets reused by the traversal, but we
+* need fname to remain unchanged to ensure the src
+* member of each struct exclude correctly back-references
+* its source file.
+*/
+   char *fname = strdup(dir-basebuf);
+
+   add_excludes_from_file_to_list(fname,
   dir-basebuf, stk-baselen,
   stk-filebuf, el, 1);
dir-exclude_stack = stk;
diff --git a/dir.h b/dir.h
index 1b4f9dc..81efee4 100644
--- a/dir.h
+++ b/dir.h
@@ -31,6 +31,9 @@ struct exclude_list {
int baselen;
 

[PATCH 8/9] Provide free_directory() for reclaiming dir_struct memory

2012-09-01 Thread Adam Spiers
Signed-off-by: Adam Spiers g...@adamspiers.org
---
 Documentation/technical/api-directory-listing.txt |  2 ++
 dir.c | 23 +--
 dir.h |  1 +
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/Documentation/technical/api-directory-listing.txt 
b/Documentation/technical/api-directory-listing.txt
index 944fc39..e339c18 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -79,4 +79,6 @@ marked. If you to exclude files, make sure you have loaded 
index first.
 
 * Use `dir.entries[]`.
 
+* Call `free_directory()` when none of the contained elements are no longer in 
use.
+
 (JC)
diff --git a/dir.c b/dir.c
index ac8c838..80f9b22 100644
--- a/dir.c
+++ b/dir.c
@@ -454,6 +454,12 @@ void add_excludes_from_file(struct dir_struct *dir, const 
char *fname)
die(cannot use %s as an exclude file, fname);
 }
 
+static void free_exclude_stack(struct exclude_stack *stk)
+{
+   free(stk-filebuf);
+   free(stk);
+}
+
 /*
  * Loads the per-directory exclude list for the substring of base
  * which has a char length of baselen.
@@ -479,8 +485,7 @@ static void prep_exclude(struct dir_struct *dir, const char 
*base, int baselen)
struct exclude *exclude = el-excludes[--el-nr];
free(exclude);
}
-   free(stk-filebuf);
-   free(stk);
+   free_exclude_stack(stk);
}
 
/* Read from the parent directories and push them down. */
@@ -1467,3 +1472,17 @@ void free_pathspec(struct pathspec *pathspec)
free(pathspec-items);
pathspec-items = NULL;
 }
+
+void free_directory(struct dir_struct *dir)
+{
+   int st;
+   for (st = EXC_CMDL; st = EXC_FILE; st++)
+   free_excludes(dir-exclude_list[st]);
+
+   struct exclude_stack *prev, *stk = dir-exclude_stack;
+   while (stk) {
+   prev = stk-prev;
+   free_exclude_stack(stk);
+   stk = prev;
+   }
+}
diff --git a/dir.h b/dir.h
index 81efee4..f7cea9c 100644
--- a/dir.h
+++ b/dir.h
@@ -128,6 +128,7 @@ extern void add_excludes_from_file(struct dir_struct *, 
const char *fname);
 extern void add_exclude(const char *string, const char *base,
int baselen, struct exclude_list *el, const char *src, 
int srcpos);
 extern void free_excludes(struct exclude_list *el);
+extern void free_directory(struct dir_struct *dir);
 extern int file_exists(const char *);
 
 extern int is_inside_dir(const char *dir);
-- 
1.7.12.155.ge5750d5.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


[PATCH 4/9] Refactor excluded_from_list

2012-09-01 Thread Adam Spiers
The excluded function uses a new helper function called
exclude_from_list_1() to perform the inner loop over all of the
exclude patterns.  The helper just tells us whether the path is
included, excluded, or undecided.

However, it may be useful to know _which_ pattern was
triggered.  So let's pass out the entire exclude match,
which contains the status information we were already
passing out.

Further patches can make use of this.

This is a modified forward port of a patch from 2009 by Jeff King:
http://article.gmane.org/gmane.comp.version-control.git/108815

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 dir.c | 40 +---
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/dir.c b/dir.c
index 57a5d11..3a532d5 100644
--- a/dir.c
+++ b/dir.c
@@ -509,22 +509,24 @@ static void prep_exclude(struct dir_struct *dir, const 
char *base, int baselen)
dir-basebuf[baselen] = '\0';
 }
 
-/* Scan the list and let the last match determine the fate.
- * Return 1 for exclude, 0 for include and -1 for undecided.
+/*
+ * Scan the given exclude list in reverse to see whether pathname
+ * should be ignored.  The first match (i.e. the last on the list), if
+ * any, determines the fate.  Returns the exclude_list element which
+ * matched, or NULL for undecided.
  */
-int excluded_from_list(const char *pathname,
-  int pathlen, const char *basename, int *dtype,
-  struct exclude_list *el)
+struct exclude *excluded_from_list_1(const char *pathname, int pathlen,
+const char *basename, int *dtype,
+struct exclude_list *el)
 {
int i;
 
if (!el-nr)
-   return -1;  /* undefined */
+   return NULL;/* undefined */
 
for (i = el-nr - 1; 0 = i; i--) {
struct exclude *x = el-excludes[i];
const char *name, *exclude = x-pattern;
-   int to_exclude = x-to_exclude;
int namelen, prefix = x-nowildcardlen;
 
if (x-flags  EXC_FLAG_MUSTBEDIR) {
@@ -538,14 +540,14 @@ int excluded_from_list(const char *pathname,
/* match basename */
if (prefix == x-patternlen) {
if (!strcmp_icase(exclude, basename))
-   return to_exclude;
+   return x;
} else if (x-flags  EXC_FLAG_ENDSWITH) {
if (x-patternlen - 1 = pathlen 
!strcmp_icase(exclude + 1, pathname + 
pathlen - x-patternlen + 1))
-   return to_exclude;
+   return x;
} else {
if (fnmatch_icase(exclude, basename, 0) == 0)
-   return to_exclude;
+   return x;
}
continue;
}
@@ -580,7 +582,23 @@ int excluded_from_list(const char *pathname,
}
 
if (!namelen || !fnmatch_icase(exclude, name, FNM_PATHNAME))
-   return to_exclude;
+   return x;
+   }
+   return NULL; /* undecided */
+}
+
+/*
+ * Scan the list and let the last match determine the fate.
+ * Return 1 for exclude, 0 for include and -1 for undecided.
+ */
+int excluded_from_list(const char *pathname,
+  int pathlen, const char *basename, int *dtype,
+  struct exclude_list *el)
+{
+   struct exclude *exclude;
+   exclude = excluded_from_list_1(pathname, pathlen, basename, dtype, el);
+   if (exclude) {
+   return exclude-to_exclude;
}
return -1; /* undecided */
 }
-- 
1.7.12.155.ge5750d5.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


[PATCH 5/9] Refactor excluded and path_excluded

2012-09-01 Thread Adam Spiers
In a similar way to the previous commit, this extracts new helper
functions excluded_1() and path_excluded_1() which return the last
exclude_list element which matched, or NULL if no match was found.
excluded() and path_excluded() become wrappers around these, and just
return 0 or 1 depending on whether any matching exclude_list element
was found.

This allows callers to find out _why_ a given path was excluded,
rather than just whether it was or not, paving the way for a new git
sub-command which allows users to test their exclude lists from the
command line.

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 dir.c | 81 +--
 dir.h |  3 +++
 2 files changed, 67 insertions(+), 17 deletions(-)

diff --git a/dir.c b/dir.c
index 3a532d5..3d438c3 100644
--- a/dir.c
+++ b/dir.c
@@ -603,23 +603,42 @@ int excluded_from_list(const char *pathname,
return -1; /* undecided */
 }
 
-static int excluded(struct dir_struct *dir, const char *pathname, int *dtype_p)
+/*
+ * Loads the exclude lists for the directory containing pathname, then
+ * scans all exclude lists to determine whether pathname is excluded.
+ * Returns the exclude_list element which matched, or NULL for
+ * undecided.
+ */
+static struct exclude *excluded_1(struct dir_struct *dir, const char 
*pathname, int *dtype_p)
 {
int pathlen = strlen(pathname);
int st;
+   struct exclude *exclude;
const char *basename = strrchr(pathname, '/');
basename = (basename) ? basename+1 : pathname;
 
prep_exclude(dir, pathname, basename-pathname);
for (st = EXC_CMDL; st = EXC_FILE; st++) {
-   switch (excluded_from_list(pathname, pathlen, basename,
-  dtype_p, dir-exclude_list[st])) {
-   case 0:
-   return 0;
-   case 1:
-   return 1;
+   exclude = excluded_from_list_1(pathname, pathlen, basename,
+  dtype_p, dir-exclude_list[st]);
+   if (exclude) {
+   return exclude;
}
}
+   return NULL;
+}
+
+/*
+ * Loads the exclude lists for the directory containing pathname, then
+ * scans all exclude lists to determine whether pathname is excluded.
+ * Returns 1 if true, otherwise 0.
+ */
+static int excluded(struct dir_struct *dir, const char *pathname, int *dtype_p)
+{
+   struct exclude *exclude = excluded_1(dir, pathname, dtype_p);
+   if (exclude) {
+   return exclude-to_exclude;
+   }
return 0;
 }
 
@@ -627,6 +646,7 @@ void path_exclude_check_init(struct path_exclude_check 
*check,
 struct dir_struct *dir)
 {
check-dir = dir;
+   check-exclude = NULL;
strbuf_init(check-path, 256);
 }
 
@@ -636,18 +656,20 @@ void path_exclude_check_clear(struct path_exclude_check 
*check)
 }
 
 /*
- * Is this name excluded?  This is for a caller like show_files() that
- * do not honor directory hierarchy and iterate through paths that are
- * possibly in an ignored directory.
+ * For each subdirectory in name, starting with the top-most, checks
+ * to see if that subdirectory is excluded, and if so, returns the
+ * corresponding exclude structure.  Otherwise, checks whether name
+ * itself (which is presumably a file) is excluded.
  *
  * A path to a directory known to be excluded is left in check-path to
  * optimize for repeated checks for files in the same excluded directory.
  */
-int path_excluded(struct path_exclude_check *check,
- const char *name, int namelen, int *dtype)
+struct exclude *path_excluded_1(struct path_exclude_check *check,
+   const char *name, int namelen, int *dtype)
 {
int i;
struct strbuf *path = check-path;
+   struct exclude *exclude;
 
/*
 * we allow the caller to pass namelen as an optimization; it
@@ -657,11 +679,18 @@ int path_excluded(struct path_exclude_check *check,
if (namelen  0)
namelen = strlen(name);
 
+   /*
+* If path is non-empty, and name is equal to path or a
+* subdirectory of path, name should be excluded, because
+* it's inside a directory which is already known to be
+* excluded and was previously left in check-path.
+*/
if (path-len 
path-len = namelen 
!memcmp(name, path-buf, path-len) 
-   (!name[path-len] || name[path-len] == '/'))
-   return 1;
+   (!name[path-len] || name[path-len] == '/')) {
+   return check-exclude;
+   }
 
strbuf_setlen(path, 0);
for (i = 0; name[i]; i++) {
@@ -669,8 +698,11 @@ int path_excluded(struct path_exclude_check *check,
 
if (ch == '/') {
int dt = DT_DIR;
-   if (excluded(check-dir, 

[PATCH 0/9] new git check-ignore sub-command

2012-09-01 Thread Adam Spiers
I was browsing stackoverflow the other day and came across this question:


http://stackoverflow.com/questions/12144633/which-gitignore-rule-is-ignoring-my-file/

A quick google revealed this thread from 2009:

http://thread.gmane.org/gmane.comp.version-control.git/108671/focus=108815

where Junio and Jeff discussed the possibility of adding a new `git
check-ignore' subcommand somewhat analogous to the existing `git
check-attr', and suggested the beginnings of an implementation.  It
struck me that it might not be too hard to follow these ideas to their
natural conclusion, so I decided it would make a fun project :-)

The following series of patches is the outcome.  I am completely new
to git hacking, so whilst I have tried very hard to follow all the
conventions and documented guidelines, please go easy on me if there
are any glaring errors ;-)  However, the added test suite should cover
the new code paths thoroughly, and I also ran check-ignore through
valgrind and made some improvements accordingly, so hopefully it's
pretty near the mark.

I have a question and some comments about my current patch series.

Firstly, I re-used pathspec-handling code from builtin/add.c, so I
moved it to a new pathspec.c file.  It looks like setup.c might have
been a better candidate, but that library is already a fairly large
collection of apparently loosely associated things, so I wasn't sure.
According to the comments, get_pathspec() is due to be superceded by
the struct pathspec interface, so perhaps it would make sense to
split setup.c up into pathspec.c and one or two other files so as to
move towards a clean demarcation of this new API?

Secondly, in the course of trying to understand the code base, my
little brain got confused and I noticed a few areas where I thought
there was potential to make things a bit clearer.  So some of my
commits are janitorial in nature.

Thirdly, currently the new sub-command hardly looks at the cache.
This is partially because it doesn't need to in the most common use
case (i.e. user is confused about why files are/aren't being ignored).
It's also because this whole project took a lot longer than I
expected, so I'm running out of time :-) Perhaps someone can add this
in the future if it's needed.  Right now the cache is only used to
prevent recursing into submodules.

Thanks,
Adam

Adam Spiers (9):
  Update directory listing API doc to match code
  Improve documentation and comments regarding directory traversal API
  Rename cryptic 'which' variable to more consistent name
  Refactor excluded_from_list
  Refactor excluded and path_excluded
  For each exclude pattern, store information about where it came from
  Extract some useful pathspec handling code from builtin/add.c into a
library
  Provide free_directory() for reclaiming dir_struct memory
  Add git-check-ignores

 .gitignore|   1 +
 Documentation/git-check-ignore.txt|  58 +
 Documentation/gitignore.txt   |   6 +-
 Documentation/technical/api-directory-listing.txt |  23 +-
 Makefile  |   3 +
 builtin.h |   1 +
 builtin/add.c |  84 +-
 builtin/check-ignore.c| 150 +++
 builtin/clean.c   |   2 +-
 builtin/ls-files.c|   3 +-
 command-list.txt  |   1 +
 contrib/completion/git-completion.bash|   1 +
 dir.c | 183 ++---
 dir.h |  37 ++-
 git.c |   1 +
 pathspec.c|  87 +++
 pathspec.h|   6 +
 t/t0007-ignores.sh| 301 ++
 18 files changed, 811 insertions(+), 137 deletions(-)
 create mode 100644 Documentation/git-check-ignore.txt
 create mode 100644 builtin/check-ignore.c
 create mode 100644 pathspec.c
 create mode 100644 pathspec.h
 create mode 100755 t/t0007-ignores.sh

-- 
1.7.12.155.ge5750d5.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


[PATCH 9/9] Add git-check-ignores

2012-09-01 Thread Adam Spiers
This works in a similar manner to git-check-attr.  Some code
was reused from add.c by refactoring out into pathspec.c.

Thanks to Jeff King and Junio C Hamano for the idea:
http://thread.gmane.org/gmane.comp.version-control.git/108671/focus=108815

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 .gitignore |   1 +
 Documentation/git-check-ignore.txt |  58 +++
 Documentation/gitignore.txt|   6 +-
 Makefile   |   1 +
 builtin.h  |   1 +
 builtin/add.c  |   2 +-
 builtin/check-ignore.c | 150 
 command-list.txt   |   1 +
 contrib/completion/git-completion.bash |   1 +
 git.c  |   1 +
 t/t0007-ignores.sh | 301 +
 11 files changed, 520 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/git-check-ignore.txt
 create mode 100644 builtin/check-ignore.c
 create mode 100755 t/t0007-ignores.sh

diff --git a/.gitignore b/.gitignore
index bb5c91e..0cbe94c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -22,6 +22,7 @@
 /git-bundle
 /git-cat-file
 /git-check-attr
+/git-check-ignore
 /git-check-ref-format
 /git-checkout
 /git-checkout-index
diff --git a/Documentation/git-check-ignore.txt 
b/Documentation/git-check-ignore.txt
new file mode 100644
index 000..3a85dbb
--- /dev/null
+++ b/Documentation/git-check-ignore.txt
@@ -0,0 +1,58 @@
+git-check-ignore(1)
+=
+
+NAME
+
+git-check-ignore - Debug gitignore / exclude files
+
+
+SYNOPSIS
+
+[verse]
+'git check-ignore' pathname...
+'git check-ignore' --stdin [-z]  list-of-paths
+
+DESCRIPTION
+---
+
+For each pathname given via the command-line or from a file via
+`--stdin`, this command will list the first exclude pattern found (if
+any) which explicitly excludes or includes that pathname.  Note that
+within any given exclude file, later patterns take precedence over
+earlier ones, so any matching pattern which this command outputs may
+not be the one you would immediately expect.
+
+OPTIONS
+---
+--stdin::
+   Read file names from stdin instead of from the command-line.
+
+-z::
+   Only meaningful with `--stdin`; paths are separated with a
+   NUL character instead of a linefeed character.
+
+OUTPUT
+--
+
+The output is a series of lines of the form:
+
+path COLON SP type SP pattern SP source SP position LF
+
+path is the path of a file being queried, type is either
+'excluded' or 'included' (for patterns prefixed with '!'), pattern
+is the matching pattern, source is the pattern's source file (either
+as an absolute path or relative to the repository root), and
+position is the position of the pattern within that source.
+
+If no pattern matches a given path, nothing will be output for that
+path.
+
+SEE ALSO
+
+linkgit:gitignore[5]
+linkgit:gitconfig[5]
+linkgit:git-ls-files[5]
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index c1f692a..7ba16fe 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -155,8 +155,10 @@ The second .gitignore prevents git from ignoring
 
 SEE ALSO
 
-linkgit:git-rm[1], linkgit:git-update-index[1],
-linkgit:gitrepository-layout[5]
+linkgit:git-rm[1],
+linkgit:git-update-index[1],
+linkgit:gitrepository-layout[5],
+linkgit:git-check-ignore[1]
 
 GIT
 ---
diff --git a/Makefile b/Makefile
index 272ab69..ce5e42f 100644
--- a/Makefile
+++ b/Makefile
@@ -825,6 +825,7 @@ BUILTIN_OBJS += builtin/branch.o
 BUILTIN_OBJS += builtin/bundle.o
 BUILTIN_OBJS += builtin/cat-file.o
 BUILTIN_OBJS += builtin/check-attr.o
+BUILTIN_OBJS += builtin/check-ignore.o
 BUILTIN_OBJS += builtin/check-ref-format.o
 BUILTIN_OBJS += builtin/checkout-index.o
 BUILTIN_OBJS += builtin/checkout.o
diff --git a/builtin.h b/builtin.h
index 8e37752..f812c97 100644
--- a/builtin.h
+++ b/builtin.h
@@ -57,6 +57,7 @@ extern int cmd_cat_file(int argc, const char **argv, const 
char *prefix);
 extern int cmd_checkout(int argc, const char **argv, const char *prefix);
 extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);
 extern int cmd_check_attr(int argc, const char **argv, const char *prefix);
+extern int cmd_check_ignore(int argc, const char **argv, const char *prefix);
 extern int cmd_check_ref_format(int argc, const char **argv, const char 
*prefix);
 extern int cmd_cherry(int argc, const char **argv, const char *prefix);
 extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);
diff --git a/builtin/add.c b/builtin/add.c
index a7ed2ad..af68c32 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -273,7 +273,7 @@ static int add_files(struct dir_struct *dir, int flags)
fprintf(stderr, _(ignore_error));
for (i = 0; i  dir-ignored_nr; i++)
fprintf(stderr, 

[PATCH 1/9] Update directory listing API doc to match code

2012-09-01 Thread Adam Spiers
7c4c97c0ac turned the flags in struct dir_struct into a single bitfield
variable, but forgot to update this document.

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 Documentation/technical/api-directory-listing.txt | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/technical/api-directory-listing.txt 
b/Documentation/technical/api-directory-listing.txt
index add6f43..0356d25 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -17,24 +17,24 @@ options are:
The name of the file to be read in each directory for excluded
files (typically `.gitignore`).
 
-`collect_ignored`::
+`flags`::
 
-   Include paths that are to be excluded in the result.
+   A bit-field of options:
 
-`show_ignored`::
+`DIR_SHOW_IGNORED`:::
 
The traversal is for finding just ignored files, not unignored
files.
 
-`show_other_directories`::
+`DIR_SHOW_OTHER_DIRECTORIES`:::
 
Include a directory that is not tracked.
 
-`hide_empty_directories`::
+`DIR_HIDE_EMPTY_DIRECTORIES`:::
 
Do not include a directory that is not tracked and is empty.
 
-`no_gitlinks`::
+`DIR_NO_GITLINKS`:::
 
If set, recurse into a directory that looks like a git
directory.  Otherwise it is shown as a directory.
-- 
1.7.12.155.ge5750d5.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


Re: Aw: Re: git blame shows wrong Not commited yet entries

2012-09-01 Thread Martin von Zweigbergk
On Fri, Aug 31, 2012 at 10:58 AM, Junio C Hamano gits...@pobox.com wrote:

 And git blame $path probably should expect $path is something that
 appear in the tree of HEAD; apparently it does not.

That probably makes sense. For anyone deciding to implement that, note
that git blame -C [-C [-C]] $path should probably not expect the
same, so the following still works.

cp COPYING COPY
git blame -C -C -C COPY

Btw, why isn't -C -C -C the same as -CCC? Should it?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html