git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 3/3] difftool: handle modified symlinks in dir-diff mode

From
David Aguilar <davvid@gmail.com>
Date
Mar 15, 2017, 20:58 UTC
Message-ID
<20170315205735.5c6i4uivzj77bfsb@gmail.com>
In-Reply-To
<xmqq8to6fk15.fsf@gitster.mtv.corp.google.com>
On Wed, Mar 15, 2017 at 11:54:14AM -0700, Junio C Hamano wrote:
Show 33 quoted lines
> David Aguilar <davvid@gmail.com> writes:
> 
> > @@ -397,7 +438,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
> >  				return error("could not write '%s'", src_path);
> >  		}
> >  
> > -		if (rmode) {
> > +		if (rmode && !S_ISLNK(rmode)) {
> >  			struct working_tree_entry *entry;
> 
> It still makes me wonder why the new !S_ISLNK() is done only for the
> right hand side, though.  In fact, the processing done for the left
> hand side and the right hand side are vastly different even without
> this patch.
> 
> I suspect this is probably because the code is not prepared to drive
> the underlying "diff" when given the "-R" option (if I am reading
> the code correctly, argv[] that came from the end-user is appended
> to "diff --raw --no-abbrev -z", so the user could ask "difftool
> --dir-diff -R ..."), in which case you would see the working tree
> files as the left hand side of the diff.  In the dir-diff mode,
> because you want to make only the working-tree side writable (and
> reflect whatever edit the user made back to the working-tree side),
> the choices you have to fix it would either be forbid "-R" (which is
> less preferrable as it is a more brittle solution between the two)
> or read the "diff --raw" output and swap the sides when you notice
> that LHS has 0{40} with non 0 mode, which is a sign that that side
> represents the working tree.
> 
> Having said all that, let's focus on the "symlink" stuff in this
> series.
> 
> Thanks.

Agreed. I can take a look at the reverse-diff question later this week separately from this issue.

The symlink test case I just added can be used as a starting ground for adding reverse-diff tests.

Thanks Junio and Dscho for the reviews,
-- 
David
Previous: Junio C Hamano
Message 3 of 3 in “difftool: handle modified symlinks in dir-diff mode”
  1. 3/3 difftool: handle modified symlinks in dir-diff modeDavid Aguilar, Mar 15, 2017
  2. Junio C HamanoMar 15, 2017
  3. David AguilarMar 15, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.