mirror of
https://github.com/apple/container.git
synced 2026-08-28 03:16:29 +00:00
fix(cp): resolve relative host paths against current directory (#1741)
Fixes #1738 `container cp` fails when the host source path is relative (e.g. `container cp file foo:/root/`), because `NSString.standardizingPath` only canonicalizes paths but does not make them absolute. The unchanged relative path is then interpreted as `/file` (root-absolute) by `URL(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`, and `container image load`. The same fix was also applied to the copy-out destination path (line 68), which had the same issue. ## Type of Change - [x] Bug fix - [ ] New feature - [ ] Breaking change - [ ] Documentation update ## Motivation and Context `container cp file foo:/root/` fails with `"copyIn: source not found '/file'"` because the relative path `file` is never expanded to an absolute path. Using `$PWD/file` works, but relative paths should work too — every other command in the codebase handles this correctly. ## Testing - [x] Tested locally — builds and all existing tests pass - [ ] Added/updated tests - [ ] Added/updated docs --------- Co-authored-by: jwhur <57657645+JaewonHur@users.noreply.github.com>
This commit is contained in:
@@ -221,7 +221,8 @@ INTEGRATION_TEST_SUITES ?= \
|
||||
TestCLISystemDF \
|
||||
TestCLIMachineCommand \
|
||||
TestCLIMachineRuntime \
|
||||
TestCLINoParallelCases
|
||||
TestCLINoParallelCases \
|
||||
TestCLICopyCommand
|
||||
|
||||
empty :=
|
||||
space := $(empty) $(empty)
|
||||
|
||||
@@ -65,16 +65,17 @@ extension Application {
|
||||
switch (srcRef, dstRef) {
|
||||
case (.container(let id, let path), .local(let localPath)):
|
||||
let srcPath = FilePath(path)
|
||||
let destPath = FilePath((localPath as NSString).standardizingPath)
|
||||
let destPath = FilePath(URL(fileURLWithPath: localPath, relativeTo: .currentDirectory()).absoluteURL.path(percentEncoded: false))
|
||||
var isDirectory: ObjCBool = false
|
||||
let exists = FileManager.default.fileExists(atPath: destPath.string, isDirectory: &isDirectory)
|
||||
|
||||
var finalDestPath = destPath
|
||||
if exists && isDirectory.boolValue {
|
||||
guard let lastComponent = srcPath.lastComponent else {
|
||||
throw ContainerizationError(.invalidArgument, message: "source path has no last component: \(path)")
|
||||
}
|
||||
let finalDest = destPath.appending(lastComponent)
|
||||
try await client.copyOut(id: id, source: path, destination: finalDest.string)
|
||||
finalDestPath = destPath.appending(lastComponent)
|
||||
try await client.copyOut(id: id, source: path, destination: finalDestPath.string)
|
||||
} else if localPath.hasSuffix("/") {
|
||||
try await client.copyOut(id: id, source: path, destination: destPath.string)
|
||||
var resultIsDir: ObjCBool = false
|
||||
@@ -89,9 +90,15 @@ extension Application {
|
||||
} else {
|
||||
try await client.copyOut(id: id, source: path, destination: destPath.string)
|
||||
}
|
||||
print(finalDestPath.string)
|
||||
case (.local(let localPath), .container(let id, let path)):
|
||||
let srcPath = FilePath((localPath as NSString).standardizingPath)
|
||||
let srcPath = FilePath(URL(fileURLWithPath: localPath, relativeTo: .currentDirectory()).absoluteURL.path(percentEncoded: false))
|
||||
var isDirectory: ObjCBool = false
|
||||
|
||||
guard let lastComponent = srcPath.lastComponent else {
|
||||
throw ContainerizationError(.invalidArgument, message: "source path has no last component: \(localPath)")
|
||||
}
|
||||
|
||||
guard FileManager.default.fileExists(atPath: srcPath.string, isDirectory: &isDirectory) else {
|
||||
throw ContainerizationError(.notFound, message: "source path does not exist: \(localPath)")
|
||||
}
|
||||
@@ -100,6 +107,8 @@ extension Application {
|
||||
}
|
||||
|
||||
try await client.copyIn(id: id, source: srcPath.string, destination: path, createParents: true)
|
||||
let printedDest = path.hasSuffix("/") ? "\(id):\(path)\(lastComponent.string)" : "\(id):\(path)"
|
||||
print(printedDest)
|
||||
case (.container, .container):
|
||||
throw ContainerizationError(.invalidArgument, message: "copying between containers is not supported")
|
||||
case (.local, .local):
|
||||
|
||||
@@ -905,4 +905,54 @@ class TestCLICopyCommand: CLITest {
|
||||
Issue.record("testCopyInDirectoryContentsToExistingDirectoryTrailingSlash failed: \(error)")
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Relative path resolution
|
||||
|
||||
@Test func testCopyInRelativeSourcePath() throws {
|
||||
do {
|
||||
let name = getTestName()
|
||||
try doCreate(name: name)
|
||||
defer { try? doStop(name: name) }
|
||||
try doStart(name: name)
|
||||
try waitForContainerRunning(name)
|
||||
|
||||
let content = "relative source"
|
||||
try content.write(to: testDir.appendingPathComponent("relfile.txt"), atomically: true, encoding: .utf8)
|
||||
|
||||
let (_, _, error, status) = try run(
|
||||
arguments: ["copy", "./relfile.txt", "\(name):/tmp/"],
|
||||
currentDirectory: testDir)
|
||||
if status != 0 { throw CLIError.executionFailed("copy failed: \(error)") }
|
||||
|
||||
let result = try doExec(name: name, cmd: ["cat", "/tmp/relfile.txt"])
|
||||
#expect(result.trimmingCharacters(in: .whitespacesAndNewlines) == content)
|
||||
try doStop(name: name)
|
||||
} catch {
|
||||
Issue.record("testCopyInRelativeSourcePath failed: \(error)")
|
||||
}
|
||||
}
|
||||
|
||||
@Test func testCopyOutRelativeDestinationPath() throws {
|
||||
do {
|
||||
let name = getTestName()
|
||||
try doCreate(name: name)
|
||||
defer { try? doStop(name: name) }
|
||||
try doStart(name: name)
|
||||
try waitForContainerRunning(name)
|
||||
|
||||
let content = "relative dest"
|
||||
_ = try doExec(name: name, cmd: ["sh", "-c", "echo -n '\(content)' > /tmp/relfile.txt"])
|
||||
|
||||
let (_, _, error, status) = try run(
|
||||
arguments: ["copy", "\(name):/tmp/relfile.txt", "./"],
|
||||
currentDirectory: testDir)
|
||||
if status != 0 { throw CLIError.executionFailed("copy failed: \(error)") }
|
||||
|
||||
let result = try String(contentsOfFile: testDir.appendingPathComponent("relfile.txt").path, encoding: .utf8)
|
||||
#expect(result == content)
|
||||
try doStop(name: name)
|
||||
} catch {
|
||||
Issue.record("testCopyOutRelativeDestinationPath failed: \(error)")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user