-
Notifications
You must be signed in to change notification settings - Fork 105
Add a PackageCompiler plugin #290
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
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.
|
nickrobinson251
left a comment
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
PackageCompilerplugin which sets up sysimage generation usingPackageCompiler.jl.This follows the same kind of setup as
Documenter: abuildsubdirectory is created, that defines its own environment in whichPackageCompilerisPkg.added, and the main package isPkg.deved.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.