-
Notifications
You must be signed in to change notification settings - Fork 101
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
Add a PackageCompiler plugin #290
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #290 +/- ##
=======================================
Coverage 94.40% 94.41%
=======================================
Files 20 21 +1
Lines 608 627 +19
=======================================
+ Hits 574 592 +18
- Misses 34 35 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for opening this!
) | ||
|
||
Sets up sysimage generation via [PackageCompiler.jl](https://github.com/JuliaLang/PackageCompiler.jl). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does PackageCompiler.jl require a certain minimum Julia version?
If so, should we document that here? e.g. in a !!! note
block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the min version is 1.3.1
https://github.com/JuliaLang/PackageCompiler.jl/blob/master/Project.toml#L11
) | ||
|
||
""" | ||
PackageCompiler(; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to include this docstring in the docs, e.g. on docs/src/user.md
in the Misc
section
using PackageCompiler | ||
|
||
# List of packages to include in the sysimage | ||
{{#SYSIMAGE_DEPS}}packages = Symbol.(keys(Pkg.project().dependencies)) # or packages = [:Plots, :DataFrames] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think we can use this because Pkg.project().dependencies
is only defined in Julia v1.4+, and is not stable API anyway.
?Pkg.project
says:
This feature requires Julia 1.4, and is considered experimental.
And we want to support as many v1.x versions as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PkgDeps might be an alternative solution https://github.com/JuliaEcosystem/PkgDeps.jl (although the min Julia version there is 1.5)
make_jl::String = default_file("build", "make.jl") | ||
precompile_jl::String = default_file("build", "precompile.jl") | ||
sysimage_name::String = "sysimage" | ||
packages::Union{Symbol, AbstractVector} = :deps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
creat_sysimage
seems to require packages names be Symbol
, so let's just support that (https://julialang.github.io/PackageCompiler.jl/dev/refs/#PackageCompiler.create_sysimage)
packages::Union{Symbol, AbstractVector} = :deps | |
packages::Union{Symbol, Vector{Symbol}} = :deps |
``` | ||
# Explicitly list packages to include into the sysimage | ||
PackageCompiler(packages = [:Plots, :DataFrames]) | ||
PackageCompiler(packages = ["Plots", "DataFrames"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PackageCompiler(packages = ["Plots", "DataFrames"]) |
|
||
- `:deps`: include in the sysimage all direct dependencies of the package. | ||
- `:pkg`: include in the sysimage the package itself. | ||
- vector of package names, as strings or symbols: include all listed packages into the sysimage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- vector of package names, as strings or symbols: include all listed packages into the sysimage. | |
- `Vector{Symbol}` of package names: include all listed packages into the sysimage. |
included in the sysimage. Supported values are: | ||
|
||
- `:deps`: include in the sysimage all direct dependencies of the package. | ||
- `:pkg`: include in the sysimage the package itself. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- `:pkg`: include in the sysimage the package itself. | |
- `:pkg`: include in the sysimage only the package itself. |
The `packages` keyword argument allows specifying which packages should be | ||
included in the sysimage. Supported values are: | ||
|
||
- `:deps`: include in the sysimage all direct dependencies of the package. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we can robustly support this option... is there anything in the PackageCompiler docs on using a pattern like this?
This PR adds a
PackageCompiler
plugin which sets up sysimage generation usingPackageCompiler.jl
.This follows the same kind of setup as
Documenter
: abuild
subdirectory is created, that defines its own environment in whichPackageCompiler
isPkg.add
ed, and the main package isPkg.dev
ed.The user can choose between several options to determine the list of packages to be included in the sysimage:
I'm not sure whether this follows best practices, and I'm willing to make any change if something can be improved.