Re: [PATCH v2.1] commit: add --ignore-submodules[=when] parameter

2014-04-22 Thread Ronald Weiss 
On 18. 4. 2014 14:28, Jens Lehmann wrote:
 Am 13.04.2014 01:41, schrieb Ronald Weiss:
 Second, there are some differences between adding standard ignored
 files, and ignored submodules:

 1) Already tracked files are never ignored, regardless of .gitignore.
   However, tracked submodules should be ignored by add -u, if told so
   by their ignore setting.

 2) So .gitignore seems to only do something when adding new files to
   the repo. However, when adding new submodules, they are probably never
   ignored (setting the ignore setting for non existent submodule seems
   like non-sense, although possible).
 
 What about diff.ignoreSubmodules=all?
 
 3) Ignored files can be ignored less explicitely (in global gitignore,
   or using a wildcard, or by ignoring parent folder). So it makes sense
   to warn the user if he tries to explicitely add an ignored file, as he
   might not be aware that the file is ignored. Submodules, however, can
   only be ignored explicitely. And when user explicitely specifies the
   submodule in an add command, he most probably really wants to add it,
   so I don't see the point in warning him and requiring the -f option.
 
 But we do have diff.ignoreSubmodules=all, so I do not agree with
 your conclusion.

Ah, I didn't know about diff.ignoreSubmodules. I agree that it defeats
two of my points :-(.

And how about the point 1), should submodules never be ignored once
already tracked, like standard ignored files? I'm almost sure that in
this case the behavior should be different, otherwise it would drop
the most useful use case of the proposed change.

 So, I think that the use cases are completely different, for submodules
 and ignored files. So trying to make add behave the same for both, might
 not be that good idea.

 I would propose - let's make add honor the ignore setting by just
 parsing if from config like the other commands do, and pass it to
 underlying diff invocations. And at the same the, let's override it for
 submodules explicitely specified on the command line, to never ignore
 such submodules, without requiring the -f option. That seems to be
 pretty easy, see below.
 
 What about doing that only when '-f' is given? Would that do what we
 want?

OK then, seems right with diff.ignoreSubmodules in mind.

But one question stays - what to do with submodules not explicitly
specified on command line (when their parent folder is being added)?
You adviced already to do what we do with standard ignored files. That
seems to be:

A) if the file is already tracked, add it
B) if not tracked, without -f, silently ignore it
C) if not tracked, with -f, add it

A) is same like point 1 above, let's forget it until we settle that one.
But B) is even more problematic, we would probably have to modify the
directory parsing code in dir.c to handle ignored submodules. Is that
what we want? If so, then I'm afraid this is getting overly complicated.

 @@ -320,6 +324,7 @@ int cmd_add(int argc, const char **argv, const char 
 *prefix)
  char *seen = NULL;
   
  git_config(add_config, NULL);
 +gitmodules_config();
 
 Wrong order, gitmodules_config() must be called before git_config()
 to make the .gitmodules settings overridable by the users config.

Right. I noticed that too just after sending the patch.

 @@ -440,6 +446,18 @@ int cmd_add(int argc, const char **argv, const char 
 *prefix)
  die(_(pathspec '%s' did not match any 
 files),
  pathspec.items[i].original);
  }
 +
 +/* disable ignore setting for any submodules specified 
 explicitly in the pathspec */
 +if (path[0] 
 +(cachepos = cache_name_pos(path, 
 pathspec.items[i].len)) = 0 
 +S_ISGITLINK(active_cache[cachepos]-ce_mode)) {
 +char *optname;
 +int optnamelen = pathspec.items[i].len + 17;
 +optname = xcalloc(optnamelen + 1, 1);
 +snprintf(optname, optnamelen + 1, 
 submodule.%s.ignore, path);
 +parse_submodule_config_option(optname, none);
 
 We should use dirty instead of none here. Modifications inside
 the submodule cannot be added without committing them there first
 and using none might incur an extra preformance penalty for
 looking through the submodule directories for such changes.

OK, that's right too.
--
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.1] commit: add --ignore-submodules[=when] parameter

2014-04-18 Thread Jens Lehmann 
Am 13.04.2014 01:41, schrieb Ronald Weiss:
 On 8. 4. 2014 20:26, Jens Lehmann wrote:
 Am 07.04.2014 23:46, schrieb Ronald Weiss:
 Then, on top of that, I'll prepare patches for add to honor ignore
 from .gitmodules, and -f implying --ignore-submodules. That might need
 more discussion, let's see.

 Makes sense.
 
 I thought more about that, and also played with the code a bit.
 
 First, I was confused when I wrote that git add doesn't honor
 submodules' ignore setting only from .gitmodules, but it does from
 .git/config. It doesn't, from neither. Sorry for the confusion. However,
 that doesn't change anything on the fact that it would be nice if add
 would honor the ignore setting, from both places.

Ok, thanks for digging deeper here.

 Second, there are some differences between adding standard ignored
 files, and ignored submodules:
 
 1) Already tracked files are never ignored, regardless of .gitignore.
  However, tracked submodules should be ignored by add -u, if told so
  by their ignore setting.
 
 2) So .gitignore seems to only do something when adding new files to
  the repo. However, when adding new submodules, they are probably never
  ignored (setting the ignore setting for non existent submodule seems
  like non-sense, although possible).

What about diff.ignoreSubmodules=all?

 3) Ignored files can be ignored less explicitely (in global gitignore,
  or using a wildcard, or by ignoring parent folder). So it makes sense
  to warn the user if he tries to explicitely add an ignored file, as he
  might not be aware that the file is ignored. Submodules, however, can
  only be ignored explicitely. And when user explicitely specifies the
  submodule in an add command, he most probably really wants to add it,
  so I don't see the point in warning him and requiring the -f option.

But we do have diff.ignoreSubmodules=all, so I do not agree with
your conclusion.

 So, I think that the use cases are completely different, for submodules
 and ignored files. So trying to make add behave the same for both, might
 not be that good idea.
 
 I would propose - let's make add honor the ignore setting by just
 parsing if from config like the other commands do, and pass it to
 underlying diff invocations. And at the same the, let's override it for
 submodules explicitely specified on the command line, to never ignore
 such submodules, without requiring the -f option. That seems to be
 pretty easy, see below.

What about doing that only when '-f' is given? Would that do what we
want?

 diff --git a/builtin/add.c b/builtin/add.c
 index 85f2110..f19e6c8 100644
 --- a/builtin/add.c
 +++ b/builtin/add.c
 @@ -284,6 +284,10 @@ static int add_config(const char *var, const char 
 *value, void *cb)
   ignore_add_errors = git_config_bool(var, value);
   return 0;
   }
 +
 + if (starts_with(var, submodule.))
 + return parse_submodule_config_option(var, value);
 +
   return git_default_config(var, value, cb);
  }
  
 @@ -320,6 +324,7 @@ int cmd_add(int argc, const char **argv, const char 
 *prefix)
   char *seen = NULL;
  
   git_config(add_config, NULL);
 + gitmodules_config();

Wrong order, gitmodules_config() must be called before git_config()
to make the .gitmodules settings overridable by the users config.

   argc = parse_options(argc, argv, prefix, builtin_add_options,
 builtin_add_usage, PARSE_OPT_KEEP_ARGV0);
 @@ -425,6 +430,7 @@ int cmd_add(int argc, const char **argv, const char 
 *prefix)
  PATHSPEC_EXCLUDE);
  
   for (i = 0; i  pathspec.nr; i++) {
 + int cachepos;
   const char *path = pathspec.items[i].match;
   if (pathspec.items[i].magic  PATHSPEC_EXCLUDE)
   continue;
 @@ -440,6 +446,18 @@ int cmd_add(int argc, const char **argv, const char 
 *prefix)
   die(_(pathspec '%s' did not match any 
 files),
   pathspec.items[i].original);
   }
 +
 + /* disable ignore setting for any submodules specified 
 explicitly in the pathspec */
 + if (path[0] 
 + (cachepos = cache_name_pos(path, 
 pathspec.items[i].len)) = 0 
 + S_ISGITLINK(active_cache[cachepos]-ce_mode)) {
 + char *optname;
 + int optnamelen = pathspec.items[i].len + 17;
 + optname = xcalloc(optnamelen + 1, 1);
 + snprintf(optname, optnamelen + 1, 
 submodule.%s.ignore, path);
 + parse_submodule_config_option(optname, none);

We should use dirty instead of none here. Modifications inside
the submodule cannot be added without committing them there first
and using none might incur an extra preformance penalty for
looking

Re: [PATCH v2.1] commit: add --ignore-submodules[=when] parameter

2014-04-12 Thread Ronald Weiss 
On 8. 4. 2014 20:26, Jens Lehmann wrote:
 Am 07.04.2014 23:46, schrieb Ronald Weiss:
 Then, on top of that, I'll prepare patches for add to honor ignore
 from .gitmodules, and -f implying --ignore-submodules. That might need
 more discussion, let's see.
 
 Makes sense.

I thought more about that, and also played with the code a bit.

First, I was confused when I wrote that git add doesn't honor
submodules' ignore setting only from .gitmodules, but it does from
.git/config. It doesn't, from neither. Sorry for the confusion. However,
that doesn't change anything on the fact that it would be nice if add
would honor the ignore setting, from both places.

Second, there are some differences between adding standard ignored
files, and ignored submodules:

1) Already tracked files are never ignored, regardless of .gitignore.
 However, tracked submodules should be ignored by add -u, if told so
 by their ignore setting.

2) So .gitignore seems to only do something when adding new files to
 the repo. However, when adding new submodules, they are probably never
 ignored (setting the ignore setting for non existent submodule seems
 like non-sense, although possible).

3) Ignored files can be ignored less explicitely (in global gitignore,
 or using a wildcard, or by ignoring parent folder). So it makes sense
 to warn the user if he tries to explicitely add an ignored file, as he
 might not be aware that the file is ignored. Submodules, however, can
 only be ignored explicitely. And when user explicitely specifies the
 submodule in an add command, he most probably really wants to add it,
 so I don't see the point in warning him and requiring the -f option.

So, I think that the use cases are completely different, for submodules
and ignored files. So trying to make add behave the same for both, might
not be that good idea.

I would propose - let's make add honor the ignore setting by just
parsing if from config like the other commands do, and pass it to
underlying diff invocations. And at the same the, let's override it for
submodules explicitely specified on the command line, to never ignore
such submodules, without requiring the -f option. That seems to be
pretty easy, see below.


diff --git a/builtin/add.c b/builtin/add.c
index 85f2110..f19e6c8 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -284,6 +284,10 @@ static int add_config(const char *var, const char *value, 
void *cb)
ignore_add_errors = git_config_bool(var, value);
return 0;
}
+
+   if (starts_with(var, submodule.))
+   return parse_submodule_config_option(var, value);
+
return git_default_config(var, value, cb);
 }
 
@@ -320,6 +324,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
char *seen = NULL;
 
git_config(add_config, NULL);
+   gitmodules_config();
 
argc = parse_options(argc, argv, prefix, builtin_add_options,
  builtin_add_usage, PARSE_OPT_KEEP_ARGV0);
@@ -425,6 +430,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
   PATHSPEC_EXCLUDE);
 
for (i = 0; i  pathspec.nr; i++) {
+   int cachepos;
const char *path = pathspec.items[i].match;
if (pathspec.items[i].magic  PATHSPEC_EXCLUDE)
continue;
@@ -440,6 +446,18 @@ int cmd_add(int argc, const char **argv, const char 
*prefix)
die(_(pathspec '%s' did not match any 
files),
pathspec.items[i].original);
}
+
+   /* disable ignore setting for any submodules specified 
explicitly in the pathspec */
+   if (path[0] 
+   (cachepos = cache_name_pos(path, 
pathspec.items[i].len)) = 0 
+   S_ISGITLINK(active_cache[cachepos]-ce_mode)) {
+   char *optname;
+   int optnamelen = pathspec.items[i].len + 17;
+   optname = xcalloc(optnamelen + 1, 1);
+   snprintf(optname, optnamelen + 1, 
submodule.%s.ignore, path);
+   parse_submodule_config_option(optname, none);
+   free(optname);
+   }
}
free(seen);
}
--  
--
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.1] commit: add --ignore-submodules[=when] parameter

