Skip to content

Conversation

DaniPopes
Copy link
Member

@DaniPopes DaniPopes commented Sep 3, 2025

No logic changes except: 2nd commit removes walkdir and doesn't sort by file name anymore because order doesn't matter when we run it in parallel

async = ["dep:tokio", "dep:futures-util"]
checksum = ["foundry-compilers-core/hasher"]
walkdir = ["dep:walkdir", "foundry-compilers-core/walkdir"]
walkdir = ["rayon", "foundry-compilers-core/walkdir"]
Copy link
Member Author

Choose a reason for hiding this comment

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

find module and related functions are gated by "walkdir", don't wanna change feature gate

Copy link
Member

Choose a reason for hiding this comment

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

makes sense

if entry.path_is_symlink() {
if let Ok(target) = utils::canonicalize(entry.path()) {
if !visited_symlink_dirs.insert(target.clone()) {
if file_type.is_symlink() {
Copy link
Member Author

Choose a reason for hiding this comment

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

this path is untested. foundry-rs/foundry#7820

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed here 8dfa926

@DaniPopes DaniPopes force-pushed the dani/par-remapping-find_many branch from b6f22db to a1cf3c2 Compare September 3, 2025 15:57
@DaniPopes DaniPopes enabled auto-merge (squash) September 3, 2025 16:01
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

seems correct amigo

very cursed piece of code 👍

async = ["dep:tokio", "dep:futures-util"]
checksum = ["foundry-compilers-core/hasher"]
walkdir = ["dep:walkdir", "foundry-compilers-core/walkdir"]
walkdir = ["rayon", "foundry-compilers-core/walkdir"]
Copy link
Member

Choose a reason for hiding this comment

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

makes sense

// iterate over all dirs that are children of the root
for dir in walkdir::WalkDir::new(dir)
.sort_by_file_name()
.follow_links(true)
Copy link
Member

Choose a reason for hiding this comment

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

we dont need to follow symlinks explicitly here, right?

Copy link
Member Author

@DaniPopes DaniPopes Sep 3, 2025

Choose a reason for hiding this comment

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

why not, lib/x can be a symlink, it should be the same logic as the inner fn

@DaniPopes DaniPopes merged commit 9828708 into main Sep 3, 2025
15 checks passed
@DaniPopes DaniPopes deleted the dani/par-remapping-find_many branch September 3, 2025 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants