Skip to content
This repository was archived by the owner on Mar 28, 2023. It is now read-only.

[cmake] validate CHECK_SYCL_ALL more thoroughly during configuration #1321

Merged
merged 1 commit into from
Oct 13, 2022

Conversation

ldrumm
Copy link

@ldrumm ldrumm commented Oct 12, 2022

I had a bit of trouble working out why lit was failing to start after configuration had succceeded. Turns out I was passing a partial tuple to cmake -DCHECK_SYCL_ALL due to a typo in a shell variable.

This patch verifies that both parts of each tuple in the CHECK_SYCL_ALL variable are nonempty and terminates configuration with an error message otherwise, which should make it a bit more obvious what's happening.

I had a bit of trouble working out why lit was failing to start after
configuration had succceeded. Turns out I was passing a partial tuple
to cmake `-DCHECK_SYCL_ALL` due to a typo in a shell variable.

This patch verifies that both parts of each tuple in the
`CHECK_SYCL_ALL` variable are nonempty and terminates configuration with
an error message otherwise, which should make it a bit more obvious
what's happening.
@ldrumm ldrumm requested a review from a team as a code owner October 12, 2022 13:54
@ldrumm ldrumm requested a review from steffenlarsen October 12, 2022 13:54
Copy link

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a good check. Thanks!

@ldrumm
Copy link
Author

ldrumm commented Oct 13, 2022

Are these test failures expected? I can't see how this would cause test failures?

@steffenlarsen
Copy link

Are these test failures expected? I can't see how this would cause test failures?

Yes. These failures were fixed in intel/llvm yesterday but have not reached the build used by this CI yet. This PR is good to go. 😄

@steffenlarsen steffenlarsen merged commit bc5e945 into intel:intel Oct 13, 2022
@ldrumm ldrumm deleted the validate-cmake-params branch October 13, 2022 13:28
myler pushed a commit to myler/llvm-test-suite that referenced this pull request Mar 22, 2023
intel#1321)

I had a bit of trouble working out why lit was failing to start after
configuration had succceeded. Turns out I was passing a partial tuple
to cmake `-DCHECK_SYCL_ALL` due to a typo in a shell variable.

This patch verifies that both parts of each tuple in the
`CHECK_SYCL_ALL` variable are nonempty and terminates configuration with
an error message otherwise, which should make it a bit more obvious
what's happening.
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
intel/llvm-test-suite#1321)

I had a bit of trouble working out why lit was failing to start after
configuration had succceeded. Turns out I was passing a partial tuple
to cmake `-DCHECK_SYCL_ALL` due to a typo in a shell variable.

This patch verifies that both parts of each tuple in the
`CHECK_SYCL_ALL` variable are nonempty and terminates configuration with
an error message otherwise, which should make it a bit more obvious
what's happening.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants