Add cpputest/4.0 #4366
Add cpputest/4.0 #4366
Conversation
ERROR: cpputest/4.0: Error in source() method, line 48
tools.get(**self.conan_data["sources"][self.version])
TypeError: get() argument after ** must be a mapping, not list
This comment has been minimized.
This comment has been minimized.
|
This is work-in-progress. The test package builds, but there are many, many options in cpputest that need to be added here. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Sorry for the number of build errors... I've changed my instructions to make sure that in future I build with |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
recipes/cpputest/all/conanfile.py
Outdated
| self._cmake.definitions["COVERAGE"] = self.options.coverage | ||
| self._cmake.definitions["TESTS"] = self.options.tests |
There was a problem hiding this comment.
I do not think consumer from CCI need this to be tested or produce coverage results...
There was a problem hiding this comment.
Thanks. I think I've addressed this in a43fa98 - is that what you meant?
| tools.rmdir(os.path.join(self.package_folder, "lib", "pkgconfig")) | ||
| tools.rmdir(os.path.join(self.package_folder, "lib", "CppUTest", "cmake")) | ||
|
|
||
| def package_info(self): |
There was a problem hiding this comment.
You need to explicitly set the pkg_config names
There was a problem hiding this comment.
I'm not exactly sure that I understand that, but I have improved code in this area in e8ee75a.
If that's not what you mean, please can I have a bit more info?
There was a problem hiding this comment.
You do not need file name ... the default is names
There was a problem hiding this comment.
| def package_info(self): | |
| Self.cpp_info.names["pkg_config"] = "whatever the .pc file is that's removed" |
Via mobile
| conan_basic_setup() | ||
|
|
||
| add_executable(${PROJECT_NAME} test_package.cpp) | ||
| target_link_libraries(${PROJECT_NAME} ${CONAN_LIBS}) |
There was a problem hiding this comment.
Since there are CMake names it's ideal if you test those here
There was a problem hiding this comment.
Thanks. Please could you clarify what I need to do?
(Like much of the code in this PR, I took this from the corresponding file in the szip project that's recommended as a good sample, in the How to provide a good recipe section in the docs on adding new recipes...)
There was a problem hiding this comment.
Welcome to the CCI club 🤝
I am suggestion adding the find_package and link to the cmake targets called out in the package info.
There was a problem hiding this comment.
Thank you ever so much - this was really helpful to learn about.
I think I have fixed it in e8ee75a.
Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
Addressing code review feedback.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Note for reviewers: My remaining concerns are around the implementations of some of the options - I think that the CppUTest library has a bunch of logic in their CMake around the options that I am not reproducing in I've asked for feedback on the Slack for this library, and will update here, if there's any reply - although it seems to be quite quiet. I'm thinking there's two main choices:
I would appreciate knowing the preferences of reviewers between 1 and 2. |
This comment has been minimized.
This comment has been minimized.
|
I vote 1 I prefer to keep it simple now and update when asked.Many packages that we have only offer part of options and people don't ask about new options. |
Thanks... I've pushed that change. I left in the empty Please do so if you would prefer it removed... Update - now removed, thanks to feedback from @prince-chrismc |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Just a quick update - all the tasks in my checklist - above are now either done, or rejected as no longer relevant.... I've made all the changes requested so far in code review - thanks - and so, as far as I know, this is now ready for a final review... :-) 🎉 🤞 |
Co-authored-by: Uilian Ries <uilianries@gmail.com>
|
All green in build 33 (
|
Specify library name and version: cpputest/4.0
See #4365
conan-center hook activated.