Skip to content

Commit

Permalink
Detect and report errors reading mergeable sources
Browse files Browse the repository at this point in the history
  • Loading branch information
hlship committed Nov 1, 2024
1 parent cebe352 commit 463f100
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 24 deletions.
13 changes: 7 additions & 6 deletions deps.edn
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
org.clojure/tools.cli #:mvn{:version "0.4.2"}
org.clojure/tools.namespace #:mvn{:version "0.3.1"}
com.cognitect.aws/ecr #:mvn{:version "798.2.678.0"}
progrock #:mvn{:version "0.1.2"}
clj-commons/spinner #:mvn{:version "0.6.0"}
progrock/progrock #:mvn{:version "0.1.2"}
com.google.cloud.tools/jib-core #:mvn{:version "0.13.0"}
org.clojure/core.async #:mvn{:version "1.0.567"}}
:aliases
Expand All @@ -17,8 +16,10 @@
:extra-deps
{cognitect/test-runner
{:git/url "https://github.com/cognitect-labs/test-runner.git"
:sha "cb96e80f6f3d3b307c59cbeb49bb0dcb3a2a780b"}
org.clojure/test.check #:mvn{:version "0.10.0-RC1"}
nubank/mockfn #:mvn{:version "0.6.1"}
:sha "cb96e80f6f3d3b307c59cbeb49bb0dcb3a2a780b"}
org.clojure/test.check #:mvn{:version "0.10.0-RC1"}
nubank/mockfn #:mvn{:version "0.6.1"}
org.apache.commons/commons-vfs2 #:mvn{:version "2.4.1"}
nubank/matcher-combinators #:mvn{:version "1.2.5"}}}}}
babashka/fs {:mvn/version "0.5.22"}
io.github.tonsky/clj-reload {:mvn/version "0.7.1"}
nubank/matcher-combinators #:mvn{:version "3.9.1"}}}}}
18 changes: 7 additions & 11 deletions src/vessel/builder.clj
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
[clojure.set :as set]
[clojure.string :as string]
[clojure.tools.namespace.find :as namespace.find]
[spinner.core :as spinner]
[vessel.misc :as misc]
[vessel.resource-merge :as merge]
[vessel.sh :as sh])
Expand Down Expand Up @@ -56,9 +55,7 @@
(sh/javac classpath target-dir java-sources))))

(defn- do-compile
"Compiles the main-ns by writing compiled .class files to target-dir.
Displays a spin animation during the process."
"Compiles the main-ns by writing compiled .class files to target-dir."
[^Symbol main-ns ^Sequential classpath source-paths ^File target-dir compiler-options]
(let [forms `(try
(binding [*compile-path* ~(str target-dir)
Expand All @@ -68,16 +65,12 @@
(println)
(.printStackTrace err#)
(System/exit 1)))
_ (misc/log :info "Compiling %s..." main-ns)
spin (spinner/create-and-start!)]
_ (misc/log :info "Compiling %s..." main-ns)]
(try
(compile-java-sources classpath source-paths target-dir)
(sh/clojure classpath
"--eval"
(pr-str forms))
(finally
(spinner/stop! spin)
(println)))))
(pr-str forms)))))

(defn- find-namespaces
"Returns all namespaces declared within the file (either a directory
Expand Down Expand Up @@ -135,9 +128,10 @@
that are copied."
[classpath-root merge-set files]
(let [f (fn [result file]
(let [{:keys [target-file input-stream last-modified]} file
(let [{:keys [target-file input-stream last-modified input-source]} file
input-stream' ^InputStream (input-stream)
merged? (merge/execute-merge-rules classpath-root
input-source
input-stream'
target-file
last-modified
Expand All @@ -162,6 +156,7 @@
(remove #(.isDirectory ^JarEntry %))
(map (fn [^JarEntry entry]
{:target-file (io/file target-dir (.getName entry))
:input-source (str jar-root "#" entry)
:input-stream #(.getInputStream jar-file entry)
:last-modified (.getTime entry)}))
(copy-or-merge-files jar-root merge-set))))
Expand All @@ -174,6 +169,7 @@
misc/filter-files
(map (fn [^File file]
{:target-file (io/file target-dir (misc/relativize file file-system-root))
:input-source (str file)
:input-stream #(io/input-stream file)
:last-modified (.lastModified file)}))
(copy-or-merge-files file-system-root merge-set)))
Expand Down
28 changes: 22 additions & 6 deletions src/vessel/resource_merge.clj
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,20 @@
::*merged-paths (atom {})}))

(defn- apply-rule
[classpath-root ^InputStream input-stream target-file last-modified rule merge-set]
[classpath-root input-source ^InputStream input-stream target-file last-modified rule merge-set]
(let [{::keys [*merged-paths]} merge-set
{:keys [read-fn merge-fn]} rule
new-value (read-fn input-stream)]
(.close input-stream)
new-value (try
(read-fn input-stream)
(catch Throwable t
(throw (ex-info (str "Unable to read " input-source ": "
(or (ex-message t)
(-> t class .getName)))
{:classpath-root classpath-root
:input-source input-source
:target-file target-file})))
(finally
(.close input-stream)))]
(swap! *merged-paths
(fn [merged-paths]
(if (contains? merged-paths target-file)
Expand All @@ -97,13 +106,20 @@

(defn execute-merge-rules
"Evaluates the inputs against the rules in the provided merge set. Returns true
if the file is subject to a merge rule (in which case, it should not be simply copied)."
[classpath-root input-stream target-file last-modified merge-set]
if the file is subject to a merge rule (in which case, it should not be simply copied).
classpath-root - root of classpath; a File that is either a director or a Jar file
input-source - string describing where the input-stream comes from
input-stream - InputStream containing content of the file
target-file - File to write from the input stream (or merged)
last-modified - timestamp of last modified time to be applied to the final output
merge-set - mutable object containing details about merging"
[classpath-root input-source input-stream target-file last-modified merge-set]
(let [{::keys [rules]} merge-set
matched-rule (find-matching-rule rules target-file)]
(if matched-rule
(do
(apply-rule classpath-root input-stream target-file last-modified matched-rule merge-set)
(apply-rule classpath-root input-source input-stream target-file last-modified matched-rule merge-set)
true)
false)))

Expand Down
2 changes: 2 additions & 0 deletions test/resources/badlib/bad-input.edn
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{:foo :bar
:baz #_ :biff}
32 changes: 31 additions & 1 deletion test/unit/vessel/builder_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
[vessel.builder :as builder]
[vessel.misc :as misc]
[vessel.sh :as sh]
[babashka.fs :as fs]
[vessel.test-helpers :refer [classpath ensure-clean-test-dir]])
(:import java.io.File))
(:import java.io.File
(clojure.lang ExceptionInfo)))

(use-fixtures :once (ensure-clean-test-dir))

Expand Down Expand Up @@ -164,3 +166,31 @@
(builder/build-app options))

(is (re-matches #".*clojure\.core/\*compiler-options\* \(clojure\.core/merge clojure\.core/\*compiler-options\* \{:direct-linking true, :testing\? true\}\).*" (last @sh-args)))))

(deftest reports-errors-when-merging-from-dir
(let [src (io/file "test/resources")
badlib (io/file src "badlib")
target (io/file "target/tests/build-test/merge-errors")
e (is (thrown? ExceptionInfo
(builder/copy-files #{badlib} target)))]
(when e
(is (= "Unable to read test/resources/badlib/bad-input.edn: Map literal must contain an even number of forms" (ex-message e)))
(is (match? {:classpath-root badlib
:input-source "test/resources/badlib/bad-input.edn"
:target-file (m/via str (str target "/classes/bad-input.edn"))}
(ex-data e))))))

(deftest reports-errors-when-merging-from-jar
(let [src (io/file "test/resources")
badlib (io/file src "badlib")
badlib-jar (io/file "target/badlib.jar")
_ (fs/zip badlib-jar badlib {:root "test/resources/badlib"})
target (io/file "target/tests/build-test/merge-errors")
e (is (thrown? ExceptionInfo
(builder/copy-files #{badlib-jar} target)))]
(when e
(is (= "Unable to read target/badlib.jar#bad-input.edn: Map literal must contain an even number of forms" (ex-message e)))
(is (match? {:classpath-root (m/via str "target/badlib.jar")
:input-source "target/badlib.jar#bad-input.edn"
:target-file (m/via str (str target "/classes/bad-input.edn"))}
(ex-data e))))))

0 comments on commit 463f100

Please sign in to comment.