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.

Sign up for GitHub

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

CMake fixes #62

Merged
merged 11 commits into from
Aug 8, 2022
Merged

CMake fixes #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f749b06
remove fake variable
h-vetinari Jul 30, 2022
87e308f
try to specify static links at compile time
hmaarrfk Jul 30, 2022
cf3b62a
output cmake files for inspectjon
hmaarrfk Jul 30, 2022
f3cc8dc
uncouple outputs
h-vetinari Jul 31, 2022
8d075f5
sort & complete tests
h-vetinari Jul 31, 2022
49e3cbc
cannot build CLI without shared libs
h-vetinari Jul 31, 2022
4011ff8
run cmake --find-package as test
h-vetinari Jul 31, 2022
f6fa361
clean up bld.bat
h-vetinari Jul 31, 2022
246a0b2
don't build static lib on windows
h-vetinari Aug 1, 2022
7c294af
disable cmake test on windows for now
h-vetinari Aug 1, 2022
22966cd
link CLI against shared builds on windows
h-vetinari Aug 1, 2022
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
21 changes: 12 additions & 9 deletions recipe/install.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
@echo on

pushd "%SRC_DIR%"\build\cmake
cmake -GNinja ^
-DCMAKE_BUILD_TYPE=Release ^
-DCMAKE_INSTALL_PREFIX="%LIBRARY_PREFIX%" ^
-DCMAKE_INSTALL_LIBDIR="lib" ^
-DCMAKE_PREFIX_PATH="%LIBRARY_PREFIX%" ^
-DZSTD_BUILD_SHARED=ON
if errorlevel 1 exit 1
-DZSTD_BUILD_SHARED=ON ^
-DZSTD_BUILD_STATIC=OFF ^
-DZSTD_PROGRAMS_LINK_SHARED=ON ^
.
if %ERRORLEVEL% neq 0 exit 1

cmake --build . --target install
if errorlevel 1 exit 1
dir
dir lib
copy lib\zstd.dll %PREFIX%\Library\bin\zstd.dll
if errorlevel 1 exit 1
if %ERRORLEVEL% neq 0 exit 1

:: duplicate DLL (+ importlib) to also have files with "lib" prefix
copy %PREFIX%\Library\bin\zstd.dll %PREFIX%\Library\bin\libzstd.dll
if errorlevel 1 exit 1
if %ERRORLEVEL% neq 0 exit 1
copy %PREFIX%\Library\lib\zstd.lib %PREFIX%\Library\lib\libzstd.lib
if errorlevel 1 exit 1
if %ERRORLEVEL% neq 0 exit 1
21 changes: 11 additions & 10 deletions recipe/install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,26 @@ if [[ ${HOST} =~ .*linux.* ]]; then
_CMAKE_EXTRA_CONFIG+=(-DRT_LIBRARIES=${LIBRT})
fi

pushd build/cmake
if [[ "$PKG_NAME" == *static ]]; then
ZSTD_BUILD_STATIC=ON
# cannot build CLI without shared lib
ZSTD_BUILD_SHARED=ON
else
ZSTD_BUILD_STATIC=OFF
ZSTD_BUILD_SHARED=ON
fi

pushd build/cmake
FULL_AR=`which ${AR}`
cmake -GNinja \
-DCMAKE_INSTALL_PREFIX="${PREFIX}" \
-DCMAKE_INSTALL_LIBDIR="lib" \
-DCMAKE_PREFIX_PATH="${PREFIX}" \
-DCMAKE_AR=${FULL_AR} \
-DZSTD_BUILD_STATIC=ON \
-DZSTD_BUILD_STATIC=${ZSTD_BUILD_STATIC} \
-DZSTD_BUILD_SHARED=${ZSTD_BUILD_SHARED} \
-DZSTD_PROGRAMS_LINK_SHARED=ON \
"${_CMAKE_EXTRA_CONFIG[@]}"

ninja install
popd

if [[ "$PKG_NAME" == *static ]]
then
# relying on conda to dedup package
echo "Keeping all files, conda will dedupe"
else
rm -rf ${PREFIX}/lib/*.a
fi
87 changes: 63 additions & 24 deletions recipe/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
{% set name = "zstd" %}
{% set version = "1.5.2" %}

package:
name: {{ name|lower }}-split
name: zstd-split
version: {{ version }}

source:
url: https://github.com/facebook/{{ name }}/archive/v{{ version }}.tar.gz
url: https://github.com/facebook/zstd/archive/v{{ version }}.tar.gz
sha256: f7de13462f7a82c29ab865820149e778cbfe01087b3a55b5332707abf9db4a6e
patches:
- windows-cmake-pkg-config.patch # [win]

build:
number: 2
number: 3

outputs:
- name: zstd
Expand All @@ -39,26 +38,43 @@ outputs:
- lz4-c
test:
requires:
- cmake
- pkg-config

commands:
- zstd -be -i5
# shared libraries
- test -f ${PREFIX}/lib/libzstd.so # [linux]
- test -f ${PREFIX}/lib/libzstd.dylib # [osx]
- if not exist %LIBRARY_BIN%\zstd.dll exit 1 # [win]
- if not exist %LIBRARY_LIB%\zstd.lib exit 1 # [win]
# duplicated for packages depending on lib-prefixed files
- if not exist %LIBRARY_BIN%\libzstd.dll exit 1 # [win]
- if not exist %LIBRARY_LIB%\libzstd.lib exit 1 # [win]

- test -f ${PREFIX}/include/{{ name }}.h # [unix]
- test ! -f ${PREFIX}/lib/lib{{ name }}.a # [unix]
- test -f ${PREFIX}/lib/lib{{ name }}.so # [linux]
- test -f ${PREFIX}/lib/lib{{ name }}.dylib # [osx]
# absence of static libraries
- test ! -f ${PREFIX}/lib/libzstd.a # [unix]
- if exist %LIBRARY_LIB%\zstd_static.lib exit 1 # [win]
- if exist %LIBRARY_LIB%\libzstd_static.lib exit 1 # [win]

- if not exist %LIBRARY_INC%\{{ name }}.h exit 1 # [win]
- if not exist %LIBRARY_BIN%\lib{{ name }}.dll exit 1 # [win]
- if not exist %LIBRARY_LIB%\lib{{ name }}.lib exit 1 # [win]
- if exist %LIBRARY_LIB%\lib{{ name }}_static.lib exit 1 # [win]
# headers
- test -f ${PREFIX}/include/zstd.h # [unix]
- if not exist %LIBRARY_INC%\zstd.h exit 1 # [win]

# CLI
- zstd -be -i5

- export PKG_CONFIG_PATH=$PREFIX/lib/pkgconfig # [unix]
- set "PKG_CONFIG_PATH=%LIBRARY_LIB%\pkgconfig" # [win]
- test -f ${PREFIX}/lib/pkgconfig/lib{{ name }}.pc # [unix]
- if not exist %LIBRARY_LIB%\pkgconfig\lib{{ name }}.pc exit 1 # [win]
- pkg-config --cflags lib{{ name }}
# pkgconfig
- test -f ${PREFIX}/lib/pkgconfig/libzstd.pc # [unix]
- if not exist %LIBRARY_LIB%\pkgconfig\libzstd.pc exit 1 # [win]
- export PKG_CONFIG_PATH=$PREFIX/lib/pkgconfig # [unix]
- set "PKG_CONFIG_PATH=%LIBRARY_LIB%\pkgconfig" # [win]
- pkg-config --cflags libzstd

# cmake
- cmake --find-package -DNAME=zstd -DLANGUAGE=C -DMODE=EXIST -DCOMPILER_ID=GNU # [linux]
- cmake --find-package -DNAME=zstd -DLANGUAGE=C -DMODE=EXIST -DCOMPILER_ID=Clang # [osx]
# disabled because compiler detection fails, see
# https://discourse.cmake.org/t/questions-about-find-package-cli-msvc/6194
# - cmake --find-package -DNAME=zstd -DLANGUAGE=C -DMODE=EXIST -DCOMPILER_ID=MSVC # [win]

- name: zstd-static
script: install.sh # [unix]
Expand All @@ -72,13 +88,36 @@ outputs:
- ninja
- make # [unix]
- cmake-no-system
host:
- {{ pin_subpackage('zstd', exact=True) }}
run:
- {{ pin_subpackage('zstd', exact=True) }}
run_constrained:
# do not co-install with shared-only lib;
# zstd-static contains the shared libs already anyway
- zstd <0.0a0
Copy link
Contributor

Choose a reason for hiding this comment

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

The issue is that the previous design was completely going against this.

The goal is:

  • Try to compile a package that depends on package a and zstd.
  • a links to zstd dynamically
  • The new package links to zstd statically.

Copy link
Member Author

@h-vetinari h-vetinari Jul 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The issue is that the previous design was completely going against this.

Respectfully, the previous design was a hack, and demonstrably broken (at least from the POV of cmake). I think it's a bad idea for the output containing the static+shared build to depend on the output for the shared-only build - it all but guarantees broken CMake metadata and/or clobbered files.

The goal is:

  • Try to compile a package that depends on package a and zstd.
  • a links to zstd dynamically
  • The new package links to zstd statically.

I don't understand your point here. Why should a package depending on the (already-now) shared zstd end up linking statically?

Copy link
Member Author

@h-vetinari h-vetinari Jul 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

On the off chance that you're referring to "zstd-static contains the shared libs already anyway" - this was previously the case already. In fact, the build breaks when passing both:

  ZSTD_BUILD_STATIC=ON
  ZSTD_BUILD_SHARED=OFF

The error was something like "the CLI cannot be built without the shared lib". I didn't investigate further how to disable building the CLI, as I wanted to keep the previous setup of having both shared & static libs within the zstd-static output - the only point I changed is that it doesn't depend on the zstd output anymore.

PS. It's obviously not great to have shared + static lib in the same output, but that's the way things are currently, not least because the upstream build doesn't make it easy to build only the static one.

Copy link
Member Author

Choose a reason for hiding this comment

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

The error was something like "the CLI cannot be built without the shared lib". I didn't investigate further how to disable building the CLI, as I wanted to keep the previous setup of having both shared & static libs within the zstd-static output - the only point I changed is that it doesn't depend on the zstd output anymore.

I did investigate this now, details in this comment resp. 22966cd

test:
requires:
- cmake
- pkg-config
commands:
- test -f ${PREFIX}/lib/lib{{ name }}.a # [unix]
# shared libraries
- test -f ${PREFIX}/lib/libzstd.so # [linux]
- test -f ${PREFIX}/lib/libzstd.dylib # [osx]

# static libraries
- test -f ${PREFIX}/lib/libzstd.a

# headers
- test -f ${PREFIX}/include/zstd.h

# CLI
- zstd -be -i5

# pkgconfig
- test -f ${PREFIX}/lib/pkgconfig/libzstd.pc
- export PKG_CONFIG_PATH=$PREFIX/lib/pkgconfig
- pkg-config --cflags libzstd

# cmake
- cmake --find-package -DNAME=zstd -DLANGUAGE=C -DMODE=EXIST -DCOMPILER_ID=GNU # [linux]
- cmake --find-package -DNAME=zstd -DLANGUAGE=C -DMODE=EXIST -DCOMPILER_ID=Clang # [osx]

about:
home: http://www.zstd.net
Expand Down