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

Add RuntimeQml Package #13407

Merged
merged 20 commits into from
Nov 2, 2022
Merged

Add RuntimeQml Package #13407

merged 20 commits into from
Nov 2, 2022

Conversation

Nomalah
Copy link
Contributor

@Nomalah Nomalah commented Oct 11, 2022

Specify library name and version: runtimeqml/latest

Requesting to add: RuntimeQml

This is a very useful tool when developing qml based applications, integrating with conan will enable projects to build with less configuration.


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the conan-center hook activated.

@CLAassistant
Copy link

CLAassistant commented Oct 11, 2022

CLA assistant check
All committers have signed the CLA.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@uilianries
Copy link
Member

It depends on conan-io/conan-docker-tools#466

@Nomalah
Copy link
Contributor Author

Nomalah commented Oct 11, 2022

It depends on conan-io/conan-docker-tools#466

This is my first contribution - It seems like the build failed because the conancenter binaries for Qt don't include ones that have QML enabled. Is there a way to build those or does this PR block building them?

Also - should qt be set to shared as default? It makes a lot of sense for this package, but I might misunderstand.

@uilianries
Copy link
Member

This is my first contribution - It seems like the build failed because the conancenter binaries for Qt don't include ones that have QML enabled. Is there a way to build those or does this PR block building them?

No. Only shared, fPIC and header_only are configured, any custom option only consumes its default value. More info: /~https://github.com/conan-io/conan-center-index/blob/3f4ce2ebdf9ed24cf12b27de6358e63a30eb9a0b/docs/packaging_policy.md#options

Also - should qt be set to shared as default? It makes a lot of sense for this package, but I might misunderstand.

No, there are many considerations like license and compatibility for ANY plaform, usually, static works better. Please, read #10096

@uilianries
Copy link
Member

In case you package requires options which are not available in Conan Center, they should be validated under valitate() and raise ConanInvalidConfiguration in case is not available, for instance:

def validate(self):
    foo = self.dependencies["foo"]
    if not foo.options.bar:
        raise ConanInvalidConfiguration(f"{self.ref} requires option foo:bar=True")

In the worst case, you will generate 0 packages, but your recipe will be available in Conan Center.

@conan-center-bot

This comment has been minimized.

@Nomalah
Copy link
Contributor Author

Nomalah commented Oct 13, 2022

Blocked by #13439 and #12906

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@Nomalah
Copy link
Contributor Author

Nomalah commented Oct 18, 2022

@uilianries Any idea on why the linting CI is failing?

@conan-center-bot

This comment has been minimized.

@uilianries uilianries closed this Oct 18, 2022
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@Nomalah Nomalah requested review from uilianries and removed request for jcar87 October 27, 2022 20:19
uilianries
uilianries previously approved these changes Oct 28, 2022
cmake_layout(self)

def requirements(self):
if Version(self.version) <= "cci.20211220":
Copy link
Contributor

Choose a reason for hiding this comment

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

This works 😱 did not even know

Copy link
Member

Choose a reason for hiding this comment

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

as we don't use that weird american format, is much easier compare using year first. After all, it's only char value comparison

Copy link
Contributor

Choose a reason for hiding this comment

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

weird american format

Heck at least I get two birthday 😆 🍰

Copy link
Contributor

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

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

Overall looks really good, just two small nits

I'd also encourage sharing the cmake build script upstream for others

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline

All green in build 20 (3999a6ec5b969c335db620ba4f01a373fb926e31):

  • runtimeqml/cci.20211220@:
    All packages built successfully! (All logs)

  • runtimeqml/cci.20220923@:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit 74ebd7f into conan-io:master Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants