darcs

Issue 1015 wrong assumptions about System.Posix.Files.fileSize (2.0.3)

Title wrong assumptions about System.Posix.Files.fileSize (2.0.3)
Priority critical Status resolved
Milestone 2.1.x Resolved in
Superseder Nosy List darcs-devel, dmitry.kurochkin, eivuokko, ganesh, gwern, jaredj, kirby, kowey, mornfall, simonmar, thorkilnaur, wglozer
Assigned To
Topics Windows

Created on 2008-08-18.15:01:23 by kowey, last changed 2010-06-15.21:47:57 by admin.

Messages
msg5578 (view) Author: kowey Date: 2008-08-18.15:01:17
This sounds pretty bad, although it does not seem like anything particularly new
to darcs (is it?)

I'm adding Petr to this bug, because the question of types associated with the
struct stat vaguely reminds me of the work he did for
http://bugs.darcs.net/issue973 (the resolution of which was to tell people to
upgrade to GHC 6.8.3)

Also, I wonder how the unix-compat package handles this
  http://hackage.haskell.org/cgi-bin/hackage-scripts/package/unix-compat
and if it would not be prudent to start using it instead of our own custom wrapper.

Finally, Simon: please send patches in whatever form.  At the very least, it'll
give us an idea of the right way to fix this.

From Simon Marlow on darcs-users:
> System.Posix.Files has confusion about COff, which on Windows is Int32, but
> since we're using stat64() on Windows the st_size field of struct stat is an
> Int64.
>
> All over the place darcs assumes that fileSize returns a COff, which it
> doesn't on Windows.  I suppose we need a type that corresponds to COff on Unix
> and Int64 on Windows, and use that instead of FileOffset.
>
> I'm a bit ignorant of how the whole configuration system works in darcs,
> so while I could fix this up with a few #ifdefs I'm sure the patch would
> probably be rejected because it needs autoconf tests or whatever.  So I'll
> leave this for now (if someone wants to tell me the right way to fix it I might
> find time to do it later).
msg5581 (view) Author: mornfall Date: 2008-08-18.16:10:10
A quite different issue really. But:
1) Where have you seen those COff's? The only one I see is under win32
implementation (private to darcs) of Posix.Files (hardly could be called "all
over place")
2) There are a couple uses of FileOffset but nothing quite hard to fix. They only
appear in a few signatures and are only compared for equality against each other
(and any sensible filesize type would be in Eq), as far as I can recall.
msg5584 (view) Author: gwern Date: 2008-08-18.17:15:44
kowey: I believe unix-compat uses FileOffset as well. As for making more
extensive use - we could make some use, but nothing like replacing the Darcs
code entirely. unix-compat just isn't very comprehensive.

gwern@craft:17273bin/unix-compat/System/PosixCompat>pull                       
                                      [ 1:12PM]
Pulling from "http://code.haskell.org/unix-compat"... 
gwern@craft:17270bin/unix-compat/System/PosixCompat>gr FileOffset              
                                      [ 1:03PM]
Files.hsc:                              fileSize         :: FileOffset,
Files.hsc:getFileSize :: FilePath -> IO FileOffset
Files.hsc:setFileSize :: FilePath -> FileOffset -> IO ()
Files.hsc:setFdSize :: Fd -> FileOffset -> IO ()

Importantly:

Types.hs:import System.Posix.Types
msg5601 (view) Author: simonmar Date: 2008-08-19.13:07:51
> Petr Ročkai <me@mornfall.net> added the comment:
>
> A quite different issue really. But:
> 1) Where have you seen those COff's? The only one I see is under win32
> implementation (private to darcs) of Posix.Files (hardly could be called
> "all
> over place")
> 2) There are a couple uses of FileOffset but nothing quite hard to fix.
> They only
> appear in a few signatures and are only compared for equality against each
> other
> (and any sensible filesize type would be in Eq), as far as I can recall.

Ok, "all over the place" was an exaggeration.  I'll take a look at patching it up.

Cheers,
        Simon
msg5630 (view) Author: kowey Date: 2008-08-22.01:50:41
Fixed in unstable with

Tue Aug 19 14:42:52 BST 2008  Simon Marlow <marlowsd@gmail.com>
  * Fix Windows build
  On Windows, System.Posix.Types.FileOffset is not the same as the type
  of the st_size field of the stat structure: the latter is Int64,
  whereas COff == Int32.
msg5841 (view) Author: kowey Date: 2008-08-31.16:20:37
Hi Simon,

I'm afraid this isn't completely fixed yet.  Any ideas?

src\win32\System\Posix\Files.hsc:49:37:
    Couldn't match expected type `FileOffset'
           against inferred type `System.Posix.Types.COff'
    In the fourth argument of `FileStatus', namely `size'
    In the first argument of `return', namely
        `(FileStatus tp mode mtime size)'
    In the expression: return (FileStatus tp mode mtime size)

This is with GHC 6.8.2, see
http://buildbot.darcs.net/builders/zooko%20allmydata%20virtual2%20Windows-XP%20i386/builds/11/steps/compile/logs/stdio

Ganesh suggests a solution on IRC:
Heffalump: hunk ./src/win32/System/Posix/Files.hsc 38
Heffalump: -    return (FileStatus tp mode mtime size)
Heffalump: +    return (FileStatus tp mode mtime (fromIntegral size))

but says he hasn't tested it
msg5863 (view) Author: simonmar Date: 2008-09-01.10:31:43
Yes, it looks like the large-file support for Windows came in 6.8.3, which is
slightly naughty because it was an API change, but then the change was only to
an internal API which you guys shouldn't really be using :-)

The fromIntegral should work around it, I imagine.
msg5864 (view) Author: kowey Date: 2008-09-01.10:42:42
On Mon, Sep 01, 2008 at 10:31:45 -0000, Simon Marlow wrote:
> The fromIntegral should work around it, I imagine.

Thanks! Ganesh, would you mind sending this in patch form?
msg5986 (view) Author: dmitry.kurochkin Date: 2008-09-12.08:18:03
I was able to build the latest darcs unstable on mingw. So looks like this is
fixed now.

Regards,
  Dmitry
msg6150 (view) Author: simon Date: 2008-09-28.00:15:34
This is marked as a release blocker (priority "critical"..). It sounds like this
was a build issue, so we don't need a test, and Dmitry has verified it's fixed.
I wonder if Simon agrees ? Can we close this ?
msg6152 (view) Author: kowey Date: 2008-09-28.10:23:15
Ganesh has sent his patch, so I think we can close this.  Thanks!
History
Date User Action Args
2008-08-18 15:01:23koweycreate
2008-08-18 16:10:12mornfallsetstatus: unread -> unknown
nosy: beschmi, kowey, wglozer, eivuokko, dagit, simonmar, jaredj, mornfall
messages: + msg5581
2008-08-18 17:15:47gwernsetnosy: + gwern
messages: + msg5584
2008-08-19 13:07:54simonmarsetnosy: beschmi, kowey, wglozer, eivuokko, dagit, simonmar, gwern, jaredj, mornfall
messages: + msg5601
2008-08-22 01:50:43koweysetstatus: unknown -> resolved-in-unstable
nosy: beschmi, kowey, wglozer, eivuokko, dagit, simonmar, gwern, jaredj, mornfall
messages: + msg5630
title: wrong assumptions about System.Posix.Files.fileSize -> wrong assumptions about System.Posix.Files.fileSize (2.0.3)
2008-08-31 16:20:40koweysetstatus: resolved-in-unstable -> unknown
nosy: + ganesh
messages: + msg5841
2008-09-01 10:31:45simonmarsetnosy: beschmi, kowey, wglozer, eivuokko, dagit, ganesh, simonmar, gwern, jaredj, mornfall
messages: + msg5863
2008-09-01 10:42:44koweysetnosy: beschmi, kowey, wglozer, eivuokko, dagit, ganesh, simonmar, gwern, jaredj, mornfall
messages: + msg5864
2008-09-12 08:18:05dmitry.kurochkinsetnosy: + dmitry.kurochkin
messages: + msg5986
2008-09-20 18:09:24simonsetstatus: unknown -> testing
nosy: + simon
2008-09-28 00:15:36simonsetnosy: beschmi, kowey, wglozer, eivuokko, dagit, ganesh, simonmar, simon, gwern, jaredj, dmitry.kurochkin, mornfall
messages: + msg6150
2008-09-28 10:23:17koweysetstatus: testing -> resolved-in-unstable
nosy: beschmi, kowey, wglozer, eivuokko, dagit, ganesh, simonmar, simon, gwern, jaredj, dmitry.kurochkin, mornfall
messages: + msg6152
2009-04-22 03:32:41twbsetstatus: resolved-in-unstable -> resolved
nosy: + thorkilnaur
2009-08-06 21:11:35adminsetnosy: - beschmi
2009-08-10 23:43:04adminsetnosy: - dagit
2009-08-25 17:25:00adminsetnosy: + darcs-devel, - simon
2009-08-27 14:18:46adminsetnosy: kowey, wglozer, darcs-devel, eivuokko, ganesh, simonmar, thorkilnaur, gwern, jaredj, dmitry.kurochkin, mornfall
2009-10-23 22:37:22adminsetnosy: + marlowsd, - simonmar
2009-10-23 23:35:49adminsetnosy: + simonmar, - marlowsd
2010-06-15 21:47:55adminsetmilestone: 2.1.x
2010-06-15 21:47:57adminsettopic: - Target-2.1
nosy: + kirby