bazelbuild / bazelbuild/bazel-skylib
File written with write_file does not end with new line
- Dominant language
- Starlark
- Stars
- 444
- Forks
- 202
- PR merge metrics
- No merged PRs in 30d
Description
https://github.com/bazelbuild/bazel-skylib/blob/505e1bc3aaae8375f857637f78bf6b3dd953862a/rules/private/write_file_private.bzl#L32
write_file uses a simple `newline.join(ctx.attr.content)`, which means the last line does not end with a new line character.
POSIX requires non-empty text files to end with new line characters:
https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/V1_chap03.html
(See 3.185 Line and 3.387 Text File)
So the files written with write_file (which are usually text files, because `content` is usually a text string list) are usually non-POSIX compliant, unless the user explicitly add an empty token at the end. For example, we do [this](https://cs.android.com/android/kernel/superproject/+/common-android-mainline:common/BUILD.bazel;l=120-126;drc=0ea044ea4c7d34b22e6cbb8bedd10c87046336b8):
```
write_file(
name = "gki_x86_64_protected_modules",
out = "gki/x86_64/protected_modules",
content = get_gki_protected_modules_list("x86_64") + [
"", # Ensure new line at the end.
],
)
```
But it is easy for developers to forget to do this.
Not being a POSIX file means that other tools may misbehave, e.g. if I `cat` two files together and the first file doesn't end with the new line, then in the middle, an unexpected line (last line of the first file + first line of the second file) is formed, causing other downstream issues (e.g. a symbol is missing).
I suggest to add a new line to
https://github.com/bazelbuild/bazel-skylib/blob/505e1bc3aaae8375f857637f78bf6b3dd953862a/rules/private/write_file_private.bzl#L32
if content is non-empty.
```
content = (newline.join(ctx.attr.content) + newline) if ctx.attr.content else "",
```
But this is a backwards-incompatible change, so I'd rather file an issue to initiate a discussion first. What's the best way to carry this forward, so hacks like the above don't go viral?
Contributor guide
Research direction
Start with rules/private/write_file_private.bzl at the linked line and inspect how write_file assembles ctx.attr.content. Confirm the expected behavior for non-empty and empty content, then review the existing tests or test entry points for write_file if present. Done means the agreed behavior is documented by tests, including the backward-compatibility decision.
Written by the indexing model from the issue text.
Assessment
- Domain
- build-system
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100