[PATCH] gitweb: fix error when highlight is enabled

2012-12-26 Thread Orgad Shaneh
Use of uninitialized value in substitution iterator at gitweb.cgi line 1560

Signed-off-by: Orgad Shaneh org...@gmail.com
---
 gitweb/gitweb.perl |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0f207f2..862b9cd 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1556,7 +1556,7 @@ sub sanitize {
return undef unless defined $str;
 
$str = to_utf8($str);
-   $str =~ s|([[:cntrl:]])|($1 =~ /[\t\n\r]/ ? $1 : quot_cec($1))|eg;
+   $str =~ s|([[:cntrl:]])|($1 =~ /([\t\n\r])/ ? $1 : quot_cec($1))|eg;
return $str;
 }
 
-- 
1.7.10.4

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


Re: [PATCH] refs: do not use cached refs in repack_without_ref

2012-12-26 Thread Michael Haggerty
On 12/21/2012 09:04 AM, Jeff King wrote:
 When we delete a ref that is packed, we rewrite the whole
 packed-refs file and simply omit the ref that no longer
 exists. However, we base the rewrite on whatever happens to
 be in our refs cache, not what is necessarily on disk. That
 opens us up to a race condition if another process is
 simultaneously packing the refs, as we will overwrite their
 newly-made pack-refs file with our potentially stale data,
 losing commits.
 [...]
 
 There are a few other interesting races in this code that this does not
 fix:
 
   1. We check to see whether the ref is packed based on the cached data.
  That means that in the following sequence:
 
a. receive-pack starts, caches packed refs; master is not packed
 
b. meanwhile, pack-refs runs and packs master
 
c. receive-pack deletes the loose ref for master (which might be
   a no-op if the pack-refs from (b) got there first). It checks
   its cached packed-refs and sees that there is nothing to
   delete.
 
  We end up leaving the entry in packed-refs. In other words, the
  deletion does not actually delete anything, but it still returns
  success.
 
  We could fix this by scanning the list of packed refs only after
  we've acquired the lock. The downside is that this would increase
  lock contention on packed-refs.lock. Right now, two deletions may
  conflict if they are deletions of packed refs. With this change,
  any two deletions might conflict, packed or not.
 
  If we work under the assumption that deletions are relatively rare,
  this is probably OK. And if you tend to keep your refs packed, it
  would not make any difference. It would have an impact on repos
  which do not pack refs, and which have frequent simultaneous
  deletions.
 
   2. The delete_ref function first deletes the loose ref, then rewrites
  the packed-refs file. This means that for a moment, the ref may
  appear to have rewound to whatever was in the packed-refs file, and
  the reader has no way of knowing.
 
  This is not a huge deal, but I think it could be fixed by swapping
  the ordering. However, I think that would open us up to the reverse
  race from above: we delete from packed-refs, then before we delete
  the loose ref, a pack-refs process repacks it. Our deletion looks
  successful, but the ref remains afterwards.

I'm sorry to take so long to respond to this patch.  Thank you for
tracking down this bug and for your careful analysis.

I think your patch is correct and should fix the first race condition
that you described.  But I think the continued existence of the other
race conditions is an indication of a fundamental problem with the
reference locking policy--independent of the in-RAM reference cache.

The tacit assumption of the current locking policy is that changes to
the packed-refs file are not critical for correctness, because loose
references take precedence over it anyway.  This is true for adding and
modifying references.  But it is not true for deleting references,
because there is no way for a deletion to be represented as a loose
reference in a way that takes precedence over the packed-refs file
(i.e., there is no way for a loose reference to say I am deleted,
regardless of what packed-refs says).  Thus the race conditions for
deleting references, whether via delete_ref() or via pack_refs() with
--prune.

The current algorithms for deleting references are:

* Delete reference foo:

  1. Acquire the lock $GIT_DIR/refs/heads/foo.lock

  2. Unlink $GIT_DIR/refs/heads/foo

  3. repack_without_ref():

 a. Acquire the lock $GIT_DIR/packed-refs.lock

 b. Write the packed-refs without the foo reference to
$GIT_DIR/packed-refs.lock

 c. Rename $GIT_DIR/packed-refs.lock to $GIT_DIR/packed-refs

  4. Release lock $GIT_DIR/refs/heads/foo.lock

* Pack references:

  1. Acquire lock $GIT_DIR/packed-refs.lock

  2. for_each_ref() call handle_one_ref():

 a. Write ref and its SHA1 to $GIT_DIR/packed-refs.lock

 b. Possibly record ref and its SHA1 in the refs_to_prune list.

  3. Commit $GIT_DIR/packed-refs

  4. prune_refs(): for each ref in the ref_to_prune list, call
 prune_ref():

 a. Lock the reference using lock_ref_sha1(), verifying that the
recorded SHA1 is still valid.  If it is, unlink the loose
reference file then free the lock; otherwise leave the loose
reference file untouched.

There is a problem if two processes try to delete a reference at the
same time, or if one process tries to delete a reference at the same
time as another process is trying to pack the references.  The reason is
that there is no transaction that spans both the rewriting of the
packed-refs file and also the deletion of the loose-ref files, and
therefore it is possible for conflicting changes to be made in the two
locations.

I think that all of the problems would 

Re: [PATCH] gitweb: fix error when highlight is enabled

2012-12-26 Thread Junio C Hamano
Orgad Shaneh org...@gmail.com writes:

 Use of uninitialized value in substitution iterator at gitweb.cgi line 1560

This is not just about squelching an error message, but more
importantly, attempting to fix an information lossage, no?

The statement captures each control character in the string to $1,
then matches a class of known/safe control chars against that
control character we just have seen. If matches, it just wants to
use that control character, otherwise it wants to apply quot_cec()
on that control character.  It forgets that $1 is reset
immediately when =~ matches with the class of known/safe control
chars, and your version attempts to fix it by recapturing it.

What if you are looking at a non-safe control, say \001?  It is
matched and is captured by ([[;cntrl:]]), making $1 -eq \001, and
then the replacement side of s///e operator, tries to match and
capture it with ([\t\n\r]), but it does *not* match.

What does that $1 you are feeding quot_cec() contain at that
point?  I _think_ $1 is left intact when the inner match fails and
you are correctly feeding \001 to quot_cec(), but it is not
immediately obvious.  Perl regexp, especially s///e, is a yucky
language X-.

I wonder if there is a better way to express what goes inside the
replacement side of this s///e construct in a more obvious way. The
updated one may be correct but it looks too subtle to my taste..

 Signed-off-by: Orgad Shaneh org...@gmail.com
 ---
  gitweb/gitweb.perl |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
 index 0f207f2..862b9cd 100755
 --- a/gitweb/gitweb.perl
 +++ b/gitweb/gitweb.perl
 @@ -1556,7 +1556,7 @@ sub sanitize {
   return undef unless defined $str;
  
   $str = to_utf8($str);
 - $str =~ s|([[:cntrl:]])|($1 =~ /[\t\n\r]/ ? $1 : quot_cec($1))|eg;
 + $str =~ s|([[:cntrl:]])|($1 =~ /([\t\n\r])/ ? $1 : quot_cec($1))|eg;
   return $str;
  }
--
To unsubscribe from this list: send the line 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] wt-status: Show ignored files in untracked dirs

2012-12-26 Thread Antoine Pelisse
When looking for ignored files, we do not recurse into untracked
directory, and simply consider the directory ignored status.
As a consequence, we don't see ignored files in those directories.

Change that behavior by recursing into untracked directories searching
for ignored files.

Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
I jumped in.

This seems to be broken since the creation of the --ignored option to
wt-status.

This fixes the issue and breaks none of the existing tests.
The behavior seems sane to me, giving something like that:

  ?? .gitignore
  ?? x
  ?? y/foo
  !! x.ignore-me
  !! y/foo.ignore-me

Cheers,
Antoine

 wt-status.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/wt-status.c b/wt-status.c
index 2a9658b..7c41488 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -516,7 +516,7 @@ static void wt_status_collect_untracked(struct wt_status *s)

if (s-show_ignored_files) {
dir.nr = 0;
-   dir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;
+   dir.flags = DIR_SHOW_IGNORED;
fill_directory(dir, s-pathspec);
for (i = 0; i  dir.nr; i++) {
struct dir_entry *ent = dir.entries[i];
--
1.8.1.rc3.11.g86c3e6e.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: git-prompt.sh vs leading white space in __git_ps1()::printf_format

2012-12-26 Thread Simon Oosthoek
* Junio C Hamano gits...@pobox.com [2012-12-25 23:47:53 -0800]:
 
 Can we make it take an optional third parameter so that we could say
 
   PROMPT_COMMAND='__git_ps1 : \h \W ;  /%s'
 
 to do the same as what the command substitution mode would have
 given for
 
   PS1=': \h \W$(__git_ps1 /%s); '
 
 perhaps?
 
 Totally untested, but perhaps along this line.
 

I tried your patch and (to my surprise, after the first reading) it worked.

I've further modified git-prompt.sh to include more usage text and I changed
the name of ps1 to gitstring, as it might be confused with PS1 upon casual
reading.

I'll be sending a format-patch patchmail ASAP...


  contrib/completion/git-prompt.sh | 24 ++--
  1 file changed, 14 insertions(+), 10 deletions(-)
 
 diff --git a/contrib/completion/git-prompt.sh 
 b/contrib/completion/git-prompt.sh
 index 9b074e1..b2579f4 100644
 --- a/contrib/completion/git-prompt.sh
 +++ b/contrib/completion/git-prompt.sh
 @@ -236,9 +236,10 @@ __git_ps1 ()
   local printf_format=' (%s)'
  
   case $# in
 - 2)  pcmode=yes
 + 2|3)pcmode=yes
   ps1pc_start=$1
   ps1pc_end=$2
 + printf_format=${3:-$printf_format}
   ;;
   0|1)printf_format=${1:-$printf_format}
   ;;
 @@ -339,6 +340,7 @@ __git_ps1 ()
  
   local f=$w$i$s$u
   if [ $pcmode = yes ]; then
 + local ps1=
   if [ -n ${GIT_PS1_SHOWCOLORHINTS-} ]; then
   local c_red='\e[31m'
   local c_green='\e[32m'
 @@ -356,29 +358,31 @@ __git_ps1 ()
   branch_color=$bad_color
   fi
  
 - # Setting PS1 directly with \[ and \] around 
 colors
 + # Setting ps1 directly with \[ and \] around 
 colors
   # is necessary to prevent wrapping issues!
 - PS1=$ps1pc_start 
 (\[$branch_color\]$branchstring\[$c_clear\]
 + ps1=\[$branch_color\]$branchstring\[$c_clear\]
  
   if [ -n $w$i$s$u$r$p ]; then
 - PS1=$PS1 
 + ps1=$ps1  
 
   fi
   if [ $w = * ]; then
 - PS1=$PS1\[$bad_color\]$w
 + ps1=$ps1\[$bad_color\]$w
   fi
   if [ -n $i ]; then
 - PS1=$PS1\[$ok_color\]$i
 + ps1=$ps1\[$ok_color\]$i
   fi
   if [ -n $s ]; then
 - PS1=$PS1\[$flags_color\]$s
 + ps1=$ps1\[$flags_color\]$s
   fi
   if [ -n $u ]; then
 - PS1=$PS1\[$bad_color\]$u
 + ps1=$ps1\[$bad_color\]$u
   fi
 - PS1=$PS1\[$c_clear\]$r$p)$ps1pc_end
 + ps1=$ps1\[$c_clear\]$r$p
   else
 - PS1=$ps1pc_start ($c${b##refs/heads/}${f:+ 
 $f}$r$p)$ps1pc_end
 + ps1=$c${b##refs/heads/}${f:+ $f}$r$p
   fi
 + ps1=$(printf -- $printf_format $ps1)
 + PS1=$ps1pc_start$ps1$ps1pc_end
   else
   # NO color option unless in PROMPT_COMMAND mode
   printf -- $printf_format $c${b##refs/heads/}${f:+ 
 $f}$r$p

/Simon
--
To unsubscribe from this list: send the line 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] wt-status: Show ignored files in untracked dirs

2012-12-26 Thread Antoine Pelisse
When looking for ignored files, we do not recurse into untracked
directory, and simply consider the directory ignored status.
As a consequence, we don't see ignored files in those directories.

Change that behavior by recursing into untracked directories, if not
ignored themselves, searching for ignored files.

Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
Actually, the previous patch breaks the case where the directory is ignored.
This one should fix both issues.
Let me know if you see any other use case that could be an issue.

 dir.c   | 7 +++
 wt-status.c | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/dir.c b/dir.c
index 5a83aa7..2863799 100644
--- a/dir.c
+++ b/dir.c
@@ -1042,6 +1042,13 @@ static enum path_treatment treat_one_path(struct 
dir_struct *dir,
return path_ignored;
}

+   /*
+* Don't recurse into ignored directories when looking for
+* ignored files, but still show the directory as ignored.
+*/
+   if (exclude  (dir-flags  DIR_SHOW_IGNORED)  dtype == DT_DIR)
+   return path_handled;
+
switch (dtype) {
default:
return path_ignored;
diff --git a/wt-status.c b/wt-status.c
index 2a9658b..7c41488 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -516,7 +516,7 @@ static void wt_status_collect_untracked(struct wt_status *s)

if (s-show_ignored_files) {
dir.nr = 0;
-   dir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;
+   dir.flags = DIR_SHOW_IGNORED;
fill_directory(dir, s-pathspec);
for (i = 0; i  dir.nr; i++) {
struct dir_entry *ent = dir.entries[i];
--
1.8.1.rc3.12.g8864e38

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


Re: [PATCH v2 10/14] For each exclude pattern, store information about where it came from

2012-12-26 Thread Adam Spiers
On Thu, Sep 20, 2012 at 02:31:57PM -0700, Junio C Hamano wrote:
 Adam Spiers g...@adamspiers.org writes:

   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;

 Hrm, don't all elements x in el share the same src, even if
 their srcpos may be different?

No not currently - please see the other mail I just sent to the
[PATCH v2 00/14] thread.
--
To unsubscribe from this list: send the line 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] log: add log.mailmap configuration option

2012-12-26 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Antoine Pelisse apeli...@gmail.com writes:

 I'm wondering if it would be needed to add a no-use-mailmap option to
 log command so that it can cancel this configuration option.

 The usual way for adding a new feature is to add a --enable-feature
 long-option without any configuration variable to let users try it
 out in the field, and then add the configuration to let it be
 default for users who opt in.  The first step should also allow a
 command line option to disable (which should come for free if you
 use parse-options API correctly).

It should be sufficient to squash something like this in.  Use the
configured value, if available, to initialize the existing mailmap
variable, which is in turn updated from the command line option with
either --use-mailmap or --no-use-mailmap.  What is left in mailmap
after the command line parsing returns is what the user told us to
use.

Thanks.

 builtin/log.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index f6936ff..16e6520 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -31,7 +31,7 @@ static int default_abbrev_commit;
 static int default_show_root = 1;
 static int decoration_style;
 static int decoration_given;
-static int use_mailmap;
+static int use_mailmap_config;
 static const char *fmt_patch_subject_prefix = PATCH;
 static const char *fmt_pretty;
 
@@ -107,6 +107,7 @@ static void cmd_log_init_finish(int argc, const char 
**argv, const char *prefix,
OPT_END()
};
 
+   mailmap = use_mailmap_config;
argc = parse_options(argc, argv, prefix,
 builtin_log_options, builtin_log_usage,
 PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
@@ -139,7 +140,7 @@ static void cmd_log_init_finish(int argc, const char 
**argv, const char *prefix,
if (source)
rev-show_source = 1;
 
-   if (mailmap || use_mailmap) {
+   if (mailmap) {
rev-mailmap = xcalloc(1, sizeof(struct string_list));
read_mailmap(rev-mailmap, NULL);
}
@@ -360,7 +361,7 @@ static int git_log_config(const char *var, const char 
*value, void *cb)
if (!prefixcmp(var, color.decorate.))
return parse_decorate_color_config(var, 15, value);
if (!strcmp(var, log.mailmap)) {
-   use_mailmap = git_config_bool(var, value);
+   use_mailmap_config = git_config_bool(var, value);
return 0;
}
 
--
To unsubscribe from this list: send the line 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] log: add log.mailmap configuration option

2012-12-26 Thread Antoine Pelisse
I was planning to send you a fix pretty close to that,

Thanks a lot Junio!

On Wed, Dec 26, 2012 at 5:14 PM, Junio C Hamano gits...@pobox.com wrote:
 Junio C Hamano gits...@pobox.com writes:

 Antoine Pelisse apeli...@gmail.com writes:

 I'm wondering if it would be needed to add a no-use-mailmap option to
 log command so that it can cancel this configuration option.

 The usual way for adding a new feature is to add a --enable-feature
 long-option without any configuration variable to let users try it
 out in the field, and then add the configuration to let it be
 default for users who opt in.  The first step should also allow a
 command line option to disable (which should come for free if you
 use parse-options API correctly).

 It should be sufficient to squash something like this in.  Use the
 configured value, if available, to initialize the existing mailmap
 variable, which is in turn updated from the command line option with
 either --use-mailmap or --no-use-mailmap.  What is left in mailmap
 after the command line parsing returns is what the user told us to
 use.

 Thanks.

  builtin/log.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

 diff --git a/builtin/log.c b/builtin/log.c
 index f6936ff..16e6520 100644
 --- a/builtin/log.c
 +++ b/builtin/log.c
 @@ -31,7 +31,7 @@ static int default_abbrev_commit;
  static int default_show_root = 1;
  static int decoration_style;
  static int decoration_given;
 -static int use_mailmap;
 +static int use_mailmap_config;
  static const char *fmt_patch_subject_prefix = PATCH;
  static const char *fmt_pretty;

 @@ -107,6 +107,7 @@ static void cmd_log_init_finish(int argc, const char 
 **argv, const char *prefix,
 OPT_END()
 };

 +   mailmap = use_mailmap_config;
 argc = parse_options(argc, argv, prefix,
  builtin_log_options, builtin_log_usage,
  PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
 @@ -139,7 +140,7 @@ static void cmd_log_init_finish(int argc, const char 
 **argv, const char *prefix,
 if (source)
 rev-show_source = 1;

 -   if (mailmap || use_mailmap) {
 +   if (mailmap) {
 rev-mailmap = xcalloc(1, sizeof(struct string_list));
 read_mailmap(rev-mailmap, NULL);
 }
 @@ -360,7 +361,7 @@ static int git_log_config(const char *var, const char 
 *value, void *cb)
 if (!prefixcmp(var, color.decorate.))
 return parse_decorate_color_config(var, 15, value);
 if (!strcmp(var, log.mailmap)) {
 -   use_mailmap = git_config_bool(var, value);
 +   use_mailmap_config = git_config_bool(var, value);
 return 0;
 }

--
To unsubscribe from this list: send the line 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/8] wildmatch: rename constants and update prototype

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

 @@ -134,101 +131,102 @@ static int dowild(const uchar *p, const uchar *text, 
 int force_lower_case)
   p_ch = NEGATE_CLASS;
  #endif
   /* Assign literal TRUE/FALSE because of matched 
 comparison. */
 - special = p_ch == NEGATE_CLASS? TRUE : FALSE;
 + special = p_ch == NEGATE_CLASS;

Leftover comment needs to be reworded as well???

   if (special) {
   /* Inverted character class. */
   p_ch = *++p;
   }

I'd prefer special used in the case '[' given a more sensible
name here.

--
To unsubscribe from this list: send the line 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] make __git_ps1 accept a third parameter in pcmode

2012-12-26 Thread Simon Oosthoek
The optional third parameter when __git_ps1 is used in
PROMPT_COMMAND mode as format string for printf to further
customize the way the git status string is embedded in the
user's PS1 prompt.

Signed-off-by: Simon Oosthoek s.oosth...@xs4all.nl
---
 contrib/completion/git-prompt.sh |   32 
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 9b074e1..2922bb3 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -24,6 +24,8 @@
 #will show username, at-sign, host, colon, cwd, then
 #various status string, followed by dollar and SP, as
 #your prompt.
+#Optionally, you can supply a third argument with a printf
+#format string to finetune the output of the branch status
 #
 # The argument to __git_ps1 will be displayed only if you are currently
 # in a git repository.  The %s token will be the name of the current
@@ -222,10 +224,12 @@ __git_ps1_show_upstream ()
 # when called from PS1 using command substitution
 # in this mode it prints text to add to bash PS1 prompt (includes branch name)
 #
-# __git_ps1 requires 2 arguments when called from PROMPT_COMMAND (pc)
+# __git_ps1 requires 2 or 3 arguments when called from PROMPT_COMMAND (pc)
 # in that case it _sets_ PS1. The arguments are parts of a PS1 string.
-# when both arguments are given, the first is prepended and the second appended
+# when two arguments are given, the first is prepended and the second appended
 # to the state string when assigned to PS1.
+# The optional third parameter will be used as printf format string to further
+# customize the output of the git-status string.
 # In this mode you can request colored hints using GIT_PS1_SHOWCOLORHINTS=true
 __git_ps1 ()
 {
@@ -236,9 +240,10 @@ __git_ps1 ()
local printf_format=' (%s)'
 
case $# in
-   2)  pcmode=yes
+   2|3)pcmode=yes
ps1pc_start=$1
ps1pc_end=$2
+   printf_format=${3:-$printf_format}
;;
0|1)printf_format=${1:-$printf_format}
;;
@@ -339,6 +344,7 @@ __git_ps1 ()
 
local f=$w$i$s$u
if [ $pcmode = yes ]; then
+   local gitstring=
if [ -n ${GIT_PS1_SHOWCOLORHINTS-} ]; then
local c_red='\e[31m'
local c_green='\e[32m'
@@ -356,29 +362,31 @@ __git_ps1 ()
branch_color=$bad_color
fi
 
-   # Setting PS1 directly with \[ and \] around 
colors
+   # Setting gitstring directly with \[ and \] 
around colors
# is necessary to prevent wrapping issues!
-   PS1=$ps1pc_start 
(\[$branch_color\]$branchstring\[$c_clear\]
+   
gitstring=\[$branch_color\]$branchstring\[$c_clear\]
 
if [ -n $w$i$s$u$r$p ]; then
-   PS1=$PS1 
+   gitstring=$gitstring  

fi
if [ $w = * ]; then
-   PS1=$PS1\[$bad_color\]$w
+   gitstring=$gitstring\[$bad_color\]$w
fi
if [ -n $i ]; then
-   PS1=$PS1\[$ok_color\]$i
+   gitstring=$gitstring\[$ok_color\]$i
fi
if [ -n $s ]; then
-   PS1=$PS1\[$flags_color\]$s
+   gitstring=$gitstring\[$flags_color\]$s
fi
if [ -n $u ]; then
-   PS1=$PS1\[$bad_color\]$u
+   gitstring=$gitstring\[$bad_color\]$u
fi
-   PS1=$PS1\[$c_clear\]$r$p)$ps1pc_end
+   gitstring=$gitstring\[$c_clear\]$r$p
else
-   PS1=$ps1pc_start ($c${b##refs/heads/}${f:+ 
$f}$r$p)$ps1pc_end
+   gitstring=$c${b##refs/heads/}${f:+ $f}$r$p
fi
+   gitstring=$(printf -- $printf_format $gitstring)
+   PS1=$ps1pc_start$gitstring$ps1pc_end
else
# NO color option unless in PROMPT_COMMAND mode
printf -- $printf_format $c${b##refs/heads/}${f:+ 
$f}$r$p
-- 
1.7.9.5

PS, I was surprised 

Re: [PATCH 1/2] log: grep author/committer using mailmap

2012-12-26 Thread Junio C Hamano
Antoine Pelisse apeli...@gmail.com writes:

 Currently mailmap can be used to display log authors and committers
 but there no way to use mailmap to find commits with mapped values.

 This commit allows those commands to work:

 git log --use-mailmap --author mapped_name_or_email
 git log --use-mailmap --committer mapped_name_or_email

 Of course it only works if the --use-mailmap option is used.

 Signed-off-by: Antoine Pelisse apeli...@gmail.com
 ---
 I probably missed something but I didn't find the connection with
 commit 2d10c55. Let me know if I went the wrong direction.

  revision.c |   53 
 
  t/t4203-mailmap.sh |   18 ++
  2 files changed, 71 insertions(+)

 diff --git a/revision.c b/revision.c
 index 95d21e6..fb9fd41 100644
 --- a/revision.c
 +++ b/revision.c
 @@ -13,6 +13,7 @@
  #include decorate.h
  #include log-tree.h
  #include string-list.h
 +#include mailmap.h

  volatile show_early_output_fn_t show_early_output;

 @@ -2219,6 +2220,50 @@ static int rewrite_parents(struct rev_info *revs, 
 struct commit *commit)
   return 0;
  }

 +static int commit_rewrite_authors(struct strbuf *buf, const char *what, 
 struct string_list *mailmap)
 +{
 + char *author, *endp;
 + size_t len;
 + struct strbuf name = STRBUF_INIT;
 + struct strbuf mail = STRBUF_INIT;
 + struct ident_split ident;
 +
 + author = strstr(buf-buf, what);
 + if (!author)
 + goto error;

This does not stop at the end of the header part and would match a
random line in the log message that happens to begin with author ;
is this something we would worry about, or would we leave it to fsck?

 + author += strlen(what);
 + endp = strstr(author, \n);

Using strchr(author, '\n') would feel more natural.  Also rename
author to person or something, as you would be using this
function for the committer information as well?

 + if (!endp)
 + goto error;
 +
 + len = endp - author;
 +
 + if (split_ident_line(ident, author, len)) {
 + error:
 + strbuf_release(name);
 + strbuf_release(mail);
 +
 + return 1;

We usually signal error by returning a negative integer.  It does
not matter too much in this case as no callers seem to check the
return value from this function, though.

 + }
 +
 + strbuf_add(name, ident.name_begin, ident.name_end - ident.name_begin);
 + strbuf_add(mail, ident.mail_begin, ident.mail_end - ident.mail_begin);
 +
 + map_user(mailmap, mail, name);
 +
 + strbuf_addf(name,  %s, mail.buf);
 +
 + strbuf_splice(buf, ident.name_begin - buf-buf,
 +   ident.mail_end - ident.name_begin + 1,
 +   name.buf, name.len);

Would it give us better performance if we splice only when
map_user() tells us that we actually rewrote the ident?

 + strbuf_release(name);
 + strbuf_release(mail);
 +
 + return 0;
 +}
 +
  static int commit_match(struct commit *commit, struct rev_info *opt)
  {
   int retval;
 @@ -2237,6 +2282,14 @@ static int commit_match(struct commit *commit, struct 
 rev_info *opt)
   if (buf.len)
   strbuf_addstr(buf, commit-buffer);

 + if (opt-mailmap) {
 + if (!buf.len)
 + strbuf_addstr(buf, commit-buffer);
 +
 + commit_rewrite_authors(buf, \nauthor , opt-mailmap);
 + commit_rewrite_authors(buf, \ncommitter , opt-mailmap);
 + }
 +
   /* Append fake message parts as needed */
   if (opt-show_notes) {
   if (!buf.len)
 diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
 index db043dc..e16187f 100755
 --- a/t/t4203-mailmap.sh
 +++ b/t/t4203-mailmap.sh
 @@ -248,11 +248,29 @@ Author: Other Author ot...@author.xx
  Author: Some Dude s...@dude.xx
  Author: A U Thor aut...@example.com
  EOF
 +
  test_expect_success 'Log output with --use-mailmap' '
   git log --use-mailmap | grep Author actual 
   test_cmp expect actual
  '

 +cat expect \EOF
 +Author: Santa Claus santa.cl...@northpole.xx
 +Author: Santa Claus santa.cl...@northpole.xx
 +EOF
 +
 +test_expect_success 'Grep author with --use-mailmap' '
 + git log --use-mailmap --author Santa | grep Author actual 
 + test_cmp expect actual
 +'
 +
 +expect
 +
 +test_expect_success 'Only grep replaced author with --use-mailmap' '
 + git log --use-mailmap --author c...@coompany.xx actual 
 + test_cmp expect actual
 +'
 +
  # git blame
  cat expect \EOF
  ^OBJI (A U Thor DATE 1) one
 --
 1.7.9.5
--
To unsubscribe from this list: send the line 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] make __git_ps1 accept a third parameter in pcmode

2012-12-26 Thread Junio C Hamano
Simon Oosthoek s.oosth...@xs4all.nl writes:

 The optional third parameter when __git_ps1 is used in
 PROMPT_COMMAND mode as format string for printf to further
 customize the way the git status string is embedded in the
 user's PS1 prompt.

 Signed-off-by: Simon Oosthoek s.oosth...@xs4all.nl
 ---

Thanks.

If we do not care about the existing users (and in this case,
because PROMPT_COMMAND mode is in no released version, we could
declare there is no existing user), another and simpler approach is
to just drop  ( and ) altogether and have the user give these as
part of the pre/post strings.

Or we could go the other way and drop pre/post strings, making
them part of the printf_format string.  Perhaps that might be a
better interface in the longer term.  Then people can use the same
pre%spost format string and do either of these:

PS1=$(__git_ps1 pre%spost)
PROMPT_COMMAND='PS1=$(__git_ps1 pre%spost)'

without __git_ps1 having a special prompt command mode, no?

I have a feeling that I am missing something major, though...

   if [ $w = * ]; then
 - PS1=$PS1\[$bad_color\]$w
 + gitstring=$gitstring\[$bad_color\]$w
   fi

Every time I looked at this line, I wondered why '*' state is
bad.  Does a user go into any bad state by having a dirty
working tree?  Same for untracked ($u) and detached.  These are all
perfectly normal part of a workflow, so while choice of red may be
fine to attract attention, calling it bad sounds misguided.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


generating format-patch options from an e-mail

2012-12-26 Thread Simon Oosthoek
Hi all

I've been very frustrated by the process to setup a commandline for git 
format-patch, to include everyone in the cc list and reply to the right 
message-id.

In my frustration I created a perl script to generate the options from a saved 
e-mail, I realise that it may be non-general and perhaps it could be written 
better using a module which understands e-mails, but well, it worked for me ;-)

Anyway, I could imagine this as optional flag of git format-patch, so you could 
say:
$ git format-patch -s --in-reply-to-email mboxfile a7fe7de8

But I'll save that as an exercise for the reader (or the future)

Cheers

Simon

--
To unsubscribe from this list: send the line 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] make __git_ps1 accept a third parameter in pcmode

2012-12-26 Thread Simon Oosthoek
* Junio C Hamano gits...@pobox.com [2012-12-26 11:45:48 -0800]:

 Simon Oosthoek s.oosth...@xs4all.nl writes:
 
  The optional third parameter when __git_ps1 is used in
  PROMPT_COMMAND mode as format string for printf to further
  customize the way the git status string is embedded in the
  user's PS1 prompt.
 
  Signed-off-by: Simon Oosthoek s.oosth...@xs4all.nl
  ---
 
 Thanks.
 
 If we do not care about the existing users (and in this case,
 because PROMPT_COMMAND mode is in no released version, we could
 declare there is no existing user), another and simpler approach is
 to just drop  ( and ) altogether and have the user give these as
 part of the pre/post strings.
 

The problem with doing it in pre-post is when inside non-git directories. You 
want to avoid any gitstring output, including the brackets, when not inside a 
repository.

Doing it all in the third parameter is perhaps a better approach, but then it 
becomes mandatory instead of optional.

 Or we could go the other way and drop pre/post strings, making
 them part of the printf_format string.  Perhaps that might be a
 better interface in the longer term.  Then people can use the same
 pre%spost format string and do either of these:
 
   PS1=$(__git_ps1 pre%spost)
   PROMPT_COMMAND='PS1=$(__git_ps1 pre%spost)'
 
 without __git_ps1 having a special prompt command mode, no?

But how to determine which mode to use?
In pcmode, you must set the PS1, in command-subsitute mode, you must print a 
formatted gitstring.

 
 I have a feeling that I am missing something major, though...

I think the fundamentally different way of setting the PS1 between the two 
modes is very confusing. Which is why I originally made a different function 
(with duplicated code) for both modes.


 
  if [ $w = * ]; then
  -   PS1=$PS1\[$bad_color\]$w
  +   gitstring=$gitstring\[$bad_color\]$w
  fi
 
 Every time I looked at this line, I wondered why '*' state is
 bad.  Does a user go into any bad state by having a dirty
 working tree?  Same for untracked ($u) and detached.  These are all
 perfectly normal part of a workflow, so while choice of red may be
 fine to attract attention, calling it bad sounds misguided.


Well, I'm most often a really casual user of git and to make this function work 
the way I want to, I found out by trial-and-error that this was a way to test 
whether it's time to colour the string red or green ;-)

I'm very open to better ways to determine the colour modes. Anyway, the colours 
are now more or less the same as what git itself uses when printing the status 
with colour hints in git status.

Cheers

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


generating format-patch options from an e-mail

2012-12-26 Thread Simon Oosthoek
Hi all

I've been very frustrated by the process to setup a commandline for git 
format-patch, to include everyone in the cc list and reply to the right 
message-id.

In my frustration I created a perl script to generate the options from a saved 
e-mail, I realise that it may be non-general and perhaps it could be written 
better using a module which understands e-mails, but well, it worked for me ;-)

Anyway, I could imagine this as optional flag of git format-patch, so you could 
say:
$ git format-patch -s --in-reply-to-email mboxfile a7fe7de8

But I'll save that as an exercise for the reader (or the future)

Cheers

Simon

PS, now with the script
#!/usr/bin/perl

use warnings;
use strict;

our @to;
our @cc;
our @id;
our $emptyline=0;

if (defined $ARGV[0] and -f $ARGV[0]) {
	open (MAIL, $ARGV[0]) or die cannot open $ARGV[0]\n;
	#while (my $line=MAIL  ($emptyline == 0) ) {
	while (my $line=MAIL ) {
			chomp $line;
			my $header=;
			my $content=;
		
			if ($line =~ /^(.*?):.*[ ,](.*?@.*?)[, ]/ ||
	$line =~ /^(.*ID?):.*[ ,](.*?)[, ]/) {
	$header=$1;
	$content=$2;
	if ($header eq From) {
			push(@to, $content);
	} if ($header eq To) {
			push(@cc, $content);
	} elsif ($header eq Cc) {
			$line =~ /:(.*)$/;
			my @ccs=split(/,/, $1);
			foreach my $addr (@ccs) {
	if ($addr =~ /(.*)/) {
			push(@cc, $1);
	} else {
			push(@cc, $addr);
	}
			}
	} elsif ($header eq Message-ID) {
			push(@id, $content);
	}
			}
			$emptyline++ if (length($line) == 0);

	}
	close (MAIL);
}


foreach my $item (@to) {
		print  --to \$item\;
}
foreach my $item (@cc) {
		print  --cc \$item\;
}
foreach my $item (@id) {
		print  --in-reply-to \$item\;
}


Re: [PATCH] make __git_ps1 accept a third parameter in pcmode

2012-12-26 Thread Junio C Hamano
Simon Oosthoek s.oosth...@xs4all.nl writes:

 The problem with doing it in pre-post is when inside non-git
 directories. You want to avoid any gitstring output, including the
 brackets, when not inside a repository.

Ah, Ok, that is probably what I missed.

 Or we could go the other way and drop pre/post strings, making
 them part of the printf_format string.  Perhaps that might be a
 better interface in the longer term.  Then people can use the same
 pre%spost format string and do either of these:
 
  PS1=$(__git_ps1 pre%spost)
  PROMPT_COMMAND='PS1=$(__git_ps1 pre%spost)'
 
 without __git_ps1 having a special prompt command mode, no?

 But how to determine which mode to use?
 In pcmode, you must set the PS1, in command-subsitute mode, you must print a 
 formatted gitstring.

The point of the above two was that __git_ps1 does not have to set
PS1 as long as the insn says user to use PROMPT_COMMAND that sets
PS1 himself, exactly as illustrated above.  In other words, replace
the last PS1=...  in the prompt command mode with an echo or
something and make the user responsible for assigning it to PS1 in
his PROMPT_COMMAND.

Or put it in another way, I was hoping that we can do without adding
the prompt command mode---if there is no two modes, there is no need
to switch between them.

But as I said, there probably is a reason why that approach does not
work, that is why I said...

 I have a feeling that I am missing something major, though...
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: generating format-patch options from an e-mail

2012-12-26 Thread Junio C Hamano
Simon Oosthoek s.oosth...@xs4all.nl writes:

 Hi all

 I've been very frustrated by the process to setup a commandline for git 
 format-patch, to include everyone in the cc list and reply to the right 
 message-id.

 In my frustration I created a perl script to generate the options from a 
 saved e-mail, I realise that it may be non-general and perhaps it could be 
 written better using a module which understands e-mails, but well, it worked 
 for me ;-)

 Anyway, I could imagine this as optional flag of git format-patch, so you 
 could say:
 $ git format-patch -s --in-reply-to-email mboxfile a7fe7de8

 But I'll save that as an exercise for the reader (or the future)

I think a much more general approach would be to turn your script
into get-msg-id script and use it like so:

  $ git format-patch --in-reply-to $(get-msg-id mboxfile) a7fe7de8

Then you can reuse that script in a context outside format-patch,
whereever you need the message-id in a single message in the
mailbox.



--
To unsubscribe from this list: send the line 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] make __git_ps1 accept a third parameter in pcmode

2012-12-26 Thread Junio C Hamano
Simon Oosthoek s.oosth...@xs4all.nl writes:

 Every time I looked at this line, I wondered why '*' state is
 bad.  Does a user go into any bad state by having a dirty
 working tree?  Same for untracked ($u) and detached.  These are all
 perfectly normal part of a workflow, so while choice of red may be
 fine to attract attention, calling it bad sounds misguided.

 Well, I'm most often a really casual user of git and to make this
 function work the way I want to, I found out by trial-and-error
 that this was a way to test whether it's time to colour the string
 red or green ;-)

 I'm very open to better ways to determine the colour
 modes. Anyway, the colours are now more or less the same as what
 git itself uses when printing the status with colour hints in git
 status.

Oh, I am not opposed to the choice of colors; something that wants
an attention from the user may be better in red.

I was merely commenting on the choice of the variable name, the user
of word bad as if the conditions that use this color were bad in
some way.
--
To unsubscribe from this list: send the line 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] make __git_ps1 accept a third parameter in pcmode

2012-12-26 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 But as I said, there probably is a reason why that approach does not
 work, that is why I said...

 I have a feeling that I am missing something major, though...

In any case, this was more like if we were doing this from scratch
conversation.

I think PROMPT_COMMAND mode that takes pre and post string has
been advertised throughout the pre-release freeze, which is long
enough in Git timescale to avoid backward incompatibility, so we are
already stuck with the function in two modes even if what I said in
the previous message (i.e. why not just one mode) worked.

I am considering to fast-track the optional third parameter patch
to the 1.8.1 final, so that we can have the same degree of
customizability in both modes.

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


Re: [PATCH] make __git_ps1 accept a third parameter in pcmode

2012-12-26 Thread Simon Oosthoek
* Junio C Hamano gits...@pobox.com [2012-12-26 12:32:20 -0800]:
 The point of the above two was that __git_ps1 does not have to set
 PS1 as long as the insn says user to use PROMPT_COMMAND that sets
 PS1 himself, exactly as illustrated above.  In other words, replace
 the last PS1=...  in the prompt command mode with an echo or
 something and make the user responsible for assigning it to PS1 in
 his PROMPT_COMMAND.
 
 Or put it in another way, I was hoping that we can do without adding
 the prompt command mode---if there is no two modes, there is no need
 to switch between them.
 
 But as I said, there probably is a reason why that approach does not
 work, that is why I said...
 

The only reason to my knowledge is that bash's handling of zero-length strings, 
like terminal colour commands, is producing a PS1 that outputs less visible 
characters than bash thinks and thus bash makes mistakes when wrapping the 
commandline. The way to prevent that is to use \[ and \] around those and that 
doesn't seem to work from a string produced from command-substitution. (BTW, 
the colours come through just fine, just the \[ and \] don't)

Another approach could be to split up the functionality and have a few support 
functions to set various variables (corresponding with the gitstring features, 
like *%+ characters and colour hints). These variables could then be used by a 
custom PROMPT_COMMAND function or a command substitution function to produce a 
gitstring. I suppose that would mean a complete rewrite or very close to it ;-)

/Simon

--
To unsubscribe from this list: send the line 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: generating format-patch options from an e-mail

2012-12-26 Thread Simon Oosthoek
* Junio C Hamano gits...@pobox.com [2012-12-26 12:35:28 -0800]:
 
  Anyway, I could imagine this as optional flag of git format-patch, so you 
  could say:
  $ git format-patch -s --in-reply-to-email mboxfile a7fe7de8
 
  But I'll save that as an exercise for the reader (or the future)
 
 I think a much more general approach would be to turn your script
 into get-msg-id script and use it like so:
 
   $ git format-patch --in-reply-to $(get-msg-id mboxfile) a7fe7de8
 
 Then you can reuse that script in a context outside format-patch,
 whereever you need the message-id in a single message in the
 mailbox.
 

That would work for the message-ID, but not for the various To: and Cc: 
addresses.

The hacky script that I sent afterwards produces a string with the various 
options to git format-patch (--to --cc --in-reply-to) based on the headers 
To:/Cc:/From:/Message-ID:

Cheers

Simon
--
To unsubscribe from this list: send the line 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] log: grep author/committer using mailmap

2012-12-26 Thread Antoine Pelisse

 +static int commit_rewrite_authors(struct strbuf *buf, const char *what, 
 struct string_list *mailmap)
 +{
 + char *author, *endp;
 + size_t len;
 + struct strbuf name = STRBUF_INIT;
 + struct strbuf mail = STRBUF_INIT;
 + struct ident_split ident;
 +
 + author = strstr(buf-buf, what);
 + if (!author)
 + goto error;

 This does not stop at the end of the header part and would match a
 random line in the log message that happens to begin with author ;
 is this something we would worry about, or would we leave it to fsck?

The only worrying case would be:
 - commit doesn't have \nauthor in the header (can that happen
without corruption?)
 - commit has \nauthor in the commit log
 - This line from commit log contains an email (split_ident_line works)
Then, I guess it's going to replace the name in the commit log.

Otherwise, it would not replace anything, as there is no author to
replace anyway.

It looks like most mechanisms using mailmap would have the same issue.

 + author += strlen(what);
 + endp = strstr(author, \n);

 Using strchr(author, '\n') would feel more natural.  Also rename
 author to person or something, as you would be using this
 function for the committer information as well?

Both fixed

 + if (!endp)
 + goto error;
 +
 + len = endp - author;
 +
 + if (split_ident_line(ident, author, len)) {
 + error:
 + strbuf_release(name);
 + strbuf_release(mail);
 +
 + return 1;

 We usually signal error by returning a negative integer.  It does
 not matter too much in this case as no callers seem to check the
 return value from this function, though.

Fixed, or would you rather see it `void` ?

 + }
 +
 + strbuf_add(name, ident.name_begin, ident.name_end - ident.name_begin);
 + strbuf_add(mail, ident.mail_begin, ident.mail_end - ident.mail_begin);
 +
 + map_user(mailmap, mail, name);
 +
 + strbuf_addf(name,  %s, mail.buf);
 +
 + strbuf_splice(buf, ident.name_begin - buf-buf,
 +   ident.mail_end - ident.name_begin + 1,
 +   name.buf, name.len);

 Would it give us better performance if we splice only when
 map_user() tells us that we actually rewrote the ident?

My intuition was that the cost of splice belongs to memoving, when the
size is different. Yet, Fixed, as it removes two copies.
--
To unsubscribe from this list: send the line 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: generating format-patch options from an e-mail

2012-12-26 Thread Pyeron, Jason J CTR (US)
 -Original Message-
 From: Simon Oosthoek
 Sent: Wednesday, December 26, 2012 4:08 PM
 
 * Junio C Hamano gits...@pobox.com [2012-12-26 12:35:28 -0800]:
  
   Anyway, I could imagine this as optional flag of git format-patch,
 so you could say:
   $ git format-patch -s --in-reply-to-email mboxfile a7fe7de8

Anyway you would need a --in-reply-to-email and a --in-reply-to-all-email, both 
should support stdin. Then you can feel free to --[no-]to and --[no-]cc (where 
is the --bcc?) to your heart's content.

It would be a nice addition.

  
   But I'll save that as an exercise for the reader (or the future)
 
  I think a much more general approach would be to turn your script
  into get-msg-id script and use it like so:
 
$ git format-patch --in-reply-to $(get-msg-id mboxfile) a7fe7de8
 
  Then you can reuse that script in a context outside format-patch,
  whereever you need the message-id in a single message in the
  mailbox.
 
 
 That would work for the message-ID, but not for the various To: and Cc:
 addresses.
 
 The hacky script that I sent afterwards produces a string with the

Nit, it does not make use of the reply-to header if present.

 various options to git format-patch (--to --cc --in-reply-to) based on
 the headers To:/Cc:/From:/Message-ID:
 
 Cheers
 
 Simon
 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html


smime.p7s
Description: S/MIME cryptographic signature


Re: [PATCH 1/2] log: grep author/committer using mailmap

2012-12-26 Thread Junio C Hamano
Antoine Pelisse apeli...@gmail.com writes:


 +static int commit_rewrite_authors(struct strbuf *buf, const char *what, 
 struct string_list *mailmap)
 +{
 + char *author, *endp;
 + size_t len;
 + struct strbuf name = STRBUF_INIT;
 + struct strbuf mail = STRBUF_INIT;
 + struct ident_split ident;
 +
 + author = strstr(buf-buf, what);
 + if (!author)
 + goto error;

 This does not stop at the end of the header part and would match a
 random line in the log message that happens to begin with author ;
 is this something we would worry about, or would we leave it to fsck?

 The only worrying case would be:
 ...

Yeah, that pretty much matches what I had in mind (the short answer:
leave it to git fsck).

 We usually signal error by returning a negative integer.  It does
 not matter too much in this case as no callers seem to check the
 return value from this function, though.

 Fixed, or would you rather see it `void` ?

Just like you can take advantage of map_user() that signals the
caller if it did anything to optimize this function, in the longer
run, it may help the (future) callers of this function if it gave I
did something vs I left it intact.  In the particular case of
this function, the error cases fall into the latter (it merely
explains why it left it intact, and there is no sensible error
recovery the caller _could_ do in any case) and I think it is not
necessary to differenciate between Returned as-is because there is
no mapping and Returned as-is because I couldn't parse the
commit.

So return 0 when it didn't do anything, return 1 when it rewrote
feels good enough, at least to me.

 + }
 +
 + strbuf_add(name, ident.name_begin, ident.name_end - 
 ident.name_begin);
 + strbuf_add(mail, ident.mail_begin, ident.mail_end - 
 ident.mail_begin);
 +
 + map_user(mailmap, mail, name);
 +
 + strbuf_addf(name,  %s, mail.buf);
 +
 + strbuf_splice(buf, ident.name_begin - buf-buf,
 +   ident.mail_end - ident.name_begin + 1,
 +   name.buf, name.len);

 Would it give us better performance if we splice only when
 map_user() tells us that we actually rewrote the ident?

 My intuition was that the cost of splice belongs to memoving, when the
 size is different. Yet, Fixed, as it removes two copies.

Thanks.

I wonder if we can further restructure the code so that it first
inspects the existing buffer to see if it even needs to copy the
original commit buffer into a strbuf only for grepping.  If that
can be easily done, then we will save even more copying, I think.

The reason I alluded to revamping the grep API to get rid of the use
of header grep mode in this codepath was exactly that.  We could:

 - change the command line parser for --author= and --committer= so
   that these do not become part of the main grep expression.
   Instead we keep them as separate grep expressions (one author
   expression that OR'es the --author= options together, the other
   for the --committer= options);

 - in this codepath, inspect the author and committer in the
   commit object buffer, map them if necessary via the mailmap
   mechanism into temporary buffers (that is different from the
   buf in the commit_match() function), then run grep_buffer()
   with the author and committer grep expressions we separated in
   the previous step. Then we combine the results from author and
   committer grep and the main grep_buffer() result ourselves in
   this function.

That may essentially amount to going in the totally opposite
direction from what 2d10c55 (git log: Unify header_filter and
message_filter into one., 2006-09-20) attempted to do.  We used to
have two grep expressions (one for header, the other one for body)
commit_match() runs grep_buffer() on and combined the results.
2d10c55 merged them into one grep expression by introducing a term
that matches only header elements.  But we would instead split the
header expression into author and committer expressions
(making it three from one) if we go the above route.

That would eliminate the need to copy and rewrite the contents of
the commit object in this codepath, which may be a big win when
names and emails that need to be rewritten are minority cases.

But I suspect that is a much larger change.  If we can reduce the
amount of copies necessary without changing the code structure, that
may be enough to reduce the performance hit from this change.

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


Re: generating format-patch options from an e-mail

2012-12-26 Thread Jeff King
On Wed, Dec 26, 2012 at 09:31:46PM +, Pyeron, Jason J CTR (US) wrote:

  That would work for the message-ID, but not for the various To: and Cc:
  addresses.
  
  The hacky script that I sent afterwards produces a string with the
 
 Nit, it does not make use of the reply-to header if present.

I do something very similar to Simon, except that rather than generating
a reply with my script, I generate the cover letter in my MUA, and then
use that response as a template. So the MUA does the heavy lifting,
understanding reply-to, culling my own address from cc, etc. The
format-patch replies then have the exact same to/cc headers as the
template, and use the template's message-id as the in-reply-to (for
proper threading).

My perl looks like this (feed the template via stdin):

  perl -ne '
if (defined $opt  /^\s+(.*)/) {
  $val .=  $1;
  next;
}
if (defined $opt) {
  print --$opt=, quotemeta($val),  ;
  $opt = $val = undef;
}
if (/^(cc|to):\s*(.*)/i) {
  $opt = lc($1);
  $val = $2;
}
elsif (/^message-id:\s*(.*)/i) {
  $opt = in-reply-to;
  $val = $1;
}
  '

That, of course, presupposes a cover letter. If I am sending a single
patch, then I just do format-patch --stdout right into my MUA's
editor), and let it handle the headers.

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


Re: GIT get corrupted on lustre

2012-12-26 Thread Jeff King
On Mon, Dec 24, 2012 at 09:08:46AM -0500, Eric Chamberland wrote:

 Doing a git clone always work fine, but when we git pull or git
 gc or git fsck, often (1/5) the local repository get corrupted.
 for example, I got this error two days ago while doing git gc:
 
 error: index file 
 .git/objects/pack/pack-7b43b1c613a851392aaf4f66916dff2577931576.idx is too 
 small
 error: refs/heads/mail_seekable does not point to a valid object!
 [...]
 We think it could be related to the fact that we are on a *Lustre*
 filesystem, which I think doesn't fully support file locking.

I don't think locking is a problem here. The problem is that you have a
corrupt .idx file (the second error is almost certainly an effect of the
first one; git cannot look in the packfile, and therefore cannot find
the object the ref points to). But we do not ever lock the .idx files.
They are generated in tmpfiles and then atomically moved into place
using a hard link.

So if anything, I would suspect that lustre has trouble with the
write/fsync/close/link sequence. Is it possible that it does not keep
the ordering, and readers might see a linked file that is missing some
data? If you wait (or do some synchronizing operation on the filesystem,
like sync, or an unmount/mount), does the repo later work, or is it
broken forever?

 #1) However, how can we *test* the filesystem (lustre) compatibility
 with git? (Is there a unit test we can run?)

Running make test in git.git would be a good start. You could also try
running the C program I'm including below. It repeatedly runs a
write/close/fsync/link sequence like the one that index-pack runs, and
then verifies the result. If it does not run forever without error, that
would be a sign of the possible ordering problem I mentioned above.

 #2) Is there a way to compile GIT to be compatible with lustre? (ex:
 no threads?)

This isn't a known issue, so I don't know offhand what compile flags
might help. The complete list is at the top of Makefile. You might try
with NO_PTHREADS=Yes, but I kind of doubt that threads are at work here.

 #3) If you *know* your filesystem doesn't allow file locking, how
 would you configure/compile GIT to work on it?

I think locking is a red herring here, as it is not used to create the
.idx files at all (and we don't do flock locking anyway; everything
happens via O_EXCL creation).

-Peff

-- 8 --
#include fcntl.h
#include unistd.h
#include stdlib.h
#include stdio.h
#include string.h

static int randomize(unsigned char *buf, int len)
{
  int i;
  len = rand() % len;
  for (i = 0; i  len; i++)
buf[i] = rand()  0xff;
  return len;
}

static int check_eof(int fd)
{
  int ch;
  int r = read(fd, ch, 1);
  if (r  0) {
perror(read error after expected EOF);
return -1;
  }
  if (r  0) {
fprintf(stderr, extra byte after expected EOF);
return -1;
  }
  return 0;
}

static int verify(int fd, const unsigned char *buf, int len)
{
  while (len) {
char to_check[4096];
int got = read(fd, to_check,
   len  sizeof(to_check) ? len : sizeof(to_check));

if (got  0) {
  perror(unable to read);
  return -1;
}
if (got == 0) {
  fprintf(stderr, premature EOF (%d bytes remaining), len);
  return -1;
}
if (memcmp(buf, to_check, got)) {
  fprintf(stderr, bytes differ);
  return -1;
}

buf += got;
len -= got;
  }

  return check_eof(fd);
}

int write_in_full(int fd, const unsigned char *buf, int len)
{
  while (len) {
int r = write(fd, buf, len);
if (r  0)
  return -1;
buf += r;
len -= r;
  }
  return 0;
}

int move_into_place(const char *old, const char *new)
{
  if (link(old, new)  0) {
perror(unable to create hard link);
return 1;
  }
  unlink(old);
  return 0;
}

int main(void)
{
  while (1) {
static unsigned char junk[1024*1024];
int len = randomize(junk, sizeof(junk));
int fd;

/* clean up from any previous round */
unlink(tmpfile);
unlink(final.idx);

fd = open(tmpfile, O_WRONLY|O_CREAT, 0666);
if (fd  0) {
  perror(unable to open tmpfile);
  return 1;
}
if (write_in_full(fd, junk, len)  0 ||
fsync(fd)  0 ||
close(fd)  0) {
  perror(unable to write);
  return 1;
}

if (move_into_place(tmpfile, final.idx)  0)
  return 1;

fd = open(final.idx, O_RDONLY);
if (fd  0) {
  perror(unable to open index file);
  return 1;
}
if (verify(fd, junk, len)  0)
  return 1;
close(fd);
  }
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


What's cooking in git.git (Dec 2012, #07; Wed, 26)

2012-12-26 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

The final release for this cycle will happen sometime next week,
hopefully.

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

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

--
[New Topics]

* jc/format-patch-reroll (2012-12-22) 7 commits
 - format-patch: add --reroll-count=$N option
 - get_patch_filename(): split into two functions
 - get_patch_filename(): drop just-numbers hack
 - get_patch_filename(): simplify function signature
 - builtin/log.c: stop using global patch_suffix
 - builtin/log.c: drop redundant numbered_files parameter from 
make_cover_letter()
 - builtin/log.c: drop unused numbered parameter from make_cover_letter()

 Teach format-patch to prefix v4- to its output files for the
 fourth iteration of a patch series, to make it easier for the
 submitter to keep separate copies for iterations.

 Needs tests and documentation updates.


* ms/subtree-fixlets (2012-12-22) 2 commits
  (merged to 'next' on 2012-12-26 at 1cb26eb)
 + git-subtree: fix typo in manpage
 + git-subtree: ignore git-subtree executable

 Will cook in 'next'.


* mz/pick-unborn (2012-12-23) 2 commits
 - learn to pick/revert into unborn branch
 - tests: move test_cmp_rev to test-lib-functions

 Will merge to 'next'.


* nd/retire-fnmatch (2012-12-22) 8 commits
 - wildmatch: advance faster in asterisk + literal patterns
 - wildmatch: make a special case for */ with FNM_PATHNAME
 - Makefile: add USE_WILDMATCH to use wildmatch as fnmatch
 - test-wildmatch: add perf command to compare wildmatch and fnmatch
 - wildmatch: support no FNM_PATHNAME mode
 - wildmatch: make dowild() take arbitrary flags
 - wildmatch: rename constants and update prototype
 - compat/fnmatch: respect NO_FNMATCH* even on glibc
 (this branch uses nd/wildmatch.)

 Replace our use of fnmatch(3) with a more feature-rich wildmatch.


* jc/test-cvs-no-init-in-existing-dir (2012-12-24) 1 commit
  (merged to 'next' on 2012-12-26 at 3b93f37)
 + t9200: let cvs init create the test repository

 Will cook in 'next'.


* os/gitweb-highlight-uncaptured (2012-12-26) 1 commit
 - gitweb: fix error when highlight is enabled

 Will merge to 'next'.


* so/prompt-command (2012-12-26) 1 commit
  (merged to 'next' on 2012-12-26 at 27c5683)
 + make __git_ps1 accept a third parameter in pcmode

 Gives the same degree of customizability to the new prompt command
 mode users as the command substitution mode has.

 Will fast-track to 'master' before 1.8.1 final.

--
[Stalled]

* jc/doc-maintainer (2012-11-27) 1 commit
 - update howto maintain git

 An early draft that is still incomplete.


* fc/remote-bzr (2012-12-13) 10 commits
 - (fixup) test-bzr.sh: fix multi-line string assignment
 - remote-bzr: detect local repositories
 - remote-bzr: add support for older versions of bzr
 - remote-bzr: add support to push special modes
 - remote-bzr: add support for fecthing special modes
 - remote-bzr: add simple tests
 - remote-bzr: update working tree upon pushing
 - remote-bzr: add support for remote repositories
 - remote-bzr: add support for pushing
 - Add new remote-bzr transport helper

 New remote helper for bzr (v3).  With minor fixes, this may be ready
 for 'next'.


* mo/cvs-server-updates (2012-12-09) 18 commits
 - t9402: Use TABs for indentation
 - t9402: Rename check.cvsCount and check.list
 - t9402: Simplify git ls-tree
 - t9402: Add missing ; Code style
 - t9402: No space after IO-redirection
 - t9402: Dont use test_must_fail cvs
 - t9402: improve check_end_tree() and check_end_full_tree()
 - t9402: sed -i is not portable
 - cvsserver Documentation: new cvs ... -r support
 - cvsserver: add t9402 to test branch and tag refs
 - cvsserver: support -r and sticky tags for most operations
 - cvsserver: Add version awareness to argsfromdir
 - cvsserver: generalize getmeta() to recognize commit refs
 - cvsserver: implement req_Sticky and related utilities
 - cvsserver: add misc commit lookup, file meta data, and file listing functions
 - cvsserver: define a tag name character escape mechanism
 - cvsserver: cleanup extra slashes in filename arguments
 - cvsserver: factor out git-log parsing logic

 Needs review by folks interested in cvsserver.


* as/check-ignore (2012-11-08) 14 commits
 - t0007: fix tests on Windows
 - Documentation/check-ignore: we show the deciding match, not the first
 - Add git-check-ignore sub-command
 - dir.c: provide free_directory() for reclaiming dir_struct memory
 - pathspec.c: move reusable code from builtin/add.c
 - dir.c: refactor treat_gitlinks()
 - dir.c: keep track of where patterns came from
 - dir.c: refactor is_path_excluded()
 - dir.c: refactor is_excluded()
 - dir.c: refactor is_excluded_from_list()
 - dir.c: rename excluded() to 

[PATCH 0/5] merge-tree fix-ups

2012-12-26 Thread Junio C Hamano
This is a small preparatory step to build a new merge strategy based
on the disused merge-tree proof-of-concept code.  It starts with a
small set of code clean-up patches and ends with two bugfixes, at
least for now.

Junio C Hamano (5):
  Which merge_file() function do you mean?
  merge-tree: lose unused flags from merge_list
  merge-tree: lose unused resolve_directories
  merge-tree: add comments to clarify what these functions are doing
  merge-tree: fix d/f conflicts

 Makefile  |   2 +-
 builtin/merge-index.c |   4 +-
 builtin/merge-tree.c  |  92 +++--
 merge-blobs.c | 124 ++
 merge-blobs.h |   8 
 merge-file.c  | 124 --
 merge-file.h  |   7 ---
 merge-recursive.c |   6 +--
 t/t4300-merge-tree.sh |  44 ++
 9 files changed, 239 insertions(+), 172 deletions(-)
 create mode 100644 merge-blobs.c
 create mode 100644 merge-blobs.h
 delete mode 100644 merge-file.c
 delete mode 100644 merge-file.h

-- 
1.8.1.rc3.356.g686f81c

--
To unsubscribe from this list: send the line 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/5] Which merge_file() function do you mean?

2012-12-26 Thread Junio C Hamano
There are two different static functions and one global function,
all of them called merge_file(), with different signatures and
purposes.  Rename them all to reduce confusion in git grep output:

 * Rename the static one in merge-index to merge_one_path(const char
   *path) as that function is about asking an external command to
   resolve conflicts in one path.

 * Rename the global one in merge-file.c that is only used by
   merge-tree to merge_blobs(), as the function takes three blobs and
   returns the merged result only in-core, without doing anything to
   the filesystem.

 * Rename the one in merge-recursive to merge_one_file(), just to be
   fair.

Also rename merge-file.[ch] to merge-blobs.[ch].

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Makefile  |   2 +-
 builtin/merge-index.c |   4 +-
 builtin/merge-tree.c  |   4 +-
 merge-blobs.c | 124 ++
 merge-blobs.h |   8 
 merge-file.c  | 124 --
 merge-file.h  |   7 ---
 merge-recursive.c |   6 +--
 8 files changed, 140 insertions(+), 139 deletions(-)
 create mode 100644 merge-blobs.c
 create mode 100644 merge-blobs.h
 delete mode 100644 merge-file.c
 delete mode 100644 merge-file.h

diff --git a/Makefile b/Makefile
index 4ad6fbd..26ec519 100644
--- a/Makefile
+++ b/Makefile
@@ -765,7 +765,7 @@ LIB_OBJS += log-tree.o
 LIB_OBJS += mailmap.o
 LIB_OBJS += match-trees.o
 LIB_OBJS += merge.o
-LIB_OBJS += merge-file.o
+LIB_OBJS += merge-blobs.o
 LIB_OBJS += merge-recursive.o
 LIB_OBJS += mergesort.o
 LIB_OBJS += name-hash.o
diff --git a/builtin/merge-index.c b/builtin/merge-index.c
index 2338832..be5e514 100644
--- a/builtin/merge-index.c
+++ b/builtin/merge-index.c
@@ -42,7 +42,7 @@ static int merge_entry(int pos, const char *path)
return found;
 }
 
-static void merge_file(const char *path)
+static void merge_one_path(const char *path)
 {
int pos = cache_name_pos(path, strlen(path));
 
@@ -102,7 +102,7 @@ int cmd_merge_index(int argc, const char **argv, const char 
*prefix)
}
die(git merge-index: unknown option %s, arg);
}
-   merge_file(arg);
+   merge_one_path(arg);
}
if (err  !quiet)
die(merge program failed);
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 897a563..ebd0d25 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -3,7 +3,7 @@
 #include xdiff-interface.h
 #include blob.h
 #include exec_cmd.h
-#include merge-file.h
+#include merge-blobs.h
 
 static const char merge_tree_usage[] = git merge-tree base-tree branch1 
branch2;
 static int resolve_directories = 1;
@@ -76,7 +76,7 @@ static void *result(struct merge_list *entry, unsigned long 
*size)
their = NULL;
if (entry)
their = entry-blob;
-   return merge_file(path, base, our, their, size);
+   return merge_blobs(path, base, our, their, size);
 }
 
 static void *origin(struct merge_list *entry, unsigned long *size)
diff --git a/merge-blobs.c b/merge-blobs.c
new file mode 100644
index 000..57211bc
--- /dev/null
+++ b/merge-blobs.c
@@ -0,0 +1,124 @@
+#include cache.h
+#include run-command.h
+#include xdiff-interface.h
+#include ll-merge.h
+#include blob.h
+#include merge-blobs.h
+
+static int fill_mmfile_blob(mmfile_t *f, struct blob *obj)
+{
+   void *buf;
+   unsigned long size;
+   enum object_type type;
+
+   buf = read_sha1_file(obj-object.sha1, type, size);
+   if (!buf)
+   return -1;
+   if (type != OBJ_BLOB)
+   return -1;
+   f-ptr = buf;
+   f-size = size;
+   return 0;
+}
+
+static void free_mmfile(mmfile_t *f)
+{
+   free(f-ptr);
+}
+
+static void *three_way_filemerge(const char *path, mmfile_t *base, mmfile_t 
*our, mmfile_t *their, unsigned long *size)
+{
+   int merge_status;
+   mmbuffer_t res;
+
+   /*
+* This function is only used by cmd_merge_tree, which
+* does not respect the merge.conflictstyle option.
+* There is no need to worry about a label for the
+* common ancestor.
+*/
+   merge_status = ll_merge(res, path, base, NULL,
+   our, .our, their, .their, NULL);
+   if (merge_status  0)
+   return NULL;
+
+   *size = res.size;
+   return res.ptr;
+}
+
+static int common_outf(void *priv_, mmbuffer_t *mb, int nbuf)
+{
+   int i;
+   mmfile_t *dst = priv_;
+
+   for (i = 0; i  nbuf; i++) {
+   memcpy(dst-ptr + dst-size, mb[i].ptr, mb[i].size);
+   dst-size += mb[i].size;
+   }
+   return 0;
+}
+
+static int generate_common_file(mmfile_t *res, mmfile_t *f1, mmfile_t *f2)
+{
+   unsigned long size = f1-size  f2-size ? f1-size : f2-size;
+   void *ptr = xmalloc(size);
+   xpparam_t xpp;
+   

[PATCH 2/5] merge-tree: lose unused flags from merge_list

2012-12-26 Thread Junio C Hamano
Drop the unused field from the structure.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 builtin/merge-tree.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index ebd0d25..b61d811 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -12,8 +12,7 @@ struct merge_list {
struct merge_list *next;
struct merge_list *link;/* other stages for this object */
 
-   unsigned int stage : 2,
-flags : 30;
+   unsigned int stage : 2;
unsigned int mode;
const char *path;
struct blob *blob;
-- 
1.8.1.rc3.356.g686f81c

--
To unsubscribe from this list: send the line 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/5] merge-tree: lose unused resolve_directories

2012-12-26 Thread Junio C Hamano
This option is always set; simplify.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 builtin/merge-tree.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index b61d811..95de162 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -6,7 +6,6 @@
 #include merge-blobs.h
 
 static const char merge_tree_usage[] = git merge-tree base-tree branch1 
branch2;
-static int resolve_directories = 1;
 
 struct merge_list {
struct merge_list *next;
@@ -198,8 +197,6 @@ static int unresolved_directory(const struct traverse_info 
*info, struct name_en
struct tree_desc t[3];
void *buf0, *buf1, *buf2;
 
-   if (!resolve_directories)
-   return 0;
p = n;
if (!p-mode) {
p++;
-- 
1.8.1.rc3.356.g686f81c

--
To unsubscribe from this list: send the line 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/5] merge-tree: add comments to clarify what these functions are doing

2012-12-26 Thread Junio C Hamano
Rename the branch1 parameter given to resolve() to ours, to
clarify what is going on.  Also, annotate the unresolved_directory()
function with some comments to show what decisions are made in each
step, and highlight two bugs that need to be fixed.

Add two tests to t4300 to illustrate these bugs.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 builtin/merge-tree.c  | 26 ++
 t/t4300-merge-tree.sh | 44 
 2 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 95de162..5704d51 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -172,17 +172,17 @@ static char *traverse_path(const struct traverse_info 
*info, const struct name_e
return make_traverse_path(path, info, n);
 }
 
-static void resolve(const struct traverse_info *info, struct name_entry 
*branch1, struct name_entry *result)
+static void resolve(const struct traverse_info *info, struct name_entry *ours, 
struct name_entry *result)
 {
struct merge_list *orig, *final;
const char *path;
 
-   /* If it's already branch1, don't bother showing it */
-   if (!branch1)
+   /* If it's already ours, don't bother showing it */
+   if (!ours)
return;
 
path = traverse_path(info, result);
-   orig = create_entry(2, branch1-mode, branch1-sha1, path);
+   orig = create_entry(2, ours-mode, ours-sha1, path);
final = create_entry(0, result-mode, result-sha1, path);
 
final-link = orig;
@@ -205,6 +205,15 @@ static int unresolved_directory(const struct traverse_info 
*info, struct name_en
}
if (!S_ISDIR(p-mode))
return 0;
+   /*
+* NEEDSWORK: this is broken. The path can originally be a file
+* and then one side may have turned it into a directory, in which
+* case we return and let the three-way merge as if the tree were
+* a regular file.  If the path that was originally a tree is
+* now a file in either branch, fill_tree_descriptor() below will
+* die when fed a blob sha1.
+*/
+
newbase = traverse_path(info, p);
buf0 = fill_tree_descriptor(t+0, n[0].sha1);
buf1 = fill_tree_descriptor(t+1, n[1].sha1);
@@ -288,20 +297,29 @@ static int threeway_callback(int n, unsigned long mask, 
unsigned long dirmask, s
/* Same in both? */
if (same_entry(entry+1, entry+2)) {
if (entry[0].sha1) {
+   /* Modified identically */
resolve(info, NULL, entry+1);
return mask;
}
+   /* Both added the same is left unresolved */
}
 
if (same_entry(entry+0, entry+1)) {
if (entry[2].sha1  !S_ISDIR(entry[2].mode)) {
+   /* We did not touch, they modified -- take theirs */
resolve(info, entry+1, entry+2);
return mask;
}
+   /*
+* If we did not touch a directory but they made it
+* into a file, we fall through and unresolved()
+* recurses down.  Likewise for the opposite case.
+*/
}
 
if (same_entry(entry+0, entry+2)) {
if (entry[1].sha1  !S_ISDIR(entry[1].mode)) {
+   /* We modified, they did not touch -- take ours */
resolve(info, NULL, entry+1);
return mask;
}
diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh
index 46c3fe7..03e8fca 100755
--- a/t/t4300-merge-tree.sh
+++ b/t/t4300-merge-tree.sh
@@ -254,4 +254,48 @@ EXPECTED
test_cmp expected actual
 '
 
+test_expect_failure 'turn file to tree' '
+   git reset --hard initial 
+   rm initial-file 
+   mkdir initial-file 
+   test_commit turn-file-to-tree initial-file/ONE CCC 
+   git merge-tree initial initial turn-file-to-tree actual 
+   cat expect -\EOF 
+   added in remote
+ their  100644 43aa4fdec31eb92e1fdc2f0ce6ea9ddb7c32bcf7 
initial-file/ONE
+   @@ -0,0 +1 @@
+   +CCC
+   removed in remote
+ base   100644 e79c5e8f964493290a409888d5413a737e8e5dd5 initial-file
+ our100644 e79c5e8f964493290a409888d5413a737e8e5dd5 initial-file
+   @@ -1 +0,0 @@
+   -initial
+   EOF
+   test_cmp expect actual
+'
+
+test_expect_failure 'turn tree to file' '
+   git reset --hard initial 
+   mkdir dir 
+   test_commit add-tree dir/path AAA 
+   test_commit add-another-tree dir/another BBB 
+   rm -fr dir 
+   test_commit make-file dir CCC 
+   git merge-tree add-tree add-another-tree make-file actual 
+   cat expect -\EOF 
+   added in local
+ our100644 ba629238ca89489f2b350e196ca445e09d8bb834 dir/another
+   removed in remote
+ base   

[PATCH 5/5] merge-tree: fix d/f conflicts

2012-12-26 Thread Junio C Hamano
The previous commit documented two known breakages revolving around
a case where one side flips a tree into a blob (or vice versa),
where the original code simply gets confused and feeds a mixture of
trees and blobs into either the recursive merge-tree (and recursing
into the blob will fail) or three-way merge (and merging tree contents
together with blobs will fail).

Fix it by feeding trees (and only trees) into the recursive
merge-tree machinery and blobs (and only blobs) into the three-way
content level merge machinery separately; when this happens, the
entire merge has to be marked as conflicting at the structure level.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 builtin/merge-tree.c  | 72 ---
 t/t4300-merge-tree.sh |  4 +--
 2 files changed, 42 insertions(+), 34 deletions(-)

diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 5704d51..e0d0b7d 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -25,7 +25,7 @@ static void add_merge_entry(struct merge_list *entry)
merge_result_end = entry-next;
 }
 
-static void merge_trees(struct tree_desc t[3], const char *base);
+static void merge_trees_recursive(struct tree_desc t[3], const char *base, int 
df_conflict);
 
 static const char *explanation(struct merge_list *entry)
 {
@@ -190,41 +190,35 @@ static void resolve(const struct traverse_info *info, 
struct name_entry *ours, s
add_merge_entry(final);
 }
 
-static int unresolved_directory(const struct traverse_info *info, struct 
name_entry n[3])
+static void unresolved_directory(const struct traverse_info *info, struct 
name_entry n[3],
+int df_conflict)
 {
char *newbase;
struct name_entry *p;
struct tree_desc t[3];
void *buf0, *buf1, *buf2;
 
-   p = n;
-   if (!p-mode) {
-   p++;
-   if (!p-mode)
-   p++;
+   for (p = n; p  n + 3; p++) {
+   if (p-mode  S_ISDIR(p-mode))
+   break;
}
-   if (!S_ISDIR(p-mode))
-   return 0;
-   /*
-* NEEDSWORK: this is broken. The path can originally be a file
-* and then one side may have turned it into a directory, in which
-* case we return and let the three-way merge as if the tree were
-* a regular file.  If the path that was originally a tree is
-* now a file in either branch, fill_tree_descriptor() below will
-* die when fed a blob sha1.
-*/
+   if (n + 3 = p)
+   return; /* there is no tree here */
 
newbase = traverse_path(info, p);
-   buf0 = fill_tree_descriptor(t+0, n[0].sha1);
-   buf1 = fill_tree_descriptor(t+1, n[1].sha1);
-   buf2 = fill_tree_descriptor(t+2, n[2].sha1);
-   merge_trees(t, newbase);
+
+#define ENTRY_SHA1(e) (((e)-mode  S_ISDIR((e)-mode)) ? (e)-sha1 : NULL)
+   buf0 = fill_tree_descriptor(t+0, ENTRY_SHA1(n + 0));
+   buf1 = fill_tree_descriptor(t+1, ENTRY_SHA1(n + 1));
+   buf2 = fill_tree_descriptor(t+2, ENTRY_SHA1(n + 2));
+#undef ENTRY_SHA1
+
+   merge_trees_recursive(t, newbase, df_conflict);
 
free(buf0);
free(buf1);
free(buf2);
free(newbase);
-   return 1;
 }
 
 
@@ -247,18 +241,26 @@ static struct merge_list *link_entry(unsigned stage, 
const struct traverse_info
 static void unresolved(const struct traverse_info *info, struct name_entry 
n[3])
 {
struct merge_list *entry = NULL;
+   int i;
+   unsigned dirmask = 0, mask = 0;
+
+   for (i = 0; i  3; i++) {
+   mask |= (1  1);
+   if (n[i].mode  S_ISDIR(n[i].mode))
+   dirmask |= (1  i);
+   }
+
+   unresolved_directory(info, n, dirmask  (dirmask != mask));
 
-   if (unresolved_directory(info, n))
+   if (dirmask == mask)
return;
 
-   /*
-* Do them in reverse order so that the resulting link
-* list has the stages in order - link_entry adds new
-* links at the front.
-*/
-   entry = link_entry(3, info, n + 2, entry);
-   entry = link_entry(2, info, n + 1, entry);
-   entry = link_entry(1, info, n + 0, entry);
+   if (n[2].mode  !S_ISDIR(n[2].mode))
+   entry = link_entry(3, info, n + 2, entry);
+   if (n[1].mode  !S_ISDIR(n[1].mode))
+   entry = link_entry(2, info, n + 1, entry);
+   if (n[0].mode  !S_ISDIR(n[0].mode))
+   entry = link_entry(1, info, n + 0, entry);
 
add_merge_entry(entry);
 }
@@ -329,15 +331,21 @@ static int threeway_callback(int n, unsigned long mask, 
unsigned long dirmask, s
return mask;
 }
 
-static void merge_trees(struct tree_desc t[3], const char *base)
+static void merge_trees_recursive(struct tree_desc t[3], const char *base, int 
df_conflict)
 {
struct traverse_info info;
 
setup_traverse_info(info, base);
+   

[PATCH v3] mergetools/p4merge: Honor $TMPDIR for the /dev/null placeholder

2012-12-26 Thread David Aguilar
Use $TMPDIR when creating the /dev/null placeholder for p4merge.
This prevents users from finding a seemingly random untracked file
in their worktree.

This is different than what mergetool does with $LOCAL and
$REMOTE because those files exist to aid users when resolving
merges.  p4merge's /dev/null placeholder is not helpful in that
situation so it is sensible to keep it out of the worktree.

Reported-by: Jeremy Morton ad...@game-point.net
Signed-off-by: David Aguilar dav...@gmail.com
---
v3 revised the commit message to better justify the change.
This is a replacement for what's current in 'next'.

 mergetools/p4merge | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/mergetools/p4merge b/mergetools/p4merge
index 295361a..52f7c8f 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -1,29 +1,21 @@
 diff_cmd () {
+   empty_file=
+
# p4merge does not like /dev/null
-   rm_local=
-   rm_remote=
if test /dev/null = $LOCAL
then
-   LOCAL=./p4merge-dev-null.LOCAL.$$
-   $LOCAL
-   rm_local=true
+   LOCAL=$(create_empty_file)
fi
if test /dev/null = $REMOTE
then
-   REMOTE=./p4merge-dev-null.REMOTE.$$
-   $REMOTE
-   rm_remote=true
+   REMOTE=$(create_empty_file)
fi
 
$merge_tool_path $LOCAL $REMOTE
 
-   if test -n $rm_local
-   then
-   rm -f $LOCAL
-   fi
-   if test -n $rm_remote
+   if test -n $empty_file
then
-   rm -f $REMOTE
+   rm -f $empty_file
fi
 }
 
@@ -33,3 +25,10 @@ merge_cmd () {
$merge_tool_path $BASE $LOCAL $REMOTE $MERGED
check_unchanged
 }
+
+create_empty_file () {
+   empty_file=${TMPDIR:-/tmp}/git-difftool-p4merge-empty-file.$$
+   $empty_file
+
+   printf $empty_file
+}
-- 
1.8.1.rc3.11.g86c3e6e
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 00/19] new git check-ignore sub-command

2012-12-26 Thread Adam Spiers
This v3 re-roll of my check-ignore series is a reasonably substantial
revamp over v2, and applies on top of Junio's current
nd/attr-match-optim-more branch (82dce998c202).

All feedback and patches from the v2 series has been incorporated, and
several other improvements too, including:

  - composite exclude_lists have been split up into
exclude_list_groups each containing one exclude_list per source

  - smaller commits for easier review

  - minor memory leaks have been fixed and verified via valgrind

  - t0007-ignores.sh has been renumbered to t0008-ignores.sh to avoid
a conflict with t0007-git-var.sh

  - improvements to documentation and comments

For reference, the v2 series was announced here:

http://thread.gmane.org/gmane.comp.version-control.git/204661/focus=206074

All tests pass except for t91*, since there seems to be some
pre-existing breakage in 82dce998c202 relating to git-svn.

Adam Spiers (19):
  api-directory-listing.txt: update to match code
  Improve documentation and comments regarding directory traversal API
  dir.c: rename cryptic 'which' variable to more consistent name
  dir.c: rename path_excluded() to is_path_excluded()
  dir.c: rename excluded_from_list() to is_excluded_from_list()
  dir.c: rename excluded() to is_excluded()
  dir.c: refactor is_excluded_from_list()
  dir.c: refactor is_excluded()
  dir.c: refactor is_path_excluded()
  dir.c: rename free_excludes() to clear_exclude_list()
  dir.c: use a single struct exclude_list per source of excludes
  dir.c: keep track of where patterns came from
  dir.c: provide clear_directory() for reclaiming dir_struct memory
  add.c: refactor treat_gitlinks()
  add.c: remove unused argument from validate_pathspec()
  pathspec.c: move reusable code from builtin/add.c
  pathspec.c: extract new validate_path() for reuse
  setup.c: document get_pathspec()
  Add git-check-ignore sub-command

 .gitignore|   1 +
 Documentation/git-check-ignore.txt|  89 
 Documentation/gitignore.txt   |   6 +-
 Documentation/technical/api-directory-listing.txt |  35 +-
 Makefile  |   3 +
 attr.c|   2 +-
 builtin.h |   1 +
 builtin/add.c |  84 +--
 builtin/check-ignore.c| 170 +++
 builtin/clean.c   |   3 +-
 builtin/ls-files.c|  11 +-
 command-list.txt  |   1 +
 contrib/completion/git-completion.bash|   1 +
 dir.c | 243 +++--
 dir.h |  87 +++-
 git.c |   1 +
 pathspec.c| 107 
 pathspec.h|   6 +
 setup.c   |  15 +
 t/t0008-ignores.sh| 595 ++
 unpack-trees.c|  14 +-
 21 files changed, 1305 insertions(+), 170 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/t0008-ignores.sh

-- 
1.7.11.2.249.g31c7954

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


[PATCH v3 01/19] api-directory-listing.txt: update to match code

2012-12-26 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
---
For some reason this patch was dropped from the v2 series when it was
applied to the 'pu' branch.

 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.11.2.249.g31c7954

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


[PATCH v3 02/19] Improve documentation and comments regarding directory traversal API

2012-12-26 Thread Adam Spiers
From the perspective of a newcomer to the codebase, the directory
traversal API has a few potentially confusing properties.  These
comments clarify a few key aspects and will hopefully make it easier
to understand for other newcomers in the future.

Signed-off-by: Adam Spiers g...@adamspiers.org
---
For some reason this patch was dropped from the v2 series when it was
applied to the 'pu' branch.

 Documentation/technical/api-directory-listing.txt |  9 +---
 dir.c |  8 ++-
 dir.h | 26 +--
 3 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/Documentation/technical/api-directory-listing.txt 
b/Documentation/technical/api-directory-listing.txt
index 0356d25..944fc39 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -9,8 +9,11 @@ Data structure
 --
 
 `struct dir_struct` structure is used to pass directory traversal
-options to the library and to record the paths discovered.  The notable
-options are:
+options to the library and to record the paths discovered.  A single
+`struct dir_struct` is used regardless of whether or not the traversal
+recursively descends into subdirectories.
+
+The notable options are:
 
 `exclude_per_dir`::
 
@@ -39,7 +42,7 @@ options are:
If set, recurse into a directory that looks like a git
directory.  Otherwise it is shown as a directory.
 
-The result of the enumeration is left in these fields::
+The result of the enumeration is left in these fields:
 
 `entries[]`::
 
diff --git a/dir.c b/dir.c
index ee8e711..89e27a6 100644
--- a/dir.c
+++ b/dir.c
@@ -2,6 +2,8 @@
  * This handles recursive filename detection with exclude
  * files, index knowledge etc..
  *
+ * See Documentation/technical/api-directory-listing.txt
+ *
  * Copyright (C) Linus Torvalds, 2005-2006
  *  Junio Hamano, 2005-2006
  */
@@ -476,6 +478,10 @@ void add_excludes_from_file(struct dir_struct *dir, const 
char *fname)
die(cannot use %s as an exclude file, fname);
 }
 
+/*
+ * Loads the per-directory exclude list for the substring of base
+ * which has a char length of baselen.
+ */
 static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
 {
struct exclude_list *el;
@@ -486,7 +492,7 @@ static void prep_exclude(struct dir_struct *dir, const char 
*base, int baselen)
(baselen + strlen(dir-exclude_per_dir) = PATH_MAX))
return; /* too long a path -- ignore */
 
-   /* Pop the ones that are not the prefix of the path being checked. */
+   /* Pop the directories that are not the prefix of the path being 
checked. */
el = dir-exclude_list[EXC_DIRS];
while ((stk = dir-exclude_stack) != NULL) {
if (stk-baselen = baselen 
diff --git a/dir.h b/dir.h
index f5c89e3..e0869bc 100644
--- a/dir.h
+++ b/dir.h
@@ -1,6 +1,8 @@
 #ifndef DIR_H
 #define DIR_H
 
+/* See Documentation/technical/api-directory-listing.txt */
+
 #include strbuf.h
 
 struct dir_entry {
@@ -13,6 +15,12 @@ struct dir_entry {
 #define EXC_FLAG_MUSTBEDIR 8
 #define EXC_FLAG_NEGATIVE 16
 
+/*
+ * Each .gitignore file will be parsed into patterns which are then
+ * appended to the relevant exclude_list (either EXC_DIRS or
+ * EXC_FILE).  exclude_lists are also used to represent the list of
+ * --exclude values passed via CLI args (EXC_CMDL).
+ */
 struct exclude_list {
int nr;
int alloc;
@@ -26,9 +34,15 @@ struct exclude_list {
} **excludes;
 };
 
+/*
+ * The contents of the per-directory exclude files are lazily read on
+ * demand and then cached in memory, one per exclude_stack struct, in
+ * order to avoid opening and parsing each one every time that
+ * directory is traversed.
+ */
 struct exclude_stack {
-   struct exclude_stack *prev;
-   char *filebuf;
+   struct exclude_stack *prev; /* the struct exclude_stack for the parent 
directory */
+   char *filebuf; /* remember pointer to per-directory exclude file 
contents so we can free() */
int baselen;
int exclude_ix;
 };
@@ -59,6 +73,14 @@ struct dir_struct {
 #define EXC_DIRS 1
 #define EXC_FILE 2
 
+   /*
+* Temporary variables which are used during loading of the
+* per-directory exclude lists.
+*
+* exclude_stack points to the top of the exclude_stack, and
+* basebuf contains the full path to the current
+* (sub)directory in the traversal.
+*/
struct exclude_stack *exclude_stack;
char basebuf[PATH_MAX];
 };
-- 
1.7.11.2.249.g31c7954

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


[PATCH v3 05/19] dir.c: rename excluded_from_list() to is_excluded_from_list()

2012-12-26 Thread Adam Spiers
Continue adopting clearer names for exclude functions.  This 'is_*'
naming pattern for functions returning booleans was discussed here:

http://thread.gmane.org/gmane.comp.version-control.git/204661/focus=204924

Also adjust their callers as necessary.

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 dir.c  | 11 ++-
 dir.h  |  4 ++--
 unpack-trees.c |  8 +---
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/dir.c b/dir.c
index f1c0abd..0800491 100644
--- a/dir.c
+++ b/dir.c
@@ -605,9 +605,9 @@ int match_pathname(const char *pathname, int pathlen,
 /* 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)
+int is_excluded_from_list(const char *pathname,
+ int pathlen, const char *basename, int *dtype,
+ struct exclude_list *el)
 {
int i;
 
@@ -654,8 +654,9 @@ static int excluded(struct dir_struct *dir, const char 
*pathname, int *dtype_p)
 
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])) {
+   switch (is_excluded_from_list(pathname, pathlen,
+ basename, dtype_p,
+ dir-exclude_list[st])) {
case 0:
return 0;
case 1:
diff --git a/dir.h b/dir.h
index c59bad8..554f87a 100644
--- a/dir.h
+++ b/dir.h
@@ -98,8 +98,8 @@ extern int within_depth(const char *name, int namelen, int 
depth, int max_depth)
 extern int fill_directory(struct dir_struct *dir, const char **pathspec);
 extern int read_directory(struct dir_struct *, const char *path, int len, 
const char **pathspec);
 
-extern int excluded_from_list(const char *pathname, int pathlen, const char 
*basename,
- int *dtype, struct exclude_list *el);
+extern int is_excluded_from_list(const char *pathname, int pathlen, const char 
*basename,
+int *dtype, struct exclude_list *el);
 struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char 
*pathname, int len);
 
 /*
diff --git a/unpack-trees.c b/unpack-trees.c
index 3ac6370..c0da094 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -836,7 +836,8 @@ static int clear_ce_flags_dir(struct cache_entry **cache, 
int nr,
 {
struct cache_entry **cache_end;
int dtype = DT_DIR;
-   int ret = excluded_from_list(prefix, prefix_len, basename, dtype, el);
+   int ret = is_excluded_from_list(prefix, prefix_len,
+   basename, dtype, el);
 
prefix[prefix_len++] = '/';
 
@@ -855,7 +856,7 @@ static int clear_ce_flags_dir(struct cache_entry **cache, 
int nr,
 * with ret (iow, we know in advance the incl/excl
 * decision for the entire directory), clear flag here without
 * calling clear_ce_flags_1(). That function will call
-* the expensive excluded_from_list() on every entry.
+* the expensive is_excluded_from_list() on every entry.
 */
return clear_ce_flags_1(cache, cache_end - cache,
prefix, prefix_len,
@@ -938,7 +939,8 @@ static int clear_ce_flags_1(struct cache_entry **cache, int 
nr,
 
/* Non-directory */
dtype = ce_to_dtype(ce);
-   ret = excluded_from_list(ce-name, ce_namelen(ce), name, 
dtype, el);
+   ret = is_excluded_from_list(ce-name, ce_namelen(ce),
+   name, dtype, el);
if (ret  0)
ret = defval;
if (ret  0)
-- 
1.7.11.2.249.g31c7954

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


[PATCH v3 07/19] dir.c: refactor is_excluded_from_list()

2012-12-26 Thread Adam Spiers
The excluded function uses a new helper function called
last_exclude_matching_from_list() 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 | 37 -
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/dir.c b/dir.c
index 8c99dc4..d1a0413 100644
--- a/dir.c
+++ b/dir.c
@@ -602,22 +602,26 @@ int match_pathname(const char *pathname, int pathlen,
return fnmatch_icase(pattern, name, FNM_PATHNAME) == 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 is_excluded_from_list(const char *pathname,
- int pathlen, const char *basename, int *dtype,
- struct exclude_list *el)
+static struct exclude *last_exclude_matching_from_list(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 *exclude = x-pattern;
-   int to_exclude = x-flags  EXC_FLAG_NEGATIVE ? 0 : 1;
int prefix = x-nowildcardlen;
 
if (x-flags  EXC_FLAG_MUSTBEDIR) {
@@ -632,7 +636,7 @@ int is_excluded_from_list(const char *pathname,
   pathlen - (basename - pathname),
   exclude, prefix, x-patternlen,
   x-flags))
-   return to_exclude;
+   return x;
continue;
}
 
@@ -640,8 +644,23 @@ int is_excluded_from_list(const char *pathname,
if (match_pathname(pathname, pathlen,
   x-base, x-baselen ? x-baselen - 1 : 0,
   exclude, prefix, x-patternlen, x-flags))
-   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 is_excluded_from_list(const char *pathname,
+ int pathlen, const char *basename, int *dtype,
+ struct exclude_list *el)
+{
+   struct exclude *exclude;
+   exclude = last_exclude_matching_from_list(pathname, pathlen, basename, 
dtype, el);
+   if (exclude)
+   return exclude-flags  EXC_FLAG_NEGATIVE ? 0 : 1;
return -1; /* undecided */
 }
 
-- 
1.7.11.2.249.g31c7954

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


[PATCH v3 06/19] dir.c: rename excluded() to is_excluded()

2012-12-26 Thread Adam Spiers
Continue adopting clearer names for exclude functions.  This is_*
naming pattern for functions returning booleans was discussed here:

http://thread.gmane.org/gmane.comp.version-control.git/204661/focus=204924

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

diff --git a/attr.c b/attr.c
index 2fc6353..5362563 100644
--- a/attr.c
+++ b/attr.c
@@ -284,7 +284,7 @@ static struct match_attr *parse_attr_line(const char *line, 
const char *src,
  * (reading the file from top to bottom), .gitattribute of the root
  * directory (again, reading the file from top to bottom) down to the
  * current directory, and then scan the list backwards to find the first match.
- * This is exactly the same as what excluded() does in dir.c to deal with
+ * This is exactly the same as what is_excluded() does in dir.c to deal with
  * .gitignore
  */
 
diff --git a/dir.c b/dir.c
index 0800491..8c99dc4 100644
--- a/dir.c
+++ b/dir.c
@@ -645,7 +645,7 @@ int is_excluded_from_list(const char *pathname,
return -1; /* undecided */
 }
 
-static int excluded(struct dir_struct *dir, const char *pathname, int *dtype_p)
+static int is_excluded(struct dir_struct *dir, const char *pathname, int 
*dtype_p)
 {
int pathlen = strlen(pathname);
int st;
@@ -695,7 +695,7 @@ int is_path_excluded(struct path_exclude_check *check,
/*
 * we allow the caller to pass namelen as an optimization; it
 * must match the length of the name, as we eventually call
-* excluded() on the whole name string.
+* is_excluded() on the whole name string.
 */
if (namelen  0)
namelen = strlen(name);
@@ -712,7 +712,7 @@ int is_path_excluded(struct path_exclude_check *check,
 
if (ch == '/') {
int dt = DT_DIR;
-   if (excluded(check-dir, path-buf, dt))
+   if (is_excluded(check-dir, path-buf, dt))
return 1;
}
strbuf_addch(path, ch);
@@ -721,7 +721,7 @@ int is_path_excluded(struct path_exclude_check *check,
/* An entry in the index; cannot be a directory with subentries */
strbuf_setlen(path, 0);
 
-   return excluded(check-dir, name, dtype);
+   return is_excluded(check-dir, name, dtype);
 }
 
 static struct dir_entry *dir_entry_new(const char *pathname, int len)
@@ -1021,7 +1021,7 @@ static enum path_treatment treat_one_path(struct 
dir_struct *dir,
  const struct path_simplify *simplify,
  int dtype, struct dirent *de)
 {
-   int exclude = excluded(dir, path-buf, dtype);
+   int exclude = is_excluded(dir, path-buf, dtype);
if (exclude  (dir-flags  DIR_COLLECT_IGNORED)
 exclude_matches_pathspec(path-buf, path-len, simplify))
dir_add_ignored(dir, path-buf, path-len);
diff --git a/dir.h b/dir.h
index 554f87a..d68a997 100644
--- a/dir.h
+++ b/dir.h
@@ -113,8 +113,8 @@ extern int match_pathname(const char *, int,
  const char *, int, int, int);
 
 /*
- * The excluded() API is meant for callers that check each level of leading
- * directory hierarchies with excluded() to avoid recursing into excluded
+ * The is_excluded() API is meant for callers that check each level of leading
+ * directory hierarchies with is_excluded() to avoid recursing into excluded
  * directories.  Callers that do not do so should use this API instead.
  */
 struct path_exclude_check {
-- 
1.7.11.2.249.g31c7954

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


[PATCH v3 08/19] dir.c: refactor is_excluded()

2012-12-26 Thread Adam Spiers
In a similar way to the previous commit, this extracts a new helper
function last_exclude_matching() which returns the last exclude_list
element which matched, or NULL if no match was found.  is_excluded()
becomes a wrapper around this, and just returns 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 | 38 +-
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/dir.c b/dir.c
index d1a0413..b9d4234 100644
--- a/dir.c
+++ b/dir.c
@@ -664,24 +664,44 @@ int is_excluded_from_list(const char *pathname,
return -1; /* undecided */
 }
 
-static int is_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 *last_exclude_matching(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 (is_excluded_from_list(pathname, pathlen,
- basename, dtype_p,
- dir-exclude_list[st])) {
-   case 0:
-   return 0;
-   case 1:
-   return 1;
-   }
+   exclude = last_exclude_matching_from_list(
+   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 is_excluded(struct dir_struct *dir, const char *pathname, int 
*dtype_p)
+{
+   struct exclude *exclude =
+   last_exclude_matching(dir, pathname, dtype_p);
+   if (exclude)
+   return exclude-flags  EXC_FLAG_NEGATIVE ? 0 : 1;
return 0;
 }
 
-- 
1.7.11.2.249.g31c7954

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


[PATCH v3 10/19] dir.c: rename free_excludes() to clear_exclude_list()

2012-12-26 Thread Adam Spiers
It is clearer to use a 'clear_' prefix for functions which empty
and deallocate the contents of a data structure without freeing
the structure itself, and a 'free_' prefix for functions which
also free the structure itself.

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

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

diff --git a/dir.c b/dir.c
index 16e10b0..41f141c 100644
--- a/dir.c
+++ b/dir.c
@@ -400,7 +400,11 @@ static void *read_skip_worktree_file_from_index(const char 
*path, size_t *size)
return data;
 }
 
-void free_excludes(struct exclude_list *el)
+/*
+ * Frees memory within el which was allocated for exclude patterns and
+ * the file buffer.  Does not free el itself.
+ */
+void clear_exclude_list(struct exclude_list *el)
 {
int i;
 
diff --git a/dir.h b/dir.h
index dcb1ad3..5664ba8 100644
--- a/dir.h
+++ b/dir.h
@@ -135,7 +135,7 @@ extern void add_excludes_from_file(struct dir_struct *, 
const char *fname);
 extern void parse_exclude_pattern(const char **string, int *patternlen, int 
*flags, int *nowildcardlen);
 extern void add_exclude(const char *string, const char *base,
int baselen, struct exclude_list *el);
-extern void free_excludes(struct exclude_list *el);
+extern void clear_exclude_list(struct exclude_list *el);
 extern int file_exists(const char *);
 
 extern int is_inside_dir(const char *dir);
diff --git a/unpack-trees.c b/unpack-trees.c
index c0da094..ad621d9 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1153,7 +1153,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, 
struct unpack_trees_options
*o-dst_index = o-result;
 
 done:
-   free_excludes(el);
+   clear_exclude_list(el);
if (o-path_exclude_check) {
path_exclude_check_clear(o-path_exclude_check);
free(o-path_exclude_check);
-- 
1.7.11.2.249.g31c7954

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


[PATCH v3 12/19] dir.c: keep track of where patterns came from

2012-12-26 Thread Adam Spiers
For exclude patterns read in from files, the filename is stored in the
exclude list, and the originating line number is stored in the
individual exclude (counting starting at 1).

For exclude patterns provided on the command line, a string describing
the source of the patterns is stored in the exclude list, and the
sequence number assigned to each exclude pattern 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|  4 ++--
 builtin/ls-files.c |  5 +++--
 dir.c  | 26 --
 dir.h  | 21 +++--
 4 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index bd18b88..72d2876 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -97,10 +97,10 @@ int cmd_clean(int argc, const char **argv, const char 
*prefix)
if (!ignored)
setup_standard_excludes(dir);
 
-   add_exclude_list(dir, EXC_CMDL);
+   add_exclude_list(dir, EXC_CMDL, --exclude option);
for (i = 0; i  exclude_list.nr; i++)
add_exclude(exclude_list.items[i].string, , 0,
-   dir.exclude_list_groups[EXC_CMDL].ary[0]);
+   dir.exclude_list_groups[EXC_CMDL].ary[0], -(i+1));
 
pathspec = get_pathspec(prefix, argv);
 
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index c448e06..d4e55c2 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;
 
 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_group *group = opt-value;
 
exc_given = 1;
-   add_exclude(arg, , 0, group-ary[0]);
+   add_exclude(arg, , 0, group-ary[0], --exclude_args);
 
return 0;
 }
@@ -524,7 +525,7 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
if (read_cache()  0)
die(index file corrupt);
 
-   add_exclude_list(dir, EXC_CMDL);
+   add_exclude_list(dir, EXC_CMDL, --exclude option);
argc = parse_options(argc, argv, prefix, builtin_ls_files_options,
ls_files_usage, 0);
if (show_tag || show_valid_bit) {
diff --git a/dir.c b/dir.c
index 3a9d89b..aefe2bb 100644
--- a/dir.c
+++ b/dir.c
@@ -349,7 +349,7 @@ void parse_exclude_pattern(const char **pattern,
 }
 
 void add_exclude(const char *string, const char *base,
-int baselen, struct exclude_list *el)
+int baselen, struct exclude_list *el, int srcpos)
 {
struct exclude *x;
int patternlen;
@@ -373,8 +373,10 @@ void add_exclude(const char *string, const char *base,
x-base = base;
x-baselen = baselen;
x-flags = flags;
+   x-srcpos = srcpos;
ALLOC_GROW(el-excludes, el-nr + 1, el-alloc);
el-excludes[el-nr++] = x;
+   x-el = el;
 }
 
 static void *read_skip_worktree_file_from_index(const char *path, size_t *size)
@@ -425,7 +427,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;
 
@@ -467,15 +469,17 @@ 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, lineno);
}
+   lineno++;
entry = buf + i + 1;
}
}
return 0;
 }
 
-struct exclude_list *add_exclude_list(struct dir_struct *dir, int group_type)
+struct exclude_list *add_exclude_list(struct dir_struct *dir,
+ int group_type, const char *src)
 {
struct exclude_list *el;
struct exclude_list_group *group;
@@ -484,6 +488,7 @@ struct exclude_list *add_exclude_list(struct dir_struct 
*dir, int group_type)
ALLOC_GROW(group-ary, group-nr + 1, group-alloc);
el = group-ary[group-nr++];
memset(el, 0, sizeof(*el));
+   el-src = src;
return el;
 }
 
@@ -493,7 +498,7 @@ struct exclude_list *add_exclude_list(struct dir_struct 
*dir, int group_type)
 void add_excludes_from_file(struct dir_struct *dir, const char *fname)
 {
struct exclude_list *el;
-   el = 

[PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes

2012-12-26 Thread Adam Spiers
Previously each exclude_list could potentially contain patterns
from multiple sources.  For example dir-exclude_list[EXC_FILE]
would typically contain patterns from .git/info/exclude and
core.excludesfile, and dir-exclude_list[EXC_DIRS] could contain
patterns from multiple per-directory .gitignore files during
directory traversal (i.e. when dir-exclude_stack was more than
one item deep).

We split these composite exclude_lists up into three groups of
exclude_lists (EXC_CMDL / EXC_DIRS / EXC_FILE as before), so that each
exclude_list now contains patterns from a single source.  This will
allow us to cleanly track the origin of each pattern simply by adding
a src field to struct exclude_list, rather than to struct exclude,
which would make memory management of the source string tricky in the
EXC_DIRS case where its contents are dynamically generated.

Similarly, by moving the filebuf member from struct exclude_stack to
struct exclude_list, it allows us to track and subsequently free
memory buffers allocated during the parsing of all exclude files,
rather than only tracking buffers allocated for files in the EXC_DIRS
group.

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 Documentation/technical/api-directory-listing.txt | 12 +++--
 builtin/clean.c   |  3 +-
 builtin/ls-files.c|  8 +--
 dir.c | 60 ---
 dir.h | 36 ++
 unpack-trees.c|  2 +-
 6 files changed, 83 insertions(+), 38 deletions(-)

diff --git a/Documentation/technical/api-directory-listing.txt 
b/Documentation/technical/api-directory-listing.txt
index 944fc39..fa9c8ae 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -67,11 +67,13 @@ marked. If you to exclude files, make sure you have loaded 
index first.
 * Prepare `struct dir_struct dir` and clear it with `memset(dir, 0,
   sizeof(dir))`.
 
-* Call `add_exclude()` to add single exclude pattern,
-  `add_excludes_from_file()` to add patterns from a file
-  (e.g. `.git/info/exclude`), and/or set `dir.exclude_per_dir`.  A
-  short-hand function `setup_standard_excludes()` can be used to set up
-  the standard set of exclude settings.
+* To add single exclude pattern, call `add_exclude_list()` and then
+  `add_exclude()`.
+
+* To add patterns from a file (e.g. `.git/info/exclude`), call
+  `add_excludes_from_file()` , and/or set `dir.exclude_per_dir`.  A
+  short-hand function `setup_standard_excludes()` can be used to set
+  up the standard set of exclude settings.
 
 * Set options described in the Data Structure section above.
 
diff --git a/builtin/clean.c b/builtin/clean.c
index 0c7b3d0..bd18b88 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char 
*prefix)
if (!ignored)
setup_standard_excludes(dir);
 
+   add_exclude_list(dir, EXC_CMDL);
for (i = 0; i  exclude_list.nr; i++)
add_exclude(exclude_list.items[i].string, , 0,
-   dir.exclude_list[EXC_CMDL]);
+   dir.exclude_list_groups[EXC_CMDL].ary[0]);
 
pathspec = get_pathspec(prefix, argv);
 
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index ef7f99a..c448e06 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -420,10 +420,10 @@ static int option_parse_z(const struct option *opt,
 static int option_parse_exclude(const struct option *opt,
const char *arg, int unset)
 {
-   struct exclude_list *list = opt-value;
+   struct exclude_list_group *group = opt-value;
 
exc_given = 1;
-   add_exclude(arg, , 0, list);
+   add_exclude(arg, , 0, group-ary[0]);
 
return 0;
 }
@@ -488,7 +488,8 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
show unmerged files in the output),
OPT_BOOLEAN(0, resolve-undo, show_resolve_undo,
show resolve-undo information),
-   { OPTION_CALLBACK, 'x', exclude, dir.exclude_list[EXC_CMDL], 
pattern,
+   { OPTION_CALLBACK, 'x', exclude,
+   dir.exclude_list_groups[EXC_CMDL], pattern,
skip files matching pattern,
0, option_parse_exclude },
{ OPTION_CALLBACK, 'X', exclude-from, dir, file,
@@ -523,6 +524,7 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
if (read_cache()  0)
die(index file corrupt);
 
+   add_exclude_list(dir, EXC_CMDL);
argc = parse_options(argc, argv, prefix, builtin_ls_files_options,
ls_files_usage, 0);
if (show_tag || show_valid_bit) {
diff --git a/dir.c b/dir.c
index 41f141c..3a9d89b 

[PATCH v3 13/19] dir.c: provide clear_directory() for reclaiming dir_struct memory

2012-12-26 Thread Adam Spiers
By the end of a directory traversal, a dir_struct instance will
typically contains pointers to various data structures on the heap.
clear_directory() provides a convenient way to reclaim that memory.

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 Documentation/technical/api-directory-listing.txt |  2 ++
 dir.c | 30 +++
 dir.h |  1 +
 3 files changed, 33 insertions(+)

diff --git a/Documentation/technical/api-directory-listing.txt 
b/Documentation/technical/api-directory-listing.txt
index fa9c8ae..fbceb62 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -81,4 +81,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 aefe2bb..0d1b7e9 100644
--- a/dir.c
+++ b/dir.c
@@ -1555,3 +1555,33 @@ void free_pathspec(struct pathspec *pathspec)
free(pathspec-items);
pathspec-items = NULL;
 }
+
+/*
+ * Frees memory within dir which was allocated for exclude lists and
+ * the exclude_stack.  Does not free dir itself.
+ */
+void clear_directory(struct dir_struct *dir)
+{
+   int i, j;
+   struct exclude_list_group *group;
+   struct exclude_list *el;
+   struct exclude_stack *stk;
+
+   for (i = EXC_CMDL; i = EXC_FILE; i++) {
+   group = dir-exclude_list_groups[i];
+   for (j = 0; j  group-nr; j++) {
+   el = group-ary[j];
+   if (i == EXC_DIRS)
+   free((char *)el-src);
+   clear_exclude_list(el);
+   }
+   free(group-ary);
+   }
+
+   stk = dir-exclude_stack;
+   while (stk) {
+   struct exclude_stack *prev = stk-prev;
+   free(stk);
+   stk = prev;
+   }
+}
diff --git a/dir.h b/dir.h
index f91770a..286de4e 100644
--- a/dir.h
+++ b/dir.h
@@ -169,6 +169,7 @@ extern void parse_exclude_pattern(const char **string, int 
*patternlen, int *fla
 extern void add_exclude(const char *string, const char *base,
int baselen, struct exclude_list *el, int srcpos);
 extern void clear_exclude_list(struct exclude_list *el);
+extern void clear_directory(struct dir_struct *dir);
 extern int file_exists(const char *);
 
 extern int is_inside_dir(const char *dir);
-- 
1.7.11.2.249.g31c7954

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


[PATCH v3 17/19] pathspec.c: extract new validate_path() for reuse

2012-12-26 Thread Adam Spiers
This will be reused by a new git check-ignore command.

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 pathspec.c | 20 ++--
 pathspec.h |  1 +
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/pathspec.c b/pathspec.c
index 8aea0d2..6724121 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -77,9 +77,20 @@ void treat_gitlinks(const char **pathspec)
 }
 
 /*
+ * Dies if the given path refers to a file inside a symlinked
+ * directory.
+ */
+void validate_path(const char *path, const char *prefix)
+{
+   if (has_symlink_leading_path(path, strlen(path))) {
+   int len = prefix ? strlen(prefix) : 0;
+   die(_('%s' is beyond a symbolic link), path + len);
+   }
+}
+
+/*
  * Normalizes argv relative to prefix, via get_pathspec(), and then
- * dies if any path in the normalized list refers to a file inside a
- * symlinked directory.
+ * runs validate_path() on each path in the normalized list.
  */
 const char **validate_pathspec(const char **argv, const char *prefix)
 {
@@ -88,10 +99,7 @@ const char **validate_pathspec(const char **argv, const char 
*prefix)
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);
-   }
+   validate_path(*p, prefix);
}
}
 
diff --git a/pathspec.h b/pathspec.h
index 8bb670b..c251441 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -2,4 +2,5 @@ extern char *find_used_pathspec(const char **pathspec);
 extern void fill_pathspec_matches(const char **pathspec, char *seen, int 
specs);
 extern const char *treat_gitlink(const char *path);
 extern void treat_gitlinks(const char **pathspec);
+extern void validate_path(const char *path, const char *prefix);
 extern const char **validate_pathspec(const char **argv, const char *prefix);
-- 
1.7.11.2.249.g31c7954

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


[PATCH v3 19/19] Add git-check-ignore sub-command

2012-12-26 Thread Adam Spiers
This works in a similar manner to git-check-attr.

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
---
Since v2, the missing documentation has been fixed, and the erroneous
tweak to t/t9902-completion.sh has been removed.

 .gitignore|   1 +
 Documentation/git-check-ignore.txt|  89 
 Documentation/gitignore.txt   |   6 +-
 Documentation/technical/api-directory-listing.txt |   2 +-
 Makefile  |   1 +
 builtin.h |   1 +
 builtin/check-ignore.c| 170 +++
 command-list.txt  |   1 +
 contrib/completion/git-completion.bash|   1 +
 git.c |   1 +
 t/t0008-ignores.sh| 595 ++
 11 files changed, 865 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/git-check-ignore.txt
 create mode 100644 builtin/check-ignore.c
 create mode 100755 t/t0008-ignores.sh

diff --git a/.gitignore b/.gitignore
index f1acd3e..20ef4e8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -19,6 +19,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..854e4d0
--- /dev/null
+++ b/Documentation/git-check-ignore.txt
@@ -0,0 +1,89 @@
+git-check-ignore(1)
+===
+
+NAME
+
+git-check-ignore - Debug gitignore / exclude files
+
+
+SYNOPSIS
+
+[verse]
+'git check-ignore' [options] pathname...
+'git check-ignore' [options] --stdin  list-of-paths
+
+DESCRIPTION
+---
+
+For each pathname given via the command-line or from a file via
+`--stdin`, show the pattern from .gitignore (or other input files to
+the exclude mechanism) that decides if the pathname is excluded or
+included.  Later patterns within a file take precedence over earlier
+ones.
+
+OPTIONS
+---
+-q, --quiet::
+   Don't output anything, just set exit status.  This is only
+   valid with a single pathname.
+
+-v, --verbose::
+   Also output details about the matching pattern (if any)
+   for each given pathname.
+
+--stdin::
+   Read file names from stdin instead of from the command-line.
+
+-z::
+   The output format is modified to be machine-parseable (see
+   below).  If `--stdin` is also given, input paths are separated
+   with a NUL character instead of a linefeed character.
+
+OUTPUT
+--
+
+By default, any of the given pathnames which match an ignore pattern
+will be output, one per line.  If no pattern matches a given path,
+nothing will be output for that path; this means that path will not be
+ignored.
+
+If `--verbose` is specified, the output is a series of lines of the form:
+
+source COLON linenum COLON pattern HT pathname
+
+pathname is the path of a file being queried, pattern is the
+matching pattern, source is the pattern's source file, and linenum
+is the line number of the pattern within that source.  If the pattern
+contained a `!` prefix or `/` suffix, it will be preserved in the
+output.  source will be an absolute path when referring to the file
+configured by `core.excludesfile`, or relative to the repository root
+when referring to `.git/info/exclude` or a per-directory exclude file.
+
+If `-z` is specified, the pathnames in the output are delimited by the
+null character; if `--verbose` is also specified then null characters
+are also used instead of colons and hard tabs:
+
+source NULL linenum NULL pattern NULL pathname NULL
+
+
+EXIT STATUS
+---
+
+0::
+   One or more of the provided paths is ignored.
+
+1::
+   None of the provided paths are ignored.
+
+128::
+   A fatal error was encountered.
+
+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 2e7328b..f401b8c 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -153,8 +153,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/Documentation/technical/api-directory-listing.txt 
b/Documentation/technical/api-directory-listing.txt
index fbceb62..9d3e352 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -81,6 +81,6 @@ marked. If you to exclude files, make sure you have loaded 
index 

[PATCH v3 18/19] setup.c: document get_pathspec()

2012-12-26 Thread Adam Spiers
Since we have just created a new pathspec-handling library, now is a
good time to add some comments explaining get_pathspec().

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 setup.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/setup.c b/setup.c
index 7663a4c..03d6d5c 100644
--- a/setup.c
+++ b/setup.c
@@ -249,6 +249,21 @@ static const char *prefix_pathspec(const char *prefix, int 
prefixlen, const char
return prefix_path(prefix, prefixlen, copyfrom);
 }
 
+/*
+ * prefix - a path relative to the root of the working tree
+ * pathspec - a list of paths underneath the prefix path
+ *
+ * Iterates over pathspec, prepending each path with prefix,
+ * and return the resulting list.
+ *
+ * If pathspec is empty, return a singleton list containing prefix.
+ *
+ * If pathspec and prefix are both empty, return an empty list.
+ *
+ * This is typically used by built-in commands such as add.c, in order
+ * to normalize argv arguments provided to the built-in into a list of
+ * paths to process, all relative to the root of the working tree.
+ */
 const char **get_pathspec(const char *prefix, const char **pathspec)
 {
const char *entry = *pathspec;
-- 
1.7.11.2.249.g31c7954

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


[PATCH v3 16/19] pathspec.c: move reusable code from builtin/add.c

2012-12-26 Thread Adam Spiers
Extract the following functions from builtin/add.c to pathspec.c, in
preparation for reuse by a new git check-ignore command:

  - fill_pathspec_matches()
  - find_used_pathspec()
  - treat_gitlink()
  - treat_gitlinks()
  - validate_pathspec()

The functions being extracted are not changed in any way, except
removal of the 'static' qualifier.

Also add a comment documenting validate_pathspec().

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 Makefile  |  2 ++
 builtin/add.c | 92 +-
 pathspec.c| 99 +++
 pathspec.h|  5 +++
 4 files changed, 107 insertions(+), 91 deletions(-)
 create mode 100644 pathspec.c
 create mode 100644 pathspec.h

diff --git a/Makefile b/Makefile
index 13293d3..48facad 100644
--- a/Makefile
+++ b/Makefile
@@ -645,6 +645,7 @@ LIB_H += pack-refs.h
 LIB_H += pack-revindex.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
@@ -758,6 +759,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 1ba2a86..d3bae78 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,47 +121,6 @@ static char *prune_directory(struct dir_struct *dir, const 
char **pathspec, int
return seen;
 }
 
-/*
- * Check whether path refers to a submodule, or something inside a
- * submodule.  If the former, returns the path with any trailing slash
- * stripped.  If the latter, dies with an error message.
- */
-const char *treat_gitlink(const char *path)
-{
-   int i, path_len = strlen(path);
-   for (i = 0; i  active_nr; i++) {
-   struct cache_entry *ce = active_cache[i];
-   if (S_ISGITLINK(ce-ce_mode)) {
-   int ce_len = ce_namelen(ce);
-   if (path_len = ce_len || path[ce_len] != '/' ||
-   memcmp(ce-name, path, ce_len))
-   /* path does not refer to this
-* submodule or anything inside it */
-   continue;
-   if (path_len == ce_len + 1) {
-   /* path refers to submodule;
-* strip trailing slash */
-   return xstrndup(ce-name, ce_len);
-   } else {
-   die (_(Path '%s' is in submodule '%.*s'),
-path, ce_len, ce-name);
-   }
-   }
-   }
-   return path;
-}
-
-void treat_gitlinks(const char **pathspec)
-{
-   int i;
-
-   if (!pathspec || !*pathspec)
-   return;
-
-   for (i = 0; pathspec[i]; i++)
-   pathspec[i] = treat_gitlink(pathspec[i]);
-}
-
 static void refresh(int verbose, const char **pathspec)
 {
char *seen;
@@ -211,23 +138,6 @@ static void refresh(int verbose, const char **pathspec)
 free(seen);
 }
 
-static const char **validate_pathspec(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))) {
-  

[PATCH v3 15/19] add.c: remove unused argument from validate_pathspec()

2012-12-26 Thread Adam Spiers
The 'argc' argument passed to validate_pathspec() was never used.

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 builtin/add.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 856d232..1ba2a86 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -211,7 +211,7 @@ static void refresh(int verbose, const char **pathspec)
 free(seen);
 }
 
-static const char **validate_pathspec(int argc, const char **argv, const char 
*prefix)
+static const char **validate_pathspec(const char **argv, const char *prefix)
 {
const char **pathspec = get_pathspec(prefix, argv);
 
@@ -262,7 +262,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(argv, prefix);
if (!pathspec)
return -1;
}
@@ -428,7 +428,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
fprintf(stderr, _(Maybe you wanted to say 'git add .'?\n));
return 0;
}
-   pathspec = validate_pathspec(argc, argv, prefix);
+   pathspec = validate_pathspec(argv, prefix);
 
if (read_cache()  0)
die(_(index file corrupt));
-- 
1.7.11.2.249.g31c7954

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


[PATCH v3 14/19] add.c: refactor treat_gitlinks()

2012-12-26 Thread Adam Spiers
Extract the body of the for loop in treat_gitlinks() into a separate
treat_gitlink() function so that it can be reused elsewhere.  This
paves the way for a new check-ignore sub-command.

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 builtin/add.c | 50 --
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index c689f37..856d232 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -153,31 +153,45 @@ static char *prune_directory(struct dir_struct *dir, 
const char **pathspec, int
return seen;
 }
 
-static void treat_gitlinks(const char **pathspec)
+/*
+ * Check whether path refers to a submodule, or something inside a
+ * submodule.  If the former, returns the path with any trailing slash
+ * stripped.  If the latter, dies with an error message.
+ */
+const char *treat_gitlink(const char *path)
 {
-   int i;
-
-   if (!pathspec || !*pathspec)
-   return;
-
+   int i, path_len = strlen(path);
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);
+   int ce_len = ce_namelen(ce);
+   if (path_len = ce_len || path[ce_len] != '/' ||
+   memcmp(ce-name, path, ce_len))
+   /* path does not refer to this
+* submodule or anything inside it */
+   continue;
+   if (path_len == ce_len + 1) {
+   /* path refers to submodule;
+* strip trailing slash */
+   return xstrndup(ce-name, ce_len);
+   } else {
+   die (_(Path '%s' is in submodule '%.*s'),
+path, ce_len, ce-name);
}
}
}
+   return path;
+}
+
+void treat_gitlinks(const char **pathspec)
+{
+   int i;
+
+   if (!pathspec || !*pathspec)
+   return;
+
+   for (i = 0; pathspec[i]; i++)
+   pathspec[i] = treat_gitlink(pathspec[i]);
 }
 
 static void refresh(int verbose, const char **pathspec)
-- 
1.7.11.2.249.g31c7954

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


[PATCH v3 09/19] dir.c: refactor is_path_excluded()

2012-12-26 Thread Adam Spiers
In a similar way to the previous commit, this extracts a new helper
function last_exclude_matching_path() which return the last
exclude_list element which matched, or NULL if no match was found.
is_path_excluded() becomes a wrapper around this, and just returns 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 | 47 ++-
 dir.h |  3 +++
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/dir.c b/dir.c
index b9d4234..16e10b0 100644
--- a/dir.c
+++ b/dir.c
@@ -709,6 +709,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);
 }
 
@@ -718,18 +719,21 @@ 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 is_path_excluded(struct path_exclude_check *check,
-const char *name, int namelen, int *dtype)
+struct exclude *last_exclude_matching_path(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
@@ -739,11 +743,17 @@ int is_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;
+   return check-exclude;
 
strbuf_setlen(path, 0);
for (i = 0; name[i]; i++) {
@@ -751,8 +761,12 @@ int is_path_excluded(struct path_exclude_check *check,
 
if (ch == '/') {
int dt = DT_DIR;
-   if (is_excluded(check-dir, path-buf, dt))
-   return 1;
+   exclude = last_exclude_matching(check-dir,
+   path-buf, dt);
+   if (exclude) {
+   check-exclude = exclude;
+   return exclude;
+   }
}
strbuf_addch(path, ch);
}
@@ -760,7 +774,22 @@ int is_path_excluded(struct path_exclude_check *check,
/* An entry in the index; cannot be a directory with subentries */
strbuf_setlen(path, 0);
 
-   return is_excluded(check-dir, name, dtype);
+   return last_exclude_matching(check-dir, name, dtype);
+}
+
+/*
+ * 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.
+ */
+int is_path_excluded(struct path_exclude_check *check,
+ const char *name, int namelen, int *dtype)
+{
+   struct exclude *exclude =
+   last_exclude_matching_path(check, name, namelen, dtype);
+   if (exclude)
+   return exclude-flags  EXC_FLAG_NEGATIVE ? 0 : 1;
+   return 0;
 }
 
 static struct dir_entry *dir_entry_new(const char *pathname, int len)
diff --git a/dir.h b/dir.h
index d68a997..dcb1ad3 100644
--- a/dir.h
+++ b/dir.h
@@ -119,10 +119,13 @@ extern int match_pathname(const char *, int,
  */
 struct path_exclude_check {
struct dir_struct *dir;
+   struct exclude *exclude;
struct strbuf path;
 };
 extern void path_exclude_check_init(struct path_exclude_check *, struct 
dir_struct *);
 extern void path_exclude_check_clear(struct path_exclude_check *);
+extern struct exclude *last_exclude_matching_path(struct 

[PATCH v3 03/19] dir.c: rename cryptic 'which' variable to more consistent name

2012-12-26 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 89e27a6..f31aa59 100644
--- a/dir.c
+++ b/dir.c
@@ -349,7 +349,7 @@ void parse_exclude_pattern(const char **pattern,
 }
 
 void add_exclude(const char *string, const char *base,
-int baselen, struct exclude_list *which)
+int baselen, struct exclude_list *el)
 {
struct exclude *x;
int patternlen;
@@ -373,8 +373,8 @@ void add_exclude(const char *string, const char *base,
x-base = base;
x-baselen = baselen;
x-flags = flags;
-   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)
@@ -416,7 +416,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;
@@ -463,7 +463,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 e0869bc..680c1eb 100644
--- a/dir.h
+++ b/dir.h
@@ -127,11 +127,11 @@ 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 parse_exclude_pattern(const char **string, int *patternlen, int 
*flags, int *nowildcardlen);
 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.11.2.249.g31c7954

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


[PATCH v3 04/19] dir.c: rename path_excluded() to is_path_excluded()

2012-12-26 Thread Adam Spiers
Start adopting clearer names for exclude functions.  This 'is_*'
naming pattern for functions returning booleans was agreed here:

http://thread.gmane.org/gmane.comp.version-control.git/204661/focus=204924

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 builtin/add.c  | 2 +-
 builtin/ls-files.c | 2 +-
 dir.c  | 4 ++--
 dir.h  | 2 +-
 unpack-trees.c | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 89dce56..c689f37 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -453,7 +453,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 !file_exists(pathspec[i])) {
if (ignore_missing) {
int dtype = DT_UNKNOWN;
-   if (path_excluded(check, pathspec[i], 
-1, dtype))
+   if (is_path_excluded(check, 
pathspec[i], -1, dtype))
dir_add_ignored(dir, 
pathspec[i], strlen(pathspec[i]));
} else
die(_(pathspec '%s' did not match any 
files),
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 31b3f2d..ef7f99a 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -203,7 +203,7 @@ static void show_ru_info(void)
 static int ce_excluded(struct path_exclude_check *check, struct cache_entry 
*ce)
 {
int dtype = ce_to_dtype(ce);
-   return path_excluded(check, ce-name, ce_namelen(ce), dtype);
+   return is_path_excluded(check, ce-name, ce_namelen(ce), dtype);
 }
 
 static void show_files(struct dir_struct *dir)
diff --git a/dir.c b/dir.c
index f31aa59..f1c0abd 100644
--- a/dir.c
+++ b/dir.c
@@ -685,8 +685,8 @@ void path_exclude_check_clear(struct path_exclude_check 
*check)
  * 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)
+int is_path_excluded(struct path_exclude_check *check,
+const char *name, int namelen, int *dtype)
 {
int i;
struct strbuf *path = check-path;
diff --git a/dir.h b/dir.h
index 680c1eb..c59bad8 100644
--- a/dir.h
+++ b/dir.h
@@ -123,7 +123,7 @@ struct path_exclude_check {
 };
 extern void path_exclude_check_init(struct path_exclude_check *, struct 
dir_struct *);
 extern void path_exclude_check_clear(struct path_exclude_check *);
-extern int path_excluded(struct path_exclude_check *, const char *, int 
namelen, int *dtype);
+extern int is_path_excluded(struct path_exclude_check *, const char *, int 
namelen, int *dtype);
 
 
 extern int add_excludes_from_file_to_list(const char *fname, const char *base, 
int baselen,
diff --git a/unpack-trees.c b/unpack-trees.c
index 33a5819..3ac6370 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1372,7 +1372,7 @@ static int check_ok_to_remove(const char *name, int len, 
int dtype,
return 0;
 
if (o-dir 
-   path_excluded(o-path_exclude_check, name, -1, dtype))
+   is_path_excluded(o-path_exclude_check, name, -1, dtype))
/*
 * ce-name is explicitly excluded, so it is Ok to
 * overwrite it.
-- 
1.7.11.2.249.g31c7954

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


Re: [PATCH v2] wt-status: Show ignored files in untracked dirs

2012-12-26 Thread Junio C Hamano
Antoine Pelisse apeli...@gmail.com writes:

 When looking for ignored files, we do not recurse into untracked
 directory, and simply consider the directory ignored status.

When asked to show ignored ones, instead of listing all ignored
files in such a directory, we just say everything in this directory
is ignored?

That sounds like a more desirable behaviour, than listing everything
there, at least to me, but perhaps I am missing something.

Care to add a test for this new behaviour?

 As a consequence, we don't see ignored files in those directories.

 Change that behavior by recursing into untracked directories, if not
 ignored themselves, searching for ignored files.

 Signed-off-by: Antoine Pelisse apeli...@gmail.com
 ---
 Actually, the previous patch breaks the case where the directory is ignored.
 This one should fix both issues.
 Let me know if you see any other use case that could be an issue.

  dir.c   | 7 +++
  wt-status.c | 2 +-
  2 files changed, 8 insertions(+), 1 deletion(-)

 diff --git a/dir.c b/dir.c
 index 5a83aa7..2863799 100644
 --- a/dir.c
 +++ b/dir.c
 @@ -1042,6 +1042,13 @@ static enum path_treatment treat_one_path(struct 
 dir_struct *dir,
   return path_ignored;
   }

 + /*
 +  * Don't recurse into ignored directories when looking for
 +  * ignored files, but still show the directory as ignored.
 +  */
 + if (exclude  (dir-flags  DIR_SHOW_IGNORED)  dtype == DT_DIR)
 + return path_handled;
 +
   switch (dtype) {
   default:
   return path_ignored;
 diff --git a/wt-status.c b/wt-status.c
 index 2a9658b..7c41488 100644
 --- a/wt-status.c
 +++ b/wt-status.c
 @@ -516,7 +516,7 @@ static void wt_status_collect_untracked(struct wt_status 
 *s)

   if (s-show_ignored_files) {
   dir.nr = 0;
 - dir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;
 + dir.flags = DIR_SHOW_IGNORED;
   fill_directory(dir, s-pathspec);
   for (i = 0; i  dir.nr; i++) {
   struct dir_entry *ent = dir.entries[i];
 --
 1.8.1.rc3.12.g8864e38
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] wt-status: Show ignored files in untracked dirs

2012-12-26 Thread Jeff King
On Wed, Dec 26, 2012 at 06:37:55PM -0800, Junio C Hamano wrote:

 Antoine Pelisse apeli...@gmail.com writes:
 
  When looking for ignored files, we do not recurse into untracked
  directory, and simply consider the directory ignored status.
 
 When asked to show ignored ones, instead of listing all ignored
 files in such a directory, we just say everything in this directory
 is ignored?
 
 That sounds like a more desirable behaviour, than listing everything
 there, at least to me, but perhaps I am missing something.

I do not use this feature myself, but I would think that it should
respect the same DIR_SHOW_OTHER_DIRECTORIES flag (or a parallel flag)
that we already hook into --untracked={all,normal}.

IOW, given:

  git init
  mkdir untracked ignored
  untracked/file
  ignored/file
  echo ignored .git/info/exclude

I would expect:

  $ git status --short --ignored --untracked=normal
  ?? untracked/
  !! ignored/

  $ git status --short --ignored --untracked=all
  ?? untracked/file
  !! ignored/file

I do not know if anybody cares about the distinction, but optionally we
could give --ignored its own selector, like:

  $ git status --short --ignored=all --untracked=normal
  ?? untracked/
  !! ignored/file

where obviously it would default to none (whereas untracked defaults
to normal). But the behavior with Antoine's patch is:

  $ git status --short --ignored --untracked=normal
  ?? untracked/
  !! ignored

  $ git status --short --ignored --untracked=all
  ?? untracked/file
  !! ignored

which seems wrong to me for two reasons:

  1. It does not recurse for ignored but untracked entries. Neither does
 the current code, but I think it should.

  2. It loses the trailing slash from the ignored directory in both
 cases (which is printed by the current code).

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


Re: [PATCH v2] wt-status: Show ignored files in untracked dirs

2012-12-26 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 IOW, given:

   git init
   mkdir untracked ignored
   untracked/file
   ignored/file
   echo ignored .git/info/exclude

 I would expect:

   $ git status --short --ignored --untracked=normal
   ?? untracked/
   !! ignored/

Sensible.

   $ git status --short --ignored --untracked=all
   ?? untracked/file
   !! ignored/file

Again sensible; OK, --untracked=all is what I was missing.

 I do not know if anybody cares about the distinction, but optionally we
 could give --ignored its own selector, like:

   $ git status --short --ignored=all --untracked=normal
   ?? untracked/
   !! ignored/file

 where obviously it would default to none (whereas untracked defaults
 to normal).

We could just say the selector for the ignored implicitly follows
what is given for --untracked, if we don't care.

 But the behavior with Antoine's patch is:

   $ git status --short --ignored --untracked=normal
   ?? untracked/
   !! ignored

   $ git status --short --ignored --untracked=all
   ?? untracked/file
   !! ignored

 which seems wrong to me for two reasons:

   1. It does not recurse for ignored but untracked entries. Neither does
  the current code, but I think it should.

   2. It loses the trailing slash from the ignored directory in both
  cases (which is printed by the current code).

Nicely analysed.  Perhaps we would want new test pieces to define
the behaviour we want to see first?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 00/19] new git check-ignore sub-command

2012-12-26 Thread Michael Leal
Adam Spiers git at 
adamspiers.org writes:

 
 This v3 re-roll of my check-
ignore series is a reasonably 
substantial
 revamp over v2, and applies on 
top of Junio's current
 nd/attr-match-optim-more 
branch (82dce998c202).
 
 All feedback and patches from 
the v2 series has been 
incorporated, and
 several other improvements 
too, including:
 
   - composite exclude_lists 
have been split up into
 exclude_list_groups each 
containing one exclude_list per 
source
 
   - smaller commits for easier 
review
 
   - minor memory leaks have 
been fixed and verified via 
valgrind
 
   - t0007-ignores.sh has been 
renumbered to t0008-ignores.sh 
to avoid
 a conflict with t0007-git-
var.sh
 
   - improvements to 
documentation and comments
 
 For reference, the v2 series 
was announced here:
 
 http://thread.gmane.org/
gmane.comp.version-
control.git/204661/
focus=206074
 
 All tests pass except for t91*, 
since there seems to be some
 pre-existing breakage in 
82dce998c202 relating to git-
svn.
 
 Adam Spiers (19):
   api-directory-listing.txt: 
update to match code
   Improve documentation and 
comments regarding directory 
traversal API
   dir.c: rename cryptic 'which' 
variable to more consistent 
name
   dir.c: rename path_excluded() 
to is_path_excluded()
   dir.c: rename 
excluded_from_list() to 
is_excluded_from_list()
   dir.c: rename excluded() to 
is_excluded()
   dir.c: refactor 
is_excluded_from_list()
   dir.c: refactor is_excluded()
   dir.c: refactor 
is_path_excluded()
   dir.c: rename free_excludes() 
to clear_exclude_list()
   dir.c: use a single struct 
exclude_list per source of 
excludes
   dir.c: keep track of where 
patterns came from
   dir.c: provide clear_directory() 
for reclaiming dir_struct memory
   add.c: refactor treat_gitlinks()
   add.c: remove unused 
argument from 
validate_pathspec()
   pathspec.c: move reusable 
code from builtin/add.c
   pathspec.c: extract new 
validate_path() for reuse
   setup.c: document 
get_pathspec()
   Add git-check-ignore sub-
command
 
  .gitignore
|   1 +
  Documentation/git-check-
ignore.txt|  89 
  Documentation/gitignore.txt   
|   6 +-
  Documentation/technical/api-
directory-listing.txt |  35 +-
  Makefile  
|   3 +
  attr.c|   
2 +-
  builtin.h 
|   1 +
  builtin/add.c 
|  84 +--
  builtin/check-ignore.c
| 170 +++
  builtin/clean.c   
|   3 +-
  builtin/ls-files.c
|  11 +-
  command-list.txt  
|   1 +
  contrib/completion/git-
completion.bash|   1 +
  dir.c | 
243 +++--
  dir.h |  
87 +++-
  git.c |   
1 +
  pathspec.c
| 107 
  pathspec.h
|   6 +
  setup.c   
|  15 +
  t/t0008-ignores.sh
| 595 
++
  unpack-trees.c
|  14 +-
  21 files changed, 1305 
insertions(+), 170 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/t0008-
ignores.sh
 




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


thomas sabo uk is really a tradition built

2012-12-26 Thread cunrule001
Thomas sabo are characterized by a few colossal as the choice Thomas Sabo
Gold. From the variety of a number of precious metal necklaces, diamond
earrings including a bracelet-type gold charm available for sale. strong
thomas sabo uk http://www.thomassabobraceletsshop.co.uk/  /strong is
really a tradition built and extremely feature. There will probably be a bed
that is not limited you should get some passion in addition to creativity.
We are completely possibly more than 600 Thomas Sabo pendant worldwide.
Remember special events basic spectacular items to ensure they are terrific.
You can provide many of these beautiful memories probably someone like a
straight-forward gift to have fun special days in their lives. More Thomas
Sabo may just be sold to most most typically associated with his office.
These kinds of may possibly be wonderful gifts for women as part of the
lives, even his or her girlfriend.

With my thomas sabo necklaces, I can spell them with my apparel comfortable
at will like animal dresses as well as the dresses to secure a social
gathering. I just now remembered i always went to a chic alliance i always
donned my strong thomas sabo bracelet
http://www.thomassabobraceletsshop.co.uk/  /strong, abounding of my
classmates said this armlet was actual admirable as well as the
architectural mastery was real cute. By cutting this affectionate of
necklaces can physical visual appeal our adolescence and vigor. I
recommended them in proudly i always bought this affectionate of cast in a
very very extremely low volume and leading premium quality. They'll likely
asked me the web site and stated they also cash to obtain this thomas sabo
necklaces afterwards.

Regarding what I realize, jewelry is getting to be a symbol of welcome and
prosperous a lot of decades, specially the popular jewelry, identical to
Thomas Sabo jewelry diamond pendant thomas sabo, that happen to be properly
well-known among teens. As for me, a devoted supporter of strong thomas
sabo charm bracelet http://www.thomassabobraceletsshop.co.uk/  /strong
to the net hold which is to be almost certainly one of one of the most
recognized jewelry product during the whole planet, Thomas Sabo Necklaces,
normally wanted a quick through the specific identical company doing work
experience as my exclusive birthday gift. The nice news is I've one
particular male of me on my eighteenth birthday. Through what I'm confident,
jewelry continues to grow for being a image of welcome and loaded decades,
specifically the credited jewelrys.
http://www.thomassabobraceletsshop.co.uk/  



--
View this message in context: 
http://git.661346.n2.nabble.com/thomas-sabo-uk-is-really-a-tradition-built-tp7573722.html
Sent from the git mailing list archive at Nabble.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


pandora charms are generally mostly of the makes

2012-12-26 Thread emilli989
strong pandora charms http://www.pandoracanadacharm.org/   /strong are
generally mostly of the makes to make available a new discounted in decided
on expensive jewelry. Look for an online site that will gives you the most
up-to-date savings proposed by Pandora. This specific allows you to obtain
additional expensive jewelry to the very same price tag. Acquiring via
websites features unique rewards. Websites present additional discounted on
account of a lesser amount of expenses fees. They have got a fantastic
dividends insurance plan so that you can mail rear solutions that unfit
anyone as well as your current targets. Moreover, websites have a very
substantial series that may be all to easy to surf and they also supply
product or service features which have been certainly not easily accessible
throughout shopping merchants

Your strong pandora bracelet charms http://www.pandoracanadacharm.org/  
/strong is usually naturally yourself nearly all family members. These
kind of pandora comprise while wonderful gives to the young lady with all
your every day having lived similar to your current little girl, close up
chum, your current partner, along with relative. Just about any distinct and
that is high priced available for you can often be blessed easy amazing
pandora expensive jewelry. Then you definitely will quickly realize out and
about these kind of fantastic tremendous series involving Disney expensive
jewelry which may have a variety of favorite people quite much like the
Jesse duck, Minney along with Mickey sensitive mouse, Kermit, Bambi,
Thumper, Tinkerbell, etc. You will note almost any Beans for this Beans.
Besides over the over series, various other Beans of men and women expensive
jewelry pandora. Inside Beans involving pandora birthstone often out and
about expensive jewelry furnished pertaining to just about every four weeks,
consequently for every single zodiac signal you could have traditional to
keep of an birthstone lure. Then you definitely will quickly realize out and
about pandora expensive jewelry which could certainly not vital destination
to all or any, nevertheless they are generally undertaking elegance right
percentage of society.

Pandora Expensive jewelry will assist you to develop your history you have
ever had using magnificently personal nevertheless cool patterns. You'll be
able to surprise the crooks to all your family or possibly go these people
onto the up coming age group, which will put their unique strong cheap
pandora charms http://www.pandoracanadacharm.org/   /strong with an by
now amazing series plus the history will certainly go forward. Referred to
as pandora expensive jewelry increasing symbolic involving desire along with
substitute, Pandora Diamond jewelry can be every bit as distinctive along
with fashionable throughout format along with design and style, and is also
currently normally desirable amidst young ladies, especially using amazing
classiness that will is probably inherited via age group for you to age
group. Pandora Wristbands beans assimilate to make massively
representational, extensive destination wristbands as well as various other
necklaces bits. Having said that, their significance could possibly be
viewed throughout exclusive strategies by simply certain purchasers. A
number of bead definitions are generally quite noticeable. People are able
to keep a good amount of definitions. A new Pandora bead destination
processed at the moment as a butterfly could very well probably perhaps
exemplifies a new personal change for better as well as your achievement of
an having lived point.
http://www.pandoracanadacharm.org/   



--
View this message in context: 
http://git.661346.n2.nabble.com/pandora-charms-are-generally-mostly-of-the-makes-tp7573723.html
Sent from the git mailing list archive at Nabble.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


nevertheless it is that arrival in pandora uk

2012-12-26 Thread fengsha999
Pandora Bracelets is everything regarding taking into account which will
1982, nevertheless it is that arrival in pandora uk on 1999 which will
without doubt started ones own magnificence. That splendid expensive jewelry
in Pandora is termed soon enough as soon as that Historic Lie in Zeus, just
who mature coming in becoming angered through Prometheus and even Epimetheus
designed for a review of fire place to help you young adults, who make use
of fire pit to help you light that evening hours precious time, thinking
that frustrated Zeus a legitimate super come to terms. pandora is without a
doubt diamond that could be without a doubt established in order to produce
inside one to a huge mixture. inches Very little a few is definitely the
somewhat the same, and even distinct can oftimes be i think resulted in and
even created with a population of clear clear a man or woman on
neurological.

You can discover an important pandora bracelets to speak about ones own
views for every party. Ones own great points in bracelets are made from
golden, magical, tumbler and even teeth enamel. They are really delightfully
fashioned with high-quality gemstones and even amazing colorations. That
picket beans is a style out and even include distinctive colorings in chic
dark. That sixteen carat golden bracelets can be nothing can beat you have
got looked at well before and even totally represent Present, that Radius in
Expectation, Take pleasure in, Great Bears, Graduating, Heart-Key,
Engagement Daisy, Call in Carnations, Teddy Display, Old classic Enchantment
through Onyx, Starlet in Steve and numerous others significant design. That
flagstone establish bracelets can be wonderful to visit and even take place
which has distinctive delightfully colored gemstones which includes
Rhodolite, Garnett, Zirconia, Red Spinel, Sapphire and even a common to be
Precious stones. You can get take pleasure in bracelets at the same time,
considering the I want One bracelets, Euro Wedding event Bracelets, Magical
Smooches Bracelets, Magical Core in Golden Enchantment; newly born baby
bracelets this includes Modest Lad and even Young daughter, Great Stroller
and even Magical Tee shirt whole ones own enchantment range utilizing
heart-warming design.

pandora charm bracelet will be centrally located inside the relatively a
better standard of designs which inturn distinctive pandora bracelets web
based that can clearly accordingly end up secure gold, secure golden and
likely an important unique blend nonetheless nonetheless with the a few that
can come together employing an important a few smooth looks. Without regard
for everything that ones own selection open for pigmentation and even manner
you may choose to found over Pandora bracelets to remain interested the.
Termed pandora bracelets increasing in symbolic in require and even
replacement, Pandora Diamond is without a doubt at the same time
confidential and even polished on theme and even model, it is at present in
general sought after concerning kids, expressly utilizing stunning grace
which will is passed down as a result of new release to help you new
release. Pandora Charms beans combine for making vastly a symbol, major
charm charms and several other expensive jewelry articles.
http://www.buycheappandorabracelets.co.uk/ 



--
View this message in context: 
http://git.661346.n2.nabble.com/nevertheless-it-is-that-arrival-in-pandora-uk-tp7573724.html
Sent from the git mailing list archive at Nabble.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