Skip to content

ci: add "Run libbpf-tools build with blazesym" to test_bcc_fedora#5462

Open
ekyooo wants to merge 1 commit intoiovisor:masterfrom
ekyooo:blazesym_ci
Open

ci: add "Run libbpf-tools build with blazesym" to test_bcc_fedora#5462
ekyooo wants to merge 1 commit intoiovisor:masterfrom
ekyooo:blazesym_ci

Conversation

@ekyooo
Copy link
Copy Markdown
Collaborator

@ekyooo ekyooo commented Feb 5, 2026

Add a new CI step to test building libbpf-tools with blazesym enabled. This ensures compatibility in environments where cargo is available.

Add a new CI step to test building libbpf-tools with blazesym enabled.
This ensures compatibility in environments where cargo is available.
Copilot AI review requested due to automatic review settings April 28, 2026 05:39
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an additional CI build step in the Fedora workflow to validate building libbpf-tools in a configuration intended to enable blazesym support (i.e., when cargo is available).

Changes:

  • Add a new GitHub Actions step to build libbpf-tools with blazesym intended to be enabled.
  • Install Rust toolchain in-container (via rustup) before building libbpf-tools in that step.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +213 to +215
echo \"USE_BLAZESYM=1 (cargo detected by Makefile)\" && \
echo \"=== Blazesym enabled, proceeding with build ===\" && \
make -j9'"
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

This step is intended to validate a blazesym-enabled build, but it doesn’t actually verify that blazesym was enabled; it only prints a message. Make the check fail-fast by forcing USE_BLAZESYM=1 on the make invocation (or by asserting the build output includes blazesym artifacts / flags), so the CI step reliably catches regressions.

Suggested change
echo \"USE_BLAZESYM=1 (cargo detected by Makefile)\" && \
echo \"=== Blazesym enabled, proceeding with build ===\" && \
make -j9'"
echo \"Forcing USE_BLAZESYM=1 for fail-fast validation\" && \
echo \"=== Blazesym enabled, proceeding with build ===\" && \
make -j9 USE_BLAZESYM=1'"

Copilot uses AI. Check for mistakes.
Comment on lines +208 to +214
'curl --proto \"=https\" --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && \
source \$HOME/.cargo/env && \
cd /bcc/libbpf-tools && make clean && \
echo \"=== Checking blazesym build configuration ===\" && \
cargo --version && \
echo \"USE_BLAZESYM=1 (cargo detected by Makefile)\" && \
echo \"=== Blazesym enabled, proceeding with build ===\" && \
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

The command string passed to the container has unnecessary escaping that changes its meaning: source \$HOME/.cargo/env prevents $HOME expansion inside the container (it will try to source a literal file named $HOME/.cargo/env), and \"...\" around curl/echo arguments will be passed as literal quotes. This will likely break cargo --version (PATH not updated) and can make the curl options invalid. Remove the backslashes and quote in a way that expands $HOME inside the container (e.g., source "$HOME/.cargo/env").

Suggested change
'curl --proto \"=https\" --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && \
source \$HOME/.cargo/env && \
cd /bcc/libbpf-tools && make clean && \
echo \"=== Checking blazesym build configuration ===\" && \
cargo --version && \
echo \"USE_BLAZESYM=1 (cargo detected by Makefile)\" && \
echo \"=== Blazesym enabled, proceeding with build ===\" && \
'curl --proto "=https" --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && \
source "$HOME/.cargo/env" && \
cd /bcc/libbpf-tools && make clean && \
echo "=== Checking blazesym build configuration ===" && \
cargo --version && \
echo "USE_BLAZESYM=1 (cargo detected by Makefile)" && \
echo "=== Blazesym enabled, proceeding with build ===" && \

Copilot uses AI. Check for mistakes.
Comment on lines +208 to +211
'curl --proto \"=https\" --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && \
source \$HOME/.cargo/env && \
cd /bcc/libbpf-tools && make clean && \
echo \"=== Checking blazesym build configuration ===\" && \
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

This step installs Rust via curl https://sh.rustup.rs | sh even though the Fedora CI image already installs rust and cargo (see docker/build/Dockerfile.fedora:42-45). This adds extra network dependency and CI time, and may introduce toolchain drift vs the image. Consider removing the rustup install and relying on the image’s cargo, failing early if cargo is missing.

Suggested change
'curl --proto \"=https\" --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && \
source \$HOME/.cargo/env && \
cd /bcc/libbpf-tools && make clean && \
echo \"=== Checking blazesym build configuration ===\" && \
'cd /bcc/libbpf-tools && make clean && \
echo \"=== Checking blazesym build configuration ===\" && \
command -v cargo >/dev/null 2>&1 || { echo \"cargo is required but not installed in bcc-docker\"; exit 1; } && \

Copilot uses AI. Check for mistakes.
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