bzip2: modernize more #13703
bzip2: modernize more #13703
Conversation
833473e to
6ec1be8
Compare
This comment has been minimized.
This comment has been minimized.
- simplify CMakeLists - test custom variables of FindBZip2 in test package - protect deletion of fPIC option - use export_conandata_patches - use can_run in test package
6ec1be8 to
187bb6d
Compare
This comment has been minimized.
This comment has been minimized.
|
I detected other pull requests that are modifying bzip2/all recipe: This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there. |
| bin_path = os.path.join(self.package_folder, "bin") | ||
| self.output.info("Appending PATH environment variable: {}".format(bin_path)) | ||
| self.env_info.PATH.append(bin_path) | ||
| if Version(conan_version).major < 2: |
There was a problem hiding this comment.
| if Version(conan_version).major < 2: |
There was a problem hiding this comment.
@jwillikers @toge Since you are also helping to opening/reviewing PRs -- please mark sure this is not being used ❤️
There was a problem hiding this comment.
Why? It's a better way than tones of TODO to track conan v1 stuff which are unused in conan v2 client. Code is better than comments.
There was a problem hiding this comment.
Conan 2.0 is still beta, which means, not totally stable. It will accept env_info soon, so it should work for both versions. Also, adding an if adds more complexity, okay a recipe is small, but still, a new condition for something that should work is too much.
There was a problem hiding this comment.
@uilianries can we avoid conditional code to check for v1/v2 from the recipe code? IMO, it should be possible to write recipe compatible with both v1 and v2 simultaneously, using only some common sub-set of v1/v2 features. if it's problematic, may v2 provide some dummy/stub objects to make migration process easier?
There was a problem hiding this comment.
@SSE4 Yes, it's possible to avoid conditional code. We have added new issues for Conan v2, keeping compatibility, or at least some stub. env_info is one case, which will be supported on beta-5, but without any practical effect, only to keep Conan v1 working with v2.
There was a problem hiding this comment.
@prince-chrismc What about its use in the header-only template? https://github.com/conan-io/conan-center-index/blob/master/docs/package_templates/header_only/all/conanfile.py
There was a problem hiding this comment.
Removing it . Thanks for pointing it out
|
duplicate of #13667 |
* bzip2: Conan V2 improvments Use export patches method. Safely delete fPIC option. Use Python F-Strings. * Remove redundant package name from topics * Modernize the CMakeLists.txt file * Fix spacing * Use can_run in test package * Incorporate changes from #13703 * More fixes * Remove use of conan_version * Update recipes/bzip2/all/test_package/CMakeLists.txt Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com> Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
* bzip2: Conan V2 improvments Use export patches method. Safely delete fPIC option. Use Python F-Strings. * Remove redundant package name from topics * Modernize the CMakeLists.txt file * Fix spacing * Use can_run in test package * Incorporate changes from conan-io#13703 * More fixes * Remove use of conan_version * Update recipes/bzip2/all/test_package/CMakeLists.txt Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com> Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
Specify library name and version: lib/1.0
This is also a good place to share with all of us why you are submitting this PR (specially if it is a new addition to ConanCenter): is it a dependency of other libraries you want to package? Are you the author of the library? Thanks!