[libcxx-commits] [PATCH] D78200: [libc++] [test] Generate static_test_env on the fly

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

@EricWF I understand your concern about using function `foo()` in the setup for testing function `foo()`. However, the flipside is that we have to reimplement e.g. `current_path()` ourselves in `filesystem_helper.h`, which entails basically copy-pasting it from the libc++ sources. And that's not much better IMO. Previously, we worked around that by using Python to do these things, but that assumes Python on the target, and we can't assume that.

If we had another way of solving the problem on Windows, we would not need this patch and everything would be much simpler. So let me ask again what I asked above:

> Do you (or anyone else) have an idea to deal with the problem described on Windows? Would you prefer if we only created symlinks on the fly (like the first version of this patch does)?

For context, here's what the problem is:

> Windows doesn't really have a concept of symlinks. So, when the monorepo is cloned, those symlinks turn to ordinary text files. Previously, if we cross-compiled libc++ for some symlink-friendly system (e. g. Linux) and ran tests on the target system, some tests would fail.

If we can solve this problem differently, I'm all for it. One possibility would be to store a tarball of the static environment in the source tree, and then automatically untar it only when on the target system -- this way Windows would never see any symlinks. Thoughts?



================
Comment at: libcxx/test/support/filesystem_test_helper.h:306
+  fs::path OldCWD;
+  CWDGuard() : OldCWD(fs::current_path()) { }
+  ~CWDGuard() { fs::current_path(OldCWD); }
----------------
EricWF wrote:
> I'm not OK with changing the tests working directory during the test.
Some tests already do this. This is just moving `CwdGuard` from `canonical.pass.cpp` to this header so it can be reused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78200