Skip to content
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

fix: More reliable arch detection in install.ps1 #4128

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

KapJI
Copy link
Contributor

@KapJI KapJI commented Dec 9, 2024

Value returned by (Get-CimInstance -Class Win32_OperatingSystem).OSArchitecture is locale dependant.

This version should work the same for any locale. And it works both in powershell.exe and pwsh.exe.

Fixes #4127

@KapJI KapJI marked this pull request as ready for review December 9, 2024 15:13
@KapJI KapJI marked this pull request as draft December 9, 2024 15:17
@KapJI KapJI force-pushed the win-arch branch 2 times, most recently from d713754 to 1b8f958 Compare December 9, 2024 15:24
@KapJI KapJI marked this pull request as ready for review December 9, 2024 15:25
@twpayne twpayne requested a review from bradenhilton December 9, 2024 15:28
Copy link
Collaborator

@bradenhilton bradenhilton left a comment

Choose a reason for hiding this comment

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

I'm happy if @nagromc is happy.

Also, I noticed that some jobs from main are running when they shouldn't be. I think the conditional needs to be moved up to the job level instead of the step level.

@KapJI
Copy link
Contributor Author

KapJI commented Dec 9, 2024

I'm going to add a bit more of error handling, please do not merge it yet.

@KapJI
Copy link
Contributor Author

KapJI commented Dec 9, 2024

I'm going to add a bit more of error handling, please do not merge it yet.

Done.

I noticed that some jobs from main are running when they shouldn't be.

I think this is expected. There are some spellcheckers and test-ubuntu is configured to run for all changes just in case.

@bradenhilton
Copy link
Collaborator

I think this is expected. There are some spellcheckers and test-ubuntu is configured to run for all changes just in case.

Testing chezmoi shouldn't be necessary when only the install scripts are changed. That was the reason why I separated them in the first place (granted, that was more of an issue before you sped up the test jobs, as the entire workflow used to run for any change).

@twpayne twpayne merged commit e038aad into twpayne:master Dec 9, 2024
25 checks passed
@KapJI KapJI deleted the win-arch branch December 9, 2024 16:29
@twpayne
Copy link
Owner

twpayne commented Dec 9, 2024

Thank you for the fast fix @KapJI and review @bradenhilton!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PowerShell installation script install.ps1 cannot download archive
3 participants