Skip to content

Allow DSC parsing through OS architecture translation layers #24852

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 24, 2025

Conversation

bdeb1337
Copy link
Contributor

PR Summary

This PR replaces the check that restricts parsing DSC configurations when the host OS is of the type ARM.
Instead of checking the host OS, I want to introduce the possibility of checking on process type instead. This way we can allow this for host OS'es that have translation layers to run the x86_64 compiled version of pwsh.

This addresses my issue in #24848 and follows the note/tip of SteveL-MSFT #18781 (comment).

PR Context

I develop DSC(v1) configurations on an ARM mac for use in a windows server environment, and I am currently prohibited of being assisted by the pwsh tooling and interpreter, because the configuration parameter is not "allowed" on ARM hosts.

PR Checklist

@bdeb1337 bdeb1337 requested a review from daxian-dbw as a code owner January 23, 2025 18:20
@daxian-dbw
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@daxian-dbw daxian-dbw self-assigned this Jan 23, 2025
Copy link
Member

@daxian-dbw daxian-dbw left a comment

Choose a reason for hiding this comment

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

I made some minor updates to the changes, including a bit cleanup.

// Configuration is not supported on ARM64
if (PsUtils.IsRunningOnProcessorArchitectureARM())
// Configuration is not supported for ARM or ARM64 process architecture.
if (PsUtils.IsRunningOnProcessArchitectureARM())
Copy link
Member

Choose a reason for hiding this comment

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

@SteveL-MSFT You only mentioned ARM64 not supported in #18781 (comment), not ARM, so should we only check for ARM64 here?

Copy link
Member

Choose a reason for hiding this comment

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

The configuration keyword is only useful on Windows and Windows only supports Arm64 (no Arm32). However, for the code, it's ok to check for ARM as long as it's a superset of 32 and 64. There should be no functional difference as we don't produce Windows Arm32 packages anymore.

Copy link
Member

Choose a reason for hiding this comment

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

So, we are good with the current check.

@bdeb1337
Copy link
Contributor Author

@microsoft-github-policy-service agree

@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Jan 31, 2025
@daxian-dbw daxian-dbw added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Jan 31, 2025
@daxian-dbw daxian-dbw merged commit c3fbd2c into PowerShell:master Feb 24, 2025
39 of 41 checks passed
Copy link
Contributor

microsoft-github-policy-service bot commented Feb 24, 2025

📣 Hey @bdeb1337, how did we do? We would love to hear your feedback with the link below! 🗣️

🔗 https://aka.ms/PSRepoFeedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log
Projects
Development

Successfully merging this pull request may close these issues.

3 participants