t7800: remove whitespace before redirect
PATCH 1/3 t7800: remove whitespace before redirect
---
Cleanup before the fix.
t/t7800-difftool.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
t/t7800-difftool.sh +1 -1
@@ -428,7 +428,7 @@ run_dir_diff_test 'difftool --dir-diff branch from subdirectory' ' git difftool --dir-diff $symlinks --extcmd ls branch >output && # "sub" must only exist in "right" # "file" and "file2" must be listed in both "left" and "right" - grep sub output > sub-output && + grep sub output >sub-output && test_line_count = 1 sub-output && grep file"$" output >file-output && test_line_count = 2 file-output &&
Signature · 1 line
2.12.0.309.gffef9e61c2
PATCH 2/3 t7800: cleanup cruft left behind by tests
---
More cleanup, this is needed because the final patch adds a test to t7800 that was sensitive to the cruft left behind.
t/t7800-difftool.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
t/t7800-difftool.sh +4 -1
@@ -591,6 +591,7 @@ test_expect_success 'difftool --no-symlinks detects conflict ' ' ' test_expect_success 'difftool properly honors gitlink and core.worktree' ' + test_when_finished rm -rf submod/ule && git submodule add ./. submod/ule && test_config -C submod/ule diff.tool checktrees && test_config -C submod/ule difftool.checktrees.cmd '\''
@@ -600,11 +601,13 @@ test_expect_success 'difftool properly honors gitlink and core.worktree' ' cd submod/ule && echo good >expect && git difftool --tool=checktrees --dir-diff HEAD~ >actual && - test_cmp expect actual + test_cmp expect actual && + rm -f expect actual ) ' test_expect_success SYMLINKS 'difftool --dir-diff symlinked directories' ' + test_when_finished git reset --hard && git init dirlinks && ( cd dirlinks &&
Signature · 1 line
2.12.0.309.gffef9e61c2
PATCH 3/3 difftool: handle modified symlinks in dir-diff mode
In reply to David AguilarDetect the null object ID for symlinks in dir-diff so that difftool can detect when symlinks are modified in the worktree.
Previously, a null symlink object ID would crash difftool. Handle null object IDs as unknown content that must be read from the worktree.
---
This was reworked a bit since the original patch. The subject line changed, a lot of comments were added, and the tests were made more extensive.
This implementation is simpler since we now use a get_symlink() function in one place and simply skip the problematic code path that does not apply to symlinks. Existing code handles writing the symlink content so write_symlink_file() from the original patch is gone.
builtin/difftool.c | 53 +++++++++++++++++++++++++++++++++++++++++----- t/t7800-difftool.sh | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 5 deletions(-)
builtin/difftool.c +48 -5
@@ -254,6 +254,49 @@ static int ensure_leading_directories(char *path) } } +/* +* Unconditional writing of a plain regular file is what +* "git difftool --dir-diff" wants to do for symlinks. We are preparing two +* temporary directories to be fed to a Git-unaware tool that knows how to +* show a diff of two directories (e.g. "diff -r A B"). +* +* Because the tool is Git-unaware, if a symbolic link appears in either of +* these temporary directories, it will try to dereference and show the +* difference of the target of the symbolic link, which is not what we want, +* as the goal of the dir-diff mode is to produce an output that is logically +* equivalent to what "git diff" produces. +* +* Most importantly, we want to get textual comparison of the result of the +* readlink(2). get_symlink() provides that---it returns the contents of +* the symlink that gets written to a regular file to force the external tool +* to compare the readlink(2) result as text, even on a filesystem that is +* capable of doing a symbolic link. +*/ +static char *get_symlink (const struct object_id *oid,const char *path) +{ +char *data; +if (is_null_oid (oid)) { +/* The symlink is unknown to Git so read from the filesystem */ +struct strbuf link =STRBUF_INIT ; +if (has_symlinks) { +if (strbuf_readlink (&link, path,strlen (path))) +die (_ ("could not read symlink %s" ), path); + }else if (strbuf_read_file (&link, path,128 )) +die (_ ("could not read symlink file %s" ), path); + + data =strbuf_detach (&link,NULL ); + }else { +enum object_type type; +unsigned long size; + data =read_sha1_file (oid->hash, &type, &size); +if (!data) +die (_ ("could not read object %s for symlink %s" ), +oid_to_hex (oid), path); + } + +return data; +} +static int run_dir_diff (const char *extcmd,int symlinks,const char *prefix,int argc,const char **argv) {
@@ -270,8 +313,6 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,struct hashmap working_tree_dups, submodules, symlinks2;struct hashmap_iter iter;struct pair_entry *entry; -enum object_type type; -unsigned long size;struct index_state wtindex;struct checkout lstate, rstate;int rc, flags =RUN_GIT_CMD , err =0 ;
@@ -377,13 +418,13 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, }if (S_ISLNK (lmode)) { -char *content =read_sha1_file (loid.hash, &type, &size); +char *content =get_symlink (&loid, src_path);add_left_or_right (&symlinks2, src_path, content,0 );free (content); }if (S_ISLNK (rmode)) { -char *content =read_sha1_file (roid.hash, &type, &size); +char *content =get_symlink (&roid, dst_path);add_left_or_right (&symlinks2, dst_path, content,1 );free (content); }
@@ -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;/* Avoid duplicate working_tree entries */
@@ -414,6 +455,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,oidcpy (&ce->oid, &roid);strcpy (ce->name, dst_path); ce->ce_namelen = dst_path_len; +if (checkout_entry (ce, &rstate,NULL ))return error ("could not write '%s'" , dst_path);
@@ -487,6 +529,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,* shows only the link itself, not the contents of the link target. * This loop replicates that behavior. */ +hashmap_iter_init (&symlinks2, &iter);while ((entry =hashmap_iter_next (&iter))) {if (*entry->left) {
t/t7800-difftool.sh +60 -0
64 lines not on this page · raw message
Signature · 1 line
2.12.0.309.gffef9e61c2
Re: [PATCH 3/3] difftool: handle modified symlinks in dir-diff mode
Hi David,
Quoted 14 lines · David Aguilar
Detect the null object ID for symlinks in dir-diff so that difftool can detect when symlinks are modified in the worktree.
Previously, a null symlink object ID would crash difftool. Handle null object IDs as unknown content that must be read from the worktree.
Helped-by Johannes Schindelin <[email protected]>Signed-off-by David Aguilar <[email protected]>---
This was reworked a bit since the original patch. The subject line changed, a lot of comments were added, and the tests were made more extensive.
Quoted 5 lines
This implementation is simpler since we now use a
get_symlink()function in one place and simply skip the problematic code path that does not apply to symlinks. Existing code handles writing the symlink content sowrite_symlink_file()from the original patch is gone.
I really like this version. It is very easy to follow, the extensive comments make a lot of sense.
Well done!
Dscho