snapframework / snap

Top-level package for the official Snap Framework libraries, includes the snaplets API as well as infrastructure for sessions, auth, and templates.
http://snapframework.com/
BSD 3-Clause "New" or "Revised" License
455 stars 68 forks source link

snap fails to install with upcoming aeson due to ByteString no longer a valid FromJSON #90

Closed ibotty closed 8 years ago

ibotty commented 11 years ago

Aeson recently dropped FromJSON ByteString instances (out of good reasons. see bos/aeson#126 (https://github.com/bos/aeson/issues/126)).

the following error is generated.

src/Snap/Snaplet/Auth/Types.hs:322:21:
    No instance for (ToJSON ByteString)
      arising from a use of `toJSON'
    Possible fix: add an instance declaration for (ToJSON ByteString)
    In the expression: toJSON x
    In an equation for `toJSON': toJSON (Role x) = toJSON x
    In the instance declaration for `ToJSON Role'

src/Snap/Snaplet/Auth/Types.hs:327:27:
    No instance for (FromJSON ByteString)
      arising from a use of `parseJSON'
    Possible fix: add an instance declaration for (FromJSON ByteString)
    In the second argument of `(.)', namely `parseJSON'
    In the expression: fmap Role . parseJSON
    In an equation for `parseJSON': parseJSON = fmap Role . parseJSON

I am pretty new in going through snap's internals, so please bear with my superficial investigation.

Role is only the tip of the iceberg though. There are quiet a few ByteStrings that want to get serialized to/from JSON. I expect that the following list is non-exhaustive, because I only looked at Snap (not all other Snap packages).

see ibotty@a1800ed249d993fc172bb2ad9bc0073c5ed1379f for my attempt to fix this problem.

mightybyte commented 11 years ago

It fails to install because these aeson changes need to be released as aeson-0.7 to conform with the PVP. We will have to change eventually, so thanks for bringing this up. I think we may need to consider rethinking some of our approach.

ozataman commented 11 years ago

I think in general JSON is a bad way to store backend data; we shouldn't be using it as the out-of-the-box user store. (Yes I know that I was the one who designed/implemented the current JSON backend ;-) ) If nothing else, JSON's rejection of binary (non-utf8) data precludes it from being an arbitrary store of data.

I propose the following:

ibotty commented 11 years ago

just to make that clear, because i mixed versions up (my cabal copy was too old). aeson 0.6.2 is already released and does not contain that change. it is upcoming but it won't be 0.6.2, but 0.7 or whatever bryan o'sullivan decides.

mightybyte commented 11 years ago

@ibotty Correct. The PVP describes how version numbers should be bumped when releasing new versions. The first of the three rules says "If...instances were added or removed, then the new A.B must be greater than the previous A.B." If Bryan conforms to this rule, then the change in question will have to be released with at least version 0.7, and that falls outside the bounds currently accepted by snap.

However, this didn't have anything to do with your cabal copy being too old. It happened because Bryan hasn't bumped the version yet in aeson. In my projects, I usually try to make an appropriate version bump as soon as I make a change that needs it. That way, you would not have encountered the build error because aeson would already be at version 0.7 in the master branch (snap just would have used 0.6). But that's just my personal preference. Bryan may have his own established process for version bumps.