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: Resolve version before checking arch #24

Merged
merged 3 commits into from
Feb 9, 2024

Conversation

abhinav
Copy link
Contributor

@abhinav abhinav commented Nov 28, 2023

Fixes a bug where gimme module reports the following error
on macOS with ARM64.

error: arm64 is not supported by this go version
try go1.5 or newer

This occurs because the check for architecture
is performed before the module version
turns into a real version number with _resolve_version.

Fix by resolving version number before checking the architecture.

Fixes a bug where `gimme module` reports the following error
on macOS with ARM64.

    error: arm64 is not supported by this go version
    try go1.5 or newer

This occurs because the check for architecture
is performed before the `module` version
turns into a real version number with `_resolve_version`.

Fix by resolving version number before checking the architecture.
Copy link
Contributor

@brackendawson brackendawson left a comment

Choose a reason for hiding this comment

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

This fix works for me from 1.16 (when darwin/arm64 support was added). But the behaviour if the go directive specifies <1.16 changes from a partially incorrect error message (should say 1.16 on darwin):

% gimme module
error: arm64 is not supported by this go version
try go1.5 or newer

to an unhelpful error message:

% gimme module
I don't have any idea what to do with '1.13.15'.
  (using download type 'auto')

Can we restore the original error message but correct the version number for darwin? 1.5 was when linux/arm64 support was added.

@brackendawson
Copy link
Contributor

Also this needs to change, perhaps:

To install the release specified in the go.mod file, or the most recent patch when no patch version is present:

Fixes the error message that is printed
if you request Go < 1.16 on darwin/arm64.
@abhinav
Copy link
Contributor Author

abhinav commented Feb 6, 2024

Sounds good!
Updated the arm64 version matching.

Agreed about the README change too.
Should've done that in #25, but late is better than never.
Updated to reflect new behavior.

@abhinav abhinav force-pushed the resolve-before-arch branch 2 times, most recently from 5125ef0 to 63f0743 Compare February 6, 2024 13:31
@brackendawson
Copy link
Contributor

New error message looks good but it still doesn't actually show up:

% uname -sm
Darwin arm64
% cat go.mod
module github.com/urfave/gimme/test

go 1.13
% ../gimme module
I don't have any idea what to do with '1.13.15'.
  (using download type 'auto')

@abhinav
Copy link
Contributor Author

abhinav commented Feb 9, 2024

That's weird.

❯ uname -sm
Darwin arm64

❯ head -n3 go.mod
module github.com/urfave/gimme

go 1.13

❯ ./gimme module
error: arm64 is not supported by this go version
try go1.16 or newer

Copy link
Contributor

@brackendawson brackendawson left a comment

Choose a reason for hiding this comment

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

My bad, it works better when you pull all the changes in the branch. Thank you.

@brackendawson brackendawson merged commit 75940b0 into urfave:main Feb 9, 2024
19 checks passed
@abhinav abhinav deleted the resolve-before-arch branch February 9, 2024 15:31
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.

2 participants