Fix _virtual_includes handling for absolute strip_include_prefix paths

[sfc-gh-abalik]

Checklist

• I have filed an issue about this change and discussed potential changes with the maintainers.
• I have received the approval from the maintainers to make this change.
• This is not a stylistic, refactoring, or cleanup change.

Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See
the Contributions section in the README for more
details.

Discussion thread for this change

Issue number: #5513

Description of this change

This change updates the VirtualIncludesHandler.java to detect if the stripPrefix is a repository-relative path (e.g. starts with '//') and if so, skips concatenating it to the package path. This enables CLion to find headers in targets that use repository-relative paths for strip_include_prefix.

I also fixed a small bug where the plugin raises an IllegalArgumentException when the strip_include_prefix ends with a '/'. To fix it we just need to trim the trailing '/' before creating the WorkspacePath object.

[ujohnny]

could you please make // a constant?

[ujohnny]

Thank you for submitting the PR and my apologies that I wasn't able to check the issue earlier. There's already a test in ExecutionRootPathResolverTest, could you please add extra tests there that test both issues mentioned?

[ujohnny]

also could you please make 2 a constant as well

[sfc-gh-abalik]

@ujohnny - I just pushed some commits to address feedback.

[sfc-gh-abalik]

@ujohnny - Could you take a look at the updates? I believe I've addressed all the feedback.

[ujohnny]

@ujohnny - Could you take a look at the updates? I believe I've addressed all the feedback.

My apologies for a little delay with reviews. Overall LGTM, though I spotted some caching issue that after upgrade to the version with the patch the header is still resolved incorrectly (from target with strip..//) unless I perform "Non-incremental sync".

[ujohnny]

Just merged it and spotted a minor issue. Could you please submit a PR with this simple change?

Index: base/src/com/google/idea/blaze/base/sync/workspace/VirtualIncludesHandler.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/base/src/com/google/idea/blaze/base/sync/workspace/VirtualIncludesHandler.java b/base/src/com/google/idea/blaze/base/sync/workspace/VirtualIncludesHandler.java
--- a/base/src/com/google/idea/blaze/base/sync/workspace/VirtualIncludesHandler.java	(revision f3554378b44dbae5d4252a7b694f55b6eb2fa3c7)
+++ b/base/src/com/google/idea/blaze/base/sync/workspace/VirtualIncludesHandler.java	(date 1707400715307)
@@ -102,7 +102,7 @@
             WorkspacePath stripPrefixWorkspacePath = stripPrefix.startsWith(ABSOLUTE_LABEL_PREFIX) ?
               new WorkspacePath(stripPrefix.substring(ABSOLUTE_LABEL_PREFIX_LENGTH)) :
               new WorkspacePath(key.getLabel().blazePackage(), stripPrefix);
-            if (key.getLabel().externalWorkspaceName() != null) {
+            if (externalWorkspaceName != null) {
                 ExecutionRootPath external = new ExecutionRootPath(
                     ExecutionRootPathResolver.externalPath.toPath()
                         .resolve(externalWorkspaceName)