Skip to content

Follow up to the cgroup PR #40519 - #41073

Merged
Ben Hillis (benhillis) merged 5 commits into
masterfrom
user/chemwolf6922/cgroup-change-followup
Jul 24, 2026
Merged

Ben Hillis (benhillis) merged 5 commits into
masterfrom
user/chemwolf6922/cgroup-change-followup

Conversation

@chemwolf6922

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

This PR serves as a follow up to comments in #40519

  1. Remove the redundant string encoding conversion in std::format.
  2. Add user warning when cgroup v1 is enabled and point the user to the correct .wslconfig setting.

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Copilot AI review requested due to automatic review settings July 14, 2026 05:25
@chemwolf6922
Feng Wang (chemwolf6922) requested a review from a team as a code owner July 14, 2026 05:25

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.

Pull request overview

This PR follows up on feedback from #40519 by simplifying a test helper’s std::format usage around wide-string inputs and by adding a user-facing warning when cgroup v1 is enabled in a configuration that indicates per-distribution cgroup isolation.

Changes:

  • Remove a redundant wide→narrow conversion in LxssWriteWslDistroConfig() by relying on the repo’s std::formatter<const wchar_t*, char> specialization.
  • Emit a user warning when cgroup v1 is selected while a per-distro cgroup path is present (indicating isolation is in effect).
  • Add the corresponding en-US localized string for the new warning message.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
test/windows/Common.cpp Simplifies path formatting by formatting LPCWSTR directly into a narrow std::format string.
src/linux/init/config.cpp Adds an EMIT_USER_WARNING when cgroup v1 is requested alongside per-distro isolation context.
localization/strings/en-US/Resources.resw Adds a new localized resource string for the cgroup v1 incompatibility warning.

Copilot AI review requested due to automatic review settings July 14, 2026 05:39

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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Copilot AI review requested due to automatic review settings July 14, 2026 05:45

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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings July 14, 2026 07:59

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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@benhillis
Ben Hillis (benhillis) merged commit 5e4f15d into master Jul 24, 2026
12 checks passed
@benhillis
Ben Hillis (benhillis) deleted the user/chemwolf6922/cgroup-change-followup branch July 24, 2026 23:37
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.

4 participants