Re: [PATCH] submodule: spell out API of submodule_move_head

Junio C Hamano wrote:

Stefan Beller [off-list ref] writes:

quoted

+/**
+ * Move the HEAD and content of the active submodule at 'path' from object id
+ * 'old' to 'new'.
+ *
+ * Updates the submodule at 'path' and files in its work tree to commit
+ * 'new'. The commit previously pointed to by the submodule is named by
+ * 'old'. This updates the submodule's HEAD, index, and work tree but
+ * does not touch its gitlink entry in the superproject.
+ *
+ * If the submodule did not previously exist, then 'old' should be NULL.
+ * Similarly, if the submodule is to be removed, 'new' should be NULL.
+ *
+ * If updating the submodule would cause local changes to be overwritten,
+ * then instead of updating the submodule, this function prints an error
+ * message and returns -1.

This is not a new issue (the removed comment did not mention this at
all), but is it correct to say that updates to "index and work tree"
was as if we did "git -C $path checkout new" (and of course, HEAD in
the $path submodule must be at 'old')?

I don't understand the question.  This comment doesn't say it's like
"git checkout" --- are you saying it should?

The function is more like "git read-tree -m -u" (or --reset when
SUBMODULE_MOVE_HEAD_FORCE is passed) than like "git checkout".
Perhaps what you are hinting at is that read-tree --recurse-submodules
is not necessarily the right primitive to implement "git checkout"
with.  But that's a separate issue from documenting the current
behavior of the function.

What should happen if 'old' does not match reality (i.e. old is NULL
but HEAD does point at some commit, old and HEAD are different,
etc.)?  Should the call be aborted?

No.

Thanks,
Jonathan

quoted

+ * If the submodule is not active, this does nothing and returns 0.
+ */
 #define SUBMODULE_MOVE_HEAD_DRY_RUN (1<<0)
 #define SUBMODULE_MOVE_HEAD_FORCE   (1<<1)
 extern int submodule_move_head(const char *path,