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

Packaging updates #457

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Packaging updates #457

wants to merge 1 commit into from

Conversation

jherico
Copy link

@jherico jherico commented Mar 1, 2024

These changes are designed to produce a more "packagable" set of artifacts suitable for inclusion in a package manager.

Sepcificially, I'd like the add astcenc to vcpkg to make it easier to consume for downstream projects, but vcpkg requires conformance to a set of best practices in the libraries that are published there.

I could accomplish that conformance through a series of patches that would be equivalent to these changes, but it's preferred to make the changes directly upstream if possible.

The biggest change is that the project no longer builds a static library plus optionally a shared library. Instead it follows the setting specified in BUILD_SHARED_LIBRARIES to build one or the other. Additionally, it no longer forces a value for MSVC_RUNTIME_LIBRARY, since for windows builds via vcpkg that's handled automatically via the selection of one of the 3 windows "triplets": x64-windows, x64-windows-static and x64-windows-static-md.

Note that vcpkg users will still be able to target specific SIMD ISAs via options. This is an existing package that does something similar.

I've also added the use of CPack configuration files so that downstream packages can integrate this more easily into their projects by calling find_package(astcenc) and getting an astcenc::astcenc imported target that they can use in target_link_libraries to get both the library linkage information and the include paths.

I've also added a new option called ASTCENC_LEGACY_NAMING which defaults to ON. This causes the output binaries to have the same names as the existing CMake config, but explicitly turning it off will cause the project to use base names with ISA or shared/static suffixes.

For simplicity and readability I've changed the names of all the targets in the cmake_core.cmake file to strings like astcenc for the library, veneer for the CLI lib, and cli for the CLI executable, then use the OUTPUT_NAME property to actually control the name of the binary.

Here's an example of building and installing with these changes...

$ cmake -B build -DASTCENC_LEGACY_NAMING=OFF -DCMAKE_INSTALL_PREFIX=/e/astc -DASTCENC_ISA_AVX2=ON
-- Selecting Windows SDK version 10.0.22621.0 to target Windows 10.0.19045.
--   AVX2 backend     - ON
--   SSE4.1 backend   - OFF
--   SSE2 backend     - OFF
--   NEON backend     - OFF
--   NONE backend     - OFF
--   NATIVE backend   - OFF
--   Invariance       - ON
--   Shared libs      - OFF
--   Decompressor     - OFF
--   Diagnostics      - OFF
--   ASAN             - OFF
--   Unit tests       - OFF
-- Configuring done
-- Generating done
-- Build files have been written to: C:/Users/bdavi/kgit/astc-encoder/build

$ cmake --build build/ --target install
MSBuild version 17.8.5+b5265ef37 for .NET Framework

  astcenc.vcxproj -> C:\Users\bdavi\kgit\astc-encoder\build\Source\Debug\astcenc.lib
  veneer.vcxproj -> C:\Users\bdavi\kgit\astc-encoder\build\Source\Debug\veneer.lib
  cli.vcxproj -> C:\Users\bdavi\kgit\astc-encoder\build\Source\Debug\astcenc.exe
  -- Install configuration: "Debug"
  -- Installing: E:/astc/include
  -- Installing: E:/astc/include/astcenc.h
  -- Up-to-date: E:/astc/lib/astcenc.lib
  -- Up-to-date: E:/astc/share/astcenc/astcencTargets.cmake
  -- Up-to-date: E:/astc/share/astcenc/astcencTargets-debug.cmake
  -- Up-to-date: E:/astc/share/astcenc/astcencConfig.cmake
  -- Up-to-date: E:/astc/bin/astcenc.exe

@jherico
Copy link
Author

jherico commented Mar 1, 2024

I believe this has broken the MacOS universal build. I'm going to investigate what the typical approach to that is and see if there are any templates for this kind of thing in vcpkg.

@jherico jherico marked this pull request as draft March 3, 2024 05:51
@jherico
Copy link
Author

jherico commented Mar 3, 2024

OK, wow. The universal binary bit is tricky, but I think there's an alternative that doesn't require lipo and custom commands.

@solidpixel
Copy link
Contributor

solidpixel commented Mar 4, 2024

The universal binary bit is tricky, but I think there's an alternative that doesn't require lipo and custom commands.

The challenge is trying to feed different compile-time constants and #defines into the code based on the target ISA. It might be possible by doing more inference of options based on the available ISA, rather than being explicit on the command line, but I failed last time I tried to do it. In particular I hit issues around multiple x86 builds, trying to get both x86 and x86h slices working.

@solidpixel solidpixel self-assigned this Mar 4, 2024
@solidpixel solidpixel self-requested a review March 4, 2024 08:57
@jherico
Copy link
Author

jherico commented Mar 5, 2024

I attempted both an approach using static linking and an approach using CMake "object" libraries, where it produces .o files but doesn't attempt to link them until they're included in an actual binary. This approach might work for making a universal binary for x86_64/arm64, but not the triplet of x86_64h/x86_64/arm64.

The approach I've taken seems to work for universal binaries as well as individual (valid) combinations of SIMD ISAs and host environments, but it does still use lipo for the universal binaries. However, it moves the bulk of the per-ISA CMake configuration into the new SIMD.cmake file and allows the for loop to be isolated to only the universal-builds, eliminating the need for a large cmake_core.cmake file that gets included multiple times.

I still need to address the unit test code though.

@jherico
Copy link
Author

jherico commented Mar 5, 2024

One question, I'm unclear on the intended difference between native and none. Is native supposed to be using no defines and just detecting whatever the compiler offers, while none is explicitly supposed to set the defines all to zero?

@solidpixel
Copy link
Contributor

none builds are designed to explicitly select the reference C backend so we can test it on platforms that would otherwise pick up SIMD support.

native builds are designed to select whatever defaults the compiler/CPU supports based on the compiler-set defines.

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

Successfully merging this pull request may close these issues.

None yet

2 participants