Page MenuHomePhabricator

Make it possible for lit.site.cfg to contain relative paths, and use it for llvm and clang
ClosedPublic

Authored by thakis on Mar 31 2020, 4:17 PM.

Details

Summary

Currently, all generated lit.site.cfg files contain absolute paths.

This makes it impossible to build on one machine, and then transfer the build output
to another machine for test execution. Being able to do this is useful for several use cases:

  1. When running tests on an ARM machine, it would be possible to build on a fast x86 machine and then copy build artifacts over after building.
  1. It allows running several test suites (clang, llvm, lld) on 3 different machines, reducing test time from sum(each test suite time) to max(each test suite time).

This patch makes it possible to pass a list of variables that should be relative in the
generated lit.site.cfg.py file to configure_lit_site_cfg(). The lit.site.cfg.py.in file needs
to call path() on these variables, so that the paths are converted to absolute form
at lit start time.

The testers would have to have an LLVM checkout at the same revision, and the build
dir would have to be at the same relative path as on the builder.

This does not yet cover how to figure out which files to copy from the builder machine
to the tester machines. (One idea is to look at the --graphviz=test.dot output and
copy all inputs of the check-llvm target.)

Diff Detail

Event Timeline

thakis created this revision.Mar 31 2020, 4:17 PM
hans accepted this revision.Apr 2 2020, 2:44 AM
hans added a subscriber: hans.

Looks reasonable to me. Just some nits.

llvm/cmake/modules/AddLLVM.cmake
1483

Maybe comment what this does?

1485

typo: Passign -> Passing

1495

Nit: maybe len_vars to match the 'PATHS' name?

This revision is now accepted and ready to land.Apr 2 2020, 2:44 AM
thakis marked 4 inline comments as done.Apr 2 2020, 10:39 AM

Thanks!

llvm/cmake/modules/AddLLVM.cmake
1483

Turns out the saving and undoing isn't needed since cmake copies all variables to new scopes, and this changes just the local copy. So this is gone now.

Just trying to wrap my head around this. Basically, this makes lit.cfg files relocatable, however doesn't everything else need to be relocatable too for anything to work? For example, if you set the rpath on some executable, that needs to be relative too, right?

So, assuming this patch, the basic idea is:

  1. Build on machine A
  2. Copy the whole build directory and LLVM tree to machine B
  3. Run lit -sv <whatever> on machine B -- everything should work if all paths in lit.cfg and all paths embedded in build artifacts are relative to the build directory.

It also requires that machine B is able to execute all the tests, so for example for the Clang test suite it needs to be able to run Clang, run FileCheck, etc. Drawing a parallel with the libc++ test suite, this would require machine B to be able to build executables for machine B (from the .pass.cpp sources) and run them on machine B.

Is that the idea?

thakis marked an inline comment as done.EditedApr 2 2020, 11:05 AM

I somehow edited my comment instead of replying to it. Editing again to restore what I deleted:

Yes, that's all correct.

  1. Copy the whole build directory and LLVM tree to machine B

(Except for implementation tweaks. For example, the test machine could possibly pull the llvm tree at the same revision that the builder builds at while the builder's still building, instead of copying (parts of) the LLVM tree from the builder after a build.)

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 2 2020, 11:23 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Thanks!

Hi,

After the series of changes you have done, I am experiencing a build error on Windows. These are some of the errors:

  • Attribute ignored -- Loadable modules not supported on this platform.

Traceback (most recent call last):

File "<string>", line 1, in <module>
File "<string>", line 1, in <genexpr>
File "C:\ProgramData\Python27\lib\ntpath.py", line 529, in relpath
  % (path_prefix, start_prefix))

ValueError: path is on drive X:, start on drive E:
CMake Error at X:/llvm-root/llvm-project/llvm/cmake/modules/AddLLVM.cmake:1508 (message):

PATHS lengths got confused

Call Stack (most recent call first):

X:/llvm-root/llvm-project/clang/test/CMakeLists.txt:33 (configure_lit_site_cfg)

CMake Error at X:/llvm-root/llvm-project/llvm/cmake/modules/AddLLVM.cmake:1517 (list):

list GET given empty list

Call Stack (most recent call first):

X:/llvm-root/llvm-project/clang/test/CMakeLists.txt:33 (configure_lit_site_cfg)

CMake Error at X:/llvm-root/llvm-project/llvm/cmake/modules/AddLLVM.cmake:1517 (list):

list GET given empty list

Call Stack (most recent call first):

X:/llvm-root/llvm-project/clang/test/CMakeLists.txt:33 (configure_lit_site_cfg)

CMake Error at X:/llvm-root/llvm-project/llvm/cmake/modules/AddLLVM.cmake:1517 (list):

list GET given empty list

Call Stack (most recent call first):

X:/llvm-root/llvm-project/clang/test/CMakeLists.txt:33 (configure_lit_site_cfg)

CMake Error at X:/llvm-root/llvm-project/llvm/cmake/modules/AddLLVM.cmake:1517 (list):

list GET given empty list

Call Stack (most recent call first):

X:/llvm-root/llvm-project/clang/test/CMakeLists.txt:33 (configure_lit_site_cfg)

My CMake command line is:

E:\llvm-root\builds\win\debug\x64>cmake -G"Visual Studio 15 2017 Win64" -Thost=x64 -DCMAKE_BUILD_TYPE=Debug -DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-pc-linux-gnu
-DLLVM_ENABLE_PROJECTS="clang" x:\llvm-root\llvm-project\llvm -DLLVM_ENABLE_ZLIB=OFF

Drive E: contains the build files
Drive X: contains the llvm sources

The only way to have a successful build is to revert : ab11b9eefa16661017c2c7b3b34c46b069f43fb7

Thanks

The following patch fixes my issues on Windows, but I haven't tested that it doesn't break anything else:

diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index 91bec7d8081..f630af211fd 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -1495,7 +1495,7 @@ function(configure_lit_site_cfg site_in site_out)
     string(REPLACE ";" "\\;" ARG_PATH_VALUES_ESCAPED "${ARG_PATH_VALUES}")
     get_filename_component(OUTPUT_DIR ${site_out} DIRECTORY)
     execute_process(COMMAND "${PYTHON_EXECUTABLE}" "-c"
-      "import os, sys; sys.stdout.write(';'.join(os.path.relpath(p, sys.argv[1]).replace(os.sep, '/') if p else '' for p in sys.argv[2].split(';')))"
+      "import os, sys; drive = os.path.splitdrive(sys.argv[1])[0]; sys.stdout.write(';'.join('' if not p else p if os.path.splitdrive(p)[0] != drive else os.path.relpath(p, sys.argv[1]).replace(os.sep, '/') for p in sys.argv[2].split(';')))"
       ${OUTPUT_DIR}
       ${ARG_PATH_VALUES_ESCAPED}
       OUTPUT_VARIABLE ARG_PATH_VALUES_RELATIVE)
grimar added a subscriber: grimar.Apr 3 2020, 6:48 AM

The following patch fixes my issues on Windows, but I haven't tested that it doesn't break anything else:

diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index 91bec7d8081..f630af211fd 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -1495,7 +1495,7 @@ function(configure_lit_site_cfg site_in site_out)
     string(REPLACE ";" "\\;" ARG_PATH_VALUES_ESCAPED "${ARG_PATH_VALUES}")
     get_filename_component(OUTPUT_DIR ${site_out} DIRECTORY)
     execute_process(COMMAND "${PYTHON_EXECUTABLE}" "-c"
-      "import os, sys; sys.stdout.write(';'.join(os.path.relpath(p, sys.argv[1]).replace(os.sep, '/') if p else '' for p in sys.argv[2].split(';')))"
+      "import os, sys; drive = os.path.splitdrive(sys.argv[1])[0]; sys.stdout.write(';'.join('' if not p else p if os.path.splitdrive(p)[0] != drive else os.path.relpath(p, sys.argv[1]).replace(os.sep, '/') for p in sys.argv[2].split(';')))"
       ${OUTPUT_DIR}
       ${ARG_PATH_VALUES_ESCAPED}
       OUTPUT_VARIABLE ARG_PATH_VALUES_RELATIVE)

This helped me, thanks for posting this! (I've just updated and faced the build issue on windows. Seems it doesn't like that I build it on D: drive)

grimar, andrewng: You both have checkout and build on different drives too, yes?

I pushed Andrew's fix (thanks!) (with minor formatting tweaks) in dbb0d8ecb3a024bd6817ebd8ad8c5c199a51d933 . Let me know if you still see issues.

rsmith added a subscriber: rsmith.Apr 3 2020, 7:49 PM

This has broken my ability to run the check-clang target on Linux. There are symlinks in the path from which I run my builds, and this patch is computing incorrect relative paths in that situation.

rsmith added a comment.Apr 3 2020, 7:54 PM

This has broken my ability to run the check-clang target on Linux. There are symlinks in the path from which I run my builds, and this patch is computing incorrect relative paths in that situation.

This patch appears to fix the problem:

diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index e0ceb364c3e..10010ce8caa 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -1505,7 +1505,7 @@ drive = os.path.splitdrive(sys.argv[1])[0]\n
 def relpath(p):\n
     if not p: return ''\n
     if os.path.splitdrive(p)[0] != drive: return p\n
-    return os.path.relpath(p, sys.argv[1]).replace(os.sep, '/')\n
+    return os.path.relpath(os.path.realpath(p), os.path.realpath(sys.argv[1])).replace(os.sep, '/')\n
 sys.stdout.write(';'.join(relpath(p) for p in sys.argv[2].split(';')))"
       ${OUTPUT_DIR}
       ${ARG_PATH_VALUES_ESCAPED}
rsmith added a comment.Apr 3 2020, 8:09 PM

This has broken my ability to run the check-clang target on Linux. There are symlinks in the path from which I run my builds, and this patch is computing incorrect relative paths in that situation.

This patch appears to fix the problem:

Turns out that's only a partial fix (it fixes ninja clang-check but not the general problem). Running lit from inside the source directory is still broken. Eg, this used to work:

clang/test$ $BUILDDIR/bin/llvm-lit SemaCXX/constant-expression.cpp

... and doesn't work any more because the relative paths in lit.site.cfg.py don't seem to resolve to the right places. Please can you fix or revert?

I pushed Andrew's fix (thanks!) (with minor formatting tweaks) in dbb0d8ecb3a024bd6817ebd8ad8c5c199a51d933 . Let me know if you still see issues.

It fixes the Windows issues when building from different drive. Thanks

This has broken my ability to run the check-clang target on Linux. There are symlinks in the path from which I run my builds, and this patch is computing incorrect relative paths in that situation.

This patch appears to fix the problem:

Turns out that's only a partial fix (it fixes ninja clang-check but not the general problem). Running lit from inside the source directory is still broken. Eg, this used to work:

clang/test$ $BUILDDIR/bin/llvm-lit SemaCXX/constant-expression.cpp

... and doesn't work any more because the relative paths in lit.site.cfg.py don't seem to resolve to the right places. Please can you fix or revert?

Can you describe your symlink setup in enough detail that I can recreate it locally please?

Can you describe your symlink setup in enough detail that I can recreate it locally please?

I've landed a workaround that should make things go for you again in 7db64e202f9. I'd still be curious to hear details about your setup, so that I can see if I can make it work better. (Right now, this now bails out when it sees a symlink and keeps an absolute path as before, which is a bit lame.)

rsmith added a comment.Apr 4 2020, 9:28 PM

This has broken my ability to run the check-clang target on Linux. There are symlinks in the path from which I run my builds, and this patch is computing incorrect relative paths in that situation.

This patch appears to fix the problem:

Turns out that's only a partial fix (it fixes ninja clang-check but not the general problem). Running lit from inside the source directory is still broken. Eg, this used to work:

clang/test$ $BUILDDIR/bin/llvm-lit SemaCXX/constant-expression.cpp

... and doesn't work any more because the relative paths in lit.site.cfg.py don't seem to resolve to the right places. Please can you fix or revert?

Can you describe your symlink setup in enough detail that I can recreate it locally please?

Two directories $s and $d within $HOME.

Source git checkout in $s
Build dir is $d/build
Symlink $d/src -> $s
Symlink $s/clang/build -> $d/build
Symlink $c -> $s/clang

Configured from $d/build with cmake ../src
Actual build is run from $c/build
Manual lit commands are run from $c/test

I can provide you with the scripts I use to create my checkouts, symlinks, and to run cmake if you like.

grimar added a comment.EditedApr 6 2020, 2:16 AM

grimar, andrewng: You both have checkout and build on different drives too, yes?

Nope. I think my configuration is just normal, except I do not use "C:".
My checkout is "D:\Work3\LLVM\llvm-project" and the build folder is inside: "D:\Work3\LLVM\llvm-project\build".

(Just in case, to clarify: I have no issues with building the todays LLVM).

grimar, andrewng: You both have checkout and build on different drives too, yes?

Sorry for not replying earlier. Yes, for most of my setups I have the source checkout on one drive and build output on another drive if available.

Thanks for committing the Windows fix.

Cheers,
Andrew

It does not work on Windows (msbuild) for me, because ${pathlist_escaped} contains paths like %(build_mode)s/bin (caused by set_llvm_build_mode). This seems to break something so that python process does not produce any output at all: I checked both OUTPUT_VARIABLE and ERROR_VARIABLE and they were empty on any invocation of execute_process. @thakis, do you have any ideas what might be wrong?

FWIW, the code does not seem to work as expected for multi-configuration environments, since the %(build_mode) substitution is not done yet and the paths are not real filesystem paths.

Herald added a project: Restricted Project. · View Herald TranscriptMay 26 2020, 6:02 PM

It does not work on Windows (msbuild) for me, because ${pathlist_escaped} contains paths like %(build_mode)s/bin (caused by set_llvm_build_mode). This seems to break something so that python process does not produce any output at all: I checked both OUTPUT_VARIABLE and ERROR_VARIABLE and they were empty on any invocation of execute_process. @thakis, do you have any ideas what might be wrong?

FWIW, the code does not seem to work as expected for multi-configuration environments, since the %(build_mode) substitution is not done yet and the paths are not real filesystem paths.

Please ignore this. It turned out the issue was in a python wrapper stuck in the middle.