[MLIR][Standalone] test Standalone against install distributions

[makslevental]

This PR adds two tests that exercise standalone build:

1. test full install, which tests building the standalone example using a full install;
2. test install distribution, which tests building the standalone example using DLLVM_DISTRIBUTION_COMPONENTS=... (specifically the one mentioned here).

The tests are gated by a CMake arg MLIR_RUN_STANDALONE_INSTALL_TESTS.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[makslevental]

It's worth mentioning that these tests become the slowest tests:

310.23s: MLIR :: Examples/standalone/test.install-dir.toy
13.58s: MLIR :: Examples/standalone/test.toy

Given the stubgen fiasco, I think it's worth it.

[makslevental]

Note, this PR depends on #158090 because currently (HEAD) the monolithic CI scripts do not install FileCheck, count, and not targets.

[rengolin]

I don't know much about CMake, so my comments aren't very helpful.

[rengolin]

I don't know what this does, but why only change this one and not the others?

[makslevental]

The way these bare string tool substitutions work is regex replace with some path to the actual executable. So what happens is all the run strings that look like # RUN: mlir-opt ... turn into # RUN: /install/bin/mlir-opt .... That's fine generally but it's a problem here because

-DLLVM_DISTRIBUTION_COMPONENTS="...;mlir-opt;..."

get's turned into

-DLLVM_DISTRIBUTION_COMPONENTS="...;/install/bin/mlir-opt;..."

The pre/post args function as barriers:

llvm-project/llvm/utils/lit/lit/llvm/subst.py

Lines 57 to 63 in bfe6b3b

	pre: If specified, the substitution will not find matches where
	the character immediately preceding the word-boundary that begins
	`key` is any of the characters in the string `pre`.
	post: If specified, the substitution will not find matches where
	the character immediately after the word-boundary that ends `key`
	is any of the characters specified in the string `post`.

why only change this one and not the others?

I also changed mlir-tblgen in the other lit.cfg.py. I could do it for the rest as well but that can be a separate PR.

[makslevental]

Note, I changed this to be actually ToolSubst("mlir-opt", post=";", unresolved="fatal").

[rengolin]

What do you mean by root? The GIT root? /? The build directory root?

Usually, when I do automatic installs for CI, I set it to %build/install, so that it's guaranteed to be writable by the build process and in the same filesystem (ex. the source directory may be NFS and slow).

[makslevental]

If CMAKE_INSTALL_PREFIX is empty and you do ninja install it'll try to install to /lib, /include, etc. That's what this means - that if you've enabled MLIR_RUN_STANDALONE_INSTALL_TESTS but you've not explicitly set CMAKE_INSTALL_PREFIX to be something then you probably made a mistake.

[makslevental]

It's worth mentioning that these tests become the slowest tests:

One of the two tests is probably redundant - we can just have the distribution tests because it (I believe) subsumes the full install (I'm assuming that LLVM_DISTRIBUTION_COMPONENTS runs all the same code paths as the conventional install plus some others for managing the components).

[boomanaiden154]

To me this seems more like an integration test that should run on a buildbot somewhere rather than within lit. Especially given the time it takes to run this test. I remember this test already being the longest running test for MLIR on many-core machines and thus stretching the runtime. The MLIR tests for me currently take about 10-15 seconds with this test enabled (at least on premerge). Adding a test that takes five minutes I don't think is tenable to run within premerge. That is at least a 20% increase in runtime for every single PR that tests MLIR for a single test which represents a huge amount of CI load.

Otherwise, two other lower level comments:

1. Passing through the CMAKE_INSTALL_PREFIX from the parent CMake is weird. This patch can probably do what libc++ does and install to a directory inside the build directory.
2. ninja install-distribution is probably the target you actually want to be running here.

[makslevental]

Passing through the CMAKE_INSTALL_PREFIX from the parent CMake is weird. This patch can probably do what libc++ does and install to a directory inside the build directory.

If you pass a new CMAKE_INSTALL_PREFIX you will be reconfiguring the users install directory. That is quite a disruptive UX (expecting the user to flip it back to their desired directory after running this test).

To me this seems more like an integration test that should run on a buildbot somewhere rather than within lit.

Fair enough. I can disable it by default and land a PR to zorg with MLIR_RUN_STANDALONE_INSTALL_TESTS=ON. But that means this code landed here will not be tested at time of land. Meaning we will have to iterate using premerge and then right before merge I will disable (and also not merge the commit corresponding to other CI PR).

[boomanaiden154]

If you pass a new CMAKE_INSTALL_PREFIX you will be reconfiguring the users install directory. That is quite a disruptive UX (expecting the user to flip it back to their desired directory after running this test).

I'm not saying to adjust CMAKE_INSTALL_PREFIX generally and overwrite it for the user. I'm saying we should set config.host_cmake_install_prefix to something other than what CMAKE_INSTALL_PREFIX is set to in the host build.

[makslevental]

should set config.host_cmake_install_prefix

And I'm saying that currently that will have no effect because we do not use that string to reconfigure CMAKE_INSTALL_PREFIX and that if we did that it would be bad for the user.