Skip to content
This repository has been archived by the owner on May 4, 2024. It is now read-only.

sui-move: [13/x][move-package/lock] Externally resolved dependencies #860

Merged
merged 4 commits into from
Feb 14, 2023

Conversation

amnn
Copy link
Collaborator

@amnn amnn commented Jan 31, 2023

Adds support for dependencies that are resolved by calling out to third-party binaries. The invocation is expected to return the transitive dependency sub-graph as a lock file, which is deserialized and merged into the main graph.

Externally resolved dependencies can appear in manifests but not lock files, and it is an error for a package to appear in the outputs of multiple resolvers (including the internal resolver). To differentiate this case from the previous case where the internal resolver encounters the same package via another path, packages in the DependencyGraph track their resolver via an optional field (None for internally resolved packages).

Another consequence of this change is that the check that a dependency's name matches its package name moves from the creation of DependencyGraph to the creation of ResolvedGraph, to pick up cases where the name mismatch occurs in an externally resolved dependency.

Clean-ups

  • Also adds support for "progress" tests (snapshot tests for progress output).
  • Stricter warnings during dependency parsing, e.g. warning when a rev field is supplied for a local dependency and other similar cases.
  • Delete a stale (unused) snapshot file.

Test Plan

New tests, including unit tests for the merging operation, and snapshot tests for external resolution lock file and resolved graph output:

move/language/tools/move-package$ cargo nextest

Stack

See also: #789 for main.

Copy link
Member

@tzakian tzakian left a comment

Choose a reason for hiding this comment

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

This looks good to me. A couple nits mainly around naming. The main thing I'd like to discuss/have addressed before landing is the IntDependency rename.

amnn added 4 commits February 14, 2023 16:26
Adds support for dependencies that are resolved by calling out to
third-party binaries.  The invocation is expected to return the
transitive dependency sub-graph as a lock file, which is deserialized
and merged into the main graph.

Externally resolved dependencies can appear in manifests but not lock
files, and it is an error for a package to appear in the outputs of
multiple resolvers (including the internal resolver).  To
differentiate this case from the previous case where the internal
resolver encounters the same package via another path, packages in the
DependencyGraph track their resolver via an optional field (`None` for
internally resolved packages).

Another consequence of this change is that the check that a
dependency's name matches its package name moves from the creation of
DependencyGraph to the creation of ResolvedGraph, to pick up cases
where the name mismatch occurs in an externally resolved dependency.

Clean-ups:

- Also adds support for "progress" tests (snapshot tests for progress
  output).
- Stricter warnings during dependency parsing, e.g. warning when a
  `rev` field is supplied for a local dependency and other similar
  cases.
- Delete a stale (unused) snapshot file.

Test Plan:

New tests, including unit tests for the merging operation, and
snapshot tests for external resolution lock file and resolved graph
output:

```
move/language/tools/move-package$ cargo nextest
```
@amnn amnn merged commit 1506b99 into move-language:sui-move Feb 14, 2023
@amnn amnn deleted the sui-lock-13-ext branch February 14, 2023 18:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants