fix: buck project was removing files generated within the same directory as part of a cleanup + add project_path option

Summary:
The "cleanup" loop was removing files that were generated by the same loop (assuming there are more than one file in the directory).
It's fixed within this diff.

To the 'project_path':
The vendor_path is a list of vendor paths so it's not really valid to use it as a destination for generated files.

Reviewed By: styurin

fbshipit-source-id: a5163c5a98

Author: Mateusz Kaczanowski

docs/concept/buckconfig.soy

@@ -2145,6 +2145,17 @@ cxx_library(
   {/param}
 {/call}
 
+{call buckconfig.entry}
+  {param section: 'go' /}
+  {param name: 'project_path' /}
+  {param example_value: 'third-party/go' /}
+  {param description}
+    You can specify the path where <code>buck project</code> will store dynamically
+    generated files (ex. genrule). This is extension to <code>$GOPATH</code>, particularly
+    usefuly while working with native go toolchain or IDE's.
+  {/param}
+{/call}
+
 {call buckconfig.section}
   {param name: 'groovy' /}
   {param description}

src/com/facebook/buck/features/go/GoBuckConfig.java

@@ -29,37 +29,77 @@
 public class GoBuckConfig {
 
   static final String SECTION = "go";
-
   private final BuckConfig delegate;
 
+  private static final String VENDOR_PATH = "vendor_path";
+  private static final String PROJECT_PATH = "project_path";
+  private static final String TEST_MAIN_GEN = "test_main_gen";
+  private static final String DEFAULT_PLATFORM = "default_platform";
+  private static final String PREFIX = "prefix";
+
   public GoBuckConfig(BuckConfig delegate) {
     this.delegate = delegate;
   }
 
-  Optional<String> getDefaultPlatform() {
-    return delegate.getValue(SECTION, "default_platform");
-  }
-
   public BuckConfig getDelegate() {
     return delegate;
   }
 
+  /**
+   * Get default Go platform
+   *
+   * @return Go platform
+   */
+  Optional<String> getDefaultPlatform() {
+    return delegate.getValue(SECTION, DEFAULT_PLATFORM);
+  }
+
+  /**
+   * Get package name either from build target attribute (package_name) if provided, otherwise use
+   * BuildTarget base path.
+   *
+   * @return Package name
+   */
   Path getDefaultPackageName(BuildTarget target) {
-    Path prefix = Paths.get(delegate.getValue(SECTION, "prefix").orElse(""));
+    Path prefix = Paths.get(delegate.getValue(SECTION, PREFIX).orElse(""));
     return prefix.resolve(target.getBasePath());
   }
 
+  /**
+   * Get vendor paths based on vendor_path section. Section allows to specify multiple vendor paths
+   * separated by colon (":").
+   *
+   * @return List of vendor paths
+   */
   ImmutableList<Path> getVendorPaths() {
     Optional<ImmutableList<String>> vendorPaths =
-        delegate.getOptionalListWithoutComments(SECTION, "vendor_path", ':');
+        delegate.getOptionalListWithoutComments(SECTION, VENDOR_PATH, ':');
 
     if (vendorPaths.isPresent()) {
       return vendorPaths.get().stream().map(Paths::get).collect(ImmutableList.toImmutableList());
     }
     return ImmutableList.of();
   }
 
+  /**
+   * Get test main generator. The tool is a middle-step utility that utilizes selected .go sources
+   * and generates the main.go which is later on compiled and used as test binary (run by "buck
+   * test").
+   *
+   * @return test_main_gen tool
+   */
   Optional<Tool> getGoTestMainGenerator(BuildRuleResolver resolver) {
-    return delegate.getView(ToolConfig.class).getTool(SECTION, "test_main_gen", resolver);
+    return delegate.getView(ToolConfig.class).getTool(SECTION, TEST_MAIN_GEN, resolver);
+  }
+
+  /**
+   * Get "project_path" location. When set, buck project will copy generated sources to given
+   * directory
+   *
+   * @return project_path path
+   */
+  Optional<Path> getProjectPath() {
+    Optional<String> path = delegate.getValue(SECTION, PROJECT_PATH);
+    return (path.isPresent()) ? Optional.of(Paths.get(path.get())) : Optional.empty();
   }
 }

src/com/facebook/buck/features/go/GoProjectCommandHelper.java

@@ -74,6 +74,7 @@ public class GoProjectCommandHelper {
   private final Console console;
   private final ListeningExecutorService executor;
   private final Parser parser;
+  private final GoBuckConfig goBuckConfig;
   private final BuckConfig buckConfig;
   private final Cell cell;
   private final boolean enableParserProfiling;
@@ -92,6 +93,7 @@ public GoProjectCommandHelper(
     this.console = projectGeneratorParameters.getConsole();
     this.executor = executor;
     this.parser = projectGeneratorParameters.getParser();
+    this.goBuckConfig = new GoBuckConfig(params.getBuckConfig());
     this.buckConfig = params.getBuckConfig();
     this.cell = params.getCell();
     this.enableParserProfiling = enableParserProfiling;
@@ -247,9 +249,9 @@ private void copyGeneratedGoCode(
     Path vendorPath;
     ProjectFilesystem projectFilesystem = cell.getFilesystem();
 
-    Optional<String> vendorConfig = buckConfig.getValue("go", "vendor_path");
-    if (vendorConfig.isPresent()) {
-      vendorPath = Paths.get(vendorConfig.get());
+    Optional<Path> projectPath = goBuckConfig.getProjectPath();
+    if (projectPath.isPresent()) {
+      vendorPath = projectPath.get();
     } else if (projectFilesystem.exists(Paths.get("src"))) {
       vendorPath = Paths.get("src", "vendor");
     } else {
@@ -259,15 +261,27 @@ private void copyGeneratedGoCode(
         Preconditions.checkNotNull(getActionGraph(targetGraphAndTargets.getTargetGraph()));
     DefaultSourcePathResolver sourcePathResolver =
         DefaultSourcePathResolver.from(new SourcePathRuleFinder(result.getActionGraphBuilder()));
+
+    // cleanup files from previous runs
     for (BuildTargetSourcePath sourcePath : generatedPackages.keySet()) {
       Path desiredPath = vendorPath.resolve(generatedPackages.get(sourcePath));
-      projectFilesystem.mkdirs(desiredPath);
-      for (Path path : projectFilesystem.getDirectoryContents(desiredPath)) {
-        if (projectFilesystem.isFile(path)) {
-          projectFilesystem.deleteFileAtPath(path);
+
+      if (projectFilesystem.isDirectory(desiredPath)) {
+        for (Path path : projectFilesystem.getDirectoryContents(desiredPath)) {
+          if (projectFilesystem.isFile(path)) {
+            projectFilesystem.deleteFileAtPath(path);
+          }
         }
+      } else {
+        projectFilesystem.mkdirs(desiredPath);
       }
+    }
+
+    // copy files generated in current run
+    for (BuildTargetSourcePath sourcePath : generatedPackages.keySet()) {
+      Path desiredPath = vendorPath.resolve(generatedPackages.get(sourcePath));
       Path generatedSrc = sourcePathResolver.getAbsolutePath(sourcePath);
+
       if (projectFilesystem.isDirectory(generatedSrc)) {
         projectFilesystem.copyFolder(generatedSrc, desiredPath);
       } else {
@@ -294,9 +308,7 @@ private Map<BuildTargetSourcePath, Path> findCodeGenerationTargets(
         Path pkgName =
             packageNameArg
                 .map(Paths::get)
-                .orElse(
-                    Paths.get(buckConfig.getValue("go", "prefix").orElse(""))
-                        .resolve(targetNode.getBuildTarget().getBasePath()));
+                .orElse(goBuckConfig.getDefaultPackageName(targetNode.getBuildTarget()));
         generatedPackages.putAll(
             ((HasSrcs) constructorArg)
                 .getSrcs()

test/com/facebook/buck/features/go/GoProjectIntegrationTest.java

@@ -64,6 +64,8 @@ public void testBuckProjectPurgeExistingVendor() throws IOException {
     assertFalse(workspace.resolve("vendor/a/b0.go").toFile().exists());
     assertTrue(workspace.resolve("vendor/a/b1.go").toFile().exists());
     assertTrue(workspace.resolve("vendor/a/b2.go").toFile().exists());
+    assertTrue(workspace.resolve("vendor/a/c1.go").toFile().exists());
+    assertTrue(workspace.resolve("vendor/a/c2.go").toFile().exists());
     assertTrue(workspace.resolve("vendor/a/b/b.go").toFile().exists());
   }
 }

test/com/facebook/buck/features/go/testdata/go_project_with_existing_vendor/BUCK.fixture

@@ -19,6 +19,8 @@ go_library(
     package_name = "a",
     srcs = [
         ":b",
+        ":c=c1.go",
+        ":c=c2.go",
     ],
 )
 
@@ -27,3 +29,22 @@ genrule(
     out = ".",
     cmd = "echo 'package a\nfunc A() {}\n' > $OUT/b1.go; echo 'package a\nfunc B() {}\n' > $OUT/b2.go;",
 )
+
+# copy files from :c separately
+genrule(
+    name = "c",
+    out = ".",
+    cmd = "echo 'package a\nfunc C() {}\n' > $OUT/c1.go; echo 'package a\nfunc D() {}\n' > $OUT/c2.go;",
+)
+
+genrule(
+    name = "c=c1.go",
+    out = "c1.go",
+    cmd = "mkdir -p `dirname $OUT` && cp $(location :c)/c1.go $OUT",
+)
+
+genrule(
+    name = "c=c2.go",
+    out = "c2.go",
+    cmd = "mkdir -p `dirname $OUT` && cp $(location :c)/c2.go $OUT",
+)