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

Please clarify relation between data-default and data-default-class #29

Open
thielema opened this issue Oct 24, 2024 · 8 comments
Open

Comments

@thielema
Copy link

I see you released data-default-0.8.0.0 yesterday. It does no longer re-export Default class from data-default-class. This is a breaking change, indeed the major version bump to 0.8 indicates that.
For instance, package hledger-web imports data-default and yesod, where yesod defines instances for data-default-class. hledger assumes that Default class from data-default and data-default-class are actually the same.
Could you clarify which package, data-default or data-default-class, shall be preferred in the future? Will data-default-class become deprecated or what will the relation between the two packages be? Changelog or Release Notes would be nice.

@mauke
Copy link
Owner

mauke commented Oct 24, 2024

Re: Changelog, see https://github.com/mauke/data-default/blob/ba1596728252c8f640d21225e728f508f2f3233c/Changes.pod#user-content-pod0.8.0.0.

The plan (starting from 0.8) is to return back to a single data-default package (like before 0.5.2).

data-default-class is not formally deprecated, but effectively requires you to stay with data-default >=0.5.2 && <0.8 because the two packages don't work together otherwise.

@sol
Copy link
Contributor

sol commented Oct 26, 2024

@mauke It will cause less friction for the ecosystem if you upload a new version of data-default-class that re-exports data-default (0.8.0).

@mauke
Copy link
Owner

mauke commented Oct 26, 2024

@sol I've been wondering about that. As far as I can see, there are three options:

  1. I do nothing.

    People can still write code compatible with either data-default or data-default-class by doing something like (untested):

    flag use-data-default
    ...
      if flag(use-data-default)
        build-depends: data-default ^>=0.8
      else
        build-depends: data-default-class ^>=0.1.2.2
    
    {-# LANGUAGE CPP #-}
    #if defined(MIN_VERSION_data_default_class)
    import Data.Default.Class
    #else
    import Data.Default
    #endif

    Pros: I don't have to do anything.
    Cons: Requires annoying compatibility checks from each user, plus CPP hacks.

  2. I do nothing.

    People can still write code compatible with both versions of data-default by doing something like this:

      build-depends: data-default >=0.7 && <0.9
    
    import Data.Default

    Pros: I don't have to do anything.
    Cons: Manual action required from anyone currently using Data.Default.Class (they have to switch to Data.Default instead). With older versions of data-default, this pulls in extra package dependencies (dlist, old-locale).

  3. I upload a dummy data-default-class that just re-exports Data.Default.

    This will be a new major version because instances were removed, so people depending on data-default-class will have to relax their version bounds:

      build-depends: data-default-class >=0.1 && <0.3
    

    Pros: In many cases where import Data.Default.Class is currently used, the Haskell code won't have to be touched at all.
    Cons: I have to do something. :-) And people still need to change their cabal files, so manual action is still required of them.

Do you think option 3 saves enough maintainer effort over 2 that it's worth it?

@sol
Copy link
Contributor

sol commented Oct 26, 2024

Do you think option 3 saves enough maintainer effort over 2 that it's worth it?

Yes.

@mauke
Copy link
Owner

mauke commented Oct 26, 2024

I've uploaded a package candidate: https://hackage.haskell.org/package/data-default-class-0.2.0.0/candidate

Does this look reasonable to you?

Disregard that, I've published https://hackage.haskell.org/package/data-default-class-0.2.0.0.

@leftaroundabout
Copy link

Thanks for unifying these packages! I suppose data-default-class should now be considered deprecated?

Unfortunately this has given rise to another, rather silly problem: if package A depends on data-default and package B depends on A and data-default-class <0.2, you can end up with two conflicting Default classes in the build. Package A may assume that Data.Default.Default and Data.Default.Class.Default are the same, but the outdated Data.Default.Class.Default that package B imports is actually different.

src/Numeric/Optimization/MIP/Solution/CPLEX.hs:159:37: error:
    • No instance for (Default XML.ParseSettings)
        arising from a use of ‘def’
    • In the first argument of ‘XML.parseText_’, namely ‘def’
      In the second argument of ‘($)’, namely ‘XML.parseText_ def t’
      In the expression: parseDoc $ XML.parseText_ def t
    |
159 | parse t = parseDoc $ XML.parseText_ def t
    |                                     ^^^

It was easy to fix this (had to remove a <0.2 constraint in a different dependency), but hard to figure out what was causing the problem because you just see "No instance of (Default SomeType)" – even though there is a Default instance.

Would it be possible to revise the old versions' dependencies to ensure one cannot have both data-default >=0.8 and data-default-class <2.0 in the build plan?

@mauke
Copy link
Owner

mauke commented Dec 5, 2024

Would it be possible to revise the old versions' dependencies to ensure one cannot have both data-default >=0.8 and data-default-class <2.0 in the build plan?

How do I do that?

@leftaroundabout
Copy link

Maybe retroactively adding data-default <0.8 as a dependency to all data-default-class 0.1.*?

I'm not sure, neither whether that would actually prevent the problem happening nor whether it would cause even more confusion elsewhere or if Hackage would even allow it.

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

4 participants