openSUSE / libpathrs

C-friendly API to make path resolution safer on Linux.
GNU Lesser General Public License v3.0
80 stars 6 forks source link

make libpathrs Rust configuration sane #24

Closed cyphar closed 2 months ago

cyphar commented 4 years ago

The current method of configuration is a really bad idea (exposing structure internals of Root is going to cause problems, I can feel it in my bones). So we should instead use a different design, but it's unclear to me what the best approach is:

Some follow-on questions (and my tentative answers):

let root = RootBuilder::new().source("/some/path").build()?;
let root = RootBuilder::new().source_file_unchecked(root_file).build()?;
// how do we do try_clone?
let root = old_root.try_clone()?.build()?; // ?!
let root = RootBuilder::new().source_root(old_root).build()?; // !?
let root = RootBuilder::new().source_file_unchecked(old_root.try_clone()?.into_file()).build()?; // lolno
cyphar commented 4 years ago

Or maybe you could make the API for try_clone be the same as it currently is, but there is a way you can consume a Root (or Handle) and turn it into a builder?

let root = old_root.try_clone()?.rebuild().set_some_config().build()?; // maybe?

The other problem is what should Root::resolve return? A builder? Does that really make too much sense? Handles don't really have a nice point where you can insert a constructor-like setup...

cyphar commented 2 months ago

I ended up going with:

let root = Root::open(rootdir)?;

// Apply ResolverFlags::NO_SYMLINKS for a single operation.
root.as_ref()
    .with_resolver_flags(ResolverFlags::NO_SYMLINKS)
    .mkdir_all("foo/bar/baz", &perm)?;

// Create a temporary RootRef to do multiple operations.
let root2 = root
    .as_ref()
    .with_resolver_flags(ResolverFlags::NO_SYMLINKS);
root2.create("one", &InodeType::Directory(perm))?;
root2.remove_all("foo")?;