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

Update dependencies #815

Closed
wants to merge 7 commits into from
Closed

Conversation

MisterDA
Copy link
Contributor

No description provided.

@MisterDA MisterDA force-pushed the update-dependencies branch 2 times, most recently from 6fde036 to dab6333 Compare December 12, 2023 15:26
@hannesm
Copy link
Member

hannesm commented Dec 12, 2023

Thanks for your changes, from what I can tell the issue is changes in 4.4.0. If you apply the following patch, it should work:

--- a/mirage/config.ml
+++ b/mirage/config.ml
@@ -202,10 +202,18 @@ let enable_metrics =
   let doc = Key.Arg.info ~doc:"Enable metrics reporting" [ "metrics" ] in
   Key.(create "metrics" Arg.(flag doc))
 
+type i0 = I0
+let i0 = Functoria.Type.v I0
+let no0 = Functoria.impl "Int" job
+
+type n1 = N1
+let n1 = Functoria.Type.v N1
+let noop1 = Functoria.impl "Set.Make" (job @-> job)
+
 let optional_monitoring time pclock stack =
   if_impl (Key.value enable_metrics)
     (mirage_monitoring $ time $ pclock $ stack)
-    noop
+    (noop1 $ no0)
 
 let () =
   register "www"

@MisterDA
Copy link
Contributor Author

Yes, I was going to ask you :) wouldn't have figured that out!

Dockerfile Outdated
RUN opam repo add mirage-dev git+https://github.com/mirage/mirage-dev.git#842c55556ffd0950d21141d6ab99e52a8d88a50f
RUN sudo ln -f /usr/bin/opam-2.1 /usr/bin/opam && cd ~/opam-repository && git pull origin master && git reset --hard 30b1b97d735732e40996cf2e6b06d478ac40633f && opam update
RUN opam pin tailwindcss.dev https://github.com/tmattio/opam-tailwindcss/archive/3e60fc32bbcf82525999d83ad0f395e16107026b.tar.gz
RUN opam repo add mirage-dev git+https://github.com/mirage/mirage-dev.git#749c02302f8f15e609332edbe827541558554a80
Copy link
Member

@hannesm hannesm Dec 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this mirage-dev overlay we should not need (famous last words) at all ;)

@hannesm
Copy link
Member

hannesm commented Dec 12, 2023

Yes, I was going to ask you :) wouldn't have figured that out!

Maybe the mirage tool should be relaxed to allow some jobs without arguments (maybe exactly the noop job) -- but the mirage repository is currently in unclear state -- the main branch isn't too usable, we could continue on the 4.4 branch...

@MisterDA
Copy link
Contributor Author

MisterDA commented Dec 12, 2023

Unfortunately the patch for Dream that was pinned seem not to have been upstreamed.
No, my bad, I was probably using an old version. See #813.

@MisterDA
Copy link
Contributor Author

Erk, the deployer is using mirage 3… I think this PR is exceeding on the time I wanted to allocate for this, haha.

hannesm added a commit to hannesm/mirage that referenced this pull request Dec 12, 2023
@hannesm
Copy link
Member

hannesm commented Dec 12, 2023

For the patch posted above, here is a fix for the mirage utility mirage/mirage@16e82ff

@hannesm
Copy link
Member

hannesm commented Dec 12, 2023

Erk, the deployer is using mirage 3… I think this PR is exceeding on the time I wanted to allocate for this, haha.

That's a bit sad. Well, I don't quite understand the whole "CI" and "deployment" stuff of this repository anyways... Eventually someone will (hopefully) fix it. Even a local build doesn't work for me (as mentioned in #792).

@MisterDA MisterDA force-pushed the update-dependencies branch from 3f58104 to 85a8348 Compare December 12, 2023 16:23
@MisterDA MisterDA force-pushed the update-dependencies branch from b577d1f to 0488633 Compare December 12, 2023 16:40
hannesm added a commit to hannesm/opam-repository that referenced this pull request Feb 13, 2024
CHANGES:

- Fix GC documentation (space overhead is 120 since best-fit is default)
  (mirage/mirage#1491 @hannesm)
- use Option in printing of GC keys (mirage/mirage#1491 @hannesm)
- allow to use git 3.15 (mirage/mirage#1491 @hannesm)
- allow empty noop job (addresses an issue with mirage/mirage#1428)
  solves an issue reported by @MisterDA in mirage/mirage-www#815
  (mirage/mirage#1491 @hannesm)
@hannesm
Copy link
Member

hannesm commented Feb 27, 2024

Thanks for your changes, from what I can tell the issue is changes in 4.4.0. If you apply the following patch, it should work:

--- a/mirage/config.ml
+++ b/mirage/config.ml
@@ -202,10 +202,18 @@ let enable_metrics =
   let doc = Key.Arg.info ~doc:"Enable metrics reporting" [ "metrics" ] in
   Key.(create "metrics" Arg.(flag doc))
 
+type i0 = I0
+let i0 = Functoria.Type.v I0
+let no0 = Functoria.impl "Int" job
+
+type n1 = N1
+let n1 = Functoria.Type.v N1
+let noop1 = Functoria.impl "Set.Make" (job @-> job)
+
 let optional_monitoring time pclock stack =
   if_impl (Key.value enable_metrics)
     (mirage_monitoring $ time $ pclock $ stack)
-    noop
+    (noop1 $ no0)
 
 let () =
   register "www"

I wanted to raise my voice that since mirage 4.4.2 got released, the above diff is not needed anymore. Not sure whether this makes any difference for this PR (which seems abandoned).

@MisterDA
Copy link
Contributor Author

Hi Hannes,
I'm currently away from computers until the beginning of April. Unfortunately I was unable to set up a working build environment before I left, and thus complete this PR. If you feel you can take anything from it please do, otherwise I'll try to catch up in a month or so.
-- Antonin

@samoht samoht marked this pull request as ready for review April 9, 2024 09:27
@samoht
Copy link
Member

samoht commented Apr 11, 2024

Thanks, some of your changes have been picked by #818 and now the CI is green again!

@samoht samoht closed this Apr 11, 2024
@MisterDA MisterDA deleted the update-dependencies branch April 11, 2024 19:40
nberth pushed a commit to nberth/opam-repository that referenced this pull request Jun 18, 2024
CHANGES:

- Fix GC documentation (space overhead is 120 since best-fit is default)
  (mirage/mirage#1491 @hannesm)
- use Option in printing of GC keys (mirage/mirage#1491 @hannesm)
- allow to use git 3.15 (mirage/mirage#1491 @hannesm)
- allow empty noop job (addresses an issue with mirage/mirage#1428)
  solves an issue reported by @MisterDA in mirage/mirage-www#815
  (mirage/mirage#1491 @hannesm)
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.

3 participants