Skip to content

Commit 6d00385

Browse files
gitstergitster
committed
Merge branch 'gc/submodule-use-super-prefix'
Another step to rewrite more parts of "git submodule" in C. * gc/submodule-use-super-prefix: submodule--helper: remove display path helper submodule--helper update: use --super-prefix submodule--helper: remove unused SUPPORT_SUPER_PREFIX flags submodule--helper: use correct display path helper submodule--helper: don't recreate recursive prefix submodule--helper update: use display path helper submodule--helper tests: add missing "display path" coverage
2 parents e3349f2 + 5ad8727 commit 6d00385

File tree

2 files changed

+84
-64
lines changed

2 files changed

+84
-64
lines changed

‎builtin/submodule--helper.c

Lines changed: 22 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,11 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr
118118
return 0;
119119
}
120120

121-
static char *do_get_submodule_displaypath(const char *path,
122-
const char *prefix,
123-
const char *super_prefix)
121+
/* the result should be freed by the caller. */
122+
static char *get_submodule_displaypath(const char *path, const char *prefix)
124123
{
124+
const char *super_prefix = get_super_prefix();
125+
125126
if (prefix && super_prefix) {
126127
BUG("cannot have prefix '%s' and superprefix '%s'",
127128
prefix, super_prefix);
@@ -137,13 +138,6 @@ static char *do_get_submodule_displaypath(const char *path,
137138
}
138139
}
139140

140-
/* the result should be freed by the caller. */
141-
static char *get_submodule_displaypath(const char *path, const char *prefix)
142-
{
143-
const char *super_prefix = get_super_prefix();
144-
return do_get_submodule_displaypath(path, prefix, super_prefix);
145-
}
146-
147141
static char *compute_rev_name(const char *sub_path, const char* object_id)
148142
{
149143
struct strbuf sb = STRBUF_INIT;
@@ -477,22 +471,18 @@ static int starts_with_dot_dot_slash(const char *const path)
477471

478472
struct init_cb {
479473
const char *prefix;
480-
const char *superprefix;
481474
unsigned int flags;
482475
};
483476
#define INIT_CB_INIT { 0 }
484477

485478
static void init_submodule(const char *path, const char *prefix,
486-
const char *superprefix, unsigned int flags)
479+
unsigned int flags)
487480
{
488481
const struct submodule *sub;
489482
struct strbuf sb = STRBUF_INIT;
490483
char *upd = NULL, *url = NULL, *displaypath;
491484

492-
/* try superprefix from the environment, if it is not passed explicitly */
493-
if (!superprefix)
494-
superprefix = get_super_prefix();
495-
displaypath = do_get_submodule_displaypath(path, prefix, superprefix);
485+
displaypath = get_submodule_displaypath(path, prefix);
496486

497487
sub = submodule_from_path(the_repository, null_oid(), path);
498488

@@ -566,7 +556,7 @@ static void init_submodule(const char *path, const char *prefix,
566556
static void init_submodule_cb(const struct cache_entry *list_item, void *cb_data)
567557
{
568558
struct init_cb *info = cb_data;
569-
init_submodule(list_item->name, info->prefix, info->superprefix, info->flags);
559+
init_submodule(list_item->name, info->prefix, info->flags);
570560
}
571561

572562
static int module_init(int argc, const char **argv, const char *prefix)
@@ -1878,7 +1868,6 @@ struct submodule_update_clone {
18781868

18791869
struct update_data {
18801870
const char *prefix;
1881-
const char *recursive_prefix;
18821871
const char *displaypath;
18831872
enum submodule_update_type update_default;
18841873
struct object_id suboid;
@@ -1947,30 +1936,20 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
19471936
const char *update_string;
19481937
enum submodule_update_type update_type;
19491938
char *key;
1950-
struct strbuf displaypath_sb = STRBUF_INIT;
1939+
struct update_data *ud = suc->update_data;
1940+
char *displaypath = get_submodule_displaypath(ce->name, ud->prefix);
19511941
struct strbuf sb = STRBUF_INIT;
1952-
const char *displaypath = NULL;
19531942
int needs_cloning = 0;
19541943
int need_free_url = 0;
19551944

19561945
if (ce_stage(ce)) {
1957-
if (suc->update_data->recursive_prefix)
1958-
strbuf_addf(&sb, "%s/%s", suc->update_data->recursive_prefix, ce->name);
1959-
else
1960-
strbuf_addstr(&sb, ce->name);
1961-
strbuf_addf(out, _("Skipping unmerged submodule %s"), sb.buf);
1946+
strbuf_addf(out, _("Skipping unmerged submodule %s"), displaypath);
19621947
strbuf_addch(out, '\n');
19631948
goto cleanup;
19641949
}
19651950

19661951
sub = submodule_from_path(the_repository, null_oid(), ce->name);
19671952

1968-
if (suc->update_data->recursive_prefix)
1969-
displaypath = relative_path(suc->update_data->recursive_prefix,
1970-
ce->name, &displaypath_sb);
1971-
else
1972-
displaypath = ce->name;
1973-
19741953
if (!sub) {
19751954
next_submodule_warn_missing(suc, out, displaypath);
19761955
goto cleanup;
@@ -2060,7 +2039,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
20602039
"--no-single-branch");
20612040

20622041
cleanup:
2063-
strbuf_release(&displaypath_sb);
2042+
free(displaypath);
20642043
strbuf_release(&sb);
20652044
if (need_free_url)
20662045
free((void*)url);
@@ -2424,11 +2403,12 @@ static void update_data_to_args(struct update_data *update_data, struct strvec *
24242403
{
24252404
enum submodule_update_type update_type = update_data->update_default;
24262405

2406+
if (update_data->displaypath) {
2407+
strvec_push(args, "--super-prefix");
2408+
strvec_pushf(args, "%s/", update_data->displaypath);
2409+
}
24272410
strvec_pushl(args, "submodule--helper", "update", "--recursive", NULL);
24282411
strvec_pushf(args, "--jobs=%d", update_data->max_jobs);
2429-
if (update_data->recursive_prefix)
2430-
strvec_pushl(args, "--recursive-prefix",
2431-
update_data->recursive_prefix, NULL);
24322412
if (update_data->quiet)
24332413
strvec_push(args, "--quiet");
24342414
if (update_data->force)
@@ -2472,19 +2452,10 @@ static void update_data_to_args(struct update_data *update_data, struct strvec *
24722452

24732453
static int update_submodule(struct update_data *update_data)
24742454
{
2475-
char *prefixed_path;
2476-
24772455
ensure_core_worktree(update_data->sm_path);
24782456

2479-
if (update_data->recursive_prefix)
2480-
prefixed_path = xstrfmt("%s%s", update_data->recursive_prefix,
2481-
update_data->sm_path);
2482-
else
2483-
prefixed_path = xstrdup(update_data->sm_path);
2484-
2485-
update_data->displaypath = get_submodule_displaypath(prefixed_path,
2486-
update_data->prefix);
2487-
free(prefixed_path);
2457+
update_data->displaypath = get_submodule_displaypath(
2458+
update_data->sm_path, update_data->prefix);
24882459

24892460
determine_submodule_update_strategy(the_repository, update_data->just_cloned,
24902461
update_data->sm_path, update_data->update_default,
@@ -2524,14 +2495,6 @@ static int update_submodule(struct update_data *update_data)
25242495
struct update_data next = *update_data;
25252496
int res;
25262497

2527-
if (update_data->recursive_prefix)
2528-
prefixed_path = xstrfmt("%s%s/", update_data->recursive_prefix,
2529-
update_data->sm_path);
2530-
else
2531-
prefixed_path = xstrfmt("%s/", update_data->sm_path);
2532-
2533-
next.recursive_prefix = get_submodule_displaypath(prefixed_path,
2534-
update_data->prefix);
25352498
next.prefix = NULL;
25362499
oidcpy(&next.oid, null_oid());
25372500
oidcpy(&next.suboid, null_oid());
@@ -2616,10 +2579,6 @@ static int module_update(int argc, const char **argv, const char *prefix)
26162579
OPT_STRING(0, "prefix", &opt.prefix,
26172580
N_("path"),
26182581
N_("path into the working tree")),
2619-
OPT_STRING(0, "recursive-prefix", &opt.recursive_prefix,
2620-
N_("path"),
2621-
N_("path into the working tree, across nested "
2622-
"submodule boundaries")),
26232582
OPT_SET_INT(0, "checkout", &opt.update_default,
26242583
N_("use the 'checkout' update strategy (default)"),
26252584
SM_UPDATE_CHECKOUT),
@@ -2705,7 +2664,6 @@ static int module_update(int argc, const char **argv, const char *prefix)
27052664
module_list_active(&list);
27062665

27072666
info.prefix = opt.prefix;
2708-
info.superprefix = opt.recursive_prefix;
27092667
if (opt.quiet)
27102668
info.flags |= OPT_QUIET;
27112669

@@ -3402,16 +3360,16 @@ struct cmd_struct {
34023360
static struct cmd_struct commands[] = {
34033361
{"list", module_list, 0},
34043362
{"name", module_name, 0},
3405-
{"clone", module_clone, 0},
3406-
{"add", module_add, SUPPORT_SUPER_PREFIX},
3407-
{"update", module_update, 0},
3363+
{"clone", module_clone, SUPPORT_SUPER_PREFIX},
3364+
{"add", module_add, 0},
3365+
{"update", module_update, SUPPORT_SUPER_PREFIX},
34083366
{"resolve-relative-url-test", resolve_relative_url_test, 0},
34093367
{"foreach", module_foreach, SUPPORT_SUPER_PREFIX},
3410-
{"init", module_init, SUPPORT_SUPER_PREFIX},
3368+
{"init", module_init, 0},
34113369
{"status", module_status, SUPPORT_SUPER_PREFIX},
34123370
{"sync", module_sync, SUPPORT_SUPER_PREFIX},
34133371
{"deinit", module_deinit, 0},
3414-
{"summary", module_summary, SUPPORT_SUPER_PREFIX},
3372+
{"summary", module_summary, 0},
34153373
{"push-check", push_check, 0},
34163374
{"absorbgitdirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
34173375
{"is-active", is_active, 0},

‎t/t7406-submodule-update.sh

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1116,4 +1116,66 @@ test_expect_success 'submodule update --filter sets partial clone settings' '
11161116
test_cmp_config -C super-filter/submodule blob:none remote.origin.partialclonefilter
11171117
'
11181118

1119+
# NEEDSWORK: Clean up the tests so that we can reuse the test setup.
1120+
# Don't reuse the existing repos because the earlier tests have
1121+
# intentionally disruptive configurations.
1122+
test_expect_success 'setup clean recursive superproject' '
1123+
git init bottom &&
1124+
test_commit -C bottom "bottom" &&
1125+
git init middle &&
1126+
git -C middle submodule add ../bottom bottom &&
1127+
git -C middle commit -m "middle" &&
1128+
git init top &&
1129+
git -C top submodule add ../middle middle &&
1130+
git -C top commit -m "top" &&
1131+
git clone --recurse-submodules top top-clean
1132+
'
1133+
1134+
test_expect_success 'submodule update should skip unmerged submodules' '
1135+
test_when_finished "rm -fr top-cloned" &&
1136+
cp -r top-clean top-cloned &&
1137+
1138+
# Create an upstream commit in each repo, starting with bottom
1139+
test_commit -C bottom upstream_commit &&
1140+
# Create middle commit
1141+
git -C middle/bottom fetch &&
1142+
git -C middle/bottom checkout -f FETCH_HEAD &&
1143+
git -C middle add bottom &&
1144+
git -C middle commit -m "upstream_commit" &&
1145+
# Create top commit
1146+
git -C top/middle fetch &&
1147+
git -C top/middle checkout -f FETCH_HEAD &&
1148+
git -C top add middle &&
1149+
git -C top commit -m "upstream_commit" &&
1150+
1151+
# Create a downstream conflict
1152+
test_commit -C top-cloned/middle/bottom downstream_commit &&
1153+
git -C top-cloned/middle add bottom &&
1154+
git -C top-cloned/middle commit -m "downstream_commit" &&
1155+
git -C top-cloned/middle fetch --recurse-submodules origin &&
1156+
test_must_fail git -C top-cloned/middle merge origin/main &&
1157+
1158+
# Make the update of "middle" a no-op, otherwise we error out
1159+
# because of its unmerged state
1160+
test_config -C top-cloned submodule.middle.update !true &&
1161+
git -C top-cloned submodule update --recursive 2>actual.err &&
1162+
cat >expect.err <<-\EOF &&
1163+
Skipping unmerged submodule middle/bottom
1164+
EOF
1165+
test_cmp expect.err actual.err
1166+
'
1167+
1168+
test_expect_success 'submodule update --recursive skip submodules with strategy=none' '
1169+
test_when_finished "rm -fr top-cloned" &&
1170+
cp -r top-clean top-cloned &&
1171+
1172+
test_commit -C top-cloned/middle/bottom downstream_commit &&
1173+
git -C top-cloned/middle config submodule.bottom.update none &&
1174+
git -C top-cloned submodule update --recursive 2>actual.err &&
1175+
cat >expect.err <<-\EOF &&
1176+
Skipping submodule '\''middle/bottom'\''
1177+
EOF
1178+
test_cmp expect.err actual.err
1179+
'
1180+
11191181
test_done

0 commit comments

Comments
 (0)