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

dont assume we can find source code for loaded packages #460

Merged
merged 2 commits into from
Apr 22, 2020

Conversation

KristofferC
Copy link
Collaborator

This can happen when Revise is put into a sysimage and it's dependencies are no longer in the Project/Manifest (as happens in JuliaLang/PackageCompiler.jl#391)

Copy link
Owner

@timholy timholy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Is there any feasible way to test this? I worry about regressions...

src/pkgs.jl Outdated Show resolved Hide resolved
@KristofferC
Copy link
Collaborator Author

For testing, you can force it with something like (assuming you have multiple precompile files stored for Example):


julia> using Revise

julia> using Example

julia> empty!(LOAD_PATH)
0-element Array{String,1}

julia> Revise.pkg_fileinfo(Base.PkgId(Example))
ERROR: MethodError: no method matching stale_cachefile(::Nothing, ::String)
Closest candidates are:
  stale_cachefile(::String, ::String) at loading.jl:1396
Stacktrace:
 [1] filter_valid_cachefiles(::Nothing, ::Array{String,1}) at /Users/kristoffercarlsson/.julia/packages/Revise/C272c/src/pkgs.jl:85
 [2] pkg_fileinfo(::Base.PkgId) at /Users/kristoffercarlsson/.julia/packages/Revise/C272c/src/pkgs.jl:98
 [3] top-level scope at REPL[5]:1
 [4] eval(::Module, ::Any) at ./boot.jl:331
 [5] eval_user_input(::Any, ::REPL.REPLBackend) at /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.4/REPL/src/REPL.jl:86
 [6] run_backend(::REPL.REPLBackend) at

but I am not sure how to cause it to happen in normal usage

@timholy
Copy link
Owner

timholy commented Apr 22, 2020

I guess we should just go ahead with this, since that test is pretty artificial and would presumably fail at a slightly later line.

@timholy timholy merged commit 7f57a53 into master Apr 22, 2020
@timholy timholy deleted the kc/pkgcompiler branch April 22, 2020 16:20
@christopher-dG
Copy link

Could we make a release with this commit? I am also dealing with the PackageCompiler issue 🙂

@timholy
Copy link
Owner

timholy commented Apr 22, 2020

I was hoping to add something else, but let's get this out as-is.

@christopher-dG
Copy link

Ah i see. I just wanted to make sure it wasn't going to get forgotten!

@timholy
Copy link
Owner

timholy commented Apr 23, 2020

Ugh, I should have read this more carefully. How on earth did the tests pass, I am really confused...

@timholy
Copy link
Owner

timholy commented Apr 23, 2020

https://travis-ci.org/github/timholy/Revise.jl/builds/677646144 claims it's the right commit, 15a561a, and shows all tests passing. Yet when I try it locally I get

Test Summary:                             | Pass  Error  Broken  Total
Revise                                    |  416      8       1    425
  PkgData                                 |    1                     1
  LineSkipping                            |    8                     8
  Equality and hashing                    |    5                     5
  Parse errors                            |    4                     4
  Signature extraction                    |    4                     4
  Comparison and line numbering           |   75              1     76
  Display                                 |    4                     4
  File paths                              |    6      1              7
  Base & stdlib file paths                |    5                     5
  Recursive types (issue #417)            |    1                     1
  Cross-module extension                  |    6                     6
  @__FILE__                               |    2                     2
  Module docstring                        |    6                     6
  Undef in docstrings                     |   72                    72
  Macro docstrings (issue #309)           |    4                     4
  Changing @inline annotations            |   16                    16
  Revising macros                         |    7                     7
  More arg-modifying macros               |    4                     4
  Line numbers                            |    5                     5
  Line numbers in backtraces and warnings |    6                     6
  New submodules                          |    3                     3
  Timing (issue #341)                     |    3                     3
  Method deletion                         |   58                    58
  Evaled toplevel                         |    4                     4
  Revision errors                         |   37                    37
  Retry on InterruptException             |   17                    17
  get_def                                 |    3      1              4
  Pkg exclusion                           |    3                     3
  Manual track                            |   17      1             18
  Auto-track user scripts                 |    4                     4
  Distributed                             |   15                    15
  Git                                     |    6                     6
  Recipes                                 |           1              1
  CodeTracking #48                        |    1                     1
  Methods at REPL                         |    2      4              6
  baremodule                              |    2                     2
ERROR: LoadError: Some tests did not pass: 416 passed, 0 failed, 8 errored, 1 broken.
in expression starting at /home/tim/.julia/dev/Revise/test/runtests.jl:71

as well as an error upon startup.

Explaining it as a weird Travis error also doesn't entirely make sense, because AppVeyor did the same thing: https://ci.appveyor.com/project/timholy/revise-jl/builds/32330643. Can anyone make sense of this?

timholy added a commit that referenced this pull request Apr 23, 2020
This is designed to catch issues like what happened in #460, but also 
more generally improve test coverage.
@timholy timholy mentioned this pull request Apr 23, 2020
timholy added a commit that referenced this pull request Apr 23, 2020
This is designed to catch issues like what happened in #460, but also 
more generally improve test coverage.
timholy added a commit that referenced this pull request Apr 24, 2020
This is designed to catch issues like what happened in #460, but also 
more generally improve test coverage.
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.

3 participants