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.

Sign up for GitHub

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

Jump to bottom

Fix #1852, relocatable changes for sysimage usage. #1853

Merged
merged 3 commits into from
Feb 1, 2022
Merged

Fix #1852, relocatable changes for sysimage usage. #1853

merged 3 commits into from
Feb 1, 2022

Conversation

MichaelHatherly
Copy link
Contributor

Uses RelocatableFolders to store all files that may be accessed
at runtime rather than baking in their file paths, which won't
point to valid file paths once a system image is created containing
Pluto and then moved to a different machine/system.

@MichaelHatherly
Fix fonsp#1852, relocatable changes for sysimage usage.
e1550a6 
Uses `RelocatableFolders` to store all files that may be accessed
at runtime rather than baking in their file paths, which won't
point to valid file paths once a system image is created containing
`Pluto` and then moved to a different machine/system.
@github-actions
Copy link
Contributor

Try this Pull Request!

Open Julia and type:

julia> import Pkg
julia> Pkg.activate(temp=true)
julia> Pkg.add(url="https://github.com/MichaelHatherly/Pluto.jl", rev="mh/relocatable")
julia> using Pluto

MichaelHatherly
MichaelHatherly commented Jan 25, 2022
View reviewed changes
src/Pluto.jl
root == "frontend" ? joinpath(FRONTEND_DIR, xs...) :
root == "sample" ? joinpath(SAMPLE_DIR, xs...) :
normpath(joinpath(dirname(dirname(pathof(Pluto))), root, xs...))
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These changes are slightly more complex than they could be if all assets that need this treatment were in a single root folder, but that change would have been much more invasive that just doing this.

MichaelHatherly
MichaelHatherly commented Jan 25, 2022
View reviewed changes
src/Pluto.jl
const ROOT_DIR = normpath(joinpath(@__DIR__, ".."))
const FRONTEND_DIR = @path(joinpath(ROOT_DIR, "frontend"))
const FRONTEND_DIST_DIR = let dir = joinpath(ROOT_DIR, "frontend-dist")
isdir(dir) ? @path(dir) : FRONTEND_DIR
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not 100% sure on the frontend and frontend-dist stuff, hopefully this logic is actually doing the correct thing.

MichaelHatherly
MichaelHatherly commented Jan 25, 2022
View reviewed changes
src/evaluation/WorkspaceManager.jl
@@ -22,9 +22,9 @@ Base.@kwdef mutable struct Workspace
end

"These expressions get evaluated whenever a new `Workspace` process is created."
const process_preamble = quote
process_preamble() = quote
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needed to change to a function such that the path could be queried at runtime rather than at using-time which would pick the wrong path to include.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, I just was planning to send in a PR to change this to a function too because it now allows users to override the preamble 🤣 What a coincidence. Thanks!

@MichaelHatherly
Copy link
Contributor Author

Testing of relocatablility for these changes is a case of doing the following steps:

  • make a system image using create_sysimage
  • delete/move the folders that need to be available
  • start julia with the system image
  • check that pluto still runs.

@dralletje
Copy link
Collaborator

👏🏽 👏🏽 👏🏽

@fonsp fonsp linked an issue Jan 26, 2022 that may be closed by this pull request
Use RelocatableFolders.jl instead of project_relative_dir to allow sysimages #1852
Closed
@dralletje dralletje merged commit 2d7e783 into fonsp:main Feb 1, 2022
@fonsp
Copy link
Owner

fonsp commented Feb 2, 2022

😅 I was going to say: This adds about 1 second of loading time on my computer because it reads and hashes all the files. But yes probably better to do a yolo merge

@fonsp
Copy link
Owner

fonsp commented Feb 2, 2022

#1884 would help

@dralletje
Copy link
Collaborator

dralletje commented Feb 2, 2022 •
edited
Loading

>_<

@dralletje
Copy link
Collaborator

@MichaelHatherly way to only load the files into memory when running with PackageCompiler?

@MichaelHatherly
Copy link
Contributor Author

This adds about 1 second of loading time on my computer because it reads and hashes all the files.

Just on precompile, or every using Pluto? Either is pretty bad, we should try mitigate that.

way to only load the files into memory when running with PackageCompiler?

May be a way to, would probably be best built into RelocatableFolders itself to be able to skip when PackageCompiler isn't what is causing the precomp, I'll take a look at it.

pankgeorg added a commit that referenced this pull request Feb 8, 2022
@pankgeorg
Merge tag 'v0.18.0' into what-if-distrubuted2
3436275 
> Try this release in your browser! _(Available 30 minutes after the release)_
>
> <a href="https://binder.plutojl.org/v0.18.0/"><img alt="Binder logo" src="https://mybinder.org/badge_logo.svg"></a>

**Generate your own binder links using [pluto-on-binder.glitch.me](https://pluto-on-binder.glitch.me/)!**

[Diff since v0.17.7](v0.17.7...v0.18.0)

**Merged pull requests:**
- Make document currentscript work (#1765) (@dralletje)
- don't count logs messages multiple times for pluto-logs width (#1843) (@Pangoraw)
- Pattern matching system for lezer code (#1849) (@dralletje)
- minor fix: Tables integration caused method ambiguation for Union{}[] (#1851) (@16bitmood)
- Fix #1852, relocatable changes for sysimage usage. (#1853) (@MichaelHatherly)
- Re-enable parseMixed (#1855) (@pankgeorg)
- DOI in REAMDE for citation (#1864) (@rikhuijzer)
- Use pkgdir instead of pathof (#1867) (@rikhuijzer)
- Hide banner in CI (#1868) (@rikhuijzer)
- Server-side events as configuration (#1871) (@ctrekker)
- fix is_stdlib() for nightly and enable nightly tests (#1874) (@Pangoraw)
- Awesome! Line wrapping! (#1876) (@dralletje)
- respect maxlog keyword for logs, fixes #1858 (#1877) (@Pangoraw)
- allow .. as an identifier, fixes #1875 (#1878) (@Pangoraw)
- Server start event (#1882) (@ctrekker)
- Julia 1.6 minimum (#1885) (@fonsp)
- New heuristic to skip intermediate bond values (#1892) (@fonsp)
- Send queued bond changes when last script finishes (#1898) (@fonsp)

**Closed issues:**
- Pkg.jl will corrupt itself on Windows (#671)
- Failure creating JuMP DenseAxisArray (#806)
- Frontend tests fail too often (#1234)
- Open multiple notebooks at start and without browser (#1278)
- Segmentation faults on Julia 1.7 M1 (#1722)
- Table of content overlaps text in static HTML (#1770)
- Pluto hangs when cell contains `(md" ", md"$t")` (#1800)
- Cell drag drop broken in Linux Chrome v97 (#1809)
- Tables integration causes method ambiguities for `pluto_showable` (#1815)
- Segfault with CSV, SQLite, tables (M1) (#1842)
- Use RelocatableFolders.jl instead of project_relative_dir to allow sysimages (#1852)
- Logging doesn't respect `maxlog` (#1858)
- TOC in dark mode is still light when aside (#1860)
- ProgressLogging: suppressed logging in Turing.jl (#1862)
- Cannot update to version 0.17.7 (#1869)
- Stdlib packages marked as not found (#1872)
- EllipsisNotation.jl (`randn((10,10))[1, ..]`) does not work (#1875)
- Too many intermediate bond values (#1891)
pankgeorg added a commit that referenced this pull request Feb 8, 2022
@pankgeorg
Merge tag 'v0.18.0' into pluto-events-interface-0.18
8eb5476 
> Try this release in your browser! _(Available 30 minutes after the release)_
>
> <a href="https://binder.plutojl.org/v0.18.0/"><img alt="Binder logo" src="https://mybinder.org/badge_logo.svg"></a>

**Generate your own binder links using [pluto-on-binder.glitch.me](https://pluto-on-binder.glitch.me/)!**

[Diff since v0.17.7](v0.17.7...v0.18.0)

**Merged pull requests:**
- Make document currentscript work (#1765) (@dralletje)
- don't count logs messages multiple times for pluto-logs width (#1843) (@Pangoraw)
- Pattern matching system for lezer code (#1849) (@dralletje)
- minor fix: Tables integration caused method ambiguation for Union{}[] (#1851) (@16bitmood)
- Fix #1852, relocatable changes for sysimage usage. (#1853) (@MichaelHatherly)
- Re-enable parseMixed (#1855) (@pankgeorg)
- DOI in REAMDE for citation (#1864) (@rikhuijzer)
- Use pkgdir instead of pathof (#1867) (@rikhuijzer)
- Hide banner in CI (#1868) (@rikhuijzer)
- Server-side events as configuration (#1871) (@ctrekker)
- fix is_stdlib() for nightly and enable nightly tests (#1874) (@Pangoraw)
- Awesome! Line wrapping! (#1876) (@dralletje)
- respect maxlog keyword for logs, fixes #1858 (#1877) (@Pangoraw)
- allow .. as an identifier, fixes #1875 (#1878) (@Pangoraw)
- Server start event (#1882) (@ctrekker)
- Julia 1.6 minimum (#1885) (@fonsp)
- New heuristic to skip intermediate bond values (#1892) (@fonsp)
- Send queued bond changes when last script finishes (#1898) (@fonsp)

**Closed issues:**
- Pkg.jl will corrupt itself on Windows (#671)
- Failure creating JuMP DenseAxisArray (#806)
- Frontend tests fail too often (#1234)
- Open multiple notebooks at start and without browser (#1278)
- Segmentation faults on Julia 1.7 M1 (#1722)
- Table of content overlaps text in static HTML (#1770)
- Pluto hangs when cell contains `(md" ", md"$t")` (#1800)
- Cell drag drop broken in Linux Chrome v97 (#1809)
- Tables integration causes method ambiguities for `pluto_showable` (#1815)
- Segfault with CSV, SQLite, tables (M1) (#1842)
- Use RelocatableFolders.jl instead of project_relative_dir to allow sysimages (#1852)
- Logging doesn't respect `maxlog` (#1858)
- TOC in dark mode is still light when aside (#1860)
- ProgressLogging: suppressed logging in Turing.jl (#1862)
- Cannot update to version 0.17.7 (#1869)
- Stdlib packages marked as not found (#1872)
- EllipsisNotation.jl (`randn((10,10))[1, ..]`) does not work (#1875)
- Too many intermediate bond values (#1891)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rikhuijzer rikhuijzer rikhuijzer left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use RelocatableFolders.jl instead of project_relative_dir to allow sysimages
4 participants
@MichaelHatherly @dralletje @fonsp @rikhuijzer