anza-xyz / move

Move compiler targeting llvm supported backends
https://discord.gg/wFgfjG9J
Apache License 2.0
108 stars 34 forks source link

Rebase onto Sui #396

Open brson opened 11 months ago

brson commented 11 months ago

We think the Sui storage model will be easier to implement on Solana than the mainline Move storage model. Let's attempt to rebase onto their tree.

@jcivlin Do you want to try this or should I?

jcivlin commented 11 months ago

I'm deep in dwarf riddle, plz go ahead. Let me know if you need help.

We think the Sui storage model will be easier to implement on Solana than the mainline Move storage model. Let's attempt to rebase onto their tree.

@jcivlin Do you want to try this or should I?

@brson I'm brain breaking with the dwarf riddle, plz go ahead. Let me know if you need help.

brson commented 11 months ago

The state of sui move is discouraging.

Their Move codebase is part of the sui tree, and the Move subdirs only slightly resemble the upstream Move code organization:

https://github.com/MystenLabs/sui/tree/main/external-crates/move

The sui-move branch of the Move repo hasn't been updated for 9 months, and I'm guessing won't be updated again:

https://github.com/move-language/move/tree/sui-move

Mysten doesn't even have a fork of the move repo under their org, so it seems like they are just maintaining their fork in the sui tree.


One way to tackle this would be to rebase onto the sui-move branch of the the move repo, to get mostly up to date with sui, then manually copy all our changes into the appropriate place in the sui tree.

brson commented 11 months ago

Well, good news is that rebasing onto the sui-move branch of the move repo is not too hard. Here is a first try:

https://github.com/brson/move/tree/solana-sui-base

The tests run, but not all tests pass.

brson commented 11 months ago

The IR test failures appear to mostly be because the compiler is emitting IR function definitions in a different order than on the main branch.

brson commented 11 months ago

The sui compiler had a curious change that caused move-build to seemingly not work with bytecode dependencies. Applying this patch made those tests pass again:

+++ b/language/move-compiler/src/command_line/compiler.rs
@@ -598,6 +598,8 @@ fn generate_interface_files_for_deps(
     let interface_files_paths =
         generate_interface_files(deps, interface_files_dir_opt, module_to_named_address, true)?;
     deps.extend(interface_files_paths);
+    // Remove bytecode files
+    deps.retain(|p| !p.path.as_str().ends_with(MOVE_COMPILED_EXTENSION));
     Ok(())
 }
jcivlin commented 11 months ago

The sui compiler had a curious change that caused move-build to seemingly not work with bytecode dependencies. Applying this patch made those tests pass again:

+++ b/language/move-compiler/src/command_line/compiler.rs
@@ -598,6 +598,8 @@ fn generate_interface_files_for_deps(
     let interface_files_paths =
         generate_interface_files(deps, interface_files_dir_opt, module_to_named_address, true)?;
     deps.extend(interface_files_paths);
+    // Remove bytecode files
+    deps.retain(|p| !p.path.as_str().ends_with(MOVE_COMPILED_EXTENSION));
     Ok(())
 }

Can it be bc we are processing modules in a reverse order. I found this to be problematic for DI building and restoring the order returned by move compiler, see #397. Screenshot 2023-12-11 at 10 12 16 PM

brson commented 11 months ago

Can it be bc we are processing modules in a reverse order. I found this to be problematic for DI building and restoring the order returned by move compiler, see #397.

Doesn't seem to be related. I applied the patch here but didn't see any change in how move-build responded to bytecode dependencies.

brson commented 11 months ago

Ok, this branch that is rebased onto the move/sui-move branch appears to pass all the tests it passed previously:

https://github.com/brson/move/tree/solana-sui-base

I have filed several new issues revealed during the rebase:

Next step is to attempt to transplant all our changes on to the sui repo's copy of move, which has a completely different layout than the move repo.

jcivlin commented 11 months ago

Can it be bc we are processing modules in a reverse order. I found this to be problematic for DI building and restoring the order returned by move compiler, see #397.

Doesn't seem to be related. I applied the patch here but didn't see any change in how move-build responded to bytecode dependencies.

The patch controls the order of modules in the same file, so if you observe it in compilation of multiple files it may not help.

brson commented 8 months ago

Update:

My previous attempt to rebase attempt failed, where I was trying to rebase onto the latest (but old) sui branch of move, then apply diffs to the sui codebase. The sui codebase has changed so much that the diffs were just unappliable.

I started a new attempt that just moves our files into the sui codebase one at a times, and it has been more successful.

My branch is here: https://github.com/brson/sui/tree/solana

Most of the tests pass now. The major outstanding problem is that sui removed from move-cli all support for alternative architectures, so none of our patches can be applied directly. I have started re-introducing multi-architecture support to move-cli, and once that works almost all tests should pass again.