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] Properly deploy plugins #3191

Merged
merged 7 commits into from Apr 19, 2018

Conversation

Projects
None yet
3 participants
@Squareys
Contributor

Squareys commented Apr 2, 2018

Hi @ras0219-msft !

I finally fixed the plugin deployment of the magnum package (see mosra/magnum#235)! As this is the first time I came in contact with Powershell, you may want to have a close look at that.

Also, to be able to build --head with upcoming features properly, I added the repective features and incremented the CONTROL-file version.

@mosra may want to have a quick glance at the depedencies of new features.

Thanks in advance,
Jonathan.

@Squareys

This comment has been minimized.

Contributor

Squareys commented Apr 3, 2018

@mosra Done with the changes discussed on gitter! :)

@mosra

Thanks a lot. 👍

Tricky question: does this automagically copy also dependencies of the plugins? (e.g. is libpng DLL that PngImporter depends on copied as well?) Or does vcpkg handle that part already? :)

@@ -41,6 +43,7 @@ vcpkg_configure_cmake(
OPTIONS
${_COMPONENT_FLAGS}
-DBUILD_STATIC=${BUILD_STATIC}
-DBUILD_PLUGINS_STATIC=${DBUILD_PLUGINS_STATIC}

This comment has been minimized.

@mosra

mosra Apr 3, 2018

Hmm, maybe just have a single BUILD_PLUGINS_STATIC variable and set it to both? Also, you have DBUILD there ;)

-DBUILD_STATIC=${BUILD_PLUGINS_STATIC}
-DBUILD_PLUGINS_STATIC=${BUILD_PLUGINS_STATIC}
Build-Depends: freeglut
Feature: anyaudioimporter
Description: (Upcoming) AnyAudioImporter plugin
Build-Depends: magnum[trade]

This comment has been minimized.

@mosra

mosra Apr 3, 2018

magnum[audio] instead

Feature: glutapplication
Description: GlutApplication library
Build-Depends: freeglut

This comment has been minimized.

@mosra

mosra Apr 3, 2018

Build-Depends: freeglut, magnum[gl]

Description: MeshTools library
Feature: opengltester
Description: OpenGLTester library
Build-Depends: corrade[testsuite]

This comment has been minimized.

@mosra

mosra Apr 3, 2018

Build-Depends: corrade[testsuite], magnum[gl]

Description: (Upcoming) Trade library
Feature: wavaudioimporter
Description: WavAudioImporter plugin
Build-Depends: magnum[audio]
Feature: windowlesswglapplication
Description: WindowlessWglApplication library
Feature: wglcontext
Description: WglContext library

This comment has been minimized.

@mosra

mosra Apr 3, 2018

Build-Depends: magnum[gl]

Feature: distancefieldconverter
Description: magnum-distancefieldconverter utility
Build-Depends: magnum[texturetools]
Feature: fontconverter
Description: magnum-fontconverter utility
Build-Depends: magnum[text]

This comment has been minimized.

@mosra

mosra Apr 3, 2018

Build-Depends: magnum[text], magnum[gl]

Feature: distancefieldconverter
Description: magnum-distancefieldconverter utility
Build-Depends: magnum[texturetools]

This comment has been minimized.

@mosra

mosra Apr 3, 2018

Build-Depends: magnum[texturetools], magnum[gl]

@@ -79,3 +87,6 @@ Feature: stbvorbisaudioimporter
Description: StbVorbisAudioImporter plugin
Build-Depends: magnum[audio]
Feature: tinygltfimporter
Description: (Upcoming) TinyGltfImporter plugin

This comment has been minimized.

@mosra

mosra Apr 3, 2018

This is not upcoming anymore :)

This comment has been minimized.

@Squareys

Squareys Apr 3, 2018

Contributor

Well, it's not in the release that has been tagged in the accompanying portfile.cmake, so "in perspective of the last release" it is indeed upcoming until that is tagged :)

This comment has been minimized.

@mosra

mosra Apr 3, 2018

Ah, sure, true, then leave it there. Sorry, my perception of time is mixed up again :)

@@ -1,31 +1,35 @@
Source: magnum-plugins
Version: 2018.02-1
Build-Depends: magnum
Version: 2018.02-2

This comment has been minimized.

@mosra

mosra Apr 3, 2018

Why no Build-Depends: magnum?

This comment has been minimized.

@Squareys

Squareys Apr 3, 2018

Contributor

I removed the magnum[trade] and didn't think :D

resolve($targetBinary)
Write-Verbose $($g_searched | out-string)

This comment has been minimized.

@mosra

mosra Apr 3, 2018

Why this was removed?

This comment has been minimized.

@Squareys

Squareys Apr 3, 2018

Contributor

Good question, thanks!

@Squareys

This comment has been minimized.

Contributor

Squareys commented Apr 3, 2018

does this automagically copy also dependencies of the plugins? (e.g. is libpng DLL that PngImporter depends on copied as well?) Or does vcpkg handle that part already? :)

Not so tricky answer: Yes! :D

Squareys added some commits Mar 23, 2018

[magnum] Properly deploy magnum plugins
Signed-off-by: Squareys <squareys@googlemail.com>
[magnum-plugins] Add tinygltfimporter feature
Signed-off-by: Squareys <squareys@googlemail.com>
[magnum][magnum-plugins] Add features: trade and any*
Prepares upcoming move of those sublibraries and allows building --head
immediately.
For current release this only adds some unused cmake flags that will be
ignored.

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

This comment has been minimized.

Contributor

Squareys commented Apr 3, 2018

@mosra Done making the requested changes, thank you very much for the review!

@ras0219-msft This is now ready to merge, thanks for the help with mosra/magnum#235 !
PS: A while ago I opened this issue here, it would be great to get some brief feedback on that.

Squareys added some commits Apr 2, 2018

[magnum] Add gl feature, cleanup dependencies, mark upcoming features
And sort features alphabetically.

Signed-off-by: Squareys <squareys@googlemail.com>
[magnum-plugins] Prepare renaming of static flag for --head installs
Signed-off-by: Squareys <squareys@googlemail.com>
@mosra

mosra approved these changes Apr 3, 2018

[magnum] Add two missing feature dependencies
Signed-off-by: Squareys <squareys@googlemail.com>
@Squareys

This comment has been minimized.

Contributor

Squareys commented Apr 5, 2018

@mosra Added the two missing feature dependencies, as discussed per gitter.

You may need to reapprove.

[magnum] Enable magnum[any*] features by default
Signed-off-by: Squareys <squareys@googlemail.com>
@mosra

mosra approved these changes Apr 6, 2018

@Squareys

This comment has been minimized.

Contributor

Squareys commented Apr 15, 2018

@ras0219-msft This is ready to be merged. (In case it wasn't clear)

@ras0219-msft

This comment has been minimized.

Contributor

ras0219-msft commented Apr 19, 2018

Thanks, this looks good to me as well!

@ras0219-msft ras0219-msft merged commit 3a3fa5c into Microsoft:master Apr 19, 2018

1 check passed

license/cla All CLA requirements met.
Details

quanrquanr added a commit to rog2/vcpkg that referenced this pull request Jul 25, 2018

[magnum] Properly deploy plugins (Microsoft#3191)
* [magnum] Properly deploy magnum plugins

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

* [magnum-plugins] Add tinygltfimporter feature

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

* [magnum][magnum-plugins] Add features: trade and any*

Prepares upcoming move of those sublibraries and allows building --head
immediately.
For current release this only adds some unused cmake flags that will be
ignored.

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

* [magnum] Add gl feature, cleanup dependencies, mark upcoming features

And sort features alphabetically.

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

* [magnum-plugins] Prepare renaming of static flag for --head installs

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

* [magnum] Add two missing feature dependencies

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

* [magnum] Enable magnum[any*] features by default

Signed-off-by: Squareys <squareys@googlemail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment