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

Use erlexec directly in relx helper functions #902

Merged
merged 2 commits into from
Jun 18, 2022

Conversation

keynslug
Copy link
Contributor

@keynslug keynslug commented Jan 6, 2022

...Instead of $BINDIR/erl.

Latter might resolve to the system-wide erl script, notably when a release has been made with {include_erts, false} and {system_libs, true} (which is usual).

In such a case even the smallest discrepancy in versions between an Erlang/OTP used to assemble a release (e.g. 24.1.7) and an Erlang/OTP running it later (e.g. 24.2.0) will draw such release impossible to start. This happens because relx helper functions employ start_clean bootfile from a release, and this bootfile attempts to start core application (kernel / stdlib) versions as seen at the build time, which will not be present system-wide at the run time.

...Instead of `$BINDIR/erl`.

Latter might resolve to system-wide `erl` script, notably when release has been
made with `{include_erts, false}` and `{system_libs, true}` (which is usual).
In such a case even the smallest discrepancy in versions between an Erlang/OTP
used to assemble a release (e.g. 24.1.7) and an OTP running it later (e.g.
24.2.0) will draw such release impossible to start. This happens because
relx helper functions employ `start_clean` bootfile from a release, and this
bootfile attempts to start core application (kernel / stdlib) versions as seen
at the build time, which will not be present system-wide at the run time.
@tsloughter
Copy link
Member

Thanks. To make sure I understand why this is needed: The issue is that system wide erl is a shell script that will be different env variables to point to where Erlang is which leads to it using the global system libs instead of the system libs as part of the release?

And this works fine even if {system_libs, false} is used?

@keynslug
Copy link
Contributor Author

Basically, yes. The issue I stumbled upon initially was that I couldn't start a release built with {system_libs, true} under 24.1 on a system with 24.2.

And this works fine even if {system_libs, false} is used?

Can't remember if I verified that. I'll do that next week and report back.

@tsloughter
Copy link
Member

hm but should it be exec erlexec? I'd say for sure yes since erl the script does that but we use dyn_erl (copied to erl, which is C code) when include_erts is true, so I need to see what it does in its code.

@tsloughter
Copy link
Member

Yea, it uses execvp:

 execvp(erlexec, argv);

So my thinking is it should be exec $BINDIR/erlexec

@keynslug
Copy link
Contributor Author

keynslug commented Jun 15, 2022

And this works fine even if {system_libs, false} is used?

Nope. For the sake of completeness here are the results of running bin/relname ping under Erlang/OTP 24.3.4 in a release assembled with Erlang/OTP 24.2.0 without and with this patch under different relx settings (in rebar.config):

master / patched {mode, prod} {mode, minimal}
{system_libs, true} ✅ / ✅ ❌ / ✅
{system_libs, false} ❌ / ❌ ❌ / ❌

Turns out this patch makes situation better only under {mode, minimal} and {system_libs, true}.

I believe releases without system libraries don't work in this scenario (different build-time / run-time OTP versions) because bin/relname still refer to $REL_DIR/start_clean which pins system libraries' versions at build-time. Though I don't know if this is even feasible or desirable to make releases runnable with such combination of settings in this scenario.

So my thinking is it should be exec $BINDIR/erlexec

This patch is concerned only with relx_get_nodename helper function behavior which is being called during release startup (in particular), I think there's no point execing into it?

@tsloughter
Copy link
Member

Right, it should not be runnable with different system libs, the user has to have the right ones installed if they want to run a release that doesn't include system libs.

And you are right about exec, I was thinking this was running erl to start the release.

@tsloughter
Copy link
Member

Ok, great. So it improves scenarios and doesn't break any existing working scenarios? I'll go ahead and merge this if I read this right.

@keynslug
Copy link
Contributor Author

So it improves scenarios and doesn't break any existing working scenarios?

Yep, seems so. Feeling a little paranoid I just verified that the patch doesn't break anything under the most common scenario where build-time and run-time Erlang/OTP versions are the same. Good news it doesn't. Bad news I just found out that combination of {mode, prod} + {system_libs, false} isn't working even on main.

I suppose this is because the script currently assumes that system libraries are located in the same place as the runtime system, and under {mode, prod} this evaluates to the release's libdir where system libraries are obviously missing given {system_libs, false}.

@tsloughter
Copy link
Member

So unrelated to this? Can you open an issue and I'll merge this and cut a release.

@tsloughter tsloughter merged commit b5182ce into erlware:main Jun 18, 2022
ferd added a commit to erlang/rebar3 that referenced this pull request Jun 18, 2022
- [Use `erlexec` directly in relx helper functions](erlware/relx#902)
- [Make rlx_util:parse_vsn parse integer versions](erlware/relx#913)
- [fix awk script check_name() in extended_bin](erlware/relx#915)
- [avoid crash when overlay is malformed](erlware/relx#916)
- [keep attributes when stripping beams](erlware/relx#906)
- [Fix {include_erts,true} in Windows releases](erlware/relx#914)
- [ensure the erl file is writable before copying dyn_erl to it](erlware/relx#903)
- Various tests added
ferd added a commit to erlang/rebar3 that referenced this pull request Jun 18, 2022
New features:

- [Add --offline option and REBAR_OFFLINE environment variable](#2643)
- [Add support for project-local plugins](#2697)
- [Add eunit --test flag](#2684)

Experimental features for which we promise no backwards compatibility in
the near future:

- [Experimental vendoring provider](#2689)
  - [Support plugins in experimental vendor provider](#2702)

Other changes:

- [Support OTP 23..25 inclusively](#2706)
- [Bump Relx to 4.7.0](#2718)
  - [Use `erlexec` directly in relx helper functions](erlware/relx#902)
  - [Make rlx_util:parse_vsn parse integer versions](erlware/relx#913)
  - [fix awk script check_name() in extended_bin](erlware/relx#915)
  - [avoid crash when overlay is malformed](erlware/relx#916)
  - [keep attributes when stripping beams](erlware/relx#906)
  - [Fix {include_erts,true} in Windows releases](erlware/relx#914)
  - [ensure the erl file is writable before copying dyn_erl to it](erlware/relx#903)
  - Various tests added
- [Properly carry overlay_vars settings for files in relx](#2711)
- [Track mib compilation artifacts](#2709)
- [Attempt to find apps in git subdirs (sparse checkouts)](#2687)
- [Do not discard parameters --system_libs and --include-erts when duplicate values exist](#2695)
- [Use default `depth` parameter for SSL](#2690)
- [Fix global cache config overriding](#2683)
- [Error out on unknown templates in 'new' command](#2676)
- [Fix a typo](#2674)
- [Bump certifi to 2.9.0](#2673)
- [add a debug message in internal dependency fetching code](#2672)
- [Use SPDX id for license in template and test](#2668)
- [Use default branch for git and git_subdir resources with no revision](#2663)

Signed-off-by: Fred Hebert <mononcqc@ferd.ca>
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.

2 participants