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.

Sign up for GitHub

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

Jump to bottom

feat: add replxx/0.0.2 #968

Merged
merged 9 commits into from
Mar 24, 2020
Merged

feat: add replxx/0.0.2 #968

merged 9 commits into from
Mar 24, 2020

Conversation

gocarlos
Copy link
Contributor

Specify library name and version: replxx/0.0.2

  • 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

Some configurations of 'replxx/0.0.2' failed in build 1 (a6011916d7b2c8db04270f6b9ef920a2fb384ae5):

@conan-center-bot
Copy link
Collaborator

Some configurations of 'replxx/0.0.2' failed in build 2 (69da0c4256a49f628468f5832ec4eff8ce87593b):

madebr
madebr reviewed Feb 25, 2020
View reviewed changes
recipes/replxx/all/conanfile.py Outdated Show resolved Hide resolved
recipes/replxx/all/conanfile.py Show resolved Hide resolved
recipes/replxx/all/test_package/CMakeLists.txt Outdated Show resolved Hide resolved
recipes/replxx/all/conanfile.py Show resolved Hide resolved
recipes/replxx/all/conanfile.py Show resolved Hide resolved
@conan-center-bot
Copy link
Collaborator

Some configurations of 'replxx/0.0.2' failed in build 3 (f3884d647ea35ae5885b1a2882abf156d60f86cd):

@conan-center-bot
Copy link
Collaborator

Some configurations of 'replxx/0.0.2' failed in build 4 (0a7cb516164ec74ac45d33fcb8e532b8f21baaff):

@conan-center-bot
Copy link
Collaborator

Some configurations of 'replxx/0.0.2' failed in build 5 (b5d48f0a1e5b75f98e445b65749c3d64e105198d):

@gocarlos gocarlos mentioned this pull request Feb 25, 2020
Open
9 tasks
@gocarlos
Copy link
Contributor Author

@madebr do you know why the pipeline is failing?

@madebr
Copy link
Contributor

madebr commented Feb 26, 2020

@gocarlos
On Windows, when building a shared library, functions and data need to be annotated using __declspec(dllexport) and __declspec(dllimport) (use google for the why).
Some libraries assume they are built as libraries by default and will prepend the functions with __declspec(dllimport) when.
Most often, some preprocessor variable such as REPLXX_STATIC will override this to nothing (which is wanted when building static libraries).

And indeed,
https://github.com/AmokHuginnsson/replxx/blob/22acff044991084922bfdee5046da7b746f9ccf3/include/replxx.hxx#L47
Adding REPLXX_STATIC to self.cpp_info.defines in package_info should do the trick.

@gocarlos
Copy link
Contributor Author

@gocarlos
On Windows, when building a shared library, functions and data need to be annotated using __declspec(dllexport) and __declspec(dllimport) (use google for the why).
Some libraries assume they are built as libraries by default and will prepend the functions with __declspec(dllimport) when.
Most often, some preprocessor variable such as REPLXX_STATIC will override this to nothing (which is wanted when building static libraries).

And indeed,
https://github.com/AmokHuginnsson/replxx/blob/22acff044991084922bfdee5046da7b746f9ccf3/include/replxx.hxx#L47
Adding REPLXX_STATIC to self.cpp_info.defines in package_info should do the trick.

should this not be part of the build script? -> should be transparent to conan ?
https://github.com/AmokHuginnsson/replxx/blob/ae65cae141fbdc9d96ac17564b0cb81543c9eaab/CMakeLists.txt#L80

@gocarlos gocarlos mentioned this pull request Feb 26, 2020
Open
@conan-center-bot
Copy link
Collaborator

All green in build 6 (392823e2dce9998ef8ef289daaecd53ffa80192b)! 😊

madebr
madebr reviewed Feb 26, 2020
View reviewed changes
recipes/replxx/all/conanfile.py Outdated Show resolved Hide resolved
recipes/replxx/all/conanfile.py Outdated Show resolved Hide resolved
recipes/replxx/all/test_package/conanfile.py Outdated Show resolved Hide resolved
recipes/replxx/all/test_package/CMakeLists.txt Show resolved Hide resolved
@gocarlos gocarlos marked this pull request as ready for review February 26, 2020 19:50
@conan-center-bot
Copy link
Collaborator

Some configurations of 'replxx/0.0.2' failed in build 7 (afea7430eef7ec37e1089cf90c956a4fd4534b86):

@conan-center-bot
Copy link
Collaborator

All green in build 8 (7428d707342e68c1dbe2143e00e77c59fd73eb57)! 😊

@madebr madebr mentioned this pull request Feb 28, 2020
Closed
@conan-center-bot
Copy link
Collaborator

All green in build 9 (7428d707342e68c1dbe2143e00e77c59fd73eb57)! 😊

uilianries
uilianries requested changes Mar 5, 2020
View reviewed changes
recipes/replxx/all/conanfile.py Outdated Show resolved Hide resolved
recipes/replxx/all/conanfile.py Outdated Show resolved Hide resolved
recipes/replxx/all/conanfile.py Outdated Show resolved Hide resolved
recipes/replxx/all/conanfile.py Outdated Show resolved Hide resolved
recipes/replxx/all/conanfile.py Outdated Show resolved Hide resolved
gocarlos and others added 6 commits March 6, 2020 12:54
@gocarlos
feat: add replxx
5e32448
@gocarlos
chore: fix path to code
6fd5b77
@gocarlos
Apply suggestions from code review
6df831f 
Co-Authored-By: Anonymous Maarten <madebr@users.noreply.github.com>

fix: replxx conan file

fix: replxx indentation
@gocarlos
fix: add static
c58fa02
@gocarlos
chore: madebr feedback
d711326
@gocarlos
fix: missing import
223697b
@conan-center-bot
Copy link
Collaborator

All green in build 10 (223697bed898b53e22d17da06ea2061d0b94260c)! 😊

gocarlos and others added 2 commits March 7, 2020 22:06
@gocarlos @uilianries
Apply suggestions from code review
75f31fb 
Co-Authored-By: Uilian Ries <uilianries@gmail.com>
@gocarlos @uilianries
Update recipes/replxx/all/conanfile.py
68aa8db 
Co-Authored-By: Uilian Ries <uilianries@gmail.com>
@conan-center-bot
Copy link
Collaborator

All green in build 12 (68aa8db95fc6d4cac3344cb869844928ecaeb833)! 😊

@uilianries uilianries requested a review from SSE4 March 9, 2020 20:12
SSE4
SSE4 previously approved these changes Mar 16, 2020
View reviewed changes
@conan-center-bot
Copy link
Collaborator

All green in build 13 (cefa7a36358e0fbe02186f4e34ac6bbe5e681a98)! 😊

uilianries
uilianries approved these changes Mar 23, 2020
View reviewed changes
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. Thanks @gocarlos !

@danimtb danimtb self-assigned this Mar 24, 2020
@danimtb danimtb merged commit e7b7fbf into conan-io:master Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@madebr madebr madebr left review comments

@danimtb danimtb danimtb approved these changes

@uilianries uilianries uilianries approved these changes

@SSE4 SSE4 SSE4 left review comments

Assignees

@danimtb danimtb

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@gocarlos @conan-center-bot @madebr @SSE4 @uilianries @danimtb