`weakly_canonical` doesn't always return an absolute path

[Pesa]

`weakly_canonical` doesn't always return an absolute path

[Pesa]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113246

[ChrisDenton]

canonical returns "A path that refers to the same file system object as absolute(p)." absolute "Composes an absolute path referencing the same file system location as p according to the operating system ([fs.conform.os]).`

Hm, but posix specifies that that an empty pathname must not be resolved successfully, e,g, realpath returns ENOENT.

[jwakely]

canonical clearly says that !exists(p) is an error. What else is there to say? There is no need to try and interpret what it means to call absolute("") when canonical("") is an error. It's an error, it doesn't return the current path. It's an error.

[Pesa]

It is worth noting that the libstdc++ implementation of std::filesystem (tested with GCC 13.2.0) exhibits the same behavior as boost::filesystem in case (1). So maybe this behavior is intended and the only issue here is the Boost documentation being ambiguous/misleading in this corner case.

I really dislike the inconsistency between (1) and (3) though. Or even worse, between

weakly_canonical("foo/bar")

and

weakly_canonical("./foo/bar")

[Lastique]

From the standard definition of weakly_canonical, it does look like the first case should be made absolute, relative to the current directory:

http://eel.is/c++draft/fs.op.weakly.canonical#1
http://eel.is/c++draft/fs.op.canonical#1
http://eel.is/c++draft/fs.op.absolute

I think, this is a genuine bug in Boost.Filesystem and libstdc++. Please, also report it to gcc developers.

[Pesa]

From the standard definition of weakly_canonical, it does look like the first case should be made absolute, relative to the current directory:

http://eel.is/c++draft/fs.op.weakly.canonical#1 http://eel.is/c++draft/fs.op.canonical#1 http://eel.is/c++draft/fs.op.absolute

To be honest, I'm not quite sure how to interpret that... the standard wording for weakly_canonical says "if any". Does that mean that canonical is not called at all if there are no leading elements that exist? Or that canonical is called with an empty path in that case? That's unclear to me.

Another related observation: std::filesystem::canonical("") throws filesystem_error with GCC/libstdc++.

[Lastique]

To be honest, I'm not quite sure how to interpret that... the standard wording for weakly_canonical says "if any". Does that mean that canonical is not called at all if there are no leading elements that exist? Or that canonical is called with an empty path in that case?

The latter.

Another related observation: std::filesystem::canonical("") throws filesystem_error with GCC/libstdc++.

This is incorrect. canonical definition in the standard says it should return an absolute path with no dot or dot-dot elements that refer to the same file as absolute(""), which means canonical("") should return current_path() (given that the current path always exists).

[Pesa]

This is incorrect. canonical definition in the standard says it should return an absolute path with no dot or dot-dot elements that refer to the same file as absolute(""),

Yes, this is true.

which means canonical("") should return current_path()

Hmm, how did you reach that conclusion? is "" the same as the current path? equivalent(path(""), path(".")) returns false.

[jwakely]

To be honest, I'm not quite sure how to interpret that... the standard wording for weakly_canonical says "if any". Does that mean that canonical is not called at all if there are no leading elements that exist? Or that canonical is called with an empty path in that case?

The latter.

That would be nonsensical, because ...

Another related observation: std::filesystem::canonical("") throws filesystem_error with GCC/libstdc++.

This is incorrect. canonical definition in the standard says it should return an absolute path with no dot or dot-dot elements that refer to the same file as absolute(""), which means canonical("") should return current_path() (given that the current path always exists).

[fs.op.canonical] very clearly says "!exists(p) is an error". Since exists("") is always false, that means canonical("") is an error. I don't know how you conclude that it should return an absolute path for anything.

canonical("") throws, and canonical("", ec) reports an error and returns an empty path. So it would be useless for weakly_canonical to call canonical with an empty path, because it means weakly_canonical would also always fail (either throwing, or returning an empty path as soon as canonical("", ec) fails).

[jwakely]

Or to put it another way, the requirement for canonical(p) to return "an absolute path with no dot or dot-dot elements that refer to the same file as absolute(p)" doesn't hold if it reports an error. If it reports an error, it throws (so doesn't return anything) or it returns an empty path. That seems very clear to me.

[Pesa]

This is incorrect. canonical definition in the standard says it should return an absolute path with no dot or dot-dot elements that refer to the same file as absolute(""), which means canonical("") should return current_path() (given that the current path always exists).

[fs.op.canonical] very clearly says "!exists(p) is an error". Since exists("") is always false, that means canonical("") is an error.

Right, that's a good point. I think I got confused by the fact that Boost's implementation of canonical("") returns the current path instead of raising an error. Even Boost's reference documentation says that !exists(p) is an error, so it looks like the bug here is in canonical() (and the documentation of weakly_canonical() could be clarified).

[Pesa]

For the record, libc++ has the same bug, it returns the current path for canonical("").
EDIT: reported as llvm/llvm-project#77170

Interestingly, it returns an absolute path for weakly_canonical("does-not-exist"), which is the behavior I was hoping for...

[Lastique]

canonical returns "A path that refers to the same file system object as absolute(p)." absolute "Composes an absolute path referencing the same file system location as p according to the operating system ([fs.conform.os]).`

If p is an empty path, it refers to the current directory and therefore always exists. absolute("") returns current_path() on POSIX systems, per http://eel.is/c++draft/fs.op.absolute#7.

Therefore canonical("") should succeed and return current_path(). Consequently, weakly_canonical("") should also return current_path() and weakly_canonical("a/b") where "a" doesn't exist should return current_path() / "a/b".

equivalent(path(""), path(".")) returns false.

Yes, because "" and "." are not equivalent. "" refers to the current directory, and "." refers to the pseudo-file "." within the current directory.

[Lastique]

If p is an empty path, it refers to the current directory and therefore always exists.

The "therefore always exists" part might not be true on POSIX, apparently, as one can delete the directory that is current for another process. This cannot be done on Windows, so there the current directory is guaranteed to exist.

That is besides the point though, as we're not talking of a case when someone deletes the current directory behind out back.

[jwakely]

Or is your point that status("") is not an error?

That doesn't seem like a useful interpretation of the standard, because every time a filesystem operation return path() on error, it's actually returning a valid path referring to the current directory. That's not the intent.

[Pesa]

canonical clearly says that !exists(p) is an error. What else is there to say? There is no need to try and interpret what it means to call absolute("") when canonical("") is an error. It's an error, it doesn't return the current path. It's an error.

FWIW, I agree. The description of canonical in the standard is clear and unambiguous in this case.

Or is your point that status("") is not an error?

That doesn't seem like a useful interpretation of the standard, because every time a filesystem operation return path() on error, it's actually returning a valid path referring to the current directory. That's not the intent.

That would be confusing indeed. Even Boost Filesystem returns false for exists(""), so not treating canonical("") as an error would be inconsistent.

From @Lastique's comments, it seems that Boost considers path("") the same as the current path in some contexts. This may be acceptable for historical reasons in Boost Filesystem v3, but I hope the behavior can be changed to adhere more closely to the C++ standard in Filesystem v4.

[Pesa]

equivalent(path(""), path(".")) returns false.

Yes, because "" and "." are not equivalent. "" refers to the current directory, and "." refers to the pseudo-file "." within the current directory.

Interesting... However, exists("") is false, therefore that invocation of equivalent() should throw per fs.op.equivalent

[Lastique]

canonical clearly says that !exists(p) is an error. What else is there to say?

Ok, I see your point. So according to this, canonical("") should fail, and consequently weakly_canonical("") and weakly_canonical("a/b"), where a doesn't exist, should too (because both end up calling canonical("")). Is this your argument?

I suppose, I could change canonical to work this way, but this would have no practical benefit, other than conformance to the standard wording. In Boost.Filesystem, I have the liberty to deviate from the standard, if it has practical benefits, and this might be such case.

Or is your point that status("") is not an error?

My point is that canonical("") should not be an error. The function definition (the Effects paragraph) does not require p to exist, but rather requires absolute(p) to exist, because that is the path canonical actually processes. Consequently, weakly_canonical("a/b") should also work and produce a path that is at least lexically canonical[1].

I think, the Remark in canonical definition is, perhaps unintentionally, overly restrictive, as it prevents the weakly_canonical("a/b") case from succeeding. I think, the note should rather say that "!exists(absolute(p)) is an error".

That doesn't seem like a useful interpretation of the standard, because every time a filesystem operation return path() on error, it's actually returning a valid path referring to the current directory. That's not the intent.

The returned paths, as well as other values, in case of errors have no meaning - they are simply placeholders because something needs to be returned if no exception is thrown. That's the extent of the path() semantics in this context.

In general, an empty path is just a relative path, and the usual path resolution rules apply when you call absolute on it. I believe, the same should apply to canonical and weakly_canonical as well, because ultimately those functions are path transformation functions, despite the fact they consult the filesystem during the operation.

[1]: The standard doesn't provide a formal definition of a canonical path, but from canonical definition, a canonical path must be (a) absolute, (b) has no dot or dot-dot elements and (c) has no symlink elements. Obviously, weakly_canonical cannot ensure (c) for the part of the path that doesn't exist, but (a) and (b) still apply. If (a) and (b) cannot be satisfied for the returned path, weakly_canonical should fail. That is, the original behavior, where weakly_canonical would produce a relative path without error, is incorrect.

[Lastique]

One side note. If you test !exists(p) in canonical implementation, you are resolving p twice. Once in exists, and another time when you call absolute(p). Generally, relative path resolution should be performed only once, because the current path is volatile and can be changed in another thread. It only makes sense to call absolute(p) once and then check its existence and further process in canonical.

[jwakely]

canonical clearly says that !exists(p) is an error. What else is there to say?

Ok, I see your point. So according to this, canonical("") should fail, and consequently weakly_canonical("") and weakly_canonical("a/b"), where a doesn't exist, should too (because both end up calling canonical("")). Is this your argument?

Yes.

I suppose, I could change canonical to work this way, but this would have no practical benefit, other than conformance to the standard wording. In Boost.Filesystem, I have the liberty to deviate from the standard, if it has practical benefits, and this might be such case.

Fair enough. But you said to report a bug to libstdc++, so I'm here pointing out that the standard is clear.

I think, the Remark in canonical definition is, perhaps unintentionally, overly restrictive, as it prevents the weakly_canonical("a/b") case from succeeding.

No it doesn't. weakly_canonical just doesn't need to call canonical("").

I think, the note should rather say that "!exists(absolute(p)) is an error".

Please submit an LWG issue if you think that it's wrong. Until then, the standard is clear.

That doesn't seem like a useful interpretation of the standard, because every time a filesystem operation return path() on error, it's actually returning a valid path referring to the current directory. That's not the intent.

The returned paths, as well as other values, in case of errors have no meaning - they are simply placeholders because something needs to be returned if no exception is thrown. That's the extent of the path() semantics in this context.

In general, an empty path is just a relative path,

In terms of the generic path format it is a relative path, but one that names nothing. Libstdc++ treats it as an invalid argument to operations that should name a file.

and the usual path resolution rules apply when you call absolute on it.

FWIW, with libstdc++ absolute("") is also an error. The spec says "Composes an absolute path referencing the same file system location as p according to the operating system." On my operating system, ls "" gives an error, and so does opendir(""). The OS does not consider "" to be the current directory.

[1]: The standard doesn't provide a formal definition of a canonical path, but from canonical definition,

(It used to, but the definition was removed because it wasn't actually used anywhere.)

[jwakely]

One side note. If you test !exists(p) in canonical implementation, you are resolving p twice. Once in exists, and another time when you call absolute(p).

absolute(p) doesn't need to resolve it, at least for POSIX. The libstdc++ implementation is basically:

if (p.empty()) {
  ec = std::make_error_code(std::errc::invalid_argument);
  return path();
}
return p.is_absolute() ? p : current_path()/p;

It only makes sense to call absolute(p) once and then check its existence and further process in canonical.

Yes, that's what libstdc++ does.

[Lastique]

I think, the Remark in canonical definition is, perhaps unintentionally, overly restrictive, as it prevents the weakly_canonical("a/b") case from succeeding.

No it doesn't. weakly_canonical just doesn't need to call canonical("").

That's not how I'm reading weakly_canonical definition. It explicitly says to compose its result by means of operator/= applied to the result of canonical call.

absolute(p) doesn't need to resolve it, at least for POSIX. The libstdc++ implementation is basically:

You're calling current_path - that's your relative path resolution in absolute. Another one is part of the exists(p) call in canonical (assuming you're following the standard wording). My point is that you should be calling current_path exactly once, in absolute. Then canonical should operate on the absolute path.

[Lastique]

But you said to report a bug to libstdc++, so I'm here pointing out that the standard is clear.

It is clear, but it doesn't make libstdc++ implementation correct. I asked to report this to libstdc++ because it exhibited the same behavior as Boost.Filesystem, namely that weakly_canonical could return a relative path without error. As I understand, the standard says the call should fail in this case.

[Pesa]

That doesn't seem like a useful interpretation of the standard, because every time a filesystem operation return path() on error, it's actually returning a valid path referring to the current directory. That's not the intent.

The returned paths, as well as other values, in case of errors have no meaning - they are simply placeholders because something needs to be returned if no exception is thrown. That's the extent of the path() semantics in this context.

@Lastique, you're basically saying that when path() is used as a function argument it means "current path", and when it's returned from a function it means "no such path" or some other error occurred. That would make the API very error prone IMHO.

If p is an empty path, it refers to the current directory and therefore always exists

If that's the case, then why is exists("") false in Boost Filesystem?

[Lastique]

@Lastique, you're basically saying that when path() is used as a function argument it means "current path", and when it's returned from a function it means "no such path" or some other error occurred.

Not every function accepts an empty path as input. In this bug, I'm discussing specifically weakly_canonical, as well as canonical and absolute in relation to it. All three of those can be considered as path transformation functions. The former two use absolute to make the input path absolute, and by its means convert empty paths to the current path - because that's how absolute is defined. It doesn't mean other functions will accept empty paths.

Also, an empty path returned from a function is not an error indication. A set error_code is. You should not be using the returned path (or any other value) if the call indicated error by setting the error_code. It doesn't really matter what exact value is returned in case of error because it should never be used.

If p is an empty path, it refers to the current directory and therefore always exists

If that's the case, then why is exists("") false in Boost Filesystem?

This was said in relation to absolute and canonical. That is, absolute("") returns current_path(), and that exists, when tested by canonical.

[Pesa]

Also, an empty path returned from a function is not an error indication. A set error_code is. You should not be using the returned path (or any other value) if the call indicated error by setting the error_code. It doesn't really matter what exact value is returned in case of error because it should never be used.

If the function has an error_code parameter, sure, I fully agree. But some functions like path::parent_path() or path::relative_path() return an empty path to indicate that the requested path does not "make sense", while the current dir is represented by a return value of ".". This isn't a problem per se because the caller can easily distinguish between path() and path("."), but overall this leads to a somewhat less consistent API when compared with functions that treat path() as the current dir.

Btw, thanks for updating the documentation!

[Lastique]

There's no such thing as a "requested path does not make sense". The function either succeeds, meaning that it returns what it is supposed to return, or fails, in which case either an error_code is set or an exception is thrown.

In particular, path::parent_path and path::relative_path may return an empty path, and this is not an indication of error.

[Pesa]

In particular, path::parent_path and path::relative_path may return an empty path, and this is not an indication of error.

My point is that an empty path returned by those functions does not mean "current path", hence the inconsistency.

[Lastique]

An empty path is an empty path. It does not mean "current path". It resolves to the current path by absolute. I've said this a number of times already, I'm not sure what's so confusing about it.