Skip to content

cli: Fix rhsm feature propagation and manpage build order #1807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 25, 2025
Merged

cli: Fix rhsm feature propagation and manpage build order #1807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,19 @@ prefix ?= /usr
# We may in the future also want to include Fedora+derivatives as
# the code is really tiny.
# (Note we should also make installation of the units conditional on the rhsm feature)
CARGO_FEATURES ?= $(shell . /usr/lib/os-release; if echo "$$ID_LIKE" |grep -qF rhel; then echo rhsm; fi)
CARGO_FEATURES_DEFAULT ?= $(shell . /usr/lib/os-release; if echo "$$ID_LIKE" |grep -qF rhel; then echo rhsm; fi)
# You can set this to override all cargo features, including the defaults
CARGO_FEATURES ?= $(CARGO_FEATURES_DEFAULT)

all: bin manpages

bin:
cargo build --release --features "$(CARGO_FEATURES)"
# Build all binaries
.PHONY: bin
bin: manpages
cargo build --release --features "$(CARGO_FEATURES)" --bins

# Note this cargo build is run without features (such as rhsm)
.PHONY: manpages
manpages:
cargo run --package xtask -- manpages
cargo run --release --package xtask -- manpages
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While this change correctly builds xtask in release mode, the manpages target is still run without knowledge of CARGO_FEATURES. This means the generated manpages will not include documentation for commands behind feature flags, such as publish-rhsm-facts under the rhsm feature.

To ensure the manpages are complete, you could pass CARGO_FEATURES as an environment variable to the xtask process. Note that the xtask crate would also need to be updated to read this environment variable and use it when generating the CLI data.

	CARGO_FEATURES="$(CARGO_FEATURES)" cargo run --release --package xtask -- manpages

Copy link
Collaborator Author

@cgwalters cgwalters Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means the generated manpages will not include documentation for commands behind feature flags, such as publish-rhsm-facts under the rhsm feature.

Yes good observation, but that is an internal only feature so it doesn't matter.


STORAGE_RELATIVE_PATH ?= $(shell realpath -m -s --relative-to="$(prefix)/lib/bootc/storage" /sysroot/ostree/bootc/storage)
install:
Expand Down
34 changes: 9 additions & 25 deletions contrib/packaging/bootc.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,36 +121,20 @@ cd %{name}-%{version}-build

%build
export SYSTEM_REINSTALL_BOOTC_INSTALL_PODMAN_PATH=%{system_reinstall_bootc_install_podman_path}
%if 0%{?container_build}
# Container build: use cargo directly with cached dependencies
export CARGO_HOME=/var/roothome/.cargo
cargo build -j%{_smp_build_ncpus} --release %{?with_rhsm:--features rhsm} \
--bin=bootc --bin=system-reinstall-bootc --bin=bootc-initramfs-setup \
%{?with_tests:--bin tests-integration}
# Build this first to avoid feature skew
make manpages
%else
# Build the main bootc binary
%if %new_cargo_macros
%cargo_build %{?with_rhsm:-f rhsm}
%else
%cargo_build %{?with_rhsm:--features rhsm}
%endif

# Build the system reinstallation CLI binary
%global cargo_args -p system-reinstall-bootc
# Build all binaries
%if 0%{?container_build}
# Container build: use cargo directly with cached dependencies to avoid RPM macro overhead
cargo build -j%{_smp_build_ncpus} --release %{?with_rhsm:--features rhsm} --bins
%else
# Non-container build: use RPM macros for proper dependency tracking
%if %new_cargo_macros
# In cargo-rpm-macros, the cargo_build macro does flag processing,
# so we need to pass '--' to signify that cargo_args is not part
# of the macro args
%cargo_build -- %cargo_args
%cargo_build %{?with_rhsm:-f rhsm} -- --bins
%else
# Older macros from rust-toolset do *not* do flag processing, so
# '--' would be passed through to cargo directly, which is not
# what we want.
%cargo_build %cargo_args
%cargo_build %{?with_rhsm:--features rhsm} -- --bins
%endif

make manpages
%endif

%if ! 0%{?container_build}
Expand Down
13 changes: 13 additions & 0 deletions tmt/tests/booted/readonly/021-test-rhsm-facts.nu
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use std assert
use tap.nu

tap begin "rhsm facts"

# Verify we have this feature
if ("/etc/rhsm" | path exists) {
bootc internals publish-rhsm-facts --help
let status = systemctl show -P ActiveState bootc-publish-rhsm-facts.service
assert equal $status "inactive"
}

tap ok