Skip to content
This repository has been archived by the owner on May 4, 2024. It is now read-only.

Commit

Permalink
[package] Allow bytecode version to be set from input
Browse files Browse the repository at this point in the history
Right now, the bytecode version can only be set by environment
variable, which is a bit messy.  This ensures that the input can
be set from other ways than just the environment variable.
  • Loading branch information
gregnazario committed Feb 22, 2023
1 parent 94c681e commit 727a1e2
Show file tree
Hide file tree
Showing 12 changed files with 73 additions and 29 deletions.
2 changes: 1 addition & 1 deletion language/move-analyzer/src/symbols.rs
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ impl Symbolicator {
let build_plan = BuildPlan::create(resolution_graph)?;
let mut typed_ast = None;
let mut diagnostics = None;
build_plan.compile_with_driver(&mut std::io::sink(), |compiler| {
build_plan.compile_with_driver(&mut std::io::sink(), None, |compiler| {
let (files, compilation_result) = compiler.run::<PASS_TYPING>()?;
let (_, compiler) = match compilation_result {
Ok(v) => v,
Expand Down
14 changes: 10 additions & 4 deletions language/move-command-line-common/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,16 @@ const BYTECODE_VERSION_ENV_VAR: &str = "MOVE_BYTECODE_VERSION";

/// Get the bytecode version from the environment variable.
// TODO: This should be configurable via toml and command line flags. See also #129.
pub fn get_bytecode_version_from_env() -> Option<u32> {
std::env::var(BYTECODE_VERSION_ENV_VAR)
.ok()
.and_then(|s| s.parse::<u32>().ok())
pub fn get_bytecode_version_from_env(from_input: Option<u32>) -> Option<u32> {
// This allows for bytecode version to come from command line flags and
// other input locations, falls back to bytecode version if not provided
if from_input.is_some() {
from_input
} else {
std::env::var(BYTECODE_VERSION_ENV_VAR)
.ok()
.and_then(|s| s.parse::<u32>().ok())
}
}

pub fn read_env_var(v: &str) -> String {
Expand Down
2 changes: 1 addition & 1 deletion language/tools/move-cli/src/base/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ pub fn run_move_unit_tests<W: Write + Send>(
// Move package system, to first grab the compilation env, construct the test plan from it, and
// then save it, before resuming the rest of the compilation and returning the results and
// control back to the Move package system.
build_plan.compile_with_driver(writer, |compiler| {
build_plan.compile_with_driver(writer, None, |compiler| {
let (files, comments_and_compiler_res) = compiler.run::<PASS_CFGIR>().unwrap();
let (_, compiler) =
diagnostics::unwrap_or_report_diagnostics(&files, comments_and_compiler_res);
Expand Down
2 changes: 1 addition & 1 deletion language/tools/move-cli/src/experimental/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ impl ExperimentalCommand {
concretize,
} => {
let state = PackageContext::new(&move_args.package_path, &move_args.build_config)?
.prepare_state(storage_dir)?;
.prepare_state(None, storage_dir)?;
experimental::commands::analyze_read_write_set(
&state,
module_file,
Expand Down
13 changes: 8 additions & 5 deletions language/tools/move-cli/src/sandbox/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ impl SandboxCommand {
move_args: &Move,
storage_dir: &Path,
) -> Result<()> {
let bytecode_version = None;
match self {
SandboxCommand::Publish {
no_republish,
Expand All @@ -213,12 +214,13 @@ impl SandboxCommand {
} => {
let context =
PackageContext::new(&move_args.package_path, &move_args.build_config)?;
let state = context.prepare_state(storage_dir)?;
let state = context.prepare_state(bytecode_version, storage_dir)?;
sandbox::commands::publish(
natives,
cost_table,
&state,
context.package(),
bytecode_version,
*no_republish,
*ignore_breaking_changes,
*with_deps,
Expand All @@ -238,7 +240,7 @@ impl SandboxCommand {
} => {
let context =
PackageContext::new(&move_args.package_path, &move_args.build_config)?;
let state = context.prepare_state(storage_dir)?;
let state = context.prepare_state(bytecode_version, storage_dir)?;
sandbox::commands::run(
natives,
cost_table,
Expand All @@ -251,6 +253,7 @@ impl SandboxCommand {
args,
type_args.to_vec(),
*gas_budget,
bytecode_version,
*dry_run,
move_args.verbose,
)
Expand All @@ -269,7 +272,7 @@ impl SandboxCommand {
),
SandboxCommand::View { file } => {
let state = PackageContext::new(&move_args.package_path, &move_args.build_config)?
.prepare_state(storage_dir)?;
.prepare_state(bytecode_version, storage_dir)?;
sandbox::commands::view(&state, file)
}
SandboxCommand::Clean {} => {
Expand All @@ -295,12 +298,12 @@ impl SandboxCommand {
}
SandboxCommand::Doctor {} => {
let state = PackageContext::new(&move_args.package_path, &move_args.build_config)?
.prepare_state(storage_dir)?;
.prepare_state(bytecode_version, storage_dir)?;
sandbox::commands::doctor(&state)
}
SandboxCommand::Generate { cmd } => {
let state = PackageContext::new(&move_args.package_path, &move_args.build_config)?
.prepare_state(storage_dir)?;
.prepare_state(bytecode_version, storage_dir)?;
handle_generate_commands(cmd, &state)
}
}
Expand Down
3 changes: 2 additions & 1 deletion language/tools/move-cli/src/sandbox/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub fn publish(
cost_table: &CostTable,
state: &OnDiskStateView,
package: &CompiledPackage,
bytecode_version: Option<u32>,
no_republish: bool,
ignore_breaking_changes: bool,
with_deps: bool,
Expand Down Expand Up @@ -81,7 +82,7 @@ pub fn publish(
}
}

let bytecode_version = get_bytecode_version_from_env();
let bytecode_version = get_bytecode_version_from_env(bytecode_version);

// use the the publish_module API from the VM if we do not allow breaking changes
if !ignore_breaking_changes {
Expand Down
4 changes: 3 additions & 1 deletion language/tools/move-cli/src/sandbox/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use move_vm_runtime::move_vm::MoveVM;
use move_vm_test_utils::gas_schedule::CostTable;
use std::{fs, path::Path};

#[allow(clippy::too_many_arguments)]
pub fn run(
natives: impl IntoIterator<Item = NativeFunctionRecord>,
cost_table: &CostTable,
Expand All @@ -37,13 +38,14 @@ pub fn run(
txn_args: &[TransactionArgument],
vm_type_args: Vec<TypeTag>,
gas_budget: Option<u64>,
bytecode_version: Option<u32>,
dry_run: bool,
verbose: bool,
) -> Result<()> {
if !script_path.exists() {
bail!("Script file {:?} does not exist", script_path)
};
let bytecode_version = get_bytecode_version_from_env();
let bytecode_version = get_bytecode_version_from_env(bytecode_version);

let bytecode = if is_bytecode_file(script_path) {
assert!(
Expand Down
8 changes: 6 additions & 2 deletions language/tools/move-cli/src/sandbox/utils/package_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,12 @@ impl PackageContext {
/// NOTE: this is the only way to get a state view in Move CLI, and thus, this function needs
/// to be run before every command that needs a state view, i.e., `publish`, `run`,
/// `view`, and `doctor`.
pub fn prepare_state(&self, storage_dir: &Path) -> Result<OnDiskStateView> {
let bytecode_version = get_bytecode_version_from_env();
pub fn prepare_state(
&self,
bytecode_version: Option<u32>,
storage_dir: &Path,
) -> Result<OnDiskStateView> {
let bytecode_version = get_bytecode_version_from_env(bytecode_version);
let state = OnDiskStateView::create(self.build_dir.as_path(), storage_dir)?;

// preload the storage with library modules (if such modules do not exist yet)
Expand Down
20 changes: 16 additions & 4 deletions language/tools/move-package/src/compilation/build_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,23 @@ impl BuildPlan {
}

/// Compilation results in the process exit upon warning/failure
pub fn compile<W: Write>(&self, writer: &mut W) -> Result<CompiledPackage> {
self.compile_with_driver(writer, |compiler| compiler.build_and_report())
pub fn compile<W: Write>(
&self,
bytecode_version: Option<u32>,
writer: &mut W,
) -> Result<CompiledPackage> {
self.compile_with_driver(writer, bytecode_version, |compiler| {
compiler.build_and_report()
})
}

/// Compilation process does not exit even if warnings/failures are encountered
pub fn compile_no_exit<W: Write>(&self, writer: &mut W) -> Result<CompiledPackage> {
self.compile_with_driver(writer, |compiler| {
pub fn compile_no_exit<W: Write>(
&self,
bytecode_version: Option<u32>,
writer: &mut W,
) -> Result<CompiledPackage> {
self.compile_with_driver(writer, bytecode_version, |compiler| {
let (files, units_res) = compiler.build()?;
match units_res {
Ok((units, warning_diags)) => {
Expand All @@ -131,6 +141,7 @@ impl BuildPlan {
pub fn compile_with_driver<W: Write>(
&self,
writer: &mut W,
bytecode_version: Option<u32>,
mut compiler_driver: impl FnMut(
Compiler,
)
Expand Down Expand Up @@ -169,6 +180,7 @@ impl BuildPlan {
&project_root,
root_package.clone(),
transitive_dependencies,
bytecode_version,
&self.resolution_graph,
&mut compiler_driver,
)?;
Expand Down
22 changes: 16 additions & 6 deletions language/tools/move-package/src/compilation/compiled_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ impl OnDiskCompiledPackage {
&self,
package_name: Symbol,
compiled_unit: &CompiledUnitWithSource,
bytecode_version: Option<u32>,
) -> Result<()> {
let root_package = self.package.compiled_package_info.package_name;
assert!(self.root_path.ends_with(root_package.as_str()));
Expand All @@ -348,7 +349,7 @@ impl OnDiskCompiledPackage {
.with_extension(MOVE_COMPILED_EXTENSION),
compiled_unit
.unit
.serialize(get_bytecode_version_from_env())
.serialize(get_bytecode_version_from_env(bytecode_version))
.as_slice(),
)?;
self.save_under(
Expand Down Expand Up @@ -536,6 +537,7 @@ impl CompiledPackage {
/* source paths */ Vec<Symbol>,
/* address mapping */ &ResolvedTable,
)>,
bytecode_version: Option<u32>,
resolution_graph: &ResolvedGraph,
mut compiler_driver: impl FnMut(
Compiler,
Expand Down Expand Up @@ -599,6 +601,7 @@ impl CompiledPackage {
deps_compiled_units.push((package_name, unit))
}
}
let bytecode_version = get_bytecode_version_from_env(bytecode_version);

let mut compiled_docs = None;
let mut compiled_abis = None;
Expand All @@ -623,7 +626,7 @@ impl CompiledPackage {

if resolution_graph.build_options.generate_abis {
compiled_abis = Some(Self::build_abis(
get_bytecode_version_from_env(),
bytecode_version,
&model,
&root_compiled_units,
));
Expand All @@ -643,7 +646,10 @@ impl CompiledPackage {
compiled_abis,
};

compiled_package.save_to_disk(project_root.join(CompiledPackageLayout::Root.path()))?;
compiled_package.save_to_disk(
project_root.join(CompiledPackageLayout::Root.path()),
bytecode_version,
)?;

Ok(compiled_package)
}
Expand Down Expand Up @@ -703,7 +709,11 @@ impl CompiledPackage {
Ok(())
}

pub(crate) fn save_to_disk(&self, under_path: PathBuf) -> Result<OnDiskCompiledPackage> {
pub(crate) fn save_to_disk(
&self,
under_path: PathBuf,
bytecode_version: Option<u32>,
) -> Result<OnDiskCompiledPackage> {
self.check_filepaths_ok()?;
assert!(under_path.ends_with(CompiledPackageLayout::Root.path()));
let root_package = self.compiled_package_info.package_name;
Expand All @@ -730,10 +740,10 @@ impl CompiledPackage {
std::fs::create_dir_all(&on_disk_package.root_path)?;

for compiled_unit in &self.root_compiled_units {
on_disk_package.save_compiled_unit(root_package, compiled_unit)?;
on_disk_package.save_compiled_unit(root_package, compiled_unit, bytecode_version)?;
}
for (dep_name, compiled_unit) in &self.deps_compiled_units {
on_disk_package.save_compiled_unit(*dep_name, compiled_unit)?;
on_disk_package.save_compiled_unit(*dep_name, compiled_unit, bytecode_version)?;
}

if let Some(docs) = &self.compiled_docs {
Expand Down
10 changes: 8 additions & 2 deletions language/tools/move-package/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ pub struct BuildConfig {
/// Skip fetching latest git dependencies
#[clap(long = "skip-fetch-latest-git-deps", global = true)]
pub skip_fetch_latest_git_deps: bool,

/// Bytecode version to compile move code
#[clap(long = "bytecode-version", global = true)]
pub bytecode_version: Option<u32>,
}

#[derive(Debug, Clone, Eq, PartialEq, PartialOrd)]
Expand All @@ -154,9 +158,10 @@ impl BuildConfig {
/// Compile the package at `path` or the containing Move package. Exit process on warning or
/// failure.
pub fn compile_package<W: Write>(self, path: &Path, writer: &mut W) -> Result<CompiledPackage> {
let bytecode_version = self.bytecode_version;
let resolved_graph = self.resolution_graph_for_package(path, writer)?;
let mutx = PackageLock::lock();
let ret = BuildPlan::create(resolved_graph)?.compile(writer);
let ret = BuildPlan::create(resolved_graph)?.compile(bytecode_version, writer);
mutx.unlock();
ret
}
Expand All @@ -168,9 +173,10 @@ impl BuildConfig {
path: &Path,
writer: &mut W,
) -> Result<CompiledPackage> {
let bytecode_version = self.bytecode_version;
let resolved_graph = self.resolution_graph_for_package(path, writer)?;
let mutx = PackageLock::lock();
let ret = BuildPlan::create(resolved_graph)?.compile_no_exit(writer);
let ret = BuildPlan::create(resolved_graph)?.compile_no_exit(bytecode_version, writer);
mutx.unlock();
ret
}
Expand Down
2 changes: 1 addition & 1 deletion language/tools/move-package/tests/test_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ impl Test<'_> {
"notlocked" => "Lock file uncommitted\n".to_string(),

"compiled" => {
let mut pkg = BuildPlan::create(resolved_package?)?.compile(&mut sink)?;
let mut pkg = BuildPlan::create(resolved_package?)?.compile(None, &mut sink)?;
scrub_compiled_package(&mut pkg.compiled_package_info);
format!("{:#?}\n", pkg.compiled_package_info)
}
Expand Down

0 comments on commit 727a1e2

Please sign in to comment.