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

first pass at handling usrmerge #154

Merged
merged 8 commits into from
Dec 8, 2022

Conversation

tigarmo
Copy link
Collaborator

@tigarmo tigarmo commented Nov 30, 2022

This commit is a first step in gracefully handling existing layers when
packing the parts' contents. Specifically, it fixes the existing
breakage where priming a file in "/lib" will break a resulting
Ubuntu-based rock because on the base "/lib" is a symlink to "/usr/lib",
and if that link is broken everything breaks.

The updates to support this are:

  • Enhance oci.Image.add_layer() to take the existing base into account,
    via a parameter that points to the extracted base (see below);
  • Update lifecycle code to pass the extracted base forward when packing;
  • Add a jammy runner to CI, because the new integrated tests need umoci
    which is only available on jammy.

The update to add_layer() is to look for a file's parent dir in the
existing base. If it does exist, and is a symlink to another dir, add
the file as belonging to the symlink target instead. For example:

  • The prime dir contains "bin/file.txt";
  • The base is ubuntu:22.04, which means that there "/bin/" is a symlink
    to "/usr/bin";
  • Instead of adding the file as "bin/file.txt", add it as
    "usr/bin/file.txt".

Note that the original idea of just adding "bin/file.txt" but not adding
the "bin/" itself seems to be flaky; it works on some configurations of
the relevant tools but not others (more investigation necessary).

@tigarmo tigarmo force-pushed the CRAFT-1491-support-usrmerge branch 5 times, most recently from 4b33ff9 to 25e71de Compare November 30, 2022 19:05
This commit is a first step in gracefully handling existing layers when
packing the parts' contents. Specifically, it fixes the existing
breakage where priming a file in "/lib" will break a resulting
Ubuntu-based rock because on the base "/lib" is a symlink to "/usr/lib",
and if that link is broken everything breaks.

The updates to support this are:

- Enhance oci.Image.add_layer() to take the existing base into account,
  via a parameter that points to the extracted base (see below);
- Update lifecycle code to pass the extracted base forward when packing;
- Add a jammy runner to CI, because the new integrated tests need umoci
  which is only available on jammy.

The update to add_layer() is to look for a file's parent dir in the
existing base. If it *does* exist, and is a symlink to another dir, add
the file as belonging to the symlink target instead. For example:

- The prime dir contains "bin/file.txt";
- The base is ubuntu:22.04, which means that there "/bin/" is a symlink
  to "/usr/bin";
- Instead of adding the file as "bin/file.txt", add it as
  "usr/bin/file.txt".
@tigarmo tigarmo changed the title WIP handle usrmerge first pass at handling usrmerge Dec 2, 2022
@tigarmo tigarmo marked this pull request as ready for review December 2, 2022 18:48
rockcraft/oci.py Outdated Show resolved Hide resolved
Co-authored-by: Alex Lowe <lengau@gmail.com>
rockcraft/lifecycle.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
tests/spread/general/big/task.yaml Show resolved Hide resolved
rockcraft/lifecycle.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

Copy link
Contributor

@cmatsuoka cmatsuoka 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, thank you.

@tigarmo tigarmo enabled auto-merge (squash) December 8, 2022 17:04
@tigarmo tigarmo merged commit 607d934 into canonical:main Dec 8, 2022
@tigarmo tigarmo deleted the CRAFT-1491-support-usrmerge branch December 8, 2022 17:43
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

5 participants