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

bump argo submodule #1300

Merged
merged 1 commit into from
Dec 10, 2021
Merged

bump argo submodule #1300

merged 1 commit into from
Dec 10, 2021

Conversation

pnwamk
Copy link
Contributor

@pnwamk pnwamk commented Oct 19, 2021

No description provided.

@pnwamk
Copy link
Contributor Author

pnwamk commented Oct 28, 2021

@RyanGlScott initially I naively put up this PR thinking it would address build issues resulting from the breaking changes to aeson in versions >= 2.0 (i.e., like those you addressed here), but shortly thereafter discovered (via a similar PR for saw-script) that there is some code in this repo that will need changed to cope with those aeson changes, e.g.:

[ 1 of 16] Compiling CryptolServer.Data.Type ( src/CryptolServer/Data/Type.hs, /home/jennifer/work/PICARD/saw-script/dist-newstyle/build/x86_64-linux/ghc-8.10.7/cryptol-remote-api-0.1.0.0/build/CryptolServer/Data/Type.o, /home/jennifer/work/PICARD/saw-script/dist-newstyle/build/x86_64-linux/ghc-8.10.7/cryptol-remote-api-0.1.0.0/build/CryptolServer/Data/Type.dyn_o ) [Cryptol.Utils.RecordMap changed]

src/CryptolServer/Data/Type.hs:222:25: error:
    • Couldn't match expected type ‘JSON.Key’ with actual type ‘T.Text’
    • In the first argument of ‘(.=)’, namely ‘T.pack (show (pp f))’
      In the expression: T.pack (show (pp f)) .= JSONType ns t'
      In the first argument of ‘JSON.object’, namely
        ‘[T.pack (show (pp f)) .= JSONType ns t' |
            (f, t') <- canonicalFields fields]’
    |
222 |           JSON.object [ T.pack (show (pp f)) .= JSONType ns t'
    |                         ^^^^^^^^^^^^^^^^^^^^

src/CryptolServer/Data/Type.hs:234:35: error:
    • Couldn't match type ‘Data.Aeson.KeyMap.KeyMap JSON.Value’
                     with ‘HM.HashMap k0 a0’
      Expected type: HM.HashMap k0 a0
        Actual type: JSON.Object
    • In the second argument of ‘HM.lookup’, namely ‘o’
      In the expression: HM.lookup "type" o
      In the expression:
        case HM.lookup "type" o of
          Just t -> asType t o
          Nothing
            -> case HM.lookup "prop" o of
                 Just p -> asProp p o
                 Nothing -> fail "Expected type or prop key"
    |
234 |             case HM.lookup "type" o of
    |                                   ^

src/CryptolServer/Data/Type.hs:237:39: error:
    • Couldn't match type ‘Data.Aeson.KeyMap.KeyMap JSON.Value’
                     with ‘HM.HashMap k1 a1’
      Expected type: HM.HashMap k1 a1
        Actual type: JSON.Object
    • In the second argument of ‘HM.lookup’, namely ‘o’
      In the expression: HM.lookup "prop" o
      In the expression:
        case HM.lookup "prop" o of
          Just p -> asProp p o
          Nothing -> fail "Expected type or prop key"
    |
237 |                 case HM.lookup "prop" o of
    |

For SAWScript I ended up just making < 2.0 the upper bound for aeson (in this PR as an easy band aid for now. Should we do that here as well? If we don't, I assume some CPP could make this code more robust... I'm hesitant to add such CPP when an upper bound will do (but perhaps I'm overly CPP averse and it's NBD). Thoughts?

@RyanGlScott
Copy link
Contributor

Ah, I see. You're right in order to support aeson-2.0.* properly, we'll likely need to add CPP. But in the meantime, I think adding an upper bound of aeson < 2.0 would suffice. (Eventually, I hope to get around and bump all of these aeson upper bounds, but please don't wait on me for that.)

@pnwamk pnwamk force-pushed the bump-argo-submodule branch from 1276092 to 2579782 Compare December 9, 2021 23:23
@pnwamk pnwamk force-pushed the bump-argo-submodule branch from 2579782 to c31f0b6 Compare December 9, 2021 23:30
@pnwamk pnwamk merged commit cec8770 into master Dec 10, 2021
@pnwamk pnwamk deleted the bump-argo-submodule branch December 10, 2021 19:26
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

Successfully merging this pull request may close these issues.

2 participants