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

GNUmakefile: minor compiler and flags improvement #1671

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

Conversation

pstef
Copy link

@pstef pstef commented Oct 20, 2024

With this applied, it's now possible to choose a different clang version, and pass the canonical GNU makefile variables CFLAGS and CXXFLAGS.

@Mastergatto
Copy link
Contributor

Mastergatto commented Oct 20, 2024

Hi, not an Ares developer here, but I'm looking forward to this PR with great interest, as I'm maintaining the AUR package. Although I'm not sure about the CFLAGS/CXXFLAGS part there.

Did you made sure that CFLAGS/CXXFLAGS aren't being overridden by later and conflicting flags? For example, see here:

ifneq ($(filter $(arch),x86 amd64),)
ifeq ($(filter cl,$(compiler)),)
ifeq ($(local),true)
flags += -march=native
else
# For official builds, default to x86-64-v2 (Intel Nehalem, AMD Bulldozer) which supports up to SSE 4.2.
flags += -march=x86-64-v2
endif
endif
endif

This part only allows choice between -march=native and -march=x86-64-v2; and here I believe that it sets the flag later than CFLAGS/CXXFLAGS thus it would override whatever the user sets for -march, due to the compiler's policy. And this is just one of a series of similar cases.

@pstef
Copy link
Author

pstef commented Oct 20, 2024

I see it as a policy question and as such, beyond the scope of this PR.

If the project's decision is to set some flags explicitly and in a way that overwrites whatever flags the user might have passed, then I'm in no position to change that policy.

The way this patch works now is that I can pass whatever I think will serve me, and if it's something that is never set (or reset) later, then it's more flexible that way and brings me some convenience. But if it is reset then probably for good reason, so the project will have the final say.

If I'm really stubborn I can change that locally, hack, fork, etc.

That's at least how I see it, and I don't have strong feelings about it.

@Mastergatto
Copy link
Contributor

The thing is, the codebase passes a lot of compiler flags as you can see, so the usefulness of CFLAGS/CXXFLAGS, as laid out in this PR, would be more limited. It may work out for you, but it cannot be said the same for other users. In fact, some AUR users asked me about this -march thing.

In response, I have already created a (yet to be published) patch for the AUR package before this PR, essentially it passes CFLAGS/CXXFLAGS at a later stage, so that it does have always the last word.

Though, as it stands currently, I'm not fully convinced of the validity of my patch and I think there must be a better way to handle CFLAGS/CXXFLAGS. Maybe by rethinking the makefile, or by just waiting for the replacement of the current build system...

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

Successfully merging this pull request may close these issues.

2 participants