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

Output of "dhall freeze --cache" no longer works with Dhall v21.0.0 #2347

Closed
joell opened this issue Dec 8, 2021 · 5 comments · Fixed by #2350
Closed

Output of "dhall freeze --cache" no longer works with Dhall v21.0.0 #2347

joell opened this issue Dec 8, 2021 · 5 comments · Fixed by #2350

Comments

@joell
Copy link

joell commented Dec 8, 2021

The dhall freeze command has a --cache flag thats described purpose is to "Add fallback unprotected imports when using integrity checks purely for caching purposes". However, in dhall-haskell 1.40.1 the code generated from using this feature no longer works if the fetched URL content does not match the hash.

Example:

# observe how dhall freeze creates a cache-only fallback import for List/map
$ dhall freeze --cache <<< 'https://prelude.dhall-lang.org/List/map'
  https://prelude.dhall-lang.org/List/map
    sha256:dd845ffb4568d40327f2a817eb42d1c6138b929ca758d50bc33112ef3c885680
? https://prelude.dhall-lang.org/List/map

# submit the same code to Dhall but with a hash that does not match the content
$ echo '
  https://prelude.dhall-lang.org/List/map
    sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
? https://prelude.dhall-lang.org/List/map' | dhall
dhall: 
↳ https://prelude.dhall-lang.org/List/map
  sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff

Error: Import integrity check failed

Expected hash:

↳ ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff

Actual hash:

↳ dd845ffb4568d40327f2a817eb42d1c6138b929ca758d50bc33112ef3c885680


1│ https://prelude.dhall-lang.org/List/map sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff

(input):1:1

The expectation I had from the description of dhall freeze --check was that the ? operator should fallback from the integrity-check-failing import to the unprotected URL import and the expression should have succeeded (as long as https://prelude.dhall-lang.org/List/map could be loaded). Put simply, it seems this should work without emitting an error message.

Is dhall freeze --check's description out of date, dhall-haskell's implementation buggy, or my interpretation of all this faulty?

(Update: Given the breaking change to ? in Dhall v21.0.0, it seems the issue here is in the output of dhall freeze --cache not being revised to adapt to the breaking change.)

@joell
Copy link
Author

joell commented Dec 8, 2021

I just found dhall-lang/dhall-lang#1181 in which the ? operator behavior was changed in a breaking way. So it appears that dhall's rejection of my incorrect-hash example correct to the v21 of standard.

However, that appears to leave the code generated by dhall freeze --cache failing to implement the functionality its --help description specifies.

Would it be sufficient to revise dhall freeze --cache to emit alternate code as follows?

$ dhall freeze --cache <<< 'https://prelude.dhall-lang.org/List/map'
  missing sha256:dd845ffb4568d40327f2a817eb42d1c6138b929ca758d50bc33112ef3c885680
? https://prelude.dhall-lang.org/List/map

This seems to satisfy the requirement that ? only "fall back to the second import if [the first import] cannot be retrieved". And it seems to work in practice.

@joell joell changed the title Import fallbacks for caching purposes (ala "dhall freeze --cache") no longer work Output of "dhall freeze --cache" no longer works with Dhall v21.0.0 Dec 8, 2021
@sjakobi
Copy link
Collaborator

sjakobi commented Dec 12, 2021

Would it be sufficient to revise dhall freeze --cache to emit alternate code as follows?

$ dhall freeze --cache <<< 'https://prelude.dhall-lang.org/List/map'
  missing sha256:dd845ffb4568d40327f2a817eb42d1c6138b929ca758d50bc33112ef3c885680
? https://prelude.dhall-lang.org/List/map

That seems to fit with the documented intention of the freeze --cache feature.

@Gabriel439, what do you think?

Gabriella439 added a commit that referenced this issue Dec 15, 2021
Fixes #2347

This now uses `missing sha256:…` for the frozen import so that the
import still succeeds even if the hash doesn't match (which was the
original intention of the `--cached` flag; otherwise it would be
useless).
@Gabriella439
Copy link
Collaborator

Fix is up here: #2350

Gabriella439 added a commit that referenced this issue Dec 15, 2021
Fixes #2347

This now uses `missing sha256:…` for the frozen import so that the
import still succeeds even if the hash doesn't match (which was the
original intention of the `--cached` flag; otherwise it would be
useless).
@joell
Copy link
Author

joell commented Dec 15, 2021

Thank you, @Gabriel439!

@Gabriella439
Copy link
Collaborator

You're welcome! 😊

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 a pull request may close this issue.

3 participants