Skip to content
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

Clean build previous artifacts after successful build #69

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ changelog
This changelog follows the patterns described here: https://keepachangelog.com/en/1.0.0/.

## Unreleased
### added
- Closed [#49](https://github.com/thedodd/trunk/issues/49): Clean build previous artifacts after successful build

## 0.6.0
### added
Expand Down
31 changes: 29 additions & 2 deletions src/pipelines/wasmbg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use indicatif::ProgressBar;
use crate::common::copy_dir_recursive;
use crate::config::RtcBuild;
use crate::pipelines::cargo::CargoBuildOutput;
use futures::StreamExt;

const SNIPPETS_DIR: &str = "snippets";

Expand Down Expand Up @@ -41,12 +42,14 @@ impl WasmBindgen {
self.progress.set_message("awaiting cargo build");
let cargo = cargo.await?;
self.progress.set_message("executing");
let arg_out_path = format!("--out-dir={}", self.bindgen_out.display());
// build to `dist/.current`
let bindgen_out = self.bindgen_out.join(PathBuf::from(".current"));
let arg_out_path = format!("--out-dir={}", bindgen_out.display());
let arg_out_name = format!("--out-name={}", &cargo.hashed_name);
let target_wasm = cargo.wasm.to_string_lossy().to_string();

// Ensure our output dir is in place.
fs::create_dir_all(self.bindgen_out.as_path())
fs::create_dir_all(bindgen_out.as_path())
.await
.context("error creating wasm-bindgen output dir")?;

Expand All @@ -67,6 +70,30 @@ impl WasmBindgen {
String::from_utf8_lossy(&build_output.stderr),
);

// wasm-bindgen build succeeded so delete everything in `dist`,
// copy everything from `dist/.current` to `dist` and
// delete `dist/.current`
let mut entries = fs::read_dir(self.bindgen_out.as_path()).await?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be walking target/wasm-bindgen/debug instead of dist.

while let Some(res) = entries.next().await {
let entry = res?;
if entry.path().to_string_lossy() != bindgen_out.to_string_lossy() {
let path = entry.path();
if path.is_dir().await {
fs::remove_dir_all(&path).await.context("Deleting directory after build failed")?
Copy link
Contributor

@MartinKavik MartinKavik Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't reliable on Windows. We should use something like https://github.com/XAMPPRocky/remove_dir_all.
See https://blog.qwaz.io/chat/issues-of-rusts-remove-dir-all-implementation-on-windows for more info.

Note: The comment is valid for all fs::remove_dir_all calls in the code-base.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MartinKavik do you know if that applies to just the std implementation, or also to the async_std::remove_dir_all implementation?

Copy link
Contributor

@rakshith-ravi rakshith-ravi Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the async-std source code (https://github.com/async-rs/async-std/blob/master/src/fs/remove_dir_all.rs#L31-L38), it doesn't seem to do that. It just uses std::fs implementation directly, and exposes it in an async way. So I don't think it would work in async-std either

} else {
fs::remove_file(&path).await.context("Deleting file after build failed")?
};
}
}

copy_dir_recursive(bindgen_out.clone(), self.bindgen_out.to_path_buf())
.await
.context("Error occurred while copying from `dist/.current` to `dist`")?;

fs::remove_dir_all(bindgen_out.as_path())
.await
.context("Error occurred while deleting `dist/.current`")?;

// Copy the generated WASM & JS loader to the dist dir.
self.progress.set_message("copying generated artifacts");
let hashed_js_name = format!("{}.js", &cargo.hashed_name);
Expand Down