This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] [test] Move the is_<platform> functions down to subclasses
ClosedPublic

Authored by mstorsjo on Mar 5 2021, 7:33 AM.

Details

Summary

If cross testing (and manually specifying a LIBCXX_TARGET_INFO in the
cmake configuration, as the default is to match the build platform),
we want the accessors for querying the target platform, is_windows,
is_darwin, to return the right value depending on which target info
class is used, not based on what platform is running the build and
driving the tests.

When LIBCXX_TARGET_INFO isn't defined, the right target info class
is chosen automatically based on the platform one is running on, so
this shouldn't make any practical difference for such setups.

Diff Detail

Event Timeline

mstorsjo created this revision.Mar 5 2021, 7:33 AM
mstorsjo requested review of this revision.Mar 5 2021, 7:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 5 2021, 7:33 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
curdeius accepted this revision as: curdeius.Mar 5 2021, 8:14 AM
curdeius added a subscriber: curdeius.

LGTM. Hopefully that doesn't break any "exotic" setups.

libcxx/utils/libcxx/test/target_info.py
24–25

Just a remark. No action needed.
platform() (here and in LinuxLocalTI) seems unused now unless I'm mistaken.
But it seems it's the case of some other methods especially in LinuxLocalTI.

ldionne accepted this revision.Mar 5 2021, 8:15 AM
ldionne added a subscriber: ldionne.

LGTM, but FYI the whole target_into.py and even config.py will eventually be replaced by simpler setups like libcxx/test/configs/libcxx-trunk-shared.cfg.in. It's a slow process but I've been chipping at it little by little.

This revision is now accepted and ready to land.Mar 5 2021, 8:15 AM

LGTM, but FYI the whole target_into.py and even config.py will eventually be replaced by simpler setups like libcxx/test/configs/libcxx-trunk-shared.cfg.in. It's a slow process but I've been chipping at it little by little.

Ok, that sounds good to me, I'm not particularly attached to the current way of things, as long as the new setup is something that is flexible enough (and it looks like its) for oddball setups :-)

mstorsjo updated this revision to Diff 328639.Mar 5 2021, 1:50 PM

Reuploading to retry CI (last round had a fair bit of spurious breakage)