Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix augment path #237

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions rio/ChangeLog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog for rio

## 0.1.21.0

* Fix minor bug in `augmentPathMap` on windows wrt [#234](https://github.com/commercialhaskell/rio/issues/234) not adhering to case-insensitive semantics

## 0.1.20.0

* Export `UnliftIO.QSem` and `UnliftIO.QSemN` in `RIO`
Expand Down
5 changes: 4 additions & 1 deletion rio/package.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: rio
version: 0.1.20.0
version: 0.1.21.0
synopsis: A standard library for Haskell
description: See README and Haddocks at <https://www.stackage.org/package/rio>
license: MIT
Expand Down Expand Up @@ -122,3 +122,6 @@ tests:
- rio
- hspec
- QuickCheck
verbatim: |
build-tool-depends:
hspec-discover:hspec-discover
22 changes: 14 additions & 8 deletions rio/src/RIO/Process.hs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE RankNTypes #-}
{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE ViewPatterns #-}

-- | Interacting with external processes.
--
Expand Down Expand Up @@ -190,7 +191,7 @@ data ProcessException
| ExecutableNotFound String [FilePath]
| ExecutableNotFoundAt FilePath
| PathsInvalidInPath [FilePath]
deriving Typeable
deriving (Typeable, Eq)
instance Show ProcessException where
show NoPathFound = "PATH not found in ProcessContext"
show (ExecutableNotFound name path) = concat
Expand Down Expand Up @@ -269,7 +270,7 @@ exeSearchPathL = processContextL.to pcPath
--
-- @since 0.0.3.0
mkProcessContext :: MonadIO m => EnvVars -> m ProcessContext
mkProcessContext tm' = do
mkProcessContext (normalizePathEnv -> tm) = do
ref <- newIORef Map.empty
return ProcessContext
{ pcTextMap = tm
Expand All @@ -287,17 +288,21 @@ mkProcessContext tm' = do
, pcWorkingDir = Nothing
}
where
-- Fix case insensitivity of the PATH environment variable on Windows.
tm
| isWindows = Map.fromList $ map (first T.toUpper) $ Map.toList tm'
| otherwise = tm'
-- Default value for PATHTEXT on Windows versions after Windows XP. (The
-- documentation of the default at
-- https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/start
-- is incomplete.)
defaultPATHEXT = ".COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH;.MSC"


-- Fix case insensitivity of the PATH environment variable on Windows,
-- by forcing all keys full uppercase.
normalizePathEnv :: EnvVars -> EnvVars
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not entirely sure if we need an inline pragma here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's reasonable as-is.

normalizePathEnv env
| isWindows = Map.fromList $ map (first T.toUpper) $ Map.toList env
| otherwise = env


-- | Reset the executable cache.
--
-- @since 0.0.3.0
Expand Down Expand Up @@ -654,7 +659,8 @@ exeExtensions = do
pc <- view processContextL
return $ pcExeExtensions pc

-- | Augment the PATH environment variable with the given extra paths.
-- | Augment the PATH environment variable with the given extra paths,
-- which are prepended (as in: they take precedence).
--
-- @since 0.0.3.0
augmentPath :: [FilePath] -> Maybe Text -> Either ProcessException Text
Expand All @@ -670,7 +676,7 @@ augmentPath dirs mpath =
--
-- @since 0.0.3.0
augmentPathMap :: [FilePath] -> EnvVars -> Either ProcessException EnvVars
augmentPathMap dirs origEnv =
augmentPathMap dirs (normalizePathEnv -> origEnv) =
do path <- augmentPath dirs mpath
return $ Map.insert "PATH" path origEnv
where
Expand Down
30 changes: 30 additions & 0 deletions rio/test/RIO/Prelude/ExtraSpec.hs
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
{-# LANGUAGE CPP #-}
{-# LANGUAGE OverloadedStrings #-}

module RIO.Prelude.ExtraSpec (spec) where

import RIO
import RIO.Process
import Test.Hspec

import qualified Data.Map as Map
import qualified Data.Text as T
import qualified System.FilePath as FP

spec :: Spec
spec = do
describe "foldMapM" $ do
Expand All @@ -11,3 +19,25 @@ spec = do
helper = pure . pure
res <- foldMapM helper [1..10]
res `shouldBe` [1..10]
describe "augmentPathMap" $ do
-- https://github.com/commercialhaskell/rio/issues/234
it "Doesn't duplicate PATH keys on windows" $ do
let pathKey :: T.Text
#if WINDOWS
pathKey = "Path"
#else
pathKey = "PATH"
#endif
origEnv :: EnvVars
origEnv = Map.fromList [ ("foo", "3")
, ("bar", "baz")
, (pathKey, makePath ["/local/bin", "/usr/bin"])
]
let res = second (fmap getPaths . Map.lookup "PATH") $ augmentPathMap ["/bin"] origEnv
res `shouldBe` Right (Just ["/bin", "/local/bin", "/usr/bin"])
where
makePath :: [T.Text] -> T.Text
makePath = T.intercalate (T.singleton FP.searchPathSeparator)

getPaths :: T.Text -> [T.Text]
getPaths = fmap T.pack . FP.splitSearchPath . T.unpack