If anyone else doesn't have any other objection, how should I proceed? Should I open a ticket in ghc's Trac asking for integration of this patch? Regards, /Ricardo On Wed, Dec 11, 2013 at 10:55 PM, Ricardo Catalinas Jiménez wrote:

Shame on me! Wrong copy&paste from fsync to fdatasync... Fixed and tested on Linux. Changelog corrected and commits squashed.

Review: https://github.com/jimenezrick/unix/compare/master...file-utils /Ricardo

On Wed, Dec 11, 2013 at 4:00 PM, Thomas Schilling wrote:

...

Also, adding a function should bump the third component, not the fourth.

On 11 December 2013 16:57, Thomas Schilling wrote:

...

There's a typo in line:

+#ifndef HAVE_FDAYASYNC

I assume it should read HAVE_FDATASYNC

Did you try to compile this on a system that has fdatasync?

On 7 December 2013 20:20, Ricardo Catalinas Jiménez wrote:

...

Hi,

Please review this pull request: https://github.com/jimenezrick/unix/compare/master...file-utils.patch

If you find interesting this patch, then: git pull https://github.com/jimenezrick/unix.git file-utils

/Ricardo _______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

On 2013-12-22 at 11:32:20 +0100, Thomas Schilling wrote:

Looks like GHC HQ maintains this package. I don't think it'll make it into the version released with 7.8, though.

fwiw, I've been wanting to review that submission but didn't get to it yet;

I do have another question, though. How should Haskell code using the `unix` library check for whether `fsync` are implemented or not?

there's a HsUnixConfig.h include file generated (and installed) you can include from your Haskell code, and use CPP

If I call `fsync` I wouldn't want to get a runtime `error` call if the platform doesn't support it.

To be fair, `unix` does not seem consistent with respect to handling potentially missing lib/syscalls; I've seen the following three cases: a) fallback to implement semantics via different calls (e.g. `System.Posix.Env.unsetEnv`), or b) the implementation returns bottom via an `error`-value (e.g. `System.Posix.Temp.mkstemps`), or c) the symbol is conditionally exported (e.g. `System.Posix.Directory.tellDirStream`)

Either I should be able to check at compile time, or I should get a defined exception call which then must be handled by the library. In either case it should be documented with the function.

It might be worth adding more information to the Haddock-comments mentioning the respective `HAVE_*` CPP symbol and whether a fallback-implementation is used. Moreover, the use of `error` to signal non-existing implementations could be reconsidered (e.g. maybe switch to a properly thrown "call-not-implemented" exception)

On Sun, Dec 22, 2013 at 8:02 PM, John Lato wrote:

On Dec 22, 2013 7:28 AM, "Herbert Valerio Riedel" wrote:

...

On 2013-12-22 at 11:32:20 +0100, Thomas Schilling wrote:

...

Looks like GHC HQ maintains this package. I don't think it'll make it into the version released with 7.8, though.

fwiw, I've been wanting to review that submission but didn't get to it yet;

...

I do have another question, though. How should Haskell code using the `unix` library check for whether `fsync` are implemented or not?

there's a HsUnixConfig.h include file generated (and installed) you can include from your Haskell code, and use CPP

...

If I call `fsync` I wouldn't want to get a runtime `error` call if the platform doesn't support it.

To be fair, `unix` does not seem consistent with respect to handling potentially missing lib/syscalls; I've seen the following three cases:

a) fallback to implement semantics via different calls (e.g. `System.Posix.Env.unsetEnv`), or

b) the implementation returns bottom via an `error`-value (e.g. `System.Posix.Temp.mkstemps`), or

c) the symbol is conditionally exported (e.g. `System.Posix.Directory.tellDirStream`)

...

Either I should be able to check at compile time, or I should get a defined exception call which then must be handled by the library. In either case it should be documented with the function.

It might be worth adding more information to the Haddock-comments mentioning the respective `HAVE_*` CPP symbol and whether a fallback-implementation is used.

Moreover, the use of `error` to signal non-existing implementations could be reconsidered (e.g. maybe switch to a properly thrown "call-not-implemented" exception)

This would definitely be better than calling error. Personally I would prefer conditional exports, as it would turn up errors sooner.

John L.

OK, sounds good. Now everything is conditionally exported: https://github.com/jimenezrick/unix/compare/master...file-utils.patch Yeah, the unix library is messy as it is having to much preprocessor infection and different ways of handling this situations.

On 2013-12-28 at 01:11:40 +0100, Ben Millwood wrote:

On Sun, Dec 22, 2013 at 12:02:16PM -0800, John Lato wrote:

...

On Dec 22, 2013 7:28 AM, "Herbert Valerio Riedel" wrote:

...

On 2013-12-22 at 11:32:20 +0100, Thomas Schilling wrote:

...

Either I should be able to check at compile time, or I should get a defined exception call which then must be handled by the library. In either case it should be documented with the function.

It might be worth adding more information to the Haddock-comments mentioning the respective `HAVE_*` CPP symbol and whether a fallback-implementation is used.

Moreover, the use of `error` to signal non-existing implementations could be reconsidered (e.g. maybe switch to a properly thrown "call-not-implemented" exception)

This would definitely be better than calling error. Personally I would prefer conditional exports, as it would turn up errors sooner.

John L.

What does client code look like, if this is your implementation strategy? Note that the network package recently changed to *stop* conditionally exporting things, because it led to bad error messages (yes, your error is caught at compile time, but it's not so clear what it *is*) and ugly client code. See e.g. https://github.com/haskell/network/issues/40

btw, as a compromise, one could export the symbol always (even if its body is exception-throwing stub) and at the same time enable a '{-# WARNING #-}' for that symbol for the not-implemented; that way one gets both, a stable API as well as compile-time warnings...

On Fri, Dec 27, 2013 at 4:11 PM, Ben Millwood wrote:

On Sun, Dec 22, 2013 at 12:02:16PM -0800, John Lato wrote:

...

On Dec 22, 2013 7:28 AM, "Herbert Valerio Riedel" wrote:

...

On 2013-12-22 at 11:32:20 +0100, Thomas Schilling wrote:

...

Either I should be able to check at compile time, or I should get a defined exception call which then must be handled by the library. In either case it should be documented with the function.

It might be worth adding more information to the Haddock-comments mentioning the respective `HAVE_*` CPP symbol and whether a fallback-implementation is used.

Moreover, the use of `error` to signal non-existing implementations could be reconsidered (e.g. maybe switch to a properly thrown "call-not-implemented" exception)

This would definitely be better than calling error. Personally I would prefer conditional exports, as it would turn up errors sooner.

John L.

What does client code look like, if this is your implementation strategy? Note that the network package recently changed to *stop* conditionally exporting things, because it led to bad error messages (yes, your error is caught at compile time, but it's not so clear what it *is*) and ugly client code. See e.g. https://github.com/haskell/network/issues/40

These days I pretty much exclusively work on linux systems, so my client code is pretty clean :) I would consider that example from the network package a different situation. Conditionally-defined data types are not the same as a function that might not be exported. If you're serializing/logging, it makes perfect sense to enumerate values that aren't supported locally for example, and I can see that working around that could lead to really ugly client code. But I don't think it makes much sense to export a function that just calls error if it's unsupported. Handling an exception is at least as awkward as CPP in that case, and in practice worse because we can't differentiate between error calls except by matching on the message string. At least a custom exception would solve that problem. But for me, conditional exports are still better. A big advantage for us is that when we update our toolchain (e.g. a new ghc release comes out), it's much faster to find problems in the toolchain build by just typechecking code rather than actually needing to run executables to test every possibly-defined function we might call (of course we do end-to-end testing as well, but this is a case where errors arising sooner leads to significantly less work). As an alternative, how about throwing an exception, but also using CPP to add a WARNING pragma to any function that's system-unavailable? Then we'd get a good message at compile-time. John L.

2013/12/27 Ricardo Catalinas Jiménez :

OK, all the new functions are conditionally exported. Branch and ticket updated.

Hmmm, I don't really like conditional exports in libraries, because you have no clue in advance if things will compile or not. How is one supposed to check for this? Do we have a 'hautoconf'' ? :-} I know that there are already conditional exports, but this doesn't make it better.