Skip to content
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

bzip2: modernize more #13703

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 29 additions & 37 deletions recipes/bzip2/all/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,44 +1,36 @@
cmake_minimum_required(VERSION 3.4)
project(bzip2 C)
project(bzip2 LANGUAGES C)

include(GNUInstallDirs)

if(MSVC OR MSVC90 OR MSVC10)
set(MSVC ON)
set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON)
endif()

set(SOURCE_SUBFOLDER ${CMAKE_CURRENT_SOURCE_DIR}/src)
set(BZ2_LIBRARY bz2)

option(BZ2_BUILD_EXE ON)

set(BZ2_TARGETS ${BZ2_LIBRARY})

add_library(${BZ2_LIBRARY} ${SOURCE_SUBFOLDER}/blocksort.c
${SOURCE_SUBFOLDER}/bzlib.c
${SOURCE_SUBFOLDER}/compress.c
${SOURCE_SUBFOLDER}/crctable.c
${SOURCE_SUBFOLDER}/decompress.c
${SOURCE_SUBFOLDER}/huffman.c
${SOURCE_SUBFOLDER}/randtable.c
${SOURCE_SUBFOLDER}/bzlib.h
${SOURCE_SUBFOLDER}/bzlib_private.h)
target_include_directories(${BZ2_LIBRARY} PRIVATE ${SOURCE_SUBFOLDER})
option(BZ2_BUILD_EXE "Build bzip2 utility" ON)

add_library(bz2
${BZ2_SRC_DIR}/blocksort.c
${BZ2_SRC_DIR}/bzlib.c
${BZ2_SRC_DIR}/compress.c
${BZ2_SRC_DIR}/crctable.c
${BZ2_SRC_DIR}/decompress.c
${BZ2_SRC_DIR}/huffman.c
${BZ2_SRC_DIR}/randtable.c
)
target_include_directories(bz2 PUBLIC ${BZ2_SRC_DIR})
set_target_properties(bz2 PROPERTIES
VERSION ${BZ2_VERSION_STRING}
SOVERSION ${BZ2_VERSION_MAJOR}
WINDOWS_EXPORT_ALL_SYMBOLS ON
)

install(
TARGETS bz2
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
)
install(FILES ${BZ2_SRC_DIR}/bzlib.h DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})

if(BZ2_BUILD_EXE)
add_executable(${CMAKE_PROJECT_NAME} ${SOURCE_SUBFOLDER}/bzip2.c)
target_link_libraries(${CMAKE_PROJECT_NAME} ${BZ2_LIBRARY})
target_include_directories(${CMAKE_PROJECT_NAME} PRIVATE ${SOURCE_SUBFOLDER})
list(APPEND BZ2_TARGETS ${CMAKE_PROJECT_NAME})
add_executable(bzip2 ${BZ2_SRC_DIR}/bzip2.c)
target_link_libraries(bzip2 PRIVATE bz2)
install(TARGETS bzip2 DESTINATION ${CMAKE_INSTALL_BINDIR})
endif()

set_target_properties(${BZ2_LIBRARY} PROPERTIES VERSION ${BZ2_VERSION_STRING} SOVERSION ${BZ2_VERSION_MAJOR})

install(TARGETS ${BZ2_TARGETS}
BUNDLE DESTINATION ${CMAKE_INSTALL_BINDIR}
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})

install(FILES ${SOURCE_SUBFOLDER}/bzlib.h DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
44 changes: 23 additions & 21 deletions recipes/bzip2/all/conanfile.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
from conan import ConanFile
from conan import ConanFile, conan_version
from conan.tools.cmake import CMake, CMakeToolchain, cmake_layout
from conan.tools.files import apply_conandata_patches, copy, get, save
from conan.tools.files import apply_conandata_patches, copy, export_conandata_patches, get, save
from conan.tools.scm import Version
import os
import textwrap

required_conan_version = ">=1.46.0"
required_conan_version = ">=1.52.0"


class Bzip2Conan(ConanFile):
Expand All @@ -29,25 +30,27 @@ class Bzip2Conan(ConanFile):

def export_sources(self):
copy(self, "CMakeLists.txt", self.recipe_folder, self.export_sources_folder)
for p in self.conan_data.get("patches", {}).get(self.version, []):
copy(self, p["patch_file"], self.recipe_folder, self.export_sources_folder)
export_conandata_patches(self)

def config_options(self):
if self.settings.os == "Windows":
del self.options.fPIC
self.license = "bzip2-{}".format(self.version)
self.license = f"bzip2-{self.version}"

def configure(self):
if self.options.shared:
del self.options.fPIC
try:
del self.options.fPIC
except Exception:
pass
try:
del self.settings.compiler.libcxx
del self.settings.compiler.libcxx
except Exception:
pass
pass
try:
del self.settings.compiler.cppstd
del self.settings.compiler.cppstd
except Exception:
pass
pass

def layout(self):
cmake_layout(self, src_folder="src")
Expand All @@ -58,8 +61,9 @@ def source(self):

def generate(self):
tc = CMakeToolchain(self)
tc.variables["BZ2_SRC_DIR"] = self.source_folder.replace("\\", "/")
tc.variables["BZ2_VERSION_STRING"] = self.version
tc.variables["BZ2_VERSION_MAJOR"] = str(self.version).split(".")[0]
tc.variables["BZ2_VERSION_MAJOR"] = Version(self.version).major
tc.variables["BZ2_BUILD_EXE"] = self.options.build_executable
tc.generate()

Expand Down Expand Up @@ -98,7 +102,7 @@ def _create_cmake_module_variables(self, module_file):

@property
def _module_file_rel_path(self):
return os.path.join("lib", "cmake", "conan-official-{}-variables.cmake".format(self.name))
return os.path.join("lib", "cmake", f"conan-official-{self.name}-variables.cmake")

def package_info(self):
self.cpp_info.set_property("cmake_find_mode", "both")
Expand All @@ -107,11 +111,9 @@ def package_info(self):
self.cpp_info.set_property("cmake_build_modules", [self._module_file_rel_path])
self.cpp_info.libs = ["bz2"]

self.cpp_info.names["cmake_find_package"] = "BZip2"
self.cpp_info.names["cmake_find_package_multi"] = "BZip2"
self.cpp_info.build_modules["cmake_find_package"] = [self._module_file_rel_path]

if self.options.build_executable:
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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if Version(conan_version).major < 2:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jwillikers @toge Since you are also helping to opening/reviewing PRs -- please mark sure this is not being used ❤️

Copy link
Contributor Author

@SpaceIm SpaceIm Oct 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#13754

Removing it . Thanks for pointing it out

self.cpp_info.names["cmake_find_package"] = "BZip2"
self.cpp_info.names["cmake_find_package_multi"] = "BZip2"
self.cpp_info.build_modules["cmake_find_package"] = [self._module_file_rel_path]
if self.options.build_executable:
self.env_info.PATH.append(os.path.join(self.package_folder, "bin"))
24 changes: 18 additions & 6 deletions recipes/bzip2/all/test_package/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,24 @@ cmake_minimum_required(VERSION 3.1)
project(test_package LANGUAGES C)

find_package(BZip2 REQUIRED)
message("BZIP2_FOUND: ${BZIP2_FOUND}")
message("BZIP2_NEED_PREFIX: ${BZIP2_NEED_PREFIX}")
message("BZIP2_INCLUDE_DIRS: ${BZIP2_INCLUDE_DIRS}")
message("BZIP2_INCLUDE_DIR: ${BZIP2_INCLUDE_DIR}")
message("BZIP2_LIBRARIES: ${BZIP2_LIBRARIES}")
message("BZIP2_VERSION_STRING: ${BZIP2_VERSION_STRING}")

add_executable(test_package test_package.c)
target_link_libraries(test_package PRIVATE BZip2::BZip2)

# Test whether variables from https://cmake.org/cmake/help/latest/module/FindBZip2.html
# are properly defined in conan generators
set(_custom_vars
BZIP2_FOUND
BZIP2_NEED_PREFIX
BZIP2_INCLUDE_DIRS
BZIP2_INCLUDE_DIR
BZIP2_LIBRARIES
BZIP2_VERSION_STRING
)
foreach(_custom_var ${_custom_vars})
if(DEFINED _custom_var)
message(STATUS "${_custom_var}: ${${_custom_var}}")
else()
message(FATAL_ERROR "${_custom_var} not defined")
endif()
endforeach()
11 changes: 6 additions & 5 deletions recipes/bzip2/all/test_package/conanfile.py
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@
from conan import ConanFile
from conan.tools.build import cross_building
from conan.tools.build import can_run
from conan.tools.cmake import CMake, cmake_layout
import os


class TestPackageConan(ConanFile):
settings = "os", "arch", "compiler", "build_type"
generators = "CMakeToolchain", "CMakeDeps", "VirtualRunEnv"

def requirements(self):
self.requires(self.tested_reference_str)
test_type = "explicit"

def layout(self):
cmake_layout(self)

def requirements(self):
self.requires(self.tested_reference_str)

def build(self):
cmake = CMake(self)
cmake.configure()
cmake.build()

def test(self):
if not cross_building(self):
if can_run(self):
bin_path = os.path.join(self.cpp.build.bindirs[0], "test_package")
self.run(bin_path, env="conanrun")
14 changes: 3 additions & 11 deletions recipes/bzip2/all/test_v1_package/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,16 +1,8 @@
cmake_minimum_required(VERSION 3.1)
project(test_package LANGUAGES C)
project(test_package)

include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup(TARGETS)

find_package(BZip2 REQUIRED)
message("BZIP2_FOUND: ${BZIP2_FOUND}")
message("BZIP2_NEED_PREFIX: ${BZIP2_NEED_PREFIX}")
message("BZIP2_INCLUDE_DIRS: ${BZIP2_INCLUDE_DIRS}")
message("BZIP2_INCLUDE_DIR: ${BZIP2_INCLUDE_DIR}")
message("BZIP2_LIBRARIES: ${BZIP2_LIBRARIES}")
message("BZIP2_VERSION_STRING: ${BZIP2_VERSION_STRING}")

add_executable(test_package ../test_package/test_package.c)
target_link_libraries(test_package PRIVATE BZip2::BZip2)
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/../test_package
${CMAKE_CURRENT_BINARY_DIR}/test_package)
3 changes: 1 addition & 2 deletions recipes/bzip2/all/test_v1_package/conanfile.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# pylint: skip-file
from conans import ConanFile, CMake, tools
import os

Expand All @@ -15,4 +14,4 @@ def build(self):
def test(self):
if not tools.cross_building(self):
bin_path = os.path.join("bin", "test_package")
self.run("%s --help" % bin_path, run_environment=True)
self.run(bin_path, run_environment=True)