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

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

Closed
glensc opened this issue Apr 24, 2019 · 13 comments
Closed

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

glensc opened this issue Apr 24, 2019 · 13 comments
Labels
good first issue A good issue for your first contribution to Homebrew/brew help wanted We want help addressing this outdated PR was locked due to age

Comments

@glensc
Copy link
Contributor

glensc commented Apr 24, 2019

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

see: glensc/homebrew-tap#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:

  • -tap in path
  • @2.8.2 in filename

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

glensc/homebrew-tap@d6b70ca

  # 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 Homebrew/homebrew-core#38782

the whole research and details can be seen in this PR comments: glensc/homebrew-tap#1

@MikeMcQuaid
Copy link
Member

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
Copy link
Contributor Author

glensc commented Apr 24, 2019

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 MikeMcQuaid added the help wanted We want help addressing this label Apr 30, 2019
@MikeMcQuaid
Copy link
Member

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.

@MikeMcQuaid MikeMcQuaid added the good first issue A good issue for your first contribution to Homebrew/brew label May 6, 2019
@astorath
Copy link

astorath commented May 6, 2019

@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
Copy link
Member

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

polothy added a commit to polothy/homebrew-versions that referenced this issue Jun 20, 2019
@madushan1000
Copy link

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
Copy link
Member

@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 added a commit to colindean/brew that referenced this issue Oct 5, 2019
Fixes Homebrew#6059

WIP because this currently _breaks_ core tap formulae that have an AT
version and have a bottle, e.g. postgresql@9.6. The filename builder
needs to know if it should drop the @Version or not, and that should
probably be a part of the `use_core_bottles!` invocation.
@colindean
Copy link
Sponsor Member

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
Copy link
Contributor

figroc commented Nov 29, 2019

Fetch doesn't add the tap name.

Sounds like a bug.

@MikeMcQuaid
Copy link
Member

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
Copy link
Contributor Author

glensc commented May 24, 2020

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
Copy link
Member

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

@glensc
Copy link
Contributor Author

glensc commented May 24, 2020

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

@lock lock bot added the outdated PR was locked due to age label Jun 24, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue A good issue for your first contribution to Homebrew/brew help wanted We want help addressing this outdated PR was locked due to age
Projects
None yet
Development

No branches or pull requests

7 participants