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

2012-12-28 Thread Junio C Hamano 
Adam Spiers g...@adamspiers.org writes:

 diff --git a/pathspec.h b/pathspec.h
 new file mode 100644
 index 000..8bb670b
 --- /dev/null
 +++ b/pathspec.h
 @@ -0,0 +1,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 const char **validate_pathspec(const char **argv, const char *prefix);

Protect this against multiple inclusion with #ifndef PATHSPEC_H.

--
To unsubscribe from this list: send the line 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 16/19] pathspec.c: move reusable code from builtin/add.c

2012-12-28 Thread Adam Spiers 
On Fri, Dec 28, 2012 at 8:32 PM, Junio C Hamano gits...@pobox.com wrote:
 Adam Spiers g...@adamspiers.org writes:

 diff --git a/pathspec.h b/pathspec.h
 new file mode 100644
 index 000..8bb670b
 --- /dev/null
 +++ b/pathspec.h
 @@ -0,0 +1,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 const char **validate_pathspec(const char **argv, const char 
 *prefix);

 Protect this against multiple inclusion with #ifndef PATHSPEC_H.

Yep good idea, how should I submit this?  It will cause a trivially
resolvable conflict with the next patch in the series (17/19):

  pathspec.c: extract new validate_path() for reuse

but I'd prefer not to re-roll 16--19 when just 16 and 17 are sufficient.
--
To unsubscribe from this list: send the line 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 16/19] pathspec.c: move reusable code from builtin/add.c

2012-12-28 Thread Junio C Hamano 
Adam Spiers g...@adamspiers.org writes:

 diff --git a/pathspec.c b/pathspec.c
 new file mode 100644
 index 000..8aea0d2
 --- /dev/null
 +++ b/pathspec.c
 @@ -0,0 +1,99 @@
 +#include cache.h
 +#include dir.h
 +#include pathspec.h
 +
 +void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
 +{

It did not matter while it was an implementation detail of git
add, but as a public function, something needs to clarify that this
fills matches *against the index*, not against a tree or the files
in the current directory.  The same comment applies to all the
internal functions this patch exposes to the outside world.
--
To unsubscribe from this list: send the line 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 16/19] pathspec.c: move reusable code from builtin/add.c

2012-12-28 Thread Adam Spiers 
On Fri, Dec 28, 2012 at 8:48 PM, Junio C Hamano gits...@pobox.com wrote:
 Adam Spiers g...@adamspiers.org writes:

 diff --git a/pathspec.c b/pathspec.c
 new file mode 100644
 index 000..8aea0d2
 --- /dev/null
 +++ b/pathspec.c
 @@ -0,0 +1,99 @@
 +#include cache.h
 +#include dir.h
 +#include pathspec.h
 +
 +void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
 +{

 It did not matter while it was an implementation detail of git
 add, but as a public function, something needs to clarify that this
 fills matches *against the index*, not against a tree or the files
 in the current directory.  The same comment applies to all the
 internal functions this patch exposes to the outside world.

Prior to submitting the v3 series, I attempted to understand
fill_pathspec_matches() and find_used_pathspec() well enough to
document them all, but I failed.  Perhaps some kind soul could explain
what is the exact purpose of these functions, and in particular the
role of char *seen in both?
--
To unsubscribe from this list: send the line 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 16/19] pathspec.c: move reusable code from builtin/add.c

2012-12-28 Thread Adam Spiers 
On Fri, Dec 28, 2012 at 8:45 PM, Adam Spiers g...@adamspiers.org wrote:
 On Fri, Dec 28, 2012 at 8:32 PM, Junio C Hamano gits...@pobox.com wrote:
 Adam Spiers g...@adamspiers.org writes:

 diff --git a/pathspec.h b/pathspec.h
 new file mode 100644
 index 000..8bb670b
 --- /dev/null
 +++ b/pathspec.h
 @@ -0,0 +1,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 const char **validate_pathspec(const char **argv, const char 
 *prefix);

 Protect this against multiple inclusion with #ifndef PATHSPEC_H.

 Yep good idea, how should I submit this?  It will cause a trivially
 resolvable conflict with the next patch in the series (17/19):

   pathspec.c: extract new validate_path() for reuse

I was wrong about that - it didn't cause a conflict, although it does
marginally change the context at the end of the pathspec.h hunk in the
above patch.

 but I'd prefer not to re-roll 16--19 when just 16 and 17 are sufficient.

Based on your other feedback, all of 16--19 require changes, and as
things stand, conveniently nothing earlier in the series does, so I'll
re-roll those four once the outstanding issues are all resolved.
--
To unsubscribe from this list: send the line 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))) {
-