move lock: derive toolchain edition and flavor from manifest #16885
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Ignored Deployments
|
rvantonder
force-pushed
the
rvt/dervie-edition-toml
branch
from
49fb8d1
to
3994c76
Compare
rvantonder
force-pushed
the
rvt/dervie-edition-toml
branch
from
3994c76
to
b99bee1
Compare
rvantonder
force-pushed
the
rvt/dervie-edition-toml
branch
from
b99bee1
to
450b615
Compare
rvantonder
force-pushed
the
rvt/dervie-edition-toml
branch
from
450b615
to
c02f2f5
Compare
| 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(); | ||
|
|
There was a problem hiding this comment.
main change: look at Move.toml for edition / flavor values.
| ); | ||
| 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 |
There was a problem hiding this comment.
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).
rvantonder
changed the title
rvantonder
changed the title
There was a problem hiding this comment.
The change seems reasonable, but got me thinking about the recursive case E.g. package A depends on package B, and:
- A specifies a toolchain in its manifest (different from the default).
- B specifies a toolchain in its manifest (different from the default).
- Both A and B specify different toolchains.
I would expect that in case 1, B should be built with the default toolchain, and in case 2 and 3 it should always be built with the toolchain version that it specifies in its own manifest, regardless of how A is configured.
Earlier we were talking about a model where everything except the builds could recover the information they needed about a package just from its lock file. But IIUC, when we build B as part of A's build, we don't modify B's lock file, so we won't be able to use A's lock file alone to understand what toolchains were used in its build. Is that a problem?
|
Addressed offline--this PR doesn't focus on a recursive lock file model, it is something to revisit later. |
rvantonder
force-pushed
the
rvt/dervie-edition-toml
branch
from
133a9b1
to
fc8e098
Compare
Description
Ensures toolchain versioning records
edition/flavorderived fromMove.tomlwhen available. This was not the case before, because I was under the impression thatBuildConfigreflects the current edition (source and comment) but this is not the case:compiler_configis really the function that merges bothBuildConfigflags (specified on the command line) andMove.tomlwith edition. The result of that computation (PackageConfig) is also discarded after compilation / resolution graph uses. So, for simplicity, and without needing to rely on the resolution graph computed value, this PR updates toolchain versioning to record edition in a similar way tocompiler_config: by parsing the manifest and merging extracted values.Test Plan
Updated test(s).
If your changes are not user-facing and do not break anything, you can skip the following section. Otherwise, please briefly describe what has changed under the Release Notes section.
Type of Change (Check all that apply)
Release notes
Move.lockwill be populated with the edition corresponding to that in theMove.toml, if it exists.Move.lockwill be generated and populated with toolchain versioning information onsui client publish.