2014-04-08 Thread Jens Lehmann 
Am 07.04.2014 23:46, schrieb Ronald Weiss:
 On 6. 4. 2014 18:28, Jens Lehmann wrote:
 Am 02.04.2014 21:56, schrieb Ronald Weiss:
 On 2. 4. 2014 20:53, Jens Lehmann wrote:
 Am 01.04.2014 23:59, schrieb Ronald Weiss:
 On 1. 4. 2014 22:23, Jens Lehmann wrote:
 Am 01.04.2014 01:35, schrieb Ronald Weiss:
 On 1. 4. 2014 0:50, Ronald Weiss wrote:
 On 31. 3. 2014 23:47, Ronald Weiss wrote:
 On Mon, Mar 31, 2014 at 8:58 PM, Jens Lehmann jens.lehm...@web.de 
 wrote:
 As Junio mentioned it would be great if you could teach the add
 command also honor the --ignore-submodule command line option in
 a companion patch. In the course of doing so you'll easily see if
 I was right or not, then please just order them in the most logical
 way.

 Well, if You (or Junio) really don't want my patch without another one
 for git add, I may try to do it. However, git add does not even honor
 the submodules' ignore setting from .gitmodules (just tested with git
 1.9.1: git add -u doesn't honor it, while git commit -a does). So
 teaching git add the --ignore-submodules switch in current state
 doesn't seem right to me. You might propose to add also support for
 the ignore setting, to make add -u and commit -a more consistent.
 That seems like a good idea, but the effort needed is getting bigger,

 Well, now I actually looked at it, and it was pretty easy after all.
 The changes below seem to enable support for both ignore setting in
 .gitmodules, and also --ignore-submodules switch, for git add, on top
 of my patch for commit.

 There is a catch. With the changes below, submodules are ignored by add
 even if explitely named on command line (eg. git add x does nothing
 if x is submodule with new commits, but with ignore=all in .gitmodules).
 That doesn't seem right.

 Any ideas, what to do about that? When exactly should such submodule be
 actually ignored?

 Me thinks git add should require the '-f' option to add an ignored
 submodule (just like it does for files) unless the user uses the
 '--ignore-submodules=none' option. And if neither of these are given
 it should fail with a list of ignored files as the documentation
 states.

 It's still not clear, at least not to me. Should '-f' suppress the
 ignore setting of all involved submodules? That would make it a
 synonyme (or a superset) of --ignore-submodules=none. Or only if the
 submodule is explicitly named on command line? That seems fuzzy to me,
 and also more tricky to implement.

 Maybe my impression that doing add together with commit would be
 easy wasn't correct after all. I won't object if you try to tackle
 commit first (but I have the slight suspicion that similar questions
 will arise concerning the addish functionality in commit too. So
 maybe after resolving those things might look clearer ;-)

 There is one big distinction. My patch for commit doesn't add any new
 problems. It just adds the --ignore-submodules argument, which is easy
 to implement and no unclear behavior decisions are needed.

 You are right that when specifying ignored submodules on commit's
 command line, there is the same problem as with git add. However, it's
 already there anyway. I don't feel in position to solve it, I'd just
 like to have git commit --ignore-submodules=none.

 With git add however, changing it to honor settings from .gitmodules
 would change behavior people might be used to, so I would be afraid to
 do that. Btw add also has the problem already, but only if somebody
 configures the submodule's ignore setting in .git/config, rather than
 .gitmodules. I don't know how much real use case that is.

 As I see it, there are now these rather easy possibilities (sorted
 from the easiest):

 1) Just teach commit the --ignore-submodules argument, as I proposed.

 1a) Teach commit to honor ignore from .git/config.
 
 But commit already honors that. It honors submodule.name.ignore from
 both .git/config and .gitmodules. It's just add which doesn't honor it
 from .gitmodules, because cmd_add() function lacks a gitmodules_config()
 call. Or do I miss something?

No, I missed that, so please forget my comment.

 2) Teach both add and commit to --ignore-submodules, but dont add that
 problematic gitmodules_config() in add.c.

 Why is that problematic after add learned --ignore-submodules=none?
 
 First, because it changes current behaviour. Which is obviously
 inconsistent currently, however I didn't find it easy to tell what's
 the right thing to do.

I believe we should be consistent here, but the overriding of that
option is the tricky part. So we need to solve that first before we
can add gitmodules_config() to add.c.

 And second, because the -f implies --ignore-submodules=none proposal,
 which seems to be the easy cure for those accustomed to the current
 behavior, seems non-trivial. Below You wrote that
 --ignore-submodules=none should be implied by -f only for files
 specified on the command line. OK. And what if a directory
 containing the submodule is specified?

That should

Re: [PATCH v2.1] commit: add --ignore-submodules[=when] parameter

2014-04-07 Thread Ronald Weiss 
On 6. 4. 2014 18:28, Jens Lehmann wrote:
 Am 02.04.2014 21:56, schrieb Ronald Weiss:
 On 2. 4. 2014 20:53, Jens Lehmann wrote:
 Am 01.04.2014 23:59, schrieb Ronald Weiss:
 On 1. 4. 2014 22:23, Jens Lehmann wrote:
 Am 01.04.2014 01:35, schrieb Ronald Weiss:
 On 1. 4. 2014 0:50, Ronald Weiss wrote:
 On 31. 3. 2014 23:47, Ronald Weiss wrote:
 On Mon, Mar 31, 2014 at 8:58 PM, Jens Lehmann jens.lehm...@web.de 
 wrote:
 As Junio mentioned it would be great if you could teach the add
 command also honor the --ignore-submodule command line option in
 a companion patch. In the course of doing so you'll easily see if
 I was right or not, then please just order them in the most logical
 way.

 Well, if You (or Junio) really don't want my patch without another one
 for git add, I may try to do it. However, git add does not even honor
 the submodules' ignore setting from .gitmodules (just tested with git
 1.9.1: git add -u doesn't honor it, while git commit -a does). So
 teaching git add the --ignore-submodules switch in current state
 doesn't seem right to me. You might propose to add also support for
 the ignore setting, to make add -u and commit -a more consistent.
 That seems like a good idea, but the effort needed is getting bigger,

 Well, now I actually looked at it, and it was pretty easy after all.
 The changes below seem to enable support for both ignore setting in
 .gitmodules, and also --ignore-submodules switch, for git add, on top
 of my patch for commit.

 There is a catch. With the changes below, submodules are ignored by add
 even if explitely named on command line (eg. git add x does nothing
 if x is submodule with new commits, but with ignore=all in .gitmodules).
 That doesn't seem right.

 Any ideas, what to do about that? When exactly should such submodule be
 actually ignored?

 Me thinks git add should require the '-f' option to add an ignored
 submodule (just like it does for files) unless the user uses the
 '--ignore-submodules=none' option. And if neither of these are given
 it should fail with a list of ignored files as the documentation
 states.

 It's still not clear, at least not to me. Should '-f' suppress the
 ignore setting of all involved submodules? That would make it a
 synonyme (or a superset) of --ignore-submodules=none. Or only if the
 submodule is explicitly named on command line? That seems fuzzy to me,
 and also more tricky to implement.

 Maybe my impression that doing add together with commit would be
 easy wasn't correct after all. I won't object if you try to tackle
 commit first (but I have the slight suspicion that similar questions
 will arise concerning the addish functionality in commit too. So
 maybe after resolving those things might look clearer ;-)

 There is one big distinction. My patch for commit doesn't add any new
 problems. It just adds the --ignore-submodules argument, which is easy
 to implement and no unclear behavior decisions are needed.

 You are right that when specifying ignored submodules on commit's
 command line, there is the same problem as with git add. However, it's
 already there anyway. I don't feel in position to solve it, I'd just
 like to have git commit --ignore-submodules=none.

 With git add however, changing it to honor settings from .gitmodules
 would change behavior people might be used to, so I would be afraid to
 do that. Btw add also has the problem already, but only if somebody
 configures the submodule's ignore setting in .git/config, rather than
 .gitmodules. I don't know how much real use case that is.

 As I see it, there are now these rather easy possibilities (sorted
 from the easiest):

 1) Just teach commit the --ignore-submodules argument, as I proposed.
 
 1a) Teach commit to honor ignore from .git/config.

But commit already honors that. It honors submodule.name.ignore from
both .git/config and .gitmodules. It's just add which doesn't honor it
from .gitmodules, because cmd_add() function lacks a gitmodules_config()
call. Or do I miss something?

 2) Teach both add and commit to --ignore-submodules, but dont add that
 problematic gitmodules_config() in add.c.
 
 Why is that problematic after add learned --ignore-submodules=none?

First, because it changes current behaviour. Which is obviously
inconsistent currently, however I didn't find it easy to tell what's
the right thing to do.

And second, because the -f implies --ignore-submodules=none proposal,
which seems to be the easy cure for those accustomed to the current
behavior, seems non-trivial. Below You wrote that
--ignore-submodules=none should be implied by -f only for files
specified on the command line. OK. And what if a directory
containing the submodule is specified?

 3) Teach both add and commit to --ignore-submodules, and also let add
 honor settings from .gitmodules, to make it more consistent with other
 commands. And, make add --force imply --ignore-submodules=none.

 I like both 1) and 2). I don't like 3), the problem of add with
 submodules' ignore

Re: [PATCH v2.1] commit: add --ignore-submodules[=when] parameter

2014-04-02 Thread Jens Lehmann 
Am 01.04.2014 23:59, schrieb Ronald Weiss:
 On 1. 4. 2014 22:23, Jens Lehmann wrote:
 Am 01.04.2014 01:35, schrieb Ronald Weiss:
 On 1. 4. 2014 0:50, Ronald Weiss wrote:
 On 31. 3. 2014 23:47, Ronald Weiss wrote:
 On Mon, Mar 31, 2014 at 8:58 PM, Jens Lehmann jens.lehm...@web.de wrote:
 As Junio mentioned it would be great if you could teach the add
 command also honor the --ignore-submodule command line option in
 a companion patch. In the course of doing so you'll easily see if
 I was right or not, then please just order them in the most logical
 way.

 Well, if You (or Junio) really don't want my patch without another one
 for git add, I may try to do it. However, git add does not even honor
 the submodules' ignore setting from .gitmodules (just tested with git
 1.9.1: git add -u doesn't honor it, while git commit -a does). So
 teaching git add the --ignore-submodules switch in current state
 doesn't seem right to me. You might propose to add also support for
 the ignore setting, to make add -u and commit -a more consistent.
 That seems like a good idea, but the effort needed is getting bigger,

 Well, now I actually looked at it, and it was pretty easy after all.
 The changes below seem to enable support for both ignore setting in
 .gitmodules, and also --ignore-submodules switch, for git add, on top
 of my patch for commit.

 There is a catch. With the changes below, submodules are ignored by add 
 even if explitely named on command line (eg. git add x does nothing if x 
 is submodule with new commits, but with ignore=all in .gitmodules).
 That doesn't seem right.

 Any ideas, what to do about that? When exactly should such submodule be 
 actually ignored?

 Me thinks git add should require the '-f' option to add an ignored
 submodule (just like it does for files) unless the user uses the
 '--ignore-submodules=none' option. And if neither of these are given
 it should fail with a list of ignored files as the documentation
 states.
 
 It's still not clear, at least not to me. Should '-f' suppress the
 ignore setting of all involved submodules? That would make it a
 synonyme (or a superset) of --ignore-submodules=none. Or only if the
 submodule is explicitly named on command line? That seems fuzzy to me,
 and also more tricky to implement.

Maybe my impression that doing add together with commit would be
easy wasn't correct after all. I won't object if you try to tackle
commit first (but I have the slight suspicion that similar questions
will arise concerning the addish functionality in commit too. So
maybe after resolving those things might look clearer ;-)
--
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.1] commit: add --ignore-submodules[=when] parameter

2014-04-02 Thread Ronald Weiss 
On 2. 4. 2014 20:53, Jens Lehmann wrote:
 Am 01.04.2014 23:59, schrieb Ronald Weiss:
 On 1. 4. 2014 22:23, Jens Lehmann wrote:
 Am 01.04.2014 01:35, schrieb Ronald Weiss:
 On 1. 4. 2014 0:50, Ronald Weiss wrote:
 On 31. 3. 2014 23:47, Ronald Weiss wrote:
 On Mon, Mar 31, 2014 at 8:58 PM, Jens Lehmann jens.lehm...@web.de 
 wrote:
 As Junio mentioned it would be great if you could teach the add
 command also honor the --ignore-submodule command line option in
 a companion patch. In the course of doing so you'll easily see if
 I was right or not, then please just order them in the most logical
 way.

 Well, if You (or Junio) really don't want my patch without another one
 for git add, I may try to do it. However, git add does not even honor
 the submodules' ignore setting from .gitmodules (just tested with git
 1.9.1: git add -u doesn't honor it, while git commit -a does). So
 teaching git add the --ignore-submodules switch in current state
 doesn't seem right to me. You might propose to add also support for
 the ignore setting, to make add -u and commit -a more consistent.
 That seems like a good idea, but the effort needed is getting bigger,

 Well, now I actually looked at it, and it was pretty easy after all.
 The changes below seem to enable support for both ignore setting in
 .gitmodules, and also --ignore-submodules switch, for git add, on top
 of my patch for commit.

 There is a catch. With the changes below, submodules are ignored by add
 even if explitely named on command line (eg. git add x does nothing
 if x is submodule with new commits, but with ignore=all in .gitmodules).
 That doesn't seem right.

 Any ideas, what to do about that? When exactly should such submodule be
 actually ignored?

 Me thinks git add should require the '-f' option to add an ignored
 submodule (just like it does for files) unless the user uses the
 '--ignore-submodules=none' option. And if neither of these are given
 it should fail with a list of ignored files as the documentation
 states.

 It's still not clear, at least not to me. Should '-f' suppress the
 ignore setting of all involved submodules? That would make it a
 synonyme (or a superset) of --ignore-submodules=none. Or only if the
 submodule is explicitly named on command line? That seems fuzzy to me,
 and also more tricky to implement.
 
 Maybe my impression that doing add together with commit would be
 easy wasn't correct after all. I won't object if you try to tackle
 commit first (but I have the slight suspicion that similar questions
 will arise concerning the addish functionality in commit too. So
 maybe after resolving those things might look clearer ;-)

There is one big distinction. My patch for commit doesn't add any new
problems. It just adds the --ignore-submodules argument, which is easy
to implement and no unclear behavior decisions are needed.

You are right that when specifying ignored submodules on commit's
command line, there is the same problem as with git add. However, it's
already there anyway. I don't feel in position to solve it, I'd just
like to have git commit --ignore-submodules=none.

With git add however, changing it to honor settings from .gitmodules
would change behavior people might be used to, so I would be afraid to
do that. Btw add also has the problem already, but only if somebody
configures the submodule's ignore setting in .git/config, rather than
.gitmodules. I don't know how much real use case that is.

As I see it, there are now these rather easy possibilities (sorted
from the easiest):

1) Just teach commit the --ignore-submodules argument, as I proposed.

2) Teach both add and commit to --ignore-submodules, but dont add that
problematic gitmodules_config() in add.c.

3) Teach both add and commit to --ignore-submodules, and also let add
honor settings from .gitmodules, to make it more consistent with other
commands. And, make add --force imply --ignore-submodules=none.

I like both 1) and 2). I don't like 3), the problem of add with
submodules' ignore setting is a bug IMHO (ignore=all in .git/config
causes strange behavior, while ignore=all in .gitmodules is ignored),
but not directly related to the --ignore-submodules param, and should
be solved separately.
--
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.1] commit: add --ignore-submodules[=when] parameter

2014-04-01 Thread Jens Lehmann 
Am 01.04.2014 01:35, schrieb Ronald Weiss:
 On 1. 4. 2014 0:50, Ronald Weiss wrote:
 On 31. 3. 2014 23:47, Ronald Weiss wrote:
 On Mon, Mar 31, 2014 at 8:58 PM, Jens Lehmann jens.lehm...@web.de wrote:
 As Junio mentioned it would be great if you could teach the add
 command also honor the --ignore-submodule command line option in
 a companion patch. In the course of doing so you'll easily see if
 I was right or not, then please just order them in the most logical
 way.

 Well, if You (or Junio) really don't want my patch without another one
 for git add, I may try to do it. However, git add does not even honor
 the submodules' ignore setting from .gitmodules (just tested with git
 1.9.1: git add -u doesn't honor it, while git commit -a does). So
 teaching git add the --ignore-submodules switch in current state
 doesn't seem right to me. You might propose to add also support for
 the ignore setting, to make add -u and commit -a more consistent.
 That seems like a good idea, but the effort needed is getting bigger,

 Well, now I actually looked at it, and it was pretty easy after all.
 The changes below seem to enable support for both ignore setting in
 .gitmodules, and also --ignore-submodules switch, for git add, on top
 of my patch for commit.
 
 There is a catch. With the changes below, submodules are ignored by add even 
 if explitely named on command line (eg. git add x does nothing if x is 
 submodule with new commits, but with ignore=all in .gitmodules).
 That doesn't seem right.
 
 Any ideas, what to do about that? When exactly should such submodule be 
 actually ignored?

Me thinks git add should require the '-f' option to add an ignored
submodule (just like it does for files) unless the user uses the
'--ignore-submodules=none' option. And if neither of these are given
it should fail with a list of ignored files as the documentation
states.
--
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.1] commit: add --ignore-submodules[=when] parameter

2014-04-01 Thread Ronald Weiss 
On 1. 4. 2014 22:23, Jens Lehmann wrote:
 Am 01.04.2014 01:35, schrieb Ronald Weiss:
 On 1. 4. 2014 0:50, Ronald Weiss wrote:
 On 31. 3. 2014 23:47, Ronald Weiss wrote:
 On Mon, Mar 31, 2014 at 8:58 PM, Jens Lehmann jens.lehm...@web.de wrote:
 As Junio mentioned it would be great if you could teach the add
 command also honor the --ignore-submodule command line option in
 a companion patch. In the course of doing so you'll easily see if
 I was right or not, then please just order them in the most logical
 way.

 Well, if You (or Junio) really don't want my patch without another one
 for git add, I may try to do it. However, git add does not even honor
 the submodules' ignore setting from .gitmodules (just tested with git
 1.9.1: git add -u doesn't honor it, while git commit -a does). So
 teaching git add the --ignore-submodules switch in current state
 doesn't seem right to me. You might propose to add also support for
 the ignore setting, to make add -u and commit -a more consistent.
 That seems like a good idea, but the effort needed is getting bigger,

 Well, now I actually looked at it, and it was pretty easy after all.
 The changes below seem to enable support for both ignore setting in
 .gitmodules, and also --ignore-submodules switch, for git add, on top
 of my patch for commit.

 There is a catch. With the changes below, submodules are ignored by add even 
 if explitely named on command line (eg. git add x does nothing if x is 
 submodule with new commits, but with ignore=all in .gitmodules).
 That doesn't seem right.

 Any ideas, what to do about that? When exactly should such submodule be 
 actually ignored?
 
 Me thinks git add should require the '-f' option to add an ignored
 submodule (just like it does for files) unless the user uses the
 '--ignore-submodules=none' option. And if neither of these are given
 it should fail with a list of ignored files as the documentation
 states.

It's still not clear, at least not to me. Should '-f' suppress the
ignore setting of all involved submodules? That would make it a
synonyme (or a superset) of --ignore-submodules=none. Or only if the
submodule is explicitly named on command line? That seems fuzzy to me,
and also more tricky to implement.
--
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.1] commit: add --ignore-submodules[=when] parameter

2014-03-31 Thread Jens Lehmann 
Am 31.03.2014 02:07, schrieb Ronald Weiss:
 Git commit honors the 'ignore' setting from .gitmodules or .git/config,
 but didn't allow to override it from command line, like other commands do.
 
 Useful when values for commit are 'all' (default) or 'none'. The others
 ('dirty' and 'untracked') have same effect as 'none', as commit is only
 interested in whether the submodule's HEAD differs from what is commited
 in the superproject.
 
 Changes in add.c and cache.h (and related compilo fix in checkout.c) are
 needed to make it work for commit -a too.
 
 Signed-off-by: Ronald Weiss weiss.ron...@gmail.com
 ---
 The previous patch version (v2) contained bug in the test, by mistake I
 have sent older version than I was testing with, sorry for that.
 
 On 30. 3. 2014 21:48, Jens Lehmann wrote:
 Looking good so far, but we definitely need tests for this new option.
 
 I added two simple tests (one for --ignore-submodules=all, another one
 for =none). But I am sure the tests could be written better, by someone
 more proficient in Git than I am.
 
 The tests immediately revealed, that the patch was not complete. It
 didn't work with commit message given on command line (-m). To make
 that work, I had to also patch the index_differs_from function in
 diff-lib.c.

Which is exactly the same function I have to tweak to make my
status/commit: always show staged submodules regardless of
ignore config patch work for git commit -m too ;-)

I was doing that slightly differently but it seems that your way
of adding the ignore_submodules_arg parameter could serve us
both. Will update my upcoming patch accordingly.

 But I wonder if it would make more sense to start by teaching the
 --ignore-submodules option to git add. Then this patch could reuse
 that for commit -a.
 
 That sounds reasonable, however I don't think that any code from my
 patch would be affected, or would it? IOW, would commit really reuse
 anything? If not (or not much), there is probably no point in
 postponing this patch, support for git add may be added later.

You might be right as I didn't really check the actual codepaths. I
was deducing this from the observation that most changes were made
to builtin/add.c. And seeing a function like add_files_to_cache()
being modified made me expect that plain add would use the same
function to stage the files.

As Junio mentioned it would be great if you could teach the add
command also honor the --ignore-submodule command line option in
a companion patch. In the course of doing so you'll easily see if
I was right or not, then please just order them in the most logical
way.

Thanks for digging into this!

  Documentation/git-commit.txt|  6 ++
  builtin/add.c   | 16 +++
  builtin/checkout.c  |  2 +-
  builtin/commit.c| 10 --
  cache.h |  2 +-
  diff-lib.c  |  7 ++-
  diff.h  |  3 ++-
  sequencer.c |  4 ++--
  t/t7513-commit-ignore-submodules.sh | 39 
 +
  9 files changed, 77 insertions(+), 12 deletions(-)
  create mode 100644 t/t7513-commit-ignore-submodules.sh
 
 diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
 index 1a7616c..c8839c8 100644
 --- a/Documentation/git-commit.txt
 +++ b/Documentation/git-commit.txt
 @@ -13,6 +13,7 @@ SYNOPSIS
  [-F file | -m msg] [--reset-author] [--allow-empty]
  [--allow-empty-message] [--no-verify] [-e] [--author=author]
  [--date=date] [--cleanup=mode] [--[no-]status]
 +[--ignore-submodules[=when]]
  [-i | -o] [-S[keyid]] [--] [file...]
  
  DESCRIPTION
 @@ -271,6 +272,11 @@ The possible options are:
  The default can be changed using the status.showUntrackedFiles
  configuration variable documented in linkgit:git-config[1].
  
 +--ignore-submodules[=when]::
 + Can be used to override any settings of the 'ignore' option
 + in linkgit:git-config[1] or linkgit:gitmodules[5].
 + when can be either none or all, which is the default.
 +
  -v::
  --verbose::
   Show unified diff between the HEAD commit and what
 diff --git a/builtin/add.c b/builtin/add.c
 index 672adc0..1086294 100644
 --- a/builtin/add.c
 +++ b/builtin/add.c
 @@ -168,7 +168,8 @@ static void update_callback(struct diff_queue_struct *q,
  
  static void update_files_in_cache(const char *prefix,
 const struct pathspec *pathspec,
 -   struct update_callback_data *data)
 +   struct update_callback_data *data,
 +   const char *ignore_submodules_arg)
  {
   struct rev_info rev;
  
 @@ -180,17 +181,24 @@ static void update_files_in_cache(const char *prefix,
   rev.diffopt.format_callback = update_callback;
   rev.diffopt.format_callback_data = data;
   rev.max_count = 0; /* do not compare

Re: [PATCH v2.1] commit: add --ignore-submodules[=when] parameter

2014-03-31 Thread Jens Lehmann 
Am 31.03.2014 20:58, schrieb Jens Lehmann:
 Am 31.03.2014 02:07, schrieb Ronald Weiss:
 The tests immediately revealed, that the patch was not complete. It
 didn't work with commit message given on command line (-m). To make
 that work, I had to also patch the index_differs_from function in
 diff-lib.c.
 
 Which is exactly the same function I have to tweak to make my
 status/commit: always show staged submodules regardless of
 ignore config patch work for git commit -m too ;-)
 
 I was doing that slightly differently but it seems that your way
 of adding the ignore_submodules_arg parameter could serve us
 both. Will update my upcoming patch accordingly.

I've been hacking on that some more (I'll post it as soon as I
have all new tests up and running) and think we might be able to
handle that even easier. Please see below:

 diff --git a/diff-lib.c b/diff-lib.c
 index 2eddc66..fec5ad4 100644
 --- a/diff-lib.c
 +++ b/diff-lib.c
 @@ -508,7 +508,8 @@ int do_diff_cache(const unsigned char *tree_sha1, struct 
 diff_options *opt)
  return 0;
  }
  
 -int index_differs_from(const char *def, int diff_flags)
 +int index_differs_from(const char *def, int diff_flags,
 +   const char *ignore_submodules_arg)
  {
  struct rev_info rev;
  struct setup_revision_opt opt;
 @@ -520,6 +521,10 @@ int index_differs_from(const char *def, int diff_flags)
  DIFF_OPT_SET(rev.diffopt, QUICK);
  DIFF_OPT_SET(rev.diffopt, EXIT_WITH_STATUS);
  rev.diffopt.flags |= diff_flags;
 +if (ignore_submodules_arg) {
 +DIFF_OPT_SET(rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);

You'll have to do this unconditionally, as this option tells the
diff machinery to ignore any submodule configurations, which is
what we want in either case. But ...

 +handle_ignore_submodules_arg(rev.diffopt, 
 ignore_submodules_arg);
 +}
  run_diff_index(rev, 1);
  if (rev.pending.alloc)
  free(rev.pending.objects);

... maybe the best way is to leave index_differs_from() unchanged
and call that function with the correct diff_flags instead:

+   int diff_flags = DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+   if (ignore_submodule_arg 
+   !strcmp(ignore_submodule_arg, all))
+   diff_flags |= DIFF_OPT_IGNORE_SUBMODULES;
+   commitable = index_differs_from(parent, diff_flags);

Not thoroughly tested yet, but that'd also prevent any fallout for
the two callsites of index_differs_from() in sequencer.c.
--
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.1] commit: add --ignore-submodules[=when] parameter

2014-03-31 Thread Ronald Weiss 
On Mon, Mar 31, 2014 at 8:58 PM, Jens Lehmann jens.lehm...@web.de wrote:
 As Junio mentioned it would be great if you could teach the add
 command also honor the --ignore-submodule command line option in
 a companion patch. In the course of doing so you'll easily see if
 I was right or not, then please just order them in the most logical
 way.

Well, if You (or Junio) really don't want my patch without another one
for git add, I may try to do it. However, git add does not even honor
the submodules' ignore setting from .gitmodules (just tested with git
1.9.1: git add -u doesn't honor it, while git commit -a does). So
teaching git add the --ignore-submodules switch in current state
doesn't seem right to me. You might propose to add also support for
the ignore setting, to make add -u and commit -a more consistent.
That seems like a good idea, but the effort needed is getting bigger,
and nobody actually complains about lack of submodule ignoring
facility in git add, while I know that the current behavior of commit
really causes trouble.
--
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.1] commit: add --ignore-submodules[=when] parameter

2014-03-31 Thread Ronald Weiss 
On 31. 3. 2014 23:47, Ronald Weiss wrote:
 On Mon, Mar 31, 2014 at 8:58 PM, Jens Lehmann jens.lehm...@web.de wrote:
 As Junio mentioned it would be great if you could teach the add
 command also honor the --ignore-submodule command line option in
 a companion patch. In the course of doing so you'll easily see if
 I was right or not, then please just order them in the most logical
 way.
 
 Well, if You (or Junio) really don't want my patch without another one
 for git add, I may try to do it. However, git add does not even honor
 the submodules' ignore setting from .gitmodules (just tested with git
 1.9.1: git add -u doesn't honor it, while git commit -a does). So
 teaching git add the --ignore-submodules switch in current state
 doesn't seem right to me. You might propose to add also support for
 the ignore setting, to make add -u and commit -a more consistent.
 That seems like a good idea, but the effort needed is getting bigger,

Well, now I actually looked at it, and it was pretty easy after all.
The changes below seem to enable support for both ignore setting in
.gitmodules, and also --ignore-submodules switch, for git add, on top
of my patch for commit.

So I'm going to do some more testing, write tests for git add with
ignoring submodules the various ways, and then post two patches
rearranged, one for git add, and second for git commit on top of that,
as you guys suggested. Including the change suggested by Jens, to not
mess with index_differs_from() in diff-lib.c, that works fine too.


diff --git a/builtin/add.c b/builtin/add.c
index 1086294..9f70327 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -350,6 +350,8 @@ static int ignore_add_errors, intent_to_add, ignore_missing;
 static int addremove = ADDREMOVE_DEFAULT;
 static int addremove_explicit = -1; /* unspecified */

+static char *ignore_submodule_arg;
+
 static int ignore_removal_cb(const struct option *opt, const char *arg, int 
unset)
 {
/* if we are told to ignore, we are not adding removals */
@@ -375,6 +377,9 @@ static struct option builtin_add_options[] = {
OPT_BOOL( 0 , refresh, refresh_only, N_(don't add, only refresh the 
index)),
OPT_BOOL( 0 , ignore-errors, ignore_add_errors, N_(just skip files 
which cannot be added because of errors)),
OPT_BOOL( 0 , ignore-missing, ignore_missing, N_(check if - even 
missing - files are ignored in dry run)),
+   { OPTION_STRING, 0, ignore-submodules, ignore_submodule_arg, 
N_(when),
+ N_(ignore changes to submodules, optional when: all, none. (Default: 
all)),
+ PARSE_OPT_OPTARG, NULL, (intptr_t)all },
OPT_END(),
 };

@@ -422,6 +427,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
int implicit_dot = 0;
struct update_callback_data update_data;

+   gitmodules_config();
git_config(add_config, NULL);

argc = parse_options(argc, argv, prefix, builtin_add_options,
@@ -584,7 +590,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
memset(pathspec, 0, sizeof(pathspec));
}
update_data.flags = flags  ~ADD_CACHE_IMPLICIT_DOT;
-   update_files_in_cache(prefix, pathspec, update_data, NULL);
+   update_files_in_cache(prefix, pathspec, update_data, 
ignore_submodule_arg);

exit_status |= !!update_data.add_errors;
if (add_new_files)
--
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.1] commit: add --ignore-submodules[=when] parameter

2014-03-31 Thread Ronald Weiss 

On 1. 4. 2014 0:50, Ronald Weiss wrote:

On 31. 3. 2014 23:47, Ronald Weiss wrote:

On Mon, Mar 31, 2014 at 8:58 PM, Jens Lehmann jens.lehm...@web.de wrote:

As Junio mentioned it would be great if you could teach the add
command also honor the --ignore-submodule command line option in
a companion patch. In the course of doing so you'll easily see if
I was right or not, then please just order them in the most logical
way.


Well, if You (or Junio) really don't want my patch without another one
for git add, I may try to do it. However, git add does not even honor
the submodules' ignore setting from .gitmodules (just tested with git
1.9.1: git add -u doesn't honor it, while git commit -a does). So
teaching git add the --ignore-submodules switch in current state
doesn't seem right to me. You might propose to add also support for
the ignore setting, to make add -u and commit -a more consistent.
That seems like a good idea, but the effort needed is getting bigger,


Well, now I actually looked at it, and it was pretty easy after all.
The changes below seem to enable support for both ignore setting in
.gitmodules, and also --ignore-submodules switch, for git add, on top
of my patch for commit.


There is a catch. With the changes below, submodules are ignored by add 
even if explitely named on command line (eg. git add x does nothing if 
x is submodule with new commits, but with ignore=all in .gitmodules).

That doesn't seem right.

Any ideas, what to do about that? When exactly should such submodule be 
actually ignored?




So I'm going to do some more testing, write tests for git add with
ignoring submodules the various ways, and then post two patches
rearranged, one for git add, and second for git commit on top of that,
as you guys suggested. Including the change suggested by Jens, to not
mess with index_differs_from() in diff-lib.c, that works fine too.


diff --git a/builtin/add.c b/builtin/add.c
index 1086294..9f70327 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -350,6 +350,8 @@ static int ignore_add_errors, intent_to_add, ignore_missing;
  static int addremove = ADDREMOVE_DEFAULT;
  static int addremove_explicit = -1; /* unspecified */

+static char *ignore_submodule_arg;
+
  static int ignore_removal_cb(const struct option *opt, const char *arg, int 
unset)
  {
 /* if we are told to ignore, we are not adding removals */
@@ -375,6 +377,9 @@ static struct option builtin_add_options[] = {
 OPT_BOOL( 0 , refresh, refresh_only, N_(don't add, only refresh the 
index)),
 OPT_BOOL( 0 , ignore-errors, ignore_add_errors, N_(just skip files 
which cannot be added because of errors)),
 OPT_BOOL( 0 , ignore-missing, ignore_missing, N_(check if - even 
missing - files are ignored in dry run)),
+   { OPTION_STRING, 0, ignore-submodules, ignore_submodule_arg, 
N_(when),
+ N_(ignore changes to submodules, optional when: all, none. (Default: 
all)),
+ PARSE_OPT_OPTARG, NULL, (intptr_t)all },
 OPT_END(),
  };

@@ -422,6 +427,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 int implicit_dot = 0;
 struct update_callback_data update_data;

+   gitmodules_config();
 git_config(add_config, NULL);

 argc = parse_options(argc, argv, prefix, builtin_add_options,
@@ -584,7 +590,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 memset(pathspec, 0, sizeof(pathspec));
 }
 update_data.flags = flags  ~ADD_CACHE_IMPLICIT_DOT;
-   update_files_in_cache(prefix, pathspec, update_data, NULL);
+   update_files_in_cache(prefix, pathspec, update_data, 
ignore_submodule_arg);

 exit_status |= !!update_data.add_errors;
 if (add_new_files)


--
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.1] commit: add --ignore-submodules[=when] parameter

2014-03-30 Thread Ronald Weiss 
Git commit honors the 'ignore' setting from .gitmodules or .git/config,
but didn't allow to override it from command line, like other commands do.

Useful when values for commit are 'all' (default) or 'none'. The others
('dirty' and 'untracked') have same effect as 'none', as commit is only
interested in whether the submodule's HEAD differs from what is commited
in the superproject.

Changes in add.c and cache.h (and related compilo fix in checkout.c) are
needed to make it work for commit -a too.

Signed-off-by: Ronald Weiss weiss.ron...@gmail.com
---
The previous patch version (v2) contained bug in the test, by mistake I
have sent older version than I was testing with, sorry for that.

On 30. 3. 2014 21:48, Jens Lehmann wrote:
 Looking good so far, but we definitely need tests for this new option.

I added two simple tests (one for --ignore-submodules=all, another one
for =none). But I am sure the tests could be written better, by someone
more proficient in Git than I am.

The tests immediately revealed, that the patch was not complete. It
didn't work with commit message given on command line (-m). To make
that work, I had to also patch the index_differs_from function in
diff-lib.c.

 But I wonder if it would make more sense to start by teaching the
 --ignore-submodules option to git add. Then this patch could reuse
 that for commit -a.

That sounds reasonable, however I don't think that any code from my
patch would be affected, or would it? IOW, would commit really reuse
anything? If not (or not much), there is probably no point in
postponing this patch, support for git add may be added later.


 Documentation/git-commit.txt|  6 ++
 builtin/add.c   | 16 +++
 builtin/checkout.c  |  2 +-
 builtin/commit.c| 10 --
 cache.h |  2 +-
 diff-lib.c  |  7 ++-
 diff.h  |  3 ++-
 sequencer.c |  4 ++--
 t/t7513-commit-ignore-submodules.sh | 39 +
 9 files changed, 77 insertions(+), 12 deletions(-)
 create mode 100644 t/t7513-commit-ignore-submodules.sh

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 1a7616c..c8839c8 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -13,6 +13,7 @@ SYNOPSIS
   [-F file | -m msg] [--reset-author] [--allow-empty]
   [--allow-empty-message] [--no-verify] [-e] [--author=author]
   [--date=date] [--cleanup=mode] [--[no-]status]
+  [--ignore-submodules[=when]]
   [-i | -o] [-S[keyid]] [--] [file...]
 
 DESCRIPTION
@@ -271,6 +272,11 @@ The possible options are:
 The default can be changed using the status.showUntrackedFiles
 configuration variable documented in linkgit:git-config[1].
 
+--ignore-submodules[=when]::
+   Can be used to override any settings of the 'ignore' option
+   in linkgit:git-config[1] or linkgit:gitmodules[5].
+   when can be either none or all, which is the default.
+
 -v::
 --verbose::
Show unified diff between the HEAD commit and what
diff --git a/builtin/add.c b/builtin/add.c
index 672adc0..1086294 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -168,7 +168,8 @@ static void update_callback(struct diff_queue_struct *q,
 
 static void update_files_in_cache(const char *prefix,
  const struct pathspec *pathspec,
- struct update_callback_data *data)
+ struct update_callback_data *data,
+ const char *ignore_submodules_arg)
 {
struct rev_info rev;
 
@@ -180,17 +181,24 @@ static void update_files_in_cache(const char *prefix,
rev.diffopt.format_callback = update_callback;
rev.diffopt.format_callback_data = data;
rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
+
+   if (ignore_submodules_arg) {
+   DIFF_OPT_SET(rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+   handle_ignore_submodules_arg(rev.diffopt, 
ignore_submodules_arg);
+   }
+
run_diff_files(rev, DIFF_RACY_IS_MODIFIED);
 }
 
 int add_files_to_cache(const char *prefix,
-  const struct pathspec *pathspec, int flags)
+  const struct pathspec *pathspec, int flags,
+  const char *ignore_submodules_arg)
 {
struct update_callback_data data;
 
memset(data, 0, sizeof(data));
data.flags = flags;
-   update_files_in_cache(prefix, pathspec, data);
+   update_files_in_cache(prefix, pathspec, data, ignore_submodules_arg);
return !!data.add_errors;
 }
 
@@ -576,7 +584,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
memset(pathspec, 0, sizeof(pathspec));
}
update_data.flags = flags  ~ADD_CACHE_IMPLICIT_DOT;
-