git diff <file> HEAD^:<file> error message

git diff <file> HEAD^:<file> error message

[Carlos Martín Nieto]

[-- Attachment #1: Type: text/plain, Size: 1434 bytes --]

Hello,

I was trying to figure out why running

   git diff HEAD^:RelNotes RelNotes

gives the expected output (on maint it tells me that the stable
version changed from 1.7.8.3 to 1.7.8.4) but swapping the arguments
doesn't.

   git diff RelNotes HEAD^:RelNotes

doesn't show the opposite patch but tells me that RelNotes doesn't
exist in HEAD^ which is clearly a lie (it sounds like it's a
misunderstanding on git's part, but it's certainly not the truth). In
both cases, verify_filename gets called and tries to stat both
HEAD^:RelNotes and RelNotes. In the "bad" (latter) case, after it
fails to find a file named "HEAD^:RelNotes" it ends up calling
diagnose_invalid_sha1_path which is not correct according to the
command just before the function

    /* Must be called only when object_name:filename doesn't exist. */

It looks like get_sha1_with_context_1 gets confused because we pass it
a filename which looks like object_name:filename even though we
earlier parsed it simply as a filename which happens to have a colon
inside it.

Another issue is that I'm not sure that the error message should even
get shown. The documentation tells me that I should be able to compare
two random blobs, though this mode doesn't seem to work if the first
argument is a file. I realise that a file isn't a blob, but since
specifying the arguments the other way around (blob, file) does work,
it looks to me like it should as well.

   cmn

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: git diff <file> HEAD^:<file> error message

[Junio C Hamano]

Carlos Martín Nieto <cmn@elego.de> writes:

> I was trying to figure out why running
>
>    git diff HEAD^:RelNotes RelNotes
>
> gives the expected output (on maint it tells me that the stable
> version changed from 1.7.8.3 to 1.7.8.4) but swapping the arguments
> doesn't.
>
>    git diff RelNotes HEAD^:RelNotes
>
> doesn't show the opposite patch ...

That comes from the general argument parsing rules of Git, namely, global
options (e.g. --paginate) first, then subcommand name, followed by dashed
options, revs and finally the paths. Once you give "RelNotes", which
cannot be a rev, you cannot give a rev.

We _could_ special case the rule for "diff", but we simply didn't bother,
as the resulting code (and the implications of special casing) would be
too ugly to live to support such a corner case usage, especially when you
could always say "-R" to reverse the output.

Re: git diff <file> HEAD^:<file> error message

[Carlos Martín Nieto]

[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]

On Wed, Jan 11, 2012 at 06:26:30PM -0800, Junio C Hamano wrote:
> Carlos Martín Nieto <cmn@elego.de> writes:
> 
> > I was trying to figure out why running
> >
> >    git diff HEAD^:RelNotes RelNotes
> >
> > gives the expected output (on maint it tells me that the stable
> > version changed from 1.7.8.3 to 1.7.8.4) but swapping the arguments
> > doesn't.
> >
> >    git diff RelNotes HEAD^:RelNotes
> >
> > doesn't show the opposite patch ...
> 
> That comes from the general argument parsing rules of Git, namely, global
> options (e.g. --paginate) first, then subcommand name, followed by dashed
> options, revs and finally the paths. Once you give "RelNotes", which
> cannot be a rev, you cannot give a rev.
> 
> We _could_ special case the rule for "diff", but we simply didn't bother,
> as the resulting code (and the implications of special casing) would be
> too ugly to live to support such a corner case usage, especially when you
> could always say "-R" to reverse the output.

The rule "non-rev stops rev parsing" is fair enough. The error message
is still very misleading, as it lies about RelNotes not being in HEAD^
and gives the impression that it was parsed as a rev (which I guess it
was, but only to show the message).

   cmn


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: git diff <file> HEAD^:<file> error message

[Nguyen Thai Ngoc Duy]

On Wed, Jan 11, 2012 at 6:18 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
> Hello,
>
> I was trying to figure out why running
>
>   git diff HEAD^:RelNotes RelNotes
>
> gives the expected output (on maint it tells me that the stable
> version changed from 1.7.8.3 to 1.7.8.4) but swapping the arguments
> doesn't.
>
>   git diff RelNotes HEAD^:RelNotes
>
> doesn't show the opposite patch but tells me that RelNotes doesn't
> exist in HEAD^ which is clearly a lie (it sounds like it's a
> misunderstanding on git's part, but it's certainly not the truth).

I find Jonathan's comment [1] interesting: "Meanwhile, there is no
plumbing command to compare two blobs. Strange".

I _think_ the main purpose of git diff is to compare a stage (a
revision, index, worktree) with another stage, filtered by path and
blob-to-blob diff is a minor thing that is needed to support "git diff
<tag> <tag>" where both tags point to a tag. It'd be better to start a
new command that diff between two blobs (or files in worktree/index).
Something pretty close to --no-index. You would not need to mess up
with setup_revisions() or verify_filename().

[1] ed84e6d (Documentation: diff can compare blobs - 2010-10-11)
-- 
Duy