Normalize path component in AssetPath constructors and From impls
#21135
Conversation
janis-bhm
added
C-Bug
|
Good point. I'm generally scared of making bigger changes than I have to, but personally I think it makes sense to basically normalize any path that potentially interacts with the path-to-id mapping |
janis-bhm
changed the title
ProcessorGatedReader read and read_metapath component in AssetPath constructors and From impls
|
These commits changed the scope of the PR significantly.. |
crates/bevy_asset/src/path.rs
Outdated
| /// Normalizes the path by collapsing all occurrences of '.' and '..' dot-segments where possible | ||
| /// as per [RFC 1808](https://datatracker.ietf.org/doc/html/rfc1808) | ||
| /// Returns a borrowed path if no normalization was necessary, otherwise returns an owned normalized path. | ||
| pub(crate) fn maybe_normalize_path<'a, P: AsRef<Path> + Into<CowArc<'a, Path>> + 'a>( |
There was a problem hiding this comment.
Could we fold this in with normalize_path above? Having two impls of the same thing is not great. Besides, the existing callsite could benefit from your improvements here. I think we can just return Cow<'a, Path>, and then convert that into a CowArc later (or in the case of the existing normalize_path, just don't set the result_path at all.
crates/bevy_asset/src/path.rs
Outdated
| } | ||
| } | ||
|
|
||
| trait NormalizeCowPath { |
There was a problem hiding this comment.
I don't like this magic. We only have a few callsites and this isn't pub anyway, so I'd prefer just making this a function that we call on our path instead of a trait.
since the gated reader tries to look up processed assets by their path, the path should be normalized for this operation
no longer refer to pub(crate) implementation detail
janis-bhm
force-pushed
the
asset/processed-relative-paths
branch
from
d1df8c6 to
7911517
Compare
|
rebase |
… is now only used there.
Objective
Fixes #21131
The
ProcessorGatedReadertries to look up assets by path in the processor'sAssetInfosso it can wait for the processor to be finished processing, and doesn't find an asset for a relative path, and so it errors, even though the asset exists, and is being processed.Solution
Since the gated reader tries to look up processed assets by their path, the path
should be normalized for this operation.
Testing
cd crates/bevy_asset; cargo testbecause of lifetimes, I'm not sure it's possible to read the processed asset with the same normalized path here.
I would expect both the path and the normalized path to resolve to the same file on the file system, ofc because filesystems are the worst enemy of the programmer, this isn't guaranteed to be the case, but I think it's a reasonable expectation here. Notably, this doesn't fix potential issues with symlinks.