Skip to content

Commit 5e3fbff

Browse files
authored
[move-ide] Added a fix to better handle mvr dependencies (#22150)
## Description By default, package manager runs all external resolvers on each build. This slows down `move-analyzer` to the point of it becoming unusable. This PR adds support for running external resolvers only on the fist build of Move source code done by `move-analyzer` (a flag added to package manager allows to unconditionally read from the lock file if it exits, without checking its freshness). ## Test plan Verified that build time in VSCode drops from 2-3 seconds to ~100ms withe MVR dependencies present
1 parent 42ad91f commit 5e3fbff

File tree

81 files changed

+120
-10
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

81 files changed

+120
-10
lines changed

external-crates/move/crates/move-analyzer/editors/code/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
"publisher": "mysten",
66
"icon": "images/move.png",
77
"license": "Apache-2.0",
8-
"version": "1.0.21",
8+
"version": "1.0.22",
99
"preview": true,
1010
"repository": {
1111
"url": "https://github.com/MystenLabs/sui.git",

external-crates/move/crates/move-analyzer/src/symbols/compilation.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,12 +162,14 @@ pub fn get_compiled_pkg(
162162
lint: LintLevel,
163163
implicit_deps: Dependencies,
164164
) -> Result<(Option<CompiledPkgInfo>, BTreeMap<PathBuf, Vec<Diagnostic>>)> {
165+
let cached_deps_exist = has_precompiled_deps(pkg_path, packages_info.clone());
165166
let build_config = move_package::BuildConfig {
166167
test_mode: true,
167168
install_dir: Some(tempdir().unwrap().path().to_path_buf()),
168169
default_flavor: Some(Flavor::Sui),
169170
lint_flag: lint.into(),
170-
skip_fetch_latest_git_deps: has_precompiled_deps(pkg_path, packages_info.clone()),
171+
force_lock_file: cached_deps_exist,
172+
skip_fetch_latest_git_deps: cached_deps_exist,
171173
implicit_dependencies: implicit_deps,
172174
..Default::default()
173175
};

external-crates/move/crates/move-package/src/lib.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,11 @@ pub struct BuildConfig {
118118
/// Additional dependencies to be automatically included in every package
119119
#[clap(skip)]
120120
pub implicit_dependencies: Dependencies,
121+
122+
/// Forces use of lock file without checking if it needs to be updated
123+
/// (regenerates it only if it doesn't exist)
124+
#[clap(skip)]
125+
pub force_lock_file: bool,
121126
}
122127

123128
#[derive(
@@ -290,6 +295,7 @@ impl BuildConfig {
290295
writer,
291296
install_dir.clone(),
292297
self.implicit_dependencies.clone(),
298+
self.force_lock_file,
293299
);
294300
let (dependency_graph, modified) = dep_graph_builder.get_graph(
295301
&DependencyKind::default(),

external-crates/move/crates/move-package/src/resolution/dependency_graph.rs

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,9 @@ pub struct DependencyGraphBuilder<Progress: Write> {
202202
/// Set of implicit dependencies to add to every package
203203
/// Invariant: all dependencies are Internal deps with dep_override set
204204
implicit_deps: Dependencies,
205+
/// Forces use of lock file without checking if it needs to be updated
206+
/// (regenerates it only if it doesn't exist)
207+
force_lock_file: bool,
205208
}
206209

207210
impl<Progress: Write> DependencyGraphBuilder<Progress> {
@@ -210,6 +213,7 @@ impl<Progress: Write> DependencyGraphBuilder<Progress> {
210213
progress_output: Progress,
211214
install_dir: PathBuf,
212215
implicit_deps: Dependencies,
216+
force_lock_file: bool,
213217
) -> Self {
214218
for (name, dep) in implicit_deps.iter() {
215219
assert!(
@@ -224,6 +228,7 @@ impl<Progress: Write> DependencyGraphBuilder<Progress> {
224228
visited_dependencies: VecDeque::new(),
225229
install_dir,
226230
implicit_deps,
231+
force_lock_file,
227232
}
228233
}
229234

@@ -258,6 +263,30 @@ impl<Progress: Write> DependencyGraphBuilder<Progress> {
258263
})
259264
.unwrap_or(None);
260265

266+
let root_pkg_id = custom_resolve_pkg_id(&root_manifest).with_context(|| {
267+
format!(
268+
"Resolving package name for '{}'",
269+
root_manifest.package.name
270+
)
271+
})?;
272+
273+
let root_pkg_name = root_manifest.package.name;
274+
if self.force_lock_file {
275+
if let Some((_, _, Some(lock_string))) = digest_and_lock_contents {
276+
eprintln!("Force-reading from lock file");
277+
return Ok((
278+
DependencyGraph::read_from_lock(
279+
root_path,
280+
root_pkg_id,
281+
root_pkg_name,
282+
&mut lock_string.as_bytes(), // safe since old_deps_digest exists
283+
None,
284+
)?,
285+
false,
286+
));
287+
}
288+
}
289+
261290
// implicits deps should be skipped if the manifest contains any of them
262291
// explicitly (or if the manifest is for a system package).
263292
let explicit_implicits: Vec<&Symbol> = self
@@ -289,14 +318,6 @@ impl<Progress: Write> DependencyGraphBuilder<Progress> {
289318
}
290319

291320
// collect sub-graphs for "regular" and "dev" dependencies
292-
let root_pkg_id = custom_resolve_pkg_id(&root_manifest).with_context(|| {
293-
format!(
294-
"Resolving package name for '{}'",
295-
root_manifest.package.name
296-
)
297-
})?;
298-
299-
let root_pkg_name = root_manifest.package.name;
300321
let (mut dep_graphs, resolved_id_deps, mut dep_names, mut overrides) = self
301322
.collect_graphs(
302323
parent,

external-crates/move/crates/move-package/src/resolution/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ pub fn download_dependency_repos<Progress: Write>(
3939
progress_output,
4040
install_dir,
4141
build_options.implicit_dependencies.clone(),
42+
build_options.force_lock_file,
4243
);
4344
let (graph, _) = dep_graph_builder.get_graph(
4445
&DependencyKind::default(),

external-crates/move/crates/move-package/tests/test_additional_addresses.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ fn test_additional_addresses() {
3232
std::io::sink(),
3333
tempdir().unwrap().path().to_path_buf(),
3434
/* implicit_deps */ Dependencies::default(),
35+
/* force_lock_file */ false,
3536
);
3637
let (dg, _) = dep_graph_builder
3738
.get_graph(
@@ -95,6 +96,7 @@ fn test_additional_addresses_already_assigned_same_value() {
9596
std::io::sink(),
9697
tempdir().unwrap().path().to_path_buf(),
9798
/* implicit_deps */ Dependencies::default(),
99+
/* force_lock_file */ false,
98100
);
99101
let (dg, _) = dep_graph_builder
100102
.get_graph(
@@ -144,6 +146,7 @@ fn test_additional_addresses_already_assigned_different_value() {
144146
std::io::sink(),
145147
tempdir().unwrap().path().to_path_buf(),
146148
/* implicit_deps */ Dependencies::default(),
149+
/* force_lock_file */ false,
147150
);
148151
let (dg, _) = dep_graph_builder
149152
.get_graph(

external-crates/move/crates/move-package/tests/test_dependency_graph.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ fn no_dep_graph() {
4444
std::io::sink(),
4545
tempfile::tempdir().unwrap().path().to_path_buf(),
4646
Dependencies::default(), /* implicit deps */
47+
/* force_lock_file */ false,
4748
);
4849
let (graph, _) = dep_graph_builder
4950
.get_graph(
@@ -167,6 +168,7 @@ fn always_deps() {
167168
std::io::sink(),
168169
tempfile::tempdir().unwrap().path().to_path_buf(),
169170
/* implicit_deps */ Dependencies::default(),
171+
/* force_lock_file */ false,
170172
);
171173
let (graph, _) = dep_graph_builder
172174
.get_graph(
@@ -584,6 +586,7 @@ fn immediate_dependencies() {
584586
std::io::sink(),
585587
tempfile::tempdir().unwrap().path().to_path_buf(),
586588
/* implicit_deps */ Dependencies::default(),
589+
/* force_lock_file */ false,
587590
);
588591
let (graph, _) = dep_graph_builder
589592
.get_graph(

external-crates/move/crates/move-package/tests/test_sources/basic_no_deps/Move@compiled.snap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,6 @@ CompiledPackageInfo {
3333
lint: false,
3434
},
3535
implicit_dependencies: {},
36+
force_lock_file: false,
3637
},
3738
}

external-crates/move/crates/move-package/tests/test_sources/basic_no_deps/Move@resolved.snap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ ResolvedGraph {
4242
lint: false,
4343
},
4444
implicit_dependencies: {},
45+
force_lock_file: false,
4546
},
4647
package_table: {
4748
"test": Package {

external-crates/move/crates/move-package/tests/test_sources/basic_no_deps_address_assigned/Move@compiled.snap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,6 @@ CompiledPackageInfo {
3535
lint: false,
3636
},
3737
implicit_dependencies: {},
38+
force_lock_file: false,
3839
},
3940
}

0 commit comments

Comments
 (0)