Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

brew audit: Is "#{version}" redundant? #32540

Closed
kkdd opened this issue Sep 23, 2014 · 6 comments
Closed

brew audit: Is "#{version}" redundant? #32540

kkdd opened this issue Sep 23, 2014 · 6 comments

Comments

@kkdd
Copy link
Contributor

kkdd commented Sep 23, 2014

Hello,
"brew audit" says "#{version}" is redundant as follows. I don't think so.

$ brew audit ricty.rb 
ricty:
 * Stable: version 3.2.3 is redundant with version scanned from URL
$ cat ricty.rb 
class Ricty < Formula
  homepage 'https://github.com/yascentur/Ricty'
  version '3.2.3'
  sha1 'be01bde44bd96a113430150ac1c06c34bf34ab09'
  url "https://github.com/yascentur/Ricty/archive/#{version}.tar.gz"
end
@jacknagel
Copy link
Contributor

Since all Homebrew sees is the resulting string, it's impossible to detect whether you've interpolated the version or not.

We prefer not to interpolate the version in order to make it easy to copy/paste URLs.

@jakepetroules
Copy link
Contributor

This is a bit ridiculous, especially when the version is specified multiple times in the URL.

version "1.4.3"
url "https://download.qt.io/official_releases/qbs/#{version}/qbs-src-#{version}.tar.gz"

Please re-think this policy. How often are people copy/pasting URLs from Homebrew formulas?

@MikeMcQuaid
Copy link
Member

@jakepetroules It seems a bit ridiculous that qbs has the version multiple times in the URL.

@bfontaine
Copy link
Contributor

@MikeMcQuaid to be fair it’s very common. The core has 642 formulae with an URL that contains the version twice or more.
Here are a few examples:

http://abcl.org/releases/1.3.2/abcl-bin-1.3.2.tar.gz
https://downloads.sourceforge.net/project/abook/abook/0.5.6/abook-0.5.6.tar.gz
https://www.apache.org/dyn/closer.cgi?path=activemq/activemq-cpp/3.8.4/activemq-cpp-library-3.8.4-src.tar.bz2
https://www.apache.org/dyn/closer.cgi?path=/activemq/5.11.2/apache-activemq-5.11.2-bin.tar.gz
https://github.com/admesh/admesh/releases/download/v0.98.2/admesh-0.98.2.tar.gz
https://downloads.sourceforge.net/project/adplug/AdPlug%20core%20library/2.2.1/adplug-2.2.1.tar.bz2
https://github.com/amadvance/advancecomp/releases/download/v1.20/advancecomp-1.20.tar.gz

@DomT4
Copy link
Member

DomT4 commented Oct 27, 2015

Stand by the comment I made in the related thread:

The version not being accepted inline in the URL is not a bug. It's an intentional choice we've made before.

We prefer full URLs for the sake of readability, making changes more obvious, etc. We pluck version out of the URL automatically and that can be used throughout the formula with the version syntax.

You're not really de-duplicating something if you replace two version numbers with one explicit version definition and two version variables. You've gone from two things to three to support the same thing.

I don't see how including an inline version and then using that version twice instead of just leaving the two version numbers in the URL is more efficient, not that either Mike or Baptiste are necessarily voicing support or opposition above.

@bfontaine
Copy link
Contributor

I’m personally 👎 on allowing the url "…#{version}…" pattern; having the full URL is more readable.

We probably spend 10× (100×?) more time reading the code than writing it; so let’s optimize it for the readability 😄

@Homebrew Homebrew locked and limited conversation to collaborators Feb 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants