Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compilation on Fedora 40 with GCC 14 #893

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kiilerix
Copy link
Contributor

@kiilerix kiilerix commented May 6, 2024

More strict correctness of C include files cause some failures, with the core of the problem explained as comments next to the solutions/workarounds.

It seems like the best solutions are careful
#define _GNU_SOURCE
or
#define _XOPEN_SOURCE 500
similar to what the code base already has in some places.

More strict correctness of C include files cause some failures, with the
core of the problem explained as comments next to the
solutions/workarounds.

It seems like the best solutions are careful
    #define _GNU_SOURCE
or
    #define _XOPEN_SOURCE 500
similar to what the code base already has in some places.
@x42
Copy link
Member

x42 commented May 6, 2024

from stat(2)

       lstat():
           /* glibc 2.19 and earlier */ _BSD_SOURCE
               || /* Since glibc 2.20 */ _DEFAULT_SOURCE
               || _XOPEN_SOURCE >= 500
               || /* Since glibc 2.10: */ _POSIX_C_SOURCE >= 200112L

So think we should define _POSIX_C_SOURCE=200809L (or _XOPEN_SOURCE).
The former ideally in the wscript. What do you think?

@kiilerix
Copy link
Contributor Author

kiilerix commented May 6, 2024

I don't have strong opinions on this topic. I only studied it enough to understand that it is complex and to find something that worked for me.

Big disclaimer: While we are talking about setting it in wscript, it is not clear if we are talking about setting it for all compilers on all platforms, or only on gcc and perhaps only on linux.

I guess I stand approximately like the response on https://lwn.net/Articles/592652/ :
Since Ardour is cross platform and has to work on platforms that expose APIs without checking feature macros correctly, there might be limited value and lurking problems if trying to do it strictly correct on modern linux.

(I do however also think there is value in getting these errors reported loud and clear so the codebase can be made as correct as possible.)

Defining _GNU_SOURCE will make gcc as sloppy as any other platform, and it will clearly(?) not influence any other platform. It thus seems like a safe choice. Especially if only adding it in few .c files where the impact is limited. This was thus my safe minimal change.

I do however also like the idea of a global _POSIX_C_SOURCE=200809L and trying to make that work everywhere, as simple as possible. Thus ideally also cleaning up all the related hacks we have in the code base. But as hinted on lwn, we might have to keep some. But can we generally assume that we only support compilers / platforms that support _POSIX_C_SOURCE=200809L reasonably correct?

@x42
Copy link
Member

x42 commented May 6, 2024

We already use _POSIX_C_SOURCE in other places for the same or similar reason libs/fst/wscript, libs/tk/ydk-pixbuf/wscript

@kiilerix
Copy link
Contributor Author

kiilerix commented May 6, 2024

Yes. And _GNU_SOURCE is already used at file level.

And there could be a risk that a global _POSIX_C_SOURCE would change something on some platforms that broke something. But the risk is probably very low.

I don't have a strong opinion. I think both can work - I just shared what worked for me ;-)

Also, setting something globally also calls for a cleanup of local hacks.

pauldavisthefirst pushed a commit that referenced this pull request May 6, 2024
from stat(2)
```
lstat():
    /* glibc 2.19 and earlier */ _BSD_SOURCE
        || /* Since glibc 2.20 */ _DEFAULT_SOURCE
        || _XOPEN_SOURCE >= 500
        || /* Since glibc 2.10: */ _POSIX_C_SOURCE >= 200112L

```
@x42
Copy link
Member

x42 commented May 7, 2024

Can this be closed?

@x42
Copy link
Member

x42 commented May 7, 2024

PS.
_GNU_SOURCE is problematic on *BSD and macOS. At some point we should remove the few uses of it (mostly a-* LV2 plugins)

@kiilerix
Copy link
Contributor Author

kiilerix commented May 7, 2024

This PR still fixes the realpath issue.

Is _GNU_SOURCE problematic on BSD in other ways than just doing what it says and not making any difference (good or bad) for non GNU compilers?

@x42
Copy link
Member

x42 commented May 7, 2024

The file libs/aaf/utils.c will be overwritten when upstream libaaf is updated. We copy it into our source tree.

Either the define is set in Ardour's wscript, or the change need to happen upstream at:
https://github.com/agfline/LibAAF/blob/master/src/common/utils.c

@markk
Copy link
Contributor

markk commented May 8, 2024

Building from master HEAD 94e0f2d is still failing for me on Fedora 40. Not sure if it's the same issue, but looks related:

Waf: Leaving directory `/home/user/build/ardour/build'
Build failed
 -> task in 'libaaf' failed with exit status 1:
        {task 140634082147728: c utils.c -> utils.c.1.o}
['/usr/lib64/ccache/gcc', '-I/home/user/build/ardour', '-DHAVE_RF64_RIFF', '-DCOMPILER_INT128_SUPPORT', '-DWAF_BUILD', '-DNDEBUG', '-fshow-column', '-O3', '-fomit-frame-pointer', '-ffast-math', '-fstrength-reduce', '-pipe', '-DARCH_X86', '-mmmx', '-msse', '-mfpmath=sse', '-DUSE_XMMINTRIN', '-DBUILD_SSE_OPTIMIZATIONS', '-DLXVST_64BIT', '-Wall', '-Wpointer-arith', '-Wcast-qual', '-Wcast-align', '-Wno-unused-parameter', '-DBOOST_SYSTEM_NO_DEPRECATED', '-DBOOST_BIND_GLOBAL_PLACEHOLDERS', '-D_ISOC9X_SOURCE', '-D_LARGEFILE64_SOURCE', '-D_FILE_OFFSET_BITS=64', '-DPROGRAM_NAME="Ardour"', '-DPROGRAM_VERSION="8"', '-Wstrict-prototypes', '-Wmissing-prototypes', '-fPIC', '-fPIC', '-Ilibs/aaf', '-I../libs/aaf', '-I/home/usere/build/ardour/build', '-DLUABINDINGDOC=1', '-DINTERNAL_SHARED_LIBS=1', '-DYTK=1', '-DHAVE_SUIL=1', '-DHAVE_ALSA=1', '-DHAVE_PULSEAUDIO=1', '-DHAVE_GLIB=1', '-DHAVE_GTHREAD=1', '-DHAVE_GLIBMM=1', '-DHAVE_SNDFILE=1', '-DHAVE_GIOMM=1', '-DHAVE_CURL=1', '-DHAVE_ARCHIVE=1', '-DHAVE_LO=1', '-DHAVE_TAGLIB=1', '-DHAVE_VAMPSDK=1', '-DHAVE_VAMPHOSTSDK=1', '-DHAVE_RUBBERBAND=1', '-DHAVE_USB=1', '-DHAVE_RUBBERBAND_3_0_0=1', '-DEXPORT_VISIBILITY_HIDDEN=0', '-DENABLE_NLS=1', '-DLXVST_SUPPORT=1', '-DVST3_SUPPORT=1', '-DUSE_FUTEX_SEMAPHORE=1', '-DHAVE_JACK=1', '-DFPU_AVX512F_SUPPORT=1', '-DFPU_AVX_FMA_SUPPORT=1', '-DCONFIG_ARCH="x86_64"', '-DHAVE_TOOLS_SANITY_CHECK=1', '-DHAVE_FFTW3F=1', '-DHAVE_UDEV=1', '-DHAVE_HIDAPI=1', '-DHAVE_AUBIO=1', '-DHAVE_AUBIO4=1', '-DHAVE_GOBJECT=1', '-DHAVE_GIO=1', '-DHAVE_LIBPNG=1', '-DHAVE_PANGO=1', '-DHAVE_CAIRO=1', '-DHAVE_PANGOCAIRO=1', '-DHAVE_GIO_UNIX=1', '-DHAVE_RANDR=1', '-DHAVE_RANDR15=1', '-DHAVE_XFREE_XINERAMA=1', '-DHAVE_GMODULE=1', '-DHAVE_X11=1', '-DHAVE_XEXT=1', '-DHAVE_SIGCPP=1', '-DHAVE_CAIROMM=1', '-DHAVE_PANGOMM=1', '-DHAVE_LV2_1_16_0=1', '-DHAVE_XML=1', '-DHAVE_EXECINFO=1', '-DHAVE_POSIX_MEMALIGN=1', '-DHAVE_GETMNTENT=1', '-DHAVE_LOCALTIME_R=1', '-DHAVE_CPPUNIT=1', '-DHAVE_CONTROL_PROTOCOL=1', '-DHAVE_MIDI_SURFACE=1', '-DHAVE_JACK_METADATA=1', '-DHAVE_JACK_PORT_RENAME=1', '-DHAVE_LRDF=1', '-DHAVE_SAMPLERATE=1', '-DHAVE_LV2=1', '-DHAVE_LV2_1_10_0=1', '-DHAVE_LV2_1_17_2=1', '-DHAVE_LV2_1_18_6=1', '-DHAVE_SERD=1', '-DHAVE_SORD=1', '-DHAVE_SRATOM=1', '-DHAVE_LILV=1', '-DLV2_SUPPORT=1', '-DLV2_EXTENDED=1', '-DHAVE_OGG=1', '-DHAVE_FLAC=1', '-DHAVE_FFTW35F=1', '-DUSE_RUBBERBAND=1', '-DCURRENT_SESSION_FILE_VERSION=7003', '-DHAVE_SYS_VFS_H=1', '-DHAVE_SYS_STATVFS_H=1', '-DHAVE_UNISTD=1', '-DHAVE_BOOST_SCOPED_PTR_HPP=1', '-DHAVE_BOOST_PTR_CONTAINER_PTR_LIST_HPP=1', '-DHAVE_BOOST_SHARED_PTR_HPP=1', '-DHAVE_BOOST_FORMAT_HPP=1', '-DHAVE_LV2_1_0_0=1', '-DHAVE_PANGOFT2=1', '-DHAVE_FONTCONFIG=1', '-DHAVE_READLINE=1', '-DHAVE_DBUS=1', '-DLIBAAF_DLL_EXPORTS=1', '-DPACKAGE="libaaf"', '-D_POSIX_C_SOURCE=200809L', '../libs/aaf/utils.c', '-c', '-o/home/user/build/ardour/build/libs/aaf/utils.c.1.o']

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants