darcs

Issue 913 cannot apply a binary patch (Regression, Windows, 2.0.0 to 2.0.1rc1+)

Title cannot apply a binary patch (Regression, Windows, 2.0.0 to 2.0.1rc1+)
Priority urgent Status resolved
Milestone Resolved in
Superseder Nosy List darcs-devel, dmitry.kurochkin, eivuokko, gwern, jaredj, kowey, satnams, simonmar, simonpj, thorkilnaur, tommy, wglozer
Assigned To
Topics Regression, Windows

Created on 2008-06-05.15:35:20 by marlowsd, last changed 2009-10-23.23:35:02 by admin.

Files
File name Uploaded Type Edit Remove
MINIMAL-addrm-binary-fail.dpatch kowey, 2008-06-12.07:34:27 text/x-darcs-patch
addrm-binary-fail.dpatch.bz2 kowey, 2008-06-11.13:42:37 application/octet-stream
Messages
msg4980 (view) Author: simonmar Date: 2008-06-05.15:35:15
With darcs2 on Windows:

$ darcs2 get http://darcs.haskell.org/packages/ndp
Unapplicable patch:
Tue Mar 14 22:43:29 GMT Standard Time 2006  Manuel M T Chakravarty 
<chak@cse.unsw.edu.au>
   * Using GADTs as closed associated types

darcs failed:  binary patch to 
./Data/Array/Parallel/Monadic/test/unit/TestBUArr couldn't apply.

darcs1 can get this repository successfully. This is 100% reproducible.

I'll bet this a binary/text file issue, and I'll also bet it's the same bug 
that causes the unapplicable patches to RnPat.lhs/RnExpr.lhs in issue842. 
Maybe this smaller test case will help track it down?

Cheers,
	Simon
msg4988 (view) Author: kowey Date: 2008-06-06.08:07:43
Thanks for the smaller example!  Assigning to myself to reproduce.

I can get that repository successfully under Linux, for what it's worth, but as
you point out, this is a Windows issue.
msg5005 (view) Author: kowey Date: 2008-06-11.13:42:37
I can indeed reproduce this on Windows.
  
I've also done some sanity checks, like making sure the original binary is the
same across Windows/Linux.

For the interested, I've boiled this down to a patch bundle that adds and
removes a binary file from an empty repository.  Note that if I try to simplify
the patch further, it applies cleanly, so I just left it at the original
TestBUArr.  To test:

# works fine on Linux, not on Windows
mkdir foo; cd foo
bunzip2 addrm-binary-fail.dpatch.bz2
darcs init
darcs apply addrm-binary-fail.dpatch

David?  I'll be happy to help with any other testing.
Attachments
msg5008 (view) Author: kowey Date: 2008-06-12.07:07:36
Simon: configuring with --disable-bytestring solves this problem (we had
re-enabled it by default because Gwern had fixed a bug... guess there is another)

Gwern: may I assign this to you (since this is bytestring-related)?  An
interesting clue is that it only breaks on Windows.  It may be worthwhile to
bring Don in on this too

David: if nobody can look into this, I guess we'll have to disable bytestring by
defaul again (grumble)
msg5009 (view) Author: kowey Date: 2008-06-12.07:34:27
Attached is a minimal example that works either on Linux (presumably) or without
bytestring, but not on Windows-with-bytestring.  We should find a good way to
add it to the suite
Attachments
msg5027 (view) Author: igloo Date: 2008-06-12.12:27:32
I haven't investigated the actual problem, but looking at FastPackedString.hs,
it says

writeFilePS :: FilePath -> PackedString -> IO ()
writeFilePS = B.writeFile

where B is Data.ByteString.Char8, which defines

writeFile :: FilePath -> ByteString -> IO ()
writeFile f txt = bracket (openFile f WriteMode) hClose
    (\h -> hPut h txt)

I assume you want Data.ByteString's

writeFile :: FilePath -> ByteString -> IO ()
writeFile f txt = bracket (openBinaryFile f WriteMode) hClose
    (\h -> hPut h txt)

(and in general want to be using Data.ByteString for everything).
msg5045 (view) Author: gwern Date: 2008-06-14.02:49:35
So, can we close this bug? It would seem Eric has tried Igloo's suggestion.
Windows users, does it work now?

On a side note, why didn't the tests catch this?

---

Thu Jun 12 08:57:19 EDT 2008  Eric Kow <E.Y.Kow@brighton.ac.uk>
  * Resolve issue913: Use Data.Bytestring for IO, not Data.Bytestring.Char8
  
  This solution comes from Ian Lynagh, who points out that the
  difference between the two is:
  
  Data.Bytestring.Char8
  ---------------------
  writeFile :: FilePath -> ByteString -> IO ()
  writeFile f txt = bracket (openFile f WriteMode) hClose
     (\h -> hPut h txt)
  
  Data.Bytestring
  ---------------
  writeFile :: FilePath -> ByteString -> IO ()
  writeFile f txt = bracket (openBinaryFile f WriteMode) hClose
     (\h -> hPut h txt)
  
  We want this change because, as the System.IO docs say:
  
    On Windows, reading a file in text mode (which is the default) will
    translate CRLF to LF, and writing will translate LF to CRLF. This is
    usually what you want with text files. With binary files this is
    undesirable; also, as usual under Microsoft operating systems, text
    mode treats control-Z as EOF. Binary mode turns off all special
    treatment of end-of-line and end-of-file characters.
msg5046 (view) Author: kowey Date: 2008-06-14.05:37:26
Hmm! It got pulled it, but for some reason the auto-resolver did not notice :-(

If you could submit a test for this [the binary patch issue], it would be great.
 I guess you would have to find a good way to create a binary file that has the
kind of sequence (presumably the bytes that happen to correspond to CRLF) that
make it fail
History
Date User Action Args
2008-06-05 15:35:20marlowsdcreate
2008-06-05 18:31:58koweylinkissue914 superseder
2008-06-06 08:07:48koweysetstatus: unread -> unknown
topic: + Windows
nosy: + wglozer, eivuokko, kowey, jaredj
messages: + msg4988
priority: bug
assignedto: kowey
2008-06-11 13:42:40koweysetfiles: + addrm-binary-fail.dpatch.bz2
nosy: + droundy
messages: + msg5005
assignedto: kowey -> droundy
2008-06-11 13:44:47koweysetpriority: bug -> urgent
nosy: droundy, tommy, beschmi, kowey, wglozer, eivuokko, dagit, igloo, simonpj, jaredj, marlowsd, satnams
topic: + Regression
title: darcs2/Windows regression: cannot get ndp repository -> cannot apply a binary patch (Regression, Windows, 2.0.0 to 2.0.1rc1+)
2008-06-12 07:07:38koweysetnosy: + gwern
messages: + msg5008
assignedto: droundy -> gwern
2008-06-12 07:34:29koweysetfiles: + MINIMAL-addrm-binary-fail.dpatch
nosy: droundy, tommy, beschmi, kowey, wglozer, eivuokko, dagit, igloo, simonpj, gwern, jaredj, marlowsd, satnams
messages: + msg5009
2008-06-12 12:27:33igloosetnosy: droundy, tommy, beschmi, kowey, wglozer, eivuokko, dagit, igloo, simonpj, gwern, jaredj, marlowsd, satnams
messages: + msg5027
2008-06-14 02:49:37gwernsetnosy: droundy, tommy, beschmi, kowey, wglozer, eivuokko, dagit, igloo, simonpj, gwern, jaredj, marlowsd, satnams
messages: + msg5045
2008-06-14 05:37:28koweysetstatus: unknown -> resolved-in-unstable
nosy: droundy, tommy, beschmi, kowey, wglozer, eivuokko, dagit, igloo, simonpj, gwern, jaredj, marlowsd, satnams
messages: + msg5046
assignedto: gwern ->
2008-09-04 21:33:34adminsetstatus: resolved-in-unstable -> resolved
nosy: droundy, tommy, beschmi, kowey, wglozer, eivuokko, dagit, igloo, simonpj, gwern, jaredj, marlowsd, satnams
2009-08-06 17:58:58adminsetnosy: + markstos, jast, Serware, dmitry.kurochkin, darcs-devel, zooko, mornfall, simon, thorkilnaur, - droundy, wglozer, eivuokko, igloo, simonpj, gwern, jaredj, marlowsd, satnams
2009-08-06 21:07:38adminsetnosy: - beschmi
2009-08-10 22:21:15adminsetnosy: + wglozer, marlowsd, satnams, eivuokko, igloo, gwern, simonpj, jaredj, - markstos, darcs-devel, zooko, jast, Serware, mornfall
2009-08-11 00:16:52adminsetnosy: - dagit
2009-08-25 17:19:34adminsetnosy: + darcs-devel, - igloo
2009-08-25 18:08:58adminsetnosy: - simon
2009-08-27 14:10:06adminsetnosy: tommy, kowey, wglozer, darcs-devel, eivuokko, simonpj, thorkilnaur, gwern, jaredj, dmitry.kurochkin, marlowsd, satnams
2009-10-23 23:35:02adminsetnosy: + simonmar, - marlowsd