Adding AssetPath::resolve() method.

[viridia]

Objective

Fixes #9473

Solution

Added resolve() method to AssetPath. This method accepts a relative asset path string and returns a "full" path that has been resolved relative to the current (self) path.

[github-actions]

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

[viridia]

Reviewers: Please pay special attention to the doc comment. I've tried to explain the behavior as clearly as I can.

The motivation for this change is largely driven by the need for assets that internally contain references to other assets. Allowing a standardized way of representing relative relationships makes assets more portable, since they can be moved around in the asset directly without having to adjust all the paths.

[MrGVSV]

Another nice feature would be to resolve paths relative to the the same directory of the current AssetPath. So rather than specifying ./foo.png or ./foo/bar.txt, users only need to put foo.png or foo/bar.txt (where the ./ is implied).

This would mean that the given relative_path would pretty much always need to be considered relative. In bevy_proto, I made it so that a leading / denoted a path relative to the asset root, but I don't think that has to be the case here since this method is meant for relative paths.

[viridia]

It's not meant for relative paths only - an asset pointing to another asset ought to be able to use both relative and "full" (I'm avoiding the word "absolute") paths interchangeably.

As far as using the leading slash, I have two objections: first, I really want the rules to be the same as the rules for combining URIs, which many developers are likely to be familiar with. Second, a leading slash really makes me think of an absolute filesystem path, which is not what this is.

[viridia]

BTW, if I were going to do as you suggest, I wouldn't call this method resolve, I would call it join or concat or something along those lines.

The design is inspired by the node.js function path.resolve(), although that function is more complicated and is much more of a swiss army knife than this one is (for one thing, it takes an arbitrary number of arguments).

[MrGVSV]

I see. In that case, maybe we can split the join functionality you already have here into a dedicated AssetPath::join method? Or maybe that can happen as a followup PR?

I would have suggested the / trick that I mentioned earlier, but I don't know how well that fits in the context of Bevy's AssetPath proper.

---

Also, I would suggest renaming the relative_path argument to path or something similar. The relative might throw off users who think that the given path will always be relative to the base path, even though that's not the case.

[viridia]

First let me say - you're not wrong.

Relative URIs show up in a lot of places: clicking on hyperlinks, XML XPointers, JSON-Schema files, and others I could name. (Thus the word "Universal" in the name). There isn't a consistent treatment as to how these are resolved, however it generally is one of two choices, which are the two choices under discussion here. For example, the behavior of clicking on a relative link is different than the behavior of new URL(relative_path, base_path) in JavaScript. For a link, a relative path that starts with a name, and not a special character, is resolved relative to the document root. For new URL(), it's resolved relative to the base path.

For Bevy, I think the decision should be whatever is going to be the most useful for the common use cases we anticipate. And also to make sure that we document the behavior clearly.

I'm open to adding a join method in a subsequent PR.

[viridia]

I have renamed the argument as you have requested. I have also re-written the doc comment a bit more to hopefully clarify some potential ambiguities.

[viridia]

Do we need anyone else to weigh in on this?

[alice-i-cecile]

Do we need anyone else to weigh in on this?

In general, Bevy requires two community approvals for any non-trivial PR. In this particular case, I'm waiting for Cart to weigh in (or at least for his #8624 to merge) to avoid horrible merge conflicts.

[viridia]

@MrGVSV @alice-i-cecile I think once this gets committed I want to add an additional feature, which is to allow relative labels as well as relative paths. The syntax would be the same as JPointer (which is used by json-schema): "#./abc/def".

Here's the use case: Say I have a widget, such as a slider, which accepts a reference to a style asset. Now, the slider has internal parts such as a track, thumb, and so on. These all need styles, so each part needs an asset reference. I'm assuming that there's no CSS-like selector matching, so each part needs it's style specified explicitly.

At the same time, however, I want to be able to apply different styles to the slider, so the style asset needs to be a parameter to the slider.

In fact, I want to be able to ship a library of "unstyled" widgets that other people can use - I supply the widget behavior, and other people add a new coat of paint for their game. The behavior and the appearance can be authored by different people.

However, it's too cumbersome if they have to pass in a separate asset path for each slider part.

The answer: relative labels. I design my slider so that all of the parts derive their styles from the base path of the style: "#./slider-thumb", "#./slider-track" and so on. So long as the user who is instantiating the slider has a style asset whose internal labels match the naming scheme, they only have to pass in a single asset path.

[alice-i-cecile]

I like that design :) Agreed on leaving it for a follow-up PR though.

[viridia]

I've gone ahead and rebased the PR against the latest HEAD (including the Asset V2 work).

Also, I wanted to mention, that it is not intended that relative paths be supported automatically by the asset loading system. Rather, these methods are a convenience for people using assets and writing asset loaders, who can decide whether or not to support relative paths, and more importantly, can decide for themselves what they consider to be the "base path".

For example, in the use case I mentioned above, the path resolution happens at the time when a UI template is instantiated, not when it is loaded - because the base path would in that scenario be a parameter that is passed in to the template.

[viridia]

I've rebased the PR against the most recent changes to main.

I'm a bit concerned about the number of calls to .clone() and .to_owned() that I had to add to make this code work. I'm also not 100% convinced that I have the lifetime parameters correct - since it's effectively creating a new AssetPath every time, the lifetime of the result might not need to have any relation to the lifetime of self.

[cart]

Given that we are covering "relative asset paths" here, I think its worth discussing the plans I have for Asset Paths in Bevy Asset V2 / Multiple Asset Sources to make sure this aligns.

We will need to be able to properly support and track "relative asset paths" to enable moving asset folders without reprocessing them. When possible, assets should load other assets using relative paths so we can encode that information during processing.

Relative asset paths are also important for Multiple Asset Sources because a loaded asset should not need to know the name of the asset source it currently exists in. Currently some asset stored in remote://foo/a.gltf referencing the foo/x.png asset path will look in the default (generally the filesystem) asset system because foo/x.png is considered an "absolute path". Not ideal!

Some basic principles I think we'll want to adopt for paths going forward:

1. foo/a.gltf: this is a relative path. It must be resolved to an absolute path based on the current context. The source is undefined so it will use the "default" asset source for the context. If the current context is custom://x/y, then it will be resolved to custom://x/y/foo/a.gltf
2. ./foo/a.gltf: this is also a relative path. It must be resolved to an absolute path based on the current context. The source is undefined so it will use the "default" asset source for the context. If the current context is custom://x/y, then it will be resolved to custom://x/y/foo/a.gltf
3. /foo/a.gltf: this is an absolute path. This source is undefined so it will use the "default" asset source for the context
4. embedded://foo/a.gltf: this is an absolute path. The source is defined so it will use the embedded source.

Note that (1) is currently treated like an absolute path in the current implementation. Making this relative would be a breaking change, but in practice, given that it is generally used in the context of server.load("foo/a.gltf"), this will still behave identically (although we should probably encourage and adopt /foo/a.gltf in these cases for explicitness). I believe strongly that this should be treated as a relative path. Especially in the context of this resolve() implementation, because treating it like an absolute path would defy user expectations (based on their experience with other "file path" / "file system" apis).

From my perspective, "resolve" translates roughly to "for the current AssetPath context / working directory, resolve the final path given a path". Or put another way "path.resolve(input) behaves roughly like path commands in the linux terminal, where the path is the working directory and input is the path command".

With that context out of the way, lets consider the results of the resolve() implementation (pseudocode for brevity).

Here are some samples from the tests in this PR:

• "alice/bob#carol".resolve("joe/next#dave") == "joe/next#dave"
  • These are two relative paths. I would interpret this as: for the relative path alice/bob#carol, navigate to joe/next#dave. "joe/next" is ambiguous as a file vs directory, but next is unambiguously a file here because of the #dave label. This feels "unresolvable" because "alice/bob#carol" is already a file / leaf. I find it resolving to "joe/next#dave" to be confusing. It does make some sense in the context of "joe/next#dev" being treated as an "absolute path" in the current asset implementation, but in the context of resolve() it makes no sense to me / defies my expectations as a user of filesystems. I don't think it is a good idea to encourage people to think about paths in this way, for this API.
• "alice/bob#carol".resolve("./martin#dave") == "alice/martin#dave"
  • ./martin#dave is a relative path, so I interpret this as "navigate to martin#dave within the context of alice/bob#carol. alice/bob#carol is already a leaf (a "relative file path leaf"), so there is no martin#dave to navigate to in this context. I would expect this to be a failure case rather than "swapping" the current file. If you can provide an example of a widely used path api that does this "swapping" behavior instead of treating this as a failure case, that would help convince me.
• "alice/bob#carol".resolve("../martin#dave") == "martin#dave"
  • I interpret this as: for alice/bob#carol, go to the parent of bob#carol and then find martin#dave in it. I would expect this to resolve to alice/martin#dave, not martin#dave. Given that you have used Node's path.resolve as inspiration, here is how an equivalent path resolves there
    

Another test case that isn't covered: "alice/bob#carol".resolve("/joe/next#dave"). I would expect this to resolve to /joe/next#dave because it was an absolute path and that will always resolve to itself.

[viridia]

@cart So I agree with most of your points. As is typical with these kinds of cases, a contributor may have a certain timidity when it comes to making breaking changes, whereas a maintainer may have no such limitation :)

We will need to add a predicate function 'isAbsolute' or 'isFullPath' that returns true if a string begins with '/' or 'scheme:/'.

I think the greatest area of contention has to do with the treatment of leaf segments in the base path. Specifically, say I have a base path such as file:/x/y/z, and I have a relative path, './foo'. There are two choices:

1. Simply concatenate the two paths and strip out the extra './', leaving us with file:/x/y/z/foo. While this might seem the obvious approach, it's actually not what you want most of the time, because 'z' is a file, not a directory. (It might be different if the base path had a trailing slash, but it does not in this case). This also means that in order for a relative path embedded within 'z' to reference file 'foo' in the same directory, you'd have to go up an extra level, e.g. ../foo.

2. Remove the leaf portion of the base path before concatenating, which yields file:/x/y/foo.

Note that this second behavior is consistent with IETF RFC 1808, which defines the behavior of relative URL paths: https://datatracker.ietf.org/doc/html/rfc1808 . Specifically, in section 4, step 6 it says:

The last segment of the base URL's path (anything following the rightmost slash "/", or the entire path if no slash is present) is removed and the embedded URL's path is appended in its place.

(It then goes on to describe how the path is normalized by removing './' and '../' segments.)

[viridia]

Now that I have actually taken the time to read RFC 1808, I think that it should be the canonical reference for this feature, rather than node.resolve() or any other implementation.

[viridia]

I have updated the PR to change the behavior to more closely conform to RFC 1808. This addresses some of the issues raised by @cart . However, the PR doesn't yet handle the "asset source" syntax, mainly because I don't have a formal definition of what characters are valid in that case.

I suspect that AssetPath is going to need to be updated in several ways to support this. For example, the documentation says that an AssetPath consists of two parts: a path and a label. However, that's no longer true, it's now three parts.

At the very least, I need a function that can extract the "scheme" part of the path, given an &str as input.

Also, the latest changes means that the algorithm can no longer panic. According to RFC 1808, if there are insufficient segments in the base path to match the ".." segments in the relative path, then any leftover ".." elements are left as-is.

[cart]

Remove the leaf portion of the base path before concatenating, which yields file:/x/y/foo.
Now that I have actually taken the time to read RFC 1808, I think that it should be the canonical reference for this feature, rather than node.resolve() or any other implementation.

I pretty strongly believe that the swapping behavior is going to confuse people. I've spent a lot of time working with "path code" in a variety of OS-es/languages/frameworks/terminals and this is the first time I've encountered this behavior. I believe others will feel the same way. Additionally, I think people will reach for resolve when they want to concatenate paths and the swap behavior will completely defy that expectation. We could have two separate apis, but I personally believe that would just confuse things more. I'd prefer resolve to be the one api people reach for when they want to work within the context of the current path and the "swap" behavior is too unintuitive to make it recommendable for concatenation scenarios (which I believe will be the most common scenario by a wide margin).

Also, the latest changes means that the algorithm can no longer panic. According to RFC 1808, if there are insufficient segments in the base path to match the ".." segments in the relative path, then any leftover ".." elements are left as-is.

I don't dislike this behavior. The alternative (which might be preferable) is to have two variants: resolve and try_resolve, where try_resolve returns Result<AssetPath, ResolveError>. resolve would just call try_resolve and panic on failure (with the helpful ResolveError message). RFC 1808 was written in a context where error messages (and error handling) aren't an option. Idk if we should conform to that at the cost of user experience.

[cart]

I know we already discussed this on Discord, but for posterity: on the topic of "asset source": that code hasn't been merged yet. It exists in the Multiple Asset Sources PR.

[viridia]

@cart Here's a demonstration. Hover over this link and look at the resulting URL.

The actual link I just typed is ./test.

The resulting URL is https://github.com/bevyengine/bevy/pull/test.

If I were to implement the behavior that you describe, the resulting URL would be https://github.com/bevyengine/bevy/pull/9528/test.

Some other examples:

• test
• ./test
• ../test
• ../../test

[cart]

Im definitely not disputing that this is how the spec works. Only that it is not how most people are trained to think about paths generally in the context of foo, ./foo, and ../foo.

I think if you were to survey Bevy users (or programmers generally) "how do the following paths resolve", you would get the following answers:

• "a/b".resolve("foo"): "a/b/foo"
• "a/b".resolve("./foo"): "a/b/foo"
• "a/b".resolve("../foo"): "a/foo"

[cart]

I personally believe that catering to user expectations is the correct move here. I think people will reach for (and therefore we should build) a "path joining api that handles . and .. as expected". Not a "Relative Uniform Resource Locators spec conforming API that can be used for path joining but has unexpected behaviors for the most common cases"

[cart]

Maybe we should just call it join instead to make this behavior clearer? Especially given that resolve has "always resolves to absolute" semantics in places like Node.js? Given that AssetPath can/will be relative in some cases, the current resolve impl would need to resolve to a relative path, which would defy Node.js user expectations for resolve.

[cart]

Although join does have strong "concatenation" vibes whereas resolve implies we're doing something more than that.

[viridia]

I think that the use case you are thinking of is different from the use case I am thinking of.

The fundamental problem I'm trying to solve is embedded paths within assets. I'm not interested in building a generic asset path resolver, except to the extent that it is necessary to solve the former problem. This means that I'm trying to optimize for user expectations around embedded paths, not user expectations around concatenating paths. These are different things, and, as I will argue, have very little overlap in terms of user experience and expectations.

I believe that for most programmers, their experience with embedded paths will derive from working with web technologies. This includes not just HTML hyperlinks, but JavaScript/TypeScript import statements and URLs embedded within CSS. Take for example, the following TypeScript code:

import { MyComponent } from "./MyComponent";

The path "./MyComponent" is resolved relative to the base path. But what is the base path? Is it the JavaScript file, or is it the directory containing the JavaScript file? MDN doesn't say, so let's put this question aside for a moment.

Similarly, in a CSS file:

.button {
  background-image: url(./images/button.png)
}

While not every web developer will have written a relative hyperlink, you would be hard-pressed not to find a web developer that has not embedded an image in a stylesheet, or imported a JavaScript module.

On the flip side, most users who work with filesystems and desktop APIs won't have nearly as much experience dealing with embedded paths, because there are few desktop file formats that contain embedded paths, and those that do are generally not human readable (e.g. Blender files). Contrast with the web, where the vast majority of content formats are both human readable, and can contain links ("hyperlinks") to other content.

Users of desktop APIs do have experience with concatenating paths, and I suspect much more so than web developers, because on the web most path concatenation is automatic and implicit. In JS, for example, it's very rare to call the browser API new URL(path, base), but it's common to call path.resolve() in Node.js.

So my first point is that I believe that embedded paths within assets should behave like embedded URIs on the web. I think that the "user expectations" argument favors this.

However, that alone does not specify how resolve() should work, because we haven't established what the "base path" for an asset is.

I'm assuming that the "base path" for resolving embedded paths is the same as the path to the asset itself, because that is what is currently available in LoadContext. However, in order to make the behavior of embedded paths consistent with the expectations outlines above, we need to resolve embedded paths relative to the directory the asset is in, not the asset itself. There are multiple ways to accomplish this. One way is by making the resolve() algorithm the same as the algorithm for relative URIs, which is what I've implemented. Another way is to manually compute the parent path before resolving, which is possible but cumbersome. A third way is to include the base path in the LoadContext, separate from the asset path.

OK, I'm going to stop here, because I want to know whether we agree on the behavior of embedded paths before we discuss the question of how resolve should work.

[cart]

Similarly, in a CSS file:

Hmmm ok I see where your head is at. People have definitely been trained to think about paths/uris this way within the context of embedded paths / web resources.

That being said, I think theres a reason the Node.js path API behaves the way it does (without the "swapping" behavior): it is operating within the context of a file system. That is also why it is consistent with "command line apis" and how Rust handles Paths:

fn main() {
    std::fs::create_dir_all("foo/bar/baz");
    std::fs::create_dir_all("foo/baz");

    // prints Ok("/playground/foo/bar/baz")
    println!("{:?}", std::path::Path::new("foo/bar").join("./baz").canonicalize());
    // prints Ok("/playground/foo/baz")
    println!("{:?}", std::path::Path::new("foo/bar").join("../baz").canonicalize());
}

On the web URIs are not a filesystem. Consider https://example.com/foo/bar. bar is assumed to be a webpage / resource that can be resolved. Likewise, if you go up a level, foo is also considered to be a resource that can be resolved, not a folder. Whenever using a "resolve" api you are guaranteed to be within the context of an actual resource (where it will often not make sense to go deeper down the hierarchy).

On the other hand, AssetPaths are a virtual filesystem. When you see the AssetPath foo/bar/baz.json, you know with certainty that foo/bar is a folder hierarchy. bar cannot / will not be some arbitrary resource to resolve. When considering a given AssetPath, you are not guaranteed to be within the context of an actual resource ... you might be in a folder. I think it makes sense to employ the filesystem mindset to such an API.

I think we can reasonably cater to both use cases by making resolve behave "filesystem like" (making it useful and intuitive outside of the context of "embedded asset paths" ... remember that not all AssetPaths are local references in the context of an asset file). Then for "embedded cases" we can just use the parent as the base path:

// unwrap is necessary for AssetPath because not all paths will have a parent
load_context.asset_path().parent().unwrap().resolve("./foo");
// we can add an accessor. Internally asset_path.unwrap() is safe because the asset's path cannot be empty
load_context.parent_asset_path().resolve("./foo")

Note that Multiple Asset Sources makes it possible to express folder-only AssetPaths and adds support for asset_path.parent().

[cart]

I'm down for resolve_embed. I especially like that its used front-and-center in the RFC.

[viridia]

OK I've updated to the latest head, resolved the conflicts, and coded two versions, resolve, and resolve_embed, both of which use a common implementation, resolve_internal.

I also changed the logic a little bit such as that ".." segments are now treated the same in both the base path and the relative path (this is more consistent, the reason for not doing it before is that I was lazy).

I'm a little bit concerned about using PathBuf because I found out that push under some circumstances will do the ".." canonicalization for you, for example if it sees something that looks like a windows drive letter. I don't like the idea that there could be unexpected behavior.

The one thing I have not done yet is added support for detecting asset sources as absolute paths. I'm trying to decide if I should leverage AssetPath::parse_internal or not. If I do, then I have to consider how to handle errors. Right now, resolve can never fail, but if I use parse_internal then I'll have to return a Result if the relative path is an invalid path.

[viridia]

BTW, Path and PathBuf are starting to scare me, they do a lot of stuff under the hood, like ignore trailing slashes, recognize backslashes, windows drive letters and such. I'm beginning to think that at some point (not now) we should consider creating our own implementation of this so that we can ensure that the behavior of asset paths are the same on all platforms - basically convert to Path only after all of the Bevy-specific path manipulations have been done.

[viridia]

Updated the PR. It now understands asset sources - any path string that begins with a source is considered absolute. This uses parse_internal under the hood.

AssetPath::resolve now can fail if the path syntax is invalid. This check may become more struct as the parse function evolves.

In this latest version, the leading slash / is stripped from the path argument - meaning that it is still considered absolute, but the slash is no longer present in the output. This is because all AssetPaths have a source (either a named source or the default) and a source always implies absoluteness. If this were not the case, then we'd have to use three slashes to indicate a truly absolute path source:///path.

Later we can introduce an AssetSourceId::Relative to allow for relative asset paths.

Reviewers: one thing to note is that I'm using a lot of calls to .to_owned(). These were necessary to get the code to compile, but they may not be needed in all cases - this is due to my inexperience with working with Cow and such. There may be better / more efficient ways of handling this that I don't know of.

[cart]

BTW, Path and PathBuf are starting to scare me, they do a lot of stuff under the hood, like ignore trailing slashes, recognize backslashes, windows drive letters and such. I'm beginning to think that at some point (not now) we should consider creating our own implementation of this so that we can ensure that the behavior of asset paths are the same on all platforms - basically convert to Path only after all of the Bevy-specific path manipulations have been done.

I'm very cool with this / feel similarly. Imo this would probably mean:

1. Implement our own Path-like thing (or just use String and str)
2. Use that directly in AssetPath
3. Also use that in the public AssetReader/Writer interfaces and only convert to path inside those implementations when the underlying impl (ex: filesystem apis) require it.

[viridia]

There are a number of refinements we can make...but "better is the enemy of good enough". I think we have convergence on the main points of the API, there's only nits left to pick.

BTW, I did not take your suggestion of adding a "try_resolve" - the reason is that AssetPath has enough methods already, and most of the time they should be handling the error - adding a panic version just saves them a call to unwrap(), something which I would discourage using when dealing with data read from files.

[cart]

I added some doc examples (pictures are worth a thousand words) and resolved my resolve_internal comments.

[viridia]

@cart Release blurb: given that it's not actually used yet, I'm not sure it needs a release blurb. However, if you want one:

"AssetPath now has methods for resolving relative paths. This allow a group of linked assets containing embedded references to be moved around in the filesystem without breaking the links. Asset loaders can voluntarily choose to use these new methods."