about compiler flags

[Orcan Ogetbil]

I have 2 questions. The first one is brought to my attention by a
first time reviewer and he has got an interesting point.

The guidelines at
https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
say:
"Adding to and overriding or filtering parts of these flags is
permitted if there's a good reason to do so; the rationale for doing
so should be reviewed and documented in the specfile especially in the
override and filter cases."

As you probably know that some packages add their own compiler flags
on top of what we specified as %optflags. Until now, I did not do
anything about these flags unless they override our %optflags. Now,
having a second thought, I realized that the above statement can be
interpreted in two ways:

1- The extra flags are added by the packager: This is the way I used
to interpret the guideline, and I always document if I add additional
flags on top of %{optflags}
2- The extra flags are added by the build script of upstream: Here is
the question: Do we need to document such cases in the specfile? I
know that we have hundreds (maybe thousands) of packages which don't
document this case in the specfile. Are such packages violating the
above guideline?

Second question: From the same link above:
"Compilers used to build packages should honor the applicable compiler
flags set in the system rpm configuration."

Does this apply to the stage where the compiler is doing the linking,
i.e: "gcc -shared -lthis -lthat..."? Do we need to honor %optflags in
this stage too? Again, I know many packages that don't pass %optflags
to the compiler during the linking. Do these pakcages violate the
guideline?

Orcan

[Orcan Ogetbil]

I sent this email to fedora-packaging a couple days ago. I thought I
CC'd the devel list but apparently I didn't.

It consists of two issues, related to possible different
interpretations of the guideline about compiler flags. I would like to
have these clarified.

On Wed, Apr 22, 2009 at 12:56 PM, Orcan Ogetbil wrote:
> I have 2 questions. The first one is brought to my attention by a
> first time reviewer and he has got an interesting point.
>
> The guidelines at
> https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
> say:
> "Adding to and overriding or filtering parts of these flags is
> permitted if there's a good reason to do so; the rationale for doing
> so should be reviewed and documented in the specfile especially in the
> override and filter cases."
>
> As you probably know that some packages add their own compiler flags
> on top of what we specified as %optflags. Until now, I did not do
> anything about these flags unless they override our %optflags. Now,
> having a second thought, I realized that the above statement can be
> interpreted in two ways:
>
> 1- The extra flags are added by the packager: This is the way I used
> to interpret the guideline, and I always document if I add additional
> flags on top of %{optflags}
> 2- The extra flags are added by the build script of upstream: Here is
> the question: Do we need to document such cases in the specfile? I
> know that we have hundreds (maybe thousands) of packages which don't
> document this case in the specfile. Are such packages violating the
> above guideline?
>
> Second question: From the same link above:
> "Compilers used to build packages should honor the applicable compiler
> flags set in the system rpm configuration."
>
> Does this apply to the stage where the compiler is doing the linking,
> i.e: "gcc -shared -lthis -lthat..."? Do we need to honor %optflags in
> this stage too? Again, I know many packages that don't pass %optflags
> to the compiler during the linking. Do these pakcages violate the
> guideline?
>
> Orcan

[Pavel Alexeev (aka Pahan-Hubbitus)]

Orcan Ogetbil пишет:
> I have 2 questions. The first one is brought to my attention by a
> first time reviewer and he has got an interesting point.
>
> The guidelines at
> https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
> say:
> "Adding to and overriding or filtering parts of these flags is
> permitted if there's a good reason to do so; the rationale for doing
> so should be reviewed and documented in the specfile especially in the
> override and filter cases."
>
> As you probably know that some packages add their own compiler flags
> on top of what we specified as %optflags. Until now, I did not do
> anything about these flags unless they override our %optflags. Now,
> having a second thought, I realized that the above statement can be
> interpreted in two ways:

As I can understand primarily this required for the allowing Fedora set 
of optimisation for current platform, for which package build. So, in 
case when maintainer add flags which is important fo building, 
performance, or changed as some workaround to bug - it is acceptable, 
but need comment. Otherwise, we MUST rewrite developer flags to fedora 
default %optflags.

Michael Schwendt point me in one of my own review request to this 
guidelines: https://bugzilla.redhat.com/show_bug.cgi?id=454980#c37

[Orcan Ogetbil]

On Fri, Apr 24, 2009 at 4:31 PM, Pavel Alexeev (aka Pahan-Hubbitus) wrote:
> As I can understand primarily this required for the allowing Fedora set of
> optimisation for current platform, for which package build. So, in case when
> maintainer add flags which is important fo building, performance, or changed
> as some workaround to bug - it is acceptable, but need comment. Otherwise,
> we MUST rewrite developer flags to fedora default %optflags.
>
> Michael Schwendt point me in one of my own review request to this
> guidelines: https://bugzilla.redhat.com/show_bug.cgi?id=454980#c37

I just took two random examples from the first page of koji (no
offence to anyone):

1- From the qt-4.5.1-3 build on x86_64:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1319567
g++ -c -m64 -pipe -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2
-fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -Wall
-W  -I../../../mkspecs/linux-g++-64 -I. -I/usr/X11R6/include -I. -o
xrandr.o xrandr.cpp

Here -W needs to be removed?

2- From the lxpanel-0.4.0-1 build on i586:
http://koji.fedoraproject.org/koji/buildinfo?buildID=99531
/bin/sh ../../../libtool --tag=CC   --mode=link gcc  -O2 -g -pipe
-Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i586 -mtune=generic
-fasynchronous-unwind-tables -module -avoid-version -rpath
/usr/lib/lxpanel/plugins -no-undefined -export-symbols-regex "^[^_].*"
-Wl,--as-needed -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,--sort-common -o
netstatus.la -rpath /usr/lib/lxpanel/plugins netstatus-sysdeps.lo
glade-support.lo netstatus-dialog-ui.lo netstatus-icon.lo
netstatus-util.lo netstatus.lo netstatus-enums.lo netstatus-iface.lo
netstatus-dialog.lo -pthread -lgtk-x11-2.0 -lgdk-x11-2.0 -latk-1.0
-lgio-2.0 -lpangoft2-1.0 -lgdk_pixbuf-2.0 -lpangocairo-1.0 -lcairo
-lpango-1.0 -lfreetype -lfontconfig -lgobject-2.0 -lgmodule-2.0
-lgthread-2.0 -lrt -lglib-2.0   -lmenu-cache -lglib-2.0

Here, "-fasynchronous-unwind-tables -module -avoid-version -rpath
/usr/lib/lxpanel/plugins -no-undefined -export-symbols-regex "^[^_].*"
-Wl,--as-needed -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,--sort-common
-rpath /usr/lib/lxpanel/plugins" need to be removed?

If what you are saying is correct, we need to "fix" hundreds of
packages. Are you sure that's the right way?

Orcan

[Patrice Dumas]

On Fri, Apr 24, 2009 at 04:52:50PM -0400, Orcan Ogetbil wrote:
> Here, "-fasynchronous-unwind-tables -module -avoid-version -rpath
> /usr/lib/lxpanel/plugins -no-undefined -export-symbols-regex "^[^_].*"
> -Wl,--as-needed -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,--sort-common
> -rpath /usr/lib/lxpanel/plugins" need to be removed?
>
> If what you are saying is correct, we need to "fix" hundreds of
> packages. Are you sure that's the right way?

Everything but -fasynchronous-unwind-tables are linker flags that 
are not at stake.

--
Pat

[Pavel Alexeev (aka Pahan-Hubbitus)]

Orcan Ogetbil wrote:
> If what you are saying is correct, we need to "fix" hundreds of
> packages. Are you sure that's the right way?

In Our packages VERY much errors and guidelines violations most of comes 
from long history (f.e. lots of packages has not any comments of its 
patches, even have many of them). I think we shouldn't expire witch-hunt 
now, but new packages must follow guidelines as much as possible.

So, if you are not agree with that in particularly you may inspire 
guidelines change proposals for Fedora Package Committee.

[Ralf Corsepius]

Orcan Ogetbil wrote:
> On Fri, Apr 24, 2009 at 4:31 PM, Pavel Alexeev (aka Pahan-Hubbitus) wrote:
> > As I can understand primarily this required for the allowing Fedora set of
> > optimisation for current platform, for which package build. So, in case when
> > maintainer add flags which is important fo building, performance, or changed
> > as some workaround to bug - it is acceptable, but need comment. Otherwise,
> > we MUST rewrite developer flags to fedora default %optflags.
> >
> > Michael Schwendt point me in one of my own review request to this
> > guidelines: https://bugzilla.redhat.com/show_bug.cgi?id=454980#c37
>
> I just took two random examples from the first page of koji (no
> offence to anyone):
>
> 1- From the qt-4.5.1-3 build on x86_64:
> http://koji.fedoraproject.org/koji/taskinfo?taskID=1319567
> g++ -c -m64 -pipe -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2
> -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -Wall
> -W  -I../../../mkspecs/linux-g++-64 -I. -I/usr/X11R6/include -I. -o
> xrandr.o xrandr.cpp
>
> Here -W needs to be removed?

man gcc

-W are harmless
> 2- From the lxpanel-0.4.0-1 build on i586:
> http://koji.fedoraproject.org/koji/buildinfo?buildID=99531
> /bin/sh ../../../libtool --tag=CC   --mode=link gcc  -O2 -g -pipe
> -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
> --param=ssp-buffer-size=4 -m32 -march=i586 -mtune=generic
> -fasynchronous-unwind-tables -module -avoid-version -rpath
> /usr/lib/lxpanel/plugins -no-undefined -export-symbols-regex "^[^_].*"
> -Wl,--as-needed -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,--sort-common -o
> netstatus.la -rpath /usr/lib/lxpanel/plugins netstatus-sysdeps.lo
> glade-support.lo netstatus-dialog-ui.lo netstatus-icon.lo
> netstatus-util.lo netstatus.lo netstatus-enums.lo netstatus-iface.lo
> netstatus-dialog.lo -pthread -lgtk-x11-2.0 -lgdk-x11-2.0 -latk-1.0
> -lgio-2.0 -lpangoft2-1.0 -lgdk_pixbuf-2.0 -lpangocairo-1.0 -lcairo
> -lpango-1.0 -lfreetype -lfontconfig -lgobject-2.0 -lgmodule-2.0
> -lgthread-2.0 -lrt -lglib-2.0   -lmenu-cache -lglib-2.0
>
> Here, "-fasynchronous-unwind-tables -module -avoid-version -rpath
> /usr/lib/lxpanel/plugins -no-undefined -export-symbols-regex "^[^_].*"
> -Wl,--as-needed -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,--sort-common
> -rpath /usr/lib/lxpanel/plugins" need to be removed?

-f* flags are not harmless. -Wl,-O1 is harmless, but likely is an ugly 
hack around to something deeply broken inside of the code.
> If what you are saying is correct, we need to "fix" hundreds of
> packages. Are you sure that's the right way?

Yes.

Ralf

[Orcan Ogetbil]

On Wed, Apr 22, 2009 at 12:56 PM, Orcan Ogetbil  wrote:
> I have 2 questions. The first one is brought to my attention by a
> first time reviewer and he has got an interesting point.
>
> The guidelines at
> https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
> say:
> "Adding to and overriding or filtering parts of these flags is
> permitted if there's a good reason to do so; the rationale for doing
> so should be reviewed and documented in the specfile especially in the
> override and filter cases."
>
> As you probably know that some packages add their own compiler flags
> on top of what we specified as %optflags. Until now, I did not do
> anything about these flags unless they override our %optflags. Now,
> having a second thought, I realized that the above statement can be
> interpreted in two ways:
>
> 1- The extra flags are added by the packager: This is the way I used
> to interpret the guideline, and I always document if I add additional
> flags on top of %{optflags}
> 2- The extra flags are added by the build script of upstream: Here is
> the question: Do we need to document such cases in the specfile? I
> know that we have hundreds (maybe thousands) of packages which don't
> document this case in the specfile. Are such packages violating the
> above guideline?
>
> Second question: From the same link above:
> "Compilers used to build packages should honor the applicable compiler
> flags set in the system rpm configuration."
>
> Does this apply to the stage where the compiler is doing the linking,
> i.e: "gcc -shared -lthis -lthat..."? Do we need to honor %optflags in
> this stage too? Again, I know many packages that don't pass %optflags
> to the compiler during the linking. Do these pakcages violate the
> guideline?
>
> Orcan

I didn't get satisfactory responses to my questions, neither here nor
in fedora-devel list. All the response we got in this thread was about
specific examples concerning the first question, and there was no
explanation in full generality. There was no attempt to answer the
second question at all. There could be two reasons for this:

Either everybody thinks that the answers are ridiculously trivial and
they don't bother to answer them,
or most people don't have a clue.

I hope it is the former one. So please, if you have answers to any of
these questions that are not case-specific, would you please share it
with me?

Thanks,
Orcan

[Tom Callaway]

On 04/22/2009 12:56 PM, Orcan Ogetbil wrote:
> As you probably know that some packages add their own compiler flags
> on top of what we specified as %optflags. Until now, I did not do
> anything about these flags unless they override our %optflags. Now,
> having a second thought, I realized that the above statement can be
> interpreted in two ways:
>
> 1- The extra flags are added by the packager: This is the way I used
> to interpret the guideline, and I always document if I add additional
> flags on top of %{optflags}

If the packager is adding optflags above and beyond what is in
%{optflags} as a manual action (not something inherited by upstream),
they need to have a darned good reason to do so, and that reason should
be documented in a comment next to this action in the spec file.
> 2- The extra flags are added by the build script of upstream: Here is
> the question: Do we need to document such cases in the specfile? I
> know that we have hundreds (maybe thousands) of packages which don't
> document this case in the specfile. Are such packages violating the
> above guideline?

In a perfect world, yes, this should be documented in the spec file, but
practically, you're right, the upstreams often set specific compiler
flags for valid reasons, but don't bother explaining them at all. In
general, when the upstream compiler flags do not conflict with our
%{optflags} and do generally sane things, I think it is okay to permit
them. I know that is vague, but its the best answer I can give. In
general, I trust that upstreams are not being idiots. If it is obvious
that this is not true, the packager may want to reconsider their choice
of additional compiler flags.
> Second question: From the same link above:
> "Compilers used to build packages should honor the applicable compiler
> flags set in the system rpm configuration."
>
> Does this apply to the stage where the compiler is doing the linking,
> i.e: "gcc -shared -lthis -lthat..."? Do we need to honor %optflags in
> this stage too? Again, I know many packages that don't pass %optflags
> to the compiler during the linking. Do these pakcages violate the
> guideline?

Generally, the answer here is no, as most of the time, these flags are
just converted to ld flags, and the only one that really ever matters
seems to be the -m32/-m64 split to ensure that resultant binary is
linked at the proper bitsize. I've only ever seen this break on sparc64,
and it is no longer an issue, since the ld defaults are now sane on that
arch.

~spot