Canonicalize CARGO_MANIFEST_DIR

[golddranks]

Objective

• Usually cargo sets CARGO_MANIFEST_DIR, and it sets it as an absolute path.
• However, the user might want to set it manually when running the built binary directly to point Bevy to the correct asset path.
• In that scenario, it makes sense to accept relative paths also.
• Currently FileAssetIo tends to crash with root set as relpath because of the same problem described here Filesystem watching crashes with symlinks pointing outside the asset folder #5689.

Solution

• Canonicalize CARGO_MANIFEST_DIR when setting the root.

[bjorn3]

std::fs::canonicalize can fail on Windows. To be specific at least one ram disk program doesn't support it.

[golddranks]

Thanks for pointing that out! In that case it makes sense to revert to the original path in case of a failure.

[alice-i-cecile]

Will this also fix #5689?

[golddranks]

@alice-i-cecile Unfortunately, it doesn't. However, I'm planning to play around with canonicalize and the co. a bit and see if there is any easy solution.

[golddranks]

Hmm, I think I failed to understand what @bjorn3 meant; according to rust-lang/rust#59117 , canonicalized path names on Windows may cause problems even in cases the canonicalization itself succeeds. This made me to change the implementation once more: it now performs the canonicalization only if the path is relative. (AssetServer doesn't expect relative paths, so it only tries to fix cases that were already broken, so this shouldn't break anything that currently works.)

[golddranks]

Once rust-lang/rust#92750 stabilizes, this should use it instead? (If relative paths are still unsupported by AssetServer at that point.)

[cart]

I don't think overloading CARGO_MANIFEST_DIR with the responsibility of "configuring deployed app asset folder root relative paths" is the right path forward, especially if it is consistently an absolute path within the context of cargo. At the very least, we should have a new environment variable for this scenario.

However, I don't think we should be encouraging "user-configurable" asset roots at all by default. Asset path logic should be determined by the app developer, not the app consumer. The defaults will work in 99.99% of cases and forcing standardization here (by default) is valuable from a tooling and ecosystem perspective.

If you want your game to support non-standard user-configurable asset paths, I think the correct solution is to use AssetServerSettings::asset_folder (and maybe add a configurable custom_root_path: Option<PathBuf> to the settings). You could wire that up to whatever environment variable you choose. And you could use whatever path processing logic you want (ex: canonicalize).

[cart]

Closing this as I've got strong opinions (see my comment above). But feel free keep the conversation going. Happy to listen to counterarguments.