feat: implement `app/utils` by emlautarom1 · Pull Request #269 · NethermindEth/pluto

emlautarom1 · 2026-03-03T00:30:44Z

Closes #245

The implementation is a lot shorter since the Rust libraries handle several edge cases that are hand-rolled in the reference Go implementation.

Only two dependencies are added to handle tar and gz files:

flate2: https://crates.io/crates/flate2
tar: https://crates.io/crates/tar

- Test `bundle_output`

- Fix an issue where temp files where being dropped earlier

- Revamp error cases

Copilot

Pull request overview

Implements the pluto-app utils module to provide helper functions for short hex formatting, bundling an output directory into a .tar.gz, extracting an archive, and recursively comparing directories (with accompanying unit tests), as requested in #245.

Changes:

Added crates/app/src/utils.rs with hex_7, bundle_output, extract_archive, and compare_directories plus tests.
Exposed the new module via crates/app/src/lib.rs.
Added tar, flate2, tempfile, and test-case dependencies (workspace + crate).

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
crates/app/src/utils.rs	New utilities for tar/gzip bundling/extraction and directory comparison; includes tests.
crates/app/src/lib.rs	Exposes the new `utils` module from the crate root.
crates/app/Cargo.toml	Adds dependencies required by `utils` and its tests.
Cargo.toml	Adds workspace dependency versions for `tar` and `flate2`.
Cargo.lock	Locks new dependency graph for `tar`/`flate2` and transitive updates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

crates/app/src/utils.rs

Copilot · 2026-03-03T00:38:54Z

crates/app/src/utils.rs

+    // Delete all files from the `target_dir`
+    fs::remove_dir_all(&target_path)?;
+    fs::create_dir_all(&target_path)?;
+
+    // Move the created tarball to the target location
+    let output_path = path::Path::new(target_path.as_ref()).join(filename.as_ref());
+    fs::rename(tar_file_path, output_path)?;


bundle_output deletes target_path (removing the original output) before it knows the archive can be moved into place. If rename fails (e.g., due to an open handle or permissions), this will lose the original directory contents and leave only the temp archive behind. Safer flow is: fully finalize/close the archive, move it into target_path, then delete everything except the archive (or otherwise ensure the move cannot fail before deleting originals).

The move operation should not fail since we have permissions to manipulate the target_path, and we assume that we can manipulate any temporary file. Nevertheless, this is indeed an unsafe operation.

The original implementation uses the suggested approach, but it can also end up corrupted due to partially deleted data. I think my proposed implementation is better due to its brevity but I'm not against changing it if it's considered appropriate.

There is also the chance that fs::create_dir_all fails and we potentially lose data. I would suggest saving everything into target_path and then deleting old files manually, and not by remove_dir_all

The challenge here is with the implementation: if we put the .tar.gz file in the target directory then we cannot use tar.append_dir_all which does a lot of heavy lifting. I don't see how operations could fail here though (if we can remove the dir then we can create it again, right?)

crates/app/src/utils.rs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

crates/app/src/utils.rs

iamquang95 · 2026-03-03T07:13:00Z

crates/app/src/utils.rs

+    dir1: impl AsRef<path::Path>,
+    dir2: impl AsRef<path::Path>,
+) -> Result<()> {
+    let mut entries1 = fs::read_dir(dir1)?.collect::<std::result::Result<Vec<_>, _>>()?;


not sure if it's worth to use tokio::fs here

It gets complicated quickly due to recursive async. Given that the extract_archive function is already sync I would leave this one also sync, and if needed we can wrap it in a tokio::spawn_blocking.

varex83 · 2026-03-03T15:57:51Z

crates/app/src/utils.rs

+/// Returns the first 7 (or less) hex chars of the provided bytes.
+pub fn hex_7(input: &[u8]) -> String {
+    let as_string = hex::encode(input);
+    as_string.chars().take(7).collect()
+}


The same function is used in the peerinfo it would be great to port it there.

Agree, but I don't immediately see a way to extract the common code (does not make sense for the example to depend on the entire pluto_app crate for a single function).
Any suggestions?

crates/app/src/utils.rs

varex83 · 2026-03-03T16:23:19Z

crates/app/src/utils.rs

+    // Delete all files from the `target_dir`
+    fs::remove_dir_all(&target_path)?;
+    fs::create_dir_all(&target_path)?;
+
+    // Move the created tarball to the target location
+    let output_path = path::Path::new(target_path.as_ref()).join(filename.as_ref());
+    fs::rename(tar_file_path, output_path)?;


There is also the chance that fs::create_dir_all fails and we potentially lose data. I would suggest saving everything into target_path and then deleting old files manually, and not by remove_dir_all

crates/app/src/utils.rs

Copilot

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

crates/app/src/utils.rs

crates/app/Cargo.toml

crates/app/src/utils.rs

varex83 · 2026-03-05T16:28:17Z

@claude review this PR

Copilot

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

crates/app/src/utils.rs

* feat: add quic upgrade * feat: add comment about peer addresses * fix: linter * fix: clean up tests * fix: linter * fix: review comments * fix: review comments

* feat: add quic upgrade * feat: add comment about peer addresses * fix: linter * fix: clean up tests * fix: linter * feat: add force direct connections * fix: linter * fix: review comments * fix: review comments * fix: review comments

Copilot

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

crates/app/src/utils.rs

Copilot

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

crates/app/src/utils.rs

Copilot · 2026-03-12T14:37:54Z

crates/app/src/utils.rs

+        if path1.is_dir() && path2.is_dir() {
+            compare_directories(&path1, &path2)?;
+        } else if path1.is_file() && path2.is_file() {
+            compare_file_contents(&path1, &path2)?;
+        } else {
+            return Err(UtilsError::TypeMismatch { path1, path2 });
+        }


compare_directories uses PathBuf::is_dir() / is_file(), which follow symlinks. A symlink loop (or a symlink pointing outside the compared trees) can cause infinite recursion or unintended reads outside the directory being compared. Consider using fs::symlink_metadata and explicitly handling symlinks (e.g., treat as a distinct type mismatch, or compare link targets without following).

varex83

LGTM

emlautarom1 added 13 commits March 2, 2026 19:00

Initial implementation

05f62fc

- Test `bundle_output`

Add hex7

b10bff1

Add compare_directories_identical

a8f378b

Add compare_directories_missing_file

1635da1

- Fix an issue where temp files where being dropped earlier

Add compare_directories_different_content

6b65e66

- Revamp error cases

Add compare_directories_different_sizes

549be80

Add compare_directories_missing_directory

30a1a10

Add compare_directories_file_vs_directory

a8eda88

Add compare_directories_directory_vs_file

25625c5

Add compare_directories_empty

460b35d

Add compare_directories_different_file_names

255c189

Add compare_directories_complex_structure

1987fc9

Add descriptive test case names

7ea7bd4

emlautarom1 requested review from Copilot, iamquang95 and varex83 March 3, 2026 00:30

Copilot started reviewing on behalf of emlautarom1 March 3, 2026 00:31 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

Remove trailing comma

3509b70

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings March 3, 2026 00:47

Copilot started reviewing on behalf of emlautarom1 March 3, 2026 00:48 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

crates/app/src/utils.rs Show resolved Hide resolved

crates/app/src/utils.rs Outdated Show resolved Hide resolved

iamquang95 reviewed Mar 3, 2026

View reviewed changes

crates/app/src/utils.rs Show resolved Hide resolved

iamquang95 reviewed Mar 3, 2026

View reviewed changes

crates/app/src/utils.rs Outdated Show resolved Hide resolved

iamquang95 reviewed Mar 3, 2026

View reviewed changes

emlautarom1 added 2 commits March 3, 2026 08:21

Validate directory names

faff6de

Ignore non dirs/files when extracting

807dee7

varex83 reviewed Mar 3, 2026

View reviewed changes

emlautarom1 added 2 commits March 4, 2026 16:01

Use named fields for errors

0bc533b

Use streaming for file comparison

893c244

Fix clippy suggestions

9cb07aa

Copilot AI review requested due to automatic review settings March 5, 2026 15:55

Copilot started reviewing on behalf of emlautarom1 March 5, 2026 15:56 View session

Copilot AI reviewed Mar 5, 2026

View reviewed changes

crates/app/src/utils.rs Outdated Show resolved Hide resolved

crates/app/Cargo.toml Show resolved Hide resolved

crates/app/src/utils.rs Outdated Show resolved Hide resolved

crates/app/src/utils.rs Show resolved Hide resolved

crates/app/src/utils.rs Outdated Show resolved Hide resolved

emlautarom1 added 2 commits March 5, 2026 13:47

Naming convention

4dc4de4

Prevent path traversal due to filename

c33a7ec

emlautarom1 requested review from iamquang95 and varex83 March 5, 2026 21:56

Merge branch 'main' into emlautarom1/app-utils

bf33c0c

Copilot AI review requested due to automatic review settings March 6, 2026 14:34

Copilot started reviewing on behalf of emlautarom1 March 6, 2026 14:34 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

crates/app/src/utils.rs Show resolved Hide resolved

crates/app/src/utils.rs Show resolved Hide resolved

crates/app/src/utils.rs Show resolved Hide resolved

iamquang95 approved these changes Mar 10, 2026

View reviewed changes

emlautarom1 requested a review from Copilot March 10, 2026 12:04

Copilot started reviewing on behalf of emlautarom1 March 10, 2026 12:05 View session

varex83 and others added 3 commits March 10, 2026 09:05

feat(p2p): add upgrade to quic (#259)

d9b564e

* feat: add quic upgrade * feat: add comment about peer addresses * fix: linter * fix: clean up tests * fix: linter * fix: review comments * fix: review comments

feat(p2p): add force direct connections (#260)

1dce1a7

* feat: add quic upgrade * feat: add comment about peer addresses * fix: linter * fix: clean up tests * fix: linter * feat: add force direct connections * fix: linter * fix: review comments * fix: review comments * fix: review comments

fix: update quinn-proto package (#275)

234606b

Copilot AI reviewed Mar 10, 2026

View reviewed changes

crates/app/src/utils.rs Show resolved Hide resolved

emlautarom1 added 3 commits March 10, 2026 10:21

Validate filename

c073a44

Merge remote-tracking branch 'origin/main' into emlautarom1/app-utils

a9d6ed1

Update Cargo.lock

0f81a9d

Copilot AI review requested due to automatic review settings March 12, 2026 14:31

Copilot started reviewing on behalf of emlautarom1 March 12, 2026 14:32 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

varex83 approved these changes Mar 13, 2026

View reviewed changes

Conversation

emlautarom1 commented Mar 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

emlautarom1 Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

varex83 Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

emlautarom1 Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iamquang95 Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

emlautarom1 Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

varex83 Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

emlautarom1 Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

varex83 Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

varex83 commented Mar 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

emlautarom1 Mar 3, 2026 •

edited

Loading