checkout extra files

Hello,

the man page of git checkout states:

git checkout [-p|--patch] [<tree-ish>] [--] <pathspec>...

It updates the named paths in the working tree from the index file or from a named <tree-ish> ...

This means that for each file denoted by pathspec, git tries to restore it from the tree-ish. However, it seems that git does more than this: it restores also files that are not denoted by pathspec. This sequence of commands shows it:

$ mkdir gittest
$ cd gittest
$ git init
Initialized empty Git repository in d:/gittest/.git/
$ touch f1
$ git add f1
$ git commit commit -m "first commit"
[master (root-commit) 94d882a] first commit
 0 files changed
 create mode 100644 f1
$ rm f1
$ git checkout 94d8 -- *
$ ls
f1

Note that the work directory is empty when the checkout is done, and that the checkout restores f1 in it, a file that is not denoted by the * pathspec. I guess that this is the intended behaviour, and that the man page should be updated to tell exactly what files git restores.

-Angelo Borsotti

Angelo Borsotti <angelo.borsotti@gmail.com> writes:

The '*' pathspec refers to every file. There are no files in the current directory, so your shell can't expand the glob and passes it to git, which then looks and sees that the glob expands to f1 in the given commit which it then checks out.

   cmn

On Mon, Sep 3, 2012 at 8:42 PM, Angelo Borsotti <angelo.borsotti@gmail.com> wrote:

I think in this case '*' remains unexpanded by the shell. Which means it is still a pathspec to checkout (iow equivalent to "git checkout 94d8 -- '*'"). Checkout in turns matches the pathspec '*' against the tree and decides to restore 'fl'.

It's confusing but I don't think there's much we can do about it.

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

The user can, by telling the shell to expand '*' that does not match to nothing, though.

Is there anything that is confusing in our documentation? I am not looking for a change to the documentation that protects it from getting misunderstood by deliberately twisted interpretations (such a change would make the resulting text would harder to read), but I do want to make sure it is not prone to confusion even to a casual and careless reader.

For this particular scenario, I do not see anything offhand that is unclear about the behaviour of Git in the documentation, even though as you pointed out, if the user is unaware that the shell passes globs unmodified when they do not match, it may lead to a confusion like this. I certainly do not want to do a full "introduction to shell" in our documentation, but if adding a short sentence or two helps to avoid confusion like this, I do not strongly object to it.

On Tue, Sep 4, 2012 at 2:36 AM, Junio C Hamano <gitster@pobox.com> wrote:

It works 99% the time, I don't think any users would bother setting that or studying the shell at that detail, until they are surprised.

Yeah, one or two sentences about two level expansion might help new users. I'll check that. Maybe in the pathspec definition, then split that part of out glossary-content.txt to be included in pathspec-using commands as a separate pathspec section. Too much?

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

Perhaps like this?

 Documentation/git-checkout.txt | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Hi all,

consider this example:

$ mkdir gittest
$ cd gittest
$ git init
Initialized empty Git repository in d:/gittest/.git/
$ touch f1
$ git add f1
$ git commit commit -m "first commit"
[master (root-commit) e6f935e] first commit
 0 files changed
 create mode 100644 f1
$ touch f2
$ git checkout e6f9 -- *
error: pathspec 'f2' did not match any file(s) known to git.

Note the error. It is clear that the set of file names that git checkout is taking is the union of the ones that match the specified path ('*') in the work directory (gittest) with the ones that match the path in the specified commit (e6f9). I am not questioning this behavior, but the documentation, which does not describe it:

   "It updates the named paths in the working tree from the index file or
   from a named <tree-ish> ..."

There are two ways to read this sentence:

    1.  "named paths" referred to working tree, i.e. the files whose
names match the
          paths among all the ones present in the working tree
    2.  "named paths" referred to the index or tree-ish, i.e. the
files whose names match
          paths among oll the ones present in the index or tree-ish

In both cases, nothing tells the user that the matching of the paths is done over the union of the set of file names of the working tree + ndex or tree-ish. Indeed, the first time I have seen the error above I got quite confused because I thought that the checkout would restore the working directory as it was at the time I made the commit, without bothering about extra files that the directory contained at the moment (and note that f2 is not even a tracked one). This behavior is a bit strange, but if it is a hundred percent clearly documented I can live with it.

I think that knowing precisely what files are involved in this command is essential for the user.

-Angelo

Angelo Borsotti <angelo.borsotti@gmail.com> writes:

It checks out the contents of the named paths out of either the index or out of the tree-ish and stores that to a file in the working tree. The working tree is not consulted by Git to tell what paths the user wants to affect at all, but if you pass fileglob, your shell may consult the working tree to expand it to concrete pathnames it gives to Git.

Angelo Borsotti <angelo.borsotti@gmail.com> writes:

At this point, the working tree presumably is empty.

Now we have f1 in the working tree in the index, and in the tree of the commit.

Now we have f1 and f2 in the working tree.

That is the same as "git checkout e6f935e -- f1 f2", as the shell expanded "*" into "f1" and "f2".

Yes?

The command tells git to check out "f1" and "f2" out of the tree of e6f935e, and git found "f1" but did not find "f2" and reported an error. I do not see a room or need for "union" to come into the picture to explain what we see in the above transcript.

Junio C Hamano <gitster@pobox.com> writes:

Actually, I "kind of sort of" can see where that "union" is coming from, if I squint my eyes hard enough.

Yes, it makes it look like the path affected has some relationship between two sets of paths:

 - set W, which consists of f1 and f2, that is the result of
   matching '*' against working tree files; and

 - set T, which consists of f1 (but not f2), that is the result of
   matching '*' against the tree contained in e6f935e

and the intersection of W and T (i.e. f1) is what is checked out.

But that is not what is happening at all. What goes on is far simpler than that.

 - the shell sees '*', matches it against working tree files, to
   obtain "f1" and "f2";

 - the shell tells "git" to "checkout e6f935e -- f1 f2";

 - "git" looks into the tree of e6f935e to find paths that match
   "f1" and "f2".

When "git" is run by the shell in the last step, it has _no_ clue that the end user typed "*" from the shell. It only sees "f1" and "f2" on the command line. There is no "set T" to be intersected with "set W", so stop thinking in those terms, and you will be fine.

Now the question is, _you_ will be fine, but can the documentation be updated in such a way so that it will help _others_ to also stop thinking about "intersection between set W and set T"? I do not have a good answer to that.

Junio C Hamano <gitster@pobox.com> writes:

Let's do this. I do not want a shell tutorial in "git checkout" documentation, but this would fit better in the documentation for the CLI convention.

-- >8 -- gitcli: contrast wildcard given to shell and to git

People who are not used to working with shell may intellectually understand how the command line argument is massaged by the shell but still have a hard time visualizing the difference between letting the shell expand fileglobs and having Git see the fileglob to use as a pathspec.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/gitcli.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

From: "Junio C Hamano" <gitster@pobox.com>
Sent: Friday, September 07, 2012 9:49 PM

The difficulty with putting it in gitcli is that it is referenced from almost nowhere, so won't provide help to the user.

Having said that, it would therefore be better to point folk at gitcli in a few more places, not just the 'see also' line at the very end of the general 'git' page, and buried within rev-parse.

"Philip Oakley" <philipoakley@iee.org> writes:

Didn't we update the very early part of git(1) for exactly for that reason recently?

Junio C Hamano <gitster@pobox.com> writes:

I don't think many people read git(1) directly, as there are many other starting points to learn Git (official tutorial, user-manual, and tens of very good tutorial on the web). On the other hand, reading git-<command> is probably much more common, as it is the only place to find exhaustive documentation about a particular command.

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

Many of which is outside what patches made against to my tree would be able to fix. I wonder if we can have some mechanism to easily notify and help the owners of these material to keep them up to date.

That "people diving into 'git --help <subcmd>', assuming everything can be learned there" is a problem within the scope of what we could control. For obvious reasons, including "glossary-contents" and "gitcli" at the beginning of documentation for each and every subcommand is not a useful solution, and referring the prerequisite reading for them in git(1) was done as the first step to solve that issue, and you are essentially saying that it is not enough.

So what is the right second step?

From: "Junio C Hamano" <gitster@pobox.com>

Oops I'd forgotten that specific change.

a simple link to the gitcli page? or, add the <pathspec> into the checkout options list and a link to a suitable place for that, which can then point to the gitcli.

From my perspective the majority of the top twenty git commands should, within each of their help pages, have an in-text link into one or other of the various 'guide' style articles, which can then be interconnected to each other. This should give the beginner help by directing them away from the details of a man page for the general issues. That is, distinguish which parts are help for those who aren't sure what they need to know, from the those parts that provide the specfic details for those do know.

I hope to have some documentation patches in the next few days on other small misunderstandings I've seen/made. Just battling my windows / msysgit / pu set-up at the moment.

On Fri, Sep 07, 2012 at 01:49:15PM -0700, Junio C Hamano wrote:

I think this is an improvement, but...

Look at the paragraph below your addition. It is typographically outside of the bulleted list you are adding to, but it really makes sense directly after the prior two bullet points, which are explicitly about disambiguation between revisions and paths. Your addition splits them apart.

Does it make sense to join that final paragraph into what is now the third bullet, and then add your new text (the fourth bullet) after?

-Peff

Jeff King <peff@peff.net> writes:

Yes, I noticed it and thought about different possibilities, including making the description of glob the first bullet point.

I am not sure. After re-reading it, I do not think the fileglob discussion belongs to the existing "Here are the rules" list in the first place. It should probably be the extended description for the first point (revisions then paths) to explain what kind of "paths" we accept there.

I generally consider follow-up paragraphs after bulleted list to be enhancements on any of the points in the list, not necessarily applying to all of them. The existing structure is:

    * point A (revisions and paths)
    * point B (-- can be used to disambiguate)
    * point C (ambiguation leads to an error)

    Note that point B and point C taken together imply corollary BC.

So something like this would be the right thing to do:

    * point A
    * point B
    * point C

    Note that point B and point C taken together imply corollary BC.
    Also note that point A implies corollary AA.

or even

    * point A
    * point B
    * point C

    Note that point A implies corollary AA.  Also note that point B
    and point C taken together imply corollary BC.

So perhaps something like this squashed in on top of the patch in question?

 Documentation/gitcli.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

On Mon, Sep 10, 2012 at 10:09:43AM -0700, Junio C Hamano wrote:

I had a vague feeling that it did not quite belong, too, but I was not sure where it should go.

I do not think so. That point is about the order of revisions and paths, and has nothing to do with the syntax of paths. Really, every element of that list is about handling revisions versus paths. I think this content does not necessarily go in such a list.

I would argue the opposite; if it is about a specific point, then put it with the point. Otherwise, you are asking the reader to remember back to an earlier point (that they may not even have read; in reference documentation, the point of a list is often to let readers skip from bullet to bullet easily).

If it is a synthesis of multiple elements in the list, then that makes more sense. And I think that is what you are implying here:

Which is fine by me. But inserting a point D that is not related to B, C, or BC, only makes it harder to read.

I don't think that makes it any better. You went from:

  * A
  * B
  * C
  * D

  By the way, B and C imply BC.

to:

  * A
  * B
  * C

  By the way, D.

  Also, B and C imply BC.

I think it would make more sense to do:

  * A
  * B
  * C

  By the way, B and C imply BC.

  Also, D.

(where obviously my "connecting" phrases do not need to be part of the text, but are meant to illustrate how I am thinking about the structure).

-Peff

Jeff King <peff@peff.net> writes:

You need to follow all the rules when composing your command line. You cannot simply ignore ones that are inconvenient for you and pick only the one you like.

The second and the third one are related in the "sides of the same coin" sense; you either have "--" in which case no disambiguation checks are done, or don't in which case your command line may get an ambiguity error, so in that sense, you could say "I am writing '--', so point C does not apply to me and I skip". But whether you do or do not say '--', you have to have your revs before pathspecs, so you cannot skip point A.

So I do not think a bullet list is designed to let the readers skip and forget (or "may not even have read"). If that is the case, perhaps we would need to use something else to give the set of rules that apply to the command line here.

I think the following is probably the best.

    * A (revs and then paths)
    * B (with "--", no dwim is done).
    * C (without "--", disambiguation kicks in. By the way, this
      means your script had better avoid this form; make sure you
      use "--").
    * D (pathspecs are patterns).

without the trailing paragraph, which is meant only for people who write their script without using "--" by mistake, i.e. it only belongs to point C.

The paragraph to encourage use of "--" in scripts belongs to the bullet point that describes the behaviour for a command line without the explicit "--" disambiguation, not a supporting explanation for the entire bulletted list.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/gitcli.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

People who are not used to working with shell may intellectually understand how the command line argument is massaged by the shell but still have a hard time visualizing the difference between letting the shell expand fileglobs and having Git see the fileglob to use as a pathspec.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/gitcli.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

On Mon, Sep 10, 2012 at 12:35:05PM -0700, Junio C Hamano wrote:

Of course. But note that I said "reference documentation". It is quite frequent that you have read it a long time ago, or understand already most of what it is saying, and you are re-reading it again looking for rules about a _specific_ element (say, wildcards). You might not have just now read the bit about disambiguation, but you do not need to; you already know it, or it might not be relevant to what you are doing.

I think we need to be realistic about the readers of our documentation. Sometimes people will sit down and read all the way through, and we need the text to flow and make sense for that case. But just as often, they will be curious about one specific point, and we need to make it as easy as possible for them to see when a new point is being made, and when they can stop reading because they have wandered into a new point that they may already know.

Which is why I think the best thing we can do for such a casual reader is make sure that the typography helps group related text together. In this specific example, imagine I am a seasoned Unix user and a new git user. If I were reading about "--" and revs versus paths, that would be news to me, because it is about git. When I see the next bullet is about quoting "*" to pass it through the shell to git, I say "Of course. That is how Unix shells work" and stop reading. It seems like a disservice to the reader to include more on the "--" disambiguation _after_ that bullet point.

I think it is OK here. As a tool for people reading the whole text, I think the list is a bad format, since the elements do not follow a good parallel structure (as you said, the second and third are much more related than the first and fourth).

So I was tempted to suggest removing the list altogether and turning it into paragraphs.

But as I said, I think breaking the points with whitespace helps the casual reader using it as a reference. I'm not sure you agree, but maybe what I've written above will change that. If not, I think I've said as much as is useful on the matter and I'll stop talking. :)

Hmph. Isn't that what I suggested in my first email? :P

I am fine with the series you sent.

-Peff

Jeff King <peff@peff.net> writes:

Until I read the current text I did not realize the trailing paragraph was to apply only to point C (no "--" disambiguates and throws errors) but somehow thought it was covering both point B (with "--" you are strict) and C, and I didn't think of a good way to incorporate it into both. But yes, the final patch ended up to be exactly what you suggested to handle the issue ;-)

Thanks.

On Tue, Sep 4, 2012 at 9:55 AM, Junio C Hamano <gitster@pobox.com> wrote:

Looks good. I was going to complain that this patch applied to git-checkout.txt only but I just saw git-add.txt also mentions about quoting wildcards. So I'm good.

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

Ok, with a log message like this:

    Document file-glob for "git checkout -- '*.c'"
    
    Just like we give a similar example in "git add" documentation.

Angelo Borsotti <angelo.borsotti@gmail.com> writes:

[please keep it in the list]

That grouping is not what it's saying. It doesn't update the files that exist in the working tree matching some glob. It updates the files in the working tree from either the index or a treeish. The pathspec refers, as always, to the data source, and '*' matches all files.

It puts the named paths on to the working tree. Is that clearer?

Whether they are missing doesn't make any difference. It updates the files you tell it from the index/tree. You told it to update all of them.

   cmn

Keep it in the list.

Angelo Borsotti <angelo.borsotti@gmail.com> writes:

So you're saying that you ran

    git checkout <tree-ish> -- *

and git complained that there was no such file? This is because the shell expanded the glob and gave git a list of files.

I can't quite parse this. Git will restore whichever files you tell it to. If you use an asterisk, then your shell will usually expand it. In the case you posted to the list there were no files, so there was nothing to expand it to. Some shells complain in this case by default, some don't and just pass the asterisk to the program and let it figure out what to do with it. This was the case in your example. You told git to expand all the files it found in that tree-ish.

   cmn

Hi Duy,

the semantics of "git checkout -- '*'" is quite clear because there is no expansion done by the shell. The wildcard is passed to git checkout, that interprets is properly.

That of "git checkout -- *" is the problem when the directory is empty. Note that this happens with the shell that is shipped with git (in the windows distro). A note in the documentation could help the user to understand this.

-Angelo

Angelo Borsotti <angelo.borsotti@gmail.com> writes:

Passing unmatched glob intact to the program has always been the standard behaviour of shells for decades, and is not specific to "the shell that is shipped with msysgit", no?

I would imagine that msysgit could be shipped with man pages for bash, but I doubt anybody would consult it when hitting this "I typed * in an empty directory" situation. In any case, what to include in the package is something msysgit folks to decide.

Hi,

figuring out what the behavior is by guessing how a command is implemented and what are its interactions with the shell is a bit hard for the user: s/he should instead get it from the documentation. I tried to figure it out from the examples I have done, and as you see, I did not get it quite right.

The issue here is that the paths must denote filenames that are present in the index or tree-ish, so, wildcards are misleading since they would instead be interpreted with respect to the working directory. A possible way to make this clear is to warn the user to quote paths that contain wildcards. Something like, e.g.:

  "Note that <paths> that contain wildcards must be quoted in order to
denote files that
   belong to the index or <tree-ish>. Otherwise, they are interpreted
by the shell with
   respect to the current directory, with a result that may depend on
the shell."

-Angelo

Angelo Borsotti <angelo.borsotti@gmail.com> writes:

When you are talking to a shell (and you almost never directly talk to Git), wildcards are always interpreted with respect to the working directory by the shell. And that is not specific to Git.

Perhaps, if you drop ", with a result..." from that sentence.

Even though that description is a bit too much on the side of "shell primer" than "git documentation" for my taste, I could see it may help some people, so I wouldn't reject such a phrasing out of hand.

Let's see what others feel.

From: "Junio C Hamano" <gitster@pobox.com>
Sent: Tuesday, September 04, 2012 9:44 PM

A comment about the need to quote wild cards would certainly be of advantage to many Windows users who won't have used a shell in that way before.

Plus I suspect that a large fraction of basic unix/linux users will have never really considered the difference between shell expansion and git expansion in this case where there are two diifferent 'file systems', as demonstrated by the initial query.

"Philip Oakley" <philipoakley@iee.org> writes:

OK, anybody wants to do a patch on top of 5635be1 (on 'pu') or on caae319 (which is 5635be1^2), which adds an example of quoted fileglob to git-checkout documentation?

Angelo Borsotti <angelo.borsotti@gmail.com> writes:

That's backwards. Saying <path> as if it means a plain vanilla pathname is a cause of confusion. The command takes pathspec, which is a pattern (see "git help glossary"). The places in the text that say <path> may need to be fixed.

It just happens that you do not realize that you are using pathspec when you say "git checkout hello.c", as the pattern "hello.c" only matches the one pathname "hello.c".

Junio C Hamano <gitster@pobox.com> writes:

I've read Documentation/git-checkout.txt and looked at the use of "paths".

the most of the "paths" (if not all) in the description are used as short-hand to mean "paths that the end user specified by giving a pathspec without repeating that expression over and over again. And it should be clear from the context, especially in places where we say things like "It updates the named paths", "update the index for the given paths", "checking out paths from the index", "when paths are given" etc.

As long as readers notice that the command takes <pathspec> on the command line, and understand <pathspec> has a specific meaning (i.e. it is a way to specify set of paths to be manipulated) and semantics, the existing text should be OK. The <paths> in synopsis section should be updated to <pathspec>, though.