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.

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

clang: Allow conditional compilation of clangd #912

Merged
merged 2 commits into from
Feb 15, 2024

Conversation

lorenzph
Copy link
Contributor

@lorenzph lorenzph commented Feb 12, 2024

clangd should not be needed in target builds. Extend PACKAGECONFIG to
enable selective disabling and disable it by default for target builds.

Additionally, this enables compilation of clang-native on Debian "bullseye"
hosts where GCC 10 fails to compile clangd with an internal compilation
error.


Contributor checklist

Reviewer Guidelines

  • When submitting a review, please pick:
    • 'Approve' if this change would be acceptable in the codebase (even if there are minor or cosmetic tweaks that could be improved).
    • 'Request Changes' if this change would not be acceptable in our codebase (e.g. bugs, changes that will make development harder in future, security/performance issues, etc).
    • 'Comment' if you don't feel you have enough information to decide either way (e.g. if you have major questions, or you don't understand the context of the change sufficiently to fully review yourself, but want to make a comment)

@lorenzph lorenzph requested a review from kraj as a code owner February 12, 2024 12:25
@lorenzph lorenzph force-pushed the clangd-packageconfig branch from 111e6bb to f9206c8 Compare February 12, 2024 15:44
Move common PACKAGECONFIG default values into a dedicated variable and
sort values alphabetically (hopefully making it easier to figure out
where to add a new value when extending the variable).

Signed-off-by: Philip Lorenz <philip.lorenz@bmw.de>
@lorenzph lorenzph force-pushed the clangd-packageconfig branch from f9206c8 to 3f95af7 Compare February 14, 2024 09:09
@quaresmajose
Copy link
Contributor

quaresmajose commented Feb 14, 2024

it looks like target is replaced by native on the commit message.
looking at the code the clangd is disabled for class-target builds and not for native.

clangd is not needed for -native builds. Extend PACKAGECONFIG to enable
selective disabling and disable it by default for native builds.

clangd should not be needed in target builds. Extend PACKAGECONFIG to
enable selective disabling and disable it by default for target builds.

Signed-off-by: Philip Lorenz <philip.lorenz@bmw.de>
@lorenzph lorenzph force-pushed the clangd-packageconfig branch from 3f95af7 to 09b0224 Compare February 14, 2024 10:21
@lorenzph
Copy link
Contributor Author

it looks like target is replaced by native on the commit message. looking at the code the clangd is disabled for class-target builds and not for native.

clangd is not needed for -native builds. Extend PACKAGECONFIG to enable
selective disabling and disable it by default for native builds.

My bad... Fixed.

Copy link
Owner

@kraj kraj left a comment

Choose a reason for hiding this comment

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

lgtm

@lorenzph
Copy link
Contributor Author

@kraj: Are the build logs of the CI run available somewhere (based on the log output it doesn't look like the CI failure is caused by this change but I'd like to make sure)?

@kraj
Copy link
Owner

kraj commented Feb 15, 2024

@kraj: Are the build logs of the CI run available somewhere (based on the log output it doesn't look like the CI failure is caused by this change but I'd like to make sure)?

they are right in github actions link above. Click on 'details'

@lorenzph
Copy link
Contributor Author

@kraj: Are the build logs of the CI run available somewhere (based on the log output it doesn't look like the CI failure is caused by this change but I'd like to make sure)?

they are right in github actions link above. Click on 'details'

I meant the detailed logs of the bitbake run (or I am simply failing to find the right link :-)). The logs from https://github.com/kraj/meta-clang/actions/runs/7899722329/attempts/2?pr=912:

 Traceback (most recent call last):
  File "/home/khem/actions-runner-meta-clang/_work/meta-clang/meta-clang/yoe/sources/poky/meta/lib/oeqa/core/decorator/__init__.py", line 35, in wrapped_f
    return func(*args, **kwargs)
  File "/home/khem/actions-runner-meta-clang/_work/meta-clang/meta-clang/yoe/sources/poky/meta/lib/oeqa/runtime/cases/parselogs.py", line 185, in test_parselogs
    self.assertEqual(errcount, 0, msg=self.msg)
AssertionError: 1 != 0 : Log: /home/khem/actions-runner-meta-clang/_work/meta-clang/meta-clang/yoe/build/tmp/work/qemuarm64-yoe-linux/yoe-sdk-image/1.0/target_logs/postinstall.log
-----------------------
Central error: * opkg_cmd_exec: Command failed to capture privilege lock: Resource temporarily unavailable.
***********************
 * opkg_lock: Could not lock /run/opkg.lock: Resource temporarily unavailable.
 * opkg_cmd_exec: Command failed to capture privilege lock: Resource temporarily unavailable.

sounds like the failure is unrelated to change (but there is a chance that the issue leading up to this would be visible from the bitbake logs).

@kraj
Copy link
Owner

kraj commented Feb 15, 2024

@kraj: Are the build logs of the CI run available somewhere (based on the log output it doesn't look like the CI failure is caused by this change but I'd like to make sure)?

they are right in github actions link above. Click on 'details'

I meant the detailed logs of the bitbake run (or I am simply failing to find the right link :-)). The logs from https://github.com/kraj/meta-clang/actions/runs/7899722329/attempts/2?pr=912:

 Traceback (most recent call last):
  File "/home/khem/actions-runner-meta-clang/_work/meta-clang/meta-clang/yoe/sources/poky/meta/lib/oeqa/core/decorator/__init__.py", line 35, in wrapped_f
    return func(*args, **kwargs)
  File "/home/khem/actions-runner-meta-clang/_work/meta-clang/meta-clang/yoe/sources/poky/meta/lib/oeqa/runtime/cases/parselogs.py", line 185, in test_parselogs
    self.assertEqual(errcount, 0, msg=self.msg)
AssertionError: 1 != 0 : Log: /home/khem/actions-runner-meta-clang/_work/meta-clang/meta-clang/yoe/build/tmp/work/qemuarm64-yoe-linux/yoe-sdk-image/1.0/target_logs/postinstall.log
-----------------------
Central error: * opkg_cmd_exec: Command failed to capture privilege lock: Resource temporarily unavailable.
***********************
 * opkg_lock: Could not lock /run/opkg.lock: Resource temporarily unavailable.
 * opkg_cmd_exec: Command failed to capture privilege lock: Resource temporarily unavailable.

sounds like the failure is unrelated to change (but there is a chance that the issue leading up to this would be visible from the bitbake logs).

thats is bitbake run ( actually runtime logs in qemuarm64 ), however, this failure is known, it happens when more than one qemu instance is running same test case, it happens rarely but happens. I have started another run and see if it goes through.

@kraj kraj merged commit ca26df0 into kraj:master Feb 15, 2024
1 check passed
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.

3 participants