[WIP] fmi4cpp: add new package #47240
Conversation
…ps://github.com/feelpp/spack into 6-libzip-links-with-external-libs-found-on-host
…ound-on-host bug libzip: links with external libs found on host and new version
…ound-on-host rm space and rename variant bz2 to bzip2
spackbot-app
bot
added
dependencies
new-package
new-variant
new-version
update-package
labels
| variant("bzip2", default=True, description="Enable bzip2 support") | ||
| variant("lzma", default=True, description="Enable lzma support") | ||
| variant("openssl", default=True, description="Enable openssl support") | ||
| variant("zstd", default=True, description="Enable zstd support") |
There was a problem hiding this comment.
Some initial questions:
fmi4cpp/package.py has: depends_on("libzip")
So fmi4cpp does not need all those variants enabled by default, correct?
I moved the remaining questions to the PR where those changes should only be: #47230 (review)
There was a problem hiding this comment.
@bernhardkaindl I set the same defaults as in the cmake of libzip.
please the PR for libzip is actually here #47230 could you please that one instead. I made the mistake to merge the libzip branch to the develop branch on feelpp/spack ( I needed it for some other work but now it spreads through the other PR I made from feelpp/spack
There was a problem hiding this comment.
@bernhardkaindl all the variant are enabled by default by libzip so if they are not installed by spack, libzip will have a look on the host system and use that. That's what happened to me and it created havock in my apps at linking stage because of conflicting libs. the previous versions of libzip spack package were not very good from this point of view.
I checked up to version 1.7 of libzip and all the variants are enabled:
https://github.com/nih-at/libzip/blob/v1.7.1/CMakeLists.txt#L10
| self.define_from_variant("ENABLE_GNUTLS", "gnutls"), | ||
| self.define_from_variant("ENABLE_MBEDTLS", "mbedtls"), | ||
| self.define_from_variant("ENABLE_OPENSSL", "openssl"), | ||
| self.define_from_variant("ENABLE_BZIP2", "bz2"), |
There was a problem hiding this comment.
This variant does not exist. Its current name is "bzip2":
==> Installing libzip-1.11.1-5aupui6t6j4ve3d6i3ffgslk3okyxkcf [28/29]
==> Error: KeyError: '"bz2" is not a variant of "libzip"'It looks you should uninstall these recipes and install them from source to be sure to catch such errors before you submit a PR, I think.
There was a problem hiding this comment.
yeah ok may be I was too fast to rename bz2 to bzip2 and didn't cheeck it now it is fixed in #47230
There was a problem hiding this comment.
I'd just let this PR wait for now.
(you can just close it, and re-open it again before you update it) until #47230 is merged.
It looks like can just remove the changes to libzip as this package does not depend on the new variants, which are the only change that is in that PR AFAICS.
So I wonder: Why you even add those variants?
There was a problem hiding this comment.
fmi4cpp doesn't care about the variants of libzip, I agree. However it is, IMHO, necessary to add them to libzip and properly define the relationship with them otherwise we get libzip depends from the host libs and from spack libs that libzip depend upon.
bernhardkaindl
changed the title
I also pushed the changes for libzip from #47230, if this PR is merged then remains only the changes for fmi4cpp