Filesystem library

[AntoinePrv]

No description provided.

[Klaim]

Why was util::replace_all not good enough here? Also I would prefer this loop to be a function.

[AntoinePrv]

So I am trying to get to a state of multiple internal "libraries" where there is no cyclic dependencies between them.
The goal is to restrict what code can be used where, so that for instance "filesystem" or "util" does not start calling code form "core". These restrictions would then restrict the opportunities for spaghetti code.

Or maybe I am being too dogmatic?

[Klaim]

It's fine but it's not clear to me how this is related to this code, is it because otherwise there is a cyclic dependency between the filesystem and utils? If so, maybe isolating that specific replace function in it's header would solve the issue?

[AntoinePrv]

@Klaim I wanted to use CMake add_library(mamba-fs OBJECT filesystem.cpp) to literally forbid including code from the rest of mamba in this library (and then do the same with mamba-util, mamba-solv-cpp...).
With more investigation, it turns out it's not so easy to do: since the headers are public, they still need to be added to libmamba to be installed, so I'm not sure it's worth the trouble.

For this particular issue, it turns out that since L"\\" and L"/" are in fact single characters, a simple call to std::replace will do (and is probably more performant). What do you think?

[Klaim]

Ok I see, following your intention would also mean splitting utility in small libraries and making the filesystem library only depend on what it needs in the utility libraries. Note that, it can be done but then these libraries needs to not be distributed separately but only be an implementation detail, linked statically into libmamba. If they are public, then yep we either have to distribute them on the side of libmamba or not make it a library, same conclusion as yours.

For std::replace I would agree in general but I didnt check if it's strictly equivalent to what the utility function was doing? Or said another way: do you see why the function was initially created? What's the difference with std::replace?

[Klaim]

Anyway if it's tested and std::replace works in that specific function, I'm fine by it.

[AntoinePrv]

For std::replace I would agree in general but I didnt check if it's strictly equivalent to what the utility function was doing? Or said another way: do you see why the function was initially created? What's the difference with std::replace?

@Klaim util::replace_all works with string_view. It will look for substrings, and replace with another string, moving all characters:

util::replace_all("ab_ab", "ab", "hello");  # "hello_hello"

[Klaim]

OK then I guess we can use replace

[JohanMabille]

Note: subdirdata has been refactored and does not depend anymore on this file, so we should not include it. This can be removed later, when fetch.hpp is removed from the code base.

[JohanMabille]

Nitpicking: should the inclusion guard be MAMBA_FS_FILESYSTEM? As for previous comment, this can be done in a further PR, to avoid conflicts if we are slow to merge this.