Multiple rust libraries with submodules #4456
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This modifies the approach we use to linking multiple sorobans. The new approach builds each soroban separately into its own
rlib
using a--locked
cargo build, followed by manually providing them as--extern
definitions to the top-level rust build of libstellar_core.rlib.It is an approach to solving problems of cargo solving/merging/advancing dependency versions when doing soroban multi-versioning (a.k.a. #4278).
The approach here is deeply indebted to @leighmcculloch -- he both had the initial idea and overcame almost every obstacle I encountered along the way. I am just the automake-wrestling keyboard monkey in this PR.
Advantages:
Disadvantages:
Dep-tree checking
There's some existing machinery in stellar-core that bakes-in the Cargo.lock file and then compares the dep-tree of each soroban host in it to fixed (manually maintained) dep-tree files we generate with the
cargo lock tree
cargo-extension.This machinery no longer works with this new scheme:
So instead I've decided to redo this task using a slightly weaker tool:
cargo tree
, which is built-in to cargo (notcargo lock tree
). This loses some precision (cargo tree
only outputs package version numbers, not checksums) but it allows us to specify the features, and exclude the dev-deps, of each submodule. Along the way I've changed it from a dynamic check to a static one: the build just won't succeed if the expected deptrees (checked-in to the stellar-core repo) don't match the actual ones (extracted at build time from the submodules). The resulting errors look like this:I think this is a generally superior developer experience for us, despite the minor loss in precision around dep identities. In practice I think the package version numbers are precise enough.
Dep-tree differences
If you take a look at the dep tree being checked in with this change for the p21 host and compare to the dep tree baked into master's current lockfile for the
[email protected]
package, you will see some slight differences: specifically you'll see that this PR downgradessmallvec 1.13.2 -> 1.10.0
,libm 0.2.8 -> 0.2.7
andwasmparser-nostd 0.100.2 -> 0.100.1
, and the removal ofahash
. These downgrades are actually a revert of changes that happened recently in e967b18 where I generalized support for multiple versions of soroban and simultaneously brought the p22 env into core: at that point I was forced (by cargo's aggressive version unification) to allow those upgrades to the dep-tree of the p21 host, even though I kinda didn't want to. I accepted them at the time as "probably unobservable and worth the bet" but, in fact, the presence of such unwanted upgrades was one of the motivating factors for this PR, that reverts them by downgrading them.Luckily we have not released anything with those unwanted-upgrades yet, so reverting and downgrading them to the exact versions that (a) shipped in p21 and (b) are baked into the lockfile of the env git repo at the point in history when p21 was released is the right thing to do here. But it was good to check!