Re: [PATCH v2 9/9] builtin/history: implement "drop" subcommand

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> +static int update_worktree(struct repository *repo,
> +			   const struct commit *old_head,
> +			   const struct commit *new_head,
> +			   bool dry_run)
> +{
> +	struct reset_head_opts opts = {
> +		.oid_from = &old_head->object.oid,
> +		.oid = &new_head->object.oid,
> +		.flags = RESET_HEAD_SKIP_REF_UPDATES,
> +	};
> +	if (dry_run)
> +		opts.flags |= RESET_HEAD_DRY_RUN;
> +	return reset_head(repo, &opts);
> +}
> + ...
> +	/*
> +	 * If HEAD will move as a result of the rewrite then we'll have to
> +	 * merge in the changes into the worktree and index. This merge can of
> +	 * course conflict, which will cause the whole operation to abort.
> +	 *
> +	 * If we had already updated the refs at that point then we'd have an
> +	 * inconsistent repository state. So we first perform a dry-run merge
> +	 * here before updating refs.
> +	 */
> +	if (!dry_run && !is_bare_repository()) {
> +		ret = find_head_tree_change(repo, &result, &old_head,
> +					    &new_head, &head_moves);
> +		if (ret < 0)
> +			goto out;
> +
> +		if (head_moves && update_worktree(repo, old_head, new_head, true) < 0) {
> +			ret = error(_("dropping this commit would "
> +				      "overwrite local changes; aborting"));
> +			goto out;
> +		}
> +	}

This block is skipped under --dry-run, but update_worktree is
equipped to (and indeed run unconditionally here) run in the dry-run
mode.  Does it mean that "git history drop --dry-run" that user runs
to see which refs may be updated will not get warned about possible
worktree conflicts that would prevent the real run from happening?
Unless there is a compelling reason not to, I think --dry-run should
be a close simulation of what would happen without it.

Thanks.