#172 Refuse %configure in %prep
Opened 2 years ago by zbyszek. Modified 4 months ago
rpms/ zbyszek/redhat-rpm-config refuse-configure-in-prep  into  rawhide

file modified
+11 -9
@@ -80,24 +80,26 @@ 

  # been set implicitly at the start of the %%build section.

  # LT_SYS_LIBRARY_PATH is used by libtool script.

  %set_build_flags \

-   CFLAGS="${CFLAGS:-%{build_cflags}}" ; export CFLAGS ; \

-   CXXFLAGS="${CXXFLAGS:-%{build_cxxflags}}" ; export CXXFLAGS ; \

-   FFLAGS="${FFLAGS:-%{build_fflags}}" ; export FFLAGS ; \

-   FCFLAGS="${FCFLAGS:-%{build_fflags}}" ; export FCFLAGS ; \

-   LDFLAGS="${LDFLAGS:-%{build_ldflags}}" ; export LDFLAGS ; \

-   LT_SYS_LIBRARY_PATH="${LT_SYS_LIBRARY_PATH:-%_libdir:}" ; export LT_SYS_LIBRARY_PATH ; \

-   CC="${CC:-%{__cc}}" ; export CC ; \

-   CXX="${CXX:-%{__cxx}}" ; export CXX

+   export CFLAGS="${CFLAGS:-%{build_cflags}}" ; \

+   export CXXFLAGS="${CXXFLAGS:-%{build_cxxflags}}" ; \

+   export FFLAGS="${FFLAGS:-%{build_fflags}}" ; \

+   export FCFLAGS="${FCFLAGS:-%{build_fflags}}" ; \

+   export LDFLAGS="${LDFLAGS:-%{build_ldflags}}" ; \

+   export LT_SYS_LIBRARY_PATH="${LT_SYS_LIBRARY_PATH:-%_libdir:}" ; \

+   export CC="${CC:-%{__cc}}" ; \

+   export CXX="${CXX:-%{__cxx}}"

  

  # Automatically use set_build_flags macro for build, check, and

  # install phases.

  # Use "%undefine _auto_set_build_flags" to disable"

  %_auto_set_build_flags 1

  %__spec_build_pre %{___build_pre} \

+   spec_build_phase=build \

    %{?_auto_set_build_flags:%{set_build_flags}} \

    %{?_generate_package_note_file}

  

  %__spec_check_pre %{___build_pre} \

+   spec_build_phase=check \

    %{?_auto_set_build_flags:%{set_build_flags}} \

    %{?_generate_package_note_file}

  
@@ -185,7 +187,7 @@ 

    done

  

  %configure \

-   %{set_build_flags}; \

+   [ "$spec_build_phase" == "build" ] || [ "$spec_build_phase" == "check" ] || { echo "error: %%configure should be called in %%build or %%check sections only"; exit 1; } \

    [ "%{_lto_cflags}"x != x ] && %{_fix_broken_configure_for_lto}; \

    [ "%_configure_gnuconfig_hack" = 1 ] && for i in $(find $(dirname %{_configure}) -name config.guess -o -name config.sub) ; do \

        [ -f /usr/lib/rpm/redhat/$(basename $i) ] && %{__rm} -f $i && %{__cp} -fv /usr/lib/rpm/redhat/$(basename $i) $i ; \

file modified
+4 -1
@@ -6,7 +6,7 @@ 

  

  Summary: Red Hat specific rpm configuration files

  Name: redhat-rpm-config

- Version: 212

+ Version: 213

  Release: 1%{?dist}

  # No version specified.

  License: GPL+
@@ -191,6 +191,9 @@ 

  %doc buildflags.md

  

  %changelog

+ * Mon Jan 24 2022 Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> - 213-1

+ - Add check that %%configure is called in %%build or %%check only

+ 

  * Sun Jan 23 2022 Robert-André Mauchin <zebob.m@gmail.com> - 212-1

  - Add package note generation to %%check preamble

  - Fix: rhbz#2043977

no initial comment

Not that I wouldn't love to see this, but won't it break a number of things? This used to be relatively common.

FWIW, I haven't seen this is any new packages… I guess it might be still used some venerable packages, but I don't think it's exactly "common".

I'd be happy to fix any cases we find… Usually it's just a matter of moving "%build" line a few lines up.

Build succeeded.

Thx for this PR. Two questions:

1) Do we know how common the misuse is? I'd like to know more then "I haven't seen this is any new packages…" ;)
2) Can there be any real use case for %configure in prep? Shouldn't there be some way to workaround the error message in that case? Probably not, just wanted to ask ...

1) 21

rpm-specs/cvechecker.spec
rpm-specs/dahdi-tools.spec
rpm-specs/dans-gdal-scripts.spec
rpm-specs/dd_rescue.spec
rpm-specs/dovecot-fts-xapian.spec
rpm-specs/fonts-tweak-tool.spec
rpm-specs/geomorph.spec
rpm-specs/htmlcxx.spec
rpm-specs/hwdata.spec
rpm-specs/ibutils.spec
rpm-specs/ipv6calc.spec
rpm-specs/libresample.spec
rpm-specs/librs232.spec
rpm-specs/libsqlite3x.spec
rpm-specs/libucl.spec
rpm-specs/openpgm.spec
rpm-specs/QtDMM.spec
rpm-specs/recorder.spec
rpm-specs/wv.spec
rpm-specs/x11trace.spec
rpm-specs/z80dasm.spec

2) I think we can assume "no". If there's is a use case, we can always add a work-around. It is also trivial to spoof the check by setting a variable.

I patched the packages listed in the previous comment to move %configure to %build:
cvechecker dd_rescue dovecot-fts-xapian fonts-tweak-tool geomorph htmlcxx hwdata ibutils ipv6calc libresample librs232 libsqlite3x libucl openpgm QtDMM recorder wv x11trace z80dasm

dans-gdal-scripts was retired by @churchyard earlier today.

dahdi-tools was an interesting case: it did %configure in %prep, because it wanted to apply patches to Makefile generated by %configure. I tried to patch Makefile.am instead, but then make wants to apply automake-1.15 to rebuild the Makefile. So then I wanted to move the patching of the Makefile to %build. But it didn't work there because %patch4 macro is not defined in %build: apparently those macros can only be used in %prep. OK, I just replaced the patch with sed. (There was another sed invocation done right new too, so I thought it's OK to just do two sed patterns.)

The above uncovered an interesting failure mode: make LDFLAGS="$LDFLAGS -fPIC" was invoked in %build. But before F36 $LDFLAGS was not set in %build at all! So we were passing just LDFLAGS=-fPIC. In F36, with the change to call %set_build_flags in the preamble, LDFLAGS is defined, and make gets additional flags and the build fails during linking. With the change I wanted to do, i.e. to move %configure down, LDFLAGS are set in %build too, and the build also fails, even in F35. So this package looked like it was passing the default LDFLAGS to make, but it really wasn't. I just filed a PR that builds successfully like in F35, but I'll leave it up to the maintainer to merge [https://src.fedoraproject.org/rpms/dahdi-tools/pull-request/2].

recorder seems to have the same issue, and the build fails. (It also failed during the mass rebuild.)

So now there are 0 packages with call %configure in %prep ;)

Since there are now zero packages, I think we should just go ahead and land this. If there are no objections, I'll land it tomorrow.

@zbyszek This needs to be rebased.
@ngompa Ping on merging this.

@zbyszek FPC agrees this is gravy, can you please rebase this so I can merge and release this to Rawhide?

macros: use a shorter form to export variables

Please don't mix in unrelated changes here.

This still need rebasing...

Metadata