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

Confusing set-env! semantics for different keys #21

Closed
dm3 opened this issue Nov 21, 2014 · 2 comments
Closed

Confusing set-env! semantics for different keys #21

dm3 opened this issue Nov 21, 2014 · 2 comments
Labels

Comments

@dm3
Copy link
Contributor

dm3 commented Nov 21, 2014

This is probably related to #16 in a more general sense.

In the following script I tried to establish additional src/resource paths and dependencies for the dev environment (calling set-env! in a dev task).:

(set-env!
 :src-paths    #{"src/clj", "src/cljs"}
 :rsc-paths    #{"resources"}
 :dependencies '[[compojure "1.2.1"]])

(deftask dev
  "Start development environment"
  []
  (set-env! :rsc-paths #{"dev/resources"})
  (set-env! :src-paths #{"dev/clj"})
  (set-env! :dependencies '[[org.clojure/tools.namespace "0.2.7"]])
  (println "Deps" (get-env :dependencies))
  (println "Src" (get-env :src-paths))
  (println "Res" (get-env :rsc-paths)))

which produces

=> boot dev
Deps [[org.clojure/tools.namespace 0.2.7]]
Src #{dev/clj src/cljs src/clj}
Res #{dev/resources}

Am I using the set-env! function correctly for the result I want to get (dev environment/profile)?

In any case the semantics of set-env! are confusing.
Currently set-env! works by delegating to merge-env!, which merges sources, overwrites resources (seems like a bug as there is a defmethod for :src, but not for :rsc) and overwrites dependencies (although it seems to be calling add-dependency! with an old dependency set).
I'd think that set-env! should always overwrite, not merge, while update-env! should accept a function analogous to clojure.core/update. There could be a simplified version of update-env! - append-env! (or conj-env!) which would always try to merge the values if it made sense and fail otherwise. Could you please explain the reasoning behind the current semantics of set-env! and merge-env!?

I think Boot is a great project and a step forward for the Clojure ecosystem. I'd love to contribute, so understanding the design decisions a bit better would help a great deal!

@micha
Copy link
Contributor

micha commented Nov 21, 2014

Hi! Thanks for opening this ticket!

I agree that set-env! is not ideal in this case. Resource and source paths should be treated the same, so that is indeed a bug which needs to be fixed. However, there is a deeper problem here: source and resource paths are on the class path. This means that there is no way to remove them or replace them with anything else, because there's no mechanism we can use to remove a directory from the classloader's class path (as far as I know). This is why set-env! always appends to :src-paths.

The set-env! function modifies the boot environment, which is basically the classloader bootstrapping environment, i.e. classpath configuration, Aether/Pomegranate configuration, JAR dependencies, etc. This env data cannot contain anything that can't be round-tripped through the Clojure reader and printer because the env must be able to be communicated across pod boundaries (pods are isolated, separate Clojure runtimes that can run concurrently in boot to perform work). Since functions are not serializable they can't be valid values for keys in the env. We exploit this in set-env!: if the value is a function then we apply the function to the current value to obtain the next value.

For example:

(let [deps (get-env :dependencies)]
  (set-env! :dependencies (conj deps '[foo/bar "1.2.3"])))

is equivalent to

(set-env! :dependencies #(conj % '[foo/bar "1.2.3"]))

I've been working on the boot-clj/boot tempdirs branch which addresses the classpath problems discussed above. (There are other complications from this classpath issue in other parts of boot also, so it's something we want to get right.) In this branch the source and resource paths are not on the classpath. Instead the contents of these directories are copied into boot's classpath as necessary by a background thread. This means that you can add or remove paths from the set of paths this background thread is syncing with the boot classpath, effectively adding or removing directories from the boot classpath at will. This, combined with some other refactoring greatly simplifies the set-env! semantics, removing the need for merge-env! in most cases. This branch also cleans up the names of things a lot (like :source-paths, :resource-paths, etc.) and adds support for the new immutable fileset machinery we've been working on.

The idea behind merge-env! and on-env! are to manage side-effects related to manipulations of the JVM environment. For instance, add-dependencies is called in merge-env! because it might raise an exception. If adding dependencies fails we don't want to add those deps to the :dependencies key of the env atom–we want to just propagate the exception (like if you're in the REPL and you add a bogus dependency, you should be able to retry without needing to clean up your env). On the other hand, sometimes we need to do things after the env has been modified successfully, which is where we use on-env!.

I'll be working on full documentation for the tempdirs stuff over the next few days, in the wiki. I hope to get the docs complete before pushing any breaking changes to master. Your feedback is greatly appreciated!

@micha micha added the question label Nov 21, 2014
@dm3
Copy link
Contributor Author

dm3 commented Nov 24, 2014

I see, thank you.

@dm3 dm3 closed this as completed Nov 24, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants