Skip to content

Feedback requested: what would be the impact of deleting System.PosixCompat.User from unix-compat? #6069

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

Closed
mitchellwrosen opened this issue Mar 13, 2023 · 4 comments

Comments

@mitchellwrosen
Copy link
Contributor

Hi there,

I've recently taken over maintainership of unix-compat, and I intend to push out a unix-2.8-compatible release as soon as I can. One remaining issue is what to make of the System.PosixCompat.User module, as the UserEntry type has changed.

Here is the first PR that attempts to graft over the differences: https://github.com/jacobstanley/unix-compat/pull/60/files

And here is my preferred solution, to delete the System.PosixCompat.User module entirely, based on the observation that System.PosixCompat.User doesn't seem useful at all, as all functions either return dummy data or throw exceptions: jacobstanley/unix-compat#62

However, I see that stack and yi-core actually use this module. For example:

stack/src/Stack/Config.hs

Lines 888 to 899 in 835318b

-- | Check whether the current user (determined with 'getEffectiveUserId') is
-- the owner for the given path.
--
-- Will always pure 'True' on Windows.
isOwnedByUser :: MonadIO m => Path Abs t -> m Bool
isOwnedByUser path = liftIO $
if osIsWindows
then pure True
else do
fileStatus <- getFileStatus (toFilePath path)
user <- getEffectiveUserID
pure (user == fileOwner fileStatus)

This code looks dubious to me: it claims to always return True on Windows, yet I believe it instead always throws an exception on Windows, which may be getting swallowed somewhere else.

But there are other uses, too, in Stack/Docker.hs. So, could you please consider the effect of deleting System.PosixCompat.User on the stack codebase, and let me know if that is a reasonable option, or if you'd rather see a different approach to unix-2.8 support?

Thanks.

@mpilgrem
Copy link
Member

@mitchellwrosen, thanks for asking. From Stack's perspective, I think it is a reasonable option to delete System.PosixCompat.User.

osIsWindows is True on Windows, so I would not expect Stack.Config.isOwnedByUser to throw an exception. Stack's Docker support throws unsupported exceptions on Windows - #2421.

@mpilgrem
Copy link
Member

Stack no longer imports System.PosixCompat.User.

@mitchellwrosen
Copy link
Contributor Author

That's great, thank you!

@hamishmack
Copy link

I think it might be a good idea to add an upper bound unix-compat <0.7 to https://hackage.haskell.org/package/stack-2.9.3 and https://hackage.haskell.org/package/stack-2.9.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants