forked from bazelbuild/bazel
-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
If the "bazel.check_label_casing" JVM property is "1", then PackageIdentifier objects will store not only the RepositoryName and PathFragment, but also the toString() values of those. These augmented PackageIdentifiers (actually: CaseCheckingPackageIdentifiers) allow telling apart PackageIdentifiers whose RepositoryName and PathFragment values are equal but are cased differently. This scenario is common on case-insensitive filesystems, where the paths "foo/x" and "FoO/X" are equal but differently cased. Motivation is to allow case-checking the package names in PackageLookupFunction. The SkyKey of PackageLookupFunction contains a PackageIdentifier, which holds a reference to the RepositoryName and PathFragment (package fragment). On case-insensitive filesystems, both of these objects check equality without regard to case, and so do PackageIdentifiers as a consequence. But PackageIdentifier objects are interned for better memory usage, which means that on a case-insensitive filesystem it's impossible to create two PackageIdentifiers whose RepositoryName and PathFragment are equal but differently cased. And that means it's also impossible to check in the PackageLookupFunction whether the requested package path (and thus the PackageIdentifier) has correct casing. With this PR, PackageIdentifier.create() will return CaseCheckingPackageIdentifier when case checking is enabled, so PackageLookupFunction will know exactly what casing the user requested, and can therefore validate that casing and deny loading the package if the casing is wrong. And that is what would fix bazelbuild#8799 Change-Id: I46106f83c0a5b16fa347dd8a07e3c5342248c605
- Loading branch information
1 parent
7d68029
commit f76615b
Showing
6 changed files
with
361 additions
and
14 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
76 changes: 76 additions & 0 deletions
76
...e/devtools/build/lib/cmdline/CaseInsensitiveFsLabelCaseCheckingPackageIdentifierTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| // Copyright 2019 The Bazel Authors. All rights reserved. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package com.google.devtools.build.lib.cmdline; | ||
|
|
||
| import static com.google.common.truth.Truth.assertThat; | ||
|
|
||
| import com.google.common.testing.EqualsTester; | ||
| import com.google.devtools.build.lib.vfs.PathFragment; | ||
| import java.io.ByteArrayInputStream; | ||
| import java.io.ByteArrayOutputStream; | ||
| import java.io.ObjectInputStream; | ||
| import java.io.ObjectOutputStream; | ||
| import org.junit.Test; | ||
| import org.junit.runner.RunWith; | ||
| import org.junit.runners.JUnit4; | ||
|
|
||
| /** | ||
| * Unit tests for {@link PackageIdentifier}. | ||
| */ | ||
| @RunWith(JUnit4.class) | ||
| public final class CaseInsensitiveFsLabelCaseCheckingPackageIdentifierTest { | ||
|
|
||
| @Test | ||
| public void testSemantics() throws Exception { | ||
| PathFragment pf1 = PathFragment.create("foo/bar"); | ||
| PathFragment pf2 = PathFragment.create("FoO/BaR"); | ||
| assertThat(pf1).isEqualTo(pf2); | ||
| // Fortunately, PathFragments are not interned so we get different instances. The ability to | ||
| // create equal, but not-same instances allows storing them in wrapper objects with their | ||
| // toString() value, thus distinguish differently cased versions of otherwise equal paths. | ||
| assertThat(pf1).isNotSameInstanceAs(pf2); | ||
| assertThat(pf1.toString()).isNotEqualTo(pf2.toString()); | ||
|
|
||
| RepositoryName rn1 = RepositoryName.create("@foo"); | ||
| RepositoryName rn2 = RepositoryName.create("@FoO"); | ||
| // Fortunately, RepositoryNames are not interned so we get different instances. The ability to | ||
| // create equal, but not-same instances allows storing them in wrapper objects with their | ||
| // toString() value, thus distinguish differently cased versions of otherwise equal paths. | ||
| assertThat(rn1).isEqualTo(rn2); | ||
| assertThat(rn1).isNotSameInstanceAs(rn2); | ||
| assertThat(rn1.toString()).isNotEqualTo(rn2.toString()); | ||
|
|
||
| PackageIdentifier id1 = PackageIdentifier.parse("@foo//bar/baz"); | ||
| PackageIdentifier id2 = PackageIdentifier.parse("@foo//BAR/baz"); | ||
| PackageIdentifier id3 = PackageIdentifier.parse("@FOO//bar/baz"); | ||
| PackageIdentifier id4 = PackageIdentifier.parse("@FOO//BAR/baz"); | ||
| // On a case-insensitive filesystem, RepositoryName ("foo" and "FOO") and PathFragment | ||
| // ("bar/baz" and "BAR/baz") are compared case-insensitively. But we also enabled label case | ||
| // checkiung, so the PackageIdentifiers are actually CaseCheckingPackageIdentifiers, so they are | ||
| // all unequal. | ||
| new EqualsTester() | ||
| .addEqualityGroup(id1) | ||
| .addEqualityGroup(id2) | ||
| .addEqualityGroup(id3) | ||
| .addEqualityGroup(id4) | ||
| .testEquals(); | ||
| assertThat(id1).isNotSameInstanceAs(id2); | ||
| assertThat(id1).isNotSameInstanceAs(id3); | ||
| assertThat(id1).isNotSameInstanceAs(id4); | ||
| assertThat(id2).isNotSameInstanceAs(id3); | ||
| assertThat(id2).isNotSameInstanceAs(id4); | ||
| assertThat(id3).isNotSameInstanceAs(id4); | ||
| } | ||
| } |
69 changes: 69 additions & 0 deletions
69
...st/java/com/google/devtools/build/lib/cmdline/CaseInsensitiveFsPackageIdentifierTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| // Copyright 2019 The Bazel Authors. All rights reserved. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package com.google.devtools.build.lib.cmdline; | ||
|
|
||
| import static com.google.common.truth.Truth.assertThat; | ||
|
|
||
| import com.google.common.testing.EqualsTester; | ||
| import com.google.devtools.build.lib.vfs.PathFragment; | ||
| import java.io.ByteArrayInputStream; | ||
| import java.io.ByteArrayOutputStream; | ||
| import java.io.ObjectInputStream; | ||
| import java.io.ObjectOutputStream; | ||
| import org.junit.Test; | ||
| import org.junit.runner.RunWith; | ||
| import org.junit.runners.JUnit4; | ||
|
|
||
| /** | ||
| * Unit tests for {@link PackageIdentifier}. | ||
| */ | ||
| @RunWith(JUnit4.class) | ||
| public final class CaseInsensitiveFsPackageIdentifierTest { | ||
|
|
||
| @Test | ||
| public void testSemantics() throws Exception { | ||
| PathFragment pf1 = PathFragment.create("foo/bar"); | ||
| PathFragment pf2 = PathFragment.create("FoO/BaR"); | ||
| assertThat(pf1).isEqualTo(pf2); | ||
| // Fortunately, PathFragments are not interned so we get different instances. The ability to | ||
| // create equal, but not-same instances allows storing them in wrapper objects with their | ||
| // toString() value, thus distinguish differently cased versions of otherwise equal paths. | ||
| assertThat(pf1).isNotSameInstanceAs(pf2); | ||
| assertThat(pf1.toString()).isNotEqualTo(pf2.toString()); | ||
|
|
||
| RepositoryName rn1 = RepositoryName.create("@foo"); | ||
| RepositoryName rn2 = RepositoryName.create("@FoO"); | ||
| // Fortunately, RepositoryNames are not interned so we get different instances. The ability to | ||
| // create equal, but not-same instances allows storing them in wrapper objects with their | ||
| // toString() value, thus distinguish differently cased versions of otherwise equal paths. | ||
| assertThat(rn1).isEqualTo(rn2); | ||
| assertThat(rn1).isNotSameInstanceAs(rn2); | ||
| assertThat(rn1.toString()).isNotEqualTo(rn2.toString()); | ||
|
|
||
| PackageIdentifier id1 = PackageIdentifier.parse("@foo//bar/baz"); | ||
| PackageIdentifier id2 = PackageIdentifier.parse("@foo//BAR/baz"); | ||
| PackageIdentifier id3 = PackageIdentifier.parse("@FOO//bar/baz"); | ||
| PackageIdentifier id4 = PackageIdentifier.parse("@FOO//BAR/baz"); | ||
| // On a case-insensitive filesystem, RepositoryName ("foo" and "FOO") and PathFragment | ||
| // ("bar/baz" and "BAR/baz") are compared case-insensitively, so all PackageIdentifiers are | ||
| // equal. | ||
| new EqualsTester().addEqualityGroup(id1, id2, id3, id4).testEquals(); | ||
| // PackageIdentifier.create() interns objects, and it returns CaseTrustingPackageIdentifier | ||
| // because label case checking is disnabled, so the interned objects are all the same instance. | ||
| assertThat(id1).isSameInstanceAs(id2); | ||
| assertThat(id1).isSameInstanceAs(id3); | ||
| assertThat(id1).isSameInstanceAs(id4); | ||
| } | ||
| } |
Oops, something went wrong.