Skip to content

Commit 6089334

Browse files
authored
Remove protobuf system deps from transitive dependencies and fix worktree creation logging (#180)
* Protobuf files are system dependencies, and are not managed by protofetch: we should not try to copy them over to the output directory, because they're not found in the protofetch cache, and therefore we will always get a "no such file or directory" error. * I levelled the worktree existence log down to debug, because when we have transitive dependencies and pruning, standard output will be cluttered.
1 parent 0137552 commit 6089334

File tree

2 files changed

+11
-18
lines changed

2 files changed

+11
-18
lines changed

src/git/repository.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ impl ProtoGitRepository<'_> {
218218
wanted_path: worktree_path.to_str().unwrap_or("").to_string(),
219219
});
220220
} else {
221-
log::info!(
221+
log::debug!(
222222
"Found existing worktree for {} at {}.",
223223
name,
224224
canonical_wanted_path.to_string_lossy()

src/proto.rs

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ fn copy_all_proto_files_for_dep(
107107
Ok(proto_mapping.into_iter().collect())
108108
}
109109

110-
/// Returns an HashSet of ProtoFileMapping to the proto files that `dep` depends on. It recursively
110+
/// Returns a HashSet of ProtoFileMapping to the proto files that `dep` depends on. It recursively
111111
/// iterates all the dependencies of `dep` and its transitive dependencies based on imports
112112
/// until no new dependencies are found.
113113
fn pruned_transitive_dependencies(
@@ -249,7 +249,8 @@ fn copy_proto_sources_for_dep(
249249
Ok(())
250250
}
251251

252-
/// Extracts the dependencies from a proto file
252+
/// Extracts the dependencies from a proto file, skipping google/protobuf imports,
253+
/// since these are provided by default by protoc.
253254
fn extract_proto_dependencies_from_file(file: &Path) -> Result<Vec<PathBuf>, ProtoError> {
254255
let mut dependencies = Vec::new();
255256
let mut reader = BufReader::new(File::open(file)?);
@@ -258,7 +259,9 @@ fn extract_proto_dependencies_from_file(file: &Path) -> Result<Vec<PathBuf>, Pro
258259
if line.starts_with("import ") {
259260
if let Some(dependency) = line.split_whitespace().nth(1) {
260261
let dependency = dependency.to_string().replace([';', '\"'], "");
261-
dependencies.push(PathBuf::from(dependency));
262+
if !dependency.starts_with("google/protobuf/") {
263+
dependencies.push(PathBuf::from(dependency));
264+
}
262265
}
263266
}
264267
line.clear();
@@ -299,9 +302,9 @@ fn collect_transitive_dependencies(
299302
.collect::<Vec<_>>()
300303
}
301304

302-
/// Collects all root dependencies based on pruning rules and transitive dependencies
303-
/// This still has a limitation. At the moment.
304-
/// If a dependency is flagged as transitive it will only be included in transitive fetching which uses pruning.
305+
/// Collects all root dependencies based on pruning rules and transitive dependencies.
306+
/// This still has a limitation at the moment:
307+
/// if a dependency is flagged as transitive it will only be included in transitive fetching which uses pruning.
305308
fn collect_all_root_dependencies(resolved: &ResolvedModule) -> Vec<ResolvedDependency> {
306309
let mut deps = Vec::new();
307310

@@ -537,8 +540,6 @@ mod tests {
537540
PathBuf::from("proto/example3.proto"),
538541
PathBuf::from("proto/example5.proto"),
539542
PathBuf::from("scalapb/scalapb.proto"),
540-
PathBuf::from("google/protobuf/descriptor.proto"),
541-
PathBuf::from("google/protobuf/struct.proto"),
542543
]
543544
.into_iter()
544545
.collect();
@@ -562,16 +563,8 @@ mod tests {
562563
.unwrap()
563564
.join(Path::new("resources/proto_out/example2.proto"));
564565
let dependencies = extract_proto_dependencies_from_file(&path).unwrap();
565-
assert_eq!(dependencies.len(), 3);
566+
assert_eq!(dependencies.len(), 1);
566567
assert_eq!(dependencies[0].to_string_lossy(), "scalapb/scalapb.proto");
567-
assert_eq!(
568-
dependencies[1].to_string_lossy(),
569-
"google/protobuf/descriptor.proto"
570-
);
571-
assert_eq!(
572-
dependencies[2].to_string_lossy(),
573-
"google/protobuf/struct.proto"
574-
);
575568
}
576569

577570
#[test]

0 commit comments

Comments
 (0)