modules: drop Basis' own bundled Zstd in favor of the real library.

This basically means users now have to either install a system zstd
package or add zstd as a CMake subproject, as described in the docs. If
Zstd isn't found, Basis gets compiled without Zstd support, meaning only
non-zstd-compressed KTX files will be supported for both import and
image conversion.

However, Basis complicates all this by doing the following insanity:

    #include "../zstd/zstd.h"

Fortunately, after patiently waiting since December 2021 for at least
one of the three fix PRs to get emrged there, I realized this could be
fixed by providing a fake include directory that would delegate to the
real zstd.h. And it seems to work, even without needing #include_next.

Ultimately updating also dev PKGBUILDs and all CIs to use this, since it
just makes no sense anymore to use the bundled version. It's slower than
the current one, and it doesn't contain any security updates.

modules/FindBasisUniversal.cmake

@@ -274,30 +274,25 @@ foreach(_component ${BasisUniversal_FIND_COMPONENTS})
                 set(BasisUniversalTranscoder_DEFINITIONS "BASISU_NO_ITERATOR_DEBUG_LEVEL")
 
                 # Try to find an external Zstandard library because that's the
-                # sanest and most flexible option. Otherwise, it's a nightmare.
+                # sanest and most flexible option.
                 #
-                # If not found, Basis bundles its own, but unfortunately as one
-                # huge file containing a decoder+encoder and another file
-                # containing just the decoder. However, because the Encoder
-                # links to Transcoder, we can't link the Transcoder to
-                # zstddeclib.c because together with Encoder linking to zstd.c
-                # this would lead to duplicate symbol errors.
-                find_package(Zstd QUIET)
+                # Yes, Basis bundles its own, but unfortunately as one huge
+                # file containing a decoder+encoder and another file containing
+                # just the decoder. Which is a problem, because the Encoder
+                # depends on Transcoder and using zstddeclib.c together with
+                # zstd.c would lead to duplicate symbol errors. Not to mention
+                # the "one huge file" makes it harder for the linker to perform
+                # DCE, leading to potential binary bloat. And, a third problem,
+                # pthreads being implicitly enabled in the amalgamated build,
+                # requiring users to link to it, even though Basis doesn't use
+                # any threaded compression in the end.
+                find_package(Zstd)
                 if(NOT Zstd_FOUND)
-                    find_path(BasisUniversalZstd_DIR NAMES zstd.c
-                        HINTS "${BASIS_UNIVERSAL_DIR}/zstd" "${BASIS_UNIVERSAL_DIR}"
-                        NO_CMAKE_FIND_ROOT_PATH)
-                    mark_as_advanced(BasisUniversalZstd_DIR)
-                    if(BasisUniversalZstd_DIR)
-                        list(APPEND BasisUniversalTranscoder_SOURCES
-                            ${BasisUniversalZstd_DIR}/zstd.c)
-                    else()
-                        # If zstd wasn't found, disable Zstandard
-                        # supercompression support at compile time. The zstd.h
-                        # include is hidden behind this definition as well.
-                        list(APPEND BasisUniversalTranscoder_DEFINITIONS
-                            "BASISD_SUPPORT_KTX2_ZSTD=0")
-                    endif()
+                    # If zstd wasn't found, disable Zstandard supercompression
+                    # support at compile time. The zstd.h include is hidden
+                    # behind this definition as well.
+                    list(APPEND BasisUniversalTranscoder_DEFINITIONS
+                        "BASISD_SUPPORT_KTX2_ZSTD=0")
                 endif()
 
                 foreach(_file ${BasisUniversalTranscoder_SOURCES})
@@ -322,18 +317,6 @@ foreach(_component ${BasisUniversal_FIND_COMPONENTS})
                 if(Zstd_FOUND)
                     set_property(TARGET BasisUniversal::Transcoder APPEND PROPERTY
                         INTERFACE_LINK_LIBRARIES Zstd::Zstd)
-                elseif(BasisUniversalZstd_DIR)
-                    # The bundled zstd.c has ZSTD_MULTITHREAD unconditionally
-                    # defined for all platforms except Emscripten, which forces
-                    # us to link to pthread. The transcoder doesn't use any of
-                    # the multithreaded interfaces so for it alone it shouldn't
-                    # lead to the same issues described for the encoder above,
-                    # however because the encoder transitively links to the
-                    # encoder, we can't link to pthread here to avoid the
-                    # crashes there. It's instead done in the BasisImporter
-                    # CMakeLists depending on the (cached) variable set here.
-                    # It's extremely brittle that way but so is Basis itself,
-                    # so what.
                 endif()
             endif()
         else()

package/archlinux/PKGBUILD-emscripten-wasm

@@ -6,10 +6,7 @@ pkgdesc="Plugins for the Magnum C++11/C++14 graphics engine (Emscripten, wasm)"
 arch=('any')
 url="https://magnum.graphics"
 license=('MIT')
-depends=('emscripten-magnum=dev.wasm' 'emscripten-faad2')
-# Emscripten doesn't have zstd on its own, so we fall back to using the one
-# bundled with Basis Universal. TODO redo when the bundled dependency is
-# dropped
+depends=('emscripten-magnum=dev.wasm' 'emscripten-faad2' 'emscripten-zstd')
 makedepends=('cmake' 'emscripten' 'corrade' 'ninja' 'basis-universal-src')
 options=(!strip !buildflags)
 

package/archlinux/PKGBUILD-emscripten-wasm-webgl2

@@ -6,10 +6,7 @@ pkgdesc="Plugins for the Magnum C++11/C++14 graphics engine (Emscripten WebGL 2,
 arch=('any')
 url="https://magnum.graphics"
 license=('MIT')
-depends=('emscripten-magnum=dev.wasm.webgl2' 'emscripten-faad2')
-# Emscripten doesn't have zstd on its own, so we fall back to using the one
-# bundled with Basis Universal. TODO redo when the bundled dependency is
-# dropped
+depends=('emscripten-magnum=dev.wasm.webgl2' 'emscripten-faad2' 'emscripten-zstd')
 makedepends=('cmake' 'emscripten' 'corrade' 'ninja' 'basis-universal-src')
 options=(!strip !buildflags)
 

package/ci/android-x86.sh

@@ -69,7 +69,7 @@ ninja install
 cd ../..
 
 # Crosscompile zstd
-export ZSTD_VERSION=1.5.0
+export ZSTD_VERSION=1.5.5
 wget --no-check-certificate https://github.com/facebook/zstd/archive/refs/tags/v$ZSTD_VERSION.tar.gz
 tar -xzf v$ZSTD_VERSION.tar.gz
 cd zstd-$ZSTD_VERSION

package/ci/appveyor.yml

@@ -118,11 +118,11 @@ install:
 - IF "%TARGET%" == "desktop" IF "%APPVEYOR_BUILD_WORKER_IMAGE%" == "Visual Studio 2017" IF "%COMPILER:~0,4%" == "msvc" appveyor DownloadFile https://ci.magnum.graphics/freetype-2.10.4-windows-2016.zip && 7z x freetype-2.10.4-windows-2016.zip -o%APPVEYOR_BUILD_FOLDER%\deps
 
 # Basis Universal
-# TODO: currently it uses the bundled zstd (because lazy), when that gets
-# dropped this needs to fetch a real zstd also
 - set BASIS_VERSION=1_15_update2
 - IF NOT EXIST %APPVEYOR_BUILD_FOLDER%\v%BASIS_VERSION%.zip appveyor DownloadFile https://github.com/BinomialLLC/basis_universal/archive/v%BASIS_VERSION%.zip
 - 7z x v%BASIS_VERSION%.zip && ren basis_universal-%BASIS_VERSION% basis_universal
+# We want to use the external Zstd instead of this
+- rmdir /s /q basis_universal\zstd
 
 # SPIRV-Tools, for MSVC 2022, 2019, 2017 and clang-cl only
 # This line REQUIRES the COMPILER variable to be set on rt builds, otherwise it
@@ -145,8 +145,8 @@ install:
 
 # zstd for MSVC 2022, 2019 and clang-cl; MinGW. MSVC 2017 seems to work well
 # with the 2019 build. Needed by Basis Universal.
-- IF "%TARGET%" == "desktop" IF "%COMPILER:~0,4%" == "msvc" appveyor DownloadFile https://ci.magnum.graphics/zstd-1.5.2-windows-2019.zip && 7z x zstd-1.5.2-windows-2019.zip -o%APPVEYOR_BUILD_FOLDER%\deps
-- IF "%TARGET%" == "desktop" IF "%COMPILER%" == "mingw" appveyor DownloadFile https://ci.magnum.graphics/zstd-1.5.2-windows-mingw.zip && 7z x zstd-1.5.2-windows-mingw.zip -o%APPVEYOR_BUILD_FOLDER%\deps
+- IF "%TARGET%" == "desktop" IF "%COMPILER:~0,4%" == "msvc" appveyor DownloadFile https://ci.magnum.graphics/zstd-1.5.5-windows-2019.zip && 7z x zstd-1.5.5-windows-2019.zip -o%APPVEYOR_BUILD_FOLDER%\deps
+- IF "%TARGET%" == "desktop" IF "%COMPILER%" == "mingw" appveyor DownloadFile https://ci.magnum.graphics/zstd-1.5.5-windows-mingw.zip && 7z x zstd-1.5.5-windows-mingw.zip -o%APPVEYOR_BUILD_FOLDER%\deps
 
 # meshoptimizer for MSVC 2022, 2019 and clang-cl; MinGW. MSVC 2017 doesn't work
 # with the 2019 build unfortunately, and can't build it because of

package/ci/circleci.yml

@@ -227,6 +227,8 @@ commands:
           mkdir -p $HOME/basis_universal && cd $HOME/basis_universal
           wget -nc https://github.com/BinomialLLC/basis_universal/archive/$BASIS_VERSION.tar.gz
           tar --strip-components 1 -xzf $BASIS_VERSION.tar.gz
+          # We want to use the external Zstd instead of this
+          rm -r zstd
 
   install-openexr:
     parameters:

package/ci/emscripten.sh

@@ -76,7 +76,10 @@ mv $HOME/deps/lib/{libfaad.a,faad.bc}
 mv $HOME/deps/lib/{libfaad_drm.a,faad_drm.bc}
 cd ..
 
-# Crosscompile zstd
+# Crosscompile zstd. Version 1.5.1+ doesn't compile with this Emscripten
+# version, saying that
+#   huf_decompress_amd64.S: Input file has an unknown suffix, don't know what to do with it!
+# Newer Emscriptens work fine, 1.5.0 doesn't have this file yet so it works.
 export ZSTD_VERSION=1.5.0
 wget --no-check-certificate https://github.com/facebook/zstd/archive/refs/tags/v$ZSTD_VERSION.tar.gz
 tar -xzf v$ZSTD_VERSION.tar.gz

package/ci/ios-simulator.sh

@@ -65,7 +65,7 @@ set -o pipefail && cmake --build . --config Release --target install -j$XCODE_JO
 cd ../..
 
 # Crosscompile zstd
-export ZSTD_VERSION=1.5.0
+export ZSTD_VERSION=1.5.5
 wget --no-check-certificate https://github.com/facebook/zstd/archive/refs/tags/v$ZSTD_VERSION.tar.gz
 tar -xzf v$ZSTD_VERSION.tar.gz
 cd zstd-$ZSTD_VERSION

src/MagnumPlugins/BasisImageConverter/BasisImageConverter.h

@@ -94,11 +94,26 @@ Current version of the plugin is tested against the
 but could possibly compile against newer versions as well.
 
 Additionally, if you're using Magnum as a CMake subproject, bundle the
-[magnum-plugins](https://github.com/mosra/magnum-plugins) and
+[magnum-plugins](https://github.com/mosra/magnum-plugins),
+[zstd](https://github.com/facebook/zstd) and
 [basis-universal](https://github.com/BinomialLLC/basis_universal) repositories
-and do the following:
+and do the following. Basis uses Zstd for KTX2 images, instead of bundling you
+can depend on an externally-installed zstd package, in which case omit the
+first part of the setup below. If Zstd isn't bundled and isn't found
+externally, Basis will be compiled without Zstd support.
 
 @code{.cmake}
+set(ZSTD_BUILD_PROGRAMS OFF CACHE BOOL "" FORCE)
+# Create a static library so the plugin is self-contained
+set(ZSTD_BUILD_SHARED OFF CACHE BOOL "" FORCE)
+set(ZSTD_BUILD_STATIC ON CACHE BOOL "" FORCE)
+# Basis doesn't use any multithreading in zstd, this prevents a need to link to
+# pthread on Linux
+set(ZSTD_MULTITHREAD_SUPPORT OFF CACHE BOOL "" FORCE)
+# Don't build Zstd tests if enable_testing() was called in parent project
+set(ZSTD_BUILD_TESTS OFF CACHE BOOL "" FORCE)
+add_subdirectory(zstd/build/cmake EXCLUDE_FROM_ALL)
+
 set(BASIS_UNIVERSAL_DIR ${CMAKE_CURRENT_SOURCE_DIR}/basis-universal)
 set(MAGNUM_WITH_BASISIMAGECONVERTER ON CACHE BOOL "" FORCE)
 add_subdirectory(magnum-plugins EXCLUDE_FROM_ALL)

src/MagnumPlugins/BasisImageConverter/CMakeLists.txt

@@ -60,7 +60,10 @@ if(MAGNUM_BASISIMAGECONVERTER_BUILD_STATIC AND MAGNUM_BUILD_STATIC_PIC)
 endif()
 target_include_directories(BasisImageConverter PUBLIC
     ${PROJECT_SOURCE_DIR}/src
-    ${PROJECT_BINARY_DIR}/src)
+    ${PROJECT_BINARY_DIR}/src
+    # Turns #include "../zstd/zstd.h" into an actual external zstd.h include.
+    # See the README in that directory for details.
+    ${PROJECT_SOURCE_DIR}/src/external/basis-zstd-include-uncrapifier/put-this-on-include-path)
 target_link_libraries(BasisImageConverter
     PUBLIC Magnum::Trade
     PRIVATE BasisUniversal::Encoder)

src/MagnumPlugins/BasisImporter/BasisImporter.h

@@ -101,11 +101,28 @@ against the [`v1_15_update2` tag](https://github.com/BinomialLLC/basis_universal
 but could possibly compile against newer versions as well.
 
 Additionally, if you're using Magnum as a CMake subproject, bundle the
-[magnum-plugins](https://github.com/mosra/magnum-plugins) and
+[magnum-plugins](https://github.com/mosra/magnum-plugins),
+[zstd](https://github.com/facebook/zstd) and
 [basis-universal](https://github.com/BinomialLLC/basis_universal) repositories
-and do the following:
+and do the following. Basis uses Zstd for KTX2 images, instead of bundling you
+can depend on an externally-installed zstd package, in which case omit the
+first part of the setup below. If Zstd isn't bundled and isn't found
+externally, Basis will be compiled without Zstd support. See
+@ref Trade-BasisImporter-binary-size below for additional options to control
+what gets built.
 
 @code{.cmake}
+set(ZSTD_BUILD_PROGRAMS OFF CACHE BOOL "" FORCE)
+# Create a static library so the plugin is self-contained
+set(ZSTD_BUILD_SHARED OFF CACHE BOOL "" FORCE)
+set(ZSTD_BUILD_STATIC ON CACHE BOOL "" FORCE)
+# Basis doesn't use any multithreading in zstd, this prevents a need to link to
+# pthread on Linux
+set(ZSTD_MULTITHREAD_SUPPORT OFF CACHE BOOL "" FORCE)
+# Don't build Zstd tests if enable_testing() was called in parent project
+set(ZSTD_BUILD_TESTS OFF CACHE BOOL "" FORCE)
+add_subdirectory(zstd/build/cmake EXCLUDE_FROM_ALL)
+
 set(BASIS_UNIVERSAL_DIR ${CMAKE_CURRENT_SOURCE_DIR}/basis-universal)
 set(MAGNUM_WITH_BASISIMPORTER ON CACHE BOOL "" FORCE)
 add_subdirectory(magnum-plugins EXCLUDE_FROM_ALL)

src/MagnumPlugins/BasisImporter/CMakeLists.txt

@@ -62,19 +62,13 @@ endif()
 target_include_directories(BasisImporter
     PUBLIC
     ${PROJECT_SOURCE_DIR}/src
-    ${PROJECT_BINARY_DIR}/src)
+    ${PROJECT_BINARY_DIR}/src
+    # Turns #include "../zstd/zstd.h" into an actual external zstd.h include.
+    # See the README in that directory for details.
+    ${PROJECT_SOURCE_DIR}/src/external/basis-zstd-include-uncrapifier/put-this-on-include-path)
 target_link_libraries(BasisImporter
     PUBLIC Magnum::Trade
     PRIVATE BasisUniversal::Transcoder)
-# If we have bundled Basis sources and the bundled Basis sources bundle zstd
-# (which has ZSTD_MULTITHREAD unconditionally defined for all platforms, SIGH),
-# link the plugin to pthread. Can't do it directly in the Find module directly
-# for BasisUniversal::Transcoder for reasons described there.
-if(BasisUniversalZstd_DIR AND NOT CORRADE_TARGET_EMSCRIPTEN)
-    set(THREADS_PREFER_PTHREAD_FLAG TRUE)
-    find_package(Threads REQUIRED)
-    target_link_libraries(BasisImporter PRIVATE Threads::Threads)
-endif()
 
 install(FILES BasisImporter.h ${CMAKE_CURRENT_BINARY_DIR}/configure.h
     DESTINATION ${MAGNUM_PLUGINS_INCLUDE_INSTALL_DIR}/BasisImporter)

src/external/basis-zstd-include-uncrapifier/README.md

@@ -0,0 +1,16 @@
+These directories exist in order to make this Basis Universal insanity
+
+```cpp
+#include "../zstd/zstd.h"
+```
+
+actually include an external Zstd installation. I.e., an up-to-date version
+with all SECURITY FIXES and OPTIMIZATIONS that happened since the original file
+was put in the Basis repository in April 2021. The `put-this-on-include-path/`
+subdirectory is added to the include path, which then makes `../zstd/zstd.h`
+point to the `zstd.h` file in the `zstd/` subdirectory here. That file then
+uses `#include <zstd.h>` (with `<>` instead of `""`) to include the external
+`zstd.h` instead of the bundled file.
+
+The real fix is in https://github.com/BinomialLLC/basis_universal/pull/228,
+waiting to get some attention since December 2021. Haha.

src/external/basis-zstd-include-uncrapifier/put-this-on-include-path/dummy

@@ -0,0 +1 @@
+

src/external/basis-zstd-include-uncrapifier/zstd/zstd.h

@@ -0,0 +1,35 @@
+#ifndef Magnum_BasisZstdIncludeUncrapifier_h
+#define Magnum_BasisZstdIncludeUncrapifier_h
+/*
+    This file is part of Magnum.
+
+    Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019,
+                2020, 2021, 2022, 2023 Vladimír Vondruš <[email protected]>
+
+    Permission is hereby granted, free of charge, to any person obtaining a
+    copy of this software and associated documentation files (the "Software"),
+    to deal in the Software without restriction, including without limitation
+    the rights to use, copy, modify, merge, publish, distribute, sublicense,
+    and/or sell copies of the Software, and to permit persons to whom the
+    Software is furnished to do so, subject to the following conditions:
+
+    The above copyright notice and this permission notice shall be included
+    in all copies or substantial portions of the Software.
+
+    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+    FROM, OUT OF OR IN CONNETCION WITH THE SOFTWARE OR THE USE OR OTHER
+    DEALINGS IN THE SOFTWARE.
+*/
+
+/* By using <> this should pick the real zstd.h and not this file again. A
+   safer bet would be #include_next, but that doesn't work on MSVC. */
+#include <zstd.h>
+#ifndef ZSTD_VERSION_MAJOR
+#error expected to have a real zstd on the include path
+#endif
+
+#endif