`create_directories` does not support Windows long paths

[adriendelsalle]

Great project, thank you!

---

It looks like the create_directories function does not support long paths on Windows.

The following adaptation of Windows: Long filename support test is not working (expected to work):

#include "ghc/filesystem.hpp"
namespace fs = ghc::filesystem;

int main()
{
    char c = 'A';
    fs::path dir{"\\\\?\\"};
    dir += fs::current_path().u8string();
    for (; c <= 'Z'; ++c) {
        std::string part = std::string(16, c);
        dir /= part;
        //fs::create_directory(dir);
    }
    fs::create_directories(dir); 

    return 0;
}

Using std::filesystem works.

I managed to make it work adding an extra && current != GHC_PLATFORM_LITERAL("\\\\?\\") here: https://github.com/gulrak/filesystem/blob/4e21ab305794f5309a1454b4ae82ab9a0f5e0d25/include/ghc/filesystem.hpp#L3905

but I'm clearly not sure it's the cleanest way to do it. I'll open a PR and suggest another test for that case too.

Thanks for your support!

[gulrak]

Thanks for the report and PR, I'll look into it when I'm home, later today.

[gulrak]

Okay, I looked into the problem and thank you again for reporting it, as it showed me, that I have to admit the test you extended was broken in the first place by an older change.

As a small introduction to the issue area: Originally ghc::filesystem completely hid the long filename prefixes from the user and generated them when needed. If a caller did prefix a path that one was removed. The generic form was always free from it.

When I switched to the wstring backend support with v1.5.0 I decided to let users use the prefix as they would have to in the std::filesystem implementation. I added them explicitly in the test, to ensure the library can use them. As the original test was iterating until the prefix magically appeared (as in the old implementation) when the path was long enough, it was breaking the loop when \\?\ was found the first time. But now, as it is added and kept at the begin of the test, it just tests `\?\\AAAAAAAAAAAAAAAA´ and that is not really a long filename so the test doesn't test what it should test anymore.

Okay, so first the Test needs to test what he actually should test:

    TemporaryDirectory t(TempOpt::change_path);
    char c = 'A';
    fs::path dir{"\\\\?\\"};
    dir += fs::current_path().u8string();
    for (; c <= 'Z'; ++c) {
        std::string part = std::string(16, c);
        dir /= part;
        CHECK_NOTHROW(fs::create_directory(dir));
        CHECK(fs::exists(dir));
        generateFile(dir / "f0");
        REQUIRE(fs::exists(dir / "f0"));
    }
    CHECK(c > 'Z');
    fs::remove_all(fs::current_path() / std::string(16, 'A'));
    CHECK(!fs::exists(fs::current_path() / std::string(16, 'A')));
    CHECK_NOTHROW(fs::create_directories(dir));
    CHECK(fs::exists(dir));
    generateFile(dir / "f0");
    CHECK(fs::exists(dir / "f0"));

This at least tests that fs::create_directory really works, but as you already knew fs::create_directories is indeed broken in respect to prefixed long filenames. Your solution works for the recommended mode of the library, but (and that is hidden well) there is still the option to disable the auto-prefixing on long path, and debugging the way the proposed fix works, indicates, that it drops the \\?\ prefix when joining it with the drive letter as \\?\ can only be concatenated with the start of an absolut path with operator+ not with operator/. The rules of operator/ in the standard are complex, Windows UNC prefixes are non-existent in there and I didn't want to mangle with that. The operation still succeeds because of the automatic prefixing enabled by default, when the path is getting too long. If it is disabled it would not reappear and still fail.

As the loop generates unnecessary temp path objects by calling root_name() and root_path() for every iteration, I prefer to refactor it a little bit, and let it take advantage of additional private methods and data as fs::canonical uses them even if it leads to fs::create_directories becoming a friend of path. That would result in this implementation:

GHC_INLINE bool create_directories(const path& p, std::error_code& ec) noexcept
{
    path current;
    ec.clear();
    bool didCreate = false;
    auto rootPathLen = p._prefixLength + p.root_name_length() + (p.has_root_directory() ? 1 : 0);
    current = p.native().substr(0, rootPathLen);
    path folders(p._path.substr(rootPathLen));
    for (path::string_type part : folders) {
        current /= part;
        std::error_code tec;
        auto fs = status(current, tec);
        if (tec && fs.type() != file_type::not_found) {
            ec = tec;
            return false;
        }
        if (!exists(fs)) {
            create_directory(current, ec);
            if (ec) {
                std::error_code tmp_ec;
                if (is_directory(current, tmp_ec)) {
                    ec.clear();
                }
                else {
                    return false;
                }
            }
            didCreate = true;
        }
#ifndef LWG_2935_BEHAVIOUR
        else if (!is_directory(fs)) {
            ec = detail::make_error_code(detail::portable_error::exists);
            return false;
        }
#endif
    }
    return didCreate;
}

So it would start the iteration of testing directories for existence with the first non-root-path element and as that, it would need no tests anymore, if the elements could be part of that.

I implemented those changes on the feature-125-windows-create_directories-long-filenames branch.

I really thank you for the effort for the (working!) fix, but the side effects in this case are subtle, and I admit the feature is not tested in a test-case with disabled auto-prefixing, as I think it is almost not used and wanted to keep the CI reaction time in some reasonable limits, so running all test with all combinations of options is just to much.

[adriendelsalle]

Thanks a lot @gulrak , for the quick response and all those details!

I tested your branch and it works like a charm :)

[gulrak]

You are welcome. If the delay is okay I want to do some more testing add some checks to the tests to check prefix compatibility in some other places where I can think of potential issues, that's why merge to master might take until the weekend.

[adriendelsalle]

Yeah sure, as soon as you can will be perfect! thanks again to be so responsive.

[adriendelsalle]

Hi @gulrak . Very sorry to be a bit pushy on that one but do you think it's possible to cut a release soon-ish ? As you can imagine I have an issue on a downstream project :) If you can't, no problem I'll find another way, just tell me!. Thanks!

[gulrak]

No worries, working on the bugfix release right now.

[gulrak]

This is now part of release v1.5.8