Skip to content

[wxwidgets] Add 'WXWIDGETS_USE_STD_CONTAINERS' option#21841

Merged
BillyONeal merged 8 commits intomicrosoft:masterfrom
Karandra:WXWIDGETS_USE_STD_CONTAINERS
Dec 8, 2021
Merged

[wxwidgets] Add 'WXWIDGETS_USE_STD_CONTAINERS' option#21841
BillyONeal merged 8 commits intomicrosoft:masterfrom
Karandra:WXWIDGETS_USE_STD_CONTAINERS

Conversation

@Karandra
Copy link
Contributor

@Karandra Karandra commented Dec 3, 2021

  • What does your PR fix?

    It adds a new option WXWIDGETS_USE_STD_CONTAINERS to set wxUSE_STD_CONTAINERS macro, which isn't controlled by wxUSE_STL that is set by WXWIDGETS_USE_STL option.

  • Does your PR follow the maintainer guide?

    Yes, I think.

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

    Yes.

@ghost
Copy link

ghost commented Dec 3, 2021

CLA assistant check
All CLA requirements met.

@LilyWangLL
Copy link
Contributor

Thanks for your PR. vcpkg_fail_port_install has been deprecated, VCPKG use supports in vcpkg.json instead of it. Could you please remove it in portfile.cmake?

@LilyWangLL LilyWangLL added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist requires:author-response labels Dec 7, 2021
Dafuq? I ran this command before comitting and you, vcpkg, did nothing and now there are changes? Ugh.
@LilyWangLL LilyWangLL added info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. and removed requires:author-response labels Dec 8, 2021
@BillyONeal
Copy link
Member

Hmm based on the documentation it appears that this causes breaking changes when turned on: https://docs.wxwidgets.org/trunk/overview_container.html

The standard container build is mostly, but not quite, compatible with the default one
[...]

I think that means it doesn't follow "do not use features to implement alternatives" https://github.com/microsoft/vcpkg/blob/master/docs/maintainers/maintainer-guide.md#do-not-use-features-to-implement-alternatives

and as a result I think if you want this setting you really need to use an overlay port. Unless I have misunderstood the situation?

@BillyONeal BillyONeal added requires:author-response and removed info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. labels Dec 8, 2021
@Karandra
Copy link
Contributor Author

Karandra commented Dec 8, 2021

That's not a feature though, just an option for a custom triplet that is off by default.

Hmm based on the documentation it appears that this causes breaking changes when turned on:

It's the same as wxUSE_STL actually. I added similar option for wxUSE_STD_CONTAINERS . There's a discussion about it on #19274, I don't really know what else I could add to this.

@BillyONeal
Copy link
Member

That's not a feature though, just an option for a custom triplet that is off by default.

Hmm based on the documentation it appears that this causes breaking changes when turned on:

It's the same as wxUSE_STL actually. I added similar option for wxUSE_STD_CONTAINERS . There's a discussion about it on #19274, I don't really know what else I could add to this.

That does it as a triplet option, not a feature.

@BillyONeal
Copy link
Member

Oh wait, I am blind. I could have swore I saw something in this PR that talked about using features to control this rather than a triplet option, let me review again.

@BillyONeal BillyONeal added info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. and removed requires:author-response labels Dec 8, 2021
@BillyONeal BillyONeal merged commit 14543ba into microsoft:master Dec 8, 2021
@BillyONeal
Copy link
Member

Thanks again!

@Karandra Karandra deleted the WXWIDGETS_USE_STD_CONTAINERS branch December 9, 2021 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants