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

Override optimization cflags set by users or build systems #169

Open
wants to merge 274 commits into
base: master
Choose a base branch
from

Conversation

friday
Copy link
Contributor

@friday friday commented Jan 12, 2019

This overrides optimization CFLAGS set in build system, and possibly user settings.

Hopefully this can avoid #99 / #103 / #112 / #162 at least a bit.

Perhaps @neuromancer could verify this works for him too? I originally used your tip to unset cflags completely, but I think this way is better as a general advice.

@tkashkin
Copy link
Owner

That should work. I'll wait for someone else to confirm it's working.

@neuromancer
Copy link
Contributor

I will test it it, but I'm not sure if I will be able to reproduce the original issue to check that the fix is doing something...

@friday
Copy link
Contributor Author

friday commented Jan 12, 2019

@neuromancer
Thanks :)
If I run cat /etc/makepkg.conf | grep CFLAGS I get:
CFLAGS="-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -fno-plt"

Not sure if this is the default in Arch, but I think it is in Antergos (which I'm using).

I think -O0 either turns all the other options off or they are benign in this case.

Also the gamehub-git package was just updated so it unsets CFLAGS, so if you use that as a base, the unset statements should be removed for the test.

@neuromancer
Copy link
Contributor

I tested all the variations of the CFLAGS (including -O0, -O1 and -O2) only using -O0 works as expected. I feel very uncomfortable disabling all the optimizations in the rest of the compilation just for this issue, but we have no other choice. I think we should merge this until we found the real cause of #99.

@friday
Copy link
Contributor Author

friday commented Jan 20, 2019

Yeah, I agree. Optimally this shouldn't be needed (but the issue is too serious not to do something)

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

Successfully merging this pull request may close these issues.

5 participants