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

Refactor release_name function to be more resilient detecting architecture on macOS [#350] #351

Closed

Conversation

zorfling
Copy link
Contributor

@zorfling zorfling commented Nov 7, 2023

Instead of assuming the architecture is the last field of uname -a, grep for the architectures we care about and set the release name accordingly.

My uname -a for reference:
Darwin ms-macbook-pro.lan 22.6.0 Darwin Kernel Version 22.6.0: Wed Oct 4 21:26:23 PDT 2023; root:xnu-8796.141.3.701.17~4/RELEASE_ARM64_T6000 arm64 arm Darwin

This is with coreutils installed through homebrew.

Fixes #350

@zorfling zorfling requested a review from a team as a code owner November 7, 2023 02:37
@reillysiemens
Copy link
Member

@zorfling, we really appreciate you taking the time to submit a fix for #350 yourself! I validated your submission myself on my mac by running

export AZUREAUTH_VERSION='0.8.4'
curl https://raw.githubusercontent.com/zorfling/microsoft-authentication-cli/user/ccolborne/macos-architecture-fix/install/install.sh | sh

and that worked just fine.

I'm not sure what's causing these CI failure in your PR, but after our team figures out what's going on there we'll be happy to take this fix.

Actually... they might be resolved just by having you update your branch from main. Can you try doing that, @zorfling?

@goagain
Copy link
Contributor

goagain commented Nov 8, 2023

@zorfling, we really appreciate you taking the time to submit a fix for #350 yourself! I validated your submission myself on my mac by running

export AZUREAUTH_VERSION='0.8.4'
curl https://raw.githubusercontent.com/zorfling/microsoft-authentication-cli/user/ccolborne/macos-architecture-fix/install/install.sh | sh

and that worked just fine.

I'm not sure what's causing these CI failure in your PR, but after our team figures out what's going on there we'll be happy to take this fix.

Actually... they might be resolved just by having you update your branch from main. Can you try doing that, @zorfling?

The PR failed because as an external developer, he doesn't have the access to our pipeline secrets. The only way I was success is dumping the PR by our team.
I also tried set the ADO_TOKEN on my personal repo, but still failed.

@reillysiemens
Copy link
Member

Ugh. You're right, @goagain. I'm not sure we have the time to properly fix that at the moment, but it would be good have these changes. @zorfling, I resubmitted these changes as #354 to work around the contribution issues, but kept your commits. I'll close this PR in favor of that one, but please know we're grateful for your submission.

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.

Architecture parsing failing on MacOS with coreutils installed through Homebrew
3 participants