Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cmake install target #926

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

theoparis
Copy link

I added a cmake install target that installs the libraries, headers and the CLI binaries to CMAKE_INSTALL_PREFIX.

This is how I built and installed luau with this PR (on a linux machine):

cmake -B build -DCMAKE_BUILD_TYPE=RelWithDebInfo -G Ninja -DCMAKE_INSTALL_PREFIX=$PWD/build/install
cmake --build build
cmake --install build

@theoparis
Copy link
Author

theoparis commented Jul 26, 2023

Sorry for not working on this lately. I almost forgot that I need to change CMAKE_SOURCE_DIR here to CMAKE_CURRENT_SOURCE_DIR so that it works inside of FetchContent or add_subdirectory. I will add it soon :)

This allows Luau to be included in other projects.
@theoparis theoparis marked this pull request as ready for review September 14, 2023 19:50
@theoparis
Copy link
Author

I forgot that cmake projects tend to provide some kind of .cmake file that can be used with find_package. I will try to figure this out soon.

@theoparis
Copy link
Author

theoparis commented Dec 28, 2023

I have added find_package support, however it has required a few additional changes to the CMakeLists.txt file to provide the ability to require a minimum version of Luau and to install the headers properly. The project VERSION variable would have to be updated every time a new version is released.

If Luau is not found it will error when configuring an example project. The prefix required for CMake to find Luau can be specified with CMAKE_PREFIX_PATH or specified in a FindLuau.cmake file that is able to be included with CMAKE_MODULE_PATH.

cmake_minimum_required(VERSION 3.27)
project(test LANGUAGES CXX VERSION 0.0.1)

find_package(Luau REQUIRED)

add_executable(test main.cpp)
target_link_libraries(
	test PRIVATE
	Luau.VM
	Luau.Compiler
	Luau.Ast
	Luau.Config
	Luau.CodeGen
	Luau.Analysis
)
target_compile_features(test PRIVATE cxx_std_23)

@@ -21,7 +21,7 @@ if(LUAU_STATIC_CRT)
set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
endif()

project(Luau LANGUAGES CXX C)
project(Luau LANGUAGES CXX C VERSION 0.607)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can include a version.
Even with automation (worse with manual update) it will require a commit to this CMake file basically each week.

Comment on lines +307 to +311
write_basic_package_version_file(
"${CMAKE_CURRENT_BINARY_DIR}/LuauConfigVersion.cmake"
VERSION "${PROJECT_VERSION}"
COMPATIBILITY AnyNewerVersion
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same problem for the version here I assume.

COMPATIBILITY AnyNewerVersion
)
export(
TARGETS Luau.Ast Luau.Analysis Luau.CodeGen Luau.VM Luau.Compiler Luau.Config Luau.Common Luau.VM.Internals
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Luau.VM.Internals should not be included.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem I had before when it wasn't there was this:

  export called with target "Luau.CodeGen" which requires target
  "Luau.VM.Internals" that is not in any export set.

I'm not sure what the best way to deal with this is yet.

Copy link
Contributor

@Jan200101 Jan200101 Mar 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like a bunch of headers, which would be exported with this, make use of VM internals.

Until that gets resolved Luau.VM.Internals needs to be exported.

)

install(
TARGETS Luau.Ast Luau.Analysis Luau.CodeGen Luau.VM Luau.Compiler Luau.Config Luau.Common Luau.VM.Internals
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Luau.VM.Internals should not be included.

"${CMAKE_CURRENT_SOURCE_DIR}/Analysis/include/"
"${CMAKE_CURRENT_SOURCE_DIR}/CodeGen/include/"
"${CMAKE_CURRENT_SOURCE_DIR}/VM/include/"
"${CMAKE_CURRENT_SOURCE_DIR}/VM/src/"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't include this src.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants