Skip to content

Add path resolution for CopyIn using Stat#727

Merged
JaewonHur merged 3 commits into
apple:mainfrom
simone-panico:add-stat-to-copy
May 19, 2026
Merged

Add path resolution for CopyIn using Stat#727
JaewonHur merged 3 commits into
apple:mainfrom
simone-panico:add-stat-to-copy

Conversation

@simone-panico
Copy link
Copy Markdown
Contributor

@simone-panico simone-panico commented May 6, 2026

This pull request enhances the handling of file and directory copy operations in Linux containers, particularly by improving destination path resolution and error handling for the copyIn operation with the Stat RPC

This PR is needed for container#1190

@JaewonHur
Copy link
Copy Markdown
Contributor

Thanks for the PR! Could you add few more tests in Sources/Integration/ContainerTests.swift.

@JaewonHur JaewonHur requested review from JaewonHur and dcantah May 11, 2026 16:52
@JaewonHur
Copy link
Copy Markdown
Contributor

JaewonHur commented May 11, 2026

@dcantah Could you take a look at this PR?
This fixes LinuxContainer.copyIn to make container copy resembles what docker does now.

One example is container copy foo container:/bar/.
The behavior really depends on what the type of object at /bar in the container.

If "foo" is file:
  if "bar" doesn't exist in container:
    throws error (cause "bar" has trailing "/")
  else if "bar" is a file:
    throws error (cause "bar" has trailing "/" but it is a file)
  else: // "bar" is directory
    copy to "/bar/foo"
else: //  "foo" is directory
  if "bar" doesn't exist in container:
    copy "foo" to "/bar"
  else if "bar" is a file:
    throws error (cause "bar" has trailing "/" but it is a file)
  else: // "bar" is directory
    copy to "/bar/foo"

As you can see here, the logic closely depends on the type of destination object and whether the destination path has trailing "/". We have no option but ends up implementing those logics in LinuxContainer.CopyIn. Because we don't expose stat RPC outside, we have no way to figure out which type the destination object is.

@JaewonHur
Copy link
Copy Markdown
Contributor

Hi @simone-panico one last part. Could you sign your commits once again? Then, let me approve this PR.

You can do followings:

git rebase --gpg-sign HEAD~4 --keep-base

If you didn't set git signing. Do followings first.

git config --global gpg.format ssh
git config --global user.signingkey <your-ssh-pub-key-for-git>

@JaewonHur
Copy link
Copy Markdown
Contributor

JaewonHur commented May 19, 2026

@simone-panico It seems commit history is broken.

Could you do followings? This one just rewrite commits based on latest main.
You should point your main branch head to the https://github.com/apple/containerization's latest main first.

While doing cherry-pick, you can check if the commit is signed by git log --show-signature -1. Please verify it is signed correctly.

# Assuming origin points to https://github.com/simone-panico/containerization
git fetch origin main
git checkout origin/main

git checkout -b add-stat-to-copy-re

git cherry-pick 65dfad
git cherry-pick 48292a
git cherry-pick 85ff49

Before force pushing, check if all changes are correct. Then,

git push --force origin add-stat-to-copy-re:add-stat-to-copy

@simone-panico
Copy link
Copy Markdown
Contributor Author

Should be fixed now :)

@JaewonHur JaewonHur merged commit db5b5b9 into apple:main May 19, 2026
5 checks passed
@simone-panico simone-panico deleted the add-stat-to-copy branch May 19, 2026 17:31
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.

2 participants