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

[magnum/magnum-plugins] Fix patches for --head builds #10158

Merged
merged 5 commits into from
Mar 28, 2020

Conversation

Squareys
Copy link
Contributor

Hi all!

I updated the magnum and magnum-plugins portfiles to work with current HEAD.
As per usual the maintainer of Magnum, @mosra, will quickly review and approve the PR.

Best,
Jonathan

Signed-off-by: Squareys <squareys@googlemail.com>
Since basisu port has been updated, we can now use it without patching
the magnum-plugins code for compatibility.

Signed-off-by: Squareys <squareys@googlemail.com>
Signed-off-by: Squareys <squareys@googlemail.com>
Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

Please update version info of these ports, see documentation.

ports/magnum/portfile.cmake Outdated Show resolved Hide resolved
ports/magnum/portfile.cmake Outdated Show resolved Hide resolved
Signed-off-by: Squareys <squareys@googlemail.com>
@Squareys
Copy link
Contributor Author

@JackBoosY Thank you for the review!

I made changes accordingly. Please have another look.

@JackBoosY
Copy link
Contributor

@Squareys Don't forget to update version info : )
Documentation.

@Squareys
Copy link
Contributor Author

@JackBoosY Ah, right, thanks! 🙏

Done with latest commit.

@JackBoosY
Copy link
Contributor

And magnum-plugins version update?

Signed-off-by: Squareys <squareys@googlemail.com>
@Squareys
Copy link
Contributor Author

Yes, sorry, @mosra just pinged me on that, too. Totally forgot I had changed that aswell 🙄

@JackBoosY
Copy link
Contributor

LGTM, we need to test all features.

@JackBoosY
Copy link
Contributor

Build failure with magnum[al-info,anyimageimporter,anyaudioimporter,anyimageconverter,anysceneimporter,audio,debugtools,distancefieldconverter,fontconverter,gl,gl-info,glfwapplication,imageconverter,magnumfont,magnumfontconverter,meshtools,objimporter,tgaimageconverter,opengltester,primitives,sdl2application,scenegraph,shaders,text,texturetools,tgaimporter,trade,wavaudioimporter,windowlesswglapplication,eglcontext,cglcontext,glxcontext,wglcontext,windowlesseglapplication,eglcontext,windowlessglxapplication]:x86-windows:

CMake Error at src/Magnum/Platform/CMakeLists.txt:515 (message):
  X11 library, required by some applications, was not found.  Set
  WITH_*X*APPLICATION to OFF to skip building them.

Looks like you forgot to pass in the following parameters:

CORRADE_TARGET_UNIX          - Defined if compiled for some Unix flavor
CORRADE_TARGET_APPLE         - Defined if compiled for Apple platforms
CORRADE_TARGET_IOS           - Defined if compiled for iOS (device or
CORRADE_TARGET_IOS_SIMULATOR - Defined if compiled for iOS Simulator
CORRADE_TARGET_WINDOWS       - Defined if compiled for Windows
CORRADE_TARGET_WINDOWS_RT    - Defined if compiled for Windows RT
CORRADE_TARGET_EMSCRIPTEN    - Defined if compiled for Emscripten
CORRADE_TARGET_ANDROID       - Defined if compiled for Android

@Squareys
Copy link
Contributor Author

Squareys commented Feb 25, 2020

@JackBoosY The magnum port has platform dependent features, e.g. windowlessglxapplication. If you build [*], then all are included. We have an open PR on that mosra/magnum#368, but didn't come to a consensus on how this should behave. aparently I lost track of the discussion there and we did come to a conclusion. Will have a look at that.
CC @mosra

@mosra
Copy link

mosra commented Feb 25, 2020

The inability to use [*] is a separate problem that needs a dedicated solution from our side and due to time constraints we weren't able to resolve that yet. @JackBoosY Would it be possible to merge this as-is?

The build should work on Windows if you install magnum[al-info,anyimageimporter,anyaudioimporter,anyimageconverter,anysceneimporter,audio,debugtools,distancefieldconverter,fontconverter,gl,gl-info,glfwapplication,imageconverter,magnumfont,magnumfontconverter,meshtools,objimporter,tgaimageconverter,opengltester,primitives,sdl2application,scenegraph,shaders,text,texturetools,tgaimporter,trade,wavaudioimporter,windowlesswglapplication,wglcontext] (i.e., with eglcontext, cglcontext, glxcontext, windowlesscglapplication, windowlesseglapplication, windowlessglxapplication, which are Linux- and macOS-specific, omitted).

@JackBoosY
Copy link
Contributor

JackBoosY commented Feb 26, 2020

@Squareys If some features are not supported with some triplets, you should add error message in portfile.cmake before vcpkg_configure_cmake.
Such as:

if ("eglcontext" IN_LIST FEATURES AND VCPKG_TARGET_IS_WINDOWS)
    message(FATAL_ERROR "Feature eglcontext only support UNIX")
endif()

When I building magnum[al-info,anyimageimporter,anyaudioimporter,anyimageconverter,anysceneimporter,audio,debugtools,distancefieldconverter,fontconverter,gl,gl-info,glfwapplication,imageconverter,magnumfont,magnumfontconverter,meshtools,objimporter,tgaimageconverter,opengltester,primitives,sdl2application,scenegraph,shaders,text,texturetools,tgaimporter,trade,wavaudioimporter,windowlesswglapplication,wglcontext]:x64-windows-static, some errors appeared:

OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveOutGetNumDevs referenced in function "void __cdecl `anonymous namespace'::ProbePlaybackDevices(void)" (?ProbePlaybackDevices@?A0xac419535@@YAXXZ)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveOutGetDevCapsW referenced in function "void __cdecl `anonymous namespace'::ProbePlaybackDevices(void)" (?ProbePlaybackDevices@?A0xac419535@@YAXXZ)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveOutOpen referenced in function "public: virtual void __cdecl `anonymous namespace'::WinMMPlayback::open(char const *)" (?open@WinMMPlayback@?A0xac419535@@UEAAXPEBD@Z)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveOutClose referenced in function "public: virtual __cdecl `anonymous namespace'::WinMMPlayback::~WinMMPlayback(void)" (??1WinMMPlayback@?A0xac419535@@UEAA@XZ)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveOutPrepareHeader referenced in function "public: void __cdecl <lambda_d66888fbdd47c4c5fd45f92954f71f52>::operator()(struct wavehdr_tag &)const " (??R<lambda_d66888fbdd47c4c5fd45f92954f71f52>@@QEBAXAEAUwavehdr_tag@@@Z)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveOutUnprepareHeader referenced in function "public: void __cdecl <lambda_d29467fa517e0290f1e0915d3ad04a3d>::operator()(struct wavehdr_tag &)const " (??R<lambda_d29467fa517e0290f1e0915d3ad04a3d>@@QEBAXAEAUwavehdr_tag@@@Z)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveOutWrite referenced in function "public: int __cdecl `anonymous namespace'::WinMMPlayback::mixerProc(void)" (?mixerProc@WinMMPlayback@?A0xac419535@@QEAAHXZ)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveInGetNumDevs referenced in function "void __cdecl `anonymous namespace'::ProbeCaptureDevices(void)" (?ProbeCaptureDevices@?A0xac419535@@YAXXZ)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveInGetDevCapsW referenced in function "void __cdecl `anonymous namespace'::ProbeCaptureDevices(void)" (?ProbeCaptureDevices@?A0xac419535@@YAXXZ)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveInOpen referenced in function "public: virtual void __cdecl `anonymous namespace'::WinMMCapture::open(char const *)" (?open@WinMMCapture@?A0xac419535@@UEAAXPEBD@Z)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveInClose referenced in function "public: virtual __cdecl `anonymous namespace'::WinMMCapture::~WinMMCapture(void)" (??1WinMMCapture@?A0xac419535@@UEAA@XZ)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveInPrepareHeader referenced in function "public: virtual bool __cdecl `anonymous namespace'::WinMMCapture::start(void)" (?start@WinMMCapture@?A0xac419535@@UEAA_NXZ)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveInUnprepareHeader referenced in function "public: virtual void __cdecl `anonymous namespace'::WinMMCapture::stop(void)" (?stop@WinMMCapture@?A0xac419535@@UEAAXXZ)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveInAddBuffer referenced in function "public: int __cdecl `anonymous namespace'::WinMMCapture::captureProc(void)" (?captureProc@WinMMCapture@?A0xac419535@@QEAAHXZ)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveInStart referenced in function "public: virtual bool __cdecl `anonymous namespace'::WinMMCapture::start(void)" (?start@WinMMCapture@?A0xac419535@@UEAA_NXZ)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveInStop referenced in function "public: virtual void __cdecl `anonymous namespace'::WinMMCapture::stop(void)" (?stop@WinMMCapture@?A0xac419535@@UEAAXXZ)
OpenAL32.lib(winmm.obj) : error LNK2019: unresolved external symbol __imp_waveInReset referenced in function "public: virtual void __cdecl `anonymous namespace'::WinMMCapture::stop(void)" (?stop@WinMMCapture@?A0xac419535@@UEAAXXZ)

Seems missing links to some libraries?

@mosra
Copy link

mosra commented Feb 26, 2020

Okay, we'll need to roll out our own FindOpenAL that correctly links to winmm on Windows (or uses the exported OpenAL Soft CMake config), the builtin CMake module doesn't. Is that a blocker for this PR? This issue was there probably since ever.

Good idea with the FATAL_ERROR.

@JackBoosY
Copy link
Contributor

@mosra Yes, we need all features to be successfully tested.

@dan-shaw
Copy link
Contributor

@mosra @Squareys @JackBoosY It is okay if not all features are passing. We just don't want to introduce any additional failures (which is why we test all the features). Since this feature combination seems to have been failing since the beginning, I would be OK with merging this PR.

@dan-shaw dan-shaw added the info:reviewed Pull Request changes follow basic guidelines label Mar 20, 2020
@JackBoosY
Copy link
Contributor

@dan-shaw Okay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants