The main motivations for disallowing git commands within an unpopulated submodule are: Whenever we run "git -C status" within an unpopulated submodule, it falls back to the superproject. This occurs since there is no .git file in the submodule directory. So superproject's status gets displayed. Also, the user's intention is not clear behind running the command in an unpopulated submodule. Hence we prefer to error out. When we run the command "git -C sub add ." within a submodule, the results observed are: In the case of the populated submodule, it acts like running “git add .“ inside the submodule. This is uncontroversial and runs as expected. In the case of the unpopulated submodule, the user's intention behind entering the above command is unclear. He may have intended to add the submodule to the superproject or to add all files inside the sub/ directory to the submodule or superproject. Hence we’ll prefer to error out in these case. Eventually, we use a check_prefix_inside_submodule to see check if the path is inside an unpopulated submodule. If it is, then we report the user about the unpopulated submodule. Signed-off-by: Prathamesh Chavan <pc44800@xxxxxxxxx> --- Since this patch effectively uses RUN_SETUP, builtin commands like 'diff' and other non-builtin commands are not filtered. For such cases, I think, we need to handle them separately. Also since currently, git-submodule is not a builtin command, the command for initializing and updating the submodule doesn't return an error message, but once it is converted to builtin, we need to handle its case explicitly. The build report of this patch is available on: https://travis-ci.org/pratham-pc/git/builds/219030999 Also, the above patch was initially my GSoC project topic, but I changed it later on and added these bug fixes to my wishlist of the proposal. (Had to send the patch again since a typo occurred while sending previous the mail) builtin/submodule--helper.c | 4 ---- git.c | 3 +++ submodule.c | 45 ++++++++++++++++++++++++++++++++++++++++ submodule.h | 6 ++++++ t/t6134-pathspec-in-submodule.sh | 2 +- 5 files changed, 55 insertions(+), 5 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 15a5430c0..4f7c7d7b8 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -219,10 +219,6 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr return 0; } -struct module_list { - const struct cache_entry **entries; - int alloc, nr; -}; #define MODULE_LIST_INIT { NULL, 0, 0 } static int module_list_compute(int argc, const char **argv, diff --git a/git.c b/git.c index 33f52acbc..eefe3fb01 100644 --- a/git.c +++ b/git.c @@ -2,6 +2,7 @@ #include "exec_cmd.h" #include "help.h" #include "run-command.h" +#include "submodule.h" const char git_usage_string[] = "git [--version] [--help] [-C <path>] [-c name=value]\n" @@ -364,6 +365,8 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) if (prefix) die("can't use --super-prefix from a subdirectory"); } + if (prefix) + check_prefix_inside_submodule(prefix); if (!help && p->option & NEED_WORK_TREE) setup_work_tree(); diff --git a/submodule.c b/submodule.c index 0a2831d84..d2c3023bf 100644 --- a/submodule.c +++ b/submodule.c @@ -545,6 +545,51 @@ void set_config_fetch_recurse_submodules(int value) config_fetch_recurse_submodules = value; } +#define MODULE_LIST_INIT { NULL, 0, 0 } + +void check_prefix_inside_submodule(const char *prefix) +{ + struct module_list list = MODULE_LIST_INIT; + int i; + + if (read_cache() < 0) + die(_("index file corrupt")); + + for (i = 0; i < active_nr; i++) { + const struct cache_entry *ce = active_cache[i]; + + if (!S_ISGITLINK(ce->ce_mode)) + continue; + + ALLOC_GROW(list.entries, list.nr + 1, list.alloc); + list.entries[list.nr++] = ce; + while (i + 1 < active_nr && + !strcmp(ce->name, active_cache[i + 1]->name)) + /* + * Skip entries with the same name in different stages + * to make sure an entry is returned only once. + */ + i++; + } + + for(i = 0; i < list.nr; i++) { + if(strlen((*list.entries[i]).name) == strlen(prefix)) { + if (!strcmp((*list.entries[i]).name, prefix)) { + /* This case cannot happen because */ + die("BUG: prefixes end with '/', but we do not record ending slashes in the index"); + } + } + else if(strlen((*list.entries[i]).name) == strlen(prefix)-1) { + const char *out = NULL; + if(skip_prefix(prefix, (*list.entries[i]).name, &out)) { + if(strlen(out) == 1 && out[0] == '/') + die(_("command from inside unpopulated submodule '%s' not supported."), (*list.entries[i]).name); + } + } + } + +} + static int has_remote(const char *refname, const struct object_id *oid, int flags, void *cb_data) { diff --git a/submodule.h b/submodule.h index 05ab674f0..5e41e5afc 100644 --- a/submodule.h +++ b/submodule.h @@ -31,6 +31,12 @@ struct submodule_update_strategy { }; #define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL} +struct module_list { + const struct cache_entry **entries; + int alloc, nr; +}; + +extern void check_prefix_inside_submodule(const char *prefix); extern int is_staging_gitmodules_ok(void); extern int update_path_in_gitmodules(const char *oldpath, const char *newpath); extern int remove_path_from_gitmodules(const char *path); diff --git a/t/t6134-pathspec-in-submodule.sh b/t/t6134-pathspec-in-submodule.sh index fd401ca60..086cc4c47 100755 --- a/t/t6134-pathspec-in-submodule.sh +++ b/t/t6134-pathspec-in-submodule.sh @@ -25,7 +25,7 @@ test_expect_success 'error message for path inside submodule' ' ' cat <<EOF >expect -fatal: Pathspec '.' is in submodule 'sub' +fatal: command from inside unpopulated submodule 'sub' not supported. EOF test_expect_success 'error message for path inside submodule from within submodule' ' -- 2.11.0