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.

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

move lock: derive toolchain edition and flavor from manifest #16885

Merged
merged 1 commit into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 2 additions & 13 deletions crates/sui-move-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,23 +158,12 @@ impl BuildConfig {
let print_diags_to_stderr = self.print_diags_to_stderr;
let run_bytecode_verifier = self.run_bytecode_verifier;
let resolution_graph = self.resolution_graph(&path)?;
let result = build_from_resolution_graph(
build_from_resolution_graph(
path.clone(),
resolution_graph,
run_bytecode_verifier,
print_diags_to_stderr,
);
if let Ok(ref compiled) = result {
compiled
.package
.compiled_package_info
.build_flags
.update_lock_file_toolchain_version(&path, env!("CARGO_PKG_VERSION").into())
.map_err(|e| SuiError::ModuleBuildFailure {
error: format!("Failed to update Move.lock toolchain version: {e}"),
})?;
}
result
Comment on lines -166 to -177
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also updated in this PR: the toolchain update is lifted into sui-move/src/build.rs and sui/src/client_commands respectively. This ensures that Move.lock is updated for a sui client publish command even when sui move build has never been called before (indeed, this build function doesn't get called on sui client publish).

)
}

pub fn resolution_graph(mut self, path: &Path) -> SuiResult<ResolvedGraph> {
Expand Down
7 changes: 6 additions & 1 deletion crates/sui-move/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl Build {
run_bytecode_verifier: true,
print_diags_to_stderr: true,
}
.build(rerooted_path)?;
.build(rerooted_path.clone())?;
if dump_bytecode_as_base64 {
check_invalid_dependencies(&pkg.dependency_ids.invalid)?;
if !with_unpublished_deps {
Expand Down Expand Up @@ -89,6 +89,11 @@ impl Build {
fs::write(layout_filename, layout_str)?
}

pkg.package
.compiled_package_info
.build_flags
.update_lock_file_toolchain_version(&rerooted_path, env!("CARGO_PKG_VERSION").into())?;

Ok(())
}
}
Expand Down
12 changes: 11 additions & 1 deletion crates/sui/src/client_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1766,7 +1766,7 @@ pub(crate) async fn compile_package(
check_unpublished_dependencies(&dependencies.unpublished)?;
};
let compiled_package = build_from_resolution_graph(
package_path,
package_path.clone(),
resolution_graph,
run_bytecode_verifier,
print_diags_to_stderr,
Expand Down Expand Up @@ -1815,6 +1815,16 @@ pub(crate) async fn compile_package(
} else {
eprintln!("{}", "Skipping dependency verification".bold().yellow());
}

compiled_package
.package
.compiled_package_info
.build_flags
.update_lock_file_toolchain_version(&package_path, env!("CARGO_PKG_VERSION").into())
.map_err(|e| SuiError::ModuleBuildFailure {
error: format!("Failed to update Move.lock toolchain version: {e}"),
})?;

Ok((dependencies, compiled_modules, compiled_package, package_id))
}

Expand Down
36 changes: 27 additions & 9 deletions external-crates/move/crates/move-package/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub mod package_hooks;
pub mod resolution;
pub mod source_package;

use anyhow::Result;
use anyhow::{anyhow, Result};
use clap::*;
use lock_file::LockFile;
use move_compiler::{
Expand All @@ -22,7 +22,11 @@ use move_core_types::account_address::AccountAddress;
use move_model::model::GlobalEnv;
use resolution::{dependency_graph::DependencyGraphBuilder, resolution_graph::ResolvedGraph};
use serde::{Deserialize, Serialize};
use source_package::{layout::SourcePackageLayout, parsed_manifest::DependencyKind};
use source_package::{
layout::SourcePackageLayout,
manifest_parser::{parse_move_manifest_string, parse_source_manifest},
parsed_manifest::DependencyKind,
};
use std::{
collections::BTreeMap,
io::{BufRead, Write},
Expand Down Expand Up @@ -326,20 +330,34 @@ impl BuildConfig {

pub fn update_lock_file_toolchain_version(
&self,
path: &PathBuf,
path: &Path,
compiler_version: String,
) -> Result<()> {
let Some(lock_file) = self.lock_file.as_ref() else {
return Ok(());
};
let path = &SourcePackageLayout::try_find_root(path)
.map_err(|e| anyhow!("Unable to find package root for {}: {e}", path.display()))?;

// Resolve edition and flavor from `Move.toml` or assign defaults.
let manifest_string =
std::fs::read_to_string(path.join(SourcePackageLayout::Manifest.path()))?;
let toml_manifest = parse_move_manifest_string(manifest_string.clone())?;
let root_manifest = parse_source_manifest(toml_manifest)?;
let edition = root_manifest
.package
.edition
.or(self.default_edition)
.unwrap_or_default();
let flavor = root_manifest
.package
.flavor
.or(self.default_flavor)
.unwrap_or_default();

Comment on lines +339 to +357
Copy link
Contributor Author

Choose a reason for hiding this comment

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

main change: look at Move.toml for edition / flavor values.

let install_dir = self.install_dir.as_ref().unwrap_or(path).to_owned();
let mut lock = LockFile::from(install_dir, lock_file)?;
update_compiler_toolchain(
&mut lock,
compiler_version,
self.default_edition.unwrap_or_default(),
self.default_flavor.unwrap_or_default(),
)?;
update_compiler_toolchain(&mut lock, compiler_version, edition, flavor)?;
let _mutx = PackageLock::lock();
lock.commit(lock_file)?;
Ok(())
Expand Down
15 changes: 13 additions & 2 deletions external-crates/move/crates/move-package/tests/test_lock_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,19 @@ flavor = "sui"
#[test]
fn update_lock_file_toolchain_version() {
let pkg = create_test_package().unwrap();
let lock_path = pkg.path().join("Move.lock");
let move_manifest = pkg.path().join("Move.toml");
// The 2024.beta in the manifest should override defaults.
fs::write(
&move_manifest,
r#"
[package]
name = "test"
edition = "2024.beta"
"#,
)
.unwrap();

let lock_path = pkg.path().join("Move.lock");
let lock = LockFile::new(
pkg.path().to_path_buf(),
/* manifest_digest */ "42".to_string(),
Expand All @@ -184,7 +195,7 @@ fn update_lock_file_toolchain_version() {

let expected = expect![[r#"
compiler-version = "0.0.1"
edition = "2024.alpha"
edition = "2024.beta"
flavor = "sui"
"#]];
expected.assert_eq(&toml);
Expand Down
Loading