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

[dxsdk-d3dx] Add new port #17175

Merged
merged 9 commits into from
Apr 30, 2021
Merged

[dxsdk-d3dx] Add new port #17175

merged 9 commits into from
Apr 30, 2021

Conversation

walbourn
Copy link
Member

@walbourn walbourn commented Apr 9, 2021

Adds a vcpkg port for the NuGet package Microsoft.DXSDK.D3DX. This provides the D3DX9, D3DX10, and D3DX11 utility libraries, their headers, and SHA-2 signed DLLs which can be redistributed side-by-side with the application per a new license.

Use of these legacy libraries is still not recommended for new projects, but this is a much more supportable way to use legacy D3DX libraries for existing projects that rely on it.

These headers are compatible with the modern Windows SDK, but are located in include/dxsdk-d3dx to isolate them.

@walbourn walbourn changed the title Dxsdkd3dx [dxsdk-d3dx] port added Apr 9, 2021
@walbourn
Copy link
Member Author

walbourn commented Apr 9, 2021

Note I had to make use of set(VCPKG_POLICY_ALLOW_OBSOLETE_MSVCRT enabled) because these D3DX legacy DLLs use msvcrt.dll. The are binary identical to the binaries that shipped in the legacy DirectX SDK (June 2010). The only difference is that they have been resigned with SHA-2.

@NancyLi1013 NancyLi1013 self-assigned this Apr 9, 2021
@NancyLi1013 NancyLi1013 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Apr 9, 2021
NancyLi1013
NancyLi1013 suggested changes Apr 9, 2021
View reviewed changes
@walbourn walbourn requested a review from NancyLi1013 April 10, 2021 22:13
@walbourn walbourn changed the title [dxsdk-d3dx] port added [dxsdk-d3dx] Add new port Apr 11, 2021
@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@walbourn
Copy link
Member Author

No idea what the OSX failures have to do with this port...

@Hoikas
Copy link
Contributor

Hoikas commented Apr 13, 2021

Looks like the disk space ran out on the macOS machine. Not related to this port.

@NancyLi1013
Copy link
Contributor

Looks like the disk space ran out on the macOS machine. Not related to this port.

As @Hoikas said above, I will rerun this PR once the disk space problem solved.

@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@NancyLi1013
Copy link
Contributor

The failures caused by popsift will be fixed by #17277.

@walbourn
Copy link
Member Author

The failures caused by popsift will be fixed by #17277.

OK. I've rebased against the latest master...

@NancyLi1013 NancyLi1013 added the info:reviewed Pull Request changes follow basic guidelines label Apr 15, 2021
@NancyLi1013
Copy link
Contributor

LGTM now, thanks for adding this port @walbourn.

@walbourn
Copy link
Member Author

This port doesn't support OSX, so no idea why it's failing that check.

@NancyLi1013
Copy link
Contributor

For osx pipeline, there is something wrong with our CI test machine. We're looking into this. You can ignore this failure, which is not related with this PR. I will rerun it once solved.

Note: Please don't update changes in a PR unless it's necessary after we have added the label 'reviewed'.

@NancyLi1013 NancyLi1013 removed the info:reviewed Pull Request changes follow basic guidelines label Apr 23, 2021
@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@NancyLi1013 NancyLi1013 added the info:reviewed Pull Request changes follow basic guidelines label Apr 23, 2021
@walbourn
Copy link
Member Author

Note: Please don't update changes in a PR unless it's necessary after we have added the label 'reviewed'.

Sorry. I was applying feedback to this open PR that I had been given in another PR about the new version semantics.

@walbourn
Copy link
Member Author

@Hoikas is there something holding up this PR?

@Hoikas
Copy link
Contributor

Hoikas commented Apr 28, 2021

Good question! Hopefully a maintainer will chime in if there are any outstanding other blockers. It LGTM.

@strega-nil-ms
Copy link
Contributor

Thanks @walbourn :), sorry it took a little while to merge

@strega-nil-ms strega-nil-ms merged commit b767888 into microsoft:master Apr 30, 2021
@walbourn walbourn deleted the dxsdkd3dx branch August 8, 2021 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hoikas Hoikas Hoikas left review comments

@NancyLi1013 NancyLi1013 NancyLi1013 approved these changes

Assignees

@NancyLi1013 NancyLi1013

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@walbourn @NancyLi1013 @Hoikas @strega-nil-ms