isSymbolicLink conflicts with System.Posix.Files

[joeyh]

Anything that imports System.Posix.Files and System.Directory and uses isSymbolicLink will start failing to build when updated to directory-1.2.6.2. Seems likely that most existing haskell code that uses System.Posix.Files.isSymbolicLink also uses System.Directory

Maybe call the one in System.Directory isSymLink instead?

[Rufflewind]

I didn't foresee this problem when I first added isSymbolicLink. Sadly, at this point changing the name would introduce a backward-incompatible change, causing even more problems, so I don't think there's anything that can be done now.

[joeyh]

On the one hand you have breakage in anything that has started using directory-1.2.6.2's isSymbolicLink in the past month or so.

On the other hand, you have breakage affecting an unknown set of software, anything that imports System.Directory unqualified plus System.Posix.Files and uses isSymbolicLink.

The first set seems much easier to deal with; it would need a PVP version bump to reflect the renamed symbol.

Dealing with the second has, for me as a user of your library, required 2 hours of my time and counting, the addition of shim modules to two different projects, and scouring the documentation for ways to not make ghc -Wall complain about import System.Directory hiding (isSymbolicLink) when it's building with a version of the module that does not export the symbol. (There seems to be no way to turn off that single warning; I had to use OPTIONS_GHC -w)

(I suppose I could start importing only specific symbols from System.Directory, but createDirectory and getCurrentDirectory etc are such core things that I'd probably spend a lot of time juggling import lists as I added and removed uses of them in my code.)

[Rufflewind]

On the other hand, you have breakage affecting an unknown set of software, anything that imports System.Directory unqualified plus System.Posix.Files and uses isSymbolicLink.

Importing symbols unqualified without explicitly naming the symbols has always been risky and is not recommended (see here and here). It's an unfortunate reality of how Haskell's module system currently works.

(There seems to be no way to turn off that single warning; I had to use OPTIONS_GHC -w)

Have you tried -fno-warn-dodgy-imports? (Trac#7167)

---

I'm really sorry for the trouble this has caused you (and other users), but the damage has already been done. Attempting to fix this now will only increase the casualties. I'll reconsider this in a future release that breaks compatibility, but that's not going to happen any time soon.

[Rufflewind]

If you wish, you can avoid the need for Utility.SystemDirectory by using a bit of CPP (or use -fno-warn-dodgy-imports), as shown in the patch below.

Alternatively you can also import System.Posix.Files as Posix and then fully qualify all uses of Posix.isSymbolicLink (there are only 6 in your project AFAICT).

diff --git a/propellor.cabal b/propellor.cabal
index e6279ae..90a468f 100644
--- a/propellor.cabal
+++ b/propellor.cabal
@@ -38,7 +38,7 @@ Description:
 Executable propellor
   Main-Is: wrapper.hs
   GHC-Options: -threaded -Wall -fno-warn-tabs -O0
-  Extensions: TypeOperators
+  Extensions: CPP, PackageImports, TypeOperators
   Hs-Source-Dirs: src
   Build-Depends:
     -- propellor needs to support the ghc shipped in Debian stable,
@@ -207,7 +207,6 @@ Library
     Utility.Process.NonConcurrent
     Utility.SafeCommand
     Utility.Scheduled
-    Utility.SystemDirectory
     Utility.Table
     Utility.ThreadScheduler
     Utility.Tmp
diff --git a/src/Propellor/Base.hs b/src/Propellor/Base.hs
index ae75589..ba69de7 100644
--- a/src/Propellor/Base.hs
+++ b/src/Propellor/Base.hs
@@ -1,4 +1,4 @@
-{-# LANGUAGE PackageImports #-}
+{-# LANGUAGE CPP, PackageImports #-}

 -- | Pulls in lots of useful modules for building and using Properties.

@@ -20,7 +20,7 @@ module Propellor.Base (
    , module Propellor.Utilities

    -- * System modules
-   , module Utility.SystemDirectory
+   , module System.Directory
    , module System.IO
    , module System.FilePath
    , module Data.Maybe
@@ -47,7 +47,10 @@ import Propellor.PropAccum
 import Propellor.Location
 import Propellor.Utilities

-import Utility.SystemDirectory
+import System.Directory
+#if MIN_VERSION_directory(1, 2, 6)
+  hiding (isSymbolicLink)
+#endif
 import System.IO
 import System.FilePath
 import Data.Maybe
diff --git a/src/Utility/Directory.hs b/src/Utility/Directory.hs
index 693e771..5a35b1f 100644
--- a/src/Utility/Directory.hs
+++ b/src/Utility/Directory.hs
@@ -10,10 +10,14 @@

 module Utility.Directory (
    module Utility.Directory,
-   module Utility.SystemDirectory
+   module System.Directory
 ) where

 import System.IO.Error
+import System.Directory
+#if MIN_VERSION_directory(1, 2, 6)
+  hiding (isSymbolicLink)
+#endif
 import Control.Monad
 import System.FilePath
 import Control.Applicative
@@ -30,7 +34,6 @@ import Utility.SafeCommand
 import Control.Monad.IfElse
 #endif

-import Utility.SystemDirectory
 import Utility.PosixFiles
 import Utility.Tmp
 import Utility.Exception
diff --git a/src/Utility/SystemDirectory.hs b/src/Utility/SystemDirectory.hs
deleted file mode 100644
index 3dd44d1..0000000
--- a/src/Utility/SystemDirectory.hs
+++ /dev/null
@@ -1,16 +0,0 @@
-{- System.Directory without its conflicting isSymbolicLink
- -
- - Copyright 2016 Joey Hess <[email protected]>
- -
- - License: BSD-2-clause
- -}
-
--- Disable warnings because only some versions of System.Directory export
--- isSymbolicLink.
-{-# OPTIONS_GHC -fno-warn-tabs -w #-}
-
-module Utility.SystemDirectory (
-   module System.Directory
-) where
-
-import System.Directory hiding (isSymbolicLink)

[Rufflewind]

Scoping this issue for 1.4.* release.