-
-
Notifications
You must be signed in to change notification settings - Fork 976
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
Build fails if -D_FORTIFY_SOURCES is in env CFLAGS #7310
Comments
I am on Arch too, it doesnt set CFLAGS as an env var. This must be |
Okay, strictly speaking, Arch doesn't set it, but makepkg does: https://gitlab.archlinux.org/archlinux/packaging/packages/pacman/-/blob/main/makepkg.conf?ref_type=heads |
The kitty-git package builds perfectly when manually running |
Okay, I don't know. I haven't tried this. Anyway, it's fixed now. :) |
@kovidgoyal doesn't 20375ee break |
It prevents it from overriding fortify source yes, but there are many |
Describe the bug
setup.py
prepends the env CFLAGS with-D_FORTIFY_SOURCES=2
. This leads to a build failure when the env CFLAGS contain a different definition. For example, Arch Linux recently switched to-D_FORTIFY_SOURCES=3
(see https://rfc.archlinux.page/0017-increase-fortification-level/). When I try to build the Arch AUR packagekitty-git
, I get this (after modifyingsetup.py
not to suppress stderr):See https://aur.archlinux.org/packages/kitty-git#comment-961665 for more reports of this.
Analysis/Suggestion
This is a bit unfortunate. Usually, later CFLAGS override earlier CFLAGS (to give the user the last word), but not in the case of
-D
. (Strictly speaking,-D_FORTIFY_SOURCES=2
belongs to CPPFLAGS, but it probably won't matter, and even Arch Linux's defaults get this wrong... edit: Apparently this is there for good reasons: https://rfc.archlinux.page/0003-buildflags/)I suggest
-D_FORTIFY_SOURCES=2
only if CFLAGS and CPPFLAGS don't contain the substring-D_FORTIFY_SOURCES=
-D_FORTIFY_SOURCES=3
(this alone would make the problem go away on Arch Linux, but it's not a proper solution...)The text was updated successfully, but these errors were encountered: