git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,
	Srinidhi Kaushik <shrinidhi.kaushik@gmail.com>
Subject: Re: [PATCH v2 1/3] diff-files --raw: handle intent-to-add files correctly
Date: Wed, 24 Jun 2020 08:26:33 -0700	[thread overview]
Message-ID: <xmqqy2oc8oye.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.2006241517320.54@tvgsbejvaqbjf.bet> (Johannes Schindelin's message of "Wed, 24 Jun 2020 15:26:20 +0200 (CEST)")

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Sure, but my intention was to synchronize the `--raw` vs the `--patch`
> output: the latter _already_ shows the correct hash. This here patch makes
> the hash in the former's output match the latter's.

That is shooting for a wrong uniformity and breaking consistency
among the `--raw` modes.

 $ git reset --hard
 $ echo "/* end */" >cache.h ;# taint
 $ git diff-files --raw
 ... this shows (M)odified with 0{40} on the postimage
 ... 0{40} for side that is known to have contents from low-level diff
 ... means "object name unknown; figure it out yourself if you need it"
 $ git update-index cache.h
 $ git diff-files --raw
 ... of course we see nothing here.  Wait for a bit.
 $ touch cache.h ;# smudge
 $ git diff-files --raw
 ... this shows (M)odified with 0{40} on the postimage
 ... again, it says "it is stat dirty so I do not bother to compute"
 $ git update-index --refresh
 $ git diff-files --raw
 ... again we see nothing.

Any tools that work on "--raw" output must be already prepared to
see 0{40} on the side that is known to have contents and must know
to grab the contents from the working tree file if they need them,
so showing the 0{40} for i-t-a entry (whose definition is "the user
said in the past that the final contents of the file will be added
later, but Git does not know what object it will be yet") cannot
break them.  And the behaviour of giving 0{40} in such a case aligns
well with what is already done for paths already added to the index
when Git does not have an already-computed object name handy.

> Besides, we're talking about the post-image of `diff-files`, i.e. the
> worktree version, here. I seem to remember that the pre-image already uses
> the all-zero hash to indicate precisely what you mentioned above.

The 0{40} you see for pre-image for (A)dded paths means a completely
different thing from the 0{40} I have been explaining in the above,
so that is not relevant here.

By definition, there is *no* contents for the pre-image side of
(A)dded paths (that is why I stressed the "side that must have
contents" in the above description---it is determined by the type of
the change), but because the format requires us to place some
hexadecimal there, we fill the space with 0{40}.  

When we do not know the object name for the side that is known to
have contents without performing extra computation (including "stat
dirty so we cannot tell without rehashing"), we also use 0{40} as a
signal to say "we do not know the actual contents", but the consumer
of "--raw" format is expected to know the difference between "this
side is known to have no data and 0{40} is here as filler" and "this
side must have contents but we see 0{40} because Git does not have
it handy in precomputed form".

The above is the same for "diff-index --raw" without "--cached";
when we have to hash before we can give the object name (e.g. the
path is stat-dirty), we give 0{40} and let the consumer figure it
out if it needs to.

 $ git reset --hard
 $ touch COPYING
 $ git diff-index --raw HEAD
 ... we see (M)odified with 0{40} on the right hand side.

When the caller asks for "--patch" or any other output format that
actually needs contents for output, however, these low-level tools
do read the contents, and as a side effect, they may hash to obtain
the object name and show it [*1*].

By the way, as I do not want to see you waste your time going in a
wrong direction just to be "different", let me make it clear that as
far as the design of low level diff plumbing is concerned, what I
said here is final.  Please don't waste your time on arguing for
changing the design now after 15 years.  I want to see your time
used in a more productive way for the project.

Thanks.


[Footnote]

*1* This division of labor to free "--raw" mode of anything remotely
    unnecessary stems from the original diff plumbing design in May
    2005 where the "--raw" mode was the only output mode, and there
    was a separate "git-diff-helper" (look for it in the mailing
    list archive if you want to learn more) that reads a "--raw"
    output and transforms it into the patch form.  That "once we
    have the raw diff, we can pipe it to post-processing and do more
    interesting things" eventually led to the design of the diffcore
    pipeline where we match up (A)dded and (D)eleted entries to
    detect renames, etc.


  reply	other threads:[~2020-06-24 15:26 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-11 12:38 [PATCH 0/3] Fix difftool problem with intent-to-add files Johannes Schindelin via GitGitGadget
2020-06-11 12:38 ` [PATCH 1/3] diff-files: fix incorrect usage of an empty tree Johannes Schindelin via GitGitGadget
2020-06-11 12:38 ` [PATCH 2/3] diff-files --raw: handle intent-to-add files correctly Johannes Schindelin via GitGitGadget
2020-06-11 12:38 ` [PATCH 3/3] difftool -d: ensure that intent-to-add files are handled correctly Johannes Schindelin via GitGitGadget
2020-06-23 12:48 ` [PATCH v2 0/3] Fix difftool problem with intent-to-add files Johannes Schindelin via GitGitGadget
2020-06-23 12:48   ` [PATCH v2 1/3] diff-files --raw: handle intent-to-add files correctly Johannes Schindelin via GitGitGadget
2020-06-24  0:09     ` Junio C Hamano
2020-06-24 13:26       ` Johannes Schindelin
2020-06-24 15:26         ` Junio C Hamano [this message]
2020-06-24 18:41           ` Junio C Hamano
2020-06-25 17:52           ` Johannes Schindelin
2020-06-24  7:11     ` Srinidhi Kaushik
2020-06-24 13:34       ` Johannes Schindelin
2020-06-24 15:51         ` Junio C Hamano
2020-06-23 12:48   ` [PATCH v2 2/3] diff-files: fix incorrect usage of an empty tree Johannes Schindelin via GitGitGadget
2020-06-24  0:10     ` Junio C Hamano
2020-06-23 12:48   ` [PATCH v2 3/3] difftool -d: ensure that intent-to-add files are handled correctly Johannes Schindelin via GitGitGadget
2020-06-24 14:47   ` [PATCH v3 0/3] Fix difftool problem with intent-to-add files Johannes Schindelin via GitGitGadget
2020-06-24 14:47     ` [PATCH v3 1/3] diff-files --raw: handle intent-to-add files correctly Johannes Schindelin via GitGitGadget
2020-06-24 14:47     ` [PATCH v3 2/3] diff-files: fix incorrect usage of an empty tree Johannes Schindelin via GitGitGadget
2020-06-24 14:47     ` [PATCH v3 3/3] difftool -d: ensure that intent-to-add files are handled correctly Johannes Schindelin via GitGitGadget
2020-06-25 17:53     ` [PATCH v4 0/2] Fix difftool problem with intent-to-add files Johannes Schindelin via GitGitGadget
2020-06-25 17:53       ` [PATCH v4 1/2] diff-files --raw: show correct post-image of " Johannes Schindelin via GitGitGadget
2020-06-25 18:08         ` Junio C Hamano
2020-07-01  9:46           ` Johannes Schindelin
2020-07-01 21:02             ` Junio C Hamano
2020-06-25 18:21         ` Junio C Hamano
2020-07-01  9:52           ` Johannes Schindelin
2020-06-26 17:49         ` Srinidhi Kaushik
2020-06-25 17:53       ` [PATCH v4 2/2] difftool -d: ensure that intent-to-add files are handled correctly Johannes Schindelin via GitGitGadget
2020-06-25 18:11         ` Junio C Hamano
2020-07-01 10:01           ` Johannes Schindelin
2020-07-01 21:03             ` Junio C Hamano
2020-07-01 21:20               ` Johannes Schindelin
2020-07-01 21:19       ` [PATCH v5 0/2] Fix difftool problem with intent-to-add files Johannes Schindelin via GitGitGadget
2020-07-01 21:19         ` [PATCH v5 1/2] diff-files --raw: show correct post-image of " Johannes Schindelin via GitGitGadget
2020-07-01 21:19         ` [PATCH v5 2/2] difftool -d: ensure that intent-to-add files are handled correctly Johannes Schindelin via GitGitGadget

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=xmqqy2oc8oye.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=shrinidhi.kaushik@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).