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

Cannot use dynamic runtime (/MD) in Windows #2120

Closed
javidcf opened this issue Nov 22, 2016 · 10 comments
Closed

Cannot use dynamic runtime (/MD) in Windows #2120

javidcf opened this issue Nov 22, 2016 · 10 comments
Assignees
Labels
P1 I'll work on this now. (Assignee required) platform: windows type: feature request
Milestone

Comments

@javidcf
Copy link
Contributor

javidcf commented Nov 22, 2016

Looking at msvc_tools.py.tpl, it does not seem to be possible to compile on Windows using the dynamic runtime (that is, using /MD / /MDd and LIBCMT). Even if one adds /MD to the copts of some rule (because passing --copt="/MD" does not even get correctly processed, but that's a different issue), the Python wrapper will add /MT or /MTd, overriding the option. I am trying to compile TensorFlow + some additional code as static libraries to integrate in a bigger project that uses /MD, and with the current behavior it's not possible.

The obvious fix would be to add an additional check to the script before self.AddOpt('/MT'), but I don't really know if there are any other reasons behind this. It seems to me that, if anything, /MD should be favored as the default option (which is what Visual Studio does nowadays).

I am using Bazel 0.4.0 (installed through Chocolatey) in x64 machine with Win10. My build command line looks like:

$ bazel build -c opt --cpu=x64_windows_msvc --host_cpu=x64_windows_msvc --verbose_failures --experimental_ui <target...>
@dslomov
Copy link
Contributor

dslomov commented Nov 23, 2016

@meteorcloudy, wdyt?

@meteorcloudy
Copy link
Member

I think only using /MT is just a legacy behavior after I created the wrapper script, I don't see anything special reason behind this. So, I think we can just add an additional check before adding /MT.

@javidcf
Copy link
Contributor Author

javidcf commented Nov 23, 2016

Sorry I forgot to mention this before, but I also noted that msvc_cl.py translates the copt -g to /MTd. I guess ideally it should translate to either /MTd or /MDd depending on the existing options, but that would require further changes I guess.

javidcf added a commit to javidcf/bazel that referenced this issue Nov 28, 2016
msvc_tools.py now prefers the last /MT / /MD option given; if none,
/MD is preferred.

Additionally, the behaviour of the copt -g has been improved to enforce
the debug version of the user-selected runtime, not necessarily /MTd.
@javidcf
Copy link
Contributor Author

javidcf commented Nov 28, 2016

I have sent a PR to better handle /MT and /MD, but I have just realized msvc_link.py also seems to link to libcmt.lib / libcmtd.lib by default. I'm not sure how important this is, though, because I am being able to create a DLL containing TensorFlow and use it in another application that uses /MD using my PR. However maybe the linker script should be adapted too somehow.

@dslomov dslomov added this to the 0.5 milestone Dec 6, 2016
@dslomov dslomov added the P1 I'll work on this now. (Assignee required) label Dec 6, 2016
bazel-io pushed a commit that referenced this issue Dec 7, 2016
msvc_tools.py now prefers the last /MT / /MD option given; if none,
/MD is preferred.

Additionally, the behaviour of the copt -g has been improved to enforce
the debug version of the user-selected runtime, not necessarily /MTd.

Issue: #2120 

Closes #2141.

--
Reviewed-on: #2141
PiperOrigin-RevId: 141294930
MOS_MIGRATED_REVID=141294930
@steven-johnson
Copy link
Contributor

AFAICT, this still isn't workable in 0.4.5; I'm building against a library that requires /MD set, and though I am building all of my code with /MD as well, the code in msvc_link.py still assumes we want to link against a libcmt variant (rather than an msvcrt variant); in my case, this produces lots of c-runtime-related link errors.

I experimentally hacked my msvc_link.py to link against the MSVCRT.lib instead, and the link errors went away... but I can't find a way to do this without such hackage. Is there a way to fix this otherwise? Is it fixed for 0.5? If not, can it be?

@javidcf
Copy link
Contributor Author

javidcf commented Apr 19, 2017

@steven-johnson I have recently been able to make it work for TensorFlow building Bazel from source and then, when compiling TensorFlow, passing --copt=/MD to bazel build.

@steven-johnson
Copy link
Contributor

javidcf@ That's good to know, but "building Bazel from source" should not be a prerequisite for being able to make this use case work :-)

@steven-johnson
Copy link
Contributor

I think this should perhaps be reopened to fix the linker issue -- it's quite possible I'm doing something wrong, but I can't get a cc_binary to link reliably when specifying /MD and linking against another (non-Bazel) Windows library that is also compiled with /MD.

bazel-io pushed a commit that referenced this issue Apr 24, 2017
See Issue #2120: if we specify /MD for copts, we should attempt to use
MSVCRTx.lib instead of LIBCMTx.lib.

Closes #2862.

PiperOrigin-RevId: 154032031
bazel-io pushed a commit that referenced this issue Apr 24, 2017
1. Moved /nologo flag into feature
2. No need to specify -m64, adding /MACHINVE:X64 as linker flag
3. Still use wrapper script to add /MT or /MD for now, because our users
   are depending on it: #2120
   We need a plan first before move them into CROSSTOOL

Change-Id: If5e4c01a900fcf9e93877e04a893879897bff3a3
PiperOrigin-RevId: 154036870
bazel-io pushed a commit that referenced this issue May 12, 2017
By default, we use /MT(/MTd for debug mode) and link to
libcmt.lib(libcmtd.lib).

Users can set USE_DYNAMIC_CRT=1 or add --action_env=USE_DYNAMIC_CRT=1 to
switch to /MD and msvcrt.lib (/MDd and msvcrtd.lib for debug mode)

Reference: https://msdn.microsoft.com/en-us/library/abx4dbyh.aspx

Fixed #2120

Change-Id: I61e65ace82163acd456bf82f2b108c5fe8d8a8ce
PiperOrigin-RevId: 155850886
hlopko pushed a commit that referenced this issue May 23, 2017
By default, we use /MT(/MTd for debug mode) and link to
libcmt.lib(libcmtd.lib).

Users can set USE_DYNAMIC_CRT=1 or add --action_env=USE_DYNAMIC_CRT=1 to
switch to /MD and msvcrt.lib (/MDd and msvcrtd.lib for debug mode)

Reference: https://msdn.microsoft.com/en-us/library/abx4dbyh.aspx

Fixed #2120

Change-Id: I61e65ace82163acd456bf82f2b108c5fe8d8a8ce
PiperOrigin-RevId: 155850886
damienmg pushed a commit that referenced this issue May 26, 2017
By default, we use /MT(/MTd for debug mode) and link to
libcmt.lib(libcmtd.lib).

Users can set USE_DYNAMIC_CRT=1 or add --action_env=USE_DYNAMIC_CRT=1 to
switch to /MD and msvcrt.lib (/MDd and msvcrtd.lib for debug mode)

Reference: https://msdn.microsoft.com/en-us/library/abx4dbyh.aspx

Fixed #2120

Change-Id: I61e65ace82163acd456bf82f2b108c5fe8d8a8ce
PiperOrigin-RevId: 155850886
hlopko pushed a commit that referenced this issue May 31, 2017
By default, we use /MT(/MTd for debug mode) and link to
libcmt.lib(libcmtd.lib).

Users can set USE_DYNAMIC_CRT=1 or add --action_env=USE_DYNAMIC_CRT=1 to
switch to /MD and msvcrt.lib (/MDd and msvcrtd.lib for debug mode)

Reference: https://msdn.microsoft.com/en-us/library/abx4dbyh.aspx

Fixed #2120

Change-Id: I61e65ace82163acd456bf82f2b108c5fe8d8a8ce
PiperOrigin-RevId: 155850886
@fire
Copy link

fire commented Aug 1, 2017

USE_DYNAMIC_CRT=1 Is the way I worked around this, but I don't know how to encode it into the BUILD.

@wangjia184
Copy link

For bazel v5.3, use --features=static_link_msvcrt parameter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) platform: windows type: feature request
Projects
None yet
Development

No branches or pull requests

6 participants