[PATCH] D59152: [libc++] Build <filesystem> support as part of the dylib

[Artem Belevich via Phabricator via llvm-commits]

tra added inline comments.


================
Comment at: libcxx/trunk/utils/libcxx/test/config.py:710-713
-        enable_fs = self.get_lit_bool('enable_filesystem', default=False)
-        if not enable_fs:
-            return
-        self.config.available_features.add('c++filesystem')
----------------
tra wrote:
> ldionne wrote:
> > tra wrote:
> > > This appears to break CUDA buildbot:
> > > http://lab.llvm.org:8011/builders/clang-cuda-build/builds/31842/steps/ninja%20check%201/logs/stdio
> > > 
> > > Apparently lit does not find that directory, even though it *is* present in the checked out tree with this config file. I guess lit has somehow misdetected libcxx_src_root, but I see nothing relevant in the logs.
> > > 
> > > Any ideas what could be wrong?
> > No idea what could be wrong, however can you check on the bot whether `<...>/libcxx/test/std/input.output/filesystems/Inputs/static_test_env` exists? This seems to be what `lit` is complaining about.
> That directory does exist. I've tracked the weirdness down to what may be a problem with [[ https://github.com/llvm-mirror/libcxx/blob/d88714db16a783850e52551a9735490df63c8b44/utils/libcxx/test/config.py#L81 | get_lit_conf() ]]:
> 
> ```
>   def get_lit_conf(self, name, default=None):
>         val = self.lit_config.params.get(name, None)
>         if val is None:
>             val = getattr(self.config, name, None)
>             if val is None:
>                 val = default
>         return val
> ```
> 
> The problem is that when `libcxx/test/config.py` is used from [[ https://github.com/llvm-mirror/libunwind/blob/master/test/lit.cfg | libunwind's lit.cfg]], `getattr(self.config, 'libcxx_src_root', None)` there actually appears to returns an empty string. No idea who/where/how sets it. It's possible that the problem is somewhere in libunwind.
> 
> Anyways, comparison with `None` fails, the empty string is returned as the result and things go downhill from there.
> 
> 
> 
I guess the empty string is the result of this line in various lit.site.cfg.in : 
llvm/projects/libcxx/test/lit.site.cfg.in
llvm/projects/libunwind/test/lit.site.cfg.in

```
config.libcxx_src_root          = "@LIBCXX_SOURCE_DIR@"
```

Perhaps `if val is None` should be changed to `if not Val` when we check result of `getattr(self.config)` as the empty string there will be a common default for unset parameters.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59152