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

Boot as shell silently quits when the script is malformed #547

Closed
alexander-yakushev opened this issue Dec 22, 2016 · 6 comments
Closed

Comments

@alexander-yakushev
Copy link
Contributor

Here's a valid test.clj:

(println "Loading")

(defn -main [& args]
  (println "Hello" args)))

When launched with boot -f test.clj it works correctly:

Loading
Hello (1 2 3)

But if I put an extra paren after the first form:

(println "Loading")) ;; <-- HERE

(defn -main [& args]
  (println "Hello" args)))

When running the same script I get:

Loading

And the exit code is zero. The expected behavior would be to signalize that the script is malformed.

@alandipert
Copy link
Contributor

boot -b | cat -n yields the following:

     1	(ns boot.user (:use boot.core boot.util boot.task.built-in))
     2	
     3	(clojure.core/comment "start global profile")
     4	
     5	(require (quote boot.repl))
     6	
     7	(deftask cider "CIDER profile" [] (with-pass-thru [_] (swap! boot.repl/*default-dependencies* concat (quote [[org.clojure/tools.nrepl "0.2.12"] [cider/cider-nrepl "0.15.0-SNAPSHOT"] [refactor-nrepl "2.3.0-SNAPSHOT"]])) (swap! boot.repl/*default-middleware* concat (quote [cider.nrepl/cider-middleware refactor-nrepl.middleware/wrap-refactor]))))
     8	
     9	(clojure.core/comment "end global profile")
    10	
    11	(clojure.core/comment "start boot script")
    12	
    13	(println "Loading")
    14	
    15	(clojure.core/comment "end boot script")
    16	
    17	(clojure.core/let [boot?__1744__auto__ true] (clojure.core/if-not boot?__1744__auto__ (clojure.core/when-let [main__1745__auto__ (clojure.core/resolve (quote boot.user/-main))] (main__1745__auto__)) (boot.core/boot "boot.task.built-in/help")))

Notice that Line 13 is well-formed, but the build.boot content afterward is not spliced into the constructed boot script. The method we're using to read in the BOOT_FILE must be silently stopping when it encounters the unmatched paren.

@alandipert alandipert added the Bug label Dec 22, 2016
@alandipert alandipert added this to the future milestone Dec 22, 2016
@alexander-yakushev
Copy link
Contributor Author

Thanks for looking into this!

@alandipert
Copy link
Contributor

The problem starts at https://github.com/boot-clj/boot/blob/master/boot/core/src/boot/main.clj#L180 and lies with read-string via boot.util/read-string-all:

boot.user=> (require '[boot.util :as util])
nil
boot.user=> (util/read-string-all "(println :foo)) (println :bar)")
((println :foo))

read-string just reads the first object from the string and doesn't complain if there is an unexpected delimiter:

boot.user=> (read-string "foo)")
foo

My thinking is: what's the benefit of properly reading these files? We could just concatenate them string-wise and then the user will see syntax errors when the whole thing gets read and evaluated.

If there is a reason to read the forms before concatenating them, then we should probably switch to a read/stream based implementation of read-string-all that throws unmatched delimiter errors instead of truncating source files.

@bhagany
Copy link
Contributor

bhagany commented Dec 22, 2016

I can file a separate bug for this if you'd like, but this is related to read-string-all - it doesn't handle aliased namespaced keywords, when the namespace was required within the string that's being read:

boot.user> (require '[boot.util :as util])
nil
boot.user> (util/read-string-all "::util/hi")
(:boot.util/hi)
boot.user> (util/read-string-all "::util/hi (require '[foo.core :as foo]) ::foo/hi")
java.lang.RuntimeException: Invalid token: ::foo/hi

I'm not sure if a read-based implementation would fix this, but hopefully the chances are good

@alandipert
Copy link
Contributor

Discussed with @micha on Slack a bit, and I think we're in agreement - the way to go here is to move away from read entirely and just concatenate strings. That way any syntax or runtime errors pop up once, when the generated script is finally run.

@bhagany I think that would solve keywords thing too.

Micha also pointed out we can probably use load-string instead of writing the generated script to a temp file and calling load-file.

@alexander-yakushev
Copy link
Contributor Author

Thanks for fixing this. I ❤️ Boot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants