git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Duy Nguyen <pclouds@gmail.com>
Cc: Anthony Sottile <asottile@umich.edu>,
	Ben Peart <peartben@gmail.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: Regression `git checkout $rev -b branch` while in a `--no-checkout` clone does not check out files
Date: Thu, 03 Jan 2019 12:25:57 -0800	[thread overview]
Message-ID: <xmqqef9th4iy.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <CACsJy8C=O=ZDvD0ReSJOyAsNDEb5Yz-iFvs7oV5zAXaFf-dw5g@mail.gmail.com> (Duy Nguyen's message of "Thu, 3 Jan 2019 17:04:47 +0700")

Duy Nguyen <pclouds@gmail.com> writes:

> I plan to revert this commit anyway when the new command "git
> switch-branch" comes. The optimization will be unconditionally in the
> new command without this hack and users are encouraged to use that one
> instead of "git checkout".

I tend to think that the behaviour is perfectly in line with what
Ben wanted to have, which is to make "checkout -b new [HEAD]" not to
touch anything in the index or the working tree at all.

It further is possible to argue that what is strange in the whole
episode is what "clone --no-checkout" does.  In such a repository,
if you say "git status", you'd notice that it is reported that all
paths have been deleted.

Now, if you instead do

	git clone $src dst
	cd dst
	git rm file
	git checkout -b new

i.e. starting from a clone with normal working tree, manually
removing a path or two, and then create a new branch starting from
that state while carrying all the local changes, you *do* want to
see that 'file' to stay missing.  After all, "do not lose the local
changes; carry them forward" is what switching branches is about.

And from that point of view, we could consider that

	git clone --no-checkout $src $dst

is equivalent to

	git clone $src $dst && git -C $dst rm -r .

Having said all that.

> Meanwhile, let's see if Ben wants to fix this or revert it.

A "fix" to Ben's optimization for this particular case should be
fairly straight-forward.  I think we have a special case in the
checkout codepath for an initial checkout and disable "carry forward
the fact that the user wanted all the paths removed", so it would be
the matter of adding yet another condition (is_cache_unborn(), which
is used to set topts.initial_checkout) to the large collection of
conditions in skip_merge_working_tree().

Back when the "optimization" was discussed, all reviewers said that
it would become maintenance nightmare to ensure that the set of
conditions accurately tracks the case where the optimization is
safe.  Now they are entitled to say "we told you so".

  reply	other threads:[~2019-01-03 20:26 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-01 23:17 Regression `git checkout $rev -b branch` while in a `--no-checkout` clone does not check out files Anthony Sottile
2019-01-02 11:08 ` Duy Nguyen
2019-01-02 16:18   ` Anthony Sottile
2019-01-03 10:04     ` Duy Nguyen
2019-01-03 20:25       ` Junio C Hamano [this message]
2019-01-03 20:35         ` Anthony Sottile
2019-01-03 21:51           ` Junio C Hamano
2019-01-03 22:05             ` Anthony Sottile
2019-01-16 14:39               ` Ben Peart
2019-01-18 18:55 ` [PATCH v1 0/2] Fix regression in checkout -b Ben Peart
2019-01-18 18:55   ` [PATCH v1 1/2] checkout: add test to demonstrate regression with checkout -b on initial commit Ben Peart
2019-01-18 19:23     ` SZEDER Gábor
2019-01-18 18:55   ` [PATCH v1 2/2] checkout: fix regression in checkout -b on intitial checkout Ben Peart
2019-01-18 20:00     ` Junio C Hamano
2019-01-19  0:52     ` SZEDER Gábor
2019-01-19  1:26   ` [PATCH v1 0/2] Fix regression in checkout -b Junio C Hamano
2019-01-21 19:50   ` [PATCH v2 " Ben Peart
2019-01-21 19:50     ` [PATCH v2 1/2] checkout: add test to demonstrate regression with checkout -b on initial commit Ben Peart
2019-01-23 17:57       ` SZEDER Gábor
2019-01-21 19:50     ` [PATCH v2 2/2] checkout: fix regression in checkout -b on intitial checkout Ben Peart
2019-01-22 14:35       ` Johannes Schindelin
2019-01-22 18:42         ` Junio C Hamano
2019-01-22 18:49         ` Jeff King
2019-01-22 18:54     ` [PATCH v2 0/2] Fix regression in checkout -b Junio C Hamano
2019-01-22 19:31       ` Ben Peart
2019-01-23 19:14         ` Junio C Hamano
2019-01-23 20:01   ` [PATCH v3 " Ben Peart
2019-01-23 20:02     ` [PATCH v3 1/2] checkout: add test demonstrating regression with checkout -b on initial commit Ben Peart
2019-01-23 20:02     ` [PATCH v3 2/2] checkout: fix regression in checkout -b on intitial checkout Ben Peart
2019-01-23 21:23     ` [PATCH v3 0/2] Fix regression in checkout -b Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqef9th4iy.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=asottile@umich.edu \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    --cc=peartben@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).