Skip to content

Commit

Permalink
apply reviewer's suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
Marcin Nowak-Liebiediew committed Aug 8, 2023
1 parent 515c2f2 commit 283849b
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 39 deletions.
14 changes: 12 additions & 2 deletions extensions-utils/src/dependencies/call.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use anyhow::Context;
use fn_error_context::context;
use std::{
env,
ffi::OsStr,
path::Path,
process::{self, Command},
Expand All @@ -11,6 +12,8 @@ use std::{
/// # Returns
/// - On success, returns stdout as a string.
/// - On error, returns an error message including stdout and stderr.
///
/// Does not stream stdout/stderr, and instead returns it after the process has exited.
#[context("Calling {} CLI failed, or, it returned an error.", binary_name)]
pub fn call_extension_bundled_binary<S, I>(
dfx_cache_path: &Path,
Expand All @@ -30,9 +33,16 @@ where
)
})?;
let binary_to_call = extension_dir_path.join(binary_name);
let mut command = Command::new(&binary_to_call);
let mut command = Command::new(binary_to_call);
// If extension's dependency calls dfx; it should call dfx in this dir.
command.env("PATH", dfx_cache_path.join("dfx"));
if let Some(path) = env::var_os("PATH") {
let mut paths = env::split_paths(&path).collect::<Vec<_>>();
paths.push(dfx_cache_path.to_path_buf());
let new_path = env::join_paths(paths)?;
command.env("PATH", new_path);
} else {
command.env("PATH", dfx_cache_path);
}
command.args(args);
let output = command
.stdin(process::Stdio::null())
Expand Down
16 changes: 13 additions & 3 deletions extensions-utils/src/dependencies/dfx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use anyhow::Context;
use fn_error_context::context;
use semver::Version;

use std::env;
use std::ffi::OsStr;
use std::path::Path;
use std::process::{self, Command};
Expand All @@ -12,6 +13,8 @@ use std::process::{self, Command};
/// # Returns
/// - On success, returns stdout as a string.
/// - On error, returns an error message including stdout and stderr.
///
/// Does not stream stdout/stderr, and instead returns it after the process has exited.
#[context("Calling {} CLI, or, it returned an error.", command)]
pub fn call_dfx_bundled_binary<S, I>(
dfx_cache_path: &Path,
Expand All @@ -23,9 +26,16 @@ where
S: AsRef<OsStr>,
{
let binary = dfx_cache_path.join(command);
let mut command = Command::new(&binary);
let mut command = Command::new(binary);
// If extension's dependency calls dfx; it should call dfx in this dir.
command.env("PATH", dfx_cache_path.join("dfx"));
if let Some(path) = env::var_os("PATH") {
let mut paths = env::split_paths(&path).collect::<Vec<_>>();
paths.push(dfx_cache_path.to_path_buf());
let new_path = env::join_paths(paths)?;
command.env("PATH", new_path);
} else {
command.env("PATH", dfx_cache_path);
}
command.args(args);
let output = command
.stdin(process::Stdio::null())
Expand Down Expand Up @@ -87,7 +97,7 @@ pub fn dfx_version(dfx_cache_path: &Path) -> Result<String, DfxError> {
.map(|c| *c as char)
.collect::<String>();
if let Some(version) = version_cmd_output.split_whitespace().last() {
Version::parse(&version) // make sure the output is really a version
Version::parse(version) // make sure the output is really a version
.map_err(DfxError::DfxVersionMalformed)
.map(|v| v.to_string())
} else {
Expand Down
4 changes: 2 additions & 2 deletions extensions-utils/src/dependencies/download_ic_binaries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ pub fn download_ic_binary(replica_rev: &str, binary_name: &str, destination_path
let mut temp = fs::File::create(&temp_file).expect("Failed to create the file");
copy(&mut d, &mut temp).expect("Failed to copy content");

fs::rename(temp_file, &destination_path).expect("Failed to move extension");
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
dfx_core::fs::set_permissions(&destination_path, std::fs::Permissions::from_mode(0o500))
dfx_core::fs::set_permissions(&temp_file, std::fs::Permissions::from_mode(0o500))
.expect("Failed to set permissions");
}
fs::rename(temp_file, destination_path).expect("Failed to move extension");
}

async fn download_bytes(url: &str) -> Vec<u8> {
Expand Down
30 changes: 15 additions & 15 deletions extensions/nns/build.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::env;
use std::path::PathBuf;

const REPLICA_REV: &str = "90e2799c255733409d0e61682685afcc2431c928";
Expand All @@ -11,26 +12,25 @@ const BINARY_DEPENDENCIES: &[(&str, &str)] = &[

fn main() {
// keep copy of the dependency in the root of the project, so that cargo-dist will be able to package it into a tarball
let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
// and also in `target/debug` or `target/release` for development purposes (e.g. cargo run)
let target_dir = manifest_dir
let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap());
// and also in `target/debug` or `target/release` for development purposes (e.g. cargo run), this is a bit hacky: https://github.com/rust-lang/cargo/issues/9661
let target_dir = PathBuf::from(std::env::var("OUT_DIR").unwrap())
.parent()
.unwrap()
.parent()
.unwrap()
.join("target")
.join(std::env::var("PROFILE").unwrap());
.parent()
.unwrap()
.to_path_buf();
for (binary_name, renamed_binary_name) in BINARY_DEPENDENCIES {
let destination_paths = (
manifest_dir.join(renamed_binary_name),
target_dir.join(renamed_binary_name),
);
dbg!(&destination_paths);
dfx_extensions_utils::download_ic_binary(REPLICA_REV, binary_name, &destination_paths.0);
if destination_paths.1.exists() {
std::fs::remove_file(&destination_paths.1).unwrap();
let bin_in_manifest_dir = manifest_dir.join(renamed_binary_name);
let bin_in_target_dir = target_dir.join(renamed_binary_name);
dbg!(&bin_in_manifest_dir, &bin_in_target_dir);
dfx_extensions_utils::download_ic_binary(REPLICA_REV, binary_name, &bin_in_manifest_dir);
if bin_in_target_dir.exists() {
std::fs::remove_file(&bin_in_target_dir).unwrap();
}
std::fs::create_dir_all(target_dir).unwrap();
std::fs::copy(destination_paths.0, destination_paths.1).unwrap();
std::fs::create_dir_all(bin_in_target_dir.parent().unwrap()).unwrap();
std::fs::copy(&bin_in_manifest_dir, &bin_in_target_dir).unwrap();
}
}
3 changes: 1 addition & 2 deletions extensions/nns/src/install_nns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,6 @@ pub async fn ic_nns_init(opts: &IcNnsInitOpts, dfx_cache_path: &Path) -> anyhow:
args.push(subnets.into());
}
call_extension_bundled_binary(dfx_cache_path, "ic-nns-init", &args)
.with_context(|| format!("Error executing `ic-nns-init` with args: {:?}", args))
}

/// Sets the exchange rate between ICP and cycles.
Expand Down Expand Up @@ -506,7 +505,7 @@ pub fn upload_nns_sns_wasms_canister_wasms(dfx_cache_path: &Path) -> anyhow::Res
];
call_extension_bundled_binary(dfx_cache_path,"sns-cli", &args)
.map_err(|e| anyhow!(
"Failed to upload {upload_name} from {wasm_path:?} to the nns-sns-wasm canister by calling `sns-cli` with args {args:?}: {e}"
"Failed to upload {upload_name} from {wasm_path:?} to the nns-sns-wasm canister by calling `sns-cli`: {e}"
))?;
}
Ok(())
Expand Down
30 changes: 15 additions & 15 deletions extensions/sns/build.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::env;
use std::path::PathBuf;

const REPLICA_REV: &str = "90e2799c255733409d0e61682685afcc2431c928";
Expand All @@ -9,26 +10,25 @@ const BINARY_DEPENDENCIES: &[(&str, &str)] = &[

fn main() {
// keep copy of the dependency in the root of the project, so that cargo-dist will be able to package it into a tarball
let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
// and also in `target/debug` or `target/release` for development purposes (e.g. cargo run)
let target_dir = manifest_dir
let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap());
// and also in `target/debug` or `target/release` for development purposes (e.g. cargo run), this is a bit hacky: https://github.com/rust-lang/cargo/issues/9661
let target_dir = PathBuf::from(std::env::var("OUT_DIR").unwrap())
.parent()
.unwrap()
.parent()
.unwrap()
.join("target")
.join(std::env::var("PROFILE").unwrap());
.parent()
.unwrap()
.to_path_buf();
for (binary_name, renamed_binary_name) in BINARY_DEPENDENCIES {
let destination_paths = (
manifest_dir.join(renamed_binary_name),
target_dir.join(renamed_binary_name),
);
dbg!(&destination_paths);
dfx_extensions_utils::download_ic_binary(REPLICA_REV, binary_name, &destination_paths.0);
if destination_paths.1.exists() {
std::fs::remove_file(&destination_paths.1).unwrap();
let bin_in_manifest_dir = manifest_dir.join(renamed_binary_name);
let bin_in_target_dir = target_dir.join(renamed_binary_name);
dbg!(&bin_in_manifest_dir, &bin_in_target_dir);
dfx_extensions_utils::download_ic_binary(REPLICA_REV, binary_name, &bin_in_manifest_dir);
if bin_in_target_dir.exists() {
std::fs::remove_file(&bin_in_target_dir).unwrap();
}
std::fs::create_dir_all(destination_paths.1.parent().unwrap()).unwrap();
std::fs::copy(destination_paths.0, destination_paths.1).unwrap();
std::fs::create_dir_all(bin_in_target_dir.parent().unwrap()).unwrap();
std::fs::copy(&bin_in_manifest_dir, &bin_in_target_dir).unwrap();
}
}

0 comments on commit 283849b

Please sign in to comment.