-
-
Notifications
You must be signed in to change notification settings - Fork 773
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
build: Bring in panda3d-thirdparty repo as thirdparty-scripts #1599
base: master
Are you sure you want to change the base?
Conversation
2cd5290
to
c40b8d3
Compare
Thanks so much for working on this! We should consider whether we really want to include the Pmw sources, or whether we should fetch them from a separate repository. I am leaning towards the latter, rather than include them with the Panda3D sources. As discussed in Discord, I would prefer keeping the scripts in the thirdparty directory (or a subdirectory thereof), or to put them in cmake/thirdparty, or something like that, rather than a separate top-level directory. |
dtool/src/prc/CMakeLists.txt
Outdated
@@ -92,6 +92,10 @@ target_include_directories(p3prc PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY | |||
target_link_libraries(p3prc p3dtool PKG::OPENSSL) | |||
target_interrogate(p3prc ALL EXTENSIONS ${P3PRC_IGATEEXT}) | |||
|
|||
if(HAVE_OPENSSL AND WIN32) | |||
target_link_libraries(p3prc ws2_32.lib crypt32.lib) |
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.
I think there's a better way to add indirect dependencies in CMake, so that any time PKG::OPENSSL
is referenced, these are picked up automatically.
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.
The .lib
-suffixes for the system libraries won't work with MinGW.
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.
I copied this from another location (p3express maybe?); I'll look into fixing this properly and updating that other location while I'm at it.
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.
I believe this is what OpenSSL:applink
(https://cmake.org/cmake/help/latest/module/FindOpenSSL.html) is for. However, it is only available on CMake 3.18+. Ubuntu 20.04 only has 3.16. We could probably do some guards to make use of this feature -- and thus require 3.18 -- only on Windows. We already do this for macOS, which requires 3.16 for ObjC (our current minimum is 3.13).
ead163f
to
98a339a
Compare
3.1.5 is needed to fix MSVC14 and MSVC 2022
NOTE: CG has been disabled on macOS builds for now The CG dependency is going away soon so its now worth the time to figure out how to get things working with macOS arm64 (if we even can).
These are using "focal" now and we probably do not need the version in the profile name (we don't for he macOS nor the Windows profiles).
8943005
to
7f03939
Compare
0e0e424
to
a752bd3
Compare
This allows us to continue using "AL/al.h" as the OpenAL include
We are not as concerned about controlling the version on these, and the ones available in the Ubuntu packages are probably more up-to-date with security patches.
a752bd3
to
b4d3867
Compare
This has linking errors when doing cmake builds, and cmake builds did not previously include fcollada.
The patch command steps are not working on rebuild. The job results are still cached.
Issue description
Solution description
Checklist
I have done my best to ensure that…