Skip to content

Verify that locally-specified configs are actually getting set - #621

Open
saiarcot895 wants to merge 2 commits into
sonic-net:masterfrom
saiarcot895:verify-kconfigs
Open

Verify that locally-specified configs are actually getting set#621
saiarcot895 wants to merge 2 commits into
sonic-net:masterfrom
saiarcot895:verify-kconfigs

Conversation

@saiarcot895

Copy link
Copy Markdown
Contributor

After local configs are set and oldconfig is run, make sure that the configs that we are locally setting are actually staying set. Note that the current code isn't comprehensive, and won't necessarily check to see that something is unset.

After local configs are set and oldconfig is run, make sure that the
configs that we are locally setting are actually staying set. Note that
the current code isn't comprehensive, and won't necessarily check to see
that something is unset.

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
Copilot AI lite review requested due to automatic review settings September 4, 2026 05:43
@saiarcot895
saiarcot895 requested a review from a team as a code owner September 4, 2026 05:43
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@mssonicbld

Copy link
Copy Markdown

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new buildcheck uses print() instead of the provided out writer used by existing checks, which can bypass the caller’s expected output capture/formatting.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a Debian-kernel packaging build-time verification step to confirm that locally provided debian/config.local/* fragments are still reflected in the generated .config after configuration is processed, helping catch cases where oldconfig (or related steps) drop locally requested settings.

Changes:

  • Introduces a new CheckConfigsAreSet check in Debian’s debian/bin/buildcheck.py (via patch) to compare merged local config fragments against the final .config.
  • Hooks the new check into the setup phase checks alongside the existing secure-boot check.
  • Applies the new patch during the kernel source preparation step in the top-level Makefile.
File summaries
File Description
patches-debian/verify-local-configs-are-applied.patch Adds a new Debian buildcheck that compares merged debian/config.local/* config fragments against the resulting .config.
Makefile Applies the new Debian buildcheck patch during the kernel source preparation workflow.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread patches-debian/verify-local-configs-are-applied.patch Outdated
Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
@mssonicbld

Copy link
Copy Markdown

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

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.

3 participants