revision: remove "submodule" from opt struct
Clean up a TODO in revision.h by removing the "submodule" field from struct setup_revision_opt. This field is only used to specify the ref store to use, so use rev_info->repo to determine the ref store instead. The only users of this field are merge-ort.c and merge-recursive.c. However, both these files specify the superproject as rev_info->repo and the submodule as setup_revision_opt->submodule. In order to be able to pass the submodule as rev_info->repo, all commits must be parsed with the submodule explicitly specified; this patch does that as well. (An incremental solution in which only some commits are parsed with explicit submodule will not work, because if the same commit is parsed twice in different repositories, there will be 2 heap-allocated object structs corresponding to that commit, and any flag set by the revision walking mechanism on one of them will not be reflected onto the other.) Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonathan Tan committed
Sep 9, 2021 at 11:47 UTC
10a0d6ae64ad4fec12ab2c1ed13d1791dde60371
4 files changed
+72
-47
merge-ort.c
+35
-18
@@ -32,6 +32,7 @@
32
#include "promisor-remote.h"
33
#include "revision.h"
34
#include "strmap.h"
35
+#include "submodule-config.h"
36
#include "submodule.h"
37
#include "tree.h"
38
#include "unpack-trees.h"
@@ -1499,7 +1500,6 @@ static int find_first_merges(struct repository *repo,
1500
xsnprintf(merged_revision, sizeof(merged_revision), "^%s",
1501
oid_to_hex(&a->object.oid));
1502
repo_init_revisions(repo, &revs, NULL);
1502
- rev_opts.submodule = path;
1503
/* FIXME: can't handle linked worktrees in submodules yet */
1504
revs.single_worktree = path != NULL;
1505
setup_revisions(ARRAY_SIZE(rev_args)-1, rev_args, &revs, &rev_opts);
@@ -1509,7 +1509,7 @@ static int find_first_merges(struct repository *repo,
1509
die("revision walk setup failed");
1510
while ((commit = get_revision(&revs)) != NULL) {
1511
struct object *o = &(commit->object);
1512
- if (in_merge_bases(b, commit))
1512
+ if (repo_in_merge_bases(repo, b, commit))
1513
add_object_array(o, NULL, &merges);
1514
}
1515
reset_revision_walk();
@@ -1524,7 +1524,7 @@ static int find_first_merges(struct repository *repo,
1524
contains_another = 0;
1525
for (j = 0; j < merges.nr; j++) {
1526
struct commit *m2 = (struct commit *) merges.objects[j].item;
1527
- if (i != j && in_merge_bases(m2, m1)) {
1527
+ if (i != j && repo_in_merge_bases(repo, m2, m1)) {
1528
contains_another = 1;
1529
break;
1530
}
@@ -1545,10 +1545,12 @@ static int merge_submodule(struct merge_options *opt,
1545
const struct object_id *b,
1546
struct object_id *result)
1547
{
1548
+ struct repository subrepo;
1549
+ struct strbuf sb = STRBUF_INIT;
1550
+ int ret = 0;
1551
struct commit *commit_o, *commit_a, *commit_b;
1552
int parent_count;
1553
struct object_array merges;
1551
- struct strbuf sb = STRBUF_INIT;
1554
1555
int i;
1556
int search = !opt->priv->call_depth;
@@ -1564,6 +1566,10 @@ static int merge_submodule(struct merge_options *opt,
1566
if (is_null_oid(b))
1567
return 0;
1568
1569
+ /*
1570
+ * NEEDSWORK: Remove this when all submodule object accesses are
1571
+ * through explicitly specified repositores.
1572
+ */
1573
if (add_submodule_odb(path)) {
1574
path_msg(opt, path, 0,
1575
_("Failed to merge submodule %s (not checked out)"),
@@ -1571,39 +1577,48 @@ static int merge_submodule(struct merge_options *opt,
1577
return 0;
1578
}
1579
1574
- if (!(commit_o = lookup_commit_reference(opt->repo, o)) ||
1575
- !(commit_a = lookup_commit_reference(opt->repo, a)) ||
1576
- !(commit_b = lookup_commit_reference(opt->repo, b))) {
1580
+ if (repo_submodule_init(&subrepo, opt->repo, path, null_oid())) {
1581
+ path_msg(opt, path, 0,
1582
+ _("Failed to merge submodule %s (not checked out)"),
1583
+ path);
1584
+ return 0;
1585
+ }
1586
+
1587
+ if (!(commit_o = lookup_commit_reference(&subrepo, o)) ||
1588
+ !(commit_a = lookup_commit_reference(&subrepo, a)) ||
1589
+ !(commit_b = lookup_commit_reference(&subrepo, b))) {
1590
path_msg(opt, path, 0,
1591
_("Failed to merge submodule %s (commits not present)"),
1592
path);
1580
- return 0;
1593
+ goto cleanup;
1594
}
1595
1596
/* check whether both changes are forward */
1584
- if (!in_merge_bases(commit_o, commit_a) ||
1585
- !in_merge_bases(commit_o, commit_b)) {
1597
+ if (!repo_in_merge_bases(&subrepo, commit_o, commit_a) ||
1598
+ !repo_in_merge_bases(&subrepo, commit_o, commit_b)) {
1599
path_msg(opt, path, 0,
1600
_("Failed to merge submodule %s "
1601
"(commits don't follow merge-base)"),
1602
path);
1590
- return 0;
1603
+ goto cleanup;
1604
}
1605
1606
/* Case #1: a is contained in b or vice versa */
1594
- if (in_merge_bases(commit_a, commit_b)) {
1607
+ if (repo_in_merge_bases(&subrepo, commit_a, commit_b)) {
1608
oidcpy(result, b);
1609
path_msg(opt, path, 1,
1610
_("Note: Fast-forwarding submodule %s to %s"),
1611
path, oid_to_hex(b));
1599
- return 1;
1612
+ ret = 1;
1613
+ goto cleanup;
1614
}
1601
- if (in_merge_bases(commit_b, commit_a)) {
1615
+ if (repo_in_merge_bases(&subrepo, commit_b, commit_a)) {
1616
oidcpy(result, a);
1617
path_msg(opt, path, 1,
1618
_("Note: Fast-forwarding submodule %s to %s"),
1619
path, oid_to_hex(a));
1606
- return 1;
1620
+ ret = 1;
1621
+ goto cleanup;
1622
}
1623
1624
/*
@@ -1615,10 +1630,10 @@ static int merge_submodule(struct merge_options *opt,
1630
1631
/* Skip the search if makes no sense to the calling context. */
1632
if (!search)
1618
- return 0;
1633
+ goto cleanup;
1634
1635
/* find commit which merges them */
1621
- parent_count = find_first_merges(opt->repo, path, commit_a, commit_b,
1636
+ parent_count = find_first_merges(&subrepo, path, commit_a, commit_b,
1637
&merges);
1638
switch (parent_count) {
1639
case 0:
@@ -1652,7 +1667,9 @@ static int merge_submodule(struct merge_options *opt,
1667
}
1668
1669
object_array_clear(&merges);
1655
- return 0;
1670
+cleanup:
1671
+ repo_clear(&subrepo);
1672
+ return ret;
1673
}
1674
1675
static void initialize_attr_index(struct merge_options *opt)
merge-recursive.c
+32
-17
@@ -24,6 +24,7 @@
24
#include "repository.h"
25
#include "revision.h"
26
#include "string-list.h"
27
+#include "submodule-config.h"
28
#include "submodule.h"
29
#include "tag.h"
30
#include "tree-walk.h"
@@ -1113,7 +1114,6 @@ static int find_first_merges(struct repository *repo,
1114
xsnprintf(merged_revision, sizeof(merged_revision), "^%s",
1115
oid_to_hex(&a->object.oid));
1116
repo_init_revisions(repo, &revs, NULL);
1116
- rev_opts.submodule = path;
1117
/* FIXME: can't handle linked worktrees in submodules yet */
1118
revs.single_worktree = path != NULL;
1119
setup_revisions(ARRAY_SIZE(rev_args)-1, rev_args, &revs, &rev_opts);
@@ -1123,7 +1123,7 @@ static int find_first_merges(struct repository *repo,
1123
die("revision walk setup failed");
1124
while ((commit = get_revision(&revs)) != NULL) {
1125
struct object *o = &(commit->object);
1126
- if (in_merge_bases(b, commit))
1126
+ if (repo_in_merge_bases(repo, b, commit))
1127
add_object_array(o, NULL, &merges);
1128
}
1129
reset_revision_walk();
@@ -1138,7 +1138,7 @@ static int find_first_merges(struct repository *repo,
1138
contains_another = 0;
1139
for (j = 0; j < merges.nr; j++) {
1140
struct commit *m2 = (struct commit *) merges.objects[j].item;
1141
- if (i != j && in_merge_bases(m2, m1)) {
1141
+ if (i != j && repo_in_merge_bases(repo, m2, m1)) {
1142
contains_another = 1;
1143
break;
1144
}
@@ -1174,6 +1174,8 @@ static int merge_submodule(struct merge_options *opt,
1174
const struct object_id *base, const struct object_id *a,
1175
const struct object_id *b)
1176
{
1177
+ struct repository subrepo;
1178
+ int ret = 0;
1179
struct commit *commit_base, *commit_a, *commit_b;
1180
int parent_count;
1181
struct object_array merges;
@@ -1197,27 +1199,36 @@ static int merge_submodule(struct merge_options *opt,
1199
if (is_null_oid(b))
1200
return 0;
1201
1202
+ /*
1203
+ * NEEDSWORK: Remove this when all submodule object accesses are
1204
+ * through explicitly specified repositores.
1205
+ */
1206
if (add_submodule_odb(path)) {
1207
output(opt, 1, _("Failed to merge submodule %s (not checked out)"), path);
1208
return 0;
1209
}
1210
1205
- if (!(commit_base = lookup_commit_reference(opt->repo, base)) ||
1206
- !(commit_a = lookup_commit_reference(opt->repo, a)) ||
1207
- !(commit_b = lookup_commit_reference(opt->repo, b))) {
1208
- output(opt, 1, _("Failed to merge submodule %s (commits not present)"), path);
1211
+ if (repo_submodule_init(&subrepo, opt->repo, path, null_oid())) {
1212
+ output(opt, 1, _("Failed to merge submodule %s (not checked out)"), path);
1213
return 0;
1214
}
1215
1216
+ if (!(commit_base = lookup_commit_reference(&subrepo, base)) ||
1217
+ !(commit_a = lookup_commit_reference(&subrepo, a)) ||
1218
+ !(commit_b = lookup_commit_reference(&subrepo, b))) {
1219
+ output(opt, 1, _("Failed to merge submodule %s (commits not present)"), path);
1220
+ goto cleanup;
1221
+ }
1222
+
1223
/* check whether both changes are forward */
1213
- if (!in_merge_bases(commit_base, commit_a) ||
1214
- !in_merge_bases(commit_base, commit_b)) {
1224
+ if (!repo_in_merge_bases(&subrepo, commit_base, commit_a) ||
1225
+ !repo_in_merge_bases(&subrepo, commit_base, commit_b)) {
1226
output(opt, 1, _("Failed to merge submodule %s (commits don't follow merge-base)"), path);
1216
- return 0;
1227
+ goto cleanup;
1228
}
1229
1230
/* Case #1: a is contained in b or vice versa */
1220
- if (in_merge_bases(commit_a, commit_b)) {
1231
+ if (repo_in_merge_bases(&subrepo, commit_a, commit_b)) {
1232
oidcpy(result, b);
1233
if (show(opt, 3)) {
1234
output(opt, 3, _("Fast-forwarding submodule %s to the following commit:"), path);
@@ -1227,9 +1238,10 @@ static int merge_submodule(struct merge_options *opt,
1238
else
1239
; /* no output */
1240
1230
- return 1;
1241
+ ret = 1;
1242
+ goto cleanup;
1243
}
1232
- if (in_merge_bases(commit_b, commit_a)) {
1244
+ if (repo_in_merge_bases(&subrepo, commit_b, commit_a)) {
1245
oidcpy(result, a);
1246
if (show(opt, 3)) {
1247
output(opt, 3, _("Fast-forwarding submodule %s to the following commit:"), path);
@@ -1239,7 +1251,8 @@ static int merge_submodule(struct merge_options *opt,
1251
else
1252
; /* no output */
1253
1242
- return 1;
1254
+ ret = 1;
1255
+ goto cleanup;
1256
}
1257
1258
/*
@@ -1251,10 +1264,10 @@ static int merge_submodule(struct merge_options *opt,
1264
1265
/* Skip the search if makes no sense to the calling context. */
1266
if (!search)
1254
- return 0;
1267
+ goto cleanup;
1268
1269
/* find commit which merges them */
1257
- parent_count = find_first_merges(opt->repo, &merges, path,
1270
+ parent_count = find_first_merges(&subrepo, &merges, path,
1271
commit_a, commit_b);
1272
switch (parent_count) {
1273
case 0:
@@ -1281,7 +1294,9 @@ static int merge_submodule(struct merge_options *opt,
1294
}
1295
1296
object_array_clear(&merges);
1284
- return 0;
1297
+cleanup:
1298
+ repo_clear(&subrepo);
1299
+ return ret;
1300
}
1301
1302
static int merge_mode_and_contents(struct merge_options *opt,
revision.c
+5
-11
@@ -2561,8 +2561,7 @@ static int for_each_good_bisect_ref(struct ref_store *refs, each_ref_fn fn, void
2561
return for_each_bisect_ref(refs, fn, cb_data, term_good);
2562
}
2563
2564
-static int handle_revision_pseudo_opt(const char *submodule,
2565
- struct rev_info *revs,
2564
+static int handle_revision_pseudo_opt(struct rev_info *revs,
2565
const char **argv, int *flags)
2566
{
2567
const char *arg = argv[0];
@@ -2570,7 +2569,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
2569
struct ref_store *refs;
2570
int argcount;
2571
2573
- if (submodule) {
2572
+ if (revs->repo != the_repository) {
2573
/*
2574
* We need some something like get_submodule_worktrees()
2575
* before we can go through all worktrees of a submodule,
@@ -2579,9 +2578,8 @@ static int handle_revision_pseudo_opt(const char *submodule,
2578
*/
2579
if (!revs->single_worktree)
2580
BUG("--single-worktree cannot be used together with submodule");
2582
- refs = get_submodule_ref_store(submodule);
2583
- } else
2584
- refs = get_main_ref_store(revs->repo);
2581
+ }
2582
+ refs = get_main_ref_store(revs->repo);
2583
2584
/*
2585
* NOTE!
@@ -2699,12 +2697,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
2697
{
2698
int i, flags, left, seen_dashdash, revarg_opt;
2699
struct strvec prune_data = STRVEC_INIT;
2702
- const char *submodule = NULL;
2700
int seen_end_of_options = 0;
2701
2705
- if (opt)
2706
- submodule = opt->submodule;
2707
-
2702
/* First, search for "--" */
2703
if (opt && opt->assume_dashdash) {
2704
seen_dashdash = 1;
@@ -2733,7 +2727,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
2727
if (!seen_end_of_options && *arg == '-') {
2728
int opts;
2729
2736
- opts = handle_revision_pseudo_opt(submodule,
2730
+ opts = handle_revision_pseudo_opt(
2731
revs, argv + i,
2732
&flags);
2733
if (opts > 0) {
revision.h
-1
@@ -339,7 +339,6 @@ extern volatile show_early_output_fn_t show_early_output;
339
struct setup_revision_opt {
340
const char *def;
341
void (*tweak)(struct rev_info *, struct setup_revision_opt *);
342
- const char *submodule; /* TODO: drop this and use rev_info->repo */
342
unsigned int assume_dashdash:1,
343
allow_exclude_promisor_objects:1;
344
unsigned revarg_opt;