cdepillabout / password

datatypes and functions for easily working with passwords in Haskell
http://hackage.haskell.org/package/password
55 stars 16 forks source link

Be able to turn off instances in `password-instances` #65

Open Vlix opened 1 year ago

Vlix commented 1 year ago

The same way you can turn off certain algorithms in password with the #63 PR, we could also have flags to turn of instances and with them dependencies of password-instances (since especially aeson and persistent can really bloat your dependency tree if you're not even using them)

cdepillabout commented 1 year ago

Related to https://github.com/cdepillabout/password/issues/1.

Basically both would be ways to pick and choose exactly which instances / dependencies to use.

Vlix commented 1 year ago

Ah, you're right. Though I'd maybe rather do it in password-instances, just so as to not create more packages. And creating a package for only 1 or 2 instances seems like a waste and doing that 3 times seems inefficient :thinking:

cdepillabout commented 1 year ago

I think there are definitely situations where splitting up password-instances into multiple packages could be easier for the end-user (although like you say, not necessarily easier for us, the maintainers).

Here's an example. Let's say we decide to add flags to password-instances in order to enable/disable certain instances. We have a package called myapp, where we're directly depending on password-instances, with only the aeson instances enabled:

flowchart TD;
    myapp --> password-instances[password-instances with only aeson flag enabled];

Now, we want to depend on another library, which also has a dependency on password-instances. But it requires the persistent instances:

flowchart TD;
    myapp --> password-instances[password-instances with only aeson flag enabled];
    myapp --> some-library
    some-library --> password-instances;

Now, the end-user will get an compilation error when they try to build some-library, because it assumes password-instances will be built with the persistent flag enabled (but we don't have it enabled).

So the downside of not splitting up password-instances is that the end-user may have to be aware of what flags password-instances is built with, even in the case where they aren't directly depending on it! This isn't too bad for people using cabal or stack (since they give you easy ways of setting flags for transitive dependencies), but it a little more annoying when using distribution-supplied packages.

Splitting up password-instances into separate packages could make this easier for end-users, since you could directly depend on what you need, and not worry about what your transitive dependencies are doing. This should "just work":

flowchart TD;
    myapp --> password-instances-aeson;
    myapp --> some-library
    some-library --> password-instances-persistent;
Vlix commented 1 year ago

That is true, but like the current password flags, I'd argue for flags that have to be manually turned off. So that anyone who wants to specifically exclude a dependency, is able to do so.

If they then get a compilation error of the thing they turned off because of transient dependencies, they can just turn it back on, no? 🤔

cdepillabout commented 1 year ago

If they then get a compilation error of the thing they turned off because of transient dependencies, they can just turn it back on, no?

Either end users aren't aware of the flags (and they get really big dependency trees whenever they depend on password-instances), or they are aware of the flags, and have the possibility of getting into confusing situations when turning them off.

I'd argue that splitting password-instances into separate packages is much more "natural" to most Haskell developers, as opposed to flags determining what gets compiled in. It is easy to understand for end-users.

Vlix commented 1 year ago

Hmmmm, ok, that's a pretty good point, I guess.

Maybe we can add flags to the instances package as a last update, when we also "deprecate" it? Just to give power users more control if they're somehow stuck using password-instances over the newer ones? (though I hope the individual packages don't need very strict lower bounds 🤔 )

cdepillabout commented 1 year ago

@Vlix Oh, I guess I should be clear that I'm thinking we should split password-instances up into multiple packages (like password-instances-aeson, password-instances-persistent, etc [^1]), and then have a single password-instances package that depends on all the individual packages, just to give end-users an easy way to get started, in case they don't want to pick and choose. Although we would of course encourage library authors to depend on the individual password-instances-* packages.

So in that setup, I agree that potentially having flags in password-instances would be fine, and it may be helpful for power users.

[^1]: In #1, you suggested that the package names should instead be password-aeson, password-persistent, etc. Those names might make more sense, especially if those packages contain helper functions, but they might not be as "SEO-friendly", since the foobar-instances naming convention is often used in Haskell.