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

[ms-gltf] Add new port #14971

Merged
merged 15 commits into from
Jan 25, 2021
Merged

[ms-gltf] Add new port #14971

merged 15 commits into from
Jan 25, 2021

Conversation

luncliff
Copy link
Contributor

@luncliff luncliff commented Dec 6, 2020

Changes

New port for https://github.com/microsoft/glTF-SDK in Windows / MacOS platforms.

What does your PR fix?

There was no port request for this.

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

Linux is not supported.
ci.baseline.txt will be updated soon.

Does your PR follow the maintainer guide?

Yes!

@luncliff luncliff marked this pull request as draft December 6, 2020 17:02
@PhoebeHui PhoebeHui added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Dec 7, 2020
@PhoebeHui
Copy link
Contributor

@luncliff, could you format the vcpkg.json file? See github.com/microsoft/vcpkg/blob/master/docs/maintainers/maintainer-guide.md#manifest for solution.

@luncliff luncliff marked this pull request as ready for review January 17, 2021 18:24
@luncliff
Copy link
Contributor Author

@PhoebeHui, I was waiting for the next release of the library since last December but seems like it can take more than months... :(
The port's version is r1.9.5.0 but the ref commit is not. Can this be allowed? I'd like to hear opinions about it.

In microsoft/glTF-SDK#66, I update some build configurations and source code. Fortunately the project's TC passed, so I'd like to use the changes if possible.

Copy link
Contributor

@PhoebeHui PhoebeHui left a comment

Choose a reason for hiding this comment

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

@luncliff, I think it's fine, we already have the comments about the ref commit, we can update that with new release in future.

@PhoebeHui PhoebeHui added requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function and removed requires:author-response labels Jan 22, 2021
@PhoebeHui
Copy link
Contributor

All features test passed with x64-windows.

@PhoebeHui PhoebeHui removed the requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function label Jan 22, 2021
@PhoebeHui PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Jan 22, 2021
@luncliff
Copy link
Contributor Author

@PhoebeHui Appreciate the changes and testing. (Sadly, things didn't go well last week)

I added a small patch because of <filesystem> support in Mac OS (x64-osx triplets).
It basically adds 2 more changes.

  • source: _LIBCPP_NO_EXPERIMENTAL_DEPRECATION_WARNING_FILESYSTEM when __APPLE__
    Like warning for std::experimental::filesystem in MS STL, headers in MacOS SDK reports the deprecation and it is considered error. The library is used only in samples so I placed a macro definition.
  • cmake: CMAKE_OSX_DEPLOYMENT_TARGET to 10.15.
    This version allows use of std::filesystem::path.
    Normally it is fine with CMake configuration, but I found that ninja places the minimum osx version to 10.11.

@dan-shaw dan-shaw merged commit bb4de6f into microsoft:master Jan 25, 2021
@luncliff luncliff deleted the port/ms-gltf branch January 30, 2021 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants