Fix canonical and read_symlink for junction points

[Flamefire]

Resolving junction points in read_symlink and canonical is wrong making it unusable: It silently returns the wrong results.

Issues identified:

• read_symlink does not handle junction points and reads the wrong struct
• canonical tries to resolve the root_name as a symlink. On windows this could be C: and refers to the current working directory on C. Hence depending if the current dir is a symlink canonical resolves the root_name to the current dir which is wrong

Supersedes #45

• Includes Remove API level checks in symlink functions in favor of runtime checks #142

[Flamefire]

Test was verified to trigger the bug: https://ci.appveyor.com/project/pdimov/filesystem/builds/22102228/job/0k3ueppx1l562jm5

[Flamefire]

There is a test failure remaining on MinGW resulting from a to low or missing define for _WIN32_WINNT. What shall be done about this? IMO it should be defined to the value for WinXP at the least if it is not defined and error if it is defined to low. Even WinXP is out-of-support so it can be dropped especially for new boost versions.

[Lastique]

Boost doesn't drop Windows versions, it's Windows SDKs that may drop some Windows versions. Legacy MinGW is pretty conservative, it appears to have stopped evolving before it had any reasonable support for NT6 APIs, so XP is a good choice for that target. With regard to Boost.Filesystem, I'm not sure there is any NT6 API used in the library, so it may make sense to target XP on any Windows SDK.

[stima]

@Flamefire I added tests for my pull request #102.

[mloskot]

Is this PR related to issues and patches submitted here?
https://svn.boost.org/trac10/ticket/10900

[Flamefire]

@mloskot It seems it adresses the same issue. Can't really compare the patches though. If this change is wanted, I can rebase

[GitEvgen]

How do I find out if this has been released and in which version?
Somewhat related to this issue: https://stackoverflow.com/questions/44393241/boostfilesystemrelative-cannot-access-the-file-because-it-is-being-used-by#

[Flamefire]

@mloskot @pdimov Requesting review of this PR. Rebased to current develop after the latest changes (indentation and such) caused major conflicts.

Not sure why it has been pending for so long given that on Windows a wrong struct is read which could be a security vulnerability. But also forgot about this before the 1.73 release

[mloskot]

@Flamefire I did skim the changes, but I'm not an expert of filesystem API. If your test passes, then I take it as all is good.

One nitpick thought, perhaps the test should be disabled on non-Windows completely with something like this <target-os>unix,<build>no

[Flamefire]

@mloskot Thanks, that's a good idea. Wasn't that familiar with B2 back then. And syntax of it is hard too, needed a colon, not comma ;-)

[Flamefire]

Also fixed a failed conflict resolution. Should be good now. Let's see what CI says. I'm no expert of this either but I'm confident in the changes. Especially as it now doesn't blindly reinterpret_cast a pointer but checks the "enum" first and fails for unknown values.

[mloskot]

needed a colon, not comma ;-)

Right, I forgot the , is a conjunction e.g. <target-os>windows,<toolset>gcc:<build>no

[Flamefire]

@mloskot @Lastique Finally fixed the remaining issues with the test. Factored out some stuff into #142 to keep this PR focused on the bugfix however this is rebased on #142 so that should be merged first.

[Lastique]

This check doesn't work on Windows 8.1. mklink exists, it prints help, and then the test decides it doesn't exist and exits.

[Flamefire]

Hm, any other idea? I have no clue why this would return non-zero

Last resort would be to check the below std::system("mklink /j junction real") directly but if that fails due to a bug in the code it will skip everything too.

[Lastique]

I don't know why std::system returns 1, but it does. It also returns 1 if the file doesn't exist. I think std::system result is not indicative and not trustworthy, we need to find another way. Possibly, from the Jamfile.

[GitEvgen]

Maybe try running it and check if the link got created. Only skip if it wasn't.
Also the test should include mklink /D ... The failures were different with that type of link. And there could be differences whether it's an absolute or relative link to absolute or relative path.

[Lastique]

I've already implemented a different check in the Jamfile.

[Lastique]

Another problem with this test is that it doesn't remove the temporary directory it creates. remove_all fails at removing "<temp_dir>/real/sub" with error 32 (directory used by another process).