[PATCH] D29185: [lit] Allow llvm's build and test systems to support paths with spaces

ben via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 17 00:54:16 PST 2017


bd1976llvm added a comment.

In https://reviews.llvm.org/D29185#677753, @chandlerc wrote:

> In https://reviews.llvm.org/D29185#677308, @mgorny wrote:
>
> > I would like to point out that if you proactively reject all paths with spaces, you wouldn't be able to add additional tests that would test support for paths with spaces ;-). While I agree that we should avoid spaces whenever possible, I think it would be reasonable to add tests for paths with spaces wherever we expect them to be well-supported and want to avoid regressions.
>
>
> You could add to lit's own tests a simple test tree under a path with a space and that would seem fine?
>
> My goal is that *within* a lit test tree, the paths of the files that lit is walking don't have spaces.


Right. Thanks for the comments.

I was initially going to go back to the position that we don't want space in paths anywhere inside the svn repro and we don't need any tests.

However, that means that any work on lit could easily break support for spaces in paths.

Therefore, I will add a "spaces in paths" test to the lit test suite and update the patch.

I am now opposed to adding explicit checks to lit and CMake for spaces in paths within llvm as this is just not needed. People are just not going to start doing this IMO.


https://reviews.llvm.org/D29185





More information about the llvm-commits mailing list