-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Migrate/openscenegraph #23704
Migrate/openscenegraph #23704
Conversation
I detected other pull requests that are modifying openscenegraph/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. |
Don't understand why I get this:
|
This comment has been minimized.
This comment has been minimized.
I'll regenerate the missing binaries for you and retrigger the PR CI once that's done, sorry about the confussion! |
Also, seems like Conan 2 did have a proper error compiling on macos (The bot just reported the Conan 1 results in this case) Summary of Conan 2: |
Yes, indeed. I don't have Mac to test on but I hope I managed to analyze the issue. If I'm correct this patch should solve it: It's a bit of a rats nest with the old cmake code in OSG and all the patches 😆 |
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit 8d49342openscenegraph/3.6.5@#95634cec2858a17131bae33555b4eb76
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Seems to be some issue with long paths on the
(and more) Any ideas on have to handle this? |
You removed |
Oh, yes. Thanks :) |
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit 0d09b4copenscenegraph/3.6.5@#1eaa17b51514e8ebe0c40bba1449b531
|
d90eb2e
to
68128c2
Compare
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit 68128c2openscenegraph/3.6.5@#529453612c17c03d1f512bf5dcba4073
|
Let me know if there's anything more I'm expected to do. |
Hi @anton-danielsson thanks a lot! We have now reviewed this PR and it looks great. We'll delay the merging for a few days while we figure some last minute things out, but nothing you need to act upon, we'll let you know once this is ready, thanks a lot for taking the time to open the PR, we appreciate it :) |
recipes/openscenegraph/all/patches/0009-replace-auto-ptr-in-plugins.patch
Outdated
Show resolved
Hide resolved
|
||
apply_conandata_patches(self) | ||
|
||
# Not sure why, but CMake fails to find the EXPAT::EXPAT target created by Conan when Fontconfig is found as a module. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably worth opening an issue for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To OSG you mean? It looks like it's pretty much dead, last commit to master was over two years ago.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I thought maybe it was an issue with the Expat Conan package. Maybe its not though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. Not really looked into it.
This patch and comment comes from the original PR that I based this PR on (https://github.com/conan-io/conan-center-index/pull/21355/files#diff-75b9f888a7159f587df53d73476654254af05ca90d777c2525335ec2d2c520d2R271-R273)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ran into this recently as well, it's because expat defines find mode to use expat
and module mode to use EXPAT
. If we had proper find_package aliases, we could use this unmodified. In my case I did a replace_in_file
, find_package(EXPAT)
-> find_package(EXPAT MODULE)
. Not sure if a similar fix would work here, but maybe that info is helpful.
…ugins.patch Co-authored-by: Jordan Williams <jordan@jwillikers.com>
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit a0fd973openscenegraph/3.6.5@#2cf144596f53456153fcd8de679fabfd
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions to reduce conflicts when merged
Conan v1 pipeline ✔️All green in build 17 (
Conan v2 pipeline ✔️
All green in build 17 (
|
Specify library name and version: openscenegraph/3.6.5
Trying to revive #21355
This PR is rebase of the changes and a fix (hopefully) to the Windows link issue.
The issue was that patching incorrectly rewrote this line:
https://github.com/openscenegraph/OpenSceneGraph/blob/master/src/osgPlugins/CMakeLists.txt#L10
which made the export/import logic broken for non-shared builds, see for example:
https://github.com/openscenegraph/OpenSceneGraph/blob/master/include/osgText/Export#L31
@valgur If you want to merge your PR instead you can cherry pick this commit:
38bd556