Brew: Add `uses_from_macos "python"` DSL

[MikeMcQuaid]

We should add a new DSL for formulae to allow stating system dependencies:

  uses_from_macos "python"
  uses_from_macos "python", :after => :el_capitan
  uses_from_macos "python", :before => :mojave

This will be treated as a no-op on macOS (although can be output by brew info --json) and provide metadata. On non-macOS platforms (e.g. Linux) this can be used to install these formulae as dependencies.

This should allow some consolidation of dependency logic between Homebrew/homebrew-core and Homebrew/linuxbrew-core.

CC @Homebrew/linux who discussed this privately in Slack before this issue was created.

[sjackman]

If a new contributor to Homebrew is interested in working on this issue, I'd be happy to point you to the place in the code where the changes are needed.

[gkpacker]

Hey @sjackman, i'd like to work on this

[sjackman]

Excellent!
Add the uses_from_macos method to the Formula class.
https://github.com/Homebrew/brew/blob/b3c0997c0a2308513615441bccb47c19f8325dfd/Library/Homebrew/formula.rb#L2359
The default implementation will simply forward to depends_on. Create two new files extend/os/dependency_collector.rb and extend/os/mac/dependency_collector.rb. On macOS, the uses_from_macos method will compare the version of macOS to the one specified by :before and :after and conditionally add the dependency. You can look at other files in these directories to understand how these files should be structured. Feel free to ask questions or open an in-progress PR, so that we can comment and help you along.

@MikeMcQuaid Do you agree with this implementation suggestion? How do you foresee adding the uses_from_macos dependency to brew info --json?

[sjackman]

cc @iMichka

[MikeMcQuaid]

Do you agree with this implementation suggestion?

I'd need to see code but broadly: yes.

How do you foresee adding the uses_from_macos dependency to brew info --json?

That's my main concern with the proposed approach; we'd need to ensure that by default these dependencies are not going to be output (on macOS or with HOMEBREW_FORCE_HOMEBREW_ON_LINUX set) but instead output them under a new entry e.g. "uses_from_macos" => ...

[iMichka]

One question: should we enforce the list of possible dependencies? (python, readline, ncurses, ...) or allow all dependencies (which may open the door to use whatever a user installed on the system, which is probably not what we want).

[MikeMcQuaid]

@iMichka Yes, good question. It may be worth having a hardcoded list of these dependencies in Homebrew/brew. To be clear I don't think this would allow random system stuff to be used regardless as superenv will still filter them out on the macOS case but being fairly strict on the usage feels sensible.

[sjackman]

brew audit should probably check against a hardcoded list.

[sjackman]

Do you agree with this implementation suggestion?

I'd need to see code but broadly: yes.

@gkpacker Feel free to open a PR that implements it as I described. Once the key logic is in place, we can discuss possible additional changes to brew info --json and brew audit.

[gkpacker]

@sjackman thanks! I never contributed to open source and i have a few questions:

This will be treated as a no-op on macOS

It should be no-op on Linux since we're adding a macOS "version checker", right?

Create two new files extend/os/dependency_collector.rb and extend/os/mac/dependency_collector.rb.

These files already exists, should i change something in them?

I use homebrew, but i don't know how it works internally, probably i'll have some difficulty to understand it

[sjackman]

thanks! I never contributed to open source and i have a few questions:

Welcome!

This will be treated as a no-op on macOS

It should be no-op on Linux since we're adding a macOS "version checker", right?

The default implementation will simply forward to depends_on, and Linux will use this default implementation. That is uses_from_macos "python", :after => :el_capitan will be equivalent to depends_on "python" on Linux.

[sjackman]

Create two new files extend/os/dependency_collector.rb and extend/os/mac/dependency_collector.rb.

These files already exists, should i change something in them?

That was a typo. Sorry. Create two new files extend/os/formula.rb and extend/os/mac/formula.rb.

I use homebrew, but i don't know how it works internally, probably i'll have some difficulty to understand it

See the documentation: https://docs.brew.sh/#contributors
and the Formula API: https://rubydoc.brew.sh/Formula.html

[gkpacker]

Thanks, @sjackman! It makes a lot more sense now. I think i can do it! When i get home i'll try to implement that

[sjackman]

Excellent! Are you also learning Ruby, or do you already know Ruby?

[gkpacker]

I already know Ruby, obviously not 100%, but it was my first language, so i have some familiarity with it

[sjackman]

Glad to hear it!

[gkpacker]

How can i open a console in the gem context like rails c?

[sjackman]

I think you want brew irb

[gkpacker]

What should we do if passed nil/invalid values to after/before?
e.g.:

uses_from_macos("foo", after: nil)

Should we support a "version range"? Like after: :el_capitan, before: high_sierra

[sjackman]

What should we do if passed nil/invalid values to after/before?

It should raise an exception if the value is not one of the known macOS versions.

Should we support a "version range"? Like after: :el_capitan, before: high_sierra

Yeah, that'd be good.

[MikeMcQuaid]

Should we support a "version range"? Like after: :el_capitan, before: high_sierra

Yeah, that'd be good.

I disagree, actually, I don't think it's necessary.

[sjackman]

Fair enough. In practice I don't think it'd make the implementation any more complicated to support both. It may even make the implementation easier. It's certainly not a priority item though.

[gkpacker]

I'll make a pr soon, but in advance, is that any other way to compare only if args[:before]/args[:after] passed?

OS::Mac::Version.from_symbol(nil) will raise Argument error: unknown version nil

I have this ATM:

class Formula
  class << self
    def uses_from_macos(dep, **args)
      present_args = args.select { |_, version| version.present? }

      depends_on(dep) if add_mac_dependency?(present_args)
    end

    private

    def add_mac_dependency?(args)
      args.blank? ||
        OS::Mac.version >= (OS::Mac::Version.from_symbol(args[:after]) if args[:after]) ||
        OS::Mac.version < (OS::Mac::Version.from_symbol(args[:before]) if args[:before])
    end
  end
end

[gkpacker]

OS::Mac.version >= nil returns true too, so if i have uses_from_macos 'foo', before: :el_capitan and sierra as my mac version, it will compare

 args.blank? || # false
        OS::Mac.version >= nil || # true 
        OS::Mac.version < OS::Mac::Version.from_symbol(:el_capitan) # false

[sjackman]

I'd suggest (untested)…

def add_mac_dependency?(args)
  return false if args[:after] && OS::Mac.version < OS::Mac::Version.from_symbol(args[:after])
  return false if args[:before] && OS::Mac.version >= OS::Mac::Version.from_symbol(args[:before])
  return true
end

[gkpacker]

@sjackman I had a problem running brew tests on rubocop_spec.rb

[gkpacker]

➜  brew git:(master) bin/brew tests --only=rubocop
Randomized with seed 52144
1 processes for 1 specs, ~ 1 specs per process
F

Failures:

  1) RuboCop when calling `rubocop` outside of the Homebrew environment loads all Formula cops without errors
     Failure/Error: expect(status).to be_a_success
       expected `#<Process::Status: pid 95753 exit 1>.success?` to return true, got false
     # ./test/rubocop_spec.rb:17:in `block (3 levels) in <top (required)>'
     # ./test/spec_helper.rb:163:in `block (2 levels) in <top (required)>'
     # ./vendor/bundle/ruby/2.3.0/gems/rspec-retry-0.6.1/lib/rspec/retry.rb:123:in `block in run'
     # ./vendor/bundle/ruby/2.3.0/gems/rspec-retry-0.6.1/lib/rspec/retry.rb:110:in `loop'
     # ./vendor/bundle/ruby/2.3.0/gems/rspec-retry-0.6.1/lib/rspec/retry.rb:110:in `run'
     # ./vendor/bundle/ruby/2.3.0/gems/rspec-retry-0.6.1/lib/rspec_ext/rspec_ext.rb:12:in `run_with_retry'
     # ./vendor/bundle/ruby/2.3.0/gems/rspec-retry-0.6.1/lib/rspec/retry.rb:37:in `block (2 levels) in setup'
     # ./vendor/bundle/ruby/2.3.0/gems/rspec-wait-0.0.9/lib/rspec/wait.rb:46:in `block (2 levels) in <top (required)>'

Finished in 0.26576 seconds (files took 1.94 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./test/rubocop_spec.rb:15 # RuboCop when calling `rubocop` outside of the Homebrew environment loads all Formula cops without errors


1 example, 1 failure

Took 2 seconds
Tests Failed

[sjackman]

I'm not the best person to troubleshoot weird gem troubles. What is the output of brew audit hello? Try brew vendor-install ruby; brew audit hello.

[gkpacker]

brew vendor-install ruby; brew audit hello fixed it, thanks

[gkpacker]

@sjackman Should i ask someone's review or just leave it here? https://github.com/Homebrew/brew/pull/6162

[sjackman]

I've commented and asked @MikeMcQuaid to review PR #6162.

[xu-cheng]

Just my two cents, you may want to support :build, :test arguments for this new DSL. For example,

uses_from_macos "m4", tag: :build
uses_from_macos "gpatch", tag: [:build, :test]

[sjackman]

Oh, yes. Thanks for the suggestion, @xu-cheng.

[connorshea]

Not sure how relevant this is, but macOS 10.15 deprecates the included scripting languages:

Scripting language runtimes such as Python, Ruby, and Perl are included in macOS for compatibility with legacy software. Future versions of macOS won’t include scripting language runtimes by default, and might require you to install additional packages. If your software depends on scripting languages, it’s recommended that you bundle the runtime within the app

[sjackman]

I hadn't heard that. Personally I think that's good news. Thanks for the information, Connor.