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

feat: improve path display and replacement during script execution #1115

Merged
merged 2 commits into from
Oct 13, 2024
Merged
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
1 change: 0 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,6 @@ pub async fn run_build_from_args(
output
}
Err(e) => {
tracing::error!("Error building package: {}", e);
return Err(e);
}
};
Expand Down
9 changes: 6 additions & 3 deletions src/package_test/run_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,13 +320,16 @@ pub async fn run_test(
let package_folder = cache_dir.join("pkgs").join(cache_key.to_string());

if package_folder.exists() {
tracing::info!("Removing previously cached package {:?}", &package_folder);
tracing::info!(
"Removing previously cached package '{}'",
package_folder.display()
);
fs::remove_dir_all(&package_folder)?;
}

let prefix = canonicalize(&config.test_prefix)?;

tracing::info!("Creating test environment in {:?}", prefix);
tracing::info!("Creating test environment in '{}'", prefix.display());

let mut channels = config.channels.clone();
channels.insert(0, Channel::from_directory(tmp_repo.path()).base_url);
Expand All @@ -349,7 +352,7 @@ pub async fn run_test(
..config.clone()
};

tracing::info!("Collecting tests from {:?}", package_folder);
tracing::info!("Collecting tests from '{}'", package_folder.display());

rattler_package_streaming::fs::extract(package_file, &package_folder).map_err(|e| {
tracing::error!("Failed to extract package: {:?}", e);
Expand Down
11 changes: 7 additions & 4 deletions src/packaging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,17 +259,20 @@ pub fn package_conda(
(false, true) => std::cmp::Ordering::Less,
}
});

let normalize_path = |p: &Path| -> String { p.display().to_string().replace('\\', "/") };

files.iter().for_each(|f| {
if f.components().next() == Some(Component::Normal("info".as_ref())) {
tracing::info!(" - {}", console::style(f.to_string_lossy()).dim())
tracing::info!(" - {}", console::style(normalize_path(f)).dim())
} else {
tracing::info!(" - {}", f.to_string_lossy())
tracing::info!(" - {}", normalize_path(f))
}
});

let output_folder =
local_channel_dir.join(output.build_configuration.target_platform.to_string());
tracing::info!("Creating target folder {:?}", output_folder);
tracing::info!("Creating target folder '{}'", output_folder.display());

fs::create_dir_all(&output_folder)?;

Expand Down Expand Up @@ -321,7 +324,7 @@ pub fn package_conda(
}
}

tracing::info!("Archive written to {:?}", out_path);
tracing::info!("Archive written to '{}'", out_path.display());

let paths_json = PathsJson::from_path(info_folder.join("paths.json"))?;
Ok((out_path, paths_json))
Expand Down
50 changes: 30 additions & 20 deletions src/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,27 @@ impl ExecutionArgs {
let mut replacements = HashMap::new();
if let Some(build_prefix) = &self.build_prefix {
replacements.insert(
build_prefix.to_string_lossy().to_string(),
build_prefix.display().to_string(),
template.replace("((var))", "BUILD_PREFIX"),
);
};
replacements.insert(
self.run_prefix.to_string_lossy().to_string(),
self.run_prefix.display().to_string(),
template.replace("((var))", "PREFIX"),
);

replacements.insert(
self.work_dir.display().to_string(),
template.replace("((var))", "SRC_DIR"),
);

// if the paths contain `\` then also replace the forward slash variants
for (k, v) in replacements.clone() {
if k.contains('\\') {
replacements.insert(k.replace('\\', "/"), v.clone());
}
}

self.secrets.iter().for_each(|(_, v)| {
replacements.insert(v.to_string(), "********".to_string());
});
Expand Down Expand Up @@ -182,14 +194,13 @@ impl Interpreter for BashInterpreter {
.await?;

if !output.status.success() {
let status_code = output.status.code().unwrap_or(1);
tracing::error!("Script failed with status {}", status_code);
tracing::error!("Work directory: '{}'", args.work_dir.display());
tracing::error!("{}", DEBUG_HELP);
return Err(std::io::Error::new(
std::io::ErrorKind::Other,
format!(
"Script failed with status {:?}.\nWork directory: {:?}\n{}",
output.status.code(),
args.work_dir,
DEBUG_HELP
),
"Script failed".to_string(),
));
}

Expand Down Expand Up @@ -317,14 +328,13 @@ impl Interpreter for NuShellInterpreter {
.await?;

if !output.status.success() {
let status_code = output.status.code().unwrap_or(1);
tracing::error!("Script failed with status {}", status_code);
tracing::error!("Work directory: '{}'", args.work_dir.display());
tracing::error!("{}", DEBUG_HELP);
return Err(std::io::Error::new(
std::io::ErrorKind::Other,
format!(
"Script failed with status {:?}.\nWork directory: {:?}\n{}",
output.status.code(),
args.work_dir,
DEBUG_HELP
),
"Script failed".to_string(),
));
}

Expand Down Expand Up @@ -383,14 +393,13 @@ impl Interpreter for CmdExeInterpreter {
.await?;

if !output.status.success() {
let status_code = output.status.code().unwrap_or(1);
tracing::error!("Script failed with status {}", status_code);
tracing::error!("Work directory: '{}'", args.work_dir.display());
tracing::error!("{}", DEBUG_HELP);
return Err(std::io::Error::new(
std::io::ErrorKind::Other,
format!(
"Script failed with status {:?}.\nWork directory: {:?}\n{}",
output.status.code(),
args.work_dir,
DEBUG_HELP
),
"Script failed".to_string(),
));
}

Expand Down Expand Up @@ -772,6 +781,7 @@ async fn run_process_with_replacements(
let mut stdout_log = String::new();
let mut stderr_log = String::new();
let mut closed = (false, false);

loop {
let (line, is_stderr) = tokio::select! {
line = stdout_lines.next_line() => (line, false),
Expand Down
27 changes: 18 additions & 9 deletions src/source/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,11 @@ pub async fn fetch_sources(

// Copy source code to work dir
if res.is_dir() {
tracing::info!("Copying source from url: {:?} to {:?}", res, dest_dir);
tracing::info!(
"Copying source from url: {} to {}",
res.display(),
dest_dir.display()
);
tool_configuration.fancy_log_handler.wrap_in_progress(
"copying source into isolated environment",
|| {
Expand All @@ -181,7 +185,11 @@ pub async fn fetch_sources(
},
)?;
} else {
tracing::info!("Copying source from url: {:?} to {:?}", res, dest_dir);
tracing::info!(
"Copying source from url: {} to {}",
res.display(),
dest_dir.display()
);

let file_name = src.file_name().unwrap_or(&file_name_from_url);
let target = dest_dir.join(file_name);
Expand All @@ -196,7 +204,7 @@ pub async fn fetch_sources(
}
Source::Path(src) => {
let src_path = recipe_dir.join(src.path()).canonicalize()?;
tracing::info!("Fetching source from path: {:?}", src_path);
tracing::info!("Fetching source from path: {}", src_path.display());

let dest_dir = if let Some(target_directory) = src.target_directory() {
work_dir.join(target_directory)
Expand Down Expand Up @@ -235,26 +243,27 @@ pub async fn fetch_sources(
.as_ref(),
) {
extract_tar(&src_path, &dest_dir, &tool_configuration.fancy_log_handler)?;
tracing::info!("Extracted to {:?}", dest_dir);
tracing::info!("Extracted to {}", dest_dir.display());
} else if src_path.extension() == Some(OsStr::new("zip")) {
extract_zip(&src_path, &dest_dir, &tool_configuration.fancy_log_handler)?;
tracing::info!("Extracted zip to {:?}", dest_dir);
tracing::info!("Extracted zip to {}", dest_dir.display());
} else if let Some(file_name) = src
.file_name()
.cloned()
.or_else(|| src_path.file_name().map(PathBuf::from))
{
let dest = dest_dir.join(&file_name);
tracing::info!(
"Copying source from path: {:?} to {:?}",
src_path,
dest_dir.join(&file_name)
"Copying source from path: {} to {}",
src_path.display(),
dest.display()
);
if let Some(checksum) = Checksum::from_path_source(src) {
if !checksum.validate(&src_path) {
return Err(SourceError::ValidationFailed);
}
}
fs::copy(&src_path, dest_dir.join(file_name))?;
fs::copy(&src_path, dest)?;
} else {
return Err(SourceError::FileNotFound(src_path));
}
Expand Down
6 changes: 3 additions & 3 deletions src/source/url_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ fn extract_to_cache(
let target = extracted_folder(path);

if target.is_dir() {
tracing::info!("Using extracted directory from cache: {:?}", target);
tracing::info!("Using extracted directory from cache: {}", target.display());
return Ok(target);
}

Expand All @@ -131,11 +131,11 @@ fn extract_to_cache(
.to_string_lossy()
.as_ref(),
) {
tracing::info!("Extracting tar file to cache: {:?}", path);
tracing::info!("Extracting tar file to cache: {}", path.display());
extract_tar(path, &target, &tool_configuration.fancy_log_handler)?;
return Ok(target);
} else if path.extension() == Some(OsStr::new("zip")) {
tracing::info!("Extracting zip file to cache: {:?}", path);
tracing::info!("Extracting zip file to cache: {}", path.display());
extract_zip(path, &target, &tool_configuration.fancy_log_handler)?;
return Ok(target);
}
Expand Down
Loading