bazel-contrib / bazel-contrib/rules_foreign_cc

`replace_symlink`'s `cp -a` makes an unpreservable file mode a hard build failure

Open
#1,579 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Starlark
Stars
737
Forks
270
PR merge metrics
No merged PRs in 30d

Description

Every `cmake()` / `configure_make()` / `meson()` target that declares a shared library emits this
at the end of its generated build script
([`linux_commands.bzl:272`](https://github.com/bazel-contrib/rules_foreign_cc/blob/f68b351c4691e747f889dc5e4c2cac3cd3b66ea2/foreign_cc/private/framework/toolchains/linux_commands.bzl#L272)):

```sh
if [[ -L "bazel-out/.../lib/libFoo.so" ]]; then
target="$(readlink -f "bazel-out/.../lib/libFoo.so")"
rm "bazel-out/.../lib/libFoo.so" && cp -a "${target}" "bazel-out/.../lib/libFoo.so"
fi
```

`cp -a` is `-dR --preserve=all`. If the destination cannot take the `chmod` that
`--preserve=mode` performs, `cp` copies the data correctly and *then* exits 1:

```
cp: preserving permissions for 'bazel-out/.../lib/libTracyClient.so': Operation not permitted
```

The wrapper runs under `set -euo pipefail`, so the action fails with a correct artifact already
on disk. We hit this on remote execution; it is not remote-specific in principle.

### Root cause

`preserving permissions for X` is gnulib's `copy_acl()` diagnostic — the **mode/ACL** copy onto
the destination. (A failed ownership copy prints `failed to preserve ownership for X`, and
coreutils deliberately tolerates that for an unprivileged uid via `chown_failure_ok()`. Ownership
is not what breaks the build; it is worth stating because it is the intuitive-but-wrong reading.)

What promotes the diagnostic from a warning to a failure is coreutils' `require_preserve`, which
both `-p` and `-a` set. Bisecting the attributes with GNU coreutils 8.32, copying onto a
destination that rejects `chmod`:

```
rc=1 cp -a :: cp: preserving permissions for '…': Operation not permitted
rc=1 cp -pR :: cp: preserving permissions for '…': Operation not permitted
rc=1 cp -p :: cp: preserving permissions for '…': Operation not permitted
rc=1 cp -R --preserve=mode :: cp: preserving permissions for '…': Operation not permitted
rc=1 cp -R --preserve=ownership :: cp: preserving permissions for '…': Operation not permitted
rc=0 cp -R --preserve=timestamps ::
rc=0 cp -R ::
```

Plain `cp` reports the same underlying condition without a message and exits 0. So the rules ask
for a guarantee they do not need — mode and ownership propagation into `bazel-out` are not part
of Bazel's output-tree semantics — and turn its absence into a build failure.

The same pattern appears in
[`symlink_to_dir`](https://github.com/bazel-contrib/rules_foreign_cc/blob/f68b351c4691e747f889dc5e4c2cac3cd3b66ea2/foreign_cc/private/framework/toolchains/linux_commands.bzl#L167)
(`cp -pR "$source" "$target"`), and in both functions in `macos_commands.bzl`,
`freebsd_commands.bzl` and `windows_commands.bzl`.

### Why this surfaces under remote execution

Not because of the uid. On an ordinary filesystem, an unprivileged uid copying a root-owned
source to a fresh destination succeeds silently: the `chown` EPERM is tolerated, and the `chmod`
succeeds because `cp` owns the file it just created. We checked this specifically, because
"non-root worker" is the natural first hypothesis and it is wrong.

It surfaces when the action's output tree lives on a filesystem that will not accept `chmod` —
which is what remote executors that materialise the action directory on a virtual or network
filesystem do, rather than on a plain local one. I will follow up in a comment with the specifics
of the worker we hit this on.

### Reproduction

`replace_symlink` is not a rare path. Any CMake project that sets `SOVERSION` — Tracy, zlib,
OpenSSL, … — installs `libFoo.so` and `libFoo.so.0` as symlinks directly into `$INSTALLDIR`,
which *is* the declared-output directory, so the `[[ -L … ]]` guard is true:

```cmake
add_library(tracylike SHARED tracylike.c)
set_target_properties(tracylike PROPERTIES VERSION 0.9.1 SOVERSION 0)
install(TARGETS tracylike LIBRARY DESTINATION lib)
```
```python
cmake(name = "tracylike", lib_source = ":srcs", out_shared_libs = ["libtracylike.so"])
```

`CMake.log` shows all three install entries, and `bazel-bin/.../build_script.sh` ends with the
`cp -a` block above.

Running that block **verbatim** under `set -euo pipefail`, once on an ordinary filesystem and
once on one that rejects `chmod`:

```
########## A. ordinary filesystem ##########
upstream exit=0
result: regular-file mode=755 contents-match=yes

########## B. output tree on a filesystem that refuses chmod ##########
upstream exit=1 cp: preserving permissions for '…/libtracylike.so': Operation not permitted
```

### Proposed fix

Copy the data and restore the timestamp separately. This is already how the rest of this repo
does it: all four `copy_dir_contents_to_dir` implementations use `cp -L -R` plus
`find … -exec touch -r`, and #583 ("Use touch not cp -p to preserve timestamps") made exactly
this change there. These two call sites were missed.

`replace_symlink`, in all four toolchain files:

```diff
- rm "{file}" && cp -a "${{target}}" "{file}"
+ rm "{file}" && cp -R "${{target}}" "{file}" && touch -r "${{target}}" "{file}"
```

`symlink_to_dir`, in `linux` / `macos` / `freebsd`:

```diff
- cp -pR "$source" "$target"
+ cp -R "$source" "$target"
```

Rationale for the details:

- **`-a` also implies `-d`.** At the `replace_symlink` call site the symlink has just been
resolved with `readlink -f` (`realpath` on macOS) and removed, so the source is a real file or
directory; `-R` covers the directory case and the no-dereference behaviour is not relied on.
- **No `touch -r` in `symlink_to_dir`.** That branch is guarded by
`[[ -L "$source" && ! -d "$source" ]]`, reachable only for dangling symlinks and symlinks to
non-regular files, since a symlink to a real file already matched `-f` above. `cp -R` recreates
the link exactly as `cp -pR` did (checked for both a dangling link and a link to `/dev/null`),
and `touch -r` would fail on a dangling link.
- **Why not `cp -R --preserve=timestamps`.** It fixes the observed error, but `--preserve=` still
sets `require_preserve`, so it remains fatal in the adjacent case where the destination
pre-exists owned by another uid (`cp: preserving times for '…': Operation not permitted`), and
BSD `cp` on macOS/FreeBSD has no `--preserve=` at all. `cp -R` + `touch -r` is uniform across
all four toolchains and matches existing style.

This is a strict narrowing of what is preserved, so it cannot break a build that previously
succeeded. The only behaviour lost is mode/ownership/ACL propagation into `bazel-out`, which
Bazel does not guarantee.

Note that `test/expected/inner_fun_text{,_macos,_freebsd}.txt` are golden files containing
`cp -pR "$source" "$target"` and must be regenerated alongside the `symlink_to_dir` change.

### Verification

I have this change on a branch and can open the PR:

- `bazel test //test/...` — 94/94 pass, including the golden diff and the shellcheck tests.
- `examples/`: `cmake_hello_world_lib`, `cmake_with_data`, `make_simple`, `meson_simple`,
`ninja_simple` — 16/16 pass.
- The `SOVERSION` repro target above rebuilds; the produced `libtracylike.so` is byte-identical
to the pre-patch artifact with the same mode (`r-xr-xr-x`).
- Verbatim generated snippet on a chmod-rejecting filesystem: `exit=1` before, `exit=0` after,
contents matching.

I could not find a way to assert the failure in CI without a second uid or a special filesystem,
so the change is covered by the existing golden and shellcheck tests rather than a new regression
test. Happy to add one if you have a preferred mechanism.

### Notes

- 0.15.1 is the newest version in both the BCR and this repo's tags, so there is no release to
upgrade into — downstream patching is currently the only option.
- Only #1469 and #1549 have touched `linux_commands.bzl` since 0.15.1. Neither changes these
lines, but #1469 rewrote `symlink_to_dir`'s variable handling, so a downstream patch's hunk
context has to be rebased on the next release — part of why we would rather land this upstream.
- Related but distinct: #306 (Bazel creates output dirs 0555, breaking `sed -i`) and #119
(`cmake_external` under remote execution, macOS→Linux).

Contributor guide

Open the contributing guide

Research direction

Start in linux_commands.bzl at replace_symlink and symlink_to_dir, then compare the corresponding functions in macos_commands.bzl, freebsd_commands.bzl, and windows_commands.bzl. Regenerate test/expected/inner_fun_text{,_macos,_freebsd}.txt as needed and run bazel test //test/... plus the listed examples; done means the golden, shellcheck, and example tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
cmake, shell
Domain
build-system
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.