[BUG] `brew upgrade` will build formula as the bottle when the bottle is not available #6063

xu-cheng · 2019-04-25T05:32:31Z

are reporting a bug others will be able to reproduce and not asking a question. If you're not sure or want to ask a question do so on our Discourse: https://discourse.brew.sh
ran a brew command and reproduced the problem with multiple formulae? If it's a problem with a single, official formula (not cask) please file this issue at Homebrew/homebrew-core: https://github.com/Homebrew/homebrew-core/issues/new/choose. If it's a brew cask problem please file this issue at https://github.com/Homebrew/homebrew-cask/issues/new/choose. If it's a tap (e.g. Homebrew/homebrew-php) problem please file this issue at the tap.
ran brew update and can still reproduce the problem?
ran brew doctor, fixed all issues and can still reproduce the problem?
ran brew config and brew doctor and included their output with your issue?

In the following situation, brew upgrade would try to build the formula as bottle (i.e., not running brew postinstall):

The formula was previously installed from bottle.
The formula got updated with no bottle available. (This seem happen more often in Linuxbrew, as many formulae got updated first with their Linux bottles being produced much later)

I find this when I try to upgrade gcc@8.

The root cause is due to the following codes.

brew/Library/Homebrew/cmd/upgrade.rb

Line 167 in 01ebe91

fi.build_bottle = args.build_bottle? || (!f.bottle_defined? && f.build.bottle?)

Noted f.bottle_defined? is false when the bottle is not available yet and f.build.bottle? is true since the formula is previously poured from the bottle.

There seems to be a discrepancy between whether a formula is built from bottle as it is or it is poured from the bottle. Also why do we need to support upgrade from a bottle build to another bottle build. Can we change the above code to just:

fi.build_bottle = args.build_bottle?

/cc @sjackman

The text was updated successfully, but these errors were encountered:

sjackman · 2019-04-25T15:10:57Z

This logic seems to go back to the very beginning of this code.
b6cca78
I see no reason to inspect f.build.bottle?, and fi.build_bottle = args.build_bottle? seems fine to me.

sjackman · 2019-04-25T15:11:35Z

cc @MikeMcQuaid

xu-cheng · 2019-04-25T15:36:09Z

FYI, the logic in b6cca78 is actually correct. Noted that it excludes the case when current installation is poured from bottle.

sjackman · 2019-04-25T15:47:50Z

~~The bug seems to have been introduced here~~: 7266ecd

Edit: it's fine here.

  def build_bottle?
	    built_as_bottle && !poured_from_bottle
  end

sjackman · 2019-04-25T15:52:15Z

The bug may have been introduced in 820b634, where tab.build_bottle? was changed to f.build.build_bottle?.

sjackman · 2019-04-25T15:57:41Z

I'm having trouble replicating the behaviour that you're experiencing though. Here's what I did:

brew install hello
brew edit hello
# Add revision 1
# Delete the bottles
# Add def post_install; ohai "Postinstall!"; end
brew upgrade hello

I see that postinstall is run.

❯❯❯ brew upgrade hello
==> Upgrading 1 outdated package:
hello 2.10 -> 2.10_1
==> Upgrading hello 
==> Downloading https://ftp.gnu.org/gnu/hello/hello-2.10.tar.gz
Already downloaded: /Users/sjackman/Library/Caches/Homebrew/downloads/0d3dc7a1e45d7ac6eb8d853c8ecccad6a4ab6abbbf38038f552e4735ad15d3cc--hello-2.10.tar.gz
==> ./configure --disable-silent-rules --prefix=/usr/local/Cellar/hello/2.10_1
==> make install
==> Postinstall!
🍺  /usr/local/Cellar/hello/2.10_1: 11 files, 125.4KB, built in 53 seconds
brew upgrade hello  35.57s user 17.51s system 95% cpu 55.533 total 69584 MB

xu-cheng · 2019-04-25T16:25:22Z

I don’t have access to my Mac for now. But on Linuxbrew, I get the following. Noted that you may need to delete bottles for all platforms (which is what happens for gcc@8).

$ brew install hello
==> Downloading https://linuxbrew.bintray.com/bottles/hello-2.10.x86_64_linux.bottle.tar.gz
Already downloaded: /tmp/chengxu/cache/homebrew/downloads/23a428a2b37762246a9389124d0f691876f0f41cec9f7807e35b457fbc568bc5--hello-2.10.x86_64_linux.bottle.tar.gz
==> Pouring hello-2.10.x86_64_linux.bottle.tar.gz
🍺  /home/comp/chengxu/usr/Cellar/hello/2.10: 52 files, 237.6KB

$ brew edit hello
Editing /home/comp/chengxu/usr/Library/Taps/homebrew/homebrew-core/Formula/hello.rb

$ brew cat hello
class Hello < Formula
  desc "Program providing model for GNU coding standards and practices"
  homepage "https://www.gnu.org/software/hello/"
  url "https://ftp.gnu.org/gnu/hello/hello-2.10.tar.gz"
  sha256 "31e066137a962676e89f69d1b65382de95a7ef7d914b8cb956f41ea72e0f516b"
  revision 1

  bottle do
    cellar :any_skip_relocation
  end

  conflicts_with "camlistore", :because => "both install `hello` binaries"

  def install
    system "./configure", "--disable-debug",
                          "--disable-dependency-tracking",
                          "--disable-silent-rules",
                          "--prefix=#{prefix}"
    system "make", "install"
  end

  def postinstall
    ohai "postinstall"
  end

  test do
    assert_equal "brew", shell_output("#{bin}/hello --greeting=brew").chomp
  end
end

$ brew upgrade hello 
==> Upgrading 1 outdated package:
hello 2.10 -> 2.10_1
==> Upgrading hello 
==> Downloading https://ftp.gnu.org/gnu/hello/hello-2.10.tar.gz
Already downloaded: /tmp/chengxu/cache/homebrew/downloads/0d3dc7a1e45d7ac6eb8d853c8ecccad6a4ab6abbbf38038f552e4735ad15d3cc--hello-2.10.tar.gz
==> ./configure --disable-silent-rules --prefix=/home/comp/chengxu/usr/Cellar/hello/2.10_1
==> make install
==> Not running post_install as we're building a bottle
You can run it manually using `brew postinstall hello`
🍺  /home/comp/chengxu/usr/Cellar/hello/2.10_1: 53 files, 245.5KB, built in 48 seconds
Removing: /home/comp/chengxu/usr/Cellar/hello/2.10... (52 files, 237.6KB)

sjackman · 2019-04-25T16:30:48Z

Ah, yep. Deleting the entire bottle section does produce the behaviour that you describe.

❯❯❯ brew upgrade hello
==> Upgrading 1 outdated package:
hello 2.10 -> 2.10_1
==> Upgrading hello 
==> Downloading https://ftp.gnu.org/gnu/hello/hello-2.10.tar.gz
Already downloaded: /Users/sjackman/Library/Caches/Homebrew/downloads/0d3dc7a1e45d7ac6eb8d853c8ecccad6a4ab6abbbf38038f552e4735ad15d3cc--hello-2.10.tar.gz
==> ./configure --disable-silent-rules --prefix=/usr/local/Cellar/hello/2.10_1
==> make install
==> Not running post_install as we're building a bottle
You can run it manually using `brew postinstall hello`
🍺  /usr/local/Cellar/hello/2.10_1: 11 files, 125.4KB, built in 1 minute 15 seconds
brew upgrade hello  33.51s user 17.69s system 65% cpu 1:18.38 total 67884 MB

xu-cheng · 2019-04-25T16:35:13Z

I think 820b634 is indeed the culprit. I think I made the mistake to assume Tab#build_bottle? and BuildOption#build_bottle? are the same back then. I would suggest to change the method name for Tab#build_bottle? which can be quite misleading.

sjackman · 2019-04-25T16:37:05Z

I would suggest to change the method name for Tab#build_bottle? which can be quite misleading.

Agreed. Perhaps past tense? Tab#built_bottle? instead of Tab#build_bottle?

sjackman · 2019-04-25T16:37:40Z

And I think Tab#built_bottle? should probably be false if the bottle was poured.

sjackman added the bug Reproducible Homebrew/brew bug label Apr 25, 2019

MikeMcQuaid mentioned this issue Apr 25, 2019

upgrade: don't build bottles unnecessarily. #6066

Merged

6 tasks

MikeMcQuaid closed this as completed in #6066 Apr 25, 2019

lock bot added the outdated PR was locked due to age label Jan 1, 2020

lock bot locked as resolved and limited conversation to collaborators Jan 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUG] `brew upgrade` will build formula as the bottle when the bottle is not available #6063

[BUG] `brew upgrade` will build formula as the bottle when the bottle is not available #6063

xu-cheng commented Apr 25, 2019

sjackman commented Apr 25, 2019

sjackman commented Apr 25, 2019

xu-cheng commented Apr 25, 2019

sjackman commented Apr 25, 2019 •

edited

sjackman commented Apr 25, 2019

sjackman commented Apr 25, 2019

xu-cheng commented Apr 25, 2019

sjackman commented Apr 25, 2019

xu-cheng commented Apr 25, 2019

sjackman commented Apr 25, 2019

sjackman commented Apr 25, 2019

[BUG] brew upgrade will build formula as the bottle when the bottle is not available #6063

[BUG] brew upgrade will build formula as the bottle when the bottle is not available #6063

Comments

xu-cheng commented Apr 25, 2019

sjackman commented Apr 25, 2019

sjackman commented Apr 25, 2019

xu-cheng commented Apr 25, 2019

sjackman commented Apr 25, 2019 • edited

sjackman commented Apr 25, 2019

sjackman commented Apr 25, 2019

xu-cheng commented Apr 25, 2019

sjackman commented Apr 25, 2019

xu-cheng commented Apr 25, 2019

sjackman commented Apr 25, 2019

sjackman commented Apr 25, 2019

[BUG] `brew upgrade` will build formula as the bottle when the bottle is not available #6063

[BUG] `brew upgrade` will build formula as the bottle when the bottle is not available #6063

sjackman commented Apr 25, 2019 •

edited