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

Problem with loading optional dependencies #52

Closed
tlnagy opened this issue Jul 29, 2018 · 8 comments
Closed

Problem with loading optional dependencies #52

tlnagy opened this issue Jul 29, 2018 · 8 comments

Comments

@tlnagy
Copy link

tlnagy commented Jul 29, 2018

I'm updating Compose.jl in GiovineItalia/Compose.jl#282 to use Requires.jl to allow for loading of optional backends at runtime instead of the error-prone precompilation time approach that we're currently using.

I'm running into a problem that appears to be related to #48, but has the extra rub of importing an optional dependencies that is not in Compose's dependencies. This seems to trigger issues with Pkg.resolve too. It's a little difficult building a minimal example since the Pkg.resolve error doesn't happen unless you're editing the package file in ~/.julia/dev/Compose

               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: https://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.7.0-beta2.0 (2018-07-13 19:54 UTC)
 _/ |\__'_|_|_|\__'_|  |  Official http://julialang.org/ release
|__/                   |  x86_64-pc-linux-gnu

julia> using Cairo

julia> using Compose
[ Info: Loading Cairo backend into Compose.jl
┌ Warning: Error requiring Cairo from Compose:
│ LoadError: ArgumentError: Package Compose does not have Cairo in its dependencies:
│  - If you have Compose checked out for development and have
│    added Cairo as a dependency but haven't updated your primary
│    environment's manifest file, try `Pkg.resolve()`.
│  - Otherwise you may need to report an issue with Compose.

│ Stacktrace:
│  [1] require(::Module, ::Symbol) at ./loading.jl:821
│  [2] include at ./boot.jl:317 [inlined]
│  [3] include_relative(::Module, ::String) at ./loading.jl:1034
│  [4] include at ./sysimg.jl:29 [inlined]
│  [5] include at /home/tamas/.julia/dev/Compose/src/Compose.jl:3 [inlined]
│  [6] macro expansion at ./logging.jl:295 [inlined]
│  [7] link_cairo() at /home/tamas/.julia/dev/Compose/src/Compose.jl:167
│  [8] top-level scope at none:0
│  [9] eval at ./boot.jl:319 [inlined]
│  [10] eval at /home/tamas/.julia/dev/Compose/src/Compose.jl:3 [inlined]
│  [11] (::getfield(Compose, Symbol("##112#118")))() at /home/tamas/.julia/dev/Requires/src/require.jl:67
│  [12] err(::getfield(Compose, Symbol("##112#118")), ::Module, ::String) at /home/tamas/.julia/dev/Requires/src/require.jl:38
│  [13] #111 at /home/tamas/.julia/dev/Requires/src/require.jl:66 [inlined]
│  [14] withpath(::getfield(Compose, Symbol("##111#117")), ::String) at /home/tamas/.julia/dev/Requires/src/require.jl:28
│  [15] #110 at /home/tamas/.julia/dev/Requires/src/require.jl:65 [inlined]
│  [16] listenpkg(::getfield(Compose, Symbol("##110#116")), ::Base.PkgId) at /home/tamas/.julia/dev/Requires/src/require.jl:13
│  [17] __init__() at /home/tamas/.julia/dev/Requires/src/require.jl:64
│  [18] _include_from_serialized(::String, ::Array{Any,1}) at ./loading.jl:600
│  [19] macro expansion at ./logging.jl:300 [inlined]
│  [20] _require_search_from_serialized(::Base.PkgId, ::String) at ./loading.jl:682
│  [21] _require(::Base.PkgId) at ./loading.jl:919
│  [22] require(::Base.PkgId) at ./loading.jl:838
│  [23] require(::Module, ::Symbol) at ./loading.jl:833
│  [24] eval(::Module, ::Any) at ./boot.jl:319
│  [25] eval_user_input(::Any, ::REPL.REPLBackend) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v0.7/REPL/src/REPL.jl:85
│  [26] macro expansion at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v0.7/REPL/src/REPL.jl:116 [inlined]
│  [27] (::getfield(REPL, Symbol("##28#29")){REPL.REPLBackend})() at ./task.jl:262
│ in expression starting at /home/tamas/.julia/dev/Compose/src/cairo_backends.jl:4
└ @ Requires ~/.julia/dev/Requires/src/require.jl:40
@tlnagy
Copy link
Author

tlnagy commented Jul 29, 2018

Here's the commit where I'm trying to get the optional dependency stuff working: GiovineItalia/Compose.jl@4e20db6

The key bit:

function link_cairo()
    @info "Loading Cairo backend into Compose.jl"
    include("cairo_backends.jl")
end

function __init__()
    @require Cairo="159f3aea-2a34-519c-b102-8c37f9878175" link_cairo()
end

@tlnagy
Copy link
Author

tlnagy commented Jul 31, 2018

Thoughts @timholy @MikeInnes @jmert?

@MikeInnes
Copy link
Collaborator

You probably have a using Cairo or import Cairo somewhere. Because of the new package loading you'll have to change that to using .Cairo or similar.

Note that Requires is not really designed for this kind of use case. It'll mean that users will have to explicitly do using Cairo, and Gadfly's behaviour will then change. Hopefully Pkg3 will eventually support this kind of thing.

@tlnagy
Copy link
Author

tlnagy commented Aug 1, 2018

Currently we rely on detecting whether Cairo is installed at compile-time which is pretty fragile (and the code to do that is broken on v0.7). We have to tell the user to manual delete the precompiled file to force recompilation of Compose.jl after they install Cairo.jl or Fontconfig.jl and if they remove either one after Compose.jl is compiled ¯_(ツ)_/¯

It might be better to force the user to explicitly import Cairo if they want to do PNG or PDF output and load the Cairo-hooks then. This is not as convenient as the current pseudo-magical approach, but should transition nicely when Pkg3 adds this ability in the future and would make Compose.jl considerably more robust.

@bjarthur, thoughts?

Also, @vtjnash since you originally suggested switching over to Requires.jl

@bjarthur
Copy link

bjarthur commented Aug 1, 2018

i have no problem with instructing the user to import Cairo.

@bjarthur
Copy link

@tlnagy we've resolved this, right? if so we should close this issue.

@tlnagy
Copy link
Author

tlnagy commented Oct 12, 2018

Yeah, it would be nice if users could do using Cairo instead of import Cairo, but that works in the mean time.

@tlnagy tlnagy closed this as completed Oct 12, 2018
@bjarthur
Copy link

@tlnagy i don't understand. are you saying users of Compose.jl have to import Cairo if they want PDF, PNG, etc., and can not using Cairo? both seem to work for me.

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

No branches or pull requests

3 participants