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

julia_project.py: fix Pkg REPL api warning #1376

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

fonsp
Copy link
Contributor

@fonsp fonsp commented Oct 28, 2024

Repo2docker currently prints this warning when using a Julia project:

Warning: The Pkg REPL mode is intended for interactive use only, and should not be used from scripts. It is recommended to use the functional API instead.
└ @ Pkg.REPLMode /srv/julia/share/julia/stdlib/v1.10/Pkg/src/REPLMode/REPLMode.jl:382
�[0m  at 12:24:22

This PR fixes that. I also used Pkg.instantiate() instead of Pkg.precompile() because it makes a bit more sense in this context: we want to guarantee that packages are instantiated (which precompiles at the end), not just that they are precompiled: https://pkgdocs.julialang.org/v1/api/#Pkg.instantiate

I believe that resolve already instantiates (and precompiles), but this is not defined in spec so it is good to add the instantiate call.

@agoose77
Copy link

agoose77 commented Dec 3, 2024

Hi @fonsp, thanks for your contribution!

@GeorgianaElena and I took a look at this PR today, and confirmed that the call to instantiate() eliminates the console warning.

As we were unfamiliar with the history of these lines of code, we investigated whether the instantiate(); resolve(); instantiate() chain was strictly necessary, and started to explore this. As we time-boxed our work on this PR, we ultimately decided that further exploring was out of scope here. And we will follow up with an issue.

So, approving & merging this PR!

@fonsp
Copy link
Contributor Author

fonsp commented Dec 4, 2024

Thanks for the response!

I believe #1383 is nice! It should save about 1sec from the startup time. From my Julia experience (not a Pkg developer but heavy user): The extra resolve call is not necessary, unless the repo contains an improperly resolved Manifest (very rare, might happen when a repository contains a subfolder that is added as dependency, and something changed in the subfolder without the user updating their main Manifest). If so, there are some cases that will get fixed by instantiate, and others that will fail and show a nice error message in the binder build log, which is fine! (It will encourage people to just fix their Manifest.)

While in the Julia mood, maybe you could take a look at #1375? This should help with stability and reproducibility of Julia projects!

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.

3 participants