Re: [PATCH] diff: add support for reading files literally with --no-index
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Jan 5, 2019, 17:39 UTC
- Message-ID
- <20190105173944.GH423984@genre.crustytoothpaste.net>
- In-Reply-To
- <xmqq36q8cjgf.fsf@gitster-ct.c.googlers.com>
On Fri, Jan 04, 2019 at 11:26:56AM -0800, Junio C Hamano wrote:
Show 20 quoted lines
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
> >> - --dereference to control whether to follow symlinks
> >
> > This is actually surprisingly difficult. The reason I implemented this
> > only for no-index mode is because there are actually several places we
> > can stat a file in the diff code, and implementing a --dereference
> > option that catches all of those cases and getting the option passed
> > down to them is non-trivial.
>
> Another thing to worry about is symlinks that point outside the
> working tree. When a tracked content "dir/link" is a symlink to
> "/etc/motd", it probably makes sense to open("/etc/motd") and read()
> it on the working tree side of the diff, and probably even on the
> index side of the diff, but what about obtaining contents for
> "dir/link" in a year-old commit under --deference mode? I am not
> sure if it makes sense to read from the filesystem in such a case.
>
> I personally am perfectly fine if this "do not compare readlink(2),
> but read contents literally" is limited to the --no-index mode.That's a good point. I think I'll stick with the current design, then, since that seems like the least surprising way forward. It also means that we don't read outside of the working tree unless --no-index is in use, which may be beneficial for security purposes.
Thanks for a helpful perspective.
-- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204