Skip to content

Conversation

@cgwalters
Copy link
Collaborator

The rhsm feature was not being propagated from the CLI crate to the lib crate, causing bootc internals publish-rhsm-facts to never be compiled in even when building with CARGO_FEATURES=rhsm.

I think this was broken when I refactored the build recently.

Change things so we build the manpages before the production binary, ensuring the production binary always ends up with the right feature flags.

Fixes: https://issues.redhat.com/browse/RHEL-130799
Assisted-by: Claude Code (Sonnet 4.5)

The rhsm feature was not being propagated from the CLI crate to the
lib crate, causing `bootc internals publish-rhsm-facts` to never be
compiled in even when building with CARGO_FEATURES=rhsm.

I think this was broken when I refactored the build recently.

Change things so we build the manpages before the production
binary, ensuring the production binary always ends up with
the right feature flags.

Fixes: https://issues.redhat.com/browse/RHEL-130799
Assisted-by: Claude Code (Sonnet 4.5)
Signed-off-by: Colin Walters <walters@verbum.org>
@bootc-bot bootc-bot bot requested a review from jmarrero November 25, 2025 17:59
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request fixes an issue where the rhsm feature was not correctly enabled in the final binary. This was caused by the manpage generation task rebuilding the release binary without the necessary features. The fix involves changing the build order in the Makefile and bootc.spec to generate manpages before building the final binaries. This prevents the final artifact from being overwritten. A new test is also added to verify that the rhsm-gated command is present in the final binary.

The changes are logical and address the main bug. However, I've noted that while the binary is now correct, the manpages will still be generated without the rhsm-gated commands. I've added a suggestion to the Makefile to show how the features could be propagated to the manpage generation process, which would provide a more complete fix.

.PHONY: manpages
manpages:
cargo run --package xtask -- manpages
cargo run --release --package xtask -- manpages
Copy link
Contributor

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

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.

@cgwalters cgwalters enabled auto-merge (rebase) November 25, 2025 19:47
@cgwalters cgwalters merged commit e747216 into bootc-dev:main Nov 25, 2025
36 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants