Re* git commit fails under some circumstances

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Laszlo Papp <djszapi@xxxxxxxxxxxx> writes:

> There are 2 bugs and 1 wish in this case:
> 1) It says "Changes to be committed:", but they are actually not
> committed, there cannot even be committed since they were not added
> "properly". This output is bogus
> 2) error: Error building trees

Thanks.

You gave us a concise reproduction recipe and the output you get from it,
which is far better than what an average new user would do.  But that is
still two out of three of what makes a good bug report.

When you utter the word "bug" (or the word "bogus") to an existing
project, please make it a habit to mention what you think should happen as
well.  This is not limited to git but in any open source project you are
not familiar with in general.

It helps tremendously to have a reproduction recipe and its output to
diagnose certain kinds of issues.  If somebody else tries to run it, and
if it gave results different from the output you are getting, you may have
uncovered some unintended environment dependency bug.

But what if your output matches what the original developers expected to
see?  Possibly after spending many hours to design how the thing ought to
behave, it is possible that they still might not have thought about a
better behaviour in some cases, and you may have an idea to present the
result in a more useful way---and your complaint might be that the current
output does not match that idea of yours.  In such a case, a bug report
that does not state what should be shown stops short of being useful.

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).

The second one is a possible interpretation, but it is a very error prone
one.  People who want to commit without these new files wouldn't have
marked them with "add -N" in the first place, and (2) feels more like
disabling a useful error checking than making the command useful.  Also it
is a silent bug to behave like (2) to people who expect (3), as they will
only later find out that some of the additions were not committed against
their will.

The third one also is a possible interpretation, but it is a very
inconsistent one.  Why should new paths marked with 'git add -N' be
treated differently from paths that were already tracked and you made
changes to the working tree?  In addition, it is a silent bug to behave
like (3) to people who expect (2), as they will only later find out that
some of the additions were committed against their will.

Note that the user who may want to see variants of (2) and (3) can still
name which paths to include (and to exclude by not naming them):

    $ edit foo bar baz
    $ git add baz
    $ git add -N foo bar
    $ git commit baz foo

or include everything

    $ git commit -a


I thought the above issues through when I wrote "git add -N" and
determined that (1) is the only sane and useful behaviour, and I still
think it is.

Having said all that, we might want to differentiate these paths in the
output from 'status'.  The 'status' command in its current form and
semantics came much later (v1.7.0) than "add -N" (introduced in v1.6.1),
and because "add -N" is such a low-key corner case (adding new paths is
far less frequently done than modifying an existing paths, and showing
only intent to add new paths is even less frequently done), we probably
didn't think about special casing them.

Currently we show these paths as both added for commit (i.e. appears in
the top portion of the verbose output, and in the first column of the -s
output) and having local modification (i.e. appears in the second part or
in the second column, respectively).  For example, here is what I get:

    $ cp COPYING RENAMING
    $ git add -N RENAMING
    $ git status -s
    AM RENAMING
    $ git status
    # ... to be committed ...
    # new file: RENAMING
    # ... have local changes ...
    # modified: RENAMING

We should be able to say something like:

    $ git status -s
    IM RENAMING
    $ git status
    # ... to be committed ...
    # new file: RENAMING (needs 'git add')
    # ... have local changes ...
    # modified: RENAMING

if we don't care about the backward compatibility.

> 3) It would be nice to have one command (with no (git) alias for sure)
> to see the difference like "git diff" but also the new files.

Doesn't "git diff" show the difference for files you told git about via
the "add -N" interface?  After the above "addition" of RENAMING, if I ask
"git diff" (or "git diff HEAD"), I get what I expect to see (addition of
the contents taken from the whole file in the working tree).

Again, please describe what you think should be the right output if it
differs from your expectation.

Also having said that, I notice that "git diff --cached" behaves as if an
empty blob is added in such a case.  I am not sure if we want to special
case this.  After all paths marked with "git add -N" does _not_ have a
concrete contents in the index by definition (as the user told "I'll tell
you later" but hasn't done so), and may want to behave more like unmerged
entries for certain operations (i.e. the path does exist, but comparing
something with it does not have a usual meaning, etc.)

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]