Bug 1543394 - redhat-rpm-config: Provide macros for extension builders
Summary: redhat-rpm-config: Provide macros for extension builders
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: Florian Festi
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1284684 1217376 1661186
TreeView+ depends on / blocked
 
Reported: 2018-02-08 11:29 UTC by Florian Weimer
Modified: 2019-02-18 09:33 UTC (History)
14 users (show)

Fixed In Version: redhat-rpm-config-127-1.fc30
Doc Type: Enhancement
Doc Text:
Clone Of:
: 1661186 (view as bug list)
Environment:
Last Closed: 2018-12-20 10:45:06 UTC
Type: Bug


Attachments (Terms of Use)
Proposed patch (18.81 KB, patch)
2018-02-08 11:29 UTC, Florian Weimer
no flags Details | Diff
Second proposed patch (2.02 KB, patch)
2018-03-01 15:41 UTC, Florian Weimer
no flags Details | Diff
Proposed patch with documentation (3.55 KB, patch)
2018-12-11 11:04 UTC, Florian Weimer
no flags Details | Diff
Patch with typo fix (3.55 KB, application/mbox)
2018-12-11 14:37 UTC, Florian Weimer
no flags Details


Links
System ID Priority Status Summary Last Updated
Red Hat Bugzilla 1544527 None NEW Using net-snmp-config --external-libs cause link failure if redhat-rpm-config not installed 2019-08-27 12:44:44 UTC
Red Hat Bugzilla 1557667 None CLOSED perl - Config_heavy.pl uses redhat-hardened-cc1 without depending on redhat-rpm-config 2019-08-27 12:44:44 UTC
Red Hat Bugzilla 1634784 None ON_QA python3 hard-codes annobin usage 2019-08-27 12:44:44 UTC

Internal Links: 1544527 1557667 1634784

Description Florian Weimer 2018-02-08 11:29:41 UTC
Created attachment 1393165 [details]
Proposed patch

Extension builders are tools which automatically build C/C++ extensions for use with other programming languages (Python, Ruby, Go, npm, and so on).

These extension builders do not want to use the default GCC flags (no optimization, no debugging information, no hardening), but they also cannot use the flags for instrumented builds redhat-rpm-config provides.

I have come up with a somewhat hackish solution to provide compiler/linker flags which have no external dependencies.  These can then be baked into extension builders, so that only gcc/gcc-c++ will be needed for building extensions, but not redhat-rpm-config, annobin, and whatever build instrumentation we add in the future.

Comment 1 Vít Ondruch 2018-02-08 15:37:05 UTC
I have risen the topic to the Ruby upstream [1]. And part of the answer was:

> Options given to configure as XCFLAGS and XLDFLAGS are used to build ruby itself, but not for extension libraries.

Now the question is how to use these with the macros proposed by you. The issue is, that they won't work with the standard %configure macro. That gives space to making mistakes.

Also, not sure if other languages supports something like X.+FLAGS macros or this is just Ruby specific.


[1] https://bugs.ruby-lang.org/issues/14422

Comment 2 Florian Weimer 2018-02-08 15:51:10 UTC
(In reply to Vít Ondruch from comment #1)
> I have risen the topic to the Ruby upstream [1]. And part of the answer was:
> 
> > Options given to configure as XCFLAGS and XLDFLAGS are used to build ruby itself, but not for extension libraries.
> 
> Now the question is how to use these with the macros proposed by you. The
> issue is, that they won't work with the standard %configure macro. That
> gives space to making mistakes.

Almost certainly, you need to change code in the builder.  Ideally, there would be run-time detection for using different flags when running under rpmbuild and when not.

Comment 3 Vít Ondruch 2018-02-08 16:11:57 UTC
(In reply to Florian Weimer from comment #2)
IOW the X flags won't help at all, since we don't want to have some configuration for interpreted and different configuration of plugins plugins, but we want to have the same configuration flags for interpreter and plugins build via rpmbuild and different flags for plugins build by the language package manager.

It won't work without modifications of the interpreted plugin manager and I don't know who would implement the necessary changes :/

Comment 4 Josh Stone 2018-02-09 01:47:16 UTC
Rust and Cargo don't directly know anything about how non-Rust code will be compiled.  Any crate can have a build.rs script, and that can do completely arbitrary things.

Usually those build.rs scripts will use the "cc" crate though.  This does look in the environment for CFLAGS/etc, and maybe we could teach it to look elsewhere, but I don't know how we would integrate rpm macros into this.

[cc]: https://crates.io/crates/cc

Comment 5 Petr Viktorin 2018-02-26 10:00:25 UTC
What's the status here?
We can hack up something specific to Python if this problem be solved generally.

Comment 6 Florian Weimer 2018-02-26 10:09:16 UTC
With the change in bug 1548397, we can now simply filter out all -specs= arguments.  This might change in the future, but for now, we can have a much simpler implementation.  I'll submit a patch for that, hopefully later today.

Comment 7 Florian Weimer 2018-03-01 15:41:01 UTC
Created attachment 1402578 [details]
Second proposed patch

This patch depends on the -z now change in bug 1548397.  It filters out the -specs= arguments we do not want hard-coded into extension builders.

Comment 8 Florian Weimer 2018-03-07 12:50:28 UTC
(In reply to Florian Weimer from comment #7)
> Created attachment 1402578 [details]
> Second proposed patch
> 
> This patch depends on the -z now change in bug 1548397.  It filters out the
> -specs= arguments we do not want hard-coded into extension builders.

Any comments on this approach?  It doesn't avoid the hard-coding, but I got the feeling that we don't want to patch out the build-flag-capturing logic in the upstream code.

Comment 9 Vít Ondruch 2018-03-07 13:01:18 UTC
I have to build Ruby with the original flags, including the -specs= option + some other options which are added by Ruby configuration process. They gets baked into rbconfig.rb and later provided as default when people are installing gems via "gem install".

So I am not sure how should I benefit from these new macros? Should I go and try to s/%{cflags}/%{extension_cflags}/ in rbconfig.rb? Not sure this will work and provide reliable results.

May if there was opposite, set of the flags I should definitely drop from the rbconfig.rb. Not sure.

Frankly, I don't like the idea of tweaking rbconfig.rb.

Comment 10 Florian Weimer 2018-03-09 10:21:15 UTC
(In reply to Vít Ondruch from comment #9)
> Frankly, I don't like the idea of tweaking rbconfig.rb.

You would have to change mkconfig.rb anyway.

Could we patch mkconfig.rb so that the generated rbconfig.rb file reads overrides for those settings from files at specific locations?  Then we ship two subpackages with different defaults, and only one would pull in annobin.

Comment 11 Florian Weimer 2018-05-28 12:55:00 UTC
We realized that avoiding the dependency on annobin and redhat-rpm-config could not be implemented without changes to extension builders, and we received little support for that from the respective package maintainers.  As a result, it does not make sense to add separate build flags in redhat-rpm-config.

Extension builders which hard-code the distribution CFLAGS should add a dependency on redhat-rpm-config and gcc (and gcc-c++ if applicable).

Comment 12 Florian Weimer 2018-12-09 15:53:39 UTC
Bug 1634784 is a downstream requirement for this change, so reopening.

Comment 13 Florian Weimer 2018-12-11 11:04:21 UTC
Created attachment 1513332 [details]
Proposed patch with documentation

This patch is essentially unchanged from the second patch, but now also includes documentation.

Comment 14 Miro Hrončok 2018-12-11 14:24:58 UTC
Excellent! Small nitpick:

{%extension_cxxflags} -> %{extension_cxxflags}

Comment 15 Florian Weimer 2018-12-11 14:37:37 UTC
Created attachment 1513414 [details]
Patch with typo fix

Fix the documentation typo mentioned in comment 14.  Thanks.

Comment 16 Miro Hrončok 2018-12-11 16:54:30 UTC
Experimental PR for python3: https://src.fedoraproject.org/rpms/python3/pull-request/75

Comment 17 Florian Weimer 2018-12-13 15:57:04 UTC
Florian, Panu, any chance that this patch can be merged?  Thanks.

Comment 18 Panu Matilainen 2018-12-20 08:43:14 UTC
I've no objections.

Comment 19 Florian Weimer 2018-12-20 10:45:06 UTC
Thanks, merged.


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