Should we define ZMQ_HAVE_STRLCPY for Linux?

[MarijnS95]

Glibc 2.38 just released, and one of its major new features is a definition for strlcpy:
https://sourceware.org/pipermail/libc-alpha/2023-July/150524.html

The strlcpy and strlcat functions have been added. They are derived from OpenBSD, and are expected to be added to a future POSIX version.

Since we don't turn on ZMQ_HAVE_STRLCPY yet compat.hpp includes #include <string.h>, ZMQ defines its own fallback implementation which isn't compatible with the declaration given by glibc:

cargo:warning=In file included from .cargo/registry/src/index.crates.io-6f17d22bba15001f/zeromq-src-0.2.5+4.3.4/vendor/src/ipc_address.cpp:31:
cargo:warning=.cargo/registry/src/index.crates.io-6f17d22bba15001f/zeromq-src-0.2.5+4.3.4/vendor/src/compat.hpp:45:1: error: ‘size_t strlcpy(char*, const char*, size_t)’ was declared ‘extern’ and later ‘static’ [-fpermissive]
cargo:warning=   45 | strlcpy (char *dest_, const char *src_, const size_t dest_size_)
cargo:warning=      | ^~~~~~~
cargo:warning=In file included from .cargo/registry/src/index.crates.io-6f17d22bba15001f/zeromq-src-0.2.5+4.3.4/vendor/src/compat.hpp:34:
cargo:warning=/usr/include/string.h:506:15: note: previous declaration of ‘size_t strlcpy(char*, const char*, size_t)’
cargo:warning=  506 | extern size_t strlcpy (char *__restrict __dest,
cargo:warning=      |               ^~~~~~~

One of the obvious downsides of moving away from cmake "recently" is that the cmake scripts maintained by ZMQ developers have specific and intentional checks for existence of these symbols to know if they should otherwise emit a fallback implementation as the above:

https://github.com/zeromq/libzmq/blob/ec013f3a17beaa475c18e8cf5e93970800e2f94a/CMakeLists.txt#L255

It seems we've just wishy-washy defined some of these while knowing barely anything about the target system, and I don't want to break anyone still on Glibc <= 2.37 so we should likely reimplement that compile-testing macro in Rust (or check for glibc version, but that seems more work / error-prone).

[jean-airoldie]

It seems we've just wishy-washy defined some of these while knowing barely anything about the target system

Yeah the current build system is convenient since we understand it and doesn't have a CMAKE dependency, but it for sure doesn't come close to replicating the original CMAKE build, mostly because of its massive complexity (~2k lines of code). Moreover we don't have an easy way to test for all the possible targets to find bugs such as the one you provided.

There's really only two scenarios in my view. We either stick with our makeshift recreation of the cmake build and we fix it bit by bit as users file issues, or we revert back to using the cmake build. The first approach doesn't sound very good but so far we haven't that many issues. The second approach is a breaking change and is mildly inconvenient to the user, but as the advantage of not needing to maintain the build (although i think we had issues with cmake before). I'm kind of on the fence currently.

[jean-airoldie]

[...] so we should likely reimplement that compile-testing macro in Rust (or check for glibc version, but that seems more work / error-prone).

The link you provided seems to refer to this macro which sounds kind of a pain to implement.

https://github.com/zeromq/libzmq/blob/ec013f3a17beaa475c18e8cf5e93970800e2f94a/CMakeLists.txt#L255
https://github.com/Kitware/CMake/blob/21edd5af1f24d12612e3f6052b9497b848566855/Modules/CheckSymbolExists.cmake#L140

From my understanding CMAKE implements this macro by trying to compile while linking explicitly to the symbol its trying to check to see if its available. Sounds like a pain, and very inneficient.

Checking for glibc seems like less work, but potentially more error-prone as you mentionned.

[MarijnS95]

@jean-airoldie I didn't mean to bring you to ideas 😉 - we dearly need this (the removal of cmake) as it massively degrades the "Rust-only" developer experience, having to have cmake available on a host system.

The macro is indeed quite simple and "the standard" in C/C++ land... I'm sure we can reproduce that quite easily with cc-rs. It's a one-time thing anyway.

[jean-airoldie]

@jean-airoldie I didn't mean to bring you to ideas wink - we dearly need this (the removal of cmake) as it massively degrades the "Rust-only" developer experience, having to have cmake available on a host system.

I'm just explaining the available solutions so you understand the situation, since you didn't seem too happy with the current quality of the build, which is understandable. The thing is, the original CMAKE build is a monster.

The macro is indeed quite simple and "the standard" in C/C++ land... I'm sure we can reproduce that quite easily with cc-rs. It's a one-time thing anyway.

I just don't like the idea of trying to compile something, if it fail, set some flag. That doesn't sound like a sane solution at all, even though that's what the original did.

[MarijnS95]

@jean-airoldie forgot to mention I contributed to the removal of cmake, we need it at @Traverse-Research. I am indeed unhappy with the obvious/expected issues it is already giving us (now on Linux and earlier on Android in #20), but would be more unhappy to have to deal with cmake on developers' Windows machines 😩

---

I'm pretty sure this is what the typical ./configure scripts also do. It's ugly to invoke in the context of a Rust build script, but afaik part of any C(++) project that's bigger than someones hobby project.

This also makes me re-realize and appreciate (my daily use of) Rust ever so slightly more, where such definition problems are pretty much not even a thing 😬