PR#1: Build against FlexiBLAS - rpms/GMT

rpms / GMT

#1 Build against FlexiBLAS

Closed 3 years ago by orion. Opened 3 years ago by iucar.

rpms/ iucar/GMT master into master

https://fedoraproject.org/wiki/Changes/FlexiBLAS_as_BLAS/LAPACK_manager

Iñaki Úcar • 3 years ago

1a618c0

GMT.spec

file modified

+15 -7

		`@@ -17,7 +17,7 @@`

		`Name: GMT`
		`Version: 6.1.0`
		`- Release: 3%{?dist}`
		`+ Release: 4%{?dist}`
		`Summary: Generic Mapping Tools`

		`License: LGPLv3+`
		`@@ -36,7 +36,11 @@`
		`BuildRequires: GraphicsMagick`
		`BuildRequires: libXt-devel libXaw-devel libXmu-devel libXext-devel`
		`BuildRequires: netcdf-devel`
		`+ %if 0%{?fedora} >= 33`
		`+ BuildRequires: pkgconfig(flexiblas)`
		`+ %else`
		`BuildRequires: openblas-devel`
		`+ %endif`
		`BuildRequires: pcre-devel`
		`BuildRequires: dcw-gmt`
		`BuildRequires: gshhg-gmt-nc4`
		`@@ -134,9 +138,7 @@`


		`%build`
		`- mkdir build`
		`- pushd build`
		`- %{cmake} \`
		`+ %{cmake} -B build \`
decathorpe commented 3 years ago Drop the `-B build` here.
		`-DGSHHG_ROOT=%{_datadir}/gshhg-gmt-nc4 \`
		`-DGMT_INSTALL_MODULE_LINKS=off \`
		`-DGMT_INSTALL_TRADITIONAL_FOLDERNAMES=off \`
		`@@ -146,9 +148,12 @@`
		`%endif`
		`-DGMT_ENABLE_OPENMP=BOOL:ON \`
		`-DGMT_USE_THREADS=BOOL:ON \`
		`- -DBASH_COMPLETION_DIR=%{completion_dir} \`
		`- ..`
		`- %make_build`
		`+ %if 0%{?fedora} >= 33`
		`+ -DBLAS_LIBRARY=-lflexiblas \`
		`+ -DLAPACK_LIBRARY=-lflexiblas \`
		`+ %endif`
		`+ -DBASH_COMPLETION_DIR=%{completion_dir}`
		`+ %make_build -C build`
decathorpe commented 3 years ago Use `%cmake_build` here.


		`%install`
		`@@ -208,6 +213,9 @@`
decathorpe commented 3 years ago Use `%cmake_install` in `%install`.


		`%changelog`
		`+ * Fri Aug 07 2020 2020 Iñaki Úcar <iucar@fedoraproject.org> - 6.1.0-4`
		`+ - https://fedoraproject.org/wiki/Changes/FlexiBLAS_as_BLAS/LAPACK_manager`
		`+`
		`* Mon Jul 27 2020 Fedora Release Engineering <releng@fedoraproject.org> - 6.1.0-3`
		`- Rebuilt for https://fedoraproject.org/wiki/Fedora_33_Mass_Rebuild`

iucar commented 3 years ago

This PR is part of a F33 System-Wide Change Proposal that aims to set FlexiBLAS
as the default BLAS/LAPACK manager for all consumers of these APIs. For further
information, see https://fedoraproject.org/wiki/Changes/FlexiBLAS_as_BLAS/LAPACK_manager

You'll find a successful scratch build for this PR in
https://koji.fedoraproject.org/koji/taskinfo?taskID=47816901

I kindly ask you to merge this PR and build the package in rawhide. Please,
report any issues you may found in https://bugzilla.redhat.com/show_bug.cgi?id=1860504

rebased onto d350f52

3 years ago

rebased onto 1a618c0

3 years ago

iucar commented 3 years ago

@decathorpe ready!

decathorpe commented 3 years ago

Can you fix the CMake related stuff in this PR please? The -B flag is not going to be supported. You can do %undefine __cmake_in_source_build at the top to force the same behaviour across fedora releases, but the -B flag for %cmake won't be supported.

(also drop the previously added __cmake_in_source_dir 1 ...)

Edited 3 years ago by decathorpe

decathorpe commented on line 29 of GMT.spec 3 years ago

Drop the -B build here.

decathorpe commented on line 45 of GMT.spec 3 years ago

Use %cmake_build here.

decathorpe commented on line 49 of GMT.spec 3 years ago

Use %cmake_install in %install.

orion commented 3 years ago

Please do not merge this until I have a chance to review it.

decathorpe commented 3 years ago

@orion ACK!

iucar commented 3 years ago

@decathorpe Please, correct me if I'm wrong, (the wiki is throwing a 503 now, so I can't verify this) but the -B is listed as an option to adapt packages to the cmake change proposal. And I'm doing it that way because it is less disruptive, especially for packages like this that are maintained for EPEL 7-8 too.

That said, please @orion let us know what's your preference in this regard.

decathorpe commented 3 years ago

From what I understand, the preferred way to have consistent behaviour across releases and EPEL (if that's really needed) is to %undefine __cmake_in_source_build and do %cmake without -S or -B switches, %cmake_build and %cmake_install. IIUC the standalone -B flag without using a corresponding -S flag is undocumented behavior.

orion commented 3 years ago

I'm closing this PR and making my own commit. There are a couple issues:

We need to also define GMT_EXCLUDE_BLAS and GMT_EXCLUDE_LAPACK in order for GMT to still build with flexiblas in the presence of a locally install openblas package.
Related to the above - you might want to submit a change to FindBLAS in cmake to find flexiblas.
I prefer the %bcond_without flexiblas idiom.
To be incredibly pedantic - since we are not using pkgconfig to locate flexiblas, the BR really should be BR: flexiblas-devel
@decathorpe is correct about using %undefine __cmake_in_source_build and the plain %cmake macros. I've used that.

Thanks for the PR though - it got me moving.

Pull-Request has been closed by orion

3 years ago

decathorpe commented 3 years ago

Thanks!

iucar commented 3 years ago

Thanks, @orion, for working on this. Some people hate the bcond idiom, some people love it... And sometimes it's hard to get it right. :) If you have more packages depending on any flavour of BLAS/LAPACK and some spare time, I would appreciate if you could adapt them too. Otherwise, you can expect more PRs. :)