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

Add mingw documentation #17219

Merged
merged 9 commits into from
Apr 28, 2021
Merged

Add mingw documentation #17219

merged 9 commits into from
Apr 28, 2021

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Apr 11, 2021

This PR adds basic documentation for using vcpkg with mingw.

The instructions weren't tested line by line from scratch. But they are based on recent work with vcpkg.

@dg0yt dg0yt marked this pull request as ready for review April 11, 2021 21:22
@NancyLi1013 NancyLi1013 added the category:documentation To resolve the issue, documentation will need to be updated label Apr 12, 2021
@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 12, 2021

What I would like to add later is comments about the "system binaries" configuration, affecting cmake and ninja. But this really needs to be tested before published. This can be done i another PR.

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 12, 2021

Ready, from my POV.

@longnguyen2004
Copy link
Contributor

Thanks for the PR! I really appreciate having proper documentation for mingw.
I think we should add a note that users should use a Mingw-w64 based toolchain, not the old MinGW based toolchain. There was a discussion about this in #14561, and an actual issue #16647. Basically put, practically everyone assumes we're using a Mingw-w64 based toolchain, so we should make that clear as well.

@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 15, 2021

I think we should add a note that users should use a Mingw-w64 based toolchain, not the old MinGW based toolchain. There was a discussion about this in #14561, and an actual issue #16647. Basically put, practically everyone assumes we're using a Mingw-w64 based toolchain, so we should make that clear as well.

I was inclined to answer again with "know your toolchain" but maybe this needs more attention indeed. I looked through the mingw mailing lists, and it confirms the MinGW project's mood as reported in #14561, in particular when users of mingw-w64 raise questions there.

To make it really clear, I would replace MinGW with Mingw-w64 in most occurences. This is not meant to exclude MinGW if someone is willing to support that in terms of documentation, triplets and or ports. But I don't think MinGW is ready for usage with vcpkg at this stage.

In particular, I won't change the mingw.md file name. However, renaming the community triplets should be seriously considered. And last not least it is scripts/toolchains/mingw.cmake which selects mingw-w64 tools by ${CMAKE_SYSTEM_PROCESSOR}-w64-mingw32- prefixes (note the w64).

@longnguyen2004
Copy link
Contributor

But I don't think MinGW is ready for usage with vcpkg at this stage.

It's not about vcpkg, it's about upstream expectation. Pretty much every libraries out there expect Mingw-w64, not MinGW.
I would be happy to add a check for it in mingw.cmake, if there's a way to differentiate between them purely by compiler output.

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 15, 2021

But I don't think MinGW is ready for usage with vcpkg at this stage.

It's not about vcpkg, it's about upstream expectation. Pretty much every libraries out there expect Mingw-w64, not MinGW.

IMO there is nothing wrong with upstream. It is downstream, i.e. vcpkg users, which expect the ports to magically work with MinGW when vcpkg talks about MinGW. But when there is no x86_64 in MinGW, it won't come through vcpkg.

I would be happy to add a check for it in mingw.cmake, if there's a way to differentiate between them purely by compiler output.

I didn't try any MinGW toolchain in the last years, and the currently broken MinGW website doesn't help. But I don't think they use the same prefix, or report the same autoconf triplet, as mingw-w64. I assume i386-pc-mingw32 or similar for MinGW. And for compiler output, for mingw-w64 targeting x64, I can run:

$ x86_64-w64-mingw32-gcc -dumpmachine
x86_64-w64-mingw32

So I would look for -w64-mingw32.

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 17, 2021

And now I realize that vcpkg passes the MinGW autotools triplets (...-pc-mingw32) to configure. IMO it should use the ...mingw-w64-w64-mingw32 autotools triplets when it targets Mingw-w64.

@longnguyen2004
Copy link
Contributor

And now I realize that vcpkg passes the MinGW autotools triplets (...-pc-mingw32) to configure. IMO it should use the ...mingw-w64-w64-mingw32 autotools triplets when it targets Mingw-w64.

To be honest, letting autotools decide what triplet to use is a bit unreliable, and I would really like for each triplet to specify its corresponding autotools triplet, but that would require some work on vcpkg_configure_make. @Neumann-A What do you think?

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 17, 2021

letting autotools decide what triplet to use is a bit unreliable

I didn't want to suggest that autotools should decide. I just want to make a distinction from vcpkg's triplets.

I would really like for each triplet to specify its corresponding autotools triplet

In the vcpkg triplet file, at least optionally? Well, this is also what I think. It would facilitate adding new or customizing existing triplets (Android, mingw-w64 ucrt). The triplet files are in the hand of the users.

that would require some work on vcpkg_configure_make

I got the impression that there is already a backlog of open PRs for this script, and I noticed some issues which aren't reported yet.

@longnguyen2004
Copy link
Contributor

I didn't want to suggest that autotools should decide. I just want to make a distinction from vcpkg's triplets.

Oh alright, I thought you were talking about the MinGW vs. Mingw-w64 issue and how the *-pc-mingw32 triplet is related to it.

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 19, 2021

I added another section how to to avoid mixing MSYS binaries from different installations.

Background:
When building port libpq from MSYS2 bash, I ran into strange issues. The root cause is a mix of differents MSYS2 installations which is cause argument passing/quoting/escaping to fail. I found that this is discouraged by the MSYS2 project.

Note: IMO the underlying problem might effect even building ports for MSVC targets.

For the record, my intermediate goal is to build the gdal. At the moment I have to add 10 PRs to vcpkg HEAD, and the next one is coming.

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 20, 2021

I had to extend the PATH recommendation to also exclude /bin. (My test installation were not fully consistent before this change.)

I think that it might make sense to handle this PATH quirk in the triplet files and/or other central files. But this is out of scope for this documentation PR.

Removing the local msys2 installation from PATH means that even simple tools like touch are not available without vcpkg_acquire_msys / before vcpkg_configure_make. But it doesn't look like multiple calls to vcpkg_acquire_msys are a good thing: They result in multiple msys roots. In some cases, such as a call to touch in the non-Windows path of port gdal, this can be solved by using the portable commands of cmake -E.

This basically marks the point where I consider this PR done: It is tested on multiple installation of Windows. (Yes, I succeeded in building gdal with mingw on Windows. It is as broken as on Linux, #9068).

Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for making this awesome doc @dg0yt !

@ras0219-msft ras0219-msft merged commit 6240751 into microsoft:master Apr 28, 2021
@dg0yt dg0yt deleted the mingw-docs branch April 29, 2021 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:documentation To resolve the issue, documentation will need to be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants