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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 10, 2017, 00:16 UTC
Message-ID: <xmqqtvz7rh68.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171009220615.27541-1-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 15 quoted lines

> +/**
> + * 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')?

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?

Show 5 quoted lines

> + * 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,