Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Impossible to use UNC filename in TemplatedCustomNamer [removeRedundantDirectorySeparators() breaks use of UNC paths, which contain \\] #225

Open
TeamPlatform1 opened this issue Feb 16, 2024 · 5 comments
Labels
bug namers Code for naming and finding approved and received files

Comments

@TeamPlatform1
Copy link

If the TemplatesCustomNamer is instructed to create a filename starting with "//SomeHost/SomeDir..", the double (back)slash is removed in "removeRedundantDirectorySeparators", which makes it impossible to use UNC paths.

@claremacrae
Copy link
Collaborator

Hi,

If the TemplatesCustomNamer is instructed to create a filename starting with "//SomeHost/SomeDir..", the double (back)slash is removed in "removeRedundantDirectorySeparators", which makes it impossible to use UNC paths.

Please give a source code sample that we can use to test the problem.

And what backslash are you referring to? I only see forward slashes.

@TeamPlatform1
Copy link
Author

TeamPlatform1 commented Feb 19, 2024

A snippet from our main.cpp:

::ApprovalTests::initializeApprovalTestsForGoogleTests();
APPROVAL_TESTS_REGISTER_MAIN_DIRECTORY
auto defaultNamer = ::ApprovalTests::Approvals::useAsDefaultNamer([]()
  {
    auto approvalFilesBaseDir = kernel::utilities::Environment::GetEnvironmentVariable(L"KERNEL_TEST_APPROVAL_FILES_BASE_DIR");
    return std::make_shared<ApprovalTests::TemplatedCustomNamer>(
      kernel::utilities::Convert::WStringToUTF8String(approvalFilesBaseDir) + 
      "/{RelativeTestSourceDirectory}/{TestFileName}.{TestCaseName}.{ApprovedOrReceived}.{FileExtension}"
    );
  });

In the environment variable I tried using something like
"//server/base/path" and
"\\server\base\path" (we're on windows).

In both cases the doubled (back)slash is eliminated in "removeRedundantDirectorySeparators". Looks like using a UNC path is currently impossible.

@claremacrae
Copy link
Collaborator

Thanks.

In case it helps in future, this is how to format code so it's readable here:

```cpp
<your code...>
```

I've made that edit in your post above.

@claremacrae claremacrae changed the title Impossible to use UNC filename in TemplatedCustomNamer Impossible to use UNC filename in TemplatedCustomNamer [removeRedundantDirectorySeparators() breaks use of UNC paths, which contain \\] Feb 19, 2024
@claremacrae claremacrae added namers Code for naming and finding approved and received files bug labels Feb 19, 2024
@claremacrae
Copy link
Collaborator

Thank you for the code.

In both cases the doubled (back)slash is eliminated in removeRedundantDirectorySeparators(). Looks like using a UNC path is currently impossible.

Yes, I'm sure that's the problem.

I see that we have no explicit tests for that function.

The fix would be be to:

  1. add tests for the existing behaviour of removeRedundantDirectorySeparators(),
  2. and then teach it to not treat double-slashes at the start of absolute paths as redundant...

@claremacrae
Copy link
Collaborator

  1. add tests for the existing behaviour of removeRedundantDirectorySeparators(),

Likely something like this:

TEST_CASE("Path retains initial slashes in UNC paths")
{
    const std::string start = R"(\\a\b\c.txt)";
    Path path = Path(start);
    CHECK("//a/b/c.txt" == path.toString("/"));
    CHECK("\\\\a\\b\\c.txt" == path.toString("\\"));
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug namers Code for naming and finding approved and received files
Projects
None yet
Development

No branches or pull requests

2 participants