Restrict base-bytes to only be selected on OCaml >= 5.0

[Leonidas-from-XIV]

Reported by @avsm in #175 (comment), this makes sure the base-bytes polyfill is only picked on OCaml 5.0.

CC @kit-ty-kate.

[kit-ty-kate]

I don't think this is the right fix. I believe you want to change your fork of findlib instead.

[Leonidas-from-XIV]

You might be right, though the only case I can see a problem would be when base-bytes+base+dune is locked on a 5.0 compiler but used on an older compiler (which might cause all kinds of issues with other packages anyway, similar to using OCaml 4.02 as a compiler for a 4.14-locked duniverse).

@kit-ty-kate What issues do you see with this fix?

[hannesm]

Just for the record, this broke every MirageOS compilation out there: An example is the mirage-ci https://ci.mirage.io/pipelines/pr-1392-mirage-4-mirage-mirage/pr-1392-mirage-4-mirage-mirage-0b43b25c4ee181306025b360bcb961372e59a5eb/x86_64-debian-10-4.13/2023-02-06/105700-ocluster-build-3935ca which is now unable to select any "base-bytes" version.

I don't quite understand how this was merged" the CI here is failing, @kit-ty-kate raised concerns....

[Leonidas-from-XIV]

I don't quite understand how this was merged" the CI here is failing, @kit-ty-kate raised concerns....

The CI was failing with an "Version is correct: Failed: Version is missing +dune suffix" error, which after looking at it looks more like a bug in the linter than an actual failure. I asked @kit-ty-kate for the concerns but didn't hear back all the while the issue reported by @avsm is still true, so I decided to merge it.

Looks like that was unwise, thanks for the report, so I'll revert the change: #183 and I'll have to look into finding a better solution.

[hannesm]

Dear @Leonidas-from-XIV, thanks for your prompt reply and commit.

The CI was failing with an "Version is correct: Failed: Version is missing +dune suffix" error, which after looking at it looks more like a bug in the linter than an actual failure.

From https://ci.mirage.io/pipelines/pr-179-mirage-4-dune-universe-opam-overlays/pr-179-mirage-4-dune-universe-opam-overlays-b2b5e7e56d1a30cdd24a04504b88e24d88cef101/arm64-debian-10-4.13 (the "Mirage CI" linked in this PR at the commit b2b5e7e, click on skeleton@unix) shows me (trimmed lots of output:

- base-bytes -> (problem)
    User requested = base+dune
    Rejected candidates:
      base-bytes.base+dune: Requires ocaml >= 5.0

This is what I was referring to. Sorry for the confusion.

[Leonidas-from-XIV]

I have (unfortunately) seen a lot of Mirage CI failures before to the point of it not being a reliable predictor of consequences of merging. But looks like that has improved, because indeed both #182 and #183 showed that the revert fixed the problem and the CI was green.

@kit-ty-kate I am not sure that changing things in our fork of findlib is a good idea, getting further and further away from upstream. In ocaml/ocamlfind#52 I can see that 1.9.6 doesn't install the META files on 5.0.0, but on older versions it still leads to conflicts.

[kit-ty-kate]

I asked @kit-ty-kate for the concerns but didn't hear back

I have many things to keep track of and simply forgot to reply. Please DM me on slack instead next time.