Add builds with \MD flag for building Python extensions

[tkralphs]

Try adding builds with the \MD flag.

[jhmgoossens]

What is the aim? To build using the Microsoft runtime as DLL rather than the Microsoft runtime as static lib?

[jhmgoossens]

There's something wrong with the MSVC binaries built with MT: missing libgcc??
Also, apologies, I'm not sure what should be the outcome of "MT" or "MD" builds. Are we expecting either statically linked COIN libaries (MT), or DLLs for the CoinUtils etc. (MD)? Or are we merely talking only about how the MS runtime libraries themselves are used?

• The PR 673 build for master:
  The MSVC MT binaries (~10MB zip) are NOT working due to missing libgcc_s_seh-1.dll and libstdc++-6.dll
  The MSVC MD binaries (~10MB zip) are working and no reference to libgcc_s_seh and libstdc++-6., and with CoinUtils-0.dll etc.

• The PR 674 build for stable:
  The MSVC MT binaries (~14.7MB zip) are NOT working due to missing libgcc_s_seh-1.dll and libstdc++-6.dll
  The MSVC MD binaries (~17.7MB zip) are working and no reference to libgcc_s_seh and libstdc++-6., but no CoinUtils-0.dll etc. Only one large executable for Clp.exe and Cbc.exe

@tkralphs I hope I'm not misunderstanding something here? Or do I have wrong expectations?
For reference, see Issue 590.

[svigerske]

I don't think that the build system on master understands --enable-msvc=MD. It probably handles it as if --enable-msvc has not been set, and then does not prefer the MS compilers over GCC.

However, -MD is in the default compliler flags.

[jhmgoossens]

Looking at the logs, with enable-msvc=MD, the build ends up using MS, not gcc (good) With enable-msvc=MT the build uses gcc (bad).
I don’t understand how the build system works, all the way from coinbrew to the actual compiling.

1. In coinbrew, there seems only support for the plain “—enable-msvc”, so why even enable-msvc=MD works as much as it does, I don’t understand.
2. In which files are the compile flags defined? And can that file distinguish the MT and MD?

[svigerske]

Default compiler flags are defined in configure, which gets it from

https://github.com/coin-or-tools/BuildTools/blob/20208f47f7bbc0056a92adefdfd43fded969f674/coin.m4#L139-L163

[jhmgoossens]

@svigerske Thanks for the hint!
Looking at configure I don't see how anything except plain --enable-msvc without argument could work.
The configure file and coin.m4 only seem to checks for --enable-msvc and then only uses flag -MD or -MDd (around line 3296). I can't see any attempt to parse a value like --enable-msvc=MT. Also, looking at the configure script overall, there doesn't seem to be any point at which configure tells the MSVC build process to use -MT such that it will compile against MS C Runtime as static libraries. Lastly, all documentation refers to --enable-msvc without argument.

@tkralphs Am I missing something, or there doesn't seem to be support for values like --enable-msvc=MT or --enable-msvc=MD?

[svigerske]

With BuildTools 0.8, --enable-msvc allowed these additional arguments. So for the builds for the stable-branch, this may still work. That's probably what Ted thinks about. But for current BuildTools, this was dropped, to simplify things.

[jhmgoossens]

Thanks! That explains a lot.

Apologies in advance for this basic question or my misunderstanding:
To my understanding, for MSVC the options "-MT" or "-MD" are merely about how to use the Microsoft C runtimes--not about how the (other) dependencies are linked, as "enable-shared" controls (I believe). Therefore, if the only thing we want to control is whether MSVC builds shall use static or dynamic MS C runtimes ("-MT" or "-MD"), then there doesn't need to be a connection with "enable-shared" etc. And there also doesn't need to be equivalent configure steps for non-MSVC builds. The only caveat is that all dependencies must use the same enable-msvc option, but the common build system can take care of that.

Therefore, couldn't support for --enable-msvc=MT/MTd/MD/MDd be as simple as a slightly more complicated if in configure line 3294 to add cases for "MT" and "MTd"? Or even slightly less complicated if when --enable-msvc=XX is present we simply put the XX option into the FLAGS?

[svigerske]

I think it's easy to just overwrite the compiler flags if one needs MT(d). As far as I know, it's advised to use MD(d) anyway, so that's the default already. Which means that I don't see the need for this PR. But that's for @tkralphs to follow-up.

[jhmgoossens]

I think it's easy to just overwrite the compiler flags if one needs MT(d).

Exactly.
But how? You mean let's achieve this not by using '--enable-msvc=XX', but another way? Problem is that configure always adds either -MD or -MDd, so simply adding "-MT" to compiler flags somehow will give duplicated conflicting options. Or?

[svigerske]

You can just set CFLAGS and CXXFLAGS. The one in configure are only the defaults that are used if none are set by the user.

[jhmgoossens]

Which means that I don't see the need for this PR.

LOL, I only now realize that Ted is trying to get "MD" builds, not "MT" builds. Should have been obvious from the PR title.

You're right, just use --enable-msvc without argument and that will make MD(d) builds. It's the MT builds via --enable-msvc=MT that don't work, but we don't need (for now). And yes, if anybody does need MT builds, then just set flags manually.

[jhmgoossens]

@tkralphs BLUF: Please Close this PR for master, because the current master already builds good MD binaries.

The current master already builds good MD binaries using simply --enable-msvc. I've double-checked the current master "-md" action artefacts, and confirm: the "-md.zip" binaries really are dynamically linked to the MS C runtime, which is what MD does.
The MT binaries produced by this PR are NOT good MT binaries.

Therefore, since the aim of this PR is to make MD binaries (not MT), this PR only makes things worse because the MTs dont work while the MD binaries are already OK.

I think Stefan understood this all along, but it took me a bit longer to get there..

[tkralphs]

Sorry for the delay! Yes, sorry, I knew, but temporarily forgot, that master is completely different, so this PR can indeed be closed. However, the reason for digging into this, as I mentioned, is to make sure we can build Python extensions. This works well in stable branches, as long as we have --enable-msvc=MD, but even though \MD is already the default in master, the DLLs that are produced by default are still not what we need for building extensions (I think). But this is a separate issue that we can address later.