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

Integrate with CodeTracking #245

Merged
merged 4 commits into from
Mar 4, 2019
Merged

Integrate with CodeTracking #245

merged 4 commits into from
Mar 4, 2019

Conversation

timholy
Copy link
Owner

@timholy timholy commented Mar 2, 2019

This sets up CodeTracking.jl as the new "query interface" of Revise.

@oxinabox
Copy link
Contributor

oxinabox commented Mar 2, 2019

I am now using this branch and it is (so far) working for me. ❤

--
update:
Actually it is not.

It would be cool if you could push your CodeTracking.jl branch at the sametime you push to this branch.
Alternatively I could just be patient.

@timholy timholy force-pushed the teh/codetracking branch from dc58ae0 to e345e3e Compare March 3, 2019 11:07
@codecov
Copy link

codecov bot commented Mar 3, 2019

Codecov Report

Merging #245 into master will increase coverage by 0.23%.
The diff coverage is 97.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #245      +/-   ##
==========================================
+ Coverage   77.47%   77.71%   +0.23%     
==========================================
  Files          11       11              
  Lines        1079     1095      +16     
==========================================
+ Hits          836      851      +15     
- Misses        243      244       +1
Impacted Files Coverage Δ
src/exprutils.jl 72.64% <100%> (+0.71%) ⬆️
src/recipes.jl 87.5% <100%> (-0.55%) ⬇️
src/utils.jl 54% <100%> (ø) ⬆️
src/Revise.jl 68.47% <100%> (+1.06%) ⬆️
src/types.jl 95.12% <93.75%> (-1.18%) ⬇️
src/pkgs.jl 88.42% <96.15%> (-0.87%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0adbade...816c3e0. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 3, 2019

Codecov Report

Merging #245 into master will decrease coverage by 14.1%.
The diff coverage is 68.83%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #245       +/-   ##
===========================================
- Coverage   77.47%   63.37%   -14.11%     
===========================================
  Files          11       11               
  Lines        1079     1095       +16     
===========================================
- Hits          836      694      -142     
- Misses        243      401      +158
Impacted Files Coverage Δ
src/exprutils.jl 71.79% <100%> (-0.14%) ⬇️
src/utils.jl 70% <100%> (+16%) ⬆️
src/pkgs.jl 57.89% <50%> (-31.4%) ⬇️
src/recipes.jl 59.09% <60%> (-28.96%) ⬇️
src/Revise.jl 57.6% <72%> (-9.81%) ⬇️
src/types.jl 68.29% <87.5%> (-28.01%) ⬇️
src/git.jl 22.58% <0%> (-48.39%) ⬇️
src/logging.jl 30.76% <0%> (-23.08%) ⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0adbade...e345e3e. Read the comment docs.

@timholy
Copy link
Owner Author

timholy commented Mar 3, 2019

It would be cool if you could push your CodeTracking.jl branch at the sametime you push to this branch. Alternatively I could just be patient.

There's a pretty complicated dance going on right now with this branch, #243, and work in JuliaInterpreter (e.g., JuliaDebug/JuliaInterpreter.jl#88). I fear you're going to have to expect some breakage for a little while yet.

Some more breaking changes are coming. I need method_at(file, lineno) for work in JuliaInterpreter, and that will likely change the internal representation that Revise and/or CodeTracking use.

@timholy
Copy link
Owner Author

timholy commented Mar 3, 2019

But it should be working as of this moment 😄

@timholy
Copy link
Owner Author

timholy commented Mar 3, 2019

The most recent commit requires timholy/CodeTracking.jl#11

@timholy timholy closed this Mar 4, 2019
@timholy timholy reopened this Mar 4, 2019
@timholy timholy merged commit a2c0fd6 into master Mar 4, 2019
@timholy timholy deleted the teh/codetracking branch March 4, 2019 19:09
aviatesk added a commit to aviatesk/Revise.jl that referenced this pull request Dec 23, 2020
There can be cases where a callback key is removed from
`user_callbacks_by_key` but it still remains in `user_callbacks_queue`.

The problem may be illustrated with the following MRE.

Say, with the following diff
```diff
diff --git a/src/callbacks.jl b/src/callbacks.jl
index bff5256..865bb24 100644
--- a/src/callbacks.jl
+++ b/src/callbacks.jl
@@ -159,6 +159,7 @@ function entr(f::Function, files, modules=nothing; 
all=false, postpone=false, pa
         sleep(pause)
         f()
     end
+    @info "added $(key)"
     try
         while true
             wait(revision_event)
@@ -168,7 +169,7 @@ function entr(f::Function, files, modules=nothing; 
all=false, postpone=false, pa
         isa(err, InterruptException) || rethrow(err)
     finally
         remove_callback(key)
+        @info "removed $(key)"
     end
     nothing
 end
-
```

and we run the following script:
> entr.jl
```julia
using Revise

const TARGET_FILE = "watched.jl"

let
    Revise.includet(TARGET_FILE)

    interrupted = false
    while !interrupted
        try
            entr([TARGET_FILE]) do
                # do something ...
            end
            interrupted = true
        catch err
            # handle expected errors, keep running

            # sync errors
            if isa(err, LoadError) ||
               (isa(err, ErrorException) && startswith(err.msg, 
"lowering returned an error")) ||
               isa(err, Revise.ReviseEvalException)
                @warn "handling sync error" err
                continue
            end

            # async errors
            if isa(err, CompositeException)
                errs = err.exceptions
                i = findfirst(e->isa(e, TaskFailedException), errs)
                if i !== nothing
                    tfe = errs[i]::TaskFailedException
                    res = tfe.task.result
                    if isa(res, LoadError) ||
                       (isa(res, ErrorException) && startswith(res.msg, 
"lowering returned an error")) ||
                       isa(res, Revise.ReviseEvalException)
                        @warn "handling async error" res
                        continue
                    end
                end
            end
            rethrow(err)
        end
    end
end
```

where `watched.jl` has something like below:
```julia
foo(a) = a # first state
# foo(a) = end # second state
```

Now, boot up REPL and `include("entr.jl")`, and then modify `watch.jl`
so that we alternately comment-in/out the lines of first/second state.
It will result in something like below on the current master:
```julia
julia> include("entr.jl")
[ Info: added #timholy#245
[ Info: removed #timholy#245
┌ Warning: handling sync error
│   err =
│    LoadError: "unexpected \"end\""
│    in expression starting at 
/Users/aviatesk/julia/packages/Revise/watched.jl:2
└ @ Main ~/julia/packages/Revise/entr.jl:22
[ Info: added #timholy#246
[ Info: removed #timholy#246
ERROR: LoadError: KeyError: key Symbol("#timholy#245") not found
Stacktrace:
  [1] getindex(h::Dict{Any, Any}, key::Symbol)
    @ Base ./dict.jl:482
  [2] macro expansion
    @ ~/julia/packages/Revise/src/callbacks.jl:116 [inlined]
  [3] macro expansion
    @ ./task.jl:382 [inlined]
  [4] process_user_callbacks!(keys::Set{Any}; throw::Bool)
    @ Revise ~/julia/packages/Revise/src/callbacks.jl:115
  [5] revise(; throw::Bool)
    @ Revise ~/julia/packages/Revise/src/packagedef.jl:816
  [6] entr(f::var"timholy#3#5", files::Vector{String}, modules::Nothing; 
all::Bool, postpone::Bool, pause::Float64)
    @ Revise ~/julia/packages/Revise/src/callbacks.jl:166
  [7] entr(f::Function, files::Vector{String}, modules::Nothing)
    @ Revise ~/julia/packages/Revise/src/callbacks.jl:156
  [8] top-level scope
    @ ~/julia/packages/Revise/entr.jl:11
  [9] include(fname::String)
    @ Base.MainInclude ./client.jl:451
 [10] top-level scope
    @ REPL[1]:1
in expression starting at 
/Users/aviatesk/julia/packages/Revise/entr.jl:5
```
, which will be fixed with this PR.
timholy pushed a commit that referenced this pull request Feb 24, 2021
…#591)

There can be cases where a callback key is removed from
`user_callbacks_by_key` but it still remains in `user_callbacks_queue`.

The problem may be illustrated with the following MRE.

Say, with the following diff
```diff
diff --git a/src/callbacks.jl b/src/callbacks.jl
index bff5256..865bb24 100644
--- a/src/callbacks.jl
+++ b/src/callbacks.jl
@@ -159,6 +159,7 @@ function entr(f::Function, files, modules=nothing; 
all=false, postpone=false, pa
         sleep(pause)
         f()
     end
+    @info "added $(key)"
     try
         while true
             wait(revision_event)
@@ -168,7 +169,7 @@ function entr(f::Function, files, modules=nothing; 
all=false, postpone=false, pa
         isa(err, InterruptException) || rethrow(err)
     finally
         remove_callback(key)
+        @info "removed $(key)"
     end
     nothing
 end
-
```

and we run the following script:
> entr.jl
```julia
using Revise

const TARGET_FILE = "watched.jl"

let
    Revise.includet(TARGET_FILE)

    interrupted = false
    while !interrupted
        try
            entr([TARGET_FILE]) do
                # do something ...
            end
            interrupted = true
        catch err
            # handle expected errors, keep running

            # sync errors
            if isa(err, LoadError) ||
               (isa(err, ErrorException) && startswith(err.msg, 
"lowering returned an error")) ||
               isa(err, Revise.ReviseEvalException)
                @warn "handling sync error" err
                continue
            end

            # async errors
            if isa(err, CompositeException)
                errs = err.exceptions
                i = findfirst(e->isa(e, TaskFailedException), errs)
                if i !== nothing
                    tfe = errs[i]::TaskFailedException
                    res = tfe.task.result
                    if isa(res, LoadError) ||
                       (isa(res, ErrorException) && startswith(res.msg, 
"lowering returned an error")) ||
                       isa(res, Revise.ReviseEvalException)
                        @warn "handling async error" res
                        continue
                    end
                end
            end
            rethrow(err)
        end
    end
end
```

where `watched.jl` has something like below:
```julia
foo(a) = a # first state
# foo(a) = end # second state
```

Now, boot up REPL and `include("entr.jl")`, and then modify `watch.jl`
so that we alternately comment-in/out the lines of first/second state.
It will result in something like below on the current master:
```julia
julia> include("entr.jl")
[ Info: added ##245
[ Info: removed ##245
┌ Warning: handling sync error
│   err =
│    LoadError: "unexpected \"end\""
│    in expression starting at 
/Users/aviatesk/julia/packages/Revise/watched.jl:2
└ @ Main ~/julia/packages/Revise/entr.jl:22
[ Info: added ##246
[ Info: removed ##246
ERROR: LoadError: KeyError: key Symbol("##245") not found
Stacktrace:
  [1] getindex(h::Dict{Any, Any}, key::Symbol)
    @ Base ./dict.jl:482
  [2] macro expansion
    @ ~/julia/packages/Revise/src/callbacks.jl:116 [inlined]
  [3] macro expansion
    @ ./task.jl:382 [inlined]
  [4] process_user_callbacks!(keys::Set{Any}; throw::Bool)
    @ Revise ~/julia/packages/Revise/src/callbacks.jl:115
  [5] revise(; throw::Bool)
    @ Revise ~/julia/packages/Revise/src/packagedef.jl:816
  [6] entr(f::var"#3#5", files::Vector{String}, modules::Nothing; 
all::Bool, postpone::Bool, pause::Float64)
    @ Revise ~/julia/packages/Revise/src/callbacks.jl:166
  [7] entr(f::Function, files::Vector{String}, modules::Nothing)
    @ Revise ~/julia/packages/Revise/src/callbacks.jl:156
  [8] top-level scope
    @ ~/julia/packages/Revise/entr.jl:11
  [9] include(fname::String)
    @ Base.MainInclude ./client.jl:451
 [10] top-level scope
    @ REPL[1]:1
in expression starting at 
/Users/aviatesk/julia/packages/Revise/entr.jl:5
```
, which will be fixed with this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants