Skip to content

Commit 3ccd681

Browse files
gitstergitster
committed
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
2 parents 55d128a + 55856a3 commit 3ccd681

File tree

4 files changed

+107
-129
lines changed

4 files changed

+107
-129
lines changed

‎builtin/rm.c‎

Lines changed: 22 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -59,27 +59,9 @@ static void print_error_files(struct string_list *files_list,
5959
}
6060
}
6161

62-
static void error_removing_concrete_submodules(struct string_list *files, int *errs)
63-
{
64-
print_error_files(files,
65-
Q_("the following submodule (or one of its nested "
66-
"submodules)\n"
67-
"uses a .git directory:",
68-
"the following submodules (or one of their nested "
69-
"submodules)\n"
70-
"use a .git directory:", files->nr),
71-
_("\n(use 'rm -rf' if you really want to remove "
72-
"it including all of its history)"),
73-
errs);
74-
string_list_clear(files, 0);
75-
}
76-
77-
static int check_submodules_use_gitfiles(void)
62+
static void submodules_absorb_gitdir_if_needed(const char *prefix)
7863
{
7964
int i;
80-
int errs = 0;
81-
struct string_list files = STRING_LIST_INIT_NODUP;
82-
8365
for (i = 0; i < list.nr; i++) {
8466
const char *name = list.entry[i].name;
8567
int pos;
@@ -99,12 +81,9 @@ static int check_submodules_use_gitfiles(void)
9981
continue;
10082

10183
if (!submodule_uses_gitfile(name))
102-
string_list_append(&files, name);
84+
absorb_git_dir_into_superproject(prefix, name,
85+
ABSORB_GITDIR_RECURSE_SUBMODULES);
10386
}
104-
105-
error_removing_concrete_submodules(&files, &errs);
106-
107-
return errs;
10887
}
10988

11089
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)
12099
int errs = 0;
121100
struct string_list files_staged = STRING_LIST_INIT_NODUP;
122101
struct string_list files_cached = STRING_LIST_INIT_NODUP;
123-
struct string_list files_submodule = STRING_LIST_INIT_NODUP;
124102
struct string_list files_local = STRING_LIST_INIT_NODUP;
125103

126104
no_head = is_null_oid(head);
@@ -187,7 +165,9 @@ static int check_local_mod(struct object_id *head, int index_only)
187165
*/
188166
if (ce_match_stat(ce, &st, 0) ||
189167
(S_ISGITLINK(ce->ce_mode) &&
190-
!ok_to_remove_submodule(ce->name)))
168+
bad_to_remove_submodule(ce->name,
169+
SUBMODULE_REMOVAL_DIE_ON_ERROR |
170+
SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED)))
191171
local_changes = 1;
192172

193173
/*
@@ -217,13 +197,8 @@ static int check_local_mod(struct object_id *head, int index_only)
217197
else if (!index_only) {
218198
if (staged_changes)
219199
string_list_append(&files_cached, name);
220-
if (local_changes) {
221-
if (S_ISGITLINK(ce->ce_mode) &&
222-
!submodule_uses_gitfile(name))
223-
string_list_append(&files_submodule, name);
224-
else
225-
string_list_append(&files_local, name);
226-
}
200+
if (local_changes)
201+
string_list_append(&files_local, name);
227202
}
228203
}
229204
print_error_files(&files_staged,
@@ -245,8 +220,6 @@ static int check_local_mod(struct object_id *head, int index_only)
245220
&errs);
246221
string_list_clear(&files_cached, 0);
247222

248-
error_removing_concrete_submodules(&files_submodule, &errs);
249-
250223
print_error_files(&files_local,
251224
Q_("the following file has local modifications:",
252225
"the following files have local modifications:",
@@ -340,6 +313,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
340313
exit(0);
341314
}
342315

316+
if (!index_only)
317+
submodules_absorb_gitdir_if_needed(prefix);
318+
343319
/*
344320
* If not forced, the file, the index and the HEAD (if exists)
345321
* 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)
356332
oidclr(&oid);
357333
if (check_local_mod(&oid, index_only))
358334
exit(1);
359-
} else if (!index_only) {
360-
if (check_submodules_use_gitfiles())
361-
exit(1);
362335
}
363336

364337
/*
@@ -387,32 +360,20 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
387360
*/
388361
if (!index_only) {
389362
int removed = 0, gitmodules_modified = 0;
390-
struct strbuf buf = STRBUF_INIT;
391363
for (i = 0; i < list.nr; i++) {
392364
const char *path = list.entry[i].name;
393365
if (list.entry[i].is_submodule) {
394-
if (is_empty_dir(path)) {
395-
if (!rmdir(path)) {
396-
removed = 1;
397-
if (!remove_path_from_gitmodules(path))
398-
gitmodules_modified = 1;
399-
continue;
400-
}
401-
} else {
402-
strbuf_reset(&buf);
403-
strbuf_addstr(&buf, path);
404-
if (!remove_dir_recursively(&buf, 0)) {
405-
removed = 1;
406-
if (!remove_path_from_gitmodules(path))
407-
gitmodules_modified = 1;
408-
strbuf_release(&buf);
409-
continue;
410-
} else if (!file_exists(path))
411-
/* Submodule was removed by user */
412-
if (!remove_path_from_gitmodules(path))
413-
gitmodules_modified = 1;
414-
/* Fallthrough and let remove_path() fail. */
415-
}
366+
struct strbuf buf = STRBUF_INIT;
367+
368+
strbuf_addstr(&buf, path);
369+
if (remove_dir_recursively(&buf, 0))
370+
die(_("could not remove '%s'"), path);
371+
strbuf_release(&buf);
372+
373+
removed = 1;
374+
if (!remove_path_from_gitmodules(path))
375+
gitmodules_modified = 1;
376+
continue;
416377
}
417378
if (!remove_path(path)) {
418379
removed = 1;
@@ -421,7 +382,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
421382
if (!removed)
422383
die_errno("git rm: '%s'", path);
423384
}
424-
strbuf_release(&buf);
425385
if (gitmodules_modified)
426386
stage_updated_gitmodules();
427387
}

‎submodule.c‎

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,45 +1143,64 @@ int submodule_uses_gitfile(const char *path)
11431143
return 1;
11441144
}
11451145

1146-
int ok_to_remove_submodule(const char *path)
1146+
/*
1147+
* Check if it is a bad idea to remove a submodule, i.e. if we'd lose data
1148+
* when doing so.
1149+
*
1150+
* Return 1 if we'd lose data, return 0 if the removal is fine,
1151+
* and negative values for errors.
1152+
*/
1153+
int bad_to_remove_submodule(const char *path, unsigned flags)
11471154
{
11481155
ssize_t len;
11491156
struct child_process cp = CHILD_PROCESS_INIT;
1150-
const char *argv[] = {
1151-
"status",
1152-
"--porcelain",
1153-
"-u",
1154-
"--ignore-submodules=none",
1155-
NULL,
1156-
};
11571157
struct strbuf buf = STRBUF_INIT;
1158-
int ok_to_remove = 1;
1158+
int ret = 0;
11591159

11601160
if (!file_exists(path) || is_empty_dir(path))
1161-
return 1;
1161+
return 0;
11621162

11631163
if (!submodule_uses_gitfile(path))
1164-
return 0;
1164+
return 1;
1165+
1166+
argv_array_pushl(&cp.args, "status", "--porcelain",
1167+
"--ignore-submodules=none", NULL);
1168+
1169+
if (flags & SUBMODULE_REMOVAL_IGNORE_UNTRACKED)
1170+
argv_array_push(&cp.args, "-uno");
1171+
else
1172+
argv_array_push(&cp.args, "-uall");
1173+
1174+
if (!(flags & SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED))
1175+
argv_array_push(&cp.args, "--ignored");
11651176

1166-
cp.argv = argv;
11671177
prepare_submodule_repo_env(&cp.env_array);
11681178
cp.git_cmd = 1;
11691179
cp.no_stdin = 1;
11701180
cp.out = -1;
11711181
cp.dir = path;
1172-
if (start_command(&cp))
1173-
die("Could not run 'git status --porcelain -uall --ignore-submodules=none' in submodule %s", path);
1182+
if (start_command(&cp)) {
1183+
if (flags & SUBMODULE_REMOVAL_DIE_ON_ERROR)
1184+
die(_("could not start 'git status in submodule '%s'"),
1185+
path);
1186+
ret = -1;
1187+
goto out;
1188+
}
11741189

11751190
len = strbuf_read(&buf, cp.out, 1024);
11761191
if (len > 2)
1177-
ok_to_remove = 0;
1192+
ret = 1;
11781193
close(cp.out);
11791194

1180-
if (finish_command(&cp))
1181-
die("'git status --porcelain -uall --ignore-submodules=none' failed in submodule %s", path);
1182-
1195+
if (finish_command(&cp)) {
1196+
if (flags & SUBMODULE_REMOVAL_DIE_ON_ERROR)
1197+
die(_("could not run 'git status in submodule '%s'"),
1198+
path);
1199+
ret = -1;
1200+
}
1201+
out:
11831202
strbuf_release(&buf);
1184-
return ok_to_remove;
1203+
return ret;
11851204
}
11861205

11871206
static int find_first_merges(struct object_array *result, const char *path,

‎submodule.h‎

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -30,55 +30,63 @@ struct submodule_update_strategy {
3030
};
3131
#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL}
3232

33-
int is_staging_gitmodules_ok(void);
34-
int update_path_in_gitmodules(const char *oldpath, const char *newpath);
35-
int remove_path_from_gitmodules(const char *path);
36-
void stage_updated_gitmodules(void);
37-
void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
33+
extern int is_staging_gitmodules_ok(void);
34+
extern int update_path_in_gitmodules(const char *oldpath, const char *newpath);
35+
extern int remove_path_from_gitmodules(const char *path);
36+
extern void stage_updated_gitmodules(void);
37+
extern void set_diffopt_flags_from_submodule_config(struct diff_options *,
3838
const char *path);
39-
int submodule_config(const char *var, const char *value, void *cb);
40-
void gitmodules_config(void);
39+
extern int submodule_config(const char *var, const char *value, void *cb);
40+
extern void gitmodules_config(void);
4141
extern void gitmodules_config_sha1(const unsigned char *commit_sha1);
4242
extern int is_submodule_initialized(const char *path);
4343
extern int is_submodule_populated(const char *path);
44-
int parse_submodule_update_strategy(const char *value,
44+
extern int parse_submodule_update_strategy(const char *value,
4545
struct submodule_update_strategy *dst);
46-
const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
47-
void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
48-
void show_submodule_summary(FILE *f, const char *path,
46+
extern const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
47+
extern void handle_ignore_submodules_arg(struct diff_options *, const char *);
48+
extern void show_submodule_summary(FILE *f, const char *path,
4949
const char *line_prefix,
5050
struct object_id *one, struct object_id *two,
5151
unsigned dirty_submodule, const char *meta,
5252
const char *del, const char *add, const char *reset);
53-
void show_submodule_inline_diff(FILE *f, const char *path,
53+
extern void show_submodule_inline_diff(FILE *f, const char *path,
5454
const char *line_prefix,
5555
struct object_id *one, struct object_id *two,
5656
unsigned dirty_submodule, const char *meta,
5757
const char *del, const char *add, const char *reset,
5858
const struct diff_options *opt);
59-
void set_config_fetch_recurse_submodules(int value);
60-
void check_for_new_submodule_commits(unsigned char new_sha1[20]);
61-
int fetch_populated_submodules(const struct argv_array *options,
59+
extern void set_config_fetch_recurse_submodules(int value);
60+
extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
61+
extern int fetch_populated_submodules(const struct argv_array *options,
6262
const char *prefix, int command_line_option,
6363
int quiet, int max_parallel_jobs);
64-
unsigned is_submodule_modified(const char *path, int ignore_untracked);
65-
int submodule_uses_gitfile(const char *path);
66-
int ok_to_remove_submodule(const char *path);
67-
int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],
68-
const unsigned char a[20], const unsigned char b[20], int search);
69-
int find_unpushed_submodules(struct sha1_array *commits, const char *remotes_name,
70-
struct string_list *needs_pushing);
64+
extern unsigned is_submodule_modified(const char *path, int ignore_untracked);
65+
extern int submodule_uses_gitfile(const char *path);
66+
67+
#define SUBMODULE_REMOVAL_DIE_ON_ERROR (1<<0)
68+
#define SUBMODULE_REMOVAL_IGNORE_UNTRACKED (1<<1)
69+
#define SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED (1<<2)
70+
extern int bad_to_remove_submodule(const char *path, unsigned flags);
71+
extern int merge_submodule(unsigned char result[20], const char *path,
72+
const unsigned char base[20],
73+
const unsigned char a[20],
74+
const unsigned char b[20], int search);
75+
extern int find_unpushed_submodules(struct sha1_array *commits,
76+
const char *remotes_name,
77+
struct string_list *needs_pushing);
7178
extern int push_unpushed_submodules(struct sha1_array *commits,
7279
const char *remotes_name,
7380
int dry_run);
74-
int parallel_submodules(void);
81+
extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
82+
extern int parallel_submodules(void);
7583

7684
/*
7785
* Prepare the "env_array" parameter of a "struct child_process" for executing
7886
* a submodule by clearing any repo-specific envirionment variables, but
7987
* retaining any config in the environment.
8088
*/
81-
void prepare_submodule_repo_env(struct argv_array *out);
89+
extern void prepare_submodule_repo_env(struct argv_array *out);
8290

8391
#define ABSORB_GITDIR_RECURSE_SUBMODULES (1<<0)
8492
extern void absorb_git_dir_into_superproject(const char *prefix,

‎t/t3600-rm.sh‎

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -569,26 +569,22 @@ test_expect_success 'rm of a conflicted unpopulated submodule succeeds' '
569569
test_cmp expect actual
570570
'
571571

572-
test_expect_success 'rm of a populated submodule with a .git directory fails even when forced' '
572+
test_expect_success 'rm of a populated submodule with a .git directory migrates git dir' '
573573
git checkout -f master &&
574574
git reset --hard &&
575575
git submodule update &&
576576
(cd submod &&
577577
rm .git &&
578578
cp -R ../.git/modules/sub .git &&
579-
GIT_WORK_TREE=. git config --unset core.worktree
579+
GIT_WORK_TREE=. git config --unset core.worktree &&
580+
rm -r ../.git/modules/sub
580581
) &&
581-
test_must_fail git rm submod &&
582-
test -d submod &&
583-
test -d submod/.git &&
584-
git status -s -uno --ignore-submodules=none >actual &&
585-
! test -s actual &&
586-
test_must_fail git rm -f submod &&
587-
test -d submod &&
588-
test -d submod/.git &&
582+
git rm submod 2>output.err &&
583+
! test -d submod &&
584+
! test -d submod/.git &&
589585
git status -s -uno --ignore-submodules=none >actual &&
590-
! test -s actual &&
591-
rm -rf submod
586+
test -s actual &&
587+
test_i18ngrep Migrating output.err
592588
'
593589

594590
cat >expect.deepmodified <<EOF
@@ -667,24 +663,19 @@ test_expect_success 'rm of a populated nested submodule with a nested .git direc
667663
git submodule update --recursive &&
668664
(cd submod/subsubmod &&
669665
rm .git &&
670-
cp -R ../../.git/modules/sub/modules/sub .git &&
666+
mv ../../.git/modules/sub/modules/sub .git &&
671667
GIT_WORK_TREE=. git config --unset core.worktree
672668
) &&
673-
test_must_fail git rm submod &&
674-
test -d submod &&
675-
test -d submod/subsubmod/.git &&
676-
git status -s -uno --ignore-submodules=none >actual &&
677-
! test -s actual &&
678-
test_must_fail git rm -f submod &&
679-
test -d submod &&
680-
test -d submod/subsubmod/.git &&
669+
git rm submod 2>output.err &&
670+
! test -d submod &&
671+
! test -d submod/subsubmod/.git &&
681672
git status -s -uno --ignore-submodules=none >actual &&
682-
! test -s actual &&
683-
rm -rf submod
673+
test -s actual &&
674+
test_i18ngrep Migrating output.err
684675
'
685676

686677
test_expect_success 'checking out a commit after submodule removal needs manual updates' '
687-
git commit -m "submodule removal" submod &&
678+
git commit -m "submodule removal" submod .gitmodules &&
688679
git checkout HEAD^ &&
689680
git submodule update &&
690681
git checkout -q HEAD^ &&

0 commit comments

Comments
 (0)