Add _XOPEN_SOURCE to C definitions
ClosedPublic

Authored by awilcox on Oct 12 2017, 2:27 AM.

Details

Summary

(This was reported on the KDE bug tracker as Bug 373175. The summary of that bug follows.)

When building on non-glibc Unix platforms, such as Solaris, NetBSD, and Linux/musl or Linux/uclibc, multiple components of KF5 fail to build. This is because -std=iso9899:1990 is specified (for strict C90 compliance) but _XOPEN_SOURCE is not defined, so none of the POSIX interfaces are exported. I have seen this reported in at least:

  • kinit
  • kscreenlocker
  • plasma-workspace

The attached patch resolves this issue on all our build boxes; additionally, applying it to my glibc builder did not change the already working result.

Test Plan

The entire Adélie Linux distribution's set of KDE packages has been built against extra-cmake-modules-5.38.0 with this patch applied. The Plasma 5 desktop and KDE software build, test, and run correctly (where applicable).

Diff Detail

Repository
R240 Extra CMake Modules
Lint
Lint Skipped
Unit
Unit Tests Skipped
awilcox created this revision.Oct 12 2017, 2:27 AM
Restricted Application added projects: Frameworks, Build System. · View Herald TranscriptOct 12 2017, 2:27 AM
Restricted Application added subscribers: Build System, Frameworks. · View Herald Transcript

Please do note that _XOPEN_SOURCE must be 600 and not 700 because some components of KDE still use rindex which is no longer supported in X/Open 7.

dfaure accepted this revision.Dec 2 2017, 7:25 PM
dfaure added a subscriber: dfaure.

I did a test compile with this locally and it seems fine indeed.

This revision is now accepted and ready to land.Dec 2 2017, 7:25 PM
mpyne added a subscriber: mpyne.Dec 2 2017, 8:31 PM

As I mentioned on the KDE bug, there is a possibility that changes like this would break FreeBSD. FreeBSD operates by a model of exporting symbols by default for legacy/BSD-specific needs. However if a feature flag is set, it limits its exports to *only* that required by the feature flag. Notably, all BSD-specific exports are removed in FreeBSD if you set a feature flag that doesn't also standardize that BSD feature, and unlike C libraries like musl and glibc, they don't support any feature flags to specifically request BSD-specific exports be exported.

See KIO commit 15451d505fdbc37b5d027f8ad1ba6aeb6314cc2d and D6597 for an example of where I had to take special pains when committing musl-specific fixes to KF5 to avoid breaking FreeBSD.

Andrew, have you been able to test your patches on a FreeBSD at all? We can catch it in CI now, but on the other hand I have already tried to fix this by pushing feature flag defines into source repositories requiring the extra defines rather than making it a KDE-wide define. I had previously verified that these fixes allowed all of KF5 (though not all of Plasma5) to build on Alpine Linux, though it's been a month since I had reverified that.

dfaure added a comment.Aug 1 2018, 8:06 AM

(BTW where is rindex being used? lxr says only in ksysguard's FreeBSD and IRIX files, surely that's not the issue?)

Restricted Application edited subscribers, added: kde-buildsystem, kde-frameworks-devel; removed: Frameworks, Build System. · View Herald TranscriptAug 1 2018, 8:06 AM

rindex was being used in kscreenlocker but that code was removed last year.

Let's push this then ;)

You don't seem to have a developer account? I can push this but I need your email address for the git authorship.

Ah, found it in bugzilla :-)

This revision was automatically updated to reflect the committed changes.

This change appears to have resulted in a severe regression of builds on FreeBSD.
Any ideas?

dfaure added a comment.Sep 1 2018, 9:20 AM

Indeed,

22:23:51 In file included from /usr/include/sys/mount.h:36:
22:23:51 /usr/include/sys/ucred.h:84:2: error: unknown type name 'u_int'; did you mean 'uint'?
22:23:51 u_int cr_version; /* structure layout version */
22:23:51 ^

https://build.kde.org/view/Frameworks/job/Frameworks%20kcoreaddons%20kf5-qt5%20FreeBSDQt5.11/5/console

I'll revert.

mpyne added a comment.Sep 2 2018, 2:53 PM

Sorry, if I'd been able to check this earlier I'd have warned against pushing, this was the kind of concern I was addressing with my earlier comment about FreeBSD. I had run into a lot of those issues when trying to get KF5 to build on Alpine.

Adding an _XOPEN_SOURCE or similar define will in general require a lot of porting effort to remove things like obsolete type names, BSD-specific extensions, functions used in later standards than the one we adopt, etc.

It was easier for me to make fixes per-Framework than to try to get everything to compile under an _XOPEN_SOURCE setting, but it may be better as a long-term concern to standardize on something and then do the one-time cost to make sure everything still builds. Certainly I think we don't want to continue to rely implicitly on glibc forever given the expansion of other form factors that use alternate libc's.