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

Support building phobos against a system copy of zlib #4742

Merged
merged 4 commits into from
Sep 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions .github/workflows/supported_llvm_versions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
host_dc: ldc-1.19.0
# FIXME: no usable official package available yet
llvm_version: https://github.com/ldc-developers/llvm-project/releases/download/ldc-v18.1.8/llvm-18.1.8-linux-x86_64.tar.xz
cmake_flags: -DRT_SUPPORT_SANITIZERS=ON
cmake_flags: -DRT_SUPPORT_SANITIZERS=ON -DPHOBOS_SYSTEM_ZLIB=ON
- job_name: macOS 14, LLVM 17, latest LDC beta
os: macos-14
host_dc: ldc-beta
Expand Down Expand Up @@ -63,13 +63,13 @@ jobs:
python3 -m pip install --user lit
fi
python3 -c "import lit.main; lit.main.main();" --version . | head -n 1
- name: 'Linux: Install gdb, llvm-symbolizer and libzstd'
- name: 'Linux: Install gdb, llvm-symbolizer, libzstd, zlib1g-dev'
if: runner.os == 'Linux'
run: |
set -eux
sudo apt-get update
# Don't use latest gdb v10+ from Ubuntu toolchain PPA with regressions, use official v9
sudo apt-get install gdb=9.1-0ubuntu1 llvm libzstd-dev
sudo apt-get install gdb=9.1-0ubuntu1 llvm libzstd-dev zlib1g-dev

- name: Try to restore cached LLVM
uses: actions/cache@v4
Expand Down
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- LLVM for prebuilt packages bumped to v18.1.8 (incl. macOS arm64). (#4712)
- Android: NDK for prebuilt package bumped from r26d to r27. (#4711)
- ldc2.conf: %%ldcconfigpath%% placeholder added - specifies the directory where current configuration file is located. (#4717)
- Add support for building against a system copy of zlib through `-DPHOBOS_SYSTEM_ZLIB=ON`. (#4742)

#### Platform support

Expand Down Expand Up @@ -923,7 +924,7 @@
- Misc. debuginfo issues, incl. adaptations to internal LLVM 5.0 changes: (#2315)
- `ref` parameters and closure parameters declared with wrong address and hence potentially showing garbage.
- Win64: parameters > 64 bit passed by value showing garbage.
- Win64: debuginfos for closure and nested variables now finally available starting with LLVM 5.0.
- Win64: debuginfos for closure and nested variables now finally available starting with LLVM 5.0.
- LLVM error `Global variable initializer type does not match global variable type!` for `T.init` with explicit initializers for dominated members in nested unions. (#2108)
- Inconsistent handling of lvalue slicees wrt. visible side-effects of slice lower/upper bound expressions. (#1433)
- Misc. dcompute issues. (#2195, #2215)
Expand Down
6 changes: 6 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ set(LDC_ENABLE_ASSERTIONS "${LLVM_ENABLE_ASSERTIONS}" CACHE BOOL "Enable LDC ass
# Allow user to specify mimalloc.o location, to be linked with `ldc2` only
set(ALTERNATIVE_MALLOC_O "" CACHE STRING "If specified, adds ALTERNATIVE_MALLOC_O object file to LDC link, to override the CRT malloc.")

# Most linux distributions have a policy of not bundling dependencies like zlib
set(PHOBOS_SYSTEM_ZLIB OFF CACHE BOOL "Use system zlib instead of Phobos' vendored version")

if(D_VERSION EQUAL 1)
message(FATAL_ERROR "D version 1 is no longer supported.
Please consider using D version 2 or checkout the 'd1' git branch for the last version supporting D version 1.")
Expand Down Expand Up @@ -290,6 +293,9 @@ if(SANITIZE)
endif()
endif()
append("${SANITIZE_CXXFLAGS}" LDC_CXXFLAGS)
if(PHOBOS_SYSTEM_ZLIB)
append("-DPHOBOS_SYSTEM_ZLIB" LDC_CXXFLAGS)
endif()
# LLVM_CXXFLAGS may contain -Werror which causes compile errors with dmd source
string(REPLACE "-Werror " "" LLVM_CXXFLAGS ${LLVM_CXXFLAGS})
if (UNIX AND NOT "${LLVM_LDFLAGS}" STREQUAL "")
Expand Down
4 changes: 4 additions & 0 deletions driver/linker-gcc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,10 @@ void ArgsBuilder::build(llvm::StringRef outputPath,
for (const auto &name : defaultLibNames) {
args.push_back("-l" + name);
}
#ifdef PHOBOS_SYSTEM_ZLIB
if (!defaultLibNames.empty() && !linkAgainstSharedDefaultLibs())
args.push_back("-lz");
#endif

// libs added via pragma(lib, libname)
for (auto ls : global.params.linkswitches) {
Expand Down
54 changes: 37 additions & 17 deletions runtime/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ if (RT_SUPPORT_SANITIZERS)
list(APPEND D_FLAGS -d-version=SupportSanitizers)
endif()

if(PHOBOS_SYSTEM_ZLIB)
Copy link
Member

@kinke kinke Sep 4, 2024

Choose a reason for hiding this comment

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

Ideally, we'd bake this into the ldc-build-runtime CMake invocations too, in

string[] args = [
"cmake",
"-DLDC_EXE_FULL=" ~ config.ldcExecutable,
"-DDMDFE_MINOR_VERSION=@DMDFE_MINOR_VERSION@",
"-DDMDFE_PATCH_VERSION=@DMDFE_PATCH_VERSION@",
];

message(STATUS "-- Building PHOBOS against system zlib")
endif()

# Auto-detect TARGET_SYSTEM from host
if("${TARGET_SYSTEM}" STREQUAL "AUTO")
set(TARGET_SYSTEM ${CMAKE_SYSTEM_NAME})
Expand Down Expand Up @@ -243,14 +247,22 @@ if(PHOBOS2_DIR)
list(REMOVE_ITEM PHOBOS2_D ${PHOBOS2_D_WINDOWS})
endif()

# Phobos C parts
file(GLOB_RECURSE PHOBOS2_C ${PHOBOS2_DIR}/etc/*.c)
# remove zlib test modules
list(REMOVE_ITEM PHOBOS2_C
${PHOBOS2_DIR}/etc/c/zlib/test/example.c
${PHOBOS2_DIR}/etc/c/zlib/test/infcover.c
${PHOBOS2_DIR}/etc/c/zlib/test/minigzip.c
)
if(PHOBOS_SYSTEM_ZLIB)
find_package(ZLIB REQUIRED)
else()
# Phobos C parts
file(GLOB_RECURSE PHOBOS2_C ${PHOBOS2_DIR}/etc/*.c)
# remove zlib test modules
list(REMOVE_ITEM PHOBOS2_C
${PHOBOS2_DIR}/etc/c/zlib/test/example.c
${PHOBOS2_DIR}/etc/c/zlib/test/infcover.c
${PHOBOS2_DIR}/etc/c/zlib/test/minigzip.c
)
CHECK_INCLUDE_FILE(unistd.h HAVE_UNISTD_H)
if (HAVE_UNISTD_H)
append("-DHAVE_UNISTD_H" CMAKE_C_FLAGS)
endif()
endif()
endif()

#
Expand Down Expand Up @@ -397,11 +409,6 @@ if("${TARGET_SYSTEM}" MATCHES "MSVC")
# warning C4996: zlib uses 'deprecated' POSIX names
append("/wd4100 /wd4127 /wd4131 /wd4206 /wd4244 /wd4245 /wd4267 /wd4996" CMAKE_C_FLAGS_RELEASE)
endif()
CHECK_INCLUDE_FILE(unistd.h HAVE_UNISTD_H)
if (HAVE_UNISTD_H)
# Needed for zlib
append("-DHAVE_UNISTD_H" CMAKE_C_FLAGS)
endif()
# 2) Set all other CMAKE_C_FLAGS variants to CMAKE_C_FLAGS_RELEASE
set(variables
CMAKE_C_FLAGS_DEBUG
Expand All @@ -412,6 +419,16 @@ foreach(variable ${variables})
set(${variable} "${CMAKE_C_FLAGS_RELEASE}")
endforeach()

function(link_zlib phobos_target library_type)
if(PHOBOS_SYSTEM_ZLIB)
if(${library_type} STREQUAL "SHARED")
target_link_libraries(${phobos_target} ZLIB::ZLIB)
endif()
else()
target_sources(${phobos_target} PRIVATE ${PHOBOS2_C})
endif()
endfunction()

# Compiles the given D modules to object files, and if enabled, bitcode files.
# The paths of the output files are appended to outlist_o and outlist_bc, respectively.
macro(dc src_files src_basedir d_flags output_basedir emit_bc all_at_once single_obj_name outlist_o outlist_bc)
Expand Down Expand Up @@ -638,8 +655,8 @@ macro(build_runtime_libs druntime_o druntime_bc phobos2_o phobos2_bc c_flags ld_
list(APPEND ${outlist_targets} druntime-ldc${target_suffix})

if(PHOBOS2_DIR)
add_library(phobos2-ldc${target_suffix} ${library_type}
${phobos2_o} ${PHOBOS2_C})
add_library(phobos2-ldc${target_suffix} ${library_type} ${phobos2_o})
link_zlib(phobos2-ldc${target_suffix} ${library_type})
set_common_library_properties(phobos2-ldc${target_suffix}
phobos2-ldc${lib_suffix} ${output_path}
"${c_flags}" "${ld_flags}" ${is_shared}
Expand Down Expand Up @@ -669,8 +686,8 @@ macro(build_runtime_libs druntime_o druntime_bc phobos2_o phobos2_bc c_flags ld_
"${c_flags}" "${ld_flags}" OFF
)

add_library(phobos2-ldc-lto${target_suffix} STATIC
${phobos2_bc} ${PHOBOS2_C})
add_library(phobos2-ldc-lto${target_suffix} STATIC ${phobos2_bc})
link_zlib(phobos2-ldc-lto${target_suffix} STATIC)
set_common_library_properties(phobos2-ldc-lto${target_suffix}
phobos2-ldc-lto${lib_suffix} ${output_path}
"${c_flags}" "${ld_flags}" OFF
Expand Down Expand Up @@ -1004,6 +1021,9 @@ function(build_test_runners name_suffix path_suffix d_flags linkflags is_shared)
LINK_FLAGS ${linkflags}
LINK_DEPENDS ${tested_lib_path}
)
if(PHOBOS_SYSTEM_ZLIB AND "${is_shared}" STREQUAL "OFF")
target_link_libraries(${phobos_name} ZLIB::ZLIB)
endif()
add_test(build-${phobos_name} "${CMAKE_COMMAND}" --build ${CMAKE_BINARY_DIR} --target ${phobos_name})
set(_GLOBAL_TESTRUNNERS "${_GLOBAL_TESTRUNNERS};${phobos_name}" CACHE INTERNAL "")
endif()
Expand Down
5 changes: 4 additions & 1 deletion runtime/ldc-build-runtime.d.in
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ struct Config {
string[] cFlags;
string[] linkerFlags;
uint numBuildJobs;
bool systemZlib;
string[string] cmakeVars;
}

Expand Down Expand Up @@ -160,6 +161,7 @@ void runCMake() {
if (config.dFlags.length) args ~= "-DD_EXTRA_FLAGS=" ~ config.dFlags.join(";");
if (config.cFlags.length) args ~= "-DRT_CFLAGS=" ~ config.cFlags.join(" ");
if (config.linkerFlags.length) args ~= "-DLD_FLAGS=" ~ config.linkerFlags.join(" ");
if (config.systemZlib) args ~= "-DPHOBOS_SYSTEM_ZLIB=ON";

foreach (pair; config.cmakeVars.byPair)
args ~= "-D" ~ pair[0] ~ '=' ~ pair[1];
Expand Down Expand Up @@ -324,7 +326,8 @@ void parseCommandLine(string[] args) {
"dFlags", "Extra LDC flags for the D modules (separated by ';')", &config.dFlags,
"cFlags", "Extra C/ASM compiler flags for the handful of C/ASM files (separated by ';')", &config.cFlags,
"linkerFlags", "Extra C linker flags for shared libraries and testrunner executables (separated by ';')", &config.linkerFlags,
"j", "Number of parallel build jobs", &config.numBuildJobs
"j", "Number of parallel build jobs", &config.numBuildJobs,
"systemZlib", "Use system zlib instead of Phobos' vendored version", &config.systemZlib,
);

// getopt() has removed all consumed args from `args`
Expand Down