All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Srinidhi Kaushik <shrinidhi.kaushik@gmail.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH v5 0/2] Fix difftool problem with intent-to-add files
Date: Wed, 01 Jul 2020 21:19:05 +0000	[thread overview]
Message-ID: <pull.654.v5.git.1593638347.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.654.v4.git.1593107621.gitgitgadget@gmail.com>

This problem was reported in 
https://github.com/git-for-windows/git/issues/2677, but the problem actually
lies with git diff --raw, and it seems that the bug has been with us ever
since --intent-to-add was introduced.

Changes since v4:

 * Improved both commit messages after feedback from Junio.

Changes since v3:

 * Instead of showing the same OID in git diff-files --raw as in git
   diff-files -p for intent-to-add files, we now imitate the logic for
   modified (non-intent-to-add) files: we show the "null" OID (i.e.
   all-zero).
   
   
 * As a consequence, we no longer just undo the inadvertent change where
   intent-to-add files were reported with the empty tree instead of the
   empty blob, but we fix the bug that has been with us since 
   run_diff_files() was taught about intent-to-add files, i.e. we fix the
   original bug of 425a28e0a4e (diff-lib: allow ita entries treated as "not
   yet exist in index", 2016-10-24), at long last.
   
   

Changes since v2:

 * Now we also drop the no-longer-used definition of hash_t in t2203.

Changes since v1:

 * Rebased onto sk/diff-files-show-i-t-a-as-new.
 * Verified that sk/diff-files-show-i-t-a-as-new does not completely resolve
   the issue (the --raw output still claims the empty blob as the
   post-image, although the difftool symptom "went away").
 * Amended the central patch of this PR to include a fix for the regression
   test that was introduced in sk/diff-files-show-i-t-a-as-new: it expected
   the raw diff to contain the hash of the empty tree object (which is
   incorrect no matter how you turn it: any hash in any raw diff should
   refer to blob objects).
 * Reordered the patches so that the central patch comes first (otherwise,
   the "empty tree" fix would cause a test failure in t2203).

Johannes Schindelin (2):
  diff-files --raw: show correct post-image of intent-to-add files
  difftool -d: ensure that intent-to-add files are handled correctly

 diff-lib.c            | 3 +--
 t/t2203-add-intent.sh | 5 ++---
 t/t7800-difftool.sh   | 8 ++++++++
 3 files changed, 11 insertions(+), 5 deletions(-)


base-commit: feea6946a5b746ff4ebf8ccdf959e303203a6011
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-654%2Fdscho%2Fdifftool-ita-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-654/dscho/difftool-ita-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/654

Range-diff vs v4:

 1:  69256ab910 ! 1:  4392ae4be6 diff-files --raw: show correct post-image of intent-to-add files
     @@ Commit message
      
                  [...] 0{40} if creation, unmerged or "look at work tree".
      
     -    This happens for example when showing modified, unstaged files.
     +    on the right hand (i.e. postimage) side. This happens for files that
     +    have unstaged modifications, and for files that are unmodified but
     +    stat-dirty.
      
          For intent-to-add files, we used to show the empty blob's hash instead.
          In c26022ea8f5 (diff: convert diff_addremove to struct object_id,
          2017-05-30), we made that worse by inadvertently changing that to the
          hash of the empty tree.
      
     -    Let's make the behavior consistent with modified files by showing
     +    Let's make the behavior consistent with files that have unstaged
     +    modifications (which applies to intent-to-add files, too) by showing
          all-zero values also for intent-to-add files.
      
          Accordingly, this patch adjusts the expectations set by the regression
 2:  9bb8d84ea9 ! 2:  5f933e852a difftool -d: ensure that intent-to-add files are handled correctly
     @@ Commit message
      
          In https://github.com/git-for-windows/git/issues/2677, a `git difftool
          -d` problem was reported. The underlying cause was a bug in `git
     -    diff-files --raw` that we just fixed.
     +    diff-files --raw` that we just fixed: it reported intent-to-add files
     +    with the empty _tree_ as the post-image OID, when we need to show
     +    an all-zero (or, "null") OID instead, to indicate to the caller that
     +    they have to look at the worktree file.
     +
     +    The symptom of that problem shown by `git difftool` was this:
     +
     +            error: unable to read sha1 file of <path> (<empty-tree-OID>)
     +            error: could not write '<filename>'
      
          Make sure that the reported `difftool` problem stays fixed.
      

-- 
gitgitgadget

  parent reply	other threads:[~2020-07-01 21:19 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
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       ` Johannes Schindelin via GitGitGadget [this message]
2020-07-01 21:19         ` [PATCH v5 1/2] diff-files --raw: show correct post-image of intent-to-add files 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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.654.v5.git.1593638347.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.