-
Notifications
You must be signed in to change notification settings - Fork 991
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
Add example test for forwards compat of custom setting #17524
base: develop2
Are you sure you want to change the base?
Conversation
available_libc_versions = conanfile.settings.libc_version.possible_values() | ||
ret = [] | ||
for possible_libc_version in available_libc_versions: | ||
if Version(possible_libc_version) >= Version(libc_version): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the other way round. Packages created with an older libc can be used with the current libc_version. So better <=
How is it solved with compatibility.py? Like this: | ||
""" | ||
settings_user_file = textwrap.dedent(""" | ||
libc_version: [1, 2, 3, 4, 5] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reduce the number of versions, a bit faster the test.
tc.save({ | ||
"dep/conanfile.py": GenConanfile("dep", "1.0") | ||
.with_settings("compiler", "libc_version"), | ||
"conanfile.py": GenConanfile("app", "1.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be dropped, and tested with conan install --requires=xxxx
|
||
def compatibility(conanfile): | ||
configs = cppstd_compat(conanfile) | ||
libc_configs = libc_compat(conanfile) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are not doing the cross-product of the configs, probably the cpstd_compat
can be dropped for simplicity, and just put the code in compatibility.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add the cross-product, even if it's just to have an example how to do it (But maybe it's better to have that directly in the compatibility.py
extension docs?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree a more complete example in the docs would be nice, so lets have the test first, then write the docs from the test.
Changelog: Omit
Docs: Omit
As part of the answer for #17523, I thought that a test that reflects the desired behaviour would be great to have, as a reference point for the future
Update: Now seeing the linked issue, OP was not exactly following this approach, but I think it's still worth it to add something like this