Skip to content

Commit 3d7f228

Browse files
author
Roy Lin
committed
fix(cli): volume rm exit code + Docker volume-inspect schema, and cp mode/large-file
Round-2 parity fixes: - volume rm now returns a non-zero exit when a removal fails (e.g. volume in use) instead of printing the error to stderr and exiting 0. - volume inspect emits a top-level JSON array of Docker-shaped objects (Name/Driver/Mountpoint/Scope/Labels/Options/CreatedAt), not the raw snake_case VolumeConfig. - cp to a box streams file bytes over the exec channel's stdin (cat > dst) instead of embedding base64 in the command line, so large files no longer hit ARG_MAX (verified: 3 MB copy succeeds); it also restores the source file's mode via chmod (cp preserves permissions, verified 0755).
1 parent f7e3c17 commit 3d7f228

2 files changed

Lines changed: 46 additions & 17 deletions

File tree

src/cli/src/commands/cp.rs

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -208,25 +208,24 @@ async fn copy_file_to_box(
208208
) -> Result<(), Box<dyn std::error::Error>> {
209209
let content =
210210
std::fs::read(host_path).map_err(|e| format!("Failed to read {host_path}: {e}"))?;
211+
let len = content.len();
212+
let mode = host_file_mode(host_path);
211213

212-
use base64::Engine;
213-
let encoded = base64::engine::general_purpose::STANDARD.encode(&content);
214-
214+
// Stream the raw bytes over the exec channel's stdin (not the command line)
215+
// so large files do not exceed ARG_MAX, and restore the source file's mode
216+
// (Docker `cp` preserves permissions).
217+
let dst = shell_escape(box_path);
215218
let request = ExecRequest {
216219
cmd: vec![
217220
"sh".to_string(),
218221
"-c".to_string(),
219-
format!(
220-
"echo '{}' | base64 -d > {}",
221-
encoded,
222-
shell_escape(box_path)
223-
),
222+
format!("cat > {dst} && chmod {mode:o} {dst}"),
224223
],
225224
timeout_ns: DEFAULT_EXEC_TIMEOUT_NS,
226225
env: vec![],
227226
working_dir: None,
228227
rootfs: None,
229-
stdin: None,
228+
stdin: Some(content),
230229
stdin_streaming: false,
231230
user: None,
232231
streaming: false,
@@ -239,13 +238,27 @@ async fn copy_file_to_box(
239238
return Err(format!("Failed to write {box_path} in box: {stderr}").into());
240239
}
241240

242-
println!(
243-
"{host_path} → {box_name}:{box_path} ({} bytes)",
244-
content.len()
245-
);
241+
println!("{host_path} → {box_name}:{box_path} ({len} bytes)");
246242
Ok(())
247243
}
248244

245+
/// Source file's permission bits (lower 12) for `cp` to restore in the box;
246+
/// defaults to 0o644 off-Unix or on stat failure.
247+
fn host_file_mode(host_path: &str) -> u32 {
248+
#[cfg(unix)]
249+
{
250+
use std::os::unix::fs::PermissionsExt;
251+
std::fs::metadata(host_path)
252+
.map(|m| m.permissions().mode() & 0o7777)
253+
.unwrap_or(0o644)
254+
}
255+
#[cfg(not(unix))]
256+
{
257+
let _ = host_path;
258+
0o644
259+
}
260+
}
261+
249262
// ---------------------------------------------------------------------------
250263
// Directory transfers
251264
// ---------------------------------------------------------------------------

src/cli/src/commands/volume.rs

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,14 +144,20 @@ async fn execute_rm(args: RmArgs) -> Result<(), Box<dyn std::error::Error>> {
144144

145145
let store = VolumeStore::default_path()?;
146146

147+
let mut errors = Vec::new();
147148
for name in &args.names {
148149
match store.remove(name, args.force) {
149150
Ok(_) => println!("{name}"),
150-
Err(e) => eprintln!("Error removing volume '{name}': {e}"),
151+
Err(e) => errors.push(format!("Error removing volume '{name}': {e}")),
151152
}
152153
}
153154

154-
Ok(())
155+
if errors.is_empty() {
156+
Ok(())
157+
} else {
158+
// A failed removal (e.g. volume in use) must be a non-zero exit, like Docker.
159+
Err(errors.join("\n").into())
160+
}
155161
}
156162

157163
async fn execute_inspect(args: InspectArgs) -> Result<(), Box<dyn std::error::Error>> {
@@ -161,8 +167,18 @@ async fn execute_inspect(args: InspectArgs) -> Result<(), Box<dyn std::error::Er
161167
.get(&args.name)?
162168
.ok_or_else(|| format!("volume '{}' not found", args.name))?;
163169

164-
let json = serde_json::to_string_pretty(&config)?;
165-
println!("{json}");
170+
// Match `docker volume inspect`: a top-level JSON array of PascalCase objects
171+
// (Mountpoint, Scope, etc.), not the raw snake_case VolumeConfig.
172+
let output = serde_json::json!([{
173+
"Name": config.name,
174+
"Driver": config.driver,
175+
"Mountpoint": config.mount_point,
176+
"Scope": "local",
177+
"Labels": config.labels,
178+
"Options": serde_json::Map::new(),
179+
"CreatedAt": config.created_at,
180+
}]);
181+
println!("{}", serde_json::to_string_pretty(&output)?);
166182
Ok(())
167183
}
168184

0 commit comments

Comments
 (0)