From 23b4d49ce8de5228110a6a572ccf805077d2994e Mon Sep 17 00:00:00 2001 From: ea-open-source Date: Thu, 26 May 2022 17:14:36 +0700 Subject: [PATCH 1/8] Add subcommand login, read token from user # Conflicts: # Cargo.lock # language/tools/move-cli/Cargo.toml --- Cargo.lock | 63 +++++++++++++++++++ language/tools/move-cli/Cargo.toml | 2 + language/tools/move-cli/src/lib.rs | 4 ++ language/tools/move-cli/src/login/cli.rs | 55 ++++++++++++++++ language/tools/move-cli/src/login/mod.rs | 1 + .../src/source_package/manifest_parser.rs | 1 + 6 files changed, 126 insertions(+) create mode 100644 language/tools/move-cli/src/login/cli.rs create mode 100644 language/tools/move-cli/src/login/mod.rs diff --git a/Cargo.lock b/Cargo.lock index 4820df75ca..461ced2fb8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -634,6 +634,16 @@ dependencies = [ "itertools 0.10.1", ] +[[package]] +name = "combine" +version = "4.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a604e93b79d1808327a6fca85a6f2d69de66461e7620f5a4cbf5fb4d1d7c948" +dependencies = [ + "bytes", + "memchr", +] + [[package]] name = "config" version = "0.11.0" @@ -1763,6 +1773,34 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "92620684d99f750bae383ecb3be3748142d6095760afd5cbcf2261e9a279d780" +[[package]] +name = "hakari" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f48905a7968260802691e8b6cc2e9426822b79eaf9164d07f2212e9327181b3e" +dependencies = [ + "atomicwrites", + "bimap", + "camino", + "cfg-if 1.0.0", + "debug-ignore", + "diffy", + "guppy", + "guppy-workspace-hack", + "include_dir", + "indenter", + "itertools 0.10.1", + "owo-colors", + "pathdiff", + "rayon", + "serde 1.0.130", + "strip-ansi-escapes", + "target-spec", + "toml", + "toml_edit 0.10.1", + "twox-hash", +] + [[package]] name = "half" version = "1.7.1" @@ -2443,6 +2481,7 @@ dependencies = [ "serde 1.0.130", "serde_yaml", "tempfile", + "toml_edit 0.14.4", "walkdir", ] @@ -5171,6 +5210,30 @@ dependencies = [ "serde 1.0.130", ] +[[package]] +name = "toml_edit" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2782eb01c48f7f6f66d63bebe64dd8302dbf6bc0be0d65a85a1854d472e2dfa8" +dependencies = [ + "combine", + "indexmap", + "itertools 0.10.1", + "kstring", +] + +[[package]] +name = "toml_edit" +version = "0.14.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5376256e44f2443f8896ac012507c19a012df0fe8758b55246ae51a2279db51f" +dependencies = [ + "combine", + "indexmap", + "itertools 0.10.1", + "serde 1.0.130", +] + [[package]] name = "tracing" version = "0.1.26" diff --git a/language/tools/move-cli/Cargo.toml b/language/tools/move-cli/Cargo.toml index efaca163b1..e060a4133e 100644 --- a/language/tools/move-cli/Cargo.toml +++ b/language/tools/move-cli/Cargo.toml @@ -21,6 +21,8 @@ tempfile = "3.2.0" walkdir = "2.3.1" codespan-reporting = "0.11.1" itertools = "0.10.0" +toml_edit = { version = "0.14.3", features = ["easy"] } + bcs = "0.1.2" move-bytecode-verifier = { path = "../../move-bytecode-verifier" } move-disassembler = { path = "../move-disassembler" } diff --git a/language/tools/move-cli/src/lib.rs b/language/tools/move-cli/src/lib.rs index e3206a904c..af439f10ef 100644 --- a/language/tools/move-cli/src/lib.rs +++ b/language/tools/move-cli/src/lib.rs @@ -11,6 +11,7 @@ use move_package::BuildConfig; pub mod base; pub mod experimental; pub mod sandbox; +mod login; /// Default directory where saved Move resources live pub const DEFAULT_STORAGE_DIR: &str = "storage"; @@ -91,6 +92,8 @@ pub enum Command { #[clap(subcommand)] cmd: experimental::cli::ExperimentalCommand, }, + #[clap(name = "login")] + Login, } pub fn run_cli( @@ -121,6 +124,7 @@ pub fn run_cli( &storage_dir, ), Command::Experimental { storage_dir, cmd } => cmd.handle_command(&move_args, &storage_dir), + Command::Login => login::cli::handle_login_commands(), } } diff --git a/language/tools/move-cli/src/login/cli.rs b/language/tools/move-cli/src/login/cli.rs new file mode 100644 index 0000000000..3832317d03 --- /dev/null +++ b/language/tools/move-cli/src/login/cli.rs @@ -0,0 +1,55 @@ +use std::collections::HashMap; +use std::io; +use std::fs::File; +use std::path::PathBuf; +use anyhow::{bail, Result}; +use toml_edit::easy::{Value}; +use toml_edit::easy::map::Map; + +pub fn handle_login_commands() -> Result<()> { + let url: &str; + if cfg!(debug_assertions) { + url = "https://movey-app-staging.herokuapp.com"; + } else { + url = "https://movey.net"; + } + println!("please paste the API Token found on {}/settings/tokens below", url); + let mut line = String::new(); + match io::stdin().read_line(&mut line) { + Ok(_) => { + if let Some('\n') = line.chars().next_back() { + line.pop(); + } + if let Some('\r') = line.chars().next_back() { + line.pop(); + } + print!("{}", line); + } + Err(err) => { + bail!("Error reading input: {}", err); + } + } + save_credential(line) +} + +fn save_credential(token: String) -> Result<()> { + let move_home = std::env::var("MOVE_HOME").unwrap_or_else(|_| { + format!( + "{}/.move", + std::env::var("HOME").expect("env var 'HOME' must be set") + ) + }); + let credential_path = move_home + "/credential.json"; + let credential_file = PathBuf::from(credential_path.clone()); + if !credential_file.exists() { + File::create(credential_path)?; + } + let mut value = Map::new(); + value.insert(String::from("token"), Value::String(token)); + let mut toml = Value::Table(Map::new()); + toml.as_table_mut().unwrap().insert(String::from("registry"), Value::Table(value)); + + let contents = toml.to_string(); + println!("{}", contents); + Ok(()) +} diff --git a/language/tools/move-cli/src/login/mod.rs b/language/tools/move-cli/src/login/mod.rs new file mode 100644 index 0000000000..4f773726a2 --- /dev/null +++ b/language/tools/move-cli/src/login/mod.rs @@ -0,0 +1 @@ +pub mod cli; diff --git a/language/tools/move-package/src/source_package/manifest_parser.rs b/language/tools/move-package/src/source_package/manifest_parser.rs index 69f17cce68..ec6a9587b1 100644 --- a/language/tools/move-package/src/source_package/manifest_parser.rs +++ b/language/tools/move-package/src/source_package/manifest_parser.rs @@ -328,6 +328,7 @@ fn parse_dependency(tval: TV) -> Result { } (None, Some(git)) => { // Look to see if a MOVE_HOME has been set. Otherwise default to $HOME + // let a = std::env::var("MOVE_HOME").unwrap(); let move_home = std::env::var("MOVE_HOME").unwrap_or_else(|_| { format!( "{}/.move", From b5fcf5ce1c79d954bbd6935d6a638bbb5865c059 Mon Sep 17 00:00:00 2001 From: ea-open-source Date: Fri, 27 May 2022 18:00:42 +0700 Subject: [PATCH 2/8] Store token to file, add unit tests # Conflicts: # Cargo.lock --- Cargo.lock | 11 ++ language/tools/move-cli/Cargo.toml | 1 + language/tools/move-cli/src/lib.rs | 2 +- language/tools/move-cli/src/login/cli.rs | 159 ++++++++++++++++++--- language/tools/move-cli/tests/cli_tests.rs | 76 ++++++++++ 5 files changed, 231 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 461ced2fb8..b634383cef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1887,6 +1887,15 @@ dependencies = [ "digest 0.9.0", ] +[[package]] +name = "home" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2456aef2e6b6a9784192ae780c0f15bc57df0e918585282325e8c8ac27737654" +dependencies = [ + "winapi", +] + [[package]] name = "humansize" version = "1.1.0" @@ -2452,6 +2461,8 @@ dependencies = [ "colored", "datatest-stable", "difference", + "home", + "include_dir", "itertools 0.10.1", "move-binary-format", "move-bytecode-source-map", diff --git a/language/tools/move-cli/Cargo.toml b/language/tools/move-cli/Cargo.toml index e060a4133e..cbf2bed810 100644 --- a/language/tools/move-cli/Cargo.toml +++ b/language/tools/move-cli/Cargo.toml @@ -51,6 +51,7 @@ move-bytecode-viewer = { path = "../move-bytecode-viewer" } [dev-dependencies] datatest-stable = "0.1.1" +home = "0.5.3" [[bin]] name = "move" diff --git a/language/tools/move-cli/src/lib.rs b/language/tools/move-cli/src/lib.rs index af439f10ef..f93bd4b8f8 100644 --- a/language/tools/move-cli/src/lib.rs +++ b/language/tools/move-cli/src/lib.rs @@ -124,7 +124,7 @@ pub fn run_cli( &storage_dir, ), Command::Experimental { storage_dir, cmd } => cmd.handle_command(&move_args, &storage_dir), - Command::Login => login::cli::handle_login_commands(), + Command::Login => login::cli::handle_login_commands(move_args.build_config.clone()), } } diff --git a/language/tools/move-cli/src/login/cli.rs b/language/tools/move-cli/src/login/cli.rs index 3832317d03..c49227fc33 100644 --- a/language/tools/move-cli/src/login/cli.rs +++ b/language/tools/move-cli/src/login/cli.rs @@ -1,12 +1,11 @@ -use std::collections::HashMap; -use std::io; +use std::{fs, io}; use std::fs::File; use std::path::PathBuf; use anyhow::{bail, Result}; -use toml_edit::easy::{Value}; +use toml_edit::easy::Value; use toml_edit::easy::map::Map; -pub fn handle_login_commands() -> Result<()> { +pub fn handle_login_commands(config: move_package::BuildConfig) -> Result<()> { let url: &str; if cfg!(debug_assertions) { url = "https://movey-app-staging.herokuapp.com"; @@ -26,30 +25,156 @@ pub fn handle_login_commands() -> Result<()> { print!("{}", line); } Err(err) => { - bail!("Error reading input: {}", err); + bail!("Error reading file: {}", err); } } - save_credential(line) + save_credential(line, config.test_mode) } -fn save_credential(token: String) -> Result<()> { - let move_home = std::env::var("MOVE_HOME").unwrap_or_else(|_| { +pub fn save_credential(token: String, is_test_mode: bool) -> Result<()> { + let mut move_home = std::env::var("MOVE_HOME").unwrap_or_else(|_| { format!( "{}/.move", std::env::var("HOME").expect("env var 'HOME' must be set") ) }); - let credential_path = move_home + "/credential.json"; - let credential_file = PathBuf::from(credential_path.clone()); + if is_test_mode { + move_home.push_str("/test") + } + fs::create_dir_all(&move_home)?; + let credential_path = move_home + "/credential.toml"; + let credential_file = PathBuf::from(&credential_path.clone()); if !credential_file.exists() { - File::create(credential_path)?; + File::create(&credential_path)?; + } + + let old_contents: String; + match fs::read_to_string(&credential_path) { + Ok(contents) => { + old_contents = contents; + }, + Err(error) => bail!("Error reading input: {}", error), } - let mut value = Map::new(); - value.insert(String::from("token"), Value::String(token)); - let mut toml = Value::Table(Map::new()); - toml.as_table_mut().unwrap().insert(String::from("registry"), Value::Table(value)); + let mut toml: Value = old_contents.parse() + .map_err(|e| anyhow::Error::from(e).context("could not parse input as TOML"))?; + + if let Some(registry) = toml.as_table_mut().unwrap().get_mut("registry") { + if let Some(toml_token) = registry.as_table_mut().unwrap().get_mut("token") { + *toml_token = Value::String(token); + } else { + registry.as_table_mut().unwrap() + .insert(String::from("token"), Value::String(token)); + } + } else { + let mut value = Map::new(); + value.insert(String::from("token"), Value::String(token)); + toml.as_table_mut().unwrap().insert(String::from("registry"), Value::Table(value)); + } + + let new_contents = toml.to_string(); + fs::write(credential_file, new_contents).expect("Unable to write file"); + let file = File::open(&credential_path)?; + set_permissions(&file, 0o600)?; + Ok(()) +} + +#[cfg(unix)] +fn set_permissions(file: &File, mode: u32) -> Result<()> { + use std::os::unix::fs::PermissionsExt; + + let mut perms = file.metadata()?.permissions(); + perms.set_mode(mode); + file.set_permissions(perms)?; + Ok(()) +} - let contents = toml.to_string(); - println!("{}", contents); +#[cfg(not(unix))] +#[allow(unused)] +fn set_permissions(file: &File, mode: u32) -> Result<()> { Ok(()) } + +#[cfg(test)] +mod tests { + use std::env; + use home::home_dir; + use super::*; + + fn setup_home_path() -> String { + let move_home = env::var("MOVE_HOME").unwrap_or_else(|_| { + env::var("HOME").unwrap_or_else(|_| { + let home_dir = home_dir().unwrap().to_string_lossy().to_string(); + env::set_var("HOME", &home_dir); + home_dir + }) + }); + let credential_file = move_home + "/.move/test/credential.toml"; + return credential_file; + } + + #[test] + fn save_credential_works_if_no_credential_file_exists() { + let credential_file = setup_home_path(); + let _ = fs::remove_file(&credential_file); + + save_credential(String::from("test_token"), true).unwrap(); + + let contents = fs::read_to_string(&credential_file).expect("Unable to read file"); + let mut toml: Value = contents.parse().unwrap(); + let registry = toml.as_table_mut().unwrap().get_mut("registry").unwrap(); + let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); + assert!(token.to_string().contains("test_token")); + } + + #[test] + fn save_credential_works_if_credential_file_is_empty() { + let credential_file = setup_home_path(); + + let _ = fs::remove_file(&credential_file); + File::create(&credential_file).unwrap(); + + save_credential(String::from("test_token"), true).unwrap(); + + let contents = fs::read_to_string(&credential_file).expect("Unable to read file"); + let mut toml: Value = contents.parse().unwrap(); + let registry = toml.as_table_mut().unwrap().get_mut("registry").unwrap(); + let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); + assert!(token.to_string().contains("test_token")); + } + + #[test] + fn save_credential_works_with_old_token_field() { + let credential_file = setup_home_path(); + File::create(&credential_file).unwrap(); + + let old_content = String::from("[registry]\ntoken = \"old_test_token\"\n"); + fs::write(&credential_file, old_content).expect("Unable to write file"); + + save_credential(String::from("test_token"), true).unwrap(); + + let contents = fs::read_to_string(&credential_file).expect("Unable to read file"); + let mut toml: Value = contents.parse().unwrap(); + let registry = toml.as_table_mut().unwrap().get_mut("registry").unwrap(); + let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); + assert!(token.to_string().contains("test_token")); + assert!(!token.to_string().contains("old_test_token")); + } + + #[test] + fn save_credential_works_with_old_empty_token_field() { + let credential_file = setup_home_path(); + File::create(&credential_file).unwrap(); + + let old_content = String::from("[registry]\ntoken = \"\"\n"); + fs::write(&credential_file, old_content).expect("Unable to write file"); + + save_credential(String::from("test_token"), true).unwrap(); + + let contents = fs::read_to_string(&credential_file).expect("Unable to read file"); + let mut toml: Value = contents.parse().unwrap(); + let registry = toml.as_table_mut().unwrap().get_mut("registry").unwrap(); + let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); + assert!(token.to_string().contains("test_token")); + assert!(!token.to_string().contains("old_test_token")); + } +} \ No newline at end of file diff --git a/language/tools/move-cli/tests/cli_tests.rs b/language/tools/move-cli/tests/cli_tests.rs index b77954862b..0b7423e626 100644 --- a/language/tools/move-cli/tests/cli_tests.rs +++ b/language/tools/move-cli/tests/cli_tests.rs @@ -2,9 +2,16 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 +use std::{env, fs}; +use std::fs::File; +use std::io::{BufRead, BufReader, Write}; use move_cli::sandbox::commands::test; use std::path::PathBuf; +use std::process::Stdio; +use home::home_dir; +use toml_edit::easy::Value; +use std::os::unix::fs::PermissionsExt; pub const CLI_METATEST_PATH: [&str; 3] = ["tests", "metatests", "args.txt"]; @@ -53,3 +60,72 @@ fn cross_process_locking_git_deps() { .expect("Package2 failed"); handle.join().unwrap(); } + +#[test] +fn save_credential_works() { + #[cfg(debug_assertions)] + const CLI_EXE: &str = "../../../target/debug/move"; + #[cfg(not(debug_assertions))] + const CLI_EXE: &str = "../../../target/release/move"; + let home = home_dir().unwrap().to_string_lossy().to_string() + "/.move/test"; + env::set_var("MOVE_HOME", &home); + match std::process::Command::new(CLI_EXE) + .current_dir(".") + .args(["login"]) + .stdin(Stdio::piped()).spawn() { + Ok(mut child) => { + let token = "test_token"; + child.stdin.as_ref().unwrap().write(token.as_bytes()).unwrap(); + child.wait().unwrap(); + Ok(()) + } + Err(error) => Err(error), + }.unwrap(); + let contents = fs::read_to_string(home + "/credential.toml").expect("Unable to read file"); + let mut toml: Value = contents.parse().unwrap(); + let registry = toml.as_table_mut().unwrap().get_mut("registry").unwrap(); + let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); + assert!(token.to_string().contains("test_token")); +} + +#[test] +fn save_credential_fails() { + #[cfg(debug_assertions)] + const CLI_EXE: &str = "../../../target/debug/move"; + #[cfg(not(debug_assertions))] + const CLI_EXE: &str = "../../../target/release/move"; + let home = home_dir().unwrap().to_string_lossy().to_string() + "/.move/test"; + let credential_file = home.clone() + "/credential.toml"; + let _ = fs::remove_file(&credential_file); + + fs::create_dir_all(&home).unwrap(); + let file = File::create(&credential_file).unwrap(); + let mut perms = file.metadata().unwrap().permissions(); + perms.set_mode(0o000); + file.set_permissions(perms).unwrap(); + + env::set_var("MOVE_HOME", &home); + match std::process::Command::new(CLI_EXE) + .current_dir(".") + .args(["login"]) + .stdin(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() { + Ok(mut child) => { + let token = "test_token"; + child.stdin.as_ref().unwrap().write(token.as_bytes()).unwrap(); + child.wait().unwrap(); + let out = BufReader::new(child.stderr.take().unwrap()); + out.lines().for_each(|line| + assert!(line.unwrap().contains("Error: Error reading input: Permission denied (os error 13)")) + ); + Ok(()) + } + Err(error) => Err(error), + }.unwrap(); + + let mut perms = file.metadata().unwrap().permissions(); + perms.set_mode(0o600); + file.set_permissions(perms).unwrap(); + let _ = fs::remove_file(&credential_file); +} From 6ceb6117f175929b4b4806150762eeca74e5335b Mon Sep 17 00:00:00 2001 From: ea-open-source Date: Mon, 30 May 2022 12:18:09 +0700 Subject: [PATCH 3/8] Refactor test code --- Cargo.lock | 43 +---- language/tools/move-cli/src/lib.rs | 2 +- language/tools/move-cli/src/login/cli.rs | 155 ++++++++++++------ language/tools/move-cli/tests/cli_tests.rs | 132 ++++++++++----- .../src/source_package/manifest_parser.rs | 1 - 5 files changed, 195 insertions(+), 138 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b634383cef..411f63a186 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1773,34 +1773,6 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "92620684d99f750bae383ecb3be3748142d6095760afd5cbcf2261e9a279d780" -[[package]] -name = "hakari" -version = "0.8.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f48905a7968260802691e8b6cc2e9426822b79eaf9164d07f2212e9327181b3e" -dependencies = [ - "atomicwrites", - "bimap", - "camino", - "cfg-if 1.0.0", - "debug-ignore", - "diffy", - "guppy", - "guppy-workspace-hack", - "include_dir", - "indenter", - "itertools 0.10.1", - "owo-colors", - "pathdiff", - "rayon", - "serde 1.0.130", - "strip-ansi-escapes", - "target-spec", - "toml", - "toml_edit 0.10.1", - "twox-hash", -] - [[package]] name = "half" version = "1.7.1" @@ -2462,7 +2434,6 @@ dependencies = [ "datatest-stable", "difference", "home", - "include_dir", "itertools 0.10.1", "move-binary-format", "move-bytecode-source-map", @@ -2492,7 +2463,7 @@ dependencies = [ "serde 1.0.130", "serde_yaml", "tempfile", - "toml_edit 0.14.4", + "toml_edit", "walkdir", ] @@ -5221,18 +5192,6 @@ dependencies = [ "serde 1.0.130", ] -[[package]] -name = "toml_edit" -version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2782eb01c48f7f6f66d63bebe64dd8302dbf6bc0be0d65a85a1854d472e2dfa8" -dependencies = [ - "combine", - "indexmap", - "itertools 0.10.1", - "kstring", -] - [[package]] name = "toml_edit" version = "0.14.4" diff --git a/language/tools/move-cli/src/lib.rs b/language/tools/move-cli/src/lib.rs index f93bd4b8f8..b25e616557 100644 --- a/language/tools/move-cli/src/lib.rs +++ b/language/tools/move-cli/src/lib.rs @@ -10,8 +10,8 @@ use move_package::BuildConfig; pub mod base; pub mod experimental; +pub mod login; pub mod sandbox; -mod login; /// Default directory where saved Move resources live pub const DEFAULT_STORAGE_DIR: &str = "storage"; diff --git a/language/tools/move-cli/src/login/cli.rs b/language/tools/move-cli/src/login/cli.rs index c49227fc33..361ccbbcff 100644 --- a/language/tools/move-cli/src/login/cli.rs +++ b/language/tools/move-cli/src/login/cli.rs @@ -1,9 +1,9 @@ -use std::{fs, io}; +use anyhow::{bail, Result}; use std::fs::File; use std::path::PathBuf; -use anyhow::{bail, Result}; -use toml_edit::easy::Value; +use std::{fs, io}; use toml_edit::easy::map::Map; +use toml_edit::easy::Value; pub fn handle_login_commands(config: move_package::BuildConfig) -> Result<()> { let url: &str; @@ -12,23 +12,33 @@ pub fn handle_login_commands(config: move_package::BuildConfig) -> Result<()> { } else { url = "https://movey.net"; } - println!("please paste the API Token found on {}/settings/tokens below", url); + println!( + "Please paste the API Token found on {}/settings/tokens below", + url + ); let mut line = String::new(); - match io::stdin().read_line(&mut line) { - Ok(_) => { - if let Some('\n') = line.chars().next_back() { - line.pop(); + loop { + match io::stdin().read_line(&mut line) { + Ok(_) => { + if let Some('\n') = line.chars().next_back() { + line.pop(); + } + if let Some('\r') = line.chars().next_back() { + line.pop(); + } + if line.len() != 0 { + break; + } + println!("Invalid API Token. Try again!"); } - if let Some('\r') = line.chars().next_back() { - line.pop(); + Err(err) => { + bail!("Error reading file: {}", err); } - print!("{}", line); - } - Err(err) => { - bail!("Error reading file: {}", err); } } - save_credential(line, config.test_mode) + save_credential(line, config.test_mode)?; + println!("Token for Movey saved."); + Ok(()) } pub fn save_credential(token: String, is_test_mode: bool) -> Result<()> { @@ -52,23 +62,28 @@ pub fn save_credential(token: String, is_test_mode: bool) -> Result<()> { match fs::read_to_string(&credential_path) { Ok(contents) => { old_contents = contents; - }, + } Err(error) => bail!("Error reading input: {}", error), } - let mut toml: Value = old_contents.parse() + let mut toml: Value = old_contents + .parse() .map_err(|e| anyhow::Error::from(e).context("could not parse input as TOML"))?; if let Some(registry) = toml.as_table_mut().unwrap().get_mut("registry") { if let Some(toml_token) = registry.as_table_mut().unwrap().get_mut("token") { *toml_token = Value::String(token); } else { - registry.as_table_mut().unwrap() + registry + .as_table_mut() + .unwrap() .insert(String::from("token"), Value::String(token)); } } else { let mut value = Map::new(); value.insert(String::from("token"), Value::String(token)); - toml.as_table_mut().unwrap().insert(String::from("registry"), Value::Table(value)); + toml.as_table_mut() + .unwrap() + .insert(String::from("registry"), Value::Table(value)); } let new_contents = toml.to_string(); @@ -96,85 +111,127 @@ fn set_permissions(file: &File, mode: u32) -> Result<()> { #[cfg(test)] mod tests { - use std::env; - use home::home_dir; use super::*; + use home::home_dir; + use std::env; - fn setup_home_path() -> String { - let move_home = env::var("MOVE_HOME").unwrap_or_else(|_| { + fn setup_move_home() -> (String, String) { + let mut move_home = env::var("MOVE_HOME").unwrap_or_else(|_| { env::var("HOME").unwrap_or_else(|_| { let home_dir = home_dir().unwrap().to_string_lossy().to_string(); env::set_var("HOME", &home_dir); home_dir }) }); - let credential_file = move_home + "/.move/test/credential.toml"; - return credential_file; + move_home.push_str("/.move/test"); + let credential_path = move_home.clone() + "/credential.toml"; + return (move_home, credential_path); + } + + fn clean_up() { + let (move_home, _) = setup_move_home(); + let _ = fs::remove_dir_all(move_home); } #[test] fn save_credential_works_if_no_credential_file_exists() { - let credential_file = setup_home_path(); - let _ = fs::remove_file(&credential_file); + let (move_home, credential_path) = setup_move_home(); + let _ = fs::remove_dir_all(&move_home); save_credential(String::from("test_token"), true).unwrap(); - let contents = fs::read_to_string(&credential_file).expect("Unable to read file"); + let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); let mut toml: Value = contents.parse().unwrap(); let registry = toml.as_table_mut().unwrap().get_mut("registry").unwrap(); let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); assert!(token.to_string().contains("test_token")); + + clean_up(); } #[test] - fn save_credential_works_if_credential_file_is_empty() { - let credential_file = setup_home_path(); + fn save_credential_works_if_empty_credential_file_exists() { + let (move_home, credential_path) = setup_move_home(); - let _ = fs::remove_file(&credential_file); - File::create(&credential_file).unwrap(); + let _ = fs::remove_dir_all(&move_home); + fs::create_dir_all(&move_home).unwrap(); + File::create(&credential_path).unwrap(); + + let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); + let mut toml: Value = contents.parse().unwrap(); + assert!(toml.as_table_mut().unwrap().get_mut("registry").is_none()); save_credential(String::from("test_token"), true).unwrap(); - let contents = fs::read_to_string(&credential_file).expect("Unable to read file"); + let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); let mut toml: Value = contents.parse().unwrap(); let registry = toml.as_table_mut().unwrap().get_mut("registry").unwrap(); let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); assert!(token.to_string().contains("test_token")); + + clean_up(); } #[test] - fn save_credential_works_with_old_token_field() { - let credential_file = setup_home_path(); - File::create(&credential_file).unwrap(); + fn save_credential_works_if_token_field_exists() { + let (move_home, credential_path) = setup_move_home(); - let old_content = String::from("[registry]\ntoken = \"old_test_token\"\n"); - fs::write(&credential_file, old_content).expect("Unable to write file"); + let _ = fs::remove_dir_all(&move_home); + fs::create_dir_all(&move_home).unwrap(); + File::create(&credential_path).unwrap(); - save_credential(String::from("test_token"), true).unwrap(); + let old_content = + String::from("[registry]\ntoken = \"old_test_token\"\nversion = \"0.0.0\"\n"); + fs::write(&credential_path, old_content).expect("Unable to write file"); - let contents = fs::read_to_string(&credential_file).expect("Unable to read file"); + let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); let mut toml: Value = contents.parse().unwrap(); let registry = toml.as_table_mut().unwrap().get_mut("registry").unwrap(); let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); - assert!(token.to_string().contains("test_token")); + assert!(token.to_string().contains("old_test_token")); + assert!(!token.to_string().contains("new_world")); + + save_credential(String::from("new_world"), true).unwrap(); + + let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); + let mut toml: Value = contents.parse().unwrap(); + let registry = toml.as_table_mut().unwrap().get_mut("registry").unwrap(); + let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); + assert!(token.to_string().contains("new_world")); assert!(!token.to_string().contains("old_test_token")); + let version = registry.as_table_mut().unwrap().get_mut("version").unwrap(); + assert!(version.to_string().contains("0.0.0")); + + clean_up(); } #[test] - fn save_credential_works_with_old_empty_token_field() { - let credential_file = setup_home_path(); - File::create(&credential_file).unwrap(); + fn save_credential_works_if_empty_token_field_exists() { + let (move_home, credential_path) = setup_move_home(); - let old_content = String::from("[registry]\ntoken = \"\"\n"); - fs::write(&credential_file, old_content).expect("Unable to write file"); + let _ = fs::remove_dir_all(&move_home); + fs::create_dir_all(&move_home).unwrap(); + File::create(&credential_path).unwrap(); + + let old_content = String::from("[registry]\ntoken = \"\"\nversion = \"0.0.0\"\n"); + fs::write(&credential_path, old_content).expect("Unable to write file"); + + let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); + let mut toml: Value = contents.parse().unwrap(); + let registry = toml.as_table_mut().unwrap().get_mut("registry").unwrap(); + let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); + assert!(!token.to_string().contains("test_token")); save_credential(String::from("test_token"), true).unwrap(); - let contents = fs::read_to_string(&credential_file).expect("Unable to read file"); + let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); let mut toml: Value = contents.parse().unwrap(); let registry = toml.as_table_mut().unwrap().get_mut("registry").unwrap(); let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); assert!(token.to_string().contains("test_token")); - assert!(!token.to_string().contains("old_test_token")); + let version = registry.as_table_mut().unwrap().get_mut("version").unwrap(); + assert!(version.to_string().contains("0.0.0")); + + clean_up(); } -} \ No newline at end of file +} diff --git a/language/tools/move-cli/tests/cli_tests.rs b/language/tools/move-cli/tests/cli_tests.rs index 0b7423e626..2a8253c355 100644 --- a/language/tools/move-cli/tests/cli_tests.rs +++ b/language/tools/move-cli/tests/cli_tests.rs @@ -2,16 +2,18 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use std::{env, fs}; -use std::fs::File; -use std::io::{BufRead, BufReader, Write}; use move_cli::sandbox::commands::test; +#[cfg(unix)] +use std::fs::File; +use std::io::Write; +use std::{env, fs}; +use home::home_dir; +#[cfg(unix)] +use std::os::unix::fs::PermissionsExt; use std::path::PathBuf; use std::process::Stdio; -use home::home_dir; use toml_edit::easy::Value; -use std::os::unix::fs::PermissionsExt; pub const CLI_METATEST_PATH: [&str; 3] = ["tests", "metatests", "args.txt"]; @@ -63,69 +65,109 @@ fn cross_process_locking_git_deps() { #[test] fn save_credential_works() { - #[cfg(debug_assertions)] - const CLI_EXE: &str = "../../../target/debug/move"; - #[cfg(not(debug_assertions))] - const CLI_EXE: &str = "../../../target/release/move"; - let home = home_dir().unwrap().to_string_lossy().to_string() + "/.move/test"; - env::set_var("MOVE_HOME", &home); - match std::process::Command::new(CLI_EXE) + let cli_exe = env!("CARGO_BIN_EXE_move"); + let (move_home, credential_path) = setup_move_home(); + assert!(fs::read_to_string(&credential_path).is_err()); + + match std::process::Command::new(cli_exe) .current_dir(".") .args(["login"]) - .stdin(Stdio::piped()).spawn() { - Ok(mut child) => { - let token = "test_token"; - child.stdin.as_ref().unwrap().write(token.as_bytes()).unwrap(); - child.wait().unwrap(); - Ok(()) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + { + Ok(child) => { + let token = "test_token"; + child + .stdin + .as_ref() + .unwrap() + .write(token.as_bytes()) + .unwrap(); + match child.wait_with_output() { + Ok(output) => { + assert!(String::from_utf8_lossy(&output.stdout).contains( + "Please paste the API Token found on \ + https://movey-app-staging.herokuapp.com/settings/tokens below" + )); + Ok(()) + } + Err(error) => Err(error), } - Err(error) => Err(error), - }.unwrap(); - let contents = fs::read_to_string(home + "/credential.toml").expect("Unable to read file"); + } + Err(error) => Err(error), + } + .unwrap(); + + let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); let mut toml: Value = contents.parse().unwrap(); let registry = toml.as_table_mut().unwrap().get_mut("registry").unwrap(); let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); assert!(token.to_string().contains("test_token")); + + clean_up(&move_home) } +#[cfg(unix)] #[test] -fn save_credential_fails() { - #[cfg(debug_assertions)] - const CLI_EXE: &str = "../../../target/debug/move"; - #[cfg(not(debug_assertions))] - const CLI_EXE: &str = "../../../target/release/move"; - let home = home_dir().unwrap().to_string_lossy().to_string() + "/.move/test"; - let credential_file = home.clone() + "/credential.toml"; - let _ = fs::remove_file(&credential_file); - - fs::create_dir_all(&home).unwrap(); - let file = File::create(&credential_file).unwrap(); +fn save_credential_fails_if_undeletable_credential_file_exists() { + let cli_exe = env!("CARGO_BIN_EXE_move"); + let (move_home, credential_path) = setup_move_home(); + let file = File::create(&credential_path).unwrap(); let mut perms = file.metadata().unwrap().permissions(); perms.set_mode(0o000); file.set_permissions(perms).unwrap(); - env::set_var("MOVE_HOME", &home); - match std::process::Command::new(CLI_EXE) + match std::process::Command::new(cli_exe) .current_dir(".") .args(["login"]) .stdin(Stdio::piped()) + .stdout(Stdio::piped()) .stderr(Stdio::piped()) - .spawn() { - Ok(mut child) => { + .spawn() + { + Ok(child) => { let token = "test_token"; - child.stdin.as_ref().unwrap().write(token.as_bytes()).unwrap(); - child.wait().unwrap(); - let out = BufReader::new(child.stderr.take().unwrap()); - out.lines().for_each(|line| - assert!(line.unwrap().contains("Error: Error reading input: Permission denied (os error 13)")) - ); - Ok(()) + child + .stdin + .as_ref() + .unwrap() + .write(token.as_bytes()) + .unwrap(); + match child.wait_with_output() { + Ok(output) => { + assert!(String::from_utf8_lossy(&output.stdout).contains( + "Please paste the API Token found on \ + https://movey-app-staging.herokuapp.com/settings/tokens below" + )); + assert!(String::from_utf8_lossy(&output.stderr) + .contains("Error: Error reading input: Permission denied (os error 13)")); + Ok(()) + } + Err(error) => Err(error), + } } Err(error) => Err(error), - }.unwrap(); + } + .unwrap(); let mut perms = file.metadata().unwrap().permissions(); perms.set_mode(0o600); file.set_permissions(perms).unwrap(); - let _ = fs::remove_file(&credential_file); + let _ = fs::remove_file(&credential_path); + + clean_up(&move_home) +} + +fn setup_move_home() -> (String, String) { + let move_home = home_dir().unwrap().to_string_lossy().to_string() + "/.move/test"; + env::set_var("MOVE_HOME", &move_home); + let _ = fs::remove_dir_all(&move_home); + fs::create_dir_all(&move_home).unwrap(); + let credential_path = move_home.clone() + "/credential.toml"; + return (move_home, credential_path); +} + +fn clean_up(move_home: &str) { + let _ = fs::remove_dir_all(move_home); } diff --git a/language/tools/move-package/src/source_package/manifest_parser.rs b/language/tools/move-package/src/source_package/manifest_parser.rs index ec6a9587b1..69f17cce68 100644 --- a/language/tools/move-package/src/source_package/manifest_parser.rs +++ b/language/tools/move-package/src/source_package/manifest_parser.rs @@ -328,7 +328,6 @@ fn parse_dependency(tval: TV) -> Result { } (None, Some(git)) => { // Look to see if a MOVE_HOME has been set. Otherwise default to $HOME - // let a = std::env::var("MOVE_HOME").unwrap(); let move_home = std::env::var("MOVE_HOME").unwrap_or_else(|_| { format!( "{}/.move", From 0ea15cc3ee60aa28587f7d428d473198bc43a506 Mon Sep 17 00:00:00 2001 From: ea-open-source Date: Thu, 30 Jun 2022 14:30:59 +0700 Subject: [PATCH 4/8] Refactor test code to run in parallel, add license header --- language/tools/move-cli/src/lib.rs | 7 +- language/tools/move-cli/src/login/cli.rs | 76 ++++++++++++++++------ language/tools/move-cli/src/login/mod.rs | 4 ++ language/tools/move-cli/tests/cli_tests.rs | 19 ++++-- 4 files changed, 76 insertions(+), 30 deletions(-) diff --git a/language/tools/move-cli/src/lib.rs b/language/tools/move-cli/src/lib.rs index b25e616557..c00086b818 100644 --- a/language/tools/move-cli/src/lib.rs +++ b/language/tools/move-cli/src/lib.rs @@ -93,7 +93,10 @@ pub enum Command { cmd: experimental::cli::ExperimentalCommand, }, #[clap(name = "login")] - Login, + Login { + #[clap(long = "test-path")] + test_path: Option, + }, } pub fn run_cli( @@ -124,7 +127,7 @@ pub fn run_cli( &storage_dir, ), Command::Experimental { storage_dir, cmd } => cmd.handle_command(&move_args, &storage_dir), - Command::Login => login::cli::handle_login_commands(move_args.build_config.clone()), + Command::Login { test_path } => login::cli::handle_login_commands(test_path.clone()), } } diff --git a/language/tools/move-cli/src/login/cli.rs b/language/tools/move-cli/src/login/cli.rs index 361ccbbcff..952a7a01a2 100644 --- a/language/tools/move-cli/src/login/cli.rs +++ b/language/tools/move-cli/src/login/cli.rs @@ -1,3 +1,7 @@ +// Copyright (c) The Diem Core Contributors +// Copyright (c) The Move Contributors +// SPDX-License-Identifier: Apache-2.0 + use anyhow::{bail, Result}; use std::fs::File; use std::path::PathBuf; @@ -5,7 +9,11 @@ use std::{fs, io}; use toml_edit::easy::map::Map; use toml_edit::easy::Value; -pub fn handle_login_commands(config: move_package::BuildConfig) -> Result<()> { +pub struct TestMode { + pub test_path: String, +} + +pub fn handle_login_commands(test_path: Option) -> Result<()> { let url: &str; if cfg!(debug_assertions) { url = "https://movey-app-staging.herokuapp.com"; @@ -36,20 +44,26 @@ pub fn handle_login_commands(config: move_package::BuildConfig) -> Result<()> { } } } - save_credential(line, config.test_mode)?; + let mut test_mode: Option = None; + if let Some(path) = test_path { + test_mode = Some(TestMode { test_path: path }); + } + save_credential(line, test_mode)?; println!("Token for Movey saved."); Ok(()) } -pub fn save_credential(token: String, is_test_mode: bool) -> Result<()> { +pub fn save_credential(token: String, test_mode: Option) -> Result<()> { let mut move_home = std::env::var("MOVE_HOME").unwrap_or_else(|_| { format!( "{}/.move", std::env::var("HOME").expect("env var 'HOME' must be set") ) }); - if is_test_mode { - move_home.push_str("/test") + if let Some(test_mode) = test_mode { + if !test_mode.test_path.is_empty() { + move_home.push_str(&test_mode.test_path); + } } fs::create_dir_all(&move_home)?; let credential_path = move_home + "/credential.toml"; @@ -115,7 +129,7 @@ mod tests { use home::home_dir; use std::env; - fn setup_move_home() -> (String, String) { + fn setup_move_home(test_path: &str) -> (String, String) { let mut move_home = env::var("MOVE_HOME").unwrap_or_else(|_| { env::var("HOME").unwrap_or_else(|_| { let home_dir = home_dir().unwrap().to_string_lossy().to_string(); @@ -123,22 +137,30 @@ mod tests { home_dir }) }); - move_home.push_str("/.move/test"); + move_home.push_str("/.move"); + if !test_path.is_empty() { + move_home.push_str(&test_path); + } else { + move_home.push_str("/test"); + } let credential_path = move_home.clone() + "/credential.toml"; return (move_home, credential_path); } - fn clean_up() { - let (move_home, _) = setup_move_home(); + fn clean_up(move_home: &str) { let _ = fs::remove_dir_all(move_home); } #[test] fn save_credential_works_if_no_credential_file_exists() { - let (move_home, credential_path) = setup_move_home(); + let (move_home, credential_path) = + setup_move_home("/save_credential_works_if_no_credential_file_exists"); let _ = fs::remove_dir_all(&move_home); - save_credential(String::from("test_token"), true).unwrap(); + let test_mode = Some(TestMode { + test_path: String::from("/save_credential_works_if_no_credential_file_exists"), + }); + save_credential(String::from("test_token"), test_mode).unwrap(); let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); let mut toml: Value = contents.parse().unwrap(); @@ -146,12 +168,13 @@ mod tests { let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); assert!(token.to_string().contains("test_token")); - clean_up(); + clean_up(&move_home); } #[test] fn save_credential_works_if_empty_credential_file_exists() { - let (move_home, credential_path) = setup_move_home(); + let (move_home, credential_path) = + setup_move_home("/save_credential_works_if_empty_credential_file_exists"); let _ = fs::remove_dir_all(&move_home); fs::create_dir_all(&move_home).unwrap(); @@ -161,7 +184,10 @@ mod tests { let mut toml: Value = contents.parse().unwrap(); assert!(toml.as_table_mut().unwrap().get_mut("registry").is_none()); - save_credential(String::from("test_token"), true).unwrap(); + let test_mode = Some(TestMode { + test_path: String::from("/save_credential_works_if_empty_credential_file_exists"), + }); + save_credential(String::from("test_token"), test_mode).unwrap(); let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); let mut toml: Value = contents.parse().unwrap(); @@ -169,12 +195,13 @@ mod tests { let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); assert!(token.to_string().contains("test_token")); - clean_up(); + clean_up(&move_home); } #[test] fn save_credential_works_if_token_field_exists() { - let (move_home, credential_path) = setup_move_home(); + let (move_home, credential_path) = + setup_move_home("/save_credential_works_if_token_field_exists"); let _ = fs::remove_dir_all(&move_home); fs::create_dir_all(&move_home).unwrap(); @@ -191,7 +218,10 @@ mod tests { assert!(token.to_string().contains("old_test_token")); assert!(!token.to_string().contains("new_world")); - save_credential(String::from("new_world"), true).unwrap(); + let test_mode = Some(TestMode { + test_path: String::from("/save_credential_works_if_token_field_exists"), + }); + save_credential(String::from("new_world"), test_mode).unwrap(); let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); let mut toml: Value = contents.parse().unwrap(); @@ -202,12 +232,13 @@ mod tests { let version = registry.as_table_mut().unwrap().get_mut("version").unwrap(); assert!(version.to_string().contains("0.0.0")); - clean_up(); + clean_up(&move_home); } #[test] fn save_credential_works_if_empty_token_field_exists() { - let (move_home, credential_path) = setup_move_home(); + let (move_home, credential_path) = + setup_move_home("/save_credential_works_if_empty_token_field_exists"); let _ = fs::remove_dir_all(&move_home); fs::create_dir_all(&move_home).unwrap(); @@ -222,7 +253,10 @@ mod tests { let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); assert!(!token.to_string().contains("test_token")); - save_credential(String::from("test_token"), true).unwrap(); + let test_mode = Some(TestMode { + test_path: String::from("/save_credential_works_if_empty_token_field_exists"), + }); + save_credential(String::from("test_token"), test_mode).unwrap(); let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); let mut toml: Value = contents.parse().unwrap(); @@ -232,6 +266,6 @@ mod tests { let version = registry.as_table_mut().unwrap().get_mut("version").unwrap(); assert!(version.to_string().contains("0.0.0")); - clean_up(); + clean_up(&move_home); } } diff --git a/language/tools/move-cli/src/login/mod.rs b/language/tools/move-cli/src/login/mod.rs index 4f773726a2..eb76e266c6 100644 --- a/language/tools/move-cli/src/login/mod.rs +++ b/language/tools/move-cli/src/login/mod.rs @@ -1 +1,5 @@ +// Copyright (c) The Diem Core Contributors +// Copyright (c) The Move Contributors +// SPDX-License-Identifier: Apache-2.0 + pub mod cli; diff --git a/language/tools/move-cli/tests/cli_tests.rs b/language/tools/move-cli/tests/cli_tests.rs index 2a8253c355..eade257d7c 100644 --- a/language/tools/move-cli/tests/cli_tests.rs +++ b/language/tools/move-cli/tests/cli_tests.rs @@ -66,12 +66,12 @@ fn cross_process_locking_git_deps() { #[test] fn save_credential_works() { let cli_exe = env!("CARGO_BIN_EXE_move"); - let (move_home, credential_path) = setup_move_home(); + let (move_home, credential_path) = setup_move_home("/save_credential_works"); assert!(fs::read_to_string(&credential_path).is_err()); match std::process::Command::new(cli_exe) .current_dir(".") - .args(["login"]) + .args(["login", "--test", "--test-path", "/save_credential_works"]) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .spawn() @@ -112,7 +112,8 @@ fn save_credential_works() { #[test] fn save_credential_fails_if_undeletable_credential_file_exists() { let cli_exe = env!("CARGO_BIN_EXE_move"); - let (move_home, credential_path) = setup_move_home(); + let (move_home, credential_path) = + setup_move_home("/save_credential_fails_if_undeletable_credential_file_exists"); let file = File::create(&credential_path).unwrap(); let mut perms = file.metadata().unwrap().permissions(); perms.set_mode(0o000); @@ -120,7 +121,12 @@ fn save_credential_fails_if_undeletable_credential_file_exists() { match std::process::Command::new(cli_exe) .current_dir(".") - .args(["login"]) + .args([ + "login", + "--test", + "--test-path", + "/save_credential_fails_if_undeletable_credential_file_exists", + ]) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) @@ -159,9 +165,8 @@ fn save_credential_fails_if_undeletable_credential_file_exists() { clean_up(&move_home) } -fn setup_move_home() -> (String, String) { - let move_home = home_dir().unwrap().to_string_lossy().to_string() + "/.move/test"; - env::set_var("MOVE_HOME", &move_home); +fn setup_move_home(test_path: &str) -> (String, String) { + let move_home = home_dir().unwrap().to_string_lossy().to_string() + "/.move" + test_path; let _ = fs::remove_dir_all(&move_home); fs::create_dir_all(&move_home).unwrap(); let credential_path = move_home.clone() + "/credential.toml"; From 2e27409e9dd0c70749b55d9e1fdb60d7e410b240 Mon Sep 17 00:00:00 2001 From: ea-open-source Date: Wed, 6 Jul 2022 15:35:26 +0700 Subject: [PATCH 5/8] Refactor tests to run in current dir --- Cargo.lock | 10 ------ language/tools/move-cli/Cargo.toml | 1 - language/tools/move-cli/src/lib.rs | 2 +- language/tools/move-cli/src/login/cli.rs | 40 +++++++++------------- language/tools/move-cli/tests/cli_tests.rs | 18 +++++----- 5 files changed, 27 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 411f63a186..18f4b62b75 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1859,15 +1859,6 @@ dependencies = [ "digest 0.9.0", ] -[[package]] -name = "home" -version = "0.5.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2456aef2e6b6a9784192ae780c0f15bc57df0e918585282325e8c8ac27737654" -dependencies = [ - "winapi", -] - [[package]] name = "humansize" version = "1.1.0" @@ -2433,7 +2424,6 @@ dependencies = [ "colored", "datatest-stable", "difference", - "home", "itertools 0.10.1", "move-binary-format", "move-bytecode-source-map", diff --git a/language/tools/move-cli/Cargo.toml b/language/tools/move-cli/Cargo.toml index cbf2bed810..e060a4133e 100644 --- a/language/tools/move-cli/Cargo.toml +++ b/language/tools/move-cli/Cargo.toml @@ -51,7 +51,6 @@ move-bytecode-viewer = { path = "../move-bytecode-viewer" } [dev-dependencies] datatest-stable = "0.1.1" -home = "0.5.3" [[bin]] name = "move" diff --git a/language/tools/move-cli/src/lib.rs b/language/tools/move-cli/src/lib.rs index c00086b818..3529c5c890 100644 --- a/language/tools/move-cli/src/lib.rs +++ b/language/tools/move-cli/src/lib.rs @@ -127,7 +127,7 @@ pub fn run_cli( &storage_dir, ), Command::Experimental { storage_dir, cmd } => cmd.handle_command(&move_args, &storage_dir), - Command::Login { test_path } => login::cli::handle_login_commands(test_path.clone()), + Command::Login { test_path } => login::cli::handle_login_commands(test_path), } } diff --git a/language/tools/move-cli/src/login/cli.rs b/language/tools/move-cli/src/login/cli.rs index 952a7a01a2..ac2bb3a746 100644 --- a/language/tools/move-cli/src/login/cli.rs +++ b/language/tools/move-cli/src/login/cli.rs @@ -3,11 +3,8 @@ // SPDX-License-Identifier: Apache-2.0 use anyhow::{bail, Result}; -use std::fs::File; -use std::path::PathBuf; -use std::{fs, io}; -use toml_edit::easy::map::Map; -use toml_edit::easy::Value; +use std::{fs, fs::File, io, path::PathBuf}; +use toml_edit::easy::{map::Map, Value}; pub struct TestMode { pub test_path: String, @@ -34,7 +31,7 @@ pub fn handle_login_commands(test_path: Option) -> Result<()> { if let Some('\r') = line.chars().next_back() { line.pop(); } - if line.len() != 0 { + if !line.is_empty() { break; } println!("Invalid API Token. Try again!"); @@ -54,20 +51,23 @@ pub fn handle_login_commands(test_path: Option) -> Result<()> { } pub fn save_credential(token: String, test_mode: Option) -> Result<()> { - let mut move_home = std::env::var("MOVE_HOME").unwrap_or_else(|_| { - format!( - "{}/.move", - std::env::var("HOME").expect("env var 'HOME' must be set") - ) - }); + let mut move_home; if let Some(test_mode) = test_mode { + move_home = std::env::var("TEST_MOVE_HOME").unwrap(); if !test_mode.test_path.is_empty() { move_home.push_str(&test_mode.test_path); } + } else { + move_home = std::env::var("MOVE_HOME").unwrap_or_else(|_| { + format!( + "{}/.move", + std::env::var("HOME").expect("env var 'HOME' must be set") + ) + }); } fs::create_dir_all(&move_home)?; let credential_path = move_home + "/credential.toml"; - let credential_file = PathBuf::from(&credential_path.clone()); + let credential_file = PathBuf::from(&credential_path); if !credential_file.exists() { File::create(&credential_path)?; } @@ -126,25 +126,19 @@ fn set_permissions(file: &File, mode: u32) -> Result<()> { #[cfg(test)] mod tests { use super::*; - use home::home_dir; use std::env; fn setup_move_home(test_path: &str) -> (String, String) { - let mut move_home = env::var("MOVE_HOME").unwrap_or_else(|_| { - env::var("HOME").unwrap_or_else(|_| { - let home_dir = home_dir().unwrap().to_string_lossy().to_string(); - env::set_var("HOME", &home_dir); - home_dir - }) - }); - move_home.push_str("/.move"); + let cwd = env::current_dir().unwrap(); + let mut move_home: String = String::from(cwd.to_string_lossy()); + env::set_var("TEST_MOVE_HOME", &move_home); if !test_path.is_empty() { move_home.push_str(&test_path); } else { move_home.push_str("/test"); } let credential_path = move_home.clone() + "/credential.toml"; - return (move_home, credential_path); + (move_home, credential_path) } fn clean_up(move_home: &str) { diff --git a/language/tools/move-cli/tests/cli_tests.rs b/language/tools/move-cli/tests/cli_tests.rs index eade257d7c..90e0fe87f8 100644 --- a/language/tools/move-cli/tests/cli_tests.rs +++ b/language/tools/move-cli/tests/cli_tests.rs @@ -5,14 +5,11 @@ use move_cli::sandbox::commands::test; #[cfg(unix)] use std::fs::File; -use std::io::Write; -use std::{env, fs}; +use std::{env, fs, io::Write}; -use home::home_dir; #[cfg(unix)] use std::os::unix::fs::PermissionsExt; -use std::path::PathBuf; -use std::process::Stdio; +use std::{path::PathBuf, process::Stdio}; use toml_edit::easy::Value; pub const CLI_METATEST_PATH: [&str; 3] = ["tests", "metatests", "args.txt"]; @@ -82,7 +79,7 @@ fn save_credential_works() { .stdin .as_ref() .unwrap() - .write(token.as_bytes()) + .write_all(token.as_bytes()) .unwrap(); match child.wait_with_output() { Ok(output) => { @@ -138,7 +135,7 @@ fn save_credential_fails_if_undeletable_credential_file_exists() { .stdin .as_ref() .unwrap() - .write(token.as_bytes()) + .write_all(token.as_bytes()) .unwrap(); match child.wait_with_output() { Ok(output) => { @@ -166,11 +163,14 @@ fn save_credential_fails_if_undeletable_credential_file_exists() { } fn setup_move_home(test_path: &str) -> (String, String) { - let move_home = home_dir().unwrap().to_string_lossy().to_string() + "/.move" + test_path; + let cwd = env::current_dir().unwrap(); + let mut move_home: String = String::from(cwd.to_string_lossy()); + env::set_var("TEST_MOVE_HOME", &move_home); + move_home.push_str(&test_path); let _ = fs::remove_dir_all(&move_home); fs::create_dir_all(&move_home).unwrap(); let credential_path = move_home.clone() + "/credential.toml"; - return (move_home, credential_path); + (move_home, credential_path) } fn clean_up(move_home: &str) { From 6a505b00c68709abfd38e632597ffcc6ee3de715 Mon Sep 17 00:00:00 2001 From: ea-open-source Date: Wed, 13 Jul 2022 15:08:18 +0700 Subject: [PATCH 6/8] Refactor to be specific to Movey --- Cargo.lock | 2 ++ language/move-command-line-common/Cargo.toml | 2 ++ language/move-command-line-common/src/env.rs | 14 +++++++++ language/move-command-line-common/src/lib.rs | 1 + .../move-command-line-common/src/movey.rs | 8 +++++ language/tools/move-cli/src/lib.rs | 10 ++++--- .../src/{login => movey_login}/cli.rs | 24 +++++---------- .../src/{login => movey_login}/mod.rs | 0 language/tools/move-cli/tests/cli_tests.rs | 30 +++++++++++-------- .../src/source_package/manifest_parser.rs | 11 ++----- 10 files changed, 61 insertions(+), 41 deletions(-) create mode 100644 language/move-command-line-common/src/movey.rs rename language/tools/move-cli/src/{login => movey_login}/cli.rs (94%) rename language/tools/move-cli/src/{login => movey_login}/mod.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index 18f4b62b75..ee0b366216 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2463,9 +2463,11 @@ version = "0.1.0" dependencies = [ "anyhow", "difference", + "dirs-next", "hex", "move-core-types", "num-bigint 0.4.0", + "once_cell", "serde 1.0.130", "sha2", "walkdir", diff --git a/language/move-command-line-common/Cargo.toml b/language/move-command-line-common/Cargo.toml index 361d3431c2..3804ad44ce 100644 --- a/language/move-command-line-common/Cargo.toml +++ b/language/move-command-line-common/Cargo.toml @@ -12,10 +12,12 @@ edition = "2021" [dependencies] anyhow = "1.0.52" difference = "2.0.0" +dirs-next = "2.0.0" walkdir = "2.3.1" sha2 = "0.9.3" hex = "0.4.3" num-bigint = "0.4.0" +once_cell = "1.7.2" serde = { version = "1.0.124", features = ["derive"] } move-core-types = { path = "../move-core/types" } diff --git a/language/move-command-line-common/src/env.rs b/language/move-command-line-common/src/env.rs index 888a01cca3..ac247159c8 100644 --- a/language/move-command-line-common/src/env.rs +++ b/language/move-command-line-common/src/env.rs @@ -2,6 +2,8 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 +use once_cell::sync::Lazy; + /// An environment variable which can be set to cause the move compiler to generate /// file formats at a given version. Only version v5 and greater are supported. const BYTECODE_VERSION_ENV_VAR: &str = "MOVE_BYTECODE_VERSION"; @@ -22,3 +24,15 @@ pub fn read_bool_env_var(v: &str) -> bool { let val = read_env_var(v).to_lowercase(); val.parse::() == Ok(true) || val.parse::() == Ok(1) } + +pub static MOVE_HOME: Lazy = Lazy::new(|| { + std::env::var("MOVE_HOME").unwrap_or_else(|_| { + format!( + "{}/.move", + dirs_next::home_dir() + .expect("user's home directory not found") + .to_str() + .unwrap() + ) + }) +}); diff --git a/language/move-command-line-common/src/lib.rs b/language/move-command-line-common/src/lib.rs index 5bb4087c9a..004903963d 100644 --- a/language/move-command-line-common/src/lib.rs +++ b/language/move-command-line-common/src/lib.rs @@ -8,6 +8,7 @@ pub mod address; pub mod character_sets; pub mod env; pub mod files; +pub mod movey; pub mod parser; pub mod testing; pub mod types; diff --git a/language/move-command-line-common/src/movey.rs b/language/move-command-line-common/src/movey.rs new file mode 100644 index 0000000000..2308ed33cf --- /dev/null +++ b/language/move-command-line-common/src/movey.rs @@ -0,0 +1,8 @@ +// Copyright (c) The Diem Core Contributors +// Copyright (c) The Move Contributors +// SPDX-License-Identifier: Apache-2.0 + +#[cfg(debug_assertions)] +pub const MOVEY_URL: &str = "https://movey-app-staging.herokuapp.com"; +#[cfg(not(debug_assertions))] +pub const MOVEY_URL: &str = "https://www.movey.net"; diff --git a/language/tools/move-cli/src/lib.rs b/language/tools/move-cli/src/lib.rs index 3529c5c890..3cbbbeb2cc 100644 --- a/language/tools/move-cli/src/lib.rs +++ b/language/tools/move-cli/src/lib.rs @@ -10,7 +10,7 @@ use move_package::BuildConfig; pub mod base; pub mod experimental; -pub mod login; +pub mod movey_login; pub mod sandbox; /// Default directory where saved Move resources live @@ -92,8 +92,8 @@ pub enum Command { #[clap(subcommand)] cmd: experimental::cli::ExperimentalCommand, }, - #[clap(name = "login")] - Login { + #[clap(name = "movey-login")] + MoveyLogin { #[clap(long = "test-path")] test_path: Option, }, @@ -127,7 +127,9 @@ pub fn run_cli( &storage_dir, ), Command::Experimental { storage_dir, cmd } => cmd.handle_command(&move_args, &storage_dir), - Command::Login { test_path } => login::cli::handle_login_commands(test_path), + Command::MoveyLogin { test_path } => { + movey_login::cli::handle_movey_login_commands(test_path) + } } } diff --git a/language/tools/move-cli/src/login/cli.rs b/language/tools/move-cli/src/movey_login/cli.rs similarity index 94% rename from language/tools/move-cli/src/login/cli.rs rename to language/tools/move-cli/src/movey_login/cli.rs index ac2bb3a746..9ffec0bd82 100644 --- a/language/tools/move-cli/src/login/cli.rs +++ b/language/tools/move-cli/src/movey_login/cli.rs @@ -3,23 +3,20 @@ // SPDX-License-Identifier: Apache-2.0 use anyhow::{bail, Result}; +use move_command_line_common::{env::MOVE_HOME, movey::MOVEY_URL}; use std::{fs, fs::File, io, path::PathBuf}; use toml_edit::easy::{map::Map, Value}; +pub const MOVEY_API_KEY_PATH: &str = "/movey_api_key.toml"; + pub struct TestMode { pub test_path: String, } -pub fn handle_login_commands(test_path: Option) -> Result<()> { - let url: &str; - if cfg!(debug_assertions) { - url = "https://movey-app-staging.herokuapp.com"; - } else { - url = "https://movey.net"; - } +pub fn handle_movey_login_commands(test_path: Option) -> Result<()> { println!( "Please paste the API Token found on {}/settings/tokens below", - url + MOVEY_URL ); let mut line = String::new(); loop { @@ -58,15 +55,10 @@ pub fn save_credential(token: String, test_mode: Option) -> Result<()> move_home.push_str(&test_mode.test_path); } } else { - move_home = std::env::var("MOVE_HOME").unwrap_or_else(|_| { - format!( - "{}/.move", - std::env::var("HOME").expect("env var 'HOME' must be set") - ) - }); + move_home = MOVE_HOME.clone(); } fs::create_dir_all(&move_home)?; - let credential_path = move_home + "/credential.toml"; + let credential_path = move_home + MOVEY_API_KEY_PATH; let credential_file = PathBuf::from(&credential_path); if !credential_file.exists() { File::create(&credential_path)?; @@ -137,7 +129,7 @@ mod tests { } else { move_home.push_str("/test"); } - let credential_path = move_home.clone() + "/credential.toml"; + let credential_path = move_home.clone() + MOVEY_API_KEY_PATH; (move_home, credential_path) } diff --git a/language/tools/move-cli/src/login/mod.rs b/language/tools/move-cli/src/movey_login/mod.rs similarity index 100% rename from language/tools/move-cli/src/login/mod.rs rename to language/tools/move-cli/src/movey_login/mod.rs diff --git a/language/tools/move-cli/tests/cli_tests.rs b/language/tools/move-cli/tests/cli_tests.rs index 90e0fe87f8..33a228746e 100644 --- a/language/tools/move-cli/tests/cli_tests.rs +++ b/language/tools/move-cli/tests/cli_tests.rs @@ -2,7 +2,8 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use move_cli::sandbox::commands::test; +use move_cli::{movey_login::cli::MOVEY_API_KEY_PATH, sandbox::commands::test}; +use move_command_line_common::movey::MOVEY_URL; #[cfg(unix)] use std::fs::File; use std::{env, fs, io::Write}; @@ -68,7 +69,12 @@ fn save_credential_works() { match std::process::Command::new(cli_exe) .current_dir(".") - .args(["login", "--test", "--test-path", "/save_credential_works"]) + .args([ + "movey-login", + "--test", + "--test-path", + "/save_credential_works", + ]) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .spawn() @@ -83,10 +89,10 @@ fn save_credential_works() { .unwrap(); match child.wait_with_output() { Ok(output) => { - assert!(String::from_utf8_lossy(&output.stdout).contains( - "Please paste the API Token found on \ - https://movey-app-staging.herokuapp.com/settings/tokens below" - )); + assert!(String::from_utf8_lossy(&output.stdout).contains(&format!( + "Please paste the API Token found on {}/settings/tokens below", + MOVEY_URL + ))); Ok(()) } Err(error) => Err(error), @@ -119,7 +125,7 @@ fn save_credential_fails_if_undeletable_credential_file_exists() { match std::process::Command::new(cli_exe) .current_dir(".") .args([ - "login", + "movey-login", "--test", "--test-path", "/save_credential_fails_if_undeletable_credential_file_exists", @@ -139,10 +145,10 @@ fn save_credential_fails_if_undeletable_credential_file_exists() { .unwrap(); match child.wait_with_output() { Ok(output) => { - assert!(String::from_utf8_lossy(&output.stdout).contains( - "Please paste the API Token found on \ - https://movey-app-staging.herokuapp.com/settings/tokens below" - )); + assert!(String::from_utf8_lossy(&output.stdout).contains(&format!( + "Please paste the API Token found on {}/settings/tokens below", + MOVEY_URL + ))); assert!(String::from_utf8_lossy(&output.stderr) .contains("Error: Error reading input: Permission denied (os error 13)")); Ok(()) @@ -169,7 +175,7 @@ fn setup_move_home(test_path: &str) -> (String, String) { move_home.push_str(&test_path); let _ = fs::remove_dir_all(&move_home); fs::create_dir_all(&move_home).unwrap(); - let credential_path = move_home.clone() + "/credential.toml"; + let credential_path = move_home.clone() + MOVEY_API_KEY_PATH; (move_home, credential_path) } diff --git a/language/tools/move-package/src/source_package/manifest_parser.rs b/language/tools/move-package/src/source_package/manifest_parser.rs index 69f17cce68..e6139c1d67 100644 --- a/language/tools/move-package/src/source_package/manifest_parser.rs +++ b/language/tools/move-package/src/source_package/manifest_parser.rs @@ -4,6 +4,7 @@ use crate::{source_package::parsed_manifest as PM, Architecture}; use anyhow::{bail, format_err, Context, Result}; +use move_command_line_common::env::MOVE_HOME; use move_core_types::account_address::{AccountAddress, AccountAddressParseError}; use move_symbol_pool::symbol::Symbol; use std::{ @@ -328,15 +329,7 @@ fn parse_dependency(tval: TV) -> Result { } (None, Some(git)) => { // Look to see if a MOVE_HOME has been set. Otherwise default to $HOME - let move_home = std::env::var("MOVE_HOME").unwrap_or_else(|_| { - format!( - "{}/.move", - dirs_next::home_dir() - .expect("user's home directory not found") - .to_str() - .unwrap() - ) - }); + let move_home = MOVE_HOME.clone(); let rev_name = match table.remove("rev") { None => bail!("Git revision not supplied for dependency"), Some(r) => Symbol::from( From 9414e8a645c712fa750a61d34e840de9243fb247 Mon Sep 17 00:00:00 2001 From: ea-open-source Date: Tue, 2 Aug 2022 18:01:48 +0700 Subject: [PATCH 7/8] Refactor movey-login logic, refactor movey tests --- language/move-command-line-common/src/lib.rs | 2 +- .../src/{movey.rs => movey_constants.rs} | 0 language/tools/move-cli/src/base/mod.rs | 1 + .../cli.rs => base/movey_login.rs} | 165 ++++++++---------- language/tools/move-cli/src/lib.rs | 12 +- .../tools/move-cli/src/movey_login/mod.rs | 5 - language/tools/move-cli/tests/cli_tests.rs | 23 +-- 7 files changed, 80 insertions(+), 128 deletions(-) rename language/move-command-line-common/src/{movey.rs => movey_constants.rs} (100%) rename language/tools/move-cli/src/{movey_login/cli.rs => base/movey_login.rs} (60%) delete mode 100644 language/tools/move-cli/src/movey_login/mod.rs diff --git a/language/move-command-line-common/src/lib.rs b/language/move-command-line-common/src/lib.rs index 004903963d..201829bad2 100644 --- a/language/move-command-line-common/src/lib.rs +++ b/language/move-command-line-common/src/lib.rs @@ -8,7 +8,7 @@ pub mod address; pub mod character_sets; pub mod env; pub mod files; -pub mod movey; +pub mod movey_constants; pub mod parser; pub mod testing; pub mod types; diff --git a/language/move-command-line-common/src/movey.rs b/language/move-command-line-common/src/movey_constants.rs similarity index 100% rename from language/move-command-line-common/src/movey.rs rename to language/move-command-line-common/src/movey_constants.rs diff --git a/language/tools/move-cli/src/base/mod.rs b/language/tools/move-cli/src/base/mod.rs index 0633fcc2dd..03b0d566c3 100644 --- a/language/tools/move-cli/src/base/mod.rs +++ b/language/tools/move-cli/src/base/mod.rs @@ -7,6 +7,7 @@ pub mod disassemble; pub mod docgen; pub mod errmap; pub mod info; +pub mod movey_login; pub mod new; pub mod prove; pub mod test; diff --git a/language/tools/move-cli/src/movey_login/cli.rs b/language/tools/move-cli/src/base/movey_login.rs similarity index 60% rename from language/tools/move-cli/src/movey_login/cli.rs rename to language/tools/move-cli/src/base/movey_login.rs index 9ffec0bd82..4e5786601e 100644 --- a/language/tools/move-cli/src/movey_login/cli.rs +++ b/language/tools/move-cli/src/base/movey_login.rs @@ -3,100 +3,85 @@ // SPDX-License-Identifier: Apache-2.0 use anyhow::{bail, Result}; -use move_command_line_common::{env::MOVE_HOME, movey::MOVEY_URL}; +use clap::Parser; +use move_command_line_common::{env::MOVE_HOME, movey_constants::MOVEY_URL}; use std::{fs, fs::File, io, path::PathBuf}; use toml_edit::easy::{map::Map, Value}; -pub const MOVEY_API_KEY_PATH: &str = "/movey_api_key.toml"; - -pub struct TestMode { - pub test_path: String, -} - -pub fn handle_movey_login_commands(test_path: Option) -> Result<()> { - println!( - "Please paste the API Token found on {}/settings/tokens below", - MOVEY_URL - ); - let mut line = String::new(); - loop { - match io::stdin().read_line(&mut line) { - Ok(_) => { - if let Some('\n') = line.chars().next_back() { - line.pop(); +pub const MOVEY_CREDENTIAL_PATH: &str = "/movey_credential.toml"; + +#[derive(Parser)] +#[clap(name = "movey-login")] +pub struct MoveyLogin; + +impl MoveyLogin { + pub fn execute(self) -> Result<()> { + println!( + "Please paste the API Token found on {}/settings/tokens below", + MOVEY_URL + ); + let mut line = String::new(); + loop { + match io::stdin().read_line(&mut line) { + Ok(_) => { + line = line.trim().to_string(); + if !line.is_empty() { + break; + } + println!("Invalid API Token. Try again!"); } - if let Some('\r') = line.chars().next_back() { - line.pop(); + Err(err) => { + bail!("Error reading file: {}", err); } - if !line.is_empty() { - break; - } - println!("Invalid API Token. Try again!"); - } - Err(err) => { - bail!("Error reading file: {}", err); } } + Self::save_credential(line, MOVE_HOME.clone())?; + println!("Token for Movey saved."); + Ok(()) } - let mut test_mode: Option = None; - if let Some(path) = test_path { - test_mode = Some(TestMode { test_path: path }); - } - save_credential(line, test_mode)?; - println!("Token for Movey saved."); - Ok(()) -} -pub fn save_credential(token: String, test_mode: Option) -> Result<()> { - let mut move_home; - if let Some(test_mode) = test_mode { - move_home = std::env::var("TEST_MOVE_HOME").unwrap(); - if !test_mode.test_path.is_empty() { - move_home.push_str(&test_mode.test_path); + pub fn save_credential(token: String, move_home: String) -> Result<()> { + fs::create_dir_all(&move_home)?; + let credential_path = move_home + MOVEY_CREDENTIAL_PATH; + let credential_file = PathBuf::from(&credential_path); + if !credential_file.exists() { + let credential_file = File::create(&credential_path)?; + set_permissions(&credential_file, 0o600)?; } - } else { - move_home = MOVE_HOME.clone(); - } - fs::create_dir_all(&move_home)?; - let credential_path = move_home + MOVEY_API_KEY_PATH; - let credential_file = PathBuf::from(&credential_path); - if !credential_file.exists() { - File::create(&credential_path)?; - } - let old_contents: String; - match fs::read_to_string(&credential_path) { - Ok(contents) => { - old_contents = contents; + let old_contents: String; + match fs::read_to_string(&credential_path) { + Ok(contents) => { + old_contents = contents; + } + Err(error) => bail!("Error reading input: {}", error), } - Err(error) => bail!("Error reading input: {}", error), - } - let mut toml: Value = old_contents - .parse() - .map_err(|e| anyhow::Error::from(e).context("could not parse input as TOML"))?; - - if let Some(registry) = toml.as_table_mut().unwrap().get_mut("registry") { - if let Some(toml_token) = registry.as_table_mut().unwrap().get_mut("token") { - *toml_token = Value::String(token); + let mut toml: Value = old_contents + .parse() + .map_err(|e| anyhow::Error::from(e).context("could not parse input as TOML"))?; + + // only update token key, keep the rest of the file intact + if let Some(registry) = toml.as_table_mut().unwrap().get_mut("registry") { + if let Some(toml_token) = registry.as_table_mut().unwrap().get_mut("token") { + *toml_token = Value::String(token); + } else { + registry + .as_table_mut() + .unwrap() + .insert(String::from("token"), Value::String(token)); + } } else { - registry - .as_table_mut() + let mut value = Map::new(); + value.insert(String::from("token"), Value::String(token)); + toml.as_table_mut() .unwrap() - .insert(String::from("token"), Value::String(token)); + .insert(String::from("registry"), Value::Table(value)); } - } else { - let mut value = Map::new(); - value.insert(String::from("token"), Value::String(token)); - toml.as_table_mut() - .unwrap() - .insert(String::from("registry"), Value::Table(value)); - } - let new_contents = toml.to_string(); - fs::write(credential_file, new_contents).expect("Unable to write file"); - let file = File::open(&credential_path)?; - set_permissions(&file, 0o600)?; - Ok(()) + let new_contents = toml.to_string(); + fs::write(credential_file, new_contents).expect("Unable to write file"); + Ok(()) + } } #[cfg(unix)] @@ -123,13 +108,12 @@ mod tests { fn setup_move_home(test_path: &str) -> (String, String) { let cwd = env::current_dir().unwrap(); let mut move_home: String = String::from(cwd.to_string_lossy()); - env::set_var("TEST_MOVE_HOME", &move_home); if !test_path.is_empty() { move_home.push_str(&test_path); } else { move_home.push_str("/test"); } - let credential_path = move_home.clone() + MOVEY_API_KEY_PATH; + let credential_path = move_home.clone() + MOVEY_CREDENTIAL_PATH; (move_home, credential_path) } @@ -142,11 +126,7 @@ mod tests { let (move_home, credential_path) = setup_move_home("/save_credential_works_if_no_credential_file_exists"); let _ = fs::remove_dir_all(&move_home); - - let test_mode = Some(TestMode { - test_path: String::from("/save_credential_works_if_no_credential_file_exists"), - }); - save_credential(String::from("test_token"), test_mode).unwrap(); + MoveyLogin::save_credential(String::from("test_token"), move_home.clone()).unwrap(); let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); let mut toml: Value = contents.parse().unwrap(); @@ -170,10 +150,7 @@ mod tests { let mut toml: Value = contents.parse().unwrap(); assert!(toml.as_table_mut().unwrap().get_mut("registry").is_none()); - let test_mode = Some(TestMode { - test_path: String::from("/save_credential_works_if_empty_credential_file_exists"), - }); - save_credential(String::from("test_token"), test_mode).unwrap(); + MoveyLogin::save_credential(String::from("test_token"), move_home.clone()).unwrap(); let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); let mut toml: Value = contents.parse().unwrap(); @@ -204,10 +181,7 @@ mod tests { assert!(token.to_string().contains("old_test_token")); assert!(!token.to_string().contains("new_world")); - let test_mode = Some(TestMode { - test_path: String::from("/save_credential_works_if_token_field_exists"), - }); - save_credential(String::from("new_world"), test_mode).unwrap(); + MoveyLogin::save_credential(String::from("new_world"), move_home.clone()).unwrap(); let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); let mut toml: Value = contents.parse().unwrap(); @@ -239,10 +213,7 @@ mod tests { let token = registry.as_table_mut().unwrap().get_mut("token").unwrap(); assert!(!token.to_string().contains("test_token")); - let test_mode = Some(TestMode { - test_path: String::from("/save_credential_works_if_empty_token_field_exists"), - }); - save_credential(String::from("test_token"), test_mode).unwrap(); + MoveyLogin::save_credential(String::from("test_token"), move_home.clone()).unwrap(); let contents = fs::read_to_string(&credential_path).expect("Unable to read file"); let mut toml: Value = contents.parse().unwrap(); diff --git a/language/tools/move-cli/src/lib.rs b/language/tools/move-cli/src/lib.rs index 3cbbbeb2cc..f7cef69805 100644 --- a/language/tools/move-cli/src/lib.rs +++ b/language/tools/move-cli/src/lib.rs @@ -4,13 +4,12 @@ use base::{ build::Build, coverage::Coverage, disassemble::Disassemble, docgen::Docgen, errmap::Errmap, - info::Info, new::New, prove::Prove, test::Test, + info::Info, movey_login::MoveyLogin, new::New, prove::Prove, test::Test, }; use move_package::BuildConfig; pub mod base; pub mod experimental; -pub mod movey_login; pub mod sandbox; /// Default directory where saved Move resources live @@ -93,10 +92,7 @@ pub enum Command { cmd: experimental::cli::ExperimentalCommand, }, #[clap(name = "movey-login")] - MoveyLogin { - #[clap(long = "test-path")] - test_path: Option, - }, + MoveyLogin(MoveyLogin), } pub fn run_cli( @@ -127,9 +123,7 @@ pub fn run_cli( &storage_dir, ), Command::Experimental { storage_dir, cmd } => cmd.handle_command(&move_args, &storage_dir), - Command::MoveyLogin { test_path } => { - movey_login::cli::handle_movey_login_commands(test_path) - } + Command::MoveyLogin(c) => c.execute(), } } diff --git a/language/tools/move-cli/src/movey_login/mod.rs b/language/tools/move-cli/src/movey_login/mod.rs deleted file mode 100644 index eb76e266c6..0000000000 --- a/language/tools/move-cli/src/movey_login/mod.rs +++ /dev/null @@ -1,5 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -pub mod cli; diff --git a/language/tools/move-cli/tests/cli_tests.rs b/language/tools/move-cli/tests/cli_tests.rs index 33a228746e..f0c6d060c2 100644 --- a/language/tools/move-cli/tests/cli_tests.rs +++ b/language/tools/move-cli/tests/cli_tests.rs @@ -2,8 +2,8 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use move_cli::{movey_login::cli::MOVEY_API_KEY_PATH, sandbox::commands::test}; -use move_command_line_common::movey::MOVEY_URL; +use move_cli::{base::movey_login::MOVEY_CREDENTIAL_PATH, sandbox::commands::test}; +use move_command_line_common::movey_constants::MOVEY_URL; #[cfg(unix)] use std::fs::File; use std::{env, fs, io::Write}; @@ -68,13 +68,9 @@ fn save_credential_works() { assert!(fs::read_to_string(&credential_path).is_err()); match std::process::Command::new(cli_exe) + .env("MOVE_HOME", &move_home) .current_dir(".") - .args([ - "movey-login", - "--test", - "--test-path", - "/save_credential_works", - ]) + .args(["movey-login"]) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .spawn() @@ -123,13 +119,9 @@ fn save_credential_fails_if_undeletable_credential_file_exists() { file.set_permissions(perms).unwrap(); match std::process::Command::new(cli_exe) + .env("MOVE_HOME", &move_home) .current_dir(".") - .args([ - "movey-login", - "--test", - "--test-path", - "/save_credential_fails_if_undeletable_credential_file_exists", - ]) + .args(["movey-login"]) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) @@ -171,11 +163,10 @@ fn save_credential_fails_if_undeletable_credential_file_exists() { fn setup_move_home(test_path: &str) -> (String, String) { let cwd = env::current_dir().unwrap(); let mut move_home: String = String::from(cwd.to_string_lossy()); - env::set_var("TEST_MOVE_HOME", &move_home); move_home.push_str(&test_path); let _ = fs::remove_dir_all(&move_home); fs::create_dir_all(&move_home).unwrap(); - let credential_path = move_home.clone() + MOVEY_API_KEY_PATH; + let credential_path = move_home.clone() + MOVEY_CREDENTIAL_PATH; (move_home, credential_path) } From 3c48bd137ac0f6bd3884678a112ad52481dade71 Mon Sep 17 00:00:00 2001 From: ea-open-source Date: Fri, 5 Aug 2022 16:12:09 +0700 Subject: [PATCH 8/8] Add OS conditional compilation when create movey credential file, more precise error when read movey credential --- .../src/movey_constants.rs | 1 - .../tools/move-cli/src/base/movey_login.rs | 34 ++++++++++++------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/language/move-command-line-common/src/movey_constants.rs b/language/move-command-line-common/src/movey_constants.rs index 2308ed33cf..4567bdbf4d 100644 --- a/language/move-command-line-common/src/movey_constants.rs +++ b/language/move-command-line-common/src/movey_constants.rs @@ -1,4 +1,3 @@ -// Copyright (c) The Diem Core Contributors // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 diff --git a/language/tools/move-cli/src/base/movey_login.rs b/language/tools/move-cli/src/base/movey_login.rs index 4e5786601e..214f5ae9c6 100644 --- a/language/tools/move-cli/src/base/movey_login.rs +++ b/language/tools/move-cli/src/base/movey_login.rs @@ -1,4 +1,3 @@ -// Copyright (c) The Diem Core Contributors // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 @@ -45,8 +44,7 @@ impl MoveyLogin { let credential_path = move_home + MOVEY_CREDENTIAL_PATH; let credential_file = PathBuf::from(&credential_path); if !credential_file.exists() { - let credential_file = File::create(&credential_path)?; - set_permissions(&credential_file, 0o600)?; + create_credential_file(&credential_path)?; } let old_contents: String; @@ -56,9 +54,12 @@ impl MoveyLogin { } Err(error) => bail!("Error reading input: {}", error), } - let mut toml: Value = old_contents - .parse() - .map_err(|e| anyhow::Error::from(e).context("could not parse input as TOML"))?; + let mut toml: Value = old_contents.parse().map_err(|e| { + anyhow::Error::from(e).context(format!( + "could not parse input at {} as TOML", + &credential_path + )) + })?; // only update token key, keep the rest of the file intact if let Some(registry) = toml.as_table_mut().unwrap().get_mut("registry") { @@ -85,21 +86,30 @@ impl MoveyLogin { } #[cfg(unix)] -fn set_permissions(file: &File, mode: u32) -> Result<()> { +fn create_credential_file(credential_path: &str) -> Result<()> { use std::os::unix::fs::PermissionsExt; + let credential_file = File::create(&credential_path)?; - let mut perms = file.metadata()?.permissions(); - perms.set_mode(mode); - file.set_permissions(perms)?; + let mut perms = credential_file.metadata()?.permissions(); + perms.set_mode(0o600); + credential_file.set_permissions(perms)?; Ok(()) } -#[cfg(not(unix))] +#[cfg(windows)] #[allow(unused)] -fn set_permissions(file: &File, mode: u32) -> Result<()> { +fn create_credential_file(credential_path: &str) -> Result<()> { + let windows_path = credential_path.replace("/", "\\"); + File::create(&windows_path)?; Ok(()) } +#[cfg(not(any(unix, windows)))] +#[allow(unused)] +fn create_credential_file(credential_path: &str) -> Result<()> { + bail!("OS not supported") +} + #[cfg(test)] mod tests { use super::*;