Skip to content

Special-case catalyst when computing a path for stdlib_dir in lit config #33195

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 31, 2020
Merged

Special-case catalyst when computing a path for stdlib_dir in lit config #33195

merged 1 commit into from
Jul 31, 2020

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Jul 29, 2020 •
edited
Loading

target_sdk_name is macosx when targeting maccatalyst, but the stdlib swift modules are placed under swift/maccatalyst instead of swift/macosx.

Using run_osis not correct in the general sense though, hence the requirement for a special case instead of just replacing target_sdk_name with it. For example, it would be invalid to use run_os instead of target_sdk_name on Windows, because the sdk_name is going to be windows while the run_os variable may be windows-msvc.

@artemcm artemcm requested a review from compnerd July 29, 2020 23:12
@artemcm
Copy link
Contributor Author

artemcm commented Jul 29, 2020

@swift-ci please test

@compnerd
Copy link
Member

Hmm, I'm not sure I understand the commit message - you say run_os is not correct, but that is exactly what you are using (as that is what will identify the catalyst build). The change itself is fine though (even though it conflicts with one of my pending changes :-().

@artemcm
Copy link
Contributor Author

artemcm commented Jul 29, 2020

Hmm, I'm not sure I understand the commit message - you say run_os is not correct, but that is exactly what you are using (as that is what will identify the catalyst build). The change itself is fine though (even though it conflicts with one of my pending changes :-().

Sorry, to be more specific, run_os is correct in the catalyst case, but not correct in the general sense, hence the requirement for a special case instead of just replacing target_sdk_name with it.
From what I can gather in lit.cfg, It would be invalid to use run_os instead of target_sdk_name on Windows, for example, because there the sdk_name is going to be windows while the run_os variable may be windows-msvc.

@compnerd
Copy link
Member

Ah, yes, run_os in general is not correct, and needs to be substituted for the one particular case. Would be nice to update the commit message.

@artemcm
Copy link
Contributor Author

artemcm commented Jul 30, 2020

@swift-ci test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - f8aa20370b6c34cb2ae79252415444fd1b9850c5

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - f8aa20370b6c34cb2ae79252415444fd1b9850c5

@compnerd
Copy link
Member

@swift-ci please test windows platform

3 similar comments
@artemcm
Copy link
Contributor Author

artemcm commented Jul 30, 2020

@swift-ci please test windows platform

@artemcm
Copy link
Contributor Author

artemcm commented Jul 30, 2020

@swift-ci please test windows platform

@artemcm
Copy link
Contributor Author

artemcm commented Jul 30, 2020

@swift-ci please test windows platform

@artemcm
Copy link
Contributor Author

artemcm commented Jul 30, 2020

This failure was just fixed in: #33204
@swift-ci please test windows platform

@artemcm
Copy link
Contributor Author

artemcm commented Jul 30, 2020

@swift-ci please test windows platform

1 similar comment
@artemcm
Copy link
Contributor Author

artemcm commented Jul 30, 2020

@swift-ci please test windows platform

@artemcm
Copy link
Contributor Author

artemcm commented Jul 30, 2020

@swift-ci please test Windows platform

@artemcm
Copy link
Contributor Author

artemcm commented Jul 30, 2020

@compnerd , the windows machine seems to be out of space.

@artemcm
Copy link
Contributor Author

artemcm commented Jul 31, 2020

@swift-ci please test Windows platform

@artemcm
Special-case catalyst when computing a path for stdlib_dir in lit config
ed34973 
target_sdk_name is `macosx` when targeting `maccatalyst`, but the stdlib swift modules are placed under `swift/maccatalyst` instead of `swift/macosx`.
Using `run_os` is not correct in the general sense though, hence the requirement for a special case instead of just replacing `target_sdk_name` with it.
For example, it would be invalid to use run_os instead of target_sdk_name on Windows, because the sdk_name is going to be windows while the run_os variable may be windows-msvc.
@artemcm
Copy link
Contributor Author

artemcm commented Jul 31, 2020

@swift-ci please test

@artemcm
Copy link
Contributor Author

artemcm commented Jul 31, 2020

@swift-ci please test Windows platform

1 similar comment
@artemcm
Copy link
Contributor Author

artemcm commented Jul 31, 2020

@swift-ci please test Windows platform

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 0f6c4e1381efad678a78fa1e466f82c23d660100

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 0f6c4e1381efad678a78fa1e466f82c23d660100

@artemcm
Copy link
Contributor Author

artemcm commented Jul 31, 2020

@swift-ci please test Windows platform

2 similar comments
@artemcm
Copy link
Contributor Author

artemcm commented Jul 31, 2020

@swift-ci please test Windows platform

@artemcm
Copy link
Contributor Author

artemcm commented Jul 31, 2020

@swift-ci please test Windows platform

@artemcm artemcm merged commit 9c2a045 into swiftlang:master Jul 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compnerd compnerd compnerd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@artemcm @compnerd @swift-ci