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

feat: use init and entrypoint from device tree #16

Merged
merged 1 commit into from
Oct 11, 2023
Merged

Conversation

cd $HOME &&
# use entrypoint from device tree when available, otherwise from command line
if [ -s /proc/device-tree/cartesi-machine/entrypoint ]; then
ENTRYPOINT=$(busybox cat /proc/device-tree/cartesi-machine/entrypoint)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to load this from string if we drop support for entrypoint in the Linux bootargs? Can't we run directly from there?

Copy link
Contributor Author

@edubart edubart Oct 6, 2023

Choose a reason for hiding this comment

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

Actually I was running /proc/device-tree/cartesi-machine/entrypoint directly at first, but then I thought the error messages of invalid commands were less user friendly, so I decided to load in a string.

Example of an error running directly:

cartesi-machine --quiet --no-init-splash -i invalid-command
sh: /proc/device-tree/cartesi-machine/entrypoint: line 1: invalid-command: not found

Example of an error using a string:

cartesi-machine --quiet --no-init-splash -i invalid-command
sh: invalid-command: not found

I could run directly again, which one you think it is better?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the first error message is too bad actually. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I changed to run it directly.

Copy link
Contributor

@diegonehab diegonehab left a comment

Choose a reason for hiding this comment

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

See comments.

@edubart edubart requested a review from diegonehab October 6, 2023 17:20
Dockerfile Show resolved Hide resolved
@edubart edubart requested review from diegonehab and removed request for diegonehab October 10, 2023 20:32
@edubart edubart merged commit 09fb3f4 into main Oct 11, 2023
2 checks passed
@edubart edubart deleted the feature/new-init branch October 11, 2023 13:16
@vfusco vfusco added this to the v0.14.0 milestone Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
4 participants