[PATCH v6 2/2] commit: add --ignore-submodules[=] parameter
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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