Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect and report errors reading mergeable sources #41

Merged
merged 2 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 - origin of the input, a File; either a directory 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))))))