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

Work around missing "stringListValue" in certain cases #280

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

mrkkrp
Copy link
Contributor

@mrkkrp mrkkrp commented Aug 9, 2024

It seems that not all targets that explicitly specify the hdrs attribute also have the stringListValue present.

@mrkkrp mrkkrp requested a review from martis42 as a code owner August 9, 2024 12:41
@@ -47,7 +47,7 @@ def get_dependencies(bazel_query: BazelQuery, target: str) -> list[Dependency]:
for x in queried_targets:
if x["type"] == "RULE" and x["rule"]["ruleClass"].startswith("cc_"):
for attr in x["rule"]["attribute"]:
if attr["name"] == "hdrs" and attr["explicitlySpecified"]:
if attr["name"] == "hdrs" and attr["explicitlySpecified"] and "stringListValue" in attrs:
Copy link
Owner

Choose a reason for hiding this comment

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

@mrkkrp thanks for this contribution

  • I believe there is a typo. It should be .. in attr:
  • Can you provide an example where this fails? Your change makes either way sense as defensive programming. Still, I am curious as the only cc_ rules with the hdrs attribute are cc_library and cc_import. With those I can't reproduce this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I applied the fix. The issue seems to be triggered when you have a cc_library that does not have any headers of its own but nevertheless specifies hdrs = [] explicitly.

It seems that not all targets that explicitly specify the `hdrs` attribute
also have the `stringListValue` present.
@mrkkrp mrkkrp force-pushed the no-string-list-value branch from 413de2e to d554be9 Compare August 12, 2024 08:44
@martis42 martis42 merged commit c29d818 into martis42:main Aug 12, 2024
11 checks passed
@mrkkrp mrkkrp deleted the no-string-list-value branch August 13, 2024 14:40
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.

2 participants