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

Resolve Windows long path issue during C++ compilation #4149

Closed
meteorcloudy opened this issue Nov 22, 2017 · 32 comments
Closed

Resolve Windows long path issue during C++ compilation #4149

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

Comments

@meteorcloudy
Copy link
Member

https://stackoverflow.com/questions/47412512/bazel-build-nccl-archive-nccl-could-not-resolve-label-ws2-32-lib
When I tried to help a user build tensorflow_serving on Windows, I encountered an error Cannot open compiler generated file: '': Invalid argument:

  C:/Program Files (x86)/Microsoft Visual Studio 14.0/VC/bin/amd64/cl.exe /c external/org_tensorflow/tensorflow/core/profiler/internal/advisor/internal_checker_runner_dummy.cc /Fobazel-out/msvc_x64-py3-opt/bin/external/org_tensorflow/tensorflow/core/profiler/internal/advisor/_objs/internal_checker_runner_dummy/external/org_tensorflow/tensorflow/core/profiler/internal/advisor/internal_checker_runner_dummy.o /nologo /DCOMPILER_MSVC /DNOMINMAX /D_WIN32_WINNT=0x0600 /D_CRT_SECURE_NO_DEPRECATE /D_CRT_SECURE_NO_WARNINGS /D_SILENCE_STDEXT_HASH_DEPRECATION_WARNINGS /bigobj /Zm500 /J /Gy /GF /EHsc /wd4351 /wd4291 /wd4250 /wd4996 /Iexternal/org_tensorflow /Ibazel-out/msvc_x64-py3-opt/genfiles/external/org_tensorflow /Iexternal/protobuf_archive /Ibazel-out/msvc_x64-py3-opt/genfiles/external/protobuf_archive /Iexternal/bazel_tools /Ibazel-out/msvc_x64-py3-opt/genfiles/external/bazel_tools /Iexternal/com_google_absl /Ibazel-out/msvc_x64-py3-opt/genfiles/external/com_google_absl /Iexternal/nsync /Ibazel-out/msvc_x64-py3-opt/genfiles/external/nsync /Iexternal/eigen_archive /Ibazel-out/msvc_x64-py3-opt/genfiles/external/eigen_archive /Iexternal/local_config_sycl /Ibazel-out/msvc_x64-py3-opt/genfiles/external/local_config_sycl /Iexternal/gif_archive /Ibazel-out/msvc_x64-py3-opt/genfiles/external/gif_archive /Iexternal/jpeg /Ibazel-out/msvc_x64-py3-opt/genfiles/external/jpeg /Iexternal/com_googlesource_code_re2 /Ibazel-out/msvc_x64-py3-opt/genfiles/external/com_googlesource_code_re2 /Iexternal/farmhash_archive /Ibazel-out/msvc_x64-py3-opt/genfiles/external/farmhash_archive /Iexternal/fft2d /Ibazel-out/msvc_x64-py3-opt/genfiles/external/fft2d /Iexternal/highwayhash /Ibazel-out/msvc_x64-py3-opt/genfiles/external/highwayhash /Iexternal/png_archive /Ibazel-out/msvc_x64-py3-opt/genfiles/external/png_archive /Iexternal/zlib_archive /Ibazel-out/msvc_x64-py3-opt/genfiles/external/zlib_archive /Iexternal/snappy /Ibazel-out/msvc_x64-py3-opt/genfiles/external/snappy /Iexternal/protobuf_archive/src /Ibazel-out/msvc_x64-py3-opt/genfiles/external/protobuf_archive/src /Iexternal/bazel_tools/tools/cpp/gcc3 /Iexternal/nsync/public /Ibazel-out/msvc_x64-py3-opt/genfiles/external/nsync/public /Iexternal/eigen_archive /Ibazel-out/msvc_x64-py3-opt/genfiles/external/eigen_archive /Iexternal/gif_archive/lib /Ibazel-out/msvc_x64-py3-opt/genfiles/external/gif_archive/lib /Iexternal/gif_archive/windows /Ibazel-out/msvc_x64-py3-opt/genfiles/external/gif_archive/windows /Iexternal/farmhash_archive/src /Ibazel-out/msvc_x64-py3-opt/genfiles/external/farmhash_archive/src /Iexternal/png_archive /Ibazel-out/msvc_x64-py3-opt/genfiles/external/png_archive /Iexternal/zlib_archive /Ibazel-out/msvc_x64-py3-opt/genfiles/external/zlib_archive /D__CLANG_SUPPORT_DYN_ANNOTATION__ /DEIGEN_MPL2_ONLY /DTENSORFLOW_USE_ABSL /DTF_USE_SNAPPY /showIncludes /MD /O2.
C:\tmp\_bazel_pcloudy\dpitg86y\execroot\tf_serving\external\org_tensorflow\tensorflow\core\profiler\internal\advisor\internal_checker_runner_dummy.cc : fatal error C1083: Cannot open compiler generated file: '': Invalid argument

It turns out that the object file the compiler trying to write C:/tmp/_bazel_pcloudy/dpitg86y/execroot/bazel-out/msvc_x64-py3-opt/bin/external/org_tensorflow/tensorflow/core/profiler/internal/advisor/_objs/internal_checker_runner_dummy/external/org_tensorflow/tensorflow/core/profiler/internal/advisor/internal_checker_runner_dummy.o is longer than 270 characters.

This happens when I already set the output directory to a short path C:/tmp.

@meteorcloudy meteorcloudy added platform: windows P2 We'll consider working on this in future. (Assignee optional) type: feature request labels Nov 22, 2017
@laszlocsomor
Copy link
Contributor

I ran into the same limitation while working on #4148.

@ulfjack mentioned on 16/11/2017 on an internal mail thread that we are looking into shortening C++ output paths. Here, as well as with #4148, we are duplicating the package path in the output path. AFAICT this is only necessary when cc_*.srcs includes two files with the same basename.

I don't know Bazel's C++ compilation machinery enough to tell with certainty whether we could change these output paths, say, to bazel-out/configname/path/to/rule._objs/basename.o and only include the package name of the source file if there'd be a basename.o clash otherwise.

@laszlocsomor
Copy link
Contributor

paging Mr. @mhlopko : what problems do you think we have to solve to get to the state I mentioned in my previous comment?

@meteorcloudy
Copy link
Member Author

Can we use a hash string under _objs instead of the full package path?
Something like C:/tmp/_bazel_pcloudy/dpitg86y/execroot/bazel-out/msvc_x64-py3-opt/bin/external/org_tensorflow/tensorflow/core/profiler/internal/advisor/_objs/<short_and_unique_hash>/internal_checker_runner_dummy.o

@laszlocsomor
Copy link
Contributor

laszlocsomor commented Nov 22, 2017

We could as well include the object file's name in the hash, so it'd be c:/<output_root>/bazel-out/<configname>/bin/<cc_rule_package_path>/_objs/<hash_of_cc_rule_name_and_root_relative_path_of_source>.o

@hlopko
Copy link
Member

hlopko commented Nov 22, 2017

Unfortunately I have no idea. I didn't even go that far to find out that we duplicate the path when there are multiple srcs with same basename. How sure are you this is the only case? I was also more interested in _solib contents, I'm not sure how much overlap there is.

In other words, I'm useless to you here :)

@laszlocsomor
Copy link
Contributor

laszlocsomor commented Nov 22, 2017

How sure are you this is the only case?

I'm not sure it's the only case, or that it's even the reason we compute paths the way we do. It's just a reason I could think of.

In other words, I'm useless to you here :)

I think you're wrong :P

@laszlocsomor
Copy link
Contributor

I'm considering this for Q1-2018 because our users have reported that lack of good documentation and best practices is a real pain.

However, I'm leaving it as P2, because there are workaround:

  • --output_user_root=x:/ or
  • shortening target names, or
  • making the package tree flatter.

@excitoon
Copy link
Contributor

excitoon commented Jan 31, 2018

I've just encountered this issue by myself. I'd like to add that it can be solved in cl python wrapper substituting paths with shorter paths using symlinks.
Also I've created small repository to easily reproduce this issue:
https://github.com/excitoon/bazel-issues/tree/master/windows-long-file-paths-of-objects
Currently it blocks our company from moving our projects to bazel -_-.

@laszlocsomor
Copy link
Contributor

@excitoon : Thanks for the repro case! We are actively looking for solutions, currently evaluating different approaches. This is one of our highest priority bugs.

@laszlocsomor laszlocsomor added P1 I'll work on this now. (Assignee required) and removed P2 We'll consider working on this in future. (Assignee optional) labels Jan 31, 2018
@laszlocsomor
Copy link
Contributor

Bumping to P1 because workarounds only get us so far, and it's only a matter of time before more people hit this bug. (Bazel itself started hitting it this week.)

@davido
Copy link
Contributor

davido commented Jan 31, 2018

We are actively looking for solutions, currently evaluating different approaches.

Have you considered to consequently write response files and then calling cl with "@responsefile.rsp"?

@laszlocsomor
Copy link
Contributor

Does that enable the compiler/linker to deal with long paths?

@rongjiecomputer
Copy link
Contributor

I've just encountered this issue by myself. I'd like to add that it can be solved in cl python wrapper substituting paths with shorter paths using symlinks.

Invocating Python is pretty slow, and the time will quickly adds up with 1000+ compilation units. Wrapper script is not an ideal solution IMO.

Does that enable the compiler/linker to deal with long paths?

No.

In C:/tmp/_bazel_pcloudy/dpitg86y/execroot/bazel-out/msvc_x64-py3-opt/bin/external/org_tensorflow/tensorflow/core/profiler/internal/advisor/_objs/internal_checker_runner_dummy/external/org_tensorflow/tensorflow/core/profiler/internal/advisor/internal_checker_runner_dummy.o, we can clearly see that external/org_tensorflow/tensorflow/core/profiler/internal/advisor is repeated twice, this is as good as halving the already-very-short 270 limit.

In CMake, it will probably only something like C:/tensorflow/build/tensorflow/core/CMakeFiles/profiler/internal/advisor.dir/internal_checker_runner_dummy.o

@meteorcloudy
Copy link
Member Author

Invocating Python is pretty slow, and the time will quickly adds up with 1000+ compilation units. Wrapper script is not an ideal solution IMO.

I agree, using a wrapper script is not a solution under discussion. We're focusing on how to short the object file path in Bazel.

@excitoon
Copy link
Contributor

excitoon commented Jan 31, 2018

Invocating Python is pretty slow, and the time will quickly adds up with 1000+ compilation units. Wrapper script is not an ideal solution IMO.

That's right, but Python wrapper is already being invoked for each unit on MSVS (it does some flags substitutions and has other logic).

@laszlocsomor
Copy link
Contributor

@excitoon : No, there's no Python wrapper for MSVC anymore. Bazel runs the compiler/linker directly.

@meteorcloudy
Copy link
Member Author

That's right, but Python wrapper is already being invoked for each unit on MSVS (it does some flags substitution and has other logic)

No, the wrapper script has been removed since 0.5.3, see 425f249

@laszlocsomor
Copy link
Contributor

@rongjiecomputer :

we can clearly see that external/org_tensorflow/tensorflow/core/profiler/internal/advisor is repeated twice, this is as good as halving the already-very-short 270 limit.

Yes, we made the same observation. @meteorcloudy is currently designing different ways to shorten the object file output paths.

Yes, CMake would probably just put the object file next to the source file, which is fine only as long as you don't compile the file with multiple compilers and/or different compiler options within the same build.

@excitoon
Copy link
Contributor

I guess, as C++ application, cl.exe is compiled with MAX_PATH=260 inside anyway and therefore that registry key won't affect it.

@laszlocsomor
Copy link
Contributor

Yes, @excitoon makes a good point.
We can't convince cl.exe to accept long paths, not even with a wrapper, nor with the registry key you mentioned.

meteorcloudy added a commit to meteorcloudy/bazel that referenced this issue Mar 20, 2018
Object file path will no longer be derived from source file path directly.
This is a preparation change for[]

Related issue bazelbuild#4149

RELNOTES: None
PiperOrigin-RevId: 189572213
Change-Id: Iae7ee48e5f1f5500ae03999618598eb9475eaed8
bazel-io pushed a commit that referenced this issue Mar 20, 2018
Object file path will no longer be derived from source file path directly.
This is a preparation change for[]

Related issue #4149

RELNOTES: None
PiperOrigin-RevId: 189722421
@meteorcloudy
Copy link
Member Author

Hey, everyone, this issue has been resolved by bb9ae6a.
You can now use --experimental_shortened_obj_file_path option to have a short object file path with Bazel@HEAD. This change will be released with 0.13.0 and the short object file will be default in the future. For more details of this change, please read the commit message.

@rongjiecomputer
Copy link
Contributor

@meteorcloudy Just boostraped Bazel with this flag on and it works flawlessly! Will try to build Tensorflow with this flag when I am free (though it looks like Tensorflow's Bazel Windows CI is red again).

I notice that the change only affects C++. com\google\devtools\build\android repeats twice in C:\Users\me\GitHub\bazel\bazel-bin\src\tools\android\java\com\google\devtools\build\android\_javac\android_builder_lib\libandroid_builder_lib_classes\com\google\devtools\build\android. Since Java package name also can be very long, I think at some point we will see a Java project hitting this issue like C++ project.

@hlopko
Copy link
Member

hlopko commented Mar 28, 2018

\o/

@meteorcloudy
Copy link
Member Author

@rongjiecomputer Glad to hear it worked! I'm working on fixing the TF's Windows build and setting up a presubmit for it. Should be done next week.

Yes, this change only affected C++, because it's only cl.exe (and other MSVC build tools) that doesn't support long path. I tested with java.exe and javac.exe, they both accept long path. So we are saved here.

@RickVM
Copy link

RickVM commented Apr 6, 2018

@rongjiecomputer how did you bootstrap Bazel?
I tried downloading the current git repo and use msys2 to execute compile.sh but this returns:
ERROR: Must specify PROTOC if not bootstrapping from the distribution artifact
where it suggests to use bazel to build from source. Or use the developer checkout as output files are not included.

I tried to build from source with the latest bazel release (0.11) and after 700 seconds of downloading the very first package of the build list i get an error stating there is no such package as androidsdk.

I figure this is not the right path and I'm most likely missing something here as I'm new to bazel and just trying to install Tensorflow serving, for which I need the fix that will be implemented in 0.13.

@rongjiecomputer
Copy link
Contributor

@RickVM I just run bazel build -c opt --experimental_shortened_obj_file_path=true //src:bazel.exe directly in the git repo.

luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
    Closes #4781.
    Fix bazelbuild/bazel#4149

    RELNOTES:
    Users can now pass --experimental_shortened_obj_file_path=true to have a shorter object file path, the object file paths (and all other related paths) will be constructed as following:
    If there's no two or more source files with the same base name:
      <bazel-bin>/<target_package_path>/_objs/<target_name>/<source_base_name>.<extension>
    otherwise:
      <bazel-bin>/<target_package_path>/_objs/<target_name>/N/<source_base_name>.<extension>
      N = the file?s order among the source files with the same basename, starts from 0.

    Examples:
    1. Output names for ["lib1/foo.cc", "lib2/bar.cc"] are ["foo", "bar"]
    2. Output names for ["foo.cc", "bar.cc", "foo.cpp", "lib/foo.cc"] are
       ["0/foo", "bar", "1/foo", "2/foo"]

    The default value of --experimental_shortened_obj_file_path option is false, but we plan to flip it to true and eventually remove this option.
    You shouldn't depend on the format of generated object file path, but if you do and this change breaks you, please use --experimental_shortened_obj_file_path=false to work around it.

    PiperOrigin-RevId: 190214375
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
    Object file path will no longer be derived from source file path directly.
    This is a preparation change for[]

    Related issue bazelbuild/bazel#4149

    RELNOTES: None
    PiperOrigin-RevId: 189722421
@laramiel
Copy link

A note here, the above link is due to the constructed include path being too long. It would be possible to shorten those paths if, when bazel constructed a virtual includes path, instead of constructing it here:

<bazel_output_base>/<hash>/execroot/<root workspace>/bazel-out/x64_windows-<config>/bin/external/com_google_protobuf/src/google/protobuf/compiler/objectivec/_virtual_includes/objectivec/google/protobuf/compiler/objectivec/text_format_decode_data.h

It constructed as a new path under bazel_output_base:

<bazel_output_base>/<hash>/_virtual/<new-hash>/google/protobuf/compiler/objectivec/text_format_decode_data.h

Then the include path would readlink that instead of using the files directly.

@meteorcloudy
Copy link
Member Author

@laramiel This looks like a nice improvement for Bazel, can you file a separate issue as feature request?

copybara-service bot pushed a commit to protocolbuffers/protobuf that referenced this issue May 4, 2024
The idea here is to set the existing config "config_msvc" not only
when "msvc-cl" is specified but also when "clang-cl" is specified.

Supporting clang-cl should be a quick workaround for those who look for
a quick solution to cl.exe's path limitation e.g.

 * bazelbuild/bazel#4149
 * bazelbuild/bazel#18683

PiperOrigin-RevId: 630590450
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

9 participants