From 31dd2884f63c8917bf61a37965bb86c6ec7ffb3f Mon Sep 17 00:00:00 2001
From: Josh Berdine <jjb@fb.com>
Date: Mon, 23 Mar 2020 06:19:31 -0700
Subject: [PATCH] [sledge] Use a cookie instead of env var to enable ppx_trace

Summary:
Passing info that affects the build through environment variables is
bad since it is not very visible, e.g. in log files. Also, those
settings are not recorded in .merlin files, leading to merlin using
the default configuration.

This diff changes ppx_trace to use a 'cookie' instead of an
environment variable, and configures dune to set this cookie based on
the selected profile. This has the benefit of using command line
arguments such as `--cookie 'ppx_trace_enabled="1"'` which appear in
build logs and .merlin files.

Reviewed By: jvillard

Differential Revision: D20590515

fbshipit-source-id: d2daceaa3
---
 sledge/ppx_sledge/dune        |  5 ++++-
 sledge/ppx_trace/ppx_trace.ml | 29 ++++++++++++++++-------------
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/sledge/ppx_sledge/dune b/sledge/ppx_sledge/dune
index eb713bc64..901c3454d 100644
--- a/sledge/ppx_sledge/dune
+++ b/sledge/ppx_sledge/dune
@@ -5,7 +5,10 @@
 
 (library
  (name ppx_sledge)
- (kind ppx_rewriter)
+ (kind
+  (ppx_rewriter
+   (cookies
+    (ppx_trace_enabled %{env:PPX_TRACE_ENABLED=0}))))
  (libraries ppx_compare ppx_expect ppx_hash ppx_here ppx_inline_test ppx_let
    ppx_sexp_conv ppx_sexp_value ppx_trace)
  (preprocess no_preprocessing))
diff --git a/sledge/ppx_trace/ppx_trace.ml b/sledge/ppx_trace/ppx_trace.ml
index 2a382b82d..80e92ee6a 100644
--- a/sledge/ppx_trace/ppx_trace.ml
+++ b/sledge/ppx_trace/ppx_trace.ml
@@ -7,16 +7,19 @@
 
 (** Extension point rewriter for debug trace logging
 
-    This ppx rewriter declares a [--debug] command line option, to be passed
-    by the build system in debug but not optimized build modes. Setting the
-    [PPX_TRACE_ENABLED] environment variable to [1] or [true] has the same
-    effect as passing [--debug].
+    This ppx rewriter reads a cookie to determine whether to rewrite in
+    "debug" mode or not. To enable "debug" mode, pass
+    [--cookie 'ppx_trace_enabled="1"'] (or with [true] instead or [1]).
+
+    named "ppx_trace_enabled" declares a [--debug] command line option, to
+    be passed by the build system in debug but not optimized build modes.
+    Setting the [PPX_TRACE_ENABLED] environment variable to [1] or [true]
+    has the same effect as passing [--debug].
 
     It rewrites [\[%Trace.info f\]] to a call
     [\[Trace.info mod_name fun_name f\]] where [mod_name] and [fun_name] are
     the enclosing module and function names in the parsetree. This is only
-    done in debug mode, if [--debug] is not passed, then [\[%Trace.info f\]]
-    is rewritten to [()].
+    done in debug mode, otherwise [\[%Trace.info f\]] is rewritten to [()].
 
     Similarly, [\[%Trace.call\]] is rewritten to a call to [Trace.call] or
     [()], and [\[%Trace.retn\]] to a call to [Trace.retn] or [Fn.id].
@@ -38,20 +41,20 @@
     only in debug mode.
 
     Additionally, [\[%debug\]] is rewritten to the compile-time boolean
-    constant determined by whether or not [--debug] is passed. *)
+    constant indicating if rewriting was done in debug mode. *)
 
 open Ppxlib
 open Ast_builder.Default
 module Ast_mapper = Selected_ast.Ast.Ast_mapper
 
-let debug =
-  ref
-    ( match Sys.getenv_opt "PPX_TRACE_ENABLED" with
-    | Some ("1" | "true") -> true
-    | _ -> false )
+let debug = ref false
 
 ;;
-Driver.add_arg "--debug" (Arg.Set debug) ~doc:"Enable debug tracing output"
+Driver.Cookies.add_simple_handler "ppx_trace_enabled" Ast_pattern.__
+  ~f:(function
+  | Some {pexp_desc= Pexp_constant (Pconst_string (("1" | "true"), _))} ->
+      debug := true
+  | _ -> () )
 
 let rec get_fun_name pat =
   match pat.ppat_desc with