PR#1: Proposed changes for fixing undefined references to hdf5 - rpms/cgnslib

rpms / cgnslib

#1 Proposed changes for fixing undefined references to hdf5

Merged 5 years ago by smani. Opened 5 years ago by sagitter.

rpms/ sagitter/cgnslib master into master

Fix shebang

sagitter • 5 years ago

56177a9

Fix Fortran linker flags for epel7

sagitter • 5 years ago

af89670

Fix undefined references to HDF5 (bz#1623439)

sagitter • 5 years ago

bf718b3

cgnslib.spec

file modified

+22 -6

		`@@ -1,6 +1,6 @@`
		`Name: cgnslib`
		`Version: 3.3.1`
		`- Release: 5%{?dist}`
		`+ Release: 7%{?dist}`
		`Summary: Computational Fluid Dynamics General Notation System`
		`License: zlib`
		`URL: http://www.cgns.org/`
		`@@ -24,7 +24,7 @@`
		`BuildRequires: tcl-devel`
		`BuildRequires: tk-devel`
		`BuildRequires: zlib-devel`
		`- Requires: hdf5 = %{_hdf5_version}`
		`+ Requires: hdf5%{?_isa} = %{_hdf5_version}`

		`%description`
		`The Computational Fluid Dynamics General Notation System (CGNS) provides a`
		`@@ -38,6 +38,7 @@`
		`Summary: Development files for %{name}`
		`Requires: %{name}%{?_isa} = %{version}-%{release}`
		`Requires: hdf5-devel%{?_isa}`
		`+ Requires: gcc-gfortran%{?_isa}`
smani commented 5 years ago Is it customary for a -devel package to require a specific compiler? Technically you could use any fortran compiler.

		`%description devel`
		`This package contains libraries and header files for`
		`@@ -50,12 +51,16 @@`
		`sed -i "s\|\${CMAKE_INSTALL_PREFIX}/lib\|\${CMAKE_INSTALL_PREFIX}/\${LIB_INSTALL_DIR}\|" CMakeLists.txt`
		`sed -i "s\|DESTINATION lib\|DESTINATION \${LIB_INSTALL_DIR}\|" src/CMakeLists.txt`

		`+ # Remove executable bit`
		`+ chmod a-x src/cgnstools/utilities/cgns_to_vtk.c`
		`+`
		`%build`
		`%cmake -DCMAKE_SKIP_RPATH=ON \`
		`-DCGNS_ENABLE_TESTS=ON \`
		`-DCGNS_ENABLE_FORTRAN=ON \`
		`-DCGNS_BUILD_CGNSTOOLS=ON \`
		`- -DCGNS_ENABLE_HDF5=ON`
		`+ -DCGNS_ENABLE_HDF5=ON \`
		`+ -DCMAKE_Fortran_FLAGS_RELEASE:STRING="$FCFLAGS -DNDEBUG $LDFLAGS -lhdf5 -fPIC"`
		`# FIXME: Ugly workaround for build order issue which results in`
		`# an incomplete libcgns.so being created during the first run`
		`make \|\| rm src/libcgns.so* && make`
		`@@ -65,12 +70,14 @@`
		`%make_install`
		`find %{buildroot} -name '*.a' -delete -print`

		`+ # Add shebang`
		`+ # NEED CHECK - really need?`
		`+ sed -i -e '1i#!%{_bindir}/sh' %{buildroot}%{_bindir}/cgconfig`
		`+`
		`%check`
		`LD_LIBRARY_PATH=%{buildroot}%{_libdir} make test`

		`- %post -p /sbin/ldconfig`
		`-`
		`- %postun -p /sbin/ldconfig`
		`+ %ldconfig_scriptlets`

		`%files`
		`%license license.txt`
		`@@ -107,6 +114,15 @@`
		`%{_fmoddir}/cgns.mod`

		`%changelog`
		`+ * Thu Aug 30 2018 Antonio Trande <sagitterATfedoraproject.org> - 3.3.1-7`
		`+ - Fix Fortran linker flags for epel7`
		`+`
		`+ * Wed Aug 29 2018 Antonio Trande <sagitterATfedoraproject.org> - 3.3.1-6`
		`+ - Fix undefined references to HDF5 (bz#1623439)`
		`+ - Add shebang to cgconfig`
		`+ - Remove spurious executable bit`
		`+ - Add Requires gcc-gfortran to the devel sub-package`
		`+`
		`* Thu Jul 12 2018 Fedora Release Engineering <releng@fedoraproject.org> - 3.3.1-5`
		`- Rebuilt for https://fedoraproject.org/wiki/Fedora_29_Mass_Rebuild`

sagitter commented 5 years ago

About bz #1623439, i've made some modifies to your SPEC file:

Fix undefined references to HDF5 (bz#1623439)
Add shebang to cgconfig
Remove spurious executable bit
Add Requires gcc-gfortran to the devel sub-package
Fix Fortran linker flags for epel7

Tested on Copr, on fedora 28+ x86_64, and rebuilt related dependent packages:
https://copr.fedorainfracloud.org/coprs/sagitter/ForTesting/builds/

Rebuilt on epel7:
https://koji.fedoraproject.org/koji/taskinfo?taskID=29376491

Please, let me know if you agree to push on upstream.

smani commented on line 22 of cgnslib.spec 5 years ago

Is it customary for a -devel package to require a specific compiler? Technically you could use any fortran compiler.

sagitter commented 5 years ago

That line is there for packaging reason because %{_fmoddir} directory, where Fortran modules are installed, is owned by gcc-gfortran:

$ rpm --eval %{_fmoddir}
/usr/lib64/gfortran/modules

$ repoquery -f /usr/lib64/gfortran/modules
gcc-gfortran-0:8.0.1-0.20.fc28.x86_64
gcc-gfortran-0:8.1.1-5.fc28.x86_64

Quoting packaging guidelines for Fortran:

As Fortran modules are architecture and GCC version specific, they MUST be placed into %{_fmoddir} (or its package-specific subfolder in case the modules have generic names), which is owned by 'gcc-gfortran'. For directory ownership any packages containing Fortran modules MUST Requires: gcc-gfortran%{_isa}.

smani commented 5 years ago

Good point, thanks!

Pull-Request has been merged by smani

5 years ago

sagitter commented 5 years ago

-Wl,--as-needed flag, forced as linker flag on fedora 30, is still breaking -lhdf5. This prevents linking to cgnslib in other package like PETsc.

I fixed this issue by

%if 0%{?fedora} > 29
LDFLAGS=$(echo "%{__global_ldflags}" | %{__sed} -e 's/-Wl,--as-needed//')
%endif

before cmake command.

sagitter commented 5 years ago

@ignatenkobrain

-Wl,--as-needed flag, forced as linker flag on fedora 30, is still breaking -lhdf5. This prevents linking to cgnslib in other package like PETsc.
I fixed this issue by
%if 0%{?fedora} > 29
LDFLAGS=$(echo "%{__global_ldflags}" | %{__sed} -e 's/-Wl,--as-needed//')
%endif

before cmake command.