Skip to content
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

[workflows] Fix lldb-tests and libclc-tests #80751

Merged
merged 1 commit into from
Feb 6, 2024
Merged

[workflows] Fix lldb-tests and libclc-tests #80751

merged 1 commit into from
Feb 6, 2024

Conversation

tstellar
Copy link
Collaborator

@tstellar tstellar commented Feb 5, 2024

This was broken by d25022b, which caused the workflow to pass an empty string to ninja as the target. The 'all' target is probably not the right target for these tests, but this is what the behavior was before
d25022b.

@tstellar
[workflows] Fix lldb-tests and libclc-tests
d65c318 
This was broken by d25022b, which
caused the workflow to pass an empty string to ninja as the target.
The 'all' target is probably not the right target for these tests,
but this is what the behavior was before
d25022b.
@llvmbot
Copy link
Member

llvmbot commented Feb 5, 2024

@llvm/pr-subscribers-github-workflow

Author: Tom Stellard (tstellar)

Changes

This was broken by d25022b, which caused the workflow to pass an empty string to ninja as the target. The 'all' target is probably not the right target for these tests, but this is what the behavior was before
d25022b.

Full diff: https://github.com/llvm/llvm-project/pull/80751.diff

3 Files Affected:

  • (modified) .github/workflows/libclc-tests.yml (-1)
  • (modified) .github/workflows/lldb-tests.yml (-1)
  • (modified) .github/workflows/llvm-project-tests.yml (+2-1)
diff --git a/.github/workflows/libclc-tests.yml b/.github/workflows/libclc-tests.yml
index 29d050db2f12c..23192f776a985 100644
--- a/.github/workflows/libclc-tests.yml
+++ b/.github/workflows/libclc-tests.yml
@@ -36,5 +36,4 @@ jobs:
     name: Test libclc
     uses: ./.github/workflows/llvm-project-tests.yml
     with:
-      build_target: ''
       projects: clang;libclc
diff --git a/.github/workflows/lldb-tests.yml b/.github/workflows/lldb-tests.yml
index ef5d7c7d581b7..6bb9721956258 100644
--- a/.github/workflows/lldb-tests.yml
+++ b/.github/workflows/lldb-tests.yml
@@ -36,5 +36,4 @@ jobs:
     name: Build lldb
     uses: ./.github/workflows/llvm-project-tests.yml
     with:
-      build_target: ''
       projects: clang;lldb
diff --git a/.github/workflows/llvm-project-tests.yml b/.github/workflows/llvm-project-tests.yml
index 494263be7f0df..3bc7bd4957fa6 100644
--- a/.github/workflows/llvm-project-tests.yml
+++ b/.github/workflows/llvm-project-tests.yml
@@ -22,8 +22,9 @@ on:
   workflow_call:
     inputs:
       build_target:
-        required: true
+        required: false
         type: string
+        default: "all"
 
       projects:
         required: true

@boomanaiden154
Copy link
Contributor

Do you have a failure example? I'm struggling to understand why exactly this failed. I would think that passing a null string to ninja would just cause it to build all rather than failing.

If we want to actually run the tests (which seems to be the intention here?) I think we need to run the check-* target, which I assume would be check-lldb and check-libclc (although not certain on those).

@tstellar
Copy link
Collaborator Author

tstellar commented Feb 5, 2024

Here is an example of the failure: https://github.com/llvm/llvm-project/actions/runs/7771992746/job/21193903687?pr=80584

For lldb, I'm just trying to build it, not run the tests.

@boomanaiden154
Copy link
Contributor

Here is an example of the failure: https://github.com/llvm/llvm-project/actions/runs/7771992746/job/21193903687?pr=80584

Thanks!

For lldb, I'm just trying to build it, not run the tests.

Ah, okay.

boomanaiden154
boomanaiden154 approved these changes Feb 5, 2024
View reviewed changes
Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

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

LGTM.

@tstellar tstellar merged commit 792d928 into llvm:main Feb 6, 2024
7 of 8 checks passed
tstellar added a commit to tstellar/llvm-project that referenced this pull request Feb 6, 2024
@tstellar
[workflows] Fix lldb-tests and libclc-tests (llvm#80751)
ff2405a 
This was broken by d25022b, which
caused the workflow to pass an empty string to ninja as the target. The
'all' target is probably not the right target for these tests, but this
is what the behavior was before
d25022b.

(cherry picked from commit 792d928)
tstellar added a commit to tstellar/llvm-project that referenced this pull request Feb 8, 2024
@tstellar
[workflows] Fix lldb-tests and libclc-tests (llvm#80751)
559243a 
This was broken by d25022b, which
caused the workflow to pass an empty string to ninja as the target. The
'all' target is probably not the right target for these tests, but this
is what the behavior was before
d25022b.

(cherry picked from commit 792d928)
tstellar added a commit that referenced this pull request Feb 8, 2024
@tstellar
[workflows] Fix lldb-tests and libclc-tests (#80751)
88d4f1a 
This was broken by d25022b, which
caused the workflow to pass an empty string to ninja as the target. The
'all' target is probably not the right target for these tests, but this
is what the behavior was before
d25022b.

(cherry picked from commit 792d928)
tstellar added a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
@tstellar
[workflows] Fix lldb-tests and libclc-tests (llvm#80751)
0c6fae6 
This was broken by d25022b, which
caused the workflow to pass an empty string to ninja as the target. The
'all' target is probably not the right target for these tests, but this
is what the behavior was before
d25022b.

(cherry picked from commit 792d928)
tstellar added a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
@tstellar
[workflows] Fix lldb-tests and libclc-tests (llvm#80751)
a47808e 
This was broken by d25022b, which
caused the workflow to pass an empty string to ninja as the target. The
'all' target is probably not the right target for these tests, but this
is what the behavior was before
d25022b.

(cherry picked from commit 792d928)
tstellar added a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
@tstellar
[workflows] Fix lldb-tests and libclc-tests (llvm#80751)
f4737d1 
This was broken by d25022b, which
caused the workflow to pass an empty string to ninja as the target. The
'all' target is probably not the right target for these tests, but this
is what the behavior was before
d25022b.

(cherry picked from commit 792d928)
@pointhex pointhex mentioned this pull request May 7, 2024
Closed
@aemerson aemerson mentioned this pull request May 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@boomanaiden154 boomanaiden154 boomanaiden154 approved these changes

Assignees
No one assigned
Labels
github:workflow
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tstellar @llvmbot @boomanaiden154