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

Cygwin / MinGW32 build_ext broken (?) #109

Closed
loehnertj opened this issue Jan 20, 2022 · 13 comments
Closed

Cygwin / MinGW32 build_ext broken (?) #109

loehnertj opened this issue Jan 20, 2022 · 13 comments
Labels
invalid This doesn't seem right

Comments

@loehnertj
Copy link

Hey guys, after updating Setuptools to 60.something, my extension build broke.

I am using Python 3.9.4 and MinGW32 (64-bit).
My extension is a single-file C module, which doesn't use any particular libraries beyond the stdlib.

Build failed, because the linker tried to link -lucrt, which could not be found.

I could trace this down to cygwincompiler.py:93, where on Nov 15 2021, support for MSVC version 1900...2000 was added / Commit eca30e8.

Previously my build was working, because I monkeypatched this function on my own. It broke by the upgrade, because my monkeypatch only became active if the "native" get_msvcr() failed.

Now I am an amateur when it comes to C compilers. But it seems to me, that get_msvcr specifies that both ucrt (ucrtbase.dll) and vcruntime140 (vcruntime140.dll) shall be linked. This fails because Python (specifically my v3.9.4) only bundles vcruntime140.

From what I've read, ucrtbase is supposed to replace vcruntime. So I would only expect one of them has to be present? If this is correct, the compilation settings should not link both libraries, but rather pick one of them.

Possible workarounds are

  • downgrading Setuptools to 59.1.1 or earlier, for the time being
  • Monkeypatching cygwincompiler.get_msvcr to return ["vcruntime140"] only.
@imba-tjd
Copy link
Contributor

imba-tjd commented Jan 22, 2022

Hi, I made this change. You can see the discussion in #41

I have some questions. Is there a MinGW32 (64-bit)? AFAIK MinGW32 only supports 32bit.

I'm using MinGW-w64 which works with -lucrt.

According to https://github.com/numpy/numpy/pull/6875, Unfortunately, linking to ucrtbase.dll directly is definitely wrong. ucrt is C runtime, and vcruntime140.dll does not contain the C runtime

How do you monkeypatch get_msvcr? I did similar thing. You can see https://github.com/imba-tjd/mingw64ccompiler. But it doesn't work after https://github.com/pypa/setuptools/pull/2896.

@loehnertj
Copy link
Author

Hi, answering the questions first:

  • I am also using MinGW-w64 installed 2021-04-27. It contains a libucrtbase.a file some, but nevertheless -lucrt fails. I'll try upgrading it, maybe this already dissolves the issue.
  • My monkeypatch, condensed (ccc is the distutils' cygwinccompiler module):
    _orig_get_msvcr = ccc.get_msvcr
    def _get_msvcr():
        try:
            return _orig_get_msvcr()
        except ValueError:
            pass
        msc_pos = sys.version.find('MSC v.')
        # ...
        if msc_ver.startswith('19'):
            # Visual Studio 2015 / Visual C++ 14.0
            # "msvcr140.dll no longer exists" http://blogs.msdn.com/b/vcblog/archive/2014/06/03/visual-studio-14-ctp.aspx
            return ['vcruntime140']
        else:
            raise ValueError("Unknown MS Compiler version %s " % msc_ver)

    ccc.get_msvcr = _get_msvcr

To understand the relation between vcruntime140 and ucrt better, I introspected it with the Dependencies tool, which leaves me headscratching.

  • As you said, vcruntime140 doesn't contain the full C stdlib, but only a handful of memory-copy/move and string functions. However, vcruntime depends on (i.e. imports) the system's ucrtbase dll.
  • OTOH in my extension, I could use printf, malloc/free, etc just fine even with just -lvcruntime140. Maybe because it implicitly imports ucrt?
  • python39.dll indeed links vcruntime140 and ucrtbase. But does that mean that Extensions must also link both of them?

With my limited C skills, I'm lost in the desert here. I'll try upgrading MinGW; if that works, we can close this as invalid...

@imba-tjd
Copy link
Contributor

imba-tjd commented Jan 24, 2022

ucrt and ucrtbase are different.

I don't think python39.dll links ucrtbase. Try this command:

> objdump -p python310.dll | findstr dll
python310.dll:     file format pei-i386
        DLL Name: VERSION.dll
        DLL Name: WS2_32.dll
        DLL Name: api-ms-win-core-path-l1-1-0.dll
        DLL Name: ADVAPI32.dll
        DLL Name: KERNEL32.dll
        DLL Name: VCRUNTIME140.dll
        DLL Name: api-ms-win-crt-math-l1-1-0.dll
        DLL Name: api-ms-win-crt-locale-l1-1-0.dll
        DLL Name: api-ms-win-crt-string-l1-1-0.dll
        DLL Name: api-ms-win-crt-runtime-l1-1-0.dll
        3af366     65  _seh_filter_dll
        DLL Name: api-ms-win-crt-stdio-l1-1-0.dll
        DLL Name: api-ms-win-crt-convert-l1-1-0.dll
        DLL Name: api-ms-win-crt-time-l1-1-0.dll
        DLL Name: api-ms-win-crt-environment-l1-1-0.dll
        DLL Name: api-ms-win-crt-process-l1-1-0.dll
        DLL Name: api-ms-win-crt-heap-l1-1-0.dll
        DLL Name: api-ms-win-crt-conio-l1-1-0.dll
        DLL Name: api-ms-win-crt-filesystem-l1-1-0.dll
Name                            003a5706 python310.dll

Thouse api-ms-win-crt dlls are ucrt rather than ucrtbase. With ucrtbase, there will be no api-ms-win-crt but only exist one ucrtbase.dll

I could use printf, malloc/free, etc just fine even with just -lvcruntime140. Maybe because it implicitly imports ucrt?

No. It implicitly imports msvcrt. You can use gcc -dumpspecs | findstr msvcrt to see that.

@lazka
Copy link
Contributor

lazka commented Jan 24, 2022

I know that a mingw-w64 toolchain that targets ucrt can't be used to build msvcrt stuff and vice versa. So I'm wondering if this function should just be removed maybe? This package only supports Python versions with ucrt anyway.

@imba-tjd
Copy link
Contributor

I have no idea about removing this function.
As I understand, python310.dll is linked with ucrt and vcruntime140. When using MinGW to build_ext, I also try to let it link with them.
Though MinGW links msvcrt by default, I used a customized spec file to change that. You can see my patch at https://github.com/imba-tjd/mingw64ccompiler/blob/master/mingw64ccompiler.py#L46. I tested that it runs hello world module.
So I think idealy this function only returns vcruntime140, but the compiler must guarantee that it links ucrt rather than msvcrt?

@loehnertj
Copy link
Author

As I understand it, "ucrt" is a "concept" which finds its actual implementation in ucrtbase.dll... the api-ms-.. DLLs actually forward to ucrtbase.dll on my system. Unfortunately I can only post a screenshot here.

grafik

You are right that my built extension imports system's MSVCRT.dll. (but if that works by itself, why do we need to link ucrt?)

Can you point me to your actual MinGW-w64 installer? The one I use had its latest release in 2018 (https://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win32/Personal%20Builds/mingw-builds/installer/mingw-w64-install.exe/download)

@lazka
Copy link
Contributor

lazka commented Jan 24, 2022

Though MinGW links msvcrt by default, I used a customized spec file to change that.

That's not supported by mingw-w64 at least. It might work if you are lucky, but you might also get linker errors or random crashes due to ABI mismatches.

@loehnertj
Copy link
Author

Thanks for the links! Can confirm that everything works again with the MinGW distribution from https://winlibs.com/. I'm fine with closing this issue.

@imba-tjd
Copy link
Contributor

Anyway, I'm not an expert on C as well. @lazka feel free to open PR to remove the function whenever you think necessary.

@lazka
Copy link
Contributor

lazka commented Jan 24, 2022

You can try using the UCRT toolchain from https://winlibs.com/ for example. This should allow you to drop all that monkey patching for spec files at least.

@loehnertj
Copy link
Author

Thinking more about this, the -lucrt is probably beneficial, to avoid exactly my mistake of linking the extension against MSVCRT, while Python is linked against UCRT. As far as I found out with a quick googling, mixing those is not really recommended.

Only the error message could be made clearer ("please upgrade your C Compiler to support UCRT"), however I don't see how this could be implemented without hacks. So let's just close this as invalid.

@jaraco
Copy link
Member

jaraco commented Feb 26, 2022

Thanks everyone for the investigation and civil discourse.

@jaraco jaraco closed this as completed Feb 26, 2022
@jaraco jaraco added the invalid This doesn't seem right label Feb 26, 2022
imba-tjd referenced this issue in pypa/setuptools Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

4 participants