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
unify '#ifdef HAVE_CONFIG_H' declaration #3127
Conversation
Isn't
|
Is there some good use case for setting the I would say for this use case, the |
I remember C compilers failing to compile with an error seeing #if defined(A) && A
It's been a while since I programmed in C, so don't remember clearly. |
This is still actual and I fixed it in several places in #3128. But now I think that we should just remove HAVE_CONFIG_H from the code. Is there any reason to keep this archaism at all? |
I still believe it should be there. As discussed for example in amule-project/amule#241 this might be useful for platforms without the config.h. Does the windows build have the I see the meson and cmake provide the config.h and they can likely run on windows too so with Meson, this might not be issue. Similar changes were done in other projects such as https://gitlab.freedesktop.org/pipewire/pipewire/-/merge_requests/748/diffs but I think they should land only after we will be sure the meson works for all the platforms we are interested and after we will remove the autotools. |
You could also remove On Windows, we can build without autoconf (and without config.h). (It is kind of cumbersome to set up initially.) If we push meson so far so that we can replace the autotools, then we can remove the ifdef's, because we will always build with config.h, but for now we should stick with the existing approach. |
I just read that the C99 standard also specifies at §6.10.1:
So I still prefer to use |
Regardless it is ok based on the C99 standard, I find the gcc warnings about undefined macros helpful as it can help spotting a mistake/typo. Removing the |
Making this an implicit rule, would mean to apply this for all the other instances where we are checking for dependencies. We can start here, but should work through the rest of the code at some point. |
@frankmorgner whats the status of this PR? Are we good with this state or do you want some changes? |
Preparation for introducing Meson.
Created with commands:
Without the patch:
With the patch: