#3 macros.python*: make LDFLAGS propagated whenever CFLAGS are
Closed 3 years ago by churchyard. Opened 3 years ago by jpokorny.
rpms/ jpokorny/python-rpm-macros ldflags  into  master

file modified
+8 -4
@@ -4,22 +4,26 @@ 

  # Use the slashes after expand so that the command starts on the same line as

  # the macro

  %py_build() %{expand:\\\

-   CFLAGS="%{optflags}" %{__python} %{py_setup} %{?py_setup_args} build --executable="%{__python2} %{py_shbang_opts}" %{?*}

+   CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\

+   %{__python} %{py_setup} %{?py_setup_args} build --executable="%{__python2} %{py_shbang_opts}" %{?*}

    sleep 1

  }

  

  %py_build_egg() %{expand:\\\

-   CFLAGS="%{optflags}" %{__python} %{py_setup} %{?py_setup_args} bdist_egg %{?*}

+   CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\

+   %{__python} %{py_setup} %{?py_setup_args} bdist_egg %{?*}

    sleep 1

  }

  

  %py_build_wheel() %{expand:\\\

-   CFLAGS="%{optflags}" %{__python} %{py_setup} %{?py_setup_args} bdist_wheel %{?*}

+   CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\

+   %{__python} %{py_setup} %{?py_setup_args} bdist_wheel %{?*}

    sleep 1

  }

  

  %py_install() %{expand:\\\

-   CFLAGS="%{optflags}" %{__python} %{py_setup} %{?py_setup_args} install -O1 --skip-build --root %{buildroot} %{?*}

+   CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\

+   %{__python} %{py_setup} %{?py_setup_args} install -O1 --skip-build --root %{buildroot} %{?*}

  }

  

  %py_install_egg() %{expand:\\\

file modified
+8 -4
@@ -9,22 +9,26 @@ 

  # Use the slashes after expand so that the command starts on the same line as

  # the macro

  %py2_build() %{expand:\\\

-   CFLAGS="%{optflags}" %{__python2} %{py_setup} %{?py_setup_args} build --executable="%{__python2} %{py2_shbang_opts}" %{?*}

+   CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\

+   %{__python2} %{py_setup} %{?py_setup_args} build --executable="%{__python2} %{py2_shbang_opts}" %{?*}

    sleep 1

  }

  

  %py2_build_egg() %{expand:\\\

-   CFLAGS="%{optflags}" %{__python2} %{py_setup} %{?py_setup_args} bdist_egg %{?*}

+   CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\

+   %{__python2} %{py_setup} %{?py_setup_args} bdist_egg %{?*}

    sleep 1

  }

  

  %py2_build_wheel() %{expand:\\\

-   CFLAGS="%{optflags}" %{__python2} %{py_setup} %{?py_setup_args} bdist_wheel %{?*}

+   CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\

+   %{__python2} %{py_setup} %{?py_setup_args} bdist_wheel %{?*}

    sleep 1

  }

  

  %py2_install() %{expand:\\\

-   CFLAGS="%{optflags}" %{__python2} %{py_setup} %{?py_setup_args} install -O1 --skip-build --root %{buildroot} %{?*}

+   CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\

+   %{__python2} %{py_setup} %{?py_setup_args} install -O1 --skip-build --root %{buildroot} %{?*}

  }

  

  %py2_install_egg() %{expand:\\\

file modified
+8 -4
@@ -10,22 +10,26 @@ 

  # Use the slashes after expand so that the command starts on the same line as

  # the macro

  %py3_build() %{expand:\\\

-   CFLAGS="%{optflags}" %{__python3} %{py_setup} %{?py_setup_args} build --executable="%{__python3} %{py3_shbang_opts}" %{?*}

+   CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\

+   %{__python3} %{py_setup} %{?py_setup_args} build --executable="%{__python3} %{py3_shbang_opts}" %{?*}

    sleep 1

  }

  

  %py3_build_egg() %{expand:\\\

-   CFLAGS="%{optflags}" %{__python3} %{py_setup} %{?py_setup_args} bdist_egg %{?*}

+   CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\

+   %{__python3} %{py_setup} %{?py_setup_args} bdist_egg %{?*}

    sleep 1

  }

  

  %py3_build_wheel() %{expand:\\\

-   CFLAGS="%{optflags}" %{__python3} %{py_setup} %{?py_setup_args} bdist_wheel %{?*}

+   CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\

+   %{__python3} %{py_setup} %{?py_setup_args} bdist_wheel %{?*}

    sleep 1

  }

  

  %py3_install() %{expand:\\\

-   CFLAGS="%{optflags}" %{__python3} %{py_setup} %{?py_setup_args} install -O1 --skip-build --root %{buildroot} %{?*}

+   CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\

+   %{__python3} %{py_setup} %{?py_setup_args} install -O1 --skip-build --root %{buildroot} %{?*}

  }

  

  %py3_install_egg() %{expand:\\\

This is to ensure the right linker flags get propagated into binaries
accompanying python packages whenever distutils module is delegated to
build them (e.g. https://bugzilla.redhat.com/show_bug.cgi?id=1541106).
Follows the conventions used with %configure in redhat/macros.

Signed-off-by: Jan Pokorný jpokorny@redhat.com

Note that I am unsure why %py*_install would need to deal with build flags.

It's probably better to use $RPM_LD_FLAGS and keep using %{optflags} (or $RPM_OPT_FLAGS) because those __ macros are really quite internal to redhat-rpm-config. I'm going to update that soon with more documentation.

rebased onto 4454af7

3 years ago

My reasoning was that explicit is better than implicit, e.g. when one
reviews the build.log, as RPM macros get resolved in-situ before
invocation by shell unlike with shell variables.

Now, I turned both flags to use ${RPM_*_FLAGS}, inter-commit
example diff for posterity:

--- a/macros.python
+++ b/macros.python
@@ -4,25 +4,25 @@
 # Use the slashes after expand so that the command starts on the same line as
 # the macro
 %py_build() %{expand:\\\
-  CFLAGS="${CFLAGS:-%__global_cflags}" LDFLAGS="${LDFLAGS:-%__global_ldflags}"\\\
+  CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\
   %{__python} %{py_setup} %{?py_setup_args} build --executable="%{__python2} %{py_shbang_opts}" %{?*}
   sleep 1
 }

 %py_build_egg() %{expand:\\\
-  CFLAGS="${CFLAGS:-%__global_cflags}" LDFLAGS="${LDFLAGS:-%__global_ldflags}"\\\
+  CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\
   %{__python} %{py_setup} %{?py_setup_args} bdist_egg %{?*}
   sleep 1
 }

 %py_build_wheel() %{expand:\\\
-  CFLAGS="${CFLAGS:-%__global_cflags}" LDFLAGS="${LDFLAGS:-%__global_ldflags}"\\\
+  CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\
   %{__python} %{py_setup} %{?py_setup_args} bdist_wheel %{?*}
   sleep 1
 }

 %py_install() %{expand:\\\
-  CFLAGS="${CFLAGS:-%__global_cflags}" LDFLAGS="${LDFLAGS:-%__global_ldflags}"\\\
+  CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}"\\\
   %{__python} %{py_setup} %{?py_setup_args} install -O1 --skip-build --root %{buildroot} %{?*}
 }

Note that I am unsure why %py*_install would need to deal with build flags.

But at least current arrangement won't hurt either, as %__spec_install_pre
also boils down to %{___build_pre} that sets those ${RPM_*_FLAGS}
variables.

@jpokorny I recently added %{build_cflags}, %{build_cxxflags} and %{build_ldflags}, so that the information is available as non-internal RPM macros as well. So you could use those if you prefer RPM macros.

Oh, I see, that would perhaps be preferred (no opinion from package
maintainers yet). The only drawbacks are:
- brings new direct dependency on redhat-rpm-config
- furthermore this dependency needs to be versioned
(I am yet to see those macros available in Rawhide)

Ping, is there any feedback by the package maintainers?

I want to look into this, but I am on vacation. Can this wait for next week?

I want to look into this, but I am on vacation. Can this wait for next week?

Didn't mean to be snappy, just thought that there can be some
early feedback when backed by 4 admins and whole dedicated SIG.

Also depends whether this is deemed an extension of
https://fedoraproject.org/wiki/Changes/BINUTILS2291
(then the completion date is in 10 days' time)
and/or something should be done about this by the
branching point (seems immediately follow said milestone).

Anyway, setting just CFLAGS while keeping linker flags
untouched seems fishy.

Actually this whole arrangement may be obsoleted
by %{extension_*flags} macros applied in a similar
fashion, I presume:

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

Is there any reason not to use %set_build_flags?

OK, this is how I understand it: This boils down to:

  • CFLAGS: changed from %{optflags} to ${CFLAGS:-${RPM_OPT_FLAGS}} (where $RPM_OPT_FLAGS is actually the same as %{optflags} but consistency with the latter).
  • LDFLAGS: changed from nothing to ${LDFLAGS:-${RPM_LD_FLAGS}}.

I say LGTM.

I don't understand @ignatenkobrain's question.

@jpokorny would you be able to rebase the PR in order for us to merge it?

Pull-Request has been closed by churchyard

3 years ago