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

[Question] Splitting OsString out of filepath #221

Closed
hasufell opened this issue Oct 20, 2023 · 6 comments
Closed

[Question] Splitting OsString out of filepath #221

hasufell opened this issue Oct 20, 2023 · 6 comments
Labels
core-libraries Governance of Core Libraries other than base

Comments

@hasufell
Copy link
Member

I want to split OsString modules out of filepath.

Motivation

OsString modules are accumulating more and more functionality that doesn't really belong in filepath. See haskell/filepath#202

Execution

Remarks/open questions

The current approach has couple of caveats:

  • filepath still keeps OsString modules, but deprecation warnings are added, telling you to use os-string
  • os-string keeps the module paths (e.g. System.OsString)
  • that means if a user adds both filepath and os-string they might need to use PackageImports to avoid compile failures of ambiguous module names

I see two alternatives:

  1. don't do a deprecation in filepath and just break API and do a major PVP bump
    • note: the current approach would require a major PVP bump as well once the deprecation phase ends
  2. rename the OsString modules in os-string package, e.g. Data.OsString instead of current System.OsString to avoid module name clashes

I think I personally prefer the PackageImports approach. Renaming modules is a different type of annoyance. Breaking API immediately is possible and would require maintaining two major version branches.


Opinions?

@parsonsmatt
Copy link

Consider attoparsec-aeson, which was recently released to carry the Data.Aeson.Parser module, which was then removed in aeson. attoparsec-aeson-2.1.0.0 is an empty package and depends on aeson < 2.2. Meanwhile attoparsec-aeson-2.2.0.0 has the actual Data.Aeson.Parser module, and aeson-2.2.0.0 removed that.

With reexported-modules in the cabal file, you shouldn't even get conflicts here.

@phadej
Copy link

phadej commented Oct 20, 2023

you shouldn't even get conflicts here.

I'll use version 1 and 2 to help avoid confusion. Version 1 is the current one.

  • ilepath-2 depends on os-string-2, re-exports Data.OsString.
  • os-string-1 is empty, depends on filepath-1, re-exports Data.OsString

We have four combinations, old-old, new-new, old-new, new-old:

  • old-old: os-string-1 and filepath-1 is fine.
  • new-new: os-string-2 and filepath-2 is fine.
  • new-old: os-string-2 and filepath-1 is-possible, so you get two Data.OsStrings modules if you depend on both os-string and filepath.
  • old-new: os-string-1 and filepath-2 is not possible as os-string-1 requires filepath-1.

So there is possible conflict. I don't think it's that bad. If users depend (possibly transitively) on both os-string and filepath, they would need to introduce an automatic flag to align the versions:

flag old-filepath
  manual: False
  
library
  if flag(old-filepath)
    build-depends: filepath <2, os-string <2
  else
    build-depends: filepath >=2, os-string >=2

Or just require filepath >=2, os-string >=2.


That said, OsString is not that widely used that I'd worry too much. https://hackage-search.serokell.io/?q=OsString is quite short list, so can be worked out individually.

@Bodigrim
Copy link
Collaborator

The situation looks similar to the one described at Cabal-syntax-3.6.

@hasufell
Copy link
Member Author

Ok, so the idea would be:

  1. release os-string-1.0.0 that is a dummy package with constraint filepath < 1.5 (current filepath is 1.4.xxx)
  2. release os-string-1.1.0 that adds the OsString modules
  3. release filepath-1.5 that removes OsString modules

Since those will be boot packages, do we require both os-string-1.0.0 and os-string-1.1.0 to be part of a GHC shipment?

@Bodigrim
Copy link
Collaborator

Bodigrim commented Nov 5, 2023

Since those will be boot packages, do we require both os-string-1.0.0 and os-string-1.1.0 to be part of a GHC shipment?

os-string-1.1.0 - yes, os-string-1.0.0 - probably no, at least I don't see why GHC would care.

@Bodigrim Bodigrim added the core-libraries Governance of Core Libraries other than base label Nov 8, 2023
@Bodigrim
Copy link
Collaborator

Bodigrim commented Nov 8, 2023

@hasufell is there anything else to discuss here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core-libraries Governance of Core Libraries other than base
Projects
None yet
Development

No branches or pull requests

4 participants