Support macros in ocamldep_flags

Summary: The things passed to ocamldep_flags in the OCaml rules are often preprocessors, e.g. `buck/platform009/build/supercaml/share/dotopam/default/lib/lwt_ppx/ppx.exe`. However, because they are added as strings, Buck has no idea they are files, and because they are files not targets, it is a kind of package boundary violation. To fix the macros we need to switch to make a proper `export_file` target and use `$(location ...)`. To make that work, `ocamldep_flags` needs to take macros, which this diff does. Reviewed By: IanChilds fbshipit-source-id: 9816dd932d97406c375e09d514a6cf2a636b1dd8
facebook · Sep 16, 2021 · 55154bb · 55154bb
1 parent e2bf51a
commit 55154bb
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 13 deletions.
diff --git a/src/com/facebook/buck/features/ocaml/OcamlBinaryDescription.java b/src/com/facebook/buck/features/ocaml/OcamlBinaryDescription.java
@@ -101,6 +101,13 @@ public BuildRule createBuildRule(
             args.getCompilerFlags(),
             args.getWarningsFlags());
 
+    ImmutableList<Arg> ocamldepFlags =
+        OcamlDescriptionEnhancer.toStringWithMacrosArgs(
+            buildTarget,
+            context.getCellPathResolver(),
+            context.getActionGraphBuilder(),
+            args.getOcamldepFlags());
+
     BuildTarget compileBuildTarget = OcamlRuleBuilder.createOcamlLinkTarget(buildTarget);
 
     ImmutableList<BuildRule> rules;
@@ -119,7 +126,7 @@ public BuildRule createBuildRule(
               /* isLibrary */ false,
               args.getBytecodeOnly().orElse(false),
               flags,
-              args.getOcamldepFlags(),
+              ocamldepFlags,
               /* buildNativePlugin */ false,
               withDownwardApi);
       rules = result.getRules();
@@ -138,7 +145,7 @@ public BuildRule createBuildRule(
               /* isLibrary */ false,
               args.getBytecodeOnly().orElse(false),
               flags,
-              args.getOcamldepFlags(),
+              ocamldepFlags,
               withDownwardApi);
       rules = ImmutableList.of(ocamlLibraryBuild);
     }
@@ -180,7 +187,7 @@ default PatternMatchedCollection<ImmutableList<String>> getPlatformLinkerFlags()
       return PatternMatchedCollection.of();
     }
 
-    ImmutableList<String> getOcamldepFlags();
+    ImmutableList<StringWithMacros> getOcamldepFlags();
 
     Optional<String> getWarningsFlags();
 

diff --git a/src/com/facebook/buck/features/ocaml/OcamlBuildContext.java b/src/com/facebook/buck/features/ocaml/OcamlBuildContext.java
@@ -76,7 +76,7 @@ abstract class OcamlBuildContext implements AddsToRuleKey {
   public abstract List<Arg> getFlags();
 
   @AddToRuleKey
-  public abstract ImmutableList<String> getOcamlDepFlags();
+  public abstract List<Arg> getOcamlDepFlags();
 
   @AddToRuleKey
   public abstract List<SourcePath> getInput();

diff --git a/src/com/facebook/buck/features/ocaml/OcamlBuildStep.java b/src/com/facebook/buck/features/ocaml/OcamlBuildStep.java
@@ -87,7 +87,9 @@ public OcamlBuildStep(
         ImmutableList.<String>builder()
             .addAll(
                 this.ocamlContext.getIncludeFlags(/* isBytecode */ false, /* excludeDeps */ true))
-            .addAll(this.ocamlContext.getOcamlDepFlags())
+            .addAll(
+                Arg.stringify(
+                    this.ocamlContext.getOcamlDepFlags(), buildContext.getSourcePathResolver()))
             .build();
 
     this.depToolStep =

diff --git a/src/com/facebook/buck/features/ocaml/OcamlLibraryDescription.java b/src/com/facebook/buck/features/ocaml/OcamlLibraryDescription.java
@@ -113,6 +113,13 @@ public BuildRule createBuildRule(
                               .getMatchingValues(ocamlPlatform.get().getFlavor().toString())))),
               args.getWarningsFlags());
 
+      ImmutableList<Arg> ocamldepFlags =
+          OcamlDescriptionEnhancer.toStringWithMacrosArgs(
+              buildTarget,
+              context.getCellPathResolver(),
+              context.getActionGraphBuilder(),
+              args.getOcamldepFlags());
+
       BuildTarget compileBuildTarget = OcamlRuleBuilder.createStaticLibraryBuildTarget(buildTarget);
 
       if (OcamlRuleBuilder.shouldUseFineGrainedRules(context.getActionGraphBuilder(), srcs)) {
@@ -129,7 +136,7 @@ public BuildRule createBuildRule(
                 /* isLibrary */ true,
                 args.getBytecodeOnly(),
                 flags,
-                args.getOcamldepFlags(),
+                ocamldepFlags,
                 !args.getBytecodeOnly() && args.getNativePlugin(),
                 downwardApiConfig.isEnabledForOCaml());
         return new OcamlStaticLibrary(
@@ -169,7 +176,7 @@ public BuildRule createBuildRule(
                 /* isLibrary */ true,
                 args.getBytecodeOnly(),
                 flags,
-                args.getOcamldepFlags(),
+                ocamldepFlags,
                 downwardApiConfig.isEnabledForOCaml());
         return new OcamlStaticLibrary(
             buildTarget,
@@ -299,7 +306,7 @@ default PatternMatchedCollection<ImmutableList<StringWithMacros>> getPlatformCom
       return PatternMatchedCollection.of();
     }
 
-    ImmutableList<String> getOcamldepFlags();
+    ImmutableList<StringWithMacros> getOcamldepFlags();
 
     ImmutableList<StringWithMacros> getLinkerFlags();
 

diff --git a/src/com/facebook/buck/features/ocaml/OcamlRuleBuilder.java b/src/com/facebook/buck/features/ocaml/OcamlRuleBuilder.java
@@ -205,7 +205,7 @@ static OcamlBuild createBulkCompileRule(
       boolean isLibrary,
       boolean bytecodeOnly,
       ImmutableList<Arg> argFlags,
-      ImmutableList<String> ocamlDepFlags,
+      ImmutableList<Arg> ocamlDepFlags,
       boolean withDownwardApi) {
     CxxPreprocessorInput cxxPreprocessorInputFromDeps =
         CxxPreprocessorInput.concat(
@@ -338,7 +338,7 @@ static OcamlGeneratedBuildRules createFineGrainedBuildRules(
       boolean isLibrary,
       boolean bytecodeOnly,
       ImmutableList<Arg> argFlags,
-      ImmutableList<String> ocamlDepFlags,
+      ImmutableList<Arg> ocamlDepFlags,
       boolean buildNativePlugin,
       boolean withDownwardApi) {
 
@@ -442,7 +442,8 @@ static OcamlGeneratedBuildRules createFineGrainedBuildRules(
 
     AbsPath baseDir = projectFilesystem.getRootPath();
     ImmutableMap<Path, ImmutableList<Path>> mlInput =
-        getMLInputWithDeps(baseDir, ocamlContext, withDownwardApi);
+        getMLInputWithDeps(
+            baseDir, ocamlContext, graphBuilder.getSourcePathResolver(), withDownwardApi);
 
     ImmutableList<SourcePath> cInput = getCInput(graphBuilder.getSourcePathResolver(), srcs);
 
@@ -477,12 +478,15 @@ private static ImmutableList<SourcePath> getCInput(
   }
 
   private static ImmutableMap<Path, ImmutableList<Path>> getMLInputWithDeps(
-      AbsPath baseDir, OcamlBuildContext ocamlContext, boolean withDownwardApi) {
+      AbsPath baseDir,
+      OcamlBuildContext ocamlContext,
+      SourcePathResolverAdapter resolverAdapter,
+      boolean withDownwardApi) {
 
     ImmutableList<String> ocamlDepFlags =
         ImmutableList.<String>builder()
             .addAll(ocamlContext.getIncludeFlags(/* isBytecode */ false, /* excludeDeps */ true))
-            .addAll(ocamlContext.getOcamlDepFlags())
+            .addAll(Arg.stringify(ocamlContext.getOcamlDepFlags(), resolverAdapter))
             .build();
 
     OcamlDepToolStep depToolStep =