Skip to content

Commit

Permalink
buck2 ocaml ide output: enable --show-output and be more consistent
Browse files Browse the repository at this point in the history
Summary:
## What

Use `DefaultInfo` instead of a custom provider type for the `.cmt` and `.cmti` files we produce for IDE support.

## Why

D52840634 introduced the ability to build IDE outputs without linking, but had the following downsides:
- `--show-output` and `--show-full-output` didn't show anything for IDE targets, which impaired debuggability
- (warning: long explanation, skippable:) I didn't make the change to use `OCamlIdeInfo` all OCaml rules, which was inconsistent and probably didn't avoid all linking. When I did try to make the change for all OCaml IDE rules, the expected .cmt files were not generated. I suspect a target needs to have a default_output in order for this line of our BXL for generating build artifacts to do anything:

https://www.internalfb.com/code/fbsource/[3905ae9c88fb4245b97cbac20c5a387c1f816655]/fbcode/common/ocaml/gen_merlin.bxl?lines=18-21

Reviewed By: ndmitchell

Differential Revision: D52952776

fbshipit-source-id: 9409ef710d6f4def1a7a4ed5ebf280850fa4d169
  • Loading branch information
mheiber authored and facebook-github-bot committed Jan 23, 2024
1 parent 6115f1e commit f5894f3
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 11 deletions.
12 changes: 7 additions & 5 deletions prelude/ocaml/ocaml.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ load("@prelude//utils:graph_utils.bzl", "breadth_first_traversal", "post_order_t
load("@prelude//utils:platform_flavors_util.bzl", "by_platform")
load("@prelude//utils:utils.bzl", "filter_and_map_idx", "flatten")
load(":makefile.bzl", "parse_makefile")
load(":ocaml_toolchain_types.bzl", "OCamlLibraryInfo", "OCamlLinkInfo", "OCamlToolchainInfo", "OcamlIdeInfo", "OtherOutputsInfo", "merge_ocaml_link_infos", "merge_other_outputs_info")
load(":ocaml_toolchain_types.bzl", "OCamlLibraryInfo", "OCamlLinkInfo", "OCamlToolchainInfo", "OtherOutputsInfo", "merge_ocaml_link_infos", "merge_other_outputs_info")

BuildMode = enum("native", "bytecode", "expand")

Expand Down Expand Up @@ -688,8 +688,9 @@ def ocaml_library_impl(ctx: AnalysisContext) -> list[Provider]:
other_outputs_info = merge_other_outputs_info(ctx, other_outputs, _attr_deps_other_outputs_infos(ctx))

info_ide = [
OcamlIdeInfo(
outputs = [cmd_args(other_outputs_info.info.project_as_args("ide"))],
DefaultInfo(
default_output = cmts_nat[0] if cmts_nat else None,
other_outputs = [cmd_args(other_outputs_info.info.project_as_args("ide"))],
),
]
info_byt = [
Expand Down Expand Up @@ -783,6 +784,7 @@ def ocaml_binary_impl(ctx: AnalysisContext) -> list[Provider]:

info_ide = [
DefaultInfo(
default_output = cmts_nat[0] if cmts_nat else None,
other_outputs = [cmd_args(other_outputs_info.info.project_as_args("ide"))],
),
]
Expand Down Expand Up @@ -874,7 +876,7 @@ def ocaml_object_impl(ctx: AnalysisContext) -> list[Provider]:

info_ide = [
DefaultInfo(
default_output = obj,
default_output = cmts[0] if cmts else None,
other_outputs = [cmd_args(other_outputs_info.info.project_as_args("ide"))],
),
]
Expand Down Expand Up @@ -956,7 +958,7 @@ def ocaml_shared_impl(ctx: AnalysisContext) -> list[Provider]:

info_ide = [
DefaultInfo(
default_output = binary_nat,
default_output = cmts_nat[0] if cmts_nat else None,
other_outputs = [cmd_args(other_outputs_info.info.project_as_args("ide"))],
),
]
Expand Down
6 changes: 0 additions & 6 deletions prelude/ocaml/ocaml_toolchain_types.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,6 @@ OCamlLinkInfo = provider(
fields = {"info": provider_field(typing.Any, default = None)},
)

OcamlIdeInfo = provider(
fields = {
"outputs": provider_field(typing.Any, default = []),
},
)

# A record of an OCaml library.
OCamlLibraryInfo = record(
# The library target name: e.g. "`foo`"
Expand Down

0 comments on commit f5894f3

Please sign in to comment.