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

windows: Add build verification step to ensure path lengths #10581

Open
tonya11en opened this issue Mar 30, 2020 · 5 comments
Open

windows: Add build verification step to ensure path lengths #10581

tonya11en opened this issue Mar 30, 2020 · 5 comments
Labels

Comments

@tonya11en
Copy link
Member

The Windows path length limit is 260 characters and can potentially be exceeded as seen in #10560. The compiler error emitted when the max path length is exceeded does not point to the actual problem by outputting No such file or directory. It is unclear if this is the only behavior.

Ideally, there would be an explicit check for these path lengths during the Windows build and failure output that clearly indicates the problem.

@sunjayBhatia
Copy link
Member

@sunjayBhatia
Copy link
Member

sunjayBhatia commented Mar 30, 2020

could this be a check in bazel/bazel issue instead of envoy? it was supposedly resolved here: bazelbuild/bazel#4149 for object files

(also b/c doing a check on each build rule/source file in envoy builds seems expensive and a lot of effort, looking into all the params files for each library etc.)

@htuch
Copy link
Member

htuch commented Mar 31, 2020

CC @wrowe

@wrowe
Copy link
Contributor

wrowe commented Mar 31, 2020

I'd respectfully suggest that this is not a problem for Envoy to resolve. Envoy isn't a build system, Envoy doesn't specify the use of component path elements in triplicate. Quoting sunjay;

its not the include path in the file, it’s the resulting bazel build tree that’s the issue and the file path that ends up in the cl.exe params file, e.g the include param for the gradient_controller.h file becomes: -Ibazel-out/x64_windows-opt/bin/source/extensions/filters/http/adaptive_concurrency/concurrency_controller/_virtual_includes/concurrency_controller_lib + (the rest of the path to the file) \_virtual_includes\concurrency_controller_lib\extensions\filters\http\adaptive_concurrency\concurrency_controller\gradient_controller.h (total length of this minus the /I is 285)
Shaving down the redundant concurrency_ part of the path reduces this length quite a bit

These design challenges live at Bazel.

It seems we need a new issue at bazelbuild/bazel to call out this triplification of pathname elts, and warn as include paths are growing excessively unusable, and provide a more helpful message than No such file or directory?

In more than 9 months of my participation porting Envoy to Windows, this is the first instance of an excessive path -I'nclude string, so I don't think it is a high-priority issue, but it is absolutely worth documenting for future searchers of the same exception, at Envoy, but across all Bazel users as well.

We have a larger issue that the COFF object file format for a debug object embeds the full compilation command line invocation, and that has a hard limit too of 48kb. We have been bouncing off of that limit, but it has as much to do with the number of unused -I'ncludes appended to the compilation command, and overly verbose choices by bazel such as the pathname elt '_virtual_includes` occurring dozens if not hundreds of time on the compilation command line. Again, these seem like bazel's challenges to address.

@laramiel
Copy link

laramiel commented Dec 5, 2023

Here's another proto which could use some trimming as in #10560.

envoy/extensions/load_balancing_policies/client_side_weighted_round_robin/v3/client_side_weighted_round_robin.proto

w.r.t your suggestion that it's a bazel thing, I filed this bug: bazelbuild/bazel#18683

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants