All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Duy Nguyen <pclouds@gmail.com>
Cc: Michael J Gruber <git@drmicha.warpmail.net>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH v2] diff-lib.c: adjust position of i-t-a entries in diff
Date: Tue, 24 Mar 2015 10:00:43 -0700	[thread overview]
Message-ID: <xmqqtwxa72kk.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CACsJy8BczCNxm3WHK1gtkXiZCbDkFD-67oOMR+eK8uwOBfQXuw@mail.gmail.com> (Duy Nguyen's message of "Tue, 24 Mar 2015 08:15:50 +0700")

Duy Nguyen <pclouds@gmail.com> writes:

> "read-tree -m" does not invoke diff, does it? If I went with my
> previous approach (modifying unpack-trees to ignore i-t-a entries)
> then this could be a problem, but because unpack-trees is untouched,
> merge operations should not be impacted by this patch.

Theoretically yes, but not quite.

I wouldn't be surprised if an enterprising soul saw an optimization
opportunity in the "read-tree -m A B" codepath.  When it finds that
a tree in A and a valid cache-tree entry that corresponds to the
tree matches, it could blow away all index entries covered by the
cache-tree entry and replace them with B, either

 (1) unconditionally when "-u" is not given; or

 (2) as long as the working tree matches the index inside that
     directory when running with "-u".

And such an optimization used to be a valid thing to do in the old
world; but (1) will break in the new world, if we drop that
invalidation---the i-t-a entries will be discarded from the index.

As i-t-a is not a norm but an abberration, I'd rather keep the
pessimizing invalidation to keep the door open for such an
optimization for a more common case, and there may be other cases
in which our correctness around i-t-a depends on.

  reply	other threads:[~2015-03-24 17:51 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-09 14:14 [PATCH/RFC 0/2] Bug fixes regarding diff and "git add -N" Nguyễn Thái Ngọc Duy
2015-03-09 14:14 ` [PATCH 1/2] diff --cached: do not report i-t-a entries as "new" Nguyễn Thái Ngọc Duy
2015-03-15  6:55   ` Junio C Hamano
2015-03-16 13:56   ` [PATCH v2] diff-lib.c: adjust position of i-t-a entries in diff Nguyễn Thái Ngọc Duy
2015-03-16 15:15     ` Michael J Gruber
2015-03-16 16:05       ` Junio C Hamano
2015-03-17 14:07         ` Duy Nguyen
2015-03-17 17:57           ` Junio C Hamano
2015-03-18 12:47             ` Duy Nguyen
2015-03-18 20:30               ` Junio C Hamano
2015-03-19  6:00                 ` Junio C Hamano
2015-03-24  1:15                   ` Duy Nguyen
2015-03-24 17:00                     ` Junio C Hamano [this message]
2015-03-23 20:52                 ` Junio C Hamano
2015-03-09 14:14 ` [PATCH 2/2] diff-files: mark i-t-a paths as "new" Nguyễn Thái Ngọc Duy
2015-03-15  7:05   ` Junio C Hamano
2015-03-09 15:45 ` [PATCH] t2203,t4011: adjust to changed intent-to-add treatment Michael J Gruber
2015-03-15  7:07   ` Junio C Hamano

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=xmqqtwxa72kk.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=pclouds@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.