https://fedoraproject.org/wiki/Changes/RPMMacrosForBuildFlags
This document represents a proposed Change. As part of the Changes process, proposals are publicly announced in order to receive community feedback. This proposal will only be implemented if approved by the Fedora Engineering Steering Committee.
== Summary == Create a corresponding macro for each compiler flag in the redhat-rpm-config macro file and create "extra flag" macros to make it easier for packages to add and remove compiler flags.
== Owner == * Name: [[User:tstellar| Tom Stellard]] * Email: tstellar@redhat.com
== Detailed Description == The macros file in the redhat-rpm-config package contains a list of default compiler flags for packages to use when compiling C, C++, and Fortran packages. There is currently no standard way to remove or add to the set of default flags. Most packages use a combination of echo and sed to remove unwanted flags or add new ones. Some examples: compiler-rt: [https://src.fedoraproject.org/rpms/compiler-rt/blob/rawhide/f/compiler-rt.sp... global optflags %(echo %{optflags} -D_DEFAULT_SOURCE)] julia: [https://src.fedoraproject.org/rpms/julia/blob/rawhide/f/julia.spec#_267 %global optflags %(echo %{optflags} | sed 's/-Wp,-D_GLIBCXX_ASSERTIONS //')]
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
These will be added to %{build_cflags}, %{build_cxxflags}, %{build_fflags}, and %{build_ldflags} respectively to allow packges to add their own flags to the default list: e.g.
%build_cflags %{optflags} %{_pkg_extra_cflags}
The proposed new macros to represent existing flags are:
%_flag_fstack_protector_strong -fstack-protector-strong %_flag_z_now -Wl,-z,now %_flag_z_defs -Wl,-z,defs %_flag_flto_auto -flto=auto %_flag_ffat_lto_objects -ffat-lto-objects %_flag_o -O2 %_flag_f_exceptions -fexceptions %_flag_g -g %_flag_grecord_gcc_switches -grecord-gcc-switches %_flag_pipe -pipe %_flag_wall -Wall %_flag_werror_format_security -Werror=format-security %_flag_fortify_source -Wp,-D_FORTIFY_SOURCE=2 %_flag_glibcxx_assertions -Wp,-D_GLIBCXX_ASSERTIONS %_flag_asynchronous_unwind_tables -fasynchronous-unwind-tables %_flag_fstack_clash_protection -fstack-clash-protection %_flag_fcf_protection -fcf-protection %_flag_mbranch_protection_standard -mbranch-protection=standard
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}
For more details see the [https://src.fedoraproject.org/fork/tstellar/rpms/redhat-rpm-config/c/0ee9a8c... Prototype Implementation].
In addition to adding these new macros, the packaging guidelines will be updated to require that all new flags added to redhat-rpm-config have their own RPM macro.
== Benefit to Fedora == * It will provide a standard way to disable existing compiler flags or enable new ones that is more simple and robust than the existing echo + sed solution. * It will make it easier to determine which packages disable or add compiler flags by doing a simple grep of the spec files. * It will make it easier for toolchain developers to experiment with adding new flags to the distribution as this can be done with a simple macro definition instead of patching redhat-rpm-config.
== Scope == * Proposal owners: ** Proposal owners will update the redhat-rpm-config package and add the new macros. ** Proposal owners will test the changes to ensure that the correct flags are still being used.
* Other developers: ** Other developers may, but are not required to, update their packages to use the new macros.
* Release engineering: [https://pagure.io/releng/issues/10819 #10819]
* Policies and guidelines: ** The Fedora packaging policy will be updated to require that new flags added to redhat-rpm-config come with their own RPM macro.
* Trademark approval: N/A (not needed for this Change) * Alignment with Objectives:
== Upgrade/compatibility impact == None.
== How To Test == * This can be tested by inspecting the value of the %{build_cflags}, %{build_cxxflags}, %{build_fflags}, and %{build_ldflags} and ensuring they are the same before and after the change. * This can be tested by modifying some of the new macros in a spec file and ensuring that the changes appear in the appropriate macro mentioned above.
== User Experience == This is a change for developers and will have no impact to the user experience.
== Dependencies == None.
== Contingency Plan == * Contingency mechanism: (What to do? Who will do it?) Change owner will revert the update to redhat-rpm-config. * Contingency deadline: Mass Rebuild * Blocks release? N/A (not a System Wide Change), No
== Documentation == None.
== Release Notes == None.
Hi Tom,
Since you are looking into this and I like this proposal, have you considered to look alto into `%extension_*flags`:
https://src.fedoraproject.org/rpms/redhat-rpm-config/blob/rawhide/f/macros#_...
This is longstanding issue:
https://bugzilla.redhat.com/1284684
Where we have several proposals for fix, but non of them is really appealing to me:
https://src.fedoraproject.org/rpms/ruby/pull-request/110
https://src.fedoraproject.org/rpms/ruby/pull-request/117
BTW isn't the `_flag_` prefix too generic? And also, the initial underscore implies that this is internal macro which should ideally not be used. So should it be rather removed or not?
Vít
Dne 02. 06. 22 v 21:27 Ben Cotton napsal(a):
https://fedoraproject.org/wiki/Changes/RPMMacrosForBuildFlags
This document represents a proposed Change. As part of the Changes process, proposals are publicly announced in order to receive community feedback. This proposal will only be implemented if approved by the Fedora Engineering Steering Committee.
== Summary == Create a corresponding macro for each compiler flag in the redhat-rpm-config macro file and create "extra flag" macros to make it easier for packages to add and remove compiler flags.
== Owner ==
- Name: [[User:tstellar| Tom Stellard]]
- Email: tstellar@redhat.com
== Detailed Description == The macros file in the redhat-rpm-config package contains a list of default compiler flags for packages to use when compiling C, C++, and Fortran packages. There is currently no standard way to remove or add to the set of default flags. Most packages use a combination of echo and sed to remove unwanted flags or add new ones. Some examples: compiler-rt: [https://src.fedoraproject.org/rpms/compiler-rt/blob/rawhide/f/compiler-rt.sp... global optflags %(echo %{optflags} -D_DEFAULT_SOURCE)] julia: [https://src.fedoraproject.org/rpms/julia/blob/rawhide/f/julia.spec#_267 %global optflags %(echo %{optflags} | sed 's/-Wp,-D_GLIBCXX_ASSERTIONS //')]
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_ldflagsThese will be added to %{build_cflags}, %{build_cxxflags}, %{build_fflags}, and %{build_ldflags} respectively to allow packges to add their own flags to the default list: e.g.
%build_cflags %{optflags} %{_pkg_extra_cflags}The proposed new macros to represent existing flags are:
%_flag_fstack_protector_strong -fstack-protector-strong %_flag_z_now -Wl,-z,now %_flag_z_defs -Wl,-z,defs %_flag_flto_auto -flto=auto %_flag_ffat_lto_objects -ffat-lto-objects %_flag_o -O2 %_flag_f_exceptions -fexceptions %_flag_g -g %_flag_grecord_gcc_switches -grecord-gcc-switches %_flag_pipe -pipe %_flag_wall -Wall %_flag_werror_format_security -Werror=format-security %_flag_fortify_source -Wp,-D_FORTIFY_SOURCE=2 %_flag_glibcxx_assertions -Wp,-D_GLIBCXX_ASSERTIONS %_flag_asynchronous_unwind_tables -fasynchronous-unwind-tables %_flag_fstack_clash_protection -fstack-clash-protection %_flag_fcf_protection -fcf-protection %_flag_mbranch_protection_standard -mbranch-protection=standardWith 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}For more details see the [https://src.fedoraproject.org/fork/tstellar/rpms/redhat-rpm-config/c/0ee9a8c... Prototype Implementation].
In addition to adding these new macros, the packaging guidelines will be updated to require that all new flags added to redhat-rpm-config have their own RPM macro.
== Benefit to Fedora ==
- It will provide a standard way to disable existing compiler flags or
enable new ones that is more simple and robust than the existing echo
- sed solution.
- It will make it easier to determine which packages disable or add
compiler flags by doing a simple grep of the spec files.
- It will make it easier for toolchain developers to experiment with
adding new flags to the distribution as this can be done with a simple macro definition instead of patching redhat-rpm-config.
== Scope ==
- Proposal owners:
** Proposal owners will update the redhat-rpm-config package and add the new macros. ** Proposal owners will test the changes to ensure that the correct flags are still being used.
- Other developers:
** Other developers may, but are not required to, update their packages to use the new macros.
Release engineering: [https://pagure.io/releng/issues/10819 #10819]
Policies and guidelines:
** The Fedora packaging policy will be updated to require that new flags added to redhat-rpm-config come with their own RPM macro.
- Trademark approval: N/A (not needed for this Change)
- Alignment with Objectives:
== Upgrade/compatibility impact == None.
== How To Test ==
- This can be tested by inspecting the value of the %{build_cflags},
%{build_cxxflags}, %{build_fflags}, and %{build_ldflags} and ensuring they are the same before and after the change.
- This can be tested by modifying some of the new macros in a spec
file and ensuring that the changes appear in the appropriate macro mentioned above.
== User Experience == This is a change for developers and will have no impact to the user experience.
== Dependencies == None.
== Contingency Plan ==
- Contingency mechanism: (What to do? Who will do it?) Change owner
will revert the update to redhat-rpm-config.
- Contingency deadline: Mass Rebuild
- Blocks release? N/A (not a System Wide Change), No
== Documentation == None.
== Release Notes == None.
On Fri, Jun 3, 2022 at 11:25 AM Vít Ondruch vondruch@redhat.com wrote:
BTW isn't the `_flag_` prefix too generic? And also, the initial underscore implies that this is internal macro which should ideally not be used. So should it be rather removed or not?
I agree that the "_flag_" prefix might be a little too generic, but what would be a better alternative? Maybe something like _optflag_, to match what they are "collected into" (i.e. %optflags)?
Also, macro names with single leading underscores are *fine* (see also %_bindir, %_libdir, %_datadir, etc.). Those with *double* leading underscores are the ones that should be considered "internal" implementation details.
Fabio
On 6/3/22 03:43, Fabio Valentini wrote:
On Fri, Jun 3, 2022 at 11:25 AM Vít Ondruch vondruch@redhat.com wrote:
BTW isn't the `_flag_` prefix too generic? And also, the initial underscore implies that this is internal macro which should ideally not be used. So should it be rather removed or not?
I agree that the "_flag_" prefix might be a little too generic, but what would be a better alternative? Maybe something like _optflag_, to match what they are "collected into" (i.e. %optflags)?
What about prefixing them with _build_flag_ The redhat-rpm-config docs[1], recommend using the %build_*flags macros instead of %optflags, so maybe we should try to match that.
[1] https://src.fedoraproject.org/rpms/redhat-rpm-config//blob/rawhide/f/buildfl...
-Tom
Also, macro names with single leading underscores are *fine* (see also %_bindir, %_libdir, %_datadir, etc.). Those with *double* leading underscores are the ones that should be considered "internal" implementation details.
Fabio _______________________________________________ devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
On 6/3/22 13:43, Fabio Valentini wrote:
On Fri, Jun 3, 2022 at 11:25 AM Vít Ondruch vondruch@redhat.com wrote:
BTW isn't the `_flag_` prefix too generic? And also, the initial underscore implies that this is internal macro which should ideally not be used. So should it be rather removed or not?
I agree that the "_flag_" prefix might be a little too generic, but what would be a better alternative? Maybe something like _optflag_, to match what they are "collected into" (i.e. %optflags)?
Also, macro names with single leading underscores are *fine* (see also %_bindir, %_libdir, %_datadir, etc.). Those with *double* leading underscores are the ones that should be considered "internal" implementation details.
Once upon a time in past I can still remember, the following rule of thumb for macro underscores was set [1]:
Use %__foo to set, %foo to get.
To me it looks like the entire set of suggested flags is basically write-only values, and thus should have two leading underscores. So, %__build_flag_whatever. Usage should always happen through the non-underscored %build_cflags and friends, which can do their own internal logic around this stuff, use Y only if X is enabled.
Other misc comments:
%_flag_flto_auto -flto=auto
Shouldn't this be %_flag_flto instead (or rather, %__build_flag_flto), just like %_flag_o does not carry the optimization level in the name?
%_flag_werror_format_security -Werror=format-security
...and ditto for this, %__build_flag_werror whose default value is -Werror=format-security ... except that unlike -flto, -Werror can appear multiple times. Dunno.
What I guess I'm after, having an actual rule for the parameter -> macro naming, one that could preferably be automated, would be beneficial. The -Wl and -Wp related macro naming would need further consideration wrt that.
%_flag_pipe seems like the odd man out there because it doesn't actually relate to code at all, but I guess consistency is the goal there.
[1] https://pagure.io/packaging-committee/issue/907
- Panu -
On 6/6/22 00:58, Panu Matilainen wrote:
On 6/3/22 13:43, Fabio Valentini wrote:
On Fri, Jun 3, 2022 at 11:25 AM Vít Ondruch vondruch@redhat.com wrote:
BTW isn't the `_flag_` prefix too generic? And also, the initial underscore implies that this is internal macro which should ideally not be used. So should it be rather removed or not?
I agree that the "_flag_" prefix might be a little too generic, but what would be a better alternative? Maybe something like _optflag_, to match what they are "collected into" (i.e. %optflags)?
Also, macro names with single leading underscores are *fine* (see also %_bindir, %_libdir, %_datadir, etc.). Those with *double* leading underscores are the ones that should be considered "internal" implementation details.
Once upon a time in past I can still remember, the following rule of thumb for macro underscores was set [1]:
Use %__foo to set, %foo to get.
To me it looks like the entire set of suggested flags is basically write-only values, and thus should have two leading underscores. So, %__build_flag_whatever. Usage should always happen through the non-underscored %build_cflags and friends, which can do their own internal logic around this stuff, use Y only if X is enabled.
I don't have a preference on one vs two underscore. I chose one, because that's what most of the existing compiler flag macros use.
Other misc comments:
%_flag_flto_auto -flto=auto
Shouldn't this be %_flag_flto instead (or rather, %__build_flag_flto), just like %_flag_o does not carry the optimization level in the name?
OK, I think that makes sense.
%_flag_werror_format_security -Werror=format-security
...and ditto for this, %__build_flag_werror whose default value is -Werror=format-security ... except that unlike -flto, -Werror can appear multiple times. Dunno.
What I guess I'm after, having an actual rule for the parameter -> macro naming, one that could preferably be automated, would be beneficial. The -Wl and -Wp related macro naming would need further consideration wrt that.
I don't think there is a way to have the consistent naming that you are looking for. Even though -Werror= usage the same format as -flto= and others, each -Werror= option is really its own separate option.
-Tom
%_flag_pipe seems like the odd man out there because it doesn't actually relate to code at all, but I guess consistency is the goal there.
[1] https://pagure.io/packaging-committee/issue/907
- Panu - _______________________________________________ devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
On 6/3/22 02:24, Vít Ondruch wrote:
Hi Tom,
Since you are looking into this and I like this proposal, have you considered to look alto into `%extension_*flags`:
https://src.fedoraproject.org/rpms/redhat-rpm-config/blob/rawhide/f/macros#_...
I have not considered this. Do you think there is some way this proposal could be extended to help solve this problem as well?
-Tom
This is longstanding issue:
https://bugzilla.redhat.com/1284684
Where we have several proposals for fix, but non of them is really appealing to me:
https://src.fedoraproject.org/rpms/ruby/pull-request/110
https://src.fedoraproject.org/rpms/ruby/pull-request/117
BTW isn't the `_flag_` prefix too generic? And also, the initial underscore implies that this is internal macro which should ideally not be used. So should it be rather removed or not?
Vít
Dne 02. 06. 22 v 21:27 Ben Cotton napsal(a):
https://fedoraproject.org/wiki/Changes/RPMMacrosForBuildFlags
This document represents a proposed Change. As part of the Changes process, proposals are publicly announced in order to receive community feedback. This proposal will only be implemented if approved by the Fedora Engineering Steering Committee.
== Summary == Create a corresponding macro for each compiler flag in the redhat-rpm-config macro file and create "extra flag" macros to make it easier for packages to add and remove compiler flags.
== Owner ==
- Name: [[User:tstellar| Tom Stellard]]
- Email: tstellar@redhat.com
== Detailed Description == The macros file in the redhat-rpm-config package contains a list of default compiler flags for packages to use when compiling C, C++, and Fortran packages. There is currently no standard way to remove or add to the set of default flags. Most packages use a combination of echo and sed to remove unwanted flags or add new ones. Some examples: compiler-rt: [https://src.fedoraproject.org/rpms/compiler-rt/blob/rawhide/f/compiler-rt.sp... global optflags %(echo %{optflags} -D_DEFAULT_SOURCE)] julia: [https://src.fedoraproject.org/rpms/julia/blob/rawhide/f/julia.spec#_267 %global optflags %(echo %{optflags} | sed 's/-Wp,-D_GLIBCXX_ASSERTIONS //')]
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
These will be added to %{build_cflags}, %{build_cxxflags}, %{build_fflags}, and %{build_ldflags} respectively to allow packges to add their own flags to the default list: e.g.
%build_cflags %{optflags} %{_pkg_extra_cflags}
The proposed new macros to represent existing flags are:
%_flag_fstack_protector_strong -fstack-protector-strong %_flag_z_now -Wl,-z,now %_flag_z_defs -Wl,-z,defs %_flag_flto_auto -flto=auto %_flag_ffat_lto_objects -ffat-lto-objects %_flag_o -O2 %_flag_f_exceptions -fexceptions %_flag_g -g %_flag_grecord_gcc_switches -grecord-gcc-switches %_flag_pipe -pipe %_flag_wall -Wall %_flag_werror_format_security -Werror=format-security %_flag_fortify_source -Wp,-D_FORTIFY_SOURCE=2 %_flag_glibcxx_assertions -Wp,-D_GLIBCXX_ASSERTIONS %_flag_asynchronous_unwind_tables -fasynchronous-unwind-tables %_flag_fstack_clash_protection -fstack-clash-protection %_flag_fcf_protection -fcf-protection %_flag_mbranch_protection_standard -mbranch-protection=standard
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}
For more details see the [https://src.fedoraproject.org/fork/tstellar/rpms/redhat-rpm-config/c/0ee9a8c... Prototype Implementation].
In addition to adding these new macros, the packaging guidelines will be updated to require that all new flags added to redhat-rpm-config have their own RPM macro.
== Benefit to Fedora ==
- It will provide a standard way to disable existing compiler flags or
enable new ones that is more simple and robust than the existing echo
- sed solution.
- It will make it easier to determine which packages disable or add
compiler flags by doing a simple grep of the spec files.
- It will make it easier for toolchain developers to experiment with
adding new flags to the distribution as this can be done with a simple macro definition instead of patching redhat-rpm-config.
== Scope ==
- Proposal owners:
** Proposal owners will update the redhat-rpm-config package and add the new macros. ** Proposal owners will test the changes to ensure that the correct flags are still being used.
- Other developers:
** Other developers may, but are not required to, update their packages to use the new macros.
Release engineering: [https://pagure.io/releng/issues/10819 #10819]
Policies and guidelines:
** The Fedora packaging policy will be updated to require that new flags added to redhat-rpm-config come with their own RPM macro.
- Trademark approval: N/A (not needed for this Change)
- Alignment with Objectives:
== Upgrade/compatibility impact == None.
== How To Test ==
- This can be tested by inspecting the value of the %{build_cflags},
%{build_cxxflags}, %{build_fflags}, and %{build_ldflags} and ensuring they are the same before and after the change.
- This can be tested by modifying some of the new macros in a spec
file and ensuring that the changes appear in the appropriate macro mentioned above.
== User Experience == This is a change for developers and will have no impact to the user experience.
== Dependencies == None.
== Contingency Plan ==
- Contingency mechanism: (What to do? Who will do it?) Change owner
will revert the update to redhat-rpm-config.
- Contingency deadline: Mass Rebuild
- Blocks release? N/A (not a System Wide Change), No
== Documentation == None.
== Release Notes == None.
devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
Dne 03. 06. 22 v 16:32 Tom Stellard napsal(a):
On 6/3/22 02:24, Vít Ondruch wrote:
Hi Tom,
Since you are looking into this and I like this proposal, have you considered to look alto into `%extension_*flags`:
https://src.fedoraproject.org/rpms/redhat-rpm-config/blob/rawhide/f/macros#_...
I have not considered this. Do you think there is some way this proposal could be extended to help solve this problem as well?
I think the current struggle is that we have `%extension_cflags` and `%build_cflags`, while we would actually need the set of ``%build_cflags - %extension_cflags`. The problem is that the `%build_cflags` order is not necessarily followed in the resulting code, so if we need to modify the resulting code, we have no way to achieve this.
Vít
-Tom
This is longstanding issue:
https://bugzilla.redhat.com/1284684
Where we have several proposals for fix, but non of them is really appealing to me:
https://src.fedoraproject.org/rpms/ruby/pull-request/110
https://src.fedoraproject.org/rpms/ruby/pull-request/117
BTW isn't the `_flag_` prefix too generic? And also, the initial underscore implies that this is internal macro which should ideally not be used. So should it be rather removed or not?
Vít
Dne 02. 06. 22 v 21:27 Ben Cotton napsal(a):
https://fedoraproject.org/wiki/Changes/RPMMacrosForBuildFlags
This document represents a proposed Change. As part of the Changes process, proposals are publicly announced in order to receive community feedback. This proposal will only be implemented if approved by the Fedora Engineering Steering Committee.
== Summary == Create a corresponding macro for each compiler flag in the redhat-rpm-config macro file and create "extra flag" macros to make it easier for packages to add and remove compiler flags.
== Owner ==
- Name: [[User:tstellar| Tom Stellard]]
- Email: tstellar@redhat.com
== Detailed Description == The macros file in the redhat-rpm-config package contains a list of default compiler flags for packages to use when compiling C, C++, and Fortran packages. There is currently no standard way to remove or add to the set of default flags. Most packages use a combination of echo and sed to remove unwanted flags or add new ones. Some examples: compiler-rt: [https://src.fedoraproject.org/rpms/compiler-rt/blob/rawhide/f/compiler-rt.sp...
global optflags %(echo %{optflags} -D_DEFAULT_SOURCE)] julia: [https://src.fedoraproject.org/rpms/julia/blob/rawhide/f/julia.spec#_267
%global optflags %(echo %{optflags} | sed 's/-Wp,-D_GLIBCXX_ASSERTIONS //')]
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
These will be added to %{build_cflags}, %{build_cxxflags}, %{build_fflags}, and %{build_ldflags} respectively to allow packges to add their own flags to the default list: e.g.
%build_cflags %{optflags} %{_pkg_extra_cflags}
The proposed new macros to represent existing flags are:
%_flag_fstack_protector_strong -fstack-protector-strong %_flag_z_now -Wl,-z,now %_flag_z_defs -Wl,-z,defs %_flag_flto_auto -flto=auto %_flag_ffat_lto_objects -ffat-lto-objects %_flag_o -O2 %_flag_f_exceptions -fexceptions %_flag_g -g %_flag_grecord_gcc_switches -grecord-gcc-switches %_flag_pipe -pipe %_flag_wall -Wall %_flag_werror_format_security -Werror=format-security %_flag_fortify_source -Wp,-D_FORTIFY_SOURCE=2 %_flag_glibcxx_assertions -Wp,-D_GLIBCXX_ASSERTIONS %_flag_asynchronous_unwind_tables -fasynchronous-unwind-tables %_flag_fstack_clash_protection -fstack-clash-protection %_flag_fcf_protection -fcf-protection %_flag_mbranch_protection_standard -mbranch-protection=standard
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}
For more details see the [https://src.fedoraproject.org/fork/tstellar/rpms/redhat-rpm-config/c/0ee9a8c...
Prototype Implementation].
In addition to adding these new macros, the packaging guidelines will be updated to require that all new flags added to redhat-rpm-config have their own RPM macro.
== Benefit to Fedora ==
- It will provide a standard way to disable existing compiler flags or
enable new ones that is more simple and robust than the existing echo
- sed solution.
- It will make it easier to determine which packages disable or add
compiler flags by doing a simple grep of the spec files.
- It will make it easier for toolchain developers to experiment with
adding new flags to the distribution as this can be done with a simple macro definition instead of patching redhat-rpm-config.
== Scope ==
- Proposal owners:
** Proposal owners will update the redhat-rpm-config package and add the new macros. ** Proposal owners will test the changes to ensure that the correct flags are still being used.
- Other developers:
** Other developers may, but are not required to, update their packages to use the new macros.
Release engineering: [https://pagure.io/releng/issues/10819 #10819]
Policies and guidelines:
** The Fedora packaging policy will be updated to require that new flags added to redhat-rpm-config come with their own RPM macro.
- Trademark approval: N/A (not needed for this Change)
- Alignment with Objectives:
== Upgrade/compatibility impact == None.
== How To Test ==
- This can be tested by inspecting the value of the %{build_cflags},
%{build_cxxflags}, %{build_fflags}, and %{build_ldflags} and ensuring they are the same before and after the change.
- This can be tested by modifying some of the new macros in a spec
file and ensuring that the changes appear in the appropriate macro mentioned above.
== User Experience == This is a change for developers and will have no impact to the user experience.
== Dependencies == None.
== Contingency Plan ==
- Contingency mechanism: (What to do? Who will do it?) Change owner
will revert the update to redhat-rpm-config.
- Contingency deadline: Mass Rebuild
- Blocks release? N/A (not a System Wide Change), No
== Documentation == None.
== Release Notes == None.
devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
* Vít Ondruch:
Dne 03. 06. 22 v 16:32 Tom Stellard napsal(a):
On 6/3/22 02:24, Vít Ondruch wrote:
Hi Tom,
Since you are looking into this and I like this proposal, have you considered to look alto into `%extension_*flags`:
https://src.fedoraproject.org/rpms/redhat-rpm-config/blob/rawhide/f/macros#_...
I have not considered this. Do you think there is some way this proposal could be extended to help solve this problem as well?
I think the current struggle is that we have `%extension_cflags` and `%build_cflags`, while we would actually need the set of ``%build_cflags - %extension_cflags`. The problem is that the `%build_cflags` order is not necessarily followed in the resulting code, so if we need to modify the resulting code, we have no way to achieve this.
I don't see how the list difference would solve your fundamental issue. Could you elaborate? Or maybe you already did, and you missed it, in which case I would appreciate a pointer.
Thanks, Florian
Dne 07. 06. 22 v 11:10 Florian Weimer napsal(a):
- Vít Ondruch:
Dne 03. 06. 22 v 16:32 Tom Stellard napsal(a):
On 6/3/22 02:24, Vít Ondruch wrote:
Hi Tom,
Since you are looking into this and I like this proposal, have you considered to look alto into `%extension_*flags`:
https://src.fedoraproject.org/rpms/redhat-rpm-config/blob/rawhide/f/macros#_...
I have not considered this. Do you think there is some way this proposal could be extended to help solve this problem as well?
I think the current struggle is that we have `%extension_cflags` and `%build_cflags`, while we would actually need the set of ``%build_cflags - %extension_cflags`. The problem is that the `%build_cflags` order is not necessarily followed in the resulting code, so if we need to modify the resulting code, we have no way to achieve this.
I don't see how the list difference would solve your fundamental issue. Could you elaborate? Or maybe you already did, and you missed it, in which case I would appreciate a pointer.
https://src.fedoraproject.org/rpms/ruby/pull-request/110
This PR ^^ has the lengthy discussion. But let me reiterate. So there is `%build_cflags` and the original assumption was that if you give these flags to the build system, they are recorded also in the output, so you could do something such as `s/%build_cflags/%extension_cflags/`. But that does not work, because the build system is free to change the `%build_cflags`. So in the output, there is something we can call e.g. `%output_build_cflags`. From these flags, we would like to remove the flags which are not usable for the extensions, i.e. the set of `%build_cflags - %extension_cflags`.
IOW, these are the `%build_cflags`:
~~~
$ rpm --eval '%build_cflags' -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection
~~~
while these are the output flags:
~~~
-O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fPIC
~~~
If we had the `%build_cflags - %extension_cflags` set, we could at least iterate over the set and remove the flags from the output.
Vít
Thanks, Florian
Ben Cotton kirjoitti 2.6.2022 klo 22.27:
- Policies and guidelines:
** The Fedora packaging policy will be updated to require that new flags added to redhat-rpm-config come with their own RPM macro.
By the proposal owners, I presume? The guidelines should also be updated to present the new macros as the preferred way of modifying build flags.
* 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?
The proposed new macros to represent existing flags are:
%_flag_fstack_protector_strong -fstack-protector-strong %_flag_z_now -Wl,-z,now %_flag_z_defs -Wl,-z,defs %_flag_flto_auto -flto=auto %_flag_ffat_lto_objects -ffat-lto-objects %_flag_o -O2 %_flag_f_exceptions -fexceptions %_flag_g -g %_flag_grecord_gcc_switches -grecord-gcc-switches %_flag_pipe -pipe %_flag_wall -Wall %_flag_werror_format_security -Werror=format-security %_flag_fortify_source -Wp,-D_FORTIFY_SOURCE=2 %_flag_glibcxx_assertions -Wp,-D_GLIBCXX_ASSERTIONS %_flag_asynchronous_unwind_tables -fasynchronous-unwind-tables %_flag_fstack_clash_protection -fstack-clash-protection %_flag_fcf_protection -fcf-protection %_flag_mbranch_protection_standard -mbranch-protection=standard
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 behavior?
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.
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?
Thanks, Florian
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_ldflagsWhy 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.
The proposed new macros to represent existing flags are:
%_flag_fstack_protector_strong -fstack-protector-strong %_flag_z_now -Wl,-z,now %_flag_z_defs -Wl,-z,defs %_flag_flto_auto -flto=auto %_flag_ffat_lto_objects -ffat-lto-objects %_flag_o -O2 %_flag_f_exceptions -fexceptions %_flag_g -g %_flag_grecord_gcc_switches -grecord-gcc-switches %_flag_pipe -pipe %_flag_wall -Wall %_flag_werror_format_security -Werror=format-security %_flag_fortify_source -Wp,-D_FORTIFY_SOURCE=2 %_flag_glibcxx_assertions -Wp,-D_GLIBCXX_ASSERTIONS %_flag_asynchronous_unwind_tables -fasynchronous-unwind-tables %_flag_fstack_clash_protection -fstack-clash-protection %_flag_fcf_protection -fcf-protection %_flag_mbranch_protection_standard -mbranch-protection=standardThis 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 behavior?
The goal is to make it easy to remove individual flags so packages can get the toolchain behavior they want.
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?
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
-Tom
Thanks, Florian _______________________________________________ devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
* 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_ldflagsWhy 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?
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
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_ldflagsWhy 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
* Tom Stellard:
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.
Independently of that, I think we should still provide a facility that is more similar to the sed hack (perhaps with additional error checking), rather than maintain a set of new variables.
Thanks, Florian
On Fri, 24 Jun 2022 at 09:32, Florian Weimer wrote:
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.
Yes, I don't think an example of how to hide undefined behaviour is a good example for the change proposal.
devel@lists.stg.fedoraproject.org
-
Ben Cotton -
Fabio Valentini -
Florian Weimer -
Jonathan Wakely -
Otto Urpelainen -
Panu Matilainen -
Tom Stellard -
Vít Ondruch