[PATCH v6 2/2] commit: add --ignore-submodules[=] parameter

2014-04-24 Thread Ronald Weiss
Allow ignoring submodules (or not) by command line switch, like diff
and status do.

Git commit honors the 'ignore' setting from .gitmodules or .git/config,
but didn't allow to override it from command line.

This patch depends on Jens Lehmann's patch "commit -m: commit staged
submodules regardless of ignore config". Without it,
"commit -m --ignore-submodules" would not work and tests introduced
here would fail.

Signed-off-by: Ronald Weiss 
---
Patch changelog:
v6
* corrected wording and formatting errors (as pointed out by Eric Sunshine)
v5
* fixed file mode of added test script (644 -> 755)
* replaced test_might_fail with test_must_fail in test script

 Documentation/git-commit.txt|  7 
 builtin/commit.c|  8 +++-
 t/t7513-commit-ignore-submodules.sh | 80 +
 3 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100755 t/t7513-commit-ignore-submodules.sh

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0bbc8f5..55995be 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -13,6 +13,7 @@ SYNOPSIS
   [-F  | -m ] [--reset-author] [--allow-empty]
   [--allow-empty-message] [--no-verify] [-e] [--author=]
   [--date=] [--cleanup=] [--[no-]status]
+  [--ignore-submodules[=]]
   [-i | -o] [-S[]] [--] [...]
 
 DESCRIPTION
@@ -277,6 +278,12 @@ The possible options are:
 The default can be changed using the status.showUntrackedFiles
 configuration variable documented in linkgit:git-config[1].
 
+--ignore-submodules[=]::
+   Can be used to override any settings of the 'submodule.*.ignore'
+   option in linkgit:git-config[1] or linkgit:gitmodules[5].
+can be either "none", "dirty, "untracked" or "all",
+   defaulting to "all".
+
 -v::
 --verbose::
Show unified diff between the HEAD commit and what
diff --git a/builtin/commit.c b/builtin/commit.c
index 5444111..dc1d8d0 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -361,7 +361,7 @@ static char *prepare_index(int argc, const char **argv, 
const char *prefix,
 */
if (all || (also && pathspec.nr)) {
fd = hold_locked_index(&index_lock, 1);
-   add_files_to_cache(also ? prefix : NULL, &pathspec, 0, NULL);
+   add_files_to_cache(also ? prefix : NULL, &pathspec, 0, 
ignore_submodule_arg);
refresh_cache_or_die(refresh_flags);
update_main_cache_tree(WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
@@ -1540,6 +1540,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
OPT_BOOL(0, "amend", &amend, N_("amend previous commit")),
OPT_BOOL(0, "no-post-rewrite", &no_post_rewrite, N_("bypass 
post-rewrite hook")),
{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, 
N_("mode"), N_("show untracked files, optional modes: all, normal, no. 
(Default: all)"), PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
+   { 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" },
/* end commit contents options */
 
OPT_HIDDEN_BOOL(0, "allow-empty", &allow_empty,
@@ -1578,6 +1581,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
argc = parse_and_validate_options(argc, argv, builtin_commit_options,
  builtin_commit_usage,
  prefix, current_head, &s);
+
+   s.ignore_submodule_arg = ignore_submodule_arg;
+
if (dry_run)
return dry_run_commit(argc, argv, prefix, current_head, &s);
index_file = prepare_index(argc, argv, prefix, current_head, 0);
diff --git a/t/t7513-commit-ignore-submodules.sh 
b/t/t7513-commit-ignore-submodules.sh
new file mode 100755
index 000..10ae178
--- /dev/null
+++ b/t/t7513-commit-ignore-submodules.sh
@@ -0,0 +1,80 @@
+#!/bin/sh
+#
+# Copyright (c) 2014 Ronald Weiss
+#
+
+test_description='Test of git commit --ignore-submodules'
+
+. ./test-lib.sh
+
+test_expect_success 'create submodule' '
+   test_create_repo sm &&
+   (
+   cd sm &&
+   >foo &&
+   git add foo &&
+   git commit -m "Add foo"
+   ) &&
+   git submodule add ./sm &&
+   git commit -m "Add sm"
+'
+
+update_sm () 

[PATCH v6 1/2] add: add --ignore-submodules[=] parameter

2014-04-24 Thread Ronald Weiss
Allow ignoring submodules (or not) by command line switch, like diff
and status do.

This commit is also a prerequisite for the next one in series, which
adds the --ignore-submodules switch to git commit. That's why a new
argument is added to public function add_files_to_cache(), and its
call sites are updated to pass NULL.

Signed-off-by: Ronald Weiss 
---
Git add currently doesn't honor ignore setting from .gitmodules or
.git/config, which is related functionality, however I'd like to
change that in another patch, coming soon.

Patch changelog:
v6
* corrected wording and formatting errors (as pointed out by Eric Sunshine)
v5
* fixed file mode of added test script (644 -> 755)
* cleaned up commit message

 Documentation/git-add.txt|  7 +-
 builtin/add.c| 16 --
 builtin/checkout.c   |  2 +-
 builtin/commit.c |  2 +-
 cache.h  |  2 +-
 t/t3704-add-ignore-submodules.sh | 47 
 6 files changed, 70 insertions(+), 6 deletions(-)
 create mode 100755 t/t3704-add-ignore-submodules.sh

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 9631526..ef69f8b 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
  [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
  [--intent-to-add | -N] [--refresh] [--ignore-errors] 
[--ignore-missing]
- [--] [...]
+ [--ignore-submodules[=]] [--] [...]
 
 DESCRIPTION
 ---
@@ -164,6 +164,11 @@ for "git add --no-all ...", i.e. ignored removed 
files.
be ignored, no matter if they are already present in the work
tree or not.
 
+--ignore-submodules[=]::
+   Can be used to override any settings of the 'submodule.*.ignore'
+   option in linkgit:git-config[1] or linkgit:gitmodules[5].
+can be either "none" or "all", defaulting to "all".
+
 \--::
This option can be used to separate command-line options from
the list of files, (useful when filenames might be mistaken
diff --git a/builtin/add.c b/builtin/add.c
index 459208a..85f2110 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -83,7 +83,8 @@ static void update_callback(struct diff_queue_struct *q,
 }
 
 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;
struct rev_info rev;
@@ -99,6 +100,12 @@ int add_files_to_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);
return !!data.add_errors;
 }
@@ -237,6 +244,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 */
@@ -262,6 +271,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(),
 };
 
@@ -434,7 +446,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
plug_bulk_checkin();
 
-   exit_status |= add_files_to_cache(prefix, &pathspec, flags);
+   exit_status |= add_files_to_cache(prefix, &pathspec, flags, 
ignore_submodule_arg);
 
if (add_new_files)
exit_status |= add_files(&dir, flags);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 07cf555..607af47 100644
--- a/builtin/checkout.c
+++ b/builtin/chec

Re: [PATCH v2.1] commit: add --ignore-submodules[=] 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


[PATCH v5 2/2] commit: add --ignore-submodules[=] parameter

2014-04-22 Thread Ronald Weiss
Allow ignoring submodules (or not) by command line switch, like diff
and status do.

Git commit honors the 'ignore' setting from .gitmodules or .git/config,
but didn't allow to override it from command line.

This patch depends on Jens Lehmann's patch "commit -m: commit staged
submodules regardless of ignore config". Without it,
"commit -m --ignore-submodules" would not work and tests introduced
here would fail.

Signed-off-by: Ronald Weiss 
---
Changes against v4 of this patch:
* fixed file mode of added test script (644 -> 755)
* replaced test_might_fail with test_must_fail in test script

 Documentation/git-commit.txt|  7 
 builtin/commit.c|  8 +++-
 t/t7513-commit-ignore-submodules.sh | 78 +
 3 files changed, 92 insertions(+), 1 deletion(-)
 create mode 100755 t/t7513-commit-ignore-submodules.sh

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0bbc8f5..de0e8fe 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -13,6 +13,7 @@ SYNOPSIS
   [-F  | -m ] [--reset-author] [--allow-empty]
   [--allow-empty-message] [--no-verify] [-e] [--author=]
   [--date=] [--cleanup=] [--[no-]status]
+  [--ignore-submodules[=]]
   [-i | -o] [-S[]] [--] [...]
 
 DESCRIPTION
@@ -277,6 +278,12 @@ The possible options are:
 The default can be changed using the status.showUntrackedFiles
 configuration variable documented in linkgit:git-config[1].
 
+--ignore-submodules[=]::
+   Can be used to override any settings of the 'submodule.*.ignore'
+   option in linkgit:git-config[1] or linkgit:gitmodules[5].
+can be either "none", "dirty, "untracked" or "all", which
+   is the default.
+
 -v::
 --verbose::
Show unified diff between the HEAD commit and what
diff --git a/builtin/commit.c b/builtin/commit.c
index a148e28..8c4d05e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -361,7 +361,7 @@ static char *prepare_index(int argc, const char **argv, 
const char *prefix,
 */
if (all || (also && pathspec.nr)) {
fd = hold_locked_index(&index_lock, 1);
-   add_files_to_cache(also ? prefix : NULL, &pathspec, 0, NULL);
+   add_files_to_cache(also ? prefix : NULL, &pathspec, 0, 
ignore_submodule_arg);
refresh_cache_or_die(refresh_flags);
update_main_cache_tree(WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
@@ -1526,6 +1526,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
OPT_BOOL(0, "amend", &amend, N_("amend previous commit")),
OPT_BOOL(0, "no-post-rewrite", &no_post_rewrite, N_("bypass 
post-rewrite hook")),
{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, 
N_("mode"), N_("show untracked files, optional modes: all, normal, no. 
(Default: all)"), PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
+   { 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" },
/* end commit contents options */
 
OPT_HIDDEN_BOOL(0, "allow-empty", &allow_empty,
@@ -1564,6 +1567,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
argc = parse_and_validate_options(argc, argv, builtin_commit_options,
  builtin_commit_usage,
  prefix, current_head, &s);
+
+   s.ignore_submodule_arg = ignore_submodule_arg;
+
if (dry_run)
return dry_run_commit(argc, argv, prefix, current_head, &s);
index_file = prepare_index(argc, argv, prefix, current_head, 0);
diff --git a/t/t7513-commit-ignore-submodules.sh 
b/t/t7513-commit-ignore-submodules.sh
new file mode 100755
index 000..52ab37d
--- /dev/null
+++ b/t/t7513-commit-ignore-submodules.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+#
+# Copyright (c) 2014 Ronald Weiss
+#
+
+test_description='Test of git commit --ignore-submodules'
+
+. ./test-lib.sh
+
+test_expect_success 'create submodule' '
+   test_create_repo sm && (
+   cd sm &&
+   >foo &&
+   git add foo &&
+   git commit -m "Add foo"
+   ) &&
+   git submodule add ./sm &&
+   git commit -m "Add sm"
+'
+
+update_sm () {
+   (cd sm &&
+   echo bar >> foo &&
+ 

[PATCH v5 1/2] add: add --ignore-submodules[=] parameter

2014-04-22 Thread Ronald Weiss
Allow ignoring submodules (or not) by command line switch, like diff
and status do.

This commit is also a prerequisite for the next one in series, which
adds the --ignore-submodules switch to git commit. That's why a new
argument is added to public function add_files_to_cache(), and it's
call sites are updated to pass NULL.

Signed-off-by: Ronald Weiss 
---
Git add currently doesn't honor ignore setting from .gitmodules or
.git/config, which is related functionality, however I'd like to
change that in another patch, coming soon.

Changes against v4 of this patch:
* fixed file mode of added test script (644 -> 755)
* cleaned up commit message

 Documentation/git-add.txt|  7 ++-
 builtin/add.c| 16 --
 builtin/checkout.c   |  2 +-
 builtin/commit.c |  2 +-
 cache.h  |  2 +-
 t/t3704-add-ignore-submodules.sh | 45 
 6 files changed, 68 insertions(+), 6 deletions(-)
 create mode 100755 t/t3704-add-ignore-submodules.sh

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 9631526..b2c936f 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
  [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
  [--intent-to-add | -N] [--refresh] [--ignore-errors] 
[--ignore-missing]
- [--] [...]
+ [--ignore-submodules[=]] [--] [...]
 
 DESCRIPTION
 ---
@@ -164,6 +164,11 @@ for "git add --no-all ...", i.e. ignored removed 
files.
be ignored, no matter if they are already present in the work
tree or not.
 
+--ignore-submodules[=]::
+   Can be used to override any settings of the 'submodule.*.ignore'
+   option in linkgit:git-config[1] or linkgit:gitmodules[5].
+can be either "none" or "all", which is the default.
+
 \--::
This option can be used to separate command-line options from
the list of files, (useful when filenames might be mistaken
diff --git a/builtin/add.c b/builtin/add.c
index 459208a..85f2110 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -83,7 +83,8 @@ static void update_callback(struct diff_queue_struct *q,
 }
 
 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;
struct rev_info rev;
@@ -99,6 +100,12 @@ int add_files_to_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);
return !!data.add_errors;
 }
@@ -237,6 +244,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 */
@@ -262,6 +271,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(),
 };
 
@@ -434,7 +446,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
plug_bulk_checkin();
 
-   exit_status |= add_files_to_cache(prefix, &pathspec, flags);
+   exit_status |= add_files_to_cache(prefix, &pathspec, flags, 
ignore_submodule_arg);
 
if (add_new_files)
exit_status |= add_files(&dir, flags);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 07cf555..607af47 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -525,7 +525,7 @@ static int merge_working_tree(const struct 

Re: [PATCH v4 2/2] commit: add --ignore-submodules[=] parameter

2014-04-21 Thread Ronald Weiss
On 18. 4. 2014 14:09, Jens Lehmann wrote:
> Am 13.04.2014 00:49, schrieb Ronald Weiss:
>> Allow ignoring submodules (or not) by command line switch, like diff
>> and status do.
>>
>> Git commit honors the 'ignore' setting from .gitmodules or .git/config,
>> but didn't allow to override it from command line.
>>
>> This patch depends on Jens Lehmann's patch "commit -m: commit staged
>> submodules regardless of ignore config". Without it,
>> "commit -m --ignore-submodules" would not work and tests introduced
>> here would fail.
> 
> Apart from the flags of the test (see below) I get three failures when
> running t7513. And I'm wondering why you are using "test_might_fail"
> there, shouldn't we know exactly if a commit should succeed or not
> and test exactly for that?

I used "test_might_fail" instead of "test_must_fail" to denote that this
test doesn't care whether "git commit" fails or not due to empty commit
message. I found it more appropriate. However, if you disagree, I can
change it to "test_must_fail", no problem for me.

Unfortunately I don't know why the test fails for you, they pass for me.
Did you apply it on top of your own patch for "commit -m", which is
a prerequisite?

I tried it again, on top of current master (cc29195 [tag: v2.0.0-rc0]).
First, I have applied your patch from here:

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

On top of that, I applied my two patches, and the tests pass fine here.
Until now I was testing on Windows, but now I tested on a Linux box,
and that makes no difference.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 1/2] add: add --ignore-submodules[=] parameter

2014-04-21 Thread Ronald Weiss
On 18. 4. 2014 13:53, Jens Lehmann wrote:
> Am 13.04.2014 00:45, schrieb Ronald Weiss:
>> Allow ignoring submodules (or not) by command line switch, like diff
>> and status do.
>>
>> Git add currently doesn't honor ignore from .gitmodules or .git/config,
>> which is related functionality, however I'd like to change that in
>> another patch, coming soon.
> 
> I think we should drop this paragraph from this commit message. Though
> I believe it's helpful to add this information after the "---" below
> to inform readers of the list of your future plans.
> 
>> This commit is also a prerequisite for the next one in series, which
>> adds the --ignore-submodules switch to git commit.
> 
> But this information definitely belongs here.
> 
>> That's why signature
>> of function add_files_to_cache was changed.
> 
> But that's necessary for this patch too, no?

No, for this patch alone, we could just use the global variable, which
is set up by parse_options(), without changing the public function and
breaking compilation in other files.

> 
>> That also requires compilo fixes in checkout.c and commit.c
> 
> Sorry, but I don't know what a "compilo fix" is ;-) .. I suspect you
> mean that add_files_to_cache() needs a new NULL argument in some
> places. What about dropping the last two sentences and adding
> something like "Add a new argument to add_files_to_cache() to pass
> the command line option and update all other call sites to pass
> NULL instead." to the first paragraph?
> 
> Apart from that and the flags of the test (see below) this patch
> is looking good to me.

OK, I'll update the commit message, and fix the file mode for the added
test script. Will repost once we sort out the problems (failing tests)
You have with the second patch (for commit).

>> diff --git a/t/t3704-add-ignore-submodules.sh 
>> b/t/t3704-add-ignore-submodules.sh
>> new file mode 100644
> 
> The flags should be 100755 here, currently "make test" fails because
> of this.

I'm sorry for this, I was testing it on Windows, where the file mode
doesn't matter, that's why I missed it.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/2] commit: add --ignore-submodules[=] parameter

2014-04-14 Thread Ronald Weiss
On 14. 4. 2014 20:30, Junio C Hamano wrote:
> Ronald Weiss  writes:
> 
>> On 8. 4. 2014 20:43, Jens Lehmann wrote:
>>>> Useful  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.
>>>
>>> Unless it outputs a status message, then 'dirty' and 'untracked' do
>>> influence what is shown there. Apart from that (and maybe tests for
>>> these two cases ;-) this is looking good to me.
>>
>> OK, I updated the patch for commit to take that into account. Also, I
>> rebased both patches onto current master. Sending them in a moment.
>>
>> If you don't have any more complaints, can I add "Acked-by: " and
>> resend the patches to Junio?
> 
> It is not "When I see no more complaints, I'll resend with your
> Ack".  An Ack is a positive thing, not lack of discovery of further
> issues.

I'm really sorry if the tone of my message sounded harsh to you, it
wasn't meant like that at all.

> Rather, it is more like "I'll wait for your Acks and then I'll
> resend with your Ack", or "If they look good, reply with Ack and let
> the maintainer pick them up".

OK.
--
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[=] 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


[PATCH v4 2/2] commit: add --ignore-submodules[=] parameter

2014-04-12 Thread Ronald Weiss
Allow ignoring submodules (or not) by command line switch, like diff
and status do.

Git commit honors the 'ignore' setting from .gitmodules or .git/config,
but didn't allow to override it from command line.

This patch depends on Jens Lehmann's patch "commit -m: commit staged
submodules regardless of ignore config". Without it,
"commit -m --ignore-submodules" would not work and tests introduced
here would fail.

Signed-off-by: Ronald Weiss 
---
 Documentation/git-commit.txt|  7 
 builtin/commit.c|  8 +++-
 t/t7513-commit-ignore-submodules.sh | 78 +
 3 files changed, 92 insertions(+), 1 deletion(-)
 create mode 100644 t/t7513-commit-ignore-submodules.sh

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0bbc8f5..de0e8fe 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -13,6 +13,7 @@ SYNOPSIS
   [-F  | -m ] [--reset-author] [--allow-empty]
   [--allow-empty-message] [--no-verify] [-e] [--author=]
   [--date=] [--cleanup=] [--[no-]status]
+  [--ignore-submodules[=]]
   [-i | -o] [-S[]] [--] [...]
 
 DESCRIPTION
@@ -277,6 +278,12 @@ The possible options are:
 The default can be changed using the status.showUntrackedFiles
 configuration variable documented in linkgit:git-config[1].
 
+--ignore-submodules[=]::
+   Can be used to override any settings of the 'submodule.*.ignore'
+   option in linkgit:git-config[1] or linkgit:gitmodules[5].
+can be either "none", "dirty, "untracked" or "all", which
+   is the default.
+
 -v::
 --verbose::
Show unified diff between the HEAD commit and what
diff --git a/builtin/commit.c b/builtin/commit.c
index a148e28..8c4d05e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -361,7 +361,7 @@ static char *prepare_index(int argc, const char **argv, 
const char *prefix,
 */
if (all || (also && pathspec.nr)) {
fd = hold_locked_index(&index_lock, 1);
-   add_files_to_cache(also ? prefix : NULL, &pathspec, 0, NULL);
+   add_files_to_cache(also ? prefix : NULL, &pathspec, 0, 
ignore_submodule_arg);
refresh_cache_or_die(refresh_flags);
update_main_cache_tree(WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
@@ -1526,6 +1526,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
OPT_BOOL(0, "amend", &amend, N_("amend previous commit")),
OPT_BOOL(0, "no-post-rewrite", &no_post_rewrite, N_("bypass 
post-rewrite hook")),
{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, 
N_("mode"), N_("show untracked files, optional modes: all, normal, no. 
(Default: all)"), PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
+   { 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" },
/* end commit contents options */
 
OPT_HIDDEN_BOOL(0, "allow-empty", &allow_empty,
@@ -1564,6 +1567,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
argc = parse_and_validate_options(argc, argv, builtin_commit_options,
  builtin_commit_usage,
  prefix, current_head, &s);
+
+   s.ignore_submodule_arg = ignore_submodule_arg;
+
if (dry_run)
return dry_run_commit(argc, argv, prefix, current_head, &s);
index_file = prepare_index(argc, argv, prefix, current_head, 0);
diff --git a/t/t7513-commit-ignore-submodules.sh 
b/t/t7513-commit-ignore-submodules.sh
new file mode 100644
index 000..52ab37d
--- /dev/null
+++ b/t/t7513-commit-ignore-submodules.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+#
+# Copyright (c) 2014 Ronald Weiss
+#
+
+test_description='Test of git commit --ignore-submodules'
+
+. ./test-lib.sh
+
+test_expect_success 'create submodule' '
+   test_create_repo sm && (
+   cd sm &&
+   >foo &&
+   git add foo &&
+   git commit -m "Add foo"
+   ) &&
+   git submodule add ./sm &&
+   git commit -m "Add sm"
+'
+
+update_sm () {
+   (cd sm &&
+   echo bar >> foo &&
+   git add foo &&
+   git commit -m "Updated foo"
+   )
+}
+
+test_expect_success 'commit -a --ignore-s

[PATCH v4 1/2] add: add --ignore-submodules[=] parameter

2014-04-12 Thread Ronald Weiss
Allow ignoring submodules (or not) by command line switch, like diff
and status do.

Git add currently doesn't honor ignore from .gitmodules or .git/config,
which is related functionality, however I'd like to change that in
another patch, coming soon.

This commit is also a prerequisite for the next one in series, which
adds the --ignore-submodules switch to git commit. That's why signature
of function add_files_to_cache was changed. That also requires compilo
fixes in checkout.c and commit.c

Signed-off-by: Ronald Weiss 
---
 Documentation/git-add.txt|  7 ++-
 builtin/add.c| 16 --
 builtin/checkout.c   |  2 +-
 builtin/commit.c |  2 +-
 cache.h  |  2 +-
 t/t3704-add-ignore-submodules.sh | 45 
 6 files changed, 68 insertions(+), 6 deletions(-)
 create mode 100644 t/t3704-add-ignore-submodules.sh

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 9631526..b2c936f 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
  [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
  [--intent-to-add | -N] [--refresh] [--ignore-errors] 
[--ignore-missing]
- [--] [...]
+ [--ignore-submodules[=]] [--] [...]
 
 DESCRIPTION
 ---
@@ -164,6 +164,11 @@ for "git add --no-all ...", i.e. ignored removed 
files.
be ignored, no matter if they are already present in the work
tree or not.
 
+--ignore-submodules[=]::
+   Can be used to override any settings of the 'submodule.*.ignore'
+   option in linkgit:git-config[1] or linkgit:gitmodules[5].
+can be either "none" or "all", which is the default.
+
 \--::
This option can be used to separate command-line options from
the list of files, (useful when filenames might be mistaken
diff --git a/builtin/add.c b/builtin/add.c
index 459208a..85f2110 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -83,7 +83,8 @@ static void update_callback(struct diff_queue_struct *q,
 }
 
 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;
struct rev_info rev;
@@ -99,6 +100,12 @@ int add_files_to_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);
return !!data.add_errors;
 }
@@ -237,6 +244,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 */
@@ -262,6 +271,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(),
 };
 
@@ -434,7 +446,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
plug_bulk_checkin();
 
-   exit_status |= add_files_to_cache(prefix, &pathspec, flags);
+   exit_status |= add_files_to_cache(prefix, &pathspec, flags, 
ignore_submodule_arg);
 
if (add_new_files)
exit_status |= add_files(&dir, flags);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 07cf555..607af47 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -525,7 +525,7 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
 * entries in the index.
 */
 
-   add_files_to_cache(NULL,

Re: [PATCH v3 2/2] commit: add --ignore-submodules[=] parameter

2014-04-12 Thread Ronald Weiss

On 8. 4. 2014 20:43, Jens Lehmann wrote:

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


Unless it outputs a status message, then 'dirty' and 'untracked' do
influence what is shown there. Apart from that (and maybe tests for
these two cases ;-) this is looking good to me.


OK, I updated the patch for commit to take that into account. Also, I
rebased both patches onto current master. Sending them in a moment.

If you don't have any more complaints, can I add "Acked-by: " and
resend the patches to Junio?
--
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 2/2] commit: add --ignore-submodules[=] parameter

2014-04-08 Thread Ronald Weiss
On 8. 4. 2014 20:43, Jens Lehmann wrote:
> Am 08.04.2014 01:03, 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  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.
> 
> Unless it outputs a status message, then 'dirty' and 'untracked' do
> influence what is shown there. Apart from that (and maybe tests for
> these two cases ;-) this is looking good to me.

Hm, You mean the status message, which is pre-inserted as comment into
the commit message, when opening an editor to write the commit message?

OK, that really makes a difference, although really small and actually
affecting nothing. I'll take it into account. But are You sure the tests
for this would be actually useful? If only effect of them would be
increasing time needed to run the full test suite, then it's better to
not have them :-). But I can do that, if You still think it's useful.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 2/2] commit: add --ignore-submodules[=] parameter

2014-04-07 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  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.

This patch depends on Jens Lehmann's patch "commit -m: commit staged
submodules regardless of ignore config". Without it,
"commit -m --ignore-submodules" would not work and tests introduced
here would fail.

Signed-off-by: Ronald Weiss 
---
 Documentation/git-commit.txt|  6 ++
 builtin/commit.c|  8 ++-
 t/t7513-commit-ignore-submodules.sh | 42 +
 3 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 t/t7513-commit-ignore-submodules.sh

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 1a7616c..8d3b2db 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -13,6 +13,7 @@ SYNOPSIS
   [-F  | -m ] [--reset-author] [--allow-empty]
   [--allow-empty-message] [--no-verify] [-e] [--author=]
   [--date=] [--cleanup=] [--[no-]status]
+  [--ignore-submodules[=]]
   [-i | -o] [-S[]] [--] [...]
 
 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[=]::
+   Can be used to override any settings of the 'submodule.*.ignore'
+   option in linkgit:git-config[1] or linkgit:gitmodules[5].
+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/commit.c b/builtin/commit.c
index 0db215b..121c185 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -360,7 +360,7 @@ static char *prepare_index(int argc, const char **argv, 
const char *prefix,
 */
if (all || (also && pathspec.nr)) {
fd = hold_locked_index(&index_lock, 1);
-   add_files_to_cache(also ? prefix : NULL, &pathspec, 0, NULL);
+   add_files_to_cache(also ? prefix : NULL, &pathspec, 0, 
ignore_submodule_arg);
refresh_cache_or_die(refresh_flags);
update_main_cache_tree(WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
@@ -1492,6 +1492,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
OPT_BOOL(0, "amend", &amend, N_("amend previous commit")),
OPT_BOOL(0, "no-post-rewrite", &no_post_rewrite, N_("bypass 
post-rewrite hook")),
{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, 
N_("mode"), N_("show untracked files, optional modes: all, normal, no. 
(Default: all)"), PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
+   { 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" },
/* end commit contents options */
 
OPT_HIDDEN_BOOL(0, "allow-empty", &allow_empty,
@@ -1531,6 +1534,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
argc = parse_and_validate_options(argc, argv, builtin_commit_options,
  builtin_commit_usage,
  prefix, current_head, &s);
+
+   s.ignore_submodule_arg = ignore_submodule_arg;
+
if (dry_run)
return dry_run_commit(argc, argv, prefix, current_head, &s);
    index_file = prepare_index(argc, argv, prefix, current_head, 0);
diff --git a/t/t7513-commit-ignore-submodules.sh 
b/t/t7513-commit-ignore-submodules.sh
new file mode 100644
index 000..83ce04c
--- /dev/null
+++ b/t/t7513-commit-ignore-submodules.sh
@@ -0,0 +1,42 @@
+#!/bin/sh
+#
+# Copyright (c) 2014 Ronald Weiss
+#
+
+test_description='Test of git commit --ignore-submodules'
+
+. ./test-lib.sh
+
+test_expect_success 'create submodule' '
+   test_create_repo sm && (
+   cd sm &&
+   >foo &&
+   git add foo &&
+   git commit -m "Add foo"
+   ) &&
+   git submodule add ./sm &&
+   git commit -m "Add sm"
+'
+
+update_sm () {
+   (cd sm &&
+   ec

[PATCH v3 1/2] add: add --ignore-submodules[=] parameter

2014-04-07 Thread Ronald Weiss
Allow overriding the ignore setting from config, using the command line
parameter like diff and status have. Git add currently doesn't honor
ignore from .gitmodules, but it does honor it from .git/config. And
support for .gitmodules will come in another patch.

Useful  values are 'none' and 'all' (the default), the other values
('dirty' and 'untracked') being equivalent to 'none' for add's purposes.

Also add ignore_submodules_arg parameter to function add_files_to_cache,
which will allow implementing --ignore-submodules also for other commands
using this function (for commit command, in particular, coming in
subsequent commit). This requires compilo fixes in checkout.c and commit.c

Signed-off-by: Ronald Weiss 
---
I have changed order of commits, from what Jens proposed, to avoid the patch
for commit (coming right after this one) having to mess too much with add's
source code.

If anyone disagrees, let me know, and I will redo it as needed.

I'll look in to the related "add [-f] vs .gitmodules:ignore=all" problem
soon.

 Documentation/git-add.txt|  7 ++-
 builtin/add.c| 21 +++
 builtin/checkout.c   |  2 +-
 builtin/commit.c |  2 +-
 cache.h  |  2 +-
 t/t3704-add-ignore-submodules.sh | 45 
 6 files changed, 71 insertions(+), 8 deletions(-)
 create mode 100644 t/t3704-add-ignore-submodules.sh

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 48754cb..be2e7b5 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
  [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
  [--intent-to-add | -N] [--refresh] [--ignore-errors] 
[--ignore-missing]
- [--] [...]
+ [--ignore-submodules[=]] [--] [...]
 
 DESCRIPTION
 ---
@@ -160,6 +160,11 @@ today's "git add ...", ignoring removed files.
be ignored, no matter if they are already present in the work
tree or not.
 
+--ignore-submodules[=]::
+   Can be used to override any settings of the 'submodule.*.ignore'
+   option in linkgit:git-config[1] or linkgit:gitmodules[5].
+can be either "none" or "all", which is the default.
+
 \--::
This option can be used to separate command-line options from
the list of files, (useful when filenames might be mistaken
diff --git a/builtin/add.c b/builtin/add.c
index 672adc0..72ef792 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;
 }
 
@@ -342,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 */
@@ -367,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 , 

Re: [PATCH v2.1] commit: add --ignore-submodules[=] 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  
>>>>>>>> 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 "add"ish 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
>> conf

Re: [PATCH v2.1] commit: add --ignore-submodules[=] 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  
>>>>>> 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 "add"ish 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[=] 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  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[=] 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  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


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

2014-03-31 Thread Ronald Weiss
On Mon, Mar 31, 2014 at 10:37 PM, Jens Lehmann  wrote:
> ... 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.

Thanks for this hint, that is really much nicer. I'll test that, and
post updated patch if it works.
--
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[=] parameter

2014-03-31 Thread Ronald Weiss
On Mon, Mar 31, 2014 at 8:58 PM, Jens Lehmann  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


[PATCH v2.1] commit: add --ignore-submodules[=] 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  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 
---
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  | -m ] [--reset-author] [--allow-empty]
   [--allow-empty-message] [--no-verify] [-e] [--author=]
   [--date=] [--cleanup=] [--[no-]status]
+  [--ignore-submodules[=]]
   [-i | -o] [-S[]] [--] [...]
 
 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[=]::
+   Can be used to override any settings of the 'ignore' option
+   in linkgit:git-config[1] or linkgit:gitmodules[5].
+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

[PATCH v2] commit: add --ignore-submodules[=] 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  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 
---
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  | -m ] [--reset-author] [--allow-empty]
   [--allow-empty-message] [--no-verify] [-e] [--author=]
   [--date=] [--cleanup=] [--[no-]status]
+  [--ignore-submodules[=]]
   [-i | -o] [-S[]] [--] [...]
 
 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[=]::
+   Can be used to override any settings of the 'ignore' option
+   in linkgit:git-config[1] or linkgit:gitmodules[5].
+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;
-   update_files_

Re: [PATCH 2/2] status: don't ignore submodules added to index

2014-03-29 Thread Ronald Weiss

On 30. 3. 2014 0:40, Ronald Weiss wrote:

That change was really too aggresive, the one below seems better, all
tests pass with it, and it still works.


Oops, some tests still fail, sorry for my blindness. Nevermind, I'm 
looking forward to Your fix.

--
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] status: don't ignore submodules added to index

2014-03-29 Thread Ronald Weiss
On 30. 3. 2014 0:16, Jens Lehmann wrote:
> Thanks, but I think this patch falls a bit short (I assume you should see
> test failures with this patch). I'm currently working on fixing that, will
> post that as soon as I finished it.

You're right, 3 tests from t7508 failed with that, I'm sorry for not
verifying that :-(.

That change was really too aggresive, the one below seems better, all
tests pass with it, and it still works. But I'm not sending it as patch
anymore, knowing that You are already working on better solution.

diff --git a/wt-status.c b/wt-status.c
index a452407..520835e 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -489,4 +489,7 @@ static void wt_status_collect_changes_index(struct 
wt_status *s)
if (s->ignore_submodule_arg) {
DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
handle_ignore_submodules_arg(&rev.diffopt, 
s->ignore_submodule_arg);
+   } else {
+   DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+   DIFF_OPT_CLR(&rev.diffopt, IGNORE_SUBMODULES);
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] status: don't ignore submodules added to index

2014-03-29 Thread Ronald Weiss
Submodules explicitly added to index by user should be never hidden in
status output.

This also fixes a bug in commit, where submodules with configured ignore
setting (in .gitmodules or .git/config), added to index by user, are not
displayed in the commit message as being commited, but they still are
commited. Unless the changed submodules are the only changes in the index,
in such case commit fails immediately with "no changes", which is even
worse.

Signed-off-by: Ronald Weiss 
---
 wt-status.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index a452407..108a048 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -486,10 +486,8 @@ static void wt_status_collect_changes_index(struct 
wt_status *s)
opt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;
setup_revisions(0, NULL, &rev, &opt);
 
-   if (s->ignore_submodule_arg) {
-   DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
-   handle_ignore_submodules_arg(&rev.diffopt, 
s->ignore_submodule_arg);
-   }
+   DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+   DIFF_OPT_CLR(&rev.diffopt, IGNORE_SUBMODULES);
 
rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
rev.diffopt.format_callback = wt_status_collect_updated_cb;
-- 
1.9.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


[PATCH 1/2] commit: add --ignore-submodules[=] parameter

2014-03-29 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  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 
---
 Documentation/git-commit.txt |  6 ++
 builtin/add.c| 16 
 builtin/checkout.c   |  2 +-
 builtin/commit.c |  8 +++-
 cache.h  |  2 +-
 5 files changed, 27 insertions(+), 7 deletions(-)

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  | -m ] [--reset-author] [--allow-empty]
   [--allow-empty-message] [--no-verify] [-e] [--author=]
   [--date=] [--cleanup=] [--[no-]status]
+  [--ignore-submodules[=]]
   [-i | -o] [-S[]] [--] [...]
 
 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[=]::
+   Can be used to override any settings of the 'ignore' option
+   in linkgit:git-config[1] or linkgit:gitmodules[5].
+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;
-   update_files_in_cache(prefix, &pathspec, &update_data);
+   update_files_in_cache(prefix, &pathspec, &update_data, NULL);
 
exit_status |= !!update_data.add_errors;
if (add_new_files)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index ada51fa..22a4b48 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -525,7 +525,7 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
 * entries in the index.
 */
 
-   add_files_to_cache(NULL, NULL, 0);
+   add_files_to_cache(NULL, NULL, 0, NULL);
/*
 * NEEDSWORK: carrying over local changes
 * when branches have different end-of-line
diff --git a/builtin/commit.c b/builtin/commit.c
index 26b2986..121c185 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -360,7 +360,7 @@ static char *prepare_index(int argc, const char **argv, 
const char *prefix,
 */
if (all || (also && pathspec.nr)) {
fd = hold_locked_index(&index_lock, 1);
-   add_files_to_cache(also ? prefix : NULL, &pathspec, 0);
+   add_files_to_cache(also ? prefix : NULL, &pathspec, 0, 
ignor

Re: git commit vs. ignore-submodules

2014-03-29 Thread Ronald Weiss
On Fri, Mar 28, 2014 at 5:47 PM, Jens Lehmann  wrote:
> Such a patch would be very much appreciated. You might want to look
> into other commands that already have the --ignore-submodules option
> to get an idea how to do that. cmd_status() in builtin/commit.c
> should be a good starting point.

OK, I managed to create a patch, will post it as separate email right
after this one. I also had to touch builtin/add.c, to make the new
--ignore-submodes option work with "git commit -a" too.

>> And also, I'd like to know git folks' opinion on whether it's OK that
>> git commit with ignore=all in .gitmodules ignores submodules even when
>> they are explicitely staged with git add.
>
> No, they should be visible in status and commit when the user chose
> to add them. I see if I can hack something up (as I've been bitten
> myself by that recently ;-).

While I was playing with that now, I discovered that the problem is
actually not in commit, but only in status. Commit really commits the
submodule, but it's not visible in the commit message (generated by
status), that confused me before. And if there are no other changes in
the index, then commit fails immediately with "no changes". A small
change in wt-status.c fixes that for me, but I'm not sure about any
undesired side effects. I'll post a patch for that too, in a few
moments.
--
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


git commit vs. ignore-submodules

2014-03-27 Thread Ronald Weiss
Hello.

As this is my first post to this list, let me first thank all the
people involved in Git development - it's really a great tool.

Now to the point. Since Git 1.8 (I think), git commit command honours
the submodules' ignore settings, configured either in .gitmodules, or
in .git/config. That's very nice and certainly correct for "git commit
-a", but it's less clear if one explicitely stages an updated
submodule using git add. Git commit will ignore it anyway, if
ignore=all is configured in .gitmodules. Maybe that's correct too, I'm
not sure about that, but it's inconvenient in our use case, especially
combined with the lack of --ignore-submodule parameter to git commit,
as git status and git diff have.

We use submodules in such a way that normally we don't ever want to
see changes in them in output of git diff and git status. So we set
ignore=all in .gitmodules for each submodule. But occasionally, we
need to add a new submodule, and sometimes also commit changed
submodule. This got harder with Git 1.8, we have to "git config
submodule..ignore none" before the commit, and "git config
--unset ..." after.

I'd like to at least add an --ignore-submodules parameter to git
commit. I though about posting a patch, but as I looked into the
commit source file, I didn't see any straightforward way to implement
it. I don't have enough free time for a deeper analysis of the
sources, I'm sorry.

So please, let me first know, whether you could possibly accept such
patch, and if so, then I'd really appreciate some hints on how to do
it.

And also, I'd like to know git folks' opinion on whether it's OK that
git commit with ignore=all in .gitmodules ignores submodules even when
they are explicitely staged with git add.

Thanks in advance for any reply,
Ronald Weiss
--
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