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

Prevent bad vm.args in Nerves firmware #884

Merged
merged 2 commits into from
Jul 7, 2023
Merged

Conversation

jjcarstens
Copy link
Member

@jjcarstens jjcarstens commented Jun 30, 2023

With Elixir 1.15, the way the Elixir IEx shell is started on device has changed and using the old way will break things.

This adds a new check during the release (firmware) initialization and stops the firmware build if the old method is still there

Note
Using Elixir 1.15
image

Using Elixir < 1.15
image

lib/nerves/release.ex Outdated Show resolved Hide resolved
@fhunleth
Copy link
Member

fhunleth commented Jul 3, 2023

I just realized that we need the opposite check as well since someone might be using Elixir 1.14. I.e., if they specify the Elixir 1.15 way, give them an error.

@jjcarstens
Copy link
Member Author

Good point. Is this also specific to OTP 26?

@fhunleth
Copy link
Member

fhunleth commented Jul 3, 2023

I don't believe so. I was using the old way with Elixir 1.14 and OTP 26. Unfortunately I feel like things in this area have been changing lately and checking again with Elixir 1.14.5 would be wise.

lib/nerves/release.ex Outdated Show resolved Hide resolved
lib/nerves/release.ex Outdated Show resolved Hide resolved
With Elixir 1.15, the way the Elixir IEx shell is started on device
has changed and using the old way will break things.

This adds a new check during the release (firmware) initialization
and stops the firmware build if the old method is still there
This file is required by Nerves, but has gone previously unchecked.
Without it, things would be very sad.

This adds a check in to prevent firmware being built if there is no
vm.args.eex file. In the future, we may want to validate several
pieces of it
Copy link
Member

@fhunleth fhunleth left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@mnishiguchi What do you think?

jjcarstens added a commit to nerves-project/nerves_bootstrap that referenced this pull request Jul 7, 2023
@jjcarstens jjcarstens merged commit 1b18eae into main Jul 7, 2023
5 checks passed
@jjcarstens jjcarstens deleted the elixir-1.15-shell-check branch July 7, 2023 14:31
jjcarstens added a commit to nerves-project/nerves_bootstrap that referenced this pull request Jul 7, 2023
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.

None yet

3 participants