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

refactor: Remove inject_files rule invocations and move injection of binaries to ext4_image #3497

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

Conversation

frankdavid
Copy link
Contributor

Removing this step from our build simplifies the build process and allows for further optimizations.

Runtimes of building the image when rootfs-tree.tar and rootfs contents are available from cache:

HostOS:

  • Before:
    • Build ext4 image from base filesystem: 30s
    • Inject rootfs: 23s
  • After:
    • Build full image with injected files: 31s

GuestOS:

  • Before:
    • Build ext4 image from base filesystem: 17s
    • Inject rootfs: 22s
  • After
    • Build full image with injected files: 21s

Diff:

  • Regressions:
    • HostOS, building the full image when there are only rootfs changes: 8s regression.
  • Same:
    • GuestOS, building the full image when there are only rootfs changes
  • Improvements:
    • HostOS, building the full image when there are changes in the base image: 22s improvement
    • GuestOS, building the full image when there are changes in the base image: 18s improvement

@frankdavid frankdavid requested a review from a team as a code owner January 17, 2025 11:49
# Note that we defer injecting the files from images_deps["rootfs"]. These are mostly slower to build.

# NOTE: e2fsdroid does not support filenames with spaces, fortunately,
# there are only two in our build.
Copy link
Member

Choose a reason for hiding this comment

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

Hm, I guess we forgot to update this last time. Maybe better would be to say:

... fortunately, these only occur in firmware that we do not use

@@ -139,80 +139,86 @@ def icos_build(

# -------------------- Extract root partition --------------------

# Note that we defer injecting the files from images_deps["rootfs"]. These are mostly slower to build.
Copy link
Member

Choose a reason for hiding this comment

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

This is no longer true, right?

Comment on lines +221 to +222
"-d",
os.path.join(fs_basedir, limit_prefix),
Copy link
Member

Choose a reason for hiding this comment

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

Same as #3476 (comment), we should make sure to investigate just in case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants