2012/9/28 Iustin Pop <[email protected]>:
> +-- | Prefixes a path with the current root directory
> +addNodePrefix :: FilePath -> FilePath
> +addNodePrefix path = do
> + let val = unsafePerformIO $ getEnv "GANETI_ROOTDIR"
> + case val of
> + Just rootdir -> rootdir ++ path
> + Nothing -> path
> +
>
> And there are a few problems with it.
>
> Generally speaking, there are two ways to use unsafePerformIO:
>
> 1) however you want, if you are a wizard and know what you're doing
> 2) by following a strict set of rules, otherwise
>
> […]
Thanks for the explanation!
> So in this case, the proper way to write this would be:
>
> {-# NOINLINE getRootDir #-}
> getRootDir :: FilePath
> getRootDir = unsafePerformIO $ getEnv "GANETI_ROOTDIR"
>
> so that this function/value is evaluated only once, and then use that in
> addNodePrefix (which would be a honest pure function).
Are there any guarantees it's only evaluated once?
> That was about unsafePerformIO. However, there are two other issues.
> First, this does not catch any exceptions, and basically requires the
> variable to be defined.
System.Posix.Env.getEnv/getEnvDefault don't raise exceptions. The
former returns a Maybe, the latter even takes a default value (only
discovered this today).
> Second, I'm not sure whether we actually need to use unsafePerformIO,
> instead of making the node daemon log files functions that take the root
> path with, reading the environment nicely in the daemon startup.
I have to say this is beyond my Haskell abilities. I'm including an
interdiff below which separates unsafePerformIO into a separate
function and makes use of getEnvDefault. How about me committing this
and you can then change it?
--- a/htools/Ganeti/Path.hs
+++ b/htools/Ganeti/Path.hs
@@ -27,17 +27,16 @@ module Ganeti.Path where
import qualified Ganeti.Constants as C
import System.FilePath
-import System.Posix.Env (getEnv)
+import System.Posix.Env (getEnvDefault)
import System.IO.Unsafe
+{-# NOINLINE getRootDir #-}
+getRootDir :: FilePath
+getRootDir = unsafePerformIO $ getEnvDefault "GANETI_ROOTDIR" ""
-- | Prefixes a path with the current root directory
addNodePrefix :: FilePath -> FilePath
-addNodePrefix path = do
- let val = unsafePerformIO $ getEnv "GANETI_ROOTDIR"
- case val of
- Just rootdir -> rootdir ++ path
- Nothing -> path
+addNodePrefix path = getRootDir ++ path
-- | Directory for data
dataDir :: FilePath
Michael
