Homebrew / brew

🍺 The missing package manager for macOS (or Linux)
https://brew.sh
BSD 2-Clause "Simplified" License
41.28k stars 9.71k forks source link

Taps created with brew extract are unable to use original bottles #6059

Closed glensc closed 4 years ago

glensc commented 5 years ago

I created versioned formula using brew extract command based on information from https://docs.brew.sh/Versions

see: https://github.com/glensc/homebrew-tap/pull/1

however, appears that brew internally determines f.name and root_url based on formula name and whether it's a tap. and there's no way to provide proper values other than hacking.

so it tries to download from:

instead of correct one:

the superfluous parts are:

so far i've come up working solution with some drawbacks:

https://github.com/glensc/homebrew-tap/commit/d6b70ca2a4eb292e397286ec9f83c7a931d5815b

  # force "kubernetes-helm" for bottle download
  # https://github.com/glensc/homebrew-tap/pull/1#issuecomment-486371612
  def name
    "kubernetes-helm"
  end

  bottle do
    # force "bottles" subdir for download
    # https://github.com/glensc/homebrew-tap/pull/1#issuecomment-486371612
    root_url "https://homebrew.bintray.com/bottles"

I think brew should provide somewhat solution here and perhaps documented on https://docs.brew.sh/Versions

also, looks like this same problem was reported to formulas repository by @astorath https://github.com/Homebrew/homebrew-core/issues/38782

the whole research and details can be seen in this PR comments: https://github.com/glensc/homebrew-tap/pull/1

MikeMcQuaid commented 5 years ago

This is currently intentional behaviour. The assumption if you brew extract a formula is that you will be modifying it. I'm open to making it suck less than it does currently, though, although it might not end up being default behaviour.

  def name
    "kubernetes-helm"
  end

What would you think about this being a bottle_name addition to the bottle do block or something? An alternative would be to special-case X.Y.Z formulae with the root_url manually set like that.

glensc commented 5 years ago

bottle_name to bottle DSL seems good.

and also I do not like overriding the root_url, it's like hardcoding default value to place where it should not be. could tap false directive do here? as much i looked at the code, the root_url was driven by is_tap setting.

some other ideas: use_core_tap true?

but, what about refactoring so that bottle download uses new attribute of formula, which defaults to .name, but can be overriden per formula? .bottle_name? something similar for root_url as well

MikeMcQuaid commented 5 years ago

I'd suggest use_core_tap! as a DSL method inside bottle do would be a good call which also automatically handles the brew extract versioning format.

astorath commented 5 years ago

@glensc , thanks for the tip with def name. I've end up creating simple brew cmd to fix extracted formulae 😄

https://github.com/astorath/homebrew-core/blob/master/cmd/brew-fixtap.rb

PS: For a title "The missing package manager for macOS", not being able to install specific package version and make versioned dependencies is a bit strange...

MikeMcQuaid commented 5 years ago

PS: For a title "The missing package manager for macOS", not being able to install specific package version and make versioned dependencies is a bit strange...

@astorath Please omit the PS with future posts, thanks. It's an intentional decision to not support every version in perpetuity because:

  1. it would mean supporting packages that upstream don't support (e.g. with known security vulnerabilities)
  2. it would mean supporting many thousands more packages which we don't have the financial or people resources to do
madushan1000 commented 5 years ago

I would like to work on this feature. Specifically adding use_core_tap dsl, and modify brew extract to patch formula on extraction. How should I go about adding the dsl? which source files should I start from?

scpeters commented 5 years ago

@madushan1000 to start, I think the dsl can be added to the BottleSpecification by adding :use_core_tap to the attr_rw list in software_spec.rb:

colindean commented 5 years ago

I'm able to reproduce this.

$ brew extract jq colindean/personal
==> Writing formula for jq from revision HEAD to /usr/local/Homebrew/Library/Taps/colindean/homebrew-personal/Formula/jq@1.6.rb
$ brew install jq@1.6
==> Installing jq@1.6 from colindean/personal
==> Downloading https://homebrew.bintray.com/bottles-personal/jq@1.6-1.6.mojave.bottle.1.tar.gz

curl: (22) The requested URL returned error: 404 Not Found
Error: Failed to download resource "jq@1.6"
Download failed: https://homebrew.bintray.com/bottles-personal/jq@1.6-1.6.mojave.bottle.1.tar.gz
Warning: Bottle installation failed: building from source.
…

I'm seeing a strange difference between install and fetch:

$ bin/brew fetch /usr/local/Homebrew/Library/Taps/colindean/homebrew-personal/Formula/jq@1.6.rb
==> Downloading https://homebrew.bintray.com/bottles/jq@1.6-1.6.mojave.bottle.1.tar.gz

curl: (22) The requested URL returned error: 404 Not Found
Error: Failed to download resource "jq@1.6"
Download failed: https://homebrew.bintray.com/bottles/jq@1.6-1.6.mojave.bottle.1.tar.gz
Warning: Bottle fetch failed: fetching the source.

Fetch doesn't add the tap name.

I got a start on it in the branch I just linked but I'm out of time for today. See the commit message for what's broken still.

figroc commented 4 years ago

Fetch doesn't add the tap name.

Sounds like a bug.

MikeMcQuaid commented 4 years ago

I've investigated this further and I'm afraid it cannot work as desired.

  # force "kubernetes-helm" for bottle download
  # https://github.com/glensc/homebrew-tap/pull/1#issuecomment-486371612
  def name
    "kubernetes-helm"
  end

This doesn't just force it for bottle download but for the entire formula. Doing this effectively makes the formula file be named kubernetes-helm.rb rather than kubernetes-helm@2.8.2.rb to Homebrew.

This is undesirable as it prevents the formula from being installed alongside kubernetes-helm.rb and may prevent upgrades of kubernetes-helm.rb.

The bottles from kubernetes-helm.rb cannot be reused by kubernetes-helm@2.8.2.rb because the prior will install into /usr/local/Cellar/kubernetes-helm and the latter will install into /usr/local/Cellar/kubernetes-helm@2.8.2. You cannot simply change the output location at extraction time given Homebrew's many internal assumptions. Even if you were to fix those, these would fail for bottles that are not cellar :any as they would need to be relocated.

Sorry for the delay in getting to this and the unclear analysis at first from my end. Setting root_url manually and changing the formula name are the only real workarounds given the above. Sorry!

glensc commented 4 years ago

So in other words, the problem with bottle re-use for another formula, is that the paths inside the bottle are absolute paths from filesystem root and are, therefore unsuitable.

MikeMcQuaid commented 4 years ago

@glensc Exactly. It's unfortunate (hence my attempt to fix this).

glensc commented 4 years ago

So, I downloaded the bottle, and looked inside. The paths are not absolute, but they do contain the recipe name and version, so the problem remains:

[/tmp] ➔ tar tvf kubernetes-helm-2.8.2.high_sierra.bottle.tar.gz |head
drwxr-xr-x brew/staff        0 2018-03-09 22:44 kubernetes-helm/2.8.2/
drwxr-xr-x brew/staff        0 2018-03-09 22:44 kubernetes-helm/2.8.2/.brew/
drwxr-xr-x brew/staff        0 2018-03-09 22:44 kubernetes-helm/2.8.2/bin/
drwxr-xr-x brew/staff        0 2018-03-09 22:44 kubernetes-helm/2.8.2/etc/
-rw-r--r-- brew/staff      585 2018-03-09 22:44 kubernetes-helm/2.8.2/INSTALL_RECEIPT.json
-rw-r--r-- brew/staff    11373 2018-03-09 22:44 kubernetes-helm/2.8.2/LICENSE
-rw-r--r-- brew/staff     3200 2018-03-09 22:44 kubernetes-helm/2.8.2/README.md
drwxr-xr-x brew/staff        0 2018-03-09 22:44 kubernetes-helm/2.8.2/share/
drwxr-xr-x brew/staff        0 2018-03-09 22:44 kubernetes-helm/2.8.2/share/man/
drwxr-xr-x brew/staff        0 2018-03-09 22:44 kubernetes-helm/2.8.2/share/man/man1/
tar: write error