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

Simplify root filesystem #19

Closed

Conversation

razvand
Copy link
Contributor

@razvand razvand commented Aug 29, 2023

Remove bin/, include/, lib/test/ and pyenv.cfg from the root filesystem archive (rootfs.tar.gz). These are not required by Python applications:

Copy link
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

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

@razvand change the : at the end of the commit message to a .. All good besides that.

Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com

@andreittr
Copy link
Contributor

andreittr commented Aug 29, 2023

Removing the bin/ directory is technically fine for running python apps, however it removes the ability for a user to customize this rootfs/virtualenv further with pip (you need bin/activate to enter the venv and customize it). Is this what we intend? If so, it's OK, we just tell users to build their own rootfs if they want customization.

edit: actually, having users build their own rootfs is probably preferable, as the activate script seems to hardcode the absolute path to the venv, so I'm not sure how well it would perform if moved :/ I have to think this issue through a bit and patch the rootfs build code, but this change is a-ok (modulo the commit message change as mentioned by @StefanJum ).

Reviewed-by: Andrei Tatar andrei@unikraft.io

Remove `bin/`, `include/`, `lib/test/` and `pyenv.cfg` from the root
filesystem archive (`rootfs.tar.gz`). These are not required by
Python applications.

Signed-off-by: Razvan Deaconescu <razvand@unikraft.io>
@razvand
Copy link
Contributor Author

razvand commented Aug 30, 2023

@StefanJum , @andreittr , made the updates.

@andreittr , after @StefanJum reviews the PR, you should approve it, you're the assignee.

Copy link
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com

@andreittr
Copy link
Contributor

Approved-by: Andrei Tatar andrei@unikraft.io

unikraft-bot pushed a commit that referenced this pull request Aug 30, 2023
Remove `bin/`, `include/`, `lib/test/` and `pyenv.cfg` from the root
filesystem archive (`rootfs.tar.gz`). These are not required by
Python applications.

Signed-off-by: Razvan Deaconescu <razvand@unikraft.io>
Reviewed-by: Andrei Tatar <andrei@unikraft.io>
Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com>
Approved-by: Andrei Tatar <andrei@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants