Skip to content

Commit d656d42

Browse files
committed
♻️ Extract SplitArchiveReader to unify memmap/streaming dispatch
Introduce SplitArchiveReader in cli/src/command/core/archive_source.rs to encapsulate #[cfg(feature = "memmap")] branching into a single location. The struct provides three methods (transform_entries, for_each_entry, for_each_read_entry) with dual cfg'd implementations, replacing the 6-line mmap boilerplate that was duplicated across 11 command files. - Migrated: delete, chmod, chown, strip, migrate, acl, xattr, diff, sort, list - Removed: run_list_archive_mem, non-memmap run_entries (dead code) - Made private: run_transform_entry (only used by archive_source.rs)
1 parent 0b55b78 commit d656d42

12 files changed

Lines changed: 230 additions & 264 deletions

File tree

cli/src/command/acl.rs

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ use crate::{
66
command::{
77
Command, ask_password,
88
core::{
9-
TransformStrategyKeepSolid, TransformStrategyUnSolid, collect_split_archives,
10-
run_entries, run_transform_entry,
9+
SplitArchiveReader, TransformStrategyKeepSolid, TransformStrategyUnSolid,
10+
collect_split_archives,
1111
},
1212
},
1313
ext::{Acls, NormalEntryExt, PermissionExt},
@@ -288,19 +288,10 @@ fn archive_get_acl(args: GetAclCommand) -> anyhow::Result<()> {
288288
let platforms = args.platform.into_iter().collect::<HashSet<_>>();
289289
let numeric_owner = args.numeric;
290290

291-
let archives = collect_split_archives(args.file.archive)?;
291+
let mut source = SplitArchiveReader::new(collect_split_archives(args.file.archive)?)?;
292292

293-
#[cfg(feature = "memmap")]
294-
let mmaps = archives
295-
.into_iter()
296-
.map(crate::utils::mmap::Mmap::try_from)
297-
.collect::<io::Result<Vec<_>>>()?;
298-
#[cfg(feature = "memmap")]
299-
let archives = mmaps.iter().map(|m| m.as_ref());
300-
301-
run_entries(
302-
archives,
303-
|| password.as_deref(),
293+
source.for_each_entry(
294+
password.as_deref(),
304295
#[hooq::skip_all]
305296
|entry| {
306297
let entry = entry?;
@@ -355,41 +346,30 @@ fn archive_set_acl(args: SetAclCommand) -> anyhow::Result<()> {
355346
}
356347
};
357348

358-
let archives = collect_split_archives(&args.file.archive)?;
359-
360-
#[cfg(feature = "memmap")]
361-
let mmaps = archives
362-
.into_iter()
363-
.map(crate::utils::mmap::Mmap::try_from)
364-
.collect::<io::Result<Vec<_>>>()?;
365-
#[cfg(feature = "memmap")]
366-
let archives = mmaps.iter().map(|m| m.as_ref());
349+
let mut source = SplitArchiveReader::new(collect_split_archives(&args.file.archive)?)?;
367350

368351
let output_path = args.file.archive.remove_part();
369352
let mut temp_file =
370353
NamedTempFile::new(|| output_path.parent().unwrap_or_else(|| ".".as_ref()))?;
371354

372355
match args.transform_strategy.strategy() {
373-
SolidEntriesTransformStrategy::UnSolid => run_transform_entry(
356+
SolidEntriesTransformStrategy::UnSolid => source.transform_entries(
374357
temp_file.as_file_mut(),
375-
archives,
376-
|| password.as_deref(),
358+
password.as_deref(),
377359
#[hooq::skip_all]
378360
|entry| Ok(Some(set_strategy.transform_entry(entry?))),
379361
TransformStrategyUnSolid,
380362
),
381-
SolidEntriesTransformStrategy::KeepSolid => run_transform_entry(
363+
SolidEntriesTransformStrategy::KeepSolid => source.transform_entries(
382364
temp_file.as_file_mut(),
383-
archives,
384-
|| password.as_deref(),
365+
password.as_deref(),
385366
#[hooq::skip_all]
386367
|entry| Ok(Some(set_strategy.transform_entry(entry?))),
387368
TransformStrategyKeepSolid,
388369
),
389370
}?;
390371

391-
#[cfg(feature = "memmap")]
392-
drop(mmaps);
372+
drop(source);
393373

394374
temp_file.persist(output_path)?;
395375

cli/src/command/chmod.rs

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ use crate::{
33
command::{
44
Command, ask_password,
55
core::{
6-
TransformStrategyKeepSolid, TransformStrategyUnSolid, collect_split_archives,
7-
run_transform_entry,
6+
SplitArchiveReader, TransformStrategyKeepSolid, TransformStrategyUnSolid,
7+
collect_split_archives,
88
},
99
},
1010
utils::{GlobPatterns, PathPartExt, env::NamedTempFile},
@@ -50,25 +50,16 @@ fn archive_chmod(args: ChmodCommand) -> anyhow::Result<()> {
5050
}
5151
let mut globs = GlobPatterns::new(args.files.iter().map(|p| p.as_str()))?;
5252

53-
let archives = collect_split_archives(&args.archive)?;
54-
55-
#[cfg(feature = "memmap")]
56-
let mmaps = archives
57-
.into_iter()
58-
.map(crate::utils::mmap::Mmap::try_from)
59-
.collect::<std::io::Result<Vec<_>>>()?;
60-
#[cfg(feature = "memmap")]
61-
let archives = mmaps.iter().map(|m| m.as_ref());
53+
let mut source = SplitArchiveReader::new(collect_split_archives(&args.archive)?)?;
6254

6355
let output_path = args.archive.remove_part();
6456
let mut temp_file =
6557
NamedTempFile::new(|| output_path.parent().unwrap_or_else(|| ".".as_ref()))?;
6658

6759
match args.transform_strategy.strategy() {
68-
SolidEntriesTransformStrategy::UnSolid => run_transform_entry(
60+
SolidEntriesTransformStrategy::UnSolid => source.transform_entries(
6961
temp_file.as_file_mut(),
70-
archives,
71-
|| password.as_deref(),
62+
password.as_deref(),
7263
#[hooq::skip_all]
7364
|entry| {
7465
let entry = entry?;
@@ -80,10 +71,9 @@ fn archive_chmod(args: ChmodCommand) -> anyhow::Result<()> {
8071
},
8172
TransformStrategyUnSolid,
8273
),
83-
SolidEntriesTransformStrategy::KeepSolid => run_transform_entry(
74+
SolidEntriesTransformStrategy::KeepSolid => source.transform_entries(
8475
temp_file.as_file_mut(),
85-
archives,
86-
|| password.as_deref(),
76+
password.as_deref(),
8777
#[hooq::skip_all]
8878
|entry| {
8979
let entry = entry?;
@@ -97,8 +87,7 @@ fn archive_chmod(args: ChmodCommand) -> anyhow::Result<()> {
9787
),
9888
}?;
9989

100-
#[cfg(feature = "memmap")]
101-
drop(mmaps);
90+
drop(source);
10291

10392
temp_file.persist(output_path)?;
10493

cli/src/command/chown.rs

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ use crate::{
33
command::{
44
Command, ask_password,
55
core::{
6-
TransformStrategyKeepSolid, TransformStrategyUnSolid, collect_split_archives,
7-
run_transform_entry,
6+
SplitArchiveReader, TransformStrategyKeepSolid, TransformStrategyUnSolid,
7+
collect_split_archives,
88
},
99
},
1010
utils::{
@@ -59,25 +59,16 @@ fn archive_chown(args: ChownCommand) -> anyhow::Result<()> {
5959
.owner
6060
.lookup_platform_owner(args.numeric_owner, !args.no_owner_lookup)?;
6161

62-
let archives = collect_split_archives(&args.archive)?;
63-
64-
#[cfg(feature = "memmap")]
65-
let mmaps = archives
66-
.into_iter()
67-
.map(crate::utils::mmap::Mmap::try_from)
68-
.collect::<io::Result<Vec<_>>>()?;
69-
#[cfg(feature = "memmap")]
70-
let archives = mmaps.iter().map(|m| m.as_ref());
62+
let mut source = SplitArchiveReader::new(collect_split_archives(&args.archive)?)?;
7163

7264
let output_path = args.archive.remove_part();
7365
let mut temp_file =
7466
NamedTempFile::new(|| output_path.parent().unwrap_or_else(|| ".".as_ref()))?;
7567

7668
match args.transform_strategy.strategy() {
77-
SolidEntriesTransformStrategy::UnSolid => run_transform_entry(
69+
SolidEntriesTransformStrategy::UnSolid => source.transform_entries(
7870
temp_file.as_file_mut(),
79-
archives,
80-
|| password.as_deref(),
71+
password.as_deref(),
8172
#[hooq::skip_all]
8273
|entry| {
8374
let entry = entry?;
@@ -89,10 +80,9 @@ fn archive_chown(args: ChownCommand) -> anyhow::Result<()> {
8980
},
9081
TransformStrategyUnSolid,
9182
),
92-
SolidEntriesTransformStrategy::KeepSolid => run_transform_entry(
83+
SolidEntriesTransformStrategy::KeepSolid => source.transform_entries(
9384
temp_file.as_file_mut(),
94-
archives,
95-
|| password.as_deref(),
85+
password.as_deref(),
9686
#[hooq::skip_all]
9787
|entry| {
9888
let entry = entry?;
@@ -106,8 +96,7 @@ fn archive_chown(args: ChownCommand) -> anyhow::Result<()> {
10696
),
10797
}?;
10898

109-
#[cfg(feature = "memmap")]
110-
drop(mmaps);
99+
drop(source);
111100

112101
temp_file.persist(output_path)?;
113102

cli/src/command/core.rs

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
mod archive_source;
12
mod ignore;
23
pub(crate) mod iter;
34
#[cfg(unix)]
@@ -12,6 +13,7 @@ pub(crate) mod safe_writer;
1213
pub(crate) mod time_filter;
1314
pub(crate) mod timestamp;
1415

16+
pub(crate) use self::archive_source::SplitArchiveReader;
1517
pub(crate) use self::path::PathnameEditor;
1618
pub(crate) use self::permission::{ModeStrategy, OwnerOptions, OwnerStrategy, Umask};
1719
pub(crate) use self::safe_writer::SafeWriter;
@@ -1336,7 +1338,7 @@ where
13361338
}
13371339

13381340
#[cfg(feature = "memmap")]
1339-
pub(crate) fn run_transform_entry<'d, 'p, W, Provider, F, Transform>(
1341+
fn run_transform_entry<'d, 'p, W, Provider, F, Transform>(
13401342
writer: W,
13411343
archives: impl IntoIterator<Item = &'d [u8]>,
13421344
mut password_provider: Provider,
@@ -1373,7 +1375,7 @@ where
13731375
}
13741376

13751377
#[cfg(not(feature = "memmap"))]
1376-
pub(crate) fn run_transform_entry<'p, W, Provider, F, Transform>(
1378+
fn run_transform_entry<'p, W, Provider, F, Transform>(
13771379
writer: W,
13781380
archives: impl IntoIterator<Item = impl Read>,
13791381
mut password_provider: Provider,
@@ -1395,19 +1397,6 @@ where
13951397
Ok(())
13961398
}
13971399

1398-
#[cfg(not(feature = "memmap"))]
1399-
pub(crate) fn run_entries<'p, Provider, F>(
1400-
archives: Vec<fs::File>,
1401-
password_provider: Provider,
1402-
processor: F,
1403-
) -> io::Result<()>
1404-
where
1405-
Provider: FnMut() -> Option<&'p [u8]>,
1406-
F: FnMut(io::Result<NormalEntry>) -> io::Result<()>,
1407-
{
1408-
run_process_archive(archives, password_provider, processor)
1409-
}
1410-
14111400
pub(crate) fn write_split_archive(
14121401
archive: impl AsRef<Path>,
14131402
entries: impl Iterator<Item = io::Result<impl Entry + Sized>>,
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
#[cfg(feature = "memmap")]
2+
use std::borrow::Cow;
3+
use std::{fs, io};
4+
5+
use pna::{NormalEntry, ReadEntry};
6+
7+
use super::TransformStrategy;
8+
9+
pub(crate) struct SplitArchiveReader {
10+
#[cfg(feature = "memmap")]
11+
mmaps: Vec<crate::utils::mmap::Mmap>,
12+
#[cfg(not(feature = "memmap"))]
13+
files: Vec<fs::File>,
14+
}
15+
16+
impl SplitArchiveReader {
17+
pub(crate) fn new(files: Vec<fs::File>) -> io::Result<Self> {
18+
#[cfg(feature = "memmap")]
19+
{
20+
let mmaps = files
21+
.into_iter()
22+
.map(crate::utils::mmap::Mmap::try_from)
23+
.collect::<io::Result<Vec<_>>>()?;
24+
Ok(Self { mmaps })
25+
}
26+
#[cfg(not(feature = "memmap"))]
27+
{
28+
Ok(Self { files })
29+
}
30+
}
31+
32+
#[cfg(not(feature = "memmap"))]
33+
pub(crate) fn transform_entries<W, F, S>(
34+
&mut self,
35+
writer: W,
36+
password: Option<&[u8]>,
37+
processor: F,
38+
strategy: S,
39+
) -> anyhow::Result<()>
40+
where
41+
W: io::Write,
42+
F: FnMut(io::Result<NormalEntry>) -> io::Result<Option<NormalEntry>>,
43+
S: TransformStrategy,
44+
{
45+
super::run_transform_entry(
46+
writer,
47+
self.files.drain(..),
48+
|| password,
49+
processor,
50+
strategy,
51+
)
52+
}
53+
54+
#[cfg(feature = "memmap")]
55+
pub(crate) fn transform_entries<'s, W, F, S>(
56+
&'s mut self,
57+
writer: W,
58+
password: Option<&[u8]>,
59+
processor: F,
60+
strategy: S,
61+
) -> anyhow::Result<()>
62+
where
63+
W: io::Write,
64+
F: FnMut(
65+
io::Result<NormalEntry<Cow<'s, [u8]>>>,
66+
) -> io::Result<Option<NormalEntry<Cow<'s, [u8]>>>>,
67+
S: TransformStrategy,
68+
{
69+
super::run_transform_entry(
70+
writer,
71+
self.mmaps.iter().map(|m| m.as_ref()),
72+
|| password,
73+
processor,
74+
strategy,
75+
)
76+
}
77+
78+
#[cfg(not(feature = "memmap"))]
79+
pub(crate) fn for_each_entry(
80+
&mut self,
81+
password: Option<&[u8]>,
82+
processor: impl FnMut(io::Result<NormalEntry>) -> io::Result<()>,
83+
) -> io::Result<()> {
84+
super::run_process_archive(self.files.drain(..), || password, processor)
85+
}
86+
87+
#[cfg(feature = "memmap")]
88+
pub(crate) fn for_each_entry<'s>(
89+
&'s mut self,
90+
password: Option<&[u8]>,
91+
mut processor: impl FnMut(io::Result<NormalEntry<Cow<'s, [u8]>>>) -> io::Result<()>,
92+
) -> io::Result<()> {
93+
super::run_read_entries_mem(
94+
self.mmaps.iter().map(|m| m.as_ref()),
95+
|entry| match entry? {
96+
ReadEntry::Solid(s) => s
97+
.entries(password)?
98+
.try_for_each(|r| processor(r.map(Into::into))),
99+
ReadEntry::Normal(n) => processor(Ok(n)),
100+
},
101+
)
102+
}
103+
104+
#[cfg(not(feature = "memmap"))]
105+
pub(crate) fn for_each_read_entry(
106+
&mut self,
107+
processor: impl FnMut(io::Result<ReadEntry>) -> io::Result<()>,
108+
) -> io::Result<()> {
109+
super::run_read_entries(self.files.drain(..), processor)
110+
}
111+
112+
#[cfg(feature = "memmap")]
113+
pub(crate) fn for_each_read_entry<'s>(
114+
&'s mut self,
115+
processor: impl FnMut(io::Result<ReadEntry<Cow<'s, [u8]>>>) -> io::Result<()>,
116+
) -> io::Result<()> {
117+
super::run_read_entries_mem(self.mmaps.iter().map(|m| m.as_ref()), processor)
118+
}
119+
}

0 commit comments

Comments
 (0)