Inclusion validation check should tolerate source file absolute paths

[amberdixon]

Sometimes clang emits absolute paths to source files in dot-d output. Fixes #14346

[jerrymarino]

Thanks for fixing this, this is definitely a blocker for Bazel 5. Is there any way we can get a test case in to ensure modules pass validation E2E and don't regress in the future? I think the module tests are pretty slim in Bazel.

Also I think a lot of the iOS community using C++ / ObjC is using modules: PodToBUILD, rules_ios and other rule sets are often enabling them by default

[googlewalt]

See my comment in #14346.

[Wyverald]

Thanks for fixing this, this is definitely a blocker for Bazel 5.

To confirm, why is this a blocker for 5.0? Is this behavior not present in Bazel 4.x?

[Wyverald]

Ah, sorry, I just saw https://github.com/bazelbuild/bazel/issues/14346#issuecomment-985911331. It seems that it's actually a release blocker then.

[amberdixon]

Hi! There is another situation where we run into problems with inclusion validation. I think it would be better for you to review this other PR instead which adds a feature flag to disable inclusion validation entirely: https://github.com/bazelbuild/bazel/pull/14394 Thanks!

[amberdixon]

Please consider #14394 instead. Thanks!!

[amberdixon]

Either this PR or the other one to disable inclusion validation (#14394) will unblock us.

However we strongly prefer that you merge the other PR (which is #14394), because the existing inclusion validation logic will likely create problems for other developers.

[amberdixon]

You can take this change under consideration in the future, if you decide to reenable the inclusion validation check for objective-c.

[amberdixon]

Going to reopen this PR, because it correctly addresses the problems we had with the inclusion validation logic (which has been disabled for objective-c in bazel5-rc3 for the time being.)

[burnpanck]

Would be great to get an actual solution for #14346 that doesn't just disable the check for obj-C. We're currently blocked by this and frantically trying to find a workaround, but so far unsuccessful. We experience this issue with C++ using clang, not even with -fmodules.

[protos-gunzinger]

Hi @burnpanck , thank you for your investigation; I am currently hitting the same roadblock when trying to use a custom toolchain with clang (for embedded ARM: https://github.com/ARM-software/LLVM-embedded-toolchain-for-Arm ). Did you find a workaround/solution for handling the absolute paths clangs outputs into the *.d files?

[burnpanck]

Nice - I am also using that embedded ARM toolchain, and it was in that context where I first encountered this issue. My current workaround is a pretty sad one: A custom built bazel using a single change, here is the diff:

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 5dfb93b07d..46de5bc212 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -287,7 +287,7 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
     this.executionInfo = executionInfo;
     this.actionName = actionName;
     this.featureConfiguration = featureConfiguration;
-    this.needsIncludeValidation = cppSemantics.needsIncludeValidation();
+    this.needsIncludeValidation = false; // cppSemantics.needsIncludeValidation();
     this.builtInIncludeDirectories = builtInIncludeDirectories;
     this.additionalInputs = null;
     this.usedModules = null;

You see, this is just a hacky way of completely disabling inclusion validation.

[protos-gunzinger]

Is there anything blocking this merge form maintainer-side? @googlewalt Or a simpler way to disable include validation, maybe in the .bazelrc or as a command line option?

Thank you in advance, it would be really awesome to be able to use some modern LLVM/Clang tooling for embedded development targets (and many others) :)

I think this is also more generally blocking usage of clang on windows (when not using the msvc clang-cl), so it would be groundwork for lots of interesting projects/approaches to ease this pain.