-
-
Notifications
You must be signed in to change notification settings - Fork 131
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
Minor dream-mirage fixes #283
Conversation
Use Dream.csrf_tag instead. See aantron@e14bd91
Earlier versions of lwt_ppx used in dream-mirage can conflict with the base ocaml version 4.08 required by dream.opam. For example: Fatal error: exception File "src/mirage/mirage.ml", line 389, characters 2-34: complex open are not supported before OCaml 4.08
For fcc20d0 I get the following error when installing the minimum bounds for dream and dream-mirage. Which I understood to mean
|
I'm currently unable to test this because of
I am working in example w-mirage, which I have copied out to its own separate directory. Do you know, by chance, what could be causing this? As a sanity check,
I see the |
Also
|
I think the problem is more about
Did you pin the |
I did and do have it pinned. To be sure:
To additionally check:
|
It's super weird because opam-monorepo doesn't see packages from the dune-universe repository. But they seem to be added by the makefile:
|
@aantron, I have taken the time to show that @tmcgilchrist patch works with a demo. Here I started with master and cherry-picked @tmcgilchrist's ebb6d57 commit.
Then I added the following Dockerfile to the base of Dream repo:
I used Docker to build, with each version of OCaml, a dream-mirage 'hello' unikernel (or 4.)
If it compiles it will run with docker run like so:
The following is a table of what worked and what did not:
In summary, I vote to at least cherry-pick ebb6d57. Nothing with mirage works without it. |
I'm running a topgit stack of patches for Dream, so I am able to do my thing, but I would love to see Mirage working again in master. |
This reverts commit fcc20d0.
I was able to get this working now and I'm ready to merge. Thank you and sorry for the huge delay! Why is there a bound of 0.0.6 on Mimic now? As for Lwt_ppx, it seems like that bound would have to be fixed upstream. If that's not an option, we can still merge it, though. If we need to do it in Dream, would it be better to put the 2.1.0 bound into dream-mirage.opam rather than dream.opam? |
I reverted the Lwt_ppx constraint change, because (1) I was unable to reproduce the problem it was trying to fix, (2) to first appearances, it seems like it should be addressed upstream and not in Dream, could be wrong about that, (3) I could not make sense of the error message "complex open are not supported before OCaml 4.08" in light of the fact that Dream has OCaml 4.08 as a lower bound. I was able to lower the mimic constraint to 0.0.5, as required by the vendored |
Thank you! Please send more PRs, or link me to any public forks that I can cherry-pick more patches from! |
Thanks for merging this fix @aantron I have not been doing any Dream / Mirage work recently and forgot about this PR. The main users of this functionality are https://github.com/ocurrent/mirage-ci, https://github.com/mirage/mirage-www and the linked ocaml-matrix library.
I think it was caused by |
The doc changes try to mirror the organisation in
dream.mli
as closely a possible.As a follow up PR I can re-organise
dream-mirage
to exactly mirror the maindream
docs.It would be helpful for them to be in sync and it makes it easier to see what has changed when diffing dream and dream-mirage. What do you think @aantron ?