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

Make Pluto optional #138

Merged
merged 6 commits into from
Feb 7, 2023
Merged

Make Pluto optional #138

merged 6 commits into from
Feb 7, 2023

Conversation

Dsantra92
Copy link
Collaborator

Introduce Requires and move Pluto dependency to optional. Close #130

@johnnychen94
Copy link
Member

johnnychen94 commented Jan 31, 2023

Since DemoCards mainly runs on CI(where latency isn't a big deal), Requires is an okay solution already.
But if you'd like to take a try, https://pkgdocs.julialang.org/dev/creating-packages/#Conditional-loading-of-code-in-packages-(Extensions) is a good "official" solution. An extra benefit using Extensions is that we can manage Pluto*'s compatibility.

Project.toml Show resolved Hide resolved
@Dsantra92
Copy link
Collaborator Author

Faced an interesting problem while following the official example

if !isdefined(Base, :get_extension)
using Requires
# print("Require import block reached")
# end

# if !isdefined(Base, :get_extension)
# print("Require __init__ block reached")
function __init__()
    @require Pluto="c3e4b0f8-55cb-11ea-2926-15256bba5781" begin
      @require PlutoStaticHTML = "359b1769-a58e-495b-9770-312e911026ad" include("../ext/PlutoNotebook.jl")
    end
end
end

Results in an error
ERROR: LoadError: UndefVarError: `@require` not defined

StackTrace
Stacktrace:
 [1] top-level scope
   @ :0
 [2] include
   @ ./Base.jl:456 [inlined]
 [3] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt128}}, source::Nothing)
   @ Base ./loading.jl:1952
 [4] top-level scope
   @ stdin:2
in expression starting at /home/dsantra/DemoCards.jl/src/DemoCards.jl:60
in expression starting at /home/dsantra/DemoCards.jl/src/DemoCards.jl:1
in expression starting at stdin:2
ERROR: Failed to precompile DemoCards [311a05b2-6137-4a5a-b473-18580a3d38b5] to "/home/dsantra/.julia/compiled/v1.9/DemoCards/jl_j7FQ0Y".
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] compilecache(pkg::Base.PkgId, path::String, internal_stderr::IO, internal_stdout::IO, keep_loaded_modules::Bool)
   @ Base ./loading.jl:2195
 [3] compilecache
   @ ./loading.jl:2068 [inlined]
 [4] _require(pkg::Base.PkgId, env::String)
   @ Base ./loading.jl:1712
 [5] _require_prelocked(uuidkey::Base.PkgId, env::String)
   @ Base ./loading.jl:1567
 [6] macro expansion
   @ ./loading.jl:1555 [inlined]
 [7] macro expansion
   @ ./lock.jl:267 [inlined]
 [8] require(into::Module, mod::Symbol)
   @ Base ./loading.jl:1518

Surprisingly works with comments removed, i.e. requires loaded as a separate if else blocks.

if !isdefined(Base, :get_extension)
using Requires
print("Require import block reached")
end

if !isdefined(Base, :get_extension)
print("Require __init__ block reached")
function __init__()
    @require Pluto="c3e4b0f8-55cb-11ea-2926-15256bba5781" begin
      @require PlutoStaticHTML = "359b1769-a58e-495b-9770-312e911026ad" include("../ext/PlutoNotebook.jl")
    end
end
end
julia> using DemoCards
[ Info: Precompiling DemoCards [311a05b2-6137-4a5a-b473-18580a3d38b5]
Democards dir
Require import block reached
Require __init__ block reached

Any idea why this is happening? Similar problems are happening for julia 1.9 beta3.

@johnnychen94
Copy link
Member

I'm not sure. It's perhaps because macros are expanded first?

@Dsantra92
Copy link
Collaborator Author

I decided to leave PlutoDemoCard in the src folder. It might not make sense, now that we are loading Pluto Notebook as an extension. But I didn't want to change the original democard function:

function democard(path::String)::AbstractDemoCard
validate_file(path)
_, ext = splitext(path)
if ext in markdown_exts
return MarkdownDemoCard(path)
elseif ext in julia_exts
if is_pluto_notebook(path)
return PlutoDemoCard(path)
else
return JuliaDemoCard(path)
end
else
return UnmatchedCard(path)
end
end

src/types/pluto.jl Outdated Show resolved Hide resolved
Copy link
Member

@johnnychen94 johnnychen94 left a comment

Choose a reason for hiding this comment

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

Once the incremental compilation issue is fixed I think it's good to go!

@Dsantra92 Dsantra92 merged commit 0d72ce2 into JuliaDocs:master Feb 7, 2023
@t-bltg
Copy link
Collaborator

t-bltg commented Feb 7, 2023

Awesome, thanks !

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.

Make Pluto dependency optional?
3 participants