Conversation
8ef6b88 to
930e128
Compare
930e128 to
5824712
Compare
eac09ac to
d72f770
Compare
d72f770 to
640e317
Compare
f1e3683 to
d365fb1
Compare
| testsuite/tools/test_in_prefi%: CAMLC = $(BEST_OCAMLC) $(STDLIBFLAGS) | ||
|
|
||
| testsuite/tools/test_in_prefix$(EXE): OC_BYTECODE_LINKFLAGS += -custom | ||
|
|
||
| testsuite/tools/test_in_prefi%: CAMLOPT = $(BEST_OCAMLOPT) $(STDLIBFLAGS) |
There was a problem hiding this comment.
| testsuite/tools/test_in_prefi%: CAMLC = $(BEST_OCAMLC) $(STDLIBFLAGS) | |
| testsuite/tools/test_in_prefix$(EXE): OC_BYTECODE_LINKFLAGS += -custom | |
| testsuite/tools/test_in_prefi%: CAMLOPT = $(BEST_OCAMLOPT) $(STDLIBFLAGS) | |
| testsuite/tools/test_in_prefix%: CAMLC = $(BEST_OCAMLC) $(STDLIBFLAGS) | |
| testsuite/tools/test_in_prefix$(EXE): OC_BYTECODE_LINKFLAGS += -custom | |
| testsuite/tools/test_in_prefix%: CAMLOPT = $(BEST_OCAMLOPT) $(STDLIBFLAGS) |
Why drop the leading O in OCAMLC and OCAMLOPT? or is it always spelled this way?
There was a problem hiding this comment.
Why drop the leading
OinOCAMLCandOCAMLOPT? or is it always spelled this way?
@shindere definitely wants to change this (the names are just legacy from Caml Light and Caml Special Light!)... I think the idea was to wait until the Makefiles are all merged... renaming Makefile variables is of course a tricky process to get right (all the recursive calls, etc.)
There was a problem hiding this comment.
Makefile patterns cannot match the empty string, which is why the x is omitted - this rule is intended to match both testsuite/tools/test_in_prefix and testsuite/tools/test_in_prefix.opt and the pattern testsuite/tools/test_in_prefix% doesn't match the first one.
cf. the same trick in stdlib/Makefile:
Lines 93 to 94 in f28bfbc
where the pattern is
%-launch-info (to match both runtime-launch-info and target_runtime-launch-info)
There was a problem hiding this comment.
Interesting Makefile craziness... thanks!
runtime/caml/startup_aux.h
Outdated
|
|
||
| #ifdef CAML_INTERNALS | ||
|
|
||
| #include <stdbool.h> |
There was a problem hiding this comment.
Is including stdbool.h fine here w.r.t (the absence of) namespaces and C++ ABI compatibility because it's guarded by CAML_INTERNALS?
There was a problem hiding this comment.
Instinctively I'd say yes, but is there an immediately cleaner way I could put it? The other option would be just to use an int, of course 😉
There was a problem hiding this comment.
I think it's fine, we're not guarding things under CAML_INTERNALS with extern "C" for C++ compat either.
| (e.g. /usr/bin and /usr/lib/ocaml share a common root of /usr, but /bin and | ||
| /lib/ocaml do not share a common root directory). The test needs to be able to | ||
| rename this common root directory by adding a suffix ".new". For this reasoa, |
There was a problem hiding this comment.
| (e.g. /usr/bin and /usr/lib/ocaml share a common root of /usr, but /bin and | |
| /lib/ocaml do not share a common root directory). The test needs to be able to | |
| rename this common root directory by adding a suffix ".new". For this reasoa, | |
| (e.g. `/usr/bin` and `/usr/lib/ocaml` share a common root of `/usr`, but `/bin` and | |
| `/lib/ocaml` do not share a common root directory). The test needs to be able to | |
| rename this common root directory by adding a suffix ".new". For this reason, |
There was a problem hiding this comment.
I had a thought (just to explain it), that putting these paths as code wasn't aiding readability (either rendered or as text) - i.e. I was finding "/usr/bin and /usr/lib/ocaml" easier to read (because each path is "introduced" by a slash anyway) than the version where the backticks are kinda getting in the way?
|
David Allsopp (2024/12/27 02:02 -0800):
@dra27 commented on this pull request.
> +testsuite/tools/test_in_prefi%: CAMLC = $(BEST_OCAMLC) $(STDLIBFLAGS)
+
+testsuite/tools/test_in_prefix$(EXE): OC_BYTECODE_LINKFLAGS += -custom
+
+testsuite/tools/test_in_prefi%: CAMLOPT = $(BEST_OCAMLOPT) $(STDLIBFLAGS)
> Why drop the leading `O` in `OCAMLC` and `OCAMLOPT`? or is it always spelled this way?
@shindere definitely wants to change this (the names are just legacy
from Caml Light and Caml Special Light!)... I _think_ the idea was to
wait until the `Makefile`s are all merged... renaming Makefile
variables is of course a tricky process to get right (all the
recursive calls, etc.)
I can only ocnfirm, @dra27 explained things in an excellent way.
|
|
Markdown fixed; and... |
|
... rebased |
In native mode, same as Sys.executable_name, in bytecode, the path to the interpreter executing Sys.executable_name, which may not be the same from the same file.
…em` (ocaml#14028) * Remove stale comments * Remove dead constructor: Scoping_let_module * Remove eqparsetree.ml
Update FlexDLL to 0.44 and reenable lib-dynlink-domains test on Windows
…b-musl tests: native-debugger: sanitize GDB+musl output
Document [row_more] and [row_fixed].
Reviewed-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>
…-variables Fix ocamltest variable handling
[minor] use a record in Compenv.process_deferred_actions
Add `Sys.runtime_executable` and `caml_sys_proc_self_exe`
Previously, Compmisc.init_path initialised the load path using Config.standard_library, but this can now be altered via an optional ?standard_library argument. This is used internally when testing compiler installations in order to allow Ccomp.call_linker to be used.
Provides a copy of the DLL search path for the test harness.
Expose the ocaml_cc_vendor variable computed in aclocal.m4
This PR implements a test harness for Relocatable OCaml (
ocaml/RFCs#53). The RFC defines "Relocatable" as meaning:--prefixargument passed toconfigure) or the working directory in which the compiler was built (the build path).Rather than building two compilers, the first property can be checked to a sufficient degree for Relocatable OCaml by searching the installed artefacts for prefix and build path. The second and third properties can be simultaneously checked by moving the compiler from prefix to a new location, and then running a series of tests to ensure that that compiler works.
Obviously, at present OCaml cannot be configured in a way which satisfies these properties, but it is of course possible to move a compiler to a new location and change the way the compiler is invoked in order to make it work. The harness in this PR implements this approach. Everything is thus tested with a combination of known failures and shims (such as setting
OCAMLLIB, or explicitly invoking bytecode binaries withocamlrun). The three further PRs which make up the proposed changes for Relocatable OCaml are based on top of this PR, and therefore update this test harness with the revised behaviour.There are two key differences between this test harness and the main ocamltest-based testsuite:
A consequence of the "in-prefix" part is that this is not a test that should be run by default and the fact it needs to operate outside the build tree has led to an additional harness, rather than additional features in ocamltest.
The harness itself has revealed various bugs not related to Relocatable OCaml at all (cf.
ocaml/flexdll#146,#13496,#13520,#13638,#13692and#13693, in addition to a fault in the partial linker alluded to#13692which will be fixed in a subsequent PR)The tests performed are covered in
testsuite/in_prefix/README.md.In terms of review, the PR includes the first commit of
#13728, which means all the changes inruntime/can be ignored (as they all come from that commit). The next five commits alter the compiler:Ccomp.call_linker(I mean really, really, simpler - I tried!). However, in order to that, the test harness needs to be able to control slightly more precisely the value ofConfig.standard_libraryas interpreted byCcomp.call_linker. Having tried various approaches, the least invasive to compiler-libs seems to be to generaliseCompmisc.init_pathvia a newCompmisc.reinit_path. There are other ways of doing this, but this one I think is the simplest that doesn't involve duplicating code fromutils/ccomp.mldirectly in the test harness.Test_ld_conftests the handling by bothocamlrunandocamlcofld.conf. This test needs to be able the content of thesearch_pathvariable inDllfor which I have added aDll.search_pathfunction.string, and for this reason I've addedMisc.Stdlib.String.to_utf_8_seqand, as always, I'm open to suggestions for a better name.runtime-launch-info, so I've exposedBytelink.read_runtime_launch_infoand its associated types.ocaml_cc_vendorvariable computed inconfigureasConfig.cc_vendorThose opening commits clearly change the compiler, and need to be reviewed as such. The last two alter the testsuite only, and I'd beseech be reviewed from a clarity perspective only! In particular, I have 12 branches based on top of this all of which alter this test harness, so please bear in mind that while adjustments are of course possible, they come at a cost. Note also that there are a few places where behaviour is not reachable or spelled out slightly obtusely: for example, the
Default_ocamlcconstructor includes an ignoredlaunch_methodvalue, and several functions have unused parameters escaped with_. These are all altered in subsequent PRs where the reason for having this differentiation becomes (hopefully) clearer.As far as possible, the harness learns of the configuration statically, rather than by probing the installation. In particular, this allows errors in the installation process to be detected (i.e. if the toplevel test simply tested all the .cma files it found in libdir, it would not catch the failure to have installed one of them - hence
$(OTHERLIBRARIES)is passed on the command line instead. Most things are statically placed in theConfigmodule -BINDIRandLIBDIRare passed on the command line, becauseConfig.bindirandConfig.standard_librarywon't have the same meaning in subsequent PRs. Especially given that the need to be able to useCcomp.call_linker, I've not written this test to be used in a cross-compiled manner by which I mean it can test an installation which is a cross compiler, but the test harness must itself be compiled with the same (host) configuration as the compiler it's testing.The final commit plumbs the tests into CI - it (should!) be passing both on GitHub Actions here and has also passed precheck (although I can't at present schedule a run of this precise version of the PR, as precheck seems to be down). It's also passing an even wider text matrix including Cygwin and multiple different shebang/executable/static/minimal tests in #175.
The harness code itself is structured with utility functions in module
Harnessand information derived about the C compiler and platform inToolchain. ModuleEnvironment(and in particularEnvironment.run_process) encapsulates the configuration of the configuration of the compiler installation.CmdlineandTest_in_prefixdrive the entire process withTestRelocation,TestBytecodeBinaries,TestDynlink,TestLinkModes,TestTopLevelandTest_ld_confcontaining the implementations of the six tests.