Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore broken but excluded file during traversing #11008

Merged
merged 3 commits into from
Aug 21, 2022

Conversation

weihanglo
Copy link
Member

What does this PR try to resolve?

Fixes #10917

Walkdir's filter_entry() won't call the predicate if the entry
is essentially an Err from its underyling IntoIter. That means
Cargo hasn't had a chance to call filter on an entry that should be
excluded but eventually return an Err and cause the loop to stop.
For instance, a broken symlink which should bee excluded by filter
will generate an error since filter closure is not called with it.

The solution is calling filter if an error occurs with a path
(because it has yet been called with that path).
If it's exactly excluded, ignore the error.

How should we test and review this PR?

Follow the reproducible steps in #10917

Additional information

Walkdir's [`filter_entry()`][1] won't call the predicate if the entry
is essentially an `Err` from its underyling `IntoIter`. That means
Cargo hasn't had a chance to call `filter` on an entry that should be
excluded but eventually return an `Err` and cause the loop to stop.
For instance, a broken symlink which should bee excluded by `filter`
will generate an error since `filter` closure is not called with it.

The solution is calling `filter` if an error occurs with a path
(because it has yet been called with that path).
If it's exactly excluded, ignore the error.

[1]: https://github.com/BurntSushi/walkdir/blob/abf3a15887758e0af54ebca827c7b6f8b311cb45/src/lib.rs#L1042-L1058
@rust-highfive
Copy link

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 20, 2022
@epage
Copy link
Contributor

epage commented Aug 21, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Aug 21, 2022

📌 Commit c0110c6 has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 21, 2022
@bors
Copy link
Contributor

bors commented Aug 21, 2022

⌛ Testing commit c0110c6 with merge 1ac83ba...

@bors
Copy link
Contributor

bors commented Aug 21, 2022

☀️ Test successful - checks-actions
Approved by: epage
Pushing 1ac83ba to master...

@bors bors merged commit 1ac83ba into rust-lang:master Aug 21, 2022
@casey
Copy link
Contributor

casey commented Aug 21, 2022

Nice!

@weihanglo weihanglo deleted the issue-10917 branch August 21, 2022 07:59
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 24, 2022
Update cargo

7 commits in 9809f8ff33c2b998919fd0432c626f0f7323697a..6da726708a4406f31f996d813790818dce837161
2022-08-16 22:10:06 +0000 to 2022-08-23 21:39:56 +0000
- Update non-ASCII crate name warning message (rust-lang/cargo#11017)
- Add more tests for aggressive or precise update (rust-lang/cargo#11011)
- Ignore broken but excluded file during traversing (rust-lang/cargo#11008)
- Improve error message for wrong target names (rust-lang/cargo#10999)
- Bump snapbox to 0.3 (rust-lang/cargo#11005)
- remove missed reference to workspace inheritance in unstable.md (rust-lang/cargo#11001)
- Warning when precise or aggressive without -p flag (rust-lang/cargo#10988)
@ehuss ehuss added this to the 1.65.0 milestone Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken symlinks prevent packaging when using package.include
6 participants