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

[vcpkg] Add new community triplet x86-windows-static-release #29034

Closed
wants to merge 1 commit into from

Conversation

xiaozhuai
Copy link
Contributor

Describe the pull request

Since there are x64-windows-static-release, arm-windows-static-release, arm64-windows-static-release and missing x86-windows-static-release. We should also add this.

  1. Add new community triplet x86-windows-static-release

@JonLiu1993 JonLiu1993 added the category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. label Jan 18, 2023
@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Jan 20, 2023
@BillyONeal BillyONeal added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed info:reviewed Pull Request changes follow basic guidelines labels Jan 21, 2023
@BillyONeal
Copy link
Member

Tagging requires: vcpkg-team-review because I want to confirm the team is OK with this.

The triplet on its face as a thing to do is reasonable, but I think we want to make sure we're principled about not necessarily creating the cross product of possible settings.

@BillyONeal
Copy link
Member

@ras0219-msft @markle11m @JavierMatosD @dan-shaw and I discussed this today.

We think there needs to be some set of guidelines for what community triplets we add.

To be a triplet it needs to be tested in our CI.
To be a community triplet we need to not know that it does not work. (This is what excluded linux-dynamic before the patchelf workaround was found)
To be a community triplet we need to know that a user actually wants to use that combination: the entire combinatoric state space of these options is impractical to generate. Someone adding x86-windows-dynamic-md just because we have x64-windows-dynamic-md and they see that it could go there is not sufficient.
We are not looking to fill every hole in the sparse matrix, due to the costs of potentially thousands of files to model that space. (That is, we would be happy to model the whole space if it were not impractically expensive to do so)

We think it might make sense to make tool changes that change the triplet system so that the obvious scheme ones are 'generated' automatically. In the medium term we are also looking to generate much of the triplet interaction by inspection of the consuming build system in manifest mode.

Because this PR looks like it's just trying to fill an entry in the sparse matrix rather than a demonstrated end user requirement, we are going to close this PR. That does not mean that we wouldn't add this community triplet if a meaningful number of users who would benefit is demonstrated. (If you are a user who is supplying this triplet regularly with an overlay, please respond here!)

Thanks for your contribution to vcpkg, and sorry we didn't land this!

@xiaozhuai
Copy link
Contributor Author

xiaozhuai commented Jan 27, 2023

@ras0219-msft @markle11m @JavierMatosD @dan-shaw and I discussed this today.

We think there needs to be some set of guidelines for what community triplets we add.

To be a triplet it needs to be tested in our CI. To be a community triplet we need to not know that it does not work. (This is what excluded linux-dynamic before the patchelf workaround was found) To be a community triplet we need to know that a user actually wants to use that combination: the entire combinatoric state space of these options is impractical to generate. Someone adding x86-windows-dynamic-md just because we have x64-windows-dynamic-md and they see that it could go there is not sufficient. We are not looking to fill every hole in the sparse matrix, due to the costs of potentially thousands of files to model that space. (That is, we would be happy to model the whole space if it were not impractically expensive to do so)

We think it might make sense to make tool changes that change the triplet system so that the obvious scheme ones are 'generated' automatically. In the medium term we are also looking to generate much of the triplet interaction by inspection of the consuming build system in manifest mode.

Because this PR looks like it's just trying to fill an entry in the sparse matrix rather than a demonstrated end user requirement, we are going to close this PR. That does not mean that we wouldn't add this community triplet if a meaningful number of users who would benefit is demonstrated. (If you are a user who is supplying this triplet regularly with an overlay, please respond here!)

Thanks for your contribution to vcpkg, and sorry we didn't land this!

In fact, I do use this triplet. I use it on making a windows virtual camera (with dshow lib). And it need provide both x64 and x86 dll to make both x86 and x64 client work. And I want these dll independs with vc runtime. So I would like to link to static runtime and other libs. That why I add such a triplet. And I believe there should be other similar situation.

@Neumann-A
Copy link
Contributor

Just have an extra folder with triplets and define that folder as an overlay? I mean I do that for https://github.com/Neumann-A/my-vcpkg-triplets

@xiaozhuai
Copy link
Contributor Author

Just have an extra folder with triplets and define that folder as an overlay? I mean I do that for https://github.com/Neumann-A/my-vcpkg-triplets

That is inneed what I need, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants