-
Notifications
You must be signed in to change notification settings - Fork 480
[image-save-load]: support stdin for filepath #734
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,14 +34,23 @@ extension Application { | |
transform: { str in | ||
URL(fileURLWithPath: str, relativeTo: .currentDirectory()).absoluteURL.path(percentEncoded: false) | ||
}) | ||
var input: String | ||
var input: String? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is fine but what you'll want to do is change L35 to something like this so str.map { URL(fileURLWithPath: $0, relativeTo: .currentDirectory()).absoluteURL.path(percentEncoded: false) } |
||
|
||
@OptionGroup | ||
var global: Flags.Global | ||
|
||
public func run() async throws { | ||
guard FileManager.default.fileExists(atPath: input) else { | ||
print("File does not exist \(input)") | ||
var filePath = "" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't quite what #559 is asking for. If
This allows us to do things like moving images between Docker and docker save foo:latest bar:latest | container image load
container save baz:latest qux:latest | docker image load There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahhh, I was unsure why the issue was talking about How is the binary data coming from the standard input? Are we prompting anything from the user (would it be the path to the temporary file or the path from the file to read the binary data from)? |
||
if input == nil { | ||
print("Path to the image tar archive: ", terminator: "") | ||
guard let userInput = readLine() else { | ||
throw ImageCommandError.invalidInput | ||
} | ||
filePath = userInput | ||
} | ||
|
||
guard FileManager.default.fileExists(atPath: input ?? filePath) else { | ||
print("File does not exist \(input ?? filePath)") | ||
Application.exit(withError: ArgumentParser.ExitCode(1)) | ||
} | ||
|
||
|
@@ -57,7 +66,7 @@ extension Application { | |
progress.start() | ||
|
||
progress.set(description: "Loading tar archive") | ||
let loaded = try await ClientImage.load(from: input) | ||
let loaded = try await ClientImage.load(from: input ?? filePath) | ||
|
||
let taskManager = ProgressTaskCoordinator() | ||
let unpackTask = await taskManager.startTask() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,7 +46,7 @@ extension Application { | |
transform: { str in | ||
URL(fileURLWithPath: str, relativeTo: .currentDirectory()).absoluteURL.path(percentEncoded: false) | ||
}) | ||
var output: String | ||
var output: String? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, map |
||
|
||
@Option( | ||
help: "Platform for the saved image (format: os/arch[/variant], takes precedence over --os and --arch)" | ||
|
@@ -59,6 +59,15 @@ extension Application { | |
@Argument var references: [String] | ||
|
||
public func run() async throws { | ||
var filePath = "" | ||
if output == nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here if output is nil we create a temporary file, defer the delete, do ClientImage.save to that file, and then copy the binary data in the file to the standard output |
||
print("Pathname for the saved image: ", terminator: "") | ||
guard let userInput = readLine() else { | ||
throw ImageCommandError.invalidInput | ||
} | ||
filePath = userInput | ||
} | ||
|
||
var p: Platform? | ||
if let platform { | ||
p = try Platform(from: platform) | ||
|
@@ -90,7 +99,7 @@ extension Application { | |
throw ContainerizationError(.invalidArgument, message: "failed to save image(s)") | ||
|
||
} | ||
try await ClientImage.save(references: references, out: output, platform: p) | ||
try await ClientImage.save(references: references, out: output ?? filePath, platform: p) | ||
|
||
progress.finish() | ||
for reference in references { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, just use
ContainerizationError
instead of creating new error types. We do have a tech debt issue #268 for improving our error handling.