-
Notifications
You must be signed in to change notification settings - Fork 0
Add pkg-precompile.jl
script
#1
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
Conversation
I'll be doing a one test of all patch releases of Julia from 1.10.0 - 1.10.9, 1.11.0 - 1.11.4 to ensure our use of internal code doesn't break with any of these versions. |
Found a corner case I need to add tests for. In most cases the Project.toml may contain a |
@@ -0,0 +1,338 @@ | |||
#!/usr/bin/env -S julia --color=yes |
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.
Maybe we should just call this precompile.jl
?
# julia-container-scripts |
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.
Maybe julia-container-build-scripts
or julia-docker-build-scripts
is a better name?
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 think this is clear enough as far as I'm concerned.
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 review the precompile script carefully and skimmed everything else. LGTM with some very minor comments/suggestions (I think just one typo?)
My main question is I guess a high level one: what does this give us over what we have been doing? pkg-precompile.jl is a pretty complex piece of code and hooks into what seem like internals of julia (although I haven't looked carefully at how internal they really are...). This is a public repo so I won't link to other internal stuff but basically this seems like it's replacing a pretty innocuous script that comes down to having one step like
# Install Julia package registries, instantiate the Julia project environment, and
# precompile Julia dependencies. We need to add the Julia registries in the same Docker
# layer which copies the Project.toml/Manifest.toml to ensure this step works in
# environments where we use Docker layer caching with a fresh mount cache.
COPY *Project.toml *Manifest.toml /tmp/
RUN --mount=type=cache,target=${JULIA_USER_DEPOT_CACHE},sharing=locked \
--mount=type=secret,id=github-token \
# Create the required project source file
mkdir -p "${JULIA_PROJECT}/src" && \
touch "${JULIA_PROJECT}/src/$(basename "${JULIA_PROJECT}").jl" && \
# Move the Project.toml and Manifest.toml potentially into the depot cache
mv /tmp/*Project.toml /tmp/*Manifest.toml "${JULIA_PROJECT}" && \
julia --project="${JULIA_PROJECT}" -e ' \
using Pkg; \
# Only use `Pkg.Registry.add` when necessary has this always downloads the entire registry. \
if isempty(Pkg.Registry.reachable_registries()) \
Pkg.Registry.add([RegistrySpec(name="Beacon", url="https://github.com/beacon-biosignals/BeaconRegistry.git"), \
RegistrySpec(name="General")]); \
else \
Pkg.Registry.update(); \
end; \
Pkg.instantiate(); \
Pkg.build(); \
Pkg.precompile(strict=true, timing=true)'
we're not really taking advantage of the relocatable precompilation (since this still works on 1.10), and it seems like moving the precompilation and final stage into a single stage is the only significant simplification this makes for our dockerfiles. but maybe there's something I'm missing??
# julia-container-scripts |
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 think this is clear enough as far as I'm concerned.
# instantiating a named Julia project. |
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.
1.10.8-1.10 require this for precompilation too.
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 determined the versions which required this from using the test suite. We are testing for this in with the GEN_PKG_SRC
Docker ARG
which is only enabled on these Julia versions where it is required.
for ext in keys(manifest[pkg.uuid].exts) | ||
# The extension UUID deterministic and based upon the parent UUID and the | ||
# extension name. e.g. https://github.com/JuliaLang/julia/blob/2fd6db2e2b96057dbfa15ee651958e03ca5ce0d9/base/loading.jl#L1561 | ||
# Note: the `Base.uuid5` implementation differs from `UUIDs.uuid5` |
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.
:AHHHHH:
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.
Yeah, this is downright dumb. We have tests for extensions so if this were to change the tests should pick up on that.
Co-authored-by: Dave Kleinschmidt <[email protected]>
Co-authored-by: Dave Kleinschmidt <[email protected]>
The Docker snippet you posted is the new revised instantiate step from the pattern we proposed in https://discourse.julialang.org/t/proposed-julia-docker-workflow-to-use-a-persistent-depot/97963. From the discussion we had with @ericphanson the issue with our Dockerfile pattern is that it requires multiple steps to function correctly and is complex enough that it can easily be broken by users extending the pattern to fit their own needs. The primary advantage the new Some of the more recent changes like using fixed modification timestamps and
As we don't mainly move our Julia depot the only advantage the relocatable changes provide us in Julia 1.11 is that
The advantages brought by the
The disadvantages are:
|
I've been looking at setting the fixed modification timestamp inside the Julia script to make things more convenient for users. At first I was worried that a Julia For example if we instantiate and use
If we instead just instantiate in one layer and then set the fixed modification times and precompile in another layer we see the instantiate layer remain the same size but have precompile increase to 216MB.
The difference here is that the first layer records one set of timestamps while the later changes the timestamps in another layer. |
Maybe we should provide a third script to do the fixed modification times? I'm considering making an all-in-one installer to simplify things further. Maybe we should just provide an I'll keep things simple for now. |
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 do think having a one-stop instantiate.jl script would be useful here. Otherwise we're back in a similar boat of needing to do some careful juggling of various non-trivial steps. But I'm okay with trying this out and seeing if the juice is worth the squeeze.
Creates a script to assist with precompiling Julia packages within a build container. This script is specifically compatible with build cache mounts which allow
.ji
files to be shared between builds for decreased build times.