-
Notifications
You must be signed in to change notification settings - Fork 442
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
Improve handling of compile_data with mixed sources #3176
Improve handling of compile_data with mixed sources #3176
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was looking at generated sources recently, this change makes sense to me.
Needs an owner, @krasimirgg we were talking about this very issue a couple of days ago :-)
|
||
# Optionally join compile data | ||
if crate.compile_data: | ||
compile_data = depset(ctx.files.compile_data, transitive = [crate.compile_data]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to worry about compile_data being None?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
afaict this shouldn't be possible, attr.label_list
defaults to []
, and we set compile_data
explicitly to a depset based on that in all codepaths, but I can add this check back if you think it's better to be defensive. It was a bit confusing to me, which is why I removed it!
03dacc4
to
7c80de4
Compare
Looks good! Please re-format the file that the Buildifier workflow is complaining about. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reformat according to https://buildkite.com/bazel/rules-rust-rustlang/builds/13739#01945f02-1578-4657-8c29-aeaef49853d9/127-128
This fixes issue where generated compile_data didn't work in some cases.
46726d1
to
e6f947e
Compare
Sorry, missed buildifier and one test affected by the last commit, pushed an updated patch now 👍 |
I'm not very well versed in starlark nor the rules_rust codebase, so feel free to ignore this and address the issue in a more fitting way, but this fixes #3171 and a related issue for me.