Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CLI] Set Move 2 as default in Aptos CLI and add move-1 flag #15431

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[CLI] Set Move 2 as default in Aptos CLI and add move-1 flag #15431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df1c4a3
add move-1 flag
rahxephon89 Nov 26, 2024
b05cbeb
handle comments
rahxephon89 Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/aptos/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
- Set Compiler v2 as the default compiler and Move 2 as the default language version except for `aptos governance`.
- Add new `--move-1` flag to use Compiler v1 and Move 1.

## [4.6.0] - 2024/11/29
- Add `--node-api-key` flag to `aptos move replay` to allow for querying the fullnode with an API key.
Expand Down
22 changes: 16 additions & 6 deletions crates/aptos/src/common/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ use aptos_types::{
};
use aptos_vm_types::output::VMOutput;
use async_trait::async_trait;
use clap::{Parser, ValueEnum};
use clap::{ArgGroup, Parser, ValueEnum};
use hex::FromHexError;
use move_core_types::{
account_address::AccountAddress, language_storage::TypeTag, vm_status::VMStatus,
Expand Down Expand Up @@ -1094,6 +1094,7 @@ impl FromStr for OptimizationLevel {

/// Options for compiling a move package dir
#[derive(Debug, Clone, Parser)]
#[clap(group = ArgGroup::new("move-version").args(&["move_1", "move_2"]).required(false))]
pub struct MovePackageDir {
/// Path to a move package (the folder with a Move.toml file). Defaults to current directory.
#[clap(long, value_parser)]
Expand Down Expand Up @@ -1163,25 +1164,33 @@ pub struct MovePackageDir {

/// ...or --compiler COMPILER_VERSION
/// Specify the version of the compiler.
/// Defaults to `1`, unless `--move-2` is selected.
/// Defaults to the latest stable compiler version (at least 2)
#[clap(long, value_parser = clap::value_parser!(CompilerVersion),
alias = "compiler",
default_value = LATEST_STABLE_COMPILER_VERSION,
Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to address #14441 and close it with this PR.

default_value_if("move_2", "true", LATEST_STABLE_COMPILER_VERSION),
default_value_if("move_1", "true", "1"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we also (at least manually) test: aptos move test and coverage, with: (a) --move-2, (b) --move-1, (c) neither. (a) and (c) should not panic on `MVC_BLOCK_V1=1, whereas (b) should.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

verbatim_doc_comment)]
pub compiler_version: Option<CompilerVersion>,

/// ...or --language LANGUAGE_VERSION
/// Specify the language version to be supported.
/// Defaults to `1`, unless `--move-2` is selected.
/// Defaults to the latest stable language version (at least 2)
#[clap(long, value_parser = clap::value_parser!(LanguageVersion),
alias = "language",
default_value = LATEST_STABLE_LANGUAGE_VERSION,
default_value_if("move_2", "true", LATEST_STABLE_LANGUAGE_VERSION),
default_value_if("move_1", "true", "1"),
verbatim_doc_comment)]
pub language_version: Option<LanguageVersion>,

/// Select bytecode, language, and compiler versions to support the latest Move 2.
#[clap(long, verbatim_doc_comment)]
pub move_2: bool,

/// Select bytecode, language, and compiler versions for Move 1.
#[clap(long, verbatim_doc_comment)]
pub move_1: bool,
}

impl Default for MovePackageDir {
Expand All @@ -1200,11 +1209,12 @@ impl MovePackageDir {
override_std: None,
skip_fetch_latest_git_deps: true,
bytecode_version: None,
compiler_version: None,
language_version: None,
compiler_version: Some(CompilerVersion::latest_stable()),
language_version: Some(LanguageVersion::latest_stable()),
skip_attribute_checks: false,
check_test_code: false,
move_2: false,
move_2: true,
move_1: false,
optimize: None,
experiments: vec![],
}
Expand Down
1 change: 0 additions & 1 deletion crates/aptos/src/move_tool/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,6 @@ impl IncludedArtifacts {
experiments.append(&mut move_options.experiments.clone());
experiments.append(&mut more_experiments);

// TODO(#14441): Remove `None |` here when we update default CompilerVersion
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment is removed because LATEST_STABLE_COMPILER_VERSION is set as the default value cc @brmataptos @vineethk

if matches!(
move_options.compiler_version,
Option::None | Some(CompilerVersion::V1)
Expand Down
Loading