From f996f5855e54388f3d4f77e46b159b2498c850d4 Mon Sep 17 00:00:00 2001 From: Teng Zhang Date: Wed, 14 Aug 2024 22:33:55 +0000 Subject: [PATCH] [CLI] Integrate movefmt into Aptos CLI (#13990) * integrate movefmt * refactor * add cli release note --- Cargo.lock | 1 + crates/aptos/CHANGELOG.md | 2 + crates/aptos/Cargo.toml | 1 + crates/aptos/src/move_tool/fmt.rs | 143 +++++++++++++++++++++++ crates/aptos/src/move_tool/mod.rs | 4 + crates/aptos/src/update/mod.rs | 20 +++- crates/aptos/src/update/movefmt.rs | 131 +++++++++++++++++++++ crates/aptos/src/update/revela.rs | 100 +++------------- crates/aptos/src/update/tool.rs | 7 +- crates/aptos/src/update/update_helper.rs | 106 +++++++++++++++++ 10 files changed, 428 insertions(+), 87 deletions(-) create mode 100644 crates/aptos/src/move_tool/fmt.rs create mode 100644 crates/aptos/src/update/movefmt.rs create mode 100644 crates/aptos/src/update/update_helper.rs diff --git a/Cargo.lock b/Cargo.lock index ba2732adb89be..871e7f57e779e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -333,6 +333,7 @@ dependencies = [ "poem", "processor", "rand 0.7.3", + "regex", "reqwest 0.11.23", "self_update", "serde", diff --git a/crates/aptos/CHANGELOG.md b/crates/aptos/CHANGELOG.md index c129286e6974e..e2a67e481ff6e 100644 --- a/crates/aptos/CHANGELOG.md +++ b/crates/aptos/CHANGELOG.md @@ -3,6 +3,8 @@ All notable changes to the Aptos CLI will be captured in this file. This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html) and the format set out by [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Unreleased +- Add `aptos update movefmt`. This installs / updates the `movefmt` binary, which is needed for the new `aptos move fmt` subcommand. +- Integrate the Move formatter `movefmt` which is now available via `aptos move fmt` ## [4.0.0] - 2024/08/13 - **Breaking Change**: change key rotation options such that user has to either pass the name of a new profile or explicitly flag that no profile should be generated, since without this update the interactive profile generator could fail out after the key has already been rotated. This forces the check for new profile validity before doing anything onchain. diff --git a/crates/aptos/Cargo.toml b/crates/aptos/Cargo.toml index 891f3c2724fde..1d0ad2e9d13be 100644 --- a/crates/aptos/Cargo.toml +++ b/crates/aptos/Cargo.toml @@ -85,6 +85,7 @@ poem = { workspace = true } # https://github.com/aptos-labs/aptos-core/pull/12568 processor = { git = "https://github.com/aptos-labs/aptos-indexer-processors.git", rev = "fa1ce4947f4c2be57529f1c9732529e05a06cb7f", default-features = false } rand = { workspace = true } +regex = { workspace = true } reqwest = { workspace = true } self_update = { git = "https://github.com/banool/self_update.git", rev = "8306158ad0fd5b9d4766a3c6bf967e7ef0ea5c4b", features = ["archive-zip", "compression-zip-deflate"] } serde = { workspace = true } diff --git a/crates/aptos/src/move_tool/fmt.rs b/crates/aptos/src/move_tool/fmt.rs new file mode 100644 index 0000000000000..a86171701cdcf --- /dev/null +++ b/crates/aptos/src/move_tool/fmt.rs @@ -0,0 +1,143 @@ +// Copyright (c) Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 + +use crate::{ + common::types::{CliCommand, CliError, CliTypedResult}, + update::get_movefmt_path, +}; +use async_trait::async_trait; +use clap::{Args, Parser}; +use std::{collections::BTreeMap, path::PathBuf, process::Command}; + +/// Format the Move source code. +#[derive(Debug, Parser)] +pub struct Fmt { + #[clap(flatten)] + pub command: FmtCommand, +} + +#[derive(clap::ValueEnum, Clone, Copy, Debug, Eq, PartialEq, PartialOrd)] +pub enum EmitMode { + Overwrite, + NewFile, + StdOut, + Diff, +} + +#[derive(Debug, Args)] +#[clap(group(clap::ArgGroup::new("input") +.required(true) +.multiple(false) +.args(&["file_path", "dir_path"]), +))] +pub struct FmtCommand { + /// How to generate and show the result after reformatting + #[clap(long, value_enum, default_value = "overwrite")] + emit_mode: EmitMode, + + /// Path to the file to be formatted + #[clap(long, group = "input")] + file_path: Option, + + /// Path to the directory to be formatted + /// if neither file_path or dir_path is set, all move files in the current folder will be formatted + #[clap(long, group = "input")] + dir_path: Option, + + /// Path for the configuration file + /// Recursively searches the given path for the + /// movefmt.toml config file + #[clap(long, value_parser)] + pub config_path: Option, + + /// Set options from command line. These settings take + /// priority over movefmt.toml. + /// Config options can be found at https://github.com/movebit/movefmt/blob/develop/doc/how_to_use.md + #[clap(long, value_parser = crate::common::utils::parse_map::, default_value = "")] + pub(crate) config: BTreeMap, + + #[clap(long, short)] + /// Print verbose output + pub verbose: bool, + + #[clap(long, short)] + /// Print less output + pub quiet: bool, +} + +#[async_trait] +impl CliCommand for Fmt { + fn command_name(&self) -> &'static str { + "Fmt" + } + + async fn execute(mut self) -> CliTypedResult { + self.command.execute().await + } +} + +impl FmtCommand { + async fn execute(self) -> CliTypedResult { + let exe = get_movefmt_path()?; + let mut cmd = Command::new(exe.as_path()); + let input_opt = self.file_path; + let dir_opt = self.dir_path; + let config_path_opt = self.config_path; + let config_map = self.config; + let verbose_flag = self.verbose; + let quiet_flag = self.quiet; + let emit_mode = match self.emit_mode { + EmitMode::Overwrite => "overwrite", + EmitMode::NewFile => "new_file", + EmitMode::StdOut => "stdout", + EmitMode::Diff => "diff", + }; + cmd.arg(format!("--emit={}", emit_mode)); + if let Some(config_path) = config_path_opt { + cmd.arg(format!("--config-path={}", config_path.as_path().display())); + } + if verbose_flag { + cmd.arg("-v"); + } else if quiet_flag { + cmd.arg("-q"); + } + if !config_map.is_empty() { + let mut config_map_str_vec = vec![]; + for (key, value) in config_map { + config_map_str_vec.push(format!("{}={}", key, value)); + } + cmd.arg(format!("--config={}", config_map_str_vec.join(","))); + } + if let Some(file_path) = input_opt { + cmd.arg(format!("--file-path={}", file_path.as_path().display())); + } else { + let dir_path = if let Some(dir_path) = dir_opt { + dir_path.as_path().display().to_string() + } else { + "./".to_string() + }; + cmd.arg(format!("--dir-path={}", dir_path)); + } + let to_cli_error = |e| CliError::IO(exe.display().to_string(), e); + let out = cmd.output().map_err(to_cli_error)?; + if out.status.success() { + // let string_res = String::from_utf8(out.stdout); + match String::from_utf8(out.stdout) { + Ok(output) => { + eprint!("{}", output); + Ok("ok".to_string()) + }, + Err(err) => Err(CliError::UnexpectedError(format!( + "output generated by formatter is not valid utf8: {}", + err + ))), + } + } else { + Err(CliError::UnexpectedError(format!( + "formatter exited with status {}: {}", + out.status, + String::from_utf8(out.stderr).unwrap_or_default() + ))) + } + } +} diff --git a/crates/aptos/src/move_tool/mod.rs b/crates/aptos/src/move_tool/mod.rs index defe545375ec1..9b12c70d8f2e8 100644 --- a/crates/aptos/src/move_tool/mod.rs +++ b/crates/aptos/src/move_tool/mod.rs @@ -21,6 +21,7 @@ use crate::{ move_tool::{ bytecode::{Decompile, Disassemble}, coverage::SummaryCoverage, + fmt::Fmt, manifest::{Dependency, ManifestNamedAddress, MovePackageManifest, PackageInfo}, }, CliCommand, CliResult, @@ -71,6 +72,7 @@ use url::Url; mod aptos_debug_natives; mod bytecode; pub mod coverage; +mod fmt; mod manifest; pub mod package_hooks; mod show; @@ -118,6 +120,7 @@ pub enum MoveTool { VerifyPackage(VerifyPackage), View(ViewFunction), Replay(Replay), + Fmt(Fmt), } impl MoveTool { @@ -152,6 +155,7 @@ impl MoveTool { MoveTool::VerifyPackage(tool) => tool.execute_serialized().await, MoveTool::View(tool) => tool.execute_serialized().await, MoveTool::Replay(tool) => tool.execute_serialized().await, + MoveTool::Fmt(tool) => tool.execute_serialized().await, } } } diff --git a/crates/aptos/src/update/mod.rs b/crates/aptos/src/update/mod.rs index fe784c91efdda..f854bca2219fe 100644 --- a/crates/aptos/src/update/mod.rs +++ b/crates/aptos/src/update/mod.rs @@ -6,12 +6,15 @@ mod aptos; mod helpers; +mod movefmt; mod revela; mod tool; +mod update_helper; use crate::common::types::CliTypedResult; use anyhow::{anyhow, Context, Result}; pub use helpers::get_additional_binaries_dir; +pub use movefmt::get_movefmt_path; pub use revela::get_revela_path; use self_update::{update::ReleaseUpdate, version::bump_is_greater, Status}; pub use tool::UpdateTool; @@ -84,10 +87,21 @@ pub struct UpdateRequiredInfo { impl UpdateRequiredInfo { pub fn update_required(&self) -> Result { match self.current_version { - Some(ref current_version) => bump_is_greater(current_version, &self.target_version) - .context( + Some(ref current_version) => { + // ignore ".beta" or ".rc" for version comparison + // because bump_is_greater only supports comparison bewteen `x.y.z` + // as a result, `1.0.0.rc1` cannot be updated to `1.0.0.rc2` + let target_version = if self.target_version.ends_with(".beta") { + &self.target_version[0..self.target_version.len() - 5] + } else if self.target_version.ends_with(".rc") { + &self.target_version[0..self.target_version.len() - 3] + } else { + &self.target_version + }; + bump_is_greater(current_version, target_version).context( "Failed to compare current and latest CLI versions, please update manually", - ), + ) + }, None => Ok(true), } } diff --git a/crates/aptos/src/update/movefmt.rs b/crates/aptos/src/update/movefmt.rs new file mode 100644 index 0000000000000..a1a77d549f322 --- /dev/null +++ b/crates/aptos/src/update/movefmt.rs @@ -0,0 +1,131 @@ +// Copyright © Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 + +use super::{update_binary, BinaryUpdater, UpdateRequiredInfo}; +use crate::{ + common::types::{CliCommand, CliTypedResult}, + update::update_helper::{build_updater, get_path}, +}; +use anyhow::{Context, Result}; +use async_trait::async_trait; +use clap::Parser; +use self_update::update::ReleaseUpdate; +use std::path::PathBuf; + +const FORMATTER_BINARY_NAME: &str = "movefmt"; +const TARGET_FORMATTER_VERSION: &str = "1.0.0.beta"; + +const FORMATTER_EXE_ENV: &str = "FORMATTER_EXE"; +#[cfg(target_os = "windows")] +const FORMATTER_EXE: &str = "movefmt.exe"; +#[cfg(not(target_os = "windows"))] +const FORMATTER_EXE: &str = "movefmt"; + +/// Update Movefmt, the tool used for formatting Move code. +#[derive(Debug, Parser)] +pub struct FormatterUpdateTool { + /// The owner of the repo to download the binary from. + #[clap(long, default_value = "movebit")] + repo_owner: String, + + /// The name of the repo to download the binary from. + #[clap(long, default_value = "movefmt")] + repo_name: String, + + /// The version to install, e.g. 1.0.1. Use with caution, the default value is a + /// version that is tested for compatibility with the version of the CLI you are + /// using. + #[clap(long, default_value = TARGET_FORMATTER_VERSION)] + target_version: String, + + /// Where to install the binary. Make sure this directory is on your PATH. If not + /// given we will put it in a standard location for your OS that the CLI will use + /// later when the tool is required. + #[clap(long)] + install_dir: Option, + + /// If set, it will check if there are updates for the tool, but not actually update + #[clap(long, default_value_t = false)] + check: bool, +} + +fn extract_movefmt_version(input: &str) -> String { + use regex::Regex; + let re = Regex::new(r"movefmt v\d+\.\d+\.\d+").unwrap(); + if let Some(caps) = re.captures(input) { + let version = caps.get(0).unwrap().as_str().to_string(); + return version.trim_start_matches("movefmt v").to_string(); + } + String::new() +} + +impl BinaryUpdater for FormatterUpdateTool { + fn check(&self) -> bool { + self.check + } + + fn pretty_name(&self) -> &'static str { + "movefmt" + } + + /// Return information about whether an update is required. + fn get_update_info(&self) -> Result { + // Get the current version, if any. + let fmt_path = get_movefmt_path(); + let current_version = match fmt_path { + Ok(path) => { + let output = std::process::Command::new(path) + .arg("--version") + .output() + .context("Failed to get current version of movefmt")?; + let stdout = String::from_utf8(output.stdout) + .context("Failed to parse current version of movefmt as UTF-8")?; + let version = extract_movefmt_version(&stdout); + if !version.is_empty() { + Some(version) + } else { + None + } + }, + Err(_) => None, + }; + + Ok(UpdateRequiredInfo { + current_version, + target_version: self.target_version.trim_start_matches('v').to_string(), + }) + } + + fn build_updater(&self, info: &UpdateRequiredInfo) -> Result> { + build_updater( + info, + self.install_dir.clone(), + self.repo_owner.clone(), + self.repo_name.clone(), + FORMATTER_BINARY_NAME, + "unknown-linux-gnu", + "apple-darwin", + "windows", + ) + } +} + +#[async_trait] +impl CliCommand for FormatterUpdateTool { + fn command_name(&self) -> &'static str { + "UpdateMovefmt" + } + + async fn execute(self) -> CliTypedResult { + update_binary(self).await + } +} + +pub fn get_movefmt_path() -> Result { + get_path( + FORMATTER_BINARY_NAME, + FORMATTER_EXE_ENV, + FORMATTER_BINARY_NAME, + FORMATTER_EXE, + ) +} diff --git a/crates/aptos/src/update/revela.rs b/crates/aptos/src/update/revela.rs index c6b27571dd32a..84fb0674793a3 100644 --- a/crates/aptos/src/update/revela.rs +++ b/crates/aptos/src/update/revela.rs @@ -1,16 +1,15 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 -use super::{get_additional_binaries_dir, update_binary, BinaryUpdater, UpdateRequiredInfo}; -use crate::common::{ - types::{CliCommand, CliTypedResult}, - utils::cli_build_information, +use super::{update_binary, BinaryUpdater, UpdateRequiredInfo}; +use crate::{ + common::types::{CliCommand, CliTypedResult}, + update::update_helper::{build_updater, get_path}, }; -use anyhow::{anyhow, bail, Context, Result}; -use aptos_build_info::BUILD_OS; +use anyhow::{Context, Result}; use async_trait::async_trait; use clap::Parser; -use self_update::{backends::github::Update, update::ReleaseUpdate}; +use self_update::update::ReleaseUpdate; use std::path::PathBuf; const REVELA_BINARY_NAME: &str = "revela"; @@ -91,44 +90,16 @@ impl BinaryUpdater for RevelaUpdateTool { } fn build_updater(&self, info: &UpdateRequiredInfo) -> Result> { - // Determine the target we should download based on how the CLI itself was built. - let arch_str = get_arch(); - let build_info = cli_build_information(); - let target = match build_info.get(BUILD_OS).context("Failed to determine build info of current CLI")?.as_str() { - "linux-aarch64" | "linux-x86_64" => "unknown-linux-gnu", - "macos-aarch64" | "macos-x86_64" => "apple-darwin", - "windows-x86_64" => "pc-windows-gnu", - wildcard => bail!("Self-updating is not supported on your OS ({}) right now, please download the binary manually", wildcard), - }; - - let target = format!("{}-{}", arch_str, target); - - let install_dir = match self.install_dir.clone() { - Some(dir) => dir, - None => { - let dir = get_additional_binaries_dir(); - // Make the directory if it doesn't already exist. - std::fs::create_dir_all(&dir) - .with_context(|| format!("Failed to create directory: {:?}", dir))?; - dir - }, - }; - - let current_version = match &info.current_version { - Some(version) => version, - None => "0.0.0", - }; - - Update::configure() - .bin_install_dir(install_dir) - .bin_name(REVELA_BINARY_NAME) - .repo_owner(&self.repo_owner) - .repo_name(&self.repo_name) - .current_version(current_version) - .target_version_tag(&format!("v{}", info.target_version)) - .target(&target) - .build() - .map_err(|e| anyhow!("Failed to build self-update configuration: {:#}", e)) + build_updater( + info, + self.install_dir.clone(), + self.repo_owner.clone(), + self.repo_name.clone(), + REVELA_BINARY_NAME, + "unknown-linux-gnu", + "apple-darwin", + "pc-windows-gnu", + ) } } @@ -143,43 +114,6 @@ impl CliCommand for RevelaUpdateTool { } } -#[cfg(target_arch = "x86_64")] -fn get_arch() -> &'static str { - "x86_64" -} - -#[cfg(target_arch = "aarch64")] -fn get_arch() -> &'static str { - "aarch64" -} - -#[cfg(not(any(target_arch = "x86_64", target_arch = "aarch64")))] -fn get_arch() -> &'static str { - unimplemented!("Self-updating is not supported on your CPU architecture right now, please download the binary manually") -} - pub fn get_revela_path() -> Result { - // Look at the environment variable first. - if let Ok(path) = std::env::var(REVELA_EXE_ENV) { - return Ok(PathBuf::from(path)); - } - - // See if it is present in the path where we usually install additional binaries. - let path = get_additional_binaries_dir().join(REVELA_BINARY_NAME); - if path.exists() && path.is_file() { - return Ok(path); - } - - // See if we can find the binary in the PATH. - if let Some(path) = pathsearch::find_executable_in_path(REVELA_EXE) { - return Ok(path); - } - - Err(anyhow!( - "Cannot locate the decompiler executable. \ - Environment variable `{}` is not set, and `{}` is not in the PATH. \ - Try running `aptos update revela` to download it.", - REVELA_EXE_ENV, - REVELA_EXE - )) + get_path("decompiler", REVELA_EXE_ENV, REVELA_BINARY_NAME, REVELA_EXE) } diff --git a/crates/aptos/src/update/tool.rs b/crates/aptos/src/update/tool.rs index 60c0c2a5e6b08..c56fd9f58b65b 100644 --- a/crates/aptos/src/update/tool.rs +++ b/crates/aptos/src/update/tool.rs @@ -2,7 +2,10 @@ // SPDX-License-Identifier: Apache-2.0 use super::{aptos::AptosUpdateTool, revela::RevelaUpdateTool}; -use crate::common::types::{CliCommand, CliResult}; +use crate::{ + common::types::{CliCommand, CliResult}, + update::movefmt::FormatterUpdateTool, +}; use clap::Subcommand; /// Update the CLI or other tools it depends on. @@ -10,6 +13,7 @@ use clap::Subcommand; pub enum UpdateTool { Aptos(AptosUpdateTool), Revela(RevelaUpdateTool), + Movefmt(FormatterUpdateTool), } impl UpdateTool { @@ -17,6 +21,7 @@ impl UpdateTool { match self { UpdateTool::Aptos(tool) => tool.execute_serialized().await, UpdateTool::Revela(tool) => tool.execute_serialized().await, + UpdateTool::Movefmt(tool) => tool.execute_serialized().await, } } } diff --git a/crates/aptos/src/update/update_helper.rs b/crates/aptos/src/update/update_helper.rs new file mode 100644 index 0000000000000..50797eae0e85f --- /dev/null +++ b/crates/aptos/src/update/update_helper.rs @@ -0,0 +1,106 @@ +// Copyright © Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 + +use crate::{ + cli_build_information, + update::{get_additional_binaries_dir, UpdateRequiredInfo}, +}; +use anyhow::{anyhow, bail, Context, Result}; +use aptos_build_info::BUILD_OS; +use self_update::{backends::github::Update, update::ReleaseUpdate}; +use std::path::PathBuf; + +#[cfg(target_arch = "x86_64")] +pub fn get_arch() -> &'static str { + "x86_64" +} + +#[cfg(target_arch = "aarch64")] +pub fn get_arch() -> &'static str { + "aarch64" +} + +#[cfg(not(any(target_arch = "x86_64", target_arch = "aarch64")))] +pub fn get_arch() -> &'static str { + unimplemented!("Self-updating is not supported on your CPU architecture right now, please download the binary manually") +} + +pub fn build_updater( + info: &UpdateRequiredInfo, + install_dir: Option, + repo_owner: String, + repo_name: String, + binary_name: &str, + linux_name: &str, + mac_os_name: &str, + windows_name: &str, +) -> Result> { + // Determine the target we should download based on how the CLI itself was built. + let arch_str = get_arch(); + let build_info = cli_build_information(); + let target = match build_info.get(BUILD_OS).context("Failed to determine build info of current CLI")?.as_str() { + "linux-aarch64" | "linux-x86_64" => linux_name, + "macos-aarch64" | "macos-x86_64" => mac_os_name, + "windows-x86_64" => windows_name, + wildcard => bail!("Self-updating is not supported on your OS ({}) right now, please download the binary manually", wildcard), + }; + + let target = format!("{}-{}", arch_str, target); + + let install_dir = match install_dir.clone() { + Some(dir) => dir, + None => { + let dir = get_additional_binaries_dir(); + // Make the directory if it doesn't already exist. + std::fs::create_dir_all(&dir) + .with_context(|| format!("Failed to create directory: {:?}", dir))?; + dir + }, + }; + + let current_version = match &info.current_version { + Some(version) => version, + None => "0.0.0", + }; + + Update::configure() + .bin_install_dir(install_dir) + .bin_name(binary_name) + .repo_owner(&repo_owner) + .repo_name(&repo_name) + .current_version(current_version) + .target_version_tag(&format!("v{}", info.target_version)) + .target(&target) + .build() + .map_err(|e| anyhow!("Failed to build self-update configuration: {:#}", e)) +} + +pub fn get_path(name: &str, exe_env: &str, binary_name: &str, exe: &str) -> Result { + // Look at the environment variable first. + if let Ok(path) = std::env::var(exe_env) { + return Ok(PathBuf::from(path)); + } + + // See if it is present in the path where we usually install additional binaries. + let path = get_additional_binaries_dir().join(binary_name); + if path.exists() && path.is_file() { + return Ok(path); + } + + // See if we can find the binary in the PATH. + if let Some(path) = pathsearch::find_executable_in_path(exe) { + return Ok(path); + } + + Err(anyhow!( + "Cannot locate the {} executable. \ + Environment variable `{}` is not set, and `{}` is not in the PATH. \ + Try running `aptos update {}` to download it and then \ + updating the environment variable `{}` or adding the executable to PATH", + name, + exe_env, + exe, + exe, + exe_env + )) +}