vcpkg icon indicating copy to clipboard operation
vcpkg copied to clipboard

[osg] Fix plugin installation and target locations

Open cbrl opened this issue 2 years ago • 3 comments

Describe the pull request

This change removes the stage in portfile.cmake which relocates the osg plugins from bin/ to plugins/. Instead, the OsgMacroUtils.cmake file is patched to install directly to the plugins folder.

#25919 contains some discussion on the issue that this changes addresses. This is opened as a draft since I was not entirely clear on the most desirable outcome. I believe this is the simplest method that achieves the results, but there may be a more preferred method or alternative location for the plugins.

  • What does your PR fix?

    This has two primary effects. The first is correcting the exported target locations in unofficial-osg, as they still pointed to bin/ after moving the plugins. The second is fixing the process on non-Windows triplets using dynamic linkage, as the plugins were not being moved in those cases.

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    All, No

  • Does your PR follow the maintainer guide?

    Yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    Yes

cbrl avatar Aug 07 '22 05:08 cbrl

Pinging @cbrl for response. Is work still being done for this PR?

JonLiu1993 avatar Sep 09 '22 08:09 JonLiu1993

Sorry for the delay. I don't believe any additional work is required, and I have successfully utilized the port on Windows and Linux with these modifications. I initially opened this as a draft because I was unsure as to how the plugins should be handled across different platforms and library linkage, but the discussion with @dg0yt in the issue linked above suggests that this is the correct path. Absent any recommended changes to this solution, I am ready to mark it for review.

cbrl avatar Sep 09 '22 14:09 cbrl

Depend on https://github.com/microsoft/vcpkg/pull/26698

CMake Error: try_run() invoked in cross-compiling mode, please set the following cache variables appropriately:
   _OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED_EXITCODE (advanced)
   _OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED_EXITCODE__TRYRUN_OUTPUT (advanced)

JonLiu1993 avatar Sep 13 '22 10:09 JonLiu1993

I think this fix makes sense. Have we tested if the applocal.ps1 (for Visual Studio integration) script has the correct behavior for the plugins folder?

dan-shaw avatar Sep 26 '22 06:09 dan-shaw

@dan-shaw AFAIK there is no generic mechanism for deploying plugins, only specific hooks for some packages (Qt, OpenNI2, Magnum, AzureKinectSensorSDK). There are many more ports with plugins (osg, pdal, ogre, ...).

dg0yt avatar Sep 26 '22 06:09 dg0yt

I haven't explicitly tested the Visual Studio integration, but from memory it does not have any knowledge of the plugins folder. However, as @dg0yt mentioned, there doesn't seem to be a well-defined mechanism for plugin deployment that it would integrate with. As-is, this PR doesn't change that behavior. I think that would be outside the scope, and would require changes to multiple ports for consistency.

cbrl avatar Oct 04 '22 15:10 cbrl