[Bug]: freetype 2.13.3 rules are incompatible with OpenSSL or Zlib in the same binary

[arvid-norlander]

What happened?

Freetype bundles parts of OpenSSL and Zlib by default, and in the cmake / autoconf etc build systems you can choose to link to zlib / openssl instead of using the bundled versions.

The build in this repo isn't set up to use those as dependencies, causing ODR errors if you try to use normal OpenSSL or normal Zlib as well as the Freetype 2.13.3 build from BCR.

It would be good if some bool_flags or similar could be added to let the user control this, as Freetype from BCR currently isn't usable by us due to this. I can make a PR but I would appreciate help on what the best way to do this is.

Basically what needs to be accomplished is to have the following in the BUILD file:

    copts = [
        "-DFT_CONFIG_OPTION_SYSTEM_ZLIB",
        "-DHAVE_OPENSSL",
        # Other flags as before
    ],
    # ...
    deps = [
        "@boringssl//:crypto",  # or @openssl//:crypto, depending on your preference, we use boringssl
        "@zlib",
    ],

Version

Development (host) and target OS/architectures:

Output of bazel --version: bazel 8.1.1

Version of relevant rules from the WORKSPACE or MODULE.bazel file: freetype 2.13.3

Language(s) and/or frameworks involved: C++?

How to reproduce

Build a project using freetype as well as at least one of OpenSSL/BoringSSL/Zlib. Get ODR errors and linker errors.

Any other information?

No response

[arvid-norlander]

cc @pswaminathan @Wyverald (who were involved with the last freetype update).

I am able to try to fix this, but I first need some guidance on how.

There needs to be some way to toggle some defines and dependencies basically. Otherwise it is C++ undefined behaviour to include both freetype and any one of OpenSSL, BoringSSL and Zlib in the same build tree.

[pswaminathan]

The BUILD file part of this could be straightforward: we could make a boolean flag that would link those externally and add the deps and macros as local defines (or one flag for each). Something like:

diff --git a/BUILD.bazel b/BUILD.bazel
index 41f3c0d..39e5c59 100644
--- a/BUILD.bazel
+++ b/BUILD.bazel
@@ -1,5 +1,19 @@
+load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
+
 licenses(["notice"])
 
+bool_flag(
+    name = "link_external_zlib",
+    build_setting_default = False,
+)
+
+config_setting(
+    name = "is_link_external_zlib",
+    flag_values = {
+        ":link_external_zlib": "true",
+    },
+)
+
 # n.b. srcs comes from the BASE_SRCS variable in upstream CMakeLists.txt
 # (c.f. https://gitlab.freedesktop.org/freetype/freetype/-/blob/VER-2-13-3/CMakeLists.txt)
 cc_library(
@@ -55,7 +69,14 @@ cc_library(
     copts = [
         "-DFT2_BUILD_LIBRARY",
         "-UDEBUG",
-    ],
+    ] + select({
+        ":is_link_external_zlib": ["-DFT_CONFIG_OPTION_SYSTEM_ZLIB"],
+        "//conditions:default": []
+    }),
+    deps = select({
+        ":is_link_external_zlib": ["@zlib"],
+        "//conditions:default": []
+    }),
     includes = ["include"],
     textual_hdrs = glob(["src/**/*.c"]),
     visibility = ["//visibility:public"],

But this does introduce questions around declared dependencies in MODULE.bazel. We could list zlib in MODULE.bazel, but with OpenSSL I'm a little less sure what a good path forward is. Do we list both in MODULE.bazel and offer a string flag to choose one or the other or none?

I don't really feel strongly one way or the other. It does look like cURL (as one example) lists both and has flags to select.

[fmeum]

bazel_deps are relatively cheap, so the approach taken by cURL seems pretty reasonable.

But if each module that depends on an SSL lib introduces its own flag, configuration becomes error-prone: all flags need to be set to the same value for the build to avoid ODR violation.

What do you think of introducing an artifical ssl_lib module that depends on both openssl and boringssl and contains the single, canonical string_flag to choose between them (and possible others)? It could then offer a cc_library that picks between the different implementations. The flag in curl could become an alias for this new centralized flag.

CC @keith

[arvid-norlander]

@fmeum I like the idea of centralising such flags. But curl seem to support three backends:

• BoringSSL
• OpenSSL
• mbedtls

BoringSSL and OpenSSL are API compatible dropins for each other as I understand it. MbedTLS doesn't seem to be that.

So Curl would have three options for backend, while some other libraries (like freetype) only have two options.

For zlib there is actually a similar situation: There is (at least) zlib, zlib-ng and zlib-rs that all provide the same API and can act as drop-in replacements for each other. I don't know of any library that implements zlib with a non-compatible API thankfully.

[arvid-norlander]

Also, I just discovered there is a similar optional bundling/dependency on bzip2. We hadn't noticed this one as we don't use bzip2 elsewhere, but I ran across this when looking into building gstreamer with bazel (where it does become a problem). Ideally all optional dependencies where a library otherwise bundles part of the library should have such flags.

[pswaminathan]

So I suppose there are two parts to this:

1. Consolidating flags for external selection of various libraries (be it for SSL or for zlib) to prevent errors related to having multiple flags setting the same value that aren't in sync
2. (a) Providing flags for Freetype to set these, and (b) selecting on those flags for different library inclusion

Point 2.a is implemented by point 1 for libraries that have lots of shared usages. Initially looked like just TLS libraries, but maybe for zlib as well. ssl_lib would have a BUILD file that has something like:

string_flag(
    name = "lib_selection",
    build_setting_default = "none",
    values = ["none", "OpenSSL", "BoringSSL", "MbedTLS"],
)

This can be selected in .bazelrc:

build --@ssl_lib//:lib_selection={OpenSSL,BoringSSL,MbedTLS}

The freetype patch then looks like:

diff --git a/BUILD.bazel b/BUILD.bazel
index 41f3c0d..fec05af 100644
--- a/BUILD.bazel
+++ b/BUILD.bazel
@@ -1,5 +1,21 @@
+load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
+
 licenses(["notice"])
 
+config_setting(
+    name = "is_openssl",
+    flag_values = {
+        "@ssl_lib//:lib_selection": "OpenSSL",
+    },
+)
+
+config_setting(
+    name = "is_boringssl",
+    flag_values = {
+        "@ssl_lib//:lib_selection": "BoringSSL",
+    },
+)
+
 # n.b. srcs comes from the BASE_SRCS variable in upstream CMakeLists.txt
 # (c.f. https://gitlab.freedesktop.org/freetype/freetype/-/blob/VER-2-13-3/CMakeLists.txt)
 cc_library(
@@ -56,6 +72,11 @@ cc_library(
         "-DFT2_BUILD_LIBRARY",
         "-UDEBUG",
     ],
+    deps = select({
+        ":is_openssl": ["@openssl//:crypto"],
+        ":is_boringssl": ["@boringssl//:crypto"],
+        "//conditions:default": []
+    }),
     includes = ["include"],
     textual_hdrs = glob(["src/**/*.c"]),
     visibility = ["//visibility:public"],

(There would need to be some handling for copts based on the OR of those flags, but let's ignore that for now.) cURL can still support MbedTLS, while Freetype does not support it, and that's ok—Freetype just ignores its presence as a flag choice.

Does this sound reasonable as a general approach?

As a more specific question for Freetype: I'll preface by saying I don't have direct experience with the issue you're running into here. We don't link against both FT and zlib. However, looking at the source, it seems as though this isn't supposed to be needed. What am I missing? https://github.com/freetype/freetype/blob/42608f77f20749dd6ddc9e0536788eaad70ea4b5/include/freetype/config/ftoption.h#L210-L214

[arvid-norlander]

What am I missing?

Based on the comment it indeed sounds like it should work. But when I didn't define that in our application I ran into linker errors. Perhaps it is a question of dynamic vs static linking triggering this? Or maybe they improved this on a recent version of freetype (it is possible I ran into the error on something slightly older than 2.13 but newer than the previous version on bazelmod, I'm not sure of the timeline here).

I could go back and try to dig up what the exact linker errors were. But even without that it would be good to avoid code bloat from having multiple copies of the same code (I know, this is a strange concept in the age of 300 MB electron apps ;-)).

OpenSSL definitely has this issue still, both OpenSSL and Freetype use the same symbol names MD5_*: https://github.com/freetype/freetype/blob/1019b1c2b9f5faf75f4d12d938cc2f874be25a65/src/base/md5.h

[fmeum]

Now that I see how many potential value a string_flag for SSL libs could have, I wonder whether this should instead be a label_flag that by default points to @openssl. This would avoid a potentially growing number of deps of the "ssl lib meta module".