From 55154bb88064194dfa4327b94566dcedd6e11945 Mon Sep 17 00:00:00 2001 From: Neil Mitchell Date: Thu, 16 Sep 2021 11:39:03 -0700 Subject: [PATCH] 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 --- .../features/ocaml/OcamlBinaryDescription.java | 13 ++++++++++--- .../buck/features/ocaml/OcamlBuildContext.java | 2 +- .../buck/features/ocaml/OcamlBuildStep.java | 4 +++- .../features/ocaml/OcamlLibraryDescription.java | 13 ++++++++++--- .../buck/features/ocaml/OcamlRuleBuilder.java | 14 +++++++++----- 5 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/com/facebook/buck/features/ocaml/OcamlBinaryDescription.java b/src/com/facebook/buck/features/ocaml/OcamlBinaryDescription.java index fc3018adc49..37bf11bad09 100644 --- 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 ocamldepFlags = + OcamlDescriptionEnhancer.toStringWithMacrosArgs( + buildTarget, + context.getCellPathResolver(), + context.getActionGraphBuilder(), + args.getOcamldepFlags()); + BuildTarget compileBuildTarget = OcamlRuleBuilder.createOcamlLinkTarget(buildTarget); ImmutableList 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> getPlatformLinkerFlags() return PatternMatchedCollection.of(); } - ImmutableList getOcamldepFlags(); + ImmutableList getOcamldepFlags(); Optional getWarningsFlags(); diff --git a/src/com/facebook/buck/features/ocaml/OcamlBuildContext.java b/src/com/facebook/buck/features/ocaml/OcamlBuildContext.java index c8d18dadc21..1d3e71fe28e 100644 --- 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 getFlags(); @AddToRuleKey - public abstract ImmutableList getOcamlDepFlags(); + public abstract List getOcamlDepFlags(); @AddToRuleKey public abstract List getInput(); diff --git a/src/com/facebook/buck/features/ocaml/OcamlBuildStep.java b/src/com/facebook/buck/features/ocaml/OcamlBuildStep.java index b51104b875d..b4877311dd0 100644 --- 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.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 index bf9d4e13590..3710f5a0e34 100644 --- 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 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> getPlatformCom return PatternMatchedCollection.of(); } - ImmutableList getOcamldepFlags(); + ImmutableList getOcamldepFlags(); ImmutableList getLinkerFlags(); diff --git a/src/com/facebook/buck/features/ocaml/OcamlRuleBuilder.java b/src/com/facebook/buck/features/ocaml/OcamlRuleBuilder.java index 74b2e53b8c2..289839d98b7 100644 --- 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 argFlags, - ImmutableList ocamlDepFlags, + ImmutableList ocamlDepFlags, boolean withDownwardApi) { CxxPreprocessorInput cxxPreprocessorInputFromDeps = CxxPreprocessorInput.concat( @@ -338,7 +338,7 @@ static OcamlGeneratedBuildRules createFineGrainedBuildRules( boolean isLibrary, boolean bytecodeOnly, ImmutableList argFlags, - ImmutableList ocamlDepFlags, + ImmutableList ocamlDepFlags, boolean buildNativePlugin, boolean withDownwardApi) { @@ -442,7 +442,8 @@ static OcamlGeneratedBuildRules createFineGrainedBuildRules( AbsPath baseDir = projectFilesystem.getRootPath(); ImmutableMap> mlInput = - getMLInputWithDeps(baseDir, ocamlContext, withDownwardApi); + getMLInputWithDeps( + baseDir, ocamlContext, graphBuilder.getSourcePathResolver(), withDownwardApi); ImmutableList cInput = getCInput(graphBuilder.getSourcePathResolver(), srcs); @@ -477,12 +478,15 @@ private static ImmutableList getCInput( } private static ImmutableMap> getMLInputWithDeps( - AbsPath baseDir, OcamlBuildContext ocamlContext, boolean withDownwardApi) { + AbsPath baseDir, + OcamlBuildContext ocamlContext, + SourcePathResolverAdapter resolverAdapter, + boolean withDownwardApi) { ImmutableList ocamlDepFlags = ImmutableList.builder() .addAll(ocamlContext.getIncludeFlags(/* isBytecode */ false, /* excludeDeps */ true)) - .addAll(ocamlContext.getOcamlDepFlags()) + .addAll(Arg.stringify(ocamlContext.getOcamlDepFlags(), resolverAdapter)) .build(); OcamlDepToolStep depToolStep =