[PATCH] D51561: [CMake] Add support for unittests that have input files

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 5 13:58:15 PDT 2018


rnk added inline comments.


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:1126
+    TARGET ${test_name}
+    POST_BUILD
+    COMMAND "${CMAKE_COMMAND}" -E make_directory ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/Inputs
----------------
zturner wrote:
> zturner wrote:
> > rnk wrote:
> > > These post build commands will only run when the unit test target rebuilds, which means changes to the inputs won't be reflected in the copied files if you edit an input, rebuild, and re-run the test.
> > > 
> > > Would it be better to embed a path to the Inputs directory in the source directory? That's how lit tests work; they don't copy inputs to the output directory. I don't see why unit tests need to be independent from the source directory.
> > How do you envision this working?  What path would be embedded exactly?  Obviously we can't hardcode anything as different peoples' machines are configured differently.  lit tests have the advantage that they have a lit.cfg file, so it can use the top-level lit.cfg to find the source directory at runtime.  Unittests have no such analogue that I'm aware of.
> FWIW, I think you can set the `DEPENDS` CMake property on this custom command so that if you change the input it will re-run.  I didn't do that here (unittest inputs tend to be binaries that change almost never), but it seems possible. 
The alternative I came up with over lunch was to have CMake write some kind of file next to the .exe, like `unittest_input_path.txt`, and then have `getInputFileDirectory()` read that file and return the path inside it. That could point to the source directory. It might also do a sanity check to ensure the directory exists.

If the DEPENDS trick does what we want, sounds good, but I think this text file is going to be simpler. No macro definitions, no calls to `configure_file`, just runtime things.


https://reviews.llvm.org/D51561





More information about the llvm-commits mailing list