-
Notifications
You must be signed in to change notification settings - Fork 537
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 build without PCH #241
Conversation
Signed-off-by: Nick Sarnie <[email protected]>
Duplicate of #223 Unfortunately, just like my PR, it's incomplete. It only adds missing includes for which compilation fails when PCH is disabled. There is at least one header that should be included yet generates no error when missing, because it only contains preprocessor defines which are then checked with "#ifdef". It's "acconf.h" and it is dynamically generated by Meson in the build directory. It looks something like this: /*
* Autogenerated by the Meson build system.
* Do not edit, your changes will be lost.
*/
#pragma once
#define P_DATA "/usr/share/aegisub"
#define P_LOCALE "/usr/share/locale"
#define WITH_FFMS2 1
#define WITH_FFTW3 1
#define WITH_FONTCONFIG 1
#define WITH_HUNSPELL 1
#define WITH_OPENAL 1
#define WITH_UCHARDET 1
#undef WITH_UPDATE_CHECKER Now you might understand why your Gentoo ebuild fails to enable some features: https://forums.gentoo.org/viewtopic-t-1172621.html I gave up and enabled PCH in my ebuild: https://github.com/stefantalpalaru/gentoo-overlay/blob/299fddb35393a52f4bf2158a9bd0c4900ea71fd5/media-video/aegisub/aegisub-3.4.2.ebuild#L114-L119 |
Nice catch. A bit more investigation shows that Aegisub was able to build without PCHs in the past but that this broke with the meson port. By replacing |
partial credit seems to be applied in your pr so sounds good to me! |
It seems that Aegisub used to be able to be built without precompiled headers, but this broke with the meson port since there the PCHs are needed to include acconf.h. (On the old build system(s), the feature flags were passed directly as defines to the compiler on Visual Studio, and acconf.h was forcibly included via -include acconf.h with autoconf.) Some distributions (gentoo in particular) disable PCHs by default for meson due to various bugs, and PCHs can also be quite a headache with language servers (I ended up running an sed one-liner after every configure to replace -include-pch in my compile_commands.json with the respective -include). Since meson doesn't seem to be able to forcibly include headers for every source file, just pass the config as a set of preprocessor defines when b_pch is disabled. If it's enabled, stick to acconf.h to not bloat the compiler command lines too much. Disable b_pch on the Ubuntu CI lanes to catch missing includes but keep it enabled on Windows and Mac for faster compilation (with compiling on Windows already taking around an hour). The added includes were mostly taken from #241 and #223. Co-authored-by: Ștefan Talpalaru <[email protected]> Co-authored-by: Nick Sarnie <[email protected]>
It seems that Aegisub used to be able to be built without precompiled headers, but this broke with the meson port since there the PCHs are needed to include acconf.h. (On the old build system(s), the feature flags were passed directly as defines to the compiler on Visual Studio, and acconf.h was forcibly included via -include acconf.h with autoconf.) Some distributions (gentoo in particular) disable PCHs by default for meson due to various bugs, and PCHs can also be quite a headache with language servers (I ended up running an sed one-liner after every configure to replace -include-pch in my compile_commands.json with the respective -include). Since meson doesn't seem to be able to forcibly include headers for every source file, just pass the config as a set of preprocessor defines when b_pch is disabled. If it's enabled, stick to acconf.h to not bloat the compiler command lines too much. For now this only works on Linux, Windows will need extra work due to windows.h being annoying, but there isn't as much of a need to build without PCHs there anyway. The added includes were mostly taken from #241 and #223. Co-authored-by: Ștefan Talpalaru <[email protected]> Co-authored-by: Nick Sarnie <[email protected]>
This fixes the build with PCH disabled on Linux. Here is the
setup
command I used to reproduce the failure.