Skip to content

[lit] Stub out lit feature detection on non-Apple platforms #23570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 26, 2019

Conversation

yln
Copy link
Contributor

@yln yln commented Mar 26, 2019

Quick fix for an oversight on my part that slipped through CI in my previous PR (#23455): the TSan runtime now has a link-time dependency on libdispatch (unfortunately).
This temporarily disables the following two tests on Linux to unbreak CI build.

Swift(linux-x86_64) :: Sanitizers/witness_table_lookup.swift
Swift(linux-x86_64) :: Sanitizers/tsan.swift

The above two tests are also the only 2 TSan (executable) tests that are run on Linux. Once I fix the TODO mentioned in this PR, I will go through all REQUIRES: tsan_runtime occurences and try to enable them by using libdispatch/foundation instead of objc_interop.

@yln
Copy link
Contributor Author

yln commented Mar 26, 2019

@swift-ci Please test

@yln yln self-assigned this Mar 26, 2019
@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 50c82a5ac99cb5466c487f0d25db7b487d341f47

This temporarily disables the following two tests to unbreak the Linux
CI build.

Swift(linux-x86_64) :: Sanitizers/witness_table_lookup.swift
Swift(linux-x86_64) :: Sanitizers/tsan.swift
@yln yln force-pushed the libdispatch-tests-stub branch from 50c82a5 to ff16bb8 Compare March 26, 2019 18:52
@yln
Copy link
Contributor Author

yln commented Mar 26, 2019

@swift-ci Please test Linux

@swiftlang swiftlang deleted a comment from swift-ci Mar 26, 2019
@yln
Copy link
Contributor Author

yln commented Mar 26, 2019

@swift-ci Please smoke test osx

@yln yln requested review from compnerd and devincoughlin March 26, 2019 22:16
@yln
Copy link
Contributor Author

yln commented Mar 26, 2019

@jrose-apple Are you okay with the updated patch? This should unblock #23508

@jrose-apple
Copy link
Contributor

I'm a little iffy on whether or not this will work because I'm not sure if we're consistent about "dispatch" meaning "Swift Dispatch" or "C Dispatch", but maybe it won't matter. It's okay for unblocking your other work, though, so go for it.

@yln
Copy link
Contributor Author

yln commented Mar 26, 2019

I'm not sure if we're consistent about "dispatch" meaning "Swift Dispatch" or "C Dispatch", but maybe it won't matter.

Good point. Here (the lit feature) means "Swift Dispatch", i.e., everything required for import Dispatch. "C Dispatch" is required for -sanitize=thread when linking. "Swift Dispatch" implies "C Dispatch" (but not the other way around). Ideally, these would all be separate things.

@yln yln merged commit 1ad5b70 into master Mar 26, 2019
@yln yln deleted the libdispatch-tests-stub branch March 26, 2019 22:34
@compnerd
Copy link
Member

compnerd commented Mar 27, 2019

This broke the test suite on Windows! https://dev.azure.com/compnerd/windows-swift/_build/results?buildId=823

@jrose-apple
Copy link
Contributor

Can you, uh, tell us how it broke the test suite on Windows?

@compnerd
Copy link
Member

#23585 should repair the issue as a quick repair, looking into what it would take to properly detect this in #23588

@compnerd
Copy link
Member

Can you, uh, tell us how it broke the test suite on Windows?

lit.py: C:\agent\_work\1\s\llvm\utils\lit\lit\TestingConfig.py:102: fatal: unable to parse config file 'C:/agent/_work/1/s/swift\\test\\lit.cfg', traceback: Traceback (most recent call last):
  File "C:\agent\_work\1\s\llvm\utils\lit\lit\TestingConfig.py", line 89, in load_from_path
    exec(compile(data, path, 'exec'), cfg_globals, None)
  File "C:/agent/_work/1/s/swift\test\lit.cfg", line 1503, in <module>
    config.available_features.remove('tsan_runtime')
KeyError: 'tsan_runtime'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants