[libc++] Provide 'buildhost-<platform> feature for the tests.

[ldionne]

Summary

The target platform could differ from the host platform for the cross platform builds. Some tests are depended on the build host features and they need to determine a proper platform environment.

This commit adds a build host platform name feature for the libc++ tests in format buildhost-<platform>, such as buildhost-linux, buildhost-darwin, buildhost-windows & etc.

The Windows host gets two features: one buildhost-windows and another based on Windows "sub-system", such as buildhost-win32, buildhost-cygwin & etc.

[ldionne]

Thanks for the patch. I'd like to sit on this for a bit and implement a more general mechanism for detecting the host triple vs the target triple. This is related to my work in D102012.

[vvereschaka]

got it, perfect. Thank you.

[ldionne]

Now that D104747 has landed, we can look at this again.

I suggest we add a feature named host=<name-of-the-host>. Would it make sense for that to be a full triple instead of just the result of sys.platform?

[ldionne]

Also, where would that feature be used? It would be helpful for me to understand which tests benefit from that feature. In general, we try not to add features if they are not used anywhere.

[mstorsjo]

In D102045#2858536, @ldionne wrote:

Also, where would that feature be used? It would be helpful for me to understand which tests benefit from that feature. In general, we try not to add features if they are not used anywhere.

I don't think it matters to have a full triple for the build host - in a cross testing scenario, we know the exact triple of the target we're compiling for, but we know pretty little about the host we're driving the tests from - within the python environment we don't know much else than what python tells us.

In D102045#2858536, @ldionne wrote:

Also, where would that feature be used? It would be helpful for me to understand which tests benefit from that feature. In general, we try not to add features if they are not used anywhere.

I think this was meant to be used as a different workaround for the thing that was fixed in D102048, but there might be other cases that could need this as well.

[vvereschaka]

Would it make sense for that to be a full triple instead of just the result of sys.platform?

I think it would be much easier using а short representation of platform name instead of a full triple and especially if it corresponds to the target platform names, but with the prefixes or something similar.
Also it is not always possible to get a correct triple for the windows platform.

Also, where would that feature be used?

• workarounds (as @mstorsjo mentioned - D102048).
• host specific tests.

Not sure it will be used often.

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?

[ldionne]

In 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.

libcxx/utils/libcxx/test/features.py

155–158

[ldionne]

Apply comments.

[ldionne]

Will ship this once the CI is green. Thanks for the patch @vvereschaka !

[ldionne]

CI is green except for the runtimes build, which is a failure that was fixed on main.

libcxx/utils/libcxx/test/features.py

DEFAULT_FEATURES += [
   Feature(name='darwin', when=lambda cfg: '__APPLE__' in compilerMacros(cfg)),
   Feature(name='windows', when=lambda cfg: '_WIN32' in compilerMacros(cfg)),
   Feature(name='windows-dll', when=lambda cfg: '_WIN32' in compilerMacros(cfg) and not '_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS' in compilerMacros(cfg)),
   Feature(name='linux', when=lambda cfg: '__linux__' in compilerMacros(cfg)),
   Feature(name='netbsd', when=lambda cfg: '__NetBSD__' in compilerMacros(cfg)),
   Feature(name='freebsd', when=lambda cfg: '__FreeBSD__' in compilerMacros(cfg))
 ]
 
+# Add features representing the build host platform name.
+# The build host could differ from the target platform for cross-compilation.
+DEFAULT_FEATURES += [
+  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'))
+]
 
 # Detect whether GDB is on the system, and if so add a substitution to access it.
 DEFAULT_FEATURES += [
   Feature(name='host-has-gdb',
     when=lambda cfg: shutil.which('gdb') is not None,
     actions=[AddSubstitution('%{gdb}', lambda cfg: shutil.which('gdb'))]
   )
 ]