From 9baef3d62819183f7648cb7971763bc86b3ab806 Mon Sep 17 00:00:00 2001 From: mattiasdrp Date: Wed, 8 Jan 2025 16:40:15 +0100 Subject: [PATCH 1/2] Manifest: Upgrade opam-repo to include ppxlib 0.34.0 --- flake.lock | 6 +++--- manifest/externals.ml | 2 +- opam/octez-internal-libs.opam | 2 +- opam/octez-libs.opam | 2 +- opam/virtual/octez-deps.opam | 2 +- opam/virtual/octez-deps.opam.locked | 2 +- scripts/version.sh | 2 +- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/flake.lock b/flake.lock index 3cf8985e44c5..2076139d677f 100644 --- a/flake.lock +++ b/flake.lock @@ -121,11 +121,11 @@ "opam-repository": { "flake": false, "locked": { - "lastModified": 1734734649, - "narHash": "sha256-/v7vZQbiCjBaXFEkSZH6llNOe6VuGiLDOFsWKDa9h6s=", + "lastModified": 1736340517, + "narHash": "sha256-V7L5xjLAA0GlP4U5gDXMgtr43tkpBYyr43X4SPMr9iI=", "owner": "ocaml", "repo": "opam-repository", - "rev": "1db3104e98a25ff2b5f859189c9408dc760260e7", + "rev": "e566c29b12a9c284b68b3f6cbe7fbd4e8452406f", "type": "github" }, "original": { diff --git a/manifest/externals.ml b/manifest/externals.ml index 62d757dd8cba..a47b6ac4b0dd 100644 --- a/manifest/externals.ml +++ b/manifest/externals.ml @@ -203,7 +203,7 @@ let optint = external_lib "optint" V.True let ppx_expect = inline_tests_backend (external_lib "ppx_expect" V.True) -let ppxlib = external_lib "ppxlib" V.True +let ppxlib = external_lib "ppxlib" V.(at_least "0.34.0") let ppxlib_metaquot = external_sublib ppxlib "ppxlib.metaquot" diff --git a/opam/octez-internal-libs.opam b/opam/octez-internal-libs.opam index ff876c710d46..4fc3d44c07da 100644 --- a/opam/octez-internal-libs.opam +++ b/opam/octez-internal-libs.opam @@ -17,7 +17,7 @@ depends: [ "ocaml" { >= "4.14" } "ppx_repr" { >= "0.6.0" } "logs" - "ppxlib" + "ppxlib" { >= "0.34.0" } "bigstringaf" { >= "0.5.0" } "fmt" { >= "0.8.7" } "astring" diff --git a/opam/octez-libs.opam b/opam/octez-libs.opam index 2a493a6376c0..66fad20189ba 100644 --- a/opam/octez-libs.opam +++ b/opam/octez-libs.opam @@ -64,7 +64,7 @@ depends: [ "progress" { >= "0.1.0" } "camlp-streams" { >= "5.0.1" } "uutf" - "ppxlib" + "ppxlib" { >= "0.34.0" } "bheap" { >= "2.0.0" } "digestif" { >= "0.9.0" } "jsonm" diff --git a/opam/virtual/octez-deps.opam b/opam/virtual/octez-deps.opam index 5366d8dc88f7..dfa87e649bbb 100644 --- a/opam/virtual/octez-deps.opam +++ b/opam/virtual/octez-deps.opam @@ -86,7 +86,7 @@ depends: [ "ppx_import" "ppx_repr" { >= "0.6.0" } "ppx_sexp_conv" - "ppxlib" + "ppxlib" { >= "0.34.0" } "prbnmcn-linalg" { = "0.0.1" } "prbnmcn-stats" { = "0.0.6" } "pringo" { >= "1.3" & < "1.4" } diff --git a/opam/virtual/octez-deps.opam.locked b/opam/virtual/octez-deps.opam.locked index ade410f0cbd6..4d14fbd19593 100644 --- a/opam/virtual/octez-deps.opam.locked +++ b/opam/virtual/octez-deps.opam.locked @@ -197,7 +197,7 @@ depends: [ "ppx_optcomp" {= "v0.16.0"} "ppx_repr" {= "0.7.0"} "ppx_sexp_conv" {= "v0.16.0"} - "ppxlib" {= "0.33.0"} + "ppxlib" {= "0.34.0"} "prbnmcn-basic-structures" {= "0.0.1"} "prbnmcn-linalg" {= "0.0.1"} "prbnmcn-stats" {= "0.0.6"} diff --git a/scripts/version.sh b/scripts/version.sh index dd93a68fa4f3..fde80395a8b7 100644 --- a/scripts/version.sh +++ b/scripts/version.sh @@ -24,7 +24,7 @@ export recommended_node_version=18.18.2 ## opam_repository is a commit hash of the public opam repository, i.e. ## https://github.com/ocaml/opam-repository -export opam_repository_tag=1db3104e98a25ff2b5f859189c9408dc760260e7 +export opam_repository_tag=e566c29b12a9c284b68b3f6cbe7fbd4e8452406f # SHA-256 hashes of the DAL SRSs, as used in 'scripts/install_dal_trusted_setup.sh' to verify # integrity of downloaded SRS. -- GitLab From 75763c4d6bcbe09b109f5f3a913eb83a52b4a7e9 Mon Sep 17 00:00:00 2001 From: mattiasdrp Date: Wed, 8 Jan 2025 16:56:47 +0100 Subject: [PATCH 2/2] Ppx_profiler: Use proper types and finally pretty-print wrong payloads --- src/lib_ppx_profiler/error.ml | 43 +++++++++++++++++++++----------- src/lib_ppx_profiler/error.mli | 10 +++++--- src/lib_ppx_profiler/rewriter.ml | 43 ++++++++++++++++---------------- 3 files changed, 57 insertions(+), 39 deletions(-) diff --git a/src/lib_ppx_profiler/error.ml b/src/lib_ppx_profiler/error.ml index 10dd04cee64b..565f5f278080 100644 --- a/src/lib_ppx_profiler/error.ml +++ b/src/lib_ppx_profiler/error.ml @@ -7,7 +7,7 @@ type error = | Invalid_action of string - | Invalid_payload + | Invalid_payload of bool * Ppxlib.payload | Invalid_aggregate of Key.t | Invalid_custom of Key.t | Invalid_mark of Key.t @@ -15,18 +15,19 @@ type error = | Invalid_span of Key.t | Invalid_stop of Key.t | Invalid_list_of_driver_ids of Ppxlib.expression list - | Improper_field of (Longident.t Location.loc * Ppxlib.expression) - | Improper_list_field of (Longident.t Location.loc * Ppxlib.expression) + | Improper_field of (Ppxlib.longident_loc * Ppxlib.expression) + | Improper_list_field of (Ppxlib.longident_loc * Ppxlib.expression) | Improper_let_binding of Ppxlib.expression - | Improper_record of (Ppxlib.Ast.longident_loc * Ppxlib.expression) list + | Improper_record of (Ppxlib.longident_loc * Ppxlib.expression) list | Malformed_attribute of Ppxlib.expression | No_verbosity of Key.t let pp_field ppf (lident, expr) = Format.fprintf ppf - "%s = %a" - (Ppxlib.Longident.name lident.Ppxlib.txt) + "%a = %a" + Ppxlib.Pprintast.longident + lident.Ppxlib.txt Ppxlib.Pprintast.expression expr @@ -44,14 +45,28 @@ let error loc err = Format.fprintf ppf "%s" (Constants.get_action constant))) Constants.constants action ) - | Invalid_payload -> + | Invalid_payload (missing_record, payload) -> ( "Invalid or empty attribute payload.", Format.asprintf "@[Accepted attributes payload are:@,\ - - `[@profiler.aggregate_* ]@,\ - - `[@profiler.mark []]@,\ - - `[@profiler.record_* ]@,\ - - `[@profiler.span_* ]@." ) + - [@profiler.aggregate_* ]@,\ + - [@profiler.mark []]@,\ + - [@profiler.record_* ]@,\ + - [@profiler.span_* ]@,\ + %aFound: @[%a@]@." + (fun ppf () -> + if missing_record then + Format.fprintf + ppf + "@[With containing the following fields:@,\ + - verbosity@,\ + - profiler_module@,\ + - metadata@,\ + - drivers_ids@]@," + else ()) + () + Ppxlib.Pprintast.payload + payload ) | Invalid_aggregate key -> ( "Invalid aggregate.", Format.asprintf @@ -103,7 +118,7 @@ let error loc err = "@[It looks like you tried to provide a list of opt-in \ drivers through the `driver_ids` field but no drivers could be \ parsed out of it. A list of opt-in drivers should be a list of \ - modules or idents like `[Opentelemetry; prometheus]@,\ + modules or idents like [Opentelemetry; prometheus]@,\ Found: { @[%a@] }@." Format.( pp_print_list @@ -153,8 +168,8 @@ let error loc err = ( "Malformed attribute.", Format.asprintf "@[Accepted attributes payload are:@,\ - - `[@profiler.mark []]'@,\ - - `[@profiler.aggregate_* ]@,\ + - [@profiler.mark []]@,\ + - [@profiler.aggregate_* ]@,\ Found %a@.'" Ppxlib.Pprintast.expression expr ) diff --git a/src/lib_ppx_profiler/error.mli b/src/lib_ppx_profiler/error.mli index 51596153e737..f35a19392745 100644 --- a/src/lib_ppx_profiler/error.mli +++ b/src/lib_ppx_profiler/error.mli @@ -7,7 +7,9 @@ type error = | Invalid_action of string - | Invalid_payload + | Invalid_payload of bool * Ppxlib.payload + (* The [bool] represents the fact that the record is missing or not to have + a better message display when encountering an invalid payload *) | Invalid_aggregate of Key.t | Invalid_custom of Key.t | Invalid_mark of Key.t @@ -15,10 +17,10 @@ type error = | Invalid_span of Key.t | Invalid_stop of Key.t | Invalid_list_of_driver_ids of Ppxlib.expression list - | Improper_field of (Longident.t Location.loc * Ppxlib.expression) - | Improper_list_field of (Longident.t Location.loc * Ppxlib.expression) + | Improper_field of (Ppxlib.longident_loc * Ppxlib.expression) + | Improper_list_field of (Ppxlib.longident_loc * Ppxlib.expression) | Improper_let_binding of Ppxlib.expression - | Improper_record of (Ppxlib.Ast.longident_loc * Ppxlib.expression) list + | Improper_record of (Ppxlib.longident_loc * Ppxlib.expression) list | Malformed_attribute of Ppxlib.expression | No_verbosity of Key.t diff --git a/src/lib_ppx_profiler/rewriter.ml b/src/lib_ppx_profiler/rewriter.ml index 311a2a004894..3ab197a15492 100644 --- a/src/lib_ppx_profiler/rewriter.ml +++ b/src/lib_ppx_profiler/rewriter.ml @@ -292,28 +292,29 @@ let extract_key_from_payload loc payload = [%e? { pexp_desc = - Pexp_apply - ( {pexp_desc = Pexp_record (record, _); _}, - [(Nolabel, structure)] ); + Pexp_apply ({pexp_desc = Pexp_record (record, _); _}, item_list); _; }]]; - ] -> - (* [@ppx {} ...] *) - let verbosity, profiler_module, metadata, driver_ids = - extract_from_record loc record - in - (match (verbosity, profiler_module, metadata) with - | None, None, None when Handled_drivers.is_empty driver_ids -> - Error.error loc Error.(Improper_record record) - | _ -> ()) ; - Key. - { - verbosity; - profiler_module; - metadata; - driver_ids; - content = extract_content_from_structure loc structure; - } + ] -> ( + match item_list with + | [(Nolabel, structure)] -> + (* [@ppx {} ...] *) + let verbosity, profiler_module, metadata, driver_ids = + extract_from_record loc record + in + (match (verbosity, profiler_module, metadata) with + | None, None, None when Handled_drivers.is_empty driver_ids -> + Error.error loc Error.(Improper_record record) + | _ -> ()) ; + Key. + { + verbosity; + profiler_module; + metadata; + driver_ids; + content = extract_content_from_structure loc structure; + } + | _ -> Error.error loc (Invalid_payload (false, payload))) | Ppxlib.PStr [[%stri [%e? structure]]] -> (* [@ppx ...] *) Key. @@ -334,7 +335,7 @@ let extract_key_from_payload loc payload = driver_ids = Handled_drivers.empty; content = Key.Empty; } - | _ -> Error.error loc Invalid_payload + | _ -> Error.error loc (Invalid_payload (true, payload)) let of_attribute handled_drivers ({Ppxlib.attr_payload; attr_loc; _} as attribute) = -- GitLab