Skip to content

Turn off shimmed META files for OCaml 5.0+#5916

Merged
nojb merged 1 commit intoocaml:mainfrom
dra27:compiler_META
Jul 11, 2022
Merged

Turn off shimmed META files for OCaml 5.0+#5916
nojb merged 1 commit intoocaml:mainfrom
dra27:compiler_META

Conversation

@dra27
Copy link
Copy Markdown
Member

@dra27 dra27 commented Jun 23, 2022

This is an update of #5824, compatible with the latest version pushed to ocaml/ocaml#11007. The current proposal is that from OCaml 5.0.0~alpha1, the compiler installs its own META files in directories under the Standard Library directory.

The previous version was not able to guarantee the operation of Dune or opam's ocaml-system because there wasn't a mechanism for reliably determining the location of the compiler's META files. The latest revision proposes that the compiler's META are therefore always installed in the same place, with Dune and ocamlfind using that fact to change their default search paths for packages accordingly.

This PR, therefore:

  • Disables the automatic generation of the compiler's META files used for OCaml 4.x.
  • Adds the Standard Library directory to the start of default_ocamlpath (i.e. the compiler's META files cannot be overridden by other findlib packages, as in Dune's behaviour for OCaml 4.x).

There is one outstanding question, which is what to do about the bytes shim package. OCaml is not proposing installing this package, partly because it doesn't reflect an actual library (the bytes package expresses a property of the stdlib package) and also because it relates to back-porting, which is not something the compiler should directly be part of (i.e. the compiler proposes to own its packages, but the bytes package has always been owned by findlib).

There are several possible courses of action:

  1. Meta.builtins could continue to return a dummy a bytes package on 5.0
  2. bytes could be handled in a similar way to bigarray - so pruned at the libraries stanza for 5.x
  3. Do nothing, and require the user's opam files to have a dependency on base-bytes or ocamlfind in order to pull in findlib's support for the bytes package

I'd personally advocate option 3, which is what the PR presently does, because, it's daft that jbuild or dune files have ever referred to the bytes library. No version of jbuilder or Dune has ever targeted OCaml 4.01 or earlier, so adding bytes to a libraries field has always implied a no-op (unless I've missed something??). The fact it's always been a no-op makes option 2's proposal for bytes different from bigarray. Dune is now correctly handling the ability to get Bigarray in a consistent way for OCaml 4.02-5.x, but the consistent way to get Bytes for OCaml 4.02-5.x is, um, not to do anything! It's a shame to do option 1 because it's yet another compatibility shim with no sunset.

nojb
nojb reviewed Jun 23, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@nojb nojb left a comment

Choose a reason for hiding this comment

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

LGTM; I'm only wondering about the fact that the stdlib directory takes precedence over the ocamlfind/opam load path. I understand this preserves the existing behaviour, but is it the "best" order? Or perhaps we don't care?

@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Jun 24, 2022

LGTM; I'm only wondering about the fact that the stdlib directory takes precedence over the ocamlfind/opam load path. I understand this preserves the existing behaviour, but is it the "best" order? Or perhaps we don't care?

My rationale (having initially done it the other way around) came from the ocamlfind patch (and keeping Dune and ocamlfind in sync is probably sensible). It means you can't install a stdlib package by accident which overrides it one of the compiler's ones (e.g. ocamlfind install stdlib ...). In 4.x, you can't do that at all, because the package actually exists in the main site-lib. With the standard library first in the default search path, you can aim at your foot, install the package (ocamlfind warns that you probably didn't mean to), but package resolution behaves as in 4.x and you still get the stdlib's version. In both 4.x and 5.x you can use OCAMLPATH to aim at your foot to your heart's content, obviously 🙂

I'm personally happy with either.

@nojb
Copy link
Copy Markdown
Collaborator

nojb commented Jun 24, 2022

I'm personally happy with either.

Sounds like a reasonable argument, good enough for me!

@Octachron Octachron mentioned this pull request Jun 27, 2022
Closed
@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Jul 2, 2022

The upstream OCaml PR has been merged, so this is good to go. It would be nice to have a Dune release with this PR around OCaml 5.0.0~alpha1, since at the moment the runtime_events package is not available in Dune. Dune release made before the next alpha1 release do need to conflict ocaml-base-compiler.5.0.0~alpha0, but only in the opam-repository opam file.

@emillon emillon added this to the 3.4.0 milestone Jul 4, 2022
@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Jul 11, 2022

Rebased and changes added

@nojb
Copy link
Copy Markdown
Collaborator

nojb commented Jul 11, 2022

@dra27 I made the mistake of using the Github UI to rebase this branch in order to merge it (it had become out of date with main in between) and now the DCO check is not passing. Could you rebase it one more time and then we will merge it right away? Thanks!

@Octachron @dra27
use the compiler META files when available
a6a2f55 
Signed-off-by: David Allsopp <david.allsopp@metastack.com>
@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Jul 11, 2022

No problem - it's passing this time!

@nojb nojb merged commit 9ffd26f into ocaml:main Jul 11, 2022
@nojb
Copy link
Copy Markdown
Collaborator

nojb commented Jul 11, 2022

Merged, thanks!

@emillon emillon mentioned this pull request Jul 20, 2022
Merged
emillon added a commit to emillon/opam-repository that referenced this pull request Jul 20, 2022
@emillon
[new release] xdg, stdune, ordering, ocamlc-loc, fiber, dyn, dune, du…
95b4730 
…ne-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.4.0)

CHANGES:

- Make `dune describe` correctly handle overlapping implementations
  for virtual libraries (ocaml/dune#5971, fixes ocaml/dune#5747, @esope)

- Building the `@check` alias should make sure the libraries and executables
  don't have dependency cycles (ocaml/dune#5892, @rgrinberg)

- [ctypes] Add support for the `errno` parameter using the `errno_policy` field
  in the ctypes settings. (ocaml/dune#5827, @droyo)

- Fix `dune coq top` when it is invoked on files from a subdirectory of the
  directory containing the associated stanza (ocaml/dune#5784, fixes ocaml/dune#5552, @ejgallego,
  @rlepigre, @Alizter)

- Fix hint when an invalid module name is found. (ocaml/dune#5922, fixes ocaml/dune#5273, @emillon)

- The `(cat)` action now supports several files. (ocaml/dune#5928, fixes ocaml/dune#5795, @emillon)

- Dune no longer uses shimmed `META` files for OCaml 5.x, solely using the ones
  installed by the compiler. (ocaml/dune#5916, @dra27)

- Fix handling of the `(deps)` field in `(test)` stanzas when there is an
  `.expected` file. (ocaml/dune#5952, ocaml/dune#5951, fixes ocaml/dune#5950, @emillon)

- Ignore insignificant filesystem events. This stops RPC in watch mode from
  flashing errors on insignificant file system events such as changes in the
  `.git/` directory. (ocaml/dune#5953, @rgrinberg)

- Fix parsing more error messages emitted by the OCaml compiler. In
  particular, messages where the excerpt line number started with a blank
  character were skipped. (ocaml/dune#5981, @rgrinberg)

- env stanza: warn if some rules are ignored because they appear after a
  wildcard rule. (ocaml/dune#5898, fixes ocaml/dune#5886, @emillon)

- On Windows, XDG_CACHE_HOME is taken to be the `FOLDERID_InternetCache` if
  unset, and XDG_CONFIG_HOME and XDG_DATA_HOME are both taken to be
  `FOLDERID_LocalAppData` if unset.  (ocaml/dune#5943, fixes ocaml/dune#5808, @nojb)
@kit-ty-kate
Copy link
Copy Markdown
Member

kit-ty-kate commented Jul 25, 2022 •
edited
Loading

It looks like this change affected the bytes compatibility library, which is not installed by OCaml 5.0 but available as compatibility builtin in earlier versions.

#=== ERROR while compiling base64.3.5.0 =======================================#
# context              2.1.2 | linux/x86_64 | ocaml-variants.5.0.0+trunk | file:///home/opam/opam-repository
# path                 ~/.opam/5.0/.opam-switch/build/base64.3.5.0
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p base64 -j 31
# exit-code            1
# env-file             ~/.opam/log/base64-19-b34c70.env
# output-file          ~/.opam/log/base64-19-b34c70.out
### output ###
# File "src/dune", line 5, characters 12-17:
# 5 |  (libraries bytes))
#                 ^^^^^
# Error: Library "bytes" not found.
# -> required by library "base64" in _build/default/src
# -> required by _build/default/META.base64
# -> required by _build/install/default/lib/base64/META
# -> required by _build/default/base64.install
# -> required by alias install
# File "src/dune", line 14, characters 12-17:
# 14 |  (libraries bytes))
#                  ^^^^^
# Error: Library "bytes" not found.
# -> required by library "base64.rfc2045" in _build/default/src
# -> required by _build/default/META.base64
# -> required by _build/install/default/lib/base64/META
# -> required by _build/default/base64.install
# -> required by alias install

EDIT: ah, i should’ve read the description... nevermind ^^"

@emillon
Copy link
Copy Markdown
Collaborator

emillon commented Jul 26, 2022

@kit-ty-kate is there anything that needs to be fixed?

@kit-ty-kate
Copy link
Copy Markdown
Member

kit-ty-kate commented Jul 26, 2022

No, sorry i should’ve read the discussion before commenting. I think removing bytes when using OCaml 5.0 is an ok decision.

samoht added a commit to samoht/mirage-monorepo that referenced this pull request Sep 7, 2022
@samoht
Fixes for dune 3.4.1
a1cac23 
See ocaml/dune#5916 (comment)
samoht added a commit to samoht/mirage-monorepo that referenced this pull request Sep 7, 2022
@samoht
Fixes for dune 3.4.1
e7a58e1 
See ocaml/dune#5916 (comment)
@samoht samoht mentioned this pull request Sep 7, 2022
Merged
@emillon emillon mentioned this pull request Mar 22, 2023
Merged
emillon added a commit to emillon/dune that referenced this pull request Mar 22, 2023
@emillon
doc: dune reads META files from the compiler
8bbeb88 
This is the case since ocaml#5916 (3.4.0).

Signed-off-by: Etienne Millon <me@emillon.org>
emillon added a commit that referenced this pull request Mar 22, 2023
@emillon
doc: dune reads META files from the compiler (#7377)
7cd8b42 
This is the case since #5916 (3.4.0).

Signed-off-by: Etienne Millon <me@emillon.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nojb nojb nojb left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.4.0

Development

Successfully merging this pull request may close these issues.

5 participants

@dra27 @nojb @kit-ty-kate @emillon @Octachron