Conversation
…"version.txt" into the documentation directory. Removed installing "README.md" from LIBXSMM's root directory as it overrides README.md provided there. The latter uses correct relative references to the other documentation parts. Note: Apparently, "FC=/path/to/gfortran spack install libxsmm" is currently needed for Spack since does not pick-up the Fortran compiler (but incorrectly uses the C compiler instead).
adamjstewart
left a comment
adamjstewart
left a comment
There was a problem hiding this comment.
Note: Apparently, "FC=/path/to/gfortran spack install libxsmm" is currently needed for Spack since does not pick-up the Fortran compiler (but incorrectly uses the C compiler instead).
Have you tried editing patch to explicitly point to Spack's compiler wrappers? You can use spack_cc or env['CC'] to do this (similarly named things for cxx, f77, and fc defined in lib/spack/spack/build_environment.py).
| install('LICENSE.md', doc_path) | ||
|
|
||
| def install(self, spec, prefix): | ||
| if '+header-only' in spec and '@1.6.2:' not in spec: |
There was a problem hiding this comment.
This could probably be converted to a conflicts() directive if you want.
| @@ -32,10 +32,11 @@ class Libxsmm(Package): | |||
| and small convolutions.''' | |||
There was a problem hiding this comment.
This package should probably be converted to a MakefilePackage. Then install would become build, manual_install would become install, and patch would become edit.
There was a problem hiding this comment.
I have addressed your recommendation in the updated PR.
| install('README.md', doc_path) | ||
| install('LICENSE', doc_path) | ||
| install('version.txt', doc_path) | ||
| install('LICENSE.md', doc_path) |
There was a problem hiding this comment.
Won't that break installation of old version?
There was a problem hiding this comment.
I addressed this issue in the updated PR.
|
|
||
| def manual_install(self, prefix): | ||
| def install(self, spec, prefix): | ||
| spec = self.spec |
There was a problem hiding this comment.
You can delete this line now
| if '~header-only' in spec: | ||
| install_tree('lib', prefix.lib) | ||
| doc_path = prefix.share + '/libxsmm/doc' | ||
| doc_path = join_path(prefix.share, 'libxsmm', 'doc') |
There was a problem hiding this comment.
You can use prefix.libxsmm.doc instead of join_path(...)
| install('LICENSE', doc_path) | ||
| install('version.txt', doc_path) | ||
|
|
||
| def build(self, spec, prefix): |
There was a problem hiding this comment.
Can you move this above install to keep things in order? Also, can you convert the below InstallError to a conflict?
Hello Adam, thank you for educating Spack to me, and your patience with this PR! Regarding the FC issue, it seems to be deeper (inside of Spack, and beyond my scope). I believe the "edit" stage can be entirely removed from the LIBXSMM package specification. An |
| variant('header-only', default=False, | ||
| description='Produce header-only installation') | ||
| # variant +header-only is only available since v1.6.2 | ||
| conflicts('+header-only', when='@:1.6.2') |
There was a problem hiding this comment.
conflicts actually has an optional msg attribute that you can set to this comment if you want.
Can you tell me more about this? That definitely shouldn't be the case. If you delete your P.S. If you're curious how our autodetection works, you can check out |
Ok, I ran the auto-detection and |
|
A login for your system would definitely help. Things I would like to look at:
|
|
P.S. Feel free to open a new issue to track this discussion. That will probably get more attention than a side comment on a PR. |
* Included LIBXSMM 1.8 into the list of available versions. * LIBXSMM 1.8.1 * LIBXSMM 1.8.2 (release notes: https://github.com/hfp/libxsmm/releases/tag/1.8.2). * LIBXSMM: Use join_path instead of hard-coding the separator. Install "version.txt" into the documentation directory. Removed installing "README.md" from LIBXSMM's root directory as it overrides README.md provided there. The latter uses correct relative references to the other documentation parts. Note: Apparently, "FC=/path/to/gfortran spack install libxsmm" is currently needed for Spack since does not pick-up the Fortran compiler (but incorrectly uses the C compiler instead). * LIBXSMM: converted Package into MakefilePackage (to address spack#6896 (comment)). * LIBXSMM: account for changed file set in 1.8.2 onward (addresses spack#6896 (review)). * Fixed incorrect behavior of "+header-only", which did not install the "src" folder. Addressed spack#6896 (comment) and spack#6896 (comment). * Use conflicts msg argument to present a friendly error message.
3 participants
Introduces LIBXSMM 1.8.2 (release notes: https://github.com/hfp/libxsmm/releases/tag/1.8.2).
Package adjustments: use join_path instead of hard-coding the separator. Install "version.txt" into the documentation directory (doc). Removed installing "README.md" (from LIBXSMM's root directory) as it overrides the README.md already provided in the documentation folder. The latter uses the correct relative references to the other parts of the documentation.
Note: Apparently, "FC=/path/to/gfortran spack install libxsmm" is currently needed for Spack since does not pick-up the Fortran compiler (but incorrectly uses the C compiler instead).