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

doc/building-plugins.dox

@@ -129,10 +129,10 @@ makepkg -fp PKGBUILD # or any other PKGBUILD file
 @endcode
 
 @note The PKGBUILDs reference a few external dependencies such as
-    `basis-universal-src` or (in case of Emscripten) `emscripten-faad2`, which
-    are not in AUR. You can find PKGBUILDs for these at
-    https://github.com/mosra/archlinux. You can also edit the package, disable
-    building of affected features and remove the dependency.
+    `basis-universal-src` or (in case of Emscripten) `emscripten-zstd` and
+    `emscripten-faad2`, which are not in AUR. You can find PKGBUILDs for these
+    at https://github.com/mosra/archlinux. You can also edit the package,
+    disable building of affected features and remove the dependency.
 
 In most cases the development PKGBUILDs also contain a @cb{.sh} check() @ce
 function which will run all unit tests before packaging. That might sometimes

doc/changelog-plugins.dox

@@ -332,6 +332,11 @@ namespace Magnum {
     plugins don't compile against the previous 1.11 version of Basis anymore,
     you have to update the dependency to the `v1_15_update2` tag (see
     [mosra/magnum-plugins#112](https://github.com/mosra/magnum-plugins/pull/112))
+-   @relativeref{Trade,BasisImporter} and @relativeref{Trade,BasisImageConverter}
+    plugins now rely on externally supplied zstd installation instead of using
+    the amalgamated file in Basis sources, which is outdated, lacks security
+    fixes and many optimizations. If no zstd is found, the Basis library is
+    compiled without zstd support. See [mosra/magnum-plugins#115](https://github.com/mosra/magnum-plugins/pull/115).
 -   The @cb{.ini} separate_rg_to_color_alpha @ce option from
     @relativeref{Trade,BasisImageConverter} is gone, use
     @cb{.ini} swizzle=rrrg @ce instead. For RG inputs this is done implicitly

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()

modules/FindMagnumPlugins.cmake

@@ -339,12 +339,30 @@ foreach(_component ${MagnumPlugins_FIND_COMPONENTS})
             if(basisu_FOUND AND NOT BASIS_UNIVERSAL_DIR)
                 set_property(TARGET MagnumPlugins::${_component} APPEND PROPERTY
                     INTERFACE_LINK_LIBRARIES basisu_encoder)
+            else()
+                # Our own build may depend on Zstd, as we replace the bundled
+                # files with an external library. Include it if present,
+                # otherwise assume it's compiled without.
+                find_package(Zstd)
+                if(Zstd_FOUND)
+                    set_property(TARGET MagnumPlugins::${_component} APPEND PROPERTY
+                        INTERFACE_LINK_LIBRARIES Zstd::Zstd)
+                endif()
             endif()
         elseif(_component STREQUAL BasisImporter)
             find_package(basisu CONFIG QUIET)
             if(basisu_FOUND AND NOT BASIS_UNIVERSAL_DIR)
                 set_property(TARGET MagnumPlugins::${_component} APPEND PROPERTY
                     INTERFACE_LINK_LIBRARIES basisu_transcoder)
+            else()
+                # Our own build may depend on Zstd, as we replace the bundled
+                # files with an external library. Include it if present,
+                # otherwise assume it's compiled without.
+                find_package(Zstd)
+                if(Zstd_FOUND)
+                    set_property(TARGET MagnumPlugins::${_component} APPEND PROPERTY
+                        INTERFACE_LINK_LIBRARIES Zstd::Zstd)
+                endif()
             endif()
 
         # BcDecImageConverter has no dependencies

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