search

c-cube / tiny_httpd

Minimal HTTP server using good old threads + blocking IO, with a small request router.
https://c-cube.github.io/tiny_httpd
75 stars 11 forks source link

Make CI closer to what users would do #39

Closed kit-ty-kate closed 2 years ago

kit-ty-kate commented 2 years ago

This should detect issues such as the ones encountered in https://github.com/ocaml/opam-repository/pull/20345 sooner

c-cube commented 2 years ago

I've tried updating this a bit by also adding OS restrictions in opam files, but it still fails weirdly: https://github.com/c-cube/tiny_httpd/runs/4697934203?check_suite_focus=true

kit-ty-kate commented 2 years ago

It’s failing because you need to call opam depext twice (once for regular install and the other with-test) In this PR I’ve tried to use opam 2.1 but it seems without success. @smorimoto do you know how opam 2.1 can be used? I’d like to avoid having the deprecated opam-depext everywhere

smorimoto commented 2 years ago

If I don't misunderstand something, you can do that by passing --with-test to opam-depext-flags. https://github.com/ocaml/setup-ocaml/blob/2bbbf95d053420b012982bfaff329f65078f30cb/action.yml#L23-L26

smorimoto commented 2 years ago

And for Windows compatibility, there's no reason to force opam 2.1 mode on CI at this point. Why do you need it?

kit-ty-kate commented 2 years ago

it wasn't for windows. Switching to opam 2.1 makes everything much shorter and faster

kit-ty-kate commented 2 years ago

is there no option to switch to opam 2.1? (with builtin depexts enabled)

smorimoto commented 2 years ago

I know it's shorter and faster, but it doesn't change that dramatically, and I think it's better to keep the workflow simple. We still recommend 2.0 mode on GHA because so many people aren't familiar with opam and don't think there's much incentive to do the hack and save a few seconds. But that's not impossible. I just added a commit to do it in PR #42.

smorimoto commented 2 years ago

The setup-ocaml roadmap is aimed to provide an OS-neutral interface to opam and the ecosystem around it, so no functionality will be added that only works with one operating system. (Even if it works, it's pretty hard to support multiple things that behave differently at the same time.) This is also why we are sticking with 2.0 mode at this point.

smorimoto commented 2 years ago

Oops, GHA doesn't seem to be able to override workflow environment variables with env fields in this case. This is because it is set to 2.0 when setup-ocaml is invoked. To support overwrite, if the OPAMCLI is configured, it must be implemented not to export it again on the setup-ocaml side...

kit-ty-kate commented 2 years ago

I’m not sure to understand why this PR was closed. The underlying issue has not been fixed

c-cube commented 2 years ago

Well, CI works and opam-ci worked as well. I did close this a bit prematurely, feel free to reopen if you're willing to pursue it :)