Replacing all ARGV references with Homebrew.args

[mofo37]

Issue reference: #5730

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? [Here's an example] (https://github.com/Homebrew/brew/blob/master/Library/Homebrew/test/PATH_spec.rb).
  Updated all existing tests.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

Questions:

• It looks like the build checks are failing. Is there a way to reproduce that locally?
• We did a global replace. We would have preferred to make smaller, incremental changes, but weren't sure if that was the best move and weren't sure how to run individual file tests. We are open to feedback.
• Where ARGV filenames are still in existence, should they be changed to Homebrew_args or something similar?

@BenMusch is this what you were expecting?

[BenMusch]

Yep, this is basically what I was planning on doing. Thanks for picking up the issue!

[MikeMcQuaid]

Wow, thanks for this! Great work so far!

• It looks like the build checks are failing. Is there a way to reproduce that locally?

Yup, run brew tests locally!

• We did a global replace. We would have preferred to make smaller, incremental changes, but weren't sure if that was the best move and weren't sure how to run individual file tests. We are open to feedback.

I'd be happy with a global change but it's probably a little bit more involved than a global search and replace, unfortunately. Note from #5730 (comment):

formula_installer.rb and build.rb should be handled last as they read from and modify ARGV:

brew/Library/Homebrew/formula_installer.rb

Lines 678 to 751 in b4f73e6

	def sanitized_argv_options
	args = []
	args << "--ignore-dependencies" if ignore_deps?
	if build_bottle?
	args << "--build-bottle"
	args << "--bottle-arch=#{ARGV.bottle_arch}" if ARGV.bottle_arch
	end
	args << "--git" if git?
	args << "--interactive" if interactive?
	args << "--verbose" if verbose?
	args << "--debug" if debug?
	args << "--cc=#{ARGV.cc}" if ARGV.cc
	args << "--default-fortran-flags" if ARGV.include? "--default-fortran-flags"
	args << "--keep-tmp" if ARGV.keep_tmp?
	if ARGV.env
	args << "--env=#{ARGV.env}"
	elsif formula.env.std? || formula.deps.select(&:build?).any? { |d| d.name == "scons" }
	args << "--env=std"
	end
	if formula.head?
	args << "--HEAD"
	elsif formula.devel?
	args << "--devel"
	end
	formula.options.each do |opt|
	name = opt.name[/^([^=]+)=$/, 1]
	value = ARGV.value(name) if name
	args << "--#{name}=#{value}" if value
	end
	args
	end
	def build_argv
	sanitized_argv_options + options.as_flags
	end
	def build
	FileUtils.rm_rf(formula.logs)
	@start_time = Time.now
	# 1. formulae can modify ENV, so we must ensure that each
	# installation has a pristine ENV when it starts, forking now is
	# the easiest way to do this
	args = %W[
	nice #{RUBY_PATH}
	-W0
	-I #{$LOAD_PATH.join(File::PATH_SEPARATOR)}
	--
	#{HOMEBREW_LIBRARY_PATH}/build.rb
	#{formula.specified_path}
	].concat(build_argv)
	Utils.safe_fork do
	if Sandbox.formula?(formula)
	sandbox = Sandbox.new
	formula.logs.mkpath
	sandbox.record_log(formula.logs/"build.sandbox.log")
	sandbox.allow_write_path(ENV["HOME"]) if ARGV.interactive?
	sandbox.allow_write_temp_and_cache
	sandbox.allow_write_log(formula)
	sandbox.allow_cvs
	sandbox.allow_fossil
	sandbox.allow_write_xcode
	sandbox.allow_write_cellar(formula)
	sandbox.exec(*args)
	else
	exec(*args)

So I'd suggest excluding at least those files and brew.rb for now (that's why brew tests will fail immediately because no brew command` can run currently.

Shout at all if I/we can help at all and thanks again for the PR!

[mofo37]

• brew tests locally do pass for us so maybe we're doing something wrong?
• after excluding build.rb, brew.rb and formula_installer.rb and running brew tests all tests passed and CI still failed.
• is there a way to pass a single file to maybe brew test to check our work as we go so we don't have to run the full suite every time?

Thanks!

[MikeMcQuaid]

• brew tests locally do pass for us so maybe we're doing something wrong?

How are you running it? Does brew --repo match the directory you're editing/pushing to GitHub?

• is there a way to pass a single file to maybe brew test to check our work as we go so we don't have to run the full suite every time?

Yup: brew tests --only=foo will run only Library/Homebrew/test/foo_spec.rb. See brew test --help for more information.

[GauthamGoli]

I'd be happy with a global change but it's probably a little bit more involved than a global search and replace, unfortunately.

I agree with @MikeMcQuaid here. An incremental approach would be best in this case.

Global change would also work as long as you have a lot of patience to fix all the failing tests one by one.

[MikeMcQuaid]

@mofo37 Let us know if you'd rather open some smaller PRs instead as per-@GauthamGoli's suggestion as he may also be able to help a bit with this project.

[mofo37]

@BenMusch did you ever get these tests to fail locally? #5762 (comment)

[BenMusch]

Sorry @mofo37, I don't remember enough about my work on this and don't really have the time to set it up again. Sorry and good luck!

[MikeMcQuaid]

@mofo37 How are you running brew tests? Does brew --repo match the directory you're editing/pushing to GitHub?

[mofo37]

@MikeMcQuaid
brew --repo returns this: /usr/local/Homebrew
I'm not sure how to point it to the directory I'm editing/pushing to GitHub.

When I point to the folder I'm working in with brew --repo <file_path>, I get this error:
Error: Invalid tap name "<file_path>"

How do I get it to point to the correct directory?

[BenMusch]

@mofo37 I don't think this is the preferred way to do things, but the quick fix I used to develop on Homebrew was just to symlink Homebrew to my source code directory and work from there. Some clearer steps:

1. Commit and push all of the changes you have on your fork of homebrew
2. cd /usr/local/Homebrew
3. git remote set-url origin git@github.com:mofo37/brew.git
4. git remote add upstream git@github.com:Homebrew/brew.git
5. ln -s /usr/local/Homebrew {your preferred source code/documents directory}/brew

This sets things up so that:

• The brew that you run via the brew command will point to your local version of homebrew
• Editing the files from either the /usr/local/Homebrew or your source/docs directory will both modify the same source, since one is just a link to the other
• Your git repo will have an upstream source pointing to @Homebrew's repo and an origin pointing to your own

Then, the only issues I ran into were brew auto-updating and changing the branches. to fix that, I set these variables in my ~/.bash_profile:

HOMEBREW_DEVELOPER=1
HOMEBREW_NO_AUTO_UPDATE=1

If you add those and then run source ~/.bash_profile, you should be good to go. Note, however, that this will make it your responsibility to manually get updates.

Again: I'm not sure if this is the preferred way of doing things, but it's what I managed to work make for myself when I worked on a few bugfixes

One last thing: This is me recalling something I last set up a few months back. If you run into any commands / config that doesn't seem right here, it's probably because I'm wrong. Feel free to ask me if anything is unclear / confusing!

[MikeMcQuaid]

Sorry @mofo37 but I'm closing this out until you're able to figure out how to test the changes locally. Happy to help with that: just shout.