xplat: add ChakraCoreHeaders target. #2129
Conversation
Currently only the ChakraCore shared library uses this target, however this target is also extremely useful for Static Library builds. To use this target with any other target library/executable, simply add the following line to cmake: add_dependencies(myTarget ChakraCoreHeaders)
|
Hi @kphillisjr, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! The agreement was validated by Microsoft and real humans are currently evaluating your PR. TTYL, MSBOT; |
There was a problem hiding this comment.
@kphillisjr Good job. Couple of changes and rest looks fine.
| @@ -52,3 +52,25 @@ target_include_directories ( | |||
| ../Runtime/Debug | |||
| ../Parser | |||
| ) | |||
|
|
|||
|
|
|||
| add_custom_target(ChakraCoreHeaders ALL | |||
There was a problem hiding this comment.
We need to have this under if(NOT CC_XCODE_PROJECT) otherwise xcode build would break.
| "${CMAKE_BINARY_DIR}/include" | ||
| ) | ||
|
|
||
| if(MSVC OR WIN32) |
There was a problem hiding this comment.
This is not necessary better not add.
There was a problem hiding this comment.
I got ChakraCore to compile using MSVC 2015 and cmake, so this change is useful.
There was a problem hiding this comment.
@kphillisjr I'm not saying we can't do that. The problem is that preprocessor flags we set and source files we do include on CMake xplat is not necessarily matching what we supposed to do for Windows. For the best ChakraCore experience on Windows, we better use msbuild with sln file.
| @@ -75,3 +75,6 @@ if(NOT CC_XCODE_PROJECT) | |||
| ${CHAKRACORE_BINARY_DIR}/ | |||
| ) | |||
| endif() | |||
|
|
|||
| add_dependencies(ChakraCore ChakraCoreHeaders) | |||
There was a problem hiding this comment.
Do we need to add the same to /bin/ch/CMakeLists.txt for STATIC builds?
There was a problem hiding this comment.
I am in the process of adding major revisions to the xplat build system. These revisions have three main features...
- Greatly reduces compile time for swap between Shared and Static Libraries... This means that a minimal number of files get changed each time.
- A new ChakraCoreStatic target. This is simply the ChakraCore target with the static preprocessor define added.
- The number of static libraries is reduced by converting the existing library to an object.
There was a problem hiding this comment.
We better add it here now and if another PR will change that later, that is something else.
This target automatically copies the ChakraCore Headers to the the path that cmake is ran from under the include folder. This makes it easier to find the headers needed for ChakraCore Development. V2: add suggestions mentioned by review in pull request chakra-core#2129.
|
@kphillisjr ping? |
|
Closing this pull request... It is superseded by pull request #2678. |
Currently only the ChakraCore shared library uses this target, however this
target is also extremely useful for Static Library builds. To use this
target with any other target library/executable, simply add the following
line to cmake:
add_dependencies(myTarget ChakraCoreHeaders)
/cc @obastemur - Here is your request for adding a target for ChakraCoreHeaders. Also, this is future proofed a little for if/when using cmake to target MSVC/windows is added.