Thread (1 message) 1 message, 1 author, 2022-07-19

Re: [PATCH v5] submodule merge: update conflict error message

From: Junio C Hamano <hidden>
Date: 2022-07-19 20:30:32

Calvin Wan [off-list ref] writes:
quoted
These work on the result of calling find_first_merges(), but is it
possible that we are asked to call this function more than once
because we see conflicted submodule updates at two or more paths?
This does get called multiple times if we see conflicted submodule
updates at two or more paths.
quoted
I may be misreading the code, but find_first_merges(), either the
version we see in this file, or the one in merge-recursive.c, or its
original introduced in 68d03e4a (Implement automatic fast-forward
merge for submodules, 2010-07-07), look safe to be called twice.  It
runs the get_revision() machinery, smudging the object flags while
walking the history, but I do not see any code that cleans up these
flags for the second traversal.
I don't quite understand which flags need to be cleaned up for the
second traversal.
UNINTERESTING, TREESAME, ADDED, SEEN, SHOWN are among the flags used
by the object walk (if MyFirstObjectWalk does not talk about them,
it probably should), and they need to be cleared before you prepare
a new "struct rev_info" and throw it at setup_revisions(),
prepare_revision_walk(), and start calling get_revision().

submodule.c::collect_changed_submodules() has its own revision walk,
but it calls reset_revision_walk() to clear these flags from all
objects in the superproject (i.e. the_repository).

I _think_ the reason why this never turned out to be a problem in
practice is because we do not run this helper twice for the same
submodule.  Even though we may smudge many objects from a submodule
with an object walk without clearing their flags, as long as we
run the next object walk in a different submodule whose object flags
are still unsmudged, it would be OK.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help