Add forc-migrate tool
#6790
Add forc-migrate tool
#6790
Conversation
|
To try out the
For the detailed documentation on migration and the usage of the tool see the scripts/mdbook-forc-documenter/examples/forc_migrate.md in this PR. |
There was a problem hiding this comment.
The tool seems fine, but the migration for #6701 is inverted: users won't need to migrate slots that specify their position with the in keyword, however any slot that doesn't specify their position will have to be set to a fixed, old, position to guarantee backwards compatibility.
The lifecycle of migrations is (primarily) for existing codebases that are deployed behind proxies and need to maintain storage compatibility.
The guide in that linked issue should be fixed to that effect too.
There was a problem hiding this comment.
Overall looks great, nice work, very nicely commented and engineered.
Left just some minor pedantic nits (feel free to ignore!).
I do have a question regarding the overall design.
Given the code to track the manual effort duration for code changes and manual transformation suggestions, I take it (at least for now) the tool will work in a semi-automatic, as opposed to fully automatic?
| crate::cli::Opt { | ||
| [ Migrate the project in the current path => "forc migrate run"] | ||
| [ Migrate the project located in another path => "forc migrate run --path {path}" ] | ||
| [ Migrate the project offline without downloading any dependency => "forc migrate run --offline" ] |
There was a problem hiding this comment.
nit:
| [ Migrate the project offline without downloading any dependency => "forc migrate run --offline" ] | |
| [ Migrate the project offline without downloading any dependencies => "forc migrate run --offline" ] |
| @@ -8,7 +8,7 @@ forc_util::cli_examples! { | |||
| [ Build the docs for a project in the current path and open it in the browser => "forc doc --open" ] | |||
| [ Build the docs for a project located in another path => "forc doc --manifest-path {path}" ] | |||
| [ Build the docs for the current project exporting private types => "forc doc --document-private-items" ] | |||
| [ Build the docs offline without downloading any dependency from the network => "forc doc --offline" ] | |||
| [ Build the docs offline without downloading any dependency => "forc doc --offline" ] | |||
There was a problem hiding this comment.
| [ Build the docs offline without downloading any dependency => "forc doc --offline" ] | |
| [ Build the docs offline without downloading any dependencies => "forc doc --offline" ] |
| For example, let's say that your Sway project is on the version _v0.66.1_, and that the latest v0.66 version is _v0.66.42_. You should first update your Fuel toolchain to the version _v0.66.42_ of `forc`, and compile your project with that version: | ||
|
|
There was a problem hiding this comment.
| For example, let's say that your Sway project is on the version _v0.66.1_, and that the latest v0.66 version is _v0.66.42_. You should first update your Fuel toolchain to the version _v0.66.42_ of `forc`, and compile your project with that version: | |
| For example, let's say that your Sway project is on version _v0.66.1_, and that the latest v0.66 version is _v0.66.42_. You should first update your Fuel toolchain to version _v0.66.42_ of `forc`, and compile your project with that version: | |
| /// Returns a single error string formed of the `error` and `instructions`. | ||
| /// The returned string is formatted to be used as an error message in the [anyhow::bail] macro. |
There was a problem hiding this comment.
| /// Returns a single error string formed of the `error` and `instructions`. | |
| /// The returned string is formatted to be used as an error message in the [anyhow::bail] macro. | |
| /// Returns a single error string formed by the `error` and `instructions`. | |
| /// The returned string is formatted to be used as an error message in the [anyhow::bail] macro. |
|
|
||
| /// Args that can be shared between all commands that `compile` a package. E.g. `check`, `run`. | ||
| #[derive(Debug, Default, Parser)] | ||
| pub(crate) struct Compile { |
There was a problem hiding this comment.
Would it make sense to move these to forc/src/cli/shared.rs instead or do you think nothing else will need these in the future?
@IGI-111 Completely agree. I gave it a deeper thought and have a design that will cover both the rare case which is supported now and also contracts behind proxies. The two will play well together and also with the SRC-14 (no suggestions to check I am moving the PR to draft until that is implemented. |
@tritao It depends on the concrete breaking changes and also on the effort we want to put into writing migrations, compared to eventual manual effort. E.g., in the sample case of changing But in a completely general case, I don't see a fully automatic migration as always possible, even theoretically. Also, the sensitivity of any migration always requires manual supervision. That's why I've opted for the proposed design in which the tool deliberately stops after a single feature is fully migrated and asks for a code review, even if all the migration steps were fully automated. Having manual supervision and semi-automatic migrations also corresponds to my experience so far with code migrations. I see the goal of the tool in taking over the mundane and repetitive effort and pointing where to look, but programmers should still be aware of the changes and understand their impact. |
| fn output_changed_lexed_program( | ||
| manifest_dir: &Path, | ||
| modified_modules: &ModifiedModules, | ||
| lexed_program: &LexedProgram, | ||
| ) -> Result<()> { | ||
| fn output_modules_rec( | ||
| manifest_dir: &Path, | ||
| modified_modules: &ModifiedModules, | ||
| lexed_module: &LexedModule, | ||
| ) -> Result<()> { | ||
| if let Some(path) = modified_modules.get_path_if_modified(&lexed_module.tree) { | ||
| let mut formatter = Formatter::from_dir(manifest_dir)?; | ||
|
|
||
| let annotated_module = Annotated { | ||
| // TODO: Handle annotations instead of stripping them. | ||
| // See: https://github.com/FuelLabs/sway/issues/6802 | ||
| attribute_list: vec![], | ||
| value: lexed_module.tree.clone(), | ||
| }; | ||
|
|
||
| let code = formatter.format_module(&annotated_module)?; | ||
|
|
||
| std::fs::write(path, code)?; | ||
| } | ||
|
|
||
| for (_, lexed_submodule) in lexed_module.submodules.iter() { | ||
| output_modules_rec(manifest_dir, modified_modules, &lexed_submodule.module)?; | ||
| } | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| output_modules_rec(manifest_dir, modified_modules, &lexed_program.root) | ||
| } |
There was a problem hiding this comment.
Is it worth make a temp backup of the files at the beginning so we only write to the original file if there were no errors?
pub(crate) fn output_changed_lexed_program(
manifest_dir: &Path,
modified_modules: &ModifiedModules,
lexed_program: &LexedProgram,
) -> Result<()> {
// Create backups before modifying files
fn output_modules_rec(.....) -> Result<()> {
.....
}
let result = output_modules_rec(manifest_dir, modified_modules, &lexed_program.root);
// Clean up backups on success
result
}
Description
This PR introduces
forc-migrate, a Forc tool for migrating Sway projects to the next breaking change version of Sway.The tool addresses two points crucial for code updates caused by breaking changes:
Besides adding the
forc-migratetool, the PR:Diagnosticto support migration diagnostics aside with errors and warnings.swayfmtto support generating source code from arbitrary lexed trees. The change is a minimal one done only in the parts ofswayfmtthat are executed by migration steps written in this PR. Adaptingswayfmtto fully support arbitrary lexed trees will be done in Refactorswayfmtto generate code from arbitrary lexed trees, not necessarily backed by source code #6779.The migration for the
referencesfeature, migratingref mutto&mut, is added only temporarily, to demonstrate the development and usage of automatic migrations that alter the original source code. This migration will be removed before the next breaking change release, because thereferencesfeature will not be a part of it.The intended usage of the tool is documented in detail in the "forc migrate" chapter of The Sway Book: Forc reference > Plugins > forc_migrate. (The generated documentation has issues that are caused by the documentation generation bug explained in #6792. These issues will be fixed in a separate PR that will fix it for all the plugins.)
We expect the
forc-migrateto evolve based on the developer's feedback. Some of the possible extensions of the tool are:forc-migratealso showed a clear need for better infrastructure for writing static analyzers and transforming Sway code. The approach used in the implementation of this PR should be seen as a pragmatic beginning, based on the reuse of what we currently have. Some future options are discussed in the comments of #6779.Demo
forc migrate showShows the breaking change features and related migration steps. This command can be run anywhere and does not require a Sway project.
forc migrate checkPerforms a dry-run of the migration on a concrete Sway project. It outputs all the occurrences in code that need to be reviewed or changed, as well as the migration time effort:
forc migrate runRuns the migration steps and guides developers through the migration process.
Checklist
Breaking*orNew Featurelabels where relevant.