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

Different build version for arm64 #99397

Merged
merged 3 commits into from
Feb 8, 2021

Conversation

pszypowicz
Copy link
Contributor

Important: Do not tick a checkbox if you haven’t performed its action. Honesty is indispensable for a smooth review process.

After making all changes to a cask, verify:

Additionally, if adding a new cask:

  • Named the cask according to the token reference.
  • Checked the cask was not already refused.
  • Checked the cask is submitted to the correct repo.
  • brew audit --new-cask {{cask_file}} worked successfully.
  • brew install --cask {{cask_file}} worked successfully.
  • brew uninstall --cask {{cask_file}} worked successfully.

@pszypowicz
Copy link
Contributor Author

Hi,

I've got this strange problem, when arm64 build (2.6.4-2) have different version then intel (2.6.4).

I'm not sure if my approach for downloading is rock solid, but actually downloads proper package.

Unfortunately the livecheck does not work, since the latest GitHub version is the on without patch version.

I would appreciate any advice to write a proper livecheck here.

@pszypowicz
Copy link
Contributor Author

pszypowicz commented Feb 5, 2021

BTW it seems that the pull request CI job is performed only from an intel arch perspective

https://github.com/Homebrew/homebrew-cask/pull/99397/checks?check_run_id=1842834224

1: Run brew audit --cask --appcast --online './Casks/keepassxc.rb'
[...]
8: warning: parser/current is loading parser/ruby26, which recognises
9: warning: 2.6.6-compliant syntax, but you are running 2.6.3.
10: warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
11 ==> Downloading https://github.com/keepassxreboot/keepassxc/releases/download/2.6.4/KeePassXC-2.6.4-x86_64.dmg
12 Already downloaded: /Users/runner/Library/Caches/Homebrew/downloads/28fcb67ddec3963b7aac72d61447e04f60a93333cb41047896ac9d05257efa2f--KeePassXC-2.6.4-x86_64.dmg
13 audit for keepassxc: passed

But on my m1 Mac:

$ brew audit --cask ./Casks/keepassxc.rb --online                                           
warning: parser/current is loading parser/ruby26, which recognizes
warning: 2.6.6-compliant syntax, but you are running 2.6.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
==> Downloading https://github.com/keepassxreboot/keepassxc/releases/download/2.6.4/KeePassXC-2.6.4-2-arm64.dmg
Already downloaded: /Users/pszypowicz/Library/Caches/Homebrew/downloads/cdf9209ad5a6b54e6cb6fd52c7ac477fc8b1afa750883cd6322499da46df7ee8--KeePassXC-2.6.4-2-arm64.dmg
audit for keepassxc: failed
 - Version '2.6.4-2' differs from '2.6.4' retrieved by livecheck.
 - exception while auditing keepassxc: undefined method `audit_exceptions' for nil:NilClass
Error: 2 problems in 1 cask detected

url "https://github.com/keepassxreboot/keepassxc/releases/download/#{version}/KeePassXC-#{version}-x86_64.dmg",
verified: "github.com/keepassxreboot/keepassxc/"
sha256 "639fdfe0379dc3f00f1bd6a72c974cfddae0d4c8ddb66f7297d0b01b1a582ede"
else
url "https://github.com/keepassxreboot/keepassxc/releases/download/#{version}/KeePassXC-#{version}-arm64.dmg",
version "2.6.4-2"
url "https://github.com/keepassxreboot/keepassxc/releases/download/#{version.sub(/-\d+/, "")}/KeePassXC-#{version}-arm64.dmg",
Copy link
Member

Choose a reason for hiding this comment

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

Rather than version.sub(/-\d+/, ""), version.split('-').first is clearer.

But do we expect that hyphen to be a regular thing, or was it a one-off?

Copy link
Contributor Author

@pszypowicz pszypowicz Feb 7, 2021

Choose a reason for hiding this comment

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

Hard to say, probably a one-off, since that was their first build for a m1 .

Buy this also introduced the problem with livecheck and I'm thinking that the proper version for the arm build could be taken with regex.

Would something like this work?

  if Hardware::CPU.intel?
    version "2.6.4"
    url "https://github.com/keepassxreboot/keepassxc/releases/download/#{version}/KeePassXC-#{version}-x86_64.dmg",
        verified: "github.com/keepassxreboot/keepassxc/"
    sha256 "639fdfe0379dc3f00f1bd6a72c974cfddae0d4c8ddb66f7297d0b01b1a582ede"
    livecheck do
      url :url
      strategy :github_latest
    end 
  else
    version "2.6.4-2"
    url "https://github.com/keepassxreboot/keepassxc/releases/download/#{version.split("-").first}/KeePassXC-#{version}-arm64.dmg",
        verified: "github.com/keepassxreboot/keepassxc/"
    sha256 "49c28e3f6341710b24d4415186ec0a52f6c0af5e72c0fc7548ddd7999d2eaa4d"
    livecheck do
      url "https://keepassxc.org/download/"
      regex(/href=.*?KeePassXC[._-]v?(\d+(?:[.\-]\d+)+)-arm64.dmg/i)
    end
  
  end

Copy link
Member

Choose a reason for hiding this comment

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

Let’s assume it’s a one-off, share the version, add the -2 in the URL, and skip the livecheck check in CI. On the next update, we’ll revisit.

Unify version, and modify url with hardcoded arm64 build version as suggested by maintainer.
@vitorgalvao vitorgalvao merged commit 4e3d423 into Homebrew:master Feb 8, 2021
@vitorgalvao
Copy link
Member

Thank you @pszypowicz.

@Homebrew Homebrew locked as resolved and limited conversation to collaborators Mar 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants