search

haskell / unix

POSIX functionality
https://hackage.haskell.org/package/unix
Other
107 stars 92 forks source link

Ensure that FilePaths don't contain interior NULs #279

Closed hasufell closed 1 year ago

hasufell commented 1 year ago

Follows:

@bgamari @Bodigrim

Bodigrim commented 1 year ago

Overall looks good. @vdukhovni could you please take a look?

Bodigrim commented 1 year ago

@vdukhovni @hs-viktor @hasufell is there any conclusion? We have to make a release soon, to accompany GHC 9.8.

hasufell commented 1 year ago

We haven't come to a conclusion wrt ioe_filename and I was actually planning to consult with GHC devs about this. Or even CLC. It's not clear what the API of this record is to the end user.

Based on that, we can more easily decide what to do.

vdukhovni commented 1 year ago

I don't feel very strongly about encoding choices, but I am quite adamant about error values not containing bottoms. Pick the encoding strategy that will be least surprising, and never ever have any of the fields of IOError throw when forced.

When working with paths that are ByteStrings, if they make sense as UTF8, assume they're UTF8, otherwise raw? Assuming the default system filename encoding (as suggested) is another. No encoding choice will be always right, we simply don't know. Part of the reason the IO operation failed, could well be a poor choice of encoding... :-(

hasufell commented 1 year ago

but I am quite adamant about error values not containing bottoms

Yes, as I explained above, the current code using TransliterateCodingFailure never throws.

Whatever we do, we have to document it in base, IMO: https://hackage.haskell.org/package/base-4.18.0.0/docs/GHC-IO-Exception.html#v:ioe_filename

It'll be utterly confusing to end users otherwise, looking for the properties of this record field.

Bodigrim commented 1 year ago

We haven't come to a conclusion wrt ioe_filename and I was actually planning to consult with GHC devs about this. Or even CLC. It's not clear what the API of this record is to the end user.

Based on that, we can more easily decide what to do.

AFAIU this is no longer a blocker, right?

hasufell commented 1 year ago

Correct. I need to update the PR.

hasufell commented 1 year ago

Should be settled.