From 40b2b94bb966f95566c2c36ef1c8fa8a816f6bf1 Mon Sep 17 00:00:00 2001 From: Chen Guo Date: Tue, 30 Jun 2015 11:14:03 +0000 Subject: [PATCH 1/7] Implement optional input files. Fixes Factual/Drake#179 --- src/drake/core.clj | 40 +++++++++++++++++++++------------ src/drake/fs.clj | 2 +- src/drake/parser.clj | 45 ++++++++++++++++++++++++++++++++++---- src/drake/parser_utils.clj | 2 +- 4 files changed, 69 insertions(+), 20 deletions(-) diff --git a/src/drake/core.clj b/src/drake/core.clj index 83a4066..ba0000f 100644 --- a/src/drake/core.clj +++ b/src/drake/core.clj @@ -106,8 +106,13 @@ it. This is safe to do since it's the default filesystem, but it gives us a bit easier compatibility with existing tools." [filename] - (let [n (dfs/normalized-path filename)] - (if (= "file" (dfs/path-fs n)) (dfs/path-filename n) n))) + (parser/modify-filename + filename + (fn [f] + (let [n (dfs/normalized-path f)] + (if (= "file" (dfs/path-fs n)) + (dfs/path-filename n) + n))))) (defn- despace-cmds "Given a sequence of commands, removes leading whitespace found in the first @@ -144,7 +149,7 @@ normalized-outputs (map normalize-filename-for-run outputs) normalized-inputs (map normalize-filename-for-run inputs) vars (merge vars - (parser/inouts-map normalized-inputs "INPUT") + (parser/existing-inputs-map normalized-inputs "INPUT") (parser/inouts-map normalized-outputs "OUTPUT")) method (methods (:method opts)) method-mode (:method-mode opts) @@ -176,6 +181,23 @@ :vars vars :opts (if-not method opts (merge (:opts method) opts))))) +(defn- make-input-stats + [file] + {:optional (parser/optional-file? file) + :exists (fs di/data-in? (parser/normalize-optional-file file)) + :file file}) + +(defn- existing-and-empty-inputs + "Remove '?' denoting optional file from front of path" + [inputs] + (let [inputs-info (map make-input-stats inputs)] + [;; Existing files + (->> (filter :exists inputs-info) + (map :file)) + ;; Non-existing, non-optional files + (->> (remove #(or (:optional %) (:exists %)) inputs-info) + (map :file))])) + (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 @@ -195,7 +217,7 @@ [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) + [inputs empty-inputs] (existing-and-empty-inputs inputs) no-outputs (empty? outputs)] (trace "should-build? forced:" forced) (trace "should-build? match-type:" match-type) @@ -320,16 +342,6 @@ true if the step was actually run; false if skipped." [parse-tree step-number {:keys [index build match-type opts]}] (let [{:keys [inputs] :as step} (get-in parse-tree [:steps index])] - ;; TODO(artem) - ;; Somewhere here, before running the step or checking timestamps, we need to - ;; check for optional files and replace them with empty strings if they're - ;; not found (according to the spec). We shouldn't just rewrite :inputs and - ;; should probably separate two versions, since the step name - ;; (used in debugging and log files names) should not vary. - ;; For now just check that none of the input files is optional. - (if (some #(= \? (first %)) inputs) - (throw+ {:msg (str "optional input files are not supported yet: " - inputs)})) (let [step-descr (step-string (branch-adjust-step step false)) step (-> step (update-in [:opts] merge opts) diff --git a/src/drake/fs.clj b/src/drake/fs.clj index 2377ba6..893eb48 100644 --- a/src/drake/fs.clj +++ b/src/drake/fs.clj @@ -464,4 +464,4 @@ (defn newest-in [path] - (pick-by-mod-time path -)) \ No newline at end of file + (pick-by-mod-time path -)) diff --git a/src/drake/parser.clj b/src/drake/parser.clj index 3ac4ab4..1be5e91 100644 --- a/src/drake/parser.clj +++ b/src/drake/parser.clj @@ -3,10 +3,12 @@ [clojure.string :as s] [clojure.core.memoize :as memo] [slingshot.slingshot :refer [throw+]] + [drake.fs :as dfs] [drake.shell :refer [shell]] [drake.steps :refer [add-dependencies calc-step-dirs]] [drake.utils :as utils :refer [clip ensure-final-newline]] [drake.parser_utils :refer :all] + [drake-interface.core :as di] [name.choi.joshua.fnparse :as p] [fs.core :as fs])) @@ -297,13 +299,34 @@ second))] ;; first is ",", second is (cons first-file rest-files))) +(defn optional-file? + "Check if the first character of a file specification is '?', + indicating it is optional" + [filename] + (= \? (first filename))) + +(defn normalize-optional-file + [filename] + (if (optional-file? filename) + (subs filename 1) + filename)) + +(defn modify-filename + [filename mod-fn] + (if (optional-file? filename) + (str "?" (mod-fn (normalize-optional-file filename))) + (mod-fn filename))) + (defn add-prefix "Appends prefix if necessary (unless prepended by '!')." [prefix file] - (cond (= \! (first file)) (clip file) - (= \/ (first file)) file - (re-matches #"[a-zA-z][a-zA-Z0-9+.-]*:.*" file) file - :else (str prefix file))) + (modify-filename + file + (fn [f] + (cond (= \! (first f)) (clip f) + (= \/ (first f)) f + (re-matches #"[a-zA-z][a-zA-Z0-9+.-]*:.*" f) f + :else (str prefix f))))) (defn add-path-sep-suffix [^String path] (if (or (empty? path) @@ -339,6 +362,20 @@ (for [[i v] (map-indexed vector items)] [(str prefix i) v]))) +(defn- fname->var + [filename] + (if (optional-file? filename) + (let [filename (normalize-optional-file filename)] + (or (dfs/fs di/data-in? filename) "")) + filename)) + +(defn existing-inputs-map + "Like inouts-map, except optional but nonexisting + input files are replaced by empty strings" + [items prefix] + (let [items (map fname->var items)] + (inouts-map items prefix))) + ;; TODO(artem) ;; Should we move this to a common library? (defn demix diff --git a/src/drake/parser_utils.clj b/src/drake/parser_utils.clj index e8bc27b..3ac85cc 100644 --- a/src/drake/parser_utils.clj +++ b/src/drake/parser_utils.clj @@ -240,4 +240,4 @@ (p/complex [_ single-quote chars (p/rep* (p/except p/anything single-quote)) _ single-quote] - (apply str chars))) \ No newline at end of file + (apply str chars))) From 5504e68e3ca460e42faf057d7973894391564032 Mon Sep 17 00:00:00 2001 From: Chen Guo Date: Tue, 30 Jun 2015 11:22:50 +0000 Subject: [PATCH 2/7] Define optional input prefix as constant. Factual/Drake#179 --- src/drake/parser.clj | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/drake/parser.clj b/src/drake/parser.clj index 1be5e91..9bf9dba 100644 --- a/src/drake/parser.clj +++ b/src/drake/parser.clj @@ -299,11 +299,13 @@ second))] ;; first is ",", second is (cons first-file rest-files))) +(def ^:private ^:const opt-flag \?) + (defn optional-file? "Check if the first character of a file specification is '?', indicating it is optional" [filename] - (= \? (first filename))) + (= opt-flag (first filename))) (defn normalize-optional-file [filename] @@ -314,7 +316,7 @@ (defn modify-filename [filename mod-fn] (if (optional-file? filename) - (str "?" (mod-fn (normalize-optional-file filename))) + (str opt-flag (mod-fn (normalize-optional-file filename))) (mod-fn filename))) (defn add-prefix From 90cd05aa86938616363822d39363239d3c4b7ae9 Mon Sep 17 00:00:00 2001 From: Chen Guo Date: Tue, 30 Jun 2015 11:27:49 +0000 Subject: [PATCH 3/7] Ensure filename is used when mapping inputs to vars. Factual/Drake#179 --- src/drake/parser.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drake/parser.clj b/src/drake/parser.clj index 9bf9dba..08a67cd 100644 --- a/src/drake/parser.clj +++ b/src/drake/parser.clj @@ -368,7 +368,7 @@ [filename] (if (optional-file? filename) (let [filename (normalize-optional-file filename)] - (or (dfs/fs di/data-in? filename) "")) + (if (dfs/fs di/data-in? filename) filename "")) filename)) (defn existing-inputs-map From 5dc4a4e973505f207613ccdc926616fb61fabd76 Mon Sep 17 00:00:00 2001 From: Chen Guo Date: Tue, 30 Jun 2015 11:59:06 +0000 Subject: [PATCH 4/7] Refactoring for optional inputs, and add some comments. Factual/drake#179 --- src/drake/core.clj | 8 +------- src/drake/parser.clj | 32 ++++++++++++++++++++++---------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/drake/core.clj b/src/drake/core.clj index ba0000f..690703d 100644 --- a/src/drake/core.clj +++ b/src/drake/core.clj @@ -181,16 +181,10 @@ :vars vars :opts (if-not method opts (merge (:opts method) opts))))) -(defn- make-input-stats - [file] - {:optional (parser/optional-file? file) - :exists (fs di/data-in? (parser/normalize-optional-file file)) - :file file}) - (defn- existing-and-empty-inputs "Remove '?' denoting optional file from front of path" [inputs] - (let [inputs-info (map make-input-stats inputs)] + (let [inputs-info (map parser/make-file-stats inputs)] [;; Existing files (->> (filter :exists inputs-info) (map :file)) diff --git a/src/drake/parser.clj b/src/drake/parser.clj index 08a67cd..8a5c685 100644 --- a/src/drake/parser.clj +++ b/src/drake/parser.clj @@ -301,21 +301,29 @@ (def ^:private ^:const opt-flag \?) -(defn optional-file? +(defn optional-input? "Check if the first character of a file specification is '?', - indicating it is optional" - [filename] - (= opt-flag (first filename))) + indicating it is an optional input" + [input] + (= opt-flag (first input))) (defn normalize-optional-file [filename] - (if (optional-file? filename) + (if (optional-input? filename) (subs filename 1) filename)) +(defn make-file-stats + [filename] + (let [filepath (normalize-optional-file filename)] + {:optional (optional-input? filename) + :exists (dfs/fs di/data-in? filepath) + :path filepath + :file filename})) + (defn modify-filename [filename mod-fn] - (if (optional-file? filename) + (if (optional-input? filename) (str opt-flag (mod-fn (normalize-optional-file filename))) (mod-fn filename))) @@ -365,11 +373,15 @@ [(str prefix i) v]))) (defn- fname->var + "Convert a file name to a var if it exists. If it + does not exist, and is not optional, use empty + string as var representation" [filename] - (if (optional-file? filename) - (let [filename (normalize-optional-file filename)] - (if (dfs/fs di/data-in? filename) filename "")) - filename)) + (let [file-stats (make-file-stats filename)] + (if (and (not (:exists file-stats)) + (:optional file-stats)) + "" + (:file file-stats)))) (defn existing-inputs-map "Like inouts-map, except optional but nonexisting From 3090f3377ca399b7e09f14745dfd11d1c65ca745 Mon Sep 17 00:00:00 2001 From: Chen Guo Date: Tue, 30 Jun 2015 22:09:17 +0000 Subject: [PATCH 5/7] Optional files: fix bug where $INPUT is set to ?file instead of file. #179 --- src/drake/parser.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drake/parser.clj b/src/drake/parser.clj index 8a5c685..fa438a9 100644 --- a/src/drake/parser.clj +++ b/src/drake/parser.clj @@ -381,7 +381,7 @@ (if (and (not (:exists file-stats)) (:optional file-stats)) "" - (:file file-stats)))) + (:path file-stats)))) (defn existing-inputs-map "Like inouts-map, except optional but nonexisting From 7cf41b812c307f04a7501210534e31b74890d699 Mon Sep 17 00:00:00 2001 From: Chen Guo Date: Wed, 1 Jul 2015 08:26:13 +0000 Subject: [PATCH 6/7] Optional files: make return for resolving empty files a map instead of a vector. Factual/Drake#179 --- src/drake/core.clj | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/drake/core.clj b/src/drake/core.clj index 690703d..cee52d1 100644 --- a/src/drake/core.clj +++ b/src/drake/core.clj @@ -185,12 +185,11 @@ "Remove '?' denoting optional file from front of path" [inputs] (let [inputs-info (map parser/make-file-stats inputs)] - [;; Existing files - (->> (filter :exists inputs-info) - (map :file)) - ;; Non-existing, non-optional files - (->> (remove #(or (:optional %) (:exists %)) inputs-info) - (map :file))])) + {:existing (->> (filter :exists inputs-info) + (map :file)) + ;; Non-existing, non-optional inputs + :missing (->> (remove (some-fn :optional :exists) inputs-info) + (map :file))})) (defn- should-build? "Given the parse tree and a step index, determines whether it should @@ -211,7 +210,7 @@ [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) - [inputs empty-inputs] (existing-and-empty-inputs inputs) + {inputs :inputs empty-inputs :missing} (existing-and-empty-inputs inputs) no-outputs (empty? outputs)] (trace "should-build? forced:" forced) (trace "should-build? match-type:" match-type) From b50e448b0d5a0a148f7d59bc9189dd7f0a576313 Mon Sep 17 00:00:00 2001 From: Chen Guo Date: Fri, 3 Jul 2015 00:02:40 +0000 Subject: [PATCH 7/7] Optional files: split out helper fns from file name modification functions. #179 --- src/drake/core.clj | 15 +++++++++------ src/drake/parser.clj | 16 ++++++++++------ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/drake/core.clj b/src/drake/core.clj index cee52d1..603e539 100644 --- a/src/drake/core.clj +++ b/src/drake/core.clj @@ -101,18 +101,21 @@ (assoc step :inputs branch-adjusted-inputs :outputs branch-adjusted-outputs))) -(defn- normalize-filename-for-run +(defn- normalize-filename-for-run* "Normalizes filename and also removes local filesystem prefix (file:) from it. This is safe to do since it's the default filesystem, but it gives us a bit easier compatibility with existing tools." [filename] + (let [n (dfs/normalized-path filename)] + (if (= "file" (dfs/path-fs n)) + (dfs/path-filename n) + n))) + +(defn- normalize-filename-for-run + [filename] (parser/modify-filename filename - (fn [f] - (let [n (dfs/normalized-path f)] - (if (= "file" (dfs/path-fs n)) - (dfs/path-filename n) - n))))) + normalize-filename-for-run*)) (defn- despace-cmds "Given a sequence of commands, removes leading whitespace found in the first diff --git a/src/drake/parser.clj b/src/drake/parser.clj index fa438a9..93706b6 100644 --- a/src/drake/parser.clj +++ b/src/drake/parser.clj @@ -327,16 +327,20 @@ (str opt-flag (mod-fn (normalize-optional-file filename))) (mod-fn filename))) -(defn add-prefix +(defn- add-prefix* "Appends prefix if necessary (unless prepended by '!')." [prefix file] + (cond (= \! (first file)) (clip file) + (= \/ (first file)) file + (re-matches #"[a-zA-z][a-zA-Z0-9+.-]*:.*" file) file + :else (str prefix file))) + +(defn add-prefix + "add-prefix*, with support for file naming conventions" + [prefix file] (modify-filename file - (fn [f] - (cond (= \! (first f)) (clip f) - (= \/ (first f)) f - (re-matches #"[a-zA-z][a-zA-Z0-9+.-]*:.*" f) f - :else (str prefix f))))) + (fn [f] (add-prefix* prefix f)))) (defn add-path-sep-suffix [^String path] (if (or (empty? path)