git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Don Slutz <Don.Slutz@SierraAtlantic.com>
To: Charles Bailey <charles@hashpling.org>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 3/7] Fix tests to work with core.autocrlf=true --	new functions
Date: Thu, 14 May 2009 10:39:41 -0400	[thread overview]
Message-ID: <4A0C2D2D.9080707@SierraAtlantic.com> (raw)
In-Reply-To: <20090514074303.GA8713@hashpling.org>

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

> 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.
>> 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." 
________________________________________________________________________________________________________________

      reply	other threads:[~2009-05-14 14:41 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-11 19:28 [PATCH 0/6] Add core.autocrlf=true on cygwin by default during tests Don Slutz
2009-05-11 19:28 ` [PATCH 1/6] " Don Slutz
2009-05-11 19:28   ` [PATCH 2/6] Fix tests to work with core.autocrlf=true Don Slutz
2009-05-11 19:28     ` [PATCH 3/6] " Don Slutz
2009-05-11 19:28       ` [PATCH 4/6] " Don Slutz
2009-05-11 19:29         ` [PATCH 5/6] " Don Slutz
2009-05-11 19:29           ` [PATCH 6/6] Add 'make test-text' Don Slutz
2009-05-11 22:20       ` [PATCH 3/6] Fix tests to work with core.autocrlf=true Charles Bailey
2009-05-14 13:49         ` Don Slutz
2009-05-11 20:04 ` [PATCH 0/6] Add core.autocrlf=true on cygwin by default during tests Eric Blake
2009-05-12 23:27   ` Junio C Hamano
2009-05-13 17:41     ` Junio C Hamano
2009-05-11 20:54 ` Johannes Schindelin
2009-05-12 18:16   ` Don Slutz
2009-05-13 19:35 ` [PATCH v2 0/7] Add GIT_TEST_AUTO_CRLF environment variable to set core.autocrlf on init Don Slutz
2009-05-13 19:35   ` [PATCH v2 1/7] " Don Slutz
2009-05-13 19:35     ` [PATCH v2 2/7] Add support functions for tests in core.autocrlf=true Don Slutz
2009-05-13 19:35       ` [PATCH v2 3/7] Fix tests to work with core.autocrlf=true -- new functions Don Slutz
2009-05-13 19:35         ` [PATCH v2 4/7] Fix tests to work with core.autocrlf=true -- force false Don Slutz
2009-05-13 19:35           ` [PATCH v2 5/7] Fix tests to work with core.autocrlf=true -- cmp to test_cmp Don Slutz
2009-05-13 19:35             ` [PATCH v2 6/7] Fix tests to work with core.autocrlf=true -- test_cmp to cmp Don Slutz
2009-05-13 19:35               ` [PATCH v2 7/7] Add 'make test-text' core.autocrlf=true Don Slutz
2009-05-14  7:43         ` [PATCH v2 3/7] Fix tests to work with core.autocrlf=true -- new functions Charles Bailey
2009-05-14 14:39           ` Don Slutz [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4A0C2D2D.9080707@SierraAtlantic.com \
    --to=don.slutz@sierraatlantic.com \
    --cc=charles@hashpling.org \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).