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

Reimplement escript archives #9386

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lucioleKi
Copy link
Contributor

The support for zip archives for packaging Erlang applications is an experimental features, and as such can be subject to incompatible changes.

The support for archives in the code server and the erl_prim_loader module complicates the implementation and make code loading slower even when no archives are used. Furthermore, the existence of the feature prevents or vatstly complicates further optimization of code loading.

Therefore, this commit removes the support for code archives from the code server, and from the code and erl_prim_loader modules.

Having multiple BEAM files and other files in an escript is still supported, but in an incompatible way for escripts created by Erlang/OTP 28. We decided to do it like this to keep escript:create/2 and escript:extract/2 as simple as possible, and make it possible to phase out the old archive format as soon as possible.

However, escripts created by Erlang/OTP 27 and earlier will run in Erlang/OTP 28, provided that they don't rely on archive features in the code server or erl_prim_loader.

@lucioleKi lucioleKi added team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI labels Feb 4, 2025
@lucioleKi lucioleKi requested a review from jhogberg February 4, 2025 13:06
@lucioleKi lucioleKi self-assigned this Feb 4, 2025
Copy link
Contributor

github-actions bot commented Feb 4, 2025

CT Test Results

    8 files    303 suites   3h 5m 21s ⏱️
5 263 tests 4 623 ✅ 638 💤 2 ❌
6 676 runs  5 937 ✅ 736 💤 3 ❌

For more details on these failures, see this check.

Results for commit 96497af.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@lucioleKi lucioleKi removed the testing currently being tested, tag is used by OTP internal CI label Feb 4, 2025
Comment on lines 811 to 815
lookup_name(Name, Db) ->
case ets:lookup(Db, Name) of
[{Name, Dir, Base, SubDirs}] -> {ok, Dir, Base, SubDirs};
[{Name, Dir}] -> {ok, Dir};
_ -> false
end.
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be a lookup_element now for slightly more efficiency

end || FileOrBin <- Files].

beam_bundle(Beams, OtherFiles) ->
Packed = term_to_binary(Beams, [{compressed,9}]),
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't beam files already compressed? If so, I'd imagine this extra compression will mostly just spend CPU time without reducing the size significantly

Copy link
Contributor

Choose a reason for hiding this comment

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

No, not necessarily, they can be compressed but they're not always so.

AppFiles = [{list_to_atom(filename:basename(filename:rootname(Name))), Bin} ||
{Name, Bin} <:- UnpackedFiles,
filename:extension(Name) =:= ".app"],
persistent_term:put(?MODULE, AppFiles),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the key escript used globally might be too collision-prone. How about at least '$escript?

Copy link
Contributor

@jhogberg jhogberg Feb 4, 2025

Choose a reason for hiding this comment

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

Good point, how about escript_app_files to make the corresponding persistent_term:get/1 more readable?

@josevalim
Copy link
Contributor

This is beautiful! 😍

Although not related to this issue, but for completeness, I have one last to do in relation to the erl_prim_loader and archives which is to remove the Paths variable from its state and keep it in the init module. We were used to rely on these Paths in the past in the code_server (which actually led to bugs) but now they are exclusively managed and used by init. Once archives are gone, it should be easier to refactor this too!

lucioleKi and others added 2 commits February 5, 2025 14:08
The support for zip archives for packaging Erlang applications is
an experimental features, and as such can be subject to incompatible
changes.

The support for archives in the code server and the `erl_prim_loader`
module complicates the implementation and make code loading slower
even when no archives are used. Furthermore, the existence of the
feature prevents or vatstly complicates further optimization of code
loading.

Therefore, this commit removes the support for code archives from
the code server, and from the `code` and `erl_prim_loader` modules.

Having multiple BEAM files and other files in an `escript` is still
supported, but in an incompatible way for escripts created by
Erlang/OTP 28. We decided to do it like this to keep
`escript:create/2` and `escript:extract/2` as simple as possible, and
make it possible to phase out the old archive format as soon as
possible.

However, escripts created by Erlang/OTP 27 and earlier will run
in Erlang/OTP 28, provided that they don't rely on archive features
in the code server or `erl_prim_loader`.

Co-authored-by: Björn Gustavsson <bjorn@erlang.org>
@lucioleKi lucioleKi force-pushed the isabell/stdlib/new-escript-archives/OTP-19470 branch from 119b997 to 96497af Compare February 5, 2025 13:09
> the code server has been removed. Archives can be still be used for
> escripts, but the internal implementation of archives for escripts
> has changed as well a how to create an escript containing multiple
> file.
Copy link
Contributor

Choose a reason for hiding this comment

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

"as well AS how ... fileS"

> `escript` scripts that use archive files should use
> In Erlang/OTP 28, the archive support in `m:erl_prim_loader` and the
> the code server has been removed. Archives can be still be used for
> escripts, but the internal implementation of archives for escripts
Copy link
Contributor

Choose a reason for hiding this comment

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

What does "internal implementation" mean here? The file format, or the code in the BEAM. If the latter, that's utterly irrelevant for users of escripts. If the former, it should be documented. (Haven't read the rest of the PR.)

Copy link
Contributor

Choose a reason for hiding this comment

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

It means that instead of storing a zip archive in the escript, the BEAM files and the other files are stored in a different way. We intentionally didn't document how exactly they are stored, because we want to force tools that handle escripts containing multiple files to use escript:create/2 and escript:extract/2.

@garazdawi garazdawi mentioned this pull request Feb 19, 2025
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:VM Assigned to OTP team VM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants