Escape paths in depfiles.

[pborsutzki]

Escape paths in generated depfiles.

Problems especially occur when using depfiles containing absolute paths on windows that include driver letters (e.g. c:\foo\bar) as the depfile format requires a colon between the output dependency and its input dependencies (output: input1 input2 ...). When Windows drive letters come into play this becomes ambiguous (c:\output: c:\input1 c:\input2). Therefore, certain characters should be escaped when generating the depfile (c\:\\output1: c\:\\input1 c\:\\input2). This PR adds a method for escaping the relevant characters.

[pborsutzki]

@microsoft-github-policy-service agree

[pborsutzki]

Hi,

thanks for coming back to me so quickly.

We do ask that all changes come with adequate testing (please see https://github.com/microsoft/DirectXShaderCompiler/blob/main/CONTRIBUTING.md#contributing-code-and-content for more details). It would also be good to have an issue filed that describes the problem being addressed.

I'll try to follow up on this. Do you have any pointers where I can find similar tests in this project from which I can start?

a quick test https://godbolt.org/z/s94s9P37G suggests that DXC is already converting \ characters to / characters in paths, so I'm not sure why escaping is needed here.

Oops, I actually didn't check for backslashes. In that case I'd then simply remove that branch.

I'd like to know more about the scenario - which tools require the :s be escaped for example?

The tool in question is the Ninja build system. So this fix/issue stems from trying to use DXC's depfiles together with Ninja in a CMake environment on Windows. When provided with a depfile containing an absolute path containing a driver letter for the output Ninja will assume everything before the first colon to be the output, everything afterwards be a list of input dependencies. This then leads to Ninja failing to recognize the output correctly and also will generate an invalid dependency:

A depfile containing c:/foo/bar.cpp: c:/foo/bar.cpp c:/foo/bar.hpp will wrongly result in a file named c that comes with three input dependencies that read /foo/bar.cpp:, c:/foo/bar.cpp and c:/foo/bar.hpp.

Are there other tools that might not support escaped :s that would be broken by this change?

Currently there is only one I know of, which is CMake using the CMP0116 policy, though this can be "worked around" by disabling the policy. But in case of colons in the output path, the CMake result is broken anyways, as CMake currently ignores the escaped colon and simply generates similar broken dependencies like Ninja does without having the colon escaped. I'll try to propose a similar change for CMake as soon as I find the time for it.

Actually I am currently not aware of any build systems out there that support depfiles other than Ninja.

how was the escaping / not escaping of #, [, ] and $ chosen?

As the format of depfiles does not seem to be clearly specified (it seems to be specified "like makefiles"), I looked at how other tools generate and parse depfiles and decided for the "maximum" of escaped characters. Though depending on the OS, some of them are illegal as part of pathes anyways.

• Ninja: Supports escaped \, :, #, $ and whitespaces.
• CMake parser does currently not support escaped colons, but escaped $, #, \ and whitespaces.
• CMake generator currently only escapes backslashes and whitespaces.
• Slang escapes #, $, :, \, [, ] and whitespaces.

I'd also be happy to only go for e.g. escaping only colons and whitespaces if you think that this would be the safer solution.

I hope that answers your questions. Thank you for your time!

[damyanp]

I'll try to follow up on this. Do you have any pointers where I can find similar tests in this project from which I can start?

This may be a good place to start:

https://github.com/microsoft/DirectXShaderCompiler/blob/main/tools/clang/test/DXC/dump_dependency.hlsl

The tool in question is the Ninja build system [...]

This excellent context would be great to capture in the issue, and it does seem like you've done your due diligence here on the suitability of your fix.

[pborsutzki]

This may be a good place to start:

Thanks for the hint!

a quick test https://godbolt.org/z/s94s9P37G suggests that DXC is already converting \ characters to / characters in paths, so I'm not sure why escaping is needed here.

I've had another look, and it looks like DXC is just dumping absolute include paths containing backslashes into depfiles. In my opinion, escaping the backslash is unavoidable.

I also put up an issue, find it here: #6570