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 darwin arch selection logic #388

Conversation

alex-shafer-1002
Copy link

@alex-shafer-1002 alex-shafer-1002 commented Mar 30, 2023

This corrects the regex to choose amd64 for versions < 1.0.2

Tested as follows:

  1. Created a script to test the logic in isolation:
#!/usr/bin/env bash

while read version; do
    if [[ "${version}" =~ ^0\. || "${version}" =~ ^1\.0\.[01]$ ]]; then
      echo "amd64  $version"
    else
      echo "ARM64  $version"
    fi;
done
  1. Piped tfenv list-remote to this script to test all current versions:
tfenv list-remote | ~/version-logic.sh
  1. Verified that the output switched from amd64 to arm64 at the expected version (truncated for brevity):
ARM64  1.0.4
ARM64  1.0.3
ARM64  1.0.2
amd64  1.0.1
amd64  1.0.0
amd64  0.15.5
  1. Temporarily patched my local installed libexec/tfenv-install with no TFENV_ARCH set in my environment and tested some relevant cases (note the arch in the download URL):
% tfenv install 1.0.1
Installing Terraform v1.0.1
Downloading release tarball from https://releases.hashicorp.com/terraform/1.0.1/terraform_1.0.1_darwin_amd64.zip
...
Installation of terraform v1.0.1 successful. To make this your default version, run 'tfenv use 1.0.1'

% tfenv install 1.0.2
Installing Terraform v1.0.2
Downloading release tarball from https://releases.hashicorp.com/terraform/1.0.2/terraform_1.0.2_darwin_arm64.zip
...
Installation of terraform v1.0.2 successful. To make this your default version, run 'tfenv use 1.0.2'

% tfenv install 1.0.10
Installing Terraform v1.0.10
Downloading release tarball from https://releases.hashicorp.com/terraform/1.0.10/terraform_1.0.10_darwin_arm64.zip
...
Installation of terraform v1.0.10 successful. To make this your default version, run 'tfenv use 1.0.10'

% tfenv install 1.0.11
Installing Terraform v1.0.11
Downloading release tarball from https://releases.hashicorp.com/terraform/1.0.11/terraform_1.0.11_darwin_arm64.zip
...
Installation of terraform v1.0.11 successful. To make this your default version, run 'tfenv use 1.0.11'

@denizgenc
Copy link
Contributor

Your script produces the exact same output when run against tfutils:master. There isn't anything wrong with the line you're changing; you're probably comparing against the latest released binary on brew, which is based on the 3.0.0 label, where tfenv-install looked like this.

(sorry if I'm being a bit defensive, I wrote the line that this PR replaces 😅)

@alex-shafer-1002
Copy link
Author

Got it, yes this was against the current brew version, we just really need a new deploy I guess.

@bryanhiestand
Copy link
Contributor

Your script produces the exact same output when run against tfutils:master. There isn't anything wrong with the line you're changing; you're probably comparing against the latest released binary on brew, which is based on the 3.0.0 label, where tfenv-install looked like this.

(sorry if I'm being a bit defensive, I wrote the line that this PR replaces 😅)

I'm having trouble following you or understanding how these two lines could produce the same output. Can you point me at the code you're talking about if it's not this?

if [[ "${version}" =~ 0\..+$ || "${version}" =~ 1\.0\.0|1$

I just opened #403 to try to fix the bugs in these regexes, but it looks like this PR fixes the behavior as well.

Current master (but not release 3.0.0) fixes the issue where arm64 is chosen instead of amd64 for versions < 1.0.2. But with the code I linked (and this PR changes), some versions > 1.0.2 use amd64 instead of arm64.

@denizgenc
Copy link
Contributor

You're definitely right, I didn't use the start of line anchors and should have been stricter about what I was matching with the regexes. Apologies to Alex

@Zordrak
Copy link
Collaborator

Zordrak commented Dec 19, 2023

Superseded by #403

@Zordrak Zordrak closed this Dec 19, 2023
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.

No arm64 builds for terraform < 1.0.2 404 during specified package install
4 participants