google / google/copybara

Running buildozer with nested targets incorrectly shortens root package labels

Open
#324 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
3.8k
Forks
345
PR merge metrics
No merged PRs in 30d

Description

I've observed strange behavior when using Copybara with `buildozer.modify`. Specifically, certain labels are rendered invalid when using a target specification that includes `...`.

Given the following `BUILD` file:

```starlark
java_library(
deps = ["//foo/bar:baz"],
)
```

And the following transformation:

```starlark
buildozer.modify(
target = "...:*",
commands = [
buildozer.cmd("replace deps //foo/bar:baz //:baz"),
],
),
```

The result of this, counter intuitively, is the following:

```starlark
java_library(
deps = [":baz"],
)
```

The omission of the `//` in front of the replacement means the target is invalid.

I've done some digging already. The `buildozer` code runs [`ShortenLabel`](https://github.com/bazelbuild/buildtools/blob/d9ed52af26ee7e03973f776739d46fd79742dc36/edit/edit.go#L1229) on the label. This will try to create the canonical form the of label. It uses the `CmdEnvironment.Pkg` for this. When running `buildozer` manually, this will be the package of the `BUILD` file in question. Everything works correctly. When running with Copybara, this package seems to always be an empty string `""`. The result is that a comparison of the context package and the target package incorrectly returns `true` [here](https://github.com/bazelbuild/buildtools/blob/d9ed52af26ee7e03973f776739d46fd79742dc36/labels/labels.go#L57). The context package is empty and the target is root level, meaning its package is also empty. The code sees the packages match and returns a relative label. This relative label is incorrect.

The core of the issue seems to be that Copybara invocations of buildozer don't populate the `CmdEnvironment.Pkg` used for the list replacement (and subsequent shortening) [here](https://github.com/bazelbuild/buildtools/blob/d9ed52af26ee7e03973f776739d46fd79742dc36/edit/buildozer.go#L532).

There are multiple possible workarounds. One would be that Copybara passed `--shorten_labels=false` to buildozer. That would disable this label shortening. Alternatively, one could investigate why the package is not populated in the buildozer command environment with Copybara. As far as I can tell the behavior of buildozer itself is correct.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.