From 66ad5c3018ee8e9f0d705b261f8ef76d2ae560ee Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Tue, 23 Jan 2018 08:07:19 -0800 Subject: [PATCH] [driver] persist some global state across infer runs Summary: Record the db schema, infer version, and run dates into infer-out/.infer_runstate.json. This allows us to check on startup whether the results directory was generated using a compatible version of infer or not, and give a better error message in the latter case than some SQLite error about mismatching tables. This will be used in a follow-up diff to record capture phases too, and avoid relying on filesystem timestamps of the infer-out/capture/foo/ directories for reactive analysis. Had to change some tests Makefiles to make sure they do not attempt to re-use stale infer-out directories, which would now fail the run. The stale infer-out directory gets deleted if `--force-delete-results-dir` is passed (but a warning still gets printed). Reviewed By: mbouaziz Differential Revision: D6760787 fbshipit-source-id: f36f7df --- .gitignore | 1 + infer/src/Makefile | 2 +- infer/src/atd/runstate.atd | 23 +++++++ infer/src/backend/infer.ml | 6 +- infer/src/base/ResultsDatabase.ml | 28 ++++---- infer/src/base/ResultsDatabase.mli | 3 + infer/src/base/ResultsDir.ml | 32 +++++++-- infer/src/base/RunState.ml | 68 +++++++++++++++++++ infer/src/base/RunState.mli | 22 ++++++ infer/src/base/Version.mli | 6 ++ infer/src/integration/Driver.ml | 14 ++-- .../Makefile | 1 - infer/tests/build_systems/genrule/Makefile | 13 ++-- infer/tests/differential.make | 6 +- infer/tests/infer.make | 12 ++-- 15 files changed, 199 insertions(+), 38 deletions(-) create mode 100644 infer/src/atd/runstate.atd create mode 100644 infer/src/base/RunState.ml create mode 100644 infer/src/base/RunState.mli diff --git a/.gitignore b/.gitignore index 4a929ce91..357c594e1 100644 --- a/.gitignore +++ b/.gitignore @@ -43,6 +43,7 @@ duplicates.txt /infer/tests/build_systems/diff/src /infer/tests/build_systems/diff_*/src /infer/tests/build_systems/buck_flavors_deterministic/capture_hash-*.sha +/infer/tests/build_systems/genrule/report.json /_release # generated by oUnit diff --git a/infer/src/Makefile b/infer/src/Makefile index c7e7f1a72..0e0151528 100644 --- a/infer/src/Makefile +++ b/infer/src/Makefile @@ -21,7 +21,7 @@ INFER_MAIN = infer #### Checkers declarations #### -INFER_ATDGEN_STUB_BASES = atd/jsonbug atd/stacktree +INFER_ATDGEN_STUB_BASES = atd/jsonbug atd/runstate atd/stacktree INFER_ATDGEN_TYPES = j t INFER_ATDGEN_STUB_ATDS = $(INFER_ATDGEN_STUB_BASES:.atd) INFER_ATDGEN_SUFFIXES = $(foreach atd_t,$(INFER_ATDGEN_TYPES),_$(atd_t).ml _$(atd_t).mli) diff --git a/infer/src/atd/runstate.atd b/infer/src/atd/runstate.atd new file mode 100644 index 000000000..3ded5f6b9 --- /dev/null +++ b/infer/src/atd/runstate.atd @@ -0,0 +1,23 @@ +type infer_version = { + major: int; + minor: int; + patch: int; + commit: string; +} + +type command = string wrap + +type run_info = { + date: string; + command: command; + infer_version: infer_version; +} + +type t = { + run_sequence: run_info list; (** successive runs that re-used the same results directory *) + results_dir_format: string; (** to check if the versions of the results dir are compatible *) +} diff --git a/infer/src/backend/infer.ml b/infer/src/backend/infer.ml index dd76daba0..0d6a092c2 100644 --- a/infer/src/backend/infer.ml +++ b/infer/src/backend/infer.ml @@ -26,7 +26,7 @@ let run driver_mode = let setup () = - match Config.command with + ( match Config.command with | Analyze -> ResultsDir.assert_results_dir "have you run capture before?" | Report | ReportDiff -> @@ -45,7 +45,9 @@ let setup () = | Explore -> ResultsDir.assert_results_dir "please run an infer analysis first" | Events -> - ResultsDir.assert_results_dir "have you run infer before?" + ResultsDir.assert_results_dir "have you run infer before?" ) ; + if CLOpt.is_originator then ( RunState.add_run_to_sequence () ; RunState.store () ) ; + () let print_active_checkers () = diff --git a/infer/src/base/ResultsDatabase.ml b/infer/src/base/ResultsDatabase.ml index 4fe0356b5..befb8d8c4 100644 --- a/infer/src/base/ResultsDatabase.ml +++ b/infer/src/base/ResultsDatabase.ml @@ -17,28 +17,32 @@ let database_filename = "results.db" let database_fullpath = Config.results_dir ^/ database_filename -let create_procedures_table db = - (* it would be nice to use "WITHOUT ROWID" here but ancient versions of sqlite do not support - it *) - SqliteUtils.exec db ~log:"creating procedures table" - ~stmt: - {| -CREATE TABLE IF NOT EXISTS procedures +let procedures_schema = + {|CREATE TABLE IF NOT EXISTS procedures ( proc_name TEXT PRIMARY KEY , attr_kind INTEGER NOT NULL , source_file TEXT NOT NULL , proc_attributes BLOB NOT NULL )|} -let create_source_files_table db = - SqliteUtils.exec db ~log:"creating source_files table" - ~stmt: - {| -CREATE TABLE IF NOT EXISTS source_files +let source_files_schema = + {|CREATE TABLE IF NOT EXISTS source_files ( source_file TEXT PRIMARY KEY , cfgs BLOB NOT NULL )|} +let schema_hum = Printf.sprintf "%s;\n%s" procedures_schema source_files_schema + +let create_procedures_table db = + (* it would be nice to use "WITHOUT ROWID" here but ancient versions of sqlite do not support + it *) + SqliteUtils.exec db ~log:"creating procedures table" ~stmt:procedures_schema + + +let create_source_files_table db = + SqliteUtils.exec db ~log:"creating source_files table" ~stmt:source_files_schema + + let create_db () = let temp_db = Filename.temp_file ~in_dir:Config.results_dir database_filename ".tmp" in let db = Sqlite3.db_open ~mutex:`FULL temp_db in diff --git a/infer/src/base/ResultsDatabase.mli b/infer/src/base/ResultsDatabase.mli index c89619cb2..f0cceb943 100644 --- a/infer/src/base/ResultsDatabase.mli +++ b/infer/src/base/ResultsDatabase.mli @@ -13,6 +13,9 @@ val database_filename : string val database_fullpath : string (** the absolute path to the database file *) +val schema_hum : string +(** some human-readable string describing the tables *) + val get_database : unit -> Sqlite3.db (** The results database. You should always use this function to access the database, as the connection to it may change during the execution (see [new_database_connection]). *) diff --git a/infer/src/base/ResultsDir.ml b/infer/src/base/ResultsDir.ml index ed6143c64..efdc8917e 100644 --- a/infer/src/base/ResultsDir.ml +++ b/infer/src/base/ResultsDir.ml @@ -8,6 +8,7 @@ *) open! IStd open! PVariant +module CLOpt = CommandLineOption module L = Logging let results_dir_dir_markers = @@ -30,16 +31,22 @@ let is_results_dir ~check_correct_version () = Result.ok_if_true has_all_markers ~error:(Printf.sprintf "'%s' not found" !not_found) +let non_empty_directory_exists results_dir = + (* Look if [results_dir] exists and is a non-empty directory. If it's an empty directory, leave it + alone. This allows users to create a temporary directory for the infer results without infer + removing it to recreate it, which could be racy. *) + Sys.is_directory results_dir = `Yes && not (Utils.directory_is_empty results_dir) + + let remove_results_dir () = - (* Look if file exists, it may not be a directory but that will be caught by the call to [is_results_dir]. If it's an empty directory, leave it alone. This allows users to create a temporary directory for the infer results without infer removing it to recreate it, which could be racy. *) - if Sys.file_exists Config.results_dir = `Yes && not (Utils.directory_is_empty Config.results_dir) - then ( + if non_empty_directory_exists Config.results_dir then ( if not Config.force_delete_results_dir then Result.iter_error (is_results_dir ~check_correct_version:false ()) ~f:(fun err -> L.(die UserError) "ERROR: '%s' exists but does not seem to be an infer results directory: %s@\nERROR: Please delete '%s' and try again@." Config.results_dir err Config.results_dir ) ; - Utils.rmtree Config.results_dir ) + Utils.rmtree Config.results_dir ) ; + RunState.reset () let prepare_logging_and_db () = @@ -50,17 +57,30 @@ let prepare_logging_and_db () = let create_results_dir () = + if non_empty_directory_exists Config.results_dir then + RunState.load_and_validate () + |> Result.iter_error ~f:(fun error -> + if Config.force_delete_results_dir then ( + L.user_warning + "%s@\nDeleting results dir because --force-delete-results-dir was passed@." error ; + remove_results_dir () ) + else L.die UserError "%s@\nPlease remove '%s' and try again" error Config.results_dir ) ; Unix.mkdir_p Config.results_dir ; Unix.mkdir_p (Config.results_dir ^/ Config.events_dir_name) ; List.iter ~f:Unix.mkdir_p results_dir_dir_markers ; - prepare_logging_and_db () + prepare_logging_and_db () ; + () let assert_results_dir advice = Result.iter_error (is_results_dir ~check_correct_version:true ()) ~f:(fun err -> L.(die UserError) "ERROR: No results directory at '%s': %s@\nERROR: %s@." Config.results_dir err advice ) ; - prepare_logging_and_db () + RunState.load_and_validate () + |> Result.iter_error ~f:(fun error -> + L.die UserError "%s@\nPlease remove '%s' and try again" error Config.results_dir ) ; + prepare_logging_and_db () ; + () let delete_capture_and_analysis_data () = diff --git a/infer/src/base/RunState.ml b/infer/src/base/RunState.ml new file mode 100644 index 000000000..e7c611049 --- /dev/null +++ b/infer/src/base/RunState.ml @@ -0,0 +1,68 @@ +(* + * Copyright (c) 2018 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + *) + +open! IStd + +let run_time_string = Time.now () |> Time.to_string + +let state0 = + let open Runstate_t in + { run_sequence= [] + ; results_dir_format= + Printf.sprintf "db_filename: %s\ndb_schema: %s" ResultsDatabase.database_filename + ResultsDatabase.schema_hum } + + +let state : Runstate_t.t ref = ref state0 + +let add_run_to_sequence () = + let run = + { Runstate_t.infer_version= Version.{Runstate_t.major; minor; patch; commit} + ; date= run_time_string + ; command= Config.command } + in + Runstate_t.(state := {(!state) with run_sequence= run :: !state.run_sequence}) + + +let state_filename = ".infer_runstate.json" + +let state_file_path = Config.results_dir ^/ state_filename + +let store () = + Utils.with_file_out state_file_path ~f:(fun oc -> + Runstate_j.string_of_t !state |> Out_channel.output_string oc ) + + +let load_and_validate () = + let error msg = + Printf.ksprintf + (fun err_msg -> + Error + (Printf.sprintf + "Incompatible results directory '%s':\n%s\nWas '%s' created using an older version of infer?" + Config.results_dir err_msg Config.results_dir) ) + msg + in + if Sys.file_exists state_file_path <> `Yes then error "save state not found" + else + try + let loaded_state = Ag_util.Json.from_file Runstate_j.read_t state_file_path in + if not + (String.equal !state.Runstate_t.results_dir_format + loaded_state.Runstate_t.results_dir_format) + then + error "Incompatible formats: found\n %s\n\nbut expected this format:\n %s\n\n" + loaded_state.results_dir_format !state.Runstate_t.results_dir_format + else ( + state := loaded_state ; + Ok () ) + with _ -> Error "error reading the save state" + + +let reset () = state := state0 diff --git a/infer/src/base/RunState.mli b/infer/src/base/RunState.mli new file mode 100644 index 000000000..5c4232177 --- /dev/null +++ b/infer/src/base/RunState.mli @@ -0,0 +1,22 @@ +(* + * Copyright (c) 2018 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + *) + +open! IStd + +val add_run_to_sequence : unit -> unit +(** add an entry with the current run date *) + +val store : unit -> unit +(** save the current state to disk *) + +val load_and_validate : unit -> (unit, string) Result.t +(** attempt to load state from disk *) + +val reset : unit -> unit +(** reset the in-memory state to what it would be if this were a fresh run of infer *) diff --git a/infer/src/base/Version.mli b/infer/src/base/Version.mli index 9e210f210..5824316ea 100644 --- a/infer/src/base/Version.mli +++ b/infer/src/base/Version.mli @@ -9,6 +9,12 @@ open! IStd +val major : int + +val minor : int + +val patch : int + val commit : string val versionString : string diff --git a/infer/src/integration/Driver.ml b/infer/src/integration/Driver.ml index ff6d5725a..ba4c22b0c 100644 --- a/infer/src/integration/Driver.ml +++ b/infer/src/integration/Driver.ml @@ -127,13 +127,19 @@ let clean_results_dir () = let check_captured_empty mode = let clean_command_opt = clean_compilation_command mode in - if Config.capture && Utils.directory_is_empty Config.captured_dir then ( + if Config.capture && Utils.directory_is_empty Config.captured_dir then + let nothing_to_compile_msg = "Nothing to compile." in + let please_run_capture_msg = + match mode with Analyze -> " Have you run `infer capture`?" | _ -> "" + in ( match clean_command_opt with | Some clean_command -> - L.user_warning "@\nNothing to compile. Try running `%s` first.@." clean_command + L.user_warning "%s%s Try running `%s` first.@." nothing_to_compile_msg + please_run_capture_msg clean_command | None -> - L.user_warning "@\nNothing to compile. Try cleaning the build first.@." ) ; - true ) + L.user_warning "%s%s Try cleaning the build first.@." nothing_to_compile_msg + please_run_capture_msg ) ; + true else false diff --git a/infer/tests/build_systems/differential_interesting_paths_filter/Makefile b/infer/tests/build_systems/differential_interesting_paths_filter/Makefile index 549b28a7c..3b663650e 100644 --- a/infer/tests/build_systems/differential_interesting_paths_filter/Makefile +++ b/infer/tests/build_systems/differential_interesting_paths_filter/Makefile @@ -45,7 +45,6 @@ previous.exp.test: $(PREVIOUS_REPORT) test: compare_reports print: current.exp.test previous.exp.test replace: replace_reports -$(DIFFERENTIAL_REPORT): $(MODIFIED_FILES_FILE) $(CURRENT_REPORT): $(QUIET)$(COPY) src/com/example/DiffClass1.java.current src/com/example/DiffClass1.java diff --git a/infer/tests/build_systems/genrule/Makefile b/infer/tests/build_systems/genrule/Makefile index e3d1bc177..1892fbfe2 100644 --- a/infer/tests/build_systems/genrule/Makefile +++ b/infer/tests/build_systems/genrule/Makefile @@ -16,7 +16,10 @@ OBJECTS = $(ROOT_DIR)/buck-out/genruletest/gen/infer/tests/build_systems/genrule JSON_REPORT = $(ROOT_DIR)/buck-out/gen/infer/tests/build_systems/genrule/module2/module2_infer/infer_out/report.json INFER_OPTIONS = --project-root $(ROOT_DIR) INFERPRINT_OPTIONS = --project-root $(ROOT_DIR) --issues-tests -CLEAN_EXTRA = $(ROOT_DIR)/buck-out/genruletest +CLEAN_EXTRA = $(ROOT_DIR)/buck-out/genruletest report.json + +# fake infer-out because we only copy the results.json from buck-out. +INFER_OUT = . include $(TESTS_DIR)/java.make include $(TESTS_DIR)/infer.make @@ -37,8 +40,10 @@ $(JSON_REPORT): $(JAVA_DEPS) $(JAVA_SOURCE_FILES) $(MAKEFILE_LIST) INFER_BIN="$(INFER_BIN)" NO_BUCKD=1 $(BUCK) build --no-cache $(INFER_TARGET)) $(QUIET)touch $@ -infer-out/report.json: $(JSON_REPORT) $(MAKEFILE_LIST) - $(QUIET)$(REMOVE_DIR) infer-out - $(QUIET)$(MKDIR_P) infer-out +report.json: $(JSON_REPORT) $(MAKEFILE_LIST) # the report contains absolute paths $(QUIET)sed -e 's#$(abspath $(TESTS_DIR))/##g' $< > $@ + +issues.exp.test$(TEST_SUFFIX): report.json $(INFER_BIN) + $(QUIET)$(INFER_BIN) report -q -a $(ANALYZER) \ + $(INFERPRINT_OPTIONS) $@ --from-json-report $< diff --git a/infer/tests/differential.make b/infer/tests/differential.make index 522a4858b..ce7e8c4fc 100644 --- a/infer/tests/differential.make +++ b/infer/tests/differential.make @@ -11,7 +11,6 @@ include $(TESTS_DIR)/base.make INFER_OUT = infer-out -DIFFERENTIAL_REPORT = $(INFER_OUT)/differential/introduced.json EXPECTED_TEST_OUTPUT = introduced.exp.test INFERPRINT_ISSUES_FIELDS = \ "bug_type,file,procedure,line_offset,procedure_id,procedure_id_without_crc" @@ -30,14 +29,13 @@ $(PREVIOUS_REPORT): $(CURRENT_REPORT) .PHONY: analyze analyze: $(CURRENT_REPORT) $(PREVIOUS_REPORT) -$(DIFFERENTIAL_REPORT): $(CURRENT_REPORT) $(PREVIOUS_REPORT) $(MAKEFILE_LIST) +$(EXPECTED_TEST_OUTPUT): $(CURRENT_REPORT) $(PREVIOUS_REPORT) $(MODIFIED_FILES_FILE) \ + $(INFER_BIN) $(MAKEFILE_LIST) $(QUIET)$(REMOVE_DIR) $(INFER_OUT) $(QUIET)$(call silent_on_success,Computing results difference in $(TEST_REL_DIR),\ $(INFER_BIN) -o $(INFER_OUT) --project-root $(CURDIR) reportdiff \ --report-current $(CURRENT_REPORT) --report-previous $(PREVIOUS_REPORT) \ $(DIFFERENTIAL_ARGS)) - -$(EXPECTED_TEST_OUTPUT): $(DIFFERENTIAL_REPORT) $(INFER_BIN) $(MAKEFILE_LIST) $(QUIET)$(INFER_BIN) report -o $(INFER_OUT) \ --issues-fields $(INFERPRINT_ISSUES_FIELDS) \ --from-json-report $(INFER_OUT)/differential/introduced.json \ diff --git a/infer/tests/infer.make b/infer/tests/infer.make index 0222e8434..488feff1f 100644 --- a/infer/tests/infer.make +++ b/infer/tests/infer.make @@ -5,6 +5,8 @@ # LICENSE file in the root directory of this source tree. An additional grant # of patent rights can be found in the PATENTS file in the same directory. +INFER_OUT ?= infer-out$(TEST_SUFFIX) + include $(TESTS_DIR)/base.make # useful to print non-default analyzer @@ -12,7 +14,7 @@ ANALYZER_STRING=$(shell if [ -n $(ANALYZER) ]; then printf ' ($(ANALYZER))'; fi) default: compile -issues.exp.test$(TEST_SUFFIX): infer-out$(TEST_SUFFIX)/report.json $(INFER_BIN) +issues.exp.test$(TEST_SUFFIX): $(INFER_OUT)/report.json $(INFER_BIN) $(QUIET)$(INFER_BIN) report -q -a $(ANALYZER) --results-dir $(