testsuite: stabilize internal type identifiers in expect tests

[Octachron]

Currently, the testsuite fails after a make install due to a shift in internal type identifiers in a handful of expect tests.

This is a subtle "bug" introduced by #11199 which made the toplevel reads the compiler-libs version of Topdirs when available in the installation directory of the compiler. Unfortunately, reading or not a cmi file modifies the internal type identifiers used by the typechecker. Consequently, the tests in the testsuite that were relying on those internal identifiers had their result dependent on the installation status of the compiler.

This PR fixes this issue by ensuring that expect_test reads topdirs.cmi a fixed number of time, independently of the installation status of the compiler.

[Octachron]

Note that this PR is only short-term fix which has the advantages of only updating expect_test.ml and the affected tests. I expect that we may want to revisit the use of Topcommon.load_topdirs_signature at at a later stage.

[gasche]

This is an ugly hack, but it's better than unreliable tests. Approved.

[garrigue]

I'm not sure I understand how this works.
I would have expected priority to be given to the in tree file, but apparently priority is given to the installed one.
What happens if the installed file is obsolete?

[gasche]

Thinking about this more, I don't understand either. The code that we had before (still around, right after this function is called) is:

ocaml/testsuite/tools/expect_test.ml

Lines 338 to 350 in f2df535

	if not !Clflags.no_std_include then begin
	match !repo_root with
	| None -> ()
	| Some dir ->
	(* If we pass [-repo-root], use the stdlib from inside the
	compiler, not the installed one. We use
	[Compenv.last_include_dirs] to make sure that the stdlib
	directory is the last one. *)
	Clflags.no_std_include := true;
	Compenv.last_include_dirs := [Filename.concat dir "stdlib"]
	end;
	Compmisc.init_path ~auto_include:Load_path.no_auto_include ();
	Toploop.initialize_toplevel_env ();

Unless I am mistaken, ocamltest always call expect_test with the -repo-root option

ocaml/ocamltest/ocaml_actions.ml

Lines 772 to 784 in f2df535

	let run_expect_once input_file principal log env =
	let expect_flags = Sys.safe_getenv "EXPECT_FLAGS" in
	let repo_root = "-repo-root " ^ Ocaml_directories.srcdir in
	let principal_flag = if principal then "-principal" else "" in
	let commandline =
	[
	Ocaml_commands.ocamlrun_expect_test;
	expect_flags;
	flags env;
	repo_root;
	principal_flag;
	input_file
	] in

so we should always have !Clflags_no_std_include = true, and I don't understand why the installed version of topdirs would ever get read by the environment-initialization code below

ocaml/testsuite/tools/expect_test.ml

Lines 349 to 350 in f2df535

	Compmisc.init_path ~auto_include:Load_path.no_auto_include ();
	Toploop.initialize_toplevel_env ();

[Octachron]

What happens is that none of the topdirs.cmi files take priority currently: both files are always loaded, and inconsistent interfaces result in a Consistbl.Make(Module_name).Inconsistency("Topdirs", ...) error. This hack make ensure that we are always loading twice topdirs.cmi.

[dra27]

The same issue affects the runtop target for launching a development toplevel. Definitely a bug to fix during the 5.0-cycle, but only affecting the development tree.

[gasche]

Thanks to @garrigue I realize that I also have questions about this PR. I would like to see the situation clarified (maybe there is a nicer fix than @Octachron's proposal?), but a working CI/testsuite is arguably even more important than technical debt in expect_test for now, so I propose to merge now and trust @Octachron to clarify things later.

[Octachron]

This is PR is indeed only intended as a bug-fixing temporary hack.
Now that the alpha release of OCaml 5.0 is out of my todo list, I will be working on a proper fix.