-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
libxsmm: add libs property #6193
Conversation
result = find_libraries(['libxsmm', 'libxsmmf'], root=self.prefix, | ||
recurse=True) | ||
result += find_libraries(['libxsmm', 'libxsmmf'], root=self.prefix, | ||
shared=False, recurse=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do u need both shared and static?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's because the library can be built with shared and/or static versions -- what's the best way to handle this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the best way to handle this?
Different packages do it differently, some have shared
variant which is used in build system and in libs()
, some just return shared always.
But it certainly strange to return both shared and static together here. You can at least try searching for static if shared were not found.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now I'd add a shared
variant that defaults true, and return either shared libs or static depending on what it's set to.
@alalazo: this is probably another place where the interface could come in handy. The libs you want probably depend on whether the caller is built shared. Might be worth thinking about how to integrate that after SC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth thinking about how to integrate that after SC.
Definitely. Probably we also need to decide on a solution for the problem raised in #5269 that works for every use case we have. Lately I am not pushing new features because I am waiting for the release to be tagged, and don't want to add noise to that.
@baip I think adding a variant:
variant('shared', default=True, description="Build shared libraries")
as @tgamblin suggested, and then having:
@property
def libs(self):
shared = True if '+shared' in self.spec else False
return find_libraries(
['libxsmm', 'libxsmmf'], root=self.prefix, shared=shared, recurse=True
)
is more along the lines of what we do in other packages, at least for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please return either shared or static in libs()
.
This allows dependents to use
spec['libxsmm'].libs
.