boot-clj / boot

Build tooling for Clojure.
https://boot-clj.github.io/
Eclipse Public License 1.0
1.75k stars 179 forks source link

Make `(sift :move)` create resources #680

Closed bhagany closed 6 years ago

bhagany commented 6 years ago

I'll be honest - I don't know if this is a good idea, but I think there is a genuine bug that should be addressed. The problem stems from calling sync-user-dirs! at the end of the task stack - doing so clobbers moves files that were moved via (sift :move) back to their original position on the filesystem, but does not apply the same transistion to the fileset. Files that have roles do not suffer this inconsistency.

The result is that any post-wraps in the task stack will find that files in the fileset they were given don't actually exist on disk. This is also a problem when serving files from the fileset - I end up missing any files that were moved with sift.

bhagany commented 6 years ago

I should also note that this problem also happens with boot.core/mv, but since I'm not sure what the right fix is, I limited the scope of this PR to await feedback.

arichiardi commented 6 years ago

Side-note, it might be what causes this boot-http problem as well.

arichiardi commented 6 years ago

Also worth noting that if this is the way to go, one alternative untested arguably more readable solution would be to always compose the :move with the :to-resource action.

bhagany commented 6 years ago

Here's a small build.boot I used to reproduce:

(set-env!
 :resource-paths #{"resources"}
 :dependencies '[[boot/core "2.8.0-SNAPSHOT"]])

(require '[boot.core :as boot])

(deftask foo->bar []
  (comp (sift :move {#"foo\.txt" "bar.txt"})
        (fn [next-task]
          (fn [fileset]
            (let [bar (boot/tmp-get fileset "bar.txt")]
              (println (.getPath (boot/tmp-file bar)))
              (Thread/sleep 20000)
              (println "syncing")
              (#'boot/sync-user-dirs!)
              (Thread/sleep 20000)
              (next-task fileset))))))

You'll also need to create a file called resources/foo.txt

I'm manually calling sync-user-dirs! here to show that it's the problem. When run, you'll see that the printed path will exist during the first sleep, and be gone during the second sleep.

arichiardi commented 6 years ago

I have tried this branch against my perun project and I have a very weird problem. Basically the classic pattern with watch does not seem to work:

(deftask dev
  [s stage VAL kw "The stage value - :dev, :staging or :prod"]
  (comp (include-js)
        (watch) 
        (blog-less)
        (serve :resource-root "public")
        (notify :audible true)))

It gets stuck at the following without further output (with -v as well):

 boot dev
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
Sifting output files...
Sifting output files...
Sifting output files...

Starting file watcher (CTRL-C to quit)...

Everything works on master.

bhagany commented 6 years ago

@arichiardi do you have the full code somewhere I can see?

arichiardi commented 6 years ago

@bhagany, no but I want to write a smaller sample. Hopefully very soon.

arichiardi commented 6 years ago

This still hangs for me with the patch applied. I have not been able to understand why yet.

arichiardi commented 6 years ago

I am going to ridicule myself with the following but :+1: so...your patch works, the fact that I thought it was hanging is a side effect of it: the watch task, instead of immediately triggering and then wait, triggers after the first change to a file happens.

The difference we see in behavior might be due to the platform (I am running on linux).

I think we should strive to preserve the behavior but if we can't I of course prefer to have the fix in.

alandipert commented 6 years ago

Thanks all. This is a solid fix.