Bug 1393492

Summary: rpm-redhat-config: Fedora 26 C/C++ build flag changes
Product: [Fedora] Fedora Reporter: Florian Weimer <fweimer>
Component: redhat-rpm-configAssignee: Panu Matilainen <pmatilai>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: awilliam, ffesti, jonathan, lslebodn, pmatilai, pnemade, praiskup, rharwood
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: redhat-rpm-config-60-1.fc26 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-11-30 12:07:49 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Attachments:
Description Flags
flags.patch none

Description Florian Weimer 2016-11-09 16:43:44 UTC
Created attachment 1219012 [details]
flags.patch

Please apply the attached patch to implement this Fedora 26 Change:

https://fedoraproject.org/wiki/Changes/Fedora26CFlags

Comment 1 Panu Matilainen 2016-11-16 12:13:16 UTC
Hmm. No objections to the change as such, but I'm wondering about the __global_conly_cflags thingie. Up to now all languages have been merrily using __global_cflags but if we're starting to add languange-specific options then perhaps instead of inventing new foo-only names we should move that basis to some other macro and save cflags/cppflags/cxxflags etc for their original purposes.

So the rpmrc entries would become something like this:

optflags: i386 %{?__global_compiler_flags} -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables

And the macros side (again something like):

%__global_compiler_flags        -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches %{_hardened_cflags}
%__global_cflags        -Werror=implicit-function-declaration -Werror=implicit-int
%__global_cxxflags      [...] # whenever we get there
%__global_ldflags       -Wl,-z,relro %{?_hardened_ldflags}

...etc, and then lump 'em together in %configure. Untested and all, but I'm think you'll get the idea. That'd actually be something to adopt at rpm upstream, the %__global_fooflags stuff was always a Fedora-specific thing for no good reasons at all.

Comment 2 Florian Weimer 2016-11-16 12:25:01 UTC
If you want to combine this with a larger flags reorganization, that's fine.

But I think it's important not to call g++ with -Wimplicit-function-declaration because the option is accepted by g++ probably by accident.  C++ does not have implied function declarations.  Likewise, -Werror=implicit-int is the default for g++, and disabled by -fpermissive (as required by the C++ language).

Comment 3 Panu Matilainen 2016-11-16 12:38:01 UTC
Yes, different languages needing different flags is well understood, its a bit of a miracle that we've gotten this far without splitting them and so cflags has been abused for a global set of things.

This being the first language-specific addition the change needs to be done now. So I'm really not suggesting anything else than what you're doing technically, only naming-wise.

Comment 4 Panu Matilainen 2016-11-29 08:34:00 UTC
After half-forgetting this for a couple of weeks :-/ I went ahead and applied the part that drops Atom optimizations  + built it for rawhide, so at least the part that directly affects built binaries is now in.

I'll try to look into sorting out the other half along the lines of comment #1 RSN, feel free to kick me if you haven't heard anything by the end of this week.

Comment 5 Panu Matilainen 2016-11-30 12:07:49 UTC
And done, as of redhat-rpm-config-60-1.fc26.

However note that for a more complete coverage of the language specific flags, there are many other packages should be updated too, to replace %optflags with appropriate language-specific macros, similar to what is done to %configure here:
http://pkgs.fedoraproject.org/cgit/rpms/redhat-rpm-config.git/commit/?id=3081c4e7d573893caa1f2813e0a658d0a53d1991
At least:

- kde-filesystem (macros.kde4)
- qt (macros.qt4)
- cmake (macros.cmake)

IIRC Python extensions are C-only so %py2_build and %py3_build should be updated to pass %__global_cflags to build instead of %optflags
- python
- python3

...and probably there are many many more.

Comment 6 Adam Williamson 2016-12-06 19:23:35 UTC
This change caused a huge amount of havoc and is basically breaking Rawhide:

https://bugzilla.redhat.com/show_bug.cgi?id=1401231

I propose it be reverted until we figure out how to do it properly.

Comment 7 Florian Weimer 2016-12-06 22:04:30 UTC
Sorry, we need to revert the -Werror=implicit-* bits.  There is no chance we can get this working in any reasonable time frame, there is simply too much breakage.

Clearly, my initial testing of this change was way off.

Comment 8 Lukas Slebodnik 2017-06-28 12:30:16 UTC
(In reply to Florian Weimer from comment #7)
> Sorry, we need to revert the -Werror=implicit-* bits.  There is no chance we
> can get this working in any reasonable time frame, there is simply too much
> breakage.
> 
> Clearly, my initial testing of this change was way off.

Is there a plan to enable -Werror=implicit-* in rawhide(f27)?

Comment 9 Florian Weimer 2017-06-28 17:25:50 UTC
(In reply to Lukas Slebodnik from comment #8)
> (In reply to Florian Weimer from comment #7)
> > Sorry, we need to revert the -Werror=implicit-* bits.  There is no chance we
> > can get this working in any reasonable time frame, there is simply too much
> > breakage.
> > 
> > Clearly, my initial testing of this change was way off.
> 
> Is there a plan to enable -Werror=implicit-* in rawhide(f27)?

No, it doesn't seem realistic at this point.

Comment 10 Robbie Harwood 2017-06-28 17:33:03 UTC
Is there a new timeframe or has the effort been wholly abandoned?