Re: Re* git commit fails under some circumstances
From: Jeff King <hidden>
Date: 2016-06-15 22:50:59
On Sat, Apr 02, 2011 at 12:16:23PM -0700, Junio C Hamano wrote:
When running "commit" and "status" with files marked with "intent to add",
I think there are three possible interpretations of what the user wants to
do.
(1) I earlier said "I'll decide the exact contents to be committed for
these paths and tell you by running 'git add' later." when I said
'git add -N'. But I forgot to do so before running "git commit".
Thanks for catching this mistake and erroring out.
(2) I said "I'll decide the exact content to be committed ... later."
when I said 'git add -N'. I am running "git commit" now, but I still
don't know what the final contents for this path should be. I
changed my mind, and I do not want to include the addition of these
paths in the commit I am making. Please do not error out, but just
ignore the earlier 'add -N' for now.
(3) I said "I'll decide the exact content to be committed ... later."
when I said 'git add -N'. I am running "git commit" now, without
explicitly telling you with 'git add' about the final contents for
these paths. Please take it as an implicit hint that I am happy with
the contents in the working tree and commit them as if I said 'git
add' on these paths, but do leave modifications to already tracked
paths that I haven't added with 'git add'.
The current behaviour of "git commit" that refuse to commit without
telling git what the final contents for these pathse is a very deliberate
design and implementation of (1).
I think that all three of those are reasonable things to want to do, and
that the current behavior of (1) is a nice, conservative default. But
where we could do better is in helping the user understand what happened
and what their options are.
In Laszlo's example, he saw:
$ git commit
lib/achievement.cpp: not added yet
lib/achievement.h: not added yet
lib/achievementparser.cpp: not added yet
lib/achievementparser.h: not added yet
error: Error building trees
I see a couple of places to improve:
1. We say "not added yet", which is a good start. We at least point
out the problematic paths. But it's perhaps a little confusing.
Those paths _have_ been added. It's just that no content was added
at them yet.
2. The "error: Error building trees" is unnecessarily scary, as it
looks like git ran into some error while trying to do what you
wanted (and indeed, that is the same message you get for something
like "oops, we couldn't write to the object db"). It's not even
clear from that output that the "not added yet" lines are the cause
of the error, unless you understand how "git commit" is
implemented.
3. There is no advice given on how to proceed.
So I think a much nicer output would be something like:
$ git commit
error: some paths could not be committed
The following paths were marked with intent-to-add, but have
not had any content added:
lib/achievement.cpp
lib/achievement.h
lib/achievementparser.cpp
lib/achievementparser.h
If you want to commit them with their current content, use "git add".
If you want git to forget about them, use "git rm --cached".
We could also provide some options to git commit to make those
operations easier. Especially "ignore these for this commit but leave
them in the index" is a little awkward to do.
Having said all that, we might want to differentiate these paths in the output from 'status'.
Yes, that was my first thought on reading Laszlo's message. We are somewhat lying to say "changes to be committed". Your suggestion:
# ... to be committed ...
# new file: RENAMING (needs 'git add')
# ... have local changes ...
# modified: RENAMINGYou could also give them a separate stanza, like: # Changes to be committed: # (use "git reset HEAD <file>..." to unsage) # # modified: some-other-file # # Paths marked for addition, but needing content: # (use "git add <file>..." to update what will be committed) # # new file: RENAMING
if we don't care about the backward compatibility.
I don't know how much we need to care about "git status" output of this form. It is parsed mainly be editors' syntax highlighters. Adding this new information might not get highlighted as nicely, but it's probably not a big deal. I am much more concerned with whether and how this information would be represented in the "git status --porcelain" format. -Peff