Re: [PATCH] difftool: handle changing symlinks in dir-diff mode

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 14, 2017, 05:52 UTC
Message-ID: <xmqqwpbsjthb.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170314022030.pqqgtaavttzb7rhq@gmail.com>

David Aguilar <davvid@gmail.com> writes:

Show 9 quoted lines

> Reading the code again, the point of add_left_or_right()
> is to populate the worktree (done later in the loop) with
> the stuff we read from Git.  Thus, if we changed just this
> section to call get_symlink() then we should not even try
> to checkout any symlink entries in !use_wt_file(...)
> block where checkout_entry() / create_symlink_file() are called.
> 
> Since the symlinks2 hashmap already populates the worktree
> then that code should instead simply skip symlinks.

OK, that would simplify things, certainly.

> I'll take a stab at adding a get_symlink() helper, adjust
> the code so that add_left_or_right() is populated, and
> special-case the checkout_entry() code path to simply skip
> over null SHA1s.

Did you mean s/null SHA1s/S_ISLNK()s/?

Show 9 quoted lines

> One minor thing I noticed is that I had to use "echo -n"
> for the stuff coming out of strbuf_readlink(), and
> plain "echo" for entries that come in via read_sha1_file()
> content passed to add_left_or_right().
>
> That suggests that maybe I should append a newline to the
> output from strbuf_readlink() so that it matches Git.
> Does that sound right?  Does Git store symlink entries
> with a terminating newline?

Do not append a newline. Unless the pathname of the target you are symlinking to ends with LF, readlink() won't end with LF, and the stand-in file shouldn't, either.

By the way, avoid "echo -n"; use "printf '%s'" instead.

Thanks.