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

qpoases/3.2.1 #3962

Merged
merged 8 commits into from
Jan 5, 2021
Merged

qpoases/3.2.1 #3962

merged 8 commits into from
Jan 5, 2021

Conversation

blackliner
Copy link
Contributor

@blackliner blackliner commented Dec 20, 2020

Specify library name and version: qpoases/3.2.1

I got 3 open points/questions to the community:

  • The library has only one STATIC target, so I want to disable shared. Is the way I did this correct? Bonus: Do I need fPIC at all?
    options = {
        "fPIC": [True, False],
        "shared": [False],
    }
  • The current library exports a target called qpOASES (no leading qpOASES::). This target is not available with the conan package anymore, so existing consumers have to rewrite their CML, right?
  • I had to add a component named qpOASES::qpOASES to make it all work, any ideas why? SOLVED: self.cpp_info.libs = tools.collect_libs(self) was missing

The important stuff:

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

@conan-center-bot
Copy link
Collaborator

Sorry, the build is only launched for Early Access Program users. You can request access writing in this issue.

@conan-center-bot
Copy link
Collaborator

Sorry, the build is only launched for Early Access Program users. You can request access writing in this issue.

prince-chrismc
prince-chrismc previously approved these changes Dec 21, 2020
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.

You certainly should not need components, just the names is enough.

We have to have a namespace for cmake but feel free to add a FIXME comment so we do not forget!

Also you sent cmake version to 3.10 by convenience we sent to 3.1 but that's not an issue.

On mobile but LGTM! Amazing work

recipes/qpoases/all/conanfile.py Outdated Show resolved Hide resolved
@blackliner
Copy link
Contributor Author

You certainly should not need components, just the names is enough.

I learned some more during yesterdays port of ACADE, self.cpp_info.libs = tools.collect_libs(self) was missing 👍

We have to have a namespace for cmake but feel free to add a FIXME comment so we do not forget!

If you can tell me what this means or point me to an example, I can try and add it.

Also you sent cmake version to 3.10 by convenience we sent to 3.1 but that's not an issue.

done

On mobile but LGTM! Amazing work

Thanks 😄

@conan-center-bot
Copy link
Collaborator

Sorry, the build is only launched for Early Access Program users. You can request access writing in this issue.

@madebr
Copy link
Contributor

madebr commented Dec 21, 2020

I think the Makefile is an afterthought.
INSTALL.txt tells to use the makefiles.

The project has some Makefiles: make_linux.mk, make_osx.mk and make_windows.mk.
Do these work?

The makefiles contain references to BLAS, LAPACK and libhsl.
Are these potential dependencies?

Copy link
Member

@uilianries uilianries left a 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, only a few points to be checked.

recipes/qpoases/all/conanfile.py Outdated Show resolved Hide resolved
recipes/qpoases/all/conanfile.py Outdated Show resolved Hide resolved
recipes/qpoases/all/conanfile.py Outdated Show resolved Hide resolved
recipes/qpoases/all/conanfile.py Show resolved Hide resolved
Co-authored-by: Uilian Ries <uilianries@gmail.com>
@conan-center-bot
Copy link
Collaborator

Failure in build 4 (67d243628fe2da64fbfb4435500c777d65a4f35f):

  • Error processing recipe (ref 'qpoases/3.2.1'): Linux x86_64, Release, gcc 4.9, libstdc++ . Options: qpoases:shared-True
    Unexpected error happened:
ERROR: qpoases/3.2.1: 'True' is not a valid 'options.shared' value.
Possible values are ['False']

@blackliner
Copy link
Contributor Author

I think the Makefile is an afterthought.
INSTALL.txt tells to use the makefiles.

The project has some Makefiles: make_linux.mk, make_osx.mk and make_windows.mk.
Do these work?

The makefiles contain references to BLAS, LAPACK and libhsl.
Are these potential dependencies?

I do not know if the INSTALL.txt is really up to date. The lib is not under active development anymore, but needed for another one: https://github.com/acado/acado Both are from the same dev.

I only ever used the CML, and it works without any obvious dependencies (no find_package nor target_link_libraries) .

@conan-center-bot
Copy link
Collaborator

Failure in build 5 (dbee32ca7aa0686f499df7d4ea41fa209501133f):

  • Error processing recipe (ref 'qpoases/3.2.1'): Linux x86_64, Release, gcc 4.9, libstdc++ . Options: qpoases:shared-True
    Unexpected error happened:
ERROR: qpoases/3.2.1: 'True' is not a valid 'options.shared' value.
Possible values are ['False']

@blackliner blackliner mentioned this pull request Dec 22, 2020
4 tasks
@conan-center-bot
Copy link
Collaborator

Some configurations of 'qpoases/3.2.1' failed in build 6 (b0c2480760e9ac98bdc102bd7d39754b6b191800):

Co-authored-by: Anonymous Maarten <madebr@users.noreply.github.com>
@conan-center-bot
Copy link
Collaborator

Some configurations of 'qpoases/3.2.1' failed in build 7 (3e8086f4444734f77bc24ae34c7a252bb459319f):

Co-authored-by: Anonymous Maarten <madebr@users.noreply.github.com>
@conan-center-bot
Copy link
Collaborator

Some configurations of 'qpoases/3.2.1' failed in build 8 (286ca5a5b462543a9c495856a10f2f239d80faf3):

@conan-center-bot
Copy link
Collaborator

All green in build 9 (7f49423c4feb91356b975e45ba18303f60aea7d5)! 😊

Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

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

LGTM

@conan-center-bot conan-center-bot merged commit 8cbc1ce into conan-io:master Jan 5, 2021
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.

6 participants