Problem with new version with packages that load other packages

[davidanthoff]

Ok, now for a real problem with version v0.5.1 that has the @timholy rewrite.

I have a package that I stripped down to:

__precompile__()
module IterableTables

using Requires

function __init__()
    @info("INIT CALLED")
    @require DataFrames="a93c6f00-e57d-5684-b7b6-d8193f3e46c0" println("WORKED")    
end

end # module

When I load it, I get:

               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: https://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.7.0-beta.238 (2018-07-10 21:07 UTC)
 _/ |\__'_|_|_|\__'_|  |  Commit 5d1a3fcba1* (0 days old master)
|__/                   |  x86_64-w64-mingw32

julia> using DataFrames

julia> using IterableTables
[ Info: INIT CALLED
[ Info: Precompiling module DataFrames
ERROR: LoadError: ArgumentError: Package Reexport not found in current path:
 - Run `Pkg.add("Reexport")` to install the Reexport package.

Stacktrace:
 [1] require(::Module, ::Symbol) at .\loading.jl:816
 [2] include at .\boot.jl:317 [inlined]
 [3] include_relative(::Module, ::String) at .\loading.jl:1034
 [4] include(::Module, ::String) at .\sysimg.jl:29
 [5] top-level scope at none:0
 [6] eval at .\boot.jl:319 [inlined]
 [7] eval(::Expr) at .\client.jl:394
 [8] top-level scope at .\none:3 [inlined]
 [9] top-level scope at .\<missing>:0
in expression starting at C:\Users\david\.julia\packages\DataFrames\Pv1t\src\DataFrames.jl:10
┌ Warning: Error requiring DataFrames from IterableTables:
│ Failed to precompile DataFrames to C:\Users\david\.julia\compiled\v0.7\DataFrames.ji.
│ Stacktrace:
│  [1] error(::String) at .\error.jl:33
│  [2] macro expansion at .\logging.jl:298 [inlined]
│  [3] compilecache(::Base.PkgId) at .\loading.jl:1169
│  [4] _require(::Base.PkgId) at .\loading.jl:942
│  [5] require(::Base.PkgId) at .\loading.jl:838
│  [6] top-level scope at none:0
│  [7] eval at .\boot.jl:319 [inlined]
│  [8] eval at C:\Users\david\.julia\dev\IterableTables\src\IterableTables.jl:2 [inlined]
│  [9] (::getfield(IterableTables, Symbol("##3#6")))() at C:\Users\david\.julia\packages\Requires\uXN8\src\require.jl:67
│  [10] err(::getfield(IterableTables, Symbol("##3#6")), ::Module, ::String) at C:\Users\david\.julia\packages\Requires\uXN8\src\require.jl:38
│  [11] #2 at C:\Users\david\.julia\packages\Requires\uXN8\src\require.jl:66 [inlined]
│  [12] withpath(::getfield(IterableTables, Symbol("##2#5")), ::String) at C:\Users\david\.julia\packages\Requires\uXN8\src\require.jl:28
│  [13] #1 at C:\Users\david\.julia\packages\Requires\uXN8\src\require.jl:65 [inlined]
│  [14] listenpkg(::getfield(IterableTables, Symbol("##1#4")), ::Base.PkgId) at C:\Users\david\.julia\packages\Requires\uXN8\src\require.jl:13
│  [15] __init__() at .\logging.jl:297
│  [16] _include_from_serialized(::String, ::Array{Any,1}) at .\loading.jl:600
│  [17] macro expansion at .\logging.jl:300 [inlined]
│  [18] _require_search_from_serialized(::Base.PkgId, ::String) at .\loading.jl:682
│  [19] _require(::Base.PkgId) at .\loading.jl:919
│  [20] require(::Base.PkgId) at .\loading.jl:838
│  [21] require(::Module, ::Symbol) at .\loading.jl:833
│  [22] eval(::Module, ::Any) at .\boot.jl:319
│  [23] eval_user_input(::Any, ::REPL.REPLBackend) at C:\cygwin\home\Administrator\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v0.7\REPL\src\REPL.jl:85
│  [24] macro expansion at C:\cygwin\home\Administrator\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v0.7\REPL\src\REPL.jl:116 [inlined]
│  [25] (::getfield(REPL, Symbol("##28#29")){REPL.REPLBackend})() at .\task.jl:257
└ @ Requires C:\Users\david\.julia\packages\Requires\uXN8\src\require.jl:40

julia>

If I then add Reexport to my global environment, I get an error that StatsBase is not found. StatsBase is the second package that is being used from DataFrames.jl.

So I guess it looks as if a package A that is being required by package B won't properly load other packages that are in the REQUIRE file for A.

[timholy]

Thanks! I am short on time, so let me just say it seems to work if you comment out this line. @vtjnash predicted this and proposed an alternative solution (https://github.com/MikeInnes/Requires.jl/pull/46#issuecomment-403954649).

[timholy]

In the short term, rather than a full glue package I suspect this might work:

@require DataFrames="a93c6f00-e57d-5684-b7b6-d8193f3e46c0" include("extras_dataframes.jl")

where extras_dataframes.jl lives in the same src and looks something like

using DataFrames

function mycoolfunction(df::DataFrame, ...)
    body
end

But if you wanted that code precompiled you'd definitely want the auxillary module as proposed by @vtjnash.

[jmert]

I don't understand why the original doesn't work, but the following patch seems to solve the problem in a similar usage case.

diff --git a/src/require.jl b/src/require.jl
index e402421..93391bd 100644
--- a/src/require.jl
+++ b/src/require.jl
@@ -58,14 +58,14 @@ macro require(pkg, expr)
     return Expr(:macrocall, Symbol("@warn"), __source__,
                 "Requires now needs a UUID; please see the readme for changes in 0.7.")
   id, modname = parsepkg(pkg)
-  pkg = Base.PkgId(Base.UUID(id), modname)
+  pkg = :(Base.PkgId(Base.UUID($id), $modname))
   quote
     if !isprecompiling()
-      listenpkg(Base.PkgId(Base.UUID($id), $modname)) do
+      listenpkg($pkg) do
         withpath($(string(__source__.file))) do
-          err($__module__, $(pkg.name)) do
+          err($__module__, $modname) do
             $(esc(:(eval($(Expr(:quote, Expr(:block,
-                                            :(const $(Symbol(pkg.name)) = Base.require($pkg)),
+                                            :(const $(Symbol(modname)) = Base.require($pkg)),
                                             expr)))))))
           end
         end

[vtjnash]

It would be better for compile times (and memory usage, and readability, and debugging / backtraces), if we could make this macro expand to a simple function call, rather than a dense nest of new function definitions:

macro requires(pkg, expr)
  id, modname = parsepkg(pkg)
  return :(do_requires($__module__, $(QuoteNode(__source__.file)), $(QuoteNode(id)), $(QuoteNode(modname)), $(QuoteNode(expr))))
end

function do_requires(m::Module, this_file, id, modname, expr)
    pkg = Base.PkgId(Base.UUID(id), modname))
    if !isprecompiling()
        listenpkg(pkg) do
            withpath(string(thisfile)) do
                err(m, modname) do
                    Core.eval(m, quote
                        const $(Symbol(modname)) = $(Base.require(pkg))
                        $expr
                    end)
                    nothing
                end
                nothing
           end
           nothing
        end
        nothing
    end
    nothing
end

[MikeInnes]

@jmert that's very odd; can you send a PR and @davidanthoff can you confirm if that patch fixes things for you?

I unfortunately can't reproduce this (on 0.7-beta) – maybe something has changed, it'd be good to verify.

[jmert]

@jmert that's very odd; can you send a PR and @davidanthoff can you confirm if that patch fixes things for you?

Will do. PR coming shortly.