file_util: fix broken symlink on Windows

[06393993]

... due to using incorrect separators.

Fix #8185.

Checklist

If applicable:

• I have updated CHANGELOG.md
• I have updated the documentation (README.md, docs/, demos/)
• I have updated the config schema (cli/src/config-schema.json)
• I have added/updated tests to cover my changes

[PhilipMetzger]

LG, since its a Windows only fix

[yuja]

Directory separators in symlink content will have to be transformed back and forth by local working copy. See #6977 (comment) for the previous discussion.

[06393993]

Directory separators in symlink content will have to be transformed back and forth by local working copy. See #6977 (comment) for the previous discussion.

Then I have a question. What's the plan for handling absolute paths?

[yuja]

Directory separators in symlink content will have to be transformed back and forth by local working copy. See #6977 (comment) for the previous discussion.

Then I have a question. What's the plan for handling absolute paths?

What's different between relative and absolute paths? Absolute paths (and symlinks pointing to out of the tree) wouldn't work on different machines, but I don't think it's wrong to translate /foo/bar to \foo\bar (or c:\foo to c:/foo.)

[06393993]

What's different between relative and absolute paths?

On Windows, absolute paths don't always start with a drive ID(e.g. C:). There are many possible prefixes. For verbatim paths (i.e., prefixed with \\?\), / should not be treated as a separator.

However, given the Windows path name convention, / can't be part of a file name or a directory name, so I also don't think there can be an actual issue with just always applying the slash conversion on snapshot and update on Windows. But we should just do a stirng level character replacement so that the following Windows absolute paths won't get messed up on the same machine¹:

• c:\temp\test-file.txt <-> c:/temp/test-file.txt
• \\127.0.0.1\c$\temp\test-file.txt <-> //127.0.0.1/c$/temp/test-file.txt
• \\LOCALHOST\c$\temp\test-file.txt <-> //LOCALHOST/c$/temp/test-file.txt
• \\.\c:\temp\test-file.txt <-> //./c:/temp/test-file.txt
• \\?\c:\temp\test-file.txt <-> //?/c:/temp/test-file.txt
• \\.\UNC\LOCALHOST\c$\temp\test-file.txt <-> //./UNC/LOCALHOST/c$/temp/test-file.txt

Partial conversion can be incorrect. For example, for \\?\c:\temp\test-file.txt, if we convert it to \\?\c:/temp/test-file.txt in Store, and use the following try_symlink implementation:

        let original = original
            .as_ref()
            .components()
            .fold(std::path::PathBuf::new(), |res, component| {
                res.join(component)
            });
        symlink_file(original, link)

an incorrect symink to \\?\c:/temp/test-file.txt will be created, because the components functions will yield \\?\c:, \, and temp/test-file.txt. For the same reason, the existing file_util::slash_path function can't be used.

Footnotes

1. The example paths come from https://learn.microsoft.com/en-us/dotnet/standard/io/file-path-formats#example-ways-to-refer-to-the-same-file, so they are very likely to be valid paths. ↩

[yuja]

But we should just do a stirng level character replacement so that the following Windows absolute paths won't get messed up on the same machine

Yes, I meant textual conversion. We might have to fix slash_path() to do that. I think we use path.components() there just because string manipulation over OsStr isn't easy.

[06393993]

Yes, I meant textual conversion.

Done. Also added test to guard the usage of absolute symlinks on Windows which fails if we use std::pah::Path::components(https://doc.rust-lang.org/std/path/struct.Path.html#method.components).

[yuja]

Can you also update the changelog?
Thanks.