Skip to content

[llvm][compiler-rt] Connect lit dependencies to test-depends targets. #81783

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
Feb 14, 2024

Conversation

drodriguez
Copy link
Contributor

compiler-rt was creating the test-depends targets and trying to fill its dependencies with a variable, but the variable was empty because it was supposed to take its value from a property. The changes in this commit grab the value of the property and add them as dependencies.

The changes in llvm are to remove the usage of DEPENDS arguments from add_custom_target, which according to the documentation is reserved for files/outputs created by add_custom_command. Use add_dependencies instead.

This is similar to the changes introduced in
4eb8458 for runtimes.

compiler-rt was creating the test-depends targets and trying to fill its
dependencies with a variable, but the variable was empty because it was
supposed to take its value from a property. The changes in this commit
grab the value of the property and add them as dependencies.

The changes in llvm are to remove the usage of `DEPENDS` arguments from
`add_custom_target`, which according to the documentation is reserved
for files/outputs created by `add_custom_command`. Use
`add_dependencies` instead.

This is similar to the changes introduced in
4eb8458 for runtimes.
Copy link
Collaborator

@smeenai smeenai left a comment

Choose a reason for hiding this comment

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

LGTM

@drodriguez drodriguez merged commit 4eb092d into llvm:main Feb 14, 2024
@drodriguez drodriguez deleted the compiler-rt-test-depends branch February 14, 2024 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants