Revise.jl icon indicating copy to clipboard operation
Revise.jl copied to clipboard

Revise loaded in -E/-e for -i not updating definitions

Open mortenpi opened this issue 6 years ago • 18 comments

When loading a Julia script as follows:

julia -i -e 'using Revise; includet("script.jl")'

Revise will not update the function definitions etc. afterwards. The original version of the script gets loaded without issues though.

Tested with Revise master (dd6e970a23e0b998883f814742723bf53b88f57f) on Julia 1.0.0 and the normal Revise workflow (using Revise; includet("script.jl") in the REPL) works without problems.

Edit: just a bit more debugging:

sh$ julia -i -e 'using Revise'
julia> includet("script.jl")

also fails (i.e. not updating), so it seems to be a problem with loading Revise in the -E/-e option.

mortenpi avatar Oct 14 '18 02:10 mortenpi

Seems to work on the master branch for me.

timholy avatar Dec 30 '18 11:12 timholy

I still see the issue with both Julia 1.0.3 and 1.2.0-DEV.74 on the Revise master. I'll see if I can put together a script + env to reliably reproduce this.

mortenpi avatar Dec 31 '18 02:12 mortenpi

I've put together a restricted environment and an exact list of commands that should reproduce the issue. Since it is dependent on the REPL loop, I couldn't really make it a single script, but at least the Julia commands can be just pasted into the REPL all at once:

https://gist.github.com/mortenpi/c944f5affbff54b1ddb13a7c14945f02#file-mwe-txt

Just to note that I am running this on Linux, as it may make a difference.

mortenpi avatar Jan 01 '19 00:01 mortenpi

Thanks for the reproducer. I must have been fooled by the fact that I launch Revise from my startup file.

So, the problem is that with julia -i -e 'using Revise', Revise's __init__ gets called before the REPL launches. As a consequence, this line doesn't run. Your script will revise automatically if you preface it with

julia> Revise.steal_repl_backend(Base.active_repl_backend)

It's possible that one could @async all that logic, but I recently replaced a bunch of them (#220) because they were slowing down startup. What do you use this for? I'm trying to gauge how important it is, and whether to fix with code or with documentation.

timholy avatar Jan 01 '19 16:01 timholy

I think I ran into this when I was working on some functions and I needed to restart Julia over and over again (I was probably updating type definitions). So I wanted to includet the script right there in the command line so that I would have a REPL with all my functions etc. right away.

But not being able to do that is a pretty minor inconvenience, so I think just documenting the workaround is fine if there is no good way to fix it at the moment. Would it be possible to make Revise print a warning if it is loaded in this way?

mortenpi avatar Jan 03 '19 00:01 mortenpi

I got hit with this again, but this time I figured out that I can just use the same solution as the docs recommend for startup.jl:

julia -i -e'atreplinit() do repl
    @eval using Revise
    @async Revise.wait_steal_repl_backend()
    @eval includet("script.jl")
end'

Would there be any way to detect and warn if Revise is not loaded properly? It could also be useful when someone naively puts just using Revise in startup.jl. But I guess it might not really be possible to define when Revise is loaded "properly" and it isn't?

mortenpi avatar Feb 14 '19 05:02 mortenpi

The concern with issuing a warning is that it would be intrusive for people who launch Revise in their startup.jl file but occasionally run a script with julia script.jl.

timholy avatar Apr 05 '19 15:04 timholy

It should not be a problem if you follow the recommended way though? The atreplinit should never get called if you're running a script, right?

mortenpi avatar Apr 07 '19 22:04 mortenpi

Yes, good point. I'd be willing to give this a try. Would you do me a favor, though? Make this change

$ git diff
diff --git a/src/Revise.jl b/src/Revise.jl
index 49f670b..4d0841d 100644
--- a/src/Revise.jl
+++ b/src/Revise.jl
@@ -1034,6 +1034,8 @@ function __init__()
             steal_repl_backend(Base.active_repl_backend::REPL.REPLBackend)
         elseif isdefined(Main, :IJulia)
             Main.IJulia.push_preexecute_hook(revise)
+        else
+            @warn "Revise may not be appropriately configured, see documentation"
         end
         if isdefined(Main, :Atom)
             setup_atom(getfield(Main, :Atom)::Module)

and play with it a little while? If it doesn't cause you any problems then I could go with it.

timholy avatar Apr 08 '19 09:04 timholy

Hmm, this particular error will pop up when you just start julia and you have the atreplinit() logic in startup.jl.

Would it be possible to have the @async Revise.wait_steal_repl_backend() line in __init__()? Or would that just horribly break everything?

mortenpi avatar Apr 10 '19 04:04 mortenpi

Check out the call to steal_repl_backend from both __init__ and wait_steal_repl_backend.

timholy avatar Apr 10 '19 07:04 timholy

Hmm, I indeed didn't look at the code that closely. However, if I understand correctly, steal_repl_backend does not get called in __init__ unless you are doing using Revise in the REPL.

I am more or less just thinking out loud here at the moment. I am currently running Revise with the following change:

diff --git a/src/Revise.jl b/src/Revise.jl
index 49f670b..8aee247 100644
--- a/src/Revise.jl
+++ b/src/Revise.jl
@@ -1034,6 +1034,10 @@ function __init__()
             steal_repl_backend(Base.active_repl_backend::REPL.REPLBackend)
         elseif isdefined(Main, :IJulia)
             Main.IJulia.push_preexecute_hook(revise)
+       else
+           @info "Setting up @async task to steal backend" # debug
+           @async Revise.wait_steal_repl_backend()
         end
         if isdefined(Main, :Atom)
             setup_atom(getfield(Main, :Atom)::Module)

and nothing in the startup.jl. This fixes the original problem

$ julia -i -e'using Revise; includet("test.jl")'

as far as I can tell. It also seems to allow you to just have using Revise in startup.jl, instead of the atreplinit() stuff.

mortenpi avatar Apr 10 '19 22:04 mortenpi

Just as a data point, I ran into the same issue just now, trying to run a script to set things up before interactive use.

mlhetland avatar May 01 '19 13:05 mlhetland

If you don't discover any negatives to this, I'd welcome a PR.

timholy avatar May 01 '19 13:05 timholy

I've tried putting this into a makefile:

atreplinit() do repl
    @eval using Revise
    @async Revise.wait_steal_repl_backend()
    # some setup
    @eval using MyModule
end

As long as no recompilation is required upon starting, this works just fine, but otherwise I get the following error:

┌ Warning: REPL initialization failed, Revise is not in automatic mode. Call `revise()` manually.
└ @ Revise ~/.julia/dev/Revise/src/Revise.jl:993

I guess that's to be expected, given the timeout behavior in wait_steal_repl_backend (i.e., it waits at most 1 second). If I just add a long sleep to the @async call (or even wrap it in my own loop along the lines of wait_steal_repl_backend's), it works well enough, though I'm not sure if that's the best solution?

I guess an option could be to let the 20 limit for iter be a keyword argument (tries?) or something, and I could just supply a really high value.

mlhetland avatar Aug 05 '19 18:08 mlhetland

What if we couple the suggestion in https://github.com/timholy/Revise.jl/issues/202#issuecomment-481887015 with a check of Base.isinteractive()? Does that solve everyone's problem?

timholy avatar Sep 01 '19 16:09 timholy

I don't really know!-) Maybe? How would you use the check? Wait until it becomes true, or something? Or is it perhaps true from the beginning, as long as Julia knows it's "heading toward" a REPL (just not quite there yet)?

mlhetland avatar Sep 02 '19 10:09 mlhetland

Discussion may move to #349

timholy avatar Sep 03 '19 08:09 timholy