bazelbuild / bazelbuild/bazel-skylib

File written with write_file does not end with new line

Open
#555 4 comments 1 reaction 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.