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

[materialx] Add port MaterialX 1.38.4 #24614

Merged
merged 6 commits into from
May 11, 2022

Conversation

theblackunknown
Copy link
Contributor

Describe the pull request

  • What does your PR fix?

This PR adds a new port materialx

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

All, No

Yes

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

Yes

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

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 you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/materialx/vcpkg.json

Valid values for the license field can be found in the documentation

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 you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/materialx/vcpkg.json

Valid values for the license field can be found in the documentation

@theblackunknown
Copy link
Contributor Author

Committed patch has also been reproted upstream in AcademySoftwareFoundation/MaterialX#940

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 you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/materialx/vcpkg.json

Valid values for the license field can be found in the documentation

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 you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/materialx/vcpkg.json

Valid values for the license field can be found in the documentation

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 you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/materialx/vcpkg.json

Valid values for the license field can be found in the documentation

@JonLiu1993 JonLiu1993 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label May 9, 2022
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.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for materialx but no changes to version or port version.
-- Version: 1.38.4
-- Old SHA: 92c7e8e35595440b7620076f8f000a21d0702e45
-- New SHA: 20b6b3a39d993f3bfb1dc0a1ca06126eb6a91f33
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

Co-authored-by: JonLiu1993 <63675417+JonLiu1993@users.noreply.github.com>
@theblackunknown
Copy link
Contributor Author

@JonLiu1993 everything should now be in order, let me know if we can proceed with the merge

@JonLiu1993 JonLiu1993 added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels May 11, 2022
@theblackunknown
Copy link
Contributor Author

Hi @JonLiu1993 , I have added one last commit as my upstream AcademySoftwareFoundation/MaterialX#940 PR was merged in the meantime and hence I do not need anymore to patch the port

@BillyONeal
Copy link
Member

I checked for problematic find_package calls:
documents/CMakeLists.txt has find_package(Doxygen), but that whole CMakeLists.txt is disabled by -DMATERIALX_BUILD_DOCS:BOOL=NO
There are several find_package(pybind11 QUIET) but I'm assuming -DMATERIALX_BUILD_PYTHON:BOOL=NO guards them.

Everything else has REQUIRED :D

Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

Thanks for the new port!

@BillyONeal BillyONeal merged commit 6a2c904 into microsoft:master May 11, 2022
@theblackunknown
Copy link
Contributor Author

Thanks for checking @BillyONeal !
Indeed I started with as few features as possible, and only then will try to gradually add them while also proposing changes upstream.
Keep an eye on my future PRs !

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