Fix DynamicLibraryTests build on Windows when LLVM_EXPORT_SYMBOLS_FOR_PLUGINS is ON

[michaelplatings]

Summary

extract_symbols.py (introduced in D18826) expects all of its library arguments to be in the same directory - typically <config>/lib. DynamicLibraryLib.lib is instead to be found in lib/<config>.
This patch intended to make DynamicLibraryLib.lib be created in <config>/lib alongside most of the other libraries.

I previously tried passing absolute paths to extract_symbols.py but this generated command lines that were too long for Visual Studio 2015: D54587

[marsupial]

I'm not a fan of writing test stuff into <buildroot>/lib.

Is this the only way the test will pass on Windows?
Is fixing extract_symbols.py to handle whatever is needed a better option?

[michaelplatings]

Thanks for the review Frederich.

I'm not a fan of writing test stuff into <buildroot>/lib.

I hear you, but for single-configuration builds that's exactly where libDynamicLibraryLib.a is already, along with libgtest.a and libgtest_main.a. This change just makes the Windows build more consistent with builds on Linux etc.

Is fixing extract_symbols.py to handle whatever is needed a better option?

I don't believe it's a better option, but I could modify extract_symbols.py to additionally look in lib/<config>. I'll go with your decision.

[michaelplatings]

@marsupial ping?

[marsupial]

I take it your using the MSVC generator?
I use ninja (which is a single-config build) all the time and it puts the the files into unittests/Support/DynamicLibrary ?
I'll try to spin up a Windows build in the next couple of days, but I'm not foreseeing my opinion being changed.

Anyone else care to weigh in?

[michaelplatings]

@marsupial that's odd. I'm using ninja also and libDynamicLibraryLib.a ends up in lib for me. Reproducer:

svn co http://llvm.org/svn/llvm-project/llvm/trunk llvm
mkdir build && cd build
cmake -G Ninja ../llvm
ninja DynamicLibraryLib
ls lib/libDynamicLibraryLib.a

In case it makes any difference, I used CMake version 3.5.1 and ninja 1.6.0.

[michaelplatings]

(moved closing parenthesis to new line)

[michaelplatings]

@marsupial if you can show me an instance where this patch significantly changes the build's behaviour then I can change the patch to mitigate that.

[marsupial]

Crap your right...I might revisit this (as I think that's a bad move on my part).
I'd still like to give a moment for anyone else to chime in; otherwise, it shouldn't be a reason to hold this up.

unittests/Support/DynamicLibrary/CMakeLists.txt

 # Needed by LLVM's CMake checks because this file defines multiple targets.
 set(LLVM_OPTIONAL_SOURCES ExportedFuncs.cpp PipSqueak.cpp)
 
 set(LLVM_LINK_COMPONENTS Support)
 
 add_library(DynamicLibraryLib STATIC
   ExportedFuncs.cpp
   )
 set_target_properties(DynamicLibraryLib PROPERTIES FOLDER "Tests")
 
+# extract_symbols.py relies on all its library arguments being in the same
+# directory, so we must set the output directory in the same way as if
+# add_llvm_library was used.
+set_output_directory(DynamicLibraryLib
+  LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
+
 add_llvm_unittest(DynamicLibraryTests
   DynamicLibraryTest.cpp
   )
 target_link_libraries(DynamicLibraryTests PRIVATE DynamicLibraryLib)
 export_executable_symbols(DynamicLibraryTests)
 
 function(dynlib_add_module NAME)
   add_library(${NAME} SHARED