Skip to content

upstream(move): Port upstream Move package tooling - #12653

Open
Dkwcs wants to merge 2 commits into
vm-lang/upstream/mainnet-1.47.1-1.50.1from
vm-lang/upstream/10834-Move-package-tooling--lcov-coverage-emitter-and-petgraph-bump
Open

upstream(move): Port upstream Move package tooling#12653
Dkwcs wants to merge 2 commits into
vm-lang/upstream/mainnet-1.47.1-1.50.1from
vm-lang/upstream/10834-Move-package-tooling--lcov-coverage-emitter-and-petgraph-bump

Conversation

@Dkwcs

@Dkwcs Dkwcs commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

[run-ci]

Description of change

Ports two upstream Move package tooling changes:

  • LCOV coverage emitter — new move coverage lcov subcommand that turns Move unit test execution traces into lcov.info, with --only-test and --differential-test variants.
  • petgraph 0.5.1 → 0.8.1 in the Move workspace. The bump picks up an upstream fix where removed edges persisted when a node was re-added; the old override pruning relied on that behaviour, so override pruning, graph merging and dependency comparison in move-package are reworked.

Upstream commits

Commit Upstream PR Description
04d8b4a19092103424e527a5ad1421dc4c71199b #21723 [move] Add lcov test coverage emitter for Move code
dcd4f884989419934a431f793f7d38d438d49534 #22220 [move-package] Bump petgraph version. Fix various bugs

Skipped

No commit was skipped. One hunk and one related upstream commit were deliberately left out:

What Reason
move-compiler/src/cfgir/translate.rs hunk from #22220 (DiGraphMap::<ConstantName, ()>::new()) Already present downstream, came in with #11228
Upstream 606fb4a52980b92df65a5042b43339e08df7ff82 (#21876, "Ignore override when comparing dependencies") Marked wont-merge for vm-language. #22220 does not depend on it — it touches only Dependency::eq, format_deps and a doc comment, none of which #22220 references

Downstream adaptations

File Adaptation Reason
move-coverage/src/lcov.rs VMControlFlowGraph imported from move_bytecode_verifier::absint instead of move_abstract_interpreter::control_flow_graph The type is generic downstream since #12469 (upstream #21574). This is upstream's own adaptation from that commit, which could not reach lcov.rs at the time because the file did not exist downstream yet
move-coverage/Cargo.toml + move-bytecode-verifier; new dependencies sorted into the existing # external / # internal groups Required by the import above; grouping follows the file's existing layout
move-cli/src/base/coverage.rs, move-coverage/src/lib.rs, move-cli/Cargo.toml Hunks re-placed Import grouping and alphabetical dependency order differ downstream
move-coverage/src/lcov.rs, move-coverage/src/differential_coverage.rs IOTA copyright year 2026 Current year, matching #12469 and #12431
move-package/src/resolution/dependency_graph.rs Downstream had already fixed the typo in the old prune_overriden_pkgs, so it was named prune_overridden_pkgs — the exact name upstream gives its new function. Both old functions were removed and replaced by the new one Local typo fix collided with upstream's rename
move-package/src/resolution/dependency_graph.rs Local fetch_kind/reroot handling in collect_graphs (#12111) and the local str_escape signature preserved Downstream-only fixes
Cargo.lock, external-crates/move/Cargo.lock Regenerated with cargo instead of applying the patch hunks Generated artifacts

#12652 Links to any relevant issues

Fixes #10834 .

How the change has been tested

  • Basic tests (linting, compilation, formatting, unit/integration tests)
  • Patch-specific tests (correctness, functionality coverage)
  • I have checked that new and existing unit tests pass locally with my changes

Ran locally:

  • cargo check --workspace --all-targets on external-crates/move: clean. The petgraph 0.5 → 0.8 bump needed no API fixes across the ten crates that use it.
  • cargo check -p iota-move in the root workspace: clean.
  • cargo test -p move-package: 200 passed, 0 failed — including all 44 override/diamond cases, the surface the pruning rewrite affects. No pending insta snapshots.
  • cargo test -p move-cli --test tracing_testsuite --features tracing: 1 passed. Generated lcov.info line counts match the expected output exactly (6012 / 5828 / 26).
  • cargo +nightly fmt, cargo ci-license: clean.
  • Both commits verified hunk-by-hunk against the upstream patches. In dependency_graph.rs the code lines match exactly — 152 added / 229 removed on both sides; the only differences anywhere are comment re-wrapping at the downstream rustfmt width and the prune_overriden_pkgsprune_overridden_pkgs spelling noted above.

Two things worth a reviewer's attention:

The tracing_testsuite target is gated behind #[cfg(feature = "tracing")]. CI enables it: scripts/ci_tests/rust_tests.py runs external-crate tests with --features tracing. Note that rust-tests runs in the merge queue, not on the PR, so the lcov assertions execute at merge time rather than on this PR's checks.

  • Three needless_borrow clippy warnings in the ported upstream code (&path.join(...)) were left as upstream wrote them. external-crates/move is a separate workspace and these crates are path dependencies, so cargo ci-clippy does not lint them.

Release Notes

  • CLI: new iota move coverage lcov subcommand emits LCOV data from Move unit test traces; move-package override resolution reworked, which changes the wording of conflicting-dependency errors.

@Dkwcs Dkwcs self-assigned this Aug 12, 2026
@Dkwcs Dkwcs added the vm-language Issues related to the VM & Language Team label Aug 12, 2026
@iota-ci iota-ci added the sc-platform Issues related to the Smart Contract Platform group. label Aug 12, 2026
@Dkwcs
Dkwcs marked this pull request as ready for review August 14, 2026 13:27
@Dkwcs
Dkwcs requested a review from a team as a code owner August 14, 2026 13:27

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

new rule was added to exclude the traces folder, but traces were not removed. Is there a reason why it is now gitignored? 🤔

@Dkwcs Dkwcs Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point - that's inherited behavior from the upstream commit, which added the .gitignore without removing traces/.
The rule doesn't affect the 24 already-tracked trace files - gitignore only applies to untracked ones. Its actual purpose is to suppress what a local run newly produces: the two traces from the new differential_test module (which upstream deliberately did not commit - they're recorded in args.exp as expected Only in traces: … output) plus the generated lcov.info files.

Upstream still tracks traces/ on main today, so we match their state.

Comment on lines +5 to +6
At C
C = { local = "deps_only/C-v1" }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this expected?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes - this comes from the upstream patch and our regenerated output matches it exactly (the test passes on this snapshot).

It's the visible effect of the override-pruning rewrite: one of the two conflicting entries is now attributed to the overridden package itself rather than to the path through the intermediate one.
All 44 override/diamond test cases pass with the rewrite.

Comment thread Cargo.lock

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not sure all these changes are needed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i mean bumping all these packages versions

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nothing hand-edited - the lock is byte-for-byte what cargo generates (checked by re-resolving from the base lock).
Only three real changes: petgraph 0.5.1 → the 0.8.3 entry already in the lock, fixedbitset 0.2.0 dropped (only 0.5.1 used it), lcov + thiserror added.

The rand/itertools lines aren't upgrades - both versions were already present, cargo just re-pointed which one other crates use after petgraph 0.5.1 left the graph.

@Dkwcs Dkwcs changed the title feat(move): Port upstream Move package tooling upstream(move): Port upstream Move package tooling Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sc-platform Issues related to the Smart Contract Platform group. vm-language Issues related to the VM & Language Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants