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 mingw dynamic libs triplet #12101

Merged
merged 6 commits into from
Jul 6, 2020

Conversation

longnguyen2004
Copy link
Contributor

@longnguyen2004 longnguyen2004 commented Jun 25, 2020

Describe the pull request

  • What does your PR fix? Fixes #
    This PR adds 4 triplets for building dynamic libs with mingw toolchain, and rename the old static libs triplets to *-static, following the *-windows triplets convention
    Currently, building with this triplet will fail, due to the post-build validation only checking for .lib files, not .dll.a files, but the building process seems to be ok, at least for CMake-based libs. So we only have that left to fix. Added VCPKG_POLICY_DLLS_WITHOUT_LIBS as a workaround.

  • Which triplets are supported/not supported? Have you updated the CI baseline?
    Should be all *-mingw triplets, but I've only tested x64-mingw

  • Does your PR follow the maintainer guide?
    Yes

@LilyWangL LilyWangL changed the title Add mingw dynamic libs triplet [vcpkg] Add mingw dynamic libs triplet Jun 29, 2020
@LilyWangL LilyWangL added the category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed label Jun 29, 2020
@LilyWangL LilyWangL requested a review from vicroms June 29, 2020 06:32
@LilyWangL
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@LilyWangL
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@LilyWangL LilyWangL marked this pull request as ready for review June 30, 2020 07:36
@LilyWangL LilyWangL added the info:reviewed Pull Request changes follow basic guidelines label Jun 30, 2020
@vicroms
Copy link
Member

vicroms commented Jul 2, 2020

The only concern I have is that this PR changes the behavior of already existing triplets for MinGW, and will break users that have already adopted those triplets.

@vicroms
Copy link
Member

vicroms commented Jul 2, 2020

I think a good compromise is to rename both triplets: <arch>-mingw-static and <arch>-mingw-dynamic.

@longnguyen2004
Copy link
Contributor Author

So *-mingw would be the same as *-mingw-static, got it

@vicroms
Copy link
Member

vicroms commented Jul 3, 2020

I say we remove <arch>-mingw, and force users to use the explicit <arch>mingw-static and <arch>mingw-dynamic triplets. That way existing users will get an error for deprecated behavior instead of changing the library linkage without them noticing.

@ras0219-msft ras0219-msft merged commit 89dec24 into microsoft:master Jul 6, 2020
@ras0219-msft
Copy link
Contributor

Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants