lowRISC / lowRISC/opentitan

[bazel] Unclear interaction between `opentitan_test` features and CLI

Open
#27,568 2 comments 1 reaction 0 assignees View on GitHub
Dominant language
SystemVerilog
Stars
3.6k
Forks
1.1k
Avg merge
2d 22h
Merged PRs (30d)
141

Description

### Description

In #25840, we added the possibility of opentitan binaries/tests to set bazel features transitively on dependencies. This was tweaked in #26068 but the general concept remained. However this was possibly merged without thinking too much about the interactions of this attribute with other aspects of the build system.

Bazel supports enabling/disabling features on the command-line by using `--features=`, e.g. `--features=lto` or `--features=-lto`. Setting things on the command-line overrides the toolchain default, which is the expected behaviour. However, it is unclear what the behaviour should be for such a target:
```python
opentitan_binary(
name = "mask_rom",
# ...
transitive_features = [
"use_lld",
"lto",
"minsize",
],
)
```
Indeed, the mask ROM does not even compile without LTO (too big) and more generally I think it is reasonable to think that if feature is set on an `opentitan_binary/test` then the command-line should not be able to override it? Or maybe we want to be more fined-grained and allow some features to be overriden and some not?

There is a more general problem with the impementation: the `transitive_features` internally translates to a transition on `//command_line_option:features` meaning that dependencies of the target (e.g. `mask_rom`) cannot differentiate between a transitive feature coming from the target, and a feature set on the command-line.

The goal of this issue is to rethink carefully and try to agree on a meaningful semantic for thos attributs.

Ping @luismarques @a-will @jwnrt @cfrantz

Contributor guide

Open the contributing guide

Research direction

Start by reading the changes and discussion in #25840 and #26068, then inspect the opentitan_binary/opentitan_test handling of transitive_features. Reproduce the interaction with Bazel --features=lto and --features=-lto using the mask_rom example. Done means the intended precedence and semantics for command-line and transitive features are agreed and documented.

Written by the indexing model from the issue text.

Assessment

Domain
build-system
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.