Windows: include validation broken with MSVC and long paths

laszlocsomor commented 7 years ago

Description of the problem / feature request / question:

If the exec root has a shortened path in it (e.g. C:/users/laszlocsomor/... -> C:/users/laszlo~1/...) then include validation fails because the execution root uses the shortened path segment and the dotD file uses long path names.

If possible, provide a minimal example to reproduce the problem:

Add logging above this line:

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java
index 5b59a06..3f5e44d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java
@@ -116,6 +116,8 @@ public class HeaderDiscovery {
         if (execPath.startsWith(execRoot)) {
           execPathFragment = execPath.relativeTo(execRoot); // funky but tolerable path
         } else {
+          System.err.printf("DEBUG: execPath=(%s) is not under execRoot=(%s)\n",
+              execPathFragment, execRoot);
           problems.add(execPathFragment.getPathString());
           continue;
         }

Build bazel:

$ bazel build src:bazel
$ export DEVBAZEL=$(realpath bazel-bin/src/bazel)

Create a workspace with a simple cc_library rule and a cc file including a header:

$ mkdir -p /c/blah/{workspace,long_user_root}

$ touch /c/blah/workspace/WORKSPACE

$ echo "cc_library(name='foo', srcs=['foo.cc'], hdrs=['foo.h'])" >/c/blah/workspace/BUILD

$ touch /c/blah/workspace/foo.h

$ cat >/c/blah/workspace/foo.cc <<EOF
#include "foo.h"
int main(int, char**) { return 0; }
EOF

Observe behavior with different user base paths, mind the differences in the --output_user_root values:

$ cd /c/blah/workspace

$ $DEVBAZEL --batch --output_user_root=/c/blah/long_u~1/ build //:foo --cpu=x64_windows_msvc
...
DEBUG: execPath=(C:/blah/long_user_root/h56$itsg/execroot/workspace/foo.h) is not under execRoot=(C:/blah/long_u~1/h56$itSG/execroot/workspace)
ERROR: C:/blah/workspace/BUILD:1:1: undeclared inclusion(s) in rule '//:foo':
this rule is missing dependency declarations for the following files included by 'foo.cc':
  'C:/blah/long_user_root/h56$itsg/execroot/workspace/foo.h'
Target //:foo failed to build
...

$ $DEVBAZEL --batch --output_user_root=/c/blah/long_user_root/ build //:foo --cpu=x64_windows_msvc
...
INFO: Build completed successfully, 3 total actions

Environment info

Operating System: Windows 10
If bazel info release returns "development version" or "(@non-git)", please tell us what source tree you compiled Bazel from; git commit hash is appreciated (git rev-parse HEAD):
```
$ git rev-parse HEAD
4f3d7780dbadc36fdc3c5992d3d094e086b9c619
```

Have you found anything relevant by searching the web? (e.g. GitHub issues, email threads in the bazel-discuss@googlegroups.com archive)

no

Anything else, information or logs or outputs that would be helpful?

(If they are large, please upload as attachment or provide link).

laszlocsomor commented 7 years ago

FYI: @meteorcloudy , @dslomov

laszlocsomor commented 7 years ago

Workaround: use --output_user_root=/c/foo, i.e. some path with no abbreviated components.

laszlocsomor commented 7 years ago

I have a fix for this, stay tuned.

laszlocsomor commented 7 years ago

Bad news: the Java File API that I was using to resolve short names seems to cache the results of getAbsolute{Path,File} and getCanonical{Path,File}, so if a previously nonexistent path ("c:/longpa~1") is later created such that the resolution should succeed ("c:/longpathname"), it still doesn't, and similarly if a previously existent path is deleted, the canonical form can still be retrieved.

Test:

  public static void main(String[] args) throws IOException  {
    File f = new File("C:\\longpa~1");
    System.out.printf(
        "Before creating c:\\longpathname:\n" +
        "  f: absolute=%s, canonical=%s, exists=%d\n",
        f.getAbsolutePath(), f.getCanonicalPath(), f.exists() ? 1 : 0);

    File g = new File("C:\\longpathname");
    g.mkdir();
    System.out.printf(
        "After creating c:\\longpathname:\n" +
        "  g: absolute=%s, canonical=%s, exists=%d\n",
        g.getAbsolutePath(), g.getCanonicalPath(), g.exists() ? 1 : 0);
    System.out.printf(
        "  f: absolute=%s, canonical=%s, exists=%d\n",
        f.getAbsolutePath(), f.getCanonicalPath(), f.exists() ? 1 : 0);

    File f2 = new File("C:\\longpa~1");
    System.out.printf(
        "  f2: absolute=%s, canonical=%s, exists=%d\n",
        f2.getAbsolutePath(), f2.getCanonicalPath(), f2.exists() ? 1 : 0);

    g.delete();
    System.out.printf(
        "After deleting c:\\longpathname:\n" +
        "  g: absolute=%s, canonical=%s, exists=%d\n",
        g.getAbsolutePath(), g.getCanonicalPath(), g.exists() ? 1 : 0);

    File g2 = new File("C:\\longpathname");
    System.out.printf(
        "  g2: absolute=%s, canonical=%s, exists=%d\n",
        g2.getAbsolutePath(), g2.getCanonicalPath(), g2.exists() ? 1 : 0);

    File h = new File("c:\\longpathn");
    h.mkdir();
    System.out.printf(
        "After creating c:\\longpathn:\n" +
        "  h: absolute=%s, canonical=%s, exists=%d\n",
        h.getAbsolutePath(), h.getCanonicalPath(), h.exists() ? 1 : 0);

    File f3 = new File("C:\\longpa~1");
    System.out.printf(
        "  f3: absolute=%s, canonical=%s, exists=%d\n",
        f3.getAbsolutePath(), f3.getCanonicalPath(), f3.exists() ? 1 : 0);
    h.delete();
  }

Output:

Before creating c:\longpathname:
  f: absolute=C:\longpa~1, canonical=C:\longpa~1, exists=0
After creating c:\longpathname:
  g: absolute=C:\longpathname, canonical=C:\longpathname, exists=1
  f: absolute=C:\longpa~1, canonical=C:\longpa~1, exists=1
  f2: absolute=C:\longpa~1, canonical=C:\longpa~1, exists=1
After deleting c:\longpathname:
  g: absolute=C:\longpathname, canonical=C:\longpathname, exists=0
  g2: absolute=C:\longpathname, canonical=C:\longpathname, exists=0
After creating c:\longpathn:
  h: absolute=c:\longpathn, canonical=C:\longpathn, exists=1
  f3: absolute=C:\longpa~1, canonical=C:\longpathn, exists=1

laszlocsomor commented 7 years ago

Out of curiosity, I implemented this in JNI (using GetLongPathNameA) and measured performance. Everything is measured twice to reveal caching, if any. Measured 6000 items because I could initialize a Java string array with that many 8dot3 names. No source code because I don't have nice clean BUILD files, as I was playing around in Eclipse and just reused Bazel's //src/main/native:windows_jni rule.

Resolution of c:\foo~1 (nonexistent, Java): C:\foo~1
Resolution of C:\foo\LO6C78~1 (existent, Java): C:\foo\longpath10
Resolution of c:\foo~1 (nonexistent, native): 
Resolution of C:\foo\LO6C78~1 (existent, native): C:\foo\longpath10
6000 distinct files (nonexistent, Java): 761 ms
6000 distinct files (nonexistent, Java): 707 ms
6000 distinct files (nonexistent, native): 112 ms
6000 distinct files (nonexistent, native): 112 ms
6000 distinct files (existent, Java): 1070 ms
6000 distinct files (existent, Java): 1054 ms
6000 distinct files (existent, native): 894 ms
6000 distinct files (existent, native): 900 ms
same file 6000 times (nonexistent, Java): 5 ms
same file 6000 times (nonexistent, Java): 3 ms
same file 6000 times (nonexistent, native): 84 ms
same file 6000 times (nonexistent, native): 82 ms
same file 6000 times (existent, Java): 1 ms
same file 6000 times (existent, Java): 1 ms
same file 6000 times (existent, native): 425 ms
same file 6000 times (existent, native): 423 ms

bazelbuild / bazel