make sysroot optional #1121

daniel-noland · 2025-12-08T19:38:43Z

Draft commit exploring the removal of the sysroot as a mandatory build component

Copilot

Pull request overview

This PR explores making the sysroot a non-mandatory build component by introducing conditional compilation and restructuring dependencies. The changes enable building the project without DPDK sysroot dependencies for development workflows while maintaining sterile builds that use the sysroot for production.

Introduces optional sysroot support through feature flags in several modules (init, dataplane, cli)
Completely removes sysroot dependencies from modules that don't need them (sysfs, hardware)
Adds a new CI workflow path for non-sterile builds using upstream Rust toolchain
Refactors dpdk-sysroot-helper to provide a cleaner use_sysroot() helper function

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
sysfs/build.rs	Completely removes build script and sysroot linking configuration
sysfs/Cargo.toml	Removes dpdk-sysroot-helper build dependency
hardware/build.rs	Completely removes build script and sysroot linking configuration
hardware/Cargo.toml	Removes dpdk-sysroot-helper build dependency
init/build.rs	Replaces direct sysroot calls with conditional `use_sysroot()` helper guarded by sysroot feature
init/Cargo.toml	Makes dpdk-sysroot-helper optional and adds sysroot feature flag
dataplane/build.rs	Replaces direct sysroot calls with conditional `use_sysroot()` helper guarded by sysroot feature
dataplane/Cargo.toml	Makes dpdk-sysroot-helper optional and adds sysroot and dpdk feature flags (dpdk requires sysroot)
dataplane/src/drivers/dpdk.rs	Gates DPDK driver code behind dpdk feature flag
cli/build.rs	Replaces direct sysroot calls with conditional `use_sysroot()` helper guarded by sysroot feature
cli/Cargo.toml	Makes dpdk-sysroot-helper optional, adds sysroot feature, and explicitly enables strum derive feature
dpdk/build.rs	Refactors to use new `use_sysroot()` helper function
dpdk-sysroot-helper/src/lib.rs	Adds new `use_sysroot()` helper function that encapsulates sysroot configuration; removes unused helper functions
Cargo.toml	Adds default-members list to workspace, excluding DPDK-dependent and sysroot-requiring modules
Cargo.lock	Updates dependency versions and reflects removal of dpdk-sysroot-helper from hardware and sysfs
.github/workflows/dev.yml	Adds support for non-sterile build matrix, installs upstream Rust toolchain for non-sterile builds, adds conditional logic for build environment setup and test execution
.cargo/config.toml	Removes hardcoded PATH, PKG_CONFIG_PATH, and rustc overrides to allow builds with standard Rust toolchain

dpdk/build.rs

Copilot · 2025-12-09T16:07:29Z

.github/workflows/dev.yml

        run: |
-          just debug_justfile="${{matrix.debug_justfile}}" profile=${{matrix.profile.name}} \
-            ${{matrix.profile.sterile}} cargo clippy --all-targets --all-features -- -D warnings
+          if [ -z "${{matrix.profile.sterile}}" ]; then


confidence: 9 tags: [logic]

The shell condition if [ -z "${{matrix.profile.sterile}}" ] is missing proper quoting around the variable. While the intent seems correct (checking if the variable is empty for non-sterile builds), the variable expansion itself should be quoted for safety:

if [ -z "${{matrix.profile.sterile}}" ]; then

However, this approach is inconsistent with the pattern used in GitHub Actions conditionals elsewhere (if: ${{ matrix.profile.sterile != 'sterile' }}). For consistency, consider using the same pattern as lines 158 and 242:

if [ "${{matrix.profile.sterile}}" != "sterile" ]; then

This would make the logic uniform across all shell-based conditionals in the workflow.

Copilot · 2025-12-09T16:07:29Z

.github/workflows/dev.yml

-            profile=${{matrix.profile.name}} \
-            target=x86_64-unknown-linux-gnu \
-            ${{matrix.profile.sterile}} cargo doc --no-deps
+          if [ -z "${{matrix.profile.sterile}}" ]; then


confidence: 9 tags: [logic]

Similar to line 368, the shell condition if [ -z "${{matrix.profile.sterile}}" ] should be made consistent with other conditionals in the workflow. Consider using:

if [ "${{matrix.profile.sterile}}" != "sterile" ]; then

This would make the logic uniform across all shell-based conditionals in the workflow.

Suggested change

if [ -z "${{matrix.profile.sterile}}" ]; then

if [ "${{matrix.profile.sterile}}" != "sterile" ]; then

Copilot · 2025-12-09T16:07:29Z

.github/workflows/dev.yml

-            registry="${{env.CACHE_REGISTRY}}" \
-            refresh-compile-env
-          just --yes debug_justfile="${{matrix.debug_justfile}}" fake-nix
+          if [ ${{matrix.profile.sterile}} = "sterile" ]; then


confidence: 9 tags: [logic]

The shell condition if [ ${{matrix.profile.sterile}} = "sterile" ] is missing quotes around the variable expansion. When matrix.profile.sterile is empty (for non-sterile builds), this will expand to if [ = "sterile" ], which is a syntax error.

The condition should be:

if [ "${{matrix.profile.sterile}}" = "sterile" ]; then

Note the quotes around ${{matrix.profile.sterile}} to ensure proper handling of empty values.

Suggested change

if [ ${{matrix.profile.sterile}} = "sterile" ]; then

if [ "${{matrix.profile.sterile}}" = "sterile" ]; then

Copilot · 2025-12-09T16:07:30Z

.github/workflows/dev.yml

                --final-status-level=skip \
                --message-format=libtest-json-plus > ./results.json
-          else
+          elif [ ${{ matrix.profile.sterile }} = "sterile" ]; then


confidence: 9 tags: [logic]

Similar to line 158, the shell condition if [ ${{ matrix.profile.sterile }} = "sterile" ] is missing quotes around the variable expansion. When matrix.profile.sterile is empty, this becomes if [ = "sterile" ], which is a syntax error.

Should be:

elif [ "${{ matrix.profile.sterile }}" = "sterile" ]; then

Suggested change

elif [ ${{ matrix.profile.sterile }} = "sterile" ]; then

elif [ "${{ matrix.profile.sterile }}" = "sterile" ]; then

Copilot · 2025-12-09T16:07:30Z

cli/Cargo.toml

 rustyline = { workspace = true , features = ["with-fuzzy", "custom-bindings"] }
 serde = {workspace = true, features = ["derive", "serde_derive"]}
-strum = { workspace = true }
+strum = { workspace = true, features = ["derive"] }


[nitpick] ```yaml
confidence: 7
tags: [other]

The addition of `features = ["derive"]` to the `strum` dependency appears unrelated to the stated purpose of this PR (making sysroot optional). While this change makes the dependency on strum's derive feature explicit (which is used in `cli/src/cliproto.rs` for `AsRefStr`, `EnumIter`, and `EnumString`), it should ideally be in a separate commit or the PR description should mention this fix. If this change is necessary to make the non-sterile build work without the sysroot, that relationship should be documented in the commit message or PR description.

This is necessary in order to make the sysroot optional. Some packages simply can't be enabled by default while preserving any useful ergonomics. Signed-off-by: Daniel Noland <[email protected]>

Removing this build.rs file facilitates removing dpdk as a strict requirement for the build. The build.rs file for the hardware package is not strictly needed. Strictly speaking, the only packages which actually need this type of build.rs logic are those which produce an actual linked artifact (.so or elf exe or some such). Signed-off-by: Daniel Noland <[email protected]>

The sysfs package does not actually require any linkage against the sysroot, so this can be safely removed. Signed-off-by: Daniel Noland <[email protected]>

This function is unused and has been for some time. Signed-off-by: Daniel Noland <[email protected]>

This is intended to combat build.rs code duplication across the project. Signed-off-by: Daniel Noland <[email protected]>

Signed-off-by: Daniel Noland <[email protected]>

The sysroot was only ever needed here to enforce consistency on the version of libc we link against. Given that the sysroot is now optional, this requirement is now also optional. Signed-off-by: Daniel Noland <[email protected]>

The sysroot is likely functionally necessary for most developers to build the init package (due to the dependency on the hardware crate), it is still possible that the developer will have their own version of the required libraries available. Such builds are entirely unsanitary and can never be used in production, but my be helpful during development. Signed-off-by: Daniel Noland <[email protected]>

Signed-off-by: Daniel Noland <[email protected]>

Now that the sysroot is optional for the dpdk package's dependencies, we can make the sysroot (and dpdk) optional for the whole dataplane. Signed-off-by: Daniel Noland <[email protected]>

This change is temporary and actually reflects substantial attrition in our build process. Specifically, this change exposes us to various forms of sterility violations for local development builds. That said, CI should still be sterile since the versions of rustc, clant, and all sysroot libs shipped in the sterile env are also the one and only valid choice to compile with / link against in those contexts. This step is also an essential proof of concept validation for the removal of the sysroot as a hard dependency. In the very near future we should restructure this project to use nix flakes to avoid the potential for the ever popular "works on my machine." We haven't had a lot of "works on my machine" in the project to date, and I would very much like to keep it that way. Signed-off-by: Daniel Noland <[email protected]>

Note: you can't use debug as a profile name without just Signed-off-by: Daniel Noland <[email protected]>

mvachhar

I really don't like the scope of breakage here in this commit. Is there any way to automatically use the compile-env if it is there and then fallback if it isn't?

mvachhar · 2025-12-09T17:10:35Z

.cargo/config.toml

@@ -1,12 +1,9 @@
 [env]
 COMPILE_ENV = { value = "compile-env", relative = true, force = false }
-PATH = { value = "compile-env/bin", relative = true, force = true }


This will now break all compiles on machines that don't have the tool chain installed locally unless they manually put compile-env on the path. Is there any way to avoid breaking this? Perhaps by just adding compile-env to path and using it if it is there?

This will certainly break my dev environment in ways I would be unhappy with.

daniel-noland self-assigned this Dec 8, 2025

daniel-noland added enhancement New feature or request dont-merge Do not merge this Pull Request dependencies/major a major version change ci:-upgrade Disable VLAB upgrade tests ci:-vlab Disable VLAB tests labels Dec 8, 2025

daniel-noland added this to the GW R3 milestone Dec 8, 2025

daniel-noland force-pushed the pr/daniel-noland/make-sysroot-optional branch 13 times, most recently from 4af284a to 8185541 Compare December 8, 2025 22:00

daniel-noland removed ci:-upgrade Disable VLAB upgrade tests ci:-vlab Disable VLAB tests labels Dec 8, 2025

daniel-noland force-pushed the pr/daniel-noland/make-sysroot-optional branch from 8185541 to a9394fe Compare December 8, 2025 22:00

daniel-noland removed the dont-merge Do not merge this Pull Request label Dec 8, 2025

daniel-noland force-pushed the pr/daniel-noland/make-sysroot-optional branch from a9394fe to 353bd10 Compare December 8, 2025 22:02

daniel-noland marked this pull request as ready for review December 8, 2025 22:22

daniel-noland requested a review from a team as a code owner December 8, 2025 22:22

daniel-noland requested review from Copilot and sergeymatov and removed request for a team December 8, 2025 22:22

Copilot started reviewing on behalf of daniel-noland December 9, 2025 15:59 View session

Copilot AI reviewed Dec 9, 2025

View reviewed changes

daniel-noland added 18 commits December 9, 2025 16:48

build: re-introduce default-members into workspace

2735ec0

This is necessary in order to make the sysroot optional. Some packages simply can't be enabled by default while preserving any useful ergonomics. Signed-off-by: Daniel Noland <[email protected]>

build(sysfs): remove unused dependency on sysroot

f3046b4

The sysfs package does not actually require any linkage against the sysroot, so this can be safely removed. Signed-off-by: Daniel Noland <[email protected]>

refactor(dpdk-sysroot-helper): drop unused get_project_root function

fdd021a

This function is unused and has been for some time. Signed-off-by: Daniel Noland <[email protected]>

refactor(dpdk-sysroot-helper): drop unused get_compile_env functon

4c6d91d

This function is unused and has been for some time. Signed-off-by: Daniel Noland <[email protected]>

feat(dpdk-sysroot-helper): intro use_sysroot function

56a313f

This is intended to combat build.rs code duplication across the project. Signed-off-by: Daniel Noland <[email protected]>

refactor(cli): invoke use_sysroot to de-duplicate code

dc7a114

Signed-off-by: Daniel Noland <[email protected]>

refactor(init): invoke use_sysroot to de-duplicate code

8dbe745

Signed-off-by: Daniel Noland <[email protected]>

refactor(dpdk): invoke use_sysroot to de-duplicate code

cc4ed20

Signed-off-by: Daniel Noland <[email protected]>

refactor(dataplane): invoke use_sysroot to de-duplicate code

84ae24f

Signed-off-by: Daniel Noland <[email protected]>

build(cli): make sysroot optional

ef7962b

The sysroot was only ever needed here to enforce consistency on the version of libc we link against. Given that the sysroot is now optional, this requirement is now also optional. Signed-off-by: Daniel Noland <[email protected]>

build: remove init from default-members list

e0bea6f

Signed-off-by: Daniel Noland <[email protected]>

build: remove dpdk and friends from default-members list

0f5f007

Signed-off-by: Daniel Noland <[email protected]>

build: remove hardware from default-members list

5516c51

Signed-off-by: Daniel Noland <[email protected]>

build(dataplane): make sysroot and dpdk optional for dataplane

f3af00d

Now that the sysroot is optional for the dpdk package's dependencies, we can make the sysroot (and dpdk) optional for the whole dataplane. Signed-off-by: Daniel Noland <[email protected]>

ci: only use sysroot on sterile builds

4621fc4

Note: you can't use debug as a profile name without just Signed-off-by: Daniel Noland <[email protected]>

daniel-noland force-pushed the pr/daniel-noland/make-sysroot-optional branch 2 times, most recently from 228e7a1 to 4621fc4 Compare December 9, 2025 16:54

mvachhar requested changes Dec 9, 2025

View reviewed changes

daniel-noland added the dont-merge Do not merge this Pull Request label Dec 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

make sysroot optional #1121

make sysroot optional #1121

Uh oh!

daniel-noland commented Dec 8, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Dec 9, 2025

Uh oh!

Copilot AI Dec 9, 2025

Uh oh!

Copilot AI Dec 9, 2025

Uh oh!

Copilot AI Dec 9, 2025

Uh oh!

Copilot AI Dec 9, 2025

Uh oh!

mvachhar left a comment

Uh oh!

mvachhar Dec 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if [ -z "${{matrix.profile.sterile}}" ]; then
	if [ "${{matrix.profile.sterile}}" != "sterile" ]; then

	if [ ${{matrix.profile.sterile}} = "sterile" ]; then
	if [ "${{matrix.profile.sterile}}" = "sterile" ]; then

	elif [ ${{ matrix.profile.sterile }} = "sterile" ]; then
	elif [ "${{ matrix.profile.sterile }}" = "sterile" ]; then

make sysroot optional #1121

Are you sure you want to change the base?

make sysroot optional #1121

Uh oh!

Conversation

daniel-noland commented Dec 8, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

mvachhar left a comment

Choose a reason for hiding this comment

Uh oh!

mvachhar Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants