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

Re: [PATCH 2/3] t7800: fix tests when difftool uses --no-symlinks

From
David Aguilar <davvid@gmail.com>
Date
Mar 23, 2013, 03:24 UTC
Message-ID
<CAJDDKr4658zFCZ8JpXUZUrS4URTb7zX0xGR4AeODDLOVjkzDGA@mail.gmail.com>
In-Reply-To
<20130322230516.GK2283@serenity.lan>
On Fri, Mar 22, 2013 at 4:05 PM, John Keeping <john@keeping.me.uk> wrote:
Show 72 quoted lines
> On Fri, Mar 22, 2013 at 03:53:38PM -0700, Junio C Hamano wrote:
>> John Keeping <john@keeping.me.uk> writes:
>>
>> > When 'git difftool --dir-diff' is using --no-symlinks (either explicitly
>> > or implicitly because it's running on Windows), any working tree files
>> > that have been copied to the temporary directory are copied back after
>> > the difftool completes.  This includes untracked files in the working
>> > tree.
>>
>> Hmph.  Why do we populate the temporary directory with a copy of an
>> untracked path in the first place?  I thought the point of dir-diff
>> was to materialize only the relevant paths to two temporaries and
>> compare these temporaries with a tool that knows how to compare two
>> directories?
>>
>> Even if you had path F in HEAD that you are no longer tracking in
>> the working tree, a normal
>>
>>       $ git diff HEAD
>>
>> would report the path F to have been deleted, so I would imagine
>> that the preimage side of the temporary directory should get a copy
>> of HEAD:F at path F, while the postimage side of the temporary
>> directory should not even have anything at path F, when dir-diff
>> runs, no?
>>
>> Isn't that the real reason why the test fails?  The path 'output' is
>> not being tracked at any revision or in the index that is involved
>> in the test, is it?
>
> Actually it is, which is what I missed earlier.
>
> A couple of tests before this 'setup change in subdirectory' does 'git
> add .' which is far more general than it needs.  Perhaps this is a
> better change:
>
> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> index bba8a9d..561c993 100755
> --- a/t/t7800-difftool.sh
> +++ b/t/t7800-difftool.sh
> @@ -314,7 +314,7 @@ test_expect_success PERL 'setup change in subdirectory' '
>         git commit -m "added sub/sub" &&
>         echo test >>file &&
>         echo test >>sub/sub &&
> -       git add . &&
> +       git add file sub/sub &&
>         git commit -m "modified both"
>  '
>
>
>> > During the tests, this means that the following sequence occurs:
>> >
>> > 1) the shell opens "output" to redirect the difftool output
>> > 2) difftool copies the empty "output" to the temporary directory
>> > 3) difftool runs "ls" which writes to "output"
>> > 4) difftool copies the empty "output" file back over the output of the
>> >    command
>> > 5) the output files doesn't contain the expected output, causing the
>> >    test to fail
>> >
>> > Avoid this by writing the output into .git/ which will not be copied or
>> > overwritten.
>>
>> It is a good idea to move these test output and expect test vectore
>> files to a different place to make it easier to distinguish them
>> from test input (e.g. "sub", "file", etc.) in general, but the
>> description of the original problem sounds like it is just working
>> around a bug to me.  What am I missing?
>
> I think there is a bug, as described in the paragraph below, and this
> test should be made independent of that.  In light of the above I think
> we can drop this patch and do this with that change instead.
I, too, was scratching my head when the Windows issue was first reported.

Thanks for clearing this up; fixing the blind "add ." certainly seems like the way to go.

-- 
David
Previous: John KeepingNext: John Keeping
Message 23 of 55 in “difftool: silence uninitialized variable warning”
  1. 1/4 difftool: silence uninitialized variable warningDavid Aguilar, Feb 21, 2013
  2. 2/4 t7800: update copyright noticeDavid Aguilar, Feb 21, 2013
  3. 3/4 t7800: modernize testsDavid Aguilar, Feb 21, 2013
  4. 4/4 t7800: "defaults" is no longer a builtin tool nameDavid Aguilar, Feb 21, 2013
  5. Junio C HamanoFeb 21, 2013
  6. Junio C HamanoFeb 21, 2013
  7. David AguilarFeb 21, 2013
  8. Johannes SixtMar 20, 2013
  9. David AguilarMar 20, 2013
  10. Johannes SixtMar 21, 2013
  11. Johannes SixtMar 22, 2013
  12. John KeepingMar 22, 2013
  13. Johannes SixtMar 22, 2013
  14. John KeepingMar 22, 2013
  15. 0/3 Improve difftool --dir-diff testsJohn Keeping, Mar 22, 2013
  16. 1/3 t7800: don't hide grep outputJohn Keeping, Mar 22, 2013
  17. Johannes SixtMar 22, 2013
  18. Junio C HamanoMar 22, 2013
  19. 2/3 t7800: fix tests when difftool uses --no-symlinksJohn Keeping, Mar 22, 2013
  20. Johannes SixtMar 22, 2013
  21. Junio C HamanoMar 22, 2013
  22. John KeepingMar 22, 2013
  23. David AguilarMar 23, 2013
  24. 3/3 t7800: run --dir-diff tests with and without symlinksJohn Keeping, Mar 22, 2013
  25. 3/3 t7800: run --dir-diff tests with and without symlinksJohn Keeping, Mar 22, 2013
  26. 0/3 difftool --dir-diff test improvementsJohn Keeping, Mar 23, 2013
  27. 1/3 t7800: don't hide grep outputJohn Keeping, Mar 23, 2013
  28. 2/3 t7800: fix tests when difftool uses --no-symlinksJohn Keeping, Mar 23, 2013
  29. Junio C HamanoMar 24, 2013
  30. John KeepingMar 24, 2013
  31. Matt McClureMar 24, 2013
  32. John KeepingMar 24, 2013
  33. Johannes SixtMar 25, 2013
  34. John KeepingMar 25, 2013
  35. difftool: don't overwrite modified filesJohn Keeping, Mar 25, 2013
  36. Johannes SixtMar 26, 2013
  37. Johannes SixtMar 26, 2013
  38. John KeepingMar 26, 2013
  39. Johannes SixtMar 26, 2013
  40. John KeepingMar 26, 2013
  41. Matt McClureMar 26, 2013
  42. John KeepingMar 26, 2013
  43. Junio C HamanoMar 25, 2013
  44. David AguilarMar 24, 2013
  45. John KeepingMar 25, 2013
  46. Junio C HamanoMar 25, 2013
  47. Matt McClureMar 24, 2013
  48. Eric SunshineMar 24, 2013
  49. 3/3 t7800: run --dir-diff tests with and without symlinksJohn Keeping, Mar 23, 2013
  50. Johannes SixtMar 25, 2013
  51. John KeepingMar 25, 2013
  52. Johannes SixtMar 25, 2013
  53. John KeepingMar 25, 2013
  54. Junio C HamanoMar 25, 2013
  55. John KeepingMar 26, 2013

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.