@soc left a comment here and I wrote a quick script to demonstrate the problem:

Joining: a with b
Result : a/b
Joining: a/ with b
Result : a/b
Joining: /a with b
Result : /a/b
Joining: a with /b
Result : /b
Joining: /a with /b
Result : /b

This is indeed bizare and an issue I think path_abs might be able to help with.

The issue with "joining not compiling" is that the API for most path operations takes in AsRef<Path>. I think you would agree that both RelativePath and AbsolutePath should be AsRef<Path>, right? This makes the issue harder as I'm not sure of a way to disallow a certain type in rust (please let me know if there is a way).

(I wish GitHub provided a better way to have discussion other than opening a bug report)

Let's talk symlinks.

I have an application which has historically taken the easy way out of path management by changing the current directory. In moving away from this setup towards a more principled design, I considered using path_abs as a means of leveraging the type system to ensure that I don't accidentally leave behind any lingering relative paths.

Reviewing the documentation, however, it is evident to me that my mental model of symlinks differs from that of the library. Of course, this is to be expected; I understand that PathAbs is opinionated, and that our ideas will inevitably clash somewhere.

But with regard to symlinks specifically, I find the model presented by this crate to be dangerous, and quite scary! I'd like to hear your point of view on this, and I wonder if there is something I am missing.

I'll describe these clashing models by making analogy to Rust's borrow semantics.

To me:

• A directory in the filesystem is like some Box<_> type. It has a unique owner (its "true" parent) and that's the only way you can move or delete it.
• A file in the filesystem is like some Arc<_> type. It may have multiple owners via hard links, and dies when the last link vanishes.
• A symlink is like some *mut _ type. It can change data belonging to its target, but only if you explicitly dereference it (by joining another path component). The pointer itself does not own its target, and its target might not even exist.

In PathAbs:

• by immediately resolving symlinks, PathAbs makes symlinks feel more like C++'s mutable references. A symlink is the directory or file it points to. To me, this is terrifying, because if I'm not careful, then code which was intended to delete a symlink could end up accidentally recursively deleting its target!

What do you think of this? Do you disagree with this risk about deleting a symlink? Is there room in PathAbs for symlinks to live as first class citizens? Am I asking too many questions? :stuck_out_tongue:

This PR adds a bunch of tests for PathArc::absolute(), in various situations: on POSIX, on Windows when the current directory is a regular path (C:\), and on Windows when the current directory is an extended-length syntax path (\\?\C:\).

I made this as a separate PR for a few reasons:

• to get feedback on the somewhat unusual way I've laid out the tests
• so that a future PR for issue #21 can highlight behaviour changes by changing these tests
• to find out whether these tests will pass on AppVeyor the same way they passed in my VM. :)

This PR also adds some "TODO" comments to tests that I've observed to fail in various environments, just for completeness.

Adds a "monotonic_path_preview" module containing my monotonic path handling code, for ease of review, as requested in #17. The module is made public, so the documentation can be viewed with cargo doc.

The Rust API guidelines say:

The Deref traits are used implicitly by the compiler in many circumstances, and interact with method resolution. The relevant rules are designed specifically to accommodate smart pointers, and so the traits should be used only for that purpose.

If I understand correctly, path_abs uses Deref to put its various path types into an inheritance hierarchy, but "Deref-as-inheritance" is often considered an anti-pattern.

When I was working on PR#27, in the code that builds up a path I instinctively reached for .push() since I expected to make a lot of changes. When I wrote something like this:

let mut res = PathAbs::new(path);
res.push(something);

...I got an error on the second line saying something like "cannot borrow immutable reference as mutable" and a warning on the first line saying "variable is marked mut but doesn't need to be", a combination that confused me for some time. Eventually I figured out that the .push() method was visible because of the Deref trait, but couldn't be used because of the lack of DerefMut. Since we can't really implement DerefMut because of Arc, we should probably drop Deref.

Once I'd figured it out, I switched to:

let mut res = PathAbs::new(path);
res = res.join(something);

...but that failed too. In this situation, .join() is inherited from PathArc, so it returns a PathArc instead of a PathAbs, and what I really need is:

let mut res = PathAbs::new(path);
res = PathAbs(res.join(something));

...which I would have found more quickly if Deref hadn't given me a .join() that didn't quite work the way I wanted.

I was pointed to this crate from the comments on a Reddit post I made, and thought I'd take notes about things that confused me while reading the documentation.

Errors always include filenames? That's awesome!

In the list of types introduced in this crate, PathArc is first and therefore presumably most important, but it's not actually used anywhere in the examples? So maybe it's just an implementation detail I should ignore?

In the crate-level docs, PathAbs says "An absolute (not necessarily canonicalized) path that may or may not exist" but the documentation for PathAbs::new() says "The path must exist or io::Error will be returned."? Must the path exist, or not?

I'm interested in working with paths that may or may not exist, but the example at the top of the file only creates these structs with ::create(), creating them on disk at the same time, so it doesn't particularly help me.

I see PathArc has an absolute() method, which does the broken thing of resolving /../ segments semantically, which means it's almost never safe to use. It's a shame; the other parts of its functionality are pretty useful, and there's other useful cleanups it could perform (like removing empty // path segments, and normalizing delimiters to / or \ depending on platform), but while there's a chance that it will corrupt a path I'd rather not use it.

The specific functionality I was looking for in my Reddit post, and which I can't seem to find in your crate, was partial canonicalization. Instead of Rust's canonicalize which requires the the entire path exist, a method that requires all-but-the-last-segment exists, or even a method that silently switches from real canonicalization to semantic canonicalization at the first segment that does not exist.

Actually, wait, this PR is wrong. The file descriptor in the API is supposed to be a preopen directory descriptor, not the descriptor for the target file.

Originally posted by @RReverser in https://github.com/vitiral/path_abs/issues/50#issuecomment-770120481

This makes PathArc::absolute() simpler, and makes the absolute_path_cannot_go_above_root test consistent between Windows and other platforms.

Fixes #21.

I have started to play a bit your crate in my application (RnR) because I had some problems with canonicalize() resolving symlinks. It seems that path_abs solves some of them.

I was wondering if you are planning to support PartialOrd and PartialEq traits in path_abs structs since they are already implemented on Path and PathBuf. I can think in several use cases for that.

Nice job btw!

This helper was added to address a problem with handling Windows paths, but after investigation the problem seems to have gone away. As suggested, the code will be cleaner and simpler without that function.

@zilbuz opened a bug in vitiral/artifact#247. They mentioned that it might be better addressed in path_abs and I agree.

Looking at https://docs.microsoft.com/fr-fr/windows/desktop/FileIO/naming-a-file#maxpath, it doesn't look like windows even allows '/' to be in the name of a file or a directory, so I don't see any harm in converting them. Honestly I thought we already did this.

It feels extremely silly to have to import traits to get methods that could sensibly be provided on the types themselves, especially traits that have so many methods (PathInfo), and especially when I have no intention to use the traits generically. (I already have my own trait with a method named join, and importing yours would cause a conflict)

Most notably:

• {PathAbs,PathDir}::{join,concat}
• {PathAbs,PathFile}::{file_name,with_file_name,with_extension} (and maybe stuff for PathDir; though the term base_name might be cause less confusion than file_name)
• PathAbs::{exists,is_file,is_dir,parent...}... in fact, nearly all of the Path methods are useful for PathAbs, making it really hard for me to see why you removed impl Deref for PathAbs. Maybe Target=PathArc was too much, but isn't at least Target=Path reasonable?

There's two aspects of internationalization relevant to path_abs:

1. It should be possible for path_abs to present its own error messages (and other human-readable text) in the user's language, instead of hard-coding specific English messages.
2. path_abs should not depend on the exact text of error messages from the underlying operating system, since they can vary.

For path_abs' own error messages, the biggest offender is probably the Error::action() method which returns a string. Possibly this could be replaced with an enum, patterned after std::io::ErrorKind. There may be other issues; the code should probably be audited.

For errors from the underlying operating system, currently the only dependency I'm aware of is the test "sanity_errors", which could be amended by checking error fields individually rather than formatting the error and checking the resulting string. Again, there may be other issues and the code should probably be audited.