Re: [PATCH v2 3/7] Fix tests to work with core.autocrlf=true -- new functions

From: Don Slutz <don.slutz@sierraatlantic.com>
Date: May 14, 2009, 14:39 UTC
Message-ID: <4A0C2D2D.9080707@SierraAtlantic.com>
In-Reply-To: <20090514074303.GA8713@hashpling.org>

On 5/14/2009 3:43 AM, Charles Bailey wrote,

Show 8 quoted lines

> On Wed, May 13, 2009 at 03:35:44PM -0400, Don Slutz wrote:
>   
>> Have I missed some previous recent discussion about this patch series?
>> I know that you referenced that long Aug 2007 thread about autocrlf,
>> but is there some more recent discussion about how the test suite
>> works / should work?
>>
>>

Not that I have seen. I did forget to send the previous reply, should now be there.

Show 15 quoted lines

>> mergetool isn't the prime implementor of autocrlf, but it does have
>> some checks to make sure that it works with autocrlf. My impression -
>> probably incorrect - has been that autocrlf is off for the purposes of
>> building and testing git on all platforms, but that some packages
>> switch it on by default on install for user convenience on platforms
>> where this is appropriate.
>>
>> Your patch seems to be about allowing the entire test suite to run
>> correctly with the autocrlf in any setting. If this is the case,
>> shouldn't the correct fix be to remove tests that are testing that
>> things work with different settings of autocrlf, because these tests
>> are effectively run by a full test suite run with autocrlf
>> alternatively set anyway?
>>
>>

Well my take is that most of the tests do not care about autocrlf, they are checking that the right file or error is happening. For example:

   git commit -m "c1"
  echo foo >file2
  git checkout -- file2

just wants to know that file2 is correctly reverted to before the change. The fact that file2 can have LF -> CRLF if autocrlf=true is not what is being checked for here. There are several tests like t0020-crlf.sh that are checking that the right thing happens.

I am assuming that t7610-mergetool.sh add the test for autocrlf=true because of some issue (bug?) that was fixed. However I have not done a full look into way the test is there. I was focused on getting the tests to pass.

Also I do expect that most people will run the tests in the default mode only. I have no plans on add the "run the tests in all possible settings of autocrlf".

This set seems like a subset (but still big) change on the path of getting git to work and pass the tests under CYGWIN on a text mount. That change is still in progress and not yet ready for the list.

   -Don

__________________________________________________________________________________________________________________ DISCLAIMER:"The information contained in this message and the attachments (if any) may be privileged and confidential and protected from disclosure. You are hereby notified that any unauthorized use, dissemination, distribution or copying of this communication, review, retransmission, or taking of any action based upon this information, by persons or entities other than the intended recipient, is strictly prohibited. If you are not the intended recipient or an employee or agent responsible for delivering this message, and have received this communication in error, please notify us immediately by replying to the message and kindly delete the original message, attachments, if any, and all its copies from your computer system. Thank you for your cooperation." ________________________________________________________________________________________________________________