Check the case of build-file paths passed to Buck before parsing to c…

…atch misspellings earlier and fix the BUCK file not found issue if target file is a symlink.

Summary: This reverts commit 0566a366d8d62fe1f7caf351ae157ea582c00fa1. And fix the issue when a BUCK file is a symlink to a file.

Reviewed By: sbalabanov

fbshipit-source-id: 62b9141a6e

src/com/facebook/buck/cli/CommandLineTargetNodeSpecParser.java

@@ -20,12 +20,15 @@
 import com.facebook.buck.core.config.AliasConfig;
 import com.facebook.buck.core.config.BuckConfig;
 import com.facebook.buck.core.exceptions.HumanReadableException;
+import com.facebook.buck.io.file.MorePaths;
+import com.facebook.buck.io.file.MorePaths.PathExistResult;
+import com.facebook.buck.io.file.MorePaths.PathExistResultWrapper;
 import com.facebook.buck.parser.BuildTargetPatternTargetNodeParser;
 import com.facebook.buck.parser.TargetNodeSpec;
 import com.facebook.buck.support.cli.args.BuckCellArg;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableSet;
-import java.nio.file.Files;
+import java.io.IOException;
 import java.nio.file.Path;
 import java.util.Optional;
 
@@ -88,9 +91,24 @@ protected String normalizeBuildTargetString(String target) {
  private void validateTargetSpec(TargetNodeSpec spec, String buildTarget) {
  Path cellPath = spec.getBuildFileSpec().getCellPath();
  Path basePath = spec.getBuildFileSpec().getBasePath();
- if (!Files.exists(cellPath.resolve(basePath))) {
- throw new HumanReadableException(
- "%s references non-existent directory %s", buildTarget, basePath);
+ Path buildTargetPath = cellPath.resolve(basePath);
+ try {
+ PathExistResultWrapper pathExist =
+ MorePaths.pathExistsCaseSensitive(buildTargetPath, cellPath);
+ if (PathExistResult.NOT_EXIST == pathExist.getResult()) {
+ throw new HumanReadableException(
+ "%s references non-existent directory %s", buildTarget, basePath);
+ }
+
+ if (PathExistResult.EXIST_CASE_MISMATCHED == pathExist.getResult()) {
+ throw new HumanReadableException(
+ "The case of the build path provided (%s) does not match the actual path. "
+ + "This is an issue even on case-insensitive file systems. "
+ + "Please check the spelling of the provided path.",
+ buildTargetPath);
+ }
+ } catch (IOException e) {
+ throw new HumanReadableException(e, e.getMessage());
  }
  }
 

src/com/facebook/buck/io/file/MorePaths.java

@@ -29,11 +29,16 @@
 import com.google.common.io.ByteSource;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.file.DirectoryStream;
 import java.nio.file.FileSystem;
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Function;
@@ -395,4 +400,110 @@ public static void createSymLink(@Nullable WindowsFS winFS, Path symLink, Path t
  public static boolean isDirectory(Path path, LinkOption... linkOptions) {
  return Files.isDirectory(normalize(path).toAbsolutePath(), linkOptions);
  }
+
+ /**
+ * Verify a file exists and also check the case of file path which {@link Files#exists(Path,
+ * LinkOption...)} failed to do so for case-insensitive file systems. This method also works for
+ * case-sensitive file systems
+ *
+ * @param path A path to a file
+ * @param rootPath the root path to start with
+ * @return A wrap contains {@link PathExistResult} and paths that actually exist but with case
+ * mismatched if the input path does not exist. See {@link PathExistResultWrapper} for more
+ * details.
+ */
+ public static PathExistResultWrapper pathExistsCaseSensitive(Path path, Path rootPath)
+ throws IOException {
+ Path absolutePath = path.isAbsolute() ? path : rootPath.resolve(path);
+ boolean fileExist = Files.exists(absolutePath);
+
+ if (path.equals(rootPath) && fileExist) {
+ return new PathExistResultWrapper(PathExistResult.EXIST_CASE_MATCHED, Optional.empty());
+ }
+
+ Path relativePath = path.isAbsolute() ? rootPath.relativize(path) : path;
+
+ List<Path> currentParentPathIgnoreCaseList = Collections.singletonList(rootPath);
+ Path parentPath = rootPath;
+
+ int nameCount = relativePath.getNameCount();
+
+ for (Path subPath : relativePath) {
+ nameCount--;
+ boolean isLastSubPath = (nameCount == 0);
+ Path currentPath = parentPath.resolve(subPath);
+ String currentFileName = currentPath.getFileName().toString();
+
+ List<Path> nextParentPathIgnoreCaseList = new ArrayList<>();
+ boolean existCurrentPathIgnoreCase = false;
+ for (Path currentParentPathIgnoreCase : currentParentPathIgnoreCaseList) {
+ try (DirectoryStream<Path> stream =
+ Files.newDirectoryStream(
+ currentParentPathIgnoreCase,
+ filePath ->
+ filePath.getFileName().toString().equalsIgnoreCase(currentFileName)
+ && (isLastSubPath || Files.isDirectory(filePath)))) {
+ Iterator<Path> it = stream.iterator();
+ if (it.hasNext()) {
+ existCurrentPathIgnoreCase = true;
+ it.forEachRemaining(nextParentPathIgnoreCaseList::add);
+ }
+ }
+ }
+ if (!existCurrentPathIgnoreCase) {
+ return new PathExistResultWrapper(PathExistResult.NOT_EXIST, Optional.empty());
+ }
+ currentParentPathIgnoreCaseList = nextParentPathIgnoreCaseList;
+ parentPath = currentPath;
+ }
+
+ boolean pathExistCaseSensitive =
+ currentParentPathIgnoreCaseList
+ .stream()
+ .anyMatch(filePath -> filePath.toString().equals(absolutePath.toString()));
+
+ return pathExistCaseSensitive
+ ? new PathExistResultWrapper(PathExistResult.EXIST_CASE_MATCHED, Optional.empty())
+ : new PathExistResultWrapper(
+ PathExistResult.EXIST_CASE_MISMATCHED, Optional.of(currentParentPathIgnoreCaseList));
+ }
+
+ /**
+ * A wrap contains {@link PathExistResult} and paths that actually exist but with case mismatched
+ * if the input path does not exist. For the first part, {@link PathExistResult#NOT_EXIST} if the
+ * file does not exist, {@link PathExistResult#EXIST_CASE_MISMATCHED} exist but case mismatched,
+ * or {@link PathExistResult#EXIST_CASE_MATCHED}exist and case matched . For the second part, will
+ * not be Optional.empty() only when the first part returns {@link
+ * PathExistResult#EXIST_CASE_MISMATCHED}, which will give a list of paths that exist but
+ * case-mismatched with the input path
+ */
+ public static class PathExistResultWrapper {
+ private final PathExistResult result;
+ private final Optional<List<Path>> pathsCaseMismatched;
+
+ public PathExistResultWrapper(
+ PathExistResult pathExistResult, Optional<List<Path>> pathsCaseMismatched) {
+ this.result = pathExistResult;
+ this.pathsCaseMismatched = pathsCaseMismatched;
+ }
+
+ public PathExistResult getResult() {
+ return result;
+ }
+
+ public Optional<List<Path>> getCaseMismatchedPaths() {
+ return pathsCaseMismatched;
+ }
+ }
+
+ /**
+ * The return result of a path existence check {@link PathExistResult#NOT_EXIST} if the file does
+ * not exist, {@link PathExistResult#EXIST_CASE_MISMATCHED} exist but case mismatched, or {@link
+ * PathExistResult#EXIST_CASE_MATCHED}exist and case matched
+ */
+ public enum PathExistResult {
+ NOT_EXIST,
+ EXIST_CASE_MATCHED,
+ EXIST_CASE_MISMATCHED
+ }
 }

test/com/facebook/buck/cli/CommandLineTargetNodeSpecParserIntegrationTest.java

@@ -29,6 +29,7 @@
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableSet;
 import java.io.IOException;
+import org.hamcrest.Matcher;
 import org.hamcrest.Matchers;
 import org.junit.Rule;
 import org.junit.Test;
@@ -88,6 +89,35 @@ public void trailingDotDotDotTargets() throws IOException {
  Matchers.containsString("//simple/.... references non-existent directory simple"));
  }
 
+ @Test
+ public void targetsTypo() throws IOException {
+ ProjectWorkspace workspace =
+ TestDataHelper.createProjectWorkspaceForScenario(this, "command_line_parser", tmp);
+ workspace.setUp();
+
+ // First check for correct usage.
+ ProcessResult result = workspace.runBuckCommand("targets", "//simple:").assertSuccess();
+ assertEquals(
+ ImmutableSet.of("//simple:simple"),
+ ImmutableSet.copyOf(
+ Splitter.on(System.lineSeparator()).omitEmptyStrings().split(result.getStdout())));
+
+ // Check for some expected failure cases.
+ result = workspace.runBuckCommand("targets", "//sImple:");
+ result.assertFailure();
+
+ Matcher<String> errorMessageMatcher =
+ Matchers.allOf(
+ Matchers.containsString("The case of the build path provided"),
+ Matchers.containsString("sImple"),
+ Matchers.containsString(
+ "does not match the actual path. "
+ + "This is an issue even on case-insensitive file systems. "
+ + "Please check the spelling of the provided path."));
+
+ assertThat(result.getStderr(), errorMessageMatcher);
+ }
+
  @Test
  public void trailingColonBuild() throws IOException {
  ProjectWorkspace workspace =

test/com/facebook/buck/io/file/BUCK

@@ -15,6 +15,7 @@ standard_java_test(
  "//third-party/java/guava:guava",
  "//third-party/java/jimfs:jimfs",
  "//third-party/java/junit:junit",
+ "//third-party/java/junitparams:junitparams",
  ],
 )
 

test/com/facebook/buck/io/file/MorePathsTest.java

@@ -23,6 +23,8 @@
 import static org.junit.Assert.assertTrue;
 
 import com.facebook.buck.io.MoreProjectFilesystems;
+import com.facebook.buck.io.file.MorePaths.PathExistResult;
+import com.facebook.buck.io.file.MorePaths.PathExistResultWrapper;
 import com.facebook.buck.io.filesystem.ProjectFilesystem;
 import com.facebook.buck.io.filesystem.TestProjectFilesystems;
 import com.facebook.buck.testutil.TemporaryPaths;
@@ -35,10 +37,16 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.Collections;
 import java.util.Optional;
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.hamcrest.Matchers;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.runner.RunWith;
 
+@RunWith(JUnitParamsRunner.class)
 public class MorePathsTest {
 
  @Rule public TemporaryPaths tmp = new TemporaryPaths();
@@ -363,6 +371,142 @@ public void splitOnCommonPrefixMixedFails() {
  assertFalse(result.isPresent());
  }
 
+ @Test
+ @Parameters({
+ "false, file/not/exist.txt," + "fileNotExistFile.txt," + "folder/file.txt/notExist.txt",
+ "true, file/not/exist.txt," + "fileNotExistFile.txt," + "folder/file.txt/notExist.txt",
+ })
+ public void testPathExist(boolean absolute, String... filesNotExist) throws IOException {
+ ProjectFilesystem projectFilesystem =
+ TestProjectFilesystems.createProjectFilesystem(tmp.getRoot());
+ tmp.newFolder("folder");
+ tmp.newFile("folder/file.txt");
+ Path fileExists = projectFilesystem.resolve(Paths.get("folder/file.txt"));
+ assertEquals(
+ MorePaths.pathExistsCaseSensitive(fileExists, tmp.getRoot()).getResult(),
+ PathExistResult.EXIST_CASE_MATCHED);
+
+ for (String fileNotExist : filesNotExist) {
+ Path fileNotExistPath = Paths.get(fileNotExist);
+ if (absolute) {
+ fileNotExistPath = projectFilesystem.resolve(fileNotExistPath);
+ }
+ assertEquals(
+ MorePaths.pathExistsCaseSensitive(fileNotExistPath, tmp.getRoot()).getResult(),
+ PathExistResult.NOT_EXIST);
+ }
+ }
+
+ @Test
+ public void testPathExistsRoot() throws IOException {
+ TestProjectFilesystems.createProjectFilesystem(tmp.getRoot());
+ assertEquals(
+ MorePaths.pathExistsCaseSensitive(tmp.getRoot(), tmp.getRoot()).getResult(),
+ PathExistResult.EXIST_CASE_MATCHED);
+ assertEquals(
+ MorePaths.pathExistsCaseSensitive(Paths.get("/rootNotExist"), tmp.getRoot()).getResult(),
+ PathExistResult.NOT_EXIST);
+ }
+
+ @Test
+ @Parameters({
+ "alpha/beta/gamma, delta.txt, "
+ + "alpha/beta/gamma/delta.txt, "
+ + "Alpha/beta/gamma/delta.txt, "
+ + "alpha/Beta/gamma/delta.txt, "
+ + "alpha/beta/Gamma/delta.txt, "
+ + "alpha/beta/gamma/Delta.txt",
+ "Alpha/Beta/Gamma, Delta.txt, "
+ + "Alpha/Beta/Gamma/Delta.txt, "
+ + "alpha/Beta/Gamma/Delta.txt, "
+ + "Alpha/beta/Gamma/Delta.txt, "
+ + "Alpha/Beta/gamma/Delta.txt, "
+ + "Alpha/Beta/Gamma/delta.txt",
+ })
+ public void testPathExistsCaseMismatched(
+ String folder, String fileName, String matchedPath, String... misMatchedPaths)
+ throws IOException {
+ ProjectFilesystem projectFilesystem =
+ TestProjectFilesystems.createProjectFilesystem(tmp.getRoot());
+ tmp.newFolder(folder.split("/"));
+ tmp.newFolder(folder.concat("/").concat(fileName));
+
+ Path pathToFileCaseMatched = projectFilesystem.resolve(Paths.get(matchedPath));
+
+ assertEquals(
+ MorePaths.pathExistsCaseSensitive(pathToFileCaseMatched, tmp.getRoot()).getResult(),
+ PathExistResult.EXIST_CASE_MATCHED);
+
+ for (String misMatchedPath : misMatchedPaths) {
+ Path pathToFileCaseMismatched = projectFilesystem.resolve(Paths.get(misMatchedPath));
+ PathExistResultWrapper pathExist =
+ MorePaths.pathExistsCaseSensitive(pathToFileCaseMismatched, tmp.getRoot());
+ assertEquals(pathExist.getResult(), PathExistResult.EXIST_CASE_MISMATCHED);
+ assertTrue(pathExist.getCaseMismatchedPaths().isPresent());
+ assertThat(
+ pathExist.getCaseMismatchedPaths().get(),
+ Matchers.is((Collections.singletonList(pathToFileCaseMatched))));
+ }
+ }
+
+ @Test
+ public void testSymlinkPathExists() throws IOException {
+ ProjectFilesystem projectFilesystem =
+ TestProjectFilesystems.createProjectFilesystem(tmp.getRoot());
+ tmp.newFolder("folder");
+ tmp.newFile("folder/file.txt");
+ Path folderExists = Paths.get("folder");
+ Path folderLinked = Paths.get("anotherFolder");
+ Path folderLinkedCaseMisMatched = Paths.get("AnotherFolder");
+ Path folderLinkedNotExist = Paths.get("AnFolder");
+
+ MoreProjectFilesystems.createRelativeSymlink(folderLinked, folderExists, projectFilesystem);
+
+ assertPathExists(folderExists, folderLinked, folderLinkedCaseMisMatched, folderLinkedNotExist);
+
+ Path fileExists = Paths.get("folder/file.txt");
+ Path fileLinked = Paths.get("folder/anotherLinkedFile.txt");
+ Path fileLinkedCaseMismatched = Paths.get("folder/AnotherLinkedFile.txt");
+ Path fileLinkedNotExist = Paths.get("folder/NotAnotherLinkedFile.txt");
+
+ MoreProjectFilesystems.createRelativeSymlink(fileLinked, fileExists, projectFilesystem);
+
+ assertPathExists(fileExists, fileLinked, fileLinkedCaseMismatched, fileLinkedNotExist);
+
+ Path fileInLinkedFolder = Paths.get("anotherFolder/file.txt");
+ Path fileLinkedInLinkedFolder = Paths.get("anotherFolder/anotherFile.txt");
+ Path fileLinkedInLinkedFolderCaseMismatched = Paths.get("AnotherFolder/AnotherFile.txt");
+ Path fileLinkedInLinkedFolderNotExist = Paths.get("anotherFolder/NotAnotherFile.txt");
+ MoreProjectFilesystems.createRelativeSymlink(
+ fileLinkedInLinkedFolder, fileExists, projectFilesystem);
+
+ assertPathExists(
+ fileInLinkedFolder,
+ fileLinkedInLinkedFolder,
+ fileLinkedInLinkedFolderCaseMismatched,
+ fileLinkedInLinkedFolderNotExist);
+ }
+
+ private void assertPathExists(
+ Path pathExists, Path pathLinked, Path pathLinkedCaseMisMatched, Path pathLinkedNotExist)
+ throws IOException {
+ assertEquals(
+ MorePaths.pathExistsCaseSensitive(pathExists, tmp.getRoot()).getResult(),
+ PathExistResult.EXIST_CASE_MATCHED);
+
+ assertEquals(
+ MorePaths.pathExistsCaseSensitive(pathLinked, tmp.getRoot()).getResult(),
+ PathExistResult.EXIST_CASE_MATCHED);
+
+ assertEquals(
+ MorePaths.pathExistsCaseSensitive(pathLinkedCaseMisMatched, tmp.getRoot()).getResult(),
+ PathExistResult.EXIST_CASE_MISMATCHED);
+
+ assertEquals(
+ MorePaths.pathExistsCaseSensitive(pathLinkedNotExist, tmp.getRoot()).getResult(),
+ PathExistResult.NOT_EXIST);
+ }
+
  private void validateSymlinklTarget(
  Path pathToExistingFileUnderProjectRoot,
  Path pathToDesiredLinkUnderProjectRoot,