On Fri, Nov 09, 2018 at 01:34:19AM -0800, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@xxxxxx>
>
> When we converted a `git checkout -q $onto^0` call to use
> `reset_head()`, we inadvertently incurred a change from a twoway_merge
> to a oneway_merge, as if we wanted a `git reset --hard` instead.
>
> This has performance ramifications under certain, though, as the
> oneway_merge needs to lstat() every single index entry whereas
> twoway_merge does not.
>
> So let's go back to the old behavior.
Makes sense. I didn't think too hard about any possible gotchas with the
twoway/oneway switch, but if that's what git-checkout was doing before,
it seems obviously safe.
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 6f6d7de156..c1cc50f3f8 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -523,11 +523,12 @@ finished_rebase:
> #define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
>
> static int reset_head(struct object_id *oid, const char *action,
> - const char *switch_to_branch, int detach_head,
> + const char *switch_to_branch,
> + int detach_head, int reset_hard,
It might be worth switching to a single flag variable here. It would
make calls like this:
> - if (reset_head(&options.onto->object.oid, "checkout", NULL, 1,
> + if (reset_head(&options.onto->object.oid, "checkout", NULL, 1, 0,
> NULL, msg.buf))
a little more self-documenting (if a little more verbose).
> - if (!oid) {
> - if (get_oid("HEAD", &head_oid)) {
> - rollback_lock_file(&lock);
> - return error(_("could not determine HEAD revision"));
> - }
> - oid = &head_oid;
> + if (get_oid("HEAD", &head_oid)) {
> + rollback_lock_file(&lock);
> + return error(_("could not determine HEAD revision"));
> }
This one could actually turn into:
ret = error(...);
goto leave_reset_head;
now. We don't have to worry about an uninitialized desc.buffer anymore
(as I mentioned in the previous email), because "nr" would be 0.
It doesn't save any lines, though (but maybe having a single
cleanup/exit point would make things easier to read; I dunno).
Take all my comments as observations, not objections. This looks OK to
me either way.
-Peff
