On 6/24/22 01:32, Florian Weimer wrote:
* Tom Stellard:
> On 6/7/22 02:18, Florian Weimer wrote:
>> * Ben Cotton:
>>
>>> This change will add new macros which will make it easier for packages
>>> to add and remove their own compiler flags. This strategy is already
>>> used to some extent with feature macros like %{_lto_cflags},
>>> %{_hardening_cflags}, etc, but these new macros will give packagers
>>> even more fine-grained control over the options.
>>>
>>> The proposed macros for adding new flags are:
>>>
>>> %_pkg_extra_cflags
>>> %_pkg_extra_cxxflags
>>> %_pkg_extra_fflags
>>> %_pkg_extra_ldflags
>> Why isn't it possible to use the environment variables for this?
>>
>
> Environment variables won't cover all cases. For example, some packages
> need the flags passed as arguments to make or configure or some other
> build system.
But then you can stick the extra flags into that argument. I don't see
why this additional hook is needed.
>> This is very misleading because in several cases, clearing the those
>> flags will not affect toolchain behavior because the flag in question
>> merely restates the toolchain default. In particular, this applies to
>> -fasynchronous-unwind-tables, and it would apply to -march= and -mcpu=
>> if those were in the list. Likewise to -Wl,-z,relro.
>> Is the goal of this proposal just to achieve a textual flags change
>> (disregarding any change in behavior), or to actually change toolchain
> The goal is to make it easy to remove individual flags so packages can
> get the toolchain behavior they want.
Sorry, this does not answer my question. What if removing the flag does
not actually change toolchain behavior?
This was not something I had considered. We could update the macro so
that if a macro was set to %{nil} then the flag to turn off the feature
is used so something like:
%build_cflags %[ "%{_flag_asynchronous_unwind_tables}" == "" ?
"-fno-asynchronous-unwind-tables" :
"%{_flag_asynchronous_unwind_tables}"] ...
Or we could just leave it as is and document that the default complier
behavior takes precedence over compiler flags. The proposed change doesn't actually
make the problem worse, because the current method for removing compiler flags
has the same problem: e.g.
%global optflags %(echo %{optflags} | sed 's/-fasynchronous-unwind-tables//
will remove the flag but not change the behavior.
- Tom
>> If the former, I'm not sure if the actual _flag names are
useful. Maybe
>> we can add an RPM macro to suppress or replace flags based on the flags
>> as they are used instead. This could also add additional error checking
>> because a name typo in the macro definition will not be immediately
>> obvious.
>>
>
> Can you give an example of what you mean by this?
You could write:
%build_cxxflags_remove -Wp,-D_GLIBCXX_ASSERTIONS
or some similar construct, and -Wp,-D_GLIBCXX_ASSERTIONS would be
removed from the build flags.
>>> With these new macros, the examples from above could be re-written as:
>>>
>>> compiler-rt: %global _pkg_extra_cflags -D_DEFAULT_SOURCE
>>> julia: %global _flag_glibcxx_assertions %{nil}
>> Do you have some background why -D_DEFAULT_SOURCE is needed? Why
>> doesn't upstream detect this?
>>
>
> It was added to workaround this bug:
https://sourceware.org/bugzilla/show_bug.cgi?id=25271
This has long since been fixed, so the workaround is no longer needed.
-D_GLIBCXX_ASSERTIONS has zero false positives. Removing it does not
make the code that triggers the asserts well-defined. Only the explicit
assertion failure is gone. So it is probably another example of a flag
where removal does not result in the hoped-for change in toolchain
behavior.
Thanks,
Florian