-
Notifications
You must be signed in to change notification settings - Fork 988
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
Qbs: Added qbs test lib api and fixed handling spaces #16382
Conversation
Also add a simple static lib functional test as well as api to create libs.
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.
Thanks for your contribution!
The CI tests are broken, because by default it assumes qbs
is installed:
pytest.fail("Required '{}' tool version '{}' is not available".format(tool_name,
> version_msg))
E Failed: Required 'qbs' tool version 'Any' is not available
It would be necessary to add a qbs
entry to the default conftest.py
so it doesn't run by default.
conan/internal/api/new/qbs_lib.py
Outdated
@@ -0,0 +1,46 @@ | |||
from conan.internal.api.new.cmake_lib import source_cpp, source_h |
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.
Nice to have a qbs template 👍
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.
Great, some notes for future, but this can be merged for next release, thanks for your contribution!
qbs.profile = "" | ||
qbs_config = {"products.{{name}}.isShared": "true" if self.options.shared else "false"} |
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 can probably be made default inside Qbs
, but can be done in a later PR. The best would be to aim to something as CMake
with the defaults of automatically managing self.options.shared
if existing.
|
||
class {{package_name}}TestConan(ConanFile): | ||
settings = "os", "compiler", "build_type", "arch" | ||
generators = "PkgConfigDeps" |
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 guess we want to move this later to QbsDeps
, isn't it? later PR too.
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 was thinking about it, but that creates a chicken-egg problem in tests - we will test QbsDeps using QbsDeps, is it OK?
I think, pkgconfig is fine here - it works fine in simple cases like this. Also, we don't invoke the pkg-config binary from Qbs (we simply read pc files and create modules, Qbs handles the rest). So no external deps here.
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.
The conan new
templates aim to generate the "canonical" and most recommended practice. If the QbsDeps
is the generator that you prefer users to use, then it should be in the conan new
template, and documented as such.
It is fine to test QbsDeps
with the conan new
template, as it is actually the closest to the user flow, so very recommended to test it with it.
Also add a simple static lib functional test as well as api to create libs.
Changelog: Feature: Add new
qbs_lib
template for theconan new
command.Changelog: Fix: Fix handling spaces in paths in
Qbs
helper.Docs: Omit
develop
branch, documenting this one.