copy_file: Do not add non-executables to default_runfiles #326
Conversation
Works around bazelbuild/bazel-skylib#326 by not using copy_file for utils.h.
Works around bazelbuild/bazel-skylib#326 by not using copy_file for utils.h.
|
@tetromino I have learned more about runfiles in the meantime and now believe that this change is not net positive: The only reason that setting runfiles is necessary at all in this case is the legacy behavior of native rules caused by bazelbuild/bazel#15043. I now think that it would make more sense to wait for the fix to land and at some point drop the |
|
I'm neutral on this - but note that stable Bazel users won't see the fix to bazelbuild/bazel#15052 until Bazel 7. |
copy_file and write_file currently include the created file in their runfiles even if it is not executable, which makes every rule depending on it have the file as a runfile (e.g. a `cc_library` depending on a copied header file via the hdrs attribute). In an ideal world, according to https://docs.bazel.build/versions/main/skylark/rules.html#runfiles-features-to-avoid, copy_file would not need to specify any runfiles in the `DefaultInfo` it returns. However, due to the existence of rules with legacy behavior, this would break compatibility (actually, already with `sh_test` in skylib's unit tests), see bazelbuild/bazel#15043. As a compromise that preserves compatibility with legacy rules but nevertheless cleans up the runfiles tree of depending rules, use the `data_runfiles` attribute of `DefaultInfo` if the copied file is not executable.
fmeum
force-pushed
the
remove-redundant-runfiles
branch
from
4070282
to
76f1099
Compare
|
@tetromino Good point. I made the change also for |
copy_file currently includes the copied file in its runfiles even if it is not executable, which makes every rule depending on it have the file as a runfile (e.g. a
cc_librarydepending on a copied header file via the hdrs attribute).In an ideal world, according to https://docs.bazel.build/versions/main/skylark/rules.html#runfiles-features-to-avoid,
copy_filewould not need to specify any runfiles in theDefaultInfoit returns - specifyingfilesshould suffice. However, due to the existence of rules with legacy behavior, this would break compatibility (actually, already withsh_testin skylib's unit tests).As a compromise that preserves compatibility with legacy rules but nevertheless cleans up the runfiles tree of depending rules, use the
data_runfilesattribute ofDefaultInfoif the copied file is not executable.(Note: I noticed the problem with legacy rules only after submitting #325, so please ignore that one).