Skip to content

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...

@LukeUsher
Copy link
Member

Closing as this is not particularly relevant after the move to cmake;

@LukeUsher LukeUsher closed this Jan 27, 2025
@pstef
Copy link
Author

pstef commented Jan 27, 2025

Thanks for letting us know.

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.

3 participants