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

[tensorflow-cc] fix build issues on Windows #12546

Merged
merged 11 commits into from
Aug 7, 2020

Conversation

jgehw
Copy link
Contributor

@jgehw jgehw commented Jul 23, 2020

occurring from default bazel config being used because customized bazel config is stored in wrong directory:

Omitting the --workspace parameter of configure.py defaults not the the current working directory but to the directory where configure.py is extracted to. This results in the customized bazel config being stored in the wrong directory and thus the default bazel config being used. This, in turn leads to tons of build errors, mainly of two types:

.\tensorflow/stream_executor/rng.h(66): error C2589: 'constant': illegal token on right side of '::'
.\tensorflow/stream_executor/rng.h(66): error C2059: syntax error: '::'
.\tensorflow/stream_executor/rng.h(72): error C2589: 'constant': illegal token on right side of '::'
.\tensorflow/stream_executor/rng.h(72): error C2059: syntax error: '::'

  1. tons of winsock2 related errors such as macro redefinitions and invalid constants, like those already reported in [tensorflow-cc:x64-windows] build failure #12506
    Closes [tensorflow-cc:x64-windows] build failure #12506

…se customized bazel config is stored in wrong directory
@LilyWangL LilyWangL self-assigned this Jul 24, 2020
@LilyWangL LilyWangL added the category:port-bug The issue is with a library, which is something the port should already support label Jul 24, 2020
@ghost
Copy link

ghost commented Jul 24, 2020

CLA assistant check
All CLA requirements met.

@JackBoosY
Copy link
Contributor

@jgehw Please sign CLA first.

Thanks.

@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jgehw
Copy link
Contributor Author

jgehw commented Jul 27, 2020

The CI runs on windows fail due to some access denied issue while setting up MSYS2. No idea what's going on there.
PS: I recall that the very first time I tried to install it in my environment also failed (it didn't crash but was hanging after setting up MSYS2). All subsequent runs however worked because MSYS2 was present then.

@JackBoosY
Copy link
Contributor

Will fix in #12603

@JackBoosY
Copy link
Contributor

When building x64-windows:

C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.26.28801\include\yvals_core.h(466): fatal error C1189: #error:  STL1001: Unexpected compiler version, expected MSVC 19.25 or newer.

jgehw added 2 commits July 30, 2020 19:35
… way it auto-detects include path avoiding mismatches between toolset and include files versions
@jgehw
Copy link
Contributor Author

jgehw commented Aug 3, 2020

Can you please re-run the failed x64_windows port? (I checked the build error logs and found a bazel package download error - bad timings because a server was down over the weekend. I had this error on my machine, too, and it disappeared after the server was back on.)

@jgehw
Copy link
Contributor Author

jgehw commented Aug 3, 2020

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 12546 in repo microsoft/vcpkg

@JackBoosY
Copy link
Contributor

Reruning...

@LilyWangL
Copy link
Contributor

@jgehw The new error:

Loading: 0 packages loaded
    currently loading: tensorflow
Analyzing: 2 targets (1 packages loaded, 0 targets configured)
INFO: An error occurred during the fetch of repository 'local_config_cc'
INFO: Call stack for the definition of repository 'local_config_cc':
 - D:/buildtrees/.bzl/_bazel_azdevops/zcqffzsa/external/bazel_tools/tools/cpp/cc_configure.bzl:97:5
 - /DEFAULT.WORKSPACE.SUFFIX:246:1
ERROR: no such package '@local_config_cc//': Traceback (most recent call last):
	File "D:/buildtrees/.bzl/_bazel_azdevops/zcqffzsa/external/bazel_tools/tools/cpp/cc_configure.bzl", line 52
		configure_windows_toolchain(repository_ctx)
	File "D:/buildtrees/.bzl/_bazel_azdevops/zcqffzsa/external/bazel_tools/tools/cpp/windows_cc_configure.bzl", line 349, in configure_windows_toolchain
		_find_missing_vc_tools(repository_ctx, vc_path)
	File "D:/buildtrees/.bzl/_bazel_azdevops/zcqffzsa/external/bazel_tools/tools/cpp/windows_cc_configure.bzl", line 248, in _find_missing_vc_tools
		_find_vcvarsall_bat_script(repository_ctx, vc_path)
	File "D:/buildtrees/.bzl/_bazel_azdevops/zcqffzsa/external/bazel_tools/tools/cpp/windows_cc_configure.bzl", line 191, in _find_vcvarsall_bat_script
		repository_ctx.path(vcvarsall).exists

… and tries to parse it), so fix the issue of inconsistency of toolset versions between auto-detection in vcpkg and bazel by letting vcpkg determine everything including toolset version
@jgehw
Copy link
Contributor Author

jgehw commented Aug 4, 2020

can you please re-run again? (the errors of the automated run seem to me to come from azure devops pipepline early stage, hopefully transient)

@LilyWangL
Copy link
Contributor

can you please re-run again? (the errors of the automated run seem to me to come from azure devops pipepline early stage, hopefully transient)

Done.

@JackBoosY
Copy link
Contributor

C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.26.28801\include\yvals_core.h(466): fatal error C1189: #error:  STL1001: Unexpected compiler version, expected MSVC 19.25 or newer.

jgehw and others added 3 commits August 5, 2020 20:25
It doesn't work if more than one VC toolkit is installed. Likely a bug in bazel. bazel is currently v3.4.1, however tensorflow v1.14 requires really outdated bazel v0.25. Going to upgrade tensorflow to v2.3 and then using up-to-date bazel, but will do this in a separate pull request, as it requires more work.
@jgehw
Copy link
Contributor Author

jgehw commented Aug 6, 2020

It turned out that tensorflow v1.14 requires really outdated bazel v0.25, so I reverted the bazel upgrade to 3.4.1 and set x64-windows CI to fail again. It only works if no more than one VC toolkit is installed. So the PR still has its value, but the CI won't succeed. As ultimate solution I'm going to upgrade tensorflow to v2.3 and then using up-to-date bazel, but will do this in a separate PR, as it requires more work.

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Aug 7, 2020
@strega-nil strega-nil merged commit 5945950 into microsoft:master Aug 7, 2020
@jgehw jgehw deleted the fix-ms-build-issues branch August 12, 2020 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[tensorflow-cc:x64-windows] build failure
5 participants