mini-fs icon indicating copy to clipboard operation
mini-fs copied to clipboard

Too limited

Open flaviojs opened this issue 5 years ago • 16 comments

I tried to use this crate in a project but it was too limited for my case.

I wanted to use another file format so I tried to implement a Store based on your Tar and Zip, but the File constructors are public only inside your crate so I can't implement Store::open.

I also needed my paths to be case-insensitive. I tried to implement a Store that maps uppercase paths to case-sensitive paths, but there is no way to list the file paths inside a Store so I can't implement it.

flaviojs avatar Apr 29 '19 15:04 flaviojs

Yeah there's currently no (safe) way to extend the crate externally unfortunately (Not on this iteration at least, I've only spent two afternoons on it so far).

I wanted to use another file format so I tried to implement a Store based on your Tar and Zip

What file format did you want to implement?

I also needed my paths to be case-insensitive

Windows user :sweat_smile: ? This is an oversight from my part as I don't have a Windows dev machine. I think I could add feature flag like force_case_insensitive that internally forces all Paths to be case insensitive.

I'll try to come up with something.

Thank you so much for your feedback!

germangb avatar Apr 29 '19 16:04 germangb

I was trying out virtual filesystem crates to see if it could be used in ja2-stracciatella.

As part of a PR there, I'm adding code for SLF files to their rust code, it's a type of archive (they call it library) that comes with the game data and there is no rust crate for it.

ja2-stracciatella needs to handle paths as case-insensitive while combining real directories and SLF libraries, even on case-sensitive systems. A virtual filesystem crate could be the answer to that (trying to avoid "reinventing the wheel").

flaviojs avatar Apr 30 '19 14:04 flaviojs

Well, If you want to stick with this library, feel free to add support for anything you need in a PR. I'd be happy to merge it.

For 0.2 I should focus on better generics.

germangb avatar Apr 30 '19 15:04 germangb

Been' thinking about this redefinition of the Store trait, with two combinators (just the declaration, nothing is implemented yet...):

  • https://germangb.github.io/mini-fs/mini_fs/v2/store/trait.Store.html

MiniFs would be generic over the File type (not sure about making it generic over Error as well. Probably not coming from an awful experience matching error types on the futures crate):

  • https://germangb.github.io/mini-fs/mini_fs/v2/struct.MiniFs.html

I would still provide a concrete file implementation for Local, Zip and Tar, but you'd be able to map to, and from it using the Store combinators.

Also entries could be listed without much effort. For that I may need to add an extra method or associated type to the trait.

germangb avatar Apr 30 '19 22:04 germangb

I'm still too green in rust to understand what you are trying to do in v2. I'll give it another try later. :)

flaviojs avatar May 01 '19 10:05 flaviojs

I've added some info on how you may go about implementing a new file format on the 0.2 branch:

  • https://github.com/germangb/mini-fs/blob/0.2/README.md#extensible
  • EDIT: hello world example: https://github.com/germangb/mini-fs/blob/0.2/examples/custom_store.rs

germangb avatar May 02 '19 00:05 germangb

I tried the 0.2 branch (ee088c8a2de563b9f723a8fe3e04efcea207d3f5).

I was able to implement a working Store for my format. :) I also noticed that the struct names of zip.rs/tar.rs were confusing me... It seems like the names use an archive perspective, while I was expecting a (virtual) filesystem perspective.

Regarding case insensitive paths, after getting more familiar with traits I got an idea that seems good. Create a trait CaseInsensitivePaths with a function resolve_case_insensitive_path that receives a case insensitive path and returns either a valid path or an error. (change names at will) Just like Store, to get the function you just need to bring the trait into scope. (after you implement it)

This way I can implement my case-insensitive layer and you don't have to open the "can of worms" that is walking over filesystems. =~

flaviojs avatar May 03 '19 01:05 flaviojs

I see, so this CaseInsensitivePath would basically become an identity when case sensitive paths are enabled and all paths should be compared after his transformation?

germangb avatar May 03 '19 07:05 germangb

Not sure what you mean... let me give you an example of the usage I was thinking:

let local = Local::new(".");
let wanted_path = Path::new("DATA.EXT");
let matching_path = local.resolve_case_insensitive_path(wanted_path)?; // example: "Data.ext"
let file = local.open(matching_path)?;

It doesn't matter if the underlying filesystem is case-sensitive or not, by using resolve_case_insensitive_path we are treating wanted_path like a case-insensitive path.

EDIT: haven't tried, but I can probably use it to make a Store that acts as a case-insensitive layer too

flaviojs avatar May 03 '19 07:05 flaviojs

With caseless paths out of the way (0.2.2) I gave it another go and realized that I need to overlay an amount of stores determined at run time because of mods.

Since the current approach uses tuples, then the overlay stores (types and amount) must be determined at compile-time. Is this interpretation correct?

flaviojs avatar May 31 '19 04:05 flaviojs

That is correct. I wasn't sure if a dynamic overlay was useful or not since I've never found myself in a sitimution on which I needed it. I think tuples makes the API concise in this regard, but I wouldn't oppose to having both a StaticOverlayFS and a DynamicOverlayFS type as part of it in the future

germangb avatar May 31 '19 06:05 germangb

Or you can implement Store for Vec<S: Store>, which is what I'm trying right now. =P

flaviojs avatar Jun 01 '19 01:06 flaviojs

I'd rather not implement it for Vec though (I can only document my own types on rust-doc :disappointed: ).

Implementing it for tuples can be confusing enough already

germangb avatar Jun 01 '19 09:06 germangb

They are a specializations of Store for standard types, so I would expect them to be documented in Store.

flaviojs avatar Jun 01 '19 16:06 flaviojs

Yes but the docs are not as visible as in structs or enums. I'll take a look a this and #8 tomorrow. Thanks for the PR :+1:

germangb avatar Jun 01 '19 16:06 germangb

Heya, I was just checking the issues for this because I was wondering about how the crate handles files changing on the fly.

I'm currently using notify-rs to watch the folder(s) I store my resources in, and when the are modified, I remount that particular filesystem, and alert any dependants about what got updated. Maybe this would be useful to you @flaviojs?

If it also helps, when I create my file managing struct, I use a builder to let the user chain adding stores like:

fn add_store<P>(self, type: StoreType, mount_path: P, path: P) -> Builder
where
    P: Into<PathBuf> + std::convert::AsRef<Path> {
self.stores = self.stores.push(Some((type, mount_path, path)));
self
}

and the when I create them I just iterate over the vector and handle creating whatever type of store it is based on the type, and then I mount it, and move onto the next.

fn build(self) -> MyAbstractedFileHandler {
    for (type, mount_path, path) in self.stores.into_iter() {
        let internal = MiniFs::new();
        if type == StoreType::LOCALFS {
            internal.mount(mount_path, path);
        }
    }
   
    MyAbstractedFileHandler {
        internal
    }
}

This lets me not have to worry about what type of store I'm using or how many, and change it on the go, it's a bit of a workaround, but for me it's mostly because I want to hot reload shaders/textures/models without rebuilding the program.

The benefit at least for me is that in production I can just have everything in an archive and only load in the archive as a single store, but in dev I can have a folder hierarchy or something and load my files from that.

ifacodes avatar Jun 07 '22 19:06 ifacodes