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(24.10): forward-port SDF for tar #492

Open
wants to merge 3 commits into
base: ubuntu-24.10
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions slices/tar.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package: tar

essential:
- tar_copyright

slices:
bins:
Copy link
Member

Choose a reason for hiding this comment

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

Let's have bins contain all the binaries. We can have individual slices rmt, tar, cat etc. and then include all of those in bins.

Choose a reason for hiding this comment

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

That would diverge from the existing SDF in 24.04 that this is a forward-port of?

Copy link
Collaborator

Choose a reason for hiding this comment

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

this is already the case.

  • tar's binaries are tarcat, rmt-tar and tar. The latter two are brought in via essential. So effectively bins contains all binaries , as does tar in 24.04

essential:
- tar_rmt
- tar_tar
contents:
/usr/sbin/tarcat:
Copy link
Member

Choose a reason for hiding this comment

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

tarcat will actually need dash and coreutils (only dd, tr, cut).

Choose a reason for hiding this comment

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

The tar package doesn't declare dependencies on dash & coreutils, so not sure I understand why the SDF would declare them?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed.
@norrisjeremy some packages won't declare dependencies to other packages that are deemed "essential" and always present on the distro.

tarcat is a shell script #!/bin/sh that runs commands such as dd cat cut tr...

there's no test for it, and there should be one. that would support this statement. And the 24.04 should be fixed as well


copyright:
contents:
/usr/share/doc/tar/copyright:

rmt:
essential:
- libc6_libs
contents:
/etc/rmt: {symlink: /usr/sbin/rmt-tar}
Copy link
Member

Choose a reason for hiding this comment

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

lrwxrwxrwx root/root         0 2024-04-08 16:20 ./etc/rmt -> /usr/sbin/rmt

I see that /etc/rmt points to /usr/sbin/rmt. The latter is created via the postinst script.

        update-alternatives --install /usr/sbin/rmt rmt /usr/sbin/rmt-tar 50 \
                --slave /usr/share/man/man8/rmt.8.gz rmt.8.gz \
                        /usr/share/man/man8/rmt-tar.8.gz

In my opinion, let's honour that chain.

Suggested change
/etc/rmt: {symlink: /usr/sbin/rmt-tar}
/etc/rmt:
/usr/sbin/rmt: {symlink: /usr/sbin/rmt-tar}

Let me know what you think.

Choose a reason for hiding this comment

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

@rebornplusplus This is a forward-port of the existing tar SDF definition from 24.04 that is missing in 24.10.
Not sure I understand why we would make changes that cause it to diverge from what was landed originally in #320?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, agreed @rebornplusplus

@norrisjeremy is it something we should fix in 24.04.

/usr/sbin/rmt-tar:

tar:
essential:
- libacl1_libs
- libc6_libs
- libselinux1_libs
contents:
/usr/bin/tar:
33 changes: 33 additions & 0 deletions tests/spread/integration/tar/task.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Nice tests. 💯

Copy link
Collaborator

Choose a reason for hiding this comment

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

yep.

please add the tarcat test too so we cover the use case discussed in the comments above.

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
summary: Integration tests for tar

execute: |
# Chisel a minimum number of slices to give us a runnable system that we can
# test in.
rootfs="$(install-slices tar_tar)"

mkdir "${rootfs}/test"

echo "This is a test file" > "${rootfs}/test/test.txt"
touch --date=2020-01-01T00:00:00Z "${rootfs}/test/test.txt"
touch --date=2020-01-01T00:00:00Z "${rootfs}/test/"

# Create an uncompressed tarball and compare to a known hash.
chroot "${rootfs}/" tar -cf test.tar /test/
[[ $(sha256sum "${rootfs}/test.tar" | cut -d' ' -f1) == f5893661db55f15954c90dee860ee6c93042ea482c303f5f162c8078cdc79928 ]]

# Decompress back to a new directory and ensure the file is correct.
mkdir "${rootfs}/test-result"
chroot "${rootfs}/" tar -xf test.tar -C /test-result
[[ "$(cat ${rootfs}/test/test.txt)" == "$(cat ${rootfs}/test-result/test/test.txt)" ]]
# Check modification times
[[ $(stat -c %Y ${rootfs}/test-result/test) == 1577836800 ]]
[[ $(stat -c %Y ${rootfs}/test-result/test) == 1577836800 ]]

# Expect an error gzipping a tarball since gzip is not installed in the chroot.
! chroot "${rootfs}/" tar -czf test.tar.gz /test/ 2| grep -q "tar (child): gzip: Cannot exec: No such file or directory"

# New environment for rmt slice.
rootfs="$(install-slices tar_rmt)"

# We can't really do much with rmt, so just check that the executable works.
chroot "${rootfs}/" rmt-tar --version
Loading