[PATCH] D103014: [lit] Make LLVM_LIT_PATH_FUNCTION to use pathlib

Kristina Bessonova via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 4 07:12:52 PDT 2021


krisb added a comment.

In D103014#2792673 <https://reviews.llvm.org/D103014#2792673>, @Meinersbur wrote:

> Besides the switch from os.path to pathlib, is the change to first build the path, then to resolve it (instead of first using abspath to resolve it, then concat the path) intentional?

Oh, surely no. I'll add back resolving first. Thank you for pointing to this!

> Could this also be fixed by making discovery.py consistently use the the map value instead its normcase-d key when building paths?

We can do smth like

  --- a/llvm/utils/lit/lit/discovery.py
  +++ b/llvm/utils/lit/lit/discovery.py
  @@ -53,8 +53,7 @@ def getTestSuite(item, litConfig, cache):
           config_map = litConfig.params.get('config_map')
           if config_map:
               cfgpath = os.path.realpath(cfgpath)
  -            cfgpath = os.path.normcase(cfgpath)
  -            target = config_map.get(cfgpath)
  +            target = config_map.get(os.path.normcase(cfgpath))
               if target:
                   cfgpath = target

for discovery.py so that site config will be loaded in the 'original' case (and this seems to be right things to do), but it will not fully fix the problem. Additionally, we can reslove the cfgpath here before loading , smth like

  --- a/llvm/utils/lit/lit/discovery.py
  +++ b/llvm/utils/lit/lit/discovery.py
  @@ -53,8 +53,7 @@ def getTestSuite(item, litConfig, cache):
           config_map = litConfig.params.get('config_map')
           if config_map:
               cfgpath = os.path.realpath(cfgpath)
  -            cfgpath = os.path.normcase(cfgpath)
  -            target = config_map.get(cfgpath)
  +            target = config_map.get(os.path.normcase(cfgpath))
               if target:
                   cfgpath = target
  +            cfgpath = str(Path(cfgpath).resolve())

and that seems to do the trick. But I still think that the implementation of `path()` function should be changed to return real correctly cased paths and do not rely on the case of `__file__`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103014/new/

https://reviews.llvm.org/D103014



More information about the llvm-commits mailing list