Re-work ARGV filtering to properly handle --HEAD
Previously, stripping arguments like `--HEAD` for dependencies failed because that flag affects the installation prefix encoded into formula objects. The previous implementation of `ARGV` filtering tried to contain all changes to a single method call before the `FormulaInstaller` forks. This update spreads things out a bit: - The Homebrew `ARGV` extension adds a new method, `filter_for_dependencies` which strips flags like `--HEAD`, yields to a block, then restores the original contents of ARGV. - The `explicitly_requested?` test, which returns true or false depending on if a formula object is a member of `ARGV.formulae`, is now a method of `Formula` objects. - `FormulaInstaller` objects now execute the installation of dependencies inside an `ARGV.filter_for_dependencies` block if the dependency was `explicitly_requested?`. Fixes Homebrew/homebrew#8668. Closes Homebrew/homebrew#7724.
This commit is contained in:
parent
9ecb2ceb29
commit
b5f942764a
@ -84,6 +84,10 @@ module Homebrew extend self
|
|||||||
unless formulae.empty?
|
unless formulae.empty?
|
||||||
perform_preinstall_checks
|
perform_preinstall_checks
|
||||||
formulae.each do |f|
|
formulae.each do |f|
|
||||||
|
# Check formula status and skip if necessary---a formula passed on the
|
||||||
|
# command line may have been installed to satisfy a dependency.
|
||||||
|
next if f.installed?
|
||||||
|
|
||||||
begin
|
begin
|
||||||
fi = FormulaInstaller.new(f)
|
fi = FormulaInstaller.new(f)
|
||||||
fi.install
|
fi.install
|
||||||
|
@ -96,6 +96,24 @@ module HomebrewArgvExtension
|
|||||||
Homebrew.help_s
|
Homebrew.help_s
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def filter_for_dependencies
|
||||||
|
# Clears some flags that affect installation, yields to a block, then
|
||||||
|
# restores to original state.
|
||||||
|
old_args = clone
|
||||||
|
|
||||||
|
%w[
|
||||||
|
--debug -d
|
||||||
|
--fresh
|
||||||
|
--interactive -i
|
||||||
|
--verbose -v
|
||||||
|
--HEAD
|
||||||
|
].each {|flag| delete flag}
|
||||||
|
|
||||||
|
yield
|
||||||
|
|
||||||
|
replace old_args
|
||||||
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
def downcased_unique_named
|
def downcased_unique_named
|
||||||
|
@ -148,6 +148,16 @@ class Formula
|
|||||||
return false
|
return false
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def explicitly_requested?
|
||||||
|
|
||||||
|
# `ARGV.formulae` will throw an exception if it comes up with an empty
|
||||||
|
# list.
|
||||||
|
#
|
||||||
|
# FIXME: `ARGV.formulae` shouldn't be throwing exceptions, see issue #8823
|
||||||
|
return false if ARGV.named.empty?
|
||||||
|
ARGV.formulae.include? self
|
||||||
|
end
|
||||||
|
|
||||||
def installed_prefix
|
def installed_prefix
|
||||||
head_prefix = HOMEBREW_CELLAR+@name+'HEAD'
|
head_prefix = HOMEBREW_CELLAR+@name+'HEAD'
|
||||||
if @version == 'HEAD' || head_prefix.directory?
|
if @version == 'HEAD' || head_prefix.directory?
|
||||||
|
@ -28,15 +28,17 @@ class FormulaInstaller
|
|||||||
needed_deps = f.recursive_deps.reject{ |d| d.installed? }
|
needed_deps = f.recursive_deps.reject{ |d| d.installed? }
|
||||||
unless needed_deps.empty?
|
unless needed_deps.empty?
|
||||||
needed_deps.each do |dep|
|
needed_deps.each do |dep|
|
||||||
fi = FormulaInstaller.new(dep)
|
if dep.explicitly_requested?
|
||||||
fi.ignore_deps = true
|
install_dependency dep
|
||||||
fi.show_header = false
|
else
|
||||||
oh1 "Installing #{f} dependency: #{dep}"
|
ARGV.filter_for_dependencies do
|
||||||
fi.install
|
# Re-create the formula object so that args like `--HEAD` won't
|
||||||
fi.caveats
|
# affect properties like the installation prefix.
|
||||||
fi.finish
|
dep = Formula.factory dep.name
|
||||||
|
install_dependency dep
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
# now show header as all the deps stuff has clouded the original issue
|
# now show header as all the deps stuff has clouded the original issue
|
||||||
show_header = true
|
show_header = true
|
||||||
end
|
end
|
||||||
@ -58,6 +60,16 @@ class FormulaInstaller
|
|||||||
raise "Nothing was installed to #{f.prefix}" unless f.installed?
|
raise "Nothing was installed to #{f.prefix}" unless f.installed?
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def install_dependency dep
|
||||||
|
fi = FormulaInstaller.new dep
|
||||||
|
fi.ignore_deps = true
|
||||||
|
fi.show_header = false
|
||||||
|
oh1 "Installing #{f} dependency: #{dep}"
|
||||||
|
fi.install
|
||||||
|
fi.caveats
|
||||||
|
fi.finish
|
||||||
|
end
|
||||||
|
|
||||||
def caveats
|
def caveats
|
||||||
if f.caveats
|
if f.caveats
|
||||||
ohai "Caveats", f.caveats
|
ohai "Caveats", f.caveats
|
||||||
@ -105,7 +117,13 @@ class FormulaInstaller
|
|||||||
# I'm guessing this is not a good way to do this, but I'm no UNIX guru
|
# I'm guessing this is not a good way to do this, but I'm no UNIX guru
|
||||||
ENV['HOMEBREW_ERROR_PIPE'] = write.to_i.to_s
|
ENV['HOMEBREW_ERROR_PIPE'] = write.to_i.to_s
|
||||||
|
|
||||||
args = filtered_args
|
args = ARGV.clone
|
||||||
|
unless args.include? '--fresh'
|
||||||
|
previous_install = Tab.for_formula f
|
||||||
|
args.concat previous_install.used_options
|
||||||
|
args.uniq! # Just in case some dupes were added
|
||||||
|
end
|
||||||
|
|
||||||
fork do
|
fork do
|
||||||
begin
|
begin
|
||||||
read.close
|
read.close
|
||||||
@ -239,49 +257,6 @@ class FormulaInstaller
|
|||||||
@show_summary_heading = true
|
@show_summary_heading = true
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
|
||||||
|
|
||||||
# This method gives us a chance to pre-process command line arguments before the
|
|
||||||
# installer forks and `Formula.install` kicks in.
|
|
||||||
def filtered_args
|
|
||||||
# Returns true if the formula attached to this installer was explicitly
|
|
||||||
# passed on the command line by the user as opposed to being automatically
|
|
||||||
# added to satisfy a dependency.
|
|
||||||
def explicitly_requested?
|
|
||||||
# `ARGV.formulae` will throw an exception if it comes up with an empty
|
|
||||||
# list.
|
|
||||||
#
|
|
||||||
# FIXME:
|
|
||||||
# `ARGV.formulae` probably shouldn't be throwing exceptions, it should be
|
|
||||||
# the caller's responsibility to check `ARGV.formulae.empty?`.
|
|
||||||
return false if ARGV.named.empty?
|
|
||||||
ARGV.formulae.include? f
|
|
||||||
end
|
|
||||||
|
|
||||||
args = ARGV.clone
|
|
||||||
|
|
||||||
# FIXME: Also need to remove `--HEAD`, however there is a problem in that
|
|
||||||
# the properties of formula objects, such as `prefix`, are influenced by
|
|
||||||
# `--HEAD` and get set when the object is created.
|
|
||||||
#
|
|
||||||
# See issue #8668
|
|
||||||
%w[
|
|
||||||
--debug -d
|
|
||||||
--fresh
|
|
||||||
--interactive -i
|
|
||||||
--verbose -v
|
|
||||||
].each {|flag| args.delete flag} unless explicitly_requested?
|
|
||||||
|
|
||||||
unless args.include? '--fresh'
|
|
||||||
previous_install = Tab.for_formula f
|
|
||||||
args.concat previous_install.used_options
|
|
||||||
end
|
|
||||||
|
|
||||||
args.uniq! # Just in case some dupes slipped by
|
|
||||||
|
|
||||||
return args
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user