Skip to content

Misc build fixes#3519

Merged
ralight merged 1 commit intoeclipse-mosquitto:masterfrom
ymorin-orange:nyma/for-upstream
Feb 25, 2026
Merged

Misc build fixes#3519
ralight merged 1 commit intoeclipse-mosquitto:masterfrom
ymorin-orange:nyma/for-upstream

Conversation

@ymorin-orange
Copy link
Contributor

  • Have you signed the Eclipse Contributor Agreement, using the same email address as you used in your commits?
  • Do each of your commits have a "Signed-off-by" line, with the correct email address? Use "git commit -s" to generate this line for you.
  • If you are contributing a bugfix, is your work based off the fixes branch?
    • ==> No, as the fixes branch is still based off the 2.0.x release series
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you successfully run make test with your changes locally?
    • ==> No, as they do not even build on master

This series brings in a few build fixes for corner cases:

  • only require C++ when building the C++ wrapper library
  • properly check for getaddrinfo_a()
  • check for getrandom() at configure time

@ralight
Copy link
Contributor

ralight commented Feb 23, 2026

@ymorin-orange Could you please explain the circumstances where these changes are required?

@ymorin-orange
Copy link
Contributor Author

@ralight Thanks for looking at this PR!

@ymorin-orange Could you please explain the circumstances where these changes are required?

The issues arrised when trying to bring the new Mosquitto version from the 2.1.x series, to Buildroot. Buildroot is a build system for embedded devices, using cross-compilation to build from source.

In Buildroot, there is a script that (basically) builds a package using various toolchains, which are known to exhibit the issues most often reported when targetting devices in the wild. This uncovers corner cases (Like dependency on MMU for fork() or dlopen(), no static building, specific C standard version, etc...)

When cross-compiling, there are cases where:

  • the toolchain lacks C++ support; this can be for various reasons, but if one has no C++ component to integrate in one's device, and one is building their own toolchain, then building a C++ toolchain is superfluous and takes time;
  • for getaddrinfo_a(), the man page does document that _GNU_SOURCE is needed. While glibc may get it defined by way of including another header before netdb.h (like stdlib.h), this is not guaranteed on other C libraries, like uClibc-ng or musl (musl has strict standards compliance), or with older/newer glibc versions where the headers have been cleaned up to no longer have unexpected side effects like leaking a define they should not have (sorry, no pointer in mind, but there were such changes a little while ago in glibc IIRC); also, the code does define _GNU_SOURCE, but the define is not defined in CMakeList.txt;
  • finally, it is better to check for getrandom() at configure time, like checking for getaddrinfo_a() is already done, rather than at compile time; not all C libraries provide it, like uClibc-ng where it can be configured out, or older versions may lack it.

I had thought the comments in the commit logs were enough; if not, tell me, so I can expand them (but I can surely fix the typoes I overlooked, of course! Sorry for those, I'm very bad at proof-reading my own words...).

@ymorin-orange
Copy link
Contributor Author

@ralight Thanks for triggering the workflows. 👍

I'll address the build failures ASAP (expect a day or two so I can sneak that in my schedule).

@codecov
Copy link

codecov bot commented Feb 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
libcommon/random_common.c 100.00% <ø> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ralight
Copy link
Contributor

ralight commented Feb 24, 2026

@ymorin-orange Thanks for the details - sorry I missed the extended comments in the commits, it's more unusual to get PRs with them in.

I've pushed the getaddrinfo commit already because that is fine as it is.

I've fixed the C++ support case and pushed a commit for that, so it's just the getrandom which needs sorting.

@ymorin-orange
Copy link
Contributor Author

ymorin-orange commented Feb 24, 2026

@ymorin-orange Thanks for the details - sorry I missed the extended comments in the commits, it's more unusual to get PRs with them in.

No worries, I'm more used to maling-list based workflows, so eh! ;-)

I've pushed the getaddrinfo commit already because that is fine as it is.
I've fixed the C++ support case and pushed a commit for that, so

Thanks! I like your C++ fix better than mine, great!

it's just the getrandom which needs sorting.

If you need more about that, feel free to ask. It basically moves the check for the availability of getrandom() from the code (build time) to the CMake (configure).

Ah... But I may have overlooked the build directly using Makefiles. That's what bothers you, right? If so, then I can address that as well. I think keeping the #else clause to trigger at build would suffice, no?

@ralight
Copy link
Contributor

ralight commented Feb 24, 2026

Ah... But I may have overlooked the build directly using Makefiles. That's what bothers you, right? If so, then I can address that as well. I think keeping the #else clause to trigger at build would suffice, no?

Exactly - at least for now the Makefiles have to keep support. make should pull in support from openssl, make WITH_TLS=no should use getrandom if available.

@ymorin-orange
Copy link
Contributor Author

Ah... But I may have overlooked the build directly using Makefiles. That's what bothers you, right? If so, then I can address that as well. I think keeping the #else clause to trigger at build would suffice, no?

Exactly - at least for now the Makefiles have to keep support. make should pull in support from openssl, make WITH_TLS=no should use getrandom if available.

OK, lemme look into that, then.

Thanks for the review!

Not all C libraries are glibc, or impersonating it; for example, musl
does not pretend to be any version of glibc. Thus, building on musl
fails when openssl is disabled, because no random-providing function is
detected, although musl does provide getrandom().

uClibc-ng on the other hand, can impersonate glibc, but availability of
getrandom() is not guatranteed there: getrandom() can be compiled out of
uClinbc-ng, or uClibc-ng can be too old to have it.

Add a configure-time check that getrandom() is available, as a fallback
when TLS is not enabled (and thus openssl is not used), and when not on
Win32 (where getting random numbers is always possible, at least from a
build perspective).

However, building with the plain Makefiles should keep working, so
slightly rework the defines checks in the code to account for the fact
that HAVE_GETRANDOM may already be defined at configure time.

Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
@ralight ralight merged commit a487cd4 into eclipse-mosquitto:master Feb 25, 2026
14 checks passed
@ralight
Copy link
Contributor

ralight commented Feb 25, 2026

Nice, thanks very much

@ymorin-orange ymorin-orange deleted the nyma/for-upstream branch February 25, 2026 08:50
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.

2 participants