Closed samuel-emrys closed 1 year ago
Hi @samuel-emrys
Thanks very much for reporting.
Indeed, there is a gap here, the cmake_config_version_compat
works only when defined in the dependency, not when overriden from the downstream consumer, because the idea is that this doesn't make sense to do it in the consumer, because the one knowing and declaring the compatibility is the package itself, not the consumer. If the consumer is going to change the compatibility, why not directly changing the version that it is requiring to lie inside the original compatibility policy?
Said that, I think there is no reason to be inconsistent and having some properties that cannot be overriden from downstream, no need to discuss the semantics of each property, so lets fix this for next release.
because the idea is that this doesn't make sense to do it in the consumer, because the one knowing and declaring the compatibility is the package itself, not the consumer. If the consumer is going to change the compatibility, why not directly changing the version that it is requiring to lie inside the original compatibility policy?
Without making comment on whether it should be dictated by the consumer, this issue was prompted when packaging up the ensmallen library. They use the syntax to specify a minimum compatible version (see the commit context): https://github.com/mlpack/ensmallen/commit/1208c62381b6f64f5feccc32a978730a0b9f9fd2#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aR60
find_package(Armadillo 9.800.0 REQUIRED)
Having said that, I think that there is a difference between the versioning policy upstream and what the consumer says they are compatible with. Upstream, assuming semver, a major version bump means "the public api has had a breaking change, in some way", which might look like a bump from 1.x to 2.x. On the consumer side, there's every possibility that the nature of the breaking change had no impact on their compatibility with the library because the breaking change was not part of their usage of the library. In that case, they may be comfortable saying, "we think we're compatible with anything newer than 0.1.1", and then revising that as a breaking change that does impact their usage comes out.
This will also mean that when packaging libraries, I won't need to do things like this:
replace_in_file(
self,
os.path.join(self.source_folder, "CMakeLists.txt"),
"find_package(Armadillo 9.800.0 REQUIRED)",
"find_package(Armadillo REQUIRED)",
)
And can instead just
def generate(self):
deps = CMakeDeps(self)
deps.set_property("armadillo", "cmake_config_version_compat", "AnyNewerVersion")
deps.generate()
Anyway, just wanted to provide some additional context. I'm glad it will be in the next release :)
Environment details
Steps to reproduce
conan new cmake_lib -d name=foo -d version=0.1.0
+find_package(armadillo 9.800.0)
add_library(foo src/foo.cpp) target_include_directories(foo PUBLIC include)
+target_link_libraries(foo armadillo::armadillo)
set_target_properties(foo PROPERTIES PUBLIC_HEADER "include/foo.h") install(TARGETS foo) diff --git a/conanfile.py b/conanfile.py index 1f8e43a..e6310ec 100644 --- a/conanfile.py +++ b/conanfile.py @@ -33,8 +33,12 @@ class fooRecipe(ConanFile): def layout(self): cmake_layout(self)
def generate(self): deps = CMakeDeps(self)
Logs