[libcxx-commits] [PATCH] D102045: [libc++] Provide 'buildhost-<platform> feature for the tests.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

In D102045#2860893 <https://reviews.llvm.org/D102045#2860893>, @vvereschaka wrote:

>> I suggest we add a feature named host=<name-of-the-host>.
>
> No objection, but looks like unusual representation for the feature.
> So it may look like that:
>
>   // REQUIRES: host=linux && host=darwin
>   ...
>   // UNSUPPORTED: host=windows
>
> is this that expected?

Yes. Lit supports `=` in features. We don't use it often, but it's definitely valid, and I think it makes sense especially since we have `target=<triple>` now. Note that the same argument could be made for `target=<triple>`, but I think it's better than `target-<triple>` because there's already dashes in the triple, and not saying `target` anywhere makes it ambiguous.

Ack regarding the fact that a full triple for the host is overkill, I'm convinced now.

LGTM with my suggested edits.



================
Comment at: libcxx/utils/libcxx/test/features.py:165-168
+  Feature(name='buildhost-{}'.format(sys.platform.lower().strip())),
+  # sys.platform can be represented by "sub-system" on Windows host, such as 'win32', 'cygwin', 'mingw' & etc.
+  # Here is a consolidated feature for the build host plaform name on Windows.
+  Feature(name='buildhost-windows', when=lambda cfg: platform.system().lower().startswith('windows'))
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102045