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 tinyobjloader recipe #58

Merged
merged 15 commits into from
Sep 23, 2019
Merged

Add tinyobjloader recipe #58

merged 15 commits into from
Sep 23, 2019

Conversation

flostellbrink
Copy link
Contributor

Specify library name and version: tinyobjloader/1.0.6, tinyobjloader/2.0-rc1

  • 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 Sep 17, 2019

CLA assistant check
All committers have signed the CLA.

recipes/tinyobjloader/1.x/conanfile.py Outdated Show resolved Hide resolved
recipes/tinyobjloader/1.x/conanfile.py Outdated Show resolved Hide resolved
recipes/tinyobjloader/1.x/conanfile.py Outdated Show resolved Hide resolved
recipes/tinyobjloader/config.yml Outdated Show resolved Hide resolved
recipes/tinyobjloader/config.yml Outdated Show resolved Hide resolved
@@ -0,0 +1,51 @@
import os
Copy link
Member

Choose a reason for hiding this comment

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

I see no difference between 1.x and 2.x maybe we can re-use the same recipe for both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only difference between 1.x and 2.x is the API, so the test case is different.
Is there a way to deduplicate the rest of the code?

Copy link
Member

Choose a reason for hiding this comment

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

I need to check if we can customize the test_package name. Let me see it.

Copy link
Member

Choose a reason for hiding this comment

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

unfortunately we don't have such feature, but we can re-use the same test_package folder for both tests. Using cmake features we can detect the package reference and build the correct test. Lemme check some good approach.

@uilianries
Copy link
Member

BTW the CI is under maintenance right now, that's why we don't have CI status on this PR.

@flostellbrink
Copy link
Contributor Author

Good to know

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

@flostellbrink
Copy link
Contributor Author

@uilianries Any idea why the bot won't build the pull request?
I kind of thought that your thumbs up on my request (#4 (comment)_) was a confirmation that I got in.

@uilianries
Copy link
Member

@flostellbrink unfortunately I have no power to add users in beta group. We need to wait for admin, which will be done soon.

@flostellbrink
Copy link
Contributor Author

Ahh okay, thanks for all the help!

@conan-center-bot
Copy link
Collaborator

The config.yml file is invalid: Only one library can be changed in the same PR: [tinyobjloader/1.x, tinyobjloader/2.x]

@danimtb
Copy link
Member

danimtb commented Sep 18, 2019

Please @flostellbrink, include just one library version per PR. This reduces a lot the build service time and pipeline and helps with the revision of the new files 😄

@uilianries
Copy link
Member

@danimtb I think we can use both versions with same recipe, I'm checking some way to do it.

@flostellbrink
Copy link
Contributor Author

@danimtb I failed at combining the test cases.
I think it might be useful to pass the version to the test case conanfile.
Another nice approach would be something like the config.yml to map versions to test cases.

@uilianries Please keep me updated. I'd be perfectly happy with splitting this into two pull requests if combining them doesn't work out :)

Signed-off-by: Uilian Ries <[email protected]>
@uilianries
Copy link
Member

Use unique test package for recipe
@flostellbrink
Copy link
Contributor Author

That's clever!

@conan-center-bot
Copy link
Collaborator

tinyobjloader/1.0.6@:

'conan export' command failed:

ERROR: Conanfile not found at /tmp/c3ipr/info_pr_58_3/conanfile.py

@conan-center-bot
Copy link
Collaborator

Some configurations of 'tinyobjloader/1.0.6' have failed:

  • Linux x86_64, Release, gcc 5, libstdc++ . Options: tinyobjloader:shared:True
    • Hooks errors detected:
      • [HOOK - conan-center.py] post_package(): ERROR: [SHARED ARTIFACTS (KB-H015)] Package with 'shared' option did not contains any shared artifact (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H015)
    • Log download
    • Log error Download
    • Profile
  • Access to all the logs

@flostellbrink
Copy link
Contributor Author

Huh, I thought that was a windows only issue.
Gonna look into it later.

@uilianries
Copy link
Member

@flostellbrink you need to add the follow statement:

cmake.definitions["TINYOBJLOADER_COMPILATION_SHARED"] = self.options.shared

@flostellbrink
Copy link
Contributor Author

Well that was painfully obvious in hindsight. Thanks a lot @uilianries !

@conan-center-bot
Copy link
Collaborator

Some configurations of 'tinyobjloader/1.0.6' have failed:

@conan-center-bot
Copy link
Collaborator

An unexpected error happened and has been reported. Help is on its way! 🏇

@uilianries
Copy link
Member

uilianries commented Sep 19, 2019

Seems like Jenkins is in trouble. I gonna restart the CI job by closing/opening your PR.

@uilianries uilianries closed this Sep 19, 2019
@uilianries uilianries reopened this Sep 19, 2019
@uilianries uilianries closed this Sep 19, 2019
@uilianries uilianries reopened this Sep 19, 2019
@conan-center-bot
Copy link
Collaborator

All green! 😊

@Croydon
Copy link
Contributor

Croydon commented Sep 19, 2019

I'm wondering if having pre-releases is a desirable goal for the CCI? 🤔

@uilianries
Copy link
Member

I'm wondering if having pre-releases is a desirable goal for the CCI? thinking

Good question. Some time ago we agreed in accept only stable versions. WDYT @lasote @danimtb ?

@danimtb
Copy link
Member

danimtb commented Sep 19, 2019

I'd say it is not the goal for this repository to have release candidates... But we need to agree on the conditions and post them as guidelines for new contributions

@flostellbrink
Copy link
Contributor Author

There are some in between versions too. 1.0.7 and 1.4.1, but they are not marked as latest release on GitHub and have a partially changed API.

So I was hesitant to include them.

I'll remove the pre-release and we can wait for a proper release.

@conan-center-bot
Copy link
Collaborator

All green! 😊

@lasote lasote merged commit d127300 into conan-io:master Sep 23, 2019
artem-kamyshev pushed a commit to artem-kamyshev/conan-center-index that referenced this pull request Sep 28, 2020
datalogics-robb pushed a commit to datalogics-robb/conan-center-index that referenced this pull request Aug 29, 2023
Merge in changes from conan-io/master
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