This is an archive of the discontinued LLVM Phabricator instance.

[ClangDepScan] Add UNSUPPORTED: system-windows to failing P1689.cppm test

Authored by TWeaver on Feb 10 2023, 8:51 AM.



This test has caused numerous windows bot failures:

The test was added in

The omission of this test from windows builds was reverted in

Diff Detail

Event Timeline

TWeaver created this revision.Feb 10 2023, 8:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2023, 8:51 AM
TWeaver requested review of this revision.Feb 10 2023, 8:51 AM
TWeaver edited the summary of this revision. (Show Details)Feb 10 2023, 8:55 AM

Let's fix the test or revert the change instead.

TWeaver updated this revision to Diff 496505.Feb 10 2023, 8:57 AM

slight confusion on the test name, sorted now

TWeaver added a comment.EditedFeb 10 2023, 9:04 AM

@thakis that's a great idea in theory, but this has been caused by the reversion of 4/4 in this series of patches, the test was added in 2/4, so wouldn't there potentially a need to revert at least 2/4, 3/4 and 4/4, which could cause further errors/failures if 1/4 is malformed.

But I'm happy to concede and wait it out for a fix.

dyung added a subscriber: dyung.Feb 10 2023, 9:12 AM

Reverts are cheap, we can just revert the last 3.

TWeaver reclaimed this revision.Feb 10 2023, 10:23 AM

There is the potential for further fall out from reverting these additional patches. I strongly feel that the best course of action here is to add the UNSUPPORTED line outlined in this patch and push this issue back onto the original author who's responsible for reverting the patch that causes this to fail.

There are several build bots failing as a result of the revert in and it's important to get those green and enable failure reporting for future incoming patches.

The original author can then address the Windows failures in this test in future, in their own time.

probinson accepted this revision.Feb 10 2023, 10:24 AM
probinson added a subscriber: probinson.

LGTM. A revert isn't cheap when it reverts a test to a broken state.

This revision is now accepted and ready to land.Feb 10 2023, 10:24 AM
TWeaver closed this revision.Feb 10 2023, 10:44 AM

Committed in

My apologies for missing the review link in the commit message.