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.

Sign up for GitHub

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

Jump to bottom

PathFragment comparison is case-sensitive on Windows (Causing failure of TensorFlow build with Bazel@HEAD) #2613

Closed
meteorcloudy opened this issue Mar 1, 2017 · 11 comments
Closed

PathFragment comparison is case-sensitive on Windows (Causing failure of TensorFlow build with Bazel@HEAD) #2613

meteorcloudy opened this issue Mar 1, 2017 · 11 comments
Assignees
@laszlocsomor
Labels
P1 I'll work on this now. (Assignee required) platform: windows type: bug
Milestone
0.5

Comments

@meteorcloudy
Copy link
Member

I noticed this error from TensorFlow job http://ci.bazel.io/job/TensorFlow/BAZEL_VERSION=HEAD,PLATFORM_NAME=windows-x86_64/676/console

java.lang.RuntimeException: Unrecoverable error while evaluating node 'FILE:[C:/jenkins/workspace/tensorflow/bazel_version/head/platform_name/windows-x86_64]/[tensorflow/tools/git/gen/head]' (requested by nodes 'RECURSIVE_PKG:rootedPath=[C:/jenkins/workspace/tensorflow/bazel_version/head/platform_name/windows-x86_64]/[tensorflow/tools/git/gen/head], excludedPaths=<omitted>)')
	at com.google.devtools.build.skyframe.ParallelEvaluator$Evaluate.run(ParallelEvaluator.java:448)
	at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:501)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.IllegalArgumentException: relativePath 'C:/jenkins/workspace/TensorFlow/BAZEL_VERSION/HEAD/PLATFORM_NAME/windows-x86_64/.git/HEAD' is absolute, but it's not under root 'C:/jenkins/workspace/tensorflow/bazel_version/head/platform_name/windows-x86_64'
	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:383)
	at com.google.devtools.build.lib.util.Preconditions.checkArgument(Preconditions.java:90)
	at com.google.devtools.build.lib.vfs.RootedPath.toRootedPath(RootedPath.java:56)
	at com.google.devtools.build.lib.vfs.RootedPath.toRootedPath(RootedPath.java:73)
	at com.google.devtools.build.lib.vfs.RootedPath.toRootedPathMaybeUnderRoot(RootedPath.java:83)
	at com.google.devtools.build.lib.skyframe.FileFunction.getSymlinkTargetRootedPath(FileFunction.java:176)
	at com.google.devtools.build.lib.skyframe.FileFunction.compute(FileFunction.java:101)
	at com.google.devtools.build.skyframe.ParallelEvaluator$Evaluate.run(ParallelEvaluator.java:374)
	... 4 more
java.lang.RuntimeException: Unrecoverable error while evaluating node 'FILE:[C:/jenkins/workspace/tensorflow/bazel_version/head/platform_name/windows-x86_64]/[tensorflow/tools/git/gen/head]' (requested by nodes 'RECURSIVE_PKG:rootedPath=[C:/jenkins/workspace/tensorflow/bazel_version/head/platform_name/windows-x86_64]/[tensorflow/tools/git/gen/head], excludedPaths=<omitted>)')
	at com.google.devtools.build.skyframe.ParallelEvaluator$Evaluate.run(ParallelEvaluator.java:448)
	at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:501)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.IllegalArgumentException: relativePath 'C:/jenkins/workspace/TensorFlow/BAZEL_VERSION/HEAD/PLATFORM_NAME/windows-x86_64/.git/HEAD' is absolute, but it's not under root 'C:/jenkins/workspace/tensorflow/bazel_version/head/platform_name/windows-x86_64'
	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:383)
	at com.google.devtools.build.lib.util.Preconditions.checkArgument(Preconditions.java:90)
	at com.google.devtools.build.lib.vfs.RootedPath.toRootedPath(RootedPath.java:56)
	at com.google.devtools.build.lib.vfs.RootedPath.toRootedPath(RootedPath.java:73)
	at com.google.devtools.build.lib.vfs.RootedPath.toRootedPathMaybeUnderRoot(RootedPath.java:83)
	at com.google.devtools.build.lib.skyframe.FileFunction.getSymlinkTargetRootedPath(FileFunction.java:176)
	at com.google.devtools.build.lib.skyframe.FileFunction.compute(FileFunction.java:101)
	at com.google.devtools.build.skyframe.ParallelEvaluator$Evaluate.run(ParallelEvaluator.java:374)

I believe this error is triggered by 9b75b68, but the real problem is in RootedPath.java. PathFragment comparison should be case-insensitive on Windows.

//cc @laszlocsomor
@meteorcloudy meteorcloudy added platform: windows P1 I'll work on this now. (Assignee required) type: bug labels Mar 1, 2017
@laszlocsomor
Copy link
Contributor

Yes, it seems like we don't ignore the path casing. Look:

Caused by: java.lang.IllegalArgumentException: relativePath
'C:/jenkins/workspace/TensorFlow/BAZEL_VERSION/HEAD/PLATFORM_NAME/windows-x86_64/.git/HEAD'
is absolute, but it's not under root
'C:/jenkins/workspace/tensorflow/bazel_version/head/platform_name/windows-x86_64'

I'll fix it.

@laszlocsomor laszlocsomor self-assigned this Mar 1, 2017
@meteorcloudy
Copy link
Member Author

Thanks!!

@laszlocsomor
Copy link
Contributor

Do you have a simple repro case for this?

@laszlocsomor
Copy link
Contributor

laszlocsomor commented Mar 1, 2017 •
edited
Loading

I think the problem is that WindowsPath overrides isTopLevelDirectory, but not isRootDirectory, and so C:/ is not regarded as root directory. EDIT: not so sure about that anymore, I keep looking at the code.

@meteorcloudy
Copy link
Member Author

Hmm.. I don't a simple repo yet.. But the cause sounds right to me.

@laszlocsomor
Copy link
Contributor

Never mind! I just wrote a simple test for it and can repro the same error. Working on a fix.

@laszlocsomor
Copy link
Contributor

And yes, you were right from the start, it is because of the PathFragment comparison being case-sensitive. I got mislead by the fact that Path comparison (and Path.startsWith) on the other hand is aware of case-(in)sensitivity.

@meteorcloudy
Copy link
Member Author

I see, but not sure how to fix it.. How can we know it's a Windows path?

@laszlocsomor
Copy link
Contributor

We can convert the Path segments to lower-case in Path.toFragment if the filesystem is case-insensitive. I have a change for that, about to send it out.

@laszlocsomor
Copy link
Contributor

https://bazel-review.googlesource.com/c/9124/

@laszlocsomor
Copy link
Contributor

Imported.

@dslomov dslomov added this to the 0.5 milestone Mar 2, 2017
@meteorcloudy meteorcloudy mentioned this issue Mar 3, 2017
Closed
hermione521 pushed a commit that referenced this issue Mar 6, 2017
@laszlocsomor @hermione521
PathFragment comparisons are now platform-aware
98cfdc0 
PathFragment's `equals`, `hashCode`, `compareTo`,
`startsWith`, `endsWith`, and `relativeTo` are now
aware of case-insensitivity when running on
Windows.

This approach is better than
https://bazel-review.googlesource.com/c/9124/
because it preserves path casing, which is
important when computing action output paths.

This change contains two additional bugfixes:
- `compareTo` now takes `driveLetter` into account
- the `InMemoryFileSystem` in `PathWindowsTest` is
not case-insensitive

Fixes #2613

--
Change-Id: I1a4250a373fff03fa02a6d8360457450b47a42a8
Reviewed-on: https://cr.bazel.build/9126
PiperOrigin-RevId: 149106930
MOS_MIGRATED_REVID=149106930
hermione521 pushed a commit that referenced this issue Mar 8, 2017
@laszlocsomor @hermione521
PathFragment comparisons are now platform-aware
cac970a 
PathFragment's `equals`, `hashCode`, `compareTo`,
`startsWith`, `endsWith`, and `relativeTo` are now
aware of case-insensitivity when running on
Windows.

This approach is better than
https://bazel-review.googlesource.com/c/9124/
because it preserves path casing, which is
important when computing action output paths.

This change contains two additional bugfixes:
- `compareTo` now takes `driveLetter` into account
- the `InMemoryFileSystem` in `PathWindowsTest` is
not case-insensitive

Fixes #2613

--
Change-Id: I1a4250a373fff03fa02a6d8360457450b47a42a8
Reviewed-on: https://cr.bazel.build/9126
PiperOrigin-RevId: 149106930
MOS_MIGRATED_REVID=149106930
hermione521 pushed a commit that referenced this issue Mar 8, 2017
@laszlocsomor @hermione521
PathFragment comparisons are now platform-aware
14e4755 
PathFragment's `equals`, `hashCode`, `compareTo`,
`startsWith`, `endsWith`, and `relativeTo` are now
aware of case-insensitivity when running on
Windows.

This approach is better than
https://bazel-review.googlesource.com/c/9124/
because it preserves path casing, which is
important when computing action output paths.

This change contains two additional bugfixes:
- `compareTo` now takes `driveLetter` into account
- the `InMemoryFileSystem` in `PathWindowsTest` is
not case-insensitive

Fixes #2613

--
Change-Id: I1a4250a373fff03fa02a6d8360457450b47a42a8
Reviewed-on: https://cr.bazel.build/9126
PiperOrigin-RevId: 149106930
MOS_MIGRATED_REVID=149106930
hermione521 pushed a commit that referenced this issue Mar 14, 2017
@laszlocsomor @hermione521
PathFragment comparisons are now platform-aware
18a559b 
PathFragment's `equals`, `hashCode`, `compareTo`,
`startsWith`, `endsWith`, and `relativeTo` are now
aware of case-insensitivity when running on
Windows.

This approach is better than
https://bazel-review.googlesource.com/c/9124/
because it preserves path casing, which is
important when computing action output paths.

This change contains two additional bugfixes:
- `compareTo` now takes `driveLetter` into account
- the `InMemoryFileSystem` in `PathWindowsTest` is
not case-insensitive

Fixes #2613

--
Change-Id: I1a4250a373fff03fa02a6d8360457450b47a42a8
Reviewed-on: https://cr.bazel.build/9126
PiperOrigin-RevId: 149106930
MOS_MIGRATED_REVID=149106930
@oakad oakad mentioned this issue Mar 5, 2019
Open
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
PathFragment comparisons are now platform-aware
f5c2dea 
    PathFragment's `equals`, `hashCode`, `compareTo`,
    `startsWith`, `endsWith`, and `relativeTo` are now
    aware of case-insensitivity when running on
    Windows.

    This approach is better than
    https://bazel-review.googlesource.com/c/9124/
    because it preserves path casing, which is
    important when computing action output paths.

    This change contains two additional bugfixes:
    - `compareTo` now takes `driveLetter` into account
    - the `InMemoryFileSystem` in `PathWindowsTest` is
    not case-insensitive

    Fixes bazelbuild/bazel#2613

    --
    Change-Id: I1a4250a373fff03fa02a6d8360457450b47a42a8
    Reviewed-on: https://cr.bazel.build/9126
    PiperOrigin-RevId: 149106930
    MOS_MIGRATED_REVID=149106930
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@laszlocsomor laszlocsomor

Labels
P1 I'll work on this now. (Assignee required) platform: windows type: bug
Projects
None yet
Milestone
0.5
Development

No branches or pull requests
3 participants
@meteorcloudy @dslomov @laszlocsomor