Bug 1393492 - rpm-redhat-config: Fedora 26 C/C++ build flag changes
Summary: rpm-redhat-config: Fedora 26 C/C++ build flag changes
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: redhat-rpm-config
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Panu Matilainen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-11-09 16:43 UTC by Florian Weimer
Modified: 2017-06-28 17:33 UTC (History)
8 users (show)

Fixed In Version: redhat-rpm-config-60-1.fc26
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-11-30 12:07:49 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
flags.patch (1.71 KB, patch)
2016-11-09 16:43 UTC, Florian Weimer
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 1401231 0 unspecified CLOSED Broken detection of toolchain features after upgrade of redhat-rpm-config 2022-09-16 06:24:16 UTC

Internal Links: 1401231

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?


Note You need to log in before you can comment on or make changes to this bug.