Merge branch 'sb/submodule-rm-absorb'

"git rm" used to refuse to remove a submodule when it has its own
git repository embedded in its working tree.  It learned to move
the repository away to $GIT_DIR/modules/ of the superproject
instead, and allow the submodule to be deleted (as long as there
will be no loss of local modifications, that is).

* sb/submodule-rm-absorb:
  rm: absorb a submodules git dir before deletion
  submodule: rename and add flags to ok_to_remove_submodule
  submodule: modernize ok_to_remove_submodule to use argv_array
  submodule.h: add extern keyword to functions
This commit is contained in:
Junio C Hamano 2017-01-18 15:12:11 -08:00
commit 3ccd681c2a
4 changed files with 108 additions and 130 deletions

View File

@ -59,27 +59,9 @@ static void print_error_files(struct string_list *files_list,
} }
} }
static void error_removing_concrete_submodules(struct string_list *files, int *errs) static void submodules_absorb_gitdir_if_needed(const char *prefix)
{
print_error_files(files,
Q_("the following submodule (or one of its nested "
"submodules)\n"
"uses a .git directory:",
"the following submodules (or one of their nested "
"submodules)\n"
"use a .git directory:", files->nr),
_("\n(use 'rm -rf' if you really want to remove "
"it including all of its history)"),
errs);
string_list_clear(files, 0);
}
static int check_submodules_use_gitfiles(void)
{ {
int i; int i;
int errs = 0;
struct string_list files = STRING_LIST_INIT_NODUP;
for (i = 0; i < list.nr; i++) { for (i = 0; i < list.nr; i++) {
const char *name = list.entry[i].name; const char *name = list.entry[i].name;
int pos; int pos;
@ -99,12 +81,9 @@ static int check_submodules_use_gitfiles(void)
continue; continue;
if (!submodule_uses_gitfile(name)) if (!submodule_uses_gitfile(name))
string_list_append(&files, name); absorb_git_dir_into_superproject(prefix, name,
ABSORB_GITDIR_RECURSE_SUBMODULES);
} }
error_removing_concrete_submodules(&files, &errs);
return errs;
} }
static int check_local_mod(struct object_id *head, int index_only) static int check_local_mod(struct object_id *head, int index_only)
@ -120,7 +99,6 @@ static int check_local_mod(struct object_id *head, int index_only)
int errs = 0; int errs = 0;
struct string_list files_staged = STRING_LIST_INIT_NODUP; struct string_list files_staged = STRING_LIST_INIT_NODUP;
struct string_list files_cached = STRING_LIST_INIT_NODUP; struct string_list files_cached = STRING_LIST_INIT_NODUP;
struct string_list files_submodule = STRING_LIST_INIT_NODUP;
struct string_list files_local = STRING_LIST_INIT_NODUP; struct string_list files_local = STRING_LIST_INIT_NODUP;
no_head = is_null_oid(head); no_head = is_null_oid(head);
@ -187,7 +165,9 @@ static int check_local_mod(struct object_id *head, int index_only)
*/ */
if (ce_match_stat(ce, &st, 0) || if (ce_match_stat(ce, &st, 0) ||
(S_ISGITLINK(ce->ce_mode) && (S_ISGITLINK(ce->ce_mode) &&
!ok_to_remove_submodule(ce->name))) bad_to_remove_submodule(ce->name,
SUBMODULE_REMOVAL_DIE_ON_ERROR |
SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED)))
local_changes = 1; local_changes = 1;
/* /*
@ -217,13 +197,8 @@ static int check_local_mod(struct object_id *head, int index_only)
else if (!index_only) { else if (!index_only) {
if (staged_changes) if (staged_changes)
string_list_append(&files_cached, name); string_list_append(&files_cached, name);
if (local_changes) { if (local_changes)
if (S_ISGITLINK(ce->ce_mode) && string_list_append(&files_local, name);
!submodule_uses_gitfile(name))
string_list_append(&files_submodule, name);
else
string_list_append(&files_local, name);
}
} }
} }
print_error_files(&files_staged, print_error_files(&files_staged,
@ -245,8 +220,6 @@ static int check_local_mod(struct object_id *head, int index_only)
&errs); &errs);
string_list_clear(&files_cached, 0); string_list_clear(&files_cached, 0);
error_removing_concrete_submodules(&files_submodule, &errs);
print_error_files(&files_local, print_error_files(&files_local,
Q_("the following file has local modifications:", Q_("the following file has local modifications:",
"the following files have local modifications:", "the following files have local modifications:",
@ -340,6 +313,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
exit(0); exit(0);
} }
if (!index_only)
submodules_absorb_gitdir_if_needed(prefix);
/* /*
* If not forced, the file, the index and the HEAD (if exists) * If not forced, the file, the index and the HEAD (if exists)
* must match; but the file can already been removed, since * must match; but the file can already been removed, since
@ -356,9 +332,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
oidclr(&oid); oidclr(&oid);
if (check_local_mod(&oid, index_only)) if (check_local_mod(&oid, index_only))
exit(1); exit(1);
} else if (!index_only) {
if (check_submodules_use_gitfiles())
exit(1);
} }
/* /*
@ -387,32 +360,20 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
*/ */
if (!index_only) { if (!index_only) {
int removed = 0, gitmodules_modified = 0; int removed = 0, gitmodules_modified = 0;
struct strbuf buf = STRBUF_INIT;
for (i = 0; i < list.nr; i++) { for (i = 0; i < list.nr; i++) {
const char *path = list.entry[i].name; const char *path = list.entry[i].name;
if (list.entry[i].is_submodule) { if (list.entry[i].is_submodule) {
if (is_empty_dir(path)) { struct strbuf buf = STRBUF_INIT;
if (!rmdir(path)) {
removed = 1; strbuf_addstr(&buf, path);
if (!remove_path_from_gitmodules(path)) if (remove_dir_recursively(&buf, 0))
gitmodules_modified = 1; die(_("could not remove '%s'"), path);
continue; strbuf_release(&buf);
}
} else { removed = 1;
strbuf_reset(&buf); if (!remove_path_from_gitmodules(path))
strbuf_addstr(&buf, path); gitmodules_modified = 1;
if (!remove_dir_recursively(&buf, 0)) { continue;
removed = 1;
if (!remove_path_from_gitmodules(path))
gitmodules_modified = 1;
strbuf_release(&buf);
continue;
} else if (!file_exists(path))
/* Submodule was removed by user */
if (!remove_path_from_gitmodules(path))
gitmodules_modified = 1;
/* Fallthrough and let remove_path() fail. */
}
} }
if (!remove_path(path)) { if (!remove_path(path)) {
removed = 1; removed = 1;
@ -421,7 +382,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
if (!removed) if (!removed)
die_errno("git rm: '%s'", path); die_errno("git rm: '%s'", path);
} }
strbuf_release(&buf);
if (gitmodules_modified) if (gitmodules_modified)
stage_updated_gitmodules(); stage_updated_gitmodules();
} }

View File

@ -1143,45 +1143,64 @@ int submodule_uses_gitfile(const char *path)
return 1; return 1;
} }
int ok_to_remove_submodule(const char *path) /*
* Check if it is a bad idea to remove a submodule, i.e. if we'd lose data
* when doing so.
*
* Return 1 if we'd lose data, return 0 if the removal is fine,
* and negative values for errors.
*/
int bad_to_remove_submodule(const char *path, unsigned flags)
{ {
ssize_t len; ssize_t len;
struct child_process cp = CHILD_PROCESS_INIT; struct child_process cp = CHILD_PROCESS_INIT;
const char *argv[] = {
"status",
"--porcelain",
"-u",
"--ignore-submodules=none",
NULL,
};
struct strbuf buf = STRBUF_INIT; struct strbuf buf = STRBUF_INIT;
int ok_to_remove = 1; int ret = 0;
if (!file_exists(path) || is_empty_dir(path)) if (!file_exists(path) || is_empty_dir(path))
return 1;
if (!submodule_uses_gitfile(path))
return 0; return 0;
cp.argv = argv; if (!submodule_uses_gitfile(path))
return 1;
argv_array_pushl(&cp.args, "status", "--porcelain",
"--ignore-submodules=none", NULL);
if (flags & SUBMODULE_REMOVAL_IGNORE_UNTRACKED)
argv_array_push(&cp.args, "-uno");
else
argv_array_push(&cp.args, "-uall");
if (!(flags & SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED))
argv_array_push(&cp.args, "--ignored");
prepare_submodule_repo_env(&cp.env_array); prepare_submodule_repo_env(&cp.env_array);
cp.git_cmd = 1; cp.git_cmd = 1;
cp.no_stdin = 1; cp.no_stdin = 1;
cp.out = -1; cp.out = -1;
cp.dir = path; cp.dir = path;
if (start_command(&cp)) if (start_command(&cp)) {
die("Could not run 'git status --porcelain -uall --ignore-submodules=none' in submodule %s", path); if (flags & SUBMODULE_REMOVAL_DIE_ON_ERROR)
die(_("could not start 'git status in submodule '%s'"),
path);
ret = -1;
goto out;
}
len = strbuf_read(&buf, cp.out, 1024); len = strbuf_read(&buf, cp.out, 1024);
if (len > 2) if (len > 2)
ok_to_remove = 0; ret = 1;
close(cp.out); close(cp.out);
if (finish_command(&cp)) if (finish_command(&cp)) {
die("'git status --porcelain -uall --ignore-submodules=none' failed in submodule %s", path); if (flags & SUBMODULE_REMOVAL_DIE_ON_ERROR)
die(_("could not run 'git status in submodule '%s'"),
path);
ret = -1;
}
out:
strbuf_release(&buf); strbuf_release(&buf);
return ok_to_remove; return ret;
} }
static int find_first_merges(struct object_array *result, const char *path, static int find_first_merges(struct object_array *result, const char *path,

View File

@ -30,55 +30,63 @@ struct submodule_update_strategy {
}; };
#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL} #define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL}
int is_staging_gitmodules_ok(void); extern int is_staging_gitmodules_ok(void);
int update_path_in_gitmodules(const char *oldpath, const char *newpath); extern int update_path_in_gitmodules(const char *oldpath, const char *newpath);
int remove_path_from_gitmodules(const char *path); extern int remove_path_from_gitmodules(const char *path);
void stage_updated_gitmodules(void); extern void stage_updated_gitmodules(void);
void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt, extern void set_diffopt_flags_from_submodule_config(struct diff_options *,
const char *path); const char *path);
int submodule_config(const char *var, const char *value, void *cb); extern int submodule_config(const char *var, const char *value, void *cb);
void gitmodules_config(void); extern void gitmodules_config(void);
extern void gitmodules_config_sha1(const unsigned char *commit_sha1); extern void gitmodules_config_sha1(const unsigned char *commit_sha1);
extern int is_submodule_initialized(const char *path); extern int is_submodule_initialized(const char *path);
extern int is_submodule_populated(const char *path); extern int is_submodule_populated(const char *path);
int parse_submodule_update_strategy(const char *value, extern int parse_submodule_update_strategy(const char *value,
struct submodule_update_strategy *dst); struct submodule_update_strategy *dst);
const char *submodule_strategy_to_string(const struct submodule_update_strategy *s); extern const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *); extern void handle_ignore_submodules_arg(struct diff_options *, const char *);
void show_submodule_summary(FILE *f, const char *path, extern void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix, const char *line_prefix,
struct object_id *one, struct object_id *two, struct object_id *one, struct object_id *two,
unsigned dirty_submodule, const char *meta, unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset); const char *del, const char *add, const char *reset);
void show_submodule_inline_diff(FILE *f, const char *path, extern void show_submodule_inline_diff(FILE *f, const char *path,
const char *line_prefix, const char *line_prefix,
struct object_id *one, struct object_id *two, struct object_id *one, struct object_id *two,
unsigned dirty_submodule, const char *meta, unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset, const char *del, const char *add, const char *reset,
const struct diff_options *opt); const struct diff_options *opt);
void set_config_fetch_recurse_submodules(int value); extern void set_config_fetch_recurse_submodules(int value);
void check_for_new_submodule_commits(unsigned char new_sha1[20]); extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
int fetch_populated_submodules(const struct argv_array *options, extern int fetch_populated_submodules(const struct argv_array *options,
const char *prefix, int command_line_option, const char *prefix, int command_line_option,
int quiet, int max_parallel_jobs); int quiet, int max_parallel_jobs);
unsigned is_submodule_modified(const char *path, int ignore_untracked); extern unsigned is_submodule_modified(const char *path, int ignore_untracked);
int submodule_uses_gitfile(const char *path); extern int submodule_uses_gitfile(const char *path);
int ok_to_remove_submodule(const char *path);
int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20], #define SUBMODULE_REMOVAL_DIE_ON_ERROR (1<<0)
const unsigned char a[20], const unsigned char b[20], int search); #define SUBMODULE_REMOVAL_IGNORE_UNTRACKED (1<<1)
int find_unpushed_submodules(struct sha1_array *commits, const char *remotes_name, #define SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED (1<<2)
struct string_list *needs_pushing); extern int bad_to_remove_submodule(const char *path, unsigned flags);
extern int merge_submodule(unsigned char result[20], const char *path,
const unsigned char base[20],
const unsigned char a[20],
const unsigned char b[20], int search);
extern int find_unpushed_submodules(struct sha1_array *commits,
const char *remotes_name,
struct string_list *needs_pushing);
extern int push_unpushed_submodules(struct sha1_array *commits, extern int push_unpushed_submodules(struct sha1_array *commits,
const char *remotes_name, const char *remotes_name,
int dry_run); int dry_run);
int parallel_submodules(void); extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
extern int parallel_submodules(void);
/* /*
* Prepare the "env_array" parameter of a "struct child_process" for executing * Prepare the "env_array" parameter of a "struct child_process" for executing
* a submodule by clearing any repo-specific envirionment variables, but * a submodule by clearing any repo-specific envirionment variables, but
* retaining any config in the environment. * retaining any config in the environment.
*/ */
void prepare_submodule_repo_env(struct argv_array *out); extern void prepare_submodule_repo_env(struct argv_array *out);
#define ABSORB_GITDIR_RECURSE_SUBMODULES (1<<0) #define ABSORB_GITDIR_RECURSE_SUBMODULES (1<<0)
extern void absorb_git_dir_into_superproject(const char *prefix, extern void absorb_git_dir_into_superproject(const char *prefix,

View File

@ -569,26 +569,22 @@ test_expect_success 'rm of a conflicted unpopulated submodule succeeds' '
test_cmp expect actual test_cmp expect actual
' '
test_expect_success 'rm of a populated submodule with a .git directory fails even when forced' ' test_expect_success 'rm of a populated submodule with a .git directory migrates git dir' '
git checkout -f master && git checkout -f master &&
git reset --hard && git reset --hard &&
git submodule update && git submodule update &&
(cd submod && (cd submod &&
rm .git && rm .git &&
cp -R ../.git/modules/sub .git && cp -R ../.git/modules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree GIT_WORK_TREE=. git config --unset core.worktree &&
rm -r ../.git/modules/sub
) && ) &&
test_must_fail git rm submod && git rm submod 2>output.err &&
test -d submod && ! test -d submod &&
test -d submod/.git && ! test -d submod/.git &&
git status -s -uno --ignore-submodules=none >actual && git status -s -uno --ignore-submodules=none >actual &&
! test -s actual && test -s actual &&
test_must_fail git rm -f submod && test_i18ngrep Migrating output.err
test -d submod &&
test -d submod/.git &&
git status -s -uno --ignore-submodules=none >actual &&
! test -s actual &&
rm -rf submod
' '
cat >expect.deepmodified <<EOF cat >expect.deepmodified <<EOF
@ -667,24 +663,19 @@ test_expect_success 'rm of a populated nested submodule with a nested .git direc
git submodule update --recursive && git submodule update --recursive &&
(cd submod/subsubmod && (cd submod/subsubmod &&
rm .git && rm .git &&
cp -R ../../.git/modules/sub/modules/sub .git && mv ../../.git/modules/sub/modules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree GIT_WORK_TREE=. git config --unset core.worktree
) && ) &&
test_must_fail git rm submod && git rm submod 2>output.err &&
test -d submod && ! test -d submod &&
test -d submod/subsubmod/.git && ! test -d submod/subsubmod/.git &&
git status -s -uno --ignore-submodules=none >actual && git status -s -uno --ignore-submodules=none >actual &&
! test -s actual && test -s actual &&
test_must_fail git rm -f submod && test_i18ngrep Migrating output.err
test -d submod &&
test -d submod/subsubmod/.git &&
git status -s -uno --ignore-submodules=none >actual &&
! test -s actual &&
rm -rf submod
' '
test_expect_success 'checking out a commit after submodule removal needs manual updates' ' test_expect_success 'checking out a commit after submodule removal needs manual updates' '
git commit -m "submodule removal" submod && git commit -m "submodule removal" submod .gitmodules &&
git checkout HEAD^ && git checkout HEAD^ &&
git submodule update && git submodule update &&
git checkout -q HEAD^ && git checkout -q HEAD^ &&