kernel
/
git
mirror of https://git.kernel.org/pub/scm/git/git.git
1
0
Fork 0
Code Releases Activity

Merge branch 'en/submodule-merge-messages-fixes' 

Further update the help messages given while merging submodules.

* en/submodule-merge-messages-fixes:
  merge-ort: provide helpful submodule update message when possible
  merge-ort: avoid surprise with new sub_flag variable
  merge-ort: remove translator lego in new "submodule conflict suggestion"
  submodule merge: update conflict error message
maint
Junio C Hamano 2022-08-25 14:42:29 -07:00
parent 795ea8776b 565577ed88
commit df3c129e24
3 changed files with 195 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

125
merge-ort.c
View File

@ -387,8 +387,24 @@ struct merge_options_internal {

/* call_depth: recursion level counter for merging merge bases */
int call_depth;

/* field that holds submodule conflict information */
struct string_list conflicted_submodules;
};

struct conflicted_submodule_item {
char *abbrev;
int flag;
};

static void conflicted_submodule_item_free(void *util, const char *str)
{
struct conflicted_submodule_item *item = util;

free(item->abbrev);
free(item);
}

struct version_info {
struct object_id oid;
unsigned short mode;
@ -517,6 +533,7 @@ enum conflict_and_info_types {
CONFLICT_SUBMODULE_NOT_INITIALIZED,
CONFLICT_SUBMODULE_HISTORY_NOT_AVAILABLE,
CONFLICT_SUBMODULE_MAY_HAVE_REWINDS,
CONFLICT_SUBMODULE_NULL_MERGE_BASE,

/* Keep this entry _last_ in the list */
NB_CONFLICT_TYPES,
@ -570,6 +587,8 @@ static const char *type_short_descriptions[] = {
"CONFLICT (submodule history not available)",
[CONFLICT_SUBMODULE_MAY_HAVE_REWINDS] =
"CONFLICT (submodule may have rewinds)",
[CONFLICT_SUBMODULE_NULL_MERGE_BASE] =
"CONFLICT (submodule lacks merge base)"
};

struct logical_conflict_info {
@ -686,6 +705,9 @@ static void clear_or_reinit_internal_opts(struct merge_options_internal *opti,

mem_pool_discard(&opti->pool, 0);

string_list_clear_func(&opti->conflicted_submodules,
conflicted_submodule_item_free);

/* Clean out callback_data as well. */
FREE_AND_NULL(renames->callback_data);
renames->callback_data_nr = renames->callback_data_alloc = 0;
@ -1744,24 +1766,32 @@ static int merge_submodule(struct merge_options *opt,

int i;
int search = !opt->priv->call_depth;
int sub_not_initialized = 1;
int sub_flag = CONFLICT_SUBMODULE_FAILED_TO_MERGE;

/* store fallback answer in result in case we fail */
oidcpy(result, opt->priv->call_depth ? o : a);

/* we can not handle deletion conflicts */
if (is_null_oid(o))
return 0;
if (is_null_oid(a))
return 0;
if (is_null_oid(b))
return 0;
if (is_null_oid(a) || is_null_oid(b))
BUG("submodule deleted on one side; this should be handled outside of merge_submodule()");

if (repo_submodule_init(&subrepo, opt->repo, path, null_oid())) {
if ((sub_not_initialized = repo_submodule_init(&subrepo,
opt->repo, path, null_oid()))) {
path_msg(opt, CONFLICT_SUBMODULE_NOT_INITIALIZED, 0,
path, NULL, NULL, NULL,
_("Failed to merge submodule %s (not checked out)"),
path);
return 0;
sub_flag = CONFLICT_SUBMODULE_NOT_INITIALIZED;
goto cleanup;
}

if (is_null_oid(o)) {
path_msg(opt, CONFLICT_SUBMODULE_NULL_MERGE_BASE, 0,
path, NULL, NULL, NULL,
_("Failed to merge submodule %s (no merge base)"),
path);
goto cleanup;
}

if (!(commit_o = lookup_commit_reference(&subrepo, o)) ||
@ -1771,6 +1801,7 @@ static int merge_submodule(struct merge_options *opt,
path, NULL, NULL, NULL,
_("Failed to merge submodule %s (commits not present)"),
path);
sub_flag = CONFLICT_SUBMODULE_HISTORY_NOT_AVAILABLE;
goto cleanup;
}

@ -1849,7 +1880,23 @@ static int merge_submodule(struct merge_options *opt,

object_array_clear(&merges);
cleanup:
repo_clear(&subrepo);
if (!opt->priv->call_depth && !ret) {
struct string_list *csub = &opt->priv->conflicted_submodules;
struct conflicted_submodule_item *util;
const char *abbrev;

util = xmalloc(sizeof(*util));
util->flag = sub_flag;
util->abbrev = NULL;
if (!sub_not_initialized) {
abbrev = repo_find_unique_abbrev(&subrepo, b, DEFAULT_ABBREV);
util->abbrev = xstrdup(abbrev);
}
string_list_append(csub, path)->util = util;
}

if (!sub_not_initialized)
repo_clear(&subrepo);
return ret;
}

@ -4434,6 +4481,63 @@ static int record_conflicted_index_entries(struct merge_options *opt)
return errs;
}

static void print_submodule_conflict_suggestion(struct string_list *csub) {
struct string_list_item *item;
struct strbuf msg = STRBUF_INIT;
struct strbuf tmp = STRBUF_INIT;
struct strbuf subs = STRBUF_INIT;

if (!csub->nr)
return;

strbuf_add_separated_string_list(&subs, " ", csub);
for_each_string_list_item(item, csub) {
struct conflicted_submodule_item *util = item->util;

/*
* NEEDSWORK: The steps to resolve these errors deserve a more
* detailed explanation than what is currently printed below.
*/
if (util->flag == CONFLICT_SUBMODULE_NOT_INITIALIZED ||
util->flag == CONFLICT_SUBMODULE_HISTORY_NOT_AVAILABLE)
continue;

/*
* TRANSLATORS: This is a line of advice to resolve a merge
* conflict in a submodule. The first argument is the submodule
* name, and the second argument is the abbreviated id of the
* commit that needs to be merged. For example:
* - go to submodule (mysubmodule), and either merge commit abc1234"
*/
strbuf_addf(&tmp, _(" - go to submodule (%s), and either merge commit %s\n"
" or update to an existing commit which has merged those changes\n"),
item->string, util->abbrev);
}

/*
* TRANSLATORS: This is a detailed message for resolving submodule
* conflicts. The first argument is string containing one step per
* submodule. The second is a space-separated list of submodule names.
*/
strbuf_addf(&msg,
_("Recursive merging with submodules currently only supports trivial cases.\n"
"Please manually handle the merging of each conflicted submodule.\n"
"This can be accomplished with the following steps:\n"
"%s"
" - come back to superproject and run:\n\n"
" git add %s\n\n"
" to record the above merge or update\n"
" - resolve any other conflicts in the superproject\n"
" - commit the resulting index in the superproject\n"),
tmp.buf, subs.buf);

printf("%s", msg.buf);

strbuf_release(&subs);
strbuf_release(&tmp);
strbuf_release(&msg);
}

void merge_display_update_messages(struct merge_options *opt,
int detailed,
struct merge_result *result)
@ -4483,6 +4587,8 @@ void merge_display_update_messages(struct merge_options *opt,
}
string_list_clear(&olist, 0);

print_submodule_conflict_suggestion(&opti->conflicted_submodules);

/* Also include needed rename limit adjustment now */
diff_warn_rename_limit("merge.renamelimit",
opti->renames.needed_limit, 0);
@ -4684,6 +4790,7 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
trace2_region_enter("merge", "allocate/init", opt->repo);
if (opt->priv) {
clear_or_reinit_internal_opts(opt->priv, 1);
string_list_init_nodup(&opt->priv->conflicted_submodules);
trace2_region_leave("merge", "allocate/init", opt->repo);
return;
}

78
t/t6437-submodule-merge.sh
View File

@ -103,8 +103,25 @@ test_expect_success 'setup for merge search' '
echo "file-c" > file-c &&
git add file-c &&
git commit -m "sub-c") &&
git commit -a -m "c" &&
git commit -a -m "c")
'

test_expect_success 'merging should conflict for non fast-forward' '
test_when_finished "git -C merge-search reset --hard" &&
(cd merge-search &&
git checkout -b test-nonforward-a b &&
if test "$GIT_TEST_MERGE_ALGORITHM" = ort
then
test_must_fail git merge c >actual &&
sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" &&
grep "$sub_expect" actual
else
test_must_fail git merge c 2> actual
fi)
'

test_expect_success 'finish setup for merge-search' '
(cd merge-search &&
git checkout -b d a &&
(cd sub &&
git checkout -b sub-d sub-b &&
@ -129,14 +146,16 @@ test_expect_success 'merge with one side as a fast-forward of the other' '
test_cmp expect actual)
'

test_expect_success 'merging should conflict for non fast-forward' '
test_expect_success 'merging should conflict for non fast-forward (resolution exists)' '
(cd merge-search &&
git checkout -b test-nonforward b &&
git checkout -b test-nonforward-b b &&
(cd sub &&
git rev-parse --short sub-d > ../expect) &&
if test "$GIT_TEST_MERGE_ALGORITHM" = ort
then
test_must_fail git merge c >actual
test_must_fail git merge c >actual &&
sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" &&
grep "$sub_expect" actual
else
test_must_fail git merge c 2> actual
fi &&
@ -161,7 +180,9 @@ test_expect_success 'merging should fail for ambiguous common parent' '
) &&
if test "$GIT_TEST_MERGE_ALGORITHM" = ort
then
test_must_fail git merge c >actual
test_must_fail git merge c >actual &&
sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" &&
grep "$sub_expect" actual
else
test_must_fail git merge c 2> actual
fi &&
@ -205,7 +226,12 @@ test_expect_success 'merging should fail for changes that are backwards' '
git commit -a -m "f" &&

git checkout -b test-backward e &&
test_must_fail git merge f)
test_must_fail git merge f >actual &&
if test "$GIT_TEST_MERGE_ALGORITHM" = ort
then
sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-d)" &&
grep "$sub_expect" actual
fi)
'


@ -476,4 +502,44 @@ test_expect_failure 'directory/submodule conflict; merge --abort works afterward
)
'

# Setup:
# - Submodule has 2 commits: a and b
# - Superproject branch 'a' adds and commits submodule pointing to 'commit a'
# - Superproject branch 'b' adds and commits submodule pointing to 'commit b'
# If these two branches are now merged, there is no merge base
test_expect_success 'setup for null merge base' '
mkdir no-merge-base &&
(cd no-merge-base &&
git init &&
mkdir sub &&
(cd sub &&
git init &&
echo "file-a" > file-a &&
git add file-a &&
git commit -m "commit a") &&
git commit --allow-empty -m init &&
git branch init &&
git checkout -b a init &&
git add sub &&
git commit -m "a" &&
git switch main &&
(cd sub &&
echo "file-b" > file-b &&
git add file-b &&
git commit -m "commit b"))
'

test_expect_success 'merging should fail with no merge base' '
(cd no-merge-base &&
git checkout -b b init &&
git add sub &&
git commit -m "b" &&
test_must_fail git merge a >actual &&
if test "$GIT_TEST_MERGE_ALGORITHM" = ort
then
sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short HEAD^1)" &&
grep "$sub_expect" actual
fi)
'

test_done

9
t/t7402-submodule-rebase.sh
View File

@ -104,7 +104,7 @@ test_expect_success 'rebasing submodule that should conflict' '
test_tick &&
git commit -m fourth &&

test_must_fail git rebase --onto HEAD^^ HEAD^ HEAD^0 &&
test_must_fail git rebase --onto HEAD^^ HEAD^ HEAD^0 >actual_output &&
git ls-files -s submodule >actual &&
(
cd submodule &&
@ -112,7 +112,12 @@ test_expect_success 'rebasing submodule that should conflict' '
echo "160000 $(git rev-parse HEAD^^) 2 submodule" &&
echo "160000 $(git rev-parse HEAD) 3 submodule"
) >expect &&
test_cmp expect actual
test_cmp expect actual &&
if test "$GIT_TEST_MERGE_ALGORITHM" = ort
then
sub_expect="go to submodule (submodule), and either merge commit $(git -C submodule rev-parse --short HEAD^0)" &&
grep "$sub_expect" actual_output
fi
'

test_done