Bug 238228

Summary: math/openblas: Install header files
Product: Ports & Packages Reporter: Jason W. Bacon <jwb>
Component: Individual Port(s)Assignee: Gleb Popov <arrowd>
Status: Closed FIXED    
Severity: Affects Only Me CC: ard_1, lwhsu, moiseev, phd_kimberlite, vvd
Priority: --- Flags: phd_kimberlite: maintainer-feedback+
Version: Latest   
Hardware: Any   
OS: Any   
URL: https://reviews.freebsd.org/D20866
Bug Depends on:    
Bug Blocks: 241626    
Description Flags
Patch to fix config_kernel.h staging issue
Synth test build logs for config_kernel.h fix none

Description Jason W. Bacon freebsd_committer 2019-05-29 23:54:09 UTC
Is there a reason that the openblas port does not install f77blas.h and openblas_config.h?

biology/gemma, for example uses openblas_config.h.  I've hacked around it for now, but would like to remove the patch at some point.

As the port installs both single- and multi-threaded libraries, we should probably have a separate openblas_config.h for each.


Comment 1 Eijiro Shibusawa 2019-08-04 21:44:14 UTC

I agree to install OpenBLAS header files.

Mr. Popov created a Phabricator review.
Please take a look at this.
Comment 2 Gleb Popov freebsd_committer 2019-09-29 05:13:34 UTC
I got a suggestion to rename "config.h" and "common.h" to "openblas_config.h" and "openblas_common.h" respectively. Former names are too common and can create a clash when used with other projects.

That's why I haven't committed it yet. I'll update the diff in near future.
Comment 3 Gleb Popov freebsd_committer 2019-10-02 18:03:13 UTC
I have updated the Diff. To all whom it concerns, please take a look.

Note there is no f77blas.h to install, but there is cblas.h and my patch makes the port install it.
Comment 4 Artyom Davidov 2019-10-16 18:53:28 UTC
(In reply to Gleb Popov from comment #2)
Since config.h is being renamed to openblas_config.h by that diff, openblas_common.h should be also modified at line 62 to include openblas_config.h instead of config.h.
Comment 5 Gleb Popov freebsd_committer 2019-10-17 11:27:28 UTC
(In reply to Artyom Davidov from comment #4)

Thanks for the pointer, I've fixed it.
Comment 6 Artyom Davidov 2019-10-17 17:56:54 UTC
Thanks Gleb!

A small side-note.
It looks like the port option DYNAMIC_ARCH is mutually exclusive with the usage of this port via its header files.
As far as I understand the CPU optimizations that will be used by this port is being configured by the settings in openblas_config.h (config.h) that is being generated during build time.
With DYNAMIC_ARCH enabled I always get openblas_config.h populated with the setting for the NEHALEM CPU family though my test build machine have another CPU type (family).
I didn't dig in to this further due to the absence of time, but maybe some one can test this behavior and comment on this.
But from the first look it seems that building this port with DYNAMIC_ARCH enabled and using it via the header files would lead to undesired behavior.
Comment 7 Eijiro Shibusawa 2019-10-28 20:56:38 UTC
I approve your new patch.
Please commit it.
Thank you for your contribution!
Comment 8 commit-hook freebsd_committer 2019-10-29 10:26:39 UTC
A commit references this bug:

Author: arrowd
Date: Tue Oct 29 10:25:43 UTC 2019
New revision: 515970
URL: https://svnweb.freebsd.org/changeset/ports/515970

  math/openblas: Install header files.

  PR:		238228
  Reviewed by:	Artyom Davidov <ard_1@mail.ru>
  Approved by:	Eijiro Shibusawa <phd_kimberlite@yahoo.co.jp> (maintainer)
  Differential Revision:	https://reviews.freebsd.org/D20866

Comment 9 Jason W. Bacon freebsd_committer 2019-10-29 13:48:53 UTC
Thanks, guys!
Comment 10 VVD 2019-10-30 11:34:44 UTC
After this commit:

===>>> Building the port required 2848 seconds
===>  Staging for openblas-0.2.20_11,1
===>   Generating temporary packing list
install  -m 0644 /tmp/work/usr/ports/math/openblas/work/lib/libopenblas.a /tmp/work/usr/ports/math/openblas/work/stage/usr/local/lib
install  -s -m 0644 /tmp/work/usr/ports/math/openblas/work/lib/libopenblas.so.0 /tmp/work/usr/ports/math/openblas/work/stage/usr/local/lib
/bin/ln -sf libopenblas.so.0 /tmp/work/usr/ports/math/openblas/work/stage/usr/local/lib/libopenblas.so
install  -m 0644 /tmp/work/usr/ports/math/openblas/work/lib/libopenblasp.a /tmp/work/usr/ports/math/openblas/work/stage/usr/local/lib
install  -s -m 0644 /tmp/work/usr/ports/math/openblas/work/lib/libopenblasp.so.0 /tmp/work/usr/ports/math/openblas/work/stage/usr/local/lib
/bin/ln -sf libopenblasp.so.0 /tmp/work/usr/ports/math/openblas/work/stage/usr/local/lib/libopenblasp.so
cd /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20 &&  /usr/bin/find . -name 'common*.h' -print | /usr/bin/xargs /usr/bin/basename | /usr/bin/xargs -I {} /bin/mv {} openblas_{}
/bin/mv /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/version.h /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/openblas_version.h
/bin/mv /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/param.h /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/openblas_param.h
/bin/mv /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/cpuid.h /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/openblas_cpuid.h
/bin/mv /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/config.h /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/openblas_config.h
/usr/bin/sed -i.bak  -e 's/"common.h"/"openblas_common.h"/'  -e 's/"version.h"/"openblas_version.h"/'  -e 's/"param.h"/"openblas_param.h"/'  -e 's/"cpuid.h"/"openblas_cpuid.h"/'  -e 's/"config.h"/"openblas_config.h"/'  -e 's/"common_/"openblas_common_/'  /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/*.h  /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/*.c  /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/interface/*.c
/bin/mkdir -p /tmp/work/usr/ports/math/openblas/work/stage/usr/local/include/openblas
install  -m 0644 /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/openblas_*.h /tmp/work/usr/ports/math/openblas/work/stage/usr/local/include/openblas
install  -m 0644 /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/cblas.h /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/config_kernel.h /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/config_last.h /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/l1param.h /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/l2param.h /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/symcopy.h  /tmp/work/usr/ports/math/openblas/work/stage/usr/local/include/openblas
install: /tmp/work/usr/ports/math/openblas/work/OpenBLAS-0.2.20/config_kernel.h: No such file or directory
*** Error code 71

Same on 12.0 amd64 and i386.
Comment 11 VVD 2019-10-30 22:31:11 UTC
Comment 12 Artyom Davidov 2019-10-31 02:17:40 UTC
Created attachment 208717 [details]
Patch to fix config_kernel.h staging issue

This patch will fix the config_kernel.h staging issue when DYNAMIC_ARCH option is disabled.
Comment 13 Artyom Davidov 2019-10-31 02:37:10 UTC
Created attachment 208718 [details]
Synth test build logs for config_kernel.h fix

This archive contains two math/openblas Synth test build log files for the config_kernel.h fix with DYNAMIC_ARCH set to disabled and enabled.

Sorry for the 7zip, but the logs are huge and they don't get compressed well enough with gzip to fit in 1MB limit.
Comment 14 VVD 2019-10-31 04:10:13 UTC
(In reply to Artyom Davidov from comment #13)
Thanks for patch!

> Sorry for the 7zip, but the logs are huge and they don't get compressed well enough with gzip to fit in 1MB limit.
tar -J
Comment 15 commit-hook freebsd_committer 2019-10-31 13:22:04 UTC
A commit references this bug:

Author: arrowd
Date: Thu Oct 31 13:21:46 UTC 2019
New revision: 516146
URL: https://svnweb.freebsd.org/changeset/ports/516146

  math/openblas: Fix build with DYNAMIC_ARCH=OFF.

  PR:		238228
  Submitted by:	Artyom Davidov <ard_1@mail.ru>