conan-io / hooks

Official Conan client hooks
MIT License
32 stars 46 forks source link

add KB-H078 in conan-center to test that system libs are all lowercase if host OS is Windows #480

Open SpaceIm opened 1 year ago

SpaceIm commented 1 year ago

closes https://github.com/conan-io/hooks/issues/479

uilianries commented 1 year ago

It could be moved to pre_export, as you are actually checking system_libs only. Early failure is better than needing to wait all build step first.

SpaceIm commented 1 year ago

It could be moved to pre_export, as you are actually checking system_libs only. Early failure is better than needing to wait all build step first.

It can't work, cpp_info.system_libs is not populated in conanfile object at pre_export time, so you would have to parse conanfile and understand branching and all the logic (with maybe lot of indirection). Moreover some recipes populate cpp_info.system_libs from a file created at package time (like abseil).

uilianries commented 1 year ago

It can't work, cpp_info.system_libs is not populated in conanfile object at pre_export time, so you would have to parse conanfile and understand branching and all the logic (with maybe lot of indirection). Moreover some recipes populate cpp_info.system_libs from a file created at package time (like abseil).

Makes sense, parsing the file content is too much.

uilianries commented 1 year ago

Doing a quick search on master branch, some recipes need to be updated before merging this hook, or it will fail for any new PR:

find recipes/ -name conanfile.py -exec grep system_libs {} + | grep -e '[A-Z]'
proj/all/conanfile.py:                self.cpp_info.components["projlib"].system_libs.append("Ole32")
sentry-native/all/conanfile.py:                self.cpp_info.system_libs.append("Version")
qt/5.x.x/conanfile.py:                    self.cpp_info.components["qtWidgets"].system_libs.append("UxTheme")
glu/all/conanfile.py:            self.cpp_info.system_libs = ["Glu32"]
taocpp-taopq/all/conanfile.py:            self.cpp_info.components["_taocpp-taopq"].system_libs = ["Ws2_32"]
openal/all/conanfile.py:            self.cpp_info.system_libs.extend(["winmm", "ole32", "shell32", "User32"])
baical-p7/all/conanfile.py:            self.cpp_info.components["p7"].system_libs .append("Ws2_32")
aeron/all/conanfile.py:            self.cpp_info.system_libs = ["wsock32", "ws2_32", "Iphlpapi"]
ixwebsocket/all/conanfile.py:                self.cpp_info.system_libs.append("Crypt32")

On the other hand, the hook could be changed to warning level.

SpaceIm commented 1 year ago

Number of recipes is reasonable. They can be fixed quickly. I've already submited a PR to fix baical-p7 & glu.

SpaceIm commented 1 year ago

I've submitted PRs for recipes you've listed. There are 3 other recipes to fix: c-client, ffmpeg, cyclonedds. I've also submitted PRs for them.

SpaceIm commented 1 year ago

@uilianries @RubenRBS I think that now, the impact on existing CCI recipes is quite limited, most of them have been fixed. There is still qt & sentry-native, and I have a PR for each of them. I've also seen a strange recipe archicad-apidevkit with uppercase windows system libs (but also propagated if macOS, so weird). Anyway it's a straightforward fix for contributors.