[llvm] r210597 - lit: warn when passed invalid pathname

Daniel Dunbar daniel at zuster.org
Tue Jun 17 11:13:31 PDT 2014


On Tue, Jun 17, 2014 at 10:48 AM, Hans Wennborg <hans at chromium.org> wrote:

> On Mon, Jun 16, 2014 at 1:27 PM, Hans Wennborg <hans at chromium.org> wrote:
> > On Mon, Jun 16, 2014 at 11:20 AM, Daniel Dunbar <daniel at zuster.org>
> wrote:
> >> Sorry, I should have caught that in review.
> >>
> >> Adam is right, the reason why this code didn't do this previously is
> because
> >> of the "virtual test discovery" mechanism that lets you name tests
> inside
> >> the "test exec root".
> >>
> >> Hans, I think it is probably necessary to put this patch later, after
> the
> >> actual discovery is done.
>
> I took a second look at this. It would be easy to move the check until
> after the discovery is done, but then my new error message is
> misleading:
>
> If we say "'foo/bar/' doesn't exist", that might not be the actual
> problem, because maybe we were also searching in ../../test, where
> foo/bar does exist - it just didn't contain any tests. I don't
> understand all the magic around test discovery, so I'm not going to
> pursue this further.
>
> I would like to simplify this code while I'm here though:
>
> --- a/utils/lit/lit/discovery.py
> +++ b/utils/lit/lit/discovery.py
> @@ -200,9 +200,7 @@ def find_tests_for_inputs(lit_config, inputs):
>      # Expand '@...' form in inputs.
>      actual_inputs = []
>      for input in inputs:
> -        if os.path.exists(input) or not input.startswith('@'):
> -            actual_inputs.append(input)
> -        else:
> +        if input.startswith('@'):
>              f = open(input[1:])
>              try:
>                  for ln in f:
> @@ -211,6 +209,8 @@ def find_tests_for_inputs(lit_config, inputs):
>                          actual_inputs.append(ln)
>              finally:
>                  f.close()
> +        else:
> +            actual_inputs.append(input)
>
> I think this reflects the intent of the code better - add all files to
> actual_inputs, except files starting with @ which are treated as
> response files. (Supporting the case of test inputs start with @
> doesn't seem important.) What do you think?
>

That is fine with me.

 - Daniel


>
> Thanks,
> Hans
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140617/6d2e40b1/attachment.html>


More information about the llvm-commits mailing list