fix(cp): resolve relative host paths against current directory#1741
fix(cp): resolve relative host paths against current directory#1741adityabagchi24 wants to merge 4 commits into
Conversation
|
@adityabagchi24 Hi! Could you check #1743. I'm happy to merge this one, but could you add followings:
|
|
@JaewonHur Thanks for the heads up on #1743. These are great additions and handles exceptions well. Please review mine as well. I've added the print output and integration tests as requested. |
| } | ||
|
|
||
| try await client.copyIn(id: id, source: srcPath.string, destination: path, createParents: true) | ||
| let printedDest = path.hasSuffix("/") ? "\(id):\(path)\(srcPath.lastComponent!)" : "\(id):\(path)" |
There was a problem hiding this comment.
We avoid using force unwrap. Could you use guard to get the last component?
There was a problem hiding this comment.
I have added guard to get the last component. Please review the changes.
|
@JaewonHur I mistakenly forgot to add the commit signatures so should I add them? |
|
Yeah please :) Could you sign your commits? |
adityabagchi24
force-pushed
the
main
branch
4 times, most recently
from
5e5f53c to
6053e4d
Compare
|
1)I have signed all my commits. Could you please review my changes when you have a chance? Thank you. |
2 participants
Fixes #1738
container cpfails when the host source path is relative (e.g.container cp file foo:/root/), becauseNSString.standardizingPathonly canonicalizes paths but does not make them absolute. The unchanged relative path is then interpreted as/file(root-absolute) byURL(fileURLWithPath:)on the runtime side.Fixed by resolving relative paths against the current working directory before use, matching the pattern already used by
container export,container image save, andcontainer image load.The same fix was also applied to the copy-out destination path (line 68), which had the same issue.
Type of Change
Motivation and Context
container cp file foo:/root/fails with"copyIn: source not found '/file'"because the relative pathfileis never expanded to an absolute path. Using$PWD/fileworks, but relative paths should work too — every other command in the codebase handles this correctly.Testing