Improve: remove the bisect_ppx dependency and clean Makefile (#1005)

* Cleanup Makefile

Remove `dune-workspace` generation
Don't use paths to `_build`
Remove bisect support
Remove plugin code from dune file

* Cleanup .gitignore

* Don't build whole package

* test-extra: Remove unused code

* test_branch: Use temporary worktree

* Add -noassert in release mode

* Remove dependency to bisect_ppx

* Add placeholder for bisect ppx

* Add tools/bisect.sh

.gitignore

@@ -1,13 +1,8 @@
 .merlin
 /_build
-/dune-workspace
 /ocamlformat.install
+/ocamlformat_diff.install
 /ocamlformat_reason.install
-/src/mod_dep.dot
 /test-extra/code
-/test-extra/bisect*out
-/test/bisect*out
-/bisect*out
-/_coverage
-_opam
-/vendor/ocamlformat_support/ocamlformat_support.install
+/_opam
+/coverage

Makefile

@@ -9,64 +9,46 @@
 #                                                                    #
 ######################################################################
 
-SHELL=bash
-
 .PHONY: default
 default: exe gen-help
 
-dune-workspace: dune-workspace.in
-	sed -e "s|@OPAM_SWITCH[@]|$$(opam switch show)|g" $< > $@
-
-.PHONY: setup
-setup: dune-workspace
-
 .PHONY: exe
-exe: setup
-	dune build _build/dev/src/ocamlformat.exe _build/release/src/ocamlformat.exe
+exe:
+	dune build src/ocamlformat.exe
 
 .PHONY: gen-help
 gen-help:
-	dune build _build/release/ocamlformat-help.txt
+	dune build ocamlformat-help.txt
 
 .PHONY: reason
-reason: setup
-	dune build _build/dev/src/ocamlformat_reason.exe _build/release/src/ocamlformat_reason.exe
+reason:
+	dune build src/ocamlformat_reason.exe
 
 .PHONY: ocamlformat-diff
 ocamlformat-diff:
-	dune build _build/dev/tools/ocamlformat-diff/ocamlformat_diff.exe _build/release/tools/ocamlformat-diff/ocamlformat_diff.exe
+	dune build tools/ocamlformat-diff/ocamlformat_diff.exe
 
-.PHONY: clean cleanbisect
-clean: cleanbisect
-	rm -rf _build dune-workspace
-cleanbisect:
-	rm -Rf _coverage
-	find ./ -name 'bisect*.out' -delete
+.PHONY: clean
+clean:
+	dune clean
 
 SRCS=$(shell \find src tools -name '[^.]*.ml' -or -name '[^.]*.mli' -or -name '[^.]*.mlt')
 
 .PHONY: fmt
 fmt:
-	dune exec --context dev -- ocamlformat -i $(SRCS)
+	dune exec -- ocamlformat -i $(SRCS)
 
-.PHONY: test regtests fixpoint test-reason
+.PHONY: test fixpoint regtests regtests-promote test-reason
 test: fixpoint regtests
 
-fixpoint: setup
-	dune exec --context dev -- ocamlformat -n 1 -i $(SRCS)
-
-regtests: setup
-	dune build @_build/dev/test/runtest
+fixpoint:
+	dune exec -- ocamlformat -n 1 -i $(SRCS)
 
-regtests-promote: setup
-	dune build @_build/dev/test/runtest --auto-promote
+regtests:
+	dune runtest
 
-test-reason: setup
-	dune build @_build/dev/test/reason/runtest-reason --auto-promote
+regtests-promote:
+	dune runtest --auto-promote
 
-.PHONY: coverage
-coverage: setup cleanbisect
-	dune build @_build/coverage/test/runtest
-	dune exec --context coverage -- ocamlformat -i $(SRCS)
-	bisect-ppx-report -I _build/coverage/ -html _coverage/ `find ./ _build/coverage/ -name 'bisect*.out'`
-	@echo "open _coverage/index.html"
+test-reason:
+	dune build @runtest-reason --auto-promote

dune

@@ -9,6 +9,10 @@
 ;                                                                    ;
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
+(env
+ (release
+  (flags (:standard -noassert))))
+
 (rule
  (targets ocamlformat-help.txt)
  (deps ./src/ocamlformat.exe)

ocamlformat.opam

@@ -25,7 +25,6 @@ depends: [
   "ocaml" {>= "4.06"}
   "base" {>= "v0.11.0"}
   "base-unix"
-  "bisect_ppx"
   "cmdliner"
   "dune" {>= "1.11.1"}
   "fpath"

ocamlformat_reason.opam

@@ -19,7 +19,6 @@ depends: [
   "ocaml" {>= "4.06"}
   "base" {>= "v0.11.0"}
   "base-unix"
-  "bisect_ppx"
   "cmdliner"
   "dune" {>= "1.11.1"}
   "fpath"

src/dune

@@ -1,41 +1,30 @@
-(* -*- tuareg -*- *)
-
-let preprocess =
-  match Jbuild_plugin.V1.context with
-  | "coverage" -> "(preprocess (pps bisect_ppx))"
-  | _ -> ""
-
-;;
-Jbuild_plugin.V1.send
-@@ Format.sprintf
-     {|
 (rule
  (targets Version.ml)
  (deps (universe))
- (action (run ocaml %%{dep:../tools/gen_version.mlt} Version.ml))
+ (action (run ocaml %{dep:../tools/gen_version.mlt} Version.ml))
  (mode fallback))
 
 (rule
  (copy#
-  compat/%%{read:compat-file}
+  compat/%{read:compat-file}
   compat.ml))
 
 (rule
  (targets compat-file)
- (action  (run %%{ocaml} %%{dep:../tools/gen_compat.ml} %%{ocaml_version})))
+ (action  (run %{ocaml} %{dep:../tools/gen_compat.ml} %{ocaml_version})))
 
 (ocamllex Literal_lexer)
 
 (executables
  (names ocamlformat ocamlformat_reason)
  (flags (:standard -open Import))
- %s
+ ;;INSERT_BISECT_HERE;;
  (libraries format_ import ocaml-migrate-parsetree odoc.model odoc.parser re uuseg uuseg.string))
 
 (rule
  (targets ocamlformat.1)
  (deps ocamlformat.exe)
- (action (with-stdout-to %%{targets} (run ./ocamlformat.exe --help=groff))))
+ (action (with-stdout-to %{targets} (run ./ocamlformat.exe --help=groff))))
 
 (install
  (section bin)
@@ -51,5 +40,3 @@ Jbuild_plugin.V1.send
  (section man)
  (files ocamlformat.1)
  (package ocamlformat))
-|}
-     preprocess

test-extra/Makefile

@@ -17,11 +17,19 @@ DIRS=code/ocamlformat code/infer code/js_of_ocaml
 # Extra test directories, for which looser checking is done
 XDIRS=code/ocaml
 
+FIND_ARGS= \
+	-name _build -not -prune -or \
+	-path code/ocamlformat/test -not -prune -or \
+	-path code/ocaml/experimental -not -prune -or \
+	-path code/ocaml/testsuite/tests/parse-errors -not -prune -or \
+	-name '*.ml' -and -not -name '*.pp.ml' -or \
+	-name '*.mli' -and -not -name '*.pp.mli'
+
 ALL_DIRS=$(DIRS) $(XDIRS)
 
 # To test using the dev or release executable
 #       make MODE=<mode>
-MODE?=dev
+MODE?=default
 
 REFMT=refmt
 
@@ -87,56 +95,24 @@ test_pull:
 	  git -C $$dir pull; \
 	done
 
-%.pp.ml: %.ml $(OCAMLFORMAT_EXE)
-	@rm -f $*.pp*.ml $*.pp.old.ast $*.pp.new.ast
-	$(OCAMLFORMAT_EXE) $< -o $*.pp.ml
-
-%.pp.mli: %.mli $(OCAMLFORMAT_EXE)
-	@rm -f $*.pp*.mli $*.old.ast $*.new.ast
-	$(OCAMLFORMAT_EXE) $< -o $*.pp.mli
-
-%.ml: %.re $(OCAMLFORMAT_EXE)
-	@rm -f $*.pp.ml $*.old.ast $*.new.ast
-	$(REFMT) --print=binary_reason $< | $(OCAMLFORMAT_EXE) $< --reason-impl -o $@
-
-%.mli: %.rei $(OCAMLFORMAT_EXE)
-	@rm -f $*.pp.mli $*.old.ast $*.new.ast
-	$(REFMT) --print=binary_reason $< | $(OCAMLFORMAT_EXE) $< --reason-intf -o $@
-
-TEST_ML:=$(shell find $(DIRS) -name _build -not -prune -or -path code/ocamlformat/test -not -prune -or -name '*'.ml -and -not -name '*'.pp.ml  2>/dev/null)
-TEST_MLI:=$(shell find $(DIRS) -name _build -not -prune -or -path code/ocamlformat/test -not -prune -or -name '*'.mli -and -not -name '*'.pp.mli 2>/dev/null)
+TEST_FILES:=$(shell find $(DIRS) $(FIND_ARGS) 2>/dev/null)
 
-XTEST_ML:=$(shell find $(XDIRS) -path code/ocaml/experimental -not -prune -or -path code/ocaml/testsuite/tests/parse-errors -not -prune -or -name '*'.ml -and -not -name '*'.pp.ml 2>/dev/null)
-XTEST_MLI:=$(shell find $(XDIRS) -path code/ocaml/experimental -not -prune -or -path code/ocaml/testsuite/tests/parse-errors -not -prune -or -name '*'.mli -and -not -name '*'.pp.mli 2>/dev/null)
-
-TEST_PP_ML:=$(patsubst %.ml,%.pp.ml,$(TEST_ML))
-TEST_PP_MLI:=$(patsubst %.mli,%.pp.mli,$(TEST_MLI))
-
-XTEST_PP_ML:=$(patsubst %.ml,%.pp.ml,$(XTEST_ML))
-XTEST_PP_MLI:=$(patsubst %.mli,%.pp.mli,$(XTEST_MLI))
-
-.PHONY: test_format
-test_format: $(TEST_PP_ML) $(TEST_PP_MLI)
+XTEST_FILES:=$(shell find $(XDIRS) $(FIND_ARGS) 2>/dev/null)
 
 .PHONY: test_inplace
-test_inplace:
-	@parallel --bar $(OCAMLFORMAT_EXE) --no-version-check -i ::: $(TEST_ML) $(TEST_MLI)
+test_inplace: $(OCAMLFORMAT_EXE)
+	@parallel --bar $(OCAMLFORMAT_EXE) --no-version-check -i ::: $(TEST_FILES)
 
-.PHONY: test_inplace
-test_extra:
-	@parallel --bar $(OCAMLFORMAT_EXE) --no-version-check --quiet -i ::: $(XTEST_ML) $(XTEST_MLI)
+.PHONY: test_extra
+test_extra: $(OCAMLFORMAT_EXE)
+	@parallel --bar $(OCAMLFORMAT_EXE) --no-version-check --quiet -i ::: $(XTEST_FILES)
 
 .PHONY: test_margins
 test_margins:
 	@for i in {100..40}; do echo $$i; OCAMLFORMAT_MARGIN=$$i $(MAKE) test || break; done
 
-.PHONY: clean_pp
-clean_pp:
-	@find $(ALL_DIRS) \
-	 \(  -name '*'.pp'*'.ml \
-	 -or -name '*'.pp'*'.mli \
-	 -or -name '*'.ast \
-	 \) -delete
+$(OCAMLFORMAT_EXE):
+	dune build "$@"
 
 print-%:
 	@echo '$*=$($*)'

tools/bisect.sh

@@ -0,0 +1,41 @@
+#!/usr/bin/env bash
+
+######################################################################
+#                                                                    #
+#                            OCamlFormat                             #
+#                                                                    #
+#  Copyright (c) 2017-present, Facebook, Inc.  All rights reserved.  #
+#                                                                    #
+#  This source code is licensed under the MIT license found in the   #
+#  LICENSE file in the root directory of this source tree.           #
+#                                                                    #
+######################################################################
+
+# usage: bisect.sh [<rev>]
+#
+# The first arg is the revision to test. By default HEAD.
+#
+# Run the testsuite with ppx_bisect enabled.
+
+set -e
+
+if [[ ! -z "$1" ]]; then
+    branch="$1"
+else
+    branch=$(git rev-parse HEAD)
+fi
+
+tmp=`mktemp -d`
+git worktree add --detach "$tmp" "$branch"
+
+sed -i 's/;;INSERT_BISECT_HERE;;/(preprocess (pps bisect_ppx))/' "$tmp/src/dune"
+
+# Run the tests
+make -C "$tmp" test
+
+dst="coverage"
+bisect-ppx-report -I "$tmp/_build/default" -html "$dst" `find "$tmp" -name 'bisect*.out'`
+echo "Coverage report generated in $dst/"
+echo " => open $dst/index.html"
+
+git worktree remove -f "$tmp"

tools/test_branch.sh

@@ -31,9 +31,17 @@ fi
 
 base=$(git merge-base master $branch)
 
-git checkout $base
-make
-OCAMLFORMAT="$2" make -C test-extra test_setup test_unstage test_clean test_pull test test_stage
-git checkout $branch
-make
-OCAMLFORMAT="$3" make -C test-extra test test_diff
+# First arg is git rev others are passed to make -C test-extra
+run_in_worktree ()
+{
+  local tmp=`mktemp -d`
+  git worktree add --detach "$tmp" "$1"
+  shift
+  make -C "$tmp"
+  local exe=`ls "$tmp"/_build/{default,dev}/src/ocamlformat.exe 2>/dev/null | head -n1`
+  OCAMLFORMAT_EXE=$exe make -C test-extra "$@"
+  git worktree remove -f "$tmp"
+}
+
+OCAMLFORMAT="$2" run_in_worktree "$base" test_setup test_unstage test_clean test_pull test test_stage
+OCAMLFORMAT="$3" run_in_worktree "$branch" test test_diff