From df9b8c0b00cd01e6cec3b657fa56aebbf7966e65 Mon Sep 17 00:00:00 2001 From: Myron Ahn Date: Fri, 1 Nov 2013 19:36:49 +0700 Subject: [PATCH 1/4] Implementation of checkpoints/temp targets #88 Squashed all the commits into one - in another branch --- resources/regtest/regtest_temp.d | 29 ++++++++ resources/regtest/regtest_temp.sh | 94 ++++++++++++++++++++++++ resources/regtest/regtest_utils.sh | 26 +++++++ src/drake/core.clj | 111 +++++++++++++++++++++++++---- src/drake/parser.clj | 41 ++++++++++- src/drake/parser_utils.clj | 1 + src/drake/steps.clj | 17 ++++- test/drake/test/parser.clj | 31 ++++++++ test/drake/test/utils.clj | 3 +- 9 files changed, 335 insertions(+), 18 deletions(-) create mode 100644 resources/regtest/regtest_temp.d create mode 100755 resources/regtest/regtest_temp.sh diff --git a/resources/regtest/regtest_temp.d b/resources/regtest/regtest_temp.d new file mode 100644 index 0000000..4f008bd --- /dev/null +++ b/resources/regtest/regtest_temp.d @@ -0,0 +1,29 @@ +; Regression testing for Drake: checkpointing/temp targets +; Relevant URLs: +; https://github.com/Factual/drake/issues/88 + +~temp1 <- start + echo -n $INPUTS > $OUTPUT + echo -n "t1" >> stepsrun + +~temp2 <- temp1 + echo -n $INPUTS > $OUTPUT + echo -n "t2" >> stepsrun + +~temp3, perm1 <- temp2 + echo -n INPUTS > $OUTPUT0 + echo -n $INPUTS > $OUTPUT1 + echo -n "t3p1" >> stepsrun + +perm2 <- perm1 + echo -n $INPUTS > $OUTPUT + echo -n "p2" >> stepsrun + +perm3 <- temp2 + echo -n $INPUTS > $OUTPUT + echo -n "p3" >> stepsrun + +perm4 <- temp3 + echo -n $INPUTS > $OUTPUT + echo -n "p4" >> stepsrun + diff --git a/resources/regtest/regtest_temp.sh b/resources/regtest/regtest_temp.sh new file mode 100755 index 0000000..f3aebbf --- /dev/null +++ b/resources/regtest/regtest_temp.sh @@ -0,0 +1,94 @@ +#!/bin/bash +# Regression testing for Drake: checkpointing/temp targets +# Relevant URLs: +# https://github.com/Factual/drake/issues/88 + +source $(dirname $0)/regtest_utils.sh + +export FILES=start\ stepsrun\ temp1\ temp2\ temp3\ perm1\ perm2\ perm3\ perm4 + +cleanup() { + for FILE in $FILES + do + rm -f $(dirname $0)/$FILE>/dev/null 2>&1 + done +} + +checkfiles() { +check_does_not_exist temp1 temp2 temp3 +check_exists perm1 perm2 perm3 perm4 +} + +echo "------------------" +echo "TESTS: checkpoints" +echo "------------------" + +# First cleanup any existing files +cleanup + +# Run script from scratch, make sure temp files are deleted +touch start +run_d regtest_temp.d -a +check_grep stepsrun "t1t2t3p1p2p3p4" +checkfiles + +# Run script again, make sure nothing happens even though temp files are deleted +rm stepsrun +run_d regtest_temp.d -a +check_does_not_exist stepsrun +checkfiles + +# Touch start, run script again, make sure all files are built +sleep 2 # get around the 1 second accuracy of lastModified() in Java +#rm stepsrun +touch start +run_d regtest_temp.d -a +check_grep stepsrun "t1t2t3p1p2p3p4" +checkfiles + +# Remove perm2, make sure only perm2 step is run +sleep 2 # get around the 1 second accuracy of lastModified() in Java +rm stepsrun +rm perm2 +run_d regtest_temp.d -a +check_grep stepsrun "p2" +checkfiles + +# Touch temp2, make sure temp3, perm1, perm2, perm3, perm4 are built +sleep 2 # get around the 1 second accuracy of lastModified() in Java +rm stepsrun +touch temp2 +run_d regtest_temp.d -a +check_grep stepsrun "t3p1p2p3p4" +checkfiles + +# Touch temp3, make sure perm4 is run +sleep 2 # get around the 1 second accuracy of lastModified() in Java +rm stepsrun +touch temp3 +run_d regtest_temp.d -a +check_grep stepsrun "p4" +checkfiles + +# Touch start, build only perm3, make sure temp1, temp2, perm3 are run +sleep 2 # get around the 1 second accuracy of lastModified() in Java +rm stepsrun +touch start +run_d regtest_temp.d -a perm3 +check_grep stepsrun "t1t2p3" +checkfiles + +# Cleanup, build only temp3, make sure temp1, temp2, temp3, perm1 are run +# Also make sure temp3 still exists +cleanup +touch start +run_d regtest_temp.d -a temp3 +check_grep stepsrun "t1t2t3p1" +check_does_not_exist temp1 temp2 +check_exists perm1 temp3 + +# Final cleanup +cleanup + +echo "ALL PASSED" + diff --git a/resources/regtest/regtest_utils.sh b/resources/regtest/regtest_utils.sh index 6d7b8eb..dc5cc76 100755 --- a/resources/regtest/regtest_utils.sh +++ b/resources/regtest/regtest_utils.sh @@ -1,6 +1,32 @@ STDOUT=$(dirname $0)/stdout.log STDERR=$(dirname $0)/stderr.log +check_exists() { + for file in "$@" + do + echo "Making sure $file exists" + if [ ! -f $file ]; then + echo "FAIL" + echo "file $file does not exist" + exit -1 + fi + done + echo "PASS" +} + +check_does_not_exist() { + for file in "$@" + do + echo "Making sure $file does not exist" + if [ -f $file ]; then + echo "FAIL" + echo "file $file exists" + exit -1 + fi + done + echo "PASS" +} + check() { if [ "$1" != "$2" ]; then echo "FAIL" diff --git a/src/drake/core.clj b/src/drake/core.clj index 9d3c059..5209b94 100644 --- a/src/drake/core.clj +++ b/src/drake/core.clj @@ -158,6 +158,47 @@ :vars vars :opts (if-not method opts (merge (method :opts) opts))))) +(defn- data-in? + "Shortcut for (fs di/data-in?)" + [file] + (fs di/data-in? file)) + +(defn- no-data-in? + "Shortcut for (not (fs di/data-in?))" + [file] + (not (data-in? file))) + +(defn- expand-outputs + "Given a set of outputs, expand any temp outputs that have no data by + recursively replacing the temp output with the outputs of any steps + that depend on that temp output - and then doing the same for the + temp outputs of the dependent steps that have no data. Temp outputs + that have data will not be expanded. This is the key algorithm that + allows Drake to deal with deleted temp targets without triggering + unnecessary rebuilds." + [parse-tree step-list step] + (let [step-set (into #{} step-list) + steps (parse-tree :steps) + real-outputs (step :real-outputs) + temp-outputs (step :temp-outputs) + temp-input-map-lookup (parse-tree :temp-input-map-lookup) + [existing-temp-outputs empty-temp-outputs] (split-with data-in? temp-outputs) + empty-temp-output-step-deps (map (comp temp-input-map-lookup normalized-path) empty-temp-outputs) + empty-temp-output-step-deps (flatten empty-temp-output-step-deps) + empty-temp-output-step-deps (filter step-set empty-temp-output-step-deps) + expanded-temp-outputs (map (comp (partial expand-outputs parse-tree step-list) steps) + empty-temp-output-step-deps) + expanded-temp-outputs (flatten expanded-temp-outputs) + ] + (trace "expand-outputs step:" step) + (trace "expand-outputs step-list:" step-list) + (trace "expand-outputs existing-temp-outputs:" existing-temp-outputs) + (trace "expand-outputs empty-temp-outputs:" empty-temp-outputs) + (trace "expand-outputs empty-temp-output-step-deps:" empty-temp-output-step-deps) + (trace "expand-outputs expanded-temp-outputs" expanded-temp-outputs) + + (concat real-outputs existing-temp-outputs expanded-temp-outputs))) + (defn- should-build? "Given the parse tree and a step index, determines whether it should be built and returns the reason (e.g. 'timestamped') or @@ -174,15 +215,21 @@ above). If this step is specified manually (root of a dependency subtree), will always fail on an empty input, since it will not be generated by any other step." - [step forced triggered match-type fail-on-empty] + [parse-tree step-list step forced triggered match-type fail-on-empty] (trace "should-build? fail-on-empty: " fail-on-empty) - (let [{:keys [inputs outputs opts]} (branch-adjust-step step false) - empty-inputs (filter #(not (fs di/data-in? %)) inputs) + (let [{:keys [real-inputs real-outputs temp-inputs temp-outputs opts]} (branch-adjust-step step false) + inputs (into real-inputs (filter data-in? temp-inputs)) + empty-inputs (filter no-data-in? real-inputs) + outputs (expand-outputs parse-tree step-list step) no-outputs (empty? outputs)] (trace "should-build? forced:" forced) (trace "should-build? match-type:" match-type) (trace "should-build? triggered:" triggered) + (trace "should-build? temp-inputs: " temp-inputs) + (trace "should-build? real-inputs: " real-inputs) (trace "should-build? inputs: " inputs) + (trace "should-build? temp-outputs: " temp-outputs) + (trace "should-build? real-outputs: " real-outputs) (trace "should-build? outputs: " outputs) (trace "should-build? empty inputs: " empty-inputs) (trace "should-build? no-outputs: " no-outputs) @@ -200,11 +247,13 @@ ;; one or more outputs are missing? (can only look for those ;; for targets which were specified explicitly, not triggered) (and (not triggered) - (some #(not (fs di/data-in? %)) outputs)) "missing output" + (some no-data-in? outputs)) "missing output" ;; timecheck disabled (= false (get-in step [:opts :timecheck])) nil ;; built as a dependency? triggered "projected timestamped" + ;; if there are nothing but temp inputs, don't build + (and (empty? inputs) (not (empty? temp-inputs))) nil ;; no-input steps are always rebuilt (empty? inputs) "no-input step" :else @@ -233,7 +282,8 @@ (reduce (fn [[new-target-steps triggered-deps] {:keys [index build match-type] :as step}] (let [step-map ((parse-tree :steps) index) - cause (should-build? step-map (= build :forced) + step-list (map :index target-steps) + cause (should-build? parse-tree step-list step-map (= build :forced) (triggered-deps index) match-type false)] (trace (format "predict-steps, index=%d, cause=%s" index cause)) (if (nil? cause) @@ -302,7 +352,7 @@ (defn- run-step "Runs one step performing all necessary checks, returns true if the step was actually run; false if skipped." - [parse-tree step-number {:keys [index build match-type opts]}] + [parse-tree steps step-number {:keys [index build match-type opts]}] (let [step ((parse-tree :steps) index) inputs (step :inputs)] ;; TODO(artem) @@ -318,7 +368,8 @@ (let [step-descr (step-string (branch-adjust-step step false)) step (update-in step [:opts] #(merge % opts)) step (prepare-step-for-run step parse-tree) - should-build (should-build? step (= build :forced) + step-list (map :index steps) + should-build (should-build? parse-tree step-list step (= build :forced) false match-type true)] (info "") (info (format "--- %d. %s: %s" @@ -463,14 +514,14 @@ (add-empty-promises-to-steps steps :exception-promise)) (defn- attempt-run-step - [parse-tree step] + [parse-tree steps step] (let [prom (:promise step)] (try ; run the step (the actual job) - (run-step parse-tree (:index step) step) + (run-step parse-tree steps (:index step) step) (deliver prom 1) ; delivers a promise of 1/success (catch Exception e - (error (str "caught exception step " (:index step) ": ") (.getMessage e) (.printStackTrace e)) + (error e (str "caught exception step " (:index step) ": " e)) (deliver (:exception-promise step) e)) (finally ; if promise not delivered, deliver a promise of 0/failure @@ -492,7 +543,7 @@ @(promises-indexed i)) deps))] (if (= successful-parent-steps (count deps)) - (attempt-run-step parse-tree step) + (attempt-run-step parse-tree steps step) (deliver prom 0))) (catch Exception e (deliver (:exception-promise step) e)) @@ -553,6 +604,36 @@ [steps] (reduce + (map (fn [step] @(:promise step)) steps))) +(defn- setup-temp-deleting-futures + "Set up a future for each temp file which waits for all + the steps which depend on this temp file to finish and + then deletes the temp file." + [parse-tree steps] + (let [steps-list (map :index steps) + steps-map (zipmap steps-list steps) + steps-set (into #{} steps-list)] + (doseq [[file deps] (parse-tree :temp-input-map-lookup)] + (let [trimmed-deps (filter steps-set deps)] + (when (not (empty? trimmed-deps)) + (future + (try + (trace "Running future for file" file "deps" trimmed-deps) + (let [successful-deps-count (reduce + + (map (fn [i] + @((steps-map i) :promise)) + trimmed-deps))] + (trace "Finished waiting for dependents of file" + file + "deps" + trimmed-deps + "successful-count" + successful-deps-count) + (when (= successful-deps-count (count trimmed-deps)) + (info "Deleting temp file:" file) + (fs di/rm file))) + (catch Exception e + (error e "Future for file" file "Caught exception"))))))))) + (defn- run-steps-async "Runs steps asynchronously. If concurrence = 1, this will run the steps in the same order as the @@ -581,6 +662,9 @@ assoc-promise (assoc-function parse-tree))] + (when (not (:keep-temp-files *options*)) + (setup-temp-deleting-futures parse-tree steps-future)) + (post event-bus (EventWorkflowBegin steps-data)) (trigger-futures jobs steps-future event-bus) @@ -911,6 +995,8 @@ "Turn on even more verbose debugging output.") (no-arg version "Show version information.") + (no-arg keep-temp-files + "Do not auto-delete temp files") (with-arg tmpdir "Specifies the temporary directory for Drake files (by default, .drake/ in the same directory the main workflow file is located)." :type :str @@ -958,8 +1044,7 @@ (error (str "drake: " (m :msg))) (shutdown (or (get m :exit) 1))) (catch Exception e - (.printStackTrace e) - (error (stack-trace-str e)) + (error e (str "Exception caught: " e)) (shutdown 1)))))) (defn run-opts [opts] diff --git a/src/drake/parser.clj b/src/drake/parser.clj index 6d5b655..df0b2da 100644 --- a/src/drake/parser.clj +++ b/src/drake/parser.clj @@ -270,6 +270,10 @@ end-marker (p/opt dollar-sign)] (str sign name end-marker))) +(def file-name-optional-temp + (p/complex [temp (p/opt tilde) + filename file-name] + (str temp filename))) (def name-list "input: comma separated names. ie., \"a.csv, b.out\" @@ -282,13 +286,32 @@ second))] ;; first is ",", second is (cons first-file rest-files))) -(defn add-prefix +(def name-list-outputs + "input: comma separated names. ie., \"a.csv, b.out\" + output: vector of the names. ie., [\"a.csv\" \"b.out\"]" + (p/complex + [first-file file-name-optional-temp + rest-files (p/rep* (p/semantics + (p/conc (p/conc (p/opt ws) comma (p/opt ws)) + file-name-optional-temp) + second))] ;; first is ",", second is + (cons first-file rest-files))) + +(defn add-prefix-helper "Appends prefix if necessary (unless prepended by '!')." [prefix file] (if (= \! (first file)) (clip file) (str prefix file))) +(defn add-prefix + "Appends prefix if necessary (unless prepended by '!'). + Also, if there is a ~ at the beginning, move it in front of the prefix." + [prefix file] + (if (= \~ (first file)) + (str "~" (add-prefix-helper prefix (clip file))) + (add-prefix-helper prefix file))) + (defn add-path-sep-suffix [path] (if (or (empty? path) (.endsWith path "/") @@ -347,7 +370,7 @@ :raw-outputs (outputs without BASE prefix), :inputs, :vars, and possibly :opts" (p/complex - [outputs (p/opt (p/invisi-conc name-list (p/opt ws))) + [outputs (p/opt (p/invisi-conc name-list-outputs (p/opt ws))) _ (p/conc arrow (p/opt inline-ws)) inputs (p/opt name-list) opts (p/opt options) @@ -364,15 +387,25 @@ [intags infiles] (demix inputs #(= \% (first %))) intags (remove-tag-symbol intags) infiles-with-base (map-base-prefix infiles) + [infiles-with-base-temp infiles-with-base] (demix infiles-with-base #(= \~ (first %))) + infiles-with-base-temp (map clip infiles-with-base-temp) + infiles-with-base (into infiles-with-base infiles-with-base-temp) [outtags outfiles] (demix outputs #(= \% (first %))) outtags (remove-tag-symbol outtags) outfiles-with-base (map-base-prefix outfiles) + [outfiles-with-base-temp outfiles-with-base] (demix outfiles-with-base #(= \~ (first %))) + outfiles-with-base-temp (map clip outfiles-with-base-temp) + outfiles-with-base (into outfiles-with-base outfiles-with-base-temp) + ;; this is used for target matching, just remove all ;; prefixes - outfiles-raw (mapv #(if (#{\! \?} (first %)) + outfiles-raw (mapv #(if (#{\~} (first %)) (clip %) %) outfiles) + outfiles-raw (mapv #(if (#{\! \?} (first %)) + (clip %) + %) outfiles-raw) ;; even though we will expand INPUT and OUTPUT variables later, ;; for now just put placeholders there for variable name checking vars (merge vars (into {} (map #(vector (first %1) "*placeholder*") @@ -385,6 +418,8 @@ :raw-outputs outfiles-raw :outputs outfiles-with-base :output-tags outtags + :temp-inputs infiles-with-base-temp + :temp-outputs outfiles-with-base-temp :vars vars :opts (if (nil? opts) {} opts)}))) diff --git a/src/drake/parser_utils.clj b/src/drake/parser_utils.clj index ed20fc6..860e080 100644 --- a/src/drake/parser_utils.clj +++ b/src/drake/parser_utils.clj @@ -139,6 +139,7 @@ (def dollar-sign (nb-char-lit \$)) (def hashtag-sign (nb-char-lit \#)) (def percent-sign (nb-char-lit \%)) +(def tilde (nb-char-lit \~)) (def fractional-part (p/conc decimal-point (p/rep* decimal-digit))) diff --git a/src/drake/steps.clj b/src/drake/steps.clj index ab88883..0873775 100644 --- a/src/drake/steps.clj +++ b/src/drake/steps.clj @@ -40,6 +40,9 @@ [raw-parse-tree] (trace "Calculating dependency graph...") (let [steps (:steps raw-parse-tree) + tempfiles (reduce into (map #(into (:temp-inputs %) (:temp-outputs %)) steps)) + tempfiles (set (map normalized-path tempfiles)) + ;; goes over steps and maps values returned by function f (a sequence) ;; into a lists of step step indexes that have them, for example, if ;; f returns list of inputs, the output map could be: @@ -58,6 +61,11 @@ normalized-input-map (step-index-map (map-key normalized-path :inputs)) normalized-output-map (step-index-map (map-key normalized-path :outputs)) + normalized-temp-input-map (step-index-map (comp (partial filter tempfiles) + (map-key normalized-path :inputs))) + normalized-temp-output-map (step-index-map (comp (partial filter tempfiles) + (map-key normalized-path :outputs))) + build-variants (fn [variants] (map step-index-map variants)) output-map-lookup-regexp (apply merge-multimaps-distinct @@ -69,11 +77,14 @@ normalized-output-map) ;;_ (prn output-map-lookup) ] + (trace "tempfiles=" tempfiles) (assoc raw-parse-tree :output-map-lookup output-map-lookup :output-map-lookup-regexp output-map-lookup-regexp :output-tags-map output-tags-map :method-map method-map + :temp-input-map-lookup normalized-temp-input-map + :temp-output-map-lookup normalized-temp-output-map ;; this basically calculates dependencies ;; (mapv is used to convert it to vector for indexed lookup in the future) :steps (mapv @@ -89,7 +100,11 @@ (concat (map normalized-input-map (map normalized-path (% :outputs))) (map input-tags-map - (% :output-tags))))) + (% :output-tags)))) + :temp-inputs (filter (comp tempfiles normalized-path) (% :inputs)) + :real-inputs (filter (comp not tempfiles normalized-path) (% :inputs)) + :temp-outputs (filter (comp tempfiles normalized-path) (% :outputs)) + :real-outputs (filter (comp not tempfiles normalized-path) (% :outputs))) steps)))) ;; No way to get to MAX_PATH from Java diff --git a/test/drake/test/parser.clj b/test/drake/test/parser.clj index 4bdfaf5..9265756 100644 --- a/test/drake/test/parser.clj +++ b/test/drake/test/parser.clj @@ -85,6 +85,8 @@ :output-tags ["outtag1" "outtag2"] :inputs '("/base/b" "/base/c") :input-tags ["intag"] + :temp-inputs [] + :temp-outputs [] :opts {}})) (let [actual-prod (first (d/step-def-line (make-state "a <- b [shell] ;comment \n")))] @@ -115,11 +117,38 @@ :inputs '("/base/b") :input-tags [] :output-tags [] + :temp-inputs [] + :temp-outputs [] :opts {}})) (is (var-eq? actual-tuple :inputs nil)) ; verify :vars state not changed ; after step has concluded )) +(deftest step-test-temp-files + (let [actual-tuple (d/step-lines (make-state + (str "~a <- b\n" + " c $[OUTPUT0]\n")))] + (is (= (dissoc (first (:steps (first actual-tuple))) :vars) + {:cmds [[\space \space \c \space #{"OUTPUT0"}]] + :raw-outputs ["a"] + :outputs ["/base/a"] + :inputs '("/base/b") + :input-tags [] + :output-tags [] + :temp-inputs [] + :temp-outputs ["/base/a"] + :opts {}})) + (is (var-eq? actual-tuple :inputs nil)) ; verify :vars state not changed + ; after step has concluded + )) + +(deftest step-test-no-temp-files-in-inputs + (is (thrown-with-msg? Exception + #"illegal syntax starting with .* for step definition" + (let [actual-tuple (d/step-lines (make-state + (str "a <- ~b\n" + " c $[OUTPUT0]\n")))])))) + (deftest template-test (is (:templates (first (d/step-lines (make-state (str ".sorted$ <- . [+template]\n" @@ -197,6 +226,8 @@ (is (= (get-in actual-prod [:steps 1 :raw-outputs 0]) "babe.txt")) (is (= (get-in actual-prod [:steps 2 :raw-outputs 0]) "belle.txt")))) + + (deftest errors-test (is (thrown-with-msg? Exception #"variable .* undefined at this point." diff --git a/test/drake/test/utils.clj b/test/drake/test/utils.clj index fe8713c..02f4802 100644 --- a/test/drake/test/utils.clj +++ b/test/drake/test/utils.clj @@ -18,7 +18,8 @@ (comment (log4j/set-loggers! :root {:level :debug :name "console" :pattern "%m%n"} "drake" {:level :debug :name "console" :pattern "%m%n"})) - (log4j/set-loggers! :root {:level :off}) + (log4j/set-loggers! :root {:level :off} + "drake" {:level :off}) (set-options {:auto true :tmpdir ".drake" :jobs 1}) From 5359d4e20d8687b3ca9220e9d1ad6f5571fc851c Mon Sep 17 00:00:00 2001 From: Myron Ahn Date: Fri, 8 Nov 2013 15:54:50 +0700 Subject: [PATCH 2/4] Updated changelog for #88 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84a2d3e..ec13ace 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ * bugfix: [#98](https://github.com/Factual/drake/issues/98) --help now doesn't run workflow (thanks marshallshen) * Upgrade to c4 0.2.0, which no longer bundles the Facebook API +* New feature: temp targets (https://github.com/Factual/drake/issues/88) ## 0.1.4 From 9f3e15ac2fdf9c8d3ec4bf79841876a546aaaf4b Mon Sep 17 00:00:00 2001 From: Myron Ahn Date: Thu, 14 Nov 2013 12:53:11 +0700 Subject: [PATCH 3/4] Temp targets #88 address all of Aaron's code review items https://github.com/Factual/drake/pull/106 Standardize on name "temp targets" Use -> for cleaner code Add comments for ramifications of error when deleting temp target Also: made sure temp target testing is in the regtest suite --- resources/regtest/run-all.sh | 3 ++- src/drake/core.clj | 36 ++++++++++++++++++++---------------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/resources/regtest/run-all.sh b/resources/regtest/run-all.sh index acf84c3..c662ce8 100755 --- a/resources/regtest/run-all.sh +++ b/resources/regtest/run-all.sh @@ -8,7 +8,8 @@ if ($(dirname $0)/regtest_fs.sh && $(dirname $0)/regtest_inputs_outputs.sh && $(dirname $0)/regtest_methods.sh && $(dirname $0)/regtest_protocol_eval.sh && - $(dirname $0)/regtest_s3.sh); then + $(dirname $0)/regtest_s3.sh && + $(dirname $0)/regtest_temp.sh); then echo "run-all: ALL TESTS PASSED" else echo "run-all: SOME TESTS FAILED" diff --git a/src/drake/core.clj b/src/drake/core.clj index 5209b94..1e55a09 100644 --- a/src/drake/core.clj +++ b/src/drake/core.clj @@ -183,13 +183,13 @@ temp-outputs (step :temp-outputs) temp-input-map-lookup (parse-tree :temp-input-map-lookup) [existing-temp-outputs empty-temp-outputs] (split-with data-in? temp-outputs) - empty-temp-output-step-deps (map (comp temp-input-map-lookup normalized-path) empty-temp-outputs) - empty-temp-output-step-deps (flatten empty-temp-output-step-deps) - empty-temp-output-step-deps (filter step-set empty-temp-output-step-deps) - expanded-temp-outputs (map (comp (partial expand-outputs parse-tree step-list) steps) - empty-temp-output-step-deps) - expanded-temp-outputs (flatten expanded-temp-outputs) - ] + empty-temp-output-step-deps (->> empty-temp-outputs + (map (comp temp-input-map-lookup normalized-path)) + flatten + (filter step-set)) + expanded-temp-outputs (->> empty-temp-output-step-deps + (map (comp (partial expand-outputs parse-tree step-list) steps)) + flatten)] (trace "expand-outputs step:" step) (trace "expand-outputs step-list:" step-list) (trace "expand-outputs existing-temp-outputs:" existing-temp-outputs) @@ -617,22 +617,26 @@ (when (not (empty? trimmed-deps)) (future (try - (trace "Running future for file" file "deps" trimmed-deps) + (trace "Running future to delete target:" file "dependencies:" trimmed-deps) (let [successful-deps-count (reduce + (map (fn [i] @((steps-map i) :promise)) trimmed-deps))] - (trace "Finished waiting for dependents of file" + (trace "Finished waiting for dependents of target:" file - "deps" + "dependencies:" trimmed-deps - "successful-count" + "successful-count:" successful-deps-count) (when (= successful-deps-count (count trimmed-deps)) - (info "Deleting temp file:" file) + (info "Deleting temp target:" file) (fs di/rm file))) (catch Exception e - (error e "Future for file" file "Caught exception"))))))))) + ; Likely to happen if there is some problem with deleting the file. + ; Catch the exception and inform the user of the problem, + ; but do not hald execution as deletion of the file is probably not + ; critical to the workflow. + (error e "Exception deleting temp target:" file))))))))) (defn- run-steps-async "Runs steps asynchronously. @@ -662,7 +666,7 @@ assoc-promise (assoc-function parse-tree))] - (when (not (:keep-temp-files *options*)) + (when (not (:keep-temp-targets *options*)) (setup-temp-deleting-futures parse-tree steps-future)) (post event-bus (EventWorkflowBegin steps-data)) @@ -995,8 +999,8 @@ "Turn on even more verbose debugging output.") (no-arg version "Show version information.") - (no-arg keep-temp-files - "Do not auto-delete temp files") + (no-arg keep-temp-targets + "Do not auto-delete temp targets") (with-arg tmpdir "Specifies the temporary directory for Drake files (by default, .drake/ in the same directory the main workflow file is located)." :type :str From a48cab0577ba7c9dfefa36f8a5877b4995d3cc37 Mon Sep 17 00:00:00 2001 From: Myron Ahn Date: Wed, 13 Nov 2013 21:59:15 -0800 Subject: [PATCH 4/4] Fix intermittent problem with calculating temp targets #88 --- src/drake/steps.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drake/steps.clj b/src/drake/steps.clj index 0873775..8096dd6 100644 --- a/src/drake/steps.clj +++ b/src/drake/steps.clj @@ -40,7 +40,7 @@ [raw-parse-tree] (trace "Calculating dependency graph...") (let [steps (:steps raw-parse-tree) - tempfiles (reduce into (map #(into (:temp-inputs %) (:temp-outputs %)) steps)) + tempfiles (reduce into [] (map #(into (:temp-inputs %) (:temp-outputs %)) steps)) tempfiles (set (map normalized-path tempfiles)) ;; goes over steps and maps values returned by function f (a sequence)