Fix Bazel incompatible issues. #648
Conversation
|
@laurentlb could you help take a look at this PR which resolve various Bazel incompatible issues? |
| if hasattr(f, "files"): # a jar file | ||
| files += list(f.files) | ||
| files += f.files.to_list() | ||
| return files |
There was a problem hiding this comment.
I think it's better to have java_files and java_files_with_data return depsets, because:
- most call sites convert the list to a depset
- you can avoid the
.to_list()above
There was a problem hiding this comment.
Thanks for the suggestion. I'll do do that in a separate PR.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: laurentlb, nlopezgi, xingao267 If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
New changes are detected. LGTM label has been removed. |
| @@ -229,8 +229,8 @@ def _assemble_image_digest(ctx, name, image, image_tarball, output_digest): | |||
|
|
|||
| ctx.actions.run( | |||
| outputs = [output_digest], | |||
| inputs = [image["config"]] + blobsums + blobs + | |||
| ([image["legacy"]] if image.get("legacy") else []), | |||
| tools = [image["config"]] + blobsums + blobs + | |||
There was a problem hiding this comment.
@xingao267 what is the reason for this change? Just trying to understand. I thought tools is for "tools" that are used to perform the action and the inputs are for inputs i.e. files that are processed by the tools. Here all these things are inputs, not files, I think. Same in other files below.
There was a problem hiding this comment.
I agree. I think I made that change because Bazel complains that some of these files were executable, so I put them into tools according to https://docs.bazel.build/versions/master/skylark/backward-compatibility.html#disallow-tools-in-action-inputs. Let me try to change it back to inputs to see if the CI complains about it.
There was a problem hiding this comment.
I wonder why they are executable... From that page:
In the rare case that your action requires a tool as input, but does not actually run the tool and therefore does not need its runfiles, the safety check will fail even though the action would have succeeded. In this case, you can bypass the check by adding a (possibly empty) tools argument to your action.
There was a problem hiding this comment.
Yea. I was surprising too. I sent https://github.com/bazelbuild/rules_docker/pull/716/files to changed it back and I think it's fine. Maybe I mistakenly changed it.
There was a problem hiding this comment.
Thanks for pointing out!
Some refs: