projectsyn / reclass-rs

Reimplementation of https://github.com/kapicorp/reclass in Rust with Python bindings through PyO3.
BSD 3-Clause "New" or "Revised" License
7 stars 0 forks source link

Evaluate switching to `std::path::absolute()` once that method is stabilized #30

Closed simu closed 3 weeks ago

simu commented 11 months ago

Context

Currently, we're using a custom to_lexical_absolute() function (copied from https://internals.rust-lang.org/t/path-to-lexical-absolute/14940) to transform relative paths to their absolute counterparts without following symlinks, cf. https://github.com/projectsyn/reclass-rs/blob/17807352949c58ef429b53b23110373a45a2d7c2/src/lib.rs#L47-L69

There's discussions around stabilizing std::path::absolute() which is currently available as an unstable feature in nightly Rust. We should check if we can replace our custom to_lexical_absolute() with this method once it's stabilized.

Alternatives

Do nothing and keep maintaining a function that could be replaced by a stdlib method.

simu commented 3 months ago

Looks like std::path::absolute() (which will be stabilized soon) will not resolve /../ segments (apparently in accordance with POSIX path resolution semantics, cf. https://doc.rust-lang.org/std/path/fn.absolute.html. However, this makes the function unsuitable for our usecase.

ssokolow commented 2 months ago

apparently in accordance with POSIX path resolution semantics

% mkdir example
% cd example
% mkdir foo
% touch foo/foo{1..3}
% mkdir bar
% touch bar/bar{1..3}
% mkdir bar/foo
% ls bar/foo/..
bar1  bar2  bar3  foo
% rmdir bar/foo
% ln -s ../foo bar/foo
% ls bar/foo/..
bar  foo

TL;DR: You can't correctly resolve /../ segments without accessing the filesystem because "POSIX path resolution semantics" say its meaning varies depending on the presence or absence of symlinks and that's what the kernel implemented.

simu commented 3 weeks ago

We explicitly don't want to resolve leaf symlinks when determining the name of reclass entities. We already resolve non-leaf symlinks when walking the reclass inventory directory tree, cf. https://github.com/projectsyn/reclass-rs/blob/7b34c220d26b893f5b118133a1ed81ae0f481873/src/lib.rs#L121-L193