-
Notifications
You must be signed in to change notification settings - Fork 764
{lib}[GCCcore/14.3.0] spdlog-1.15.3 #24391
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -5,6 +5,7 @@ version = '1.15.3' | |||||
|
|
||||||
| homepage = 'https://github.com/gabime/spdlog' | ||||||
| description = "Very fast, header-only/compiled, C++ logging library." | ||||||
|
|
||||||
| toolchain = {'name': 'GCCcore', 'version': '14.3.0'} | ||||||
| toolchainopts = {'pic': True} | ||||||
|
|
||||||
|
|
@@ -17,15 +18,13 @@ builddependencies = [ | |||||
| ('CMake', '4.0.3'), | ||||||
| ] | ||||||
|
|
||||||
| _shared_configopts = " ".join([ | ||||||
| "-DSPDLOG_BUILD_SHARED=ON", | ||||||
| "-DSPDLOG_BUILD_PIC=ON", | ||||||
| ]) | ||||||
| _base_configopts = ' -DSPDLOG_BUILD_TESTS=ON' | ||||||
| configopts = [f'{_base_configopts} -DSPDLOG_BUILD_SHARED={x}' for x in ('OFF', 'ON')] | ||||||
|
|
||||||
| configopts = ["", _shared_configopts] | ||||||
| runtest = 'test' | ||||||
|
|
||||||
| sanity_check_paths = { | ||||||
| 'files': ['include/%(name)s/%(name)s.h', 'lib/lib%(name)s.a', f'lib/lib%(name)s.{SHLIB_EXT}'], | ||||||
| 'files': [f'include/{name}/{name}.h', 'lib/libspdlog.a', 'lib/libspdlog.%s' % SHLIB_EXT], | ||||||
|
||||||
| 'files': [f'include/{name}/{name}.h', 'lib/libspdlog.a', 'lib/libspdlog.%s' % SHLIB_EXT], | |
| 'files': ['include/%(name)s/%(name)s.h', 'lib/lib%(name)s.a', f'lib/lib%(name)s.{SHLIB_EXT}'], |
%(name)sare EB templates. I do not expect it to ever be different fromnamein the EC but i would keep it as such w.r.t other ECsSHLIB_EXTis also an EB variable that will be injected in the EC. with python linting it appears undefined but it will be replaced when the EC is loaded
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.
My understanding was the %(foo)s is better replaced with f'{foo}' as it is easier to read. Did I get that wrong?
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.
They are not equivalent. What i was referring to is the % used to place SHLIB_EXT in the string.
%(name)s is not a string formatting but an EB template that is replaced by EB when reading the EasyConfig.
The ones under https://docs.easybuild.io/version-specific/easyconfig-templates/#template-namesvalues-as-set-in-easyconfig are probably the only trivial ones that will be equivalent to the value in the EC
EDIT: for context we already have some {name} in EB but templates are the majority
(easybuild-dev) crivella@crivella-desktop:~/Documents/GIT/easybuild-easyconfigs $ grep -l -R '{name}' * | wc -l
124
(easybuild-dev) crivella@crivella-desktop:~/Documents/GIT/easybuild-easyconfigs $ grep -l -R '%(name)s' * | wc -l
2249
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.
Would keep consistent usage of f-strings without re-introducing old % formatting.
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.
Well, I done my PR without being aware of what you already done @Crivella . So that is why I had already the f-string for the bit I knew how to do it, but I was not sure how to do it for the SHLIB_EXT bit. So now that is consistent, thanks fo your help.
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.
Not sure if we want to remove
SPDLOG_BUILD_PIC=ONThere 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 is doing what we are already doing in the CMake EB when setting the
fpicTC option to truehttps://github.com/gabime/spdlog/blob/486b55554f11c9cccc913e11a87085b2a91f706f/CMakeLists.txt#L149-L151
But i would still leave it there to ensure that we follow what the source CMake dictates in case they add more stuff in the future
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.
I was not aware of the other PR so mine was based on a different 'template' and thus, I did not remove it, I simply never added it. I have put that back, thanks for finding this!