From: Michael Geddes <michael@frog•wheelycreek.net>
To: msysgit@googlegroups•com
Cc: Junio C Hamano <gitster@pobox•com>, Stepan Kasal <kasal@ucw•cz>,
GIT Mailing-list <git@vger•kernel.org>,
Thomas Braun <thomas.braun@virtuell-zuhause•de>
Subject: Re: Re: [PATCH v2] t5000, t5003: do not use test_cmp to compare binary files
Date: Thu, 05 Jun 2014 10:01:50 +0800 [thread overview]
Message-ID: <5268585.VtKVR75oeq@majorie> (raw)
In-Reply-To: <xmqq8upcv8jj.fsf@gitster.dls.corp.google.com>
I have the problem that the overridden test_cmp crashes on a couple of places
where it is doing a binary compare, so this is definitely needed.
I actually used cmp -q in my override as it's the return code that is most
important.
//.
On Wed, 4 Jun 2014 11:22:56 AM Junio C Hamano wrote:
> Stepan Kasal <kasal@ucw•cz> writes:
> > test_cmp() is primarily meant to compare text files (and display the
> > difference for debug purposes).
> >
> > Raw "cmp" is better suited to compare binary files (tar, zip, etc.).
> >
> > On MinGW, test_cmp is a shell function mingw_test_cmp that tries to
> > read both files into environment, stripping CR characters (introduced
> > in commit 4d715ac0).
> >
> > This function usually speeds things up, as fork is extremly slow on
> > Windows. But no wonder that this function is extremely slow and
> > sometimes even crashes when comparing large tar or zip files.
> >
> > Signed-off-by: Stepan Kasal <kasal@ucw•cz>
> > ---
> >
> > Hi Thomas,
> >
> > On Wed, Jun 04, 2014 at 02:59:44PM +0200, Thomas Braun wrote:
> >> Using test_cmp_bin instead of cmp would result in then four assertions
> >> for comparing arbitrary data
> >> test_cmp
> >> test_i18ncmp
> >> test_cmp_text
> >> test_cmp_bin
> >> where I think the purpose of each function is clear from its name.
> >
> > [test_cmp_text does not exist (yet)]
> >
> > OK, I agree, hence this modified version of the patch.
>
> Yeah, I think the above reasoning is sound. And I do not think we
> ever need to have test_cmp_text -- our payload and our messages
> compared by tests to make sure our expectations hold are text by
> default.
>
> Will queue; thanks.
--
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups•com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups•com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
---
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups•com.
For more options, visit https://groups.google.com/d/optout.
prev parent reply other threads:[~2014-06-05 2:02 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-04 11:42 [PATCH] t5000, t5003: do not use test_cmp to compare binary files Stepan Kasal
2014-06-04 12:13 ` Thomas Braun
2014-06-04 12:42 ` Stepan Kasal
2014-06-04 12:59 ` Thomas Braun
2014-06-04 15:57 ` [PATCH v2] " Stepan Kasal
2014-06-04 18:22 ` Junio C Hamano
2014-06-05 2:01 ` Michael Geddes [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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5268585.VtKVR75oeq@majorie \
--to=michael@frog$(echo .)wheelycreek.net \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=kasal@ucw$(echo .)cz \
--cc=msysgit@googlegroups$(echo .)com \
--cc=thomas.braun@virtuell-zuhause$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox