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.

Sign up for GitHub

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

Jump to bottom

Ogre no absolute #22354

Merged
merged 1 commit into from
Jan 28, 2022
Merged

Ogre no absolute #22354

merged 1 commit into from
Jan 28, 2022

Conversation

autoantwort
Copy link
Contributor

For #20469

@Cheney-W Cheney-W added category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines labels Jan 5, 2022
@strega-nil-ms strega-nil-ms added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Jan 6, 2022
strega-nil-ms
strega-nil-ms reviewed Jan 7, 2022
View reviewed changes
ports/ogre/portfile.cmake
@@ -124,7 +124,13 @@ if(VCPKG_TARGET_IS_WINDOWS)
endforeach()
endif()

file(GLOB share_cfgs ${CURRENT_PACKAGES_DIR}/share/OGRE/*.cfg)
Copy link
Contributor

Choose a reason for hiding this comment

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

I also can't test this change; what are these *.cfg files used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

e.g.: resources.cfg:

# Ogre Core Resources
[OgreInternal]
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/ShadowVolume
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/RTShaderLib/materials
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/RTShaderLib/GLSL
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/RTShaderLib/HLSL_Cg
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/Terrain/

# Resources required by the sample browser and most samples.
[Essential]
Zip=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/packs/SdkTrays.zip
Zip=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/packs/profiler.zip
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/thumbnails

# Common sample resources needed by many of the samples.
# Rarely used resources should be separately loaded by the
# samples which require them.
[General]
# Bites uses the next entry to discover the platform shaders
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/fonts
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/materials/programs
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/materials/scripts
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/materials/textures
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/materials/textures/terrain
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/models
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/particle
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/DeferredShadingMedia
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/DeferredShadingMedia/DeferredShading/post
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/PCZAppMedia
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/materials/scripts/SSAO
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/materials/textures/SSAO
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/volumeTerrain
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/CSMShadows
Zip=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/packs/cubemap.zip
Zip=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/packs/cubemapsJS.zip
Zip=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/packs/dragon.zip
Zip=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/packs/fresneldemo.zip
Zip=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/packs/ogretestmap.zip
Zip=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/packs/ogredance.zip
Zip=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/packs/Sinbad.zip
Zip=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/packs/skybox.zip
Zip=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/volumeTerrain/volumeTerrainBig.zip

FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/PBR
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/PBR/filament
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/materials/textures/PBR
Zip=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/packs/DamagedHelmet.zip
Zip=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/packs/filament_shaders.zip

FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/HLMS

[BSPWorld]
Zip=/home/leander/vcpkg/packages/ogre_x64-linux/share/OGRE/Media/packs/oa_rpg3dm2.pk3

# Materials for visual tests
[Tests]
FileSystem=/home/leander/vcpkg/packages/ogre_x64-linux/Tests/Media
This file is currently only loaded in test. But maybe also by end users and defines where to find stuff

Copy link
Contributor

@ras0219-msft ras0219-msft Jan 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

According to https://ogrecave.github.io/ogre/api/1.12/setup.html#autotoc_md294:

Ogre must find plugins.cfg and resources.cfg to function properly.

[for plugins.cfg] Relative paths are resolved relative to the location of plugins.cfg.
[for resources.cfg] Relative paths are resolved relative to the location of resources.cfg

Therefore this should replace ${CURRENT_PACKAGES_DIR} with ../../ assuming users will use the cfg files from the installed directory.

@strega-nil-ms strega-nil-ms added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Jan 7, 2022
@Cheney-W
Copy link
Contributor

@strega-nil-ms Could you please take a look again? Thank you!

@Cheney-W Cheney-W added the info:reviewed Pull Request changes follow basic guidelines label Jan 26, 2022
ras0219-msft
ras0219-msft requested changes Jan 26, 2022
View reviewed changes
ports/ogre/portfile.cmake
@@ -124,7 +124,13 @@ if(VCPKG_TARGET_IS_WINDOWS)
endforeach()
endif()

file(GLOB share_cfgs ${CURRENT_PACKAGES_DIR}/share/OGRE/*.cfg)
Copy link
Contributor

@ras0219-msft ras0219-msft Jan 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

According to https://ogrecave.github.io/ogre/api/1.12/setup.html#autotoc_md294:

Ogre must find plugins.cfg and resources.cfg to function properly.

[for plugins.cfg] Relative paths are resolved relative to the location of plugins.cfg.
[for resources.cfg] Relative paths are resolved relative to the location of resources.cfg

Therefore this should replace ${CURRENT_PACKAGES_DIR} with ../../ assuming users will use the cfg files from the installed directory.

@ras0219-msft ras0219-msft removed the info:reviewed Pull Request changes follow basic guidelines label Jan 26, 2022
github-actions[bot]
github-actions bot approved these changes Jan 27, 2022
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/ogre/vcpkg.json

Valid values for the license field are listed at https://spdx.org/licenses/

@Cheney-W Cheney-W added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Jan 27, 2022
@ras0219-msft ras0219-msft merged commit b89b8a2 into microsoft:master Jan 28, 2022
@ras0219-msft
Copy link
Contributor

LGTM, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@strega-nil-ms strega-nil-ms strega-nil-ms left review comments

@ras0219-msft ras0219-msft ras0219-msft requested changes

@github-actions github-actions[bot] github-actions[bot] approved these changes

Assignees

@Cheney-W Cheney-W

Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@autoantwort @Cheney-W @ras0219-msft @strega-nil-ms