Relocatable ocamlfind with sprinkles - #122
Conversation
Relative paths in findlib (including in `path`) are interpreted relative the directory containing findlib.conf or findlib.conf.d (interpreting relative paths of files in findlib.conf.d relative to the directory containing findlib.conf.d rather than to the file itself allows a findlib.conf path to be moved to/from findlib.conf.d without changing its semantics).
If -sitelib is passed a relative path, this is interpreted relative to the path used for findlib.conf (i.e. the path given for -config). For example, `./configure -config $(opam var lib)/findlib.conf -sitelib .` has the same meaning as passing `-sitelib $(opam var lib)`. If Relocatable OCaml is being used, and the OCaml Standard Library is in a directory underneath `-config` (as an opam switch does, for example), and the ocamlfind binary is being installed to the same directory as the compiler, then Relocatable Findlib is enabled. In mode, `ocamlfind` searches for `findlib.conf` relative to the location of OCaml's Standard Library and `topfind` similarly locates the `-sitelib` directory relative to OCaml's Standard Library. `findlib.conf` is also configured to use relative paths.
OCaml 5.4 interprets entries beginning with an explicit ./ or ../ as being relative to the directory containing ld.conf. ocamlfind recognises this too for the DLL consistency checks while ensuring that existing explicit-relative entries are preserved when ld.conf entries are being added.
When Findlib is configured to use a shared stublibs directory (which, in particular, is the configuration opam uses), ocamlfind checks to see that the shared stublibs directory is mentioned in ld.conf. Previously, this check tries to take into account path normalisation and case-insensitive file-systems, but the check is thwarted by OCaml 5.4's new explicit-relative lines in ld.conf (for opam, it therefore can't recognise that ../stublibs refers to `$(opam var lib)/stublibs`). OCaml 4.13+ has Unix.realpath, so we detect the presence of that and use it to canonicalise the paths. This works reliably because all the directories are known to exist at the point of the check. Earlier versions (which may also use the same ld.conf syntax owing to the Relocatable OCaml backports) will continue to display the innocuous warning.
Works around cloning problems in OCaml 5.5
shym
left a comment
There was a problem hiding this comment.
Thank you very much for that PR!
I’ve only a series of small comments and suggestions, everything very light.
And beware that, to paraphrase approximately Knuth, I’ve only read through this, not tested it…
|
|
||
| #---------------------------------------------------------------------- | ||
| # For Relocatable Findlib, the path of site-lib relative to the OCaml | ||
| # Standard Library directory. For normal Findlib, equal to |
There was a problem hiding this comment.
| # Standard Library directory. For normal Findlib, equal to | |
| # Standard Library directory. For non-relocatable Findlib, equal to |
I’d like to see the relocatable version be the new normal one.
| elif [ "x$d1" = "x${d1#$d2}" ] || [ "x$d3" = "x{$d3##/}" ]; then | ||
| non_relocatable_reason="findlib.conf and the Standard Library don't share a common path" |
There was a problem hiding this comment.
| elif [ "x$d1" = "x${d1#$d2}" ] || [ "x$d3" = "x{$d3##/}" ]; then | |
| non_relocatable_reason="findlib.conf and the Standard Library don't share a common path" | |
| elif [ "x$d1" = "x${d1#$d2/}" ]; then | |
| non_relocatable_reason="findlib.conf and the Standard Library don't share a common path" |
I think that combines both tests, namely detects whether stdlib is in a strict subdirectory of where findlib.conf will be located, or I’m mistaken?
I read that condition as a bit more restricted that the reason suggests, if that’s the case.
(Side note, this can do probably something funny if findlib.conf’s path is using pattern characters but, oh, well)
| ocaml_config_from_stdlib="$(echo "$d3" | sed -e 's|[^/]*[^/]|..|g')" | ||
| ocaml_config_from_stdlib="${ocaml_config_from_stdlib##/}" | ||
| ocaml_sitelib_from_stdlib="$ocaml_config_from_stdlib" |
There was a problem hiding this comment.
| ocaml_config_from_stdlib="$(echo "$d3" | sed -e 's|[^/]*[^/]|..|g')" | |
| ocaml_config_from_stdlib="${ocaml_config_from_stdlib##/}" | |
| ocaml_sitelib_from_stdlib="$ocaml_config_from_stdlib" | |
| ocaml_sitelib_from_stdlib="$(echo "$d3" | sed -e 's|[^/]*[^/]|..|g')" | |
| ocaml_sitelib_from_stdlib="${ocaml_sitelib_from_stdlib#/}" |
I suggest to get rid of the temporary new var ocaml_sitelib_from_stdlib and to use the shortest prefix removal: when it’s a fixed-length string, I find it a bit weird to ask for the longest one.
| ocaml_sitelib_from_stdlib="${ocaml_sitelib_from_stdlib}${dir_sep}${ocamlfind_conf_destdir}" | ||
| fi | ||
| # d3 should be of the form "/.../.../...". Convert all the | ||
| # characters between slashes to Filaname.dirname and then remove |
There was a problem hiding this comment.
| # characters between slashes to Filaname.dirname and then remove | |
| # characters between slashes to Filename.dirname and then remove |
| # d3 should be of the form "/.../.../...". Convert all the | ||
| # characters between slashes to Filaname.dirname and then remove | ||
| # the slashes. macOS sed doesn't support EREs. | ||
| ocamlfind_conf_from_stdlib="fun p -> $(echo "$d3" | sed -e 's|[^/]*[^/]|Filename.dirname (|g;s|/||g')p$(echo "$d3" | sed -e 's|[^/]*[^/]|)|g;s|/||g')" |
There was a problem hiding this comment.
| ocamlfind_conf_from_stdlib="fun p -> $(echo "$d3" | sed -e 's|[^/]*[^/]|Filename.dirname (|g;s|/||g')p$(echo "$d3" | sed -e 's|[^/]*[^/]|)|g;s|/||g')" | |
| ocamlfind_conf_from_stdlib="fun p -> $(echo "$d3" | sed -e 's|//*[^/]*[^/]|Filename.dirname (|g')p$(echo "$d3" | sed -e 's|//*[^/]*[^/]|)|g')" |
One s is enough, isn’t it?
| $(SH) $(TOP)/tools/patch '@RELATIVE_PATHS@' '$(RELATIVE_PATHS)' | \ | ||
| sed -e 's;@AUTOLINK@;$(OCAML_AUTOLINK);g' \ | ||
| -e 's;@SYSTEM@;$(SYSTEM);g' \ | ||
| -e 's;@CONFIG_FROM_STDLIB@;$(CONFIG_FROM_STDLIB);g' \ |
There was a problem hiding this comment.
| -e 's;@CONFIG_FROM_STDLIB@;$(CONFIG_FROM_STDLIB);g' \ | |
| -e 's;@CONFIG_FROM_STDLIB@;$(CONFIG_FROM_STDLIB);g' \ |
to fix alignment
| let rec split acc dir = | ||
| let dirname = Filename.dirname dir in | ||
| let basename = Filename.basename dir in | ||
| if dirname = Filename.current_dir_name then | ||
| List.fold_left Filename.concat "" (basename :: acc) | ||
| else | ||
| split (Filename.basename dir :: acc) dirname in |
There was a problem hiding this comment.
Does split do more than trimming an initial ./ (in a portable way)? (ie, it might be worth a comment to explain it, and maybe rename it trim_dotslash, as the result is re-concatenated)
(And I wondered about a non-recursive function then, even if the depth is probably quite limited, and I understand this is annoying to cover the case of a .// prefix but this annoying case could be just ignored I’d say)
| since OCaml 5.4. The interpretation of non-absolute lines in ld.conf | ||
| prior to OCaml 5.4 was not useful, so this behaviour is done without |
| (** Entries in ld.conf *) | ||
| type ldconf_entry = { | ||
| raw: string; (** Raw entry line *) | ||
| eff: string (** Evaluated entry line *) |
There was a problem hiding this comment.
| eff: string (** Evaluated entry line *) | |
| eff: string | |
| (** Effective entry line with explicit-relative paths resolved to absolute | |
| paths *) |
| run: | | ||
| first="$(opam var bin | tr -d '\r')/ocamlfind${{ runner.os == 'Windows' && '.exe' || '' }}" | ||
| second="$(opam var --switch=second bin | tr -d '\r')/ocamlfind${{ runner.os == 'Windows' && '.exe' || '' }}" |
There was a problem hiding this comment.
| run: | | |
| first="$(opam var bin | tr -d '\r')/ocamlfind${{ runner.os == 'Windows' && '.exe' || '' }}" | |
| second="$(opam var --switch=second bin | tr -d '\r')/ocamlfind${{ runner.os == 'Windows' && '.exe' || '' }}" | |
| env: | |
| EXE: ${{ runner.os == 'Windows' && '.exe' || '' }} | |
| run: | | |
| first="$(opam var bin | tr -d '\r')/ocamlfind$EXE" | |
| second="$(opam var --switch=second bin | tr -d '\r')/ocamlfind$EXE" |
maybe, if only to shorten lines?
|
@dra27 Is there anything I can do to help get this merged, and then released? |
This PR both adapts ocamlfind for OCaml 5.5 and also extends it to be relocatable itself. Relocatable in this instance is according to my definition in ocaml/RFCs#53 - in practice, this means that where the system toolchain supports it, the installation of findlib to two different prefixes for the same compiler configuration should be byte-for-byte identical.
As with the finest of wines, it is best reviewed commit-by-commit:
topfindby instead being able to generate the two files which Relocatable ocamlfind #72 merged. This paves the way for there being three variants. The trick instead is that lines which should be present in OCaml 4.00+ are prefixed4x:findlib.confto interpret relative paths. As noted in the commit message, paths are interpreted relative tofindlib.confor, for files loaded fromfindlib.conf.d, relative to the directory containingfindlib.conf.d.configureto support being configured for potentially relocatable builds. This is done by passing a relative path to-sitelib, which is interpreted relative to the location given for-config. That sounds rather complicated, but as can be seen in theopamfile, it just means that where before we configured with-sitelib libwe now configure with-sitelib ..configurethen computes the relative path of the directory containingfindlib.conffrom OCaml's Standard Library.ld.conf.Unix.realpathand uses it on the resulting paths for comparisons.