From b8f9dcb2b4d8fff56d979946762b8572076e415e Mon Sep 17 00:00:00 2001 From: Timo Kluck Date: Thu, 12 Mar 2020 17:51:58 +0100 Subject: [PATCH 1/8] Support user-specified callbacks This feature is one half of what `Revise.entr` already does. The function `entr` allows the user to install callbacks for when a file or module changes, and also enters a blocking loop that catches any errors that might arise from the callbacks. This new `add_callback` function allows installing callbacks and keeping them around in the background. This commit also reimplements `entr` in terms of `add_callback` and `remove_callback`. --- src/Revise.jl | 137 +++++++++++++++++++++++++++++++++++------------ src/pkgs.jl | 1 + test/runtests.jl | 33 ++++++++++++ 3 files changed, 136 insertions(+), 35 deletions(-) diff --git a/src/Revise.jl b/src/Revise.jl index 3e03ff8b..ef86ca67 100644 --- a/src/Revise.jl +++ b/src/Revise.jl @@ -94,6 +94,74 @@ This list gets populated by callbacks that watch directories for updates. """ const revision_queue = Set{Tuple{PkgData,String}}() +const revision_event = Condition() + +""" + Revise.user_callbacks_queue + +Global variable, `user_callbacks_queue` holds `key` values for which the +file has changed but the user hooks have not yet been called. +""" +const user_callbacks_queue = Set{Any}() + +const user_callbacks_by_file = Dict{String, Set{Any}}() +const user_callbacks_by_key = Dict{Any, Any}() + +function add_callback(f, files, modules=nothing; key=gensym()) + remove_callback(key) + + files = map(abspath, files) + init_watching(files) + + if modules !== nothing + for mod in modules + id = PkgId(mod) + pkgdata = pkgdatas[id] + for file in srcfiles(pkgdata) + absname = joinpath(basedir(pkgdata), file) + push!(files, absname) + track(mod, absname) + end + end + end + + for file in files + cb = get!(Set, user_callbacks_by_file, file) + push!(cb, key) + user_callbacks_by_key[key] = f + end + + return key +end + +function remove_callback(key) + for cbs in values(user_callbacks_by_file) + delete!(cbs, key) + end + delete!(user_callbacks_by_key, key) + # TODO: stop watching these files + nothing +end + +function process_user_callbacks!(keys = user_callbacks_queue; throw=false) + try + # use (a)sync so any exceptions get nicely collected into CompositeException + @sync for key in keys + f = user_callbacks_by_key[key] + @async Base.invokelatest(f) + end + catch err + if throw + rethrow(err) + else + @warn "[Revise] Ignoring callback errors" err + end + finally + empty!(keys) + end +end + + """ Revise.queue_errors @@ -102,6 +170,8 @@ Global variable, maps `(pkgdata, filename)` pairs that errored upon last revisio """ const queue_errors = Dict{Tuple{PkgData,String},Tuple{Exception, Any}}() +const NOPACKAGE = PkgId(nothing, "") + """ Revise.pkgdatas @@ -110,7 +180,7 @@ and julia objects, and allows re-evaluation of code in the proper module scope. It is a dictionary indexed by PkgId: `pkgdatas[id]` returns a value of type [`Revise.PkgData`](@ref). """ -const pkgdatas = Dict{PkgId,PkgData}() +const pkgdatas = Dict{PkgId,PkgData}(NOPACKAGE => PkgData(NOPACKAGE)) const moduledeps = Dict{Module,DepDict}() function get_depdict(mod::Module) @@ -471,7 +541,7 @@ function init_watching(pkgdata::PkgData, files) end return nothing end -init_watching(files) = init_watching(PkgId(Main), files) +init_watching(files) = init_watching(pkgdatas[NOPACKAGE], files) """ revise_dir_queued(dirname) @@ -494,9 +564,16 @@ This is generally called via a [`Revise.Rescheduler`](@ref). latestfiles, stillwatching = watch_files_via_dir(dirname) # will block here until file(s) change for (file, id) in latestfiles key = joinpath(dirname, file) - pkgdata = pkgdatas[id] - if hasfile(pkgdata, key) # issue #228 - push!(revision_queue, (pkgdata, relpath(key, pkgdata))) + if key in keys(user_callbacks_by_file) # TODO: also do this for per-file watching + union!(user_callbacks_queue, user_callbacks_by_file[key]) + notify(revision_event) + end + if id != NOPACKAGE + pkgdata = pkgdatas[id] + if hasfile(pkgdata, key) # issue #228 + push!(revision_queue, (pkgdata, relpath(key, pkgdata))) + notify(revision_event) + end end end return stillwatching @@ -520,6 +597,7 @@ function revise_file_queued(pkgdata::PkgData, file) sleep(0.1) # in case git has done a delete/replace cycle if !file_exists(file) push!(revision_queue, (pkgdata, file0)) # process file deletions + notify(revision_event) return false end end @@ -529,6 +607,7 @@ function revise_file_queued(pkgdata::PkgData, file) dirfull, basename = splitdir(file) if haskey(watched_files, dirfull) push!(revision_queue, (pkgdata, file0)) + notify(revision_event) return true end return false @@ -592,11 +671,13 @@ function errors(revision_errors=keys(queue_errors)) end """ - revise() + revise(; throw=false) `eval` any changes in the revision queue. See [`Revise.revision_queue`](@ref). +If `throw` is `true`, throw any errors that occur during revision or callback; +otherwise these are only logged. """ -function revise() +function revise(; throw=false) sleep(0.01) # in case the file system isn't quite done writing out the new files # Do all the deletion first. This ensures that a method that moved from one file to another @@ -649,6 +730,9 @@ function revise() Use Revise.errors() to report errors again.""" end tracking_Main_includes[] && queue_includes(Main) + + process_user_callbacks!(throw=throw) + nothing end revise(backend::REPL.REPLBackend) = revise() @@ -792,39 +876,22 @@ This will print "update" every time `"/tmp/watched.txt"` or any of the code defi """ function entr(f::Function, files, modules=nothing; postpone=false, pause=0.02) yield() - files = collect(files) # because we may add to this list - if modules !== nothing - for mod in modules - id = PkgId(mod) - pkgdata = pkgdatas[id] - for file in srcfiles(pkgdata) - push!(files, joinpath(basedir(pkgdata), file)) - end - end + postpone || f() + key = add_callback(files, modules) do + sleep(pause) + f() end - active = true + mycallbacks = [key] try - @sync begin - postpone || f() - for file in files - waitfor = isdir(file) ? watch_folder : watch_file - @async while active - ret = waitfor(file, 1) - if active && (ret.changed || ret.renamed) - sleep(pause) - revise() - Base.invokelatest(f) - end - end - end + while true + wait(revision_event) + revise(throw=true) end catch err - if isa(err, InterruptException) - active = false - else - rethrow(err) - end + isa(err, InterruptException) || rethrow(err) end + remove_callback(key) + nothing end """ diff --git a/src/pkgs.jl b/src/pkgs.jl index ae294e23..5a7d715f 100644 --- a/src/pkgs.jl +++ b/src/pkgs.jl @@ -491,6 +491,7 @@ function watch_manifest(mfile) maybe_parse_from_cache!(pkgdata, file) push!(revision_queue, (pkgdata, file)) push!(files, file) + notify(revision_event) end # Update the directory pkgdata.info.basedir = pkgdir diff --git a/test/runtests.jl b/test/runtests.jl index 974b554d..61f6d6b0 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -2780,6 +2780,39 @@ do_test("entr with modules") && @testset "entr with modules" begin end +do_test("callbacks") && @testset "callbacks" begin + + mktemp() do path, io + contents = Ref("") + key = Revise.add_callback([path]) do + contents[] = read(path, String) + end + + sleep(mtimedelay) + + write(io, "abc") + flush(io) + sleep(mtimedelay) + revise() + @test contents[] == "abc" + + write(io, "def") + flush(io) + sleep(mtimedelay) + revise() + @test contents[] == "abcdef" + + Revise.remove_callback(key) + + write(io, "ghi") + flush(io) + sleep(mtimedelay) + revise() + @test contents[] == "abcdef" + end + +end + println("beginning cleanup") GC.gc(); GC.gc() From 9fbdd82d7969b7c0f7ffee82972e77c513b2e687 Mon Sep 17 00:00:00 2001 From: Timo Kluck Date: Wed, 25 Mar 2020 13:33:22 +0100 Subject: [PATCH 2/8] Callbacks: add extra sleep(...) for CI --- test/runtests.jl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/runtests.jl b/test/runtests.jl index 61f6d6b0..a01c17fe 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -2796,6 +2796,8 @@ do_test("callbacks") && @testset "callbacks" begin revise() @test contents[] == "abc" + sleep(mtimedelay) + write(io, "def") flush(io) sleep(mtimedelay) @@ -2803,6 +2805,7 @@ do_test("callbacks") && @testset "callbacks" begin @test contents[] == "abcdef" Revise.remove_callback(key) + sleep(mtimedelay) write(io, "ghi") flush(io) From 886be8be1ea56c3f99c9f7579174d539621ba9e8 Mon Sep 17 00:00:00 2001 From: Timo Kluck Date: Wed, 25 Mar 2020 13:43:36 +0100 Subject: [PATCH 3/8] revise_file_queued: Add user_callbacks_by_file support --- src/Revise.jl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Revise.jl b/src/Revise.jl index ef86ca67..5b0841f7 100644 --- a/src/Revise.jl +++ b/src/Revise.jl @@ -564,7 +564,7 @@ This is generally called via a [`Revise.Rescheduler`](@ref). latestfiles, stillwatching = watch_files_via_dir(dirname) # will block here until file(s) change for (file, id) in latestfiles key = joinpath(dirname, file) - if key in keys(user_callbacks_by_file) # TODO: also do this for per-file watching + if key in keys(user_callbacks_by_file) union!(user_callbacks_queue, user_callbacks_by_file[key]) notify(revision_event) end @@ -603,6 +603,12 @@ function revise_file_queued(pkgdata::PkgData, file) end wait_changed(file) # will block here until the file changes + + if file in keys(user_callbacks_by_file) + union!(user_callbacks_queue, user_callbacks_by_file[file]) + notify(revision_event) + end + # Check to see if we're still watching this file dirfull, basename = splitdir(file) if haskey(watched_files, dirfull) From 5eebd46492ece6272f3ba19727ef7ea63666ee8e Mon Sep 17 00:00:00 2001 From: Timo Kluck Date: Sun, 29 Mar 2020 13:54:31 +0200 Subject: [PATCH 4/8] Don't rely on open file handle / flush to trigger file watch As observed by @mlhetland, this does not seem to work on all platforms. --- test/runtests.jl | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index a01c17fe..a7735db8 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -2782,7 +2782,11 @@ end do_test("callbacks") && @testset "callbacks" begin - mktemp() do path, io + append(path, x...) = open(path, append=true) do io + write(io, x...) + end + + mktemp() do path, _ contents = Ref("") key = Revise.add_callback([path]) do contents[] = read(path, String) @@ -2790,16 +2794,14 @@ do_test("callbacks") && @testset "callbacks" begin sleep(mtimedelay) - write(io, "abc") - flush(io) + append(path, "abc") sleep(mtimedelay) revise() @test contents[] == "abc" sleep(mtimedelay) - write(io, "def") - flush(io) + append(path, "def") sleep(mtimedelay) revise() @test contents[] == "abcdef" @@ -2807,8 +2809,7 @@ do_test("callbacks") && @testset "callbacks" begin Revise.remove_callback(key) sleep(mtimedelay) - write(io, "ghi") - flush(io) + append(path, "ghi") sleep(mtimedelay) revise() @test contents[] == "abcdef" From 60d10d67be98cc94721b267fb501d87b409f4f64 Mon Sep 17 00:00:00 2001 From: Timo Kluck Date: Fri, 3 Apr 2020 11:31:03 +0200 Subject: [PATCH 5/8] add/remove callbacks: docstrings --- src/Revise.jl | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/Revise.jl b/src/Revise.jl index 5b0841f7..6d405518 100644 --- a/src/Revise.jl +++ b/src/Revise.jl @@ -107,6 +107,18 @@ const user_callbacks_queue = Set{Any}() const user_callbacks_by_file = Dict{String, Set{Any}}() const user_callbacks_by_key = Dict{Any, Any}() +""" + key = Revise.add_callback(f, files, modules=nothing; key=gensym()) + +Add a user-specified callback, to be executed during the first run of +`revise()` after a file in `files` or a module in `modules` is changed on the +file system. In an interactive session like the REPL, Juno or Jupyter, this +means the callback executes immediately before executing a new command / cell. + +You can use the return value `key` to remove the callback later +(`Revise.remove_callback`) or to update it using another call +to `Revise.add_callback` with `key=key`. +""" function add_callback(f, files, modules=nothing; key=gensym()) remove_callback(key) @@ -134,6 +146,12 @@ function add_callback(f, files, modules=nothing; key=gensym()) return key end +""" + Revise.remove_callback(key) + +Remove a callback previously installed by a call to `Revise.add_callback(...)`. +See its docstring for details. +""" function remove_callback(key) for cbs in values(user_callbacks_by_file) delete!(cbs, key) From 035dcc8f8718e3faa15283dfe51a65dc6ae4bb41 Mon Sep 17 00:00:00 2001 From: Timo Kluck Date: Fri, 3 Apr 2020 11:39:27 +0200 Subject: [PATCH 6/8] remove_callback: don't stop watching files (for now) --- src/Revise.jl | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Revise.jl b/src/Revise.jl index 6d405518..cae83b81 100644 --- a/src/Revise.jl +++ b/src/Revise.jl @@ -157,7 +157,15 @@ function remove_callback(key) delete!(cbs, key) end delete!(user_callbacks_by_key, key) - # TODO: stop watching these files + + # possible future work: we may stop watching (some of) these files + # now. But we don't really keep track of what background tasks are running + # and Julia doesn't have an ergonomic way of task cancellation yet (see + # e.g. + # https://github.com/JuliaLang/Juleps/blob/master/StructuredConcurrency.md + # so we'll omit this for now. The downside is that in pathological cases, + # this may exhaust inotify resources. + nothing end From 346ceced0aa1ae07c1ed96196548fb3de8520d27 Mon Sep 17 00:00:00 2001 From: Timo Kluck Date: Fri, 3 Apr 2020 11:50:04 +0200 Subject: [PATCH 7/8] add_callback: add test for version with modulesy --- test/runtests.jl | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/runtests.jl b/test/runtests.jl index a7735db8..e4275b71 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -2815,6 +2815,40 @@ do_test("callbacks") && @testset "callbacks" begin @test contents[] == "abcdef" end + testdir = newtestdir() + modname = "A355" + srcfile = joinpath(testdir, modname * ".jl") + + function setvalue(x) + open(srcfile, "w") do io + print(io, "module $modname test() = $x end") + end + end + + setvalue(1) + + sleep(mtimedelay) + @eval using A355 + sleep(mtimedelay) + + A355_result = Ref(0) + + Revise.add_callback([], [A355]) do + A355_result[] = A355.test() + end + + sleep(mtimedelay) + setvalue(2) + # belt and suspenders -- make sure we trigger entr: + sleep(mtimedelay) + touch(srcfile) + + yry() + + @test A355_result[] == 2 + + rm_precompile(modname) + end println("beginning cleanup") From 03278a8abc3d6e8341310688626431220cfe1b2b Mon Sep 17 00:00:00 2001 From: Timo Kluck Date: Fri, 10 Apr 2020 10:29:03 +0200 Subject: [PATCH 8/8] entr: remove unused variable --- src/Revise.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Revise.jl b/src/Revise.jl index cae83b81..5ddc6ca8 100644 --- a/src/Revise.jl +++ b/src/Revise.jl @@ -913,7 +913,6 @@ function entr(f::Function, files, modules=nothing; postpone=false, pause=0.02) sleep(pause) f() end - mycallbacks = [key] try while true wait(revision_event)