Re: [Rpm-maint] [rpm-software-management/rpm] Adopt compiler flags related enhancements from Fedora (#692)
Merged #692 into master. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/692#event-2339175588___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Adopt compiler flags related enhancements from Fedora (#692)
Any estimation when this will be merged? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/692#issuecomment-491786233___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Adopt compiler flags related enhancements from Fedora (#692)
pmatilai commented on this pull request.
> +
+# C++ compiler flags. This is traditionally called CXXFLAGS in makefiles.
+%build_cxxflags %{optflags}
+
+# Fortran compiler flags. Makefiles use both FFLAGS and FCFLAGS as
+# the corresponding variable names.
+%build_fflags %{optflags} %{?_fmoddir:-I%{_fmoddir}}
+
+# Link editor flags. This is usually called LDFLAGS in makefiles.
+#%build_ldflags -Wl,-z,relro %{?_lto_cflags}
+
+# Expands to shell code to seot the compiler/linker environment
+# variables CFLAGS, CXXFLAGS, FFLAGS, FCFLAGS, LDFLAGS if they have
+# not been set already.
+%set_build_flags \
+ CFLAGS="${CFLAGS:-%{build_cflags}}" ; export CFLAGS ; \
Whatever. There's just no real benefit to making it potentially more
incompatible.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/692#discussion_r281161254___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Adopt compiler flags related enhancements from Fedora (#692)
Conan-Kudo approved this pull request.
Looks good, just one issue...
> +# compiler flags.
+
+# C compiler flags. This is traditionally called CFLAGS in makefiles.
+# Historically also available as %%{optflags}, and %%build sets the
+# environment variable RPM_OPT_FLAGS to this value.
+%build_cflags %{optflags}
+
+# C++ compiler flags. This is traditionally called CXXFLAGS in makefiles.
+%build_cxxflags %{optflags}
+
+# Fortran compiler flags. Makefiles use both FFLAGS and FCFLAGS as
+# the corresponding variable names.
+%build_fflags %{optflags} %{?_fmoddir:-I%{_fmoddir}}
+
+# Link editor flags. This is usually called LDFLAGS in makefiles.
+#%build_ldflags -Wl,-z,relro %{?_lto_cflags}
Why are we commenting this out? The LTO settings won't take effect as is...
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/692#pullrequestreview-233946051___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Adopt compiler flags related enhancements from Fedora (#692)
Conan-Kudo commented on this pull request.
> +# compiler flags.
+
+# C compiler flags. This is traditionally called CFLAGS in makefiles.
+# Historically also available as %%{optflags}, and %%build sets the
+# environment variable RPM_OPT_FLAGS to this value.
+%build_cflags %{optflags}
+
+# C++ compiler flags. This is traditionally called CXXFLAGS in makefiles.
+%build_cxxflags %{optflags}
+
+# Fortran compiler flags. Makefiles use both FFLAGS and FCFLAGS as
+# the corresponding variable names.
+%build_fflags %{optflags} %{?_fmoddir:-I%{_fmoddir}}
+
+# Link editor flags. This is usually called LDFLAGS in makefiles.
+#%build_ldflags -Wl,-z,relro %{?_lto_cflags}
OK
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/692#discussion_r281159767___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Adopt compiler flags related enhancements from Fedora (#692)
ignatenkobrain commented on this pull request.
> +
+# C++ compiler flags. This is traditionally called CXXFLAGS in makefiles.
+%build_cxxflags %{optflags}
+
+# Fortran compiler flags. Makefiles use both FFLAGS and FCFLAGS as
+# the corresponding variable names.
+%build_fflags %{optflags} %{?_fmoddir:-I%{_fmoddir}}
+
+# Link editor flags. This is usually called LDFLAGS in makefiles.
+#%build_ldflags -Wl,-z,relro %{?_lto_cflags}
+
+# Expands to shell code to seot the compiler/linker environment
+# variables CFLAGS, CXXFLAGS, FFLAGS, FCFLAGS, LDFLAGS if they have
+# not been set already.
+%set_build_flags \
+ CFLAGS="${CFLAGS:-%{build_cflags}}" ; export CFLAGS ; \
https://pubs.opengroup.org/onlinepubs/009695399/utilities/export.html
POSIX says:
```
IEEE Std 1003.1-2001/Cor 1-2002, item XCU/TC1/D6/6 is applied, adding the
following text to the end of the first paragraph of the DESCRIPTION: ``If the
name of a variable is followed by = word, then the value of that variable shall
be set to word.''. The reason for this change is that the SYNOPSIS for export
includes:
export name[=word]...
but the meaning of the optional ``= word'' is never explained in the text.
```
So I think only non-POSIX conformant shells use `foo=bar; export foo`
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/692#discussion_r281158530___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Adopt compiler flags related enhancements from Fedora (#692)
pmatilai commented on this pull request.
> +
+# C++ compiler flags. This is traditionally called CXXFLAGS in makefiles.
+%build_cxxflags %{optflags}
+
+# Fortran compiler flags. Makefiles use both FFLAGS and FCFLAGS as
+# the corresponding variable names.
+%build_fflags %{optflags} %{?_fmoddir:-I%{_fmoddir}}
+
+# Link editor flags. This is usually called LDFLAGS in makefiles.
+#%build_ldflags -Wl,-z,relro %{?_lto_cflags}
+
+# Expands to shell code to seot the compiler/linker environment
+# variables CFLAGS, CXXFLAGS, FFLAGS, FCFLAGS, LDFLAGS if they have
+# not been set already.
+%set_build_flags \
+ CFLAGS="${CFLAGS:-%{build_cflags}}" ; export CFLAGS ; \
Well, but rpm only ever uses /bin/sh. We can't assume bash.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/692#discussion_r281157957___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Adopt compiler flags related enhancements from Fedora (#692)
ignatenkobrain commented on this pull request.
> +
+# C++ compiler flags. This is traditionally called CXXFLAGS in makefiles.
+%build_cxxflags %{optflags}
+
+# Fortran compiler flags. Makefiles use both FFLAGS and FCFLAGS as
+# the corresponding variable names.
+%build_fflags %{optflags} %{?_fmoddir:-I%{_fmoddir}}
+
+# Link editor flags. This is usually called LDFLAGS in makefiles.
+#%build_ldflags -Wl,-z,relro %{?_lto_cflags}
+
+# Expands to shell code to seot the compiler/linker environment
+# variables CFLAGS, CXXFLAGS, FFLAGS, FCFLAGS, LDFLAGS if they have
+# not been set already.
+%set_build_flags \
+ CFLAGS="${CFLAGS:-%{build_cflags}}" ; export CFLAGS ; \
since we are moving this to RPM, I think it is better to go directly with
`export CFLAGS=…`. It is very very old version of bash which did not support
this. I think even RHEL6 has good bash enough.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/692#pullrequestreview-233944099___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Adopt compiler flags related enhancements from Fedora (#692)
This is bascially a more elaborate version of #660 : - Add language specific %build_fooflags macros to allow separate distro-wide defaults for c, c++ and fortran - Add %build_ldflags for specifying distro-wide ldflags (how on earth we didn't have this?) - Split FOOFLAGS= setting out of %configure to %set_build_flags macro to make them available outside autotools You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/692 -- Commit Summary -- * Adopt language-specific %build_fooflags macros from Fedora * Split compiler flag out of %configure to separate %set_build_flags macro -- File Changes -- M macros.in (32) -- Patch Links -- https://github.com/rpm-software-management/rpm/pull/692.patch https://github.com/rpm-software-management/rpm/pull/692.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/692 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
