Skip to content

Commit 88364b1

Browse files
nrcnrc
authored
Merge pull request #1744 from rbtcollins/lesscopies
Less copying during dist installation
2 parents d587c8a + d74a13c commit 88364b1

File tree

4 files changed

+91
-47
lines changed

4 files changed

+91
-47
lines changed

‎src/dist/component/components.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,16 @@ impl<'a> ComponentBuilder<'a> {
109109
.push(ComponentPart("dir".to_owned(), path.clone()));
110110
self.tx.copy_dir(&self.name, path, src)
111111
}
112-
112+
pub fn move_file(&mut self, path: PathBuf, src: &Path) -> Result<()> {
113+
self.parts
114+
.push(ComponentPart("file".to_owned(), path.clone()));
115+
self.tx.move_file(&self.name, path, src)
116+
}
117+
pub fn move_dir(&mut self, path: PathBuf, src: &Path) -> Result<()> {
118+
self.parts
119+
.push(ComponentPart("dir".to_owned(), path.clone()));
120+
self.tx.move_dir(&self.name, path, src)
121+
}
113122
pub fn finish(mut self) -> Result<Transaction<'a>> {
114123
// Write component manifest
115124
let path = self.components.rel_component_manifest(&self.name);

‎src/dist/component/package.rs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,19 @@ pub trait Package: fmt::Debug {
3535
pub struct DirectoryPackage {
3636
path: PathBuf,
3737
components: HashSet<String>,
38+
copy: bool,
3839
}
3940

4041
impl DirectoryPackage {
41-
pub fn new(path: PathBuf) -> Result<Self> {
42+
pub fn new(path: PathBuf, copy: bool) -> Result<Self> {
4243
validate_installer_version(&path)?;
4344

4445
let content = utils::read_file("package components", &path.join("components"))?;
4546
let components = content.lines().map(|l| l.to_owned()).collect();
4647
Ok(DirectoryPackage {
4748
path: path,
4849
components: components,
50+
copy: copy,
4951
})
5052
}
5153
}
@@ -97,8 +99,20 @@ impl Package for DirectoryPackage {
9799
let src_path = root.join(&path);
98100

99101
match &*part.0 {
100-
"file" => builder.copy_file(path.clone(), &src_path)?,
101-
"dir" => builder.copy_dir(path.clone(), &src_path)?,
102+
"file" => {
103+
if self.copy {
104+
builder.copy_file(path.clone(), &src_path)?
105+
} else {
106+
builder.move_file(path.clone(), &src_path)?
107+
}
108+
}
109+
"dir" => {
110+
if self.copy {
111+
builder.copy_dir(path.clone(), &src_path)?
112+
} else {
113+
builder.move_dir(path.clone(), &src_path)?
114+
}
115+
}
102116
_ => return Err(ErrorKind::CorruptComponent(name.to_owned()).into()),
103117
}
104118

@@ -187,7 +201,7 @@ impl<'a> TarPackage<'a> {
187201
unpack_without_first_dir(&mut archive, &*temp_dir)?;
188202

189203
Ok(TarPackage(
190-
DirectoryPackage::new(temp_dir.to_owned())?,
204+
DirectoryPackage::new(temp_dir.to_owned(), false)?,
191205
temp_dir,
192206
))
193207
}

‎src/dist/component/transaction.rs

Lines changed: 55 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,22 @@ impl<'a> Transaction<'a> {
133133
Ok(())
134134
}
135135

136+
/// Move a file to a relative path of the install prefix.
137+
pub fn move_file(&mut self, component: &str, relpath: PathBuf, src: &Path) -> Result<()> {
138+
assert!(relpath.is_relative());
139+
let item = ChangedItem::move_file(&self.prefix, component, relpath, src)?;
140+
self.change(item);
141+
Ok(())
142+
}
143+
144+
/// Recursively move a directory to a relative path of the install prefix.
145+
pub fn move_dir(&mut self, component: &str, relpath: PathBuf, src: &Path) -> Result<()> {
146+
assert!(relpath.is_relative());
147+
let item = ChangedItem::move_dir(&self.prefix, component, relpath, src)?;
148+
self.change(item);
149+
Ok(())
150+
}
151+
136152
pub fn temp(&self) -> &'a temp::Cfg {
137153
self.temp_cfg
138154
}
@@ -195,8 +211,12 @@ impl<'a> ChangedItem<'a> {
195211
}
196212
Ok(())
197213
}
198-
fn add_file(prefix: &InstallPrefix, component: &str, relpath: PathBuf) -> Result<(Self, File)> {
199-
let abs_path = prefix.abs_path(&relpath);
214+
fn dest_abs_path(
215+
prefix: &InstallPrefix,
216+
component: &str,
217+
relpath: &PathBuf,
218+
) -> Result<PathBuf> {
219+
let abs_path = prefix.abs_path(relpath);
200220
if utils::path_exists(&abs_path) {
201221
Err(ErrorKind::ComponentConflict {
202222
name: component.to_owned(),
@@ -207,53 +227,34 @@ impl<'a> ChangedItem<'a> {
207227
if let Some(p) = abs_path.parent() {
208228
utils::ensure_dir_exists("component", p, &|_| ())?;
209229
}
210-
let file = File::create(&abs_path)
211-
.chain_err(|| format!("error creating file '{}'", abs_path.display()))?;
212-
213-
Ok((ChangedItem::AddedFile(relpath), file))
230+
Ok(abs_path)
214231
}
215232
}
233+
fn add_file(prefix: &InstallPrefix, component: &str, relpath: PathBuf) -> Result<(Self, File)> {
234+
let abs_path = ChangedItem::dest_abs_path(prefix, component, &relpath)?;
235+
let file = File::create(&abs_path)
236+
.chain_err(|| format!("error creating file '{}'", abs_path.display()))?;
237+
Ok((ChangedItem::AddedFile(relpath), file))
238+
}
216239
fn copy_file(
217240
prefix: &InstallPrefix,
218241
component: &str,
219242
relpath: PathBuf,
220243
src: &Path,
221244
) -> Result<Self> {
222-
let abs_path = prefix.abs_path(&relpath);
223-
if utils::path_exists(&abs_path) {
224-
Err(ErrorKind::ComponentConflict {
225-
name: component.to_owned(),
226-
path: relpath.clone(),
227-
}
228-
.into())
229-
} else {
230-
if let Some(p) = abs_path.parent() {
231-
utils::ensure_dir_exists("component", p, &|_| ())?;
232-
}
233-
utils::copy_file(src, &abs_path)?;
234-
Ok(ChangedItem::AddedFile(relpath))
235-
}
245+
let abs_path = ChangedItem::dest_abs_path(prefix, component, &relpath)?;
246+
utils::copy_file(src, &abs_path)?;
247+
Ok(ChangedItem::AddedFile(relpath))
236248
}
237249
fn copy_dir(
238250
prefix: &InstallPrefix,
239251
component: &str,
240252
relpath: PathBuf,
241253
src: &Path,
242254
) -> Result<Self> {
243-
let abs_path = prefix.abs_path(&relpath);
244-
if utils::path_exists(&abs_path) {
245-
Err(ErrorKind::ComponentConflict {
246-
name: component.to_owned(),
247-
path: relpath.clone(),
248-
}
249-
.into())
250-
} else {
251-
if let Some(p) = abs_path.parent() {
252-
utils::ensure_dir_exists("component", p, &|_| ())?;
253-
}
254-
utils::copy_dir(src, &abs_path, &|_| ())?;
255-
Ok(ChangedItem::AddedDir(relpath))
256-
}
255+
let abs_path = ChangedItem::dest_abs_path(prefix, component, &relpath)?;
256+
utils::copy_dir(src, &abs_path, &|_| ())?;
257+
Ok(ChangedItem::AddedDir(relpath))
257258
}
258259
fn remove_file(
259260
prefix: &InstallPrefix,
@@ -311,4 +312,24 @@ impl<'a> ChangedItem<'a> {
311312
Ok(ChangedItem::ModifiedFile(relpath, None))
312313
}
313314
}
315+
fn move_file(
316+
prefix: &InstallPrefix,
317+
component: &str,
318+
relpath: PathBuf,
319+
src: &Path,
320+
) -> Result<Self> {
321+
let abs_path = ChangedItem::dest_abs_path(prefix, component, &relpath)?;
322+
utils::rename_file("component", src, &abs_path)?;
323+
Ok(ChangedItem::AddedFile(relpath))
324+
}
325+
fn move_dir(
326+
prefix: &InstallPrefix,
327+
component: &str,
328+
relpath: PathBuf,
329+
src: &Path,
330+
) -> Result<Self> {
331+
let abs_path = ChangedItem::dest_abs_path(prefix, component, &relpath)?;
332+
utils::rename_dir("component", src, &abs_path)?;
333+
Ok(ChangedItem::AddedDir(relpath))
334+
}
314335
}

‎tests/dist_install.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ fn package_contains() {
6767

6868
mock.build(tempdir.path());
6969

70-
let package = DirectoryPackage::new(tempdir.path().to_owned()).unwrap();
70+
let package = DirectoryPackage::new(tempdir.path().to_owned(), true).unwrap();
7171
assert!(package.contains("mycomponent", None));
7272
assert!(package.contains("mycomponent2", None));
7373
}
@@ -88,7 +88,7 @@ fn package_bad_version() {
8888
let mut ver = File::create(tempdir.path().join("rust-installer-version")).unwrap();
8989
writeln!(ver, "100").unwrap();
9090

91-
assert!(DirectoryPackage::new(tempdir.path().to_owned()).is_err());
91+
assert!(DirectoryPackage::new(tempdir.path().to_owned(), true).is_err());
9292
}
9393

9494
#[test]
@@ -122,7 +122,7 @@ fn basic_install() {
122122

123123
let components = Components::open(prefix.clone()).unwrap();
124124

125-
let pkg = DirectoryPackage::new(pkgdir.path().to_owned()).unwrap();
125+
let pkg = DirectoryPackage::new(pkgdir.path().to_owned(), true).unwrap();
126126

127127
let tx = pkg.install(&components, "mycomponent", None, tx).unwrap();
128128
tx.commit();
@@ -168,7 +168,7 @@ fn multiple_component_install() {
168168

169169
let components = Components::open(prefix.clone()).unwrap();
170170

171-
let pkg = DirectoryPackage::new(pkgdir.path().to_owned()).unwrap();
171+
let pkg = DirectoryPackage::new(pkgdir.path().to_owned(), true).unwrap();
172172

173173
let tx = pkg.install(&components, "mycomponent", None, tx).unwrap();
174174
let tx = pkg.install(&components, "mycomponent2", None, tx).unwrap();
@@ -218,7 +218,7 @@ fn uninstall() {
218218

219219
let components = Components::open(prefix.clone()).unwrap();
220220

221-
let pkg = DirectoryPackage::new(pkgdir.path().to_owned()).unwrap();
221+
let pkg = DirectoryPackage::new(pkgdir.path().to_owned(), true).unwrap();
222222

223223
let tx = pkg.install(&components, "mycomponent", None, tx).unwrap();
224224
let tx = pkg.install(&components, "mycomponent2", None, tx).unwrap();
@@ -275,7 +275,7 @@ fn component_bad_version() {
275275

276276
let components = Components::open(prefix.clone()).unwrap();
277277

278-
let pkg = DirectoryPackage::new(pkgdir.path().to_owned()).unwrap();
278+
let pkg = DirectoryPackage::new(pkgdir.path().to_owned(), true).unwrap();
279279

280280
let tx = pkg.install(&components, "mycomponent", None, tx).unwrap();
281281
tx.commit();
@@ -336,7 +336,7 @@ fn unix_permissions() {
336336

337337
let components = Components::open(prefix.clone()).unwrap();
338338

339-
let pkg = DirectoryPackage::new(pkgdir.path().to_owned()).unwrap();
339+
let pkg = DirectoryPackage::new(pkgdir.path().to_owned(), true).unwrap();
340340

341341
let tx = pkg.install(&components, "mycomponent", None, tx).unwrap();
342342
tx.commit();
@@ -421,7 +421,7 @@ fn install_to_prefix_that_does_not_exist() {
421421

422422
let components = Components::open(prefix.clone()).unwrap();
423423

424-
let pkg = DirectoryPackage::new(pkgdir.path().to_owned()).unwrap();
424+
let pkg = DirectoryPackage::new(pkgdir.path().to_owned(), true).unwrap();
425425

426426
let tx = pkg.install(&components, "mycomponent", None, tx).unwrap();
427427
tx.commit();

0 commit comments

Comments
 (0)