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

qwt 6.1.6 #5887

Merged
merged 20 commits into from
Jun 17, 2021
Merged

qwt 6.1.6 #5887

merged 20 commits into from
Jun 17, 2021

Conversation

ericLemanissier
Copy link
Contributor

Specify library name and version: qwt/6.1.6

all the work has be done by @boussaffawalid , kudos to him !

closes #4528


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

boussaffawalid and others added 18 commits February 10, 2021 12:49
Co-authored-by: ericLemanissier <ericLemanissier@users.noreply.github.com>
Co-authored-by: ericLemanissier <ericLemanissier@users.noreply.github.com>
Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
Co-authored-by: ericLemanissier <ericLemanissier@users.noreply.github.com>
Co-authored-by: ericLemanissier <ericLemanissier@users.noreply.github.com>
Co-authored-by: ericLemanissier <ericLemanissier@users.noreply.github.com>
Co-authored-by: ericLemanissier <ericLemanissier@users.noreply.github.com>
@ghost
Copy link

ghost commented Jun 14, 2021

I detected other pull requests that are modifying qwt/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@conan-center-bot

This comment has been minimized.

Comment on lines 91 to 95
self.run("{} && qmake {}".format(vcvars, self._source_subfolder), run_environment=True)
self.run("{} && nmake".format(vcvars))
else:
self.run("qmake {}".format(self._source_subfolder), run_environment=True)
self.run("make -j {}".format(tools.cpu_count()))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it robust?

What about MinGW?
Cross compilation? (qt is not in build requirements, I guess qmake comes from qt recipe).

Copy link
Contributor

Choose a reason for hiding this comment

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

yes you are right: qmake comes from qt recipe.
Can a recipe be both a requirement and a build_requirement at the same time ?

Copy link
Contributor

@SpaceIm SpaceIm Jun 14, 2021

Choose a reason for hiding this comment

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

yes, it works like a charm with 2 profiles. It also works with 1 profile, but it breaks quickly when versions are different in requirements and build requirements.

AFAIR, conan plans to change 1 profile so that it behaves like 2 profiles.

Until it's implemented, you can do:

def requirements(self):
    self.requires("qt/5.15.2")

def build_requirements(self):
    if tools.cross_building(self.settings):
        self.build_requires("qt/5.15.2")

Copy link
Contributor Author

@ericLemanissier ericLemanissier Jun 14, 2021

Choose a reason for hiding this comment

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

qmake is always intended to run on the build platform, so it should be fine without this change right ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Conan v2 will always work as if there were two profiles defined. Here in CCI we are now adding our first cross-building scenario (we are fighting with M1) and the plan is to start using two profiles for all the builds (for native ones) at some point in time. We need to check that they work as expected, specially regarding the computation of package IDs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's required to have things like MinGW and cross-compilation in the first iteration.
having recipe what works only for native builds is still better than having no recipes, and it would probably satisfy ~90% of users.
things like MinGW and cross-building could be added on demand.

jgsogo
jgsogo previously approved these changes Jun 14, 2021
@SSE4 SSE4 requested a review from uilianries June 15, 2021 08:41
SSE4
SSE4 previously approved these changes Jun 15, 2021
@@ -0,0 +1,4 @@
sources:
"6.1.6":
url: "https://sourceforge.net/projects/qwt/files/qwt/6.1.6/qwt-6.1.6.zip"
Copy link
Contributor

Choose a reason for hiding this comment

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

tar.gz?

recipes/qwt/all/test_package/CMakeLists.txt Show resolved Hide resolved
"thermometers, wheels and knobs to control or display values, arrays, or ranges of type double."
)
settings = "os", "compiler", "build_type", "arch"
options = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
@ericLemanissier ericLemanissier dismissed stale reviews from SSE4 and jgsogo via 126f9c6 June 15, 2021 19:13
@conan-center-bot

This comment has been minimized.

prince-chrismc
prince-chrismc previously approved these changes Jun 15, 2021
@prince-chrismc
Copy link
Contributor

expanded command line ... too long

RIP I've never seen that 🤯

@boussaffawalid
Copy link
Contributor

probbaly it is better to use Ninja instead of nmake, also it may make sense to enable ninja response file. (CMAKE_NINJA_FORCE_RESPONSE_FILE)

@conan-center-bot
Copy link
Collaborator

All green in build 3 (ff0457c95d8fea23a4d48c5638d7d3c3795592dd):

  • qwt/6.1.6@:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit a2feaf7 into conan-io:master Jun 17, 2021
@ericLemanissier ericLemanissier deleted the patch-2 branch June 17, 2021 08:08
madebr pushed a commit to madebr/conan-center-index that referenced this pull request Jun 21, 2021
* added recipe for qwt  6.1.6

* Update recipes/qwt/all/conanfile.py

Co-authored-by: ericLemanissier <ericLemanissier@users.noreply.github.com>

* Update recipes/qwt/all/conanfile.py

Co-authored-by: ericLemanissier <ericLemanissier@users.noreply.github.com>

* fixed conan test

* Update recipes/qwt/all/conanfile.py

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

* Update recipes/qwt/all/conanfile.py

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

* del self.options.fPIC for test package

* del self.options.fPIC for test package

* use get_safe to check fPIC option

* set target_compile_options to -fPIC

* fix debug lib name for non windows system

* Update recipes/qwt/all/conanfile.py

Co-authored-by: ericLemanissier <ericLemanissier@users.noreply.github.com>

* Update recipes/qwt/all/test_package/CMakeLists.txt

Co-authored-by: ericLemanissier <ericLemanissier@users.noreply.github.com>

* Revert "Update recipes/qwt/all/test_package/CMakeLists.txt"

This reverts commit 7aa4fc3.

* Update recipes/qwt/all/conanfile.py

Co-authored-by: ericLemanissier <ericLemanissier@users.noreply.github.com>

* Update recipes/qwt/all/conanfile.py

Co-authored-by: ericLemanissier <ericLemanissier@users.noreply.github.com>

* specify required_conan_version

* Apply suggestions from code review

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>

* use jom instead of nmake

Co-authored-by: Walid Boussaffa <boussaffa.walid@outlook.com>
Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
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.

7 participants