Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Replace fetch-binary-deps.py with pure CMake #121

Closed
wants to merge 7 commits into from
Closed

Replace fetch-binary-deps.py with pure CMake #121

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a7b7682
add CMake code for managing binary deps
mossheim May 12, 2020
838d211
remove fetch-binary-deps.py
mossheim May 12, 2020
7b7b39e
update build instructions
mossheim May 12, 2020
7c1f290
simplify VulkanMemoryAllocator dependency on vulkan
mossheim May 12, 2020
6e02f55
travis: update scripts
mossheim May 12, 2020
1e87830
make s3 URL less site-specific
mossheim May 12, 2020
ebd7c02
travis: test commit
mossheim May 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@ bin
# ignore vim/gvim swap cruft
.*.swp

# third party binary dependencies
third_party/binary-deps/ffmpeg-ext
third_party/binary-deps/vulkan-ext
third_party/binary-deps/swiftshader-ext
17 changes: 6 additions & 11 deletions Building-Scintillator.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,15 @@
```sh
sudo apt-get install libxrandr libxinerama-dev libxcursor-dev libxi-dev gperf

(if you wanna build docs)
# (if you wanna build docs)
sudo apt-get install doxygen graphviz

git submodule update --init --recursive

install python3 if needed

fetch binary deps:

```
python3 tools/fetch-binary-deps.py
```
# install python3 if needed

mkdir build
cd build
cmake ..
cmake .. # will download and unpack binary dependencies
make


```
80 changes: 6 additions & 74 deletions src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,73 +264,13 @@ elseif(WIN32)
add_executable(scinsynth ${scintillator_synth_files})
endif()

# Force using the locally sourced libraries as opposed to system libs
find_library(SHADERC_LIBRARY
NAMES shaderc_combined
PATHS "${SCIN_EXT_INSTALL_DIR}/lib"
NO_DEFAULT_PATH
)
if (WIN32)
find_library(AVCODEC_LIBRARY
NAMES avcodec
PATHS "${SCIN_EXT_INSTALL_DIR}/bin"
NO_DEFAULT_PATH
)
find_library(AVFORMAT_LIBRARY
NAMES avformat
PATHS "${SCIN_EXT_INSTALL_DIR}/bin"
NO_DEFAULT_PATH
)
find_library(AVUTIL_LIBRARY
NAMES avutil
PATHS "${SCIN_EXT_INSTALL_DIR}/bin"
NO_DEFAULT_PATH
)
find_library(SWSCALE_LIBRARY
NAMES swscale
PATHS "${SCIN_EXT_INSTALL_DIR}/bin"
NO_DEFAULT_PATH
)
find_library(Vulkan_LIBRARY
NAMES vulkan-1
PATHS "${SCIN_EXT_INSTALL_DIR}/lib"
NO_DEFAULT_PATH
)
else()
find_library(AVCODEC_LIBRARY
NAMES avcodec
PATHS "${SCIN_EXT_INSTALL_DIR}/lib"
NO_DEFAULT_PATH
)
find_library(AVFORMAT_LIBRARY
NAMES avformat
PATHS "${SCIN_EXT_INSTALL_DIR}/lib"
NO_DEFAULT_PATH
)
find_library(AVUTIL_LIBRARY
NAMES avutil
PATHS "${SCIN_EXT_INSTALL_DIR}/lib"
NO_DEFAULT_PATH
)
find_library(SWSCALE_LIBRARY
NAMES swscale
PATHS "${SCIN_EXT_INSTALL_DIR}/lib"
NO_DEFAULT_PATH
)
find_library(Vulkan_LIBRARY
NAMES vulkan
PATHS "${SCIN_EXT_INSTALL_DIR}/lib"
NO_DEFAULT_PATH
)
endif()

target_link_libraries(scinsynth
"${Vulkan_LIBRARY}"
${AVCODEC_LIBRARY}
${AVFORMAT_LIBRARY}
${AVUTIL_LIBRARY}
${SHADERC_LIBRARY}
${SWSCALE_LIBRARY}
shaderc_combined
vulkan
avcodec
avformat
avutil
swscale
${SCIN_EXT_INSTALL_DIR}/lib/liblo.${SCIN_LIBLO_LIBRARY_SUFFIX}
VulkanMemoryAllocator
gflags
Expand All @@ -340,14 +280,6 @@ target_link_libraries(scinsynth
spdlog
)

target_include_directories(scinsynth PRIVATE
${AVCODEC_INCLUDE_DIR}
${AVFORMAT_INCLUDE_DIR}
${AVUTIL_INCLUDE_DIR}
${Vulkan_INCLUDE_DIRS}
${SWSCALE_INCLUDE_DIR}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i don't think these were doing anything before -- find_library just finds a library file; but it's find_package with an appropriately written CMake find-module that will often set "lib_INCLUDE_DIR". from what i can tell, things just worked before because everything was unpacked into the build dir.

Copy link
Member

Choose a reason for hiding this comment

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

Could be the case, always better to remove cruft like this if we can. Probably those are there from historical times when Scintillator included those dependencies in third_party as submodules.

)

if(UNIX AND NOT APPLE)
target_link_libraries(scinsynth
stdc++fs
Expand Down
8 changes: 4 additions & 4 deletions third_party/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,9 @@ add_subdirectory(glfw)
add_subdirectory(glm)

#### VulkanMemoryAllocator
include_directories(VulkanMemoryAllocator/src)
include_directories(${Vulkan_INCLUDE_DIRS})
add_library(VulkanMemoryAllocator STATIC "VulkanMemoryAllocatorBuild.cpp")
target_include_directories(VulkanMemoryAllocator INTERFACE VulkanMemoryAllocator/src)
target_include_directories(VulkanMemoryAllocator PRIVATE "${SCIN_EXT_INSTALL_DIR}/include")
target_include_directories(VulkanMemoryAllocator PUBLIC VulkanMemoryAllocator/src)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

PUBLIC = INTERFACE && PRIVATE

Copy link
Member

Choose a reason for hiding this comment

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

So the memory allocator needs the vulkan SDK to compile itself. I thought PRIVATE was telling cmake to provide the path the vulkan SDK header to that library but not transitively provide that path to dependent libraries.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you had it set to INTERFACE before, which tells cmake to only transitively provide this include directory, but not use it when compiling VulkanMemoryAllocator itself. it looks like scinsynth depends on these headers too, though -- if I set this to PRIVATE, then i see this:

In file included from ../src/scinsynth.cpp:13:
../src/vulkan/Buffer.hpp:6:10: fatal error: vk_mem_alloc.h: No such file or directory
    6 | #include "vk_mem_alloc.h"
      |          ^~~~~~~~~~~~~~~~
compilation terminated.

Copy link
Member

Choose a reason for hiding this comment

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

That's correct. The Vulkan 1.0 standard, which we are limited to for the moment because the MacOS Vulkan emulation only supports 1.0, allows for a very small number of allocated objects. Best practice is to do a single memory allocation and BYO memory allocator, and we're using this library for exactly that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense! i'm just pointing out that the memory allocator lib isn't the only part of this project that requires these headers.

target_link_libraries(VulkanMemoryAllocator PUBLIC vulkan)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think this dependency should be public, actually. since VulkanMemoryAllocator is static, it won't be linked with libvulkan when it's built; so anything linking against VulkanMemoryAllocator should link against vulkan too. this also makes the include directories a little more generic


#### googletest
set(INSTALL_GTEST OFF CACHE BOOL "" FORCE)
Expand All @@ -78,3 +76,5 @@ add_subdirectory(googletest)

#### yaml-cpp
add_subdirectory(yaml-cpp)

add_subdirectory(binary-deps)
124 changes: 124 additions & 0 deletions third_party/binary-deps/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
#### gather binary dependencies

# On each supported operating system Scintillator relies on several compile-time dependencies. These can be fairly
# heavy-weight compilations, for example on Travis right now the Vulkan SDK compile takes over an hour to perform. In
# the interest of expediency we have set up independent build services for these particular dependencies, and save
# the build output from Travis to an S3 instance. This script downloads the os-specific dependent binaries, validates
# the download, and extracts them to the expected location for the rest of the build script to use.

# Used in DownloadBinaryDep
if(CMAKE_SYSTEM_NAME STREQUAL "Windows")
set(dep_os_name windows)
elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
set(dep_os_name osx)
elseif(CMAKE_SYSTEM_NAME STREQUAL "Linux")
set(dep_os_name linux)
else()
message(FATAL_ERROR "Unsupported operating system: ${CMAKE_SYSTEM_NAME}")
endif()

# Downloads, verifies, and unpacks a binary dependency from AWS. If SCIN_CLOBBER_BINARY_DEPS is true, then remove it
# even if it exists; otherwise if SCIN_CHECK_BINARY_DEPS_UPDATES is true check and inform the user if a newer dep
# version is available; otherwise only act if the directory doesn't exist yet.
function(DownloadBinaryDep dep_name)
set(dep_dir ${CMAKE_CURRENT_SOURCE_DIR}/${dep_name})
if(EXISTS ${dep_dir})
if(SCIN_CLOBBER_BINARY_DEPS)
message(VERBOSE "Removing preexisting folder for ${dep_name} at ${dep_dir}")
file(REMOVE_RECURSE ${dep_dir})
elseif(NOT SCIN_CHECK_BINARY_DEPS_UPDATES)
message(VERBOSE "Skipping check for ${dep_name} updates")
message(VERBOSE "Skipping setup for ${dep_name}: directory ${dep_dir} exists")
return()
endif()
endif()

# We use this later to determine whether to note an update if a new version is available.
if(EXISTS ${dep_dir})
set(dep_dir_exists 1)
else()
set(dep_dir_exists 0)
endif()

file(MAKE_DIRECTORY ${dep_dir}) # may already exist, no error posted

# Download xyz-latest.html and parse it to find the real ZIP URL
set(dep_latest_file_name ${dep_name}-${dep_os_name}-latest.html)
set(dep_latest_url http://scintillator-synth-coverage.s3-website-us-west-1.amazonaws.com/binaries/${dep_name}/${dep_latest_file_name})
set(dep_latest_file ${dep_dir}/${dep_latest_file_name})
message(VERBOSE "Download latest-redirect from ${dep_latest_url}")
file(DOWNLOAD ${dep_latest_url} ${dep_latest_file})
file(STRINGS ${dep_latest_file} dep_latest_html_contents)
string(REGEX MATCH "http://.*\\.tgz" dep_zip_url ${dep_latest_html_contents})
message(VERBOSE "URL from latest-redirect: ${dep_zip_url}")
if(NOT dep_zip_url)
message(FATAL_ERROR "No redirect URL found in file from ${dep_latest_url}, contents: ${dep_latest_html_contents}")
endif()

# Check for updates; if we are set to not clobber and the directory already existed, this is where we bail out
get_filename_component(dep_zip_file_name ${dep_zip_url} NAME)
set(dep_zip_file ${dep_dir}/${dep_zip_file_name})
if(dep_dir_exists)
if(NOT EXISTS ${dep_zip_file})
message(STATUS "Update is available for ${dep_name}: ${dep_zip_file}")
message(STATUS "Remove ${dep_dir} and rerun CMake configuration to install it")
else()
message(STATUS "No update available for ${dep_name}")
endif()
return()
endif()

message(STATUS "Downloading and unpacking ${dep_name} in ${dep_dir}")

# Get expected hash for zip
string(REGEX REPLACE "\\.tgz$" ".sha256" dep_hash_url "${dep_zip_url}")
string(REGEX REPLACE "\\.tgz$" ".sha256" dep_hash_file "${dep_zip_file}")
message(VERBOSE "Download ${dep_hash_url} to ${dep_hash_file}")
file(DOWNLOAD ${dep_hash_url} ${dep_hash_file})
file(READ ${dep_hash_file} dep_hash_file_contents LIMIT 64)
string(STRIP ${dep_hash_file_contents} dep_expected_hash) # strip trailing newline added by file(READ)
message(VERBOSE "Got hash: ${dep_expected_hash}")

# Download and unpack zip
message(VERBOSE "Download ${dep_zip_url} to ${dep_zip_file_name}")
file(DOWNLOAD ${dep_zip_url} ${dep_zip_file} SHOW_PROGRESS EXPECTED_HASH SHA256=${dep_expected_hash})
message(VERBOSE "Decompress ${dep_zip_file}")
execute_process(COMMAND ${CMAKE_COMMAND} -E tar xz ${dep_zip_file}
OUTPUT_VARIABLE tar_out ERROR_VARIABLE tar_err WORKING_DIRECTORY ${dep_dir})
message(VERBOSE "tar stdout: ${tar_out}\ntar stderr: ${tar_err}")

message(STATUS "Finished unpacking ${dep_name}")
endfunction()

# Provides a target `lib_name` with appropriate include and link properties.
function(CreateBinaryDepLibrary dep_name lib_name alt_lib_name)
message(VERBOSE "Creating binary-dep library ${lib_name}")
find_library(${lib_name}_library
NAMES ${lib_name} ${alt_lib_name}
PATHS ${dep_name}
PATH_SUFFIXES lib bin
NO_DEFAULT_PATH
)
# "interface" libraries only have an interface, no build steps
add_library(${lib_name} INTERFACE IMPORTED GLOBAL)
target_include_directories(${lib_name} INTERFACE ${dep_name}/include)
target_link_libraries(${lib_name} INTERFACE ${${lib_name}_library})
endfunction()

#### Real commands

# Download dependencies
DownloadBinaryDep(vulkan-ext)
DownloadBinaryDep(ffmpeg-ext)
DownloadBinaryDep(swiftshader-ext)

# Force using the locally sourced libraries as opposed to system libs
CreateBinaryDepLibrary(vulkan-ext shaderc_combined "")
CreateBinaryDepLibrary(vulkan-ext vulkan vulkan-1)
CreateBinaryDepLibrary(ffmpeg-ext avcodec "")
CreateBinaryDepLibrary(ffmpeg-ext avformat "")
CreateBinaryDepLibrary(ffmpeg-ext avutil "")
CreateBinaryDepLibrary(ffmpeg-ext swscale "")

option(SCIN_CHECK_BINARY_DEPS_UPDATES "Check on every configure step whether a new version of each binary dependency is available" OFF)
option(SCIN_CLOBBER_BINARY_DEPS "Forcibly redownload and unpack binary dependencies on every configure step" OFF)
112 changes: 0 additions & 112 deletions tools/fetch-binary-deps.py
View file
Open in desktop

This file was deleted.