Optimize _virtual_includes when paths are only stripped#671
Optimize _virtual_includes when paths are only stripped#671keith wants to merge 3 commits intobazelbuild:mainfrom
Conversation
keith
requested review from
c-mita,
hvadehra,
pzembrod and
trybka
as code owners
|
im resurrecting this because of my comment here bazelbuild/bazel#17591 (comment) even with sandboxing disabled the |
In this case it's not neccessary to create _virtual_includes, just setting the correct include path is enough. This should alleviate Windows problems with too long paths. This is a reapply of bazelbuild/bazel@a091d90 Fixes bazelbuild/bazel#17591
keith
force-pushed
the
ks/optimize-_virtual_includes-when-paths-are-only-stripped
branch
from
0d37055 to
413d632
Compare
armandomontanez
added
P1
|
The original bazelbuild/bazel@a091d90 got rolled back due to a change in include order, though whatever context the rollback had is gone now. |
|
huh do you know if that case was a non-sandboxed execution? i would assume this would stay in the same order as before, but if it's not sandboxed there could be more there (which should then fail layering_check or dotd validation) |
|
that was part of the original rollback as well, but im still feeling like non-sandboxed executions that also disabled layering_check and dotd validation should be more important that this potential optimization |
|
maybe a nice way to introduce this would be to put it behind a feature that must be enabled by the toolchain. i think then i would add that feature to the default toolchains, off by default, and users could opt in based on this tradeoff. we could also automatically enable it if dotd file validation or layering_check was enabled too |
This must be enabled by default to allow skipping this. We auto-enable it if you are using dotd validation, or layering_check, since those are better mechanisms for catching these issues, and those work even if sandboxing is disabled.
|
a threw in a feature for this, and enabled it by default assuming some of the other features are enabled. the checking for that isn't perfect since rules_cc does a lot of post processing, so maybe we just don't want to do that piece and make users opt in |
|
I decided it was nicer to just not try to be smart about it and entirely leave it up to the user to enable |
|
unrelated test failure, possibly fixed by #663 |
|
#663 should have gone in a while ago, checking what's going on with copybara for that. |
3 participants
In this case it's not neccessary to create _virtual_includes, just
setting the correct include path is enough.
This should alleviate Windows problems with too long paths.
This is a reapply of bazelbuild/bazel@a091d90
Fixes bazelbuild/bazel#17591