Adds getArgs :: IO [OsString]

[tbidne]

Resolves #338.

[tomjaguarpaw]

This seems all fine to me, but I don't really "own" this library and don't know who does, so I don't know who gives final sign-off.

[Bodigrim]

@tomjaguarpaw I'm pretty sure that you have all necessary authority to make decisions here, since Michael passed the torch. Otherwise you can reach to CLC for advice, but this PR does not look particularly controversial to me.

[tbidne]

Two questions I have:

1. Do we need to do anything about js or wasm? I see javascript_HOST_ARCH and wasm32_HOST_ARCH pragmas used in various places. I tried the legacy string getArgs and unix getArgs on the js backend, and both give empty args without an error, so maybe it's fine?

2. What of the rest of System.Environment? I don't mean to feature-creep, but I think it would be helpful to have an idea of what is to become of the rest of that module. Does the argument in favor of adding getArgs here ("args pertaining to the current process") also apply to the environment and/or program name/path? If so, everything in that module could eventually go here.

   Right now, getEnv :: String -> IO String and getEnvironment :: IO [(String, String)] exist in unix and Win32 for PosixString and WindowsString, respectively, so these could also be easily shimmed here in this PR. The others do not exist yet.

Edit:

3. Another point. Using getArgs from Win32 requires version 2.14+. This version uses -XNumericUnderscores, which is GHC 8.6.1+, so probably we'd need base >= 4.12.0.0, and to drop < 8.6 from CI. Also Win32 should upgrade its lower bound.

   See Restore compatibility with older GHC win32#229.

   Actually, maybe this doesn't require any action here because it's purely a Win32 problem that needs solving. What should happen is that issue should be merged and a new hackage release uploaded, and the offending versions marked deprecated so the solver doesn't pick them. But this doesn't have anything to do with process or any other reverse dependency.

[tomjaguarpaw]

What of the rest of System.Environment?

I think if we're going to include one of its elements we may as well include them all.

[tbidne]

What of the rest of System.Environment?

I think if we're going to include one of its elements we may as well include them all.

Okay, I've added the remaining functions (getEnv and getEnvironment) that currently exist in all 3 (System.Environment, Win32, and unix). Here is a list of outstanding functions that should probably also be here, once they exist in Win32 and unix:

• Env.getProgName :: IO OsString
• Env.executablePath :: Maybe (IO (Maybe OsString))
• Env.getExecutablePath :: IO OsString
• Env.lookupEnv :: OsString -> IO (Maybe OsString)
• Env.setEnv :: OsString -> OsString -> IO ()
• Env.unsetEnv :: OsString -> IO ()
• Env.withArgs :: [OsString] -> IO a -> IO a
• Env.withProgName :: OsString -> IO a -> IO a

[tbidne]

Some bikeshedding:

1. System.Process.Env (current) or System.Process.Environment (like System.Environment)?
2. Generic haddocks e.g. 'System.Environment.getArgs' for 'OsString' or copied from System.Environment?

[tomjaguarpaw]

1. I'd prefer keeping it as similar as possible to existing module names, so unless there's also something called Env that we're copying, I'd stick to Environment

[tbidne]

Okay, renamed Env -> Environment and added the MIN_VERSION_filepath(1, 5, 0) cpp. Two CI questions:

1. Do we care about the failing GHC 8.4 + Windows job, due to this? To recap, the problem is that we now require a recent version of Win32, which unfortunately added NumericUnderscores, causing GHC < 8.6.1 to fail.

   Ideally that PR would be merged and released, so CI here would go back to green once the solver starts picking the new release. But it's been open for a year, so that might not be anytime soon. The combination could be excluded here, if we want.

2. Do we want to test the flag on CI in some capacity? It's easy enough to add a new matrix for os-string/no os-string, though of course that basically doubles the number of jobs, from 41 to 82 (it's not quite that bad, since some older combinations should be excluded). This is well within the free tier limit (256 I think), but still, it will make CI take longer.

[tomjaguarpaw]

I suspect it's probably fine to give up on GHC 8.4.

[tomjaguarpaw]

Alright, this is looking good! @tbidne, is this ready to merge in your opinion?

Regarding testing more in the CI matrix, I'm happy for that to be added at a later date.

[tbidne]

LGTM.

[tomjaguarpaw]

Released as https://hackage.haskell.org/package/process-1.6.26.0

Thanks @tbidne and @hasufell

[tbidne]

Thanks @tomjaguarpaw